[PATCH net-next 0/5] ethtool: add support to set/get tx spare buf size and rx buf len

[PATCH net-next 0/5] ethtool: add support to set/get tx spare buf size and rx buf len

[Guangbin Huang]

This series add support to set/get tx spare buf size and rx buf len via
ethtool and hns3 driver implements them.

Tx spare buf size is used for tx copybreak feature which for small size
packet or frag. Use ethtool --get-tunable command to get it, and ethtool
--set-tunable command to set it, examples are as follow:

1. set tx spare buf size to 102400:
$ ethtool --set-tunable eth1 tx-buf-size 102400

2. get tx spare buf size:
$ ethtool --get-tunable eth1 tx-buf-size
tx-buf-size: 102400

Rx buf len is buffer length of each rx BD. Use ethtool -g command to get
it, and ethtool -G command to set it, examples are as follow:

1. set rx buf len to 4096
$ ethtool -G eth1 rx-buf-len 4096

2. get rx buf len
$ ethtool -g eth1
...
RX Buf Len:     4096


Hao Chen (5):
  ethtool: add support to set/get tx spare buf size
  net: hns3: add support to set/get tx spare buf via ethtool for hns3
    driver
  ethtool: add support to set/get rx buf len
  net: hns3: add support to set/get rx buf len via ethtool for hns3
    driver
  net: hns3: remove the way to set tx spare buf via module parameter

 drivers/net/ethernet/hisilicon/hns3/hns3_enet.c    | 11 +--
 drivers/net/ethernet/hisilicon/hns3/hns3_enet.h    |  2 +
 drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c | 98 ++++++++++++++++++++--
 include/uapi/linux/ethtool.h                       |  3 +
 include/uapi/linux/ethtool_netlink.h               |  1 +
 net/ethtool/ioctl.c                                |  1 +
 net/ethtool/netlink.h                              |  2 +-
 net/ethtool/rings.c                                | 11 ++-
 8 files changed, 112 insertions(+), 17 deletions(-)

-- 
2.8.1

[PATCH net-next 1/5] ethtool: add support to set/get tx spare buf size

[Guangbin Huang]

From: Hao Chen <chenhao288@hisilicon.com>

Add support for ethtool to set/get tx spare buf size.

Signed-off-by: Hao Chen <chenhao288@hisilicon.com>
Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com>
---
 include/uapi/linux/ethtool.h | 1 +
 net/ethtool/ioctl.c          | 1 +
 2 files changed, 2 insertions(+)

diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index b6db6590baf0..266e95e4fb33 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -231,6 +231,7 @@ enum tunable_id {
 	ETHTOOL_RX_COPYBREAK,
 	ETHTOOL_TX_COPYBREAK,
 	ETHTOOL_PFC_PREVENTION_TOUT, /* timeout in msecs */
+	ETHTOOL_TX_COPYBREAK_BUF_SIZE,
 	/*
 	 * Add your fresh new tunable attribute above and remember to update
 	 * tunable_strings[] in net/ethtool/common.c
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index f2abc3152888..9fc801298fde 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -2377,6 +2377,7 @@ static int ethtool_tunable_valid(const struct ethtool_tunable *tuna)
 	switch (tuna->id) {
 	case ETHTOOL_RX_COPYBREAK:
 	case ETHTOOL_TX_COPYBREAK:
+	case ETHTOOL_TX_COPYBREAK_BUF_SIZE:
 		if (tuna->len != sizeof(u32) ||
 		    tuna->type_id != ETHTOOL_TUNABLE_U32)
 			return -EINVAL;
-- 
2.8.1

[PATCH net-next 2/5] net: hns3: add support to set/get tx spare buf via ethtool for hns3 driver

[Guangbin Huang]

From: Hao Chen <chenhao288@hisilicon.com>

Tx spare buf size is used for tx copybreak feature, the feature is
used for small size packet or frag. It adds a queue based tx shared
bounce buffer to memcpy the small packet when the len of xmitted skb is
below tx_copybreak(value to distinguish small size and normal size),
and reduce the overhead of dma map and unmap when IOMMU is on.

Support setting it via ethtool --set-tunable parameter and getting
it via ethtool --get-tunable parameter.

Signed-off-by: Hao Chen <chenhao288@hisilicon.com>
Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns3/hns3_enet.c    |  4 +-
 drivers/net/ethernet/hisilicon/hns3/hns3_enet.h    |  2 +
 drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c | 56 ++++++++++++++++++++++
 3 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index 39d01ca026da..c945fa4b3c9c 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -5533,8 +5533,8 @@ static int hns3_reset_notify_uninit_enet(struct hnae3_handle *handle)
 	return 0;
 }
 
-static int hns3_reset_notify(struct hnae3_handle *handle,
-			     enum hnae3_reset_notify_type type)
+int hns3_reset_notify(struct hnae3_handle *handle,
+		      enum hnae3_reset_notify_type type)
 {
 	int ret = 0;
 
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
index dfad9060c284..3287d846d7e0 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
@@ -705,6 +705,8 @@ void hns3_set_vector_coalesce_tx_ql(struct hns3_enet_tqp_vector *tqp_vector,
 				    u32 ql_value);
 
 void hns3_request_update_promisc_mode(struct hnae3_handle *handle);
+int hns3_reset_notify(struct hnae3_handle *handle,
+		      enum hnae3_reset_notify_type type);
 
 #ifdef CONFIG_HNS3_DCB
 void hns3_dcbnl_setup(struct hnae3_handle *handle);
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
index b8d9851aefc5..5a21b9eb9820 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
@@ -1664,6 +1664,7 @@ static int hns3_get_tunable(struct net_device *netdev,
 			    void *data)
 {
 	struct hns3_nic_priv *priv = netdev_priv(netdev);
+	struct hnae3_handle *h = priv->ae_handle;
 	int ret = 0;
 
 	switch (tuna->id) {
@@ -1674,6 +1675,9 @@ static int hns3_get_tunable(struct net_device *netdev,
 	case ETHTOOL_RX_COPYBREAK:
 		*(u32 *)data = priv->rx_copybreak;
 		break;
+	case ETHTOOL_TX_COPYBREAK_BUF_SIZE:
+		*(u32 *)data = h->kinfo.tx_spare_buf_size;
+		break;
 	default:
 		ret = -EOPNOTSUPP;
 		break;
@@ -1682,12 +1686,44 @@ static int hns3_get_tunable(struct net_device *netdev,
 	return ret;
 }
 
+static int hns3_set_tx_spare_buf_size(struct net_device *netdev,
+				      u32 data)
+{
+	struct hns3_nic_priv *priv = netdev_priv(netdev);
+	struct hnae3_handle *h = priv->ae_handle;
+	int ret;
+
+	if (test_bit(HNS3_NIC_STATE_RESETTING, &priv->state))
+		return -EBUSY;
+
+	h->kinfo.tx_spare_buf_size = data;
+
+	ret = hns3_reset_notify(h, HNAE3_DOWN_CLIENT);
+	if (ret)
+		return ret;
+
+	ret = hns3_reset_notify(h, HNAE3_UNINIT_CLIENT);
+	if (ret)
+		return ret;
+
+	ret = hns3_reset_notify(h, HNAE3_INIT_CLIENT);
+	if (ret)
+		return ret;
+
+	ret = hns3_reset_notify(h, HNAE3_UP_CLIENT);
+	if (ret)
+		hns3_reset_notify(h, HNAE3_UNINIT_CLIENT);
+
+	return ret;
+}
+
 static int hns3_set_tunable(struct net_device *netdev,
 			    const struct ethtool_tunable *tuna,
 			    const void *data)
 {
 	struct hns3_nic_priv *priv = netdev_priv(netdev);
 	struct hnae3_handle *h = priv->ae_handle;
+	u32 old_tx_spare_buf_size, new_tx_spare_buf_size;
 	int i, ret = 0;
 
 	switch (tuna->id) {
@@ -1705,6 +1741,26 @@ static int hns3_set_tunable(struct net_device *netdev,
 			priv->ring[i].rx_copybreak = priv->rx_copybreak;
 
 		break;
+	case ETHTOOL_TX_COPYBREAK_BUF_SIZE:
+		old_tx_spare_buf_size = h->kinfo.tx_spare_buf_size;
+		new_tx_spare_buf_size = *(u32 *)data;
+		ret = hns3_set_tx_spare_buf_size(netdev, new_tx_spare_buf_size);
+		if (ret) {
+			int ret1;
+
+			netdev_warn(netdev,
+				    "change tx spare buf size fail, revert to old value\n");
+			ret1 = hns3_set_tx_spare_buf_size(netdev,
+							  old_tx_spare_buf_size);
+			if (ret1) {
+				netdev_err(netdev,
+					   "revert to old tx spare buf size fail\n");
+				return ret1;
+			}
+
+		return ret;
+		}
+		break;
 	default:
 		ret = -EOPNOTSUPP;
 		break;
-- 
2.8.1

[PATCH net-next 3/5] ethtool: add support to set/get rx buf len

[Guangbin Huang]

From: Hao Chen <chenhao288@hisilicon.com>

Add support to set rx buf len via ethtool -G parameter and get
rx buf len via ethtool -g parameter.

Signed-off-by: Hao Chen <chenhao288@hisilicon.com>
Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com>
---
 include/uapi/linux/ethtool.h         |  2 ++
 include/uapi/linux/ethtool_netlink.h |  1 +
 net/ethtool/netlink.h                |  2 +-
 net/ethtool/rings.c                  | 11 +++++++++--
 4 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 266e95e4fb33..6e26586274b3 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -516,6 +516,7 @@ struct ethtool_coalesce {
  *	jumbo ring
  * @tx_pending: Current maximum supported number of pending entries
  *	per TX ring
+ * @rx_buf_len: Current supported size of rx ring BD buffer.
  *
  * If the interface does not have separate RX mini and/or jumbo rings,
  * @rx_mini_max_pending and/or @rx_jumbo_max_pending will be 0.
@@ -533,6 +534,7 @@ struct ethtool_ringparam {
 	__u32	rx_mini_pending;
 	__u32	rx_jumbo_pending;
 	__u32	tx_pending;
+	__u32	rx_buf_len;
 };
 
 /**
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 5545f1ca9237..3883fa4168e9 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -325,6 +325,7 @@ enum {
 	ETHTOOL_A_RINGS_RX_MINI,			/* u32 */
 	ETHTOOL_A_RINGS_RX_JUMBO,			/* u32 */
 	ETHTOOL_A_RINGS_TX,				/* u32 */
+	ETHTOOL_A_RINGS_RX_BUF_LEN,                     /* u32 */
 
 	/* add new constants above here */
 	__ETHTOOL_A_RINGS_CNT,
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index e8987e28036f..3183f1fc6990 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -355,7 +355,7 @@ extern const struct nla_policy ethnl_features_set_policy[ETHTOOL_A_FEATURES_WANT
 extern const struct nla_policy ethnl_privflags_get_policy[ETHTOOL_A_PRIVFLAGS_HEADER + 1];
 extern const struct nla_policy ethnl_privflags_set_policy[ETHTOOL_A_PRIVFLAGS_FLAGS + 1];
 extern const struct nla_policy ethnl_rings_get_policy[ETHTOOL_A_RINGS_HEADER + 1];
-extern const struct nla_policy ethnl_rings_set_policy[ETHTOOL_A_RINGS_TX + 1];
+extern const struct nla_policy ethnl_rings_set_policy[ETHTOOL_A_RINGS_RX_BUF_LEN + 1];
 extern const struct nla_policy ethnl_channels_get_policy[ETHTOOL_A_CHANNELS_HEADER + 1];
 extern const struct nla_policy ethnl_channels_set_policy[ETHTOOL_A_CHANNELS_COMBINED_COUNT + 1];
 extern const struct nla_policy ethnl_coalesce_get_policy[ETHTOOL_A_COALESCE_HEADER + 1];
diff --git a/net/ethtool/rings.c b/net/ethtool/rings.c
index 4e097812a967..8847b1daf477 100644
--- a/net/ethtool/rings.c
+++ b/net/ethtool/rings.c
@@ -49,7 +49,8 @@ static int rings_reply_size(const struct ethnl_req_info *req_base,
 	       nla_total_size(sizeof(u32)) +	/* _RINGS_RX */
 	       nla_total_size(sizeof(u32)) +	/* _RINGS_RX_MINI */
 	       nla_total_size(sizeof(u32)) +	/* _RINGS_RX_JUMBO */
-	       nla_total_size(sizeof(u32));	/* _RINGS_TX */
+	       nla_total_size(sizeof(u32)) +	/* _RINGS_TX */
+	       nla_total_size(sizeof(u32));     /* _RINGS_RX_BUF_LEN */
 }
 
 static int rings_fill_reply(struct sk_buff *skb,
@@ -78,7 +79,10 @@ static int rings_fill_reply(struct sk_buff *skb,
 	     (nla_put_u32(skb, ETHTOOL_A_RINGS_TX_MAX,
 			  ringparam->tx_max_pending) ||
 	      nla_put_u32(skb, ETHTOOL_A_RINGS_TX,
-			  ringparam->tx_pending))))
+			  ringparam->tx_pending)))  ||
+	    (ringparam->rx_buf_len &&
+	     (nla_put_u32(skb, ETHTOOL_A_RINGS_RX_BUF_LEN,
+			  ringparam->rx_buf_len))))
 		return -EMSGSIZE;
 
 	return 0;
@@ -105,6 +109,7 @@ const struct nla_policy ethnl_rings_set_policy[] = {
 	[ETHTOOL_A_RINGS_RX_MINI]		= { .type = NLA_U32 },
 	[ETHTOOL_A_RINGS_RX_JUMBO]		= { .type = NLA_U32 },
 	[ETHTOOL_A_RINGS_TX]			= { .type = NLA_U32 },
+	[ETHTOOL_A_RINGS_RX_BUF_LEN]            = { .type = NLA_U32 },
 };
 
 int ethnl_set_rings(struct sk_buff *skb, struct genl_info *info)
@@ -142,6 +147,8 @@ int ethnl_set_rings(struct sk_buff *skb, struct genl_info *info)
 	ethnl_update_u32(&ringparam.rx_jumbo_pending,
 			 tb[ETHTOOL_A_RINGS_RX_JUMBO], &mod);
 	ethnl_update_u32(&ringparam.tx_pending, tb[ETHTOOL_A_RINGS_TX], &mod);
+	ethnl_update_u32(&ringparam.rx_buf_len,
+			 tb[ETHTOOL_A_RINGS_RX_BUF_LEN], &mod);
 	ret = 0;
 	if (!mod)
 		goto out_ops;
-- 
2.8.1

[PATCH net-next 4/5] net: hns3: add support to set/get rx buf len via ethtool for hns3 driver

[Guangbin Huang]

From: Hao Chen <chenhao288@hisilicon.com>

Rx buf len is for rx BD buffer size, support setting it via ethtool -G
parameter and getting it via ethtool -g parameter.

Signed-off-by: Hao Chen <chenhao288@hisilicon.com>
Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c | 42 ++++++++++++++++++----
 1 file changed, 36 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
index 5a21b9eb9820..c78b3a377197 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
@@ -616,7 +616,7 @@ static void hns3_get_ringparam(struct net_device *netdev,
 {
 	struct hns3_nic_priv *priv = netdev_priv(netdev);
 	struct hnae3_handle *h = priv->ae_handle;
-	int queue_num = h->kinfo.num_tqps;
+	int rx_queue_index = h->kinfo.num_tqps;
 
 	if (hns3_nic_resetting(netdev)) {
 		netdev_err(netdev, "dev resetting!");
@@ -627,7 +627,8 @@ static void hns3_get_ringparam(struct net_device *netdev,
 	param->rx_max_pending = HNS3_RING_MAX_PENDING;
 
 	param->tx_pending = priv->ring[0].desc_num;
-	param->rx_pending = priv->ring[queue_num].desc_num;
+	param->rx_pending = priv->ring[rx_queue_index].desc_num;
+	param->rx_buf_len = priv->ring[rx_queue_index].buf_size;
 }
 
 static void hns3_get_pauseparam(struct net_device *netdev,
@@ -1031,12 +1032,20 @@ static struct hns3_enet_ring *hns3_backup_ringparam(struct hns3_nic_priv *priv)
 static int hns3_check_ringparam(struct net_device *ndev,
 				struct ethtool_ringparam *param)
 {
+#define RX_BUF_LEN_2K 2048
+#define RX_BUF_LEN_4K 4096
 	if (hns3_nic_resetting(ndev))
 		return -EBUSY;
 
 	if (param->rx_mini_pending || param->rx_jumbo_pending)
 		return -EINVAL;
 
+	if (param->rx_buf_len != RX_BUF_LEN_2K &&
+	    param->rx_buf_len != RX_BUF_LEN_4K) {
+		netdev_err(ndev, "Rx buf len only support 2048 and 4096\n");
+		return -EINVAL;
+	}
+
 	if (param->tx_pending > HNS3_RING_MAX_PENDING ||
 	    param->tx_pending < HNS3_RING_MIN_PENDING ||
 	    param->rx_pending > HNS3_RING_MAX_PENDING ||
@@ -1049,6 +1058,22 @@ static int hns3_check_ringparam(struct net_device *ndev,
 	return 0;
 }
 
+static int hns3_change_rx_buf_len(struct net_device *ndev, u32 rx_buf_len)
+{
+	struct hns3_nic_priv *priv = netdev_priv(ndev);
+	struct hnae3_handle *h = priv->ae_handle;
+	int i;
+
+	h->kinfo.rx_buf_len = rx_buf_len;
+
+	for (i = 0; i < h->kinfo.num_tqps; i++) {
+		h->kinfo.tqp[i]->buf_size = rx_buf_len;
+		priv->ring[i + h->kinfo.num_tqps].buf_size = rx_buf_len;
+	}
+
+	return 0;
+}
+
 static int hns3_set_ringparam(struct net_device *ndev,
 			      struct ethtool_ringparam *param)
 {
@@ -1059,6 +1084,7 @@ static int hns3_set_ringparam(struct net_device *ndev,
 	u32 old_tx_desc_num, new_tx_desc_num;
 	u32 old_rx_desc_num, new_rx_desc_num;
 	u16 queue_num = h->kinfo.num_tqps;
+	u32 old_rx_buf_len;
 	int ret, i;
 
 	ret = hns3_check_ringparam(ndev, param);
@@ -1070,8 +1096,10 @@ static int hns3_set_ringparam(struct net_device *ndev,
 	new_rx_desc_num = ALIGN(param->rx_pending, HNS3_RING_BD_MULTIPLE);
 	old_tx_desc_num = priv->ring[0].desc_num;
 	old_rx_desc_num = priv->ring[queue_num].desc_num;
+	old_rx_buf_len = priv->ring[queue_num].buf_size;
 	if (old_tx_desc_num == new_tx_desc_num &&
-	    old_rx_desc_num == new_rx_desc_num)
+	    old_rx_desc_num == new_rx_desc_num &&
+	    param->rx_buf_len == old_rx_buf_len)
 		return 0;
 
 	tmp_rings = hns3_backup_ringparam(priv);
@@ -1082,19 +1110,21 @@ static int hns3_set_ringparam(struct net_device *ndev,
 	}
 
 	netdev_info(ndev,
-		    "Changing Tx/Rx ring depth from %u/%u to %u/%u\n",
+		    "Changing Tx/Rx ring depth from %u/%u to %u/%u, Changing rx buffer len to %d\n",
 		    old_tx_desc_num, old_rx_desc_num,
-		    new_tx_desc_num, new_rx_desc_num);
+		    new_tx_desc_num, new_rx_desc_num, param->rx_buf_len);
 
 	if (if_running)
 		ndev->netdev_ops->ndo_stop(ndev);
 
 	hns3_change_all_ring_bd_num(priv, new_tx_desc_num, new_rx_desc_num);
+	hns3_change_rx_buf_len(ndev, param->rx_buf_len);
 	ret = hns3_init_all_ring(priv);
 	if (ret) {
-		netdev_err(ndev, "Change bd num fail, revert to old value(%d)\n",
+		netdev_err(ndev, "set ringparam fail, revert to old value(%d)\n",
 			   ret);
 
+		hns3_change_rx_buf_len(ndev, old_rx_buf_len);
 		hns3_change_all_ring_bd_num(priv, old_tx_desc_num,
 					    old_rx_desc_num);
 		for (i = 0; i < h->kinfo.num_tqps * 2; i++)
-- 
2.8.1

[PATCH net-next 5/5] net: hns3: remove the way to set tx spare buf via module parameter

[Guangbin Huang]

From: Hao Chen <chenhao288@hisilicon.com>

The way to set tx spare buf via module parameter is not such
convenient as the way to set it via ethtool.

So,remove the way to set tx spare buf via module parameter.

Signed-off-by: Hao Chen <chenhao288@hisilicon.com>
Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index c945fa4b3c9c..92d7e18f008f 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -53,10 +53,6 @@ static int debug = -1;
 module_param(debug, int, 0);
 MODULE_PARM_DESC(debug, " Network interface message level setting");
 
-static unsigned int tx_spare_buf_size;
-module_param(tx_spare_buf_size, uint, 0400);
-MODULE_PARM_DESC(tx_spare_buf_size, "Size used to allocate tx spare buffer");
-
 static unsigned int tx_sgl = 1;
 module_param(tx_sgl, uint, 0600);
 MODULE_PARM_DESC(tx_sgl, "Minimum number of frags when using dma_map_sg() to optimize the IOMMU mapping");
@@ -1037,8 +1033,7 @@ static void hns3_init_tx_spare_buffer(struct hns3_enet_ring *ring)
 	dma_addr_t dma;
 	int order;
 
-	alloc_size = tx_spare_buf_size ? tx_spare_buf_size :
-		     ring->tqp->handle->kinfo.tx_spare_buf_size;
+	alloc_size = ring->tqp->handle->kinfo.tx_spare_buf_size;
 	if (!alloc_size)
 		return;
 
-- 
2.8.1

Re: [PATCH net-next 1/5] ethtool: add support to set/get tx spare buf size

[Jakub Kicinski]

On Wed, 25 Aug 2021 14:40:51 +0800 Guangbin Huang wrote:
> From: Hao Chen <chenhao288@hisilicon.com>
> 
> Add support for ethtool to set/get tx spare buf size.
> 
> Signed-off-by: Hao Chen <chenhao288@hisilicon.com>
> Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com>
> ---
>  include/uapi/linux/ethtool.h | 1 +
>  net/ethtool/ioctl.c          | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index b6db6590baf0..266e95e4fb33 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -231,6 +231,7 @@ enum tunable_id {
>  	ETHTOOL_RX_COPYBREAK,
>  	ETHTOOL_TX_COPYBREAK,
>  	ETHTOOL_PFC_PREVENTION_TOUT, /* timeout in msecs */
> +	ETHTOOL_TX_COPYBREAK_BUF_SIZE,

We need good documentation for the new tunable.

>  	/*
>  	 * Add your fresh new tunable attribute above and remember to update
>  	 * tunable_strings[] in net/ethtool/common.c
> diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
> index f2abc3152888..9fc801298fde 100644
> --- a/net/ethtool/ioctl.c
> +++ b/net/ethtool/ioctl.c
> @@ -2377,6 +2377,7 @@ static int ethtool_tunable_valid(const struct ethtool_tunable *tuna)
>  	switch (tuna->id) {
>  	case ETHTOOL_RX_COPYBREAK:
>  	case ETHTOOL_TX_COPYBREAK:
> +	case ETHTOOL_TX_COPYBREAK_BUF_SIZE:
>  		if (tuna->len != sizeof(u32) ||
>  		    tuna->type_id != ETHTOOL_TUNABLE_U32)
>  			return -EINVAL;

Re: [PATCH net-next 3/5] ethtool: add support to set/get rx buf len

[Jakub Kicinski]

On Wed, 25 Aug 2021 14:40:53 +0800 Guangbin Huang wrote:
> From: Hao Chen <chenhao288@hisilicon.com>
> 
> Add support to set rx buf len via ethtool -G parameter and get
> rx buf len via ethtool -g parameter.
> 
> Signed-off-by: Hao Chen <chenhao288@hisilicon.com>
> Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com>

> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index 266e95e4fb33..6e26586274b3 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -516,6 +516,7 @@ struct ethtool_coalesce {
>   *	jumbo ring
>   * @tx_pending: Current maximum supported number of pending entries
>   *	per TX ring
> + * @rx_buf_len: Current supported size of rx ring BD buffer.

How about "Current length of buffers on the rx ring"?

>   * If the interface does not have separate RX mini and/or jumbo rings,
>   * @rx_mini_max_pending and/or @rx_jumbo_max_pending will be 0.
> @@ -533,6 +534,7 @@ struct ethtool_ringparam {
>  	__u32	rx_mini_pending;
>  	__u32	rx_jumbo_pending;
>  	__u32	tx_pending;
> +	__u32	rx_buf_len;
>  };

You can't extend this structure, because it's used by the IOCTL
interface directly. You need to pass the new value to the drivers 
in a different way. You can look at what Yufeng Mo did recently
for an example approach.

> @@ -105,6 +109,7 @@ const struct nla_policy ethnl_rings_set_policy[] = {
>  	[ETHTOOL_A_RINGS_RX_MINI]		= { .type = NLA_U32 },
>  	[ETHTOOL_A_RINGS_RX_JUMBO]		= { .type = NLA_U32 },
>  	[ETHTOOL_A_RINGS_TX]			= { .type = NLA_U32 },
> +	[ETHTOOL_A_RINGS_RX_BUF_LEN]            = { .type = NLA_U32 },

We should prevent user space for passing 0 as input, so we can use it
as a special value in the kernel. NLA_POLICY_MIN()

>  };
>  
>  int ethnl_set_rings(struct sk_buff *skb, struct genl_info *info)
> @@ -142,6 +147,8 @@ int ethnl_set_rings(struct sk_buff *skb, struct genl_info *info)
>  	ethnl_update_u32(&ringparam.rx_jumbo_pending,
>  			 tb[ETHTOOL_A_RINGS_RX_JUMBO], &mod);
>  	ethnl_update_u32(&ringparam.tx_pending, tb[ETHTOOL_A_RINGS_TX], &mod);
> +	ethnl_update_u32(&ringparam.rx_buf_len,
> +			 tb[ETHTOOL_A_RINGS_RX_BUF_LEN], &mod);
>  	ret = 0;
>  	if (!mod)
>  		goto out_ops;

We need a way of preventing drivers which don't support the new option
from just silently ignoring it. Please add a capability like
cap_link_lanes_supported and reject non-0 ETHTOOL_A_RINGS_RX_BUF_LEN
if it's not set.

Re: [PATCH net-next 1/5] ethtool: add support to set/get tx spare buf size

[huangguangbin (A)]

On 2021/8/25 22:56, Jakub Kicinski wrote:
> On Wed, 25 Aug 2021 14:40:51 +0800 Guangbin Huang wrote:
>> From: Hao Chen <chenhao288@hisilicon.com>
>>
>> Add support for ethtool to set/get tx spare buf size.
>>
>> Signed-off-by: Hao Chen <chenhao288@hisilicon.com>
>> Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com>
>> ---
>>   include/uapi/linux/ethtool.h | 1 +
>>   net/ethtool/ioctl.c          | 1 +
>>   2 files changed, 2 insertions(+)
>>
>> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
>> index b6db6590baf0..266e95e4fb33 100644
>> --- a/include/uapi/linux/ethtool.h
>> +++ b/include/uapi/linux/ethtool.h
>> @@ -231,6 +231,7 @@ enum tunable_id {
>>   	ETHTOOL_RX_COPYBREAK,
>>   	ETHTOOL_TX_COPYBREAK,
>>   	ETHTOOL_PFC_PREVENTION_TOUT, /* timeout in msecs */
>> +	ETHTOOL_TX_COPYBREAK_BUF_SIZE,
> 
> We need good documentation for the new tunable.
Ok.

> 
>>   	/*
>>   	 * Add your fresh new tunable attribute above and remember to update
>>   	 * tunable_strings[] in net/ethtool/common.c
>> diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
>> index f2abc3152888..9fc801298fde 100644
>> --- a/net/ethtool/ioctl.c
>> +++ b/net/ethtool/ioctl.c
>> @@ -2377,6 +2377,7 @@ static int ethtool_tunable_valid(const struct ethtool_tunable *tuna)
>>   	switch (tuna->id) {
>>   	case ETHTOOL_RX_COPYBREAK:
>>   	case ETHTOOL_TX_COPYBREAK:
>> +	case ETHTOOL_TX_COPYBREAK_BUF_SIZE:
>>   		if (tuna->len != sizeof(u32) ||
>>   		    tuna->type_id != ETHTOOL_TUNABLE_U32)
>>   			return -EINVAL;
> 
> .
> 

Re: [PATCH net-next 3/5] ethtool: add support to set/get rx buf len

[huangguangbin (A)]

On 2021/8/25 23:09, Jakub Kicinski wrote:
> On Wed, 25 Aug 2021 14:40:53 +0800 Guangbin Huang wrote:
>> From: Hao Chen <chenhao288@hisilicon.com>
>>
>> Add support to set rx buf len via ethtool -G parameter and get
>> rx buf len via ethtool -g parameter.
>>
>> Signed-off-by: Hao Chen <chenhao288@hisilicon.com>
>> Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com>
> 
>> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
>> index 266e95e4fb33..6e26586274b3 100644
>> --- a/include/uapi/linux/ethtool.h
>> +++ b/include/uapi/linux/ethtool.h
>> @@ -516,6 +516,7 @@ struct ethtool_coalesce {
>>    *	jumbo ring
>>    * @tx_pending: Current maximum supported number of pending entries
>>    *	per TX ring
>> + * @rx_buf_len: Current supported size of rx ring BD buffer.
> 
> How about "Current length of buffers on the rx ring"?
> 
Yes, thanks.

>>    * If the interface does not have separate RX mini and/or jumbo rings,
>>    * @rx_mini_max_pending and/or @rx_jumbo_max_pending will be 0.
>> @@ -533,6 +534,7 @@ struct ethtool_ringparam {
>>   	__u32	rx_mini_pending;
>>   	__u32	rx_jumbo_pending;
>>   	__u32	tx_pending;
>> +	__u32	rx_buf_len;
>>   };
> 
> You can't extend this structure, because it's used by the IOCTL
> interface directly. You need to pass the new value to the drivers
> in a different way. You can look at what Yufeng Mo did recently
> for an example approach.
> 
Ok, thanks your advice, we will modify this patch.

>> @@ -105,6 +109,7 @@ const struct nla_policy ethnl_rings_set_policy[] = {
>>   	[ETHTOOL_A_RINGS_RX_MINI]		= { .type = NLA_U32 },
>>   	[ETHTOOL_A_RINGS_RX_JUMBO]		= { .type = NLA_U32 },
>>   	[ETHTOOL_A_RINGS_TX]			= { .type = NLA_U32 },
>> +	[ETHTOOL_A_RINGS_RX_BUF_LEN]            = { .type = NLA_U32 },
> 
> We should prevent user space for passing 0 as input, so we can use it
> as a special value in the kernel. NLA_POLICY_MIN()
> 
Ok.

>>   };
>>   
>>   int ethnl_set_rings(struct sk_buff *skb, struct genl_info *info)
>> @@ -142,6 +147,8 @@ int ethnl_set_rings(struct sk_buff *skb, struct genl_info *info)
>>   	ethnl_update_u32(&ringparam.rx_jumbo_pending,
>>   			 tb[ETHTOOL_A_RINGS_RX_JUMBO], &mod);
>>   	ethnl_update_u32(&ringparam.tx_pending, tb[ETHTOOL_A_RINGS_TX], &mod);
>> +	ethnl_update_u32(&ringparam.rx_buf_len,
>> +			 tb[ETHTOOL_A_RINGS_RX_BUF_LEN], &mod);
>>   	ret = 0;
>>   	if (!mod)
>>   		goto out_ops;
> 
> We need a way of preventing drivers which don't support the new option
> from just silently ignoring it. Please add a capability like
> cap_link_lanes_supported and reject non-0 ETHTOOL_A_RINGS_RX_BUF_LEN
> if it's not set.
> .
Ok.

> 

Re: [PATCH net-next 1/5] ethtool: add support to set/get tx spare buf size

[Michal Kubecek]

[-- Attachment #1: Type: text/plain, Size: 1951 bytes --]

On Wed, Aug 25, 2021 at 02:40:51PM +0800, Guangbin Huang wrote:
> From: Hao Chen <chenhao288@hisilicon.com>
> 
> Add support for ethtool to set/get tx spare buf size.
> 
> Signed-off-by: Hao Chen <chenhao288@hisilicon.com>
> Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com>
> ---
>  include/uapi/linux/ethtool.h | 1 +
>  net/ethtool/ioctl.c          | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index b6db6590baf0..266e95e4fb33 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -231,6 +231,7 @@ enum tunable_id {
>  	ETHTOOL_RX_COPYBREAK,
>  	ETHTOOL_TX_COPYBREAK,
>  	ETHTOOL_PFC_PREVENTION_TOUT, /* timeout in msecs */
> +	ETHTOOL_TX_COPYBREAK_BUF_SIZE,
>  	/*
>  	 * Add your fresh new tunable attribute above and remember to update
>  	 * tunable_strings[] in net/ethtool/common.c
> diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
> index f2abc3152888..9fc801298fde 100644
> --- a/net/ethtool/ioctl.c
> +++ b/net/ethtool/ioctl.c
> @@ -2377,6 +2377,7 @@ static int ethtool_tunable_valid(const struct ethtool_tunable *tuna)
>  	switch (tuna->id) {
>  	case ETHTOOL_RX_COPYBREAK:
>  	case ETHTOOL_TX_COPYBREAK:
> +	case ETHTOOL_TX_COPYBREAK_BUF_SIZE:
>  		if (tuna->len != sizeof(u32) ||
>  		    tuna->type_id != ETHTOOL_TUNABLE_U32)
>  			return -EINVAL;
> -- 
> 2.8.1
> 

IMHO this illustrates quite well what I had in mind some time ago when
I expressed my doubts if the concept of tunables in this form still
makes sense as the main benefit - workaround for lack of extensibility
of the ioctl interface - is gone. With this patch, 3 out of 4 tunables
are related to copybreak and it would IMHO make sense to group them
together as attributes of a new message and ethtool subcommand.
Configuration of header split could also belong there when/if
implemented.

Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]