* [v1,ethtool] ethtool: add setting frame preemption of traffic classes @ 2019-11-27 9:58 Po Liu 2019-12-03 16:27 ` Ivan Khoronzhuk 0 siblings, 1 reply; 5+ messages in thread From: Po Liu @ 2019-11-27 9:58 UTC (permalink / raw) To: rmk+kernel, linville, netdev-owner, davem, linux-kernel, netdev Cc: vinicius.gomes, simon.horman, Claudiu Manoil, Vladimir Oltean, Xiaoliang Yang, Roy Zang, Mingkai Hu, Jerry Huang, Leo Li, 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 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@nxp.com> --- 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [v1,ethtool] ethtool: add setting frame preemption of traffic classes 2019-11-27 9:58 [v1,ethtool] ethtool: add setting frame preemption of traffic classes Po Liu @ 2019-12-03 16:27 ` Ivan Khoronzhuk 2019-12-03 17:42 ` Murali Karicheri 0 siblings, 1 reply; 5+ messages in thread From: Ivan Khoronzhuk @ 2019-12-03 16:27 UTC (permalink / raw) To: Po Liu Cc: rmk+kernel, linville, netdev-owner, davem, linux-kernel, netdev, vinicius.gomes, simon.horman, Claudiu Manoil, Vladimir Oltean, Xiaoliang Yang, Roy Zang, Mingkai Hu, Jerry Huang, Leo Li 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 .... 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@nxp.com> >--- > 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 > -- Regards, Ivan Khoronzhuk ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [v1,ethtool] ethtool: add setting frame preemption of traffic classes 2019-12-03 16:27 ` Ivan Khoronzhuk @ 2019-12-03 17:42 ` Murali Karicheri 2019-12-04 8:14 ` [EXT] " Po Liu 0 siblings, 1 reply; 5+ messages in thread From: Murali Karicheri @ 2019-12-03 17:42 UTC (permalink / raw) To: Po Liu, rmk+kernel, linville, netdev-owner, davem, linux-kernel, netdev, vinicius.gomes, simon.horman, Claudiu Manoil, Vladimir Oltean, Xiaoliang Yang, Roy Zang, Mingkai Hu, Jerry Huang, Leo Li 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@nxp.com> >> --- >> 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 >> > ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [EXT] Re: [v1,ethtool] ethtool: add setting frame preemption of traffic classes 2019-12-03 17:42 ` Murali Karicheri @ 2019-12-04 8:14 ` Po Liu 2019-12-04 16:59 ` Murali Karicheri 0 siblings, 1 reply; 5+ messages in thread From: Po Liu @ 2019-12-04 8:14 UTC (permalink / raw) To: Murali Karicheri, rmk+kernel, linville, netdev-owner, davem, linux-kernel, netdev, vinicius.gomes, simon.horman, Claudiu Manoil, Vladimir Oltean, Xiaoliang Yang, Roy Zang, Mingkai Hu, Jerry Huang, Leo Li, Ivan Khoronzhuk Hi Murali, Ivan, Thank you for your feedback. Maybe it is better to use "RFC" type for these patches for the discussion. Br, Po Liu > -----Original Message----- > From: Murali Karicheri <m-karicheri2@ti.com> > Sent: 2019年12月4日 1:42 > To: Po Liu <po.liu@nxp.com>; rmk+kernel@armlinux.org.uk; > linville@tuxdriver.com; netdev-owner@vger.kernel.org; davem@davemloft.net; > linux-kernel@vger.kernel.org; netdev@vger.kernel.org; > vinicius.gomes@intel.com; simon.horman@netronome.com; Claudiu Manoil > <claudiu.manoil@nxp.com>; Vladimir Oltean <vladimir.oltean@nxp.com>; > Xiaoliang Yang <xiaoliang.yang_1@nxp.com>; Roy Zang <roy.zang@nxp.com>; > Mingkai Hu <mingkai.hu@nxp.com>; Jerry Huang <jerry.huang@nxp.com>; Leo > Li <leoyang.li@nxp.com> > Subject: [EXT] Re: [v1,ethtool] ethtool: add setting frame preemption of traffic > classes > > Caution: EXT Email > > 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 > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.ti.c > om%2Flit%2Fug%2Fspruid7d%2Fspruid7d.pdf&data=02%7C01%7Cpo.liu% > 40nxp.com%7C15544a9db1ff422e2fdf08d77817647f%7C686ea1d3bc2b4c6fa > 92cd99c5c301635%7C0%7C1%7C637109914194485742&sdata=VD5kg% > 2FY1SDDWjwMZHjgUNlFrcvXnDvdIPGblWkx4DXs%3D&reserved=0 > > 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 > I thought the fragment size set 64 is enough for Qbv. Anyway, if you prefer more details setting. I think it is better to set a specific serial type. For example, '-p' for set tc-mask N and min-fragsize 64, '-P' for status. > 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 The verify process maybe disabled as default. > 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. Furthermore, this setting could be extend for a serial setting for mac and traffic class. > > 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@nxp.com> > >> --- > >> 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 > >> > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [EXT] Re: [v1,ethtool] ethtool: add setting frame preemption of traffic classes 2019-12-04 8:14 ` [EXT] " Po Liu @ 2019-12-04 16:59 ` Murali Karicheri 0 siblings, 0 replies; 5+ messages in thread From: Murali Karicheri @ 2019-12-04 16:59 UTC (permalink / raw) To: Po Liu, rmk+kernel, linville, netdev-owner, davem, linux-kernel, netdev, vinicius.gomes, simon.horman, Claudiu Manoil, Vladimir Oltean, Xiaoliang Yang, Roy Zang, Mingkai Hu, Jerry Huang, Leo Li, Ivan Khoronzhuk On 12/04/2019 03:14 AM, Po Liu wrote: > Hi Murali, Ivan, > > Thank you for your feedback. Maybe it is better to use "RFC" type for these patches for the discussion. > Yes please. Murali > > Br, > Po Liu > >> -----Original Message----- >> From: Murali Karicheri <m-karicheri2@ti.com> >> Sent: 2019年12月4日 1:42 >> To: Po Liu <po.liu@nxp.com>; rmk+kernel@armlinux.org.uk; >> linville@tuxdriver.com; netdev-owner@vger.kernel.org; davem@davemloft.net; >> linux-kernel@vger.kernel.org; netdev@vger.kernel.org; >> vinicius.gomes@intel.com; simon.horman@netronome.com; Claudiu Manoil >> <claudiu.manoil@nxp.com>; Vladimir Oltean <vladimir.oltean@nxp.com>; >> Xiaoliang Yang <xiaoliang.yang_1@nxp.com>; Roy Zang <roy.zang@nxp.com>; >> Mingkai Hu <mingkai.hu@nxp.com>; Jerry Huang <jerry.huang@nxp.com>; Leo >> Li <leoyang.li@nxp.com> >> Subject: [EXT] Re: [v1,ethtool] ethtool: add setting frame preemption of traffic >> classes >> >> Caution: EXT Email >> >> 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 >> >> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.ti.c >> om%2Flit%2Fug%2Fspruid7d%2Fspruid7d.pdf&data=02%7C01%7Cpo.liu% >> 40nxp.com%7C15544a9db1ff422e2fdf08d77817647f%7C686ea1d3bc2b4c6fa >> 92cd99c5c301635%7C0%7C1%7C637109914194485742&sdata=VD5kg% >> 2FY1SDDWjwMZHjgUNlFrcvXnDvdIPGblWkx4DXs%3D&reserved=0 >> >> 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 >> > > I thought the fragment size set 64 is enough for Qbv. Anyway, if you prefer more details setting. I think it is better to set a specific serial type. For example, '-p' for set tc-mask N and min-fragsize 64, '-P' for status. > >> 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 > > The verify process maybe disabled as default. > >> 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. > > Furthermore, this setting could be extend for a serial setting for mac and traffic class. > >> >> 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@nxp.com> >>>> --- >>>> 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 >>>> >>> > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-12-04 16:56 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-11-27 9:58 [v1,ethtool] ethtool: add setting frame preemption of traffic classes Po Liu 2019-12-03 16:27 ` Ivan Khoronzhuk 2019-12-03 17:42 ` Murali Karicheri 2019-12-04 8:14 ` [EXT] " Po Liu 2019-12-04 16:59 ` Murali Karicheri
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).