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: <62E2DD49-AC21-46DE-8E5B-EBC67230FA7D@intel.com>
Date:   Wed, 29 May 2019 17:06:49 +0000
From:   "Patel, Vedang" <vedang.patel@...el.com>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Cong Wang <xiyou.wangcong@...il.com>,
        "jiri@...nulli.us" <jiri@...nulli.us>,
        "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
        "Gomes, Vinicius" <vinicius.gomes@...el.com>,
        "l@...ileo.org" <l@...ileo.org>
Subject: Re: [PATCH net-next v1 3/7] taprio: Add the skeleton to enable
 hardware offloading

Thanks Jacub for they inputs.

> On May 28, 2019, at 3:45 PM, Jakub Kicinski <jakub.kicinski@...ronome.com> wrote:
> 
> On Tue, 28 May 2019 10:46:44 -0700, Vedang Patel wrote:
>> From: Vinicius Costa Gomes <vinicius.gomes@...el.com>
>> 
>> This adds the UAPI and the core bits necessary for userspace to
>> request hardware offloading to be enabled.
>> 
>> The future commits will enable hybrid or full offloading for taprio. This
>> commit sets up the infrastructure to enable it via the netlink interface.
>> 
>> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@...el.com>
>> Signed-off-by: Vedang Patel <vedang.patel@...el.com>
> 
> Other qdiscs offload by default, this offload-level selection here is a
> little bit inconsistent with that :(
> 
There are 2 different offload modes introduced in this patch.

1. Txtime offload mode: This mode depends on skip_sock_check flag being set in the etf qdisc. Also, it requires some manual configuration which might be specific to the network adapter card. For example, for the i210 card, the user will have to route all the traffic to the highest priority queue and install etf qdisc with offload enabled on that queue. So, I don’t think this mode should be enabled by default.
2. Full offload mode: This mode is currently not supported by any network driver. The support for this will be coming soon. But, we can enable this mode by default. 

Also, from what Vinicius tells me, offload modes for cbs, etf and mqprio are also disabled by default. So, it will make more sense to keep it disabled to be consistent with other qdiscs similar to this one.
>> @@ -731,6 +857,9 @@ static int taprio_change(struct Qdisc *sch, struct nlattr *opt,
>> 	if (err < 0)
>> 		return err;
>> 
>> +	if (tb[TCA_TAPRIO_ATTR_OFFLOAD_FLAGS])
>> +		offload_flags = nla_get_u32(tb[TCA_TAPRIO_ATTR_OFFLOAD_FLAGS]);
> 
> You should make sure user doesn't set unknown bits.  Otherwise using
> other bits will not be possible in the future.
> 
Yes, I agree here, will include this in the next patchset.

Thanks,
Vedang
>> 	new_admin = kzalloc(sizeof(*new_admin), GFP_KERNEL);
>> 	if (!new_admin) {
>> 		NL_SET_ERR_MSG(extack, "Not enough memory for a new schedule");

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ