[PATCH 0/3] fix ipv6 support

[PATCH 0/3] fix ipv6 support

[Josef Bacik]

These patches are on top of Michael Chang's bootp6 code (which we also really
need so it would be great if those could go in as well).  At Facebook we have
ipv6 only clusters that we need to be able to provision over the network.  The
current grub2 support for ipv6 is broken in a few ways.  The routing stuff
needed to be reworked to handle talking to an ipv6 router properly.  I tried to
not change the code too much in order to make sure I didn't regress the ipv4
support.

There is also a patch for the efinet driver, without it we are unable to do any
networking with our UEFI boxes.

I'd appreciate reviews on these patches, this is my first foray into grub2.
Facebook is going to be replacing ipxe with grub2 and so we are dedicated to
getting grub2 into shape in order to be used to do network based provisioning
well.  Thanks,

Josef

[PATCH 1/3] efinet: handle get_status() properly

[Josef Bacik]

The EFI SNP documentation isn't super clear on the value that is returned in
txbuf when calling into GetStatus.  The documentation says its the pointer to
the recycle buffer, but the documentation for Transmit() says that it should be
the pointer to the buffer that we transmitted.  On the boxes I'm using it's just
a random pointer (usually 0x1).  It is definitely transmitting stuff, but the
get_status call is not returning the pointer to the txbuf we passed in.  Looking
at a few EFI implementations and other SNP drivers it seems like there is
confusion everywhere on this.  So since we only transmit one buffer at a time,
just assume that a non-NULL txbuf means that our transmit happened properly.
With this patch I can now do networking on our EFI enabled boxes.  Thanks,

cc: Peter Jones <pjones@redhat.com>
Signed-off-by: Josef Bacik <jbacik@fb.com>
---
 grub-core/net/drivers/efi/efinet.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/grub-core/net/drivers/efi/efinet.c b/grub-core/net/drivers/efi/efinet.c
index f27a117..4d3f8aa 100644
--- a/grub-core/net/drivers/efi/efinet.c
+++ b/grub-core/net/drivers/efi/efinet.c
@@ -47,19 +47,11 @@ send_card_buffer (struct grub_net_card *dev,
 	if (st != GRUB_EFI_SUCCESS)
 	  return grub_error (GRUB_ERR_IO,
 			     N_("couldn't send network packet"));
-	if (txbuf == dev->txbuf)
+	if (txbuf)
 	  {
 	    dev->txbusy = 0;
 	    break;
 	  }
-	if (txbuf)
-	  {
-	    st = efi_call_7 (net->transmit, net, 0, dev->last_pkt_size,
-			     dev->txbuf, NULL, NULL, NULL);
-	    if (st != GRUB_EFI_SUCCESS)
-	      return grub_error (GRUB_ERR_IO,
-				 N_("couldn't send network packet"));
-	  }
 	if (limit_time < grub_get_time_ms ())
 	  return grub_error (GRUB_ERR_TIMEOUT,
 			     N_("couldn't send network packet"));
@@ -84,8 +76,9 @@ send_card_buffer (struct grub_net_card *dev,
      we run in the GRUB_ERR_TIMEOUT case above.
      Perhaps a timeout in the FW has discarded the recycle buffer.
    */
+  txbuf = NULL;
   st = efi_call_3 (net->get_status, net, 0, &txbuf);
-  dev->txbusy = !(st == GRUB_EFI_SUCCESS && txbuf == dev->txbuf);
+  dev->txbusy = !(st == GRUB_EFI_SUCCESS && txbuf);
 
   return GRUB_ERR_NONE;
 }
-- 
1.8.5.6

[PATCH 2/3] net: add local route when creating link local ipv6 interface

[Josef Bacik]

In order to talk to link local ipv6 addresses we need to have a route out of the
link local interface for this to work.  Thanks,

Signed-off-by: Josef Bacik <jbacik@fb.com>
---
 grub-core/net/net.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/grub-core/net/net.c b/grub-core/net/net.c
index 21a4e94..f96297a 100644
--- a/grub-core/net/net.c
+++ b/grub-core/net/net.c
@@ -297,7 +297,9 @@ grub_net_ipv6_get_link_local (struct grub_net_card *card,
   struct grub_net_network_level_interface *inf;
   char *name;
   char *ptr;
+  grub_err_t err;
   grub_net_network_level_address_t addr;
+  grub_net_network_level_netaddress_t netaddr;
 
   name = grub_malloc (grub_strlen (card->name)
 		      + GRUB_NET_MAX_STR_HWADDR_LEN
@@ -325,7 +327,17 @@ grub_net_ipv6_get_link_local (struct grub_net_card *card,
       ptr += grub_strlen (ptr);
     }
   ptr = grub_stpcpy (ptr, ":link");
-  return grub_net_add_addr_real (name, card, &addr, hwaddr, 0);
+  inf = grub_net_add_addr_real (name, card, &addr, hwaddr, 0);
+  if (!inf)
+    return NULL;
+  netaddr.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
+  netaddr.ipv6.base[0] = grub_cpu_to_be64_compile_time (0xfe80ULL << 48);
+  netaddr.ipv6.base[1] = 0;
+  netaddr.ipv6.masksize = 64;
+  err = grub_net_add_route (name, netaddr, inf);
+  if (err != GRUB_ERR_NONE)
+    return NULL;
+  return inf;
 }
 
 /* FIXME: allow to specify mac address.  */
-- 
1.8.5.6

[PATCH 3/3] net: fix ipv6 routing

[Josef Bacik]

Currently we cannot talk to ipv6 addresses outside of our local link area
because we don't setup our routes properly nor do we talk to the router
properly.  Currently net_ipv6_autoconf adds routes for the local link prefix and
ties it to the global link device.  This isn't helpful at all, we completely
drop the router part of the router advertisement.  So instead create a "default
route" flag in routes and create a new route with the local link prefix and the
router source address.

The second part of this problem is that the routing code thinks in ipv4 terms,
so it expects to find a route to the address we're looking for in our routing
table.  If we find a gateway then we just look for which interface matches the
network for the gateway.  This doesn't work for ipv6 since the router gives us
its local link address, so the gateway will find the local link interface,
instead of the global interface.  So to deal with this do a few things

1) Create a new "default route" flag for any router advertisements we get when
doing the ipv6 autoconf stuff.  Then we create a normal gateway route with the
default route flag set.

2) When looking for routes keep track of any default routes we find, and if we
don't find an exact route that matches we use the default route, which will set
the gateway to the router, and then we will find the local link interface that
matches this router.

3) Translate the local link interface to the global link interface.  We build
the ip packets based on the address on the interface, and this needs to have the
global address, not the local one.  So loop through our interfaces until we find
a global address with the same hwaddress as the local link interface and use
that instead.  This way we get the right addresses on our IP packets.

With this patch I can now do net_ipv6_autoconf and immediately talk to ipv6
addresses outside of my local network.  Thanks,

Signed-off-by: Josef Bacik <jbacik@fb.com>
---
 grub-core/net/bootp.c                  |  2 +-
 grub-core/net/drivers/ieee1275/ofnet.c |  2 +-
 grub-core/net/icmp6.c                  | 50 ++++++++++------------
 grub-core/net/net.c                    | 76 +++++++++++++++++++++-------------
 include/grub/net.h                     | 28 ++++++++++++-
 5 files changed, 99 insertions(+), 59 deletions(-)

diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
index 37d1cfa..ed6cf44 100644
--- a/grub-core/net/bootp.c
+++ b/grub-core/net/bootp.c
@@ -83,7 +83,7 @@ parse_dhcp_vendor (const char *name, const void *vend, int limit, int *mask)
 	      grub_memcpy (&gw.ipv4, ptr, sizeof (gw.ipv4));
 	      rname = grub_xasprintf ("%s:default", name);
 	      if (rname)
-		grub_net_add_route_gw (rname, target, gw);
+		grub_net_add_route_gw (rname, target, gw, 0);
 	      grub_free (rname);
 	    }
 	  break;
diff --git a/grub-core/net/drivers/ieee1275/ofnet.c b/grub-core/net/drivers/ieee1275/ofnet.c
index eea8e71..801ab1c 100644
--- a/grub-core/net/drivers/ieee1275/ofnet.c
+++ b/grub-core/net/drivers/ieee1275/ofnet.c
@@ -221,7 +221,7 @@ grub_ieee1275_parse_bootpath (const char *devpath, char *bootpath,
       target.ipv4.masksize = 0;
       rname = grub_xasprintf ("%s:default", ((*card)->name));
       if (rname)
-        grub_net_add_route_gw (rname, target, gateway_addr);
+        grub_net_add_route_gw (rname, target, gateway_addr, 0);
       else
         return grub_errno;
     }
diff --git a/grub-core/net/icmp6.c b/grub-core/net/icmp6.c
index 7953e68..fad04a9 100644
--- a/grub-core/net/icmp6.c
+++ b/grub-core/net/icmp6.c
@@ -373,7 +373,10 @@ grub_net_recv_icmp6_packet (struct grub_net_buff *nb,
 	    if (ohdr->type == OPTION_PREFIX && ohdr->len == 4)
 	      {
 		struct prefix_option *opt = (struct prefix_option *) ptr;
+		struct grub_net_route *route;
 		struct grub_net_slaac_mac_list *slaac;
+
+		grub_net_network_level_netaddress_t netaddr;
 		if (!(opt->flags & FLAG_SLAAC)
 		    || (grub_be_to_cpu64 (opt->prefix[0]) >> 48) == 0xfe80
 		    || (grub_be_to_cpu32 (opt->preferred_lifetime)
@@ -391,18 +394,12 @@ grub_net_recv_icmp6_packet (struct grub_net_buff *nb,
 		for (slaac = card->slaac_list; slaac; slaac = slaac->next)
 		  {
 		    grub_net_network_level_address_t addr;
-		    grub_net_network_level_netaddress_t netaddr;
-
 		    if (slaac->address.type
 			!= GRUB_NET_LINK_LEVEL_PROTOCOL_ETHERNET)
 		      continue;
 		    addr.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
 		    addr.ipv6[0] = opt->prefix[0];
 		    addr.ipv6[1] = grub_net_ipv6_get_id (&slaac->address);
-		    netaddr.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
-		    netaddr.ipv6.base[0] = opt->prefix[0];
-		    netaddr.ipv6.base[1] = 0;
-		    netaddr.ipv6.masksize = 64;
 
 		    FOR_NET_NETWORK_LEVEL_INTERFACES (inf)
 		    {
@@ -410,29 +407,28 @@ grub_net_recv_icmp6_packet (struct grub_net_buff *nb,
 			  && grub_net_addr_cmp (&inf->address, &addr) == 0)
 			break;
 		    }
-		    /* Update lease time if needed here once we have
-		       lease times.  */
-		    if (inf)
-		      continue;
+		    if (!inf)
+		      slaac->slaac_counter++;
+		  }
 
-		    grub_dprintf ("net", "creating slaac\n");
+		netaddr.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
+		netaddr.ipv6.base[0] = opt->prefix[0];
+		netaddr.ipv6.base[1] = 0;
+		netaddr.ipv6.masksize = 64;
 
-		    {
-		      char *name;
-		      name = grub_xasprintf ("%s:%d",
-					     slaac->name, slaac->slaac_counter++);
-		      if (!name)
-			{
-			  grub_errno = GRUB_ERR_NONE;
-			  continue;
-			}
-		      inf = grub_net_add_addr (name, 
-					       card, &addr,
-					       &slaac->address, 0);
-		      grub_net_add_route (name, netaddr, inf);
-		      grub_free (name);
-		    }
-		  }
+		FOR_NET_ROUTES(route)
+		{
+		  if (!grub_memcmp(&route->gw, source, sizeof (route->gw)))
+		    break;
+		}
+		/* Update lease time if needed here once we have
+		   lease times.  */
+		if (route)
+		  continue;
+
+		grub_dprintf ("net", "creating default route\n");
+
+		grub_net_add_route_gw ("default6", netaddr, *source, 1);
 	      }
 	  }
 	if (ptr != nb->tail)
diff --git a/grub-core/net/net.c b/grub-core/net/net.c
index f96297a..185b635 100644
--- a/grub-core/net/net.c
+++ b/grub-core/net/net.c
@@ -37,21 +37,6 @@ GRUB_MOD_LICENSE ("GPLv3+");
 
 char *grub_net_default_server;
 
-struct grub_net_route
-{
-  struct grub_net_route *next;
-  struct grub_net_route **prev;
-  grub_net_network_level_netaddress_t target;
-  char *name;
-  struct grub_net_network_level_protocol *prot;
-  int is_gateway;
-  union
-  {
-    struct grub_net_network_level_interface *interface;
-    grub_net_network_level_address_t gw;
-  };
-};
-
 struct grub_net_route *grub_net_routes = NULL;
 struct grub_net_network_level_interface *grub_net_network_level_interfaces = NULL;
 struct grub_net_card *grub_net_cards = NULL;
@@ -422,14 +407,6 @@ grub_cmd_ipv6_autoconf (struct grub_command *cmd __attribute__ ((unused)),
   return err;
 }
 
-static inline void
-grub_net_route_register (struct grub_net_route *route)
-{
-  grub_list_push (GRUB_AS_LIST_P (&grub_net_routes),
-		  GRUB_AS_LIST (route));
-}
-
-#define FOR_NET_ROUTES(var) for (var = grub_net_routes; var; var = var->next)
 
 static int
 parse_ip (const char *val, grub_uint32_t *ip, const char **rest)
@@ -650,6 +627,11 @@ route_cmp (const struct grub_net_route *a, const struct grub_net_route *b)
 	return +1;
       if (a->target.ipv6.masksize < b->target.ipv6.masksize)
 	return -1;
+      /* We want to prefer non-default routes over default routes */
+      if (a->default_route > b->default_route)
+	return -1;
+      if (a->default_route < b->default_route)
+	return +1;
       break;
     case GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV4:
       if (a->target.ipv4.masksize > b->target.ipv4.masksize)
@@ -661,6 +643,32 @@ route_cmp (const struct grub_net_route *a, const struct grub_net_route *b)
   return 0;
 }
 
+static struct grub_net_network_level_interface *
+grub_ipv6_global_interface (struct grub_net_network_level_interface *inf)
+{
+  grub_net_link_level_address_t hwaddr;
+  struct grub_net_network_level_interface *tmp;
+
+  /* We don't care about non ipv6 interfaces */
+  if (inf->address.type != GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6)
+    return inf;
+
+  /* If this isn't a link local interface then we're good to go */
+  if (inf->address.ipv6[0] != grub_cpu_to_be64_compile_time (0xfe80ULL << 48))
+    return inf;
+
+  grub_memcpy(&hwaddr, &inf->hwaddress, sizeof (inf->hwaddress));
+
+  FOR_NET_NETWORK_LEVEL_INTERFACES (tmp)
+  {
+    if (tmp->address.ipv6[0] == grub_cpu_to_be64_compile_time (0xfe80ULL << 48))
+      continue;
+    if (grub_net_hwaddr_cmp(&tmp->hwaddress, &hwaddr) == 0)
+      return tmp;
+  }
+  return inf;
+}
+
 grub_err_t
 grub_net_route_address (grub_net_network_level_address_t addr,
 			grub_net_network_level_address_t *gateway,
@@ -680,22 +688,29 @@ grub_net_route_address (grub_net_network_level_address_t addr,
   for (depth = 0; depth < routecnt + 2 && depth < GRUB_UINT_MAX; depth++)
     {
       struct grub_net_route *bestroute = NULL;
+      struct grub_net_route *defaultroute = NULL;
+
       FOR_NET_ROUTES(route)
       {
 	if (depth && prot != route->prot)
 	  continue;
+	if (!defaultroute && route->default_route)
+	  defaultroute = route;
 	if (!match_net (&route->target, &curtarget))
 	  continue;
 	if (route_cmp (route, bestroute) > 0)
 	  bestroute = route;
       }
-      if (bestroute == NULL)
-	return grub_error (GRUB_ERR_NET_NO_ROUTE,
-			   N_("destination unreachable"));
+      if (bestroute == NULL) {
+	if (defaultroute == NULL)
+	  return grub_error (GRUB_ERR_NET_NO_ROUTE,
+			     N_("destination unreachable"));
+	bestroute = defaultroute;
+      }
 
       if (!bestroute->is_gateway)
 	{
-	  *interf = bestroute->interface;
+	  *interf = grub_ipv6_global_interface(bestroute->interface);
 	  return GRUB_ERR_NONE;
 	}
       if (depth == 0)
@@ -1121,7 +1136,7 @@ grub_net_add_route (const char *name,
 grub_err_t
 grub_net_add_route_gw (const char *name,
 		       grub_net_network_level_netaddress_t target,
-		       grub_net_network_level_address_t gw)
+		       grub_net_network_level_address_t gw, int default_route)
 {
   struct grub_net_route *route;
 
@@ -1139,6 +1154,7 @@ grub_net_add_route_gw (const char *name,
   route->target = target;
   route->is_gateway = 1;
   route->gw = gw;
+  route->default_route = default_route;
 
   grub_net_route_register (route);
 
@@ -1164,7 +1180,7 @@ grub_cmd_addroute (struct grub_command *cmd __attribute__ ((unused)),
       err = grub_net_resolve_address (args[3], &gw);
       if (err)
 	return err;
-      return grub_net_add_route_gw (args[0], target, gw);
+      return grub_net_add_route_gw (args[0], target, gw, 0);
     }
   else
     {
@@ -1239,6 +1255,8 @@ grub_cmd_listroutes (struct grub_command *cmd __attribute__ ((unused)),
       }
     else
       grub_printf ("%s", route->interface->name);      
+    if (route->default_route)
+      grub_printf (" default route");
     grub_printf ("\n");
   }
   return GRUB_ERR_NONE;
diff --git a/include/grub/net.h b/include/grub/net.h
index 4571b72..4e98ddd 100644
--- a/include/grub/net.h
+++ b/include/grub/net.h
@@ -192,6 +192,22 @@ typedef struct grub_net_network_level_netaddress
   };
 } grub_net_network_level_netaddress_t;
 
+struct grub_net_route
+{
+  struct grub_net_route *next;
+  struct grub_net_route **prev;
+  grub_net_network_level_netaddress_t target;
+  char *name;
+  struct grub_net_network_level_protocol *prot;
+  int is_gateway;
+  int default_route;
+  union
+  {
+    struct grub_net_network_level_interface *interface;
+    grub_net_network_level_address_t gw;
+  };
+};
+
 #define FOR_PACKETS(cont,var) for (var = (cont).first; var; var = var->next)
 
 static inline grub_err_t
@@ -368,6 +384,16 @@ grub_net_card_unregister (struct grub_net_card *card);
 #define FOR_NET_CARDS_SAFE(var, next) for (var = grub_net_cards, next = (var ? var->next : 0); var; var = next, next = (var ? var->next : 0))
 
 
+extern struct grub_net_route *grub_net_routes;
+
+static inline void
+grub_net_route_register (struct grub_net_route *route)
+{
+  grub_list_push (GRUB_AS_LIST_P (&grub_net_routes),
+		  GRUB_AS_LIST (route));
+}
+
+#define FOR_NET_ROUTES(var) for (var = grub_net_routes; var; var = var->next)
 struct grub_net_session *
 grub_net_open_tcp (char *address, grub_uint16_t port);
 
@@ -393,7 +419,7 @@ grub_net_add_route (const char *name,
 grub_err_t
 grub_net_add_route_gw (const char *name,
 		       grub_net_network_level_netaddress_t target,
-		       grub_net_network_level_address_t gw);
+		       grub_net_network_level_address_t gw, int default_route);
 
 
 #define GRUB_NET_BOOTP_MAC_ADDR_LEN	16
-- 
1.8.5.6

Re: [PATCH 0/3] fix ipv6 support

[Michael Chang]

On Wed, Aug 05, 2015 at 10:50:37AM -0700, Josef Bacik wrote:
> These patches are on top of Michael Chang's bootp6 code (which we also really
> need so it would be great if those could go in as well).  At Facebook we have
> ipv6 only clusters that we need to be able to provision over the network.  The
> current grub2 support for ipv6 is broken in a few ways.  The routing stuff
> needed to be reworked to handle talking to an ipv6 router properly.  I tried to
> not change the code too much in order to make sure I didn't regress the ipv4
> support.

Thanks for your feedback, that's very encouraging to me. :-)

The patch is still in review and need to solve some issues addressed by
Andrei. I was distracted by some other stuff later and my own server
hosted virtual machine to develope and test ipv6 was accidently broken
by system update of virtualization package. The major changes that cause
the failure seems to be qemu changes to enable ipxe UEFI option rom
which somehow did not provide ipv6 support in it's UNDI driver, along
with some other issues in edk2/ovmf itself.

Anyway boop6 patches seems to be innocent to the new problem I have and
can continue the work by not loading iPXE UEFI option rom from qemu. To
whom who may be interested here is libvirt guest config I use to disable
option rom and use edk2's network stack.

    <interface type='network'>
      <rom bar='off'/>
    </interface>

I will continue on working on the patches and follow through review for
the patch set with upstream.

Thanks.
Michael

Re: [PATCH 0/3] fix ipv6 support

[Josef Bacik]

On 08/06/2015 12:06 AM, Michael Chang wrote:
> On Wed, Aug 05, 2015 at 10:50:37AM -0700, Josef Bacik wrote:
>> These patches are on top of Michael Chang's bootp6 code (which we also really
>> need so it would be great if those could go in as well).  At Facebook we have
>> ipv6 only clusters that we need to be able to provision over the network.  The
>> current grub2 support for ipv6 is broken in a few ways.  The routing stuff
>> needed to be reworked to handle talking to an ipv6 router properly.  I tried to
>> not change the code too much in order to make sure I didn't regress the ipv4
>> support.
>
> Thanks for your feedback, that's very encouraging to me. :-)
>
> The patch is still in review and need to solve some issues addressed by
> Andrei. I was distracted by some other stuff later and my own server
> hosted virtual machine to develope and test ipv6 was accidently broken
> by system update of virtualization package. The major changes that cause
> the failure seems to be qemu changes to enable ipxe UEFI option rom
> which somehow did not provide ipv6 support in it's UNDI driver, along
> with some other issues in edk2/ovmf itself.
>
> Anyway boop6 patches seems to be innocent to the new problem I have and
> can continue the work by not loading iPXE UEFI option rom from qemu. To
> whom who may be interested here is libvirt guest config I use to disable
> option rom and use edk2's network stack.
>
>      <interface type='network'>
>        <rom bar='off'/>
>      </interface>
>
> I will continue on working on the patches and follow through review for
> the patch set with upstream.
>

That would be great, anything you post I'll pull in immediately and test 
on real hardware in one of our ipv6 only clusters to make sure it's 
still working.  From my perspective none of your code has given me any 
problems, it pulls the dhcp information that uefi has already properly, 
and manually doing the bootp thing also works properly.  Thanks,

Josef