On Tue, Oct 8, 2024 at 11:11 AM Jakub Kicinski <kuba@xxxxxxxxxx> wrote: > > On Sat, 5 Oct 2024 15:29:54 +0900 Taehee Yoo wrote: > > > > I think a single value of 0 that means disable RX copybreak is more > > > > clear and intuitive. Also, I think we can allow 64 to be a valid > > > > value. > > > > > > > > So, 0 means to disable. 1 to 63 are -EINVAL and 64 to 1024 are valid. Thanks. > > > > > > Please spend a little time and see what other drivers do. Ideally we > > > want one consistent behaviour for all drivers that allow copybreak to > > > be disabled. > > > > There is no specific disable value in other drivers. > > But some other drivers have min/max rx-copybreak value. > > If rx-copybreak is low enough, it will not be worked. > > So, min value has been working as a disable value actually. > > > > I think Andrew's point makes sense. > > So I would like to change min value from 65 to 64, not add a disable value. > > 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. > 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.
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature