forked from avramd/debian-systemd
-
Notifications
You must be signed in to change notification settings - Fork 0
Backport BOOTP from systemd/main (v257-8) to debian/systemd v252 Bookworm .deb #1
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
Open
avramd
wants to merge
5
commits into
agd-debian/252.31-1_deb12u1-base
Choose a base branch
from
agd-backport-bootp
base: agd-debian/252.31-1_deb12u1-base
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
9b4b62f
Backport BOOTP from systemd/main v258
avramd a0892bb
add bootp integration test
avramd c78889b
Add patch for .deb build
avramd e18f25e
collapse unnecessarily spread out conditionals
avramd a87c6b0
Remove changelog entry; we don't expect to submit this & it complicat…
avramd File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,10 @@ | ||
systemd (252.31-1~deb12u1.1) UNRELEASED; urgency=medium | ||
|
||
* Non-maintainer upload. | ||
* Backport BOOTP support from systemd/main v258 | ||
|
||
-- Avram Dorfman <[email protected]> Tue, 14 Jan 2025 13:27:10 -0500 | ||
|
||
systemd (252.31-1~deb12u1) bookworm; urgency=medium | ||
|
||
* New upstream version 252.31 | ||
|
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -116,6 +116,7 @@ struct sd_dhcp_client { | |
sd_dhcp_lease *lease; | ||
usec_t start_delay; | ||
int ip_service_type; | ||
bool bootp; | ||
|
||
/* Ignore ifindex when generating iaid. See dhcp_identifier_set_iaid(). */ | ||
bool test_mode; | ||
|
@@ -651,6 +652,19 @@ int sd_dhcp_client_set_fallback_lease_lifetime(sd_dhcp_client *client, uint32_t | |
return 0; | ||
} | ||
|
||
int sd_dhcp_client_set_bootp(sd_dhcp_client *client, int bootp) { | ||
assert_return(client, -EINVAL); | ||
assert_return(!sd_dhcp_client_is_running(client), -EBUSY); | ||
|
||
client->bootp = bootp; | ||
|
||
/* For BOOTP mode, we don't want to send any request options by default. */ | ||
set_free(client->req_opts); | ||
client->req_opts = NULL; | ||
|
||
return 0; | ||
} | ||
|
||
static int client_notify(sd_dhcp_client *client, int event) { | ||
assert(client); | ||
|
||
|
@@ -759,9 +773,15 @@ static int client_message_init( | |
if (!packet) | ||
return -ENOMEM; | ||
|
||
r = dhcp_message_init(&packet->dhcp, BOOTREQUEST, client->xid, type, | ||
client->arp_type, client->hw_addr.length, client->hw_addr.bytes, | ||
optlen, &optoffset); | ||
if (client->bootp) { | ||
optoffset = 0; | ||
r = bootp_message_init(&packet->dhcp, BOOTREQUEST, client->xid, client->arp_type, | ||
client->hw_addr.length, client->hw_addr.bytes); | ||
} else | ||
r = dhcp_message_init(&packet->dhcp, BOOTREQUEST, client->xid, type, | ||
client->arp_type, client->hw_addr.length, client->hw_addr.bytes, | ||
optlen, &optoffset); | ||
|
||
if (r < 0) | ||
return r; | ||
|
||
|
@@ -791,35 +811,38 @@ static int client_message_init( | |
if (client->request_broadcast || client->arp_type != ARPHRD_ETHER) | ||
packet->dhcp.flags = htobe16(0x8000); | ||
|
||
/* If no client identifier exists, construct an RFC 4361-compliant one */ | ||
if (client->client_id_len == 0) { | ||
size_t duid_len; | ||
if (!client->bootp) { | ||
/* Some DHCP servers will refuse to issue an DHCP lease if the Client | ||
Identifier option is not set */ | ||
|
||
client->client_id.type = 255; | ||
/* If no client identifier exists, construct an RFC 4361-compliant one */ | ||
if (client->client_id_len == 0) { | ||
size_t duid_len; | ||
|
||
r = dhcp_identifier_set_iaid(client->ifindex, &client->hw_addr, | ||
/* legacy_unstable_byteorder = */ true, | ||
/* use_mac = */ client->test_mode, | ||
&client->client_id.ns.iaid); | ||
if (r < 0) | ||
return r; | ||
client->client_id.type = 255; | ||
|
||
r = dhcp_identifier_set_duid_en(client->test_mode, &client->client_id.ns.duid, &duid_len); | ||
if (r < 0) | ||
return r; | ||
r = dhcp_identifier_set_iaid(client->ifindex, &client->hw_addr, | ||
/* legacy_unstable_byteorder = */ true, | ||
/* use_mac = */ client->test_mode, | ||
&client->client_id.ns.iaid); | ||
if (r < 0) | ||
return r; | ||
|
||
client->client_id_len = sizeof(client->client_id.type) + sizeof(client->client_id.ns.iaid) + duid_len; | ||
} | ||
r = dhcp_identifier_set_duid_en(client->test_mode, &client->client_id.ns.duid, &duid_len); | ||
if (r < 0) | ||
return r; | ||
|
||
/* Some DHCP servers will refuse to issue an DHCP lease if the Client | ||
Identifier option is not set */ | ||
if (client->client_id_len) { | ||
r = dhcp_option_append(&packet->dhcp, optlen, &optoffset, 0, | ||
SD_DHCP_OPTION_CLIENT_IDENTIFIER, | ||
client->client_id_len, | ||
&client->client_id); | ||
if (r < 0) | ||
return r; | ||
client->client_id_len = sizeof(client->client_id.type) + sizeof(client->client_id.ns.iaid) + duid_len; | ||
} | ||
|
||
if (client->client_id_len) { | ||
r = dhcp_option_append(&packet->dhcp, optlen, &optoffset, 0, | ||
SD_DHCP_OPTION_CLIENT_IDENTIFIER, | ||
client->client_id_len, | ||
&client->client_id); | ||
if (r < 0) | ||
return r; | ||
} | ||
} | ||
|
||
/* RFC2131 section 3.5: | ||
|
@@ -835,7 +858,7 @@ static int client_message_init( | |
MAY contain the Parameter Request List option. */ | ||
/* NOTE: in case that there would be an option to do not send | ||
* any PRL at all, the size should be checked before sending */ | ||
if (!set_isempty(client->req_opts) && type != DHCP_RELEASE) { | ||
if (!set_isempty(client->req_opts) && type != DHCP_RELEASE && !client->bootp) { | ||
_cleanup_free_ uint8_t *opts = NULL; | ||
size_t n_opts, i = 0; | ||
void *val; | ||
|
@@ -883,7 +906,7 @@ static int client_message_init( | |
*/ | ||
/* RFC7844 section 3: | ||
SHOULD NOT contain any other option. */ | ||
if (!client->anonymize && IN_SET(type, DHCP_DISCOVER, DHCP_REQUEST)) { | ||
if (!client->bootp && !client->anonymize && IN_SET(type, DHCP_DISCOVER, DHCP_REQUEST)) { | ||
be16_t max_size = htobe16(MIN(client->mtu - DHCP_IP_UDP_SIZE, (uint32_t) UINT16_MAX)); | ||
r = dhcp_option_append(&packet->dhcp, optlen, &optoffset, 0, | ||
SD_DHCP_OPTION_MAXIMUM_MESSAGE_SIZE, | ||
|
@@ -1026,23 +1049,42 @@ static int client_send_discover(sd_dhcp_client *client) { | |
*/ | ||
/* RFC7844 section 3: | ||
SHOULD NOT contain any other option. */ | ||
if (!client->anonymize && client->last_addr != INADDR_ANY) { | ||
if (!client->bootp && !client->anonymize && client->last_addr != INADDR_ANY) { | ||
r = dhcp_option_append(&discover->dhcp, optlen, &optoffset, 0, | ||
SD_DHCP_OPTION_REQUESTED_IP_ADDRESS, | ||
4, &client->last_addr); | ||
if (r < 0) | ||
return r; | ||
} | ||
|
||
r = client_append_common_discover_request_options(client, discover, &optoffset, optlen); | ||
if (r < 0) | ||
return r; | ||
if (!client->bootp) { | ||
r = client_append_common_discover_request_options(client, discover, &optoffset, optlen); | ||
if (r < 0) | ||
return r; | ||
} | ||
|
||
r = dhcp_option_append(&discover->dhcp, optlen, &optoffset, 0, | ||
SD_DHCP_OPTION_END, 0, NULL); | ||
if (r < 0) | ||
return r; | ||
|
||
/* RFC1542 section 3.5: | ||
* if the client has no information to communicate to the server, | ||
* the octet immediately following the magic cookie SHOULD be set | ||
* to the "End" tag (255) and the remaining octets of the 'vend' | ||
* field SHOULD be set to zero. | ||
*/ | ||
/* Use this RFC, along with the fact that some BOOTP servers require | ||
* a 64-byte vend field, to suggest that we always zero and send 64 | ||
* bytes in the options field. The first four bites are the "magic" | ||
* field, so this only needs to add 60 bytes. | ||
*/ | ||
if (client->bootp) | ||
if (optoffset < 60 && optlen >= 60) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. condense this down to a single |
||
memset(&discover->dhcp.options[optoffset], 0, optlen - optoffset); | ||
optoffset = 60; | ||
} | ||
|
||
/* We currently ignore: | ||
The client SHOULD wait a random time between one and ten seconds to | ||
desynchronize the use of DHCP at startup. | ||
|
@@ -1465,10 +1507,28 @@ static int client_handle_offer(sd_dhcp_client *client, DHCPMessage *offer, size_ | |
|
||
r = dhcp_option_parse(offer, len, dhcp_lease_parse_options, lease, NULL); | ||
if (r != DHCP_OFFER) { | ||
log_dhcp_client(client, "received message was not an OFFER, ignoring"); | ||
return -ENOMSG; | ||
if (r == -ENOMSG && client->bootp) { | ||
// Treat a non-DHCP BOOTREPLY like a DHCP ACK, so keep processing | ||
log_dhcp_client(client, "BOOTREPLY received"); | ||
r = DHCP_ACK; | ||
} else { | ||
log_dhcp_client(client, "received message was not an OFFER, ignoring"); | ||
return -ENOMSG; | ||
} | ||
} | ||
|
||
if (client->bootp) { | ||
// This was USEC_INFINITY in systemd v258, but that's a long unsigned int; casting doesn't seem like a good idea | ||
lease->lifetime = UINT32_MAX; | ||
log_dhcp_client(client, "Using infinite lease. BOOTP siaddr=(%#x), DHCP Server Identifier=(%#x)", | ||
offer->siaddr, | ||
lease->server_address); | ||
|
||
lease->server_address = offer->siaddr ? offer->siaddr : lease->server_address; | ||
lease->next_server = 0; | ||
} else | ||
lease->next_server = offer->siaddr; | ||
|
||
lease->next_server = offer->siaddr; | ||
lease->address = offer->yiaddr; | ||
|
||
|
@@ -1478,7 +1538,11 @@ static int client_handle_offer(sd_dhcp_client *client, DHCPMessage *offer, size_ | |
if (lease->address == 0 || | ||
lease->server_address == 0 || | ||
lease->lifetime == 0) { | ||
log_dhcp_client(client, "received lease lacks address, server address or lease lifetime, ignoring"); | ||
log_dhcp_client(client, "received lease lacks address(%#x), server address(%#x) or lease lifetime(%#llx), ignoring.", | ||
lease->address, | ||
lease->server_address, | ||
(unsigned long long) lease->lifetime | ||
); | ||
return -ENOMSG; | ||
} | ||
|
||
|
@@ -1498,7 +1562,10 @@ static int client_handle_offer(sd_dhcp_client *client, DHCPMessage *offer, size_ | |
if (client_notify(client, SD_DHCP_CLIENT_EVENT_SELECTING) < 0) | ||
return -ENOMSG; | ||
|
||
log_dhcp_client(client, "OFFER"); | ||
if (client->bootp) | ||
log_dhcp_client(client, "BOOTREPLY"); | ||
else | ||
log_dhcp_client(client, "OFFER"); | ||
|
||
return 0; | ||
} | ||
|
@@ -1551,8 +1618,8 @@ static int client_handle_ack(sd_dhcp_client *client, DHCPMessage *ack, size_t le | |
|
||
if (client->client_id_len) { | ||
r = dhcp_lease_set_client_id(lease, | ||
(uint8_t *) &client->client_id, | ||
client->client_id_len); | ||
(uint8_t *) &client->client_id, | ||
client->client_id_len); | ||
if (r < 0) | ||
return r; | ||
} | ||
|
@@ -1575,8 +1642,11 @@ static int client_handle_ack(sd_dhcp_client *client, DHCPMessage *ack, size_t le | |
if (lease->address == INADDR_ANY || | ||
lease->server_address == INADDR_ANY || | ||
lease->lifetime == 0) { | ||
log_dhcp_client(client, "received lease lacks address, server " | ||
"address or lease lifetime, ignoring"); | ||
log_dhcp_client(client, "received lease lacks address(%#x), server address(%#x) or lease lifetime(%#llx), ignoring.", | ||
lease->address, | ||
lease->server_address, | ||
(unsigned long long) lease->lifetime | ||
); | ||
return -ENOMSG; | ||
} | ||
|
||
|
@@ -1729,14 +1799,25 @@ static int client_handle_message(sd_dhcp_client *client, DHCPMessage *message, i | |
0, 0, | ||
client_timeout_resend, client, | ||
client->event_priority, "dhcp4-resend-timer", true); | ||
break; | ||
|
||
// Treat a non-DHCP BOOTREPLY like a DHCP ACK, so allow | ||
// fall through to the next case for these. | ||
if (! client->bootp) | ||
break; | ||
[[fallthrough]]; | ||
|
||
case DHCP_STATE_REBOOTING: | ||
case DHCP_STATE_REQUESTING: | ||
case DHCP_STATE_RENEWING: | ||
case DHCP_STATE_REBINDING: | ||
if (client->bootp) | ||
if (client->state == DHCP_STATE_REQUESTING) | ||
r = SD_DHCP_CLIENT_EVENT_IP_ACQUIRE; | ||
else | ||
r = SD_DHCP_CLIENT_EVENT_RENEW; | ||
else | ||
r = client_handle_ack(client, message, len); | ||
|
||
r = client_handle_ack(client, message, len); | ||
if (r == -ENOMSG) | ||
return 0; /* invalid message, let's ignore it */ | ||
if (r == -EADDRNOTAVAIL) { | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a style choice but it looks like this block and the one on 861 could all be inside the
if
block on 814? Alternately you could add!client->bootp
to theif
blocks on lines on 819 and 838 - that would not be my first choice unless your goal is to minimize the diff.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like you are right, nice catch. It wasn't always like that, there was a big refactor towards the end after I discovered that my initial attempt at keeping things simple... wasn't.
I'll give this a try. There's a part of me that's loathe to go back to that stage, rebuild, and then do significant verification. But logically it kinda seems like it's pretty straight forward that the logic is identical and short of seeing it compile I don't really need to retest anything.