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