RE: [PATCH/RFC net-next] tc: allow drivers to accept gact with PIPE when offloading

[Tianyu Yuan <tianyu.yuan@corigine.com>]

On Mon, Nov 29, 2022 at 8:35 PM , Eelco Chaudron wrote:
> 
> On 28 Nov 2022, at 14:33, Marcelo Leitner wrote:
> 
> > On Mon, Nov 28, 2022 at 02:17:40PM +0100, Eelco Chaudron wrote:
> >>
> >>
> >> On 28 Nov 2022, at 14:11, Marcelo Leitner wrote:
> >>
> >>> On Mon, Nov 28, 2022 at 07:11:05AM +0000, Tianyu Yuan wrote:
> > ...
> >>>>
> >>>> Furthermore, I think the current stats for each action mentioned in
> >>>> 2) cannot represent the real hw stats and this is why [ RFC
> >>>> net-next v2 0/2] (net: flow_offload: add support for per action hw stats)
> will come up.
> >>>
> >>> Exactly. Then, when this patchset (or similar) come up, it won't
> >>> update all actions with the same stats anymore. It will require a
> >>> set of stats from hw for the gact with PIPE action here. But if
> >>> drivers are ignoring this action, they can't have specific stats for
> >>> it. Or am I missing something?
> >>>
> >>> So it is better for the drivers to reject the whole flow instead of
> >>> simply ignoring it, and let vswitchd probe if it should or should
> >>> not use this action.
> >>
> >> Please note that OVS does not probe features per interface, but does it
> per datapath. So if it’s supported in pipe in tc software, we will use it. If the
> driver rejects it, we will probably end up with the tc software rule only.
> >
> > Ah right. I remember it will pick 1 interface for testing and use
> > those results everywhere, which then I don't know if it may or may not
> > be a representor port or not. Anyhow, then it should use skip_sw, to
> > try to probe for the offloading part. Otherwise I'm afraid tc sw will
> > always accept this flow and trick the probing, yes.
> 
> Well, it depends on how you look at it. In theory, we should be hardware
> agnostic, meaning what if you have different hardware in your system? OVS
> only supports global offload enablement.
> 
> Tianyu how are you planning to support this from the OVS side? How would
> you probe kernel and/or hardware support for this change?

Currently in the test demo, I just extend gact with PIPE (previously only SHOT as default and 
GOTO_CHAIN when chain exists), and then put such a gact with PIPE at the first place of each
filter which will be transacted with kernel tc.

About the tc sw datapath mentioned, we don't have to make changes because gact with PIPE
has already been supported in current tc implementation and it could act like a 'counter' And
for the hardware we just need to ignore this PIPE and the stats of this action will still be updated
in kernel side and sent to userspace.

I agree with that the unsupported actions should be rejected by drivers, so may another approach
could work without ignoring PIPE in all the related drivers, that we directly make put the flower stats
from driver into the socket which is used to transact with userspace and userspace(e.g. OVS) update
the flow stats using this stats instead of the parsing the action stats. How do you think of this?

Cheers,
Tianyu
> 
> //Eelco