netdev - Re: [PATCH RFC] flow_dissector: Add FLOW_DISSECTOR_F

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] [day] [month] [year] [list]

Message-ID: <CALx6S356v5Ogwv0G3Pnok2FkFphNbkw-cRR5MRtQRDmV2_vMXA@mail.gmail.com>
Date:   Mon, 2 Oct 2017 09:05:10 -0700
From:   Tom Herbert <tom@...bertland.com>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     Tom Herbert <tom@...ntonium.net>,
        "David S. Miller" <davem@...emloft.net>,
        Hannes Frederic Sowa <hannes@...essinduktion.org>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Rohit Seth <rohit@...ntonium.net>
Subject: Re: [PATCH RFC] flow_dissector: Add FLOW_DISSECTOR_F_FLOWER

On Mon, Oct 2, 2017 at 7:49 AM, Jiri Pirko <jiri@...nulli.us> wrote:
> Fri, Sep 29, 2017 at 09:13:42PM CEST, tom@...ntonium.net wrote:
>>This patch is RFC and would be applied after "flow_dissector:
>>Protocol specific flow dissector offload"
>>
>>In order to maitain uAPI in flower, the FLOW_DISSECTOR_F_FLOWER flag
>>is added to indicate to flow_dissector that the caller is flower.
>>As new funtionality is addes to flow_dissector that would break
>>the flower uAPI, the code can be wrapped in "if (!(flags &
>>FLOW_DISSECTOR_F_FLOWER)).
>>
>>In this patch the conditional is use around protocol specific
>>dissection (e.g. DPI into VXLAN) as well as the code that
>>enforces a depth of parsing to prevent DPI. The latter was a
>>recent patch that would introduce a parsing limit to flower that
>>did not exist before (i.e. would break uAPI).
>>
>>Signed-off-by: Tom Herbert <tom@...ntonium.net>
>>---
>> include/net/flow_dissector.h |  1 +
>> net/core/flow_dissector.c    | 17 +++++++++++------
>> net/sched/cls_flow.c         |  3 ++-
>> 3 files changed, 14 insertions(+), 7 deletions(-)
>>
>>diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
>>index ad75bbfd1c9c..ca315107d147 100644
>>--- a/include/net/flow_dissector.h
>>+++ b/include/net/flow_dissector.h
>>@@ -214,6 +214,7 @@ enum flow_dissector_key_id {
>> #define FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL   BIT(2)
>> #define FLOW_DISSECTOR_F_STOP_AT_ENCAP                BIT(3)
>> #define FLOW_DISSECTOR_F_STOP_AT_L4           BIT(4)
>>+#define FLOW_DISSECTOR_F_FLOWER                       BIT(5)
>
> I don't like flow_dissector to have any user-specific bits. Note that
> the same dissection may be used not only from flower, but from other
> code as well (OVS). Flow dissector should not care who the caller is.

I agree with that, but unfortunately that's now how it works in
reality. As pointed out flower has assumed flow_dissector semantics as
its uAPI, so we can't change flow dissector with out considering this
one specific caller even if all other use cases of flow dissector
don't care.

If you don't like this approach, then please suggest an alternative
that will achieve the same effect.

Thanks,
Tom