| Message ID | 20210924142959.7798-4-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 |
| 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 | 8 maintainers not CCed: arnd@arndb.de petr.vorel@gmail.com idosch@nvidia.com corbet@lwn.net hkallweit1@gmail.com yangbo.lu@nxp.com vladyslavt@nvidia.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 | fail | Errors and warnings before: 2043 this patch: 957 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: line length of 86 exceeds 80 columns |
| netdev/build_allmodconfig_warn | fail | Errors and warnings before: 2063 this patch: 853 |
| netdev/header_inline | success | Link |
On Fri, 24 Sep 2021 22:29:56 +0800 Guangbin Huang wrote: > @@ -621,9 +631,13 @@ struct ethtool_ops { > struct kernel_ethtool_coalesce *, > struct netlink_ext_ack *); > void (*get_ringparam)(struct net_device *, > - struct ethtool_ringparam *); > + struct ethtool_ringparam *, > + struct ethtool_ringparam_ext *, > + struct netlink_ext_ack *); > int (*set_ringparam)(struct net_device *, > - struct ethtool_ringparam *); > + struct ethtool_ringparam *, > + struct ethtool_ringparam_ext *, > + struct netlink_ext_ack *); You need to make the driver changes together with this chunk. Otherwise the build will be broken between the two during bisection.
On Fri, Sep 24, 2021 at 10:29:56PM +0800, Guangbin Huang wrote: > From: Hao Chen <chenhao288@hisilicon.com> > > Add support to set rx buf len via ethtool -G parameter and get > rx buf len via ethtool -g parameter. > > Signed-off-by: Hao Chen <chenhao288@hisilicon.com> > Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com> > --- > Documentation/networking/ethtool-netlink.rst | 2 ++ > include/linux/ethtool.h | 18 ++++++++++++++++-- > include/uapi/linux/ethtool.h | 8 ++++++++ > include/uapi/linux/ethtool_netlink.h | 1 + > net/ethtool/netlink.h | 2 +- > net/ethtool/rings.c | 17 ++++++++++++++++- > 6 files changed, 44 insertions(+), 4 deletions(-) > > diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst > index a47b0255aaf9..9734b7c1e05d 100644 > --- a/Documentation/networking/ethtool-netlink.rst > +++ b/Documentation/networking/ethtool-netlink.rst > @@ -841,6 +841,7 @@ Kernel response contents: > ``ETHTOOL_A_RINGS_RX_MINI`` u32 size of RX mini ring > ``ETHTOOL_A_RINGS_RX_JUMBO`` u32 size of RX jumbo ring > ``ETHTOOL_A_RINGS_TX`` u32 size of TX ring > + ``ETHTOOL_A_RINGS_RX_BUF_LEN`` u32 size of buffers on the ring > ==================================== ====== ========================== > > > @@ -857,6 +858,7 @@ Request contents: > ``ETHTOOL_A_RINGS_RX_MINI`` u32 size of RX mini ring > ``ETHTOOL_A_RINGS_RX_JUMBO`` u32 size of RX jumbo ring > ``ETHTOOL_A_RINGS_TX`` u32 size of TX ring > + ``ETHTOOL_A_RINGS_RX_BUF_LEN`` u32 size of buffers on the ring > ==================================== ====== ========================== > > Kernel checks that requested ring sizes do not exceed limits reported by Would it make sense to let driver report also maximum supported value like it does for existing ring parameters (ring sizes)? Michal
On 2021/9/25 1:47, Jakub Kicinski wrote: > On Fri, 24 Sep 2021 22:29:56 +0800 Guangbin Huang wrote: >> @@ -621,9 +631,13 @@ struct ethtool_ops { >> struct kernel_ethtool_coalesce *, >> struct netlink_ext_ack *); >> void (*get_ringparam)(struct net_device *, >> - struct ethtool_ringparam *); >> + struct ethtool_ringparam *, >> + struct ethtool_ringparam_ext *, >> + struct netlink_ext_ack *); >> int (*set_ringparam)(struct net_device *, >> - struct ethtool_ringparam *); >> + struct ethtool_ringparam *, >> + struct ethtool_ringparam_ext *, >> + struct netlink_ext_ack *); > > You need to make the driver changes together with this chunk. > Otherwise the build will be broken between the two during bisection. > . > Ok, thanks.
On 2021/9/25 7:14, Michal Kubecek wrote: > On Fri, Sep 24, 2021 at 10:29:56PM +0800, Guangbin Huang wrote: >> From: Hao Chen <chenhao288@hisilicon.com> >> >> Add support to set rx buf len via ethtool -G parameter and get >> rx buf len via ethtool -g parameter. >> >> Signed-off-by: Hao Chen <chenhao288@hisilicon.com> >> Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com> >> --- >> Documentation/networking/ethtool-netlink.rst | 2 ++ >> include/linux/ethtool.h | 18 ++++++++++++++++-- >> include/uapi/linux/ethtool.h | 8 ++++++++ >> include/uapi/linux/ethtool_netlink.h | 1 + >> net/ethtool/netlink.h | 2 +- >> net/ethtool/rings.c | 17 ++++++++++++++++- >> 6 files changed, 44 insertions(+), 4 deletions(-) >> >> diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst >> index a47b0255aaf9..9734b7c1e05d 100644 >> --- a/Documentation/networking/ethtool-netlink.rst >> +++ b/Documentation/networking/ethtool-netlink.rst >> @@ -841,6 +841,7 @@ Kernel response contents: >> ``ETHTOOL_A_RINGS_RX_MINI`` u32 size of RX mini ring >> ``ETHTOOL_A_RINGS_RX_JUMBO`` u32 size of RX jumbo ring >> ``ETHTOOL_A_RINGS_TX`` u32 size of TX ring >> + ``ETHTOOL_A_RINGS_RX_BUF_LEN`` u32 size of buffers on the ring >> ==================================== ====== ========================== >> >> >> @@ -857,6 +858,7 @@ Request contents: >> ``ETHTOOL_A_RINGS_RX_MINI`` u32 size of RX mini ring >> ``ETHTOOL_A_RINGS_RX_JUMBO`` u32 size of RX jumbo ring >> ``ETHTOOL_A_RINGS_TX`` u32 size of TX ring >> + ``ETHTOOL_A_RINGS_RX_BUF_LEN`` u32 size of buffers on the ring >> ==================================== ====== ========================== >> >> Kernel checks that requested ring sizes do not exceed limits reported by > > Would it make sense to let driver report also maximum supported value > like it does for existing ring parameters (ring sizes)? > > Michal > We think it have no sense to report maximum supported value. Rx buf len of hns3 driver only supports 2048 and 4096 at present, they are discrete value and checked by driver.
diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst index a47b0255aaf9..9734b7c1e05d 100644 --- a/Documentation/networking/ethtool-netlink.rst +++ b/Documentation/networking/ethtool-netlink.rst @@ -841,6 +841,7 @@ Kernel response contents: ``ETHTOOL_A_RINGS_RX_MINI`` u32 size of RX mini ring ``ETHTOOL_A_RINGS_RX_JUMBO`` u32 size of RX jumbo ring ``ETHTOOL_A_RINGS_TX`` u32 size of TX ring + ``ETHTOOL_A_RINGS_RX_BUF_LEN`` u32 size of buffers on the ring ==================================== ====== ========================== @@ -857,6 +858,7 @@ Request contents: ``ETHTOOL_A_RINGS_RX_MINI`` u32 size of RX mini ring ``ETHTOOL_A_RINGS_RX_JUMBO`` u32 size of RX jumbo ring ``ETHTOOL_A_RINGS_TX`` u32 size of TX ring + ``ETHTOOL_A_RINGS_RX_BUF_LEN`` u32 size of buffers on the ring ==================================== ====== ========================== Kernel checks that requested ring sizes do not exceed limits reported by diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index 849524b55d89..61e42a0b60d3 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h @@ -67,6 +67,14 @@ enum { ETH_RSS_HASH_FUNCS_COUNT }; +/** + * enum ethtool_supported_ring_param - indicator caps for setting ring params + * @ETHTOOL_RING_USE_RX_BUF_LEN: capture for setting rx_buf_len + */ +enum ethtool_supported_ring_param { + ETHTOOL_RING_USE_RX_BUF_LEN = BIT(0), +}; + #define __ETH_RSS_HASH_BIT(bit) ((u32)1 << (bit)) #define __ETH_RSS_HASH(name) __ETH_RSS_HASH_BIT(ETH_RSS_HASH_##name##_BIT) @@ -420,6 +428,7 @@ struct ethtool_module_eeprom { * @cap_link_lanes_supported: indicates if the driver supports lanes * parameter. * @supported_coalesce_params: supported types of interrupt coalescing. + * @supported_ring_params: supported ring params. * @get_drvinfo: Report driver/device information. Should only set the * @driver, @version, @fw_version and @bus_info fields. If not * implemented, the @driver and @bus_info fields will be filled in @@ -596,6 +605,7 @@ struct ethtool_module_eeprom { struct ethtool_ops { u32 cap_link_lanes_supported:1; u32 supported_coalesce_params; + u32 supported_ring_params; void (*get_drvinfo)(struct net_device *, struct ethtool_drvinfo *); int (*get_regs_len)(struct net_device *); void (*get_regs)(struct net_device *, struct ethtool_regs *, void *); @@ -621,9 +631,13 @@ struct ethtool_ops { struct kernel_ethtool_coalesce *, struct netlink_ext_ack *); void (*get_ringparam)(struct net_device *, - struct ethtool_ringparam *); + struct ethtool_ringparam *, + struct ethtool_ringparam_ext *, + struct netlink_ext_ack *); int (*set_ringparam)(struct net_device *, - struct ethtool_ringparam *); + struct ethtool_ringparam *, + struct ethtool_ringparam_ext *, + struct netlink_ext_ack *); void (*get_pause_stats)(struct net_device *dev, struct ethtool_pause_stats *pause_stats); void (*get_pauseparam)(struct net_device *, diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h index 266e95e4fb33..83544186cbb5 100644 --- a/include/uapi/linux/ethtool.h +++ b/include/uapi/linux/ethtool.h @@ -535,6 +535,14 @@ struct ethtool_ringparam { __u32 tx_pending; }; +/** + * struct ethtool_ringparam_ext - RX/TX ring configuration + * @rx_buf_len: Current length of buffers on the rx ring. + */ +struct ethtool_ringparam_ext { + __u32 rx_buf_len; +}; + /** * struct ethtool_channels - configuring number of network channel * @cmd: ETHTOOL_{G,S}CHANNELS diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h index 5545f1ca9237..3883fa4168e9 100644 --- a/include/uapi/linux/ethtool_netlink.h +++ b/include/uapi/linux/ethtool_netlink.h @@ -325,6 +325,7 @@ enum { ETHTOOL_A_RINGS_RX_MINI, /* u32 */ ETHTOOL_A_RINGS_RX_JUMBO, /* u32 */ ETHTOOL_A_RINGS_TX, /* u32 */ + ETHTOOL_A_RINGS_RX_BUF_LEN, /* u32 */ /* add new constants above here */ __ETHTOOL_A_RINGS_CNT, diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h index e8987e28036f..3183f1fc6990 100644 --- a/net/ethtool/netlink.h +++ b/net/ethtool/netlink.h @@ -355,7 +355,7 @@ extern const struct nla_policy ethnl_features_set_policy[ETHTOOL_A_FEATURES_WANT extern const struct nla_policy ethnl_privflags_get_policy[ETHTOOL_A_PRIVFLAGS_HEADER + 1]; extern const struct nla_policy ethnl_privflags_set_policy[ETHTOOL_A_PRIVFLAGS_FLAGS + 1]; extern const struct nla_policy ethnl_rings_get_policy[ETHTOOL_A_RINGS_HEADER + 1]; -extern const struct nla_policy ethnl_rings_set_policy[ETHTOOL_A_RINGS_TX + 1]; +extern const struct nla_policy ethnl_rings_set_policy[ETHTOOL_A_RINGS_RX_BUF_LEN + 1]; extern const struct nla_policy ethnl_channels_get_policy[ETHTOOL_A_CHANNELS_HEADER + 1]; extern const struct nla_policy ethnl_channels_set_policy[ETHTOOL_A_CHANNELS_COMBINED_COUNT + 1]; extern const struct nla_policy ethnl_coalesce_get_policy[ETHTOOL_A_COALESCE_HEADER + 1]; diff --git a/net/ethtool/rings.c b/net/ethtool/rings.c index 4e097812a967..dd645d1334be 100644 --- a/net/ethtool/rings.c +++ b/net/ethtool/rings.c @@ -10,6 +10,7 @@ struct rings_req_info { struct rings_reply_data { struct ethnl_reply_data base; struct ethtool_ringparam ringparam; + struct ethtool_ringparam_ext ringparam_ext; }; #define RINGS_REPDATA(__reply_base) \ @@ -49,7 +50,8 @@ static int rings_reply_size(const struct ethnl_req_info *req_base, nla_total_size(sizeof(u32)) + /* _RINGS_RX */ nla_total_size(sizeof(u32)) + /* _RINGS_RX_MINI */ nla_total_size(sizeof(u32)) + /* _RINGS_RX_JUMBO */ - nla_total_size(sizeof(u32)); /* _RINGS_TX */ + nla_total_size(sizeof(u32)) + /* _RINGS_TX */ + nla_total_size(sizeof(u32)); /* _RINGS_RX_BUF_LEN */ } static int rings_fill_reply(struct sk_buff *skb, @@ -105,10 +107,12 @@ const struct nla_policy ethnl_rings_set_policy[] = { [ETHTOOL_A_RINGS_RX_MINI] = { .type = NLA_U32 }, [ETHTOOL_A_RINGS_RX_JUMBO] = { .type = NLA_U32 }, [ETHTOOL_A_RINGS_TX] = { .type = NLA_U32 }, + [ETHTOOL_A_RINGS_RX_BUF_LEN] = NLA_POLICY_MIN(NLA_U32, 1), }; int ethnl_set_rings(struct sk_buff *skb, struct genl_info *info) { + struct ethtool_ringparam_ext ringparam_ext = {}; struct ethtool_ringparam ringparam = {}; struct ethnl_req_info req_info = {}; struct nlattr **tb = info->attrs; @@ -142,6 +146,8 @@ int ethnl_set_rings(struct sk_buff *skb, struct genl_info *info) ethnl_update_u32(&ringparam.rx_jumbo_pending, tb[ETHTOOL_A_RINGS_RX_JUMBO], &mod); ethnl_update_u32(&ringparam.tx_pending, tb[ETHTOOL_A_RINGS_TX], &mod); + ethnl_update_u32(&ringparam_ext.rx_buf_len, + tb[ETHTOOL_A_RINGS_RX_BUF_LEN], &mod); ret = 0; if (!mod) goto out_ops; @@ -164,6 +170,15 @@ int ethnl_set_rings(struct sk_buff *skb, struct genl_info *info) goto out_ops; } + if (ringparam_ext.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], + "not supported setting rx buf len"); + goto out_ops; + } + ret = dev->ethtool_ops->set_ringparam(dev, &ringparam); if (ret < 0) goto out_ops;