| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1e451e89-a801-5f6d-e7e6-5563dc95267d@ti.com>
Date: Wed, 4 Dec 2019 11:59:49 -0500
From: Murali Karicheri <m-karicheri2@...com>
To: Po Liu <po.liu@....com>,
"rmk+kernel@...linux.org.uk" <rmk+kernel@...linux.org.uk>,
"linville@...driver.com" <linville@...driver.com>,
"netdev-owner@...r.kernel.org" <netdev-owner@...r.kernel.org>,
"davem@...emloft.net" <davem@...emloft.net>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"vinicius.gomes@...el.com" <vinicius.gomes@...el.com>,
"simon.horman@...ronome.com" <simon.horman@...ronome.com>,
Claudiu Manoil <claudiu.manoil@....com>,
Vladimir Oltean <vladimir.oltean@....com>,
Xiaoliang Yang <xiaoliang.yang_1@....com>,
Roy Zang <roy.zang@....com>, Mingkai Hu <mingkai.hu@....com>,
Jerry Huang <jerry.huang@....com>, Leo Li <leoyang.li@....com>,
Ivan Khoronzhuk <ivan.khoronzhuk@...aro.org>
Subject: Re: [EXT] Re: [v1,ethtool] ethtool: add setting frame preemption of
traffic classes
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@...com>
>> Sent: 2019年12月4日 1:42
>> To: Po Liu <po.liu@....com>; rmk+kernel@...linux.org.uk;
>> linville@...driver.com; netdev-owner@...r.kernel.org; davem@...emloft.net;
>> linux-kernel@...r.kernel.org; netdev@...r.kernel.org;
>> vinicius.gomes@...el.com; simon.horman@...ronome.com; Claudiu Manoil
>> <claudiu.manoil@....com>; Vladimir Oltean <vladimir.oltean@....com>;
>> Xiaoliang Yang <xiaoliang.yang_1@....com>; Roy Zang <roy.zang@....com>;
>> Mingkai Hu <mingkai.hu@....com>; Jerry Huang <jerry.huang@....com>; Leo
>> Li <leoyang.li@....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@....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
>>>>
>>>
>
Powered by blists - more mailing lists