diff mbox series

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

Message ID 20211014113943.16231-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 Series has a cover letter
netdev/fixes_present success Fixes tag not required for -next series
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 10 maintainers not CCed: idosch@nvidia.com arnd@arndb.de george.mccollister@gmail.com petr.vorel@gmail.com hkallweit1@gmail.com gustavoars@kernel.org yangbo.lu@nxp.com alexanderduyck@fb.com austindh.kim@gmail.com alobakin@pm.me
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 2046 this patch: 2046
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success No Fixes tag
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 21 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 2040 this patch: 2040
netdev/header_inline success No static functions without inline keyword in header files

Commit Message

Guangbin Huang Oct. 14, 2021, 11:39 a.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>
---
 include/uapi/linux/ethtool.h | 1 +
 net/ethtool/common.c         | 1 +
 net/ethtool/ioctl.c          | 1 +
 3 files changed, 3 insertions(+)

Comments

Eric Dumazet Oct. 14, 2021, 1:58 p.m. UTC | #1 
On 10/14/21 4:39 AM, Guangbin Huang wrote:
> From: Hao Chen <chenhao288@hisilicon.com>
> 
> Add support for ethtool to set/get tx copybreak buf size.

What is the unit ?

Frankly, having to size the 'buffer' based on number of slots in TX ring buffer
is not good.

What happens later when/if ethtool -G tx xxxxx' is trying
to change number of slots ?

The 'tx copybreak' should instead give a number of bytes per TX ring slot.

Eg, 128 or 256 bytes.

This is very similar to what drivers using net/core/tso.c do.

They usually use TSO_HEADER_SIZE for this.

> 
> Signed-off-by: Hao Chen <chenhao288@hisilicon.com>
> Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com>
> ---
>  include/uapi/linux/ethtool.h | 1 +
>  net/ethtool/common.c         | 1 +
>  net/ethtool/ioctl.c          | 1 +
>  3 files changed, 3 insertions(+)
> 
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index a2223b685451..7bc4b8def12c 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 bf6e8c2f9bf7..617ebc4183d9 100644
> --- a/net/ethtool/ioctl.c
> +++ b/net/ethtool/ioctl.c
> @@ -2383,6 +2383,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;
>
Guangbin Huang Oct. 19, 2021, 1:44 p.m. UTC | #2 
On 2021/10/14 21:58, Eric Dumazet wrote:
> 
> 
> On 10/14/21 4:39 AM, Guangbin Huang wrote:
>> From: Hao Chen <chenhao288@hisilicon.com>
>>
>> Add support for ethtool to set/get tx copybreak buf size.
> 
> What is the unit ?
> 
> Frankly, having to size the 'buffer' based on number of slots in TX ring buffer
> is not good.
> 
> What happens later when/if ethtool -G tx xxxxx' is trying
> to change number of slots ?
> 
> The 'tx copybreak' should instead give a number of bytes per TX ring slot.
> 
> Eg, 128 or 256 bytes.
> 
> This is very similar to what drivers using net/core/tso.c do.
> 
> They usually use TSO_HEADER_SIZE for this.
> 
Hi Eric,
The unit is bytes.

Tx copybreak buf is a queue based tx shared bounce buffer and it is allocated
based on page size, not based on slot numbers. When the len of xmitted skb is
below tx_copybreak, tx copybreak buf will share buffer of specific size(slot size)
for tx to memcpy the small packet.

So, what you mentioned seems not related to this patch.

>>
>> Signed-off-by: Hao Chen <chenhao288@hisilicon.com>
>> Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com>
>> ---
>>   include/uapi/linux/ethtool.h | 1 +
>>   net/ethtool/common.c         | 1 +
>>   net/ethtool/ioctl.c          | 1 +
>>   3 files changed, 3 insertions(+)
>>
>> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
>> index a2223b685451..7bc4b8def12c 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 bf6e8c2f9bf7..617ebc4183d9 100644
>> --- a/net/ethtool/ioctl.c
>> +++ b/net/ethtool/ioctl.c
>> @@ -2383,6 +2383,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;
>>
> .
>
diff mbox series

Patch

diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index a2223b685451..7bc4b8def12c 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 bf6e8c2f9bf7..617ebc4183d9 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -2383,6 +2383,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;