Closed mivsvit closed 1 month ago
Sorry for the late response.
In the ACK the siaddr
(Next server IP address
) is zero, but giaddr
(Relay agent IP address
) is set, which is not expected on packets received by clients. I could implement to renew to broadcast if server address is zero, but need to check if this is allowed by RFC.
Dynamic Host Configuration Protocol (ACK)
Message type: Boot Reply (2)
...
Your (client) IP address: 100.66.48.150
Next server IP address: 0.0.0.0
Relay agent IP address: 10.9.23.3
Which DHCP server is used here?
Apologies for not spotting your reply earlier.
It's ISC's Kea DHCP server.
The BNG has a local DHCP proxy service sitting between kea and the client and will include the siaddr
/ Next-Server
if it is present in the response from the external server.
If I configure Kea to add a siaddr
then the bngblaster behaviour is as you describe.
Maybe there is some confusion between siaddr and DHCP Option 54 Server Identifier?
My understanding is that option 54 Server-Identifier rather than siaddr should be used to determine the destination address for the renew as [rfc2131 section-4.1] says "DHCP clients MUST use the IP address provided in the 'server identifier' option for any unicast requests to the DHCP server."
And for siaddr, the RFC says "A DHCP server may return its own address in the 'siaddr' field, if the server is prepared to supply the next bootstrap service (e.g., delivery of an operating system executable image)."
There is no next stage of the bootstrap- the server is not providing an operating system for a broadband CPE or emulated bngblaster client so I would not have thought it is appropriate to set next-server.
I've compared the bngblaster behaviour with ISC's dhclient. With no siaddr present it sends the renews to the unicast address it received the Offer/Ack from which is also the address in the option 54 Server-ID. This is the behaviour I expected.
Edit: I see the behaviour I would expect with bngblaster if I make this code change.
diff --git a/code/bngblaster/src/bbl_tx.c b/code/bngblaster/src/bbl_tx.c
index 0bb814c..af75533 100644
--- a/code/bngblaster/src/bbl_tx.c
+++ b/code/bngblaster/src/bbl_tx.c
@@ -1197,7 +1197,7 @@ bbl_tx_encode_packet_dhcp(bbl_session_s *session)
session->stats.dhcp_tx_request++;
LOG(DHCP, "DHCP (ID: %u) DHCP-Request send\n", session->session_id);
eth.dst = session->dhcp_server_mac;
- ipv4.dst = session->dhcp_server;
+ ipv4.dst = session->dhcp_server_identifier;
header.ciaddr = session->ip_address;
break;
case BBL_DHCP_RELEASE:
@@ -1205,7 +1205,7 @@ bbl_tx_encode_packet_dhcp(bbl_session_s *session)
session->stats.dhcp_tx_release++;
LOG(DHCP, "DHCP (ID: %u) DHCP-Release send\n", session->session_id);
eth.dst = session->dhcp_server_mac;
- ipv4.dst = session->dhcp_server;
+ ipv4.dst = session->dhcp_server_identifier;
header.ciaddr = session->ip_address;
dhcp.option_server_identifier = true;
dhcp.server_identifier = session->dhcp_server_identifier;
@@ -1692,4 +1692,4 @@ bbl_tx(bbl_interface_s *interface, uint8_t *buf, uint16_t *len)
network_interface = network_interface->next;
}
return result;
Describe the bug
DHCPv4 logs from bngblaster show session established successfully but renews failing. After the dhcp retry count is exceeded, a full DORA, where the discover and request are sent to the all-1s broadcast address succeeds.
A packet capture from bngblaster shows a null destination address for the DHCP renew and releases. Expected behaviour is that the siaddr from DHCP option 54 is used as a unicast destination address of the renew and releases.
Packet capture of the first DHCP ACK (packet 14), including the siaddr:
To Reproduce
Version (
bngblaster -v
):JSON configuration:
Expected behavior
The siaddr (in the above example, 172.31.19.2) is the expected destination address for the DHCP request and release packets rather than 0.0.0.0.
rfc2131 section-4.4.5 says "At time T1 the client moves to RENEWING state and sends (via unicast) a DHCPREQUEST message to the server to extend its lease." and rfc2131 section-4.1 says "DHCP clients MUST use the IP address provided in the 'server identifier' option for any unicast requests to the DHCP server."
This appears to be the intention from the code: https://github.com/rtbrick/bngblaster/blob/a913ac73c1d42f9d93149969aacfd5d1415202b1/code/bngblaster/src/bbl_dhcp.c#L281 https://github.com/rtbrick/bngblaster/blob/a913ac73c1d42f9d93149969aacfd5d1415202b1/code/bngblaster/src/bbl_tx.c#L1185