Subject: Re: [PATCH net-next v3 1/2] cxgb4: add TC-MATCHALL classifier egress offload

From: Jiri Pirko <[email-masked]>
Date: Fri, 15 Nov 2019 16:32:47 +0100

Fri, Nov 15, 2019 at 04:08:30PM CET, [email-masked] wrote:
>On Friday, November 11/15/19, 2019 at 14:58:45 +0100, Jiri Pirko wrote:
>> Fri, Nov 15, 2019 at 01:14:20PM CET, [email-masked] wrote:
>> 
>> [...]
>> 
>> 
>> >+static int cxgb4_matchall_egress_validate(struct net_device *dev,
>> >+					  struct tc_cls_matchall_offload *cls)
>> >+{
>> >+	struct netlink_ext_ack *extack = cls->common.extack;
>> >+	struct flow_action *actions = &cls->rule->action;
>> >+	struct port_info *pi = netdev2pinfo(dev);
>> >+	struct flow_action_entry *entry;
>> >+	u64 max_link_rate;
>> >+	u32 i, speed;
>> >+	int ret;
>> >+
>> >+	if (cls->common.prio != 1) {
>> >+		NL_SET_ERR_MSG_MOD(extack,
>> >+				   "Egress MATCHALL offload must have prio 1");
>> 
>> I don't understand why you need it to be prio 1.
>
>This is to maintain rule ordering with the kernel. Jakub has suggested
>this in my earlier series [1][2]. I see similar checks in various
>drivers (mlx5 and nfp), while offloading matchall with policer.

I don't think that is correct. If matchall is the only filter there, it
does not matter which prio is it. It matters only in case there are
other filters.

The code should just check for other filters and forbid to insert the
rule if other filters have higher prio (lower number).

>
>[1] http://patchwork.ozlabs.org/patch/1194936/#2304413
>[2] http://patchwork.ozlabs.org/patch/1194301/#2303749
>
>Thanks,
>Rahul