Re: [PATCH net-next 3/4] net: macb: add safeguards for jumbo frame larger than 10240

[Charles Perry]

On Fri, Mar 06, 2026 at 01:04:26PM +0000, Simon Horman wrote:
> On Thu, Mar 05, 2026 at 06:24:24AM -0800, Charles Perry wrote:
> > On Thu, Mar 05, 2026 at 11:40:10AM +0000, Simon Horman wrote:
> > > On Tue, Mar 03, 2026 at 10:03:17AM -0800, Charles Perry wrote:
> > > > The RX buffers for GEM can have a maximum size of 16320 bytes
> > > > (0xff in the RXBS field of the DMACFG register means 255*64 =
> > > > 16320 bytes).
> > > > 
> > > > The "jumbo_max_length" field (bits 0..13) of the DCFG2 register
> > > > can take a value of up to 16383 (0x3FFF). This field is not used
> > > > when determining the max MTU, instead an hardcoded value
> > > > (jumbo_max_len) is used for each platform. Right now the maximum
> > > > value for jumbo_max_len is 10240 (0x2800).
> > > > 
> > > > GEM uses one buffer per packet which means that one buffer must
> > > > allow room for the max MTU plus L2 encapsulation and alignment.
> > > > 
> > > > This commit adds a limit to max_mtu and rx_buffer_size so that
> > > > the RXBS field can never overflow when a large MTU is used.
> > > > 
> > > > With this commit, it is now possible to add new platforms that
> > > > have their gem_jumbo_max_length set to 16383.
> > > > 
> > > > Signed-off-by: Charles Perry <charles.perry@xxxxxxxxxxxxx>
> > > 
> > > Hi Charles,
> > > 
> > > I am sorry if this question is a bit naïve.
> > > 
> > > I understand the need to clamp the max_mtu to avoid overflowing RXBS.
> > > And that this hasn't been an issue up until now due to the maximum
> > > value of jumbo_max_len used in the driver.
> > > 
> > > But I'm unclear on the relationship between DCFG2 and the max_mtu.
> > > Why does it need to be set to a value larger than that corresponding to
> > > the maximum mtu and RX buf size?
> > > 
> > 
> > Hello Simon,
> > 
> > The DCFG2 register is the max_mtu value, there's some public documentation
> > for this for AMD versal [1]. "gem_jumbo_max_length" is a define in the RTL
> > code, the hardware designer probably makes a tradeoff between gate count
> > and the max_mtu. The maximum value for this is 0x3FFF (16383).
> > 
> > The maximum buffer size is 255 * 64 = 16320
> > 
> > The GEM driver, in its current state, uses one buffer per frame, so the MTU
> > needs to be clamped at the maximum buffer size.
> > 
> > We could just set 16320 instead of 16383 into the "jumbo_max_len" of
> > "struct macb_config" but it would mix information about what the hardware
> > supports vs what the software support. My approach is to put what the
> > hardware support in "struct macb_config" and clamp it later when
> > calculating max_mtu because we know we have a software limitation.
> 
> Hi Charles,
> 
> Thanks for the explanation. I agree that it is best not to conflate
> software and hardware support. And that the approach you have taken
> here makes sense.
> 
> I do think it would be nice to add a bit more detail to the commit message,
> along the lines of the text above. But I'll leave that call up to you.
> 

Ok, I'll clarify what is hardware specific vs software specific.

Thank you for the review,
Charles

> Overall, this looks good to me.
> 
> Reviewed-by: Simon Horman <horms@xxxxxxxxxx>