All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Search for specific config files using UUID, MAC and IP
@ 2019-10-18  7:59 Javier Martinez Canillas
  2019-10-18  7:59 ` [PATCH v2 1/3] Add %X option to printf functions Javier Martinez Canillas
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Javier Martinez Canillas @ 2019-10-18  7:59 UTC (permalink / raw)
  To: grub-devel
  Cc: Vladimir Serbinenko, Daniel Kiper, Paulo Flabiano Smorigo,
	Javier Martinez Canillas

Hello,

This is a 2nd version of a series that have been carried in Fedora (and
other distros) for a long time. It allows to fetch GRUB configurations
that are customized for each machine by using the client UUID, MAC and
IP addresses as suffixes for the grub.cfg file.

The procedure is similar to what is used by pxelinux and yaboot, i.e:
http://www.syslinux.org/wiki/index.php/PXELINUX#config

Patch #1 adds a %X format specifier to make the grub_printf() function
more similar to printf() and not require to use %x plus grub_toupper().

Patch #2 sets net_<interface>_client{id,uuid} environment variables by
using the DHCP client ID and UUID options if these are found.

Finally Patch #3 adds the support to fetch the config files using the
UUID, MAC and IP as suffixes. If these aren't found, the default GRUB
config filename is used as a fallback.

Best regards,
Javier

Changes in v2:
- Put patch that adds the %X format specifier first in the series.
- Use the existing grub_env_set_net_property() and remove duplicated code.
- Add Reviewed-by tag from Daniel Kiper.

Paulo Flabiano Smorigo (3):
  Add %X option to printf functions
  Set net_<interface>_client{id,uuid} variables from DHCP options
  Search for specific config files for netboot

 grub-core/kern/misc.c   |   7 ++-
 grub-core/net/bootp.c   |  48 ++++++++++++++---
 grub-core/net/net.c     | 117 ++++++++++++++++++++++++++++++++++++++++
 grub-core/normal/main.c |  17 ++++--
 include/grub/net.h      |   5 ++
 5 files changed, 180 insertions(+), 14 deletions(-)

-- 
2.21.0



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/3] Add %X option to printf functions
  2019-10-18  7:59 [PATCH v2 0/3] Search for specific config files using UUID, MAC and IP Javier Martinez Canillas
@ 2019-10-18  7:59 ` Javier Martinez Canillas
  2019-10-18  7:59 ` [PATCH v2 2/3] Set net_<interface>_client{id, uuid} variables from DHCP options Javier Martinez Canillas
  2019-10-18  7:59 ` [PATCH v2 3/3] Search for specific config files for netboot Javier Martinez Canillas
  2 siblings, 0 replies; 6+ messages in thread
From: Javier Martinez Canillas @ 2019-10-18  7:59 UTC (permalink / raw)
  To: grub-devel
  Cc: Vladimir Serbinenko, Daniel Kiper, Paulo Flabiano Smorigo,
	Javier Martinez Canillas

From: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>

The printf(3) function has support for the %X format specifier, to output
an unsigned hexadecimal integer in uppercase.

This can be achived in GRUB using the %x format specifier in grub_printf()
and calling grub_toupper(), but it is more convenient if there is support
for %X in grub_printf().

Signed-off-by: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

---

Changes in v2:
- Put patch that adds the %X format specifier first in the series.

 grub-core/kern/misc.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
index 3b633d51f4c..76e7fb22872 100644
--- a/grub-core/kern/misc.c
+++ b/grub-core/kern/misc.c
@@ -588,7 +588,7 @@ grub_divmod64 (grub_uint64_t n, grub_uint64_t d, grub_uint64_t *r)
 static inline char *
 grub_lltoa (char *str, int c, unsigned long long n)
 {
-  unsigned base = (c == 'x') ? 16 : 10;
+  unsigned base = ((c == 'x') || (c == 'X')) ? 16 : 10;
   char *p;
 
   if ((long long) n < 0 && c == 'd')
@@ -603,7 +603,7 @@ grub_lltoa (char *str, int c, unsigned long long n)
     do
       {
 	unsigned d = (unsigned) (n & 0xf);
-	*p++ = (d > 9) ? d + 'a' - 10 : d + '0';
+	*p++ = (d > 9) ? d + ((c == 'x') ? 'a' : 'A') - 10 : d + '0';
       }
     while (n >>= 4);
   else
@@ -676,6 +676,7 @@ parse_printf_args (const char *fmt0, struct printf_args *args,
 	{
 	case 'p':
 	case 'x':
+	case 'X':
 	case 'u':
 	case 'd':
 	case 'c':
@@ -762,6 +763,7 @@ parse_printf_args (const char *fmt0, struct printf_args *args,
       switch (c)
 	{
 	case 'x':
+	case 'X':
 	case 'u':
 	  args->ptr[curn].type = UNSIGNED_INT + longfmt;
 	  break;
@@ -900,6 +902,7 @@ grub_vsnprintf_real (char *str, grub_size_t max_len, const char *fmt0,
 	  c = 'x';
 	  /* Fall through. */
 	case 'x':
+	case 'X':
 	case 'u':
 	case 'd':
 	  {
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/3] Set net_<interface>_client{id, uuid} variables from DHCP options
  2019-10-18  7:59 [PATCH v2 0/3] Search for specific config files using UUID, MAC and IP Javier Martinez Canillas
  2019-10-18  7:59 ` [PATCH v2 1/3] Add %X option to printf functions Javier Martinez Canillas
@ 2019-10-18  7:59 ` Javier Martinez Canillas
  2019-10-21 14:40   ` [PATCH v2 2/3] Set net_<interface>_client{id,uuid} " Daniel Kiper
  2019-10-18  7:59 ` [PATCH v2 3/3] Search for specific config files for netboot Javier Martinez Canillas
  2 siblings, 1 reply; 6+ messages in thread
From: Javier Martinez Canillas @ 2019-10-18  7:59 UTC (permalink / raw)
  To: grub-devel
  Cc: Vladimir Serbinenko, Daniel Kiper, Paulo Flabiano Smorigo,
	Javier Martinez Canillas

From: Paulo Flabiano Smorigo <pfsmorigo@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 <pfsmorigo@br.ibm.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

---

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,
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 3/3] Search for specific config files for netboot
  2019-10-18  7:59 [PATCH v2 0/3] Search for specific config files using UUID, MAC and IP Javier Martinez Canillas
  2019-10-18  7:59 ` [PATCH v2 1/3] Add %X option to printf functions Javier Martinez Canillas
  2019-10-18  7:59 ` [PATCH v2 2/3] Set net_<interface>_client{id, uuid} variables from DHCP options Javier Martinez Canillas
@ 2019-10-18  7:59 ` Javier Martinez Canillas
  2 siblings, 0 replies; 6+ messages in thread
From: Javier Martinez Canillas @ 2019-10-18  7:59 UTC (permalink / raw)
  To: grub-devel
  Cc: Vladimir Serbinenko, Daniel Kiper, Paulo Flabiano Smorigo,
	Javier Martinez Canillas

From: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>

This patch implements a search for a specific configuration when the config
file is on a remoteserver. It uses the following order:
   1) DHCP client UUID option.
   2) MAC address (in lower case hexadecimal with dash separators);
   3) IP (in upper case hexadecimal) or IPv6;
   4) The original grub.cfg file.

This procedure is similar to what is used by pxelinux and yaboot:
http://www.syslinux.org/wiki/index.php/PXELINUX#config

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=873406

Signed-off-by: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

---

Changes in v2:
- Add Reviewed-by tag from Daniel Kiper.

 grub-core/net/net.c     | 117 ++++++++++++++++++++++++++++++++++++++++
 grub-core/normal/main.c |  17 ++++--
 include/grub/net.h      |   3 ++
 3 files changed, 133 insertions(+), 4 deletions(-)

diff --git a/grub-core/net/net.c b/grub-core/net/net.c
index d5d726a315e..f066b02ebad 100644
--- a/grub-core/net/net.c
+++ b/grub-core/net/net.c
@@ -1735,6 +1735,123 @@ grub_net_restore_hw (void)
   return GRUB_ERR_NONE;
 }
 
+grub_err_t
+grub_net_search_config_file (char *config)
+{
+  grub_size_t config_len;
+  char *suffix;
+
+  auto int search_through (grub_size_t num_tries, grub_size_t slice_size);
+  int search_through (grub_size_t num_tries, grub_size_t slice_size)
+  {
+    while (num_tries-- > 0)
+      {
+        grub_dprintf ("net", "attempt to fetch config %s\n", config);
+
+        grub_file_t file;
+        file = grub_file_open (config, GRUB_FILE_TYPE_CONFIG);
+
+        if (file)
+          {
+            grub_file_close (file);
+            return 0;
+          }
+        else
+          {
+            if (grub_errno == GRUB_ERR_IO)
+              grub_errno = GRUB_ERR_NONE;
+          }
+
+        if (grub_strlen (suffix) < slice_size)
+          break;
+
+        config[grub_strlen (config) - slice_size] = '\0';
+      }
+
+    return 1;
+  }
+
+  config_len = grub_strlen (config);
+  config[config_len] = '-';
+  suffix = config + config_len + 1;
+
+  struct grub_net_network_level_interface *inf;
+  FOR_NET_NETWORK_LEVEL_INTERFACES (inf)
+    {
+      /* By the Client UUID. */
+
+      char client_uuid_var[sizeof ("net_") + grub_strlen (inf->name) +
+                           sizeof ("_clientuuid") + 1];
+      grub_snprintf (client_uuid_var, sizeof (client_uuid_var),
+                     "net_%s_clientuuid", inf->name);
+
+      const char *client_uuid;
+      client_uuid = grub_env_get (client_uuid_var);
+
+      if (client_uuid)
+        {
+          grub_strcpy (suffix, client_uuid);
+          if (search_through (1, 0) == 0) return GRUB_ERR_NONE;
+        }
+
+      /* By the MAC address. */
+
+      /* Add ethernet type */
+      grub_strcpy (suffix, "01-");
+
+      grub_net_hwaddr_to_str (&inf->hwaddress, suffix + 3);
+
+      char *ptr;
+      for (ptr = suffix; *ptr; ptr++)
+        if (*ptr == ':')
+          *ptr = '-';
+
+      if (search_through (1, 0) == 0) return GRUB_ERR_NONE;
+
+      /* By IP address */
+
+      switch ((&inf->address)->type)
+        {
+        case GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV4:
+          {
+            grub_uint32_t n = grub_be_to_cpu32 ((&inf->address)->ipv4);
+            grub_snprintf (suffix, GRUB_NET_MAX_STR_ADDR_LEN, "%02X%02X%02X%02X", \
+                           ((n >> 24) & 0xff), ((n >> 16) & 0xff),      \
+                           ((n >> 8) & 0xff), ((n >> 0) & 0xff));
+
+            if (search_through (8, 1) == 0) return GRUB_ERR_NONE;
+            break;
+          }
+        case GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6:
+          {
+            char buf[GRUB_NET_MAX_STR_ADDR_LEN];
+            struct grub_net_network_level_address base;
+            base.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
+            grub_memcpy (&base.ipv6, ((&inf->address)->ipv6), 16);
+            grub_net_addr_to_str (&base, buf);
+
+            for (ptr = buf; *ptr; ptr++)
+              if (*ptr == ':')
+                *ptr = '-';
+
+            grub_snprintf (suffix, GRUB_NET_MAX_STR_ADDR_LEN, "%s", buf);
+            if (search_through (1, 0) == 0) return GRUB_ERR_NONE;
+            break;
+          }
+        case GRUB_NET_NETWORK_LEVEL_PROTOCOL_DHCP_RECV:
+          return grub_error (GRUB_ERR_BUG, "shouldn't reach here");
+        default:
+          return grub_error (GRUB_ERR_BUG,
+                             "unsupported address type %d", (&inf->address)->type);
+        }
+    }
+
+  /* Remove the remaining minus sign at the end. */
+  config[config_len] = '\0';
+
+  return GRUB_ERR_NONE;
+}
+
 static struct grub_preboot *fini_hnd;
 
 static grub_command_t cmd_addaddr, cmd_deladdr, cmd_addroute, cmd_delroute;
diff --git a/grub-core/normal/main.c b/grub-core/normal/main.c
index 1b03dfd57b9..cdeb58f1a1e 100644
--- a/grub-core/normal/main.c
+++ b/grub-core/normal/main.c
@@ -323,10 +323,19 @@ grub_cmd_normal (struct grub_command *cmd __attribute__ ((unused)),
 
       prefix = grub_env_get ("prefix");
       if (prefix)
-	{
-	  config = grub_xasprintf ("%s/grub.cfg", prefix);
-	  if (! config)
-	    goto quit;
+        {
+          grub_size_t config_len;
+          config_len = grub_strlen (prefix) +
+                      sizeof ("/grub.cfg-XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX");
+          config = grub_malloc (config_len);
+
+          if (! config)
+            goto quit;
+
+          grub_snprintf (config, config_len, "%s/grub.cfg", prefix);
+
+          if (grub_strncmp (prefix + 1, "tftp", sizeof ("tftp") - 1) == 0)
+            grub_net_search_config_file (config);
 
 	  grub_enter_normal_mode (config);
 	  grub_free (config);
diff --git a/include/grub/net.h b/include/grub/net.h
index 556c54e579f..ff6d347f7da 100644
--- a/include/grub/net.h
+++ b/include/grub/net.h
@@ -578,4 +578,7 @@ extern char *grub_net_default_server;
 
 #define VLANTAG_IDENTIFIER 0x8100
 
+grub_err_t
+grub_net_search_configfile (char *config);
+
 #endif /* ! GRUB_NET_HEADER */
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/3] Set net_<interface>_client{id,uuid} variables from DHCP options
  2019-10-18  7:59 ` [PATCH v2 2/3] Set net_<interface>_client{id, uuid} variables from DHCP options Javier Martinez Canillas
@ 2019-10-21 14:40   ` Daniel Kiper
  2019-10-22  8:08     ` [PATCH v2 2/3] Set net_<interface>_client{id, uuid} " Javier Martinez Canillas
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Kiper @ 2019-10-21 14:40 UTC (permalink / raw)
  To: Javier Martinez Canillas; +Cc: grub-devel, Vladimir Serbinenko

On Fri, Oct 18, 2019 at 09:59:09AM +0200, Javier Martinez Canillas wrote:
> From: Paulo Flabiano Smorigo <pfsmorigo@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 <pfsmorigo@br.ibm.com>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

It seems to me that this RB should land in patch #3. Does not it?
Anyway, Reviewed-by: Daniel Kiper <daniel.kiper@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?

Daniel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/3] Set net_<interface>_client{id, uuid} variables from DHCP options
  2019-10-21 14:40   ` [PATCH v2 2/3] Set net_<interface>_client{id,uuid} " Daniel Kiper
@ 2019-10-22  8:08     ` Javier Martinez Canillas
  0 siblings, 0 replies; 6+ messages in thread
From: Javier Martinez Canillas @ 2019-10-22  8:08 UTC (permalink / raw)
  To: Daniel Kiper; +Cc: grub-devel, Vladimir Serbinenko

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 <pfsmorigo@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 <pfsmorigo@br.ibm.com>
>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
>> Reviewed-by: Daniel Kiper <daniel.kiper@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.kiper@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



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-10-22  8:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-18  7:59 [PATCH v2 0/3] Search for specific config files using UUID, MAC and IP Javier Martinez Canillas
2019-10-18  7:59 ` [PATCH v2 1/3] Add %X option to printf functions Javier Martinez Canillas
2019-10-18  7:59 ` [PATCH v2 2/3] Set net_<interface>_client{id, uuid} variables from DHCP options Javier Martinez Canillas
2019-10-21 14:40   ` [PATCH v2 2/3] Set net_<interface>_client{id,uuid} " Daniel Kiper
2019-10-22  8:08     ` [PATCH v2 2/3] Set net_<interface>_client{id, uuid} " Javier Martinez Canillas
2019-10-18  7:59 ` [PATCH v2 3/3] Search for specific config files for netboot Javier Martinez Canillas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.