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

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

 



On Tue, Oct 22, 2024 at 9:24 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`.
>
> Reviewed-by: Brett Creeley <brett.creeley@xxxxxxx>
> Tested-by: Stanislav Fomichev <sdf@xxxxxxxxxxx>
> Signed-off-by: Taehee Yoo <ap420073@xxxxxxxxx>
> ---
>
> v4:
>  - Remove min rx-copybreak value.
>  - Add Review tag from Brett.
>  - Add Test tag from Stanislav.
>
> v3:
>  - Update copybreak value after closing nic and before opening nic when
>    the device is running.
>
> v2:
>  - Define max/vim rx_copybreak value.
>
>  drivers/net/ethernet/broadcom/bnxt/bnxt.c     | 23 +++++----
>  drivers/net/ethernet/broadcom/bnxt/bnxt.h     |  5 +-
>  .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 48 ++++++++++++++++++-
>  3 files changed, 65 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index bda3742d4e32..0f5fe9ba691d 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c

> @@ -4510,7 +4513,8 @@ void bnxt_set_ring_params(struct bnxt *bp)
>                                   ALIGN(max(NET_SKB_PAD, XDP_PACKET_HEADROOM), 8) -
>                                   SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
>                 } else {
> -                       rx_size = SKB_DATA_ALIGN(BNXT_RX_COPY_THRESH + NET_IP_ALIGN);
> +                       rx_size = SKB_DATA_ALIGN(bp->rx_copybreak +
> +                                                NET_IP_ALIGN);

When rx_copybreak is 0 or very small, rx_size will be very small and
will be a problem.  We need rx_size to be big enough to contain the
packet header, so rx_size cannot be below some minimum (256?).

>                         rx_space = rx_size + NET_SKB_PAD +
>                                 SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
>                 }
> @@ -6424,8 +6428,8 @@ static int bnxt_hwrm_vnic_set_hds(struct bnxt *bp, struct bnxt_vnic_info *vnic)
>                                           VNIC_PLCMODES_CFG_REQ_FLAGS_HDS_IPV6);
>                 req->enables |=
>                         cpu_to_le32(VNIC_PLCMODES_CFG_REQ_ENABLES_HDS_THRESHOLD_VALID);
> -               req->jumbo_thresh = cpu_to_le16(bp->rx_copy_thresh);
> -               req->hds_threshold = cpu_to_le16(bp->rx_copy_thresh);
> +               req->jumbo_thresh = cpu_to_le16(bp->rx_copybreak);
> +               req->hds_threshold = cpu_to_le16(bp->rx_copybreak);

Similarly, these thresholds should not go to 0 when rx_copybreak becomes small.

>         }
>         req->vnic_id = cpu_to_le32(vnic->fw_vnic_id);
>         return hwrm_req_send(bp, req);

> @@ -4769,7 +4813,7 @@ static int bnxt_run_loopback(struct bnxt *bp)
>         cpr = &rxr->bnapi->cp_ring;
>         if (bp->flags & BNXT_FLAG_CHIP_P5_PLUS)
>                 cpr = rxr->rx_cpr;
> -       pkt_size = min(bp->dev->mtu + ETH_HLEN, bp->rx_copy_thresh);
> +       pkt_size = min(bp->dev->mtu + ETH_HLEN, bp->rx_copybreak);

The loopback test will also not work if rx_copybreak is very small.  I
think we should always use 256 bytes for the loopback test packet
size.  Thanks.

>         skb = netdev_alloc_skb(bp->dev, pkt_size);
>         if (!skb)
>                 return -ENOMEM;

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