diff mbox series

[V2,net-next,1/6] ethtool: add support to set/get tx copybreak buf size via ethtool

Message ID 20210924142959.7798-2-huangguangbin2@huawei.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series ethtool: add support to set/get tx copybreak buf size and rx buf len | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 12 maintainers not CCed: austindh.kim@gmail.com alobakin@pm.me arnd@arndb.de petr.vorel@gmail.com idosch@nvidia.com george.mccollister@gmail.com irusskikh@marvell.com corbet@lwn.net hkallweit1@gmail.com alexanderduyck@fb.com yangbo.lu@nxp.com linux-doc@vger.kernel.org
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 2043 this patch: 2043
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 51 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 2063 this patch: 2063
netdev/header_inline success Link

Commit Message

Guangbin Huang Sept. 24, 2021, 2:29 p.m. UTC 
From: Hao Chen <chenhao288@hisilicon.com>

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

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

Comments

Michal Kubecek Sept. 24, 2021, 11:05 p.m. UTC | #1 
On Fri, Sep 24, 2021 at 10:29:54PM +0800, Guangbin Huang wrote:
> From: Hao Chen <chenhao288@hisilicon.com>
> 
> Add support for ethtool to set/get tx copybreak buf size.
> 
> Signed-off-by: Hao Chen <chenhao288@hisilicon.com>
> Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com>
> ---
>  Documentation/networking/ethtool-netlink.rst | 24 ++++++++++++++++++++
>  include/uapi/linux/ethtool.h                 |  1 +
>  net/ethtool/common.c                         |  1 +
>  net/ethtool/ioctl.c                          |  1 +
>  4 files changed, 27 insertions(+)
> 
> diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
> index d9b55b7a1a4d..a47b0255aaf9 100644
> --- a/Documentation/networking/ethtool-netlink.rst
> +++ b/Documentation/networking/ethtool-netlink.rst
> @@ -1521,6 +1521,30 @@ Kernel response contents:
>    ``ETHTOOL_A_PHC_VCLOCKS_INDEX``       s32     PHC index array
>    ====================================  ======  ==========================
>  
> +TUNABLE_SET
> +===========
> +
> +Request contents:
> +
> +  =====================================  ======  ==========================
> +  ``ETHTOOL_TX_COPYBREAK_BUF_SIZE``      u32     buf size for tx copybreak
> +  =====================================  ======  ==========================
> +
> +Tx copybreak 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.
> +
> +TUNABLE_GET
> +===========
> +
> +Kernel response contents:
> +
> +  ====================================  ======  ==========================
> +  ``ETHTOOL_TX_COPYBREAK_BUF_SIZE``     u32     buf size for tx copybreak
> +  ====================================  ======  ==========================

I have to repeat my concerns expressed in

  https://lore.kernel.org/netdev/20210826072618.2lu6spapkzdcuhyv@lion.mk-sys.cz

and earlier in more details in

  https://lore.kernel.org/netdev/20200325164958.GZ31519@unicorn.suse.cz

That being said, I don't understand why this patch adds description of
two new message types to the documentation of ethtool netlink API but it
does not actually add them to the API. Instead, it adds the new tunable
to ioctl API.

Michal

> +
>  Request translation
>  ===================
>  
> 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/common.c b/net/ethtool/common.c
> index c63e0739dc6a..0c5210015911 100644
> --- a/net/ethtool/common.c
> +++ b/net/ethtool/common.c
> @@ -89,6 +89,7 @@ tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN] = {
>  	[ETHTOOL_RX_COPYBREAK]	= "rx-copybreak",
>  	[ETHTOOL_TX_COPYBREAK]	= "tx-copybreak",
>  	[ETHTOOL_PFC_PREVENTION_TOUT] = "pfc-prevention-tout",
> +	[ETHTOOL_TX_COPYBREAK_BUF_SIZE] = "tx-copybreak-buf-size",
>  };
>  
>  const char
> diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
> index 999e2a6bed13..a6600e361c34 100644
> --- a/net/ethtool/ioctl.c
> +++ b/net/ethtool/ioctl.c
> @@ -2381,6 +2381,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.33.0
>
Guangbin Huang Sept. 29, 2021, 2:19 a.m. UTC | #2 
On 2021/9/25 7:05, Michal Kubecek wrote:
> On Fri, Sep 24, 2021 at 10:29:54PM +0800, Guangbin Huang wrote:
>> From: Hao Chen <chenhao288@hisilicon.com>
>>
>> Add support for ethtool to set/get tx copybreak buf size.
>>
>> Signed-off-by: Hao Chen <chenhao288@hisilicon.com>
>> Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com>
>> ---
>>   Documentation/networking/ethtool-netlink.rst | 24 ++++++++++++++++++++
>>   include/uapi/linux/ethtool.h                 |  1 +
>>   net/ethtool/common.c                         |  1 +
>>   net/ethtool/ioctl.c                          |  1 +
>>   4 files changed, 27 insertions(+)
>>
>> diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
>> index d9b55b7a1a4d..a47b0255aaf9 100644
>> --- a/Documentation/networking/ethtool-netlink.rst
>> +++ b/Documentation/networking/ethtool-netlink.rst
>> @@ -1521,6 +1521,30 @@ Kernel response contents:
>>     ``ETHTOOL_A_PHC_VCLOCKS_INDEX``       s32     PHC index array
>>     ====================================  ======  ==========================
>>   
>> +TUNABLE_SET
>> +===========
>> +
>> +Request contents:
>> +
>> +  =====================================  ======  ==========================
>> +  ``ETHTOOL_TX_COPYBREAK_BUF_SIZE``      u32     buf size for tx copybreak
>> +  =====================================  ======  ==========================
>> +
>> +Tx copybreak 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.
>> +
>> +TUNABLE_GET
>> +===========
>> +
>> +Kernel response contents:
>> +
>> +  ====================================  ======  ==========================
>> +  ``ETHTOOL_TX_COPYBREAK_BUF_SIZE``     u32     buf size for tx copybreak
>> +  ====================================  ======  ==========================
> 
> I have to repeat my concerns expressed in
> 
>    https://lore.kernel.org/netdev/20210826072618.2lu6spapkzdcuhyv@lion.mk-sys.cz
> 
> and earlier in more details in
> 
>    https://lore.kernel.org/netdev/20200325164958.GZ31519@unicorn.suse.cz
> 
> That being said, I don't understand why this patch adds description of
> two new message types to the documentation of ethtool netlink API but it
> does not actually add them to the API. Instead, it adds the new tunable
> to ioctl API.
> 
> Michal
> 

Hi Michal, thanks for your opinion.
Is there any documentation for ioctl API? We didn't find it.
Or we add a new documentation of ioctl API for the new tunable?

Guangbin
diff mbox series

Patch

diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index d9b55b7a1a4d..a47b0255aaf9 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -1521,6 +1521,30 @@  Kernel response contents:
   ``ETHTOOL_A_PHC_VCLOCKS_INDEX``       s32     PHC index array
   ====================================  ======  ==========================
 
+TUNABLE_SET
+===========
+
+Request contents:
+
+  =====================================  ======  ==========================
+  ``ETHTOOL_TX_COPYBREAK_BUF_SIZE``      u32     buf size for tx copybreak
+  =====================================  ======  ==========================
+
+Tx copybreak 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.
+
+TUNABLE_GET
+===========
+
+Kernel response contents:
+
+  ====================================  ======  ==========================
+  ``ETHTOOL_TX_COPYBREAK_BUF_SIZE``     u32     buf size for tx copybreak
+  ====================================  ======  ==========================
+
 Request translation
 ===================
 
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/common.c b/net/ethtool/common.c
index c63e0739dc6a..0c5210015911 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -89,6 +89,7 @@  tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN] = {
 	[ETHTOOL_RX_COPYBREAK]	= "rx-copybreak",
 	[ETHTOOL_TX_COPYBREAK]	= "tx-copybreak",
 	[ETHTOOL_PFC_PREVENTION_TOUT] = "pfc-prevention-tout",
+	[ETHTOOL_TX_COPYBREAK_BUF_SIZE] = "tx-copybreak-buf-size",
 };
 
 const char
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 999e2a6bed13..a6600e361c34 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -2381,6 +2381,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;