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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Oct 3, 2024 at 10:23 AM Taehee Yoo <ap420073@xxxxxxxxx> wrote:
>
> On Fri, Oct 4, 2024 at 2:14 AM Michael Chan <michael.chan@xxxxxxxxxxxx> wrote:
> >
>
> Hi Michael,
> Thanks a lot for the review!
>
> > On Thu, Oct 3, 2024 at 9:06 AM Taehee Yoo <ap420073@xxxxxxxxx> wrote:
> > >
> > > The bnxt_en driver supports rx-copybreak, but it couldn't be set by
> > > userspace. Only the default value(256) has worked.
> > > This patch makes the bnxt_en driver support following command.
> > > `ethtool --set-tunable <devname> rx-copybreak <value> ` and
> > > `ethtool --get-tunable <devname> rx-copybreak`.
> > >
> > > Signed-off-by: Taehee Yoo <ap420073@xxxxxxxxx>
> > > ---
> > >
> > > v3:
> > > - Update copybreak value before closing nic.
> > >
> > > v2:
> > > - Define max/vim rx_copybreak value.
> > >
> > > drivers/net/ethernet/broadcom/bnxt/bnxt.c | 24 +++++----
> > > drivers/net/ethernet/broadcom/bnxt/bnxt.h | 6 ++-
> > > .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 49 ++++++++++++++++++-
> > > 3 files changed, 68 insertions(+), 11 deletions(-)
> > >
> >
> > > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> > > index 69231e85140b..cff031993223 100644
> > > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> > > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> > > @@ -34,6 +34,10 @@
> > > #include <linux/firmware/broadcom/tee_bnxt_fw.h>
> > > #endif
> > >
> > > +#define BNXT_DEFAULT_RX_COPYBREAK 256
> > > +#define BNXT_MIN_RX_COPYBREAK 65
> > > +#define BNXT_MAX_RX_COPYBREAK 1024
> > > +
> >
> > Sorry for the late review. Perhaps we should also support a value of
> > zero which means to disable RX copybreak.
>
> I agree that we need to support disabling rx-copybreak.
> What about 0 ~ 64 means to disable rx-copybreak?
> Or should only 0 be allowed to disable rx-copybreak?
>

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.

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature


[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux