Re: [PATCH net-next v3 1/7] bnxt_en: add support for rx-copybreak ethtool command

[Michael Chan <michael.chan@broadcom.com>]

[-- Attachment #1: Type: text/plain, Size: 1384 bytes --]

On Tue, Oct 8, 2024 at 12:53 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Tue, 8 Oct 2024 12:38:18 -0700 Michael Chan wrote:
> > > Where does the min value of 64 come from? Ethernet min frame length?
> >
> > The length is actually the ethernet length minus the 4-byte CRC.  So
> > 60 is the minimum length that the driver will see.  Anything smaller
> > coming from the wire will be a runt frame discarded by the chip.
>
> Also for VF to VF traffic?

Good point.  Loopback traffic is not subject to padding and can be
smaller than 60 bytes.  So, lower limit checking doesn't make much
sense anymore.

>
> > > IIUC the copybreak threshold is purely a SW feature, after this series.
> > > If someone sets the copybreak value to, say 13 it will simply never
> > > engage but it's not really an invalid setting, IMHO. Similarly setting
> > > it to 0 makes intuitive sense (that's how e1000e works, AFAICT).
> >
> > Right, setting it to 0 or 13 will have the same effect of disabling
> > it.  0 makes more intuitive sense.
>
> Agreed on 0 making sense, but not sure if rejecting intermediate values
> buys us anything. As Andrew mentioned consistency is important. I only
> checked two drivers (e1000e and gve) and they don't seem to check
> the lower limit.

Sure, so the range should be 0 to 1024.  Any value close to 0 will
effectively disable it.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]