netdev - Re: [RFC net-next PATCH 4/5] net: new XDP feature for reading HW rxhash from drivers

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]

Date:   Sun, 21 May 2017 15:10:29 -0700
From:   Tom Herbert <tom@...bertland.com>
To:     Jesper Dangaard Brouer <brouer@...hat.com>
Cc:     Daniel Borkmann <borkmann@...earbox.net>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: [RFC net-next PATCH 4/5] net: new XDP feature for reading HW
 rxhash from drivers

On Sun, May 21, 2017 at 9:04 AM, Jesper Dangaard Brouer
<brouer@...hat.com> wrote:
> On Sat, 20 May 2017 09:16:09 -0700
> Tom Herbert <tom@...bertland.com> wrote:
>
>> > +/* XDP rxhash have an associated type, which is related to the RSS
>> > + * (Receive Side Scaling) standard, but NIC HW have different mapping
>> > + * and support. Thus, create mapping that is interesting for XDP.  XDP
>> > + * would primarly want insight into L3 and L4 protocol info.
>> > + *
>> > + * TODO: Likely need to get extended with "L3_IPV6_EX" due RSS standard
>> > + *
>> > + * The HASH_TYPE will be returned from bpf helper as the top 32-bit of
>> > + * the 64-bit rxhash (internally type stored in xdp_buff->flags).
>> > + */
>> > +#define XDP_HASH(x)            ((x) & ((1ULL << 32)-1))
>> > +#define XDP_HASH_TYPE(x)       ((x) >> 32)
>> > +
>> > +#define XDP_HASH_TYPE_L3_SHIFT 0
>> > +#define XDP_HASH_TYPE_L3_BITS  3
>> > +#define XDP_HASH_TYPE_L3_MASK  ((1ULL << XDP_HASH_TYPE_L3_BITS)-1)
>> > +#define XDP_HASH_TYPE_L3(x)    ((x) & XDP_HASH_TYPE_L3_MASK)
>> > +enum {
>> > +       XDP_HASH_TYPE_L3_IPV4 = 1,
>> > +       XDP_HASH_TYPE_L3_IPV6,
>> > +};
>> > +
>> > +#define XDP_HASH_TYPE_L4_SHIFT XDP_HASH_TYPE_L3_BITS
>> > +#define XDP_HASH_TYPE_L4_BITS  5
>> > +#define XDP_HASH_TYPE_L4_MASK                                          \
>> > +       (((1ULL << XDP_HASH_TYPE_L4_BITS)-1) << XDP_HASH_TYPE_L4_SHIFT)
>> > +#define XDP_HASH_TYPE_L4(x)    ((x) & XDP_HASH_TYPE_L4_MASK)
>> > +enum {
>> > +       _XDP_HASH_TYPE_L4_TCP = 1,
>> > +       _XDP_HASH_TYPE_L4_UDP,
>> > +};
>> > +#define XDP_HASH_TYPE_L4_TCP (_XDP_HASH_TYPE_L4_TCP << XDP_HASH_TYPE_L4_SHIFT)
>> > +#define XDP_HASH_TYPE_L4_UDP (_XDP_HASH_TYPE_L4_UDP << XDP_HASH_TYPE_L4_SHIFT)
>> > +
>> Hi Jesper,
>>
>> Why do we need these indicators for protocol specific hash? It seems
>> like L4 and L3 is useful differentiation and protocol agnostic (I'm
>> still holding out hope that SCTP will be deployed some day ;-) )
>
> I'm not sure I understood the question fully, but let me try to answer
> anyway.  To me it seems obvious that you would want to know the
> protocol/L4 type, as this helps avoid hash collisions between UDP and
> TCP flows.  I can easily imagine someone constructing an UDP packet
> that could hash collide with a given TCP flow.
>
> And yes, i40 support matching SCTP, and we will create a
> XDP_HASH_TYPE_L4_SCTP when adding XDP rxhash support for that driver.
>
But where would this information be used? We don't save it in skbuff,
don't use it in RPS, RFS. RSS doesn't use it for packet steering so
the hash collision problem already exists at the device level. If
there is a collision problem between two protocols then maybe hash
should be over 5-tuple instead...

Tom

> --
> Best regards,
>   Jesper Dangaard Brouer
>   MSc.CS, Principal Kernel Engineer at Red Hat
>   LinkedIn: http://www.linkedin.com/in/brouer