Re: [RFC/PATCH net-next v3 8/8] flow_offload: validate flags of filter and actions

On 2021-10-30 22:27, Baowen Zheng wrote:

Thanks for your review, after some considerarion, I think I understand what you are meaning.

[..]

quoted

quoted

quoted

I know Jamal suggested to have skip_sw for actions, but it complicates
the code and I'm still not entirely understand why it is necessary.

If the hardware can independently accept an action offload then
skip_sw per action makes total sense. BTW, my understanding is

Example configuration that seems bizarre to me is when offloaded shared
action has skip_sw flag set but filter doesn't. Then behavior of
classifier that points to such action diverges between hardware and
software (different lists of actions are applied). We always try to make
offloaded TC data path behave exactly the same as software and, even
though here it would be explicit and deliberate, I don't see any
practical use-case for this.

We add the skip_sw to keep compatible with the filter flags and give the user an
option to specify if the action should run in software. I understand what you mean,
maybe our example is not proper, we need to prevent the filter to run in software if the
actions it applies is skip_sw, so we need to add more validation to check about this.
Also I think your suggestion makes full sense if there is no use case to specify the action
should not run in sw and indeed it will make our implement more simple if we omit the
skip_sw option.
Jamal, WDYT?

Let me use an example to illustrate my concern:

#add a policer offload it
tc actions add action police skip_sw rate ... index 20
#now add filter1 which is offloaded
tc filter add dev $DEV1 proto ip parent ffff: flower \
    skip_sw ip_proto tcp action police index 20
#add filter2 likewise offloaded
tc filter add dev $DEV1 proto ip parent ffff: flower \
    skip_sw ip_proto udp action police index 20

All good so far...
#Now add a filter3 which is s/w only
tc filter add dev $DEV1 proto ip parent ffff: flower \
    skip_hw ip_proto icmp action police index 20

filter3 should not be allowed.

If we had added the policer without skip_sw and without
skip_hw then i think filter3 should have been legal
(we just need to account for stats in_hw vs in_sw).

Not sure if that makes sense (and addresses Vlad's earlier
comment).


cheers,
jamal