Skip to content

fix(dhcp): can not renew an ip address#1092

Merged
squeed merged 2 commits intocontainernetworking:mainfrom
lisongmin:fix-dhcp-can-not-renew
Oct 14, 2024
Merged

fix(dhcp): can not renew an ip address#1092
squeed merged 2 commits intocontainernetworking:mainfrom
lisongmin:fix-dhcp-can-not-renew

Conversation

@lisongmin
Copy link
Copy Markdown
Contributor

@lisongmin lisongmin commented Sep 14, 2024

The dhcp server is systemd-networkd, and the dhcp
plugin can request an ip but can not renew it.
The systemd-networkd just ignore the renew request.

2024/09/14 21:46:00 no DHCP packet received within 10s
2024/09/14 21:46:00 retrying in 31.529038 seconds
2024/09/14 21:46:42 no DHCP packet received within 10s
2024/09/14 21:46:42 retrying in 63.150490 seconds
2024/09/14 21:47:45 98184616c91f15419f5cacd012697f85afaa2daeb5d3233e28b0ec21589fb45a/iot/eth1: no more tries
2024/09/14 21:47:45 98184616c91f15419f5cacd012697f85afaa2daeb5d3233e28b0ec21589fb45a/iot/eth1: renewal time expired, rebinding
2024/09/14 21:47:45 Link "eth1" down. Attempting to set up
2024/09/14 21:47:45 98184616c91f15419f5cacd012697f85afaa2daeb5d3233e28b0ec21589fb45a/iot/eth1: lease rebound, expiration is 2024-09-14 22:47:45.309270751 +0800 CST m=+11730.048516519

Follow the https://datatracker.ietf.org/doc/html/rfc2131#section-4.3.6, following options must not be sent in renew

  • Requested IP Address
  • Server Identifier

And renew should using unicast but not broadcast.

@lisongmin
Copy link
Copy Markdown
Contributor Author

lisongmin commented Sep 14, 2024

I don't know how to set to using unicast address, but it just work

14:33:08.610940 IP (tos 0x0, ttl 16, id 28928, offset 0, flags [none], proto UDP (17), length 366)
    0.0.0.0.68 > 255.255.255.255.67: [no cksum] BOOTP/DHCP, Request from ce:18:d4:e1:99:4c, length 338, xid 0x4c6a0e05, Flags [none] (0x0000)
          Client-IP 192.168.4.143
          Client-Ethernet-Address ce:18:d4:e1:99:4c
          Vendor-rfc1048 Extensions
            Magic Cookie 0x63825363
            DHCP-Message (53), length 1: Request
            Client-ID (61), length 74: "98184616c91f15419f5cacd012697f85afaa2daeb5d3233e28b0ec21589fb45a/iot/eth1"
            Hostname (12), length 13: "homeassistant"
            Parameter-Request (55), length 1:
              Subnet-Mask (1)
            END (255), length 0
14:33:08.611137 IP (tos 0xc0, ttl 64, id 60414, offset 0, flags [DF], proto UDP (17), length 311)
    192.168.4.1.67 > 192.168.4.143.68: [bad udp cksum 0x8b15 -> 0x844f!] BOOTP/DHCP, Reply, length 283, xid 0x4c6a0e05, Flags [none] (0x0000)
          Your-IP 192.168.4.143
          Client-Ethernet-Address ce:18:d4:e1:99:4c
          Vendor-rfc1048 Extensions
            Magic Cookie 0x63825363
            DHCP-Message (53), length 1: ACK
            Lease-Time (51), length 4: 60
            Subnet-Mask (1), length 4: 255.255.255.0
            Default-Gateway (3), length 4: 192.168.4.1
            Domain-Name-Server (6), length 4: 192.168.4.1
            TZ-Name (101), length 7: "Etc/UTC"
            Server-ID (54), length 4: 192.168.4.1
            END (255), length 0

@lisongmin lisongmin force-pushed the fix-dhcp-can-not-renew branch from 778a06d to 7961199 Compare September 14, 2024 14:43
@squeed
Copy link
Copy Markdown
Member

squeed commented Sep 16, 2024

We can't just edit vendor files; they need to exactly match upstream. If our library has a bug, we should ideally fix that or find a new library.

@lisongmin
Copy link
Copy Markdown
Contributor Author

lisongmin commented Sep 16, 2024

We can't just edit vendor files; they need to exactly match upstream. If our library has a bug, we should ideally fix that or find a new library.

Oh, I didn't realize that was vendor code.

Since the upstream code has been inactive for 6 years,
I think we should switch to https://github.com/insomniacslk/dhcp.

What do you think? Any other good ideas?

@squeed
Copy link
Copy Markdown
Member

squeed commented Sep 17, 2024

@lisongmin sounds good! If it's a difficult replacement, let me know and we can consider forking. But in general, it's better to switch to a supported upstream :-).

@lisongmin lisongmin force-pushed the fix-dhcp-can-not-renew branch 2 times, most recently from 37c0092 to 8d7931d Compare September 19, 2024 01:50
@lisongmin lisongmin marked this pull request as draft September 19, 2024 01:50
@lisongmin lisongmin force-pushed the fix-dhcp-can-not-renew branch 4 times, most recently from 89be595 to 8a624c9 Compare September 20, 2024 03:49
@lisongmin
Copy link
Copy Markdown
Contributor Author

lisongmin commented Sep 20, 2024 •
edited
Loading

Main changed in this PR:

  1. replace d2g/dhcp4client with insomniacslk/dhcp
  2. Using dnsmasq as the dhcp server in test (replace d2g/dhcp4server)
  3. Reduce the dhcp test time from (169s -> 72s) by adding --resendtimeout daemon option, the resend timeout represent the total duration we should wait for acquire/renew to complete, and if it is expire, no more retries.
  4. We can now cancel the backoff retries on CmdDel, so we can stop quickly on retries.

@lisongmin
Copy link
Copy Markdown
Contributor Author

lisongmin commented Sep 20, 2024

It seems there is a test error, but not relative to this PR:

• [FAILED] [0.011 seconds]
sbr test [It] Works with multiple IPs
/home/runner/work/plugins/plugins/plugins/meta/sbr/sbr_linux_test.go:406
  [FAILED] Expected
      <bool>: false
  to be true
  In [It] at: /home/runner/work/plugins/plugins/plugins/meta/sbr/sbr_linux_test.go:522 @ 09/20/24 03:58:32.946

@lisongmin lisongmin marked this pull request as ready for review September 20, 2024 04:03
@lisongmin lisongmin force-pushed the fix-dhcp-can-not-renew branch 2 times, most recently from 40b11ca to 87db4f5 Compare September 20, 2024 07:06
@LionelJouin
Copy link
Copy Markdown
Member

LionelJouin commented Sep 20, 2024

It seems there is a test error, but not relative to this PR:

• [FAILED] [0.011 seconds]
sbr test [It] Works with multiple IPs
/home/runner/work/plugins/plugins/plugins/meta/sbr/sbr_linux_test.go:406
  [FAILED] Expected
      <bool>: false
  to be true
  In [It] at: /home/runner/work/plugins/plugins/plugins/meta/sbr/sbr_linux_test.go:522 @ 09/20/24 03:58:32.946

I looked into it, I will try to fix it soon. An issue has been created: #1096

@lisongmin lisongmin force-pushed the fix-dhcp-can-not-renew branch from 87db4f5 to ba43c52 Compare October 12, 2024 15:41
@lisongmin
Copy link
Copy Markdown
Contributor Author

lisongmin commented Oct 12, 2024

I have rebased to the latest code, feel free to have a look at it, thanks.

@squeed
Copy link
Copy Markdown
Member

squeed commented Oct 14, 2024

@lisongmin looks good! Just needs a minor rebase, then we can get this in.

@lisongmin
fix(dhcp): can not renew an ip address
6cc15fd 
The dhcp server is systemd-networkd, and the dhcp
plugin can request an ip but can not renew it.
The systemd-networkd just ignore the renew request.

```
2024/09/14 21:46:00 no DHCP packet received within 10s
2024/09/14 21:46:00 retrying in 31.529038 seconds
2024/09/14 21:46:42 no DHCP packet received within 10s
2024/09/14 21:46:42 retrying in 63.150490 seconds
2024/09/14 21:47:45 98184616c91f15419f5cacd012697f85afaa2daeb5d3233e28b0ec21589fb45a/iot/eth1: no more tries
2024/09/14 21:47:45 98184616c91f15419f5cacd012697f85afaa2daeb5d3233e28b0ec21589fb45a/iot/eth1: renewal time expired, rebinding
2024/09/14 21:47:45 Link "eth1" down. Attempting to set up
2024/09/14 21:47:45 98184616c91f15419f5cacd012697f85afaa2daeb5d3233e28b0ec21589fb45a/iot/eth1: lease rebound, expiration is 2024-09-14 22:47:45.309270751 +0800 CST m=+11730.048516519
```

Follow the https://datatracker.ietf.org/doc/html/rfc2131#section-4.3.6,
following options must not be sent in renew

- Requested IP Address
- Server Identifier

Since the upstream code has been inactive for 6 years,
we should switch to another dhcpv4 library.
The new selected one is https://github.com/insomniacslk/dhcp.

Signed-off-by: Songmin Li <[email protected]>
@lisongmin
feat(dhcp): Cancel backoff retry on stop
f6326c5 
Signed-off-by: Songmin Li <[email protected]>
@lisongmin lisongmin force-pushed the fix-dhcp-can-not-renew branch from ba43c52 to f6326c5 Compare October 14, 2024 11:20
@lisongmin
Copy link
Copy Markdown
Contributor Author

lisongmin commented Oct 14, 2024

Great to hear that. And the rebase is done.

@squeed squeed merged commit a4fc6f9 into containernetworking:main Oct 14, 2024
@lisongmin lisongmin deleted the fix-dhcp-can-not-renew branch October 15, 2024 01:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@squeed squeed squeed approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@lisongmin @squeed @LionelJouin