* [PATCH net-next v5 0/4] ethtool: provide the dim profile fine-tuning channel
@ 2024-04-09 12:03 Heng Qi
2024-04-09 12:03 ` [PATCH net-next v5 1/4] ethtool: provide customized dim profile management Heng Qi
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Heng Qi @ 2024-04-09 12:03 UTC (permalink / raw
To: netdev, virtualization
Cc: Jakub Kicinski, David S. Miller, Eric Dumazet, Paolo Abeni,
Jason Wang, Michael S. Tsirkin, Ratheesh Kannoth,
Alexander Lobakin, Xuan Zhuo
The NetDIM library provides excellent acceleration for many modern
network cards. However, the default profiles of DIM limits its maximum
capabilities for different NICs, so providing a way which the NIC can
be custom configured is necessary.
Currently, interaction with the driver is still based on the commonly
used "ethtool -C". The driver declares its supported parameters based
on .supported_coalesce_params, and implements driver-related custom
restrictions in .set_coalesce and .get_coalesce.
Please review, thank you very much!
Changelog
=====
v4->v5:
- Update some snippets from Kuba, Thanks.
v3->v4:
- Some tiny updates and patch 1 only add a new comment.
v2->v3:
- Break up the attributes to avoid the use of raw c structs.
- Use per-device profile instead of global profile in the driver.
v1->v2:
- Use ethtool tool instead of net-sysfs
V1 link:
https://lore.kernel.org/all/1710421773-61277-1-git-send-email-hengqi@linux.alibaba.com/#r
Heng Qi (4):
ethtool: provide customized dim profile management
linux/dim: move profiles from .c to .h file
virtio-net: refactor dim initialization/destruction
virtio-net: support dim profile fine-tuning
Documentation/netlink/specs/ethtool.yaml | 33 +++++
Documentation/networking/ethtool-netlink.rst | 8 ++
drivers/net/virtio_net.c | 78 ++++++++++--
include/linux/dim.h | 45 +++++++
include/linux/ethtool.h | 16 ++-
include/uapi/linux/ethtool_netlink.h | 24 ++++
lib/dim/net_dim.c | 44 -------
net/ethtool/coalesce.c | 172 ++++++++++++++++++++++++++-
8 files changed, 360 insertions(+), 60 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net-next v5 1/4] ethtool: provide customized dim profile management
2024-04-09 12:03 [PATCH net-next v5 0/4] ethtool: provide the dim profile fine-tuning channel Heng Qi
@ 2024-04-09 12:03 ` Heng Qi
2024-04-10 1:44 ` Jakub Kicinski
2024-04-09 12:03 ` [PATCH net-next v5 2/4] linux/dim: move profiles from .c to .h file Heng Qi
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Heng Qi @ 2024-04-09 12:03 UTC (permalink / raw
To: netdev, virtualization
Cc: Jakub Kicinski, David S. Miller, Eric Dumazet, Paolo Abeni,
Jason Wang, Michael S. Tsirkin, Ratheesh Kannoth,
Alexander Lobakin, Xuan Zhuo
The NetDIM library, currently leveraged by an array of NICs, delivers
excellent acceleration benefits. Nevertheless, NICs vary significantly
in their dim profile list prerequisites.
Specifically, virtio-net backends may present diverse sw or hw device
implementation, making a one-size-fits-all parameter list impractical.
On Alibaba Cloud, the virtio DPU's performance under the default DIM
profile falls short of expectations, partly due to a mismatch in
parameter configuration.
I also noticed that ice/idpf/ena and other NICs have customized
profilelist or placed some restrictions on dim capabilities.
Motivated by this, I tried adding new params for "ethtool -C" that provides
a per-device control to modify and access a device's interrupt parameters.
Usage
========
1. Query the currently customized list of the device
$ ethtool -c ethx
...
rx-eqe-profile:
{.usec = 1, .pkts = 256, .comps = 0,},
{.usec = 8, .pkts = 256, .comps = 0,},
{.usec = 64, .pkts = 256, .comps = 0,},
{.usec = 128, .pkts = 256, .comps = 0,},
{.usec = 256, .pkts = 256, .comps = 0,}
rx-cqe-profile: n/a
tx-eqe-profile: n/a
tx-cqe-profile: n/a
2. Tune
$ ethtool -C ethx rx-eqe-profile 1,1,0_2,2,0_3,3,0_4,4,0_5,5,0
$ ethtool -c ethx
...
rx-eqe-profile:
{.usec = 1, .pkts = 1, .comps = 0,},
{.usec = 2, .pkts = 2, .comps = 0,},
{.usec = 3, .pkts = 3, .comps = 0,},
{.usec = 4, .pkts = 4, .comps = 0,},
{.usec = 5, .pkts = 5, .comps = 0,}
rx-cqe-profile: n/a
tx-eqe-profile: n/a
tx-cqe-profile: n/a
3. Hint
If the device does not support some type of customized dim
profiles, the corresponding "n/a" will display.
Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
---
Documentation/netlink/specs/ethtool.yaml | 33 +++++
Documentation/networking/ethtool-netlink.rst | 8 ++
include/linux/dim.h | 7 ++
include/linux/ethtool.h | 16 ++-
include/uapi/linux/ethtool_netlink.h | 24 ++++
lib/dim/net_dim.c | 6 -
net/ethtool/coalesce.c | 172 ++++++++++++++++++++++++++-
7 files changed, 257 insertions(+), 9 deletions(-)
diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
index 87ae7b3..1a560ff 100644
--- a/Documentation/netlink/specs/ethtool.yaml
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -413,6 +413,18 @@ attribute-sets:
-
name: combined-count
type: u32
+ -
+ name: moderation
+ attributes:
+ -
+ name: usec
+ type: u16
+ -
+ name: pkts
+ type: u16
+ -
+ name: comps
+ type: u16
-
name: coalesce
@@ -502,6 +514,23 @@ attribute-sets:
-
name: tx-aggr-time-usecs
type: u32
+ -
+ name: rx-eqe-profile
+ type: nest
+ nested-attributes: moderation
+ -
+ name: rx-cqe-profile
+ type: nest
+ nested-attributes: moderation
+ -
+ name: tx-eqe-profile
+ type: nest
+ nested-attributes: moderation
+ -
+ name: tx-cqe-profile
+ type: nest
+ nested-attributes: moderation
+
-
name: pause-stat
attributes:
@@ -1313,6 +1342,10 @@ operations:
- tx-aggr-max-bytes
- tx-aggr-max-frames
- tx-aggr-time-usecs
+ - rx-eqe-profile
+ - rx-cqe-profile
+ - tx-eqe-profile
+ - tx-cqe-profile
dump: *coalesce-get-op
-
name: coalesce-set
diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index 5dc42f7..4d9eecf 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -1040,6 +1040,10 @@ Kernel response contents:
``ETHTOOL_A_COALESCE_TX_AGGR_MAX_BYTES`` u32 max aggr size, Tx
``ETHTOOL_A_COALESCE_TX_AGGR_MAX_FRAMES`` u32 max aggr packets, Tx
``ETHTOOL_A_COALESCE_TX_AGGR_TIME_USECS`` u32 time (us), aggr, Tx
+ ``ETHTOOL_A_COALESCE_RX_EQE_PROFILE`` nested profile of DIM EQE, Rx
+ ``ETHTOOL_A_COALESCE_RX_CQE_PROFILE`` nested profile of DIM CQE, Rx
+ ``ETHTOOL_A_COALESCE_TX_EQE_PROFILE`` nested profile of DIM EQE, Tx
+ ``ETHTOOL_A_COALESCE_TX_CQE_PROFILE`` nested profile of DIM CQE, Tx
=========================================== ====== =======================
Attributes are only included in reply if their value is not zero or the
@@ -1105,6 +1109,10 @@ Request contents:
``ETHTOOL_A_COALESCE_TX_AGGR_MAX_BYTES`` u32 max aggr size, Tx
``ETHTOOL_A_COALESCE_TX_AGGR_MAX_FRAMES`` u32 max aggr packets, Tx
``ETHTOOL_A_COALESCE_TX_AGGR_TIME_USECS`` u32 time (us), aggr, Tx
+ ``ETHTOOL_A_COALESCE_RX_EQE_PROFILE`` nested profile of DIM EQE, Rx
+ ``ETHTOOL_A_COALESCE_RX_CQE_PROFILE`` nested profile of DIM CQE, Rx
+ ``ETHTOOL_A_COALESCE_TX_EQE_PROFILE`` nested profile of DIM EQE, Tx
+ ``ETHTOOL_A_COALESCE_TX_CQE_PROFILE`` nested profile of DIM CQE, Tx
=========================================== ====== =======================
Request is rejected if it attributes declared as unsupported by driver (i.e.
diff --git a/include/linux/dim.h b/include/linux/dim.h
index f343bc9..43398f5 100644
--- a/include/linux/dim.h
+++ b/include/linux/dim.h
@@ -10,6 +10,13 @@
#include <linux/types.h>
#include <linux/workqueue.h>
+/* Number of DIM profiles and period mode. */
+#define NET_DIM_PARAMS_NUM_PROFILES 5
+#define NET_DIM_DEFAULT_RX_CQ_PKTS_FROM_EQE 256
+#define NET_DIM_DEFAULT_TX_CQ_PKTS_FROM_EQE 128
+#define NET_DIM_DEF_PROFILE_CQE 1
+#define NET_DIM_DEF_PROFILE_EQE 1
+
/*
* Number of events between DIM iterations.
* Causes a moderation of the algorithm run.
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 6fd9107..2da3ba7 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -18,6 +18,7 @@
#include <linux/if_ether.h>
#include <linux/netlink.h>
#include <uapi/linux/ethtool.h>
+#include <linux/dim.h>
struct compat_ethtool_rx_flow_spec {
u32 flow_type;
@@ -238,6 +239,10 @@ struct kernel_ethtool_coalesce {
u32 tx_aggr_max_bytes;
u32 tx_aggr_max_frames;
u32 tx_aggr_time_usecs;
+ struct dim_cq_moder rx_eqe_profile[NET_DIM_PARAMS_NUM_PROFILES];
+ struct dim_cq_moder rx_cqe_profile[NET_DIM_PARAMS_NUM_PROFILES];
+ struct dim_cq_moder tx_eqe_profile[NET_DIM_PARAMS_NUM_PROFILES];
+ struct dim_cq_moder tx_cqe_profile[NET_DIM_PARAMS_NUM_PROFILES];
};
/**
@@ -284,7 +289,11 @@ bool ethtool_convert_link_mode_to_legacy_u32(u32 *legacy_u32,
#define ETHTOOL_COALESCE_TX_AGGR_MAX_BYTES BIT(24)
#define ETHTOOL_COALESCE_TX_AGGR_MAX_FRAMES BIT(25)
#define ETHTOOL_COALESCE_TX_AGGR_TIME_USECS BIT(26)
-#define ETHTOOL_COALESCE_ALL_PARAMS GENMASK(26, 0)
+#define ETHTOOL_COALESCE_RX_EQE_PROFILE BIT(27)
+#define ETHTOOL_COALESCE_RX_CQE_PROFILE BIT(28)
+#define ETHTOOL_COALESCE_TX_EQE_PROFILE BIT(29)
+#define ETHTOOL_COALESCE_TX_CQE_PROFILE BIT(30)
+#define ETHTOOL_COALESCE_ALL_PARAMS GENMASK(30, 0)
#define ETHTOOL_COALESCE_USECS \
(ETHTOOL_COALESCE_RX_USECS | ETHTOOL_COALESCE_TX_USECS)
@@ -316,6 +325,11 @@ bool ethtool_convert_link_mode_to_legacy_u32(u32 *legacy_u32,
(ETHTOOL_COALESCE_TX_AGGR_MAX_BYTES | \
ETHTOOL_COALESCE_TX_AGGR_MAX_FRAMES | \
ETHTOOL_COALESCE_TX_AGGR_TIME_USECS)
+#define ETHTOOL_COALESCE_PROFILE \
+ (ETHTOOL_COALESCE_RX_EQE_PROFILE | \
+ ETHTOOL_COALESCE_RX_CQE_PROFILE | \
+ ETHTOOL_COALESCE_TX_EQE_PROFILE | \
+ ETHTOOL_COALESCE_TX_CQE_PROFILE)
#define ETHTOOL_STAT_NOT_SET (~0ULL)
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 23e225f..81c6d9e 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -416,12 +416,36 @@ enum {
ETHTOOL_A_COALESCE_TX_AGGR_MAX_BYTES, /* u32 */
ETHTOOL_A_COALESCE_TX_AGGR_MAX_FRAMES, /* u32 */
ETHTOOL_A_COALESCE_TX_AGGR_TIME_USECS, /* u32 */
+ ETHTOOL_A_COALESCE_RX_EQE_PROFILE, /* nest - _A_MODERATIONS_MODERATION */
+ ETHTOOL_A_COALESCE_RX_CQE_PROFILE, /* nest - _A_MODERATIONS_MODERATION */
+ ETHTOOL_A_COALESCE_TX_EQE_PROFILE, /* nest - _A_MODERATIONS_MODERATION */
+ ETHTOOL_A_COALESCE_TX_CQE_PROFILE, /* nest - _A_MODERATIONS_MODERATION */
/* add new constants above here */
__ETHTOOL_A_COALESCE_CNT,
ETHTOOL_A_COALESCE_MAX = (__ETHTOOL_A_COALESCE_CNT - 1)
};
+enum {
+ ETHTOOL_A_MODERATIONS_UNSPEC,
+ ETHTOOL_A_MODERATIONS_MODERATION, /* nest, _A_MODERATION_* */
+
+ /* add new constants above here */
+ __ETHTOOL_A_MODERATIONS_CNT,
+ ETHTOOL_A_MODERATIONS_MAX = (__ETHTOOL_A_MODERATIONS_CNT - 1)
+};
+
+enum {
+ ETHTOOL_A_MODERATION_UNSPEC,
+ ETHTOOL_A_MODERATION_USEC, /* u16 */
+ ETHTOOL_A_MODERATION_PKTS, /* u16 */
+ ETHTOOL_A_MODERATION_COMPS, /* u16 */
+
+ /* add new constants above here */
+ __ETHTOOL_A_MODERATION_CNT,
+ ETHTOOL_A_MODERATION_MAX = (__ETHTOOL_A_MODERATION_CNT - 1)
+};
+
/* PAUSE */
enum {
diff --git a/lib/dim/net_dim.c b/lib/dim/net_dim.c
index 4e32f7a..67d5beb 100644
--- a/lib/dim/net_dim.c
+++ b/lib/dim/net_dim.c
@@ -11,12 +11,6 @@
* There are different set of profiles for RX/TX CQs.
* Each profile size must be of NET_DIM_PARAMS_NUM_PROFILES
*/
-#define NET_DIM_PARAMS_NUM_PROFILES 5
-#define NET_DIM_DEFAULT_RX_CQ_PKTS_FROM_EQE 256
-#define NET_DIM_DEFAULT_TX_CQ_PKTS_FROM_EQE 128
-#define NET_DIM_DEF_PROFILE_CQE 1
-#define NET_DIM_DEF_PROFILE_EQE 1
-
#define NET_DIM_RX_EQE_PROFILES { \
{.usec = 1, .pkts = NET_DIM_DEFAULT_RX_CQ_PKTS_FROM_EQE,}, \
{.usec = 8, .pkts = NET_DIM_DEFAULT_RX_CQ_PKTS_FROM_EQE,}, \
diff --git a/net/ethtool/coalesce.c b/net/ethtool/coalesce.c
index 83112c1..e6a399a 100644
--- a/net/ethtool/coalesce.c
+++ b/net/ethtool/coalesce.c
@@ -51,6 +51,10 @@ static u32 attr_to_mask(unsigned int attr_type)
__CHECK_SUPPORTED_OFFSET(COALESCE_TX_USECS_HIGH);
__CHECK_SUPPORTED_OFFSET(COALESCE_TX_MAX_FRAMES_HIGH);
__CHECK_SUPPORTED_OFFSET(COALESCE_RATE_SAMPLE_INTERVAL);
+__CHECK_SUPPORTED_OFFSET(COALESCE_RX_EQE_PROFILE);
+__CHECK_SUPPORTED_OFFSET(COALESCE_RX_CQE_PROFILE);
+__CHECK_SUPPORTED_OFFSET(COALESCE_TX_EQE_PROFILE);
+__CHECK_SUPPORTED_OFFSET(COALESCE_TX_CQE_PROFILE);
const struct nla_policy ethnl_coalesce_get_policy[] = {
[ETHTOOL_A_COALESCE_HEADER] =
@@ -82,6 +86,13 @@ static int coalesce_prepare_data(const struct ethnl_req_info *req_base,
static int coalesce_reply_size(const struct ethnl_req_info *req_base,
const struct ethnl_reply_data *reply_base)
{
+ int modersz = nla_total_size(0) + /* _MODERATIONS_MODERATION, nest */
+ nla_total_size(sizeof(u16)) + /* _MODERATION_USEC */
+ nla_total_size(sizeof(u16)) + /* _MODERATION_PKTS */
+ nla_total_size(sizeof(u16)); /* _MODERATION_COMPS */
+ int total_modersz = nla_total_size(0) + /* _{R,T}X_{E,C}QE_PROFILE, nest */
+ modersz * NET_DIM_PARAMS_NUM_PROFILES;
+
return nla_total_size(sizeof(u32)) + /* _RX_USECS */
nla_total_size(sizeof(u32)) + /* _RX_MAX_FRAMES */
nla_total_size(sizeof(u32)) + /* _RX_USECS_IRQ */
@@ -108,7 +119,8 @@ static int coalesce_reply_size(const struct ethnl_req_info *req_base,
nla_total_size(sizeof(u8)) + /* _USE_CQE_MODE_RX */
nla_total_size(sizeof(u32)) + /* _TX_AGGR_MAX_BYTES */
nla_total_size(sizeof(u32)) + /* _TX_AGGR_MAX_FRAMES */
- nla_total_size(sizeof(u32)); /* _TX_AGGR_TIME_USECS */
+ nla_total_size(sizeof(u32)) + /* _TX_AGGR_TIME_USECS */
+ total_modersz * 4; /* _{R,T}X_{E,C}QE_PROFILE */
}
static bool coalesce_put_u32(struct sk_buff *skb, u16 attr_type, u32 val,
@@ -127,6 +139,61 @@ static bool coalesce_put_bool(struct sk_buff *skb, u16 attr_type, u32 val,
return nla_put_u8(skb, attr_type, !!val);
}
+/**
+ * coalesce_put_profile - fill reply with a nla nest with four child nla nests.
+ * @skb: socket buffer the message is stored in
+ * @attr_type: nest attr type ETHTOOL_A_COALESCE_*X_*QE_PROFILE
+ * @profile: data passed to userspace
+ * @supported_params: modifiable parameters supported by the driver
+ *
+ * Put a dim profile nest attribute. Refer to ETHTOOL_A_MODERATIONS_MODERATION.
+ */
+static bool coalesce_put_profile(struct sk_buff *skb, u16 attr_type,
+ const struct dim_cq_moder *profile,
+ u32 supported_params)
+{
+ struct nlattr *profile_attr, *moder_attr;
+ bool valid = false;
+ int i;
+
+ for (i = 0; i < NET_DIM_PARAMS_NUM_PROFILES; i++) {
+ if (profile[i].usec || profile[i].pkts || profile[i].comps) {
+ valid = true;
+ break;
+ }
+ }
+
+ if (!valid || !(supported_params & attr_to_mask(attr_type)))
+ return false;
+
+ profile_attr = nla_nest_start(skb, attr_type);
+ if (!profile_attr)
+ return -EMSGSIZE;
+
+ for (i = 0; i < NET_DIM_PARAMS_NUM_PROFILES; i++) {
+ moder_attr = nla_nest_start(skb, ETHTOOL_A_MODERATIONS_MODERATION);
+ if (!moder_attr)
+ goto nla_cancel_profile;
+
+ if (nla_put_u16(skb, ETHTOOL_A_MODERATION_USEC, profile[i].usec) ||
+ nla_put_u16(skb, ETHTOOL_A_MODERATION_PKTS, profile[i].pkts) ||
+ nla_put_u16(skb, ETHTOOL_A_MODERATION_COMPS, profile[i].comps))
+ goto nla_cancel_moder;
+
+ nla_nest_end(skb, moder_attr);
+ }
+
+ nla_nest_end(skb, profile_attr);
+
+ return 0;
+
+nla_cancel_moder:
+ nla_nest_cancel(skb, moder_attr);
+nla_cancel_profile:
+ nla_nest_cancel(skb, profile_attr);
+ return -EMSGSIZE;
+}
+
static int coalesce_fill_reply(struct sk_buff *skb,
const struct ethnl_req_info *req_base,
const struct ethnl_reply_data *reply_base)
@@ -189,7 +256,15 @@ static int coalesce_fill_reply(struct sk_buff *skb,
coalesce_put_u32(skb, ETHTOOL_A_COALESCE_TX_AGGR_MAX_FRAMES,
kcoal->tx_aggr_max_frames, supported) ||
coalesce_put_u32(skb, ETHTOOL_A_COALESCE_TX_AGGR_TIME_USECS,
- kcoal->tx_aggr_time_usecs, supported))
+ kcoal->tx_aggr_time_usecs, supported) ||
+ coalesce_put_profile(skb, ETHTOOL_A_COALESCE_RX_EQE_PROFILE,
+ kcoal->rx_eqe_profile, supported) ||
+ coalesce_put_profile(skb, ETHTOOL_A_COALESCE_RX_CQE_PROFILE,
+ kcoal->rx_cqe_profile, supported) ||
+ coalesce_put_profile(skb, ETHTOOL_A_COALESCE_TX_EQE_PROFILE,
+ kcoal->tx_eqe_profile, supported) ||
+ coalesce_put_profile(skb, ETHTOOL_A_COALESCE_TX_CQE_PROFILE,
+ kcoal->tx_cqe_profile, supported))
return -EMSGSIZE;
return 0;
@@ -227,6 +302,16 @@ static int coalesce_fill_reply(struct sk_buff *skb,
[ETHTOOL_A_COALESCE_TX_AGGR_MAX_BYTES] = { .type = NLA_U32 },
[ETHTOOL_A_COALESCE_TX_AGGR_MAX_FRAMES] = { .type = NLA_U32 },
[ETHTOOL_A_COALESCE_TX_AGGR_TIME_USECS] = { .type = NLA_U32 },
+ [ETHTOOL_A_COALESCE_RX_EQE_PROFILE] = { .type = NLA_NESTED },
+ [ETHTOOL_A_COALESCE_RX_CQE_PROFILE] = { .type = NLA_NESTED },
+ [ETHTOOL_A_COALESCE_TX_EQE_PROFILE] = { .type = NLA_NESTED },
+ [ETHTOOL_A_COALESCE_TX_CQE_PROFILE] = { .type = NLA_NESTED },
+};
+
+static const struct nla_policy coalesce_set_profile_policy[] = {
+ [ETHTOOL_A_MODERATION_USEC] = {.type = NLA_U16},
+ [ETHTOOL_A_MODERATION_PKTS] = {.type = NLA_U16},
+ [ETHTOOL_A_MODERATION_COMPS] = {.type = NLA_U16},
};
static int
@@ -253,6 +338,69 @@ static int coalesce_fill_reply(struct sk_buff *skb,
return 1;
}
+/**
+ * ethnl_update_profile - get a nla nest with four child nla nests from userspace.
+ * @dst: data get from the driver and modified by ethnl_update_profile.
+ * @nests: nest attr ETHTOOL_A_COALESCE_*X_*QE_PROFILE to set driver's profile.
+ * @mod: whether the data is modified
+ * @extack: Netlink extended ack
+ *
+ * Layout of nests:
+ * Nested ETHTOOL_A_COALESCE_*X_*QE_PROFILE attr
+ * Nested ETHTOOL_A_MODERATIONS_MODERATION attr
+ * ETHTOOL_A_MODERATION_USEC attr
+ * ETHTOOL_A_MODERATION_PKTS attr
+ * ETHTOOL_A_MODERATION_COMPS attr
+ * ...
+ * Nested ETHTOOL_A_MODERATIONS_MODERATION attr
+ * ETHTOOL_A_MODERATION_USEC attr
+ * ETHTOOL_A_MODERATION_PKTS attr
+ * ETHTOOL_A_MODERATION_COMPS attr
+ *
+ * Returns 0 on success or a negative error code.
+ */
+static inline int ethnl_update_profile(struct dim_cq_moder *dst,
+ const struct nlattr *nests,
+ bool *mod,
+ struct netlink_ext_ack *extack)
+{
+ struct nlattr *tb_moder[ARRAY_SIZE(coalesce_set_profile_policy)];
+ struct dim_cq_moder profile[NET_DIM_PARAMS_NUM_PROFILES];
+ struct nlattr *nest;
+ int ret, rem, i = 0;
+
+ if (!nests)
+ return 0;
+
+ nla_for_each_nested_type(nest, ETHTOOL_A_MODERATIONS_MODERATION, nests, rem) {
+ ret = nla_parse_nested(tb_moder,
+ ARRAY_SIZE(coalesce_set_profile_policy) - 1,
+ nest, coalesce_set_profile_policy,
+ extack);
+ if (ret)
+ return ret;
+
+ if (NL_REQ_ATTR_CHECK(extack, nest, tb_moder, ETHTOOL_A_MODERATION_USEC) ||
+ NL_REQ_ATTR_CHECK(extack, nest, tb_moder, ETHTOOL_A_MODERATION_PKTS) ||
+ NL_REQ_ATTR_CHECK(extack, nest, tb_moder, ETHTOOL_A_MODERATION_COMPS))
+ return -EINVAL;
+
+ profile[i].usec = nla_get_u16(tb_moder[ETHTOOL_A_MODERATION_USEC]);
+ profile[i].pkts = nla_get_u16(tb_moder[ETHTOOL_A_MODERATION_PKTS]);
+ profile[i].comps = nla_get_u16(tb_moder[ETHTOOL_A_MODERATION_COMPS]);
+
+ if (dst[i].usec != profile[i].usec || dst[i].pkts != profile[i].pkts ||
+ dst[i].comps != profile[i].comps)
+ *mod = true;
+
+ i++;
+ }
+
+ memcpy(dst, profile, sizeof(profile));
+
+ return 0;
+}
+
static int
__ethnl_set_coalesce(struct ethnl_req_info *req_info, struct genl_info *info,
bool *dual_change)
@@ -316,6 +464,26 @@ static int coalesce_fill_reply(struct sk_buff *skb,
tb[ETHTOOL_A_COALESCE_TX_AGGR_MAX_FRAMES], &mod);
ethnl_update_u32(&kernel_coalesce.tx_aggr_time_usecs,
tb[ETHTOOL_A_COALESCE_TX_AGGR_TIME_USECS], &mod);
+ ret = ethnl_update_profile(kernel_coalesce.rx_eqe_profile,
+ tb[ETHTOOL_A_COALESCE_RX_EQE_PROFILE],
+ &mod, info->extack);
+ if (ret < 0)
+ return ret;
+ ret = ethnl_update_profile(kernel_coalesce.rx_cqe_profile,
+ tb[ETHTOOL_A_COALESCE_RX_CQE_PROFILE],
+ &mod, info->extack);
+ if (ret < 0)
+ return ret;
+ ret = ethnl_update_profile(kernel_coalesce.tx_eqe_profile,
+ tb[ETHTOOL_A_COALESCE_TX_EQE_PROFILE],
+ &mod, info->extack);
+ if (ret < 0)
+ return ret;
+ ret = ethnl_update_profile(kernel_coalesce.tx_cqe_profile,
+ tb[ETHTOOL_A_COALESCE_TX_CQE_PROFILE],
+ &mod, info->extack);
+ if (ret < 0)
+ return ret;
/* Update operation modes */
ethnl_update_bool32(&coalesce.use_adaptive_rx_coalesce,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net-next v5 2/4] linux/dim: move profiles from .c to .h file
2024-04-09 12:03 [PATCH net-next v5 0/4] ethtool: provide the dim profile fine-tuning channel Heng Qi
2024-04-09 12:03 ` [PATCH net-next v5 1/4] ethtool: provide customized dim profile management Heng Qi
@ 2024-04-09 12:03 ` Heng Qi
2024-04-09 12:03 ` [PATCH net-next v5 3/4] virtio-net: refactor dim initialization/destruction Heng Qi
2024-04-09 12:03 ` [PATCH net-next v5 4/4] virtio-net: support dim profile fine-tuning Heng Qi
3 siblings, 0 replies; 11+ messages in thread
From: Heng Qi @ 2024-04-09 12:03 UTC (permalink / raw
To: netdev, virtualization
Cc: Jakub Kicinski, David S. Miller, Eric Dumazet, Paolo Abeni,
Jason Wang, Michael S. Tsirkin, Ratheesh Kannoth,
Alexander Lobakin, Xuan Zhuo
Place profiles into dim.h file so that the subsequent patch can use
it to initialize driver's custom profiles.
Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
---
include/linux/dim.h | 38 ++++++++++++++++++++++++++++++++++++++
lib/dim/net_dim.c | 38 --------------------------------------
2 files changed, 38 insertions(+), 38 deletions(-)
diff --git a/include/linux/dim.h b/include/linux/dim.h
index 43398f5..cf3a9d1 100644
--- a/include/linux/dim.h
+++ b/include/linux/dim.h
@@ -18,6 +18,44 @@
#define NET_DIM_DEF_PROFILE_EQE 1
/*
+ * Net DIM profiles:
+ * There are different set of profiles for each CQ period mode.
+ * There are different set of profiles for RX/TX CQs.
+ * Each profile size must be of NET_DIM_PARAMS_NUM_PROFILES
+ */
+#define NET_DIM_RX_EQE_PROFILES { \
+ {.usec = 1, .pkts = NET_DIM_DEFAULT_RX_CQ_PKTS_FROM_EQE,}, \
+ {.usec = 8, .pkts = NET_DIM_DEFAULT_RX_CQ_PKTS_FROM_EQE,}, \
+ {.usec = 64, .pkts = NET_DIM_DEFAULT_RX_CQ_PKTS_FROM_EQE,}, \
+ {.usec = 128, .pkts = NET_DIM_DEFAULT_RX_CQ_PKTS_FROM_EQE,}, \
+ {.usec = 256, .pkts = NET_DIM_DEFAULT_RX_CQ_PKTS_FROM_EQE,} \
+}
+
+#define NET_DIM_RX_CQE_PROFILES { \
+ {.usec = 2, .pkts = 256,}, \
+ {.usec = 8, .pkts = 128,}, \
+ {.usec = 16, .pkts = 64,}, \
+ {.usec = 32, .pkts = 64,}, \
+ {.usec = 64, .pkts = 64,} \
+}
+
+#define NET_DIM_TX_EQE_PROFILES { \
+ {.usec = 1, .pkts = NET_DIM_DEFAULT_TX_CQ_PKTS_FROM_EQE,}, \
+ {.usec = 8, .pkts = NET_DIM_DEFAULT_TX_CQ_PKTS_FROM_EQE,}, \
+ {.usec = 32, .pkts = NET_DIM_DEFAULT_TX_CQ_PKTS_FROM_EQE,}, \
+ {.usec = 64, .pkts = NET_DIM_DEFAULT_TX_CQ_PKTS_FROM_EQE,}, \
+ {.usec = 128, .pkts = NET_DIM_DEFAULT_TX_CQ_PKTS_FROM_EQE,} \
+}
+
+#define NET_DIM_TX_CQE_PROFILES { \
+ {.usec = 5, .pkts = 128,}, \
+ {.usec = 8, .pkts = 64,}, \
+ {.usec = 16, .pkts = 32,}, \
+ {.usec = 32, .pkts = 32,}, \
+ {.usec = 64, .pkts = 32,} \
+}
+
+/*
* Number of events between DIM iterations.
* Causes a moderation of the algorithm run.
*/
diff --git a/lib/dim/net_dim.c b/lib/dim/net_dim.c
index 67d5beb..c7cf457 100644
--- a/lib/dim/net_dim.c
+++ b/lib/dim/net_dim.c
@@ -5,44 +5,6 @@
#include <linux/dim.h>
-/*
- * Net DIM profiles:
- * There are different set of profiles for each CQ period mode.
- * There are different set of profiles for RX/TX CQs.
- * Each profile size must be of NET_DIM_PARAMS_NUM_PROFILES
- */
-#define NET_DIM_RX_EQE_PROFILES { \
- {.usec = 1, .pkts = NET_DIM_DEFAULT_RX_CQ_PKTS_FROM_EQE,}, \
- {.usec = 8, .pkts = NET_DIM_DEFAULT_RX_CQ_PKTS_FROM_EQE,}, \
- {.usec = 64, .pkts = NET_DIM_DEFAULT_RX_CQ_PKTS_FROM_EQE,}, \
- {.usec = 128, .pkts = NET_DIM_DEFAULT_RX_CQ_PKTS_FROM_EQE,}, \
- {.usec = 256, .pkts = NET_DIM_DEFAULT_RX_CQ_PKTS_FROM_EQE,} \
-}
-
-#define NET_DIM_RX_CQE_PROFILES { \
- {.usec = 2, .pkts = 256,}, \
- {.usec = 8, .pkts = 128,}, \
- {.usec = 16, .pkts = 64,}, \
- {.usec = 32, .pkts = 64,}, \
- {.usec = 64, .pkts = 64,} \
-}
-
-#define NET_DIM_TX_EQE_PROFILES { \
- {.usec = 1, .pkts = NET_DIM_DEFAULT_TX_CQ_PKTS_FROM_EQE,}, \
- {.usec = 8, .pkts = NET_DIM_DEFAULT_TX_CQ_PKTS_FROM_EQE,}, \
- {.usec = 32, .pkts = NET_DIM_DEFAULT_TX_CQ_PKTS_FROM_EQE,}, \
- {.usec = 64, .pkts = NET_DIM_DEFAULT_TX_CQ_PKTS_FROM_EQE,}, \
- {.usec = 128, .pkts = NET_DIM_DEFAULT_TX_CQ_PKTS_FROM_EQE,} \
-}
-
-#define NET_DIM_TX_CQE_PROFILES { \
- {.usec = 5, .pkts = 128,}, \
- {.usec = 8, .pkts = 64,}, \
- {.usec = 16, .pkts = 32,}, \
- {.usec = 32, .pkts = 32,}, \
- {.usec = 64, .pkts = 32,} \
-}
-
static const struct dim_cq_moder
rx_profile[DIM_CQ_PERIOD_NUM_MODES][NET_DIM_PARAMS_NUM_PROFILES] = {
NET_DIM_RX_EQE_PROFILES,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net-next v5 3/4] virtio-net: refactor dim initialization/destruction
2024-04-09 12:03 [PATCH net-next v5 0/4] ethtool: provide the dim profile fine-tuning channel Heng Qi
2024-04-09 12:03 ` [PATCH net-next v5 1/4] ethtool: provide customized dim profile management Heng Qi
2024-04-09 12:03 ` [PATCH net-next v5 2/4] linux/dim: move profiles from .c to .h file Heng Qi
@ 2024-04-09 12:03 ` Heng Qi
2024-04-09 12:03 ` [PATCH net-next v5 4/4] virtio-net: support dim profile fine-tuning Heng Qi
3 siblings, 0 replies; 11+ messages in thread
From: Heng Qi @ 2024-04-09 12:03 UTC (permalink / raw
To: netdev, virtualization
Cc: Jakub Kicinski, David S. Miller, Eric Dumazet, Paolo Abeni,
Jason Wang, Michael S. Tsirkin, Ratheesh Kannoth,
Alexander Lobakin, Xuan Zhuo
Extract the initialization and destruction actions
of dim for use in the next patch.
Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
---
drivers/net/virtio_net.c | 37 ++++++++++++++++++++++++++-----------
1 file changed, 26 insertions(+), 11 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index c22d111..a64656e 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2274,6 +2274,13 @@ static int virtnet_enable_queue_pair(struct virtnet_info *vi, int qp_index)
return err;
}
+static void virtnet_dim_clean(struct virtnet_info *vi,
+ int start_qnum, int end_qnum)
+{
+ for (; start_qnum <= end_qnum; start_qnum++)
+ cancel_work_sync(&vi->rq[start_qnum].dim.work);
+}
+
static int virtnet_open(struct net_device *dev)
{
struct virtnet_info *vi = netdev_priv(dev);
@@ -2297,11 +2304,9 @@ static int virtnet_open(struct net_device *dev)
err_enable_qp:
disable_delayed_refill(vi);
cancel_delayed_work_sync(&vi->refill);
-
- for (i--; i >= 0; i--) {
+ virtnet_dim_clean(vi, 0, i - 1);
+ for (i--; i >= 0; i--)
virtnet_disable_queue_pair(vi, i);
- cancel_work_sync(&vi->rq[i].dim.work);
- }
return err;
}
@@ -2466,7 +2471,7 @@ static int virtnet_rx_resize(struct virtnet_info *vi,
if (running) {
napi_disable(&rq->napi);
- cancel_work_sync(&rq->dim.work);
+ virtnet_dim_clean(vi, qindex, qindex);
}
err = virtqueue_resize(rq->vq, ring_num, virtnet_rq_unmap_free_buf);
@@ -2716,10 +2721,9 @@ static int virtnet_close(struct net_device *dev)
/* Make sure refill_work doesn't re-enable napi! */
cancel_delayed_work_sync(&vi->refill);
- for (i = 0; i < vi->max_queue_pairs; i++) {
+ virtnet_dim_clean(vi, 0, vi->max_queue_pairs - 1);
+ for (i = 0; i < vi->max_queue_pairs; i++)
virtnet_disable_queue_pair(vi, i);
- cancel_work_sync(&vi->rq[i].dim.work);
- }
return 0;
}
@@ -4418,6 +4422,19 @@ static int virtnet_find_vqs(struct virtnet_info *vi)
return ret;
}
+static void virtnet_dim_init(struct virtnet_info *vi)
+{
+ int i;
+
+ if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_VQ_NOTF_COAL))
+ return;
+
+ for (i = 0; i < vi->max_queue_pairs; i++) {
+ INIT_WORK(&vi->rq[i].dim.work, virtnet_rx_dim_work);
+ vi->rq[i].dim.mode = DIM_CQ_PERIOD_MODE_START_FROM_EQE;
+ }
+}
+
static int virtnet_alloc_queues(struct virtnet_info *vi)
{
int i;
@@ -4437,6 +4454,7 @@ static int virtnet_alloc_queues(struct virtnet_info *vi)
goto err_rq;
INIT_DELAYED_WORK(&vi->refill, refill_work);
+ virtnet_dim_init(vi);
for (i = 0; i < vi->max_queue_pairs; i++) {
vi->rq[i].pages = NULL;
netif_napi_add_weight(vi->dev, &vi->rq[i].napi, virtnet_poll,
@@ -4445,9 +4463,6 @@ static int virtnet_alloc_queues(struct virtnet_info *vi)
virtnet_poll_tx,
napi_tx ? napi_weight : 0);
- INIT_WORK(&vi->rq[i].dim.work, virtnet_rx_dim_work);
- vi->rq[i].dim.mode = DIM_CQ_PERIOD_MODE_START_FROM_EQE;
-
sg_init_table(vi->rq[i].sg, ARRAY_SIZE(vi->rq[i].sg));
ewma_pkt_len_init(&vi->rq[i].mrg_avg_pkt_len);
sg_init_table(vi->sq[i].sg, ARRAY_SIZE(vi->sq[i].sg));
--
1.8.3.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net-next v5 4/4] virtio-net: support dim profile fine-tuning
2024-04-09 12:03 [PATCH net-next v5 0/4] ethtool: provide the dim profile fine-tuning channel Heng Qi
` (2 preceding siblings ...)
2024-04-09 12:03 ` [PATCH net-next v5 3/4] virtio-net: refactor dim initialization/destruction Heng Qi
@ 2024-04-09 12:03 ` Heng Qi
2024-04-10 1:40 ` Jakub Kicinski
3 siblings, 1 reply; 11+ messages in thread
From: Heng Qi @ 2024-04-09 12:03 UTC (permalink / raw
To: netdev, virtualization
Cc: Jakub Kicinski, David S. Miller, Eric Dumazet, Paolo Abeni,
Jason Wang, Michael S. Tsirkin, Ratheesh Kannoth,
Alexander Lobakin, Xuan Zhuo
Virtio-net has different types of back-end device
implementations. In order to effectively optimize
the dim library's gains for different device
implementations, let's use the new interface params
to fine-tune the profile list.
Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
---
drivers/net/virtio_net.c | 41 +++++++++++++++++++++++++++++++++++++++--
1 file changed, 39 insertions(+), 2 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index a64656e..03840e9 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -57,6 +57,8 @@
#define VIRTNET_DRIVER_VERSION "1.0.0"
+static const struct dim_cq_moder rx_eqe_conf[] = NET_DIM_RX_EQE_PROFILES;
+
static const unsigned long guest_offloads[] = {
VIRTIO_NET_F_GUEST_TSO4,
VIRTIO_NET_F_GUEST_TSO6,
@@ -335,6 +337,9 @@ struct virtnet_info {
struct virtnet_interrupt_coalesce intr_coal_tx;
struct virtnet_interrupt_coalesce intr_coal_rx;
+ /* DIM profile list */
+ struct dim_cq_moder rx_eqe_conf[NET_DIM_PARAMS_NUM_PROFILES];
+
unsigned long guest_offloads;
unsigned long guest_offloads_capable;
@@ -3584,7 +3589,7 @@ static void virtnet_rx_dim_work(struct work_struct *work)
if (!rq->dim_enabled)
continue;
- update_moder = net_dim_get_rx_moderation(dim->mode, dim->profile_ix);
+ update_moder = vi->rx_eqe_conf[dim->profile_ix];
if (update_moder.usec != rq->intr_coal.max_usecs ||
update_moder.pkts != rq->intr_coal.max_packets) {
err = virtnet_send_rx_ctrl_coal_vq_cmd(vi, qnum,
@@ -3627,6 +3632,27 @@ static int virtnet_should_update_vq_weight(int dev_flags, int weight,
return 0;
}
+static int virtnet_update_profile(struct virtnet_info *vi,
+ struct kernel_ethtool_coalesce *kc)
+{
+ int i;
+
+ if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_VQ_NOTF_COAL)) {
+ for (i = 0; i < NET_DIM_PARAMS_NUM_PROFILES; i++)
+ if (kc->rx_eqe_profile[i].comps)
+ return -EINVAL;
+ } else {
+ return -EOPNOTSUPP;
+ }
+
+ for (i = 0; i < NET_DIM_PARAMS_NUM_PROFILES; i++) {
+ vi->rx_eqe_conf[i].usec = kc->rx_eqe_profile[i].usec;
+ vi->rx_eqe_conf[i].pkts = kc->rx_eqe_profile[i].pkts;
+ }
+
+ return 0;
+}
+
static int virtnet_set_coalesce(struct net_device *dev,
struct ethtool_coalesce *ec,
struct kernel_ethtool_coalesce *kernel_coal,
@@ -3653,6 +3679,10 @@ static int virtnet_set_coalesce(struct net_device *dev,
}
}
+ ret = virtnet_update_profile(vi, kernel_coal);
+ if (ret)
+ return ret;
+
if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_NOTF_COAL))
ret = virtnet_send_notf_coal_cmds(vi, ec);
else
@@ -3689,6 +3719,10 @@ static int virtnet_get_coalesce(struct net_device *dev,
ec->tx_max_coalesced_frames = 1;
}
+ if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_VQ_NOTF_COAL))
+ memcpy(kernel_coal->rx_eqe_profile, vi->rx_eqe_conf,
+ sizeof(vi->rx_eqe_conf));
+
return 0;
}
@@ -3868,7 +3902,8 @@ static int virtnet_set_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info)
static const struct ethtool_ops virtnet_ethtool_ops = {
.supported_coalesce_params = ETHTOOL_COALESCE_MAX_FRAMES |
- ETHTOOL_COALESCE_USECS | ETHTOOL_COALESCE_USE_ADAPTIVE_RX,
+ ETHTOOL_COALESCE_USECS | ETHTOOL_COALESCE_USE_ADAPTIVE_RX |
+ ETHTOOL_COALESCE_RX_EQE_PROFILE,
.get_drvinfo = virtnet_get_drvinfo,
.get_link = ethtool_op_get_link,
.get_ringparam = virtnet_get_ringparam,
@@ -4433,6 +4468,8 @@ static void virtnet_dim_init(struct virtnet_info *vi)
INIT_WORK(&vi->rq[i].dim.work, virtnet_rx_dim_work);
vi->rq[i].dim.mode = DIM_CQ_PERIOD_MODE_START_FROM_EQE;
}
+
+ memcpy(vi->rx_eqe_conf, rx_eqe_conf, sizeof(vi->rx_eqe_conf));
}
static int virtnet_alloc_queues(struct virtnet_info *vi)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v5 4/4] virtio-net: support dim profile fine-tuning
2024-04-09 12:03 ` [PATCH net-next v5 4/4] virtio-net: support dim profile fine-tuning Heng Qi
@ 2024-04-10 1:40 ` Jakub Kicinski
2024-04-10 3:09 ` Heng Qi
0 siblings, 1 reply; 11+ messages in thread
From: Jakub Kicinski @ 2024-04-10 1:40 UTC (permalink / raw
To: Heng Qi
Cc: netdev, virtualization, David S. Miller, Eric Dumazet,
Paolo Abeni, Jason Wang, Michael S. Tsirkin, Ratheesh Kannoth,
Alexander Lobakin, Xuan Zhuo
On Tue, 9 Apr 2024 20:03:24 +0800 Heng Qi wrote:
> + /* DIM profile list */
> + struct dim_cq_moder rx_eqe_conf[NET_DIM_PARAMS_NUM_PROFILES];
Can you please wrap this into a structure with other necessary
information and add a pointer in struct net_device instead.
What's the point of every single driver implementing the same
boilerplate memcpy() in its get_coalesce / set_coalesce callbacks?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v5 1/4] ethtool: provide customized dim profile management
2024-04-09 12:03 ` [PATCH net-next v5 1/4] ethtool: provide customized dim profile management Heng Qi
@ 2024-04-10 1:44 ` Jakub Kicinski
2024-04-10 3:13 ` Heng Qi
0 siblings, 1 reply; 11+ messages in thread
From: Jakub Kicinski @ 2024-04-10 1:44 UTC (permalink / raw
To: Heng Qi
Cc: netdev, virtualization, David S. Miller, Eric Dumazet,
Paolo Abeni, Jason Wang, Michael S. Tsirkin, Ratheesh Kannoth,
Alexander Lobakin, Xuan Zhuo
On Tue, 9 Apr 2024 20:03:21 +0800 Heng Qi wrote:
> +/**
> + * coalesce_put_profile - fill reply with a nla nest with four child nla nests.
> + * @skb: socket buffer the message is stored in
> + * @attr_type: nest attr type ETHTOOL_A_COALESCE_*X_*QE_PROFILE
> + * @profile: data passed to userspace
> + * @supported_params: modifiable parameters supported by the driver
> + *
> + * Put a dim profile nest attribute. Refer to ETHTOOL_A_MODERATIONS_MODERATION.
unfortunately kdoc got more picky and it also wants us to document
return values now, you gotta add something like
* Returns: true if ..
actually this functions seems to return negative error codes as bool..
> +static bool coalesce_put_profile(struct sk_buff *skb, u16 attr_type,
> + const struct dim_cq_moder *profile,
> + u32 supported_params)
> +{
> + struct nlattr *profile_attr, *moder_attr;
> + bool valid = false;
> + int i;
> +
> + for (i = 0; i < NET_DIM_PARAMS_NUM_PROFILES; i++) {
> + if (profile[i].usec || profile[i].pkts || profile[i].comps) {
> + valid = true;
> + break;
> + }
> + }
> +
> + if (!valid || !(supported_params & attr_to_mask(attr_type)))
> + return false;
> +
> + profile_attr = nla_nest_start(skb, attr_type);
> + if (!profile_attr)
> + return -EMSGSIZE;
> +
> + for (i = 0; i < NET_DIM_PARAMS_NUM_PROFILES; i++) {
> + moder_attr = nla_nest_start(skb, ETHTOOL_A_MODERATIONS_MODERATION);
> + if (!moder_attr)
> + goto nla_cancel_profile;
> +
> + if (nla_put_u16(skb, ETHTOOL_A_MODERATION_USEC, profile[i].usec) ||
> + nla_put_u16(skb, ETHTOOL_A_MODERATION_PKTS, profile[i].pkts) ||
> + nla_put_u16(skb, ETHTOOL_A_MODERATION_COMPS, profile[i].comps))
u16 in netlink is almost always the wrong choice, sizes are rounded
up to 4B, anyway, let's use u32 at netlink level.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v5 4/4] virtio-net: support dim profile fine-tuning
2024-04-10 1:40 ` Jakub Kicinski
@ 2024-04-10 3:09 ` Heng Qi
2024-04-11 1:54 ` Jakub Kicinski
0 siblings, 1 reply; 11+ messages in thread
From: Heng Qi @ 2024-04-10 3:09 UTC (permalink / raw
To: Jakub Kicinski
Cc: netdev, virtualization, David S. Miller, Eric Dumazet,
Paolo Abeni, Jason Wang, Michael S. Tsirkin, Ratheesh Kannoth,
Alexander Lobakin, Xuan Zhuo
在 2024/4/10 上午9:40, Jakub Kicinski 写道:
> On Tue, 9 Apr 2024 20:03:24 +0800 Heng Qi wrote:
>> + /* DIM profile list */
>> + struct dim_cq_moder rx_eqe_conf[NET_DIM_PARAMS_NUM_PROFILES];
> Can you please wrap this into a structure with other necessary
> information and add a pointer in struct net_device instead.
>
> What's the point of every single driver implementing the same
> boilerplate memcpy() in its get_coalesce / set_coalesce callbacks?
The point is that the driver may check whether the user has set
parameters that it does not want.
For example, virtio may not want the modification of comps, and ice/idpf
may not want the modification
of comps and pkts.
But you inspired me to think from another perspective. If parameters are
placed in netdevice, the
goal of user modification is to modify the profile of netdevice, and the
driver can obtain its own
target parameters from it as needed. Do you like this?
In addition, if the netdevice way is preferred, I would like to confirm
whether we still continue the
"ethtool -C" way, that is, complete the modification of netdevice
profile in __ethnl_set_coalesce()?
Thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v5 1/4] ethtool: provide customized dim profile management
2024-04-10 1:44 ` Jakub Kicinski
@ 2024-04-10 3:13 ` Heng Qi
2024-04-11 13:58 ` Simon Horman
0 siblings, 1 reply; 11+ messages in thread
From: Heng Qi @ 2024-04-10 3:13 UTC (permalink / raw
To: Jakub Kicinski
Cc: netdev, virtualization, David S. Miller, Eric Dumazet,
Paolo Abeni, Jason Wang, Michael S. Tsirkin, Ratheesh Kannoth,
Alexander Lobakin, Xuan Zhuo
在 2024/4/10 上午9:44, Jakub Kicinski 写道:
> On Tue, 9 Apr 2024 20:03:21 +0800 Heng Qi wrote:
>> +/**
>> + * coalesce_put_profile - fill reply with a nla nest with four child nla nests.
>> + * @skb: socket buffer the message is stored in
>> + * @attr_type: nest attr type ETHTOOL_A_COALESCE_*X_*QE_PROFILE
>> + * @profile: data passed to userspace
>> + * @supported_params: modifiable parameters supported by the driver
>> + *
>> + * Put a dim profile nest attribute. Refer to ETHTOOL_A_MODERATIONS_MODERATION.
> unfortunately kdoc got more picky and it also wants us to document
> return values now,
Will add it.
> you gotta add something like
>
> * Returns: true if ..
>
> actually this functions seems to return negative error codes as bool..
This works, in its wrapper function its error will be passed.:)
>
>> +static bool coalesce_put_profile(struct sk_buff *skb, u16 attr_type,
>> + const struct dim_cq_moder *profile,
>> + u32 supported_params)
>> +{
>> + struct nlattr *profile_attr, *moder_attr;
>> + bool valid = false;
>> + int i;
>> +
>> + for (i = 0; i < NET_DIM_PARAMS_NUM_PROFILES; i++) {
>> + if (profile[i].usec || profile[i].pkts || profile[i].comps) {
>> + valid = true;
>> + break;
>> + }
>> + }
>> +
>> + if (!valid || !(supported_params & attr_to_mask(attr_type)))
>> + return false;
>> +
>> + profile_attr = nla_nest_start(skb, attr_type);
>> + if (!profile_attr)
>> + return -EMSGSIZE;
>> +
>> + for (i = 0; i < NET_DIM_PARAMS_NUM_PROFILES; i++) {
>> + moder_attr = nla_nest_start(skb, ETHTOOL_A_MODERATIONS_MODERATION);
>> + if (!moder_attr)
>> + goto nla_cancel_profile;
>> +
>> + if (nla_put_u16(skb, ETHTOOL_A_MODERATION_USEC, profile[i].usec) ||
>> + nla_put_u16(skb, ETHTOOL_A_MODERATION_PKTS, profile[i].pkts) ||
>> + nla_put_u16(skb, ETHTOOL_A_MODERATION_COMPS, profile[i].comps))
> u16 in netlink is almost always the wrong choice, sizes are rounded
> up to 4B, anyway, let's use u32 at netlink level.
OK.
Thanks for your comments.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v5 4/4] virtio-net: support dim profile fine-tuning
2024-04-10 3:09 ` Heng Qi
@ 2024-04-11 1:54 ` Jakub Kicinski
0 siblings, 0 replies; 11+ messages in thread
From: Jakub Kicinski @ 2024-04-11 1:54 UTC (permalink / raw
To: Heng Qi
Cc: netdev, virtualization, David S. Miller, Eric Dumazet,
Paolo Abeni, Jason Wang, Michael S. Tsirkin, Ratheesh Kannoth,
Alexander Lobakin, Xuan Zhuo
On Wed, 10 Apr 2024 11:09:16 +0800 Heng Qi wrote:
> The point is that the driver may check whether the user has set
> parameters that it does not want.
> For example, virtio may not want the modification of comps, and ice/idpf
> may not want the modification
> of comps and pkts.
If it's simply about the fields, not the range of values, flags of
what's supported would suffice. If we need more complicated checks
you can treat the driver callback as a validation callback, and
when driver returns success - copy in the core.
> But you inspired me to think from another perspective. If parameters are
> placed in netdevice, the
> goal of user modification is to modify the profile of netdevice, and the
> driver can obtain its own
> target parameters from it as needed. Do you like this?
Yes, IIUC.
> In addition, if the netdevice way is preferred, I would like to confirm
> whether we still continue the
> "ethtool -C" way, that is, complete the modification of netdevice
> profile in __ethnl_set_coalesce()?
That'd be great. If the driver validation is complex we can keep some
form of the SET callback. But we definitely don't need to extend the
GET callback since core will have the values, and can return them to
the user.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v5 1/4] ethtool: provide customized dim profile management
2024-04-10 3:13 ` Heng Qi
@ 2024-04-11 13:58 ` Simon Horman
0 siblings, 0 replies; 11+ messages in thread
From: Simon Horman @ 2024-04-11 13:58 UTC (permalink / raw
To: Heng Qi
Cc: Jakub Kicinski, netdev, virtualization, David S. Miller,
Eric Dumazet, Paolo Abeni, Jason Wang, Michael S. Tsirkin,
Ratheesh Kannoth, Alexander Lobakin, Xuan Zhuo
On Wed, Apr 10, 2024 at 11:13:15AM +0800, Heng Qi wrote:
>
>
> 在 2024/4/10 上午9:44, Jakub Kicinski 写道:
> > On Tue, 9 Apr 2024 20:03:21 +0800 Heng Qi wrote:
> > > +/**
> > > + * coalesce_put_profile - fill reply with a nla nest with four child nla nests.
> > > + * @skb: socket buffer the message is stored in
> > > + * @attr_type: nest attr type ETHTOOL_A_COALESCE_*X_*QE_PROFILE
> > > + * @profile: data passed to userspace
> > > + * @supported_params: modifiable parameters supported by the driver
> > > + *
> > > + * Put a dim profile nest attribute. Refer to ETHTOOL_A_MODERATIONS_MODERATION.
> > unfortunately kdoc got more picky and it also wants us to document
> > return values now,
>
> Will add it.
>
> > you gotta add something like
> >
> > * Returns: true if ..
> >
> > actually this functions seems to return negative error codes as bool..
>
> This works, in its wrapper function its error will be passed.:)
Ok, but I think that in that case the function should return false on error.
...
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-04-11 13:58 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-09 12:03 [PATCH net-next v5 0/4] ethtool: provide the dim profile fine-tuning channel Heng Qi
2024-04-09 12:03 ` [PATCH net-next v5 1/4] ethtool: provide customized dim profile management Heng Qi
2024-04-10 1:44 ` Jakub Kicinski
2024-04-10 3:13 ` Heng Qi
2024-04-11 13:58 ` Simon Horman
2024-04-09 12:03 ` [PATCH net-next v5 2/4] linux/dim: move profiles from .c to .h file Heng Qi
2024-04-09 12:03 ` [PATCH net-next v5 3/4] virtio-net: refactor dim initialization/destruction Heng Qi
2024-04-09 12:03 ` [PATCH net-next v5 4/4] virtio-net: support dim profile fine-tuning Heng Qi
2024-04-10 1:40 ` Jakub Kicinski
2024-04-10 3:09 ` Heng Qi
2024-04-11 1:54 ` Jakub Kicinski
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.