On Sat, Oct 05, 2019 at 12:44:25AM +0200, Javier Martinez Canillas wrote:
> From: Paulo Flabiano Smorigo <pfsmor...@br.ibm.com>
>
> This patch sets a net_<interface>_clientid and net_<interface>_clientuuid
> GRUB environment variables, using the DHCP client ID and UUID options if
> these are found.
>
> In the same way than net_<interface>_<option> variables are set for other
> options such domain name, boot file, next server, etc.
>
> Signed-off-by: Paulo Flabiano Smorigo <pfsmor...@br.ibm.com>
> Signed-off-by: Javier Martinez Canillas <javi...@redhat.com>
In general "Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com>"
but a few nit picks below...
> ---
>
> grub-core/net/bootp.c | 85 +++++++++++++++++++++++++++++++++++++++----
> include/grub/net.h | 2 +
> 2 files changed, 79 insertions(+), 8 deletions(-)
>
> diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
> index 04cfbb04504..0e6e41a1699 100644
> --- a/grub-core/net/bootp.c
> +++ b/grub-core/net/bootp.c
> @@ -95,6 +95,49 @@ enum
> /* Max timeout when waiting for BOOTP/DHCP reply */
> #define GRUB_DHCP_MAX_PACKET_TIMEOUT 32
>
> +static char *
> +grub_env_write_readonly (struct grub_env_var *var __attribute__ ((unused)),
> + const char *val __attribute__ ((unused)))
s/grub_env_write_readonly/grub_env_ro_write/?
> +{
> + return NULL;
> +}
> +
> +static void
> +set_env_limn_ro (const char *intername, const char *suffix,
> + const char *value, grub_size_t len)
I have hard time reading this. What does "limn" stand for?
Could we make the name of this function less cryptic?
> +{
> + char *varname, *varvalue;
> + char *ptr;
Please add empty line here.
> + varname = grub_xasprintf ("net_%s_%s", intername, suffix);
> + if (!varname)
> + return;
> + for (ptr = varname; *ptr; ptr++)
> + if (*ptr == ':')
> + *ptr = '_';
> + varvalue = grub_malloc (len + 1);
grub_zalloc() and then you can...
> + if (!varvalue)
> + {
> + grub_free (varname);
> + return;
> + }
> +
> + grub_memcpy (varvalue, value, len);
> + varvalue[len] = 0;
...drop this...
Daniel
> + grub_env_set (varname, varvalue);
> + grub_register_variable_hook (varname, 0, grub_env_write_readonly);
> + grub_env_export (varname);
> + grub_free (varname);
> + grub_free (varvalue);
> +}
> +
> +static char
> +hexdigit (grub_uint8_t val)
> +{
> + if (val < 10)
> + return val + '0';
> + return val + 'a' - 10;
> +}
> +
> static const void *
> find_dhcp_option (const struct grub_net_bootp_packet *bp, grub_size_t size,
> grub_uint8_t opt_code, grub_uint8_t *opt_len)
> @@ -152,6 +195,9 @@ again:
> if (i + taglength >= size)
> return NULL;
>
> + grub_dprintf("net", "DHCP option %u (0x%02x) found with length %u.\n",
> + tagtype, tagtype, taglength);
> +
> /* FIXME RFC 3396 options concatentation */
> if (tagtype == opt_code)
> {
> @@ -354,6 +400,37 @@ grub_net_configure_by_dhcp_ack (const char *name,
> }
> grub_net_add_ipv4_local (inter, mask);
>
> + opt = find_dhcp_option (bp, size, GRUB_NET_BOOTP_CLIENT_ID, &opt_len);
> + if (opt)
> + {
> + set_env_limn_ro (name, "clientid", (char *) opt, opt_len);
> + }
> +
> + opt = find_dhcp_option (bp, size, GRUB_NET_BOOTP_CLIENT_UUID, &opt_len);
> + if (opt && opt_len == 17)
> + {
> + /* The format is 9cfe245e-d0c8-bd45-a79f-54ea5fbd3d97 */
> +
> + opt += 1;
> + opt_len -= 1;
> +
> + char *val = grub_malloc (2 * opt_len + 4 + 1);
> + int i = 0;
> + int j = 0;
> + for (i = 0; i < opt_len; i++)
> + {
> + val[2 * i + j] = hexdigit (opt[i] >> 4);
> + val[2 * i + 1 + j] = hexdigit (opt[i] & 0xf);
> +
> + if ((i == 3) || (i == 5) || (i == 7) || (i == 9))
> + {
> + j++;
> + val[2 * i + 1+ j] = '-';
> + }
> + }
> + set_env_limn_ro (name, "clientuuid", (char *) val, 2 * opt_len + 4);
> + }
> +
> /* We do not implement dead gateway detection and the first entry SHOULD
> be preferred one */
> opt = find_dhcp_option (bp, size, GRUB_NET_BOOTP_ROUTER, &opt_len);
> @@ -631,14 +708,6 @@ grub_net_process_dhcp (struct grub_net_buff *nb,
> }
> }
>
> -static char
> -hexdigit (grub_uint8_t val)
> -{
> - if (val < 10)
> - return val + '0';
> - return val + 'a' - 10;
> -}
> -
> static grub_err_t
> grub_cmd_dhcpopt (struct grub_command *cmd __attribute__ ((unused)),
> int argc, char **args)
> diff --git a/include/grub/net.h b/include/grub/net.h
> index 4a9069a1474..556c54e579f 100644
> --- a/include/grub/net.h
> +++ b/include/grub/net.h
> @@ -462,6 +462,8 @@ enum
> GRUB_NET_BOOTP_DOMAIN = 0x0f,
> GRUB_NET_BOOTP_ROOT_PATH = 0x11,
> GRUB_NET_BOOTP_EXTENSIONS_PATH = 0x12,
> + GRUB_NET_BOOTP_CLIENT_ID = 0x3d,
> + GRUB_NET_BOOTP_CLIENT_UUID = 0x61,
> GRUB_NET_DHCP_REQUESTED_IP_ADDRESS = 50,
> GRUB_NET_DHCP_OVERLOAD = 52,
> GRUB_NET_DHCP_MESSAGE_TYPE = 53,
> --
> 2.21.0
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
