IPv6: fix rt_lookup in pmtu_discovery

I believe you are looking for the following patch.

I don't think it solves the problem of source based routing to any
(significantly) greater extent than the previous patch.
That would actually require iterating over all IPs assigned to any
interface on our machine.

However, I really don't understand the code (nor the precise issues
involved) sufficiently well to be certain of that.

I actually think the ipv4 code path suffers from a very similar problem.

Mind you - I don't think these issues are really significant problems,
because they just mean we do PMTU once per (dest ip, src ip) instead
of once per (dest ip).

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

David, could you comment on this?

2010/1/13 Maciej Żenczykowski <zenczykowski@gmail.com>:
> I believe you are looking for the following patch.
>
> I don't think it solves the problem of source based routing to any
> (significantly) greater extent than the previous patch.
> That would actually require iterating over all IPs assigned to any
> interface on our machine.
>
> However, I really don't understand the code (nor the precise issues
> involved) sufficiently well to be certain of that.
>
> I actually think the ipv4 code path suffers from a very similar problem.
>
> Mind you - I don't think these issues are really significant problems,
> because they just mean we do PMTU once per (dest ip, src ip) instead
> of once per (dest ip).
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index c2bd74c..dd8e3b3 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -1562,14 +1562,13 @@ out:
>  *     i.e. Path MTU discovery
>  */
>
> -void rt6_pmtu_discovery(struct in6_addr *daddr, struct in6_addr *saddr,
> -                       struct net_device *dev, u32 pmtu)
> +static void rt6_do_pmtu_disc(struct in6_addr *daddr, struct in6_addr *saddr,
> +                            struct net *net, u32 pmtu, int ifindex)
>  {
>        struct rt6_info *rt, *nrt;
> -       struct net *net = dev_net(dev);
>        int allfrag = 0;
>
> -       rt = rt6_lookup(net, daddr, saddr, dev->ifindex, 0);
> +       rt = rt6_lookup(net, daddr, saddr, ifindex, 0);
>        if (rt == NULL)
>                return;
>
> @@ -1637,6 +1636,31 @@ out:
>        dst_release(&rt->u.dst);
>  }
>
> +void rt6_pmtu_discovery(struct in6_addr *daddr, struct in6_addr *saddr,
> +                       struct net_device *dev, u32 pmtu)
> +{
> +       struct net *net = dev_net(dev);
> +
> +       /*
> +        * RFC 1981 states that a node "MUST reduce the size of the packets it
> +        * is sending along the path" that caused the Packet Too Big message.
> +        * Since it's not possible in the general case to determine which
> +        * interface was used to send the original packet, we update the MTU
> +        * on the interface that will be used to send future packets. We also
> +        * update the MTU on the interface that received the Packet Too Big in
> +        * case the original packet was forced out that interface with
> +        * SO_BINDTODEVICE or similar. This is the next best thing to the
> +        * correct behaviour, which would be to update the MTU on all
> +        * interfaces.
> +        */
> +       rt6_do_pmtu_disc(daddr, saddr, net, pmtu, 0);
> +       rt6_do_pmtu_disc(daddr, saddr, net, pmtu, dev->ifindex);
> +       /* also support source address based routing */
> +       rt6_do_pmtu_disc(daddr, NULL, net, pmtu, 0);
> +       rt6_do_pmtu_disc(daddr, NULL, net, pmtu, dev->ifindex);
> +}
> +
> +
>  /*
>  *     Misc support functions
>  */
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Maciej Żenczykowski <zenczykowski@gmail.com>
Date: Wed, 20 Jan 2010 14:55:10 -0800

> David, could you comment on this?

I'll get to your patch when I get to it.  I have tons of other thing
that have much higher priority than this.

If it's in patchwork in "under review" state (which this one is), that
means it's in my backlog and just poking me about it will serve only
to irritate me and put it even lower ino my TODO list.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

>> David, could you comment on this?
>
> I'll get to your patch when I get to it.  I have tons of other thing
> that have much higher priority than this.
>
> If it's in patchwork in "under review" state (which this one is), that
> means it's in my backlog and just poking me about it will serve only
> to irritate me and put it even lower ino my TODO list.

Thank you, and sorry for the trouble.

Maciej
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Maciej Żenczykowski <zenczykowski@gmail.com>
Date: Wed, 13 Jan 2010 16:51:30 -0800

> I believe you are looking for the following patch.
> 
> I don't think it solves the problem of source based routing to any
> (significantly) greater extent than the previous patch.
> That would actually require iterating over all IPs assigned to any
> interface on our machine.
> 
> However, I really don't understand the code (nor the precise issues
> involved) sufficiently well to be certain of that.
> 
> I actually think the ipv4 code path suffers from a very similar problem.
> 
> Mind you - I don't think these issues are really significant problems,
> because they just mean we do PMTU once per (dest ip, src ip) instead
> of once per (dest ip).

Yes, this is about right.

Please resubmit with a proper Signed-off-by: tag so I can apply
this, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html