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 <[email protected]>
> ---
> 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.