Conversation
|
Getting some golint errors...will fix them shortly ;) #Closed |
|
Fixed In reply to: 510266494 [](ancestors = 510266494) |
|
@notti - This is ready for review :) |
|
|
||
| // MPLSEchoFlagRespondOnlyIfTTLExpired tweaks the "TTL expired" scenario behavior. | ||
| // Can only be set in echo request packets. If set to 1 in an incoming echo request and the TTL of the incoming | ||
| // MPLS label is greater than 1, the receiving node MUST drop the oacket and MUST NOT send back an echo reply. |
| ) | ||
|
|
||
| // SetMPLSEchoGlobalFlag returns an MPLSEchoGlobalFlag bitmask with the given flag enabled. | ||
| func SetMPLSEchoGlobalFlag(b, flag MPLSEchoGlobalFlags) MPLSEchoGlobalFlags { return b | flag } |
There was a problem hiding this comment.
Is this function necessary? It is not used anywhere and looks trivial.
| func SetMPLSEchoGlobalFlag(b, flag MPLSEchoGlobalFlags) MPLSEchoGlobalFlags { return b | flag } | ||
|
|
||
| // ClearMPLSEchoGlobalFlag returns an MPLSEchoGlobalFlag bitmask with the given flag disabled. | ||
| func ClearMPLSEchoGlobalFlag(b, flag MPLSEchoGlobalFlags) MPLSEchoGlobalFlags { return b &^ flag } |
There was a problem hiding this comment.
Is this function necessary? It is not used anywhere and looks trivial.
| func ClearMPLSEchoGlobalFlag(b, flag MPLSEchoGlobalFlags) MPLSEchoGlobalFlags { return b &^ flag } | ||
|
|
||
| // HasMPLSEchoGlobalFlag returns TRUE if the given flag is enabled in the bitmask. | ||
| func HasMPLSEchoGlobalFlag(b, flag MPLSEchoGlobalFlags) bool { return b&flag != 0 } |
There was a problem hiding this comment.
Is this function necessary? It is not used anywhere and looks trivial.
| // | ||
| // Types greater than or equal to 32768 (i.e., with the high-order bit equal to 1) are optional TLVs that SHOULD be | ||
| // ignored if the implementation does not understand or support them. | ||
| TLVs []*MPLSEchoTLV |
There was a problem hiding this comment.
Could you make this TLVs []MPLSEchoTLV, which would allow preallocating like options in IP4 or TCP?
|
|
||
| currentTLVStartOffset := 32 | ||
| for currentTLVStartOffset < len(data) { | ||
| currentTLV := &MPLSEchoTLV{} |
There was a problem hiding this comment.
See comment in the data structure (use preallocation like options in IP4 or TCP) and I think not using a pointer would be better here.
Memory allocations should be avoided.
|
|
||
| currentTLVStartOffset := 32 | ||
| for _, currentTLV := range m.TLVs { | ||
| tlvByteRepresentation, err := currentTLV.EncodeAsBytes() |
There was a problem hiding this comment.
This allocates a byte slice for every TLV. Memory allocations should be avoided. Can you change EncodeAsBytes so it writes to the target buffer directly?
| @@ -0,0 +1,933 @@ | |||
| // Copyright 2018 GoPacket Authors. All rights reserved. | |||
There was a problem hiding this comment.
Can you make the testfile more in line with the existing tests? (without the helper functions)
2 participants
SUMMARY
Implementing MPLS Echo protocol, as specified by RFC8029. This enables Label Switched Path (LSP) Ping/Traceroute operations to be conducted inside Multiprotocol Label Switching (MPLS) networks for data plane failure detection and localization.
An MPLS echo request/reply is a (possibly labeled) IPv4 or IPv6 UDP packet. Wireshark already displays MPLS Echo as a separate layer in packet captures.
VALIDATION
./gc.