2019-11-27 10:02:39

by Po Liu

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

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.

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.

Signed-off-by: Po Liu <[email protected]>
---
include/linux/netdev_features.h | 5 ++++-
include/uapi/linux/ethtool.h | 5 ++++-
net/core/ethtool.c | 1 +
3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
index 4b19c544c59a..299750a8b414 100644
--- a/include/linux/netdev_features.h
+++ b/include/linux/netdev_features.h
@@ -80,6 +80,7 @@ enum {

NETIF_F_GRO_HW_BIT, /* Hardware Generic receive offload */
NETIF_F_HW_TLS_RECORD_BIT, /* Offload TLS record */
+ NETIF_F_HW_PREEMPTION_BIT, /* Hardware TC frame preemption */

/*
* Add your fresh new feature above and remember to update
@@ -150,6 +151,7 @@ enum {
#define NETIF_F_GSO_UDP_L4 __NETIF_F(GSO_UDP_L4)
#define NETIF_F_HW_TLS_TX __NETIF_F(HW_TLS_TX)
#define NETIF_F_HW_TLS_RX __NETIF_F(HW_TLS_RX)
+#define NETIF_F_PREEMPTION __NETIF_F(HW_PREEMPTION)

/* Finds the next feature with the highest number of the range of start till 0.
*/
@@ -175,7 +177,8 @@ static inline int find_next_netdev_feature(u64 feature, unsigned long start)
/* Features valid for ethtool to change */
/* = all defined minus driver/device-class-related */
#define NETIF_F_NEVER_CHANGE (NETIF_F_VLAN_CHALLENGED | \
- NETIF_F_LLTX | NETIF_F_NETNS_LOCAL)
+ NETIF_F_LLTX | NETIF_F_NETNS_LOCAL | \
+ NETIF_F_PREEMPTION)

/* remember that ((t)1 << t_BITS) is undefined in C99 */
#define NETIF_F_ETHTOOL_BITS ((__NETIF_F_BIT(NETDEV_FEATURE_COUNT - 1) | \
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index d4591792f0b4..12ffb34afbfa 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -1776,6 +1776,8 @@ enum ethtool_reset_flags {
};
#define ETH_RESET_SHARED_SHIFT 16

+/* Disable preemtion. */
+#define PREEMPTION_DISABLE 0x0

/**
* struct ethtool_link_settings - link control and status
@@ -1886,7 +1888,8 @@ struct ethtool_link_settings {
__s8 link_mode_masks_nwords;
__u8 transceiver;
__u8 reserved1[3];
- __u32 reserved[7];
+ __u32 preemption;
+ __u32 reserved[6];
__u32 link_mode_masks[0];
/* layout of link_mode_masks fields:
* __u32 map_supported[link_mode_masks_nwords];
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index cd9bc67381b2..6ffcd8a602b8 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -111,6 +111,7 @@ static const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN]
[NETIF_F_HW_TLS_RECORD_BIT] = "tls-hw-record",
[NETIF_F_HW_TLS_TX_BIT] = "tls-hw-tx-offload",
[NETIF_F_HW_TLS_RX_BIT] = "tls-hw-rx-offload",
+ [NETIF_F_HW_PREEMPTION_BIT] = "tx-preemption",
};

static const char
--
2.17.1


2019-11-27 18:59:07

by David Miller

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


net-next is closed, please repost this series when net-next opens back up.

Thank you.

2019-12-03 15:12:33

by Ivan Khoronzhuk

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

On Wed, Nov 27, 2019 at 09:59:18AM +0000, Po Liu wrote:

Hi, Po Liu

>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.
>
>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.
>
>Signed-off-by: Po Liu <[email protected]>
>---
> include/linux/netdev_features.h | 5 ++++-
> include/uapi/linux/ethtool.h | 5 ++++-
> net/core/ethtool.c | 1 +
> 3 files changed, 9 insertions(+), 2 deletions(-)
>
>diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
>index 4b19c544c59a..299750a8b414 100644
>--- a/include/linux/netdev_features.h
>+++ b/include/linux/netdev_features.h
>@@ -80,6 +80,7 @@ enum {
>
> NETIF_F_GRO_HW_BIT, /* Hardware Generic receive offload */
> NETIF_F_HW_TLS_RECORD_BIT, /* Offload TLS record */
>+ NETIF_F_HW_PREEMPTION_BIT, /* Hardware TC frame preemption */
>
> /*
> * Add your fresh new feature above and remember to update
>@@ -150,6 +151,7 @@ enum {
> #define NETIF_F_GSO_UDP_L4 __NETIF_F(GSO_UDP_L4)
> #define NETIF_F_HW_TLS_TX __NETIF_F(HW_TLS_TX)
> #define NETIF_F_HW_TLS_RX __NETIF_F(HW_TLS_RX)
>+#define NETIF_F_PREEMPTION __NETIF_F(HW_PREEMPTION)
>
> /* Finds the next feature with the highest number of the range of start till 0.
> */
>@@ -175,7 +177,8 @@ static inline int find_next_netdev_feature(u64 feature, unsigned long start)
> /* Features valid for ethtool to change */
> /* = all defined minus driver/device-class-related */
> #define NETIF_F_NEVER_CHANGE (NETIF_F_VLAN_CHALLENGED | \
>- NETIF_F_LLTX | NETIF_F_NETNS_LOCAL)
>+ NETIF_F_LLTX | NETIF_F_NETNS_LOCAL | \
>+ NETIF_F_PREEMPTION)
>
> /* remember that ((t)1 << t_BITS) is undefined in C99 */
> #define NETIF_F_ETHTOOL_BITS ((__NETIF_F_BIT(NETDEV_FEATURE_COUNT - 1) | \
>diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
>index d4591792f0b4..12ffb34afbfa 100644
>--- a/include/uapi/linux/ethtool.h
>+++ b/include/uapi/linux/ethtool.h
>@@ -1776,6 +1776,8 @@ enum ethtool_reset_flags {
> };
> #define ETH_RESET_SHARED_SHIFT 16
>
>+/* Disable preemtion. */
>+#define PREEMPTION_DISABLE 0x0
>
> /**
> * struct ethtool_link_settings - link control and status
>@@ -1886,7 +1888,8 @@ struct ethtool_link_settings {
> __s8 link_mode_masks_nwords;
> __u8 transceiver;
> __u8 reserved1[3];
>- __u32 reserved[7];
>+ __u32 preemption;

Why 32 when only 8 is needed?

>+ __u32 reserved[6];
> __u32 link_mode_masks[0];
> /* layout of link_mode_masks fields:
> * __u32 map_supported[link_mode_masks_nwords];
>diff --git a/net/core/ethtool.c b/net/core/ethtool.c
>index cd9bc67381b2..6ffcd8a602b8 100644
>--- a/net/core/ethtool.c
>+++ b/net/core/ethtool.c
>@@ -111,6 +111,7 @@ static const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN]
> [NETIF_F_HW_TLS_RECORD_BIT] = "tls-hw-record",
> [NETIF_F_HW_TLS_TX_BIT] = "tls-hw-tx-offload",
> [NETIF_F_HW_TLS_RX_BIT] = "tls-hw-rx-offload",
>+ [NETIF_F_HW_PREEMPTION_BIT] = "tx-preemption",

What about tx-frame-preempt? or frame-preemption?

> };
>
> static const char
>--
>2.17.1
>

--
Regards,
Ivan Khoronzhuk

2019-12-13 22:09:14

by Andre Guedes

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

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

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.

Please share your thoughts on this.

Regards,

Andre

2020-01-18 00:06:05

by Vinicius Costa Gomes

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

Hi,

Po Liu <[email protected]> writes:

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

I think that we should keep the concept of 'traffic classes' outside of
ethtool. Ethtool should only care about queues (or similar concepts).
And unless I am missing something here, what you mean by 'traffic class'
here is really a hardware queue.

>
> 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.
>
> 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.
>
> Signed-off-by: Po Liu <[email protected]>
> ---
> include/linux/netdev_features.h | 5 ++++-
> include/uapi/linux/ethtool.h | 5 ++++-
> net/core/ethtool.c | 1 +
> 3 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
> index 4b19c544c59a..299750a8b414 100644
> --- a/include/linux/netdev_features.h
> +++ b/include/linux/netdev_features.h
> @@ -80,6 +80,7 @@ enum {
>
> NETIF_F_GRO_HW_BIT, /* Hardware Generic receive offload */
> NETIF_F_HW_TLS_RECORD_BIT, /* Offload TLS record */
> + NETIF_F_HW_PREEMPTION_BIT, /* Hardware TC frame preemption */
>
> /*
> * Add your fresh new feature above and remember to update
> @@ -150,6 +151,7 @@ enum {
> #define NETIF_F_GSO_UDP_L4 __NETIF_F(GSO_UDP_L4)
> #define NETIF_F_HW_TLS_TX __NETIF_F(HW_TLS_TX)
> #define NETIF_F_HW_TLS_RX __NETIF_F(HW_TLS_RX)
> +#define NETIF_F_PREEMPTION __NETIF_F(HW_PREEMPTION)
>
> /* Finds the next feature with the highest number of the range of start till 0.
> */
> @@ -175,7 +177,8 @@ static inline int find_next_netdev_feature(u64 feature, unsigned long start)
> /* Features valid for ethtool to change */
> /* = all defined minus driver/device-class-related */
> #define NETIF_F_NEVER_CHANGE (NETIF_F_VLAN_CHALLENGED | \
> - NETIF_F_LLTX | NETIF_F_NETNS_LOCAL)
> + NETIF_F_LLTX | NETIF_F_NETNS_LOCAL | \
> + NETIF_F_PREEMPTION)
>
> /* remember that ((t)1 << t_BITS) is undefined in C99 */
> #define NETIF_F_ETHTOOL_BITS ((__NETIF_F_BIT(NETDEV_FEATURE_COUNT - 1) | \
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index d4591792f0b4..12ffb34afbfa 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -1776,6 +1776,8 @@ enum ethtool_reset_flags {
> };
> #define ETH_RESET_SHARED_SHIFT 16
>
> +/* Disable preemtion. */
> +#define PREEMPTION_DISABLE 0x0
>
> /**
> * struct ethtool_link_settings - link control and status
> @@ -1886,7 +1888,8 @@ struct ethtool_link_settings {
> __s8 link_mode_masks_nwords;
> __u8 transceiver;
> __u8 reserved1[3];
> - __u32 reserved[7];
> + __u32 preemption;
> + __u32 reserved[6];
> __u32 link_mode_masks[0];
> /* layout of link_mode_masks fields:
> * __u32 map_supported[link_mode_masks_nwords];
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index cd9bc67381b2..6ffcd8a602b8 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -111,6 +111,7 @@ static const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN]
> [NETIF_F_HW_TLS_RECORD_BIT] = "tls-hw-record",
> [NETIF_F_HW_TLS_TX_BIT] = "tls-hw-tx-offload",
> [NETIF_F_HW_TLS_RX_BIT] = "tls-hw-rx-offload",
> + [NETIF_F_HW_PREEMPTION_BIT] = "tx-preemption",
> };
>
> static const char
> --
> 2.17.1

2020-01-22 18:05:36

by Karicheri, Muralidharan

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

Hi, Vinicius,

On 01/17/2020 07:03 PM, Vinicius Costa Gomes wrote:
> Hi,
>
> Po Liu <[email protected]> writes:
>
>> 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.
>
> I think that we should keep the concept of 'traffic classes' outside of
> ethtool. Ethtool should only care about queues (or similar concepts).
> And unless I am missing something here, what you mean by 'traffic class'
> here is really a hardware queue.
>
Agree.

So this is my understanding..

Once driver support FPE, then drive accepts tc gate operation,
Set-And-Hold MAC & Set-And-Release-MAC. Otherwise it can only offload
SetGateStates.

So candidates for ethtool command.

Set

- FPE enable/disable
- framePreemptionStatusTable of Table 12-30—Frame Preemption Parameter
table of 802.1Q 2018 which are defined as Read/Write. This is an array
of Hw queues and if they are preemptible or express.
- Additionally 802.3br defines Min fragment size that is a candidate
for ethtool command.

Show the following:-

- holdAdvance
- releaseAdvance
- preemptionActive
- holdRequest

There is also Table 12-29—The Gate Parameter Table for taprio. These
related to taprio schedule is already part of tc command. However there
are below parameters that requires configuration by user as well.
Can we use ethtool for the same as this
- queueMaxSDUTable

Below is what I read about this which is also for hw queues AFAIK.

==== from 802.1Q====================================================
A per-traffic class queue queueMaxSDU parameter defines the maximum
service data unit size for each queue; frames that exceed queueMaxSDU
are discarded. The value of queueMaxSDU for each queue is configurable
by management its default value is the maximum SDU size supported by the
MAC procedures employed on the LAN to which the frame is to be relayed
=======================================================================

I have question about the below parameters in The Gate Parameter Table
that are not currently supported by tc command. Looks like they need to
be shown to user for management.

- ConfigChange - Looks like this needs to be controlled by
user. After sending admin command, user send this trigger to start
copying admin schedule to operation schedule. Is this getting
added to tc command?
- ConfigChangeTime - The time at which the administrative variables
that determine the cycle are to be copied across to the
corresponding operational variables, expressed as a PTP timescale
- TickGranularity - the management parameters specified in Gate
Parameter Table allow a management station to discover the
characteristics of an implementation’s cycle timer clock
(TickGranularity) and to set the parameters for the gating cycle
accordingly.
- ConfigPending - A Boolean variable, set TRUE to indicate that
there is a new cycle configuration awaiting installation.
- ConfigChangeError - Error in configuration (AdminBaseTime <
CurrentTime)
- SupportedListMax - Maximum supported Admin/Open shed list.

Is there a plan to export these from driver through tc show or such
command? The reason being, there would be applications developed to
manage configuration/schedule of TSN nodes that would requires these
information from the node. So would need a support either in tc or
some other means to retrieve them from hardware or driver. That is my
understanding...

Regards,

Murali
>>
>> 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.
>>
>> 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.
>>
>> Signed-off-by: Po Liu <[email protected]>
>> ---
>> include/linux/netdev_features.h | 5 ++++-
>> include/uapi/linux/ethtool.h | 5 ++++-
>> net/core/ethtool.c | 1 +
>> 3 files changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
>> index 4b19c544c59a..299750a8b414 100644
>> --- a/include/linux/netdev_features.h
>> +++ b/include/linux/netdev_features.h
>> @@ -80,6 +80,7 @@ enum {
>>
>> NETIF_F_GRO_HW_BIT, /* Hardware Generic receive offload */
>> NETIF_F_HW_TLS_RECORD_BIT, /* Offload TLS record */
>> + NETIF_F_HW_PREEMPTION_BIT, /* Hardware TC frame preemption */
>>
>> /*
>> * Add your fresh new feature above and remember to update
>> @@ -150,6 +151,7 @@ enum {
>> #define NETIF_F_GSO_UDP_L4 __NETIF_F(GSO_UDP_L4)
>> #define NETIF_F_HW_TLS_TX __NETIF_F(HW_TLS_TX)
>> #define NETIF_F_HW_TLS_RX __NETIF_F(HW_TLS_RX)
>> +#define NETIF_F_PREEMPTION __NETIF_F(HW_PREEMPTION)
>>
>> /* Finds the next feature with the highest number of the range of start till 0.
>> */
>> @@ -175,7 +177,8 @@ static inline int find_next_netdev_feature(u64 feature, unsigned long start)
>> /* Features valid for ethtool to change */
>> /* = all defined minus driver/device-class-related */
>> #define NETIF_F_NEVER_CHANGE (NETIF_F_VLAN_CHALLENGED | \
>> - NETIF_F_LLTX | NETIF_F_NETNS_LOCAL)
>> + NETIF_F_LLTX | NETIF_F_NETNS_LOCAL | \
>> + NETIF_F_PREEMPTION)
>>
>> /* remember that ((t)1 << t_BITS) is undefined in C99 */
>> #define NETIF_F_ETHTOOL_BITS ((__NETIF_F_BIT(NETDEV_FEATURE_COUNT - 1) | \
>> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
>> index d4591792f0b4..12ffb34afbfa 100644
>> --- a/include/uapi/linux/ethtool.h
>> +++ b/include/uapi/linux/ethtool.h
>> @@ -1776,6 +1776,8 @@ enum ethtool_reset_flags {
>> };
>> #define ETH_RESET_SHARED_SHIFT 16
>>
>> +/* Disable preemtion. */
>> +#define PREEMPTION_DISABLE 0x0
>>
>> /**
>> * struct ethtool_link_settings - link control and status
>> @@ -1886,7 +1888,8 @@ struct ethtool_link_settings {
>> __s8 link_mode_masks_nwords;
>> __u8 transceiver;
>> __u8 reserved1[3];
>> - __u32 reserved[7];
>> + __u32 preemption;
>> + __u32 reserved[6];
>> __u32 link_mode_masks[0];
>> /* layout of link_mode_masks fields:
>> * __u32 map_supported[link_mode_masks_nwords];
>> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
>> index cd9bc67381b2..6ffcd8a602b8 100644
>> --- a/net/core/ethtool.c
>> +++ b/net/core/ethtool.c
>> @@ -111,6 +111,7 @@ static const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN]
>> [NETIF_F_HW_TLS_RECORD_BIT] = "tls-hw-record",
>> [NETIF_F_HW_TLS_TX_BIT] = "tls-hw-tx-offload",
>> [NETIF_F_HW_TLS_RX_BIT] = "tls-hw-rx-offload",
>> + [NETIF_F_HW_PREEMPTION_BIT] = "tx-preemption",
>> };
>>
>> static const char
>> --
>> 2.17.1
>

--
Murali Karicheri
Texas Instruments

2020-01-23 13:31:43

by Vladimir Oltean

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

Hi Murali,

On Wed, 22 Jan 2020 at 20:04, Murali Karicheri <[email protected]> wrote:
>
> I have question about the below parameters in The Gate Parameter Table
> that are not currently supported by tc command. Looks like they need to
> be shown to user for management.
>
> - ConfigChange - Looks like this needs to be controlled by
> user. After sending admin command, user send this trigger to start
> copying admin schedule to operation schedule. Is this getting
> added to tc command?

"The ConfigChange parameter signals the start of a
configuration change for the gate
when it is set to TRUE. This should only be done
when the various administrative parameters
are all set to appropriate values."

As far as my understanding goes, all tc-taprio commands currently
behave as though this boolean is implicitly set to TRUE after the
structures have been set up. I'm not sure there is any value in doing
otherwise.

> - ConfigChangeTime - The time at which the administrative variables
> that determine the cycle are to be copied across to the
> corresponding operational variables, expressed as a PTP timescale

This is the base-time of the admin schedule, no?

"The PTPtime at which the next config change is scheduled to occur.
The value is a representation of a PTPtime value,
consisting of a 48-bit integer
number of seconds and a 32-bit integer number of nanoseconds."

> - TickGranularity - the management parameters specified in Gate
> Parameter Table allow a management station to discover the
> characteristics of an implementation’s cycle timer clock
> (TickGranularity) and to set the parameters for the gating cycle
> accordingly.

Not sure who is going to use this and for what purpose, but ok.

> - ConfigPending - A Boolean variable, set TRUE to indicate that
> there is a new cycle configuration awaiting installation.

I had tried to export something like this (driver calls back into
sch_taprio.c when hw has applied the config, this would result in
ConfigPending = FALSE), but ultimately didn't finish the idea, and it
caused some problems too, due to incorrect RCU usage.

> - ConfigChangeError - Error in configuration (AdminBaseTime <
> CurrentTime)

This can be exported similarly.

> - SupportedListMax - Maximum supported Admin/Open shed list.
>
> Is there a plan to export these from driver through tc show or such
> command? The reason being, there would be applications developed to
> manage configuration/schedule of TSN nodes that would requires these
> information from the node. So would need a support either in tc or
> some other means to retrieve them from hardware or driver. That is my
> understanding...
>

Not sure what answer you expect to receive for "is there any plan".
You can go ahead and propose something, as long as it is reasonably
useful to have.

> Regards,
>
> Murali
>
> --
> Murali Karicheri
> Texas Instruments

Thanks,
-Vladimir

2020-01-23 17:56:54

by Vinicius Costa Gomes

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

Hi,

Vladimir Oltean <[email protected]> writes:

> Hi Murali,
>
> On Wed, 22 Jan 2020 at 20:04, Murali Karicheri <[email protected]> wrote:
>>
>> I have question about the below parameters in The Gate Parameter Table
>> that are not currently supported by tc command. Looks like they need to
>> be shown to user for management.
>>
>> - ConfigChange - Looks like this needs to be controlled by
>> user. After sending admin command, user send this trigger to start
>> copying admin schedule to operation schedule. Is this getting
>> added to tc command?
>
> "The ConfigChange parameter signals the start of a
> configuration change for the gate
> when it is set to TRUE. This should only be done
> when the various administrative parameters
> are all set to appropriate values."
>
> As far as my understanding goes, all tc-taprio commands currently
> behave as though this boolean is implicitly set to TRUE after the
> structures have been set up. I'm not sure there is any value in doing
> otherwise.
>
>> - ConfigChangeTime - The time at which the administrative variables
>> that determine the cycle are to be copied across to the
>> corresponding operational variables, expressed as a PTP timescale
>
> This is the base-time of the admin schedule, no?
>
> "The PTPtime at which the next config change is scheduled to occur.
> The value is a representation of a PTPtime value,
> consisting of a 48-bit integer
> number of seconds and a 32-bit integer number of nanoseconds."
>
>> - TickGranularity - the management parameters specified in Gate
>> Parameter Table allow a management station to discover the
>> characteristics of an implementation’s cycle timer clock
>> (TickGranularity) and to set the parameters for the gating cycle
>> accordingly.
>
> Not sure who is going to use this and for what purpose, but ok.
>
>> - ConfigPending - A Boolean variable, set TRUE to indicate that
>> there is a new cycle configuration awaiting installation.
>
> I had tried to export something like this (driver calls back into
> sch_taprio.c when hw has applied the config, this would result in
> ConfigPending = FALSE), but ultimately didn't finish the idea, and it
> caused some problems too, due to incorrect RCU usage.
>

If this should be exported, this should be done from taprio, perhaps
adding a new field to what is exported via the dump() callback, which
should be quite easy.

>> - ConfigChangeError - Error in configuration (AdminBaseTime <
>> CurrentTime)
>
> This can be exported similarly.

In my view, having this as a "runtime" error is not useful, as we can
verify this at configuration time.

>
>> - SupportedListMax - Maximum supported Admin/Open shed list.
>>
>> Is there a plan to export these from driver through tc show or such
>> command? The reason being, there would be applications developed to
>> manage configuration/schedule of TSN nodes that would requires these
>> information from the node. So would need a support either in tc or
>> some other means to retrieve them from hardware or driver. That is my
>> understanding...
>>

Hm, now I understamd what you meant here...

>
> Not sure what answer you expect to receive for "is there any plan".
> You can go ahead and propose something, as long as it is reasonably
> useful to have.

... if this is indeed useful, perhaps one way to do is to add a subcommand
to TC_SETUP_QDISC_TAPRIO, so we can retrieve the stats/information we want
from the driver. Similar to what cls_flower does.

>
>> Regards,
>>
>> Murali
>>
>> --
>> Murali Karicheri
>> Texas Instruments
>
> Thanks,
> -Vladimir

Cheers,
--
Vinicius

2020-02-10 20:11:42

by Karicheri, Muralidharan

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

Hi Vladimir,

On 01/23/2020 08:30 AM, Vladimir Oltean wrote:
> Hi Murali,
>
> On Wed, 22 Jan 2020 at 20:04, Murali Karicheri <[email protected]> wrote:
>>
>> I have question about the below parameters in The Gate Parameter Table
>> that are not currently supported by tc command. Looks like they need to
>> be shown to user for management.
>>
>> - ConfigChange - Looks like this needs to be controlled by
>> user. After sending admin command, user send this trigger to start
>> copying admin schedule to operation schedule. Is this getting
>> added to tc command?
>
> "The ConfigChange parameter signals the start of a
> configuration change for the gate
> when it is set to TRUE. This should only be done
> when the various administrative parameters
> are all set to appropriate values."
>
> As far as my understanding goes, all tc-taprio commands currently
> behave as though this boolean is implicitly set to TRUE after the
> structures have been set up. I'm not sure there is any value in doing
> otherwise.
>
That is my understanding as well. However I found this in the 802.1Q
and want to see if someone has insight into this parameter. So perhaps
we can ignore this for now and re-visit when there is a real need for
the same.

>> - ConfigChangeTime - The time at which the administrative variables
>> that determine the cycle are to be copied across to the
>> corresponding operational variables, expressed as a PTP timescale
>
> This is the base-time of the admin schedule, no?

I think there is cycle time extension possible and in that case,
ConfigChangeTime may be different from Admin base-time. So this gives
the actual time when the cycle configuration is actually applied.

>
> "The PTPtime at which the next config change is scheduled to occur.
> The value is a representation of a PTPtime value,
> consisting of a 48-bit integer
> number of seconds and a 32-bit integer number of nanoseconds."
>
>> - TickGranularity - the management parameters specified in Gate
>> Parameter Table allow a management station to discover the
>> characteristics of an implementation’s cycle timer clock
>> (TickGranularity) and to set the parameters for the gating cycle
>> accordingly.
>
> Not sure who is going to use this and for what purpose, but ok.

Same here. Not sure how this will be used by application.

>
>> - ConfigPending - A Boolean variable, set TRUE to indicate that
>> there is a new cycle configuration awaiting installation.
>
> I had tried to export something like this (driver calls back into
> sch_taprio.c when hw has applied the config, this would result in
> ConfigPending = FALSE), but ultimately didn't finish the idea, and it
> caused some problems too, due to incorrect RCU usage.
>

Ok. Hope you plan to send a patch for this in the future.

>> - ConfigChangeError - Error in configuration (AdminBaseTime <
>> CurrentTime)
>
> This can be exported similarly.

Ok.

>
>> - SupportedListMax - Maximum supported Admin/Open shed list.
>>
>> Is there a plan to export these from driver through tc show or such
>> command? The reason being, there would be applications developed to
>> manage configuration/schedule of TSN nodes that would requires these
>> information from the node. So would need a support either in tc or
>> some other means to retrieve them from hardware or driver. That is my
>> understanding...
>>
>
> Not sure what answer you expect to receive for "is there any plan".

To avoid duplicating the work if someone is already working on this.

> You can go ahead and propose something, as long as it is reasonably
> useful to have.
Ok. Will keep in mind.
>
>> Regards,
>>
>> Murali
>>
>> --
>> Murali Karicheri
>> Texas Instruments
>
> Thanks,
> -Vladimir
>

--
Murali Karicheri
Texas Instruments

2020-02-10 20:25:15

by Karicheri, Muralidharan

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

Hi,


On 01/23/2020 12:50 PM, Vinicius Costa Gomes wrote:
> Hi,
>
> Vladimir Oltean <[email protected]> writes:
>
>> Hi Murali,
>>
>> On Wed, 22 Jan 2020 at 20:04, Murali Karicheri <[email protected]> wrote:
>>>
>>> I have question about the below parameters in The Gate Parameter Table
>>> that are not currently supported by tc command. Looks like they need to
>>> be shown to user for management.
>>>
>>> - ConfigChange - Looks like this needs to be controlled by
>>> user. After sending admin command, user send this trigger to start
>>> copying admin schedule to operation schedule. Is this getting
>>> added to tc command?
>>
>> "The ConfigChange parameter signals the start of a
>> configuration change for the gate
>> when it is set to TRUE. This should only be done
>> when the various administrative parameters
>> are all set to appropriate values."
>>
>> As far as my understanding goes, all tc-taprio commands currently
>> behave as though this boolean is implicitly set to TRUE after the
>> structures have been set up. I'm not sure there is any value in doing
>> otherwise.
>>
>>> - ConfigChangeTime - The time at which the administrative variables
>>> that determine the cycle are to be copied across to the
>>> corresponding operational variables, expressed as a PTP timescale
>>
>> This is the base-time of the admin schedule, no?
>>
>> "The PTPtime at which the next config change is scheduled to occur.
>> The value is a representation of a PTPtime value,
>> consisting of a 48-bit integer
>> number of seconds and a 32-bit integer number of nanoseconds."
>>
>>> - TickGranularity - the management parameters specified in Gate
>>> Parameter Table allow a management station to discover the
>>> characteristics of an implementation’s cycle timer clock
>>> (TickGranularity) and to set the parameters for the gating cycle
>>> accordingly.
>>
>> Not sure who is going to use this and for what purpose, but ok.
>>
>>> - ConfigPending - A Boolean variable, set TRUE to indicate that
>>> there is a new cycle configuration awaiting installation.
>>
>> I had tried to export something like this (driver calls back into
>> sch_taprio.c when hw has applied the config, this would result in
>> ConfigPending = FALSE), but ultimately didn't finish the idea, and it
>> caused some problems too, due to incorrect RCU usage.
>>
>
> If this should be exported, this should be done from taprio, perhaps
> adding a new field to what is exported via the dump() callback, which
> should be quite easy.
>
We are still working to send a patch for taprio offload on our hardware
and it may take a while to get to this. So if someone can help to add
the required kernel/driver interface for this, that will be great!

>>> - ConfigChangeError - Error in configuration (AdminBaseTime <
>>> CurrentTime)
>>
>> This can be exported similarly.
>
> In my view, having this as a "runtime" error is not useful, as we can
> verify this at configuration time.

Looks like this is not an error per 802.1Q standard if I understood it
correctly.

This is what I see.
=======================================================================
From 802.1Q 2018, 8.6.9.1.1 SetCycleStartTime()

If AdminBaseTime is set to the same time in the past in all bridges and
end stations, OperBaseTime is always in the past, and all cycles start
synchronized. Using AdminBaseTime in the past is appropriate when you
can start schedules prior to starting the application that uses the
schedules. Use of AdminBaseTime in the future is intended to change a
currently running schedule in all bridges and end stations to a new
schedule at a future time. Using AdminBaseTime in the future is
appropriate when schedules must be changed without stopping the
application
========================================================================

>
>>
>>> - SupportedListMax - Maximum supported Admin/Open shed list.
>>>
>>> Is there a plan to export these from driver through tc show or such
>>> command? The reason being, there would be applications developed to
>>> manage configuration/schedule of TSN nodes that would requires these
>>> information from the node. So would need a support either in tc or
>>> some other means to retrieve them from hardware or driver. That is my
>>> understanding...
>>>
>
> Hm, now I understamd what you meant here...
>
>>
>> Not sure what answer you expect to receive for "is there any plan".
>> You can go ahead and propose something, as long as it is reasonably
>> useful to have.
>
> ... if this is indeed useful, perhaps one way to do is to add a subcommand
> to TC_SETUP_QDISC_TAPRIO, so we can retrieve the stats/information we want
> from the driver. Similar to what cls_flower does.
>

What I understand is that there will be some work done to allow auto
configuration of TSN nodes from user space and that would need access to
all or some of the above parameters along with tc command to configure
the same. May be a open source project for this or some custom
application? Any such projects existing??

Regards,

Murali
>>
>>> Regards,
>>>
>>> Murali
>>>
>>> --
>>> Murali Karicheri
>>> Texas Instruments
>>
>> Thanks,
>> -Vladimir
>
> Cheers,
> --
> Vinicius
>

--
Murali Karicheri
Texas Instruments

2020-02-11 21:39:56

by Vinicius Costa Gomes

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

Murali Karicheri <[email protected]> writes:

> We are still working to send a patch for taprio offload on our hardware
> and it may take a while to get to this. So if someone can help to add
> the required kernel/driver interface for this, that will be great!

Will add this to my todo list. But if anyone else has the spare cycles
feel free to have a go at it.

>
>>>> - ConfigChangeError - Error in configuration (AdminBaseTime <
>>>> CurrentTime)
>>>
>>> This can be exported similarly.
>>
>> In my view, having this as a "runtime" error is not useful, as we can
>> verify this at configuration time.
>
> Looks like this is not an error per 802.1Q standard if I understood it
> correctly.
>
> This is what I see.
> =======================================================================
> From 802.1Q 2018, 8.6.9.1.1 SetCycleStartTime()
>
> If AdminBaseTime is set to the same time in the past in all bridges and
> end stations, OperBaseTime is always in the past, and all cycles start
> synchronized. Using AdminBaseTime in the past is appropriate when you
> can start schedules prior to starting the application that uses the
> schedules. Use of AdminBaseTime in the future is intended to change a
> currently running schedule in all bridges and end stations to a new
> schedule at a future time. Using AdminBaseTime in the future is
> appropriate when schedules must be changed without stopping the
> application
> ========================================================================
>

What I meant here is the case that I already have an "oper" schedule
running, so my "oper->base_time" is in the past, and I try to add an
"admin" schedule with a "base_time" also in the past. What's the
expected behavior in this case? The text about stopping/starting
applications doesn't seem to apply to the way the tc subsystem interacts
with the applications.

>>
>>>
>>>> - SupportedListMax - Maximum supported Admin/Open shed list.
>>>>
>>>> Is there a plan to export these from driver through tc show or such
>>>> command? The reason being, there would be applications developed to
>>>> manage configuration/schedule of TSN nodes that would requires these
>>>> information from the node. So would need a support either in tc or
>>>> some other means to retrieve them from hardware or driver. That is my
>>>> understanding...
>>>>
>>
>> Hm, now I understamd what you meant here...
>>
>>>
>>> Not sure what answer you expect to receive for "is there any plan".
>>> You can go ahead and propose something, as long as it is reasonably
>>> useful to have.
>>
>> ... if this is indeed useful, perhaps one way to do is to add a subcommand
>> to TC_SETUP_QDISC_TAPRIO, so we can retrieve the stats/information we want
>> from the driver. Similar to what cls_flower does.
>>
>
> What I understand is that there will be some work done to allow auto
> configuration of TSN nodes from user space and that would need access to
> all or some of the above parameters along with tc command to configure
> the same. May be a open source project for this or some custom
> application? Any such projects existing??

Yeah, this is a big missing piece for TSN. I've heard 'netopeer2' and
'sysrepo' mentioned when similar questions were asked, but I have still
to take a look at them and see what's missing. (Or if they are the right
tool for the job)


--
Vinicius

2020-02-21 21:42:10

by Vinicius Costa Gomes

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

Hi,

Po Liu <[email protected]> writes:

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

Any updates on this series? If you think that there's something that I
could help, just tell.


Cheers,
--
Vinicius

2020-02-22 03:27:38

by Po Liu

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

Hi Vinicius,


Br,
Po Liu

> -----Original Message-----
> From: Vinicius Costa Gomes <[email protected]>
> Sent: 2020??2??22?? 5:44
> To: Po Liu <[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]; linux-
> [email protected]; [email protected]
> Cc: [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 Liu <[email protected]> writes:
>
> > 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.
> >
> > 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.
> >
>
> Any updates on this series? If you think that there's something that I could help,
> just tell.

Sorry for the long time not involve the discussion. I am focus on other tsn code for tc flower.
If you can take more about this preemption serial, that would be good.

I summary some suggestions from Marali Karicheri and Ivan Khornonzhuk and by you and also others:
- Add config the fragment size, hold advance, release advance and flags;
My comments about the fragment size is in the Qbu spec limit the fragment size " the minimum non-final fragment size is 64,
128, 192, or 256 octets " this setting would affect the guardband setting for Qbv. But the ethtool setting could not involve this issues but by the taprio side.
- " Furthermore, this setting could be extend for a serial setting for mac and traffic class." "Better not to using the traffic class concept."
Could adding a serial setting by "ethtool --preemption xxx" or other name. I don' t think it is good to involve in the queue control since queues number may bigger than the TC number.
- The ethtool is the better choice to configure the preemption
I agree.

Thanks??
>
>
> Cheers,
> --
> Vinicius

2020-02-25 19:34:17

by Karicheri, Muralidharan

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

Hi Vinicius,

On 02/11/2020 02:22 PM, Vinicius Costa Gomes wrote:
> Murali Karicheri <[email protected]> writes:
>
>> We are still working to send a patch for taprio offload on our hardware
>> and it may take a while to get to this. So if someone can help to add
>> the required kernel/driver interface for this, that will be great!
>
> Will add this to my todo list. But if anyone else has the spare cycles
> feel free to have a go at it.
>
Thanks! We have made some progress in sending the base driver to netdev
list now https://lkml.org/lkml/2020/2/22/157

This device is taprio offload capable. Next step is to add taprio
offload to this driver. Then other features will follow.

>>
>>>>> - ConfigChangeError - Error in configuration (AdminBaseTime <
>>>>> CurrentTime)
>>>>
>>>> This can be exported similarly.
>>>
>>> In my view, having this as a "runtime" error is not useful, as we can
>>> verify this at configuration time.
>>
>> Looks like this is not an error per 802.1Q standard if I understood it
>> correctly.
>>
>> This is what I see.
>> =======================================================================
>> From 802.1Q 2018, 8.6.9.1.1 SetCycleStartTime()
>>
>> If AdminBaseTime is set to the same time in the past in all bridges and
>> end stations, OperBaseTime is always in the past, and all cycles start
>> synchronized. Using AdminBaseTime in the past is appropriate when you
>> can start schedules prior to starting the application that uses the
>> schedules. Use of AdminBaseTime in the future is intended to change a
>> currently running schedule in all bridges and end stations to a new
>> schedule at a future time. Using AdminBaseTime in the future is
>> appropriate when schedules must be changed without stopping the
>> application
>> ========================================================================
>>
>
> What I meant here is the case that I already have an "oper" schedule
> running, so my "oper->base_time" is in the past, and I try to add an
> "admin" schedule with a "base_time" also in the past. What's the
> expected behavior in this case? The text about stopping/starting
> applications doesn't seem to apply to the way the tc subsystem interacts
> with the applications.
>
> I try to add an "admin" schedule with a "base_time" also in the past.
> What's the expected behavior in this case?

Ok got it. I don't think this behavior is explained in the spec. I would
assume a sane thing to do is to switch to admin schedule if
admin->base_time is newer than oper->base_time and flag
the ConfigChangeError to be compliant to the spec, but frankly speaking
I don't know how application is going to use this. It is a low priority
item IMO and can be added as needed.

Regards,

Murali
>>>
>>>>
>>>>> - SupportedListMax - Maximum supported Admin/Open shed list.
>>>>>
>>>>> Is there a plan to export these from driver through tc show or such
>>>>> command? The reason being, there would be applications developed to
>>>>> manage configuration/schedule of TSN nodes that would requires these
>>>>> information from the node. So would need a support either in tc or
>>>>> some other means to retrieve them from hardware or driver. That is my
>>>>> understanding...
>>>>>
>>>
>>> Hm, now I understamd what you meant here...
>>>
>>>>
>>>> Not sure what answer you expect to receive for "is there any plan".
>>>> You can go ahead and propose something, as long as it is reasonably
>>>> useful to have.
>>>
>>> ... if this is indeed useful, perhaps one way to do is to add a subcommand
>>> to TC_SETUP_QDISC_TAPRIO, so we can retrieve the stats/information we want
>>> from the driver. Similar to what cls_flower does.
>>>
>>
>> What I understand is that there will be some work done to allow auto
>> configuration of TSN nodes from user space and that would need access to
>> all or some of the above parameters along with tc command to configure
>> the same. May be a open source project for this or some custom
>> application? Any such projects existing??
>
> Yeah, this is a big missing piece for TSN. I've heard 'netopeer2' and
> 'sysrepo' mentioned when similar questions were asked, but I have still
> to take a look at them and see what's missing. (Or if they are the right
> tool for the job)
>
>

--
Murali Karicheri
Texas Instruments

2020-03-12 23:34:17

by Vinicius Costa Gomes

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

Hi,

Po Liu <[email protected]> writes:

> Hi Vinicius,
>
>
> Br,
> Po Liu
>
>> -----Original Message-----
>> From: Vinicius Costa Gomes <[email protected]>
>> Sent: 2020年2月22日 5:44
>> To: Po Liu <[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]; linux-
>> [email protected]; [email protected]
>> Cc: [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 Liu <[email protected]> writes:
>>
>> > 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.
>> >
>> > 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.
>> >
>>
>> Any updates on this series? If you think that there's something that I could help,
>> just tell.
>
> Sorry for the long time not involve the discussion. I am focus on other tsn code for tc flower.
> If you can take more about this preemption serial, that would be good.
>
> I summary some suggestions from Marali Karicheri and Ivan Khornonzhuk and by you and also others:
> - Add config the fragment size, hold advance, release advance and flags;
> My comments about the fragment size is in the Qbu spec limit the fragment size " the minimum non-final fragment size is 64,
> 128, 192, or 256 octets " this setting would affect the guardband setting for Qbv. But the ethtool setting could not involve this issues but by the taprio side.
> - " Furthermore, this setting could be extend for a serial setting for mac and traffic class." "Better not to using the traffic class concept."
> Could adding a serial setting by "ethtool --preemption xxx" or other name. I don' t think it is good to involve in the queue control since queues number may bigger than the TC number.
> - The ethtool is the better choice to configure the preemption
> I agree.

Just a quick update. I was able to dedicate some time to this, and have
something aproaching RFC-quality, but it needs more testing.

So, question, what were you using for testing this? Anything special?

And btw, thanks for the summary of the discussion.

>
> Thanks!
>>
>>
>> Cheers,
>> --
>> Vinicius


--
Vinicius

2020-03-13 06:01:00

by Po Liu

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




Br,
Po Liu

> -----Original Message-----
> From: Vinicius Costa Gomes <[email protected]>
> Sent: 2020年3月13日 7:35
> To: Po Liu <[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]; [email protected]
> Cc: [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]>; Murali Karicheri <[email protected]>; Ivan
> Khoronzhuk <[email protected]>
> Subject: RE: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame
> preemption of traffic classes
>
> Caution: EXT Email
>
> Hi,
>
> Po Liu <[email protected]> writes:
>
> > Hi Vinicius,
> >
> >
> > Br,
> > Po Liu
> >
> >> -----Original Message-----
> >> From: Vinicius Costa Gomes <[email protected]>
> >> Sent: 2020年2月22日 5:44
> >> To: Po Liu <[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]; linux- [email protected];
> >> [email protected]
> >> Cc: [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 Liu <[email protected]> writes:
> >>
> >> > 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.
> >> >
> >> > 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.
> >> >
> >>
> >> Any updates on this series? If you think that there's something that
> >> I could help, just tell.
> >
> > Sorry for the long time not involve the discussion. I am focus on other tsn
> code for tc flower.
> > If you can take more about this preemption serial, that would be good.
> >
> > I summary some suggestions from Marali Karicheri and Ivan Khornonzhuk
> and by you and also others:
> > - Add config the fragment size, hold advance, release advance and flags;
> > My comments about the fragment size is in the Qbu spec limit the
> > fragment size " the minimum non-final fragment size is 64, 128, 192, or 256
> octets " this setting would affect the guardband setting for Qbv. But the
> ethtool setting could not involve this issues but by the taprio side.
> > - " Furthermore, this setting could be extend for a serial setting for mac and
> traffic class." "Better not to using the traffic class concept."
> > Could adding a serial setting by "ethtool --preemption xxx" or other name.
> I don' t think it is good to involve in the queue control since queues number
> may bigger than the TC number.
> > - The ethtool is the better choice to configure the preemption
> > I agree.
>
> Just a quick update. I was able to dedicate some time to this, and have
> something aproaching RFC-quality, but it needs more testing.
>
> So, question, what were you using for testing this? Anything special?

I tested on http://www.nxp.com/LS1028A. There is nothing special for preemption.

>
> And btw, thanks for the summary of the discussion.


>
> >
> > Thanks!
> >>
> >>
> >> Cheers,
> >> --
> >> Vinicius
>
>
> --
> Vinicius

2020-03-18 14:09:15

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 03/12/2020 07:34 PM, Vinicius Costa Gomes wrote:
> Hi,
>
> Po Liu <[email protected]> writes:
>
>> Hi Vinicius,
>>
>>
>> Br,
>> Po Liu
>>
>>> -----Original Message-----
>>> From: Vinicius Costa Gomes <[email protected]>
>>> Sent: 2020年2月22日 5:44
>>> To: Po Liu <[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]; linux-
>>> [email protected]; [email protected]
>>> Cc: [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 Liu <[email protected]> writes:
>>>
>>>> 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.
>>>>
>>>> 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.
>>>>
>>>
>>> Any updates on this series? If you think that there's something that I could help,
>>> just tell.
>>
>> Sorry for the long time not involve the discussion. I am focus on other tsn code for tc flower.
>> If you can take more about this preemption serial, that would be good.
>>
>> I summary some suggestions from Marali Karicheri and Ivan Khornonzhuk and by you and also others:
>> - Add config the fragment size, hold advance, release advance and flags;
>> My comments about the fragment size is in the Qbu spec limit the fragment size " the minimum non-final fragment size is 64,
>> 128, 192, or 256 octets " this setting would affect the guardband setting for Qbv. But the ethtool setting could not involve this issues but by the taprio side.
>> - " Furthermore, this setting could be extend for a serial setting for mac and traffic class." "Better not to using the traffic class concept."
>> Could adding a serial setting by "ethtool --preemption xxx" or other name. I don' t think it is good to involve in the queue control since queues number may bigger than the TC number.
>> - The ethtool is the better choice to configure the preemption
>> I agree.
>
> Just a quick update. I was able to dedicate some time to this, and have
> something aproaching RFC-quality, but it needs more testing.
>
Great! I have got my frame preemption working on my SoC. Currently I am
using some defaults. I test it by using statistics provided by the
SoC. I will be able to integrate and test your patch using my internal
version and will include it in my patch to upstream once I am ready.

Regards,

Murali
> So, question, what were you using for testing this? Anything special?
>
> And btw, thanks for the summary of the discussion.
>
>>
>> Thanks!
>>>
>>>
>>> Cheers,
>>> --
>>> Vinicius
>
>

--
Murali Karicheri
Texas Instruments

2020-05-13 20:47:36

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 3/18/20 10:07 AM, Murali Karicheri wrote:
> Hi Vinicius,
>
> On 03/12/2020 07:34 PM, Vinicius Costa Gomes wrote:
>> Hi,
>>
>> Po Liu <[email protected]> writes:
>>
>>> Hi Vinicius,
>>>
>>>
>>> Br,
>>> Po Liu
>>>
>>>> -----Original Message-----
>>>> From: Vinicius Costa Gomes <[email protected]>
>>>> Sent: 2020年2月22日 5:44
>>>> To: Po Liu <[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]; linux-
>>>> [email protected]; [email protected]
>>>> Cc: [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 Liu <[email protected]> writes:
>>>>
>>>>> 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.
>>>>>
>>>>> 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.
>>>>>
>>>>
>>>> Any updates on this series? If you think that there's something that
>>>> I could help,
>>>> just tell.
>>>
>>> Sorry for the long time not involve the discussion. I am focus on
>>> other tsn code for tc flower.
>>> If you can take more about this preemption serial, that would be good.
>>>
>>> I summary some suggestions from Marali Karicheri and Ivan Khornonzhuk
>>> and by you and also others:
>>> - Add config the fragment size, hold advance, release advance and flags;
>>>      My comments about the fragment size is in the Qbu spec limit the
>>> fragment size " the minimum non-final fragment size is 64,
>>> 128, 192, or 256 octets " this setting would affect the guardband
>>> setting for Qbv. But the ethtool setting could not involve this
>>> issues but by the taprio side.
>>> - " Furthermore, this setting could be extend for a serial setting
>>> for mac and traffic class."  "Better not to using the traffic class
>>> concept."
>>>     Could adding a serial setting by "ethtool --preemption xxx" or
>>> other name. I don' t think it is good to involve in the queue control
>>> since queues number may bigger than the TC number.
>>> - The ethtool is the better choice to configure the preemption
>>>    I agree.
>>
>> Just a quick update. I was able to dedicate some time to this, and have
>> something aproaching RFC-quality, but it needs more testing.
>>
> Great! I have got my frame preemption working on my SoC. Currently I am
> using some defaults. I test it by using statistics provided by the
> SoC. I will be able to integrate and test your patch using my internal
> version and will include it in my patch to upstream once I am ready.
>
Any progress on your side for a patch for the support?

I have posted my EST offload series for AM65x CPSW to netdev list today
at

https://marc.info/?l=linux-netdev&m=158937640015582&w=2
https://marc.info/?l=linux-netdev&m=158937639515579&w=2
https://marc.info/?l=linux-netdev&m=158937638315573&w=2

Next on my list of things to do is the IET FPE support for which I need
to have ethtool interface to allow configuring the express/preemptible
queues and feature enable/disable. Currently I am using a ethtool
priv-flags and some defaults. If you can post a patch, I will be able
to integrate and test it on AM65x CPSW driver and provide my comments/
Tested-by:

Regards,

Murali

> Regards,
>
> Murali
>> So, question, what were you using for testing this? Anything special?
>>
>> And btw, thanks for the summary of the discussion.
>>
>>>
>>> Thanks!
>>>>
>>>>
>>>> Cheers,
>>>> --
>>>> Vinicius
>>
>>
>

--
Murali Karicheri
Texas Instruments