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? Thanks a lot! Taehee Yoo