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

On 2021/10/13 2:26, Jakub Kicinski wrote:

On Tue, 12 Oct 2021 21:41:24 +0800 Guangbin Huang wrote:

quoted

From: Hao Chen <redacted>

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 <redacted>
Signed-off-by: Guangbin Huang <redacted>

quoted

+  ``ETHTOOL_A_RINGS_RX_BUF_LEN``        u32     size of buffers on the ring
    ====================================  ======  ==========================

Does the documentation build without warnings?

Hi Jakub, there is no warning when we build documentation. It seems that the third
column needs more '=' symbol, we add it in next version.

quoted

diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 266e95e4fb33..83544186cbb5 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h

@@ -535,6 +535,14 @@ struct ethtool_ringparam {
  	__u32	tx_pending;
  };
  
+/**
+ * struct ethtool_ringparam_ext - RX/TX ring configuration
+ * @rx_buf_len: Current length of buffers on the rx ring.
+ */
+struct ethtool_ringparam_ext {
+	__u32	rx_buf_len;
+};

This can be moved to include/linux/ethtool.h, user space does not need
to know about this structure.

Ok.

quoted

+	if (ringparam_ext.rx_buf_len != 0 &&
+	    !(ops->supported_ring_params & ETHTOOL_RING_USE_RX_BUF_LEN)) {
+		ret = -EOPNOTSUPP;
+		NL_SET_ERR_MSG_ATTR(info->extack,
+				    tb[ETHTOOL_A_RINGS_RX_BUF_LEN],
+				    "setting not supported rx buf len");

"setting rx buf len not supported" sounds better

Ok.

quoted

+		goto out_ops;
+	}
+
  	ret = dev->ethtool_ops->set_ringparam(dev, &ringparam);
  	if (ret < 0)
  		goto out_ops;

.