netdev - Re: [PATCH net-next v3 2/2] net: ethernet: ti: am65-cpsw: enable DSCP to priority map for RX

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ZzX4eD/0i8SOOZGP@debian>
Date: Thu, 14 Nov 2024 14:17:44 +0100
From: Guillaume Nault <gnault@...hat.com>
To: Roger Quadros <rogerq@...nel.org>
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 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
>