RE: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes

[Vinicius Costa Gomes]

Hi Po,

Po Liu <po.liu@xxxxxxx> writes:

> Hi Andre,
>
>
> Br,
> Po Liu
>
>> -----Original Message-----
>> From: Andre Guedes <andre.guedes@xxxxxxxxxxxxxxx>
>> Sent: 2019å12æ11æ 10:53
>> To: alexandru.ardelean@xxxxxxxxxx; allison@xxxxxxxxxxx; andrew@xxxxxxx;
>> ayal@xxxxxxxxxxxx; davem@xxxxxxxxxxxxx; f.fainelli@xxxxxxxxx;
>> gregkh@xxxxxxxxxxxxxxxxxxx; hauke.mehrtens@xxxxxxxxx;
>> hkallweit1@xxxxxxxxx; jiri@xxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
>> netdev@xxxxxxxxxxxxxxx; pablo@xxxxxxxxxxxxx; saeedm@xxxxxxxxxxxx;
>> tglx@xxxxxxxxxxxxx; Po Liu <po.liu@xxxxxxx>
>> Cc: vinicius.gomes@xxxxxxxxx; simon.horman@xxxxxxxxxxxxx; Claudiu Manoil
>> <claudiu.manoil@xxxxxxx>; Vladimir Oltean <vladimir.oltean@xxxxxxx>;
>> Alexandru Marginean <alexandru.marginean@xxxxxxx>; Xiaoliang Yang
>> <xiaoliang.yang_1@xxxxxxx>; Roy Zang <roy.zang@xxxxxxx>; Mingkai Hu
>> <mingkai.hu@xxxxxxx>; Jerry Huang <jerry.huang@xxxxxxx>; Leo Li
>> <leoyang.li@xxxxxxx>; Po Liu <po.liu@xxxxxxx>
>> Subject: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of
>> traffic classes
>> 
>> Caution: EXT Email
>> 
>> Hi Po,
>> 
>> Quoting Po Liu (2019-11-27 01:59:18)
>> > IEEE Std 802.1Qbu standard defined the frame preemption of port
>> > traffic classes. This patch introduce a method to set traffic classes
>> > preemption. Add a parameter 'preemption' in struct
>> > ethtool_link_settings. The value will be translated to a binary, each
>> > bit represent a traffic class. Bit "1" means preemptable traffic
>> > class. Bit "0" means express traffic class.  MSB represent high number
>> > traffic class.
>> >
>> > If hardware support the frame preemption, driver could set the
>> > ethernet device with hw_features and features with NETIF_F_PREEMPTION
>> > when initializing the port driver.
>> >
>> > User can check the feature 'tx-preemption' by command 'ethtool -k
>> > devname'. If hareware set preemption feature. The property would be a
>> > fixed value 'on' if hardware support the frame preemption.
>> > Feature would show a fixed value 'off' if hardware don't support the
>> > frame preemption.

Having some knobs in ethtool to enable when/how Frame Preemption is
advertised on the wire makes sense. I also agree that it should be "on"
by default.

>> >
>> > Command 'ethtool devname' and 'ethtool -s devname preemption N'
>> > would show/set which traffic classes are frame preemptable.
>> >
>> > Port driver would implement the frame preemption in the function
>> > get_link_ksettings() and set_link_ksettings() in the struct ethtool_ops.
>> 
>> In an early RFC series [1], we proposed a way to support frame preemption. I'm
>> not sure if you have considered it before implementing this other proposal
>> based on ethtool interface so I thought it would be a good idea to bring that up
>> to your attention, just in case.
>  
> Sorry, I didn't notice the RFC proposal. Using ethtool set the
> preemption just thinking about 8021Qbu as standalone. And not limit to
> the taprio if user won't set 802.1Qbv.

I see your point of using frame-preemption "standalone", I have two
ideas:

 1. add support in taprio to be configured without any schedule in the
 "full offload" mode. In practice, allowing taprio to work somewhat
 similar to (mqprio + frame-preemption), changes in the code should de
 fairly small;

 2. extend mqprio to support frame-preemption;

>
> As some feedback  also want to set the MAC merge minimal fragment size
> and get some more information of 802.3br.

The minimal fragment size, I guess, also makes sense to be kept in
ethtool. That is we have a sane default, and allow the user to change
this setting for special cases.

>
>> 
>> In that initial proposal, Frame Preemption feature is configured via taprio qdisc.
>> For example:
>> 
>> $ tc qdisc add dev IFACE parent root handle 100 taprio \
>>       num_tc 3 \
>>       map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
>>       queues 1@0 1@1 2@2 \
>>       preemption 0 1 1 1 \
>>       base-time 10000000 \
>>       sched-entry S 01 300000 \
>>       sched-entry S 02 300000 \
>>       sched-entry S 04 400000 \
>>       clockid CLOCK_TAI
>> 
>> It also aligns with the gate control operations Set-And-Hold-MAC and Set-And-
>> Release-MAC that can be set via 'sched-entry' (see Table 8.7 from
>> 802.1Q-2018 for further details.
>  
> I am curious about Set-And-Hold-Mac via 'sched-entry'. Actually, it
> could be understand as guardband by hardware preemption. MAC should
> auto calculate the nano seconds before  express entry slot start to
> break to two fragments. Set-And-Hold-MAC should minimal larger than
> the fragment-size oct times.

Another interesting point. My first idea is that when the schedule is
offloaded to the driver and the driver detects that the "entry" width is
smaller than the fragment side, the driver could reject that schedule
with a nice error message.

>
>> 
>> Please share your thoughts on this.
>
> I am good to see there is frame preemption proposal. Each way is ok
> for me but ethtool is more flexible. I've seen the RFC the code. The
> hardware offload is in the mainline, but preemption is not yet, I
> don't know why. Could you post it again?

It's not mainline because this kind of stuff will not be accepted
upstream without in-tree users. And you are the first one to propose
such a thing :-)

It's just now that I have something that supports frame-preemption, the
code I have is approaching RFC-like quality. I will send another RFC
this week hopefully, and we can see how things look in practice.


Cheers,
--
Vinicius