Added macvlan and ipvlan drivers#964
Added macvlan and ipvlan drivers#964aboch merged 2 commits intomoby:masterfrom nerdalert:ipvlan_macvlan
Conversation
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/docker/libnetwork/driverapi" |
There was a problem hiding this comment.
You need to add this import for the CI to pass:
_ "github.com/docker/libnetwork/testutils"
|
@nerdalert 👏 thanks for the contribution. We will review it and provide feedback. |
drivers/ipvlan/ipvlan_state.go
Outdated
| d.Lock() | ||
| networks := d.networks | ||
| d.Unlock() | ||
| n, ok := networks[nid] |
There was a problem hiding this comment.
We don't need networks, we only need to lock around this line only,
d.Lock()
n, ok := d.networks[nid]
d.Unlock()
drivers/ipvlan/ipvlan_network.go
Outdated
| for label, value := range labels { | ||
| switch label { | ||
| case hostIfaceOpt: | ||
| // parse driver option '-o --ipvlan_mode' |
There was a problem hiding this comment.
I think this comment and the one below should be swapped.
- Notes at: https://gist.github.com/nerdalert/c0363c15d20986633fda Signed-off-by: Brent Salisbury <[email protected]>
drivers/macvlan/macvlan_setup.go
Outdated
| return nil | ||
| } | ||
| } | ||
| return fmt.Errorf("required kernel module '%s' was not found in /proc/modules, kernel version >= 3.19 is recommended", macvlanType) |
There was a problem hiding this comment.
Is this true ?
I thought macvlan is supported in earlier kernels ?
|
@nerdalert Given the importance of getting feedback from users, shall we get these drivers under experimental for 1.11 and get enough feedback & also fix the L3 routing use-case. WDYT ? |
|
Agree with @mavenugo. Being two new drivers, it will take some time with incremental changes to get them where we want them to be. |
|
@nerdalert I understand the need for But, as discussed,in the above case, if the user has not specified the host interface, we should consider it as an |
|
@nerdalert few more comments as discussed. (commenting it here for better tracking)
|
|
@mavenugo great suggestions, experimental sounds perfect.
|
drivers/ipvlan/ipvlan_network.go
Outdated
| } | ||
| if !parentExists(config.Parent) { | ||
| // if the parent iface was not specified, create a dummy link | ||
| if config.Parent == stringid.TruncateID(config.ID) { |
There was a problem hiding this comment.
I think it is better to check for config.Internal here.
|
@nerdalert tried your patch e2e & am happy to see tagged use-case working across multi-host with appropriate configuration (in my OSX & virtual-box bridged adapter). These 2 drivers are really going to make the underlay integration extremely simple and I cant wait to get user feedback on these. Apart from a few minor comments above, this PR gets a big 👍 from me. |
|
@nerdalert I have a couple of macvlan networks with and without parent interfaces. When I tried to create and use an ipvlan network, I get the following error. Which seems wrong. Also the same issue when I tried to create an entirely new parent interface Can you pls take a look @ it ? |
|
I've been running some tests with an Arista ToR acting as the gateway for macvlan/ipvlan, with and without 802.1q. So far so good! macvlan bridge mode: ipvlan L2 mode with 802.1q: Let me know if there are any specific tests you would like to have run against physical hardware. |
|
Thanks @fredhsu appreciate you testing the physical side of the equation. |
|
Regarding the |
|
Thanks @nerdalert. LGTM |
|
LGTM ping @chenchun |
Signed-off-by: Madhu Venugopal <[email protected]>
|
👍 Introducing them in experimental. LGTM, thanks for your work @nerdalert |
Added macvlan and ipvlan drivers
6 participants
Signed-off-by: Brent Salisbury [email protected]