Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed endianity problem with Geneve tunnel VNI #582

Merged
merged 1 commit into from
Dec 6, 2020

Conversation

mlitvin
Copy link

@mlitvin mlitvin commented Oct 19, 2020

It seems that VNI (ID in the code) is passed in host order.

The change include tests to verify that we use the correct endianity: It create a tunnel using ip link and read the ID of that tunnel proving that reading has the correct endianity. The test that set and read proves that the write endianity is the same as the read.

@mlitvin
Copy link
Author

mlitvin commented Oct 19, 2020

Test failure is because the tests are running on ubuntu 16.04 where iproute2 doesn't support dstport for Geneve.

So the options are:

  1. Move to more modern ubuntu version in the travis configuration.
  2. Remove the part about dstport for that test
  3. attempt to identify the iproute version in the unit test code.
  4. give up on comparing the code to iproute2 (it is not "pure" unit testing)

@vishvananda
Copy link
Owner

can you try modifying the travis config in your branch to use a newer version of ubuntu? That way we can see if any other tests start failing.

@mlitvin
Copy link
Author

mlitvin commented Oct 19, 2020

Doesn't look good, we have problems with modprobe nf_conntrack_ipv4 and qdisc

@mlitvin
Copy link
Author

mlitvin commented Oct 25, 2020

Pull request was changes so we don't attempt to explicitly set the distort. It rely (and verify) the implicit Geneve port so we don't actually loose a lot in testing.

link_linux.go Outdated Show resolved Hide resolved
@aboch
Copy link
Collaborator

aboch commented Nov 22, 2020

please rebase

@mlitvin mlitvin changed the title Add Geneve link support Fixed endianity problem with Geneve tunnel VNI Dec 6, 2020
@mlitvin
Copy link
Author

mlitvin commented Dec 6, 2020

Most of the support was done in a separate commit, but the endianity problem need to be fixed

@aboch
Copy link
Collaborator

aboch commented Dec 6, 2020

Thanks.
Please squash your commits into one

2. Parse remote IP
3. Added unit test to test geneve paramters against "ip link"
@aboch
Copy link
Collaborator

aboch commented Dec 6, 2020

LGTM

@aboch aboch merged commit 88079d9 into vishvananda:master Dec 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants