netdev - Re: [PATCH net-next 2/7] nfp: compile flower vxlan tunnel metadata match fields

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:   Tue, 26 Sep 2017 17:12:51 +0300
From:   Or Gerlitz <gerlitz.or@...il.com>
To:     John Hurley <john.hurley@...ronome.com>
Cc:     Simon Horman <simon.horman@...ronome.com>,
        David Miller <davem@...emloft.net>,
        Jakub Kicinski <jakub.kicinski@...ronome.com>,
        Linux Netdev List <netdev@...r.kernel.org>,
        oss-drivers@...ronome.com
Subject: Re: [PATCH net-next 2/7] nfp: compile flower vxlan tunnel metadata
 match fields

On Tue, Sep 26, 2017 at 4:58 PM, John Hurley <john.hurley@...ronome.com> wrote:
> On Mon, Sep 25, 2017 at 7:35 PM, Or Gerlitz <gerlitz.or@...il.com> wrote:
>> On Mon, Sep 25, 2017 at 1:23 PM, Simon Horman
>> <simon.horman@...ronome.com> wrote:
>>> From: John Hurley <john.hurley@...ronome.com>
>>>
>>> Compile ovs-tc flower vxlan metadata match fields for offloading. Only
>>
>> anything in the npf kernel bits has direct relation to ovs? what?
>>
>
> Sorry, this is a typo  and should refer to TC.
>
>>> +++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c
>>> @@ -52,8 +52,25 @@
>>>          BIT(FLOW_DISSECTOR_KEY_PORTS) | \
>>>          BIT(FLOW_DISSECTOR_KEY_ETH_ADDRS) | \
>>>          BIT(FLOW_DISSECTOR_KEY_VLAN) | \
>>> +        BIT(FLOW_DISSECTOR_KEY_ENC_KEYID) | \
>>> +        BIT(FLOW_DISSECTOR_KEY_ENC_IPV4_ADDRS) | \
>>> +        BIT(FLOW_DISSECTOR_KEY_ENC_IPV6_ADDRS) | \
>>
>> this series takes care of IPv6 tunnels too?
>
> IPv6 is not included in this set.
> The reason the IPv6 bit is included here is to account for behavior we
> have noticed in TC flower.
> If, for example, I add a filter with the following match fields:
> 'protocol ip flower enc_src_ip 10.0.0.1 enc_dst_ip 10.0.0.2
> enc_dst_port 4789 enc_key_id 123'
> The 'used_keys' value in the dissector marks both IPv4 and IPv6 encap
> addresses as 'used'.
> I am not sure if this is a bug in TC or that we are expected to check
> the enc_control fields to determine if IPv4 or v6 addresses are used.

you should have your code to check enc_control->addr_type to be
FLOW_DISSECTOR_KEY_IPV4_ADDRS or IPV6_ADDRS


> Including the IPv6 used_keys bit in our whitelist approach allows us
> to accept legitimate IPv4 tunnel rules in these situations.

mmm can please take a look on fl_init_dissector() and tell me if you
see why FLOW_DISSECTOR_KEY_IPV6_ADDRS is set for ipv4 tunnels,
I am not sure.

> If it is found to be IPv6 when the rule is parsed, it will be rejected here.