[prev in list] [next in list] [prev in thread] [next in thread]
List: busybox
Subject: Re: [PATCH] udhcpc: add support for sending DHCPINFORM requests
From: Luca Boccassi <bluca () debian ! org>
Date: 2022-08-30 21:35:43
Message-ID: 6dba2a568f75d69144159428fcb273a90ed8c0cf.camel () debian ! org
[Download RAW message or body]
[Attachment #2 (multipart/signed)]
> Apologies for the delay.
>
> On Wed, Apr 20, 2022 at 8:35 PM Sinan Kaya <okaya at kernel.org>
> wrote:
> > From b906997217b363c459fdbd2824bfe6c5ac69607e Mon Sep 17 00:00:00
> 2001
> > From: Sinan Kaya <okaya at kernel.org>
> > Date: Tue, 19 Apr 2022 13:47:19 +0000
> > Subject: [PATCH] udhcpc: add support for sending DHCPINFORM
> requests
> >
> > It is useful for applications to be able to query DHCP options
> > without renewing IP address.
>
> I would like a more extended information.
>
> Imagine someone who never needed to do this before.
> Explain to them what would happen when udhcpc
> is run with -I option. How would DHCP handshake change?
>
> Give a real-world example
> (say, one based on your situation which prompted you
> to make this patch).
>
> Give an example where -e IP is useful.
>
> Maybe give references to RFCs which document
> DHCPINFORM usage.
Added in v2.
> > /* Broadcast a DHCP request message */
> > /* RFC 2131 3.1 paragraph 3:
> > - * "The client _broadcasts_ a DHCPREQUEST message..."
> > + * "The client _broadcasts_ a DHCPREQUEST/INFORM message..."
>
> Incorrect. That's not what RFC 2131 3.1 paragraph 3 says.
Rearranged the comment in v2.
> > */
> > /* NOINLINE: limit stack usage in caller */
> > -static NOINLINE int send_select(uint32_t server, uint32_t
> requested)
> > +static NOINLINE int send_select(uint32_t server, uint32_t
> requested,
> > int inform)
> > {
> > struct dhcp_packet packet;
> > struct in_addr temp_addr;
> > char server_str[sizeof("255.255.255.255")];
> > + const char *direction;
> >
> > /*
> > * RFC 2131 4.3.2 DHCPREQUEST message
> > @@ -766,11 +771,12 @@ static NOINLINE int send_select(uint32_t
> server,
> > uint32_t requested)
> > /* Fill in: op, htype, hlen, cookie, chaddr fields,
> > * xid field, message type option:
> > */
> > - init_packet(&packet, DHCPREQUEST);
> > + init_packet(&packet, inform ? DHCPINFORM: DHCPREQUEST);
> >
> > udhcp_add_simple_option(&packet, DHCP_REQUESTED_IP,
> requested);
> >
> > - udhcp_add_simple_option(&packet, DHCP_SERVER_ID, server);
> > + if (server)
> > + udhcp_add_simple_option(&packet, DHCP_SERVER_ID,
> server);
>
> Do you mean, if server is not specified, SERVER_ID may be omitted?
> Should be omitted?
> Should this change be made regardless of DHCPINFORM support?
Removed in v2.
> >
> > /* Add options: maxsize,
> > * "param req" option according to -O, options specified
> with -x
> > @@ -780,11 +786,19 @@ static NOINLINE int send_select(uint32_t
> server,
> > uint32_t requested)
> > temp_addr.s_addr = server;
> > strcpy(server_str, inet_ntoa(temp_addr));
> > temp_addr.s_addr = requested;
> > - bb_info_msg("broadcasting select for %s, server %s",
> > - inet_ntoa(temp_addr),
> > - server_str
> > + if (server)
> > + direction = "unicasting";
> > + else
> > + direction = "broadcasting";
> > +
> > + bb_info_msg("%s select for %s, server %s",
> > + direction,
> > + inet_ntoa(temp_addr),
> > + server_str
> > );
>
> RFC 2131 3.1 paragraph 3 says:
> "The client broadcasts a DHCPREQUEST message".
> Always. Even if server-id is known.
For DHCPREQUEST yes, for DHCPINFORM no:
RFC 2131 paragraph 4.4.3:
The client then unicasts the DHCPINFORM to the DHCP server if it
knows the server's address, otherwise it broadcasts the message to
the limited (all 1s) broadcast address.
Rearranged in v2 so that the server/unicast mode is only used for
DHCPINFORM.
> > - return raw_bcast_from_client_data_ifindex(&packet,
> INADDR_ANY);
> > +
> > + // return raw_bcast_from_client_data_ifindex(&packet,
> INADDR_ANY);
> > + return bcast_or_ucast(&packet, requested, server);
> > }
> >
> > /* Unicast or broadcast a DHCP renew message */
> > @@ -1161,9 +1175,9 @@ static void client_background(void)
> > //usage:# define IF_UDHCP_VERBOSE(...)
> > //usage:#endif
> > //usage:#define udhcpc_trivial_usage
> > -//usage:
> > "[-fbq"IF_UDHCP_VERBOSE("v")"RB]"IF_FEATURE_UDHCPC_ARPING("
> > [-a[MSEC]]")" [-t N] [-T SEC] [-A SEC|-n]\n"
> > +//usage:
> > "[-fbq"IF_UDHCP_VERBOSE("v")"RBI]"IF_FEATURE_UDHCPC_ARPING("
> > [-a[MSEC]]")" [-t N] [-T SEC] [-A SEC|-n]\n"
> > //usage: " [-i IFACE]"IF_FEATURE_UDHCP_PORT(" [-P
> PORT]")" [-s
> > PROG] [-p PIDFILE]\n"
> > -//usage: " [-oC] [-r IP] [-V VENDOR] [-F NAME] [-x
> OPT:VAL]...
> > [-O OPT]..."
> > +//usage: " [-oC] [-r IP] [-e IP] [-V VENDOR] [-F NAME]
> [-x
> > OPT:VAL]... [-O OPT]..."
> > //usage:#define udhcpc_full_usage "\n"
> > //usage: "\n -i IFACE Interface to use (default
> > "CONFIG_UDHCPC_DEFAULT_INTERFACE")"
> > //usage: IF_FEATURE_UDHCP_PORT(
> > @@ -1172,6 +1186,7 @@ static void client_background(void)
> > //usage: "\n -s PROG Run PROG at DHCP events
> (default
> > "CONFIG_UDHCPC_DEFAULT_SCRIPT")"
> > //usage: "\n -p FILE Create pidfile"
> > //usage: "\n -B Request broadcast replies"
> > +//usage: "\n -I Request using inform"
>
> This barely explains anything.
Updated in v2.
> > +//usage: "\n -e IP Request this server IP
> address"
>
> This is not correct. We do not "request server IPs".
> We send requests to servers (sometimes by unicasting to their IPs,
> but usually via broadcast).
Updated in v2.
> > +
> > + if (opt & OPT_I)
> > + use_inform = 1;
>
> Why do you need to have use_inform variable when (opt & OPT_I)
> works just as well?
Removed in v2.
--
Kind regards,
Luca Boccassi
["signature.asc" (application/pgp-signature)]
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic