added support for Foo-over-UDP netlink calls #285
Conversation
|
Looking at this error (https://travis-ci.org/vishvananda/netlink/builds/289425912#L541) and some other tests complaining about travis' kernel being too old, I think the same goes for |
|
@vishvananda how is checking for |
|
I’m not quite happy with this result so far. Am I corect that next to managing Route Attributes there currently is no support for using ‘regular’ netlink attributes (like I’m parsing for FOU in this request)? I would rather implement that and use it for FOU than implement this specifically for FOU. |
fou_linux.go
Outdated
| var fouFamilyId int | ||
|
|
||
| func FouFamilyId() (int, error) { | ||
| if fouFamilyId == 0 { |
There was a problem hiding this comment.
Please make this more readable as
func FouFamilyId() (int, error) {
if fouFamilyId != 0 {
return fouFamilyId, nil
}
fams, err := GenlFamilyList()
if err != nil {
return -1, err
}
for _, f := range fams {
if f.Name == FOU_GENL_NAME {
fouFamilyId = int(f.ID)
return fouFamilyId, nil
}
}
return -1, fmt.Errorf("could not find genl family %s", FOU_GENL_NAME)
}
| FOU_ENCAP_MAX = FOU_ENCAP_GUE | ||
| ) | ||
|
|
||
| var fouFamilyId int |
There was a problem hiding this comment.
Wondering if Handle would be a good place where to store the generic netlink families ids.
There was a problem hiding this comment.
Do you mean adding a map of netlink families to the Handle struct?
fou_linux.go
Outdated
|
|
||
| func FouFamilyId() (int, error) { | ||
| if fouFamilyId == 0 { | ||
| fams, err := GenlFamilyList() |
There was a problem hiding this comment.
BTW, any reason for not using GenlFamilyGet() ?
fou_linux.go
Outdated
| for _, m := range msgs { | ||
| if f, err := deserializeFouMsg(m); err != nil { | ||
| return fous, err | ||
| } else { |
fou_linux.go
Outdated
| return nil, err | ||
| } | ||
|
|
||
| fous := []Fou{} |
There was a problem hiding this comment.
to avoid reallocation of the slice at each append, you can define this as
fous := make([]Fou, 0, len(msgs))
fou_test.go
Outdated
| } | ||
|
|
||
| func TestFouAddDel(t *testing.T) { | ||
| tearDown := setUpNetlinkTest(t) |
There was a problem hiding this comment.
Please add the call to minKernelRequired(t, M, N) so that this test only runs on kernel which knows about FOU
|
Please squash the commits into one as they are all about the same feature. |
|
@aboch thanks for your suggestions, I've applied them. As you seem invested, what do you think regarding the implementation of parsing regular netlink attributes? It seems foolish to implement it just for decoding FOU attributes while it applies to more non-routing attributes as well. I'll squash the commits when everything is ok. |
|
I think we can get this PR in as is. Are you sure you can't use |
skoef
force-pushed
the
FooOverUDP
branch
3 times, most recently
from
9e9dada to
6603cd9
Compare
|
hmm, I think something went wrong rebasing. I merged master in between my development and I see all those commits as well instead of 1 merge commit. Should I squash all these commits into 1? |
|
|
|
Thanks @skoef . Please |
Signed-off-by: Reinier Schoof <reinier@skoef.nl>
|
I just did, happy to help! |
|
Not sure how this |
|
Thanks @skoef , LGTM |
The Foo-over-UDP patches from Tom Herbert added some netlink calls to the kernel. Here is the support for these calls, similar to
ip fou addandip fou delas well as a list function based onFOU_CMD_GET, to my knowledge currently not (yet) implemented iniproute2.