On Sun, 22 Sep 2019 14:51:44 +0300, Paul Blakey wrote:
> The skb extension is currently used for miss path of software offloading OvS 
> rules with recirculation to tc.
> However, we are also preparing patches to support the hardware side of things.
> 
> After the userspace OvS patches to support connection tracking, we'll have 
> two users for
> tc multi chain rules, one from those actually using tc and those translated 
> from OvS rules.
> 
> With both of these use cases, there is similar issue of 'miss', from hardware 
> to tc and tc to OvS and the skb
> extension will serve to recover from both.
> 
> Consider, for example, the following multi chain tc rules:
> 
> tc filter add dev1 ... chain 0 flower dst_mac aa:bb:cc:dd:ee:ff action pedit 
> munge ex set mac dst 02:a0:98:4e:8f:d1 pipe action goto chain 1
> tc filter add dev1 ... chain 1 flower ip_dst 1.1.1.1 action mirred egress 
> redirect dev2
> tc filter add dev1 ... chain 1 flower ip_dst 2.2.2.2 action mirred egress 
> redirect dev2
> 
> It's possible we offload the first two rules, but fail to offload the third 
> rule, because of some hardware failure (e.g unsupported match or action).
> If a packet with (dst_mac=aa:bb:cc:dd:ee:ff) and (dst ip=2.2.2.2) arrives,
> we execute the goto chain 1 in hardware (and the pedit), and continue in 
> chain 1, where we miss.
> 
> Currently we re-start the processing in tc from chain 0, even though we 
> already did part of the processing in hardware.
> The match on the dst_mac will fail as we already modified it, and we won't 
> execute the third rule action.
> In addition, if we did manage to execute any software tc rules, the packet 
> will be counted twice.
> 
> We'll re-use this extension to solve this issue that currently exists by 
> letting drivers tell tc on which chain to start the classification.
> 
> Regarding the config, we suggest changing the default to N and letting users 
> decide to enable it, see attached patch.

Partial offloads are very hard. Could we possibly take a page out of
routing offload's book and do a all or nothing offload in presence of
multiple chains?

> ------------------------------------------------------------
> 
> Subject: [PATCH net-next] net/sched: Set default of CONFIG_NET_TC_SKB_EXT to N
> 
> This a new feature, it is prefered that it defaults to N.
> 
> Fixes: 95a7233c452a ('net: openvswitch: Set OvS recirc_id from tc chain 
> index')
> Signed-off-by: Paul Blakey <pa...@mellanox.com>
> ---
>  net/sched/Kconfig | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/net/sched/Kconfig b/net/sched/Kconfig
> index b3faafe..4bb10b7 100644
> --- a/net/sched/Kconfig
> +++ b/net/sched/Kconfig
> @@ -966,7 +966,6 @@ config NET_IFE_SKBTCINDEX
>  config NET_TC_SKB_EXT
>       bool "TC recirculation support"
>       depends on NET_CLS_ACT
> -     default y if NET_CLS_ACT
>       select SKB_EXTENSIONS
>  
>       help

 - Linus suggested we hide this option from user and autoselect if
   possible.
 - As Daniel said all distros will enable this.
 - If correctness is really our concern, giving users an option to
   select whether they want something to behave correctly seems like
   a hack of lowest kind. Perhaps it's time to add a CONFIG for TC
   offload in general? That's a meaningful set of functionality.

Reply via email to