From: Jie Wang <[email protected]>
These three patches add tx push in ring params and adapt the set and get APIs
of ring params.
ChangeLog:
RFC V4->V1
1.Add detailed description about the tx push mode
2.Modify the patch subject title
link: https://lore.kernel.org/netdev/[email protected]/
RFC V3->RFC V4
1.Put three request checks before rtnl_lock() in ethnl_set_rings
2.Add tx push feature description in Documentation/networking/ethtool-netlink.rst
3.Use netdev_dbg to track changes in hns3_set_tx_push
link: https://lore.kernel.org/netdev/[email protected]/
RFC V2->RFC V3
1.Add tx push documentation in Documentation/networking/ethtool-netlink.rst
2.Use u8 to store tx push in struct kernel_ethtool_ringparam
3.Add ETHTOOL_RING_USE_TX_PUSH to reject setting for unsupported driver
4.Use NLA_POLICY_MAX(NLA_U8, 1) to limit the tx push value
link: https://lore.kernel.org/netdev/[email protected]/
RFC V1->RFC V2
1.Extend tx push param in ringparam, suggested by Jakub Kicinski.
link: https://lore.kernel.org/netdev/[email protected]/
Jie Wang (3):
net: ethtool: extend ringparam set/get APIs for tx_push
net: ethtool: move checks before rtnl_lock() in ethnl_set_rings
net: hns3: add tx push support in hns3 ring param process
Documentation/networking/ethtool-netlink.rst | 8 +++
.../ethernet/hisilicon/hns3/hns3_ethtool.c | 33 ++++++++++-
include/linux/ethtool.h | 3 +
include/uapi/linux/ethtool_netlink.h | 1 +
net/ethtool/netlink.h | 2 +-
net/ethtool/rings.c | 56 ++++++++++++-------
6 files changed, 81 insertions(+), 22 deletions(-)
--
2.33.0
From: Jie Wang <[email protected]>
Currently these two checks in ethnl_set_rings are added after rtnl_lock()
which will do useless works if the request is invalid.
So this patch moves these checks before the rtnl_lock() to avoid these
costs.
Signed-off-by: Jie Wang <[email protected]>
Signed-off-by: Guangbin Huang <[email protected]>
---
net/ethtool/rings.c | 38 ++++++++++++++++++++------------------
1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/net/ethtool/rings.c b/net/ethtool/rings.c
index 9ed60c507d97..46415d8fc256 100644
--- a/net/ethtool/rings.c
+++ b/net/ethtool/rings.c
@@ -152,6 +152,26 @@ int ethnl_set_rings(struct sk_buff *skb, struct genl_info *info)
if (!ops->get_ringparam || !ops->set_ringparam)
goto out_dev;
+ if (tb[ETHTOOL_A_RINGS_RX_BUF_LEN] &&
+ nla_get_u32(tb[ETHTOOL_A_RINGS_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 rx buf len not supported");
+ goto out_dev;
+ }
+
+ if (tb[ETHTOOL_A_RINGS_CQE_SIZE] &&
+ nla_get_u32(tb[ETHTOOL_A_RINGS_CQE_SIZE]) &&
+ !(ops->supported_ring_params & ETHTOOL_RING_USE_CQE_SIZE)) {
+ ret = -EOPNOTSUPP;
+ NL_SET_ERR_MSG_ATTR(info->extack,
+ tb[ETHTOOL_A_RINGS_CQE_SIZE],
+ "setting cqe size not supported");
+ goto out_dev;
+ }
+
if (tb[ETHTOOL_A_RINGS_TX_PUSH] &&
!(ops->supported_ring_params & ETHTOOL_RING_USE_TX_PUSH)) {
ret = -EOPNOTSUPP;
@@ -201,24 +221,6 @@ int ethnl_set_rings(struct sk_buff *skb, struct genl_info *info)
goto out_ops;
}
- if (kernel_ringparam.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 rx buf len not supported");
- goto out_ops;
- }
-
- if (kernel_ringparam.cqe_size &&
- !(ops->supported_ring_params & ETHTOOL_RING_USE_CQE_SIZE)) {
- ret = -EOPNOTSUPP;
- NL_SET_ERR_MSG_ATTR(info->extack,
- tb[ETHTOOL_A_RINGS_CQE_SIZE],
- "setting cqe size not supported");
- goto out_ops;
- }
-
ret = dev->ethtool_ops->set_ringparam(dev, &ringparam,
&kernel_ringparam, info->extack);
if (ret < 0)
--
2.33.0
On Fri, 8 Apr 2022 15:12:44 +0800 Guangbin Huang wrote:
> + if (tb[ETHTOOL_A_RINGS_RX_BUF_LEN] &&
> + nla_get_u32(tb[ETHTOOL_A_RINGS_RX_BUF_LEN]) != 0 &&
I think we can drop the value checking. If attribute is present and
drivers doesn't support - reject. I don't think that would require
any changes to existing user space but please double check.
> + !(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 rx buf len not supported");
> + goto out_dev;
> + }
> +
> + if (tb[ETHTOOL_A_RINGS_CQE_SIZE] &&
> + nla_get_u32(tb[ETHTOOL_A_RINGS_CQE_SIZE]) &&
> + !(ops->supported_ring_params & ETHTOOL_RING_USE_CQE_SIZE)) {
> + ret = -EOPNOTSUPP;
> + NL_SET_ERR_MSG_ATTR(info->extack,
> + tb[ETHTOOL_A_RINGS_CQE_SIZE],
> + "setting cqe size not supported");
> + goto out_dev;
> + }
On 2022/4/9 5:58, Jakub Kicinski wrote:
> On Fri, 8 Apr 2022 15:12:44 +0800 Guangbin Huang wrote:
>> + if (tb[ETHTOOL_A_RINGS_RX_BUF_LEN] &&
>> + nla_get_u32(tb[ETHTOOL_A_RINGS_RX_BUF_LEN]) != 0 &&
>
> I think we can drop the value checking. If attribute is present and
> drivers doesn't support - reject. I don't think that would require
> any changes to existing user space but please double check.
>
I have checked user space code and tested the delete version on my
server, these value checking will be dropped in v2.
>> + !(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 rx buf len not supported");
>> + goto out_dev;
>> + }
>> +
>> + if (tb[ETHTOOL_A_RINGS_CQE_SIZE] &&
>> + nla_get_u32(tb[ETHTOOL_A_RINGS_CQE_SIZE]) &&
>> + !(ops->supported_ring_params & ETHTOOL_RING_USE_CQE_SIZE)) {
>> + ret = -EOPNOTSUPP;
>> + NL_SET_ERR_MSG_ATTR(info->extack,
>> + tb[ETHTOOL_A_RINGS_CQE_SIZE],
>> + "setting cqe size not supported");
>> + goto out_dev;
>> + }
>
> .
>