netdev - Re: [PATCH V4 net 0/2] ipv6: fix flowlabel issue for reset packet

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALx6S3498iPfNs4Wt5RWjoxZS0nXHT=qvdSgZhZbqxswqJqdZw@mail.gmail.com>
Date:   Fri, 18 Aug 2017 07:50:03 -0700
From:   Tom Herbert <tom@...bertland.com>
To:     Martin KaFai Lau <kafai@...com>
Cc:     Shaohua Li <shli@...nel.org>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH V4 net 0/2] ipv6: fix flowlabel issue for reset packet

> We had been using the auto_flowlabels=1 (i.e. essentially enable flowlabel)
> mainly because we want to take the benefit of dst_negative_advice() when
> tcp_write_timeout() happens.
>
> During our test, our system handles quite well with changing flowlabel.
> The only exception we have hit is the TCP_RST sent from an inet_timewait_sock.
>
Martin,

That is interesting data. Have you determined why the middlebox has a
problem with flow label change in TW state but not other states?

Tom

> If we keep the flowlabel consistent (or persistent sk_txhash), there
> is no practical usage for us to turn on flowlabel and the problem also goes
> away.  We have it off for now.
>
>>
>> > There seems to have other bug in this side. From my understanding, commit
>> > 265f94ff54d6(net: Recompute sk_txhash on negative routing advice) tries to
>> > select a different route. But the multipath selection code
>> > (rt6_multipath_select) doesn't use sk_txhash or skb->hash, it does use
>> > fl6.flowlabel, but that is the flowlabel user sets. So looks like the commit
>> > doesn't change anything.
>> >
>> The routing functions typically don't use sock of skbuff, but use flow
>> structs instead. It may be reasonable to add a hash to those.
> The localhost's mutlipath selection is another existing issue.  AFAICT,
> it does not take the sk_txhash (or skb->hash) into account and the following
> dst_negative_advice() will also have no effect in the route selction.
> It is another issue to be fixed and to be figured out how to pass the
> sk_txhash down. (1)
>
> Shaohua is proposing to record the 20 bits of the sk_txhash in the
> tw_flowlabel of the 'struct inet_timewait_sock'.  The tw_flowlabel could
> potentially be used to do the multipath selection once we figured out
> how to tackle (1).
>
> Thanks,
> Martin
>
>
>>
>> > What's the 'src port for UDP encap'? I can't find the code setting skb->hash
>> > to sk_txhash in UDP side.
>> >
>> udp_flow_src_port is function call by UDP encaps to set source port.
>> This is call skb_get_hash. sk_set_txhash is function to set txhash
>> right now to random value. skb_set_hash_from_sk set skb->hash when
>> skbuff is owned by socket (skb_set_owner_w).
>>
>> Thanks,
>> Tom
>>
>>
>> > Thanks,
>> > Shaohua