netdev - RE: [PATCH net-next v2 2/2] net: macb: Add support for partial store and forward

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:
 <BN8PR12MB28522459ECB41284DF38F61AF7759@BN8PR12MB2852.namprd12.prod.outlook.com>
Date: Fri, 12 May 2023 11:53:00 +0000
From: "Somisetty, Pranavi" <pranavi.somisetty@....com>
To: "Claudiu.Beznea@...rochip.com" <Claudiu.Beznea@...rochip.com>,
	"Nicolas.Ferre@...rochip.com" <Nicolas.Ferre@...rochip.com>,
	"davem@...emloft.net" <davem@...emloft.net>, "edumazet@...gle.com"
	<edumazet@...gle.com>, "kuba@...nel.org" <kuba@...nel.org>,
	"pabeni@...hat.com" <pabeni@...hat.com>, "richardcochran@...il.com"
	<richardcochran@...il.com>, "linux@...linux.org.uk" <linux@...linux.org.uk>,
	"palmer@...belt.com" <palmer@...belt.com>
CC: "git (AMD-Xilinx)" <git@....com>, "Simek, Michal" <michal.simek@....com>,
	"Katakam, Harini" <harini.katakam@....com>, "Pandey, Radhey Shyam"
	<radhey.shyam.pandey@....com>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-riscv@...ts.infradead.org"
	<linux-riscv@...ts.infradead.org>
Subject: RE: [PATCH net-next v2 2/2] net: macb: Add support for partial store
 and forward



> -----Original Message-----
> From: Claudiu.Beznea@...rochip.com <Claudiu.Beznea@...rochip.com>
> Sent: Friday, May 12, 2023 1:28 PM
> To: Somisetty, Pranavi <pranavi.somisetty@....com>;
> Nicolas.Ferre@...rochip.com; davem@...emloft.net;
> edumazet@...gle.com; kuba@...nel.org; pabeni@...hat.com;
> richardcochran@...il.com; linux@...linux.org.uk; palmer@...belt.com
> Cc: git (AMD-Xilinx) <git@....com>; Simek, Michal
> <michal.simek@....com>; Katakam, Harini <harini.katakam@....com>;
> Pandey, Radhey Shyam <radhey.shyam.pandey@....com>;
> netdev@...r.kernel.org; linux-kernel@...r.kernel.org; linux-
> riscv@...ts.infradead.org
> Subject: Re: [PATCH net-next v2 2/2] net: macb: Add support for partial store
> and forward
> 
> On 11.05.2023 10:12, Pranavi Somisetty wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know
> > the content is safe
> >
> > When the receive partial store and forward mode is activated, the
> > receiver will only begin to forward the packet to the external AHB or
> > AXI slave when enough packet data is stored in the packet buffer.
> > The amount of packet data required to activate the forwarding process
> > is programmable via watermark registers which are located at the same
> > address as the partial store and forward enable bits. Adding support
> > to read this rx-watermark value from device-tree, to program the
> > watermark registers and enable partial store and forwarding.
> >
> > Signed-off-by: Maulik Jodhani <maulik.jodhani@...inx.com>
> > Signed-off-by: Michal Simek <michal.simek@...inx.com>
> > Signed-off-by: Harini Katakam <harini.katakam@...inx.com>
> > Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@...inx.com>
> > Signed-off-by: Pranavi Somisetty <pranavi.somisetty@....com>
> > ---

<snip>
> > +       /* Disable RX partial store and forward and reset watermark value */
> > +       if (bp->caps & MACB_CAPS_PARTIAL_STORE_FORWARD) {
> > +               watermark_reset_value = (1 << (GEM_BFEXT(RX_PBUF_ADDR,
> > + gem_readl(bp, DCFG2)))) - 1;
> 
> Is this block needed? Maybe all you need here is just to disable the rx partial
> store and forward?

Yes, the value doesn’t need to be written, disabling rx partial store and forward
is enough, will take care.
<snip>
> 
> > +                       retval = of_property_read_u16(bp->pdev->dev.of_node,
> > +                                                     "rx-watermark",
> > +
> > + &bp->rx_watermark);
> 
> E.g. SAMA7G5 has PBUFRXCUT.watermark on 10 bits. Is it the same on
> Xynqmp?

On ZynqMP its 12 bits.

> For compatibility with future implementations and stable DT interface it
> would be better to just keep rx-watermark DT property on 32 bits.
> 

I don’t think it can extend beyond u16, considering, the maximum pbuf depth
field in DCFG2, is only 4 bits (22:25). 

I'll do a re-spin addressing your and other reviewer's comments.

Thanks
Pranavi