[PATCH net-next v11 0/4] ethtool: provide the dim profile fine-tuning channel

[PATCH net-next v11 0/4] ethtool: provide the dim profile fine-tuning channel

[Heng Qi]

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
=====
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 +
 drivers/net/virtio_net.c                     |  48 +++-
 include/linux/dim.h                          | 117 ++++++++
 include/linux/ethtool.h                      |   7 +-
 include/linux/netdevice.h                    |   5 +
 include/uapi/linux/ethtool_netlink.h         |  22 ++
 lib/dim/net_dim.c                            | 145 +++++++++-
 net/ethtool/coalesce.c                       | 278 ++++++++++++++++++-
 9 files changed, 642 insertions(+), 15 deletions(-)

-- 
2.32.0.3.g01195cf9f

[PATCH net-next v11 1/4] linux/dim: move useful macros to .h file

[Heng Qi]

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

[PATCH net-next v11 2/4] ethtool: provide customized dim profile management

[Heng Qi]

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 +
 include/linux/dim.h                          |  62 +++++
 include/linux/ethtool.h                      |   7 +-
 include/linux/netdevice.h                    |   5 +
 include/uapi/linux/ethtool_netlink.h         |  22 ++
 lib/dim/net_dim.c                            |  71 +++++
 net/ethtool/coalesce.c                       | 278 ++++++++++++++++++-
 8 files changed, 478 insertions(+), 2 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/include/linux/dim.h b/include/linux/dim.h
index 43398f5eade2..4b1630f4672b 100644
--- a/include/linux/dim.h
+++ b/include/linux/dim.h
@@ -9,6 +9,7 @@
 #include <linux/module.h>
 #include <linux/types.h>
 #include <linux/workqueue.h>
+#include <linux/netdevice.h>
 
 /* Number of DIM profiles and period mode. */
 #define NET_DIM_PARAMS_NUM_PROFILES 5
@@ -45,12 +46,47 @@
  * @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 {
+	/* See DIM_PROFILE_* */
+	u8 profile_flags;
+
+	/* See DIM_COALESCE_* for Rx and Tx */
+	u8 coal_flags;
+
+	/* Rx DIM period count mode: CQE or EQE */
+	u8 dim_rx_mode;
+
+	/* Tx DIM period count mode: CQE or EQE */
+	u8 dim_tx_mode;
+
+	/* DIM profile list for Rx */
+	struct dim_cq_moder __rcu *rx_profile;
+
+	/* DIM profile list for Tx */
+	struct dim_cq_moder __rcu *tx_profile;
+
+	/* Rx DIM worker function scheduled by net_dim() */
+	void (*rx_dim_work)(struct work_struct *work);
+
+	/* Tx DIM worker function scheduled by net_dim() */
+	void (*tx_dim_work)(struct work_struct *work);
 };
 
 /**
@@ -198,6 +234,32 @@ 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
+ *    void (*rx_dim_work)(struct work_struct *work);
+ *
+ * @tx_dim_work: Tx worker called after dim decision
+ *    void (*tx_dim_work)(struct work_struct *work);
+ *
+ * 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..902815b517dc 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)
@@ -316,6 +318,9 @@ 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_PROFILE |	\
+	 ETHTOOL_COALESCE_TX_PROFILE)
 
 #define ETHTOOL_STAT_NOT_SET	(~0ULL)
 
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index f849e7d110ed..4bf6ea2074aa 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2400,6 +2400,11 @@ struct net_device {
 	/** @page_pools: page pools created for this netdevice */
 	struct hlist_head	page_pools;
 #endif
+
+#if IS_ENABLED(CONFIG_DIMLIB)
+	/** @irq_moder: dim related parameters for this netdevice */
+	struct dim_irq_moder	*irq_moder;
+#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 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..b3e01619f929 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,76 @@ 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)
+		goto err_moder;
+
+	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;
+		WRITE_ONCE(moder->dim_rx_mode, rx_mode);
+		rxp = kmemdup(rx_profile[rx_mode], len, GFP_KERNEL);
+		if (!rxp)
+			goto err_rx_profile;
+
+		rcu_assign_pointer(moder->rx_profile, rxp);
+	}
+
+	if (profile_flags & DIM_PROFILE_TX) {
+		moder->tx_dim_work = tx_dim_work;
+		WRITE_ONCE(moder->dim_tx_mode, tx_mode);
+		txp = kmemdup(tx_profile[tx_mode], len, GFP_KERNEL);
+		if (!txp)
+			goto err_tx_profile;
+
+		rcu_assign_pointer(moder->tx_profile, txp);
+	}
+
+	return 0;
+
+err_tx_profile:
+	kfree(rxp);
+err_rx_profile:
+	kfree(moder);
+err_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/ethtool/coalesce.c b/net/ethtool/coalesce.c
index 83112c1a71ae..2978e58c74ee 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,6 +137,74 @@ 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)
@@ -134,6 +212,12 @@ 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;
+#if IS_ENABLED(CONFIG_DIMLIB)
+	struct net_device *dev = req_base->dev;
+	struct dim_irq_moder *moder = dev->irq_moder;
+	u8 coal_flags;
+	int ret;
+#endif
 	u32 supported = data->supported_params;
 
 	if (coalesce_put_u32(skb, ETHTOOL_A_COALESCE_RX_USECS,
@@ -192,11 +276,49 @@ static int coalesce_fill_reply(struct sk_buff *skb,
 			     kcoal->tx_aggr_time_usecs, supported))
 		return -EMSGSIZE;
 
+#if IS_ENABLED(CONFIG_DIMLIB)
+	if (!moder)
+		return 0;
+
+	coal_flags = moder->coal_flags;
+	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),
+					   coal_flags);
+		if (ret) {
+			rcu_read_unlock();
+			return ret;
+		}
+	}
+
+	if (moder->profile_flags & DIM_PROFILE_TX) {
+		ret = coalesce_put_profile(skb, ETHTOOL_A_COALESCE_TX_PROFILE,
+					   rcu_dereference(moder->tx_profile),
+					   coal_flags);
+		if (ret) {
+			rcu_read_unlock();
+			return ret;
+		}
+	}
+	rcu_read_unlock();
+#endif
 	return 0;
 }
 
 /* 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 +349,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 +360,9 @@ ethnl_set_coalesce_validate(struct ethnl_req_info *req_info,
 			    struct genl_info *info)
 {
 	const struct ethtool_ops *ops = req_info->dev->ethtool_ops;
+#if IS_ENABLED(CONFIG_DIMLIB)
+	struct net_device *dev = req_info->dev;
+#endif
 	struct nlattr **tb = info->attrs;
 	u32 supported_params;
 	u16 a;
@@ -243,6 +372,15 @@ 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 IS_ENABLED(CONFIG_DIMLIB)
+	if (dev->irq_moder) {
+		if (dev->irq_moder->profile_flags & DIM_PROFILE_RX)
+			supported_params |= ETHTOOL_COALESCE_RX_PROFILE;
+
+		if (dev->irq_moder->profile_flags & DIM_PROFILE_TX)
+			supported_params |= ETHTOOL_COALESCE_TX_PROFILE;
+	}
+#endif
 	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,12 +391,133 @@ 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++;
+	}
+
+	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)
 {
 	struct kernel_ethtool_coalesce kernel_coalesce = {};
 	struct net_device *dev = req_info->dev;
+#if IS_ENABLED(CONFIG_DIMLIB)
+	struct dim_irq_moder *irq_moder = dev->irq_moder;
+#endif
 	struct ethtool_coalesce coalesce = {};
 	bool mod_mode = false, mod = false;
 	struct nlattr **tb = info->attrs;
@@ -317,6 +576,23 @@ __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 IS_ENABLED(CONFIG_DIMLIB)
+	if (irq_moder && irq_moder->profile_flags & DIM_PROFILE_RX) {
+		ret = ethnl_update_profile(dev, &irq_moder->rx_profile,
+					   tb[ETHTOOL_A_COALESCE_RX_PROFILE],
+					   info->extack);
+		if (ret < 0)
+			return ret;
+	}
+
+	if (irq_moder && irq_moder->profile_flags & DIM_PROFILE_TX) {
+		ret = ethnl_update_profile(dev, &irq_moder->tx_profile,
+					   tb[ETHTOOL_A_COALESCE_TX_PROFILE],
+					   info->extack);
+		if (ret < 0)
+			return ret;
+	}
+#endif
 	/* 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

[PATCH net-next v11 3/4] dim: add new interfaces for initialization and getting results

[Heng Qi]

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 4b1630f4672b..e8e24667ea23 100644
--- a/include/linux/dim.h
+++ b/include/linux/dim.h
@@ -260,6 +260,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 b3e01619f929..bc74ef36855d 100644
--- a/lib/dim/net_dim.c
+++ b/lib/dim/net_dim.c
@@ -166,6 +166,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();
+
+	dim->mode = READ_ONCE(dev->irq_moder->dim_rx_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();
+
+	dim->mode = READ_ONCE(dev->irq_moder->dim_tx_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

[PATCH net-next v11 4/4] virtio-net: support dim profile fine-tuning

[Heng Qi]

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 | 48 ++++++++++++++++++++++++++++++++++------
 1 file changed, 41 insertions(+), 7 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 1fa84790041b..72924c7b508d 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2446,7 +2446,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;
@@ -2612,7 +2612,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);
@@ -2874,7 +2874,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;
@@ -4363,7 +4363,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 = 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,
@@ -5054,6 +5054,37 @@ 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;
+	struct net_device *dev = vi->dev;
+	int ret, i;
+
+	profile_flags |= DIM_PROFILE_RX;
+	coal_flags |= DIM_COALESCE_USEC | DIM_COALESCE_PKTS;
+	ret = net_dim_init_irq_moder(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,
@@ -5333,9 +5364,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));
@@ -5753,6 +5781,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
@@ -5896,6 +5928,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

Re: [PATCH net-next v11 2/4] ethtool: provide customized dim profile management

[kernel test robot]

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/20240501-013413
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240430173136.15807-3-hengqi%40linux.alibaba.com
patch subject: [PATCH net-next v11 2/4] ethtool: provide customized dim profile management
config: openrisc-defconfig (https://download.01.org/0day-ci/archive/20240501/202405011004.Rkw6IrSl-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/20240501/202405011004.Rkw6IrSl-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/202405011004.Rkw6IrSl-lkp@intel.com/

All warnings (new ones prefixed by >>):

   net/ethtool/coalesce.c: In function 'ethnl_update_profile':
   net/ethtool/coalesce.c:453:46: error: 'struct net_device' has no member named 'irq_moder'
     453 |         struct dim_irq_moder *irq_moder = dev->irq_moder;
         |                                              ^~
   net/ethtool/coalesce.c: At top level:
>> net/ethtool/coalesce.c:446:12: warning: 'ethnl_update_profile' defined but not used [-Wunused-function]
     446 | static int ethnl_update_profile(struct net_device *dev,
         |            ^~~~~~~~~~~~~~~~~~~~
>> net/ethtool/coalesce.c:151:12: warning: 'coalesce_put_profile' defined but not used [-Wunused-function]
     151 | static int coalesce_put_profile(struct sk_buff *skb, u16 attr_type,
         |            ^~~~~~~~~~~~~~~~~~~~


vim +/ethnl_update_profile +446 net/ethtool/coalesce.c

   424	
   425	/**
   426	 * ethnl_update_profile - get a profile nest with child nests from userspace.
   427	 * @dev: netdevice to update the profile
   428	 * @dst: profile get from the driver and modified by ethnl_update_profile.
   429	 * @nests: nest attr ETHTOOL_A_COALESCE_*X_PROFILE to set profile.
   430	 * @extack: Netlink extended ack
   431	 *
   432	 * Layout of nests:
   433	 *   Nested ETHTOOL_A_COALESCE_*X_PROFILE attr
   434	 *     Nested ETHTOOL_A_PROFILE_IRQ_MODERATION attr
   435	 *       ETHTOOL_A_IRQ_MODERATION_USEC attr
   436	 *       ETHTOOL_A_IRQ_MODERATION_PKTS attr
   437	 *       ETHTOOL_A_IRQ_MODERATION_COMPS attr
   438	 *     ...
   439	 *     Nested ETHTOOL_A_PROFILE_IRQ_MODERATION attr
   440	 *       ETHTOOL_A_IRQ_MODERATION_USEC attr
   441	 *       ETHTOOL_A_IRQ_MODERATION_PKTS attr
   442	 *       ETHTOOL_A_IRQ_MODERATION_COMPS attr
   443	 *
   444	 * Return: 0 on success or a negative error code.
   445	 */
 > 446	static int ethnl_update_profile(struct net_device *dev,
   447					struct dim_cq_moder __rcu **dst,
   448					const struct nlattr *nests,
   449					struct netlink_ext_ack *extack)
   450	{
   451		int len_irq_moder = ARRAY_SIZE(coalesce_irq_moderation_policy);
   452		struct nlattr *tb[ARRAY_SIZE(coalesce_irq_moderation_policy)];
 > 453		struct dim_irq_moder *irq_moder = dev->irq_moder;
   454		struct dim_cq_moder *new_profile, *old_profile;
   455		int ret, rem, i = 0, len;
   456		struct nlattr *nest;
   457	
   458		if (!nests)
   459			return 0;
   460	
   461		if (!*dst)
   462			return -EOPNOTSUPP;
   463	
   464		old_profile = rtnl_dereference(*dst);
   465		len = NET_DIM_PARAMS_NUM_PROFILES * sizeof(*old_profile);
   466		new_profile = kmemdup(old_profile, len, GFP_KERNEL);
   467		if (!new_profile)
   468			return -ENOMEM;
   469	
   470		nla_for_each_nested_type(nest, ETHTOOL_A_PROFILE_IRQ_MODERATION,
   471					 nests, rem) {
   472			ret = nla_parse_nested(tb, len_irq_moder - 1, nest,
   473					       coalesce_irq_moderation_policy,
   474					       extack);
   475			if (ret)
   476				goto err_out;
   477	
   478			ret = ethnl_update_irq_moder(irq_moder, &new_profile[i].usec,
   479						     ETHTOOL_A_IRQ_MODERATION_USEC,
   480						     tb, DIM_COALESCE_USEC,
   481						     extack);
   482			if (ret)
   483				goto err_out;
   484	
   485			ret = ethnl_update_irq_moder(irq_moder, &new_profile[i].pkts,
   486						     ETHTOOL_A_IRQ_MODERATION_PKTS,
   487						     tb, DIM_COALESCE_PKTS,
   488						     extack);
   489			if (ret)
   490				goto err_out;
   491	
   492			ret = ethnl_update_irq_moder(irq_moder, &new_profile[i].comps,
   493						     ETHTOOL_A_IRQ_MODERATION_COMPS,
   494						     tb, DIM_COALESCE_COMPS,
   495						     extack);
   496			if (ret)
   497				goto err_out;
   498	
   499			i++;
   500		}
   501	
   502		rcu_assign_pointer(*dst, new_profile);
   503		kfree_rcu(old_profile, rcu);
   504	
   505		return 0;
   506	
   507	err_out:
   508		kfree(new_profile);
   509		return ret;
   510	}
   511	

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

Re: [PATCH net-next v11 2/4] ethtool: provide customized dim profile management

[Heng Qi]

On Wed, 1 May 2024 10:36:03 +0800, kernel test robot <lkp@intel.com> wrote:
> 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/20240501-013413
> base:   net-next/main
> patch link:    https://lore.kernel.org/r/20240430173136.15807-3-hengqi%40linux.alibaba.com
> patch subject: [PATCH net-next v11 2/4] ethtool: provide customized dim profile management
> config: openrisc-defconfig (https://download.01.org/0day-ci/archive/20240501/202405011004.Rkw6IrSl-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/20240501/202405011004.Rkw6IrSl-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/202405011004.Rkw6IrSl-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
>    net/ethtool/coalesce.c: In function 'ethnl_update_profile':
>    net/ethtool/coalesce.c:453:46: error: 'struct net_device' has no member named 'irq_moder'
>      453 |         struct dim_irq_moder *irq_moder = dev->irq_moder;
>          |                                              ^~
>    net/ethtool/coalesce.c: At top level:
> >> net/ethtool/coalesce.c:446:12: warning: 'ethnl_update_profile' defined but not used [-Wunused-function]
>      446 | static int ethnl_update_profile(struct net_device *dev,
>          |            ^~~~~~~~~~~~~~~~~~~~
> >> net/ethtool/coalesce.c:151:12: warning: 'coalesce_put_profile' defined but not used [-Wunused-function]
>      151 | static int coalesce_put_profile(struct sk_buff *skb, u16 attr_type,
>          |            ^~~~~~~~~~~~~~~~~~~~
> 

This is a known minor issue, to reduce the use of 'IS_ENABLED(CONFIG_DIMLIB)'
mentioned in v10. Since the calls of ethnl_update_profile() and
coalesce_put_profile() will only occur when IS_ENABLED(CONFIG_DIMLIB) returns
true, the robot's warning can be ignored the code is safe.

All NIPA test cases running on my local pass successfully on V11.

Alternatively, I remake the series to have IS_ENABLED(CONFIG_DIMLIB) back,
up to Kuba (and others). :)

Thanks.

> 
> vim +/ethnl_update_profile +446 net/ethtool/coalesce.c
> 
>    424	
>    425	/**
>    426	 * ethnl_update_profile - get a profile nest with child nests from userspace.
>    427	 * @dev: netdevice to update the profile
>    428	 * @dst: profile get from the driver and modified by ethnl_update_profile.
>    429	 * @nests: nest attr ETHTOOL_A_COALESCE_*X_PROFILE to set profile.
>    430	 * @extack: Netlink extended ack
>    431	 *
>    432	 * Layout of nests:
>    433	 *   Nested ETHTOOL_A_COALESCE_*X_PROFILE attr
>    434	 *     Nested ETHTOOL_A_PROFILE_IRQ_MODERATION attr
>    435	 *       ETHTOOL_A_IRQ_MODERATION_USEC attr
>    436	 *       ETHTOOL_A_IRQ_MODERATION_PKTS attr
>    437	 *       ETHTOOL_A_IRQ_MODERATION_COMPS attr
>    438	 *     ...
>    439	 *     Nested ETHTOOL_A_PROFILE_IRQ_MODERATION attr
>    440	 *       ETHTOOL_A_IRQ_MODERATION_USEC attr
>    441	 *       ETHTOOL_A_IRQ_MODERATION_PKTS attr
>    442	 *       ETHTOOL_A_IRQ_MODERATION_COMPS attr
>    443	 *
>    444	 * Return: 0 on success or a negative error code.
>    445	 */
>  > 446	static int ethnl_update_profile(struct net_device *dev,
>    447					struct dim_cq_moder __rcu **dst,
>    448					const struct nlattr *nests,
>    449					struct netlink_ext_ack *extack)
>    450	{
>    451		int len_irq_moder = ARRAY_SIZE(coalesce_irq_moderation_policy);
>    452		struct nlattr *tb[ARRAY_SIZE(coalesce_irq_moderation_policy)];
>  > 453		struct dim_irq_moder *irq_moder = dev->irq_moder;
>    454		struct dim_cq_moder *new_profile, *old_profile;
>    455		int ret, rem, i = 0, len;
>    456		struct nlattr *nest;
>    457	
>    458		if (!nests)
>    459			return 0;
>    460	
>    461		if (!*dst)
>    462			return -EOPNOTSUPP;
>    463	
>    464		old_profile = rtnl_dereference(*dst);
>    465		len = NET_DIM_PARAMS_NUM_PROFILES * sizeof(*old_profile);
>    466		new_profile = kmemdup(old_profile, len, GFP_KERNEL);
>    467		if (!new_profile)
>    468			return -ENOMEM;
>    469	
>    470		nla_for_each_nested_type(nest, ETHTOOL_A_PROFILE_IRQ_MODERATION,
>    471					 nests, rem) {
>    472			ret = nla_parse_nested(tb, len_irq_moder - 1, nest,
>    473					       coalesce_irq_moderation_policy,
>    474					       extack);
>    475			if (ret)
>    476				goto err_out;
>    477	
>    478			ret = ethnl_update_irq_moder(irq_moder, &new_profile[i].usec,
>    479						     ETHTOOL_A_IRQ_MODERATION_USEC,
>    480						     tb, DIM_COALESCE_USEC,
>    481						     extack);
>    482			if (ret)
>    483				goto err_out;
>    484	
>    485			ret = ethnl_update_irq_moder(irq_moder, &new_profile[i].pkts,
>    486						     ETHTOOL_A_IRQ_MODERATION_PKTS,
>    487						     tb, DIM_COALESCE_PKTS,
>    488						     extack);
>    489			if (ret)
>    490				goto err_out;
>    491	
>    492			ret = ethnl_update_irq_moder(irq_moder, &new_profile[i].comps,
>    493						     ETHTOOL_A_IRQ_MODERATION_COMPS,
>    494						     tb, DIM_COALESCE_COMPS,
>    495						     extack);
>    496			if (ret)
>    497				goto err_out;
>    498	
>    499			i++;
>    500		}
>    501	
>    502		rcu_assign_pointer(*dst, new_profile);
>    503		kfree_rcu(old_profile, rcu);
>    504	
>    505		return 0;
>    506	
>    507	err_out:
>    508		kfree(new_profile);
>    509		return ret;
>    510	}
>    511	
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki

Re: [PATCH net-next v11 2/4] ethtool: provide customized dim profile management

[Michael S. Tsirkin]

On Wed, May 01, 2024 at 12:45:36PM +0800, Heng Qi wrote:
> On Wed, 1 May 2024 10:36:03 +0800, kernel test robot <lkp@intel.com> wrote:
> > 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/20240501-013413
> > base:   net-next/main
> > patch link:    https://lore.kernel.org/r/20240430173136.15807-3-hengqi%40linux.alibaba.com
> > patch subject: [PATCH net-next v11 2/4] ethtool: provide customized dim profile management
> > config: openrisc-defconfig (https://download.01.org/0day-ci/archive/20240501/202405011004.Rkw6IrSl-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/20240501/202405011004.Rkw6IrSl-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/202405011004.Rkw6IrSl-lkp@intel.com/
> > 
> > All warnings (new ones prefixed by >>):
> > 
> >    net/ethtool/coalesce.c: In function 'ethnl_update_profile':
> >    net/ethtool/coalesce.c:453:46: error: 'struct net_device' has no member named 'irq_moder'
> >      453 |         struct dim_irq_moder *irq_moder = dev->irq_moder;
> >          |                                              ^~
> >    net/ethtool/coalesce.c: At top level:
> > >> net/ethtool/coalesce.c:446:12: warning: 'ethnl_update_profile' defined but not used [-Wunused-function]
> >      446 | static int ethnl_update_profile(struct net_device *dev,
> >          |            ^~~~~~~~~~~~~~~~~~~~
> > >> net/ethtool/coalesce.c:151:12: warning: 'coalesce_put_profile' defined but not used [-Wunused-function]
> >      151 | static int coalesce_put_profile(struct sk_buff *skb, u16 attr_type,
> >          |            ^~~~~~~~~~~~~~~~~~~~
> > 
> 
> This is a known minor issue,
> to reduce the use of 'IS_ENABLED(CONFIG_DIMLIB)'
> mentioned in v10. Since the calls of ethnl_update_profile() and
> coalesce_put_profile() will only occur when IS_ENABLED(CONFIG_DIMLIB) returns
> true, the robot's warning can be ignored the code is safe.

I don't get it. The build seems to fail. How is this a minor issue?


> All NIPA test cases running on my local pass successfully on V11.
> 
> Alternatively, I remake the series to have IS_ENABLED(CONFIG_DIMLIB) back,
> up to Kuba (and others). :)
> 
> Thanks.
> 
> > 
> > vim +/ethnl_update_profile +446 net/ethtool/coalesce.c
> > 
> >    424	
> >    425	/**
> >    426	 * ethnl_update_profile - get a profile nest with child nests from userspace.
> >    427	 * @dev: netdevice to update the profile
> >    428	 * @dst: profile get from the driver and modified by ethnl_update_profile.
> >    429	 * @nests: nest attr ETHTOOL_A_COALESCE_*X_PROFILE to set profile.
> >    430	 * @extack: Netlink extended ack
> >    431	 *
> >    432	 * Layout of nests:
> >    433	 *   Nested ETHTOOL_A_COALESCE_*X_PROFILE attr
> >    434	 *     Nested ETHTOOL_A_PROFILE_IRQ_MODERATION attr
> >    435	 *       ETHTOOL_A_IRQ_MODERATION_USEC attr
> >    436	 *       ETHTOOL_A_IRQ_MODERATION_PKTS attr
> >    437	 *       ETHTOOL_A_IRQ_MODERATION_COMPS attr
> >    438	 *     ...
> >    439	 *     Nested ETHTOOL_A_PROFILE_IRQ_MODERATION attr
> >    440	 *       ETHTOOL_A_IRQ_MODERATION_USEC attr
> >    441	 *       ETHTOOL_A_IRQ_MODERATION_PKTS attr
> >    442	 *       ETHTOOL_A_IRQ_MODERATION_COMPS attr
> >    443	 *
> >    444	 * Return: 0 on success or a negative error code.
> >    445	 */
> >  > 446	static int ethnl_update_profile(struct net_device *dev,
> >    447					struct dim_cq_moder __rcu **dst,
> >    448					const struct nlattr *nests,
> >    449					struct netlink_ext_ack *extack)
> >    450	{
> >    451		int len_irq_moder = ARRAY_SIZE(coalesce_irq_moderation_policy);
> >    452		struct nlattr *tb[ARRAY_SIZE(coalesce_irq_moderation_policy)];
> >  > 453		struct dim_irq_moder *irq_moder = dev->irq_moder;
> >    454		struct dim_cq_moder *new_profile, *old_profile;
> >    455		int ret, rem, i = 0, len;
> >    456		struct nlattr *nest;
> >    457	
> >    458		if (!nests)
> >    459			return 0;
> >    460	
> >    461		if (!*dst)
> >    462			return -EOPNOTSUPP;
> >    463	
> >    464		old_profile = rtnl_dereference(*dst);
> >    465		len = NET_DIM_PARAMS_NUM_PROFILES * sizeof(*old_profile);
> >    466		new_profile = kmemdup(old_profile, len, GFP_KERNEL);
> >    467		if (!new_profile)
> >    468			return -ENOMEM;
> >    469	
> >    470		nla_for_each_nested_type(nest, ETHTOOL_A_PROFILE_IRQ_MODERATION,
> >    471					 nests, rem) {
> >    472			ret = nla_parse_nested(tb, len_irq_moder - 1, nest,
> >    473					       coalesce_irq_moderation_policy,
> >    474					       extack);
> >    475			if (ret)
> >    476				goto err_out;
> >    477	
> >    478			ret = ethnl_update_irq_moder(irq_moder, &new_profile[i].usec,
> >    479						     ETHTOOL_A_IRQ_MODERATION_USEC,
> >    480						     tb, DIM_COALESCE_USEC,
> >    481						     extack);
> >    482			if (ret)
> >    483				goto err_out;
> >    484	
> >    485			ret = ethnl_update_irq_moder(irq_moder, &new_profile[i].pkts,
> >    486						     ETHTOOL_A_IRQ_MODERATION_PKTS,
> >    487						     tb, DIM_COALESCE_PKTS,
> >    488						     extack);
> >    489			if (ret)
> >    490				goto err_out;
> >    491	
> >    492			ret = ethnl_update_irq_moder(irq_moder, &new_profile[i].comps,
> >    493						     ETHTOOL_A_IRQ_MODERATION_COMPS,
> >    494						     tb, DIM_COALESCE_COMPS,
> >    495						     extack);
> >    496			if (ret)
> >    497				goto err_out;
> >    498	
> >    499			i++;
> >    500		}
> >    501	
> >    502		rcu_assign_pointer(*dst, new_profile);
> >    503		kfree_rcu(old_profile, rcu);
> >    504	
> >    505		return 0;
> >    506	
> >    507	err_out:
> >    508		kfree(new_profile);
> >    509		return ret;
> >    510	}
> >    511	
> > 
> > -- 
> > 0-DAY CI Kernel Test Service
> > https://github.com/intel/lkp-tests/wiki

Re: [PATCH net-next v11 2/4] ethtool: provide customized dim profile management

[kernel test robot]

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/20240501-013413
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240430173136.15807-3-hengqi%40linux.alibaba.com
patch subject: [PATCH net-next v11 2/4] ethtool: provide customized dim profile management
config: openrisc-defconfig (https://download.01.org/0day-ci/archive/20240501/202405011418.EYj7bgrd-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/20240501/202405011418.EYj7bgrd-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/202405011418.EYj7bgrd-lkp@intel.com/

All errors (new ones prefixed by >>):

   net/ethtool/coalesce.c: In function 'ethnl_update_profile':
>> net/ethtool/coalesce.c:453:46: error: 'struct net_device' has no member named 'irq_moder'
     453 |         struct dim_irq_moder *irq_moder = dev->irq_moder;
         |                                              ^~
   net/ethtool/coalesce.c: At top level:
   net/ethtool/coalesce.c:446:12: warning: 'ethnl_update_profile' defined but not used [-Wunused-function]
     446 | static int ethnl_update_profile(struct net_device *dev,
         |            ^~~~~~~~~~~~~~~~~~~~
   net/ethtool/coalesce.c:151:12: warning: 'coalesce_put_profile' defined but not used [-Wunused-function]
     151 | static int coalesce_put_profile(struct sk_buff *skb, u16 attr_type,
         |            ^~~~~~~~~~~~~~~~~~~~


vim +453 net/ethtool/coalesce.c

   424	
   425	/**
   426	 * ethnl_update_profile - get a profile nest with child nests from userspace.
   427	 * @dev: netdevice to update the profile
   428	 * @dst: profile get from the driver and modified by ethnl_update_profile.
   429	 * @nests: nest attr ETHTOOL_A_COALESCE_*X_PROFILE to set profile.
   430	 * @extack: Netlink extended ack
   431	 *
   432	 * Layout of nests:
   433	 *   Nested ETHTOOL_A_COALESCE_*X_PROFILE attr
   434	 *     Nested ETHTOOL_A_PROFILE_IRQ_MODERATION attr
   435	 *       ETHTOOL_A_IRQ_MODERATION_USEC attr
   436	 *       ETHTOOL_A_IRQ_MODERATION_PKTS attr
   437	 *       ETHTOOL_A_IRQ_MODERATION_COMPS attr
   438	 *     ...
   439	 *     Nested ETHTOOL_A_PROFILE_IRQ_MODERATION attr
   440	 *       ETHTOOL_A_IRQ_MODERATION_USEC attr
   441	 *       ETHTOOL_A_IRQ_MODERATION_PKTS attr
   442	 *       ETHTOOL_A_IRQ_MODERATION_COMPS attr
   443	 *
   444	 * Return: 0 on success or a negative error code.
   445	 */
   446	static int ethnl_update_profile(struct net_device *dev,
   447					struct dim_cq_moder __rcu **dst,
   448					const struct nlattr *nests,
   449					struct netlink_ext_ack *extack)
   450	{
   451		int len_irq_moder = ARRAY_SIZE(coalesce_irq_moderation_policy);
   452		struct nlattr *tb[ARRAY_SIZE(coalesce_irq_moderation_policy)];
 > 453		struct dim_irq_moder *irq_moder = dev->irq_moder;
   454		struct dim_cq_moder *new_profile, *old_profile;
   455		int ret, rem, i = 0, len;
   456		struct nlattr *nest;
   457	
   458		if (!nests)
   459			return 0;
   460	
   461		if (!*dst)
   462			return -EOPNOTSUPP;
   463	
   464		old_profile = rtnl_dereference(*dst);
   465		len = NET_DIM_PARAMS_NUM_PROFILES * sizeof(*old_profile);
   466		new_profile = kmemdup(old_profile, len, GFP_KERNEL);
   467		if (!new_profile)
   468			return -ENOMEM;
   469	
   470		nla_for_each_nested_type(nest, ETHTOOL_A_PROFILE_IRQ_MODERATION,
   471					 nests, rem) {
   472			ret = nla_parse_nested(tb, len_irq_moder - 1, nest,
   473					       coalesce_irq_moderation_policy,
   474					       extack);
   475			if (ret)
   476				goto err_out;
   477	
   478			ret = ethnl_update_irq_moder(irq_moder, &new_profile[i].usec,
   479						     ETHTOOL_A_IRQ_MODERATION_USEC,
   480						     tb, DIM_COALESCE_USEC,
   481						     extack);
   482			if (ret)
   483				goto err_out;
   484	
   485			ret = ethnl_update_irq_moder(irq_moder, &new_profile[i].pkts,
   486						     ETHTOOL_A_IRQ_MODERATION_PKTS,
   487						     tb, DIM_COALESCE_PKTS,
   488						     extack);
   489			if (ret)
   490				goto err_out;
   491	
   492			ret = ethnl_update_irq_moder(irq_moder, &new_profile[i].comps,
   493						     ETHTOOL_A_IRQ_MODERATION_COMPS,
   494						     tb, DIM_COALESCE_COMPS,
   495						     extack);
   496			if (ret)
   497				goto err_out;
   498	
   499			i++;
   500		}
   501	
   502		rcu_assign_pointer(*dst, new_profile);
   503		kfree_rcu(old_profile, rcu);
   504	
   505		return 0;
   506	
   507	err_out:
   508		kfree(new_profile);
   509		return ret;
   510	}
   511	

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

Re: [PATCH net-next v11 2/4] ethtool: provide customized dim profile management

[kernel test robot]

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/20240501-013413
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240430173136.15807-3-hengqi%40linux.alibaba.com
patch subject: [PATCH net-next v11 2/4] ethtool: provide customized dim profile management
config: um-allmodconfig (https://download.01.org/0day-ci/archive/20240501/202405011500.J5qSgquf-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 37ae4ad0eef338776c7e2cffb3896153d43dcd90)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240501/202405011500.J5qSgquf-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/202405011500.J5qSgquf-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from net/ethtool/coalesce.c:3:
   In file included from include/linux/dim.h:12:
   In file included from include/linux/netdevice.h:38:
   In file included from include/net/net_namespace.h:43:
   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:8:
   In file included from include/linux/cacheflush.h:5:
   In file included from arch/um/include/asm/cacheflush.h:4:
   In file included from arch/um/include/asm/tlbflush.h:9:
   In file included from include/linux/mm.h:2208:
   include/linux/vmstat.h:522:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     522 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   In file included from net/ethtool/coalesce.c:3:
   In file included from include/linux/dim.h:12:
   In file included from include/linux/netdevice.h:38:
   In file included from include/net/net_namespace.h:43:
   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 include/linux/dim.h:12:
   In file included from include/linux/netdevice.h:38:
   In file included from include/net/net_namespace.h:43:
   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 include/linux/dim.h:12:
   In file included from include/linux/netdevice.h:38:
   In file included from include/net/net_namespace.h:43:
   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:453:41: error: no member named 'irq_moder' in 'struct net_device'
     453 |         struct dim_irq_moder *irq_moder = dev->irq_moder;
         |                                           ~~~  ^
   13 warnings and 1 error generated.


vim +453 net/ethtool/coalesce.c

   424	
   425	/**
   426	 * ethnl_update_profile - get a profile nest with child nests from userspace.
   427	 * @dev: netdevice to update the profile
   428	 * @dst: profile get from the driver and modified by ethnl_update_profile.
   429	 * @nests: nest attr ETHTOOL_A_COALESCE_*X_PROFILE to set profile.
   430	 * @extack: Netlink extended ack
   431	 *
   432	 * Layout of nests:
   433	 *   Nested ETHTOOL_A_COALESCE_*X_PROFILE attr
   434	 *     Nested ETHTOOL_A_PROFILE_IRQ_MODERATION attr
   435	 *       ETHTOOL_A_IRQ_MODERATION_USEC attr
   436	 *       ETHTOOL_A_IRQ_MODERATION_PKTS attr
   437	 *       ETHTOOL_A_IRQ_MODERATION_COMPS attr
   438	 *     ...
   439	 *     Nested ETHTOOL_A_PROFILE_IRQ_MODERATION attr
   440	 *       ETHTOOL_A_IRQ_MODERATION_USEC attr
   441	 *       ETHTOOL_A_IRQ_MODERATION_PKTS attr
   442	 *       ETHTOOL_A_IRQ_MODERATION_COMPS attr
   443	 *
   444	 * Return: 0 on success or a negative error code.
   445	 */
   446	static int ethnl_update_profile(struct net_device *dev,
   447					struct dim_cq_moder __rcu **dst,
   448					const struct nlattr *nests,
   449					struct netlink_ext_ack *extack)
   450	{
   451		int len_irq_moder = ARRAY_SIZE(coalesce_irq_moderation_policy);
   452		struct nlattr *tb[ARRAY_SIZE(coalesce_irq_moderation_policy)];
 > 453		struct dim_irq_moder *irq_moder = dev->irq_moder;
   454		struct dim_cq_moder *new_profile, *old_profile;
   455		int ret, rem, i = 0, len;
   456		struct nlattr *nest;
   457	
   458		if (!nests)
   459			return 0;
   460	
   461		if (!*dst)
   462			return -EOPNOTSUPP;
   463	
   464		old_profile = rtnl_dereference(*dst);
   465		len = NET_DIM_PARAMS_NUM_PROFILES * sizeof(*old_profile);
   466		new_profile = kmemdup(old_profile, len, GFP_KERNEL);
   467		if (!new_profile)
   468			return -ENOMEM;
   469	
   470		nla_for_each_nested_type(nest, ETHTOOL_A_PROFILE_IRQ_MODERATION,
   471					 nests, rem) {
   472			ret = nla_parse_nested(tb, len_irq_moder - 1, nest,
   473					       coalesce_irq_moderation_policy,
   474					       extack);
   475			if (ret)
   476				goto err_out;
   477	
   478			ret = ethnl_update_irq_moder(irq_moder, &new_profile[i].usec,
   479						     ETHTOOL_A_IRQ_MODERATION_USEC,
   480						     tb, DIM_COALESCE_USEC,
   481						     extack);
   482			if (ret)
   483				goto err_out;
   484	
   485			ret = ethnl_update_irq_moder(irq_moder, &new_profile[i].pkts,
   486						     ETHTOOL_A_IRQ_MODERATION_PKTS,
   487						     tb, DIM_COALESCE_PKTS,
   488						     extack);
   489			if (ret)
   490				goto err_out;
   491	
   492			ret = ethnl_update_irq_moder(irq_moder, &new_profile[i].comps,
   493						     ETHTOOL_A_IRQ_MODERATION_COMPS,
   494						     tb, DIM_COALESCE_COMPS,
   495						     extack);
   496			if (ret)
   497				goto err_out;
   498	
   499			i++;
   500		}
   501	
   502		rcu_assign_pointer(*dst, new_profile);
   503		kfree_rcu(old_profile, rcu);
   504	
   505		return 0;
   506	
   507	err_out:
   508		kfree(new_profile);
   509		return ret;
   510	}
   511	

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

Re: [PATCH net-next v11 2/4] ethtool: provide customized dim profile management

[Jakub Kicinski]

On Wed, 1 May 2024 12:45:36 +0800 Heng Qi wrote:
> >    net/ethtool/coalesce.c: At top level:  
>  [...]  
> >      446 | static int ethnl_update_profile(struct net_device *dev,
> >          |            ^~~~~~~~~~~~~~~~~~~~  
>  [...]  
> >      151 | static int coalesce_put_profile(struct sk_buff *skb, u16 attr_type,
> >          |            ^~~~~~~~~~~~~~~~~~~~
> >   
> 
> This is a known minor issue, to reduce the use of 'IS_ENABLED(CONFIG_DIMLIB)'
> mentioned in v10. Since the calls of ethnl_update_profile() and
> coalesce_put_profile() will only occur when IS_ENABLED(CONFIG_DIMLIB) returns
> true, the robot's warning can be ignored the code is safe.
> 
> All NIPA test cases running on my local pass successfully on V11.
> 
> Alternatively, I remake the series to have IS_ENABLED(CONFIG_DIMLIB) back,
> up to Kuba (and others). :)

You should remove the ifdef around the member in struct net_device.
It's too much code complication to save one pointer in the struct.

Re: [PATCH net-next v11 2/4] ethtool: provide customized dim profile management

[Heng Qi]

On Wed, 1 May 2024 07:44:20 -0700, Jakub Kicinski <kuba@kernel.org> wrote:
> On Wed, 1 May 2024 12:45:36 +0800 Heng Qi wrote:
> > >    net/ethtool/coalesce.c: At top level:  
> >  [...]  
> > >      446 | static int ethnl_update_profile(struct net_device *dev,
> > >          |            ^~~~~~~~~~~~~~~~~~~~  
> >  [...]  
> > >      151 | static int coalesce_put_profile(struct sk_buff *skb, u16 attr_type,
> > >          |            ^~~~~~~~~~~~~~~~~~~~
> > >   
> > 
> > This is a known minor issue, to reduce the use of 'IS_ENABLED(CONFIG_DIMLIB)'
> > mentioned in v10. Since the calls of ethnl_update_profile() and
> > coalesce_put_profile() will only occur when IS_ENABLED(CONFIG_DIMLIB) returns
> > true, the robot's warning can be ignored the code is safe.
> > 
> > All NIPA test cases running on my local pass successfully on V11.
> > 
> > Alternatively, I remake the series to have IS_ENABLED(CONFIG_DIMLIB) back,
> > up to Kuba (and others). :)
> 
> You should remove the ifdef around the member in struct net_device.
> It's too much code complication to save one pointer in the struct.

Makes sense.

Thanks.

Re: [PATCH net-next v11 2/4] ethtool: provide customized dim profile management

[Michael S. Tsirkin]

On Wed, May 01, 2024 at 11:11:47PM +0800, Heng Qi wrote:
> On Wed, 1 May 2024 07:44:20 -0700, Jakub Kicinski <kuba@kernel.org> wrote:
> > On Wed, 1 May 2024 12:45:36 +0800 Heng Qi wrote:
> > > >    net/ethtool/coalesce.c: At top level:  
> > >  [...]  
> > > >      446 | static int ethnl_update_profile(struct net_device *dev,
> > > >          |            ^~~~~~~~~~~~~~~~~~~~  
> > >  [...]  
> > > >      151 | static int coalesce_put_profile(struct sk_buff *skb, u16 attr_type,
> > > >          |            ^~~~~~~~~~~~~~~~~~~~
> > > >   
> > > 
> > > This is a known minor issue, to reduce the use of 'IS_ENABLED(CONFIG_DIMLIB)'
> > > mentioned in v10. Since the calls of ethnl_update_profile() and
> > > coalesce_put_profile() will only occur when IS_ENABLED(CONFIG_DIMLIB) returns
> > > true, the robot's warning can be ignored the code is safe.
> > > 
> > > All NIPA test cases running on my local pass successfully on V11.
> > > 
> > > Alternatively, I remake the series to have IS_ENABLED(CONFIG_DIMLIB) back,
> > > up to Kuba (and others). :)
> > 
> > You should remove the ifdef around the member in struct net_device.
> > It's too much code complication to save one pointer in the struct.
> 
> Makes sense.
> 
> Thanks.

if you really want to you can add a comment
 /* only used if IS_ENABLED(CONFIG_DIMLIB) */

Re: [PATCH net-next v11 2/4] ethtool: provide customized dim profile management

[Heng Qi]

On Wed, 1 May 2024 11:48:23 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Wed, May 01, 2024 at 11:11:47PM +0800, Heng Qi wrote:
> > On Wed, 1 May 2024 07:44:20 -0700, Jakub Kicinski <kuba@kernel.org> wrote:
> > > On Wed, 1 May 2024 12:45:36 +0800 Heng Qi wrote:
> > > > >    net/ethtool/coalesce.c: At top level:  
> > > >  [...]  
> > > > >      446 | static int ethnl_update_profile(struct net_device *dev,
> > > > >          |            ^~~~~~~~~~~~~~~~~~~~  
> > > >  [...]  
> > > > >      151 | static int coalesce_put_profile(struct sk_buff *skb, u16 attr_type,
> > > > >          |            ^~~~~~~~~~~~~~~~~~~~
> > > > >   
> > > > 
> > > > This is a known minor issue, to reduce the use of 'IS_ENABLED(CONFIG_DIMLIB)'
> > > > mentioned in v10. Since the calls of ethnl_update_profile() and
> > > > coalesce_put_profile() will only occur when IS_ENABLED(CONFIG_DIMLIB) returns
> > > > true, the robot's warning can be ignored the code is safe.
> > > > 
> > > > All NIPA test cases running on my local pass successfully on V11.
> > > > 
> > > > Alternatively, I remake the series to have IS_ENABLED(CONFIG_DIMLIB) back,
> > > > up to Kuba (and others). :)
> > > 
> > > You should remove the ifdef around the member in struct net_device.
> > > It's too much code complication to save one pointer in the struct.
> > 
> > Makes sense.
> > 
> > Thanks.
> 
> if you really want to you can add a comment
>  /* only used if IS_ENABLED(CONFIG_DIMLIB) */
> 

Ok, I'll add this.

Thanks.

Re: [PATCH net-next v11 2/4] ethtool: provide customized dim profile management

[Simon Horman]

On Wed, May 01, 2024 at 01:31:34AM +0800, Heng Qi wrote:

...

> diff --git a/include/linux/dim.h b/include/linux/dim.h

...

> @@ -198,6 +234,32 @@ 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
> + *    void (*rx_dim_work)(struct work_struct *work);
> + *
> + * @tx_dim_work: Tx worker called after dim decision
> + *    void (*tx_dim_work)(struct work_struct *work);
> + *

Hi Heng Qi,

The above seems to result in make htmldocs issuing the following warnings:

 .../net_dim:175: ./include/linux/dim.h:244: WARNING: Inline emphasis start-string without end-string.
 .../net_dim:175: ./include/linux/dim.h:244: WARNING: Inline emphasis start-string without end-string.
 .../net_dim:175: ./include/linux/dim.h:247: WARNING: Inline emphasis start-string without end-string.
 .../net_dim:175: ./include/linux/dim.h:247: WARNING: Inline emphasis start-string without end-string.

I suggest the following alternative:

 * @rx_dim_work: Rx worker called after dim decision
 * @tx_dim_work: Tx worker called after dim decision

Exercised using Sphinx 7.2.6

...

> diff --git a/net/ethtool/coalesce.c b/net/ethtool/coalesce.c

...

> @@ -134,6 +212,12 @@ 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;
> +#if IS_ENABLED(CONFIG_DIMLIB)
> +	struct net_device *dev = req_base->dev;
> +	struct dim_irq_moder *moder = dev->irq_moder;
> +	u8 coal_flags;
> +	int ret;
> +#endif
>  	u32 supported = data->supported_params;

It's a minor nit, but here goes: please consider using reverse xmas tree
order - longest line to shortest - for local variable declarations in
Networking code.

In this case, I appreciate that it's not strictly possible without
introducing more than one IS_ENABLED condition, which seems worse.
So, as that condition breaks the visual flow, what I suggest in this
case is having a second block of local variable declarations, like this:

	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;
	u32 supported = data->supported_params;                  

#if IS_ENABLED(CONFIG_DIMLIB)
	struct dim_irq_moder *moder = dev->irq_moder;
	struct net_device *dev = req_base->dev;
	u8 coal_flags;
	int ret;
#endif 

Or better still, if it can be done cleanly, moving all the IS_ENABLED()
code in this function into a separate function, with an no-op
implementation in the case that CONFIG_DIMLIB is not enabled. In general
I'd recommend that approach over sprinkling IS_ENABLED or #ifdef inside
functions. Because in my experience it tends to lead to more readable code.

In any case, the following tool is helpful for isolating
reverse xmas tree issues.

https://github.com/ecree-solarflare/xmastree


>  	if (coalesce_put_u32(skb, ETHTOOL_A_COALESCE_RX_USECS,
> @@ -192,11 +276,49 @@ static int coalesce_fill_reply(struct sk_buff *skb,
>  			     kcoal->tx_aggr_time_usecs, supported))
>  		return -EMSGSIZE;
>  
> +#if IS_ENABLED(CONFIG_DIMLIB)
> +	if (!moder)
> +		return 0;
> +
> +	coal_flags = moder->coal_flags;
> +	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),
> +					   coal_flags);
> +		if (ret) {
> +			rcu_read_unlock();
> +			return ret;
> +		}
> +	}
> +
> +	if (moder->profile_flags & DIM_PROFILE_TX) {
> +		ret = coalesce_put_profile(skb, ETHTOOL_A_COALESCE_TX_PROFILE,
> +					   rcu_dereference(moder->tx_profile),
> +					   coal_flags);
> +		if (ret) {
> +			rcu_read_unlock();
> +			return ret;
> +		}
> +	}
> +	rcu_read_unlock();
> +#endif
>  	return 0;
>  }

...

Re: [PATCH net-next v11 2/4] ethtool: provide customized dim profile management

[Heng Qi]

On Fri, 3 May 2024 14:52:44 +0100, Simon Horman <horms@kernel.org> wrote:
> On Wed, May 01, 2024 at 01:31:34AM +0800, Heng Qi wrote:
> 
> ...
> 
> > diff --git a/include/linux/dim.h b/include/linux/dim.h
> 
> ...
> 
> > @@ -198,6 +234,32 @@ 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
> > + *    void (*rx_dim_work)(struct work_struct *work);
> > + *
> > + * @tx_dim_work: Tx worker called after dim decision
> > + *    void (*tx_dim_work)(struct work_struct *work);
> > + *
> 
> Hi Heng Qi,

Hi Simon,

> 
> The above seems to result in make htmldocs issuing the following warnings:
> 
>  .../net_dim:175: ./include/linux/dim.h:244: WARNING: Inline emphasis start-string without end-string.
>  .../net_dim:175: ./include/linux/dim.h:244: WARNING: Inline emphasis start-string without end-string.
>  .../net_dim:175: ./include/linux/dim.h:247: WARNING: Inline emphasis start-string without end-string.
>  .../net_dim:175: ./include/linux/dim.h:247: WARNING: Inline emphasis start-string without end-string.
> 
> I suggest the following alternative:
> 
>  * @rx_dim_work: Rx worker called after dim decision
>  * @tx_dim_work: Tx worker called after dim decision

Will update this.

> 
> Exercised using Sphinx 7.2.6
> 
> ...
> 
> > diff --git a/net/ethtool/coalesce.c b/net/ethtool/coalesce.c
> 
> ...
> 
> > @@ -134,6 +212,12 @@ 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;
> > +#if IS_ENABLED(CONFIG_DIMLIB)
> > +	struct net_device *dev = req_base->dev;
> > +	struct dim_irq_moder *moder = dev->irq_moder;
> > +	u8 coal_flags;
> > +	int ret;
> > +#endif
> >  	u32 supported = data->supported_params;
> 
> It's a minor nit, but here goes: please consider using reverse xmas tree
> order - longest line to shortest - for local variable declarations in
> Networking code.
> 
> In this case, I appreciate that it's not strictly possible without
> introducing more than one IS_ENABLED condition, which seems worse.
> So, as that condition breaks the visual flow, what I suggest in this
> case is having a second block of local variable declarations, like this:
> 
> 	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;
> 	u32 supported = data->supported_params;                  
> 
> #if IS_ENABLED(CONFIG_DIMLIB)
> 	struct dim_irq_moder *moder = dev->irq_moder;
> 	struct net_device *dev = req_base->dev;

Note: The line above depends on the line below. :)

However, even so, I will try to make a change to keep "xmas tree order".

> 	u8 coal_flags;
> 	int ret;
> #endif 
> 
> Or better still, if it can be done cleanly, moving all the IS_ENABLED()
> code in this function into a separate function, with an no-op
> implementation in the case that CONFIG_DIMLIB is not enabled. In general
> I'd recommend that approach over sprinkling IS_ENABLED or #ifdef inside
> functions. Because in my experience it tends to lead to more readable code.
> 

As discussed in the previous replies, IS_ENABLED(CONFIG_DIMLIB) will be removed
in the next version, so we'll do not have this problem, but your suggestion is
good.

Thanks.

> In any case, the following tool is helpful for isolating
> reverse xmas tree issues.
> 
> https://github.com/ecree-solarflare/xmastree
> 
> 
> >  	if (coalesce_put_u32(skb, ETHTOOL_A_COALESCE_RX_USECS,
> > @@ -192,11 +276,49 @@ static int coalesce_fill_reply(struct sk_buff *skb,
> >  			     kcoal->tx_aggr_time_usecs, supported))
> >  		return -EMSGSIZE;
> >  
> > +#if IS_ENABLED(CONFIG_DIMLIB)
> > +	if (!moder)
> > +		return 0;
> > +
> > +	coal_flags = moder->coal_flags;
> > +	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),
> > +					   coal_flags);
> > +		if (ret) {
> > +			rcu_read_unlock();
> > +			return ret;
> > +		}
> > +	}
> > +
> > +	if (moder->profile_flags & DIM_PROFILE_TX) {
> > +		ret = coalesce_put_profile(skb, ETHTOOL_A_COALESCE_TX_PROFILE,
> > +					   rcu_dereference(moder->tx_profile),
> > +					   coal_flags);
> > +		if (ret) {
> > +			rcu_read_unlock();
> > +			return ret;
> > +		}
> > +	}
> > +	rcu_read_unlock();
> > +#endif
> >  	return 0;
> >  }
> 
> ...