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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191115153247.GD2158@nanopsycho>
Date:   Fri, 15 Nov 2019 16:32:47 +0100
From:   Jiri Pirko <jiri@...nulli.us>
To:     Rahul Lakkireddy <rahul.lakkireddy@...lsio.com>
Cc:     netdev@...r.kernel.org, jakub.kicinski@...ronome.com,
        davem@...emloft.net, nirranjan@...lsio.com, vishal@...lsio.com,
        dt@...lsio.com
Subject: Re: [PATCH net-next v3 1/2] cxgb4: add TC-MATCHALL classifier egress
 offload

Fri, Nov 15, 2019 at 04:08:30PM CET, rahul.lakkireddy@...lsio.com 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, rahul.lakkireddy@...lsio.com 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