Re: [PATCH V2] net: fix ipv6 routing

[Vladimir 'φ-coder/phcoder' Serbinenko]

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 <address@hidden>
>> ---
>> 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
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel

signature.asc
Description: OpenPGP digital signature