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

macvlan: set mac address from args #480

Merged
merged 1 commit into from
May 27, 2020

Conversation

clinta
Copy link
Contributor

@clinta clinta commented Apr 30, 2020

This change sets the mac address if specified during the creation of the
macvlan interface. This is superior to setting it via the tuning plugin
because this ensures the mac address is set before an IP is set,
allowing a container to get a reserved IP address from DHCP.

Related #450

@dcbw
Copy link
Member

dcbw commented May 6, 2020

@clinta looks mostly good, but can you update the testcases too with a case to set the MAC on creation?

@clinta
Copy link
Contributor Author

clinta commented May 6, 2020

A test has been added to test setting the mac address.

@squeed
Copy link
Member

squeed commented May 13, 2020

Sorry for the run-around, but I'm going to ask for one more small change :-). Then we can get this merged.

Can you add support for the mac capability arg? See https://github.com/containernetworking/cni/blob/master/CONVENTIONS.md#well-known-capabilities

Thanks!

@clinta
Copy link
Contributor Author

clinta commented May 18, 2020

I can do that. I'll look at the tuning plugin some more to see how it's done. I'm a bit new to CNI and added this for my own needs using podman, and I don't quite understand how the mac address in the network config is useful. Doesn't that result in every container on a network getting the same mac address?

@squeed
Copy link
Member

squeed commented May 20, 2020

@clinta thanks! Let me know if you have any questions.

CNI has 3 kinds of dynamic configuration: args, CNI_ARGS, and capability arguments. The latter is newest, and allows for a sort of negotiation between the runtime and the plugins around optional parameters. It's more structured CNI_ARGS, but provides essentially the same functionality.

You don't write the mac parameter to the CNI configuration file. Rather, you tell the runtime (and libcni) where to substitute in the mac address at runtime.

Make sense?

This change sets the mac address if specified during the creation of the
macvlan interface. This is superior to setting it via the tuning plugin
because this ensures the mac address is set before an IP is set,
allowing a container to get a reserved IP address from DHCP.

Related containernetworking#450

Signed-off-by: Clint Armstrong <[email protected]>
@clinta
Copy link
Contributor Author

clinta commented May 20, 2020

Thanks, that cleared it up. I wasn't seeing at first how RuntimeConfig was set by the container runtime. I've added this functionality, and another test for it.

@squeed
Copy link
Member

squeed commented May 27, 2020

Looks great, thanks!

@squeed
Copy link
Member

squeed commented May 27, 2020

Now, do you want to add this parameter to the bridge and p2p plugins :-) ?

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