From: Guangbin Huang <huangguangbin2@xxxxxxxxxx> Date: Thu, 14 Oct 2021 19:39:37 +0800 Hi there, > From: Hao Chen <chenhao288@xxxxxxxxxxxxx> > > 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
