| 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: <8bfe8acc-9514-4ba8-9498-2427ddb0bb78@kernel.org>
Date: Thu, 14 Nov 2024 12:12:47 +0200
From: Roger Quadros <rogerq@...nel.org>
To: Guillaume Nault <gnault@...hat.com>
Cc: Siddharth Vadapalli <s-vadapalli@...com>,
Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Simon Horman <horms@...nel.org>, linux-omap@...r.kernel.org,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org, srk@...com,
Pekka Varis <p-varis@...com>
Subject: Re: [PATCH net-next v3 2/2] net: ethernet: ti: am65-cpsw: enable DSCP
to priority map for RX
On 14/11/2024 11:41, Roger Quadros wrote:
>
>
> On 14/11/2024 02:16, Guillaume Nault wrote:
>> On Sat, Nov 09, 2024 at 01:00:08PM +0200, Roger Quadros wrote:
>>> AM65 CPSW hardware can map the 6-bit DSCP/TOS field to
>>> appropriate priority queue via DSCP to Priority mapping registers
>>> (CPSW_PN_RX_PRI_MAP_REG).
>>>
>>> We use the upper 3 bits of the DSCP field that indicate IP Precedence
>>> to map traffic to 8 priority queues.
>>>
>>> Signed-off-by: Roger Quadros <rogerq@...nel.org>
>>> ---
>>> drivers/net/ethernet/ti/am65-cpsw-nuss.c | 54 ++++++++++++++++++++++++++++++++
>>> 1 file changed, 54 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
>>> index 0520e9f4bea7..fab35e6aac7f 100644
>>> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
>>> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
>>> @@ -71,6 +71,8 @@
>>> #define AM65_CPSW_PORT_REG_RX_PRI_MAP 0x020
>>> #define AM65_CPSW_PORT_REG_RX_MAXLEN 0x024
>>>
>>> +#define AM65_CPSW_PORTN_REG_CTL 0x004
>>> +#define AM65_CPSW_PORTN_REG_DSCP_MAP 0x120
>>> #define AM65_CPSW_PORTN_REG_SA_L 0x308
>>> #define AM65_CPSW_PORTN_REG_SA_H 0x30c
>>> #define AM65_CPSW_PORTN_REG_TS_CTL 0x310
>>> @@ -94,6 +96,10 @@
>>> /* AM65_CPSW_PORT_REG_PRI_CTL */
>>> #define AM65_CPSW_PORT_REG_PRI_CTL_RX_PTYPE_RROBIN BIT(8)
>>>
>>> +/* AM65_CPSW_PN_REG_CTL */
>>> +#define AM65_CPSW_PN_REG_CTL_DSCP_IPV4_EN BIT(1)
>>> +#define AM65_CPSW_PN_REG_CTL_DSCP_IPV6_EN BIT(2)
>>> +
>>> /* AM65_CPSW_PN_TS_CTL register fields */
>>> #define AM65_CPSW_PN_TS_CTL_TX_ANX_F_EN BIT(4)
>>> #define AM65_CPSW_PN_TS_CTL_TX_VLAN_LT1_EN BIT(5)
>>> @@ -176,6 +182,53 @@ static void am65_cpsw_port_set_sl_mac(struct am65_cpsw_port *slave,
>>> writel(mac_lo, slave->port_base + AM65_CPSW_PORTN_REG_SA_L);
>>> }
>>>
>>> +#define AM65_CPSW_DSCP_MAX GENMASK(5, 0)
>>> +#define AM65_CPSW_PRI_MAX GENMASK(2, 0)
>>> +#define AM65_CPSW_DSCP_PRI_PER_REG 8
>>> +#define AM65_CPSW_DSCP_PRI_SIZE 4 /* in bits */
>>> +static int am65_cpsw_port_set_dscp_map(struct am65_cpsw_port *slave, u8 dscp, u8 pri)
>>> +{
>>> + int reg_ofs;
>>> + int bit_ofs;
>>> + u32 val;
>>> +
>>> + if (dscp > AM65_CPSW_DSCP_MAX)
>>> + return -EINVAL;
>>> +
>>> + if (pri > AM65_CPSW_PRI_MAX)
>>> + return -EINVAL;
>>> +
>>> + /* 32-bit register offset to this dscp */
>>> + reg_ofs = (dscp / AM65_CPSW_DSCP_PRI_PER_REG) * 4;
>>> + /* bit field offset to this dscp */
>>> + bit_ofs = AM65_CPSW_DSCP_PRI_SIZE * (dscp % AM65_CPSW_DSCP_PRI_PER_REG);
>>> +
>>> + val = readl(slave->port_base + AM65_CPSW_PORTN_REG_DSCP_MAP + reg_ofs);
>>> + val &= ~(AM65_CPSW_PRI_MAX << bit_ofs); /* clear */
>>> + val |= pri << bit_ofs; /* set */
>>> + writel(val, slave->port_base + AM65_CPSW_PORTN_REG_DSCP_MAP + reg_ofs);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void am65_cpsw_port_enable_dscp_map(struct am65_cpsw_port *slave)
>>> +{
>>> + int dscp, pri;
>>> + u32 val;
>>> +
>>> + /* Map IP Precedence field to Priority */
>>> + for (dscp = 0; dscp <= AM65_CPSW_DSCP_MAX; dscp++) {
>>> + pri = dscp >> 3; /* Extract IP Precedence */
>>> + am65_cpsw_port_set_dscp_map(slave, dscp, pri);
>>> + }
>>> +
>>> + /* enable port IPV4 and IPV6 DSCP for this port */
>>> + val = readl(slave->port_base + AM65_CPSW_PORTN_REG_CTL);
>>> + val |= AM65_CPSW_PN_REG_CTL_DSCP_IPV4_EN |
>>> + AM65_CPSW_PN_REG_CTL_DSCP_IPV6_EN;
>>> + writel(val, slave->port_base + AM65_CPSW_PORTN_REG_CTL);
>>> +}
>>
>> It seems that this hardware is capable of mapping all possible DSCP
> yes.
>
>> values. Then why restricting the mapping to the 3 high order bits only?
>
> Currently, the 64 DSCP values are mapped to 8 User Priorities (UP) based
> on just the Class Selector Codepoint field (first 3 bits of DSCP).
>
> But now looking at rfc8325#section-4.3.
> "Note: All unused codepoints are RECOMMENDED to be mapped to UP 0"
>
> So what this patch does doesn't look like a good idea.
>
>> According to RFC 8325 section 2.3, this seem to be a common practice,
>> which this RFC considers a problem:
>> https://datatracker.ietf.org/doc/html/rfc8325#section-2.3
>
> Good to know about this.
>
>>
>> I know this RFC is about 802.11, not 802.1p, but as far as I know, the
>> user priority (UP) are the same for both, so that shouldn't make a
>> difference.
>>
>> So what about following the IETF mapping found in section 4.3?
>> https://datatracker.ietf.org/doc/html/rfc8325#section-4.3
>
> Thanks for this tip.
> I will update this patch to have the default DSCP to UP mapping as per
> above link and map all unused DSCP to UP 0.
How does the below code look in this regard?
static void am65_cpsw_port_enable_dscp_map(struct am65_cpsw_port *slave)
{
int dscp, pri;
u32 val;
/* Default DSCP to User Priority mapping as per:
* https://datatracker.ietf.org/doc/html/rfc8325#section-4.3
*/
for (dscp = 0; dscp <= AM65_CPSW_DSCP_MAX; dscp++) {
switch (dscp) {
case 56: /* CS7 */
case 48: /* CS6 */
pri = 7;
break;
case 46: /* EF */
case 44: /* VA */
pri = 6;
break;
case 40: /* CS5 */
pri = 5;
break;
case 32: /* CS4 */
case 34: /* AF41 */
case 36: /* AF42 */
case 38: /* AF43 */
case 24: /* CS3 */
case 26: /* AF31 */
case 28: /* AF32 */
case 30: /* AF33 */
pri = 4;
break;
case 17: /* AF21 */
case 20: /* AF22 */
case 22: /* AF23 */
pri = 3;
break;
case 8: /* CS1 */
pri = 1;
break;
default:
pri = 0;
break;
}
am65_cpsw_port_set_dscp_map(slave, dscp, pri);
}
/* enable port IPV4 and IPV6 DSCP for this port */
val = readl(slave->port_base + AM65_CPSW_PORTN_REG_CTL);
val |= AM65_CPSW_PN_REG_CTL_DSCP_IPV4_EN |
AM65_CPSW_PN_REG_CTL_DSCP_IPV6_EN;
writel(val, slave->port_base + AM65_CPSW_PORTN_REG_CTL);
}
>
> Is there any mechanism/API for network administrator to change this
> default mapping in the network drivers?
>
>>
>>> static void am65_cpsw_sl_ctl_reset(struct am65_cpsw_port *port)
>>> {
>>> cpsw_sl_reset(port->slave.mac_sl, 100);
>>> @@ -921,6 +974,7 @@ static int am65_cpsw_nuss_ndo_slave_open(struct net_device *ndev)
>>> common->usage_count++;
>>>
>>> am65_cpsw_port_set_sl_mac(port, ndev->dev_addr);
>>> + am65_cpsw_port_enable_dscp_map(port);
>>>
>>> if (common->is_emac_mode)
>>> am65_cpsw_init_port_emac_ale(port);
>>>
>>> --
>>> 2.34.1
>>>
>>>
>>
>
--
cheers,
-roger
Powered by blists - more mailing lists