netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v6 0/4] ethtool: provide the dim profile fine-tuning channel
@ 2024-04-11 14:12 Heng Qi
  2024-04-11 14:12 ` [PATCH net-next v6 1/4] linux/dim: move useful macros to .h file Heng Qi
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Heng Qi @ 2024-04-11 14:12 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".

Since the profile now exists in netdevice, adding a function similar
to net_dim_get_rx_moderation_dev() with netdevice as argument is
nice, but this would be better along with cleaning up the rest of
the drivers, which we can get to very soon after this set.

Please review, thank you very much!

Changelog
=====
v5->v6:
  - Place the profile in netdevice to bypass the driver.
    The interaction code of ethtool <-> kernel has not changed at all,
    only the interaction part of kernel <-> driver has changed.

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):
  linux/dim: move useful macros to .h file
  ethtool: provide customized dim profile management
  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                     |  45 +++++--
 include/linux/dim.h                          |  13 ++
 include/linux/ethtool.h                      |  12 +-
 include/linux/netdevice.h                    |  15 +++
 include/uapi/linux/ethtool_netlink.h         |  24 ++++
 lib/dim/net_dim.c                            |  10 +-
 net/core/dev.c                               |  63 +++++++++
 net/ethtool/coalesce.c                       | 184 ++++++++++++++++++++++++++-
 10 files changed, 383 insertions(+), 24 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH net-next v6 1/4] linux/dim: move useful macros to .h file
  2024-04-11 14:12 [PATCH net-next v6 0/4] ethtool: provide the dim profile fine-tuning channel Heng Qi
@ 2024-04-11 14:12 ` Heng Qi
  2024-04-11 14:12 ` [PATCH net-next v6 2/4] ethtool: provide customized dim profile management Heng Qi
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 17+ messages in thread
From: Heng Qi @ 2024-04-11 14:12 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

These will be used in subsequent patches, including
newly declared profile arrays.

Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
---
 include/linux/dim.h | 13 +++++++++++++
 lib/dim/net_dim.c   | 10 ++--------
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/include/linux/dim.h b/include/linux/dim.h
index f343bc9..8149d2d 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.
@@ -127,6 +134,12 @@ enum dim_cq_period_mode {
 	DIM_CQ_PERIOD_NUM_MODES
 };
 
+extern const struct dim_cq_moder
+rx_profile[DIM_CQ_PERIOD_NUM_MODES][NET_DIM_PARAMS_NUM_PROFILES];
+
+extern const struct dim_cq_moder
+tx_profile[DIM_CQ_PERIOD_NUM_MODES][NET_DIM_PARAMS_NUM_PROFILES];
+
 /**
  * enum dim_state - DIM algorithm states
  *
diff --git a/lib/dim/net_dim.c b/lib/dim/net_dim.c
index 4e32f7a..a649d90 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,}, \
@@ -49,13 +43,13 @@
 	{.usec = 64, .pkts = 32,}   \
 }
 
-static const struct dim_cq_moder
+const struct dim_cq_moder
 rx_profile[DIM_CQ_PERIOD_NUM_MODES][NET_DIM_PARAMS_NUM_PROFILES] = {
 	NET_DIM_RX_EQE_PROFILES,
 	NET_DIM_RX_CQE_PROFILES,
 };
 
-static const struct dim_cq_moder
+const struct dim_cq_moder
 tx_profile[DIM_CQ_PERIOD_NUM_MODES][NET_DIM_PARAMS_NUM_PROFILES] = {
 	NET_DIM_TX_EQE_PROFILES,
 	NET_DIM_TX_CQE_PROFILES,
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next v6 2/4] ethtool: provide customized dim profile management
  2024-04-11 14:12 [PATCH net-next v6 0/4] ethtool: provide the dim profile fine-tuning channel Heng Qi
  2024-04-11 14:12 ` [PATCH net-next v6 1/4] linux/dim: move useful macros to .h file Heng Qi
@ 2024-04-11 14:12 ` Heng Qi
  2024-04-11 15:19   ` Brett Creeley
                     ` (5 more replies)
  2024-04-11 14:12 ` [PATCH net-next v6 3/4] virtio-net: refactor dim initialization/destruction Heng Qi
  2024-04-11 14:12 ` [PATCH net-next v6 4/4] virtio-net: support dim profile fine-tuning Heng Qi
  3 siblings, 6 replies; 17+ messages in thread
From: Heng Qi @ 2024-04-11 14:12 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/ethtool.h                      |  12 +-
 include/linux/netdevice.h                    |  15 +++
 include/uapi/linux/ethtool_netlink.h         |  24 ++++
 net/core/dev.c                               |  63 +++++++++
 net/ethtool/coalesce.c                       | 184 ++++++++++++++++++++++++++-
 7 files changed, 336 insertions(+), 3 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/ethtool.h b/include/linux/ethtool.h
index 6fd9107..1dcfbe5 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;
@@ -284,7 +285,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 +321,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/linux/netdevice.h b/include/linux/netdevice.h
index d45f330..d2f499a 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -48,6 +48,7 @@
 #include <uapi/linux/netdev.h>
 #include <linux/hashtable.h>
 #include <linux/rbtree.h>
+#include <linux/dim.h>
 #include <net/net_trackers.h>
 #include <net/net_debug.h>
 #include <net/dropreason-core.h>
@@ -1649,6 +1650,9 @@ struct net_device_ops {
  * @IFF_SEE_ALL_HWTSTAMP_REQUESTS: device wants to see calls to
  *	ndo_hwtstamp_set() for all timestamp requests regardless of source,
  *	even if those aren't HWTSTAMP_SOURCE_NETDEV.
+ * @IFF_PROFILE_USEC: device supports adjusting the DIM profile's usec field
+ * @IFF_PROFILE_PKTS: device supports adjusting the DIM profile's pkts field
+ * @IFF_PROFILE_COMPS: device supports adjusting the DIM profile's comps field
  */
 enum netdev_priv_flags {
 	IFF_802_1Q_VLAN			= 1<<0,
@@ -1685,6 +1689,9 @@ enum netdev_priv_flags {
 	IFF_TX_SKB_NO_LINEAR		= BIT_ULL(31),
 	IFF_CHANGE_PROTO_DOWN		= BIT_ULL(32),
 	IFF_SEE_ALL_HWTSTAMP_REQUESTS	= BIT_ULL(33),
+	IFF_PROFILE_USEC		= BIT_ULL(34),
+	IFF_PROFILE_PKTS		= BIT_ULL(35),
+	IFF_PROFILE_COMPS		= BIT_ULL(36),
 };
 
 #define IFF_802_1Q_VLAN			IFF_802_1Q_VLAN
@@ -2400,6 +2407,14 @@ struct net_device {
 	/** @page_pools: page pools created for this netdevice */
 	struct hlist_head	page_pools;
 #endif
+
+#if IS_ENABLED(CONFIG_DIMLIB)
+	/* DIM profile lists for different dim cq modes */
+	struct dim_cq_moder *rx_eqe_profile;
+	struct dim_cq_moder *rx_cqe_profile;
+	struct dim_cq_moder *tx_eqe_profile;
+	struct dim_cq_moder *tx_cqe_profile;
+#endif
 };
 #define to_net_dev(d) container_of(d, struct net_device, dev)
 
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/net/core/dev.c b/net/core/dev.c
index 854a3a2..bc38f33 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -154,6 +154,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/prandom.h>
 #include <linux/once_lite.h>
+#include <linux/ethtool.h>
 #include <net/netdev_rx_queue.h>
 #include <net/page_pool/types.h>
 #include <net/page_pool/helpers.h>
@@ -10229,6 +10230,42 @@ static void netdev_do_free_pcpu_stats(struct net_device *dev)
 	}
 }
 
+static int dev_dim_profile_init(struct net_device *dev)
+{
+	int length = NET_DIM_PARAMS_NUM_PROFILES * sizeof(*dev->rx_eqe_profile);
+	u32 supported = dev->ethtool_ops->supported_coalesce_params;
+
+	if (!(dev->priv_flags & (IFF_PROFILE_USEC | IFF_PROFILE_PKTS | IFF_PROFILE_COMPS)))
+		return 0;
+
+	if (supported & ETHTOOL_COALESCE_RX_EQE_PROFILE) {
+		dev->rx_eqe_profile = kzalloc(length, GFP_KERNEL);
+		if (!dev->rx_eqe_profile)
+			return -ENOMEM;
+		memcpy(dev->rx_eqe_profile, rx_profile[0], length);
+	}
+	if (supported & ETHTOOL_COALESCE_RX_CQE_PROFILE) {
+		dev->rx_cqe_profile = kzalloc(length, GFP_KERNEL);
+		if (!dev->rx_cqe_profile)
+			return -ENOMEM;
+		memcpy(dev->rx_cqe_profile, rx_profile[1], length);
+	}
+	if (supported & ETHTOOL_COALESCE_TX_EQE_PROFILE) {
+		dev->tx_eqe_profile = kzalloc(length, GFP_KERNEL);
+		if (!dev->tx_eqe_profile)
+			return -ENOMEM;
+		memcpy(dev->tx_eqe_profile, tx_profile[0], length);
+	}
+	if (supported & ETHTOOL_COALESCE_TX_CQE_PROFILE) {
+		dev->tx_cqe_profile = kzalloc(length, GFP_KERNEL);
+		if (!dev->tx_cqe_profile)
+			return -ENOMEM;
+		memcpy(dev->tx_cqe_profile, tx_profile[1], length);
+	}
+
+	return 0;
+}
+
 /**
  * register_netdevice() - register a network device
  * @dev: device to register
@@ -10258,6 +10295,10 @@ int register_netdevice(struct net_device *dev)
 	if (ret)
 		return ret;
 
+	ret = dev_dim_profile_init(dev);
+	if (ret)
+		return ret;
+
 	spin_lock_init(&dev->addr_list_lock);
 	netdev_set_addr_lockdep_class(dev);
 
@@ -11011,6 +11052,26 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 }
 EXPORT_SYMBOL(alloc_netdev_mqs);
 
+static void netif_free_profile(struct net_device *dev)
+{
+	u32 supported = dev->ethtool_ops->supported_coalesce_params;
+
+	if (!(dev->priv_flags & (IFF_PROFILE_USEC | IFF_PROFILE_PKTS | IFF_PROFILE_COMPS)))
+		return;
+
+	if (supported & ETHTOOL_COALESCE_RX_EQE_PROFILE)
+		kfree(dev->rx_eqe_profile);
+
+	if (supported & ETHTOOL_COALESCE_RX_CQE_PROFILE)
+		kfree(dev->rx_cqe_profile);
+
+	if (supported & ETHTOOL_COALESCE_TX_EQE_PROFILE)
+		kfree(dev->tx_eqe_profile);
+
+	if (supported & ETHTOOL_COALESCE_TX_CQE_PROFILE)
+		kfree(dev->tx_cqe_profile);
+}
+
 /**
  * free_netdev - free network device
  * @dev: device
@@ -11036,6 +11097,8 @@ void free_netdev(struct net_device *dev)
 		return;
 	}
 
+	netif_free_profile(dev);
+
 	netif_free_tx_queues(dev);
 	netif_free_rx_queues(dev);
 
diff --git a/net/ethtool/coalesce.c b/net/ethtool/coalesce.c
index 83112c1..7b542c3e 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,67 @@ 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.
+ *
+ * Returns false to indicate successful placement or no placement, and
+ * returns true to pass the -EMSGSIZE error to the wrapper.
+ */
+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, emsg = !!-EMSGSIZE;
+	int i;
+
+	if (!profile)
+		return false;
+
+	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 emsg;
+
+	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 emsg;
+}
+
 static int coalesce_fill_reply(struct sk_buff *skb,
 			       const struct ethnl_req_info *req_base,
 			       const struct ethnl_reply_data *reply_base)
@@ -134,6 +207,7 @@ static int coalesce_fill_reply(struct sk_buff *skb,
 	const struct coalesce_reply_data *data = COALESCE_REPDATA(reply_base);
 	const struct kernel_ethtool_coalesce *kcoal = &data->kernel_coalesce;
 	const struct ethtool_coalesce *coal = &data->coalesce;
+	struct net_device *dev = req_base->dev;
 	u32 supported = data->supported_params;
 
 	if (coalesce_put_u32(skb, ETHTOOL_A_COALESCE_RX_USECS,
@@ -189,7 +263,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,
+				 dev->rx_eqe_profile, supported) ||
+	    coalesce_put_profile(skb, ETHTOOL_A_COALESCE_RX_CQE_PROFILE,
+				 dev->rx_cqe_profile, supported) ||
+	    coalesce_put_profile(skb, ETHTOOL_A_COALESCE_TX_EQE_PROFILE,
+				 dev->tx_eqe_profile, supported) ||
+	    coalesce_put_profile(skb, ETHTOOL_A_COALESCE_TX_CQE_PROFILE,
+				 dev->tx_cqe_profile, supported))
 		return -EMSGSIZE;
 
 	return 0;
@@ -227,6 +309,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 +345,73 @@ 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 net_device *dev,
+				       struct dim_cq_moder *dst,
+				       const struct nlattr *nests,
+				       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;
+
+	if (!dst)
+		return -EOPNOTSUPP;
+
+	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 && !(dev->priv_flags & IFF_PROFILE_USEC)) ||
+		    (dst[i].pkts != profile[i].pkts && !(dev->priv_flags & IFF_PROFILE_PKTS)) ||
+		    (dst[i].comps != profile[i].comps && !(dev->priv_flags & IFF_PROFILE_COMPS)))
+			return -EOPNOTSUPP;
+
+		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)
@@ -317,6 +476,27 @@ static int coalesce_fill_reply(struct sk_buff *skb,
 	ethnl_update_u32(&kernel_coalesce.tx_aggr_time_usecs,
 			 tb[ETHTOOL_A_COALESCE_TX_AGGR_TIME_USECS], &mod);
 
+	ret = ethnl_update_profile(dev, dev->rx_eqe_profile,
+				   tb[ETHTOOL_A_COALESCE_RX_EQE_PROFILE],
+				   info->extack);
+	if (ret < 0)
+		return ret;
+	ret = ethnl_update_profile(dev, dev->rx_cqe_profile,
+				   tb[ETHTOOL_A_COALESCE_RX_CQE_PROFILE],
+				   info->extack);
+	if (ret < 0)
+		return ret;
+	ret = ethnl_update_profile(dev, dev->tx_eqe_profile,
+				   tb[ETHTOOL_A_COALESCE_TX_EQE_PROFILE],
+				   info->extack);
+	if (ret < 0)
+		return ret;
+	ret = ethnl_update_profile(dev, dev->tx_cqe_profile,
+				   tb[ETHTOOL_A_COALESCE_TX_CQE_PROFILE],
+				   info->extack);
+	if (ret < 0)
+		return ret;
+
 	/* Update operation modes */
 	ethnl_update_bool32(&coalesce.use_adaptive_rx_coalesce,
 			    tb[ETHTOOL_A_COALESCE_USE_ADAPTIVE_RX], &mod_mode);
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next v6 3/4] virtio-net: refactor dim initialization/destruction
  2024-04-11 14:12 [PATCH net-next v6 0/4] ethtool: provide the dim profile fine-tuning channel Heng Qi
  2024-04-11 14:12 ` [PATCH net-next v6 1/4] linux/dim: move useful macros to .h file Heng Qi
  2024-04-11 14:12 ` [PATCH net-next v6 2/4] ethtool: provide customized dim profile management Heng Qi
@ 2024-04-11 14:12 ` Heng Qi
  2024-04-11 14:12 ` [PATCH net-next v6 4/4] virtio-net: support dim profile fine-tuning Heng Qi
  3 siblings, 0 replies; 17+ messages in thread
From: Heng Qi @ 2024-04-11 14:12 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] 17+ messages in thread

* [PATCH net-next v6 4/4] virtio-net: support dim profile fine-tuning
  2024-04-11 14:12 [PATCH net-next v6 0/4] ethtool: provide the dim profile fine-tuning channel Heng Qi
                   ` (2 preceding siblings ...)
  2024-04-11 14:12 ` [PATCH net-next v6 3/4] virtio-net: refactor dim initialization/destruction Heng Qi
@ 2024-04-11 14:12 ` Heng Qi
  2024-04-11 15:23   ` Brett Creeley
  3 siblings, 1 reply; 17+ messages in thread
From: Heng Qi @ 2024-04-11 14:12 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 | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index a64656e..813d9ed 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -3584,7 +3584,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 = dev->rx_eqe_profile[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,
@@ -3868,7 +3868,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,
@@ -4424,6 +4425,7 @@ static int virtnet_find_vqs(struct virtnet_info *vi)
 
 static void virtnet_dim_init(struct virtnet_info *vi)
 {
+	struct net_device *dev = vi->dev;
 	int i;
 
 	if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_VQ_NOTF_COAL))
@@ -4433,6 +4435,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;
 	}
+
+	dev->priv_flags |= IFF_PROFILE_USEC | IFF_PROFILE_PKTS;
 }
 
 static int virtnet_alloc_queues(struct virtnet_info *vi)
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v6 2/4] ethtool: provide customized dim profile management
  2024-04-11 14:12 ` [PATCH net-next v6 2/4] ethtool: provide customized dim profile management Heng Qi
@ 2024-04-11 15:19   ` Brett Creeley
  2024-04-12  2:07     ` Heng Qi
  2024-04-12  3:56   ` kernel test robot
                     ` (4 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Brett Creeley @ 2024-04-11 15:19 UTC (permalink / raw)
  To: Heng Qi, netdev, virtualization
  Cc: Jakub Kicinski, David S. Miller, Eric Dumazet, Paolo Abeni,
	Jason Wang, Michael S. Tsirkin, Ratheesh Kannoth,
	Alexander Lobakin, Xuan Zhuo



On 4/11/2024 7:12 AM, Heng Qi wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> 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.

What if the user specifies a *-eqe-profile and *-cqe-profile for rx 
and/or tx? Is that supported? If so, which one is the active profile?

Maybe I missed this, but it doesn't seem like the output from "ethtool 
-c ethX" shows the active profile it just dumps the profile configurations.

Thanks,

Brett

[snip]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v6 4/4] virtio-net: support dim profile fine-tuning
  2024-04-11 14:12 ` [PATCH net-next v6 4/4] virtio-net: support dim profile fine-tuning Heng Qi
@ 2024-04-11 15:23   ` Brett Creeley
  2024-04-12  2:19     ` Heng Qi
  0 siblings, 1 reply; 17+ messages in thread
From: Brett Creeley @ 2024-04-11 15:23 UTC (permalink / raw)
  To: Heng Qi, netdev, virtualization
  Cc: Jakub Kicinski, David S. Miller, Eric Dumazet, Paolo Abeni,
	Jason Wang, Michael S. Tsirkin, Ratheesh Kannoth,
	Alexander Lobakin, Xuan Zhuo



On 4/11/2024 7:12 AM, Heng Qi wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> 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 | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index a64656e..813d9ed 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -3584,7 +3584,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 = dev->rx_eqe_profile[dim->profile_ix];

Looking at this it seems like the driver has to be aware of the active 
profile type, i.e. eqe or cqe. Why not continue to use the dim->mode and 
also the net_dim_get_rx_moderation() helper?

Does the dim->mode not get updated based on the ethtool command(s) 
setting up the new and active profile?

Thanks,

Brett

>                  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,
> @@ -3868,7 +3868,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,
> @@ -4424,6 +4425,7 @@ static int virtnet_find_vqs(struct virtnet_info *vi)
> 
>   static void virtnet_dim_init(struct virtnet_info *vi)
>   {
> +       struct net_device *dev = vi->dev;
>          int i;
> 
>          if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_VQ_NOTF_COAL))
> @@ -4433,6 +4435,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;
>          }
> +
> +       dev->priv_flags |= IFF_PROFILE_USEC | IFF_PROFILE_PKTS;
>   }
> 
>   static int virtnet_alloc_queues(struct virtnet_info *vi)
> --
> 1.8.3.1
> 
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v6 2/4] ethtool: provide customized dim profile management
  2024-04-11 15:19   ` Brett Creeley
@ 2024-04-12  2:07     ` Heng Qi
  2024-04-12 15:33       ` Brett Creeley
  0 siblings, 1 reply; 17+ messages in thread
From: Heng Qi @ 2024-04-12  2:07 UTC (permalink / raw)
  To: Brett Creeley, netdev, virtualization
  Cc: Jakub Kicinski, David S. Miller, Eric Dumazet, Paolo Abeni,
	Jason Wang, Michael S. Tsirkin, Ratheesh Kannoth,
	Alexander Lobakin, Xuan Zhuo



在 2024/4/11 下午11:19, Brett Creeley 写道:
>
>
> On 4/11/2024 7:12 AM, Heng Qi wrote:
>> Caution: This message originated from an External Source. Use proper 
>> caution when opening attachments, clicking links, or responding.
>>
>>
>> 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.
>
> What if the user specifies a *-eqe-profile and *-cqe-profile for rx 
> and/or tx? Is that supported? If so, which one is the active profile?


I think you mean GET? GET currently does not support any parameters, the 
working profile will be displayed.

>
> Maybe I missed this, but it doesn't seem like the output from "ethtool 
> -c ethX" shows the active profile it just dumps the profile 
> configurations.

Now it is required that dev->priv_flags is set to one of 
IFF_PROFILE_{USEC, PKTS, COMPS} (which means that the
driver supports configurable profiles) before the profile can be queried 
or do you want to query without this restriction?

Thanks!

>
> Thanks,
>
> Brett
>
> [snip]


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v6 4/4] virtio-net: support dim profile fine-tuning
  2024-04-11 15:23   ` Brett Creeley
@ 2024-04-12  2:19     ` Heng Qi
  0 siblings, 0 replies; 17+ messages in thread
From: Heng Qi @ 2024-04-12  2:19 UTC (permalink / raw)
  To: Brett Creeley
  Cc: Jakub Kicinski, David S. Miller, Eric Dumazet, Paolo Abeni,
	Jason Wang, Michael S. Tsirkin, Ratheesh Kannoth,
	Alexander Lobakin, Xuan Zhuo, open list:NETWORKING DRIVERS,
	open list:VIRTIO CORE AND NET DRIVERS



在 2024/4/11 下午11:23, Brett Creeley 写道:
>
>
> On 4/11/2024 7:12 AM, Heng Qi wrote:
>> Caution: This message originated from an External Source. Use proper 
>> caution when opening attachments, clicking links, or responding.
>>
>>
>> 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 | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index a64656e..813d9ed 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -3584,7 +3584,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 = dev->rx_eqe_profile[dim->profile_ix];
>
> Looking at this it seems like the driver has to be aware of the active 
> profile type, i.e. eqe or cqe. Why not continue to use the dim->mode 
> and also the net_dim_get_rx_moderation() helper?

At this point I plan to issue a patch set after this to clean up all 
related drivers together,
which I have mentioned in the cover-letter:

"
Since the profile now exists in netdevice, adding a function similar
to net_dim_get_rx_moderation_dev() with netdevice as argument is
nice, but this would be better along with cleaning up the rest of
the drivers, which we can get to very soon after this set.
"

>
> Does the dim->mode not get updated based on the ethtool command(s) 
> setting up the new and active profile?

This will not be updated.

Thanks.

>
> Thanks,
>
> Brett
>
>>                  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,
>> @@ -3868,7 +3868,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,
>> @@ -4424,6 +4425,7 @@ static int virtnet_find_vqs(struct virtnet_info 
>> *vi)
>>
>>   static void virtnet_dim_init(struct virtnet_info *vi)
>>   {
>> +       struct net_device *dev = vi->dev;
>>          int i;
>>
>>          if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_VQ_NOTF_COAL))
>> @@ -4433,6 +4435,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;
>>          }
>> +
>> +       dev->priv_flags |= IFF_PROFILE_USEC | IFF_PROFILE_PKTS;
>>   }
>>
>>   static int virtnet_alloc_queues(struct virtnet_info *vi)
>> -- 
>> 1.8.3.1
>>
>>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v6 2/4] ethtool: provide customized dim profile management
  2024-04-11 14:12 ` [PATCH net-next v6 2/4] ethtool: provide customized dim profile management Heng Qi
  2024-04-11 15:19   ` Brett Creeley
@ 2024-04-12  3:56   ` kernel test robot
  2024-04-12  4:39   ` kernel test robot
                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2024-04-12  3:56 UTC (permalink / raw)
  To: Heng Qi, netdev, virtualization
  Cc: llvm, oe-kbuild-all, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Jason Wang, Michael S. Tsirkin, Ratheesh Kannoth,
	Alexander Lobakin, Xuan Zhuo

Hi Heng,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Heng-Qi/linux-dim-move-useful-macros-to-h-file/20240411-221400
base:   net-next/main
patch link:    https://lore.kernel.org/r/1712844751-53514-3-git-send-email-hengqi%40linux.alibaba.com
patch subject: [PATCH net-next v6 2/4] ethtool: provide customized dim profile management
config: riscv-defconfig (https://download.01.org/0day-ci/archive/20240412/202404121112.lymscQm1-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 8b3b4a92adee40483c27f26c478a384cd69c6f05)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240412/202404121112.lymscQm1-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202404121112.lymscQm1-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> net/ethtool/coalesce.c:373: warning: Function parameter or struct member 'dev' not described in 'ethnl_update_profile'
>> net/ethtool/coalesce.c:373: warning: Excess function parameter 'mod' description in 'ethnl_update_profile'


vim +373 net/ethtool/coalesce.c

   347	
   348	/**
   349	 * ethnl_update_profile - get a nla nest with four child nla nests from userspace.
   350	 * @dst: data get from the driver and modified by ethnl_update_profile.
   351	 * @nests: nest attr ETHTOOL_A_COALESCE_*X_*QE_PROFILE to set driver's profile.
   352	 * @mod: whether the data is modified
   353	 * @extack: Netlink extended ack
   354	 *
   355	 * Layout of nests:
   356	 *   Nested ETHTOOL_A_COALESCE_*X_*QE_PROFILE attr
   357	 *     Nested ETHTOOL_A_MODERATIONS_MODERATION attr
   358	 *       ETHTOOL_A_MODERATION_USEC attr
   359	 *       ETHTOOL_A_MODERATION_PKTS attr
   360	 *       ETHTOOL_A_MODERATION_COMPS attr
   361	 *     ...
   362	 *     Nested ETHTOOL_A_MODERATIONS_MODERATION attr
   363	 *       ETHTOOL_A_MODERATION_USEC attr
   364	 *       ETHTOOL_A_MODERATION_PKTS attr
   365	 *       ETHTOOL_A_MODERATION_COMPS attr
   366	 *
   367	 * Returns 0 on success or a negative error code.
   368	 */
   369	static inline int ethnl_update_profile(struct net_device *dev,
   370					       struct dim_cq_moder *dst,
   371					       const struct nlattr *nests,
   372					       struct netlink_ext_ack *extack)
 > 373	{
   374		struct nlattr *tb_moder[ARRAY_SIZE(coalesce_set_profile_policy)];
   375		struct dim_cq_moder profile[NET_DIM_PARAMS_NUM_PROFILES];
   376		struct nlattr *nest;
   377		int ret, rem, i = 0;
   378	
   379		if (!nests)
   380			return 0;
   381	
   382		if (!dst)
   383			return -EOPNOTSUPP;
   384	
   385		nla_for_each_nested_type(nest, ETHTOOL_A_MODERATIONS_MODERATION, nests, rem) {
   386			ret = nla_parse_nested(tb_moder,
   387					       ARRAY_SIZE(coalesce_set_profile_policy) - 1,
   388					       nest, coalesce_set_profile_policy,
   389					       extack);
   390			if (ret)
   391				return ret;
   392	
   393			if (NL_REQ_ATTR_CHECK(extack, nest, tb_moder, ETHTOOL_A_MODERATION_USEC) ||
   394			    NL_REQ_ATTR_CHECK(extack, nest, tb_moder, ETHTOOL_A_MODERATION_PKTS) ||
   395			    NL_REQ_ATTR_CHECK(extack, nest, tb_moder, ETHTOOL_A_MODERATION_COMPS))
   396				return -EINVAL;
   397	
   398			profile[i].usec = nla_get_u16(tb_moder[ETHTOOL_A_MODERATION_USEC]);
   399			profile[i].pkts = nla_get_u16(tb_moder[ETHTOOL_A_MODERATION_PKTS]);
   400			profile[i].comps = nla_get_u16(tb_moder[ETHTOOL_A_MODERATION_COMPS]);
   401	
   402			if ((dst[i].usec != profile[i].usec && !(dev->priv_flags & IFF_PROFILE_USEC)) ||
   403			    (dst[i].pkts != profile[i].pkts && !(dev->priv_flags & IFF_PROFILE_PKTS)) ||
   404			    (dst[i].comps != profile[i].comps && !(dev->priv_flags & IFF_PROFILE_COMPS)))
   405				return -EOPNOTSUPP;
   406	
   407			i++;
   408		}
   409	
   410		memcpy(dst, profile, sizeof(profile));
   411	
   412		return 0;
   413	}
   414	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v6 2/4] ethtool: provide customized dim profile management
  2024-04-11 14:12 ` [PATCH net-next v6 2/4] ethtool: provide customized dim profile management Heng Qi
  2024-04-11 15:19   ` Brett Creeley
  2024-04-12  3:56   ` kernel test robot
@ 2024-04-12  4:39   ` kernel test robot
  2024-04-12 17:44   ` kernel test robot
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2024-04-12  4:39 UTC (permalink / raw)
  To: Heng Qi, netdev, virtualization
  Cc: llvm, oe-kbuild-all, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Jason Wang, Michael S. Tsirkin, Ratheesh Kannoth,
	Alexander Lobakin, Xuan Zhuo

Hi Heng,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Heng-Qi/linux-dim-move-useful-macros-to-h-file/20240411-221400
base:   net-next/main
patch link:    https://lore.kernel.org/r/1712844751-53514-3-git-send-email-hengqi%40linux.alibaba.com
patch subject: [PATCH net-next v6 2/4] ethtool: provide customized dim profile management
config: um-allnoconfig (https://download.01.org/0day-ci/archive/20240412/202404121200.pplgT1xP-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240412/202404121200.pplgT1xP-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202404121200.pplgT1xP-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from net/core/dev.c:80:
   In file included from include/linux/sched/isolation.h:7:
   In file included from include/linux/tick.h:8:
   In file included from include/linux/clockchips.h:14:
   In file included from include/linux/clocksource.h:22:
   In file included from arch/um/include/asm/io.h:24:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     547 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     560 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
         |                                                   ^
   In file included from net/core/dev.c:80:
   In file included from include/linux/sched/isolation.h:7:
   In file included from include/linux/tick.h:8:
   In file included from include/linux/clockchips.h:14:
   In file included from include/linux/clocksource.h:22:
   In file included from arch/um/include/asm/io.h:24:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     573 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
         |                                                   ^
   In file included from net/core/dev.c:80:
   In file included from include/linux/sched/isolation.h:7:
   In file included from include/linux/tick.h:8:
   In file included from include/linux/clockchips.h:14:
   In file included from include/linux/clocksource.h:22:
   In file included from arch/um/include/asm/io.h:24:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     584 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     594 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     604 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     692 |         readsb(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     700 |         readsw(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     708 |         readsl(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     717 |         writesb(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     726 |         writesw(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     735 |         writesl(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
>> net/core/dev.c:10235:58: error: no member named 'rx_eqe_profile' in 'struct net_device'
    10235 |         int length = NET_DIM_PARAMS_NUM_PROFILES * sizeof(*dev->rx_eqe_profile);
          |                                                            ~~~  ^
   net/core/dev.c:10242:8: error: no member named 'rx_eqe_profile' in 'struct net_device'
    10242 |                 dev->rx_eqe_profile = kzalloc(length, GFP_KERNEL);
          |                 ~~~  ^
   net/core/dev.c:10243:13: error: no member named 'rx_eqe_profile' in 'struct net_device'
    10243 |                 if (!dev->rx_eqe_profile)
          |                      ~~~  ^
   net/core/dev.c:10245:15: error: no member named 'rx_eqe_profile' in 'struct net_device'
    10245 |                 memcpy(dev->rx_eqe_profile, rx_profile[0], length);
          |                        ~~~  ^
>> net/core/dev.c:10248:8: error: no member named 'rx_cqe_profile' in 'struct net_device'
    10248 |                 dev->rx_cqe_profile = kzalloc(length, GFP_KERNEL);
          |                 ~~~  ^
   net/core/dev.c:10249:13: error: no member named 'rx_cqe_profile' in 'struct net_device'
    10249 |                 if (!dev->rx_cqe_profile)
          |                      ~~~  ^
   net/core/dev.c:10251:15: error: no member named 'rx_cqe_profile' in 'struct net_device'
    10251 |                 memcpy(dev->rx_cqe_profile, rx_profile[1], length);
          |                        ~~~  ^
>> net/core/dev.c:10254:8: error: no member named 'tx_eqe_profile' in 'struct net_device'
    10254 |                 dev->tx_eqe_profile = kzalloc(length, GFP_KERNEL);
          |                 ~~~  ^
   net/core/dev.c:10255:13: error: no member named 'tx_eqe_profile' in 'struct net_device'
    10255 |                 if (!dev->tx_eqe_profile)
          |                      ~~~  ^
   net/core/dev.c:10257:15: error: no member named 'tx_eqe_profile' in 'struct net_device'
    10257 |                 memcpy(dev->tx_eqe_profile, tx_profile[0], length);
          |                        ~~~  ^
>> net/core/dev.c:10260:8: error: no member named 'tx_cqe_profile' in 'struct net_device'
    10260 |                 dev->tx_cqe_profile = kzalloc(length, GFP_KERNEL);
          |                 ~~~  ^
   net/core/dev.c:10261:13: error: no member named 'tx_cqe_profile' in 'struct net_device'
    10261 |                 if (!dev->tx_cqe_profile)
          |                      ~~~  ^
   net/core/dev.c:10263:15: error: no member named 'tx_cqe_profile' in 'struct net_device'
    10263 |                 memcpy(dev->tx_cqe_profile, tx_profile[1], length);
          |                        ~~~  ^
   net/core/dev.c:11063:14: error: no member named 'rx_eqe_profile' in 'struct net_device'
    11063 |                 kfree(dev->rx_eqe_profile);
          |                       ~~~  ^
   net/core/dev.c:11066:14: error: no member named 'rx_cqe_profile' in 'struct net_device'
    11066 |                 kfree(dev->rx_cqe_profile);
          |                       ~~~  ^
   net/core/dev.c:11069:14: error: no member named 'tx_eqe_profile' in 'struct net_device'
    11069 |                 kfree(dev->tx_eqe_profile);
          |                       ~~~  ^
   net/core/dev.c:11072:14: error: no member named 'tx_cqe_profile' in 'struct net_device'
    11072 |                 kfree(dev->tx_cqe_profile);
          |                       ~~~  ^
   12 warnings and 17 errors generated.
--
   In file included from net/ethtool/coalesce.c:3:
   In file included from net/ethtool/netlink.h:6:
   In file included from include/linux/ethtool_netlink.h:6:
   In file included from include/uapi/linux/ethtool_netlink.h:12:
   In file included from include/linux/ethtool.h:18:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/um/include/asm/hardirq.h:5:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/um/include/asm/io.h:24:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     547 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     560 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
         |                                                   ^
   In file included from net/ethtool/coalesce.c:3:
   In file included from net/ethtool/netlink.h:6:
   In file included from include/linux/ethtool_netlink.h:6:
   In file included from include/uapi/linux/ethtool_netlink.h:12:
   In file included from include/linux/ethtool.h:18:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/um/include/asm/hardirq.h:5:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/um/include/asm/io.h:24:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     573 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
         |                                                   ^
   In file included from net/ethtool/coalesce.c:3:
   In file included from net/ethtool/netlink.h:6:
   In file included from include/linux/ethtool_netlink.h:6:
   In file included from include/uapi/linux/ethtool_netlink.h:12:
   In file included from include/linux/ethtool.h:18:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/um/include/asm/hardirq.h:5:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/um/include/asm/io.h:24:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     584 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     594 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     604 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     692 |         readsb(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     700 |         readsw(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     708 |         readsl(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     717 |         writesb(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     726 |         writesw(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     735 |         writesl(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
>> net/ethtool/coalesce.c:268:11: error: no member named 'rx_eqe_profile' in 'struct net_device'
     268 |                                  dev->rx_eqe_profile, supported) ||
         |                                  ~~~  ^
>> net/ethtool/coalesce.c:270:11: error: no member named 'rx_cqe_profile' in 'struct net_device'
     270 |                                  dev->rx_cqe_profile, supported) ||
         |                                  ~~~  ^
>> net/ethtool/coalesce.c:272:11: error: no member named 'tx_eqe_profile' in 'struct net_device'
     272 |                                  dev->tx_eqe_profile, supported) ||
         |                                  ~~~  ^
>> net/ethtool/coalesce.c:274:11: error: no member named 'tx_cqe_profile' in 'struct net_device'
     274 |                                  dev->tx_cqe_profile, supported))
         |                                  ~~~  ^
   net/ethtool/coalesce.c:479:39: error: no member named 'rx_eqe_profile' in 'struct net_device'
     479 |         ret = ethnl_update_profile(dev, dev->rx_eqe_profile,
         |                                         ~~~  ^
   net/ethtool/coalesce.c:484:39: error: no member named 'rx_cqe_profile' in 'struct net_device'
     484 |         ret = ethnl_update_profile(dev, dev->rx_cqe_profile,
         |                                         ~~~  ^
   net/ethtool/coalesce.c:489:39: error: no member named 'tx_eqe_profile' in 'struct net_device'
     489 |         ret = ethnl_update_profile(dev, dev->tx_eqe_profile,
         |                                         ~~~  ^
   net/ethtool/coalesce.c:494:39: error: no member named 'tx_cqe_profile' in 'struct net_device'
     494 |         ret = ethnl_update_profile(dev, dev->tx_cqe_profile,
         |                                         ~~~  ^
   12 warnings and 8 errors generated.


vim +10235 net/core/dev.c

 10232	
 10233	static int dev_dim_profile_init(struct net_device *dev)
 10234	{
 10235		int length = NET_DIM_PARAMS_NUM_PROFILES * sizeof(*dev->rx_eqe_profile);
 10236		u32 supported = dev->ethtool_ops->supported_coalesce_params;
 10237	
 10238		if (!(dev->priv_flags & (IFF_PROFILE_USEC | IFF_PROFILE_PKTS | IFF_PROFILE_COMPS)))
 10239			return 0;
 10240	
 10241		if (supported & ETHTOOL_COALESCE_RX_EQE_PROFILE) {
 10242			dev->rx_eqe_profile = kzalloc(length, GFP_KERNEL);
 10243			if (!dev->rx_eqe_profile)
 10244				return -ENOMEM;
 10245			memcpy(dev->rx_eqe_profile, rx_profile[0], length);
 10246		}
 10247		if (supported & ETHTOOL_COALESCE_RX_CQE_PROFILE) {
 10248			dev->rx_cqe_profile = kzalloc(length, GFP_KERNEL);
 10249			if (!dev->rx_cqe_profile)
 10250				return -ENOMEM;
 10251			memcpy(dev->rx_cqe_profile, rx_profile[1], length);
 10252		}
 10253		if (supported & ETHTOOL_COALESCE_TX_EQE_PROFILE) {
 10254			dev->tx_eqe_profile = kzalloc(length, GFP_KERNEL);
 10255			if (!dev->tx_eqe_profile)
 10256				return -ENOMEM;
 10257			memcpy(dev->tx_eqe_profile, tx_profile[0], length);
 10258		}
 10259		if (supported & ETHTOOL_COALESCE_TX_CQE_PROFILE) {
 10260			dev->tx_cqe_profile = kzalloc(length, GFP_KERNEL);
 10261			if (!dev->tx_cqe_profile)
 10262				return -ENOMEM;
 10263			memcpy(dev->tx_cqe_profile, tx_profile[1], length);
 10264		}
 10265	
 10266		return 0;
 10267	}
 10268	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v6 2/4] ethtool: provide customized dim profile management
  2024-04-12  2:07     ` Heng Qi
@ 2024-04-12 15:33       ` Brett Creeley
  2024-04-12 16:05         ` Heng Qi
  0 siblings, 1 reply; 17+ messages in thread
From: Brett Creeley @ 2024-04-12 15:33 UTC (permalink / raw)
  To: Heng Qi, netdev, virtualization
  Cc: Jakub Kicinski, David S. Miller, Eric Dumazet, Paolo Abeni,
	Jason Wang, Michael S. Tsirkin, Ratheesh Kannoth,
	Alexander Lobakin, Xuan Zhuo



On 4/11/2024 7:07 PM, Heng Qi wrote:
> Caution: This message originated from an External Source. Use proper 
> caution when opening attachments, clicking links, or responding.
> 
> 
> 在 2024/4/11 下午11:19, Brett Creeley 写道:
>>
>>
>> On 4/11/2024 7:12 AM, Heng Qi wrote:
>>> Caution: This message originated from an External Source. Use proper
>>> caution when opening attachments, clicking links, or responding.
>>>
>>>
>>> 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.
>>
>> What if the user specifies a *-eqe-profile and *-cqe-profile for rx
>> and/or tx? Is that supported? If so, which one is the active profile?
> 
> 
> I think you mean GET? GET currently does not support any parameters, the
> working profile will be displayed.

Yeah, I meant the GET operation. As long as the currently in-use/working 
profile is displayed that makes sense.

> 
>>
>> Maybe I missed this, but it doesn't seem like the output from "ethtool
>> -c ethX" shows the active profile it just dumps the profile
>> configurations.
> 
> Now it is required that dev->priv_flags is set to one of
> IFF_PROFILE_{USEC, PKTS, COMPS} (which means that the
> driver supports configurable profiles) before the profile can be queried
> or do you want to query without this restriction?

No, just as I mentioned above it seems okay.

> 
> Thanks!
> 
>>
>> Thanks,
>>
>> Brett
>>
>> [snip]
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v6 2/4] ethtool: provide customized dim profile management
  2024-04-12 15:33       ` Brett Creeley
@ 2024-04-12 16:05         ` Heng Qi
  0 siblings, 0 replies; 17+ messages in thread
From: Heng Qi @ 2024-04-12 16:05 UTC (permalink / raw)
  To: Brett Creeley, netdev, virtualization
  Cc: Jakub Kicinski, David S. Miller, Eric Dumazet, Paolo Abeni,
	Jason Wang, Michael S. Tsirkin, Ratheesh Kannoth,
	Alexander Lobakin, Xuan Zhuo



在 2024/4/12 下午11:33, Brett Creeley 写道:
>
>
> On 4/11/2024 7:07 PM, Heng Qi wrote:
>> Caution: This message originated from an External Source. Use proper 
>> caution when opening attachments, clicking links, or responding.
>>
>>
>> 在 2024/4/11 下午11:19, Brett Creeley 写道:
>>>
>>>
>>> On 4/11/2024 7:12 AM, Heng Qi wrote:
>>>> Caution: This message originated from an External Source. Use proper
>>>> caution when opening attachments, clicking links, or responding.
>>>>
>>>>
>>>> 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.
>>>
>>> What if the user specifies a *-eqe-profile and *-cqe-profile for rx
>>> and/or tx? Is that supported? If so, which one is the active profile?
>>
>>
>> I think you mean GET? GET currently does not support any parameters, the
>> working profile will be displayed.
>
> Yeah, I meant the GET operation. As long as the currently 
> in-use/working profile is displayed that makes sense.
>
>>
>>>
>>> Maybe I missed this, but it doesn't seem like the output from "ethtool
>>> -c ethX" shows the active profile it just dumps the profile
>>> configurations.
>>
>> Now it is required that dev->priv_flags is set to one of
>> IFF_PROFILE_{USEC, PKTS, COMPS} (which means that the
>> driver supports configurable profiles) before the profile can be queried
>> or do you want to query without this restriction?
>
> No, just as I mentioned above it seems okay.

Okay, then I will fix the robot warning and send out a new version.

Thanks.

>
>>
>> Thanks!
>>
>>>
>>> Thanks,
>>>
>>> Brett
>>>
>>> [snip]
>>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v6 2/4] ethtool: provide customized dim profile management
  2024-04-11 14:12 ` [PATCH net-next v6 2/4] ethtool: provide customized dim profile management Heng Qi
                     ` (2 preceding siblings ...)
  2024-04-12  4:39   ` kernel test robot
@ 2024-04-12 17:44   ` kernel test robot
  2024-04-13  2:26   ` Jakub Kicinski
  2024-04-13  2:27   ` Jakub Kicinski
  5 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2024-04-12 17:44 UTC (permalink / raw)
  To: Heng Qi, netdev, virtualization
  Cc: oe-kbuild-all, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Jason Wang, Michael S. Tsirkin, Ratheesh Kannoth,
	Alexander Lobakin, Xuan Zhuo

Hi Heng,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Heng-Qi/linux-dim-move-useful-macros-to-h-file/20240411-221400
base:   net-next/main
patch link:    https://lore.kernel.org/r/1712844751-53514-3-git-send-email-hengqi%40linux.alibaba.com
patch subject: [PATCH net-next v6 2/4] ethtool: provide customized dim profile management
config: openrisc-defconfig (https://download.01.org/0day-ci/archive/20240413/202404130138.7jOMaraz-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240413/202404130138.7jOMaraz-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202404130138.7jOMaraz-lkp@intel.com/

All errors (new ones prefixed by >>):

   net/core/dev.c: In function 'dev_dim_profile_init':
>> net/core/dev.c:10235:63: error: 'struct net_device' has no member named 'rx_eqe_profile'
   10235 |         int length = NET_DIM_PARAMS_NUM_PROFILES * sizeof(*dev->rx_eqe_profile);
         |                                                               ^~
   net/core/dev.c:10242:20: error: 'struct net_device' has no member named 'rx_eqe_profile'
   10242 |                 dev->rx_eqe_profile = kzalloc(length, GFP_KERNEL);
         |                    ^~
   net/core/dev.c:10243:25: error: 'struct net_device' has no member named 'rx_eqe_profile'
   10243 |                 if (!dev->rx_eqe_profile)
         |                         ^~
   net/core/dev.c:10245:27: error: 'struct net_device' has no member named 'rx_eqe_profile'
   10245 |                 memcpy(dev->rx_eqe_profile, rx_profile[0], length);
         |                           ^~
>> net/core/dev.c:10248:20: error: 'struct net_device' has no member named 'rx_cqe_profile'
   10248 |                 dev->rx_cqe_profile = kzalloc(length, GFP_KERNEL);
         |                    ^~
   net/core/dev.c:10249:25: error: 'struct net_device' has no member named 'rx_cqe_profile'
   10249 |                 if (!dev->rx_cqe_profile)
         |                         ^~
   net/core/dev.c:10251:27: error: 'struct net_device' has no member named 'rx_cqe_profile'
   10251 |                 memcpy(dev->rx_cqe_profile, rx_profile[1], length);
         |                           ^~
>> net/core/dev.c:10254:20: error: 'struct net_device' has no member named 'tx_eqe_profile'
   10254 |                 dev->tx_eqe_profile = kzalloc(length, GFP_KERNEL);
         |                    ^~
   net/core/dev.c:10255:25: error: 'struct net_device' has no member named 'tx_eqe_profile'
   10255 |                 if (!dev->tx_eqe_profile)
         |                         ^~
   net/core/dev.c:10257:27: error: 'struct net_device' has no member named 'tx_eqe_profile'
   10257 |                 memcpy(dev->tx_eqe_profile, tx_profile[0], length);
         |                           ^~
>> net/core/dev.c:10260:20: error: 'struct net_device' has no member named 'tx_cqe_profile'
   10260 |                 dev->tx_cqe_profile = kzalloc(length, GFP_KERNEL);
         |                    ^~
   net/core/dev.c:10261:25: error: 'struct net_device' has no member named 'tx_cqe_profile'
   10261 |                 if (!dev->tx_cqe_profile)
         |                         ^~
   net/core/dev.c:10263:27: error: 'struct net_device' has no member named 'tx_cqe_profile'
   10263 |                 memcpy(dev->tx_cqe_profile, tx_profile[1], length);
         |                           ^~
   net/core/dev.c: In function 'netif_free_profile':
   net/core/dev.c:11063:26: error: 'struct net_device' has no member named 'rx_eqe_profile'
   11063 |                 kfree(dev->rx_eqe_profile);
         |                          ^~
   net/core/dev.c:11066:26: error: 'struct net_device' has no member named 'rx_cqe_profile'
   11066 |                 kfree(dev->rx_cqe_profile);
         |                          ^~
   net/core/dev.c:11069:26: error: 'struct net_device' has no member named 'tx_eqe_profile'
   11069 |                 kfree(dev->tx_eqe_profile);
         |                          ^~
   net/core/dev.c:11072:26: error: 'struct net_device' has no member named 'tx_cqe_profile'
   11072 |                 kfree(dev->tx_cqe_profile);
         |                          ^~
--
   net/ethtool/coalesce.c: In function 'coalesce_fill_reply':
>> net/ethtool/coalesce.c:268:37: error: 'struct net_device' has no member named 'rx_eqe_profile'
     268 |                                  dev->rx_eqe_profile, supported) ||
         |                                     ^~
>> net/ethtool/coalesce.c:270:37: error: 'struct net_device' has no member named 'rx_cqe_profile'
     270 |                                  dev->rx_cqe_profile, supported) ||
         |                                     ^~
>> net/ethtool/coalesce.c:272:37: error: 'struct net_device' has no member named 'tx_eqe_profile'
     272 |                                  dev->tx_eqe_profile, supported) ||
         |                                     ^~
>> net/ethtool/coalesce.c:274:37: error: 'struct net_device' has no member named 'tx_cqe_profile'
     274 |                                  dev->tx_cqe_profile, supported))
         |                                     ^~
   net/ethtool/coalesce.c: In function '__ethnl_set_coalesce':
   net/ethtool/coalesce.c:479:44: error: 'struct net_device' has no member named 'rx_eqe_profile'
     479 |         ret = ethnl_update_profile(dev, dev->rx_eqe_profile,
         |                                            ^~
   net/ethtool/coalesce.c:484:44: error: 'struct net_device' has no member named 'rx_cqe_profile'
     484 |         ret = ethnl_update_profile(dev, dev->rx_cqe_profile,
         |                                            ^~
   net/ethtool/coalesce.c:489:44: error: 'struct net_device' has no member named 'tx_eqe_profile'
     489 |         ret = ethnl_update_profile(dev, dev->tx_eqe_profile,
         |                                            ^~
   net/ethtool/coalesce.c:494:44: error: 'struct net_device' has no member named 'tx_cqe_profile'
     494 |         ret = ethnl_update_profile(dev, dev->tx_cqe_profile,
         |                                            ^~


vim +10235 net/core/dev.c

 10232	
 10233	static int dev_dim_profile_init(struct net_device *dev)
 10234	{
 10235		int length = NET_DIM_PARAMS_NUM_PROFILES * sizeof(*dev->rx_eqe_profile);
 10236		u32 supported = dev->ethtool_ops->supported_coalesce_params;
 10237	
 10238		if (!(dev->priv_flags & (IFF_PROFILE_USEC | IFF_PROFILE_PKTS | IFF_PROFILE_COMPS)))
 10239			return 0;
 10240	
 10241		if (supported & ETHTOOL_COALESCE_RX_EQE_PROFILE) {
 10242			dev->rx_eqe_profile = kzalloc(length, GFP_KERNEL);
 10243			if (!dev->rx_eqe_profile)
 10244				return -ENOMEM;
 10245			memcpy(dev->rx_eqe_profile, rx_profile[0], length);
 10246		}
 10247		if (supported & ETHTOOL_COALESCE_RX_CQE_PROFILE) {
 10248			dev->rx_cqe_profile = kzalloc(length, GFP_KERNEL);
 10249			if (!dev->rx_cqe_profile)
 10250				return -ENOMEM;
 10251			memcpy(dev->rx_cqe_profile, rx_profile[1], length);
 10252		}
 10253		if (supported & ETHTOOL_COALESCE_TX_EQE_PROFILE) {
 10254			dev->tx_eqe_profile = kzalloc(length, GFP_KERNEL);
 10255			if (!dev->tx_eqe_profile)
 10256				return -ENOMEM;
 10257			memcpy(dev->tx_eqe_profile, tx_profile[0], length);
 10258		}
 10259		if (supported & ETHTOOL_COALESCE_TX_CQE_PROFILE) {
 10260			dev->tx_cqe_profile = kzalloc(length, GFP_KERNEL);
 10261			if (!dev->tx_cqe_profile)
 10262				return -ENOMEM;
 10263			memcpy(dev->tx_cqe_profile, tx_profile[1], length);
 10264		}
 10265	
 10266		return 0;
 10267	}
 10268	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v6 2/4] ethtool: provide customized dim profile management
  2024-04-11 14:12 ` [PATCH net-next v6 2/4] ethtool: provide customized dim profile management Heng Qi
                     ` (3 preceding siblings ...)
  2024-04-12 17:44   ` kernel test robot
@ 2024-04-13  2:26   ` Jakub Kicinski
  2024-04-13 10:14     ` Heng Qi
  2024-04-13  2:27   ` Jakub Kicinski
  5 siblings, 1 reply; 17+ messages in thread
From: Jakub Kicinski @ 2024-04-13  2:26 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 Thu, 11 Apr 2024 22:12:29 +0800 Heng Qi wrote:
> +#include <linux/dim.h>
>  #include <net/net_trackers.h>
>  #include <net/net_debug.h>
>  #include <net/dropreason-core.h>
> @@ -1649,6 +1650,9 @@ struct net_device_ops {
>   * @IFF_SEE_ALL_HWTSTAMP_REQUESTS: device wants to see calls to
>   *	ndo_hwtstamp_set() for all timestamp requests regardless of source,
>   *	even if those aren't HWTSTAMP_SOURCE_NETDEV.
> + * @IFF_PROFILE_USEC: device supports adjusting the DIM profile's usec field
> + * @IFF_PROFILE_PKTS: device supports adjusting the DIM profile's pkts field
> + * @IFF_PROFILE_COMPS: device supports adjusting the DIM profile's comps field
>   */
>  enum netdev_priv_flags {
>  	IFF_802_1Q_VLAN			= 1<<0,
> @@ -1685,6 +1689,9 @@ enum netdev_priv_flags {
>  	IFF_TX_SKB_NO_LINEAR		= BIT_ULL(31),
>  	IFF_CHANGE_PROTO_DOWN		= BIT_ULL(32),
>  	IFF_SEE_ALL_HWTSTAMP_REQUESTS	= BIT_ULL(33),
> +	IFF_PROFILE_USEC		= BIT_ULL(34),
> +	IFF_PROFILE_PKTS		= BIT_ULL(35),
> +	IFF_PROFILE_COMPS		= BIT_ULL(36),
>  };
>  
>  #define IFF_802_1Q_VLAN			IFF_802_1Q_VLAN
> @@ -2400,6 +2407,14 @@ struct net_device {
>  	/** @page_pools: page pools created for this netdevice */
>  	struct hlist_head	page_pools;
>  #endif
> +
> +#if IS_ENABLED(CONFIG_DIMLIB)
> +	/* DIM profile lists for different dim cq modes */
> +	struct dim_cq_moder *rx_eqe_profile;
> +	struct dim_cq_moder *rx_cqe_profile;
> +	struct dim_cq_moder *tx_eqe_profile;
> +	struct dim_cq_moder *tx_cqe_profile;
> +#endif

just one pointer to a new wrapper struct, put the pointers and a flag
field in there.

netdevice.h is included by thousands of files, please use a forward
declaration for the type and avoid including dim.h

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v6 2/4] ethtool: provide customized dim profile management
  2024-04-11 14:12 ` [PATCH net-next v6 2/4] ethtool: provide customized dim profile management Heng Qi
                     ` (4 preceding siblings ...)
  2024-04-13  2:26   ` Jakub Kicinski
@ 2024-04-13  2:27   ` Jakub Kicinski
  5 siblings, 0 replies; 17+ messages in thread
From: Jakub Kicinski @ 2024-04-13  2:27 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 Thu, 11 Apr 2024 22:12:29 +0800 Heng Qi wrote:
> +        name: usec
> +        type: u16
> +      -
> +        name: pkts
> +        type: u16
> +      -
> +        name: comps
> +        type: u16

I think I mentioned already - u32 please.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v6 2/4] ethtool: provide customized dim profile management
  2024-04-13  2:26   ` Jakub Kicinski
@ 2024-04-13 10:14     ` Heng Qi
  0 siblings, 0 replies; 17+ messages in thread
From: Heng Qi @ 2024-04-13 10:14 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/13 上午10:26, Jakub Kicinski 写道:
> On Thu, 11 Apr 2024 22:12:29 +0800 Heng Qi wrote:
>> +#include <linux/dim.h>
>>   #include <net/net_trackers.h>
>>   #include <net/net_debug.h>
>>   #include <net/dropreason-core.h>
>> @@ -1649,6 +1650,9 @@ struct net_device_ops {
>>    * @IFF_SEE_ALL_HWTSTAMP_REQUESTS: device wants to see calls to
>>    *	ndo_hwtstamp_set() for all timestamp requests regardless of source,
>>    *	even if those aren't HWTSTAMP_SOURCE_NETDEV.
>> + * @IFF_PROFILE_USEC: device supports adjusting the DIM profile's usec field
>> + * @IFF_PROFILE_PKTS: device supports adjusting the DIM profile's pkts field
>> + * @IFF_PROFILE_COMPS: device supports adjusting the DIM profile's comps field
>>    */
>>   enum netdev_priv_flags {
>>   	IFF_802_1Q_VLAN			= 1<<0,
>> @@ -1685,6 +1689,9 @@ enum netdev_priv_flags {
>>   	IFF_TX_SKB_NO_LINEAR		= BIT_ULL(31),
>>   	IFF_CHANGE_PROTO_DOWN		= BIT_ULL(32),
>>   	IFF_SEE_ALL_HWTSTAMP_REQUESTS	= BIT_ULL(33),
>> +	IFF_PROFILE_USEC		= BIT_ULL(34),
>> +	IFF_PROFILE_PKTS		= BIT_ULL(35),
>> +	IFF_PROFILE_COMPS		= BIT_ULL(36),
>>   };
>>   
>>   #define IFF_802_1Q_VLAN			IFF_802_1Q_VLAN
>> @@ -2400,6 +2407,14 @@ struct net_device {
>>   	/** @page_pools: page pools created for this netdevice */
>>   	struct hlist_head	page_pools;
>>   #endif
>> +
>> +#if IS_ENABLED(CONFIG_DIMLIB)
>> +	/* DIM profile lists for different dim cq modes */
>> +	struct dim_cq_moder *rx_eqe_profile;
>> +	struct dim_cq_moder *rx_cqe_profile;
>> +	struct dim_cq_moder *tx_eqe_profile;
>> +	struct dim_cq_moder *tx_cqe_profile;
>> +#endif
> just one pointer to a new wrapper struct, put the pointers and a flag
> field in there.
>
> netdevice.h is included by thousands of files, please use a forward
> declaration for the type and avoid including dim.h

I will update this.

Thanks for the constructive comments!


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2024-04-13 10:14 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-11 14:12 [PATCH net-next v6 0/4] ethtool: provide the dim profile fine-tuning channel Heng Qi
2024-04-11 14:12 ` [PATCH net-next v6 1/4] linux/dim: move useful macros to .h file Heng Qi
2024-04-11 14:12 ` [PATCH net-next v6 2/4] ethtool: provide customized dim profile management Heng Qi
2024-04-11 15:19   ` Brett Creeley
2024-04-12  2:07     ` Heng Qi
2024-04-12 15:33       ` Brett Creeley
2024-04-12 16:05         ` Heng Qi
2024-04-12  3:56   ` kernel test robot
2024-04-12  4:39   ` kernel test robot
2024-04-12 17:44   ` kernel test robot
2024-04-13  2:26   ` Jakub Kicinski
2024-04-13 10:14     ` Heng Qi
2024-04-13  2:27   ` Jakub Kicinski
2024-04-11 14:12 ` [PATCH net-next v6 3/4] virtio-net: refactor dim initialization/destruction Heng Qi
2024-04-11 14:12 ` [PATCH net-next v6 4/4] virtio-net: support dim profile fine-tuning Heng Qi
2024-04-11 15:23   ` Brett Creeley
2024-04-12  2:19     ` Heng Qi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).