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. > >> If it is found to be IPv6 when the rule is parsed, it will be rejected here.
