On Fri, Oct 4, 2024 at 2:43 AM Michael Chan <michael.chan@xxxxxxxxxxxx> wrote: > > 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. Thanks for that, It's clear to me. I will change it as you suggested. Thanks a lot! Taehee
