linux-doc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v13 0/4] ethtool: provide the dim profile fine-tuning channel
@ 2024-05-09  4:47 Heng Qi
  2024-05-09  4:47 ` [PATCH net-next v13 1/4] linux/dim: move useful macros to .h file Heng Qi
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Heng Qi @ 2024-05-09  4:47 UTC (permalink / raw)
  To: netdev, virtualization
  Cc: Jakub Kicinski, David S . Miller, Paolo Abeni, Eric Dumazet,
	Jason Wang, Michael S . Tsirkin, Brett Creeley, Ratheesh Kannoth,
	Alexander Lobakin, Xuan Zhuo, Tal Gilboa, Jonathan Corbet,
	linux-doc, Maxime Chevallier, Jiri Pirko, Paul Greenwalt,
	Ahmed Zaki, Vladimir Oltean, Kory Maincent, Andrew Lunn,
	justinstitt, donald.hunter

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, the way is based on the commonly used "ethtool -C".

Please review, thank you very much!

Changelog
=====
v12->v13:
  - Rebase net-next to fix the one-line conflict.
  - Update tiny comments.
  - Config ETHTOOL_NETLINK to select DIMLIB.

v11->v12:
  - Remove the use of IS_ENABLED(DIMLIB).
  - Update Simon's htmldoc hint.

v10->v11:
  - Fix and clean up some issues from Kuba, thanks.
  - Rebase net-next/main

v9->v10:
  - Collect dim related flags/mode/work into one place.
  - Use rx_profile + tx_profile instead of four profiles.
  - Add several helps.
  - Update commit logs.

v8->v9:
  - Fix the compilation error of conflicting names of rx_profile in
    dim.h and ice driver: in dim.h, rx_profile is replaced with
    dim_rx_profile. So does tx_profile.

v7->v8:
  - Use kmemdup() instead of kzalloc()/memcpy() in dev_dim_profile_init().

v6->v7:
  - A new wrapper struct pointer is used in struct net_device.
  - Add IS_ENABLED(CONFIG_DIMLIB) to avoid compiler warnings.
  - Profile fields changed from u16 to u32.

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.

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.

Heng Qi (4):
  linux/dim: move useful macros to .h file
  ethtool: provide customized dim profile management
  dim: add new interfaces for initialization and getting results
  virtio-net: support dim profile fine-tuning

 Documentation/netlink/specs/ethtool.yaml     |  31 +++
 Documentation/networking/ethtool-netlink.rst |   4 +
 Documentation/networking/net_dim.rst         |  42 +++
 drivers/net/virtio_net.c                     |  47 +++-
 include/linux/dim.h                          | 113 ++++++++
 include/linux/ethtool.h                      |   4 +-
 include/linux/netdevice.h                    |   3 +
 include/uapi/linux/ethtool_netlink.h         |  22 ++
 lib/dim/net_dim.c                            | 144 +++++++++-
 net/Kconfig                                  |   1 +
 net/ethtool/coalesce.c                       | 263 ++++++++++++++++++-
 11 files changed, 658 insertions(+), 16 deletions(-)

-- 
2.32.0.3.g01195cf9f


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

* [PATCH net-next v13 1/4] linux/dim: move useful macros to .h file
  2024-05-09  4:47 [PATCH net-next v13 0/4] ethtool: provide the dim profile fine-tuning channel Heng Qi
@ 2024-05-09  4:47 ` Heng Qi
  2024-05-09  4:47 ` [PATCH net-next v13 2/4] ethtool: provide customized dim profile management Heng Qi
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 15+ messages in thread
From: Heng Qi @ 2024-05-09  4:47 UTC (permalink / raw)
  To: netdev, virtualization
  Cc: Jakub Kicinski, David S . Miller, Paolo Abeni, Eric Dumazet,
	Jason Wang, Michael S . Tsirkin, Brett Creeley, Ratheesh Kannoth,
	Alexander Lobakin, Xuan Zhuo, Tal Gilboa, Jonathan Corbet,
	linux-doc, Maxime Chevallier, Jiri Pirko, Paul Greenwalt,
	Ahmed Zaki, Vladimir Oltean, Kory Maincent, Andrew Lunn,
	justinstitt, donald.hunter

Useful macros will be used effectively elsewhere.
These will be utilized in subsequent patches.

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

diff --git a/include/linux/dim.h b/include/linux/dim.h
index f343bc9aa2ec..43398f5eade2 100644
--- a/include/linux/dim.h
+++ b/include/linux/dim.h
@@ -10,6 +10,13 @@
 #include <linux/types.h>
 #include <linux/workqueue.h>
 
+/* Number of DIM profiles and period mode. */
+#define NET_DIM_PARAMS_NUM_PROFILES 5
+#define NET_DIM_DEFAULT_RX_CQ_PKTS_FROM_EQE 256
+#define NET_DIM_DEFAULT_TX_CQ_PKTS_FROM_EQE 128
+#define NET_DIM_DEF_PROFILE_CQE 1
+#define NET_DIM_DEF_PROFILE_EQE 1
+
 /*
  * Number of events between DIM iterations.
  * Causes a moderation of the algorithm run.
diff --git a/lib/dim/net_dim.c b/lib/dim/net_dim.c
index 4e32f7aaac86..67d5beb34dc3 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,}, \
-- 
2.32.0.3.g01195cf9f


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

* [PATCH net-next v13 2/4] ethtool: provide customized dim profile management
  2024-05-09  4:47 [PATCH net-next v13 0/4] ethtool: provide the dim profile fine-tuning channel Heng Qi
  2024-05-09  4:47 ` [PATCH net-next v13 1/4] linux/dim: move useful macros to .h file Heng Qi
@ 2024-05-09  4:47 ` Heng Qi
  2024-05-09 23:09   ` kernel test robot
  2024-05-09  4:47 ` [PATCH net-next v13 3/4] dim: add new interfaces for initialization and getting results Heng Qi
  2024-05-09  4:47 ` [PATCH net-next v13 4/4] virtio-net: support dim profile fine-tuning Heng Qi
  3 siblings, 1 reply; 15+ messages in thread
From: Heng Qi @ 2024-05-09  4:47 UTC (permalink / raw)
  To: netdev, virtualization
  Cc: Jakub Kicinski, David S . Miller, Paolo Abeni, Eric Dumazet,
	Jason Wang, Michael S . Tsirkin, Brett Creeley, Ratheesh Kannoth,
	Alexander Lobakin, Xuan Zhuo, Tal Gilboa, Jonathan Corbet,
	linux-doc, Maxime Chevallier, Jiri Pirko, Paul Greenwalt,
	Ahmed Zaki, Vladimir Oltean, Kory Maincent, Andrew Lunn,
	justinstitt, donald.hunter

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
========
The target NIC is named ethx.

Assume that ethx only declares support for rx profile setting
(with DIM_PROFILE_RX flag set in profile_flags) and supports modification
of usec and pkt fields.

1. Query the currently customized list of the device

$ ethtool -c ethx
...
rx-profile:
{.usec =   1, .pkts = 256, .comps = n/a,},
{.usec =   8, .pkts = 256, .comps = n/a,},
{.usec =  64, .pkts = 256, .comps = n/a,},
{.usec = 128, .pkts = 256, .comps = n/a,},
{.usec = 256, .pkts = 256, .comps = n/a,}
tx-profile:   n/a

2. Tune
$ ethtool -C ethx rx-profile 1,1,n_2,n,n_3,3,n_4,4,n_n,5,n
"n" means do not modify this field.
$ ethtool -c ethx
...
rx-profile:
{.usec =   1, .pkts =   1, .comps = n/a,},
{.usec =   2, .pkts = 256, .comps = n/a,},
{.usec =   3, .pkts =   3, .comps = n/a,},
{.usec =   4, .pkts =   4, .comps = n/a,},
{.usec = 256, .pkts =   5, .comps = n/a,}
tx-profile:   n/a

3. Hint
If the device does not support some type of customized dim profiles,
the corresponding "n/a" will display.

If the "n/a" field is being modified, -EOPNOTSUPP will be reported.

Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
---
 Documentation/netlink/specs/ethtool.yaml     |  31 +++
 Documentation/networking/ethtool-netlink.rst |   4 +
 Documentation/networking/net_dim.rst         |  42 +++
 include/linux/dim.h                          |  58 ++++
 include/linux/ethtool.h                      |   4 +-
 include/linux/netdevice.h                    |   3 +
 include/uapi/linux/ethtool_netlink.h         |  22 ++
 lib/dim/net_dim.c                            |  70 +++++
 net/Kconfig                                  |   1 +
 net/ethtool/coalesce.c                       | 263 ++++++++++++++++++-
 10 files changed, 495 insertions(+), 3 deletions(-)

diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
index 00dc61358be8..6c2ab3d1c22f 100644
--- a/Documentation/netlink/specs/ethtool.yaml
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -414,6 +414,26 @@ attribute-sets:
         name: combined-count
         type: u32
 
+  -
+    name: irq-moderation
+    attributes:
+      -
+        name: usec
+        type: u32
+      -
+        name: pkts
+        type: u32
+      -
+        name: comps
+        type: u32
+  -
+    name: profile
+    attributes:
+      -
+        name: irq-moderation
+        type: nest
+        multi-attr: true
+        nested-attributes: irq-moderation
   -
     name: coalesce
     attributes:
@@ -502,6 +522,15 @@ attribute-sets:
       -
         name: tx-aggr-time-usecs
         type: u32
+      -
+        name: rx-profile
+        type: nest
+        nested-attributes: profile
+      -
+        name: tx-profile
+        type: nest
+        nested-attributes: profile
+
   -
     name: pause-stat
     attributes:
@@ -1325,6 +1354,8 @@ operations:
             - tx-aggr-max-bytes
             - tx-aggr-max-frames
             - tx-aggr-time-usecs
+            - rx-profile
+            - tx-profile
       dump: *coalesce-get-op
     -
       name: coalesce-set
diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index 8bc71f249448..11e15d2fa731 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -1040,6 +1040,8 @@ 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_PROFILE``            nested  profile of DIM, Rx
+  ``ETHTOOL_A_COALESCE_TX_PROFILE``            nested  profile of DIM, Tx
   ===========================================  ======  =======================
 
 Attributes are only included in reply if their value is not zero or the
@@ -1105,6 +1107,8 @@ 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_PROFILE``            nested  profile of DIM, Rx
+  ``ETHTOOL_A_COALESCE_TX_PROFILE``            nested  profile of DIM, Tx
   ===========================================  ======  =======================
 
 Request is rejected if it attributes declared as unsupported by driver (i.e.
diff --git a/Documentation/networking/net_dim.rst b/Documentation/networking/net_dim.rst
index 3bed9fd95336..8908fd7b0a8d 100644
--- a/Documentation/networking/net_dim.rst
+++ b/Documentation/networking/net_dim.rst
@@ -169,6 +169,48 @@ usage is not complete but it should make the outline of the usage clear.
 	...
   }
 
+
+Tuning DIM
+==========
+
+Net DIM serves a range of network devices and delivers excellent acceleration
+benefits. Yet, it has been observed that some preset configurations of DIM may
+not align seamlessly with the varying specifications of network devices, and
+this discrepancy has been identified as a factor to the suboptimal performance
+outcomes of DIM-enabled network devices, related to a mismatch in profiles.
+
+To address this issue, Net DIM introduces a per-device control to modify and
+access a device's ``rx-profile`` and ``tx-profile`` parameters:
+Assume that the target network device is named ethx, and ethx only declares
+support for RX profile setting and supports modification of ``usec`` field
+and ``pkts`` field (See the data structure:
+:c:type:`struct dim_cq_moder <dim_cq_moder>`).
+
+You can use ethtool to modify the current RX DIM profile where all
+values are 64::
+
+    $ ethtool -C ethx rx-profile 1,1,n_2,2,n_3,n,n_n,4,n_n,n,n
+
+``n`` means do not modify this field, and ``_`` separates structure
+elements of the profile array.
+
+Querying the current profiles using::
+
+    $ ethtool -c ethx
+    ...
+    rx-profile:
+    {.usec =   1, .pkts =   1, .comps = n/a,},
+    {.usec =   2, .pkts =   2, .comps = n/a,},
+    {.usec =   3, .pkts =  64, .comps = n/a,},
+    {.usec =  64, .pkts =   4, .comps = n/a,},
+    {.usec =  64, .pkts =  64, .comps = n/a,}
+    tx-profile:   n/a
+
+If the network device does not support specific fields of DIM profiles,
+the corresponding ``n/a`` will display. If the ``n/a`` field is being
+modified, error messages will be reported.
+
+
 Dynamic Interrupt Moderation (DIM) library API
 ==============================================
 
diff --git a/include/linux/dim.h b/include/linux/dim.h
index 43398f5eade2..e0f39bd85432 100644
--- a/include/linux/dim.h
+++ b/include/linux/dim.h
@@ -10,6 +10,8 @@
 #include <linux/types.h>
 #include <linux/workqueue.h>
 
+struct net_device;
+
 /* Number of DIM profiles and period mode. */
 #define NET_DIM_PARAMS_NUM_PROFILES 5
 #define NET_DIM_DEFAULT_RX_CQ_PKTS_FROM_EQE 256
@@ -45,12 +47,45 @@
  * @pkts: CQ packet counter suggestion (by DIM)
  * @comps: Completion counter
  * @cq_period_mode: CQ period count mode (from CQE/EQE)
+ * @rcu: for asynchronous kfree_rcu
  */
 struct dim_cq_moder {
 	u16 usec;
 	u16 pkts;
 	u16 comps;
 	u8 cq_period_mode;
+	struct rcu_head rcu;
+};
+
+#define DIM_PROFILE_RX		BIT(0)	/* support rx profile modification */
+#define DIM_PROFILE_TX		BIT(1)	/* support tx profile modification */
+
+#define DIM_COALESCE_USEC	BIT(0)	/* support usec field modification */
+#define DIM_COALESCE_PKTS	BIT(1)	/* support pkts field modification */
+#define DIM_COALESCE_COMPS	BIT(2)	/* support comps field modification */
+
+/**
+ * struct dim_irq_moder - Structure for irq moderation information.
+ * Used to collect irq moderation related information.
+ *
+ * @profile_flags: DIM_PROFILE_*
+ * @coal_flags: DIM_COALESCE_* for Rx and Tx
+ * @dim_rx_mode: Rx DIM period count mode: CQE or EQE
+ * @dim_tx_mode: Tx DIM period count mode: CQE or EQE
+ * @rx_profile: DIM profile list for Rx
+ * @tx_profile: DIM profile list for Tx
+ * @rx_dim_work: Rx DIM worker scheduled by net_dim()
+ * @tx_dim_work: Tx DIM worker scheduled by net_dim()
+ */
+struct dim_irq_moder {
+	u8 profile_flags;
+	u8 coal_flags;
+	u8 dim_rx_mode;
+	u8 dim_tx_mode;
+	struct dim_cq_moder __rcu *rx_profile;
+	struct dim_cq_moder __rcu *tx_profile;
+	void (*rx_dim_work)(struct work_struct *work);
+	void (*tx_dim_work)(struct work_struct *work);
 };
 
 /**
@@ -198,6 +233,29 @@ enum dim_step_result {
 	DIM_ON_EDGE,
 };
 
+/**
+ * net_dim_init_irq_moder - collect information to initialize irq moderation
+ * @dev: target network device
+ * @profile_flags: Rx or Tx profile modification capability
+ * @coal_flags: irq moderation params flags
+ * @rx_mode: CQ period mode for Rx
+ * @tx_mode: CQ period mode for Tx
+ * @rx_dim_work: Rx worker called after dim decision
+ * @tx_dim_work: Tx worker called after dim decision
+ *
+ * Return: 0 on success or a negative error code.
+ */
+int net_dim_init_irq_moder(struct net_device *dev, u8 profile_flags,
+			   u8 coal_flags, u8 rx_mode, u8 tx_mode,
+			   void (*rx_dim_work)(struct work_struct *work),
+			   void (*tx_dim_work)(struct work_struct *work));
+
+/**
+ * net_dim_free_irq_moder - free fields for irq moderation
+ * @dev: target network device
+ */
+void net_dim_free_irq_moder(struct net_device *dev);
+
 /**
  *	dim_on_top - check if current state is a good place to stop (top location)
  *	@dim: DIM context
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 6fd9107d3cc0..959196af7f5a 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -284,7 +284,9 @@ 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_PROFILE		BIT(27)
+#define ETHTOOL_COALESCE_TX_PROFILE		BIT(28)
+#define ETHTOOL_COALESCE_ALL_PARAMS		GENMASK(28, 0)
 
 #define ETHTOOL_COALESCE_USECS						\
 	(ETHTOOL_COALESCE_RX_USECS | ETHTOOL_COALESCE_TX_USECS)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index cf261fb89d73..312e5a753f6b 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2403,6 +2403,9 @@ struct net_device {
 	/** @page_pools: page pools created for this netdevice */
 	struct hlist_head	page_pools;
 #endif
+
+	/** @irq_moder: dim parameters used if IS_ENABLED(CONFIG_DIMLIB). */
+	struct dim_irq_moder	*irq_moder;
 };
 #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 f17dbe54bf5e..e1b57edb0342 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -416,12 +416,34 @@ 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 */
+	/* nest - _A_PROFILE_IRQ_MODERATION */
+	ETHTOOL_A_COALESCE_RX_PROFILE,
+	/* nest - _A_PROFILE_IRQ_MODERATION */
+	ETHTOOL_A_COALESCE_TX_PROFILE,
 
 	/* add new constants above here */
 	__ETHTOOL_A_COALESCE_CNT,
 	ETHTOOL_A_COALESCE_MAX = (__ETHTOOL_A_COALESCE_CNT - 1)
 };
 
+enum {
+	ETHTOOL_A_PROFILE_UNSPEC,
+	/* nest, _A_IRQ_MODERATION_* */
+	ETHTOOL_A_PROFILE_IRQ_MODERATION,
+	__ETHTOOL_A_PROFILE_CNT,
+	ETHTOOL_A_PROFILE_MAX = (__ETHTOOL_A_PROFILE_CNT - 1)
+};
+
+enum {
+	ETHTOOL_A_IRQ_MODERATION_UNSPEC,
+	ETHTOOL_A_IRQ_MODERATION_USEC,			/* u32 */
+	ETHTOOL_A_IRQ_MODERATION_PKTS,			/* u32 */
+	ETHTOOL_A_IRQ_MODERATION_COMPS,			/* u32 */
+
+	__ETHTOOL_A_IRQ_MODERATION_CNT,
+	ETHTOOL_A_IRQ_MODERATION_MAX = (__ETHTOOL_A_IRQ_MODERATION_CNT - 1)
+};
+
 /* PAUSE */
 
 enum {
diff --git a/lib/dim/net_dim.c b/lib/dim/net_dim.c
index 67d5beb34dc3..0cd41277c7a3 100644
--- a/lib/dim/net_dim.c
+++ b/lib/dim/net_dim.c
@@ -4,6 +4,7 @@
  */
 
 #include <linux/dim.h>
+#include <linux/rtnetlink.h>
 
 /*
  * Net DIM profiles:
@@ -95,6 +96,75 @@ net_dim_get_def_tx_moderation(u8 cq_period_mode)
 }
 EXPORT_SYMBOL(net_dim_get_def_tx_moderation);
 
+int net_dim_init_irq_moder(struct net_device *dev, u8 profile_flags,
+			   u8 coal_flags, u8 rx_mode, u8 tx_mode,
+			   void (*rx_dim_work)(struct work_struct *work),
+			   void (*tx_dim_work)(struct work_struct *work))
+{
+	struct dim_cq_moder *rxp = NULL, *txp;
+	struct dim_irq_moder *moder;
+	int len;
+
+	dev->irq_moder = kzalloc(sizeof(*dev->irq_moder), GFP_KERNEL);
+	if (!dev->irq_moder)
+		return -ENOMEM;
+
+	moder = dev->irq_moder;
+	len = NET_DIM_PARAMS_NUM_PROFILES * sizeof(*moder->rx_profile);
+
+	moder->coal_flags = coal_flags;
+	moder->profile_flags = profile_flags;
+
+	if (profile_flags & DIM_PROFILE_RX) {
+		moder->rx_dim_work = rx_dim_work;
+		moder->dim_rx_mode = rx_mode;
+		rxp = kmemdup(rx_profile[rx_mode], len, GFP_KERNEL);
+		if (!rxp)
+			goto free_moder;
+
+		rcu_assign_pointer(moder->rx_profile, rxp);
+	}
+
+	if (profile_flags & DIM_PROFILE_TX) {
+		moder->tx_dim_work = tx_dim_work;
+		moder->dim_tx_mode = tx_mode;
+		txp = kmemdup(tx_profile[tx_mode], len, GFP_KERNEL);
+		if (!txp)
+			goto free_rxp;
+
+		rcu_assign_pointer(moder->tx_profile, txp);
+	}
+
+	return 0;
+
+free_rxp:
+	kfree(rxp);
+free_moder:
+	kfree(moder);
+	return -ENOMEM;
+}
+EXPORT_SYMBOL(net_dim_init_irq_moder);
+
+/* RTNL lock is held. */
+void net_dim_free_irq_moder(struct net_device *dev)
+{
+	struct dim_cq_moder *rxp, *txp;
+
+	if (!dev->irq_moder)
+		return;
+
+	rxp = rtnl_dereference(dev->irq_moder->rx_profile);
+	txp = rtnl_dereference(dev->irq_moder->tx_profile);
+
+	rcu_assign_pointer(dev->irq_moder->rx_profile, NULL);
+	rcu_assign_pointer(dev->irq_moder->tx_profile, NULL);
+
+	kfree_rcu(rxp, rcu);
+	kfree_rcu(txp, rcu);
+	kfree(dev->irq_moder);
+}
+EXPORT_SYMBOL(net_dim_free_irq_moder);
+
 static int net_dim_step(struct dim *dim)
 {
 	if (dim->tired == (NET_DIM_PARAMS_NUM_PROFILES * 2))
diff --git a/net/Kconfig b/net/Kconfig
index f0a8692496ff..e5e66ba3fdad 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -502,6 +502,7 @@ config FAILOVER
 
 config ETHTOOL_NETLINK
 	bool "Netlink interface for ethtool"
+	select DIMLIB
 	default y
 	help
 	  An alternative userspace interface for ethtool based on generic
diff --git a/net/ethtool/coalesce.c b/net/ethtool/coalesce.c
index 83112c1a71ae..1cfd9f6fdcd4 100644
--- a/net/ethtool/coalesce.c
+++ b/net/ethtool/coalesce.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 
+#include <linux/dim.h>
 #include "netlink.h"
 #include "common.h"
 
@@ -82,6 +83,14 @@ 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) + /* _PROFILE_IRQ_MODERATION, nest */
+		      nla_total_size(sizeof(u32)) + /* _IRQ_MODERATION_USEC */
+		      nla_total_size(sizeof(u32)) + /* _IRQ_MODERATION_PKTS */
+		      nla_total_size(sizeof(u32));  /* _IRQ_MODERATION_COMPS */
+
+	int total_modersz = nla_total_size(0) +  /* _{R,T}X_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 +117,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 * 2;		/* _{R,T}X_PROFILE */
 }
 
 static bool coalesce_put_u32(struct sk_buff *skb, u16 attr_type, u32 val,
@@ -127,14 +137,84 @@ 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_PROFILE
+ * @profile: data passed to userspace
+ * @coal_flags: modifiable parameters supported by the driver
+ *
+ * Put a dim profile nest attribute. Refer to ETHTOOL_A_PROFILE_IRQ_MODERATION.
+ *
+ * Return: 0 on success or a negative error code.
+ */
+static int coalesce_put_profile(struct sk_buff *skb, u16 attr_type,
+				const struct dim_cq_moder *profile,
+				u8 coal_flags)
+{
+	struct nlattr *profile_attr, *moder_attr;
+	int i, ret;
+
+	if (!profile || !coal_flags)
+		return 0;
+
+	profile_attr = nla_nest_start(skb, attr_type);
+	if (!profile_attr)
+		return -EMSGSIZE;
+
+	for (i = 0; i < NET_DIM_PARAMS_NUM_PROFILES; i++) {
+		moder_attr = nla_nest_start(skb,
+					    ETHTOOL_A_PROFILE_IRQ_MODERATION);
+		if (!moder_attr) {
+			ret = -EMSGSIZE;
+			goto cancel_profile;
+		}
+
+		if (coal_flags & DIM_COALESCE_USEC) {
+			ret = nla_put_u32(skb, ETHTOOL_A_IRQ_MODERATION_USEC,
+					  profile[i].usec);
+			if (ret)
+				goto cancel_moder;
+		}
+
+		if (coal_flags & DIM_COALESCE_PKTS) {
+			ret = nla_put_u32(skb, ETHTOOL_A_IRQ_MODERATION_PKTS,
+					  profile[i].pkts);
+			if (ret)
+				goto cancel_moder;
+		}
+
+		if (coal_flags & DIM_COALESCE_COMPS) {
+			ret = nla_put_u32(skb, ETHTOOL_A_IRQ_MODERATION_COMPS,
+					  profile[i].comps);
+			if (ret)
+				goto cancel_moder;
+		}
+
+		nla_nest_end(skb, moder_attr);
+	}
+
+	nla_nest_end(skb, profile_attr);
+
+	return 0;
+
+cancel_moder:
+	nla_nest_cancel(skb, moder_attr);
+cancel_profile:
+	nla_nest_cancel(skb, profile_attr);
+	return ret;
+}
+
 static int coalesce_fill_reply(struct sk_buff *skb,
 			       const struct ethnl_req_info *req_base,
 			       const struct ethnl_reply_data *reply_base)
 {
 	const struct coalesce_reply_data *data = COALESCE_REPDATA(reply_base);
 	const struct kernel_ethtool_coalesce *kcoal = &data->kernel_coalesce;
+	struct dim_irq_moder *moder = req_base->dev->irq_moder;
 	const struct ethtool_coalesce *coal = &data->coalesce;
 	u32 supported = data->supported_params;
+	int ret = 0;
 
 	if (coalesce_put_u32(skb, ETHTOOL_A_COALESCE_RX_USECS,
 			     coal->rx_coalesce_usecs, supported) ||
@@ -192,11 +272,41 @@ static int coalesce_fill_reply(struct sk_buff *skb,
 			     kcoal->tx_aggr_time_usecs, supported))
 		return -EMSGSIZE;
 
-	return 0;
+	if (!moder)
+		return 0;
+
+	rcu_read_lock();
+	if (moder->profile_flags & DIM_PROFILE_RX) {
+		ret = coalesce_put_profile(skb, ETHTOOL_A_COALESCE_RX_PROFILE,
+					   rcu_dereference(moder->rx_profile),
+					   moder->coal_flags);
+		if (ret)
+			goto out;
+	}
+
+	if (moder->profile_flags & DIM_PROFILE_TX)
+		ret = coalesce_put_profile(skb, ETHTOOL_A_COALESCE_TX_PROFILE,
+					   rcu_dereference(moder->tx_profile),
+					   moder->coal_flags);
+
+out:
+	rcu_read_unlock();
+	return ret;
 }
 
 /* COALESCE_SET */
 
+static const struct nla_policy coalesce_irq_moderation_policy[] = {
+	[ETHTOOL_A_IRQ_MODERATION_USEC]	= { .type = NLA_U32 },
+	[ETHTOOL_A_IRQ_MODERATION_PKTS]	= { .type = NLA_U32 },
+	[ETHTOOL_A_IRQ_MODERATION_COMPS] = { .type = NLA_U32 },
+};
+
+static const struct nla_policy coalesce_profile_policy[] = {
+	[ETHTOOL_A_PROFILE_IRQ_MODERATION] =
+		NLA_POLICY_NESTED(coalesce_irq_moderation_policy),
+};
+
 const struct nla_policy ethnl_coalesce_set_policy[] = {
 	[ETHTOOL_A_COALESCE_HEADER]		=
 		NLA_POLICY_NESTED(ethnl_header_policy),
@@ -227,6 +337,10 @@ const struct nla_policy ethnl_coalesce_set_policy[] = {
 	[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_PROFILE] =
+		NLA_POLICY_NESTED(coalesce_profile_policy),
+	[ETHTOOL_A_COALESCE_TX_PROFILE] =
+		NLA_POLICY_NESTED(coalesce_profile_policy),
 };
 
 static int
@@ -234,6 +348,7 @@ ethnl_set_coalesce_validate(struct ethnl_req_info *req_info,
 			    struct genl_info *info)
 {
 	const struct ethtool_ops *ops = req_info->dev->ethtool_ops;
+	struct dim_irq_moder *irq_moder = req_info->dev->irq_moder;
 	struct nlattr **tb = info->attrs;
 	u32 supported_params;
 	u16 a;
@@ -243,6 +358,12 @@ ethnl_set_coalesce_validate(struct ethnl_req_info *req_info,
 
 	/* make sure that only supported parameters are present */
 	supported_params = ops->supported_coalesce_params;
+	if (irq_moder && irq_moder->profile_flags & DIM_PROFILE_RX)
+		supported_params |= ETHTOOL_COALESCE_RX_PROFILE;
+
+	if (irq_moder && irq_moder->profile_flags & DIM_PROFILE_TX)
+		supported_params |= ETHTOOL_COALESCE_TX_PROFILE;
+
 	for (a = ETHTOOL_A_COALESCE_RX_USECS; a < __ETHTOOL_A_COALESCE_CNT; a++)
 		if (tb[a] && !(supported_params & attr_to_mask(a))) {
 			NL_SET_ERR_MSG_ATTR(info->extack, tb[a],
@@ -253,6 +374,128 @@ ethnl_set_coalesce_validate(struct ethnl_req_info *req_info,
 	return 1;
 }
 
+/**
+ * ethnl_update_irq_moder - update a specific field in the given profile
+ * @irq_moder: place that collects dim related information
+ * @irq_field: field in profile to modify
+ * @attr_type: attr type ETHTOOL_A_IRQ_MODERATION_*
+ * @tb: netlink attribute with new values or null
+ * @coal_bit: DIM_COALESCE_* bit from coal_flags
+ * @extack: netlink extended ack
+ *
+ * Return: 0 on success or a negative error code.
+ */
+static int ethnl_update_irq_moder(struct dim_irq_moder *irq_moder,
+				  u16 *irq_field, u16 attr_type,
+				  struct nlattr **tb, u8 coal_bit,
+				  struct netlink_ext_ack *extack)
+{
+	int ret = 0;
+
+	if (!tb[attr_type])
+		return 0;
+
+	if (irq_moder->coal_flags & coal_bit) {
+		*irq_field = nla_get_u32(tb[attr_type]);
+	} else {
+		NL_SET_BAD_ATTR(extack, tb[attr_type]);
+		ret = -EOPNOTSUPP;
+	}
+
+	return ret;
+}
+
+/**
+ * ethnl_update_profile - get a profile nest with child nests from userspace.
+ * @dev: netdevice to update the profile
+ * @dst: profile get from the driver and modified by ethnl_update_profile.
+ * @nests: nest attr ETHTOOL_A_COALESCE_*X_PROFILE to set profile.
+ * @extack: Netlink extended ack
+ *
+ * Layout of nests:
+ *   Nested ETHTOOL_A_COALESCE_*X_PROFILE attr
+ *     Nested ETHTOOL_A_PROFILE_IRQ_MODERATION attr
+ *       ETHTOOL_A_IRQ_MODERATION_USEC attr
+ *       ETHTOOL_A_IRQ_MODERATION_PKTS attr
+ *       ETHTOOL_A_IRQ_MODERATION_COMPS attr
+ *     ...
+ *     Nested ETHTOOL_A_PROFILE_IRQ_MODERATION attr
+ *       ETHTOOL_A_IRQ_MODERATION_USEC attr
+ *       ETHTOOL_A_IRQ_MODERATION_PKTS attr
+ *       ETHTOOL_A_IRQ_MODERATION_COMPS attr
+ *
+ * Return: 0 on success or a negative error code.
+ */
+static int ethnl_update_profile(struct net_device *dev,
+				struct dim_cq_moder __rcu **dst,
+				const struct nlattr *nests,
+				struct netlink_ext_ack *extack)
+{
+	int len_irq_moder = ARRAY_SIZE(coalesce_irq_moderation_policy);
+	struct nlattr *tb[ARRAY_SIZE(coalesce_irq_moderation_policy)];
+	struct dim_irq_moder *irq_moder = dev->irq_moder;
+	struct dim_cq_moder *new_profile, *old_profile;
+	int ret, rem, i = 0, len;
+	struct nlattr *nest;
+
+	if (!nests)
+		return 0;
+
+	if (!*dst)
+		return -EOPNOTSUPP;
+
+	old_profile = rtnl_dereference(*dst);
+	len = NET_DIM_PARAMS_NUM_PROFILES * sizeof(*old_profile);
+	new_profile = kmemdup(old_profile, len, GFP_KERNEL);
+	if (!new_profile)
+		return -ENOMEM;
+
+	nla_for_each_nested_type(nest, ETHTOOL_A_PROFILE_IRQ_MODERATION,
+				 nests, rem) {
+		ret = nla_parse_nested(tb, len_irq_moder - 1, nest,
+				       coalesce_irq_moderation_policy,
+				       extack);
+		if (ret)
+			goto err_out;
+
+		ret = ethnl_update_irq_moder(irq_moder, &new_profile[i].usec,
+					     ETHTOOL_A_IRQ_MODERATION_USEC,
+					     tb, DIM_COALESCE_USEC,
+					     extack);
+		if (ret)
+			goto err_out;
+
+		ret = ethnl_update_irq_moder(irq_moder, &new_profile[i].pkts,
+					     ETHTOOL_A_IRQ_MODERATION_PKTS,
+					     tb, DIM_COALESCE_PKTS,
+					     extack);
+		if (ret)
+			goto err_out;
+
+		ret = ethnl_update_irq_moder(irq_moder, &new_profile[i].comps,
+					     ETHTOOL_A_IRQ_MODERATION_COMPS,
+					     tb, DIM_COALESCE_COMPS,
+					     extack);
+		if (ret)
+			goto err_out;
+
+		i++;
+	}
+
+	/* After the profile is modified, dim itself is a dynamic
+	 * mechanism and will quickly fit to the appropriate
+	 * coalescing parameters according to the new profile.
+	 */
+	rcu_assign_pointer(*dst, new_profile);
+	kfree_rcu(old_profile, rcu);
+
+	return 0;
+
+err_out:
+	kfree(new_profile);
+	return ret;
+}
+
 static int
 __ethnl_set_coalesce(struct ethnl_req_info *req_info, struct genl_info *info,
 		     bool *dual_change)
@@ -317,6 +560,22 @@ __ethnl_set_coalesce(struct ethnl_req_info *req_info, struct genl_info *info,
 	ethnl_update_u32(&kernel_coalesce.tx_aggr_time_usecs,
 			 tb[ETHTOOL_A_COALESCE_TX_AGGR_TIME_USECS], &mod);
 
+	if (dev->irq_moder && dev->irq_moder->profile_flags & DIM_PROFILE_RX) {
+		ret = ethnl_update_profile(dev, &dev->irq_moder->rx_profile,
+					   tb[ETHTOOL_A_COALESCE_RX_PROFILE],
+					   info->extack);
+		if (ret < 0)
+			return ret;
+	}
+
+	if (dev->irq_moder && dev->irq_moder->profile_flags & DIM_PROFILE_TX) {
+		ret = ethnl_update_profile(dev, &dev->irq_moder->tx_profile,
+					   tb[ETHTOOL_A_COALESCE_TX_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);
-- 
2.32.0.3.g01195cf9f


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

* [PATCH net-next v13 3/4] dim: add new interfaces for initialization and getting results
  2024-05-09  4:47 [PATCH net-next v13 0/4] ethtool: provide the dim profile fine-tuning channel Heng Qi
  2024-05-09  4:47 ` [PATCH net-next v13 1/4] linux/dim: move useful macros to .h file Heng Qi
  2024-05-09  4:47 ` [PATCH net-next v13 2/4] ethtool: provide customized dim profile management Heng Qi
@ 2024-05-09  4:47 ` Heng Qi
  2024-05-09  4:47 ` [PATCH net-next v13 4/4] virtio-net: support dim profile fine-tuning Heng Qi
  3 siblings, 0 replies; 15+ messages in thread
From: Heng Qi @ 2024-05-09  4:47 UTC (permalink / raw)
  To: netdev, virtualization
  Cc: Jakub Kicinski, David S . Miller, Paolo Abeni, Eric Dumazet,
	Jason Wang, Michael S . Tsirkin, Brett Creeley, Ratheesh Kannoth,
	Alexander Lobakin, Xuan Zhuo, Tal Gilboa, Jonathan Corbet,
	linux-doc, Maxime Chevallier, Jiri Pirko, Paul Greenwalt,
	Ahmed Zaki, Vladimir Oltean, Kory Maincent, Andrew Lunn,
	justinstitt, donald.hunter

DIM-related mode and work have been collected in one same place,
so new interfaces are added to provide convenience.

Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
---
 include/linux/dim.h | 48 ++++++++++++++++++++++++++++++++
 lib/dim/net_dim.c   | 68 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 116 insertions(+)

diff --git a/include/linux/dim.h b/include/linux/dim.h
index e0f39bd85432..1b581ff25a15 100644
--- a/include/linux/dim.h
+++ b/include/linux/dim.h
@@ -256,6 +256,54 @@ int net_dim_init_irq_moder(struct net_device *dev, u8 profile_flags,
  */
 void net_dim_free_irq_moder(struct net_device *dev);
 
+/**
+ * net_dim_setting - initialize DIM's cq mode and schedule worker
+ * @dev: target network device
+ * @dim: DIM context
+ * @is_tx: true indicates the tx direction, false indicates the rx direction
+ */
+void net_dim_setting(struct net_device *dev, struct dim *dim, bool is_tx);
+
+/**
+ * net_dim_work_cancel - synchronously cancel dim's worker
+ * @dim: DIM context
+ */
+void net_dim_work_cancel(struct dim *dim);
+
+/**
+ * net_dim_get_rx_irq_moder - get DIM rx results based on profile_ix
+ * @dev: target network device
+ * @dim: DIM context
+ *
+ * Return: DIM irq moderation
+ */
+struct dim_cq_moder
+net_dim_get_rx_irq_moder(struct net_device *dev, struct dim *dim);
+
+/**
+ * net_dim_get_tx_irq_moder - get DIM tx results based on profile_ix
+ * @dev: target network device
+ * @dim: DIM context
+ *
+ * Return: DIM irq moderation
+ */
+struct dim_cq_moder
+net_dim_get_tx_irq_moder(struct net_device *dev, struct dim *dim);
+
+/**
+ * net_dim_set_rx_mode - set DIM rx cq mode
+ * @dev: target network device
+ * @rx_mode: target rx cq mode
+ */
+void net_dim_set_rx_mode(struct net_device *dev, u8 rx_mode);
+
+/**
+ * net_dim_set_tx_mode - set DIM tx cq mode
+ * @dev: target network device
+ * @tx_mode: target tx cq mode
+ */
+void net_dim_set_tx_mode(struct net_device *dev, u8 tx_mode);
+
 /**
  *	dim_on_top - check if current state is a good place to stop (top location)
  *	@dim: DIM context
diff --git a/lib/dim/net_dim.c b/lib/dim/net_dim.c
index 0cd41277c7a3..d7e7028e9b19 100644
--- a/lib/dim/net_dim.c
+++ b/lib/dim/net_dim.c
@@ -165,6 +165,74 @@ void net_dim_free_irq_moder(struct net_device *dev)
 }
 EXPORT_SYMBOL(net_dim_free_irq_moder);
 
+void net_dim_setting(struct net_device *dev, struct dim *dim, bool is_tx)
+{
+	struct dim_irq_moder *irq_moder = dev->irq_moder;
+
+	if (!irq_moder)
+		return;
+
+	if (is_tx) {
+		INIT_WORK(&dim->work, irq_moder->tx_dim_work);
+		dim->mode = READ_ONCE(irq_moder->dim_tx_mode);
+		return;
+	}
+
+	INIT_WORK(&dim->work, irq_moder->rx_dim_work);
+	dim->mode = READ_ONCE(irq_moder->dim_rx_mode);
+}
+EXPORT_SYMBOL(net_dim_setting);
+
+void net_dim_work_cancel(struct dim *dim)
+{
+	cancel_work_sync(&dim->work);
+}
+EXPORT_SYMBOL(net_dim_work_cancel);
+
+struct dim_cq_moder net_dim_get_rx_irq_moder(struct net_device *dev,
+					     struct dim *dim)
+{
+	struct dim_cq_moder res, *profile;
+
+	rcu_read_lock();
+	profile = rcu_dereference(dev->irq_moder->rx_profile);
+	res = profile[dim->profile_ix];
+	rcu_read_unlock();
+
+	res.cq_period_mode = dim->mode;
+
+	return res;
+}
+EXPORT_SYMBOL(net_dim_get_rx_irq_moder);
+
+struct dim_cq_moder net_dim_get_tx_irq_moder(struct net_device *dev,
+					     struct dim *dim)
+{
+	struct dim_cq_moder res, *profile;
+
+	rcu_read_lock();
+	profile = rcu_dereference(dev->irq_moder->tx_profile);
+	res = profile[dim->profile_ix];
+	rcu_read_unlock();
+
+	res.cq_period_mode = dim->mode;
+
+	return res;
+}
+EXPORT_SYMBOL(net_dim_get_tx_irq_moder);
+
+void net_dim_set_rx_mode(struct net_device *dev, u8 rx_mode)
+{
+	WRITE_ONCE(dev->irq_moder->dim_rx_mode, rx_mode);
+}
+EXPORT_SYMBOL(net_dim_set_rx_mode);
+
+void net_dim_set_tx_mode(struct net_device *dev, u8 tx_mode)
+{
+	WRITE_ONCE(dev->irq_moder->dim_tx_mode, tx_mode);
+}
+EXPORT_SYMBOL(net_dim_set_tx_mode);
+
 static int net_dim_step(struct dim *dim)
 {
 	if (dim->tired == (NET_DIM_PARAMS_NUM_PROFILES * 2))
-- 
2.32.0.3.g01195cf9f


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

* [PATCH net-next v13 4/4] virtio-net: support dim profile fine-tuning
  2024-05-09  4:47 [PATCH net-next v13 0/4] ethtool: provide the dim profile fine-tuning channel Heng Qi
                   ` (2 preceding siblings ...)
  2024-05-09  4:47 ` [PATCH net-next v13 3/4] dim: add new interfaces for initialization and getting results Heng Qi
@ 2024-05-09  4:47 ` Heng Qi
  3 siblings, 0 replies; 15+ messages in thread
From: Heng Qi @ 2024-05-09  4:47 UTC (permalink / raw)
  To: netdev, virtualization
  Cc: Jakub Kicinski, David S . Miller, Paolo Abeni, Eric Dumazet,
	Jason Wang, Michael S . Tsirkin, Brett Creeley, Ratheesh Kannoth,
	Alexander Lobakin, Xuan Zhuo, Tal Gilboa, Jonathan Corbet,
	linux-doc, Maxime Chevallier, Jiri Pirko, Paul Greenwalt,
	Ahmed Zaki, Vladimir Oltean, Kory Maincent, Andrew Lunn,
	justinstitt, donald.hunter

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
initialize and query dim results from a customized profile list.

Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
---
 drivers/net/virtio_net.c | 47 ++++++++++++++++++++++++++++++++++------
 1 file changed, 40 insertions(+), 7 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 218a446c4c27..9e0624ae962d 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2447,7 +2447,7 @@ static int virtnet_open(struct net_device *dev)
 
 	for (i--; i >= 0; i--) {
 		virtnet_disable_queue_pair(vi, i);
-		cancel_work_sync(&vi->rq[i].dim.work);
+		net_dim_work_cancel(&vi->rq[i].dim);
 	}
 
 	return err;
@@ -2613,7 +2613,7 @@ static int virtnet_rx_resize(struct virtnet_info *vi,
 
 	if (running) {
 		napi_disable(&rq->napi);
-		cancel_work_sync(&rq->dim.work);
+		net_dim_work_cancel(&rq->dim);
 	}
 
 	err = virtqueue_resize(rq->vq, ring_num, virtnet_rq_unmap_free_buf);
@@ -2873,7 +2873,7 @@ static int virtnet_close(struct net_device *dev)
 
 	for (i = 0; i < vi->max_queue_pairs; i++) {
 		virtnet_disable_queue_pair(vi, i);
-		cancel_work_sync(&vi->rq[i].dim.work);
+		net_dim_work_cancel(&vi->rq[i].dim);
 	}
 
 	return 0;
@@ -4403,7 +4403,7 @@ static void virtnet_rx_dim_work(struct work_struct *work)
 	if (!rq->dim_enabled)
 		goto out;
 
-	update_moder = net_dim_get_rx_moderation(dim->mode, dim->profile_ix);
+	update_moder = net_dim_get_rx_irq_moder(dev, dim);
 	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,
@@ -5101,6 +5101,36 @@ static void virtnet_tx_timeout(struct net_device *dev, unsigned int txqueue)
 		   jiffies_to_usecs(jiffies - READ_ONCE(txq->trans_start)));
 }
 
+static int virtnet_init_irq_moder(struct virtnet_info *vi)
+{
+	u8 profile_flags = 0, coal_flags = 0;
+	int ret, i;
+
+	profile_flags |= DIM_PROFILE_RX;
+	coal_flags |= DIM_COALESCE_USEC | DIM_COALESCE_PKTS;
+	ret = net_dim_init_irq_moder(vi->dev, profile_flags, coal_flags,
+				     DIM_CQ_PERIOD_MODE_START_FROM_EQE,
+				     0, virtnet_rx_dim_work, NULL);
+
+	if (ret)
+		return ret;
+
+	for (i = 0; i < vi->max_queue_pairs; i++)
+		net_dim_setting(vi->dev, &vi->rq[i].dim, false);
+
+	return 0;
+}
+
+static void virtnet_free_irq_moder(struct virtnet_info *vi)
+{
+	if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_VQ_NOTF_COAL))
+		return;
+
+	rtnl_lock();
+	net_dim_free_irq_moder(vi->dev);
+	rtnl_unlock();
+}
+
 static const struct net_device_ops virtnet_netdev = {
 	.ndo_open            = virtnet_open,
 	.ndo_stop   	     = virtnet_close,
@@ -5380,9 +5410,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));
@@ -5803,6 +5830,10 @@ static int virtnet_probe(struct virtio_device *vdev)
 		for (i = 0; i < vi->max_queue_pairs; i++)
 			if (vi->sq[i].napi.weight)
 				vi->sq[i].intr_coal.max_packets = 1;
+
+		err = virtnet_init_irq_moder(vi);
+		if (err)
+			goto free;
 	}
 
 #ifdef CONFIG_SYSFS
@@ -5954,6 +5985,8 @@ static void virtnet_remove(struct virtio_device *vdev)
 	disable_rx_mode_work(vi);
 	flush_work(&vi->rx_mode_work);
 
+	virtnet_free_irq_moder(vi);
+
 	unregister_netdev(vi->dev);
 
 	net_failover_destroy(vi->failover);
-- 
2.32.0.3.g01195cf9f


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

* Re: [PATCH net-next v13 2/4] ethtool: provide customized dim profile management
  2024-05-09  4:47 ` [PATCH net-next v13 2/4] ethtool: provide customized dim profile management Heng Qi
@ 2024-05-09 23:09   ` kernel test robot
  2024-05-12 16:36     ` Heng Qi
  0 siblings, 1 reply; 15+ messages in thread
From: kernel test robot @ 2024-05-09 23:09 UTC (permalink / raw)
  To: Heng Qi, netdev, virtualization
  Cc: llvm, oe-kbuild-all, Jakub Kicinski, David S . Miller,
	Paolo Abeni, Eric Dumazet, Jason Wang, Michael S . Tsirkin,
	Brett Creeley, Ratheesh Kannoth, Alexander Lobakin, Xuan Zhuo,
	Tal Gilboa, Jonathan Corbet, linux-doc, Maxime Chevallier,
	Jiri Pirko, Paul Greenwalt, Ahmed Zaki, Vladimir Oltean,
	Kory Maincent, Andrew Lunn, justinstitt, donald.hunter

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/20240509-125007
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240509044747.101237-3-hengqi%40linux.alibaba.com
patch subject: [PATCH net-next v13 2/4] ethtool: provide customized dim profile management
config: arm-randconfig-002-20240510 (https://download.01.org/0day-ci/archive/20240510/202405100654.5PbLQXnL-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project b910bebc300dafb30569cecc3017b446ea8eafa0)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240510/202405100654.5PbLQXnL-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/202405100654.5PbLQXnL-lkp@intel.com/

All errors (new ones prefixed by >>):

>> ld.lld: error: undefined symbol: lockdep_rtnl_is_held
   >>> referenced by net_dim.c
   >>>               lib/dim/net_dim.o:(net_dim_free_irq_moder) in archive vmlinux.a
   >>> referenced by net_dim.c
   >>>               lib/dim/net_dim.o:(net_dim_free_irq_moder) in archive vmlinux.a

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

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

* Re: [PATCH net-next v13 2/4] ethtool: provide customized dim profile management
  2024-05-09 23:09   ` kernel test robot
@ 2024-05-12 16:36     ` Heng Qi
  2024-05-13 14:22       ` Jakub Kicinski
  0 siblings, 1 reply; 15+ messages in thread
From: Heng Qi @ 2024-05-12 16:36 UTC (permalink / raw)
  To: kernel test robot
  Cc: llvm, oe-kbuild-all, Jakub Kicinski, David S . Miller,
	Paolo Abeni, Eric Dumazet, Jason Wang, Michael S . Tsirkin,
	Brett Creeley, Ratheesh Kannoth, Alexander Lobakin, Xuan Zhuo,
	Tal Gilboa, Jonathan Corbet, linux-doc, Maxime Chevallier,
	Jiri Pirko, Paul Greenwalt, Ahmed Zaki, Vladimir Oltean,
	Kory Maincent, Andrew Lunn, justinstitt, donald.hunter, netdev,
	virtualization

On Fri, 10 May 2024 07:09:52 +0800, kernel test robot <lkp@intel.com> wrote:
> 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/20240509-125007
> base:   net-next/main
> patch link:    https://lore.kernel.org/r/20240509044747.101237-3-hengqi%40linux.alibaba.com
> patch subject: [PATCH net-next v13 2/4] ethtool: provide customized dim profile management
> config: arm-randconfig-002-20240510 (https://download.01.org/0day-ci/archive/20240510/202405100654.5PbLQXnL-lkp@intel.com/config)
> compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project b910bebc300dafb30569cecc3017b446ea8eafa0)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240510/202405100654.5PbLQXnL-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/202405100654.5PbLQXnL-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
> >> ld.lld: error: undefined symbol: lockdep_rtnl_is_held

This failed use case seems to come from this series triggering a problem that
has not been triggered historically, namely lockdep_rtnl_is_held() is not called
in an environment where CONFIG_NET is not configured and CONFIG_PROVE_LOCKING is
configured:
  If CONFIG_PROVE_LOCKING is configured as Y and CONFIG_NET is n, then
  lockdep_rtnl_is_held is in an undefined state at this time.

So I think we should declare "CONFIG_PROVE_LOCKING depends on CONFIG_NET".
How do you think?

Thanks!

>    >>> referenced by net_dim.c
>    >>>               lib/dim/net_dim.o:(net_dim_free_irq_moder) in archive vmlinux.a
>    >>> referenced by net_dim.c
>    >>>               lib/dim/net_dim.o:(net_dim_free_irq_moder) in archive vmlinux.a
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH net-next v13 2/4] ethtool: provide customized dim profile management
  2024-05-12 16:36     ` Heng Qi
@ 2024-05-13 14:22       ` Jakub Kicinski
  2024-05-13 14:52         ` Heng Qi
  0 siblings, 1 reply; 15+ messages in thread
From: Jakub Kicinski @ 2024-05-13 14:22 UTC (permalink / raw)
  To: Heng Qi
  Cc: kernel test robot, llvm, oe-kbuild-all, David S . Miller,
	Paolo Abeni, Eric Dumazet, Jason Wang, Michael S . Tsirkin,
	Brett Creeley, Ratheesh Kannoth, Alexander Lobakin, Xuan Zhuo,
	Tal Gilboa, Jonathan Corbet, linux-doc, Maxime Chevallier,
	Jiri Pirko, Paul Greenwalt, Ahmed Zaki, Vladimir Oltean,
	Kory Maincent, Andrew Lunn, justinstitt, donald.hunter, netdev,
	virtualization

On Mon, 13 May 2024 00:36:58 +0800 Heng Qi wrote:
> This failed use case seems to come from this series triggering a problem that
> has not been triggered historically, namely lockdep_rtnl_is_held() is not called
> in an environment where CONFIG_NET is not configured and CONFIG_PROVE_LOCKING is
> configured:
>   If CONFIG_PROVE_LOCKING is configured as Y and CONFIG_NET is n, then
>   lockdep_rtnl_is_held is in an undefined state at this time.
> 
> So I think we should declare "CONFIG_PROVE_LOCKING depends on CONFIG_NET".
> How do you think?

Doesn't sound right, can we instead make building lib/dim/net_dim.c
dependent on CONFIG_NET? Untested but I'm thinking something like:

diff --git a/lib/dim/Makefile b/lib/dim/Makefile
index c4cc4026c451..c02c306e2975 100644
--- a/lib/dim/Makefile
+++ b/lib/dim/Makefile
@@ -4,4 +4,8 @@
 
 obj-$(CONFIG_DIMLIB) += dimlib.o
 
-dimlib-objs := dim.o net_dim.o rdma_dim.o
+dimlib-objs := dim.o rdma_dim.o
+
+ifeq ($(CONFIG_NET),y)
+dimlib-objs += net_dim.o
+endif



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

* Re: [PATCH net-next v13 2/4] ethtool: provide customized dim profile management
  2024-05-13 14:22       ` Jakub Kicinski
@ 2024-05-13 14:52         ` Heng Qi
  2024-05-13 15:24           ` Jakub Kicinski
  0 siblings, 1 reply; 15+ messages in thread
From: Heng Qi @ 2024-05-13 14:52 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: kernel test robot, llvm, oe-kbuild-all, David S . Miller,
	Paolo Abeni, Eric Dumazet, Jason Wang, Michael S . Tsirkin,
	Brett Creeley, Ratheesh Kannoth, Alexander Lobakin, Xuan Zhuo,
	Tal Gilboa, Jonathan Corbet, linux-doc, Maxime Chevallier,
	Jiri Pirko, Paul Greenwalt, Ahmed Zaki, Vladimir Oltean,
	Kory Maincent, Andrew Lunn, justinstitt, donald.hunter, netdev,
	virtualization

On Mon, 13 May 2024 07:22:49 -0700, Jakub Kicinski <kuba@kernel.org> wrote:
> On Mon, 13 May 2024 00:36:58 +0800 Heng Qi wrote:
> > This failed use case seems to come from this series triggering a problem that
> > has not been triggered historically, namely lockdep_rtnl_is_held() is not called
> > in an environment where CONFIG_NET is not configured and CONFIG_PROVE_LOCKING is
> > configured:
> >   If CONFIG_PROVE_LOCKING is configured as Y and CONFIG_NET is n, then
> >   lockdep_rtnl_is_held is in an undefined state at this time.
> > 
> > So I think we should declare "CONFIG_PROVE_LOCKING depends on CONFIG_NET".
> > How do you think?
> 
> Doesn't sound right, `can we instead make building lib/dim/net_dim.c

Why? IIUC, the reason is that if CONFIG_NET is not set to Y, the net/core
directory will not be compiled, so the lockdep_rtnl_is_held symbol is not
present.

> dependent on CONFIG_NET? Untested but I'm thinking something like:
> 
> diff --git a/lib/dim/Makefile b/lib/dim/Makefile
> index c4cc4026c451..c02c306e2975 100644
> --- a/lib/dim/Makefile
> +++ b/lib/dim/Makefile
> @@ -4,4 +4,8 @@
>  
>  obj-$(CONFIG_DIMLIB) += dimlib.o
>  
> -dimlib-objs := dim.o net_dim.o rdma_dim.o
> +dimlib-objs := dim.o rdma_dim.o
> +
> +ifeq ($(CONFIG_NET),y)
> +dimlib-objs += net_dim.o
> +endif

1. This is unlikely to work if the kernel is configured as[1]:

[1] kernel configuration
CONFIG_NET=n, CONFIG_ETHTOOL_NETLINK=n, CONFIG_PROVE_LOCKING=y,
(CONFIG_FSL_MC_DPIO=y && CONFIG_FSL_MC_BUS=y) select CONFIG_DIMLIB=y.


Then, because CONFIG_NET is not enabled, so there is no net_dim.o,
the following warning appears:

ld.lld: error: undefined symbol: net_dim_get_rx_moderation
referenced by dpio-service.c
drivers/soc/fsl/dpio/dpio-service.o:(dpaa2_io_dim_work) in archive vmlinux.a

ld.lld: error: undefined symbol: net_dim
referenced by dpio-service.c
drivers/soc/fsl/dpio/dpio-service.o:(dpaa2_io_update_net_dim) in archive vmlinux.a

2. If we declare "CONFIG_DIMLIB depends on CONFIG_NET",
if the configuration is still [1]:
Then the result is:
CONFIG_DIMLIB=Y (selected by CONFIG_FSL_MC_DPIO=y && CONFIG_FSL_MC_BUS=y),
CONFIG_NET=n, but we declared "CONFIG_DIMLIB depends on CONFIG_NET",
there is still a compilation error because the lockdep_rtnl_is_held symbol
cannot be found.

3. If we declare "CONFIG_DIMLIB select CONFIG_NET" and kernel configuration is [1],
then a circular dependency warning will appear:

CONFIG_DIMLIB select CONFIG_NET, ETHTOOL_NETLINK=Y(depends on CONFIG_NET)
selects CONFIG_DIMLIB. CONFIG_DIMLIB will select CONFIG_NET...

> 
> 

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

* Re: [PATCH net-next v13 2/4] ethtool: provide customized dim profile management
  2024-05-13 14:52         ` Heng Qi
@ 2024-05-13 15:24           ` Jakub Kicinski
  2024-05-13 15:39             ` Heng Qi
  0 siblings, 1 reply; 15+ messages in thread
From: Jakub Kicinski @ 2024-05-13 15:24 UTC (permalink / raw)
  To: Heng Qi
  Cc: kernel test robot, llvm, oe-kbuild-all, David S . Miller,
	Paolo Abeni, Eric Dumazet, Jason Wang, Michael S . Tsirkin,
	Brett Creeley, Ratheesh Kannoth, Alexander Lobakin, Xuan Zhuo,
	Tal Gilboa, Jonathan Corbet, linux-doc, Maxime Chevallier,
	Jiri Pirko, Paul Greenwalt, Ahmed Zaki, Vladimir Oltean,
	Kory Maincent, Andrew Lunn, justinstitt, donald.hunter, netdev,
	virtualization

On Mon, 13 May 2024 22:52:13 +0800 Heng Qi wrote:
> > > So I think we should declare "CONFIG_PROVE_LOCKING depends on CONFIG_NET".
> > > How do you think?  
> > 
> > Doesn't sound right, `can we instead make building lib/dim/net_dim.c  
> 
> Why? IIUC, the reason is that if CONFIG_NET is not set to Y, the net/core
> directory will not be compiled, so the lockdep_rtnl_is_held symbol is not
> present.

Maybe I don't understand what you;re proposing. 
Show an actual diff please.

> > dependent on CONFIG_NET? Untested but I'm thinking something like:
> > 
> > diff --git a/lib/dim/Makefile b/lib/dim/Makefile
> > index c4cc4026c451..c02c306e2975 100644
> > --- a/lib/dim/Makefile
> > +++ b/lib/dim/Makefile
> > @@ -4,4 +4,8 @@
> >  
> >  obj-$(CONFIG_DIMLIB) += dimlib.o
> >  
> > -dimlib-objs := dim.o net_dim.o rdma_dim.o
> > +dimlib-objs := dim.o rdma_dim.o
> > +
> > +ifeq ($(CONFIG_NET),y)
> > +dimlib-objs += net_dim.o
> > +endif  
> 
> 1. This is unlikely to work if the kernel is configured as[1]:
> 
> [1] kernel configuration
> CONFIG_NET=n, CONFIG_ETHTOOL_NETLINK=n, CONFIG_PROVE_LOCKING=y,
> (CONFIG_FSL_MC_DPIO=y && CONFIG_FSL_MC_BUS=y) select CONFIG_DIMLIB=y.
> 
> 
> Then, because CONFIG_NET is not enabled, so there is no net_dim.o,
> the following warning appears:
> 
> ld.lld: error: undefined symbol: net_dim_get_rx_moderation
> referenced by dpio-service.c
> drivers/soc/fsl/dpio/dpio-service.o:(dpaa2_io_dim_work) in archive vmlinux.a
> 
> ld.lld: error: undefined symbol: net_dim
> referenced by dpio-service.c
> drivers/soc/fsl/dpio/dpio-service.o:(dpaa2_io_update_net_dim) in archive vmlinux.a

Simple, dpio-service should depend on NET if it wants NET_DIM

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

* Re: [PATCH net-next v13 2/4] ethtool: provide customized dim profile management
  2024-05-13 15:24           ` Jakub Kicinski
@ 2024-05-13 15:39             ` Heng Qi
  2024-05-13 18:42               ` Jakub Kicinski
  0 siblings, 1 reply; 15+ messages in thread
From: Heng Qi @ 2024-05-13 15:39 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: kernel test robot, llvm, oe-kbuild-all, David S . Miller,
	Paolo Abeni, Eric Dumazet, Jason Wang, Michael S . Tsirkin,
	Brett Creeley, Ratheesh Kannoth, Alexander Lobakin, Xuan Zhuo,
	Tal Gilboa, Jonathan Corbet, linux-doc, Maxime Chevallier,
	Jiri Pirko, Paul Greenwalt, Ahmed Zaki, Vladimir Oltean,
	Kory Maincent, Andrew Lunn, justinstitt, donald.hunter, netdev,
	virtualization

On Mon, 13 May 2024 08:24:12 -0700, Jakub Kicinski <kuba@kernel.org> wrote:
> On Mon, 13 May 2024 22:52:13 +0800 Heng Qi wrote:
> > > > So I think we should declare "CONFIG_PROVE_LOCKING depends on CONFIG_NET".
> > > > How do you think?  
> > > 
> > > Doesn't sound right, `can we instead make building lib/dim/net_dim.c  
> > 
> > Why? IIUC, the reason is that if CONFIG_NET is not set to Y, the net/core
> > directory will not be compiled, so the lockdep_rtnl_is_held symbol is not
> > present.
> 
> Maybe I don't understand what you;re proposing. 
> Show an actual diff please.

Like this:

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 291185f54ee4..fbb20d0d08c1 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1299,7 +1299,7 @@ config LOCK_DEBUGGING_SUPPORT

 config PROVE_LOCKING
        bool "Lock debugging: prove locking correctness"
-       depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
+       depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT && NET
        select LOCKDEP
        select DEBUG_SPINLOCK
        select DEBUG_MUTEXES if !PREEMPT_RT


And there is:

In include/linux/rtnetlink.h:

#ifdef CONFIG_PROVE_LOCKING
extern bool lockdep_rtnl_is_held(void);
#else
static inline bool lockdep_rtnl_is_held(void)
{
        return true;
}
#endif /* #ifdef CONFIG_PROVE_LOCKING */


In net/core/rtnetlink.c:

#ifdef CONFIG_PROVE_LOCKING
bool lockdep_rtnl_is_held(void)
{
        return lockdep_is_held(&rtnl_mutex);
}
EXPORT_SYMBOL(lockdep_rtnl_is_held);
#endif /* #ifdef CONFIG_PROVE_LOCKING */

Thanks.

> 
> > > dependent on CONFIG_NET? Untested but I'm thinking something like:
> > > 
> > > diff --git a/lib/dim/Makefile b/lib/dim/Makefile
> > > index c4cc4026c451..c02c306e2975 100644
> > > --- a/lib/dim/Makefile
> > > +++ b/lib/dim/Makefile
> > > @@ -4,4 +4,8 @@
> > >  
> > >  obj-$(CONFIG_DIMLIB) += dimlib.o
> > >  
> > > -dimlib-objs := dim.o net_dim.o rdma_dim.o
> > > +dimlib-objs := dim.o rdma_dim.o
> > > +
> > > +ifeq ($(CONFIG_NET),y)
> > > +dimlib-objs += net_dim.o
> > > +endif  
> > 
> > 1. This is unlikely to work if the kernel is configured as[1]:
> > 
> > [1] kernel configuration
> > CONFIG_NET=n, CONFIG_ETHTOOL_NETLINK=n, CONFIG_PROVE_LOCKING=y,
> > (CONFIG_FSL_MC_DPIO=y && CONFIG_FSL_MC_BUS=y) select CONFIG_DIMLIB=y.
> > 
> > 
> > Then, because CONFIG_NET is not enabled, so there is no net_dim.o,
> > the following warning appears:
> > 
> > ld.lld: error: undefined symbol: net_dim_get_rx_moderation
> > referenced by dpio-service.c
> > drivers/soc/fsl/dpio/dpio-service.o:(dpaa2_io_dim_work) in archive vmlinux.a
> > 
> > ld.lld: error: undefined symbol: net_dim
> > referenced by dpio-service.c
> > drivers/soc/fsl/dpio/dpio-service.o:(dpaa2_io_update_net_dim) in archive vmlinux.a
> 
> Simple, dpio-service should depend on NET if it wants NET_DIM

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

* Re: [PATCH net-next v13 2/4] ethtool: provide customized dim profile management
  2024-05-13 15:39             ` Heng Qi
@ 2024-05-13 18:42               ` Jakub Kicinski
  2024-05-14  2:08                 ` Heng Qi
  0 siblings, 1 reply; 15+ messages in thread
From: Jakub Kicinski @ 2024-05-13 18:42 UTC (permalink / raw)
  To: Heng Qi
  Cc: kernel test robot, llvm, oe-kbuild-all, David S . Miller,
	Paolo Abeni, Eric Dumazet, Jason Wang, Michael S . Tsirkin,
	Brett Creeley, Ratheesh Kannoth, Alexander Lobakin, Xuan Zhuo,
	Tal Gilboa, Jonathan Corbet, linux-doc, Maxime Chevallier,
	Jiri Pirko, Paul Greenwalt, Ahmed Zaki, Vladimir Oltean,
	Kory Maincent, Andrew Lunn, justinstitt, donald.hunter, netdev,
	virtualization

On Mon, 13 May 2024 23:39:04 +0800 Heng Qi wrote:
>  config PROVE_LOCKING
>         bool "Lock debugging: prove locking correctness"
> -       depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
> +       depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT && NET

We can't make lockdep dependent on NET.
People working on other subsystems should be able to use LOCKDEP 
with minimal builds.

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

* Re: [PATCH net-next v13 2/4] ethtool: provide customized dim profile management
  2024-05-13 18:42               ` Jakub Kicinski
@ 2024-05-14  2:08                 ` Heng Qi
  2024-05-14 13:46                   ` Jakub Kicinski
  2024-05-14 16:36                   ` Andrew Lunn
  0 siblings, 2 replies; 15+ messages in thread
From: Heng Qi @ 2024-05-14  2:08 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: kernel test robot, llvm, oe-kbuild-all, David S . Miller,
	Paolo Abeni, Eric Dumazet, Jason Wang, Michael S . Tsirkin,
	Brett Creeley, Ratheesh Kannoth, Alexander Lobakin, Xuan Zhuo,
	Tal Gilboa, Jonathan Corbet, linux-doc, Maxime Chevallier,
	Jiri Pirko, Paul Greenwalt, Ahmed Zaki, Vladimir Oltean,
	Kory Maincent, Andrew Lunn, justinstitt, donald.hunter, netdev,
	virtualization

On Mon, 13 May 2024 11:42:33 -0700, Jakub Kicinski <kuba@kernel.org> wrote:
> On Mon, 13 May 2024 23:39:04 +0800 Heng Qi wrote:
> >  config PROVE_LOCKING
> >         bool "Lock debugging: prove locking correctness"
> > -       depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
> > +       depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT && NET
> 
> We can't make lockdep dependent on NET.
> People working on other subsystems should be able to use LOCKDEP 
> with minimal builds.


Got it. Then I declare "DIMLIB depends on NET" and clean up other places.

One more friendly request, I see net-next is closed today, but our downstream
kernel release deadline is 5.20, so I want to test and release the new v14 today,
is it ok?

Thanks!

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

* Re: [PATCH net-next v13 2/4] ethtool: provide customized dim profile management
  2024-05-14  2:08                 ` Heng Qi
@ 2024-05-14 13:46                   ` Jakub Kicinski
  2024-05-14 16:36                   ` Andrew Lunn
  1 sibling, 0 replies; 15+ messages in thread
From: Jakub Kicinski @ 2024-05-14 13:46 UTC (permalink / raw)
  To: Heng Qi
  Cc: kernel test robot, llvm, oe-kbuild-all, David S . Miller,
	Paolo Abeni, Eric Dumazet, Jason Wang, Michael S . Tsirkin,
	Brett Creeley, Ratheesh Kannoth, Alexander Lobakin, Xuan Zhuo,
	Tal Gilboa, Jonathan Corbet, linux-doc, Maxime Chevallier,
	Jiri Pirko, Paul Greenwalt, Ahmed Zaki, Vladimir Oltean,
	Kory Maincent, Andrew Lunn, justinstitt, donald.hunter, netdev,
	virtualization

On Tue, 14 May 2024 10:08:15 +0800 Heng Qi wrote:
> > We can't make lockdep dependent on NET.
> > People working on other subsystems should be able to use LOCKDEP 
> > with minimal builds.  
> 
> Got it. Then I declare "DIMLIB depends on NET" and clean up other places.

I'm not sure if there's any legit DIM (but not net_dim) user, if there
isn't that SGTM. The RDMA is fine to depend on NET.

> One more friendly request, I see net-next is closed today, but our downstream
> kernel release deadline is 5.20, so I want to test and release the new v14 today,
> is it ok?

You'll need to post as RFC since it can't be applied but you can post
to continue review. Please wait a day or two tho, I'm currently swamped
trying to go thru the PR preparations.

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

* Re: [PATCH net-next v13 2/4] ethtool: provide customized dim profile management
  2024-05-14  2:08                 ` Heng Qi
  2024-05-14 13:46                   ` Jakub Kicinski
@ 2024-05-14 16:36                   ` Andrew Lunn
  1 sibling, 0 replies; 15+ messages in thread
From: Andrew Lunn @ 2024-05-14 16:36 UTC (permalink / raw)
  To: Heng Qi
  Cc: Jakub Kicinski, kernel test robot, llvm, oe-kbuild-all,
	David S . Miller, Paolo Abeni, Eric Dumazet, Jason Wang,
	Michael S . Tsirkin, Brett Creeley, Ratheesh Kannoth,
	Alexander Lobakin, Xuan Zhuo, Tal Gilboa, Jonathan Corbet,
	linux-doc, Maxime Chevallier, Jiri Pirko, Paul Greenwalt,
	Ahmed Zaki, Vladimir Oltean, Kory Maincent, justinstitt,
	donald.hunter, netdev, virtualization

> One more friendly request, I see net-next is closed today, but our downstream
> kernel release deadline is 5.20, so I want to test and release the new v14 today,
> is it ok?

5.20? Didn't Linus run out of fingers and thumbs and went from 5.19 to
6.0?

Anyway, this is your internal issue, since netdev only accepts new
features patches for the next kernel release, not stable kernels. If
you are forced to do upstream first, you are going to have to wait
until the next cycle starts, and net-next reopens.

      Andrew

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

end of thread, other threads:[~2024-05-14 16:36 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-09  4:47 [PATCH net-next v13 0/4] ethtool: provide the dim profile fine-tuning channel Heng Qi
2024-05-09  4:47 ` [PATCH net-next v13 1/4] linux/dim: move useful macros to .h file Heng Qi
2024-05-09  4:47 ` [PATCH net-next v13 2/4] ethtool: provide customized dim profile management Heng Qi
2024-05-09 23:09   ` kernel test robot
2024-05-12 16:36     ` Heng Qi
2024-05-13 14:22       ` Jakub Kicinski
2024-05-13 14:52         ` Heng Qi
2024-05-13 15:24           ` Jakub Kicinski
2024-05-13 15:39             ` Heng Qi
2024-05-13 18:42               ` Jakub Kicinski
2024-05-14  2:08                 ` Heng Qi
2024-05-14 13:46                   ` Jakub Kicinski
2024-05-14 16:36                   ` Andrew Lunn
2024-05-09  4:47 ` [PATCH net-next v13 3/4] dim: add new interfaces for initialization and getting results Heng Qi
2024-05-09  4:47 ` [PATCH net-next v13 4/4] virtio-net: support dim profile fine-tuning 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).