| Message ID | 1622021262-8881-3-git-send-email-tanhuazhong@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ethtool: extend coalesce uAPI | expand |
| Context | Check | Description |
|---|---|---|
| netdev/apply | fail | Patch does not apply to net-next |
| netdev/tree_selection | success | Clearly marked for net-next |
On Wed, 26 May 2021 17:27:40 +0800 Huazhong Tan wrote: > Currently, there many drivers who support CQE mode configuration, > some configure it as a fixed when initialized, some provide an > interface to change it by ethtool private flags. In order make it > more generic, add 'ETHTOOL_A_COALESCE_USE_CQE_TX' and > 'ETHTOOL_A_COALESCE_USE_CQE_RX' attribute and expand struct > kernel_ethtool_coalesce with use_cqe_mode_tx and use_cqe_mode_rx, > then these parameters can be accessed by ethtool netlink coalesce > uAPI. > > Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com> > diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst > index 25131df..975394e 100644 > --- a/Documentation/networking/ethtool-netlink.rst > +++ b/Documentation/networking/ethtool-netlink.rst > @@ -937,6 +937,8 @@ Kernel response contents: > ``ETHTOOL_A_COALESCE_TX_USECS_HIGH`` u32 delay (us), high Tx > ``ETHTOOL_A_COALESCE_TX_MAX_FRAMES_HIGH`` u32 max packets, high Tx > ``ETHTOOL_A_COALESCE_RATE_SAMPLE_INTERVAL`` u32 rate sampling interval > + ``ETHTOOL_A_COALESCE_USE_CQE_TX`` bool Tx CQE mode > + ``ETHTOOL_A_COALESCE_USE_CQE_RX`` bool Rx CQE mode > =========================================== ====== ======================= > > Attributes are only included in reply if their value is not zero or the > @@ -975,6 +977,8 @@ Request contents: > ``ETHTOOL_A_COALESCE_TX_USECS_HIGH`` u32 delay (us), high Tx > ``ETHTOOL_A_COALESCE_TX_MAX_FRAMES_HIGH`` u32 max packets, high Tx > ``ETHTOOL_A_COALESCE_RATE_SAMPLE_INTERVAL`` u32 rate sampling interval > + ``ETHTOOL_A_COALESCE_USE_CQE_TX`` bool Tx CQE mode > + ``ETHTOOL_A_COALESCE_USE_CQE_RX`` bool Rx CQE mode > =========================================== ====== ======================= > > Request is rejected if it attributes declared as unsupported by driver (i.e. You need to thoroughly document the semantics. Can you point us to which drivers/devices implement similar modes of operation (if they exist we need to make sure semantics match)? > diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h > index 1030540..9d0a386 100644 > --- a/include/linux/ethtool.h > +++ b/include/linux/ethtool.h > @@ -179,6 +179,8 @@ __ethtool_get_link_ksettings(struct net_device *dev, > > struct kernel_ethtool_coalesce { > struct ethtool_coalesce base; > + __u32 use_cqe_mode_tx; > + __u32 use_cqe_mode_rx; No __ in front, this is not a user space structure. Why not bool or a bitfield? > }; > > /** > @@ -216,6 +223,8 @@ const struct nla_policy ethnl_coalesce_set_policy[] = { > [ETHTOOL_A_COALESCE_TX_USECS_HIGH] = { .type = NLA_U32 }, > [ETHTOOL_A_COALESCE_TX_MAX_FRAMES_HIGH] = { .type = NLA_U32 }, > [ETHTOOL_A_COALESCE_RATE_SAMPLE_INTERVAL] = { .type = NLA_U32 }, > + [ETHTOOL_A_COALESCE_USE_CQE_MODE_TX] = { .type = NLA_U8 }, > + [ETHTOOL_A_COALESCE_USE_CQE_MODE_RX] = { .type = NLA_U8 }, This needs a policy to make sure values are <= 1.
On 2021/5/27 8:00, Jakub Kicinski wrote: > On Wed, 26 May 2021 17:27:40 +0800 Huazhong Tan wrote: >> Currently, there many drivers who support CQE mode configuration, >> some configure it as a fixed when initialized, some provide an >> interface to change it by ethtool private flags. In order make it >> more generic, add 'ETHTOOL_A_COALESCE_USE_CQE_TX' and >> 'ETHTOOL_A_COALESCE_USE_CQE_RX' attribute and expand struct >> kernel_ethtool_coalesce with use_cqe_mode_tx and use_cqe_mode_rx, >> then these parameters can be accessed by ethtool netlink coalesce >> uAPI. >> >> Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com> >> diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst >> index 25131df..975394e 100644 >> --- a/Documentation/networking/ethtool-netlink.rst >> +++ b/Documentation/networking/ethtool-netlink.rst >> @@ -937,6 +937,8 @@ Kernel response contents: >> ``ETHTOOL_A_COALESCE_TX_USECS_HIGH`` u32 delay (us), high Tx >> ``ETHTOOL_A_COALESCE_TX_MAX_FRAMES_HIGH`` u32 max packets, high Tx >> ``ETHTOOL_A_COALESCE_RATE_SAMPLE_INTERVAL`` u32 rate sampling interval >> + ``ETHTOOL_A_COALESCE_USE_CQE_TX`` bool Tx CQE mode >> + ``ETHTOOL_A_COALESCE_USE_CQE_RX`` bool Rx CQE mode >> =========================================== ====== ======================= >> >> Attributes are only included in reply if their value is not zero or the >> @@ -975,6 +977,8 @@ Request contents: >> ``ETHTOOL_A_COALESCE_TX_USECS_HIGH`` u32 delay (us), high Tx >> ``ETHTOOL_A_COALESCE_TX_MAX_FRAMES_HIGH`` u32 max packets, high Tx >> ``ETHTOOL_A_COALESCE_RATE_SAMPLE_INTERVAL`` u32 rate sampling interval >> + ``ETHTOOL_A_COALESCE_USE_CQE_TX`` bool Tx CQE mode >> + ``ETHTOOL_A_COALESCE_USE_CQE_RX`` bool Rx CQE mode >> =========================================== ====== ======================= >> >> Request is rejected if it attributes declared as unsupported by driver (i.e. > You need to thoroughly document the semantics. Can you point us to > which drivers/devices implement similar modes of operation (if they > exist we need to make sure semantics match)? Ok, will complement the semantics. Currently, only mlx5 provides a interface to update this mode through ethtool priv-flag. other drivers like broadcom and ice just use a fixed EQE and do not have update interface. >> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h >> index 1030540..9d0a386 100644 >> --- a/include/linux/ethtool.h >> +++ b/include/linux/ethtool.h >> @@ -179,6 +179,8 @@ __ethtool_get_link_ksettings(struct net_device *dev, >> >> struct kernel_ethtool_coalesce { >> struct ethtool_coalesce base; >> + __u32 use_cqe_mode_tx; >> + __u32 use_cqe_mode_rx; > No __ in front, this is not a user space structure. > Why not bool or a bitfield? bool is enough, __u32 is used here to be consistent with fields in struct ethtool_coalesce. This seems unnecessary? >> }; >> >> /** >> @@ -216,6 +223,8 @@ const struct nla_policy ethnl_coalesce_set_policy[] = { >> [ETHTOOL_A_COALESCE_TX_USECS_HIGH] = { .type = NLA_U32 }, >> [ETHTOOL_A_COALESCE_TX_MAX_FRAMES_HIGH] = { .type = NLA_U32 }, >> [ETHTOOL_A_COALESCE_RATE_SAMPLE_INTERVAL] = { .type = NLA_U32 }, >> + [ETHTOOL_A_COALESCE_USE_CQE_MODE_TX] = { .type = NLA_U8 }, >> + [ETHTOOL_A_COALESCE_USE_CQE_MODE_RX] = { .type = NLA_U8 }, > This needs a policy to make sure values are <= 1. > > .
On Thu, 27 May 2021 10:00:44 +0800 Huazhong Tan wrote: > >> @@ -179,6 +179,8 @@ __ethtool_get_link_ksettings(struct net_device *dev, > >> > >> struct kernel_ethtool_coalesce { > >> struct ethtool_coalesce base; > >> + __u32 use_cqe_mode_tx; > >> + __u32 use_cqe_mode_rx; > > No __ in front, this is not a user space structure. > > Why not bool or a bitfield? > > bool is enough, __u32 is used here to be consistent with > > fields in struct ethtool_coalesce. > > This seems unnecessary? Yup, I think the IOCTL made everything a __u32 for uniformity of the uAPI and to avoid holes and paddings. This is an internal kernel structure so natural types like bool are better.
diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst index 25131df..975394e 100644 --- a/Documentation/networking/ethtool-netlink.rst +++ b/Documentation/networking/ethtool-netlink.rst @@ -937,6 +937,8 @@ Kernel response contents: ``ETHTOOL_A_COALESCE_TX_USECS_HIGH`` u32 delay (us), high Tx ``ETHTOOL_A_COALESCE_TX_MAX_FRAMES_HIGH`` u32 max packets, high Tx ``ETHTOOL_A_COALESCE_RATE_SAMPLE_INTERVAL`` u32 rate sampling interval + ``ETHTOOL_A_COALESCE_USE_CQE_TX`` bool Tx CQE mode + ``ETHTOOL_A_COALESCE_USE_CQE_RX`` bool Rx CQE mode =========================================== ====== ======================= Attributes are only included in reply if their value is not zero or the @@ -975,6 +977,8 @@ Request contents: ``ETHTOOL_A_COALESCE_TX_USECS_HIGH`` u32 delay (us), high Tx ``ETHTOOL_A_COALESCE_TX_MAX_FRAMES_HIGH`` u32 max packets, high Tx ``ETHTOOL_A_COALESCE_RATE_SAMPLE_INTERVAL`` u32 rate sampling interval + ``ETHTOOL_A_COALESCE_USE_CQE_TX`` bool Tx CQE mode + ``ETHTOOL_A_COALESCE_USE_CQE_RX`` bool Rx CQE mode =========================================== ====== ======================= Request is rejected if it attributes declared as unsupported by driver (i.e. diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index 1030540..9d0a386 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h @@ -179,6 +179,8 @@ __ethtool_get_link_ksettings(struct net_device *dev, struct kernel_ethtool_coalesce { struct ethtool_coalesce base; + __u32 use_cqe_mode_tx; + __u32 use_cqe_mode_rx; }; /** @@ -220,7 +222,9 @@ bool ethtool_convert_link_mode_to_legacy_u32(u32 *legacy_u32, #define ETHTOOL_COALESCE_TX_USECS_HIGH BIT(19) #define ETHTOOL_COALESCE_TX_MAX_FRAMES_HIGH BIT(20) #define ETHTOOL_COALESCE_RATE_SAMPLE_INTERVAL BIT(21) -#define ETHTOOL_COALESCE_ALL_PARAMS GENMASK(21, 0) +#define ETHTOOL_COALESCE_USE_CQE_RX BIT(22) +#define ETHTOOL_COALESCE_USE_CQE_TX BIT(23) +#define ETHTOOL_COALESCE_ALL_PARAMS GENMASK(23, 0) #define ETHTOOL_COALESCE_USECS \ (ETHTOOL_COALESCE_RX_USECS | ETHTOOL_COALESCE_TX_USECS) diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h index 825cfda..1ad32a4 100644 --- a/include/uapi/linux/ethtool_netlink.h +++ b/include/uapi/linux/ethtool_netlink.h @@ -375,6 +375,8 @@ enum { ETHTOOL_A_COALESCE_TX_USECS_HIGH, /* u32 */ ETHTOOL_A_COALESCE_TX_MAX_FRAMES_HIGH, /* u32 */ ETHTOOL_A_COALESCE_RATE_SAMPLE_INTERVAL, /* u32 */ + ETHTOOL_A_COALESCE_USE_CQE_MODE_TX, /* u8 */ + ETHTOOL_A_COALESCE_USE_CQE_MODE_RX, /* u8 */ /* add new constants above here */ __ETHTOOL_A_COALESCE_CNT, diff --git a/net/ethtool/coalesce.c b/net/ethtool/coalesce.c index 70e6331..2000c56 100644 --- a/net/ethtool/coalesce.c +++ b/net/ethtool/coalesce.c @@ -101,7 +101,9 @@ static int coalesce_reply_size(const struct ethnl_req_info *req_base, nla_total_size(sizeof(u32)) + /* _RX_MAX_FRAMES_HIGH */ nla_total_size(sizeof(u32)) + /* _TX_USECS_HIGH */ nla_total_size(sizeof(u32)) + /* _TX_MAX_FRAMES_HIGH */ - nla_total_size(sizeof(u32)); /* _RATE_SAMPLE_INTERVAL */ + nla_total_size(sizeof(u32)) + /* _RATE_SAMPLE_INTERVAL */ + nla_total_size(sizeof(u8)) + /* _USE_CQE_MODE_TX */ + nla_total_size(sizeof(u8)); /* _USE_CQE_MODE_RX */ } static bool coalesce_put_u32(struct sk_buff *skb, u16 attr_type, u32 val, @@ -125,7 +127,8 @@ static int coalesce_fill_reply(struct sk_buff *skb, const struct ethnl_reply_data *reply_base) { const struct coalesce_reply_data *data = COALESCE_REPDATA(reply_base); - const struct ethtool_coalesce *cbase = &data->coalesce.base; + const struct kernel_ethtool_coalesce *coalesce = &data->coalesce; + const struct ethtool_coalesce *cbase = &coalesce->base; u32 supported = data->supported_params; if (coalesce_put_u32(skb, ETHTOOL_A_COALESCE_RX_USECS, @@ -171,7 +174,11 @@ static int coalesce_fill_reply(struct sk_buff *skb, coalesce_put_u32(skb, ETHTOOL_A_COALESCE_TX_MAX_FRAMES_HIGH, cbase->tx_max_coalesced_frames_high, supported) || coalesce_put_u32(skb, ETHTOOL_A_COALESCE_RATE_SAMPLE_INTERVAL, - cbase->rate_sample_interval, supported)) + cbase->rate_sample_interval, supported) || + coalesce_put_bool(skb, ETHTOOL_A_COALESCE_USE_CQE_MODE_TX, + coalesce->use_cqe_mode_tx, supported) || + coalesce_put_bool(skb, ETHTOOL_A_COALESCE_USE_CQE_MODE_RX, + coalesce->use_cqe_mode_rx, supported)) return -EMSGSIZE; return 0; @@ -216,6 +223,8 @@ const struct nla_policy ethnl_coalesce_set_policy[] = { [ETHTOOL_A_COALESCE_TX_USECS_HIGH] = { .type = NLA_U32 }, [ETHTOOL_A_COALESCE_TX_MAX_FRAMES_HIGH] = { .type = NLA_U32 }, [ETHTOOL_A_COALESCE_RATE_SAMPLE_INTERVAL] = { .type = NLA_U32 }, + [ETHTOOL_A_COALESCE_USE_CQE_MODE_TX] = { .type = NLA_U8 }, + [ETHTOOL_A_COALESCE_USE_CQE_MODE_RX] = { .type = NLA_U8 }, }; int ethnl_set_coalesce(struct sk_buff *skb, struct genl_info *info) @@ -305,6 +314,10 @@ int ethnl_set_coalesce(struct sk_buff *skb, struct genl_info *info) tb[ETHTOOL_A_COALESCE_TX_MAX_FRAMES_HIGH], &mod); ethnl_update_u32(&cbase->rate_sample_interval, tb[ETHTOOL_A_COALESCE_RATE_SAMPLE_INTERVAL], &mod); + ethnl_update_bool32(&coalesce.use_cqe_mode_tx, + tb[ETHTOOL_A_COALESCE_USE_CQE_MODE_TX], &mod); + ethnl_update_bool32(&coalesce.use_cqe_mode_rx, + tb[ETHTOOL_A_COALESCE_USE_CQE_MODE_RX], &mod); ret = 0; if (!mod) goto out_ops; diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h index 8abcbc1..868e33e 100644 --- a/net/ethtool/netlink.h +++ b/net/ethtool/netlink.h @@ -369,7 +369,7 @@ extern const struct nla_policy ethnl_rings_set_policy[ETHTOOL_A_RINGS_TX + 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]; -extern const struct nla_policy ethnl_coalesce_set_policy[ETHTOOL_A_COALESCE_RATE_SAMPLE_INTERVAL + 1]; +extern const struct nla_policy ethnl_coalesce_set_policy[ETHTOOL_A_COALESCE_MAX + 1]; extern const struct nla_policy ethnl_pause_get_policy[ETHTOOL_A_PAUSE_HEADER + 1]; extern const struct nla_policy ethnl_pause_set_policy[ETHTOOL_A_PAUSE_TX + 1]; extern const struct nla_policy ethnl_eee_get_policy[ETHTOOL_A_EEE_HEADER + 1];
Currently, there many drivers who support CQE mode configuration, some configure it as a fixed when initialized, some provide an interface to change it by ethtool private flags. In order make it more generic, add 'ETHTOOL_A_COALESCE_USE_CQE_TX' and 'ETHTOOL_A_COALESCE_USE_CQE_RX' attribute and expand struct kernel_ethtool_coalesce with use_cqe_mode_tx and use_cqe_mode_rx, then these parameters can be accessed by ethtool netlink coalesce uAPI. Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com> --- Documentation/networking/ethtool-netlink.rst | 4 ++++ include/linux/ethtool.h | 6 +++++- include/uapi/linux/ethtool_netlink.h | 2 ++ net/ethtool/coalesce.c | 19 ++++++++++++++++--- net/ethtool/netlink.h | 2 +- 5 files changed, 28 insertions(+), 5 deletions(-)