Re: [PATCH net-next] flow_dissector: Add IPv6 flow label to symmetric keys

From: Tom Herbert <tom@herbertland.com>
To: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Cc: David Miller <davem@davemloft.net>,
	Network Development <netdev@vger.kernel.org>
Subject: Re: [PATCH net-next] flow_dissector: Add IPv6 flow label to symmetric keys
Date: Mon, 17 Aug 2020 07:44:04 -0700	[thread overview]
Message-ID: <CALx6S36=C6P2khdWcZXfi3Cg4WktFBWUe_2U8L6=-vPD-ZZ81A@mail.gmail.com> (raw)
In-Reply-To: <CA+FuTSfw+XT9kQ4y-dY_M8zc2TksCSSEa-0WY+muyv_yr9_H=A@mail.gmail.com>

On Wed, Aug 5, 2020 at 1:27 AM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> On Tue, Aug 4, 2020 at 5:57 PM Tom Herbert <tom@herbertland.com> wrote:
> >
> > The definition for symmetric keys does not include the flow label so
> > that when symmetric keys is used a non-zero IPv6 flow label is not
> > extracted. Symmetric keys are used in functions to compute the flow
> > hash for packets, and these functions also set "stop at flow label".
> > The upshot is that for IPv6 packets with a non-zero flow label, hashes
> > are only based on the address two tuple and there is no input entropy
> > from transport layer information. This patch fixes this bug.
>
> If this is a bug fix, it should probably target net and have a Fixes tag.
>
> Should the actual fix be to remove the
> FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL argument from
> __skb_get_hash_symmetric?
>
> The original commit mentions the symmetric key flow dissector to
> compute a flat symmetric hash over only the protocol, addresses and
> ports.
>
> Autoflowlabel uses symmetric __get_hash_from_flowi6 to derive a flow
> label, but this cannot be generally relied on. RFC 6437 suggests
> even a PRNG as input, for instance.

Willem,

Yes, the "reliability" of flow labels argument is brought up often
especially by routing vendors that seemingly want to continue doing
their expensive DPI. I'm not exactly what "reliability" means in this
context. I think it refers to the fact that some network devices might
change the flow label in the middle of a flow (AFAIK we squashed all
the instances in Linux stack where flow labels can change mid flow).
In reality, our ability to do "reliable" DPI is dwindling anyway due
to crypto, UDP encapsulation, and transports like QUIC that don't
identify flows solely based 4-tuple.

In any case, IMO the flow label should be part of the input entropy to
the hash even if we do DPI. It's sufficient entropy along with the
IPv6 addresses to produce a flow hash with conformant properties in
more cases than DPI alone can provide due to the limitations above.
One obvious problem with DPI is cost, especially in cases we need to
parse over a bunch of headers just to locate transport ports (i.e.
potential DOS). If the cost is shown negligible or there's other
benefits then maybe we could eliminate
FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL.

Tom