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

On 2021-11-03 10:48, Baowen Zheng wrote:

On November 3, 2021 10:16 PM, Jamal Hadi Salim wrote:

quoted

On 2021-11-03 10:03, Baowen Zheng wrote:

quoted

Thanks for your reply.
On November 3, 2021 9:34 PM, Jamal Hadi Salim wrote:

quoted

On 2021-11-03 08:33, Jamal Hadi Salim wrote:

quoted

On 2021-11-03 07:30, Baowen Zheng wrote:

quoted

On November 3, 2021 6:14 PM, Jamal Hadi Salim wrote:

[..]

quoted

Sorry for more clarification about another case that Vlad mentioned:
#add a policer action with skip_hw
tc actions add action police skip_hw rate ... index 20 #Now add a
filter5 which has no flag tc filter add dev $DEV1 proto ip parent
ffff: flower \
         ip_proto icmp action police index 20 I think the filter5 could
be legal, since it will not run in hardware.
Driver will check failed when try to offload this filter. So the filter5 will only

run in software.

quoted

WDYT?

I think this one also has ambiguity. If the filter doesnt specify skip_sw or
skip_hw it will run both in s/w and h/w. I am worried if that looks suprising to
someone debugging after because in h/w there is filter 5 but no policer but in
s/w twin we have filter 5 and policer index 20.

In this case, the filter will not in h/w because when the driver tries to offload the filter,
It will found the action is not in h/w and return failed, then the filter will not in h/w, so the filter will only
In software.

So you have partial failure? That doesnt sound good. What do you return
to the user - "success" or "somehow success"?
I worry it is still ambigous. Did the user really intend to do that?
If they did maybe they should have just added it to s/w instead of h/w
and s/w and then get saved by the driver?

quoted

It could be design intent, but in my opinion we have priorities to resolve such
ambiguities in policies.

If we use the rule which says the flags have to match exactly then we can
simplify resolving any ambiguity - which will make it illegal, no?

When you mentioned " match exactly ", do you mean the flags of the filter and the actions should be
exactly same?
Please consider the case that filter has flag and the action does not have any flag.

See above.

I think we should allow this case.
Because it is legal before our patch, we do not expect to break this use case, yes?
So maybe the "match exactly" just limits action flags, when action has flags(skip_sw or skip_hw), the filter must have
exactly the same flags.

Maybe i am missing something but nothing should break.
I think what you mean is when the action is specified with
the filter. The flags should be the same in that case.

Example, filter 1:
tc filter add dev $DEV1 proto ip paren ffff: flower \
ip_proto icmp action police blah

where flag is 0 implies this filter goes both in h/w and s/w.
If i dump the policer or the filter i will see some index provided by
the kernel and i should be able to see both s/w and h/w
counters.

Same thing if i did:

Example filter 2:
tc filter add dev $DEV1 proto ip paren ffff: flower \
skip_sw ip_proto udp action police blah

both filter + action will have where flag of skip_sw when i dump
implies this filter goes only in h/w and any displayed index
is allocated by the kernel.


Our challenge is when someone specifies a specific action by index
and tries to use it ambigously.

cheers,
jamal