Add ERSPAN Type II header support#484
Add ERSPAN Type II header support#484tcheever wants to merge 1 commit intogoogle:masterfrom tcheever:erspan2
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
|
|
I signed it! |
|
Dear @tcheever would you double check if you signed the CLA with the right email address ? It should be the one linked to you GitHub account (see the CLA robot message) |
|
@lhausermann Hmm, I do have the correct email address and github username in the CLA. I did not have a publicly visible email address in my github settings, however, so I've exposed that now. |
|
CLAs look good, thanks! |
lhausermann
left a comment
lhausermann
left a comment
There was a problem hiding this comment.
After CLA is signed, I made some review of your code. Would you mind make some cleanups ?
Thanks for your contribution
layers/erspan2.go
Outdated
|
|
||
| // Header contains all of the fields found in an ERSPAN2 header | ||
| // https://tools.ietf.org/html/draft-foschiano-erspan-03 | ||
| type Header struct { |
There was a problem hiding this comment.
This should be called ERPSANIIHeader
There was a problem hiding this comment.
Ah yes, good point. I had the different layers separated into their own packages so callers would have erspan2.Header or erspan2.NewHeader(). There are a few things, as you've pointed out, than need to be cleaned up a bit to fit into the layers package.
layers/erspan2.go
Outdated
| return nil | ||
| } | ||
|
|
||
| // Conn - A connection to a device to send packets within ERSPAN2 encapsulation |
There was a problem hiding this comment.
I don't think this "Conn" struct + methods should be in a Layer. I guess you are using that in your app code to send packets, but a layer should stick to DecodeFromBytes and SerializeTo methods
There was a problem hiding this comment.
Yeah, had a feeling that would be the feedback but worth a shot! I'll remove the connection portions.
layers/erspan2.go
Outdated
| } | ||
|
|
||
| // Dial - Connect to a device given an IP address string. | ||
| func Dial(addr string, opts *ConnOpts) (*Conn, error) { |
layers/erspan2.go
Outdated
| return e.opts | ||
| } | ||
|
|
||
| const ( |
There was a problem hiding this comment.
If needed, please use the const already defined in Gopacket other layers (ie IP / Ethernet)
layers/erspan2_test.go
Outdated
|
|
||
| // Need root permissions or net_raw_sock linux capabilities for Dial() | ||
| // This test is skipped but here for documentation | ||
| func TestDialWriteClose(t *testing.T) { |
|
Fixing the golint complaints |
|
Ok, I fixed golint on my system. For some reason the binary wasn't executable anymore even though it had adequate permissions and was of the right format. Updating golint fixed the issue. |
|
Laurent, quick ping on this: are you okay with the changes made? Also, it looks like this isn't currently linked into LayerType stuff. Could you add a Layer type to layers/layertypes.go? |
4 participants
Also includes an ERSPAN connection type with basic functions.