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

dhcp ipam: support customizing dhcp options from CNI args #670

Merged
merged 10 commits into from
Dec 15, 2021

Conversation

SilverBut
Copy link
Contributor

@SilverBut SilverBut commented Oct 2, 2021

Make some DHCP options sent to the server can be set from parsing CNI_ARGS.

This closes #435 and #660.

Signed-off-by: SilverBut <SilverBut@users.noreply.github.com>
Signed-off-by: SilverBut <SilverBut@users.noreply.github.com>
Signed-off-by: SilverBut <SilverBut@users.noreply.github.com>
Signed-off-by: SilverBut <SilverBut@users.noreply.github.com>
Signed-off-by: SilverBut <SilverBut@users.noreply.github.com>
Almost every first retry of DHCP will fail due to interface is not up. Add a
fast retry to reduce unnecessary latency.

Signed-off-by: SilverBut <SilverBut@users.noreply.github.com>
Signed-off-by: SilverBut <SilverBut@users.noreply.github.com>
@SilverBut SilverBut changed the title dhcp ipam: support customizing dhcp options from CHi args dhcp ipam: support customizing dhcp options from CNI args Oct 2, 2021
@SilverBut SilverBut marked this pull request as draft October 2, 2021 15:33
@SilverBut SilverBut marked this pull request as ready for review October 2, 2021 15:38
Signed-off-by: SilverBut <SilverBut@users.noreply.github.com>
First byte of client ID is type, instead of value. See this from
RFC2132:

   Code   Len   Type  Client-Identifier
   +-----+-----+-----+-----+-----+---
   |  61 |  n  |  t1 |  i1 |  i2 | ...
   +-----+-----+-----+-----+-----+---

Signed-off-by: SilverBut <SilverBut@users.noreply.github.com>
Copy link
Member

@jellonek jellonek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<3

@mccv1r0
Copy link
Member

mccv1r0 commented Oct 6, 2021

/lgtm

@vivekskrishna
Copy link

looks good

clientID := containerID + "/" + netName + "/" + ifName
// defined in RFC 2132, length size can not be larger than 1 octet. So we truncate 254 to make everyone happy.
if len(clientID) > 254 {
clientID = clientID[0:254]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, containerID and ifName are enough to be composed as clientID.

Copy link
Contributor Author

@SilverBut SilverBut Nov 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually not changed to ensure backward compatibility, since I'm not sure if anyone is using this default config in somewhere of their config...

Signed-off-by: SilverBut <SilverBut@users.noreply.github.com>
@dcbw
Copy link
Member

dcbw commented Dec 15, 2021

/lgtm
Thanks!

@dcbw dcbw merged commit cc32993 into containernetworking:master Dec 15, 2021
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.

macvlan + dhcp doesn't forward hostname to dhcp server
7 participants