LVS
lvs-devel
Google
 
Web LinuxVirtualServer.org

Re: [PATCH] ipvs: fix ipv6 icmp forwarding in natted services

To: Julian Anastasov <ja@xxxxxx>
Subject: Re: [PATCH] ipvs: fix ipv6 icmp forwarding in natted services
Cc: Art -kwaak- van Breemen <ard@xxxxxxxxxxxxxxx>, Ansis Atteka <aatteka@xxxxxxxxxx>, lvs-devel@xxxxxxxxxxxxxxx, Jesper Dangaard Brouer <brouer@xxxxxxxxxx>, Patrick McHardy <kaber@xxxxxxxxx>
From: Hans Schillstrom <hans@xxxxxxxxxxxxxxx>
Date: Wed, 19 Feb 2014 23:08:43 +0100
Hi Julian

As usual I'm to quick to send the mail...

On Wed, 2014-02-19 at 23:34 +0200, Julian Anastasov wrote:
>       Hello,
> 
> On Wed, 19 Feb 2014, Hans Schillstrom wrote:
> 
> > The problem is if icmp6 is not the first header it will not work...
> > i.e. it can be other headers before icmp and if you have -1 you will not
> > always get the icmp header.
> > 
> > 
> > The patch that broke it was:
> > commit 9195bb8e381d81d5a315f911904cdf0cfcc919b8
> > Author: Ansis Atteka <aatteka@xxxxxxxxxx>
> > 
> > Before there was a 
> > while (nexthdr != target) {
> > ..
> > }
> > 
> > now it's  
> > 
> > do {
> > ..
> > } while (!found)
> > 
> > which doesn't work for ipvs, when target is != -1
> > 
> > If you specify a target and it's the first header you should break.
> > 
> > I need to look deeper into the other users also to see that it doesn't
> > break anything.
> > 
> > Ansis, I don't think it will break your patch or ?
> > 
> > 
> > --- a/net/ipv6/exthdrs_core.c     2014-02-19 16:36:22.031686037 +0100
> > +++ b/net/ipv6/exthdrs_core.c     2014-02-19 16:37:28.838082168 +0100
> > @@ -211,6 +211,9 @@ int ipv6_find_hdr(const struct sk_buff *
> >                 unsigned int hdrlen;
> >                 found = (nexthdr == target);
> >  
> > +               if (found && (target > 0))
> > +                       break;
> > +
> 
>       This is against the goal of the above commit.
> 
> >                 if ((!ipv6_ext_hdr(nexthdr)) || nexthdr == NEXTHDR_NONE)
> > {
> >                         if (target < 0)
> >                                 break;
> 
>       May be above check should be:
> 
>       if (target < 0 || found)
>               break;

It will work for hmark and it looks like it will work for others,
with -1 

Maybe Patrick have another opinion...

> We have to check some callers with -1, may be some
> need check for NEXTHDR_NONE, for example, tproxy_tg6_v1(),
> also the second call in hmark_pkt_set_htuple_ipv6(). 


> Not sure
> about nft_set_pktinfo_ipv6 and its callers.

> Regards
> 
> --
> Julian Anastasov <ja@xxxxxx>


Regards
Hans

Attachment: smime.p7s
Description: S/MIME cryptographic signature

<Prev in Thread] Current Thread [Next in Thread>