| 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: <76dd6141-5852-43ae-af98-f0edf0bc10f5@kernel.org>
Date: Thu, 14 Nov 2024 11:41:06 +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 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.
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