[RFCv3 PATCH net-next 0/2] net-next: ethool: add support to get/set tx push by ethtool -G/g

[RFCv3 PATCH net-next 0/2] net-next: ethool: add support to get/set tx push by ethtool -G/g

[Jie Wang]

These two patches add tx push in ring parms and adapt the set and get APIs 
of ring params

The former discussion please see [1].
[1]:https://lore.kernel.org/netdev/20220315032108.57228-1-wangjie125@huawei.com/
[2]:https://lore.kernel.org/netdev/20220326085102.14111-1-wangjie125@huawei.com/

ChangeLog:

V2->V3
1.Add tx push documentation in Documentation/networking/ethtool-netlink.rst
2.Use u8 to store tx push in struct kernel_ethtool_ringparam
3.Add ETHTOOL_RING_USE_TX_PUSH to reject setting for unsupported driver
4.Use NLA_POLICY_MAX(NLA_U8, 1) to limit the tx push value

V1->V2
extend tx push param in ringparam, suggested by Jakub Kicinski.

Jie Wang (2):
  net-next: ethtool: extend ringparam set/get APIs for tx_push
  net-next: hn3: add tx push support in hns3 ring param process

 Documentation/networking/ethtool-netlink.rst  |  2 ++
 .../ethernet/hisilicon/hns3/hns3_ethtool.c    | 33 ++++++++++++++++++-
 include/linux/ethtool.h                       |  3 ++
 include/uapi/linux/ethtool_netlink.h          |  1 +
 net/ethtool/netlink.h                         |  2 +-
 net/ethtool/rings.c                           | 18 ++++++++--
 6 files changed, 55 insertions(+), 4 deletions(-)

-- 
2.33.0

[RFCv3 PATCH net-next 1/2] net-next: ethtool: extend ringparam set/get APIs for tx_push

[Jie Wang]

Currently tx push is a standard driver feature which controls use of a fast
path descriptor push. So this patch extends the ringparam APIs and data
structures to support set/get tx push by ethtool -G/g.

Signed-off-by: Jie Wang <wangjie125@huawei.com>
---
 Documentation/networking/ethtool-netlink.rst |  2 ++
 include/linux/ethtool.h                      |  3 +++
 include/uapi/linux/ethtool_netlink.h         |  1 +
 net/ethtool/netlink.h                        |  2 +-
 net/ethtool/rings.c                          | 18 ++++++++++++++++--
 5 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index 24d9be69065d..424159027309 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -862,6 +862,7 @@ Kernel response contents:
   ``ETHTOOL_A_RINGS_RX_BUF_LEN``        u32     size of buffers on the ring
   ``ETHTOOL_A_RINGS_TCP_DATA_SPLIT``    u8      TCP header / data split
   ``ETHTOOL_A_RINGS_CQE_SIZE``          u32     Size of TX/RX CQE
+  ``ETHTOOL_A_RINGS_TX_PUSH``           u8      flag of TX Push mode
   ====================================  ======  ===========================
 
 ``ETHTOOL_A_RINGS_TCP_DATA_SPLIT`` indicates whether the device is usable with
@@ -887,6 +888,7 @@ Request contents:
   ``ETHTOOL_A_RINGS_TX``                u32     size of TX ring
   ``ETHTOOL_A_RINGS_RX_BUF_LEN``        u32     size of buffers on the ring
   ``ETHTOOL_A_RINGS_CQE_SIZE``          u32     Size of TX/RX CQE
+  ``ETHTOOL_A_RINGS_TX_PUSH``           u8      flag of TX Push mode
   ====================================  ======  ===========================
 
 Kernel checks that requested ring sizes do not exceed limits reported by
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 4af58459a1e7..ede4f9154cd2 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -71,11 +71,13 @@ enum {
  * struct kernel_ethtool_ringparam - RX/TX ring configuration
  * @rx_buf_len: Current length of buffers on the rx ring.
  * @tcp_data_split: Scatter packet headers and data to separate buffers
+ * @tx_push: The flag of tx push mode
  * @cqe_size: Size of TX/RX completion queue event
  */
 struct kernel_ethtool_ringparam {
 	u32	rx_buf_len;
 	u8	tcp_data_split;
+	u8	tx_push;
 	u32	cqe_size;
 };
 
@@ -87,6 +89,7 @@ struct kernel_ethtool_ringparam {
 enum ethtool_supported_ring_param {
 	ETHTOOL_RING_USE_RX_BUF_LEN = BIT(0),
 	ETHTOOL_RING_USE_CQE_SIZE   = BIT(1),
+	ETHTOOL_RING_USE_TX_PUSH    = BIT(2),
 };
 
 #define __ETH_RSS_HASH_BIT(bit)	((u32)1 << (bit))
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 979850221b8d..d2fb4f7be61b 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -338,6 +338,7 @@ enum {
 	ETHTOOL_A_RINGS_RX_BUF_LEN,                     /* u32 */
 	ETHTOOL_A_RINGS_TCP_DATA_SPLIT,			/* u8 */
 	ETHTOOL_A_RINGS_CQE_SIZE,			/* u32 */
+	ETHTOOL_A_RINGS_TX_PUSH,			/* u8 */
 
 	/* add new constants above here */
 	__ETHTOOL_A_RINGS_CNT,
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 29d01662a48b..7919ddb2371c 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -363,7 +363,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_CQE_SIZE + 1];
+extern const struct nla_policy ethnl_rings_set_policy[ETHTOOL_A_RINGS_TX_PUSH + 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 9f33c9689b56..2bc2d91f2e66 100644
--- a/net/ethtool/rings.c
+++ b/net/ethtool/rings.c
@@ -55,7 +55,8 @@ static int rings_reply_size(const struct ethnl_req_info *req_base,
 	       nla_total_size(sizeof(u32)) +	/* _RINGS_TX */
 	       nla_total_size(sizeof(u32)) +	/* _RINGS_RX_BUF_LEN */
 	       nla_total_size(sizeof(u8))  +	/* _RINGS_TCP_DATA_SPLIT */
-	       nla_total_size(sizeof(u32));	/* _RINGS_CQE_SIZE */
+	       nla_total_size(sizeof(u32)  +	/* _RINGS_CQE_SIZE */
+	       nla_total_size(sizeof(u8)));	/* _RINGS_TX_PUSH */
 }
 
 static int rings_fill_reply(struct sk_buff *skb,
@@ -94,7 +95,8 @@ static int rings_fill_reply(struct sk_buff *skb,
 	     (nla_put_u8(skb, ETHTOOL_A_RINGS_TCP_DATA_SPLIT,
 			 kr->tcp_data_split))) ||
 	    (kr->cqe_size &&
-	     (nla_put_u32(skb, ETHTOOL_A_RINGS_CQE_SIZE, kr->cqe_size))))
+	     (nla_put_u32(skb, ETHTOOL_A_RINGS_CQE_SIZE, kr->cqe_size))) ||
+	    nla_put_u8(skb, ETHTOOL_A_RINGS_TX_PUSH, !!kr->tx_push))
 		return -EMSGSIZE;
 
 	return 0;
@@ -123,6 +125,7 @@ const struct nla_policy ethnl_rings_set_policy[] = {
 	[ETHTOOL_A_RINGS_TX]			= { .type = NLA_U32 },
 	[ETHTOOL_A_RINGS_RX_BUF_LEN]            = NLA_POLICY_MIN(NLA_U32, 1),
 	[ETHTOOL_A_RINGS_CQE_SIZE]		= NLA_POLICY_MIN(NLA_U32, 1),
+	[ETHTOOL_A_RINGS_TX_PUSH]		= NLA_POLICY_MAX(NLA_U8, 1),
 };
 
 int ethnl_set_rings(struct sk_buff *skb, struct genl_info *info)
@@ -165,6 +168,8 @@ int ethnl_set_rings(struct sk_buff *skb, struct genl_info *info)
 			 tb[ETHTOOL_A_RINGS_RX_BUF_LEN], &mod);
 	ethnl_update_u32(&kernel_ringparam.cqe_size,
 			 tb[ETHTOOL_A_RINGS_CQE_SIZE], &mod);
+	ethnl_update_u8(&kernel_ringparam.tx_push,
+			tb[ETHTOOL_A_RINGS_TX_PUSH], &mod);
 	ret = 0;
 	if (!mod)
 		goto out_ops;
@@ -205,6 +210,15 @@ int ethnl_set_rings(struct sk_buff *skb, struct genl_info *info)
 		goto out_ops;
 	}
 
+	if (kernel_ringparam.tx_push &&
+	    !(ops->supported_ring_params & ETHTOOL_RING_USE_TX_PUSH)) {
+		ret = -EOPNOTSUPP;
+		NL_SET_ERR_MSG_ATTR(info->extack,
+				    tb[ETHTOOL_A_RINGS_TX_PUSH],
+				    "setting tx push not supported");
+		goto out_ops;
+	}
+
 	ret = dev->ethtool_ops->set_ringparam(dev, &ringparam,
 					      &kernel_ringparam, info->extack);
 	if (ret < 0)
-- 
2.33.0

[RFCv3 PATCH net-next 2/2] net-next: hn3: add tx push support in hns3 ring param process

[Jie Wang]

This patch adds tx push param to hns3 ring param and adapts the set and get
API of ring params. So users can set it by cmd ethtool -G and get it by cmd
ethtool -g.

Signed-off-by: Jie Wang <wangjie125@huawei.com>
---
 .../ethernet/hisilicon/hns3/hns3_ethtool.c    | 33 ++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
index 6469238ae090..5bc509f90d2a 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
@@ -664,6 +664,8 @@ static void hns3_get_ringparam(struct net_device *netdev,
 	param->tx_pending = priv->ring[0].desc_num;
 	param->rx_pending = priv->ring[rx_queue_index].desc_num;
 	kernel_param->rx_buf_len = priv->ring[rx_queue_index].buf_size;
+	kernel_param->tx_push = test_bit(HNS3_NIC_STATE_TX_PUSH_ENABLE,
+					 &priv->state);
 }
 
 static void hns3_get_pauseparam(struct net_device *netdev,
@@ -1114,6 +1116,30 @@ static int hns3_change_rx_buf_len(struct net_device *ndev, u32 rx_buf_len)
 	return 0;
 }
 
+static int hns3_set_tx_push(struct net_device *netdev, u32 tx_push)
+{
+	struct hns3_nic_priv *priv = netdev_priv(netdev);
+	struct hnae3_handle *h = hns3_get_handle(netdev);
+	struct hnae3_ae_dev *ae_dev = pci_get_drvdata(h->pdev);
+	u32 old_state = test_bit(HNS3_NIC_STATE_TX_PUSH_ENABLE, &priv->state);
+
+	if (!test_bit(HNAE3_DEV_SUPPORT_TX_PUSH_B, ae_dev->caps) && tx_push)
+		return -EOPNOTSUPP;
+
+	if (tx_push == old_state)
+		return 0;
+
+	netdev_info(netdev, "Changing tx push from %s to %s\n",
+		    old_state ? "on" : "off", tx_push ? "on" : "off");
+
+	if (tx_push)
+		set_bit(HNS3_NIC_STATE_TX_PUSH_ENABLE, &priv->state);
+	else
+		clear_bit(HNS3_NIC_STATE_TX_PUSH_ENABLE, &priv->state);
+
+	return 0;
+}
+
 static int hns3_set_ringparam(struct net_device *ndev,
 			      struct ethtool_ringparam *param,
 			      struct kernel_ethtool_ringparam *kernel_param,
@@ -1133,6 +1159,10 @@ static int hns3_set_ringparam(struct net_device *ndev,
 	if (ret)
 		return ret;
 
+	ret = hns3_set_tx_push(ndev, kernel_param->tx_push);
+	if (ret)
+		return ret;
+
 	/* Hardware requires that its descriptors must be multiple of eight */
 	new_tx_desc_num = ALIGN(param->tx_pending, HNS3_RING_BD_MULTIPLE);
 	new_rx_desc_num = ALIGN(param->rx_pending, HNS3_RING_BD_MULTIPLE);
@@ -1849,7 +1879,8 @@ static int hns3_set_tunable(struct net_device *netdev,
 				 ETHTOOL_COALESCE_MAX_FRAMES |		\
 				 ETHTOOL_COALESCE_USE_CQE)
 
-#define HNS3_ETHTOOL_RING	ETHTOOL_RING_USE_RX_BUF_LEN
+#define HNS3_ETHTOOL_RING	(ETHTOOL_RING_USE_RX_BUF_LEN |		\
+				 ETHTOOL_RING_USE_TX_PUSH)
 
 static int hns3_get_ts_info(struct net_device *netdev,
 			    struct ethtool_ts_info *info)
-- 
2.33.0

Re: [RFCv3 PATCH net-next 1/2] net-next: ethtool: extend ringparam set/get APIs for tx_push

[Michal Kubecek]

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

On Tue, Mar 29, 2022 at 05:19:12PM +0800, Jie Wang wrote:
> Currently tx push is a standard driver feature which controls use of a fast
> path descriptor push. So this patch extends the ringparam APIs and data
> structures to support set/get tx push by ethtool -G/g.
> 
> Signed-off-by: Jie Wang <wangjie125@huawei.com>
> ---
[...]
> diff --git a/net/ethtool/rings.c b/net/ethtool/rings.c
> index 9f33c9689b56..2bc2d91f2e66 100644
> --- a/net/ethtool/rings.c
> +++ b/net/ethtool/rings.c
[...]
> @@ -205,6 +210,15 @@ int ethnl_set_rings(struct sk_buff *skb, struct genl_info *info)
>  		goto out_ops;
>  	}
>  
> +	if (kernel_ringparam.tx_push &&
> +	    !(ops->supported_ring_params & ETHTOOL_RING_USE_TX_PUSH)) {
> +		ret = -EOPNOTSUPP;
> +		NL_SET_ERR_MSG_ATTR(info->extack,
> +				    tb[ETHTOOL_A_RINGS_TX_PUSH],
> +				    "setting tx push not supported");
> +		goto out_ops;
> +	}
> +
>  	ret = dev->ethtool_ops->set_ringparam(dev, &ringparam,
>  					      &kernel_ringparam, info->extack);
>  	if (ret < 0)

This only disallows setting the parameter to true but allows requests
trying to set it to false. I would rather prefer

	if (tb[ETHTOOL_A_RINGS_TX_PUSH] &&
	    !(ops->supported_ring_params & ETHTOOL_RING_USE_TX_PUSH)) {
		...
	}

and putting the check before rtnl_lock() so that we do not do useless
work if the request is invalid.

But the same can be said about the two checks we already have so those
should be probably changed and moved as well.

Michal

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

Re: [RFCv3 PATCH net-next 2/2] net-next: hn3: add tx push support in hns3 ring param process

[Michal Kubecek]

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

On Tue, Mar 29, 2022 at 05:19:13PM +0800, Jie Wang wrote:
> This patch adds tx push param to hns3 ring param and adapts the set and get
> API of ring params. So users can set it by cmd ethtool -G and get it by cmd
> ethtool -g.
> 
> Signed-off-by: Jie Wang <wangjie125@huawei.com>
> ---
>  .../ethernet/hisilicon/hns3/hns3_ethtool.c    | 33 ++++++++++++++++++-
>  1 file changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
> index 6469238ae090..5bc509f90d2a 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
> @@ -664,6 +664,8 @@ static void hns3_get_ringparam(struct net_device *netdev,
>  	param->tx_pending = priv->ring[0].desc_num;
>  	param->rx_pending = priv->ring[rx_queue_index].desc_num;
>  	kernel_param->rx_buf_len = priv->ring[rx_queue_index].buf_size;
> +	kernel_param->tx_push = test_bit(HNS3_NIC_STATE_TX_PUSH_ENABLE,
> +					 &priv->state);
>  }
>  
>  static void hns3_get_pauseparam(struct net_device *netdev,
> @@ -1114,6 +1116,30 @@ static int hns3_change_rx_buf_len(struct net_device *ndev, u32 rx_buf_len)
>  	return 0;
>  }
>  
> +static int hns3_set_tx_push(struct net_device *netdev, u32 tx_push)
> +{
> +	struct hns3_nic_priv *priv = netdev_priv(netdev);
> +	struct hnae3_handle *h = hns3_get_handle(netdev);
> +	struct hnae3_ae_dev *ae_dev = pci_get_drvdata(h->pdev);
> +	u32 old_state = test_bit(HNS3_NIC_STATE_TX_PUSH_ENABLE, &priv->state);
> +
> +	if (!test_bit(HNAE3_DEV_SUPPORT_TX_PUSH_B, ae_dev->caps) && tx_push)
> +		return -EOPNOTSUPP;
> +
> +	if (tx_push == old_state)
> +		return 0;
> +
> +	netdev_info(netdev, "Changing tx push from %s to %s\n",
> +		    old_state ? "on" : "off", tx_push ? "on" : "off");

A nitpick: do we really want an unconditional log message for each
change? If someone wants to monitor them, that's what the netlink
notifications were created for.

Michal

> +
> +	if (tx_push)
> +		set_bit(HNS3_NIC_STATE_TX_PUSH_ENABLE, &priv->state);
> +	else
> +		clear_bit(HNS3_NIC_STATE_TX_PUSH_ENABLE, &priv->state);
> +
> +	return 0;
> +}
> +
>  static int hns3_set_ringparam(struct net_device *ndev,
>  			      struct ethtool_ringparam *param,
>  			      struct kernel_ethtool_ringparam *kernel_param,

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

Re: [RFCv3 PATCH net-next 1/2] net-next: ethtool: extend ringparam set/get APIs for tx_push

[Jakub Kicinski]

On Tue, 29 Mar 2022 17:19:12 +0800 Jie Wang wrote:
> diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
> index 24d9be69065d..424159027309 100644
> --- a/Documentation/networking/ethtool-netlink.rst
> +++ b/Documentation/networking/ethtool-netlink.rst
> @@ -862,6 +862,7 @@ Kernel response contents:
>    ``ETHTOOL_A_RINGS_RX_BUF_LEN``        u32     size of buffers on the ring
>    ``ETHTOOL_A_RINGS_TCP_DATA_SPLIT``    u8      TCP header / data split
>    ``ETHTOOL_A_RINGS_CQE_SIZE``          u32     Size of TX/RX CQE
> +  ``ETHTOOL_A_RINGS_TX_PUSH``           u8      flag of TX Push mode
>    ====================================  ======  ===========================
>  
>  ``ETHTOOL_A_RINGS_TCP_DATA_SPLIT`` indicates whether the device is usable with
> @@ -887,6 +888,7 @@ Request contents:
>    ``ETHTOOL_A_RINGS_TX``                u32     size of TX ring
>    ``ETHTOOL_A_RINGS_RX_BUF_LEN``        u32     size of buffers on the ring
>    ``ETHTOOL_A_RINGS_CQE_SIZE``          u32     Size of TX/RX CQE
> +  ``ETHTOOL_A_RINGS_TX_PUSH``           u8      flag of TX Push mode
>    ====================================  ======  ===========================
>  
>  Kernel checks that requested ring sizes do not exceed limits reported by

You need to also describe what it does. Do you have a user manual 
or some form of feature documentation that could be used as a starting
point. We're happy to help with the wording and grammar but you need 
to give us a description of the feature so we're not guessing.

Re: [RFCv3 PATCH net-next 1/2] net-next: ethtool: extend ringparam set/get APIs for tx_push

[wangjie (L)]

On 2022/3/30 3:09, Michal Kubecek wrote:
> On Tue, Mar 29, 2022 at 05:19:12PM +0800, Jie Wang wrote:
>> Currently tx push is a standard driver feature which controls use of a fast
>> path descriptor push. So this patch extends the ringparam APIs and data
>> structures to support set/get tx push by ethtool -G/g.
>>
>> Signed-off-by: Jie Wang <wangjie125@huawei.com>
>> ---
> [...]
>> diff --git a/net/ethtool/rings.c b/net/ethtool/rings.c
>> index 9f33c9689b56..2bc2d91f2e66 100644
>> --- a/net/ethtool/rings.c
>> +++ b/net/ethtool/rings.c
> [...]
>> @@ -205,6 +210,15 @@ int ethnl_set_rings(struct sk_buff *skb, struct genl_info *info)
>>  		goto out_ops;
>>  	}
>>
>> +	if (kernel_ringparam.tx_push &&
>> +	    !(ops->supported_ring_params & ETHTOOL_RING_USE_TX_PUSH)) {
>> +		ret = -EOPNOTSUPP;
>> +		NL_SET_ERR_MSG_ATTR(info->extack,
>> +				    tb[ETHTOOL_A_RINGS_TX_PUSH],
>> +				    "setting tx push not supported");
>> +		goto out_ops;
>> +	}
>> +
>>  	ret = dev->ethtool_ops->set_ringparam(dev, &ringparam,
>>  					      &kernel_ringparam, info->extack);
>>  	if (ret < 0)
>
> This only disallows setting the parameter to true but allows requests
> trying to set it to false. I would rather prefer
>
> 	if (tb[ETHTOOL_A_RINGS_TX_PUSH] &&
> 	    !(ops->supported_ring_params & ETHTOOL_RING_USE_TX_PUSH)) {
> 		...
> 	}
>
> and putting the check before rtnl_lock() so that we do not do useless
> work if the request is invalid.
>
> But the same can be said about the two checks we already have so those
> should be probably changed and moved as well.
>
> Michal

OK, I will move these three checks in a new patch.

>

Re: [RFCv3 PATCH net-next 1/2] net-next: ethtool: extend ringparam set/get APIs for tx_push

[wangjie (L)]

On 2022/3/30 7:20, Jakub Kicinski wrote:
> On Tue, 29 Mar 2022 17:19:12 +0800 Jie Wang wrote:
>> diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
>> index 24d9be69065d..424159027309 100644
>> --- a/Documentation/networking/ethtool-netlink.rst
>> +++ b/Documentation/networking/ethtool-netlink.rst
>> @@ -862,6 +862,7 @@ Kernel response contents:
>>    ``ETHTOOL_A_RINGS_RX_BUF_LEN``        u32     size of buffers on the ring
>>    ``ETHTOOL_A_RINGS_TCP_DATA_SPLIT``    u8      TCP header / data split
>>    ``ETHTOOL_A_RINGS_CQE_SIZE``          u32     Size of TX/RX CQE
>> +  ``ETHTOOL_A_RINGS_TX_PUSH``           u8      flag of TX Push mode
>>    ====================================  ======  ===========================
>>
>>  ``ETHTOOL_A_RINGS_TCP_DATA_SPLIT`` indicates whether the device is usable with
>> @@ -887,6 +888,7 @@ Request contents:
>>    ``ETHTOOL_A_RINGS_TX``                u32     size of TX ring
>>    ``ETHTOOL_A_RINGS_RX_BUF_LEN``        u32     size of buffers on the ring
>>    ``ETHTOOL_A_RINGS_CQE_SIZE``          u32     Size of TX/RX CQE
>> +  ``ETHTOOL_A_RINGS_TX_PUSH``           u8      flag of TX Push mode
>>    ====================================  ======  ===========================
>>
>>  Kernel checks that requested ring sizes do not exceed limits reported by
>
> You need to also describe what it does. Do you have a user manual
> or some form of feature documentation that could be used as a starting
> point. We're happy to help with the wording and grammar but you need
> to give us a description of the feature so we're not guessing.
yes, I have some feature focumentations. I will add this part in next 
version.
>
> .
>

Re: [RFCv3 PATCH net-next 2/2] net-next: hn3: add tx push support in hns3 ring param process

[wangjie (L)]

On 2022/3/30 3:44, Michal Kubecek wrote:
> On Tue, Mar 29, 2022 at 05:19:13PM +0800, Jie Wang wrote:
>> This patch adds tx push param to hns3 ring param and adapts the set and get
>> API of ring params. So users can set it by cmd ethtool -G and get it by cmd
>> ethtool -g.
>>
>> Signed-off-by: Jie Wang <wangjie125@huawei.com>
>> ---
>>  .../ethernet/hisilicon/hns3/hns3_ethtool.c    | 33 ++++++++++++++++++-
>>  1 file changed, 32 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
>> index 6469238ae090..5bc509f90d2a 100644
>> --- a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
>> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
>> @@ -664,6 +664,8 @@ static void hns3_get_ringparam(struct net_device *netdev,
>>  	param->tx_pending = priv->ring[0].desc_num;
>>  	param->rx_pending = priv->ring[rx_queue_index].desc_num;
>>  	kernel_param->rx_buf_len = priv->ring[rx_queue_index].buf_size;
>> +	kernel_param->tx_push = test_bit(HNS3_NIC_STATE_TX_PUSH_ENABLE,
>> +					 &priv->state);
>>  }
>>
>>  static void hns3_get_pauseparam(struct net_device *netdev,
>> @@ -1114,6 +1116,30 @@ static int hns3_change_rx_buf_len(struct net_device *ndev, u32 rx_buf_len)
>>  	return 0;
>>  }
>>
>> +static int hns3_set_tx_push(struct net_device *netdev, u32 tx_push)
>> +{
>> +	struct hns3_nic_priv *priv = netdev_priv(netdev);
>> +	struct hnae3_handle *h = hns3_get_handle(netdev);
>> +	struct hnae3_ae_dev *ae_dev = pci_get_drvdata(h->pdev);
>> +	u32 old_state = test_bit(HNS3_NIC_STATE_TX_PUSH_ENABLE, &priv->state);
>> +
>> +	if (!test_bit(HNAE3_DEV_SUPPORT_TX_PUSH_B, ae_dev->caps) && tx_push)
>> +		return -EOPNOTSUPP;
>> +
>> +	if (tx_push == old_state)
>> +		return 0;
>> +
>> +	netdev_info(netdev, "Changing tx push from %s to %s\n",
>> +		    old_state ? "on" : "off", tx_push ? "on" : "off");
>
> A nitpick: do we really want an unconditional log message for each
> change? If someone wants to monitor them, that's what the netlink
> notifications were created for.
>
Actually this log is no need to display for each change, I will use 
netdev_dbg instead.
> Michal
>
>> +
>> +	if (tx_push)
>> +		set_bit(HNS3_NIC_STATE_TX_PUSH_ENABLE, &priv->state);
>> +	else
>> +		clear_bit(HNS3_NIC_STATE_TX_PUSH_ENABLE, &priv->state);
>> +
>> +	return 0;
>> +}
>> +
>>  static int hns3_set_ringparam(struct net_device *ndev,
>>  			      struct ethtool_ringparam *param,
>>  			      struct kernel_ethtool_ringparam *kernel_param,