[PATCH V2] net: fix ipv6 routing

[PATCH V2] net: fix ipv6 routing

[Josef Bacik]

ipv6 routing in grub2 is broken, we cannot talk to anything outside our local
network or anything that doesn't route in our global namespace.  This patch
fixes this by doing a couple of things

1) Read the router information off of the router advertisement.  If we have a
router lifetime we need to take the source address and create a route from it.

2) Changes the routing stuff slightly to allow you to specify a gateway _and_ an
interface.  Since the router advertisements come in on the link local address we
need to associate it with the global address on the card.  So when we are
processing the router advertisement, either use the SLAAC interface we create
and add the route to that interface, or loop through the global addresses we
currently have on our interface and associate it with one of those addresses.
We need to have a special case here for the default route so that it gets used,
we do this by setting the masksize to 0 to mean it encompasses all networks.
The routing code will automatically select the best route so if there is a
closer match we will use that.

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

Signed-off-by: Josef Bacik <jbacik@fb.com>
---
V1->V2:
-reworked the route stuff to hold an interface for gateways as well.
-fixed the slaac stuff so the route information is separate from the interface
 configuration

 grub-core/net/bootp.c                  |  2 +-
 grub-core/net/drivers/ieee1275/ofnet.c |  4 +--
 grub-core/net/icmp6.c                  | 63 +++++++++++++++++++++++++++++++++-
 grub-core/net/net.c                    | 40 ++++++++-------------
 include/grub/net.h                     | 25 +++++++++++++-
 5 files changed, 103 insertions(+), 31 deletions(-)

diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
index 37d1cfa..9fc47bd 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, NULL);
 	      grub_free (rname);
 	    }
 	  break;
diff --git a/grub-core/net/drivers/ieee1275/ofnet.c b/grub-core/net/drivers/ieee1275/ofnet.c
index eea8e71..d238628 100644
--- a/grub-core/net/drivers/ieee1275/ofnet.c
+++ b/grub-core/net/drivers/ieee1275/ofnet.c
@@ -151,7 +151,7 @@ grub_ieee1275_parse_bootpath (const char *devpath, char *bootpath,
   grub_net_network_level_address_t client_addr, gateway_addr, subnet_mask;
   grub_net_link_level_address_t hw_addr;
   grub_net_interface_flags_t flags = 0;
-  struct grub_net_network_level_interface *inter;
+  struct grub_net_network_level_interface *inter = NULL;
 
   hw_addr.type = GRUB_NET_LINK_LEVEL_PROTOCOL_ETHERNET;
 
@@ -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, inter);
       else
         return grub_errno;
     }
diff --git a/grub-core/net/icmp6.c b/grub-core/net/icmp6.c
index 7953e68..79a4a30 100644
--- a/grub-core/net/icmp6.c
+++ b/grub-core/net/icmp6.c
@@ -115,6 +115,7 @@ grub_net_recv_icmp6_packet (struct grub_net_buff *nb,
 			    grub_uint8_t ttl)
 {
   struct icmp_header *icmph;
+  struct grub_net_network_level_interface *orig_inf = inf;
   grub_err_t err;
   grub_uint16_t checksum;
 
@@ -345,8 +346,25 @@ grub_net_recv_icmp6_packet (struct grub_net_buff *nb,
       {
 	grub_uint8_t *ptr;
 	struct option_header *ohdr;
+	struct router_adv *radv;
+	struct grub_net_network_level_interface *route_inf = NULL;
+	int default_route = 0;
 	if (icmph->code)
 	  break;
+	radv = (void *)nb->data;
+	if (grub_be_to_cpu16 (radv->router_lifetime) > 0)
+	  {
+	    struct grub_net_route *route;
+
+	    FOR_NET_ROUTES (route)
+	    {
+	      if (!grub_memcmp (&route->gw, source, sizeof (route->gw)))
+		break;
+	    }
+	    if (route == NULL)
+	      default_route = 1;
+	  }
+
 	err = grub_netbuff_pull (nb, sizeof (struct router_adv));
 	if (err)
 	  {
@@ -413,7 +431,11 @@ grub_net_recv_icmp6_packet (struct grub_net_buff *nb,
 		    /* Update lease time if needed here once we have
 		       lease times.  */
 		    if (inf)
-		      continue;
+		      {
+			if (!route_inf)
+			  route_inf = inf;
+			continue;
+		      }
 
 		    grub_dprintf ("net", "creating slaac\n");
 
@@ -429,12 +451,51 @@ grub_net_recv_icmp6_packet (struct grub_net_buff *nb,
 		      inf = grub_net_add_addr (name, 
 					       card, &addr,
 					       &slaac->address, 0);
+		      if (!route_inf)
+			route_inf = inf;
 		      grub_net_add_route (name, netaddr, inf);
 		      grub_free (name);
 		    }
 		  }
 	      }
 	  }
+	if (default_route)
+	  {
+	    char *name;
+	    grub_net_network_level_netaddress_t netaddr;
+	    name = grub_xasprintf ("default6:%s", card->name);
+	    if (!name)
+	      {
+		grub_errno = GRUB_ERR_NONE;
+		goto next;
+	      }
+	    /* Default routes take alll of the traffic, so make the mask huge */
+	    netaddr.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
+	    netaddr.ipv6.masksize = 0;
+	    netaddr.ipv6.base[0] = 0;
+	    netaddr.ipv6.base[1] = 0;
+
+	    /* May not have gotten slaac info, find a global address on this
+	      card.  */
+	    if (route_inf == NULL)
+	      {
+		FOR_NET_NETWORK_LEVEL_INTERFACES (inf)
+		{
+		  if (inf->card == card && inf != orig_inf
+		      && inf->address.type == GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6
+		      && grub_net_hwaddr_cmp(&inf->hwaddress,
+					     &orig_inf->hwaddress) == 0)
+		    {
+		      route_inf = inf;
+		      break;
+		    }
+		}
+	      }
+	    if (route_inf != NULL)
+	      grub_net_add_route_gw (name, netaddr, *source, route_inf);
+	    grub_free (name);
+	  }
+next:
 	if (ptr != nb->tail)
 	  break;
       }
diff --git a/grub-core/net/net.c b/grub-core/net/net.c
index 21a4e94..65bea28 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;
@@ -410,14 +395,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)
@@ -524,6 +501,8 @@ match_net (const grub_net_network_level_netaddress_t *net,
     case GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6:
       {
 	grub_uint64_t mask[2];
+	if (net->ipv6.masksize == 0)
+	  return 1;
 	if (net->ipv6.masksize <= 64)
 	  {
 	    mask[0] = 0xffffffffffffffffULL << (64 - net->ipv6.masksize);
@@ -687,7 +666,14 @@ grub_net_route_address (grub_net_network_level_address_t addr,
 	  return GRUB_ERR_NONE;
 	}
       if (depth == 0)
-	*gateway = bestroute->gw;
+	{
+	  *gateway = bestroute->gw;
+	  if (bestroute->interface != NULL)
+	    {
+	      *interf = bestroute->interface;
+	      return GRUB_ERR_NONE;
+	    }
+	}
       curtarget = bestroute->gw;
     }
 
@@ -1109,7 +1095,8 @@ 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,
+		       struct grub_net_network_level_interface *inter)
 {
   struct grub_net_route *route;
 
@@ -1127,6 +1114,7 @@ grub_net_add_route_gw (const char *name,
   route->target = target;
   route->is_gateway = 1;
   route->gw = gw;
+  route->interface = inter;
 
   grub_net_route_register (route);
 
@@ -1152,7 +1140,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, NULL);
     }
   else
     {
diff --git a/include/grub/net.h b/include/grub/net.h
index 4571b72..a1ff519 100644
--- a/include/grub/net.h
+++ b/include/grub/net.h
@@ -192,6 +192,18 @@ 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;
+  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 +380,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 +415,8 @@ 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,
+		       struct grub_net_network_level_interface *inter);
 
 
 #define GRUB_NET_BOOTP_MAC_ADDR_LEN	16
-- 
2.1.0

Re: [PATCH V2] net: fix ipv6 routing

[Andrei Borzenkov]

28.08.2015 18:24, Josef Bacik пишет:
> ipv6 routing in grub2 is broken, we cannot talk to anything outside our local
> network or anything that doesn't route in our global namespace.  This patch
> fixes this by doing a couple of things
>
> 1) Read the router information off of the router advertisement.  If we have a
> router lifetime we need to take the source address and create a route from it.
>
> 2) Changes the routing stuff slightly to allow you to specify a gateway _and_ an
> interface.  Since the router advertisements come in on the link local address we
> need to associate it with the global address on the card.  So when we are
> processing the router advertisement, either use the SLAAC interface we create
> and add the route to that interface, or loop through the global addresses we
> currently have on our interface and associate it with one of those addresses.
> We need to have a special case here for the default route so that it gets used,
> we do this by setting the masksize to 0 to mean it encompasses all networks.
> The routing code will automatically select the best route so if there is a
> closer match we will use that.
>

I think this is OK; minor comments below. We probably want to augment 
net_route to allow both gateway and interface as well.

> With this patch I can now talk to ipv6 addresses outside of my local network.
> Thanks,
>
> Signed-off-by: Josef Bacik <jbacik@fb.com>
> ---
> V1->V2:
> -reworked the route stuff to hold an interface for gateways as well.
> -fixed the slaac stuff so the route information is separate from the interface
>   configuration
>
>   grub-core/net/bootp.c                  |  2 +-
>   grub-core/net/drivers/ieee1275/ofnet.c |  4 +--
>   grub-core/net/icmp6.c                  | 63 +++++++++++++++++++++++++++++++++-
>   grub-core/net/net.c                    | 40 ++++++++-------------
>   include/grub/net.h                     | 25 +++++++++++++-
>   5 files changed, 103 insertions(+), 31 deletions(-)
>
> diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
> index 37d1cfa..9fc47bd 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, NULL);
>   	      grub_free (rname);
>   	    }
>   	  break;
> diff --git a/grub-core/net/drivers/ieee1275/ofnet.c b/grub-core/net/drivers/ieee1275/ofnet.c
> index eea8e71..d238628 100644
> --- a/grub-core/net/drivers/ieee1275/ofnet.c
> +++ b/grub-core/net/drivers/ieee1275/ofnet.c
> @@ -151,7 +151,7 @@ grub_ieee1275_parse_bootpath (const char *devpath, char *bootpath,
>     grub_net_network_level_address_t client_addr, gateway_addr, subnet_mask;
>     grub_net_link_level_address_t hw_addr;
>     grub_net_interface_flags_t flags = 0;
> -  struct grub_net_network_level_interface *inter;
> +  struct grub_net_network_level_interface *inter = NULL;
>
>     hw_addr.type = GRUB_NET_LINK_LEVEL_PROTOCOL_ETHERNET;
>
> @@ -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, inter);
>         else
>           return grub_errno;
>       }
> diff --git a/grub-core/net/icmp6.c b/grub-core/net/icmp6.c
> index 7953e68..79a4a30 100644
> --- a/grub-core/net/icmp6.c
> +++ b/grub-core/net/icmp6.c
> @@ -115,6 +115,7 @@ grub_net_recv_icmp6_packet (struct grub_net_buff *nb,
>   			    grub_uint8_t ttl)
>   {
>     struct icmp_header *icmph;
> +  struct grub_net_network_level_interface *orig_inf = inf;
>     grub_err_t err;
>     grub_uint16_t checksum;
>
> @@ -345,8 +346,25 @@ grub_net_recv_icmp6_packet (struct grub_net_buff *nb,
>         {
>   	grub_uint8_t *ptr;
>   	struct option_header *ohdr;
> +	struct router_adv *radv;
> +	struct grub_net_network_level_interface *route_inf = NULL;
> +	int default_route = 0;
>   	if (icmph->code)
>   	  break;
> +	radv = (void *)nb->data;
> +	if (grub_be_to_cpu16 (radv->router_lifetime) > 0)

This should come after grub_netbuff_pull error check to make sure we get 
ohis struct.


> +	  {
> +	    struct grub_net_route *route;
> +
> +	    FOR_NET_ROUTES (route)
> +	    {
> +	      if (!grub_memcmp (&route->gw, source, sizeof (route->gw)))
> +		break;
> +	    }
> +	    if (route == NULL)
> +	      default_route = 1;
> +	  }
> +
>   	err = grub_netbuff_pull (nb, sizeof (struct router_adv));
>   	if (err)
>   	  {
> @@ -413,7 +431,11 @@ grub_net_recv_icmp6_packet (struct grub_net_buff *nb,
>   		    /* Update lease time if needed here once we have
>   		       lease times.  */
>   		    if (inf)
> -		      continue;
> +		      {
> +			if (!route_inf)
> +			  route_inf = inf;
> +			continue;
> +		      }
>
>   		    grub_dprintf ("net", "creating slaac\n");
>
> @@ -429,12 +451,51 @@ grub_net_recv_icmp6_packet (struct grub_net_buff *nb,
>   		      inf = grub_net_add_addr (name,
>   					       card, &addr,
>   					       &slaac->address, 0);
> +		      if (!route_inf)
> +			route_inf = inf;
>   		      grub_net_add_route (name, netaddr, inf);
>   		      grub_free (name);
>   		    }
>   		  }
>   	      }
>   	  }
> +	if (default_route)
> +	  {
> +	    char *name;
> +	    grub_net_network_level_netaddress_t netaddr;
> +	    name = grub_xasprintf ("default6:%s", card->name);

It is usually <card>:<something-else>, not other way round. DHCPv4 sets 
default route to card:dhcp:default. May be card:ra:default6 to indicate 
auto-configuration.

> +	    if (!name)
> +	      {
> +		grub_errno = GRUB_ERR_NONE;
> +		goto next;
> +	      }
> +	    /* Default routes take alll of the traffic, so make the mask huge */
                                      typo

> +	    netaddr.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
> +	    netaddr.ipv6.masksize = 0;
> +	    netaddr.ipv6.base[0] = 0;
> +	    netaddr.ipv6.base[1] = 0;
> +
> +	    /* May not have gotten slaac info, find a global address on this
> +	      card.  */
> +	    if (route_inf == NULL)
> +	      {
> +		FOR_NET_NETWORK_LEVEL_INTERFACES (inf)
> +		{
> +		  if (inf->card == card && inf != orig_inf
> +		      && inf->address.type == GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6
> +		      && grub_net_hwaddr_cmp(&inf->hwaddress,
> +					     &orig_inf->hwaddress) == 0)
> +		    {
> +		      route_inf = inf;
> +		      break;
> +		    }
> +		}
> +	      }
> +	    if (route_inf != NULL)
> +	      grub_net_add_route_gw (name, netaddr, *source, route_inf);
> +	    grub_free (name);
> +	  }
> +next:
>   	if (ptr != nb->tail)
>   	  break;
>         }
> diff --git a/grub-core/net/net.c b/grub-core/net/net.c
> index 21a4e94..65bea28 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;
> @@ -410,14 +395,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)
> @@ -524,6 +501,8 @@ match_net (const grub_net_network_level_netaddress_t *net,
>       case GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6:
>         {
>   	grub_uint64_t mask[2];
> +	if (net->ipv6.masksize == 0)
> +	  return 1;
>   	if (net->ipv6.masksize <= 64)
>   	  {
>   	    mask[0] = 0xffffffffffffffffULL << (64 - net->ipv6.masksize);
> @@ -687,7 +666,14 @@ grub_net_route_address (grub_net_network_level_address_t addr,
>   	  return GRUB_ERR_NONE;
>   	}
>         if (depth == 0)
> -	*gateway = bestroute->gw;
> +	{
> +	  *gateway = bestroute->gw;
> +	  if (bestroute->interface != NULL)
> +	    {
> +	      *interf = bestroute->interface;
> +	      return GRUB_ERR_NONE;
> +	    }
> +	}
>         curtarget = bestroute->gw;
>       }
>
> @@ -1109,7 +1095,8 @@ 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,
> +		       struct grub_net_network_level_interface *inter)

This actually makes two functions rather redundant. But it is better 
merge them separately.

>   {
>     struct grub_net_route *route;
>
> @@ -1127,6 +1114,7 @@ grub_net_add_route_gw (const char *name,
>     route->target = target;
>     route->is_gateway = 1;
>     route->gw = gw;
> +  route->interface = inter;
>
>     grub_net_route_register (route);
>
> @@ -1152,7 +1140,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, NULL);
>       }
>     else
>       {
> diff --git a/include/grub/net.h b/include/grub/net.h
> index 4571b72..a1ff519 100644
> --- a/include/grub/net.h
> +++ b/include/grub/net.h
> @@ -192,6 +192,18 @@ 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;
> +  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 +380,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 +415,8 @@ 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,
> +		       struct grub_net_network_level_interface *inter);
>
>
>   #define GRUB_NET_BOOTP_MAC_ADDR_LEN	16
>

Re: [PATCH V2] net: fix ipv6 routing

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 13384 bytes --]

Were Andrey's comments ever adressed? Other than his comments the patch
looks good.
On 20.09.2015 11:50, Andrei Borzenkov wrote:
> 28.08.2015 18:24, Josef Bacik пишет:
>> ipv6 routing in grub2 is broken, we cannot talk to anything outside
>> our local
>> network or anything that doesn't route in our global namespace.  This
>> patch
>> fixes this by doing a couple of things
>>
>> 1) Read the router information off of the router advertisement.  If we
>> have a
>> router lifetime we need to take the source address and create a route
>> from it.
>>
>> 2) Changes the routing stuff slightly to allow you to specify a
>> gateway _and_ an
>> interface.  Since the router advertisements come in on the link local
>> address we
>> need to associate it with the global address on the card.  So when we are
>> processing the router advertisement, either use the SLAAC interface we
>> create
>> and add the route to that interface, or loop through the global
>> addresses we
>> currently have on our interface and associate it with one of those
>> addresses.
>> We need to have a special case here for the default route so that it
>> gets used,
>> we do this by setting the masksize to 0 to mean it encompasses all
>> networks.
>> The routing code will automatically select the best route so if there
>> is a
>> closer match we will use that.
>>
> 
> I think this is OK; minor comments below. We probably want to augment
> net_route to allow both gateway and interface as well.
> 
>> With this patch I can now talk to ipv6 addresses outside of my local
>> network.
>> Thanks,
>>
>> Signed-off-by: Josef Bacik <jbacik@fb.com>
>> ---
>> V1->V2:
>> -reworked the route stuff to hold an interface for gateways as well.
>> -fixed the slaac stuff so the route information is separate from the
>> interface
>>   configuration
>>
>>   grub-core/net/bootp.c                  |  2 +-
>>   grub-core/net/drivers/ieee1275/ofnet.c |  4 +--
>>   grub-core/net/icmp6.c                  | 63
>> +++++++++++++++++++++++++++++++++-
>>   grub-core/net/net.c                    | 40 ++++++++-------------
>>   include/grub/net.h                     | 25 +++++++++++++-
>>   5 files changed, 103 insertions(+), 31 deletions(-)
>>
>> diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
>> index 37d1cfa..9fc47bd 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, NULL);
>>             grub_free (rname);
>>           }
>>         break;
>> diff --git a/grub-core/net/drivers/ieee1275/ofnet.c
>> b/grub-core/net/drivers/ieee1275/ofnet.c
>> index eea8e71..d238628 100644
>> --- a/grub-core/net/drivers/ieee1275/ofnet.c
>> +++ b/grub-core/net/drivers/ieee1275/ofnet.c
>> @@ -151,7 +151,7 @@ grub_ieee1275_parse_bootpath (const char *devpath,
>> char *bootpath,
>>     grub_net_network_level_address_t client_addr, gateway_addr,
>> subnet_mask;
>>     grub_net_link_level_address_t hw_addr;
>>     grub_net_interface_flags_t flags = 0;
>> -  struct grub_net_network_level_interface *inter;
>> +  struct grub_net_network_level_interface *inter = NULL;
>>
>>     hw_addr.type = GRUB_NET_LINK_LEVEL_PROTOCOL_ETHERNET;
>>
>> @@ -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, inter);
>>         else
>>           return grub_errno;
>>       }
>> diff --git a/grub-core/net/icmp6.c b/grub-core/net/icmp6.c
>> index 7953e68..79a4a30 100644
>> --- a/grub-core/net/icmp6.c
>> +++ b/grub-core/net/icmp6.c
>> @@ -115,6 +115,7 @@ grub_net_recv_icmp6_packet (struct grub_net_buff *nb,
>>                   grub_uint8_t ttl)
>>   {
>>     struct icmp_header *icmph;
>> +  struct grub_net_network_level_interface *orig_inf = inf;
>>     grub_err_t err;
>>     grub_uint16_t checksum;
>>
>> @@ -345,8 +346,25 @@ grub_net_recv_icmp6_packet (struct grub_net_buff
>> *nb,
>>         {
>>       grub_uint8_t *ptr;
>>       struct option_header *ohdr;
>> +    struct router_adv *radv;
>> +    struct grub_net_network_level_interface *route_inf = NULL;
>> +    int default_route = 0;
>>       if (icmph->code)
>>         break;
>> +    radv = (void *)nb->data;
>> +    if (grub_be_to_cpu16 (radv->router_lifetime) > 0)
> 
> This should come after grub_netbuff_pull error check to make sure we get
> ohis struct.
> 
> 
>> +      {
>> +        struct grub_net_route *route;
>> +
>> +        FOR_NET_ROUTES (route)
>> +        {
>> +          if (!grub_memcmp (&route->gw, source, sizeof (route->gw)))
>> +        break;
>> +        }
>> +        if (route == NULL)
>> +          default_route = 1;
>> +      }
>> +
>>       err = grub_netbuff_pull (nb, sizeof (struct router_adv));
>>       if (err)
>>         {
>> @@ -413,7 +431,11 @@ grub_net_recv_icmp6_packet (struct grub_net_buff
>> *nb,
>>               /* Update lease time if needed here once we have
>>                  lease times.  */
>>               if (inf)
>> -              continue;
>> +              {
>> +            if (!route_inf)
>> +              route_inf = inf;
>> +            continue;
>> +              }
>>
>>               grub_dprintf ("net", "creating slaac\n");
>>
>> @@ -429,12 +451,51 @@ grub_net_recv_icmp6_packet (struct grub_net_buff
>> *nb,
>>                 inf = grub_net_add_addr (name,
>>                              card, &addr,
>>                              &slaac->address, 0);
>> +              if (!route_inf)
>> +            route_inf = inf;
>>                 grub_net_add_route (name, netaddr, inf);
>>                 grub_free (name);
>>               }
>>             }
>>             }
>>         }
>> +    if (default_route)
>> +      {
>> +        char *name;
>> +        grub_net_network_level_netaddress_t netaddr;
>> +        name = grub_xasprintf ("default6:%s", card->name);
> 
> It is usually <card>:<something-else>, not other way round. DHCPv4 sets
> default route to card:dhcp:default. May be card:ra:default6 to indicate
> auto-configuration.
> 
>> +        if (!name)
>> +          {
>> +        grub_errno = GRUB_ERR_NONE;
>> +        goto next;
>> +          }
>> +        /* Default routes take alll of the traffic, so make the mask
>> huge */
>                                      typo
> 
>> +        netaddr.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
>> +        netaddr.ipv6.masksize = 0;
>> +        netaddr.ipv6.base[0] = 0;
>> +        netaddr.ipv6.base[1] = 0;
>> +
>> +        /* May not have gotten slaac info, find a global address on this
>> +          card.  */
>> +        if (route_inf == NULL)
>> +          {
>> +        FOR_NET_NETWORK_LEVEL_INTERFACES (inf)
>> +        {
>> +          if (inf->card == card && inf != orig_inf
>> +              && inf->address.type ==
>> GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6
>> +              && grub_net_hwaddr_cmp(&inf->hwaddress,
>> +                         &orig_inf->hwaddress) == 0)
>> +            {
>> +              route_inf = inf;
>> +              break;
>> +            }
>> +        }
>> +          }
>> +        if (route_inf != NULL)
>> +          grub_net_add_route_gw (name, netaddr, *source, route_inf);
>> +        grub_free (name);
>> +      }
>> +next:
>>       if (ptr != nb->tail)
>>         break;
>>         }
>> diff --git a/grub-core/net/net.c b/grub-core/net/net.c
>> index 21a4e94..65bea28 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;
>> @@ -410,14 +395,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)
>> @@ -524,6 +501,8 @@ match_net (const
>> grub_net_network_level_netaddress_t *net,
>>       case GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6:
>>         {
>>       grub_uint64_t mask[2];
>> +    if (net->ipv6.masksize == 0)
>> +      return 1;
>>       if (net->ipv6.masksize <= 64)
>>         {
>>           mask[0] = 0xffffffffffffffffULL << (64 - net->ipv6.masksize);
>> @@ -687,7 +666,14 @@ grub_net_route_address
>> (grub_net_network_level_address_t addr,
>>         return GRUB_ERR_NONE;
>>       }
>>         if (depth == 0)
>> -    *gateway = bestroute->gw;
>> +    {
>> +      *gateway = bestroute->gw;
>> +      if (bestroute->interface != NULL)
>> +        {
>> +          *interf = bestroute->interface;
>> +          return GRUB_ERR_NONE;
>> +        }
>> +    }
>>         curtarget = bestroute->gw;
>>       }
>>
>> @@ -1109,7 +1095,8 @@ 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,
>> +               struct grub_net_network_level_interface *inter)
> 
> This actually makes two functions rather redundant. But it is better
> merge them separately.
> 
>>   {
>>     struct grub_net_route *route;
>>
>> @@ -1127,6 +1114,7 @@ grub_net_add_route_gw (const char *name,
>>     route->target = target;
>>     route->is_gateway = 1;
>>     route->gw = gw;
>> +  route->interface = inter;
>>
>>     grub_net_route_register (route);
>>
>> @@ -1152,7 +1140,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, NULL);
>>       }
>>     else
>>       {
>> diff --git a/include/grub/net.h b/include/grub/net.h
>> index 4571b72..a1ff519 100644
>> --- a/include/grub/net.h
>> +++ b/include/grub/net.h
>> @@ -192,6 +192,18 @@ 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;
>> +  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 +380,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 +415,8 @@ 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,
>> +               struct grub_net_network_level_interface *inter);
>>
>>
>>   #define GRUB_NET_BOOTP_MAC_ADDR_LEN    16
>>
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

Re: [PATCH V2] net: fix ipv6 routing

[Josef Bacik]

On 01/22/2016 08:01 AM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> Were Andrey's comments ever adressed? Other than his comments the patch
> looks good.

I haven't addressed them yet, had other issues come up and this is 
working for us fine in production.  I'm getting a test environment set 
up so I can make his changes and then verify nothing broke and then I'll 
send a V3.  Thanks,

Josef

Re: [PATCH V2] net: fix ipv6 routing

[Andrei Borzenkov]

22.01.2016 17:20, Josef Bacik пишет:
> On 01/22/2016 08:01 AM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>> Were Andrey's comments ever adressed? Other than his comments the patch
>> looks good.
> 
> I haven't addressed them yet, had other issues come up and this is
> working for us fine in production.  I'm getting a test environment set
> up so I can make his changes and then verify nothing broke and then I'll
> send a V3.  Thanks,
> 

I am confused - does patch series you sent later include updated version
or not?

Re: [PATCH V2] net: fix ipv6 routing

[Josef Bacik]

On 02/22/2016 03:07 AM, Andrei Borzenkov wrote:
> 22.01.2016 17:20, Josef Bacik пишет:
>> On 01/22/2016 08:01 AM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>>> Were Andrey's comments ever adressed? Other than his comments the patch
>>> looks good.
>>
>> I haven't addressed them yet, had other issues come up and this is
>> working for us fine in production.  I'm getting a test environment set
>> up so I can make his changes and then verify nothing broke and then I'll
>> send a V3.  Thanks,
>>
>
> I am confused - does patch series you sent later include updated version
> or not?
>

Yup it had the updated version (at least it should have).  Thanks,

Josef