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

[Simon Horman <horms@kernel.org>]

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@microchip.com>
> > 
> > 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.

Overall, this looks good to me.

Reviewed-by: Simon Horman <horms@kernel.org>