Hello Daniel,
On 10/21/19 4:40 PM, Daniel Kiper wrote:
> On Fri, Oct 18, 2019 at 09:59:09AM +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>
>> Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com>
>
> It seems to me that this RB should land in patch #3. Does not it?
Ups, sorry about that.
> Anyway, Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com> :-)
> except one thing...
>
>> ---
>>
>> Changes in v2:
>> - Use the existing grub_env_set_net_property() and remove duplicated code.
>>
>> grub-core/net/bootp.c | 48 +++++++++++++++++++++++++++++++++++--------
>> include/grub/net.h | 2 ++
>> 2 files changed, 42 insertions(+), 8 deletions(-)
>>
>> diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
>> index 04cfbb04504..377006da938 100644
>> --- a/grub-core/net/bootp.c
>> +++ b/grub-core/net/bootp.c
>> @@ -95,6 +95,14 @@ enum
>> /* Max timeout when waiting for BOOTP/DHCP reply */
>> #define GRUB_DHCP_MAX_PACKET_TIMEOUT 32
>>
>> +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 +160,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)
>> {
>> @@ -406,6 +417,35 @@ grub_net_configure_by_dhcp_ack (const char *name,
>> if (opt && opt_len)
>> grub_env_set_net_property (name, "extensionspath", (const char *) opt,
>> opt_len);
>>
>> + opt = find_dhcp_option (bp, size, GRUB_NET_BOOTP_CLIENT_ID, &opt_len);
>> + if (opt && opt_len)
>> + grub_env_set_net_property (name, "clientid", (const 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] = '-';
>> + }
>> + }
>> + grub_env_set_net_property (name, "clientuuid", (char *) val, 2 *
>> opt_len + 4);
>> + }
>> +
>> inter->dhcp_ack = grub_malloc (size);
>> if (inter->dhcp_ack)
>> {
>> @@ -631,14 +671,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,
>
> I have just realized that this enum is a mixture of decimals and hexes.
> This is a mess. Could you add a patch before that patch changing all
> decimals to hexes and sort them properly?
>
Sure. I'll do that.
I also noticed that we have a patch that adds documentation about the
netboot grub.cfg selection order so I'll also include that patch in v3.
> Daniel
>
Best regards,
--
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
