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

From: Baowen Zheng <baowen.zheng@corigine.com>
To: Jamal Hadi Salim <jhs@mojatatu.com>,
	Simon Horman <simon.horman@corigine.com>,
	Vlad Buslov <vladbu@nvidia.com>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Roi Dayan <roid@nvidia.com>, Ido Schimmel <idosch@nvidia.com>,
	Cong Wang <xiyou.wangcong@gmail.com>,
	Jiri Pirko <jiri@resnulli.us>,
	Baowen Zheng <notifications@github.com>,
	Louis Peens <louis.peens@corigine.com>,
	oss-drivers <oss-drivers@corigine.com>,
	Oz Shlomo <ozsh@nvidia.com>
Subject: RE: [RFC/PATCH net-next v3 8/8] flow_offload: validate flags of filter and actions
Date: Wed, 3 Nov 2021 14:48:40 +0000	[thread overview]
Message-ID: <DM5PR1301MB2172BBAA594802D4F8F88670E78C9@DM5PR1301MB2172.namprd13.prod.outlook.com> (raw)
In-Reply-To: <cd624f2b-a693-84eb-d3f4-81d869caad93@mojatatu.com>

On November 3, 2021 10:16 PM, Jamal Hadi Salim wrote:
>On 2021-11-03 10:03, Baowen Zheng wrote:
>> Thanks for your reply.
>> On November 3, 2021 9:34 PM, Jamal Hadi Salim wrote:
>>> On 2021-11-03 08:33, Jamal Hadi Salim wrote:
>>>> On 2021-11-03 07:30, Baowen Zheng wrote:
>>>>> On November 3, 2021 6:14 PM, Jamal Hadi Salim wrote:
>
>
>[..]
>
>> 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.
>> 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.
>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. 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. 
WDYT?