Prepare fl_set_key_flags/fl_dump_key_flags() for use with
TCA_FLOWER_KEY_ENC_FLAGS{,_MASK}.

This patch adds an encap argument, similar to fl_set_key_ip/
fl_dump_key_ip(), and determine the flower keys based on the
encap argument, and use them in the rest of the two functions.

Since these functions are so far, only called with encap set false,
then there is no functional change.

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
 net/sched/cls_flower.c | 40 ++++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index eef570c577ac7..6a5cecfd95619 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -1166,19 +1166,28 @@ static void fl_set_key_flag(u32 flower_key, u32 flower_mask,
 	}
 }
 
-static int fl_set_key_flags(struct nlattr **tb, u32 *flags_key,
+static int fl_set_key_flags(struct nlattr **tb, bool encap, u32 *flags_key,
 			    u32 *flags_mask, struct netlink_ext_ack *extack)
 {
+	int fl_key, fl_mask;
 	u32 key, mask;
 
+	if (encap) {
+		fl_key = TCA_FLOWER_KEY_ENC_FLAGS;
+		fl_mask = TCA_FLOWER_KEY_ENC_FLAGS_MASK;
+	} else {
+		fl_key = TCA_FLOWER_KEY_FLAGS;
+		fl_mask = TCA_FLOWER_KEY_FLAGS_MASK;
+	}
+
 	/* mask is mandatory for flags */
-	if (!tb[TCA_FLOWER_KEY_FLAGS_MASK]) {
+	if (NL_REQ_ATTR_CHECK(extack, NULL, tb, fl_mask)) {
 		NL_SET_ERR_MSG(extack, "Missing flags mask");
 		return -EINVAL;
 	}
 
-	key = be32_to_cpu(nla_get_be32(tb[TCA_FLOWER_KEY_FLAGS]));
-	mask = be32_to_cpu(nla_get_be32(tb[TCA_FLOWER_KEY_FLAGS_MASK]));
+	key = be32_to_cpu(nla_get_be32(tb[fl_key]));
+	mask = be32_to_cpu(nla_get_be32(tb[fl_mask]));
 
 	*flags_key  = 0;
 	*flags_mask = 0;
@@ -2086,7 +2095,7 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
 		return ret;
 
 	if (tb[TCA_FLOWER_KEY_FLAGS]) {
-		ret = fl_set_key_flags(tb, &key->control.flags,
+		ret = fl_set_key_flags(tb, false, &key->control.flags,
 				       &mask->control.flags, extack);
 		if (ret)
 			return ret;
@@ -3084,12 +3093,22 @@ static void fl_get_key_flag(u32 dissector_key, u32 dissector_mask,
 	}
 }
 
-static int fl_dump_key_flags(struct sk_buff *skb, u32 flags_key, u32 flags_mask)
+static int fl_dump_key_flags(struct sk_buff *skb, bool encap,
+			     u32 flags_key, u32 flags_mask)
 {
-	u32 key, mask;
+	int fl_key, fl_mask;
 	__be32 _key, _mask;
+	u32 key, mask;
 	int err;
 
+	if (encap) {
+		fl_key = TCA_FLOWER_KEY_ENC_FLAGS;
+		fl_mask = TCA_FLOWER_KEY_ENC_FLAGS_MASK;
+	} else {
+		fl_key = TCA_FLOWER_KEY_FLAGS;
+		fl_mask = TCA_FLOWER_KEY_FLAGS_MASK;
+	}
+
 	if (!memchr_inv(&flags_mask, 0, sizeof(flags_mask)))
 		return 0;
 
@@ -3105,11 +3124,11 @@ static int fl_dump_key_flags(struct sk_buff *skb, u32 flags_key, u32 flags_mask)
 	_key = cpu_to_be32(key);
 	_mask = cpu_to_be32(mask);
 
-	err = nla_put(skb, TCA_FLOWER_KEY_FLAGS, 4, &_key);
+	err = nla_put(skb, fl_key, 4, &_key);
 	if (err)
 		return err;
 
-	return nla_put(skb, TCA_FLOWER_KEY_FLAGS_MASK, 4, &_mask);
+	return nla_put(skb, fl_mask, 4, &_mask);
 }
 
 static int fl_dump_key_geneve_opt(struct sk_buff *skb,
@@ -3632,7 +3651,8 @@ static int fl_dump_key(struct sk_buff *skb, struct net *net,
 	if (fl_dump_key_ct(skb, &key->ct, &mask->ct))
 		goto nla_put_failure;
 
-	if (fl_dump_key_flags(skb, key->control.flags, mask->control.flags))
+	if (fl_dump_key_flags(skb, false, key->control.flags,
+			      mask->control.flags))
 		goto nla_put_failure;
 
 	if (fl_dump_key_val(skb, &key->hash.hash, TCA_FLOWER_KEY_HASH,
-- 
2.45.1

hello Asbjørn,

some update on this work: I tested your patch after adapting iproute2
bits (e.g. using TCA_FLOWER_KEY_FLAGS_TUNNEL_<CSUM|DONT_FRAGMENT|OAM|CRIT>

from

https://lore.kernel.org/netdev/20240611235355.177667-2-ast@fiberby.net/

Now: functional tests on TCA_FLOWER_KEY_ENC_FLAGS systematically fail. I must
admit that I didn't complete 100% of the analysis, but IMO there is at least an
endianness problem here. See below:

On Tue, Jun 11, 2024 at 11:53:35PM +0000, Asbjørn Sloth Tønnesen wrote:
> Prepare fl_set_key_flags/fl_dump_key_flags() for use with
> TCA_FLOWER_KEY_ENC_FLAGS{,_MASK}.
> 
> This patch adds an encap argument, similar to fl_set_key_ip/
> fl_dump_key_ip(), and determine the flower keys based on the
> encap argument, and use them in the rest of the two functions.
> 
> Since these functions are so far, only called with encap set false,
> then there is no functional change.
> 
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
> ---
>  net/sched/cls_flower.c | 40 ++++++++++++++++++++++++++++++----------
>  1 file changed, 30 insertions(+), 10 deletions(-)
> 
> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> index eef570c577ac7..6a5cecfd95619 100644
> --- a/net/sched/cls_flower.c
> +++ b/net/sched/cls_flower.c
> @@ -1166,19 +1166,28 @@ static void fl_set_key_flag(u32 flower_key, u32 flower_mask,
>  	}
>  }
>  
> -static int fl_set_key_flags(struct nlattr **tb, u32 *flags_key,
> +static int fl_set_key_flags(struct nlattr **tb, bool encap, u32 *flags_key,
>  			    u32 *flags_mask, struct netlink_ext_ack *extack)
>  {
> +	int fl_key, fl_mask;
>  	u32 key, mask;
>  
> +	if (encap) {
> +		fl_key = TCA_FLOWER_KEY_ENC_FLAGS;
> +		fl_mask = TCA_FLOWER_KEY_ENC_FLAGS_MASK;
> +	} else {
> +		fl_key = TCA_FLOWER_KEY_FLAGS;
> +		fl_mask = TCA_FLOWER_KEY_FLAGS_MASK;
> +	}
> +
>  	/* mask is mandatory for flags */
> -	if (!tb[TCA_FLOWER_KEY_FLAGS_MASK]) {
> +	if (NL_REQ_ATTR_CHECK(extack, NULL, tb, fl_mask)) {
>  		NL_SET_ERR_MSG(extack, "Missing flags mask");
>  		return -EINVAL;
>  	}
>  
> -	key = be32_to_cpu(nla_get_be32(tb[TCA_FLOWER_KEY_FLAGS]));
> -	mask = be32_to_cpu(nla_get_be32(tb[TCA_FLOWER_KEY_FLAGS_MASK]));
> +	key = be32_to_cpu(nla_get_be32(tb[fl_key]));
> +	mask = be32_to_cpu(nla_get_be32(tb[fl_mask]));


I think that (at least) the above hunk is wrong - or at least, it is a
functional discontinuity that causes failure in my test. While the
previous bitmask storing tunnel control flags was in host byte ordering,
the information on IP fragmentation are stored in network byte ordering.

So, if we want to use this enum

--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -677,6 +677,11 @@ enum {
 enum {
 	TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT = (1 << 0),
 	TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST = (1 << 1),
+	/* FLOW_DIS_ENCAPSULATION (1 << 2) is not exposed to userspace */
+	TCA_FLOWER_KEY_FLAGS_TUNNEL_CSUM = (1 << 3),
+	TCA_FLOWER_KEY_FLAGS_TUNNEL_DONT_FRAGMENT = (1 << 4),
+	TCA_FLOWER_KEY_FLAGS_TUNNEL_OAM = (1 << 5),
+	TCA_FLOWER_KEY_FLAGS_TUNNEL_CRIT_OPT = (1 << 6),
 };
 
consistently, we should keep using network byte ordering for
TCA_FLOWER_KEY_FLAGS_TUNNEL_* flags (for a reason that I don't understand,
because metadata are not transmitted on wire. But maybe I'm missing something).

Shall I convert iproute2 to flip those bits like it happens for
TCA_FLOWER_KEY_FLAGS ? thanks!

-- 
davide

Hi Davide,

On 6/21/24 10:11 AM, Davide Caratti wrote:
> hello Asbjørn,
> 
> some update on this work: I tested your patch after adapting iproute2
> bits (e.g. using TCA_FLOWER_KEY_FLAGS_TUNNEL_<CSUM|DONT_FRAGMENT|OAM|CRIT>

Could you please post your iproute2 code?


> from
> 
> https://lore.kernel.org/netdev/20240611235355.177667-2-ast@fiberby.net/
> 
> Now: functional tests on TCA_FLOWER_KEY_ENC_FLAGS systematically fail. I must
> admit that I didn't complete 100% of the analysis, but IMO there is at least an
> endianness problem here. See below:
> 
> On Tue, Jun 11, 2024 at 11:53:35PM +0000, Asbjørn Sloth Tønnesen wrote:
>> Prepare fl_set_key_flags/fl_dump_key_flags() for use with
>> TCA_FLOWER_KEY_ENC_FLAGS{,_MASK}.
>>
>> This patch adds an encap argument, similar to fl_set_key_ip/
>> fl_dump_key_ip(), and determine the flower keys based on the
>> encap argument, and use them in the rest of the two functions.
>>
>> Since these functions are so far, only called with encap set false,
>> then there is no functional change.
>>
>> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
>> ---
>>   net/sched/cls_flower.c | 40 ++++++++++++++++++++++++++++++----------
>>   1 file changed, 30 insertions(+), 10 deletions(-)
>>
>> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
>> index eef570c577ac7..6a5cecfd95619 100644
>> --- a/net/sched/cls_flower.c
>> +++ b/net/sched/cls_flower.c
>> @@ -1166,19 +1166,28 @@ static void fl_set_key_flag(u32 flower_key, u32 flower_mask,
>>   	}
>>   }
>>   
>> -static int fl_set_key_flags(struct nlattr **tb, u32 *flags_key,
>> +static int fl_set_key_flags(struct nlattr **tb, bool encap, u32 *flags_key,
>>   			    u32 *flags_mask, struct netlink_ext_ack *extack)
>>   {
>> +	int fl_key, fl_mask;
>>   	u32 key, mask;
>>   
>> +	if (encap) {
>> +		fl_key = TCA_FLOWER_KEY_ENC_FLAGS;
>> +		fl_mask = TCA_FLOWER_KEY_ENC_FLAGS_MASK;
>> +	} else {
>> +		fl_key = TCA_FLOWER_KEY_FLAGS;
>> +		fl_mask = TCA_FLOWER_KEY_FLAGS_MASK;
>> +	}
>> +
>>   	/* mask is mandatory for flags */
>> -	if (!tb[TCA_FLOWER_KEY_FLAGS_MASK]) {
>> +	if (NL_REQ_ATTR_CHECK(extack, NULL, tb, fl_mask)) {
>>   		NL_SET_ERR_MSG(extack, "Missing flags mask");
>>   		return -EINVAL;
>>   	}
>>   
>> -	key = be32_to_cpu(nla_get_be32(tb[TCA_FLOWER_KEY_FLAGS]));
>> -	mask = be32_to_cpu(nla_get_be32(tb[TCA_FLOWER_KEY_FLAGS_MASK]));
>> +	key = be32_to_cpu(nla_get_be32(tb[fl_key]));
>> +	mask = be32_to_cpu(nla_get_be32(tb[fl_mask]));
> 
> 
> I think that (at least) the above hunk is wrong - or at least, it is a
> functional discontinuity that causes failure in my test. While the
> previous bitmask storing tunnel control flags was in host byte ordering,
> the information on IP fragmentation are stored in network byte ordering.
> 
> So, if we want to use this enum
> 
> --- a/include/uapi/linux/pkt_cls.h
> +++ b/include/uapi/linux/pkt_cls.h
> @@ -677,6 +677,11 @@ enum {
>   enum {
>   	TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT = (1 << 0),
>   	TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST = (1 << 1),
> +	/* FLOW_DIS_ENCAPSULATION (1 << 2) is not exposed to userspace */
> +	TCA_FLOWER_KEY_FLAGS_TUNNEL_CSUM = (1 << 3),
> +	TCA_FLOWER_KEY_FLAGS_TUNNEL_DONT_FRAGMENT = (1 << 4),
> +	TCA_FLOWER_KEY_FLAGS_TUNNEL_OAM = (1 << 5),
> +	TCA_FLOWER_KEY_FLAGS_TUNNEL_CRIT_OPT = (1 << 6),
>   };
>  
> consistently, we should keep using network byte ordering for
> TCA_FLOWER_KEY_FLAGS_TUNNEL_* flags (for a reason that I don't understand,
> because metadata are not transmitted on wire. But maybe I'm missing something).
> 
> Shall I convert iproute2 to flip those bits like it happens for
> TCA_FLOWER_KEY_FLAGS ? thanks!

It is always preferred to have a well-defined endianness for binary protocols, even
if it might only be used locally for now.

I would base it on flags_str in tc/f_flower.c, so it can reuse
flower_parse_matching_flags() and add a new "enc_flags" argument mirroring "ip_flags".
Then there shouldn't be a difference in endianness. The naming of "ip_flags" is
questionable, since it is only named in an IP-specific way in iproute2.

Most of the patches in this series are just extending and mirroring what is already
done for the existing flags, so I am pretty sure that part should work.

The part I am most unsure about is patch 5, since I don't have a lot of experience
with the bitmap infrastructure, I will make another reply to that patch.

-- 
Best regards
Asbjørn Sloth Tønnesen
Network Engineer
Fiberby - AS42541

hello Asbjørn,

thanks for your patience!

On Fri, Jun 21, 2024 at 02:45:28PM +0000, Asbjørn Sloth Tønnesen wrote:
> 
> Could you please post your iproute2 code?

sure, will clean it up and share it today in ML.
 
> > from
> > 
> > https://lore.kernel.org/netdev/20240611235355.177667-2-ast@fiberby.net/
> > 
> > Now: functional tests on TCA_FLOWER_KEY_ENC_FLAGS systematically fail. I must
> > admit that I didn't complete 100% of the analysis, but IMO there is at least an
> > endianness problem here. See below:
> > 
> > On Tue, Jun 11, 2024 at 11:53:35PM +0000, Asbjørn Sloth Tønnesen wrote:

[...]
 
> It is always preferred to have a well-defined endianness for binary protocols, even
> if it might only be used locally for now.

given the implementation of fl_set_key_flags() in patch 2,

	key = be32_to_cpu(nla_get_be32(tb[fl_key]));
	mask = be32_to_cpu(nla_get_be32(tb[fl_mask]));

when fl_key and fl_mask are TCA_FLOWER_KEY_ENC_FLAGS and TCA_FLOWER_KEY_ENC_FLAGS_MASK,
I assume that we want to turn them to network ordering, like it's already being done for
TCA_FLOWER_KEY_FLAGS and TCA_FLOWER_KEY_FLAGS_MASK.

So, we must htonl() the policy mask in the second hunk in patch 7,something like:

@@ -746,9 +746,9 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
 	[TCA_FLOWER_L2_MISS]		= NLA_POLICY_MAX(NLA_U8, 1),
 	[TCA_FLOWER_KEY_CFM]		= { .type = NLA_NESTED },
 	[TCA_FLOWER_KEY_ENC_FLAGS]	= NLA_POLICY_MASK(NLA_U32,
-							  TUNNEL_FLAGS_PRESENT),
+							  htonl(TCA_FLOWER_KEY_ENC_FLAGS_POLICY_MASK)),
 	[TCA_FLOWER_KEY_ENC_FLAGS_MASK]	= NLA_POLICY_MASK(NLA_U32,
-							  TUNNEL_FLAGS_PRESENT),
+							  htonl(TCA_FLOWER_KEY_ENC_FLAGS_POLICY_MASK)),
 };

And for the same reason, the flower code in patch 3 needs to be changed as follows:

@@ -676,8 +680,10 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
 	[TCA_FLOWER_KEY_ENC_UDP_SRC_PORT_MASK]	= { .type = NLA_U16 },
 	[TCA_FLOWER_KEY_ENC_UDP_DST_PORT]	= { .type = NLA_U16 },
 	[TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK]	= { .type = NLA_U16 },
-	[TCA_FLOWER_KEY_FLAGS]		= { .type = NLA_U32 },
-	[TCA_FLOWER_KEY_FLAGS_MASK]	= { .type = NLA_U32 },
+	[TCA_FLOWER_KEY_FLAGS]		= NLA_POLICY_MASK(NLA_U32,
+							  ntohl(TCA_FLOWER_KEY_FLAGS_POLICY_MASK)),
+	[TCA_FLOWER_KEY_FLAGS_MASK]	= NLA_POLICY_MASK(NLA_U32,
+							  ntohl(TCA_FLOWER_KEY_FLAGS_POLICY_MASK)),
 	[TCA_FLOWER_KEY_ICMPV4_TYPE]	= { .type = NLA_U8 },
 	[TCA_FLOWER_KEY_ICMPV4_TYPE_MASK] = { .type = NLA_U8 },
 	[TCA_FLOWER_KEY_ICMPV4_CODE]	= { .type = NLA_U8 },

Otherwise it will break the following use case (taken from tc_flower.sh kselftest):

# tc qdisc add dev lo clsact
# tc filter add dev lo ingress protocol ip pref 1 handle 101 flower ip_flags frag action continue
RTNETLINK answers: Invalid argument
We have an error talking to the kernel

because TCA_FLOWER_KEY_FLAGS_POLICY_MASK and TCA_FLOWER_KEY_ENC_FLAGS_POLICY_MASK
are in host byte order _ so netlink policy mask validation will fail unless we turn
the mask to network byte order.

(And I see we don't have a tdc selftest  for 'ip_flags', this might be a
good chance to add it :-) )

-- 
davide

On Wed, Jun 26, 2024 at 11:49 AM Davide Caratti <dcaratti@redhat.com> wrote:
>
> So, we must htonl() the policy mask in the second hunk in patch 7,something like:
>

or maybe better (but still untested), use NLA_BE32, like netfilter does in [1]

[1] https://elixir.bootlin.com/linux/latest/A/ident/NF_NAT_RANGE_MASK

-- 
davide

Hi Davide,

On 6/26/24 10:01 AM, Davide Caratti wrote:
> On Wed, Jun 26, 2024 at 11:49 AM Davide Caratti <dcaratti@redhat.com> wrote:
>>
>> So, we must htonl() the policy mask in the second hunk in patch 7,something like:

Good catch.

> or maybe better (but still untested), use NLA_BE32, like netfilter does in [1]
> 
> [1] https://elixir.bootlin.com/linux/latest/A/ident/NF_NAT_RANGE_MASK

Yes, that is better. It should work, as it triggers a htonl() in nla_validate_mask().

-- 
Best regards
Asbjørn Sloth Tønnesen
Network Engineer
Fiberby - AS42541

hello Asbjørn,

On Wed, Jun 26, 2024 at 11:55:31AM +0000, Asbjørn Sloth Tønnesen wrote:
> Hi Davide,
> 
> On 6/26/24 10:01 AM, Davide Caratti wrote:
> > On Wed, Jun 26, 2024 at 11:49 AM Davide Caratti <dcaratti@redhat.com> wrote:
> > > 
> > > So, we must htonl() the policy mask in the second hunk in patch 7,something like:
> 
> Good catch.
> 
> > or maybe better (but still untested), use NLA_BE32, like netfilter does in [1]
> > 
> > [1] https://elixir.bootlin.com/linux/latest/A/ident/NF_NAT_RANGE_MASK
> 
> Yes, that is better. It should work, as it triggers a htonl() in nla_validate_mask().

NLA_BE32 proved to fix the byte ordering problem:

 - it allows to set TCA_FLOWER_KEY_ENC_FLAGS_MASK and read it back consistently
 - it sets correct FLOW_DIS_F_* bits in 'enc_control'

FTR, I used this hunk on top of your RFC series:

-- >8 --
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -679,9 +679,9 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
        [TCA_FLOWER_KEY_ENC_UDP_SRC_PORT_MASK]  = { .type = NLA_U16 },
        [TCA_FLOWER_KEY_ENC_UDP_DST_PORT]       = { .type = NLA_U16 },
        [TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK]  = { .type = NLA_U16 },
-       [TCA_FLOWER_KEY_FLAGS]          = NLA_POLICY_MASK(NLA_U32,
+       [TCA_FLOWER_KEY_FLAGS]          = NLA_POLICY_MASK(NLA_BE32,
                                                          TCA_FLOWER_KEY_FLAGS_POLICY_MASK),
-       [TCA_FLOWER_KEY_FLAGS_MASK]     = NLA_POLICY_MASK(NLA_U32,
+       [TCA_FLOWER_KEY_FLAGS_MASK]     = NLA_POLICY_MASK(NLA_BE32,
                                                          TCA_FLOWER_KEY_FLAGS_POLICY_MASK),
        [TCA_FLOWER_KEY_ICMPV4_TYPE]    = { .type = NLA_U8 },
        [TCA_FLOWER_KEY_ICMPV4_TYPE_MASK] = { .type = NLA_U8 },
@@ -744,9 +744,9 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
        [TCA_FLOWER_KEY_SPI_MASK]       = { .type = NLA_U32 },
        [TCA_FLOWER_L2_MISS]            = NLA_POLICY_MAX(NLA_U8, 1),
        [TCA_FLOWER_KEY_CFM]            = { .type = NLA_NESTED },
-       [TCA_FLOWER_KEY_ENC_FLAGS]      = NLA_POLICY_MASK(NLA_U32,
+       [TCA_FLOWER_KEY_ENC_FLAGS]      = NLA_POLICY_MASK(NLA_BE32,
                                                          TCA_FLOWER_KEY_ENC_FLAGS_POLICY_MASK),
-       [TCA_FLOWER_KEY_ENC_FLAGS_MASK] = NLA_POLICY_MASK(NLA_U32,
+       [TCA_FLOWER_KEY_ENC_FLAGS_MASK] = NLA_POLICY_MASK(NLA_BE32,
                                                          TCA_FLOWER_KEY_ENC_FLAGS_POLICY_MASK),
 };

-- >8 --

but I think I found another small problem. You removed FLOW_DISSECTOR_KEY_ENC_FLAGS
from TC flower, re-using 'enc_control' instead; however, the FLOW_DISSECTOR_KEY_ENC_CONTROL
bit is set only if flower tries to match 'enc_ipv4' or 'enc_ipv6'. We don't notice
the problem with 'ip_flags' because AFAIS flow dissector copies those bits even with
no relevant FLOW_DISSECTOR_KEY* requested. When matching tunnel flags instead, we
will end up in skb_flow_dissect_tunne_info() with 


	/* A quick check to see if there might be something to do. */
	if (!dissector_uses_key(flow_dissector,
				FLOW_DISSECTOR_KEY_ENC_KEYID) &&
	    !dissector_uses_key(flow_dissector,
				FLOW_DISSECTOR_KEY_ENC_IPV4_ADDRS) &&
	    !dissector_uses_key(flow_dissector,
				FLOW_DISSECTOR_KEY_ENC_IPV6_ADDRS) &&
	    !dissector_uses_key(flow_dissector,
				FLOW_DISSECTOR_KEY_ENC_CONTROL) &&
	    !dissector_uses_key(flow_dissector,
				FLOW_DISSECTOR_KEY_ENC_PORTS) &&
	    !dissector_uses_key(flow_dissector,
				FLOW_DISSECTOR_KEY_ENC_IP) &&
	    !dissector_uses_key(flow_dissector,
				FLOW_DISSECTOR_KEY_ENC_OPTS))
		return;

 
^^ a kernel that returns without loading tunnel info, because "there is nothing
to do". So, the attempt to put a valid value in patch9 regardless of the address
family is not sufficient. IMO it can be fixed with the following hunk:

-- >8 --
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -2199,7 +2199,8 @@ static void fl_init_dissector(struct flow_dissector *dissector,
        FL_KEY_SET_IF_MASKED(mask, keys, cnt,
                             FLOW_DISSECTOR_KEY_ENC_IPV6_ADDRS, enc_ipv6);
        if (FL_KEY_IS_MASKED(mask, enc_ipv4) ||
-           FL_KEY_IS_MASKED(mask, enc_ipv6))
+           FL_KEY_IS_MASKED(mask, enc_ipv6) ||
+           FL_KEY_IS_MASKED(mask, enc_control))
                FL_KEY_SET(keys, cnt, FLOW_DISSECTOR_KEY_ENC_CONTROL,
                           enc_control);
        FL_KEY_SET_IF_MASKED(mask, keys, cnt,
-- >8 --

at least it passes my functional test (that I didn't send yet, together with
iproute bits :(  promise will do that now)

-- 
davide