Re: [v1,ethtool] ethtool: add setting frame preemption of traffic classes

From: Murali Karicheri
Date: Tue Dec 03 2019 - 12:37:55 EST


Hi Po Liu,

Thanks for working on this! Some suggestion below as we are working on
adding this support to Texas Instrument's CPSW driver on AM65x family
of SoCs as well. TRM for that is provided below for your reference.
Relevant section for IET (Frame pre-emption) is

12.2.1.4.6.6.1IET Configuration

http://www.ti.com/lit/ug/spruid7d/spruid7d.pdf

On 12/03/2019 11:27 AM, Ivan Khoronzhuk wrote:
On Wed, Nov 27, 2019 at 09:58:52AM +0000, Po Liu wrote:

Hi Po Liu,

IEEE Std 802.1Qbu standard defined the frame preemption of port
trffic classes. User can set a value to hardware. 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.

ethtool -k devname

This command would show if the tx-preemption feature is available.
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.

ethtool devname

This command would show include an item 'preemption'. A following
value '0' means all traffic classes are 'express'. A value none zero
means traffic classes preemption capabilities. 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.

ethtool -s devname preemption N

What about other potential parameters like MAC fragment size, mac hold?
Shouldn't be it considered along with other FP parameters to provide correct
interface later?

Say, preemption, lets name it fp-mask or frame-preemption-mask.
Then other potential setting can be similar and added later:

frame-preemption-mask
frame-preemption-fragsize
frame-preemption-machold
Need additional capabilities as described by Ivan above. Thanks Ivan!

So it would be better to use feature/sub-parameter format so that it can be extended as needed in the future.

For express/Preemptable mask setting it becomes

ethtool -s devname frame-preemption tc-mask N

For setting min fragment size

ethtool -s devname frame-preemption min-fragsize 64

Also the device may be capable of doing Verify process to detect the
capability of neighbor device and show the status. So we should have a
way to show this status as well when user type

ethtool devname

We are working currently to add this feature to CPSW driver on AM65x
which will be upstream-ed soon. So want to have this done in an
way that we can extend it later.

Similarly for taprio, there are some parameters that user
might want to tune such as Max SDU size per tc class. I hope
we could use ethtool to set the same on a similar way.

Thanks

Murali

....

mac-hold it's rather flag, at least I've used it as priv-flag.
so can or so

frame-preemption-flags


This command would set which traffic classes are frame preemptable.
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.

Signed-off-by: Po Liu <Po.Liu@xxxxxxx>
---
ethtool-copy.h |Â 6 +++++-
ethtool.8.inÂÂ |Â 8 ++++++++
ethtool.cÂÂÂÂÂ | 18 ++++++++++++++++++
3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/ethtool-copy.h b/ethtool-copy.h
index 9afd2e6..e04bdf3 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -1662,6 +1662,9 @@ static __inline__ int ethtool_validate_duplex(__u8 duplex)
#define AUTONEG_DISABLEÂÂÂÂÂÂÂ 0x00
#define AUTONEG_ENABLEÂÂÂÂÂÂÂ 0x01

+/* Disable preemtion. */
+#define PREEMPTION_DISABLEÂÂÂ 0x0
+
/* MDI or MDI-X status/control - if MDI/MDI_X/AUTO is set then
Â* the driver is required to renegotiate link
Â*/
@@ -1878,7 +1881,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/ethtool.8.in b/ethtool.8.in
index 062695a..7d612b2 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -236,6 +236,7 @@ ethtool \- query or control network driver and hardware settings
.B2 autoneg on off
.BN advertise
.BN phyad
+.BN preemption
.B2 xcvr internal external
.RB [ wol \ \*(WO]
.RB [ sopass \ \*(MA]
@@ -703,6 +704,13 @@ lBÂÂÂ lÂÂÂ lB.
.BI phyad \ N
PHY address.
.TP
+.BI preemption \ N
+Set preemptable traffic classes by bits.
+.B A
+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.
+.TP
.A2 xcvr internal external
Selects transceiver type. Currently only internal and external can be
specified, in the future further types might be added.
diff --git a/ethtool.c b/ethtool.c
index acf183d..d5240f8 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -928,6 +928,12 @@ dump_link_usettings(const struct ethtool_link_usettings *link_usettings)
ÂÂÂÂÂÂÂ }
ÂÂÂÂ}

+ÂÂÂ if (link_usettings->base.preemption == PREEMPTION_DISABLE)
+ÂÂÂÂÂÂÂ fprintf(stdout, "ÂÂÂ Preemption: 0x0 (off)\n");
+ÂÂÂ else
+ÂÂÂÂÂÂÂ fprintf(stdout, "ÂÂÂ Preemption: 0x%x\n",
+ÂÂÂÂÂÂÂÂÂÂÂ link_usettings->base.preemption);
+
ÂÂÂÂreturn 0;
}

@@ -2869,6 +2875,7 @@ static int do_sset(struct cmd_context *ctx)
ÂÂÂÂint port_wanted = -1;
ÂÂÂÂint mdix_wanted = -1;
ÂÂÂÂint autoneg_wanted = -1;
+ÂÂÂ int preemption_wanted = -1;
ÂÂÂÂint phyad_wanted = -1;
ÂÂÂÂint xcvr_wanted = -1;
ÂÂÂÂu32 *full_advertising_wanted = NULL;
@@ -2957,6 +2964,12 @@ static int do_sset(struct cmd_context *ctx)
ÂÂÂÂÂÂÂÂÂÂÂ } else {
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ exit_bad_args();
ÂÂÂÂÂÂÂÂÂÂÂ }
+ÂÂÂÂÂÂÂ } else if (!strcmp(argp[i], "preemption")) {
+ÂÂÂÂÂÂÂÂÂÂÂ gset_changed = 1;
+ÂÂÂÂÂÂÂÂÂÂÂ i += 1;
+ÂÂÂÂÂÂÂÂÂÂÂ if (i >= argc)
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ exit_bad_args();
+ÂÂÂÂÂÂÂÂÂÂÂ preemption_wanted = get_u32(argp[i], 16);
ÂÂÂÂÂÂÂ } else if (!strcmp(argp[i], "advertise")) {
ÂÂÂÂÂÂÂÂÂÂÂ gset_changed = 1;
ÂÂÂÂÂÂÂÂÂÂÂ i += 1;
@@ -3094,6 +3107,9 @@ static int do_sset(struct cmd_context *ctx)
ÂÂÂÂÂÂÂÂÂÂÂ }
ÂÂÂÂÂÂÂÂÂÂÂ if (autoneg_wanted != -1)
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ link_usettings->base.autoneg = autoneg_wanted;
+ÂÂÂÂÂÂÂÂÂÂÂ if (preemption_wanted != -1)
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ link_usettings->base.preemption
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ = preemption_wanted;
ÂÂÂÂÂÂÂÂÂÂÂ if (phyad_wanted != -1)
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ link_usettings->base.phy_address = phyad_wanted;
ÂÂÂÂÂÂÂÂÂÂÂ if (xcvr_wanted != -1)
@@ -3186,6 +3202,8 @@ static int do_sset(struct cmd_context *ctx)
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ fprintf(stderr, "Â not setting transceiver\n");
ÂÂÂÂÂÂÂÂÂÂÂ if (mdix_wanted != -1)
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ fprintf(stderr, "Â not setting mdix\n");
+ÂÂÂÂÂÂÂÂÂÂÂ if (preemption_wanted != -1)
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ fprintf(stderr, "Â not setting preemption\n");
ÂÂÂÂÂÂÂ }
ÂÂÂÂ}

--
2.17.1