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.