2019-12-19 00:44:27

by Ivan Khoronzhuk

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

On Mon, Dec 16, 2019 at 01:44:13PM -0800, Vinicius Costa Gomes wrote:
>Hi Po,
>
>Po Liu <[email protected]> writes:
>
>> Hi Andre,
>>
>>
>> Br,
>> Po Liu
>>
>>> -----Original Message-----
>>> From: Andre Guedes <[email protected]>
>>> Sent: 2019年12月11日 10:53
>>> To: [email protected]; [email protected]; [email protected];
>>> [email protected]; [email protected]; [email protected];
>>> [email protected]; [email protected];
>>> [email protected]; [email protected]; [email protected];
>>> [email protected]; [email protected]; [email protected];
>>> [email protected]; Po Liu <[email protected]>
>>> Cc: [email protected]; [email protected]; Claudiu Manoil
>>> <[email protected]>; Vladimir Oltean <[email protected]>;
>>> Alexandru Marginean <[email protected]>; Xiaoliang Yang
>>> <[email protected]>; Roy Zang <[email protected]>; Mingkai Hu
>>> <[email protected]>; Jerry Huang <[email protected]>; Leo Li
>>> <[email protected]>; Po Liu <[email protected]>
>>> 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;

+

And if follow mqprio settings logic then preemption also can be enabled
immediately while configuring taprio first time, and similarly new ADMIN
can't change it and can be set w/o preemption option afterwards.

So that following is correct:

OPER
$ tc qdisc add dev IFACE parent root handle 100 taprio \
base-time 10000000 \
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
flags 1

then
ADMIN
$ tc qdisc add dev IFACE parent root handle 100 taprio \
base-time 12000000 \
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
sched-entry S 01 300000 \
sched-entry S 02 300000 \
flags 1

then
ADMIN
$ tc qdisc add dev IFACE parent root handle 100 taprio \
base-time 13000000 \
sched-entry S 01 300000 \
sched-entry S 02 300000 \
flags 1

BUT:

1) The question is only should it be in this way? I mean preemption to be
enabled immediately? Also should include other parameters like fragment size.

2) What if I want to use frame preemption with another "transmission selection
algorithm"? Say another one "time sensitive" - CBS? How is it going to be
stacked?

In this case ethtool looks better, allowing this "MAC level" feature, to be
configured separately.

>
> 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.

Looks ok, if entry command is RELEASE or SET only, but not HOLD, and
only if it contains express queues. And if for entry is expectable to have
interval shorter, the entry has to be marked as HOLD then.

But not every offload is able to support mac/hold per sched (there is
no HOLD/RELEASE commands in this case). For this case seems like here can
be 2 cases:

1) there is no “gate close” event for the preemptible traffic
2) there is "gate close" event for the preemptable traffic

And both can have the following impact, if assume the main reason to
this guard check is to guarantee the express queue cannot be blocked while
this "close to short" interval opening ofc:

If a preemption fragment is started before "express" frame, then interval
should allow to complete preemption fragment and has to have enough time
to insert express frame. So here situation when maximum packet size per
each queue can have place.

In case of TI am65 this queue MTU is configurable per queue (for similar
reasons and couple more (packet fill feature for instance)) and can be
used for guard check also, but not clear where it should be. Seems like
it should be done using ethtool also, but can be needed for taprio
interface....

>>
>>>
>>> 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

--
Regards,
Ivan Khoronzhuk


2019-12-19 01:54:52

by Vinicius Costa Gomes

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

Hi Ivan,

Ivan Khoronzhuk <[email protected]> writes:

>>>> 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;
>
> +
>
> And if follow mqprio settings logic then preemption also can be enabled
> immediately while configuring taprio first time, and similarly new ADMIN
> can't change it and can be set w/o preemption option afterwards.
>
> So that following is correct:
>
> OPER
> $ tc qdisc add dev IFACE parent root handle 100 taprio \
> base-time 10000000 \
> 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
> flags 1
>
> then
> ADMIN
> $ tc qdisc add dev IFACE parent root handle 100 taprio \
> base-time 12000000 \
> 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
> sched-entry S 01 300000 \
> sched-entry S 02 300000 \
> flags 1
>
> then
> ADMIN
> $ tc qdisc add dev IFACE parent root handle 100 taprio \
> base-time 13000000 \
> sched-entry S 01 300000 \
> sched-entry S 02 300000 \
> flags 1
>
> BUT:
>
> 1) The question is only should it be in this way? I mean preemption to be
> enabled immediately? Also should include other parameters like
> fragment size.

We can decide what things are allowed/useful here. For example, it might
make sense to allow "preemption" to be changed. We can extend taprio to
support changing the fragment size, if that makes sense.

>
> 2) What if I want to use frame preemption with another "transmission selection
> algorithm"? Say another one "time sensitive" - CBS? How is it going to be
> stacked?

I am not seeing any (conceptual*) problems when plugging a cbs (for
example) qdisc into one of taprio children. Or, are you talking about a
more general problem?

* here I am considering that support for taprio without an schedule is
added.

>
> In this case ethtool looks better, allowing this "MAC level" feature, to be
> configured separately.

My only issue with using ethtool is that then we would have two
different interfaces for "complementary" features. And it would make
things even harder to configure and debug. The fact that one talks about
traffic classes and the other transmission queues doesn't make me more
comfortable as well.

On the other hand, as there isn't a way to implement frame preemption in
software, I agree that it makes it kind of awkward to have it in the tc
subsystem.

At this point, I am slightly in favor of the taprio approach (yes, I am
biased :-), but I can be convinced otherwise. I will be only a little
sad if we choose to go with ethtool for now, and then add support up in
the stack, something similar to "ethtool -N" and "tc-flower".

>
>>
>> 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.
>
> Looks ok, if entry command is RELEASE or SET only, but not HOLD, and
> only if it contains express queues. And if for entry is expectable to have
> interval shorter, the entry has to be marked as HOLD then.
>
> But not every offload is able to support mac/hold per sched (there is
> no HOLD/RELEASE commands in this case). For this case seems like here can
> be 2 cases:

Yeah, the hw I have in hand also doesn't support the HOLD/RELEASE
commands.

>
> 1) there is no “gate close” event for the preemptible traffic
> 2) there is "gate close" event for the preemptable traffic
>
> And both can have the following impact, if assume the main reason to
> this guard check is to guarantee the express queue cannot be blocked while
> this "close to short" interval opening ofc:
>
> If a preemption fragment is started before "express" frame, then interval
> should allow to complete preemption fragment and has to have enough time
> to insert express frame. So here situation when maximum packet size per
> each queue can have place.
>
> In case of TI am65 this queue MTU is configurable per queue (for similar
> reasons and couple more (packet fill feature for instance)) and can be
> used for guard check also, but not clear where it should be. Seems like
> it should be done using ethtool also, but can be needed for taprio
> interface....

For now, at least for the hardware I am working on, something like this
is configurable, but I wasn't planning on exposing it, using the maximum
ethernet frame size seemed a good default.

>
>>>
>>>>
>>>> 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
>
> --
> Regards,
> Ivan Khoronzhuk

Cheers,
--
Vinicius

2019-12-30 16:52:14

by Karicheri, Muralidharan

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

Hi Vinicius,

On 12/18/2019 08:54 PM, Vinicius Costa Gomes wrote:
> Hi Ivan,
>
> Ivan Khoronzhuk <[email protected]> writes:
>
>>>>> 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;
>>
>> +
>>
>> And if follow mqprio settings logic then preemption also can be enabled
>> immediately while configuring taprio first time, and similarly new ADMIN
>> can't change it and can be set w/o preemption option afterwards.
>>
>> So that following is correct:
>>
>> OPER
>> $ tc qdisc add dev IFACE parent root handle 100 taprio \
>> base-time 10000000 \
>> 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
>> flags 1
>>
>> then
>> ADMIN
>> $ tc qdisc add dev IFACE parent root handle 100 taprio \
>> base-time 12000000 \
>> 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
>> sched-entry S 01 300000 \
>> sched-entry S 02 300000 \
>> flags 1
>>
>> then
>> ADMIN
>> $ tc qdisc add dev IFACE parent root handle 100 taprio \
>> base-time 13000000 \
>> sched-entry S 01 300000 \
>> sched-entry S 02 300000 \
>> flags 1
>>
>> BUT:
>>
>> 1) The question is only should it be in this way? I mean preemption to be
>> enabled immediately? Also should include other parameters like
>> fragment size.
>
> We can decide what things are allowed/useful here. For example, it might
> make sense to allow "preemption" to be changed. We can extend taprio to
> support changing the fragment size, if that makes sense.
>
The point is it make sense to configure pre-emption related parameters
independently of taprio since they are not related. User may use just
pre-emption to reduce latency in the communication path for their end
application.. So there should be a way to configure it independently of
taprio. Right??

>>
>> 2) What if I want to use frame preemption with another "transmission selection
>> algorithm"? Say another one "time sensitive" - CBS? How is it going to be
>> stacked?
>
> I am not seeing any (conceptual*) problems when plugging a cbs (for
> example) qdisc into one of taprio children. Or, are you talking about a
> more general problem?
>

If I understand it correctly problem is not stacking taprio with cbs,
but rather pre-emption with other qdiscs and allow configuring
the parameters such as frag size. How do I use frame pre-emption as
an independent feature and configure frag size? Ethool appears to be
better from this point of view as Ivan has mentioned below.

Murali

> * here I am considering that support for taprio without an schedule is
> added.
>
>>
>> In this case ethtool looks better, allowing this "MAC level" feature, to be
>> configured separately.
>
> My only issue with using ethtool is that then we would have two
> different interfaces for "complementary" features. And it would make
> things even harder to configure and debug. The fact that one talks about
> traffic classes and the other transmission queues doesn't make me more
> comfortable as well.
>
> On the other hand, as there isn't a way to implement frame preemption in
> software, I agree that it makes it kind of awkward to have it in the tc
> subsystem.
>
> At this point, I am slightly in favor of the taprio approach (yes, I am
> biased :-), but I can be convinced otherwise. I will be only a little
> sad if we choose to go with ethtool for now, and then add support up in
> the stack, something similar to "ethtool -N" and "tc-flower".
>
>>
>>>
>>> 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.
>>
>> Looks ok, if entry command is RELEASE or SET only, but not HOLD, and
>> only if it contains express queues. And if for entry is expectable to have
>> interval shorter, the entry has to be marked as HOLD then.
>>
>> But not every offload is able to support mac/hold per sched (there is
>> no HOLD/RELEASE commands in this case). For this case seems like here can
>> be 2 cases:
>
> Yeah, the hw I have in hand also doesn't support the HOLD/RELEASE
> commands.
>
>>
>> 1) there is no “gate close” event for the preemptible traffic
>> 2) there is "gate close" event for the preemptable traffic
>>
>> And both can have the following impact, if assume the main reason to
>> this guard check is to guarantee the express queue cannot be blocked while
>> this "close to short" interval opening ofc:
>>
>> If a preemption fragment is started before "express" frame, then interval
>> should allow to complete preemption fragment and has to have enough time
>> to insert express frame. So here situation when maximum packet size per
>> each queue can have place.
>>
>> In case of TI am65 this queue MTU is configurable per queue (for similar
>> reasons and couple more (packet fill feature for instance)) and can be
>> used for guard check also, but not clear where it should be. Seems like
>> it should be done using ethtool also, but can be needed for taprio
>> interface....
>
> For now, at least for the hardware I am working on, something like this
> is configurable, but I wasn't planning on exposing it, using the maximum
> ethernet frame size seemed a good default.
>
>>
>>>>
>>>>>
>>>>> 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
>>
>> --
>> Regards,
>> Ivan Khoronzhuk
>
> Cheers,
> --
> Vinicius
>

--
Murali Karicheri
Texas Instruments

2019-12-30 16:58:51

by Karicheri, Muralidharan

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

Hi Vinicius,

On 12/18/2019 08:54 PM, Vinicius Costa Gomes wrote:
> Hi Ivan,
>
> Ivan Khoronzhuk <[email protected]> writes:
>
>>>>> 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;
>>
>> +
>>
>> And if follow mqprio settings logic then preemption also can be enabled
>> immediately while configuring taprio first time, and similarly new ADMIN
>> can't change it and can be set w/o preemption option afterwards.
>>
>> So that following is correct:
>>
>> OPER
>> $ tc qdisc add dev IFACE parent root handle 100 taprio \
>> base-time 10000000 \
>> 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
>> flags 1
>>
>> then
>> ADMIN
>> $ tc qdisc add dev IFACE parent root handle 100 taprio \
>> base-time 12000000 \
>> 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
>> sched-entry S 01 300000 \
>> sched-entry S 02 300000 \
>> flags 1
>>
>> then
>> ADMIN
>> $ tc qdisc add dev IFACE parent root handle 100 taprio \
>> base-time 13000000 \
>> sched-entry S 01 300000 \
>> sched-entry S 02 300000 \
>> flags 1
>>
>> BUT:
>>
>> 1) The question is only should it be in this way? I mean preemption to be
>> enabled immediately? Also should include other parameters like
>> fragment size.
>
> We can decide what things are allowed/useful here. For example, it might
> make sense to allow "preemption" to be changed. We can extend taprio to
> support changing the fragment size, if that makes sense.
>
>>
>> 2) What if I want to use frame preemption with another "transmission selection
>> algorithm"? Say another one "time sensitive" - CBS? How is it going to be
>> stacked?
>
> I am not seeing any (conceptual*) problems when plugging a cbs (for
> example) qdisc into one of taprio children. Or, are you talking about a
> more general problem?
>
> * here I am considering that support for taprio without an schedule is
> added.
>
>>
>> In this case ethtool looks better, allowing this "MAC level" feature, to be
>> configured separately.
>
> My only issue with using ethtool is that then we would have two
> different interfaces for "complementary" features. And it would make
> things even harder to configure and debug. The fact that one talks about
> traffic classes and the other transmission queues doesn't make me more
> comfortable as well.
>
> On the other hand, as there isn't a way to implement frame preemption in
> software, I agree that it makes it kind of awkward to have it in the tc
> subsystem.
Absolutely. I think frame pre-emption feature flag, per queue express/
pre-empt state, frag size, timers (hold/release) to be configured
independently (perhaps through ethtool) and then taprio should check
this with the lower device and then allow supporting additional Gate
operations such as Hold/release if supported by underlying device.

What do you think? Why to abuse tc for this?

Thanks and regards,

Murali
>
> At this point, I am slightly in favor of the taprio approach (yes, I am
> biased :-), but I can be convinced otherwise. I will be only a little
> sad if we choose to go with ethtool for now, and then add support up in
> the stack, something similar to "ethtool -N" and "tc-flower".
>

>>
>>>
>>> 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.
>>
>> Looks ok, if entry command is RELEASE or SET only, but not HOLD, and
>> only if it contains express queues. And if for entry is expectable to have
>> interval shorter, the entry has to be marked as HOLD then.
>>
>> But not every offload is able to support mac/hold per sched (there is
>> no HOLD/RELEASE commands in this case). For this case seems like here can
>> be 2 cases:
>
> Yeah, the hw I have in hand also doesn't support the HOLD/RELEASE
> commands.
>
>>
>> 1) there is no “gate close” event for the preemptible traffic
>> 2) there is "gate close" event for the preemptable traffic
>>
>> And both can have the following impact, if assume the main reason to
>> this guard check is to guarantee the express queue cannot be blocked while
>> this "close to short" interval opening ofc:
>>
>> If a preemption fragment is started before "express" frame, then interval
>> should allow to complete preemption fragment and has to have enough time
>> to insert express frame. So here situation when maximum packet size per
>> each queue can have place.
>>
>> In case of TI am65 this queue MTU is configurable per queue (for similar
>> reasons and couple more (packet fill feature for instance)) and can be
>> used for guard check also, but not clear where it should be. Seems like
>> it should be done using ethtool also, but can be needed for taprio
>> interface....
>
> For now, at least for the hardware I am working on, something like this
> is configurable, but I wasn't planning on exposing it, using the maximum
> ethernet frame size seemed a good default.
>
>>
>>>>
>>>>>
>>>>> 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
>>
>> --
>> Regards,
>> Ivan Khoronzhuk
>
> Cheers,
> --
> Vinicius
>

--
Murali Karicheri
Texas Instruments

2020-01-09 01:08:34

by Andre Guedes

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

Hi,

> >>> 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;
> >>
> >> +
> >>
> >> And if follow mqprio settings logic then preemption also can be enabled
> >> immediately while configuring taprio first time, and similarly new ADMIN
> >> can't change it and can be set w/o preemption option afterwards.
> >>
> >> So that following is correct:
> >>
> >> OPER
> >> $ tc qdisc add dev IFACE parent root handle 100 taprio \
> >> base-time 10000000 \
> >> 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
> >> flags 1
> >>
> >> then
> >> ADMIN
> >> $ tc qdisc add dev IFACE parent root handle 100 taprio \
> >> base-time 12000000 \
> >> 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
> >> sched-entry S 01 300000 \
> >> sched-entry S 02 300000 \
> >> flags 1
> >>
> >> then
> >> ADMIN
> >> $ tc qdisc add dev IFACE parent root handle 100 taprio \
> >> base-time 13000000 \
> >> sched-entry S 01 300000 \
> >> sched-entry S 02 300000 \
> >> flags 1
> >>
> >> BUT:
> >>
> >> 1) The question is only should it be in this way? I mean preemption to be
> >> enabled immediately? Also should include other parameters like
> >> fragment size.
> >
> > We can decide what things are allowed/useful here. For example, it might
> > make sense to allow "preemption" to be changed. We can extend taprio to
> > support changing the fragment size, if that makes sense.
> >
> >>
> >> 2) What if I want to use frame preemption with another "transmission selection
> >> algorithm"? Say another one "time sensitive" - CBS? How is it going to be
> >> stacked?
> >
> > I am not seeing any (conceptual*) problems when plugging a cbs (for
> > example) qdisc into one of taprio children. Or, are you talking about a
> > more general problem?
> >
> > * here I am considering that support for taprio without an schedule is
> > added.
> >
> >>
> >> In this case ethtool looks better, allowing this "MAC level" feature, to be
> >> configured separately.
> >
> > My only issue with using ethtool is that then we would have two
> > different interfaces for "complementary" features. And it would make
> > things even harder to configure and debug. The fact that one talks about
> > traffic classes and the other transmission queues doesn't make me more
> > comfortable as well.
> >
> > On the other hand, as there isn't a way to implement frame preemption in
> > software, I agree that it makes it kind of awkward to have it in the tc
> > subsystem.
> Absolutely. I think frame pre-emption feature flag, per queue express/
> pre-empt state, frag size, timers (hold/release) to be configured
> independently (perhaps through ethtool) and then taprio should check
> this with the lower device and then allow supporting additional Gate
> operations such as Hold/release if supported by underlying device.
>
> What do you think? Why to abuse tc for this?
>

After reading all this great discussion and revisiting the 802.1Q and 802.3br
specs, I'm now leaning towards to not coupling Frame Preemption support under
taprio qdisc. Besides what have been discussed, Annex S.2 from 802.1Q-2018
foresees FP without EST so it makes me feel like we should keep them separate.

Regarding the FP configuration knobs, the following seems reasonable to me:
* Enable/disable FP feature
* Preemptable queue mapping
* Fragment size multiplier

I'm not sure about the knob 'timers (hold/release)' described in the quotes
above. I couldn't find a match in the specs. If it refers to 'holdAdvance' and
'releaseAdvance' parameters described in 802.1Q-2018, I believe they are not
configurable. Do we know any hardware where they are configurable?

Regards,

Andre

2020-01-09 09:08:11

by Jose Abreu

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

From: Andre Guedes <[email protected]>
Date: Jan/09/2020, 01:07:37 (UTC+00:00)

> After reading all this great discussion and revisiting the 802.1Q and 802.3br
> specs, I'm now leaning towards to not coupling Frame Preemption support under
> taprio qdisc. Besides what have been discussed, Annex S.2 from 802.1Q-2018
> foresees FP without EST so it makes me feel like we should keep them separate.

I agree that EST and FP can be used individually. But how can you
specify the hold and release commands for gates without changing taprio qdisc user space API ?

> Regarding the FP configuration knobs, the following seems reasonable to me:
> * Enable/disable FP feature
> * Preemptable queue mapping
> * Fragment size multiplier
>
> I'm not sure about the knob 'timers (hold/release)' described in the quotes
> above. I couldn't find a match in the specs. If it refers to 'holdAdvance' and
> 'releaseAdvance' parameters described in 802.1Q-2018, I believe they are not
> configurable. Do we know any hardware where they are configurable?

Synopsys' HW supports reconfiguring these parameters. They are, however,
fixed independently of Queues. i.e. all queues will have same holdAdvance / releaseAdvance.

---
Thanks,
Jose Miguel Abreu

2020-01-09 20:41:11

by Andre Guedes

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

Hi Jose,

Quoting Jose Abreu (2020-01-09 00:59:24)
> From: Andre Guedes <[email protected]>
> Date: Jan/09/2020, 01:07:37 (UTC+00:00)
>
> > After reading all this great discussion and revisiting the 802.1Q and 802.3br
> > specs, I'm now leaning towards to not coupling Frame Preemption support under
> > taprio qdisc. Besides what have been discussed, Annex S.2 from 802.1Q-2018
> > foresees FP without EST so it makes me feel like we should keep them separate.
>
> I agree that EST and FP can be used individually. But how can you
> specify the hold and release commands for gates without changing taprio qdisc user space API ?

The 'hold' and 'release' are operations from the GCL, which is part of EST. So
they should still be specified via taprio. No changing in the user space API is
required since these operations are already supported in taprio API. What is
missing today is just the 'tc' side of it, which you already have a patch for
it.

> > Regarding the FP configuration knobs, the following seems reasonable to me:
> > * Enable/disable FP feature
> > * Preemptable queue mapping
> > * Fragment size multiplier
> >
> > I'm not sure about the knob 'timers (hold/release)' described in the quotes
> > above. I couldn't find a match in the specs. If it refers to 'holdAdvance' and
> > 'releaseAdvance' parameters described in 802.1Q-2018, I believe they are not
> > configurable. Do we know any hardware where they are configurable?
>
> Synopsys' HW supports reconfiguring these parameters. They are, however,
> fixed independently of Queues. i.e. all queues will have same holdAdvance / releaseAdvance.

Good to know. Is the datasheet publicly available? If so, could you please
point me to it? I'd like to learn more about the FP knobs provided by
different HW.

Regards,

Andre

2020-01-10 14:37:03

by Jose Abreu

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

From: Andre Guedes <[email protected]>
Date: Jan/09/2020, 18:04:55 (UTC+00:00)

> Quoting Jose Abreu (2020-01-09 00:59:24)
> > From: Andre Guedes <[email protected]>
> > Date: Jan/09/2020, 01:07:37 (UTC+00:00)
> >
> > > After reading all this great discussion and revisiting the 802.1Q and 802.3br
> > > specs, I'm now leaning towards to not coupling Frame Preemption support under
> > > taprio qdisc. Besides what have been discussed, Annex S.2 from 802.1Q-2018
> > > foresees FP without EST so it makes me feel like we should keep them separate.
> >
> > I agree that EST and FP can be used individually. But how can you
> > specify the hold and release commands for gates without changing taprio qdisc user space API ?
>
> The 'hold' and 'release' are operations from the GCL, which is part of EST. So
> they should still be specified via taprio. No changing in the user space API is
> required since these operations are already supported in taprio API. What is
> missing today is just the 'tc' side of it, which you already have a patch for
> it.

OK. Should we ask to merge it as-is then ?

> > > Regarding the FP configuration knobs, the following seems reasonable to me:
> > > * Enable/disable FP feature
> > > * Preemptable queue mapping
> > > * Fragment size multiplier
> > >
> > > I'm not sure about the knob 'timers (hold/release)' described in the quotes
> > > above. I couldn't find a match in the specs. If it refers to 'holdAdvance' and
> > > 'releaseAdvance' parameters described in 802.1Q-2018, I believe they are not
> > > configurable. Do we know any hardware where they are configurable?
> >
> > Synopsys' HW supports reconfiguring these parameters. They are, however,
> > fixed independently of Queues. i.e. all queues will have same holdAdvance / releaseAdvance.
>
> Good to know. Is the datasheet publicly available? If so, could you please
> point me to it? I'd like to learn more about the FP knobs provided by
> different HW.

I'm afraid its not available unless you are a Synopsys customer. You
should however be able to figure out the behavior by reading my patch
that adds support for FPE in XGMAC and QoS cores. I can clarify any
doubts.

---
Thanks,
Jose Miguel Abreu

2020-01-10 16:04:32

by Vladimir Oltean

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

Hi Andre,

On Thu, 9 Jan 2020 at 03:08, Andre Guedes <[email protected]> wrote:
>
> Hi,
>
> > >>> 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;
> > >>
> > >> +
> > >>
> > >> And if follow mqprio settings logic then preemption also can be enabled
> > >> immediately while configuring taprio first time, and similarly new ADMIN
> > >> can't change it and can be set w/o preemption option afterwards.
> > >>
> > >> So that following is correct:
> > >>
> > >> OPER
> > >> $ tc qdisc add dev IFACE parent root handle 100 taprio \
> > >> base-time 10000000 \
> > >> 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
> > >> flags 1
> > >>
> > >> then
> > >> ADMIN
> > >> $ tc qdisc add dev IFACE parent root handle 100 taprio \
> > >> base-time 12000000 \
> > >> 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
> > >> sched-entry S 01 300000 \
> > >> sched-entry S 02 300000 \
> > >> flags 1
> > >>
> > >> then
> > >> ADMIN
> > >> $ tc qdisc add dev IFACE parent root handle 100 taprio \
> > >> base-time 13000000 \
> > >> sched-entry S 01 300000 \
> > >> sched-entry S 02 300000 \
> > >> flags 1
> > >>
> > >> BUT:
> > >>
> > >> 1) The question is only should it be in this way? I mean preemption to be
> > >> enabled immediately? Also should include other parameters like
> > >> fragment size.
> > >
> > > We can decide what things are allowed/useful here. For example, it might
> > > make sense to allow "preemption" to be changed. We can extend taprio to
> > > support changing the fragment size, if that makes sense.
> > >
> > >>
> > >> 2) What if I want to use frame preemption with another "transmission selection
> > >> algorithm"? Say another one "time sensitive" - CBS? How is it going to be
> > >> stacked?
> > >
> > > I am not seeing any (conceptual*) problems when plugging a cbs (for
> > > example) qdisc into one of taprio children. Or, are you talking about a
> > > more general problem?
> > >
> > > * here I am considering that support for taprio without an schedule is
> > > added.
> > >
> > >>
> > >> In this case ethtool looks better, allowing this "MAC level" feature, to be
> > >> configured separately.
> > >
> > > My only issue with using ethtool is that then we would have two
> > > different interfaces for "complementary" features. And it would make
> > > things even harder to configure and debug. The fact that one talks about
> > > traffic classes and the other transmission queues doesn't make me more
> > > comfortable as well.
> > >
> > > On the other hand, as there isn't a way to implement frame preemption in
> > > software, I agree that it makes it kind of awkward to have it in the tc
> > > subsystem.
> > Absolutely. I think frame pre-emption feature flag, per queue express/
> > pre-empt state, frag size, timers (hold/release) to be configured
> > independently (perhaps through ethtool) and then taprio should check
> > this with the lower device and then allow supporting additional Gate
> > operations such as Hold/release if supported by underlying device.
> >
> > What do you think? Why to abuse tc for this?
> >
>
> After reading all this great discussion and revisiting the 802.1Q and 802.3br
> specs, I'm now leaning towards to not coupling Frame Preemption support under
> taprio qdisc. Besides what have been discussed, Annex S.2 from 802.1Q-2018
> foresees FP without EST so it makes me feel like we should keep them separate.
>
> Regarding the FP configuration knobs, the following seems reasonable to me:
> * Enable/disable FP feature
> * Preemptable queue mapping
> * Fragment size multiplier
>
> I'm not sure about the knob 'timers (hold/release)' described in the quotes
> above. I couldn't find a match in the specs. If it refers to 'holdAdvance' and
> 'releaseAdvance' parameters described in 802.1Q-2018, I believe they are not
> configurable. Do we know any hardware where they are configurable?
>

On NXP LS1028A, HOLD_ADVANCE is configurable on both ENETC and the
Felix switch (its default value is 127 bytes). Same as Synopsys, it is
a global setting and not per queue or per GCL entry.
RELEASE_ADVANCE is not configurable.
Regardless, I am not sure if there is any value in configuring this
knob. Remember that the minimum guard band size still needs to be
twice as large as the minimum Ethernet frame size.

As for the main topic of tc-taprio vs ethtool for configuring frame
preemption, I think ethtool is the more natural place for configuring
the traffic class to pMAC/eMAC mapping, global enable bit, and
fragment size, while tc-taprio is the more natural place for
configuring hold/release on individual GCL entries. The tc-taprio
offload can check the netdev support of the ethtool feature, just as
it checks right now the support for PTP clock for the full offload
feature.

Introducing tc-taprio with a null schedule is not really natural, and
frame preemption is a hardware-only feature that cannot be emulated,
so it is odd to enable it through tc.

> Regards,
>
> Andre

Regards,
-Vladimir

2020-01-10 21:00:41

by Andre Guedes

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

Hi Vladimir,

Quoting Vladimir Oltean (2020-01-10 08:02:45)
> > I'm not sure about the knob 'timers (hold/release)' described in the quotes
> > above. I couldn't find a match in the specs. If it refers to 'holdAdvance' and
> > 'releaseAdvance' parameters described in 802.1Q-2018, I believe they are not
> > configurable. Do we know any hardware where they are configurable?
> >
>
> On NXP LS1028A, HOLD_ADVANCE is configurable on both ENETC and the
> Felix switch (its default value is 127 bytes). Same as Synopsys, it is
> a global setting and not per queue or per GCL entry.
> RELEASE_ADVANCE is not configurable.
> Regardless, I am not sure if there is any value in configuring this
> knob. Remember that the minimum guard band size still needs to be
> twice as large as the minimum Ethernet frame size.

Not adding this knob now sounds reasonable to me too, but let's consider it in
the design so we can easily add it later, in case we need it.

Regards,

Andre

2020-01-17 23:48:30

by Vinicius Costa Gomes

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

Hi,

Murali Karicheri <[email protected]> writes:

[...]

>>>
>>> 2) What if I want to use frame preemption with another "transmission selection
>>> algorithm"? Say another one "time sensitive" - CBS? How is it going to be
>>> stacked?
>>
>> I am not seeing any (conceptual*) problems when plugging a cbs (for
>> example) qdisc into one of taprio children. Or, are you talking about a
>> more general problem?
>>
>
> If I understand it correctly problem is not stacking taprio with cbs,
> but rather pre-emption with other qdiscs and allow configuring
> the parameters such as frag size. How do I use frame pre-emption as
> an independent feature and configure frag size? Ethool appears to be
> better from this point of view as Ivan has mentioned below.
>

Yeah, after all the discussion here, I think we came to the rough
consensus that ethtool is the best place we have to enable/disable frame
preemption and its configuration knobs.

I just have a few comments to add to the series (sorry for the long
delay).


Cheers,
--
Vinicius