[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: Denys Vlasenko <vda.linux () googlemail ! com>
Date: 2022-07-13 11:16:39
Message-ID: CAK1hOcNN_YFFsuURJRp53+OEhBjogzfete8czPOwZEh_2sOAyw () mail ! gmail ! com
[Download RAW message or body]
Apologies for the delay.
On Wed, Apr 20, 2022 at 8:35 PM Sinan Kaya <okaya@kernel.org> wrote:
> From b906997217b363c459fdbd2824bfe6c5ac69607e Mon Sep 17 00:00:00 2001
> From: Sinan Kaya <okaya@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.
> /* 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.
> */
> /* 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?
>
> /* 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.
> - 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.
> +//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).
> +
> + if (opt & OPT_I)
> + use_inform = 1;
Why do you need to have use_inform variable when (opt & OPT_I)
works just as well?
_______________________________________________
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