On Tue, Sep 26, 2017 at 6:11 PM, John Hurley <john.hur...@netronome.com> wrote: > On Tue, Sep 26, 2017 at 3:12 PM, Or Gerlitz <gerlitz...@gmail.com> wrote: >> On Tue, Sep 26, 2017 at 4:58 PM, John Hurley <john.hur...@netronome.com> >> wrote: >>> On Mon, Sep 25, 2017 at 7:35 PM, Or Gerlitz <gerlitz...@gmail.com> wrote: >>>> On Mon, Sep 25, 2017 at 1:23 PM, Simon Horman >>>> <simon.hor...@netronome.com> wrote: >>>>> From: John Hurley <john.hur...@netronome.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. > > > The fl_init_dissector uses the FL_KEY_SET_IF_MASKED macro to set an > array of keys which are then translated to the used_keys values. > The FL_KEY_SET_IF_MASKED takes a 'struct fl_flow_key' as input and > checks if any mask bits are set in a particular field - if so it > eventually marks it as used. > In struct fl_flow_key, the encap ipv4 and ipv6 addresses are > represented as a union of the 2. > Therefore, if we have masked bits set for IPv4, they are also being > set for the IPv6 field.
I see, do you consider it a bug?
