Bug 873406 - Add ability to autoload grub2's config file when netbooting
Summary: Add ability to autoload grub2's config file when netbooting
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: grub2
Version: 18
Hardware: ppc64
OS: All
unspecified
urgent
Target Milestone: ---
Assignee: Peter Jones
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-11-05 19:06 UTC by IBM Bug Proxy
Modified: 2017-08-31 09:27 UTC (History)
10 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2012-12-06 07:25:40 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
[PATCH 1/3] Add %X option to printf functions. (1.64 KB, patch)
2012-11-27 19:45 UTC, Paulo Flabiano Smorigo
no flags Details | Diff
[PATCH 2/3] DHCP client ID and UUID options added. (3.47 KB, patch)
2012-11-27 19:46 UTC, Paulo Flabiano Smorigo
no flags Details | Diff
[PATCH 3/3] Search for specific config file for netboot (6.22 KB, patch)
2012-11-27 19:46 UTC, Paulo Flabiano Smorigo
no flags Details | Diff


Links
System ID Private Priority Status Summary Last Updated
IBM Linux Technology Center 86537 0 None None None 2019-06-10 16:47:59 UTC

Description IBM Bug Proxy 2012-11-05 19:06:43 UTC
== Comment: #0 - Brent J. Baude <baude.com> - 2012-11-05 13:40:08 ==
+++ This bug was initially created as a clone of Bug #80686 +++

Similar to yaboot and pxelinux, grub2 network boot needs to auto load a config file. A few things that we need to fix:

1. net_bootp must default to the network we loaded grub2 off. RIght now it seems to default to the last network card. We need to use the info in boot-device to find the correct interface.

2. We need to load a machine specific config file over tftp. The algorithm used by both yaboot and pxelinux can be found at:

http://syslinux.zytor.com/pxe.php#config

Comment 1 IBM Bug Proxy 2012-11-05 20:00:35 UTC
------- Comment From baude.com 2012-11-05 19:53 EDT-------
Original response from Paulo below:

I made a patch to do the pxe config. The procedure looks for the following files, just like pxe/yaboot do:

(tftp,192.168.1.1)//boot/grub/grub.cfg-01-e4-1f-13-f4-ad-c2
(tftp,192.168.1.1)//boot/grub/grub.cfg-C0A80164
(tftp,192.168.1.1)//boot/grub/grub.cfg-C0A8016
(tftp,192.168.1.1)//boot/grub/grub.cfg-C0A801
(tftp,192.168.1.1)//boot/grub/grub.cfg-C0A80
(tftp,192.168.1.1)//boot/grub/grub.cfg-C0A8
(tftp,192.168.1.1)//boot/grub/grub.cfg-C0A
(tftp,192.168.1.1)//boot/grub/grub.cfg-C0
(tftp,192.168.1.1)//boot/grub/grub.cfg-C
(tftp,192.168.1.1)//boot/grub/grub.cfg

After talking with GRUB2 maintainer about it, he suggested to put a script inside grub.cfg to do this job. I made one with the content:

{{{
configfile=$prefix/grub.cfg-$net_ofnet_network_mac
source "$configfile"

ip=$net_ofnet_network_ip

for c in 1 2 3 4; do
configfile=$prefix/grub.cfg-$ip
source "$configfile"
regexp --set=ip '^(.*)\..+$' "$ip"
done
}}}

It searches:
(tftp,9.8.234.191)//boot/grub/grub.cfg-e4:1f:13:f4:ad:c2
(tftp,9.8.234.191)//boot/grub/grub.cfg-9.8.234.243
(tftp,9.8.234.191)//boot/grub/grub.cfg-9.8.234
(tftp,9.8.234.191)//boot/grub/grub.cfg-9.8
(tftp,9.8.234.191)//boot/grub/grub.cfg-9

Do you think this approach is good?
What do you think about use IP as decimal?

Comment 2 Paulo Flabiano Smorigo 2012-11-27 19:45:02 UTC
Created attachment 653057 [details]
[PATCH 1/3]  Add %X option to printf functions.

Comment 3 Paulo Flabiano Smorigo 2012-11-27 19:46:04 UTC
Created attachment 653058 [details]
[PATCH 2/3] DHCP client ID and UUID options added.

Comment 4 Paulo Flabiano Smorigo 2012-11-27 19:46:44 UTC
Created attachment 653059 [details]
[PATCH 3/3] Search for specific config file for netboot

Comment 5 Paulo Flabiano Smorigo 2012-11-27 19:49:07 UTC
Those patches implement a search for a specific configuration when the config
file is on a remote server. 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

Comment 6 Fedora Update System 2012-11-27 20:36:06 UTC
grub2-2.00-13.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/grub2-2.00-13.fc18

Comment 7 Fedora Update System 2012-11-28 02:12:14 UTC
Package grub2-2.00-13.fc18:
* should fix your issue,
* was pushed to the Fedora 18 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing grub2-2.00-13.fc18'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2012-19181/grub2-2.00-13.fc18
then log in and leave karma (feedback).

Comment 8 Fedora Update System 2012-12-06 07:25:42 UTC
grub2-2.00-13.fc18 has been pushed to the Fedora 18 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 9 Lukas Zapletal 2017-08-30 09:53:56 UTC
Apologize to comment on such an old BZ, but we see weird behavior in Grub2 (grub2-2.02-0.64.el7.x86_64). For some reason it tries to load a file with dash character in the end:

/grub2/grub.cfg-01-52-54-00-ac-d2-3d-

which apparently does not exist in our case. Do you have an idea what could cause this behavior?

 in.tftpd[135674]: RRQ from 192.168.100.15 filename grub2/shim.efi
 in.tftpd[135674]: tftp: client does not accept options
 in.tftpd[135675]: RRQ from 192.168.100.15 filename grub2/shim.efi
 in.tftpd[135675]: Client 192.168.100.15 finished grub2/shim.efi
 in.tftpd[135676]: RRQ from 192.168.100.15 filename grub2/grubx64.efi
 in.tftpd[135676]: Client 192.168.100.15 finished grub2/grubx64.efi
 in.tftpd[135677]: RRQ from 192.168.100.15 filename /grub2/grub.cfg-01-52-54-00-ac-d2-3d-
 in.tftpd[135677]: Client 192.168.100.15 File not found /grub2/grub.cfg-01-52-54-00-ac-d2-3d-
 in.tftpd[135678]: RRQ from 192.168.100.15 filename /grub2/grub.cfg-C0A8640F
 in.tftpd[135678]: Client 192.168.100.15 File not found /grub2/grub.cfg-C0A8640F
 in.tftpd[135679]: RRQ from 192.168.100.15 filename /grub2/grub.cfg-C0A8640
 in.tftpd[135679]: Client 192.168.100.15 File not found /grub2/grub.cfg-C0A8640
 in.tftpd[135680]: RRQ from 192.168.100.15 filename /grub2/grub.cfg-C0A864
 in.tftpd[135680]: Client 192.168.100.15 File not found /grub2/grub.cfg-C0A864
 in.tftpd[135681]: RRQ from 192.168.100.15 filename /grub2/grub.cfg-C0A86
 in.tftpd[135681]: Client 192.168.100.15 File not found /grub2/grub.cfg-C0A86
 in.tftpd[135682]: RRQ from 192.168.100.15 filename /grub2/grub.cfg-C0A8
 in.tftpd[135682]: Client 192.168.100.15 File not found /grub2/grub.cfg-C0A8
 in.tftpd[135683]: RRQ from 192.168.100.15 filename /grub2/grub.cfg-C0A
 in.tftpd[135683]: Client 192.168.100.15 File not found /grub2/grub.cfg-C0A
 in.tftpd[135684]: RRQ from 192.168.100.15 filename /grub2/grub.cfg-C0
 in.tftpd[135684]: Client 192.168.100.15 File not found /grub2/grub.cfg-C0
 in.tftpd[135685]: RRQ from 192.168.100.15 filename /grub2/grub.cfg-C
 in.tftpd[135685]: Client 192.168.100.15 File not found /grub2/grub.cfg-C
 in.tftpd[135686]: RRQ from 192.168.100.15 filename /grub2/grub.cfg
 in.tftpd[135686]: Client 192.168.100.15 finished /grub2/grub.cfg
 in.tftpd[135687]: RRQ from 192.168.100.15 filename /EFI/redhat/x86_64-efi/command.lst
 in.tftpd[135687]: Client 192.168.100.15 File not found /EFI/redhat/x86_64-efi/command.lst
 in.tftpd[135688]: RRQ from 192.168.100.15 filename /EFI/redhat/x86_64-efi/fs.lst
 in.tftpd[135688]: Client 192.168.100.15 File not found /EFI/redhat/x86_64-efi/fs.lst
 in.tftpd[135689]: RRQ from 192.168.100.15 filename /EFI/redhat/x86_64-efi/crypto.lst
 in.tftpd[135689]: Client 192.168.100.15 File not found /EFI/redhat/x86_64-efi/crypto.lst
 in.tftpd[135690]: RRQ from 192.168.100.15 filename /EFI/redhat/x86_64-efi/terminal.lst
 in.tftpd[135690]: Client 192.168.100.15 File not found /EFI/redhat/x86_64-efi/terminal.lst
 in.tftpd[135691]: RRQ from 192.168.100.15 filename /grub2/grub.cfg
 in.tftpd[135691]: Client 192.168.100.15 finished /grub2/grub.cfg

Comment 10 Lukas Zapletal 2017-08-30 10:11:47 UTC
Looking into codebase, I don't really see anything relevant there:

+      /* 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;


void
grub_net_hwaddr_to_str (const grub_net_link_level_address_t *addr, char *str)
{
  str[0] = 0;
  switch (addr->type)
    {
    case GRUB_NET_LINK_LEVEL_PROTOCOL_ETHERNET:
      {
  char *ptr;
  unsigned i;
  for (ptr = str, i = 0; i < ARRAY_SIZE (addr->mac); i++)
    {
      grub_snprintf (ptr, GRUB_NET_MAX_STR_HWADDR_LEN - (ptr - str),
         "%02x:", addr->mac[i] & 0xff);
      ptr += (sizeof ("XX:") - 1);
    }
      return;
      }
    }
  grub_printf (_("Unsupported hw address type %d\n"), addr->type);
}

This is a libvirt VM booting in EFI via edk2.git-ovmf-x64-0-20170829.b2915.g1696b221b1.noarch is it possible that the firmware would return ":" or "-" in the end?

Comment 11 Lukas Zapletal 2017-08-30 11:49:58 UTC
Additional observation, when I print $net_default_mac I see

52:54:00:a3:87:02:

which confirms my thought this is a bug in OVMF. Sorry for the noise here.

Comment 12 Laszlo Ersek 2017-08-30 15:02:08 UTC
Lukas,

the code that you are *looking* at is correct, but the code that you are *running* is not correct. That's because the symptom you are reporting does not match the code you quoted.

This is the culprit grub2 patch (which you are running, but not looking at):

commit [irrelevant]
Author: Andrzej Kacprowski <andrzej.kacprowski>
Date:   Fri Apr 21 10:06:20 2017 +0200

    Add support for non-Ethernet network cards
    
    This patch replaces fixed 6-byte link layer address with
    up to 32-byte variable sized address.
    This allows supporting Infiniband and Omni-Path fabric
    which use 20-byte address, but other network card types
    can also take advantage of this change.
    The network card driver is responsible for replacing L2
    header provided by grub2 if needed.
    This approach is compatible with UEFI network stack which
    also allows up to 32-byte variable size link address.
    
    The BOOTP/DHCP packet format is limited to 16 byte client
    hardware address, if link address is more that 16-bytes
    then chaddr field in BOOTP it will be set to 0 as per rfc4390.
    
    Resolves: rhbz#1370642
    
    Signed-off-by: Andrzej Kacprowski <andrzej.kacprowski>
    
    Conflicts:
            grub-core/net/ip.c

Namely, *before* applying this patch, the grub2 code had indeed matched what you quote above:

- "include/grub/net.h":

#define GRUB_NET_MAX_STR_HWADDR_LEN (sizeof ("XX:XX:XX:XX:XX:XX"))

- "grub-core/net/net.c", function grub_net_hwaddr_to_str():

        for (ptr = str, i = 0; i < ARRAY_SIZE (addr->mac); i++)
          {
            grub_snprintf (ptr, GRUB_NET_MAX_STR_HWADDR_LEN - (ptr - str),
                           "%02x:", addr->mac[i] & 0xff);
            ptr += (sizeof ("XX:") - 1);
          }

This loop
- formats every byte of the MAC addres with the format string "%02x:",
- and it relies on the GRUB_NET_MAX_STR_HWADDR_LEN macro to *prevent* the
  grub_snprintf() function from formatting the trailing colon (":") for the
  6th MAC address byte.

*After* the patch, the limit was raised like this (room for 32 octets):

-#define GRUB_NET_MAX_STR_HWADDR_LEN (sizeof ("XX:XX:XX:XX:XX:XX"))
+#define GRUB_NET_MAX_STR_HWADDR_LEN (sizeof (\
+       "XX:XX:XX:XX:XX:XX:XX:XX:"\
+       "XX:XX:XX:XX:XX:XX:XX:XX:"\
+       "XX:XX:XX:XX:XX:XX:XX:XX:"\
+       "XX:XX:XX:XX:XX:XX:XX:XX"))

And the loop was changed like this:

-    case GRUB_NET_LINK_LEVEL_PROTOCOL_ETHERNET:
-      {
-       char *ptr;
-       unsigned i;
-       for (ptr = str, i = 0; i < ARRAY_SIZE (addr->mac); i++)
-         {
-           grub_snprintf (ptr, GRUB_NET_MAX_STR_HWADDR_LEN - (ptr - str),
-                          "%02x:", addr->mac[i] & 0xff);
-           ptr += (sizeof ("XX:") - 1);
-         }
-      return;
-      }
+       str[0] = 0;
+       grub_printf (_("Unsupported hw address type %d len %d\n"),
+                   addr->type, addr->len);
+       return;
+    }
+  for (ptr = str, i = 0; i < addr->len; i++)
+    {
+      ptr += grub_snprintf (ptr, GRUB_NET_MAX_STR_HWADDR_LEN - (ptr - str),
+                    "%02x:", addr->mac[i] & 0xff);

The loop change is actually irrelevant; it preserved the same logic. The macro change is important however.

Namely, due to the new definition of GRUB_NET_MAX_STR_HWADDR_LEN, if you now format a MAC address that has strictly less than 32 octets, you will end up with a trailing colon (":"), simply because now the grub_snprintf() function has *room* for that colon. As I wrote above, the loop (both pre- and post-patch) relies on running out of room for *not* producing the last colon.

In brief, this regression was introduced in the fix for RHBZ#1370642.

Comment 13 Lukas Zapletal 2017-08-31 09:27:03 UTC
Thank you very much, for the record I filed new bug at https://bugzilla.redhat.com/show_bug.cgi?id=1487107


Note You need to log in before you can comment on or make changes to this bug.