grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH 2/2] efinet: fix lost packets due to active MNP instances


From: Andrei Borzenkov
Subject: [PATCH 2/2] efinet: fix lost packets due to active MNP instances
Date: Sun, 19 Apr 2015 11:01:11 +0300

EDK2 network stack is based on Managed Network Protocol which is layered
on top of Simple Management Protocol and does background polling. This
polling races with grub for received (and probably trasmitted) packets
which causes either serious slowdown or complete failure to load files.

Additionaly PXE driver creates two child devices - IPv4 and IPv6 - with
bound SNP instance as well. This means we get three cards for every
physical adapter when enumerating. Not only is this confusing, this
may result in grub ignoring packets that come in via the "wrong" card.

Example of device hierarchy is

 Ctrl[91] PciRoot(0x0)/Pci(0x3,0x0)
   Ctrl[95] PciRoot(0x0)/Pci(0x3,0x0)/MAC(525400123456,0x1)
     Ctrl[B4] PciRoot(0x0)/Pci(0x3,0x0)/MAC(525400123456,0x1)/IPv4(0.0.0.0)
     Ctrl[BC] 
PciRoot(0x0)/Pci(0x3,0x0)/MAC(525400123456,0x1)/IPv6(0000:0000:0000:0000:0000:0000:0000:0000)

Skip PXE created virtual devices and open SNP on base device exclusively.
This destroys all child MNP instances and stops background polling.  We
cannot do it for virtual devices anyway as PXE would probably attempt
to destroy them when stopped and current OVMF crashes when we try to do it.

Exclusive open cannot be done when enumerating cards, as it would destroy
PXE information we need to autoconfigure interface; and it cannot be done
during autoconfiguration as we need to do it for non-PXE boot as well. So
move SNP open to card ->open method and add matching ->close to clean up.

References: https://savannah.gnu.org/bugs/?41731
---
 grub-core/net/drivers/efi/efinet.c | 147 ++++++++++++++++++++++++++++---------
 1 file changed, 112 insertions(+), 35 deletions(-)

diff --git a/grub-core/net/drivers/efi/efinet.c 
b/grub-core/net/drivers/efi/efinet.c
index f171f20..5777016 100644
--- a/grub-core/net/drivers/efi/efinet.c
+++ b/grub-core/net/drivers/efi/efinet.c
@@ -142,9 +142,72 @@ get_card_packet (struct grub_net_card *dev)
   return nb;
 }
 
+static grub_err_t
+open_card (struct grub_net_card *dev)
+{
+  grub_efi_simple_network_t *net;
+
+  net = grub_efi_open_protocol (dev->efi_handle, &net_io_guid,
+                               GRUB_EFI_OPEN_PROTOCOL_BY_EXCLUSIVE);
+  if (! net)
+    /* This should not happen... Why?  */
+    return GRUB_ERR_BAD_DEVICE;
+
+  if (net->mode->state == GRUB_EFI_NETWORK_STOPPED
+      && efi_call_1 (net->start, net) != GRUB_EFI_SUCCESS)
+    return GRUB_ERR_BAD_DEVICE;
+
+  if (net->mode->state == GRUB_EFI_NETWORK_STOPPED)
+    return GRUB_ERR_BAD_DEVICE;
+
+  if (net->mode->state == GRUB_EFI_NETWORK_STARTED
+      && efi_call_3 (net->initialize, net, 0, 0) != GRUB_EFI_SUCCESS)
+    return GRUB_ERR_BAD_DEVICE;
+
+  dev->mtu = net->mode->max_packet_size;
+
+  dev->txbufsize = ALIGN_UP (dev->mtu, 64) + 256;
+  dev->txbuf = grub_zalloc (dev->txbufsize);
+  if (!dev->txbuf)
+    {
+      grub_print_error ();
+      return GRUB_ERR_BAD_DEVICE;
+    }
+  dev->txbusy = 0;
+
+  dev->rcvbufsize = ALIGN_UP (dev->mtu, 64) + 256;
+
+  grub_memcpy (dev->default_address.mac,
+              net->mode->current_address,
+              sizeof (dev->default_address.mac));
+
+  dev->efi_net = net;
+  return GRUB_ERR_NONE;
+}
+
+static void
+close_card (struct grub_net_card *dev)
+{
+  if (dev->txbuf)
+    {
+      grub_free (dev->txbuf);
+      dev->txbuf = NULL;
+    }
+
+  if (dev->rcvbuf)
+    {
+      grub_free (dev->rcvbuf);
+      dev->rcvbuf = NULL;
+    }
+
+  /* FIXME do we need to close Simple Network Protocol here? */
+}
+
 static struct grub_net_card_driver efidriver =
   {
     .name = "efinet",
+    .open = open_card,
+    .close = close_card,
     .send = send_card_buffer,
     .recv = get_card_packet
   };
@@ -172,24 +235,29 @@ grub_efinet_findcards (void)
     return;
   for (handle = handles; num_handles--; handle++)
     {
-      grub_efi_simple_network_t *net;
       struct grub_net_card *card;
-
-      net = grub_efi_open_protocol (*handle, &net_io_guid,
-                                   GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL);
-      if (! net)
-       /* This should not happen... Why?  */
+      grub_efi_device_path_t *dp, *parent = NULL, *child = NULL;
+
+      /* EDK2 UEFI PXE drivers creates IPv4 and IPv6 messaging devices as
+        children of main MAC messaging device. We only need one device with
+        bound SNP per physical card, otherwise they compete with each other
+        when polling for incoming packets.
+       */
+      dp = grub_efi_get_device_path (*handle);
+      if (!dp)
        continue;
-
-      if (net->mode->state == GRUB_EFI_NETWORK_STOPPED
-         && efi_call_1 (net->start, net) != GRUB_EFI_SUCCESS)
-       continue;
-
-      if (net->mode->state == GRUB_EFI_NETWORK_STOPPED)
-       continue;
-
-      if (net->mode->state == GRUB_EFI_NETWORK_STARTED
-         && efi_call_3 (net->initialize, net, 0, 0) != GRUB_EFI_SUCCESS)
+      for (; ! GRUB_EFI_END_ENTIRE_DEVICE_PATH (dp); dp = 
GRUB_EFI_NEXT_DEVICE_PATH (dp))
+       {
+         parent = child;
+         child = dp;
+       }
+      if (child
+         && GRUB_EFI_DEVICE_PATH_TYPE (child) == 
GRUB_EFI_MESSAGING_DEVICE_PATH_TYPE
+         && (GRUB_EFI_DEVICE_PATH_SUBTYPE (child) == 
GRUB_EFI_IPV4_DEVICE_PATH_SUBTYPE
+             || GRUB_EFI_DEVICE_PATH_SUBTYPE (child) == 
GRUB_EFI_IPV6_DEVICE_PATH_SUBTYPE)
+         && parent
+         && GRUB_EFI_DEVICE_PATH_TYPE (parent) == 
GRUB_EFI_MESSAGING_DEVICE_PATH_TYPE
+         && GRUB_EFI_DEVICE_PATH_SUBTYPE (parent) == 
GRUB_EFI_MAC_ADDRESS_DEVICE_PATH_SUBTYPE)
        continue;
 
       card = grub_zalloc (sizeof (struct grub_net_card));
@@ -200,28 +268,11 @@ grub_efinet_findcards (void)
          return;
        }
 
-      card->mtu = net->mode->max_packet_size;
-      card->txbufsize = ALIGN_UP (card->mtu, 64) + 256;
-      card->txbuf = grub_zalloc (card->txbufsize);
-      if (!card->txbuf)
-       {
-         grub_print_error ();
-         grub_free (handles);
-         grub_free (card);
-         return;
-       }
-      card->txbusy = 0;
-
-      card->rcvbufsize = ALIGN_UP (card->mtu, 64) + 256;
-
       card->name = grub_xasprintf ("efinet%d", i++);
       card->driver = &efidriver;
       card->flags = 0;
       card->default_address.type = GRUB_NET_LINK_LEVEL_PROTOCOL_ETHERNET;
-      grub_memcpy (card->default_address.mac,
-                  net->mode->current_address,
-                  sizeof (card->default_address.mac));
-      card->efi_net = net;
+      card->efi_net = NULL;
       card->efi_handle = *handle;
 
       grub_net_card_register (card);
@@ -251,7 +302,33 @@ grub_efi_net_config_real (grub_efi_handle_t hnd, char 
**device,
     if (! cdp)
       continue;
     if (grub_efi_compare_device_paths (dp, cdp) != 0)
-      continue;
+      {
+       grub_efi_device_path_t *ldp, *dup_dp, *dup_ldp;
+       int match;
+
+       /* EDK2 PXE implementation creates pseudo devices with type IPv4/IPv6
+          as children of Ethernet card and binds PXE and Load File protocols
+          to it. Loaded Image Device Path protocol will point to these pseudo
+          devices. We skip them when enumerating cards, so here we need to
+          find matching MAC device.
+         */
+       ldp = grub_efi_find_last_device_path (dp);
+       if (GRUB_EFI_DEVICE_PATH_TYPE (ldp) != 
GRUB_EFI_MESSAGING_DEVICE_PATH_TYPE
+           || (GRUB_EFI_DEVICE_PATH_SUBTYPE (ldp) != 
GRUB_EFI_IPV4_DEVICE_PATH_SUBTYPE
+               && GRUB_EFI_DEVICE_PATH_SUBTYPE (ldp) != 
GRUB_EFI_IPV6_DEVICE_PATH_SUBTYPE))
+         continue;
+       dup_dp = grub_efi_duplicate_device_path (dp);
+       if (!dup_dp)
+         continue;
+       dup_ldp = grub_efi_find_last_device_path (dup_dp);
+       dup_ldp->type = GRUB_EFI_END_DEVICE_PATH_TYPE;
+       dup_ldp->subtype = GRUB_EFI_END_ENTIRE_DEVICE_PATH_SUBTYPE;
+       dup_ldp->length = sizeof (*dup_ldp);
+       match = grub_efi_compare_device_paths (dup_dp, cdp) == 0;
+       grub_free (dup_dp);
+       if (!match)
+         continue;
+      }
     pxe = grub_efi_open_protocol (hnd, &pxe_io_guid,
                                  GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL);
     if (! pxe)
-- 
2.1.4




reply via email to

[Prev in Thread] Current Thread [Next in Thread]