netdev - Re: [PATCH net-next v4 02/12] taprio: Add support for frame preemption 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] [day] [month] [year] [list]

Message-ID: <87h76zcezs.fsf@intel.com>
Date:   Mon, 11 Apr 2022 17:38:47 -0700
From:   Vinicius Costa Gomes <vinicius.gomes@...el.com>
To:     Vladimir Oltean <vladimir.oltean@....com>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "jhs@...atatu.com" <jhs@...atatu.com>,
        "xiyou.wangcong@...il.com" <xiyou.wangcong@...il.com>,
        "jiri@...nulli.us" <jiri@...nulli.us>,
        "kuba@...nel.org" <kuba@...nel.org>, Po Liu <po.liu@....com>,
        "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
        "anthony.l.nguyen@...el.com" <anthony.l.nguyen@...el.com>,
        "mkubecek@...e.cz" <mkubecek@...e.cz>
Subject: Re: [PATCH net-next v4 02/12] taprio: Add support for frame
 preemption offload

Vladimir Oltean <vladimir.oltean@....com> writes:

> On Mon, Apr 11, 2022 at 04:31:03PM -0700, Vinicius Costa Gomes wrote:
>> > First line in taprio_disable_offload() is:
>> >
>> > 	if (!FULL_OFFLOAD_IS_ENABLED(q->flags))
>> > 		return 0;
>> >
>> > but you said it yourself below that the preemptible queues thing is
>> > independent of whether you have taprio offload or not (or taprio at
>> > all). So the queues will never be reset back to the eMAC if you don't
>> > use full offload (yes, this includes txtime offload too). In fact, it's
>> > so independent, that I don't even know why we add them to taprio in the
>> > first place :)
>>
>> That I didn't change taprio_disable_offload() was a mistake caused in
>> part by the limitations of the hardware I have (I cannot have txtime
>> offload and frame preemption enabled at the same time), so I didn't
>> catch that.
>>
>> > I think the argument had to do with the hold/advance commands (other
>> > frame preemption stuff that's already in taprio), but those are really
>> > special and only to be used in the Qbv+Qbu combination, but the pMAC
>> > traffic classes? I don't know... Honestly I thought that me asking to
>> > see preemptible queues implemented for mqprio as well was going to
>> > discourage you, but oh well...
>>
>> Now, the real important part, if this should be communicated to the
>> driver via taprio or via ethtool/netlink.
>>
>> I don't really have strong opinions on this anymore, the two options are
>> viable/possible.
>>
>> This is going to be a niche feature, agreed, so thinking that going with
>> the one that gives the user more flexibility perhaps is best, i.e. using
>> ethtool/netlink to communicate which queues should be marked as
>> preemptible or express.
>
> So we're back at this, very well.
>
> I was just happening to be looking at clause 36 of 802.1Q (Priority Flow Control),
> a feature exchanged through DCBX where flows of a certain priority can be
> configured as lossless on a port, and generate PAUSE frames. This is essentially
> the extension of 802.3 annex 31B MAC Control PAUSE operation with the ability to
> enable/disable flow control on a per-priority basis.
>
> The priority in PFC (essentially synonymous with "traffic class") is the same
> priority as the priority in frame preemption. And you know how PFC is configured
> in Linux? Not through the qdisc, but through DCB_ATTR_PFC_CFG, a nested dcbnl
> netlink attribute with one nested u8 attribute per priority value
> (DCB_PFC_UP_ATTR_0 to DCB_PFC_UP_ATTR_7).
>
> Not saying we should follow the exact same model as PFC, just saying that I'm
> hard pressed to find a good reason why the "preemptable traffic classes"
> information should sit in a layer which is basically independent of the frame
> preemption feature itself.

Ok, going to take this as another point in favor of going the ethtool
route.


Thank you,
-- 
Vinicius