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
smime.p7s
Description: S/MIME cryptographic signature
|