From 827b690fa7b9dd006f305449c342fb1b9bff6fb7 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Tue, 15 Jun 2021 01:02:01 +0200 Subject: [PATCH] udhcpc[6]: do not pass xid around, keep it in client_data.xid function old new delta perform_release 105 169 +64 perform_d6_release 259 262 +3 init_d6_packet 84 85 +1 send_d6_discover 286 285 -1 send_d6_select 128 126 -2 send_d6_renew 176 174 -2 send_d6_info_request 65 63 -2 udhcpc_main 2555 2551 -4 send_select 130 122 -8 send_renew 99 91 -8 send_discover 89 81 -8 udhcpc6_main 2636 2602 -34 send_release 74 - -74 ------------------------------------------------------------------------------ (add/remove: 0/1 grow/shrink: 3/9 up/down: 68/-143) Total: -75 bytes Signed-off-by: Denys Vlasenko --- networking/udhcp/d6_dhcpc.c | 56 ++++++++++++++++++------------------- networking/udhcp/dhcpc.c | 51 +++++++++++++-------------------- networking/udhcp/dhcpc.h | 1 + 3 files changed, 48 insertions(+), 60 deletions(-) diff --git a/networking/udhcp/d6_dhcpc.c b/networking/udhcp/d6_dhcpc.c index 7f288f891..802a27521 100644 --- a/networking/udhcp/d6_dhcpc.c +++ b/networking/udhcp/d6_dhcpc.c @@ -479,15 +479,15 @@ static ALWAYS_INLINE uint32_t random_xid(void) } /* Initialize the packet with the proper defaults */ -static uint8_t *init_d6_packet(struct d6_packet *packet, char type, uint32_t xid) +static uint8_t *init_d6_packet(struct d6_packet *packet, char type) { uint8_t *ptr; unsigned secs; memset(packet, 0, sizeof(*packet)); - packet->d6_xid32 = xid; - packet->d6_msg_type = type; + packet->d6_xid32 = client_data.xid; + packet->d6_msg_type = type; /* union, overwrites lowest byte of d6_xid32 */ /* ELAPSED_TIME option is required to be present by the RFC, * and some servers do check for its presense. [which?] @@ -585,13 +585,13 @@ static int d6_mcast_from_client_data_ifindex(struct d6_packet *packet, uint8_t * * about parameter values the client would like to have returned. */ /* NOINLINE: limit stack usage in caller */ -static NOINLINE int send_d6_info_request(uint32_t xid) +static NOINLINE int send_d6_info_request(void) { struct d6_packet packet; uint8_t *opt_ptr; - /* Fill in: msg type */ - opt_ptr = init_d6_packet(&packet, D6_MSG_INFORMATION_REQUEST, xid); + /* Fill in: msg type, xid, ELAPSED_TIME */ + opt_ptr = init_d6_packet(&packet, D6_MSG_INFORMATION_REQUEST); /* Add options: client-id, * "param req" option according to -O, options specified with -x @@ -684,14 +684,14 @@ static NOINLINE int send_d6_info_request(uint32_t xid) +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ */ /* NOINLINE: limit stack usage in caller */ -static NOINLINE int send_d6_discover(uint32_t xid, struct in6_addr *requested_ipv6) +static NOINLINE int send_d6_discover(struct in6_addr *requested_ipv6) { struct d6_packet packet; uint8_t *opt_ptr; unsigned len; - /* Fill in: msg type */ - opt_ptr = init_d6_packet(&packet, D6_MSG_SOLICIT, xid); + /* Fill in: msg type, xid, ELAPSED_TIME */ + opt_ptr = init_d6_packet(&packet, D6_MSG_SOLICIT); /* Create new IA_NA, optionally with included IAADDR with requested IP */ free(client6_data.ia_na); @@ -763,13 +763,13 @@ static NOINLINE int send_d6_discover(uint32_t xid, struct in6_addr *requested_ip * messages from the server. */ /* NOINLINE: limit stack usage in caller */ -static NOINLINE int send_d6_select(uint32_t xid) +static NOINLINE int send_d6_select(void) { struct d6_packet packet; uint8_t *opt_ptr; - /* Fill in: msg type */ - opt_ptr = init_d6_packet(&packet, D6_MSG_REQUEST, xid); + /* Fill in: msg type, xid, ELAPSED_TIME */ + opt_ptr = init_d6_packet(&packet, D6_MSG_REQUEST); /* server id */ opt_ptr = mempcpy(opt_ptr, client6_data.server_id, client6_data.server_id->len + 2+2); @@ -836,13 +836,13 @@ static NOINLINE int send_d6_select(uint32_t xid) * about parameter values the client would like to have returned. */ /* NOINLINE: limit stack usage in caller */ -static NOINLINE int send_d6_renew(uint32_t xid, struct in6_addr *server_ipv6, struct in6_addr *our_cur_ipv6) +static NOINLINE int send_d6_renew(struct in6_addr *server_ipv6, struct in6_addr *our_cur_ipv6) { struct d6_packet packet; uint8_t *opt_ptr; - /* Fill in: msg type */ - opt_ptr = init_d6_packet(&packet, DHCPREQUEST, xid); + /* Fill in: msg type, xid, ELAPSED_TIME */ + opt_ptr = init_d6_packet(&packet, DHCPREQUEST); /* server id */ opt_ptr = mempcpy(opt_ptr, client6_data.server_id, client6_data.server_id->len + 2+2); @@ -877,8 +877,8 @@ int send_d6_release(struct in6_addr *server_ipv6, struct in6_addr *our_cur_ipv6) uint8_t *opt_ptr; struct option_set *ci; - /* Fill in: msg type, client id */ - opt_ptr = init_d6_packet(&packet, D6_MSG_RELEASE, random_xid()); + /* Fill in: msg type, xid, ELAPSED_TIME */ + opt_ptr = init_d6_packet(&packet, D6_MSG_RELEASE); /* server id */ opt_ptr = mempcpy(opt_ptr, client6_data.server_id, client6_data.server_id->len + 2+2); /* IA NA (contains our current IP) */ @@ -1097,6 +1097,7 @@ static void perform_d6_release(struct in6_addr *server_ipv6, struct in6_addr *ou || client_data.state == RENEW_REQUESTED ) { bb_simple_info_msg("unicasting a release"); + client_data.xid = random_xid(); //TODO: can omit? send_d6_release(server_ipv6, our_cur_ipv6); /* unicast */ } bb_simple_info_msg("entering released state"); @@ -1183,7 +1184,6 @@ int udhcpc6_main(int argc UNUSED_PARAM, char **argv) struct in6_addr srv6_buf; struct in6_addr ipv6_buf; struct in6_addr *requested_ipv6; - uint32_t xid = 0; int packet_num; int timeout; /* must be signed */ int lease_remaining; /* must be signed */ @@ -1387,13 +1387,13 @@ int udhcpc6_main(int argc UNUSED_PARAM, char **argv) if (!discover_retries || packet_num < discover_retries) { if (packet_num == 0) { change_listen_mode(LISTEN_RAW); - xid = random_xid(); + client_data.xid = random_xid(); } /* multicast */ if (opt & OPT_l) - send_d6_info_request(xid); + send_d6_info_request(); else - send_d6_discover(xid, requested_ipv6); + send_d6_discover(requested_ipv6); timeout = discover_timeout; packet_num++; continue; @@ -1427,7 +1427,7 @@ int udhcpc6_main(int argc UNUSED_PARAM, char **argv) case REQUESTING: if (!discover_retries || packet_num < discover_retries) { /* send multicast select packet */ - send_d6_select(xid); + send_d6_select(); timeout = discover_timeout; packet_num++; continue; @@ -1459,9 +1459,9 @@ int udhcpc6_main(int argc UNUSED_PARAM, char **argv) * into INIT_SELECTING state. */ if (opt & OPT_l) - send_d6_info_request(xid); + send_d6_info_request(); else - send_d6_renew(xid, &srv6_buf, requested_ipv6); + send_d6_renew(&srv6_buf, requested_ipv6); timeout = discover_timeout; packet_num++; continue; @@ -1484,9 +1484,9 @@ int udhcpc6_main(int argc UNUSED_PARAM, char **argv) * try to find DHCP server using broadcast */ if (lease_remaining > 0 && packet_num < 3) { if (opt & OPT_l) - send_d6_info_request(xid); + send_d6_info_request(); else /* send a broadcast renew request */ - send_d6_renew(xid, /*server_ipv6:*/ NULL, requested_ipv6); + send_d6_renew(/*server_ipv6:*/ NULL, requested_ipv6); timeout = discover_timeout; packet_num++; continue; @@ -1581,9 +1581,9 @@ int udhcpc6_main(int argc UNUSED_PARAM, char **argv) packet_end = (uint8_t*)&packet + len; } - if ((packet.d6_xid32 & htonl(0x00ffffff)) != xid) { + if ((packet.d6_xid32 & htonl(0x00ffffff)) != client_data.xid) { log1("xid %x (our is %x)%s", - (unsigned)(packet.d6_xid32 & htonl(0x00ffffff)), (unsigned)xid, + (unsigned)(packet.d6_xid32 & htonl(0x00ffffff)), (unsigned)client_data.xid, ", ignoring packet" ); continue; diff --git a/networking/udhcp/dhcpc.c b/networking/udhcp/dhcpc.c index b80ea2f40..1c3c478ac 100644 --- a/networking/udhcp/dhcpc.c +++ b/networking/udhcp/dhcpc.c @@ -605,7 +605,7 @@ static void init_packet(struct dhcp_packet *packet, char type) /* Fill in: op, htype, hlen, cookie fields; message type option: */ udhcp_init_header(packet, type); - packet->xid = random_xid(); + packet->xid = client_data.xid; client_data.last_secs = monotonic_sec(); if (client_data.first_secs == 0) @@ -719,17 +719,15 @@ static int bcast_or_ucast(struct dhcp_packet *packet, uint32_t ciaddr, uint32_t /* Broadcast a DHCP discover packet to the network, with an optionally requested IP */ /* NOINLINE: limit stack usage in caller */ -static NOINLINE int send_discover(uint32_t xid, uint32_t requested) +static NOINLINE int send_discover(uint32_t requested) { struct dhcp_packet packet; /* Fill in: op, htype, hlen, cookie, chaddr fields, - * random xid field (we override it below), - * message type option: + * xid field, message type option: */ init_packet(&packet, DHCPDISCOVER); - packet.xid = xid; if (requested) udhcp_add_simple_option(&packet, DHCP_REQUESTED_IP, requested); @@ -747,7 +745,7 @@ static NOINLINE int send_discover(uint32_t xid, uint32_t requested) * "The client _broadcasts_ a DHCPREQUEST message..." */ /* NOINLINE: limit stack usage in caller */ -static NOINLINE int send_select(uint32_t xid, uint32_t server, uint32_t requested) +static NOINLINE int send_select(uint32_t server, uint32_t requested) { struct dhcp_packet packet; struct in_addr temp_addr; @@ -766,12 +764,10 @@ static NOINLINE int send_select(uint32_t xid, uint32_t server, uint32_t requeste * include that list in all subsequent messages. */ /* Fill in: op, htype, hlen, cookie, chaddr fields, - * random xid field (we override it below), - * message type option: + * xid field, message type option: */ init_packet(&packet, DHCPREQUEST); - packet.xid = xid; udhcp_add_simple_option(&packet, DHCP_REQUESTED_IP, requested); udhcp_add_simple_option(&packet, DHCP_SERVER_ID, server); @@ -793,7 +789,7 @@ static NOINLINE int send_select(uint32_t xid, uint32_t server, uint32_t requeste /* Unicast or broadcast a DHCP renew message */ /* NOINLINE: limit stack usage in caller */ -static NOINLINE int send_renew(uint32_t xid, uint32_t server, uint32_t ciaddr) +static NOINLINE int send_renew(uint32_t server, uint32_t ciaddr) { struct dhcp_packet packet; @@ -812,12 +808,10 @@ static NOINLINE int send_renew(uint32_t xid, uint32_t server, uint32_t ciaddr) * replying to the client. */ /* Fill in: op, htype, hlen, cookie, chaddr fields, - * random xid field (we override it below), - * message type option: + * xid field, message type option: */ init_packet(&packet, DHCPREQUEST); - packet.xid = xid; packet.ciaddr = ciaddr; /* Add options: maxsize, @@ -839,7 +833,7 @@ static NOINLINE int send_renew(uint32_t xid, uint32_t server, uint32_t ciaddr) #if ENABLE_FEATURE_UDHCPC_ARPING /* Broadcast a DHCP decline message */ /* NOINLINE: limit stack usage in caller */ -static NOINLINE int send_decline(/*uint32_t xid,*/ uint32_t server, uint32_t requested) +static NOINLINE int send_decline(uint32_t server, uint32_t requested) { struct dhcp_packet packet; @@ -848,14 +842,6 @@ static NOINLINE int send_decline(/*uint32_t xid,*/ uint32_t server, uint32_t req */ init_packet(&packet, DHCPDECLINE); -#if 0 - /* RFC 2131 says DHCPDECLINE's xid is randomly selected by client, - * but in case the server is buggy and wants DHCPDECLINE's xid - * to match the xid which started entire handshake, - * we use the same xid we used in initial DHCPDISCOVER: - */ - packet.xid = xid; -#endif /* DHCPDECLINE uses "requested ip", not ciaddr, to store offered IP */ udhcp_add_simple_option(&packet, DHCP_REQUESTED_IP, requested); @@ -1145,6 +1131,7 @@ static void perform_release(uint32_t server_addr, uint32_t requested_ip) temp_addr.s_addr = requested_ip; bb_info_msg("unicasting a release of %s to %s", inet_ntoa(temp_addr), buffer); + client_data.xid = random_xid(); //TODO: can omit? send_release(server_addr, requested_ip); /* unicast */ } bb_simple_info_msg("entering released state"); @@ -1233,7 +1220,6 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv) int discover_retries = 3; uint32_t server_addr = server_addr; /* for compiler */ uint32_t requested_ip = 0; - uint32_t xid = xid; /* for compiler */ int packet_num; int timeout; /* must be signed */ int lease_remaining; /* must be signed */ @@ -1458,10 +1444,10 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv) if (!discover_retries || packet_num < discover_retries) { if (packet_num == 0) { change_listen_mode(LISTEN_RAW); - xid = random_xid(); + client_data.xid = random_xid(); } /* broadcast */ - send_discover(xid, requested_ip); + send_discover(requested_ip); timeout = discover_timeout; packet_num++; continue; @@ -1495,7 +1481,7 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv) case REQUESTING: if (packet_num < 3) { /* send broadcast select packet */ - send_select(xid, server_addr, requested_ip); + send_select(server_addr, requested_ip); timeout = discover_timeout; packet_num++; continue; @@ -1526,7 +1512,7 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv) * Anyway, it does recover by eventually failing through * into INIT_SELECTING state. */ - if (send_renew(xid, server_addr, requested_ip) >= 0) { + if (send_renew(server_addr, requested_ip) >= 0) { timeout = discover_timeout; packet_num++; continue; @@ -1555,7 +1541,7 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv) * try to find DHCP server using broadcast */ if (lease_remaining > 0 && packet_num < 3) { /* send a broadcast renew request */ - send_renew(xid, 0 /*INADDR_ANY*/, requested_ip); + send_renew(0 /*INADDR_ANY*/, requested_ip); timeout = discover_timeout; packet_num++; continue; @@ -1648,9 +1634,9 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv) continue; } - if (packet.xid != xid) { + if (packet.xid != client_data.xid) { log1("xid %x (our is %x)%s", - (unsigned)packet.xid, (unsigned)xid, + (unsigned)packet.xid, (unsigned)client_data.xid, ", ignoring packet" ); continue; @@ -1779,7 +1765,8 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv) ) { bb_simple_info_msg("offered address is in use " "(got ARP reply), declining"); - send_decline(/*xid,*/ server_addr, packet.yiaddr); + client_data.xid = random_xid(); //TODO: can omit? + send_decline(server_addr, packet.yiaddr); if (client_data.state != REQUESTING) d4_run_script_deconfig(); @@ -1814,7 +1801,7 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv) timeout = (unsigned)lease_remaining / 2; client_data.state = BOUND; /* make future renew packets use different xid */ - /* xid = random_xid(); ...but why bother? */ + /* client_data.xid = random_xid(); ...but why bother? */ packet_num = 0; continue; /* back to main loop */ } diff --git a/networking/udhcp/dhcpc.h b/networking/udhcp/dhcpc.h index cd9ead6bd..19b054b32 100644 --- a/networking/udhcp/dhcpc.h +++ b/networking/udhcp/dhcpc.h @@ -11,6 +11,7 @@ struct client_data_t { uint8_t client_mac[6]; /* Our mac address */ IF_FEATURE_UDHCP_PORT(uint16_t port;) int ifindex; /* Index number of the interface to use */ + uint32_t xid; uint8_t opt_mask[256 / 8]; /* Bitmask of options to send (-O option) */ // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TODO: DHCPv6 has 16-bit option numbers const char *interface; /* The name of the interface to use */