net_util: add support for IPv6 addresses on tap interfaces #7048
Conversation
net_gen/src/ipv6.rs
Outdated
| @@ -0,0 +1,41 @@ | |||
| // Copyright © 2021 Intel Corporation | |||
There was a problem hiding this comment.
I'm not sure what the rules are here. I'd think it'd be either copyright the project or the company you work for.
Most of the original files were written before cloud-hypervisor became part of Linux Foundation. Not sure if Linux Foundation has rules about copyright ownership/CLAs/etc.
@rbradford @liuw @likebreath @cloud-hypervisor/cloud-hypervisor-reviewers thoughts?
There was a problem hiding this comment.
I think we are still doing the company copyright for new files that we are adding. Is there a policy change when moved to Linux Foundation?
For this file it is even more tricky since the code is generated via bindgen.
There was a problem hiding this comment.
You can just put it "Copyright 2025 Cloud Hypervisor Authors" - the git history is the canonical source of copyright information (which is why it's important to correctly cite in there where stuff is copied from if its imported.)
There was a problem hiding this comment.
You can just put it "Copyright 2025 Cloud Hypervisor Authors" - the git history is the canonical source of copyright information (which is why it's important to correctly cite in there where stuff is copied from if its imported.)
This concerns me a bit — how do I tell from git history whether copyright is owned by an individual or their employer, for example?
gpanders
force-pushed
the
push-tvkyxkwrrxkk
branch
from
65009cc to
576203d
Compare
|
Code looks good - i've always sort of assumed that folks were happy with manually setting up the IP on the TAP for IPV64 and that the complexity added would be a bit too much to add to CH. But i'm very pleasantly surprised - lets see how the CI handles it. |
gpanders
force-pushed
the
push-tvkyxkwrrxkk
branch
from
576203d to
d055034
Compare
|
Updated the copyright header and addressed the clippy warnings in |
gpanders
force-pushed
the
push-tvkyxkwrrxkk
branch
from
d055034 to
6e2eb0f
Compare
gpanders
changed the title
|
I updated the commit message to satisfy the commit message linter. Can a maintainer re-trigger CI? |
|
It looks like all the integration tests are failing - I guess network is failing to come up for the VMs. Perhaps you could try running just one test directly. |
gpanders
force-pushed
the
push-tvkyxkwrrxkk
branch
from
6e2eb0f to
38ac7ea
Compare
|
Figured out what the issue was. I was setting both However, We were also unconditionally calling the SIOCSIFNETMASK ioctl even if no netmask was specified. Again because The fix is to:
(Below is my incorrect original explanation, keeping for posterity). Details
|
Allow tap interfaces to be configured with an IPv6 address. The change is fairly straightforward: we need to update the API types and CLI parsing to accept either an IPv6 or IPv4 and then match on the IP address type when the tap device is configured. For IPv6 addresses, the netmask (prefix) must be provided at the same time as the address itself (in the SIOCSIFADDR ioctl). They cannot be configured separately. So we remove the separate "set_netmask" function and convert "set_ip_addr" to also accept a netmask. For IPv4 addresses, the IP address and netmask were already always set together, so this should have no functional impact for users of IPv4 addresses. Signed-off-by: Gregory Anders <[email protected]>
gpanders
force-pushed
the
push-tvkyxkwrrxkk
branch
from
38ac7ea to
260fa38
Compare
|
Thank you! |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
dce82a3
5 participants
Allow tap interfaces to be configured with an IPv6 address. The change is fairly straightforward: we need to update the API types and CLI parsing to accept either an IPv6 or IPv4 and then match on the IP address type when the tap device is configured.
For IPv6 addresses, the netmask (prefix) must be provided at the same time as the address itself (in the SIOCSIFADDR ioctl). They cannot be configured separately. So we remove the separate "set_netmask" function and convert "set_ip_addr" to also accept a netmask and then set both the IP address and netmask together. For IPv4 addresses, the IP address and netmask were already always set together, so this should have no functional impact for users of IPv4 addresses.