netdev - Re: [PATCH V4 net-next 0/6] ethtool: add support to set/get tx copybreak buf size and rx buf len

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20211014131420.23598-1-alexandr.lobakin@intel.com>
Date:   Thu, 14 Oct 2021 15:14:20 +0200
From:   Alexander Lobakin <alexandr.lobakin@...el.com>
To:     Guangbin Huang <huangguangbin2@...wei.com>
Cc:     Alexander Lobakin <alexandr.lobakin@...el.com>,
        davem@...emloft.net, kuba@...nel.org, mkubecek@...e.cz,
        andrew@...n.ch, amitc@...lanox.com, idosch@...sch.org,
        danieller@...dia.com, jesse.brandeburg@...el.com,
        anthony.l.nguyen@...el.com, jdike@...toit.com, richard@....at,
        anton.ivanov@...bridgegreys.com, netanel@...zon.com,
        akiyano@...zon.com, gtzalik@...zon.com, saeedb@...zon.com,
        chris.snook@...il.com, ulli.kroll@...glemail.com,
        linus.walleij@...aro.org, jeroendb@...gle.com, csully@...gle.com,
        awogbemila@...gle.com, jdmason@...zu.us, rain.1986.08.12@...il.com,
        zyjzyj2000@...il.com, kys@...rosoft.com, haiyangz@...rosoft.com,
        mst@...hat.com, jasowang@...hat.com, doshir@...are.com,
        pv-drivers@...are.com, jwi@...ux.ibm.com, kgraul@...ux.ibm.com,
        hca@...ux.ibm.com, gor@...ux.ibm.com, johannes@...solutions.net,
        netdev@...r.kernel.org, lipeng321@...wei.com,
        chenhao288@...ilicon.com, linux-s390@...r.kernel.org
Subject: Re: [PATCH V4 net-next 0/6] ethtool: add support to set/get tx copybreak buf size and rx buf len

From: Guangbin Huang <huangguangbin2@...wei.com>
Date: Thu, 14 Oct 2021 19:39:37 +0800

Hi there,

> From: Hao Chen <chenhao288@...ilicon.com>
> 
> This series add support to set/get tx copybreak buf size and rx buf len via
> ethtool and hns3 driver implements them.
> 
> Tx copybreak 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

Isn't that supposed to be changed on changing Tx copybreak value
itsef?
And what if I set Tx copybreak buf size value lower than Tx
copybreak? I see no sanity checks for this.

> 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

Isn't that supposed to be changed on changing MTU?
And what if I set Rx buf len value lower than MTU? I see no checks
as well.

That means, do we _really_ need two new tunables?

> Change log:
> V3 -> V4
> 1.Fix a few allmodconfig compile warning.
> 2.Add more '=' synbol to ethtool-netlink.rst to refine format.
> 3.Move definement of struct ethtool_ringparam_ext to include/linux/ethtool.h.
> 4.Move related modify of rings_fill_reply() from patch 4/6 to patch 3/6.
> 
> V2 -> V3
> 1.Remove documentation for tx copybreak buf size, there is description for
> it in userspace ethtool.
> 2.Move extending parameters for get/set_ringparam function from patch3/6
> to patch 4/6.
> 
> V1 -> V2
> 1.Add documentation for rx buf len and tx copybreak buf size.
> 2.Extend structure ringparam_ext for extenal ring params.
> 3.Change type of ETHTOOL_A_RINGS_RX_BUF_LEN from NLA_U32 to
>   NLA_POLICY_MIN(NLA_U32, 1).
> 4.Add supported_ring_params in ethtool_ops to indicate if support external
>   params.
> 

[snip]

Thanks,
Al