| Message ID | 20241109-am65-cpsw-multi-rx-dscp-v3-2-1cfb76928490@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | net: ethernet: ti: am65-cpsw: enable DSCP to priority map for RX | expand |
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@kernel.org> Reviewed-by: Siddharth Vadapalli <s-vadapalli@ti.com> Regards, Siddharth.
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@kernel.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 values. Then why restricting the mapping to the 3 high order bits only? 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 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 > 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 > >
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@kernel.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 >> >> >
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@kernel.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 >>> >>> >> >
On Thu, Nov 14, 2024 at 12:12:47PM +0200, Roger Quadros wrote: > On 14/11/2024 11:41, Roger Quadros wrote: > > On 14/11/2024 02:16, Guillaume Nault wrote: > >> 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? Looks generally good to me. A few comments inline though. > 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 Maybe also add a link to https://datatracker.ietf.org/doc/html/rfc8622#section-11 which defines the LE PHB (Low Effort) and updates RFC 8325 accordingly. > */ > 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 */ Until case 32 (CS4) you've kept the order of RFC 8325, table 1. It'd make life easier for reviewers if you could keep this order here. That is, moving CS4 after AF43 and CS3 after AF33. > pri = 4; > break; > case 17: /* AF21 */ AF21 is 18, not 17. > case 20: /* AF22 */ > case 22: /* AF23 */ > pri = 3; > break; > case 8: /* CS1 */ Let's be complete and add the case for LE (RFC 8622), which also maps to 1. > 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 >
On Thu, Nov 14, 2024 at 11:41:06AM +0200, Roger Quadros wrote: > On 14/11/2024 02:16, Guillaume Nault wrote: > > 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? I'm not aware of any (appart from manual update with tc), but I could have missed something. It'd probably make sense to have such a mechanism though. > >> 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 >
On 14/11/2024 14:02, Guillaume Nault wrote: > On Thu, Nov 14, 2024 at 12:12:47PM +0200, Roger Quadros wrote: >> On 14/11/2024 11:41, Roger Quadros wrote: >>> On 14/11/2024 02:16, Guillaume Nault wrote: >>>> 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? > > Looks generally good to me. A few comments inline though. > >> 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 > > Maybe also add a link to > https://datatracker.ietf.org/doc/html/rfc8622#section-11 > which defines the LE PHB (Low Effort) and updates RFC 8325 accordingly. > >> */ >> 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 */ > > Until case 32 (CS4) you've kept the order of RFC 8325, table 1. > It'd make life easier for reviewers if you could keep this order > here. That is, moving CS4 after AF43 and CS3 after AF33. > >> pri = 4; >> break; >> case 17: /* AF21 */ > > AF21 is 18, not 17. > >> case 20: /* AF22 */ >> case 22: /* AF23 */ >> pri = 3; >> break; >> case 8: /* CS1 */ > > Let's be complete and add the case for LE (RFC 8622), which also > maps to 1. All comments are valid. I will fix and send v4 for this series. > >> pri = 1; >> break; For sake of completeness I will mention CS2, AF11, AF12, AF13 here that can fallback to default case. >> 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); >> } >> >>>
On Thu, Nov 14, 2024 at 02:47:07PM +0200, Roger Quadros wrote: > > > On 14/11/2024 14:02, Guillaume Nault wrote: > > On Thu, Nov 14, 2024 at 12:12:47PM +0200, Roger Quadros wrote: > >> On 14/11/2024 11:41, Roger Quadros wrote: > >>> On 14/11/2024 02:16, Guillaume Nault wrote: > >>>> 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? > > > > Looks generally good to me. A few comments inline though. > > > >> 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 > > > > Maybe also add a link to > > https://datatracker.ietf.org/doc/html/rfc8622#section-11 > > which defines the LE PHB (Low Effort) and updates RFC 8325 accordingly. > > > >> */ > >> 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 */ > > > > Until case 32 (CS4) you've kept the order of RFC 8325, table 1. > > It'd make life easier for reviewers if you could keep this order > > here. That is, moving CS4 after AF43 and CS3 after AF33. > > > >> pri = 4; > >> break; > >> case 17: /* AF21 */ > > > > AF21 is 18, not 17. > > > >> case 20: /* AF22 */ > >> case 22: /* AF23 */ > >> pri = 3; > >> break; > >> case 8: /* CS1 */ > > > > Let's be complete and add the case for LE (RFC 8622), which also > > maps to 1. > > All comments are valid. I will fix and send v4 for this series. > > > > >> pri = 1; > >> break; > > For sake of completeness I will mention CS2, AF11, AF12, AF13 > here that can fallback to default case. Yes, very nice. > >> 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); > >> } > >> > >>> > > -- > cheers, > -roger >
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); +} + 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);
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@kernel.org> --- drivers/net/ethernet/ti/am65-cpsw-nuss.c | 54 ++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+)