On 9/11/2024 7:55 AM, Taehee Yoo wrote:
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
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>
---
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 | 47 ++++++++++++++++++-
3 files changed, 66 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
<snip>
+static void bnxt_init_ring_params(struct bnxt *bp)
+{
+ bp->rx_copybreak = BNXT_DEFAULT_RX_COPYBREAK;
+}
+
/* bp->rx_ring_size, bp->tx_ring_size, dev->mtu, BNXT_FLAG_{G|L}RO flags must
* be set on entry.
*/
@@ -4465,7 +4470,6 @@ void bnxt_set_ring_params(struct bnxt *bp)
rx_space = rx_size + ALIGN(max(NET_SKB_PAD, XDP_PACKET_HEADROOM), 8) +
SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
- bp->rx_copy_thresh = BNXT_RX_COPY_THRESH;
ring_size = bp->rx_ring_size;
bp->rx_agg_ring_size = 0;
bp->rx_agg_nr_pages = 0;
@@ -4510,7 +4514,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);
Tiny nit, but why did you wrap NET_IP_ALIGN to the next line?
rx_space = rx_size + NET_SKB_PAD +
SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
}
@@ -6424,8 +6429,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);
}
req->vnic_id = cpu_to_le32(vnic->fw_vnic_id);
return hwrm_req_send(bp, req);
@@ -15864,6 +15869,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
bnxt_init_l2_fltr_tbl(bp);
bnxt_set_rx_skb_mode(bp, false);
bnxt_set_tpa_flags(bp);
+ bnxt_init_ring_params(bp);
bnxt_set_ring_params(bp);
bnxt_rdma_aux_device_init(bp);
rc = bnxt_set_dflt_rings(bp, true);
<snip>
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index f71cc8188b4e..201c3fcba04e 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -4319,6 +4319,49 @@ static int bnxt_get_eee(struct net_device *dev, struct ethtool_keee *edata)
return 0;
}
+static int bnxt_set_tunable(struct net_device *dev,
+ const struct ethtool_tunable *tuna,
+ const void *data)
+{
+ struct bnxt *bp = netdev_priv(dev);
+ u32 rx_copybreak;
+
+ switch (tuna->id) {
+ case ETHTOOL_RX_COPYBREAK:
+ rx_copybreak = *(u32 *)data;
+ if (rx_copybreak < BNXT_MIN_RX_COPYBREAK ||
+ rx_copybreak > BNXT_MAX_RX_COPYBREAK)
+ return -EINVAL;
+ if (rx_copybreak != bp->rx_copybreak) {
+ bp->rx_copybreak = rx_copybreak;
Should bp->rx_copybreak get set before closing the interface in the
netif_running case? Can changing this while traffic is running cause any
unexpected issues?
I wonder if this would be better/safer?
if (netif_running(dev)) {
bnxt_close_nic(bp, false, false);
bp->rx_copybreak = rx_copybreak;
bnxt_set_ring_params(bp);
bnxt_open_nic(bp, false, false);
} else {
bp->rx_copybreak = rx_copybreak;
}
Thanks,
Brett
+ if (netif_running(dev)) {
+ bnxt_close_nic(bp, false, false);
+ bnxt_set_ring_params(bp);
+ bnxt_open_nic(bp, false, false);
+ }
+ }
+ return 0;
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
<snip>
