[PATCH net-next v4 0/4] nfp: series of minor driver improvements

[PATCH net-next v4 0/4] nfp: series of minor driver improvements

[Louis Peens]

This short series bundles two unrelated but small updates to the nfp
driver.

Patch1: Add new define for devlink string "board.part_number"
Patch2: Make use of this field in the nfp driver
Patch3: Add new 'dim' profiles
Patch4: Make use of these new profiles in the nfp driver

Changes since V3:
- Fixed: Documentation/networking/devlink/devlink-info.rst:150:
    WARNING: Title underline too short.

Changes since V2:
- After some discussion on the previous series it was agreed that only
  the "board.part_number" field makes sense in the common code. The
  "board.model" field which was moved to devlink common code in V1 is
  now kept in the driver. The field is specific to the nfp driver,
  exposing the codename of the board.
- In summary, add "board.part_number" to devlink, and populate it
  in the the nfp driver.

Changes since V1:
- Move nfp local defines to devlink common code as it is quite generic.
- Add new 'dim' profile instead of using driver local overrides, as this
  allows use of the 'dim' helpers.
- This expanded 2 patches to 4, as the common code changes are split
  into seperate patches.

Fei Qin (4):
  devlink: add a new info version tag
  nfp: update devlink device info output
  dim: introduce a specific dim profile for better latency
  nfp: use new dim profiles for better latency

 .../networking/devlink/devlink-info.rst        |  5 +++++
 Documentation/networking/devlink/nfp.rst       |  5 ++++-
 .../net/ethernet/netronome/nfp/nfp_devlink.c   |  1 +
 .../ethernet/netronome/nfp/nfp_net_common.c    |  4 ++--
 include/linux/dim.h                            |  2 ++
 include/net/devlink.h                          |  4 +++-
 lib/dim/net_dim.c                              | 18 ++++++++++++++++++
 7 files changed, 35 insertions(+), 4 deletions(-)

-- 
2.34.1

[PATCH net-next v4 1/4] devlink: add a new info version tag

[Louis Peens]

From: Fei Qin <fei.qin@corigine.com>

Add definition and documentation for the new generic
info "board.part_number".

The new one is for part number specific use, and board.id
is modified to match the documentation in devlink-info.

Signed-off-by: Fei Qin <fei.qin@corigine.com>
Signed-off-by: Louis Peens <louis.peens@corigine.com>
---
 Documentation/networking/devlink/devlink-info.rst | 5 +++++
 include/net/devlink.h                             | 4 +++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/devlink/devlink-info.rst b/Documentation/networking/devlink/devlink-info.rst
index 1242b0e6826b..805ee673113b 100644
--- a/Documentation/networking/devlink/devlink-info.rst
+++ b/Documentation/networking/devlink/devlink-info.rst
@@ -146,6 +146,11 @@ board.manufacture
 
 An identifier of the company or the facility which produced the part.
 
+board.part_number
+-----------------
+
+Part number of the board design.
+
 fw
 --
 
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 9ac394bdfbe4..2100e62c2c2d 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -599,12 +599,14 @@ enum devlink_param_generic_id {
 	.validate = _validate,						\
 }
 
-/* Part number, identifier of board design */
+/* Identifier of board design */
 #define DEVLINK_INFO_VERSION_GENERIC_BOARD_ID	"board.id"
 /* Revision of board design */
 #define DEVLINK_INFO_VERSION_GENERIC_BOARD_REV	"board.rev"
 /* Maker of the board */
 #define DEVLINK_INFO_VERSION_GENERIC_BOARD_MANUFACTURE	"board.manufacture"
+/* Part number of board design */
+#define DEVLINK_INFO_VERSION_GENERIC_BOARD_PART_NUMBER	"board.part_number"
 
 /* Part number, identifier of asic design */
 #define DEVLINK_INFO_VERSION_GENERIC_ASIC_ID	"asic.id"
-- 
2.34.1

Re: [PATCH net-next v4 1/4] devlink: add a new info version tag

[Simon Horman]

On Fri, Apr 05, 2024 at 10:15:44AM +0200, Louis Peens wrote:
> From: Fei Qin <fei.qin@corigine.com>
> 
> Add definition and documentation for the new generic
> info "board.part_number".
> 
> The new one is for part number specific use, and board.id
> is modified to match the documentation in devlink-info.
> 
> Signed-off-by: Fei Qin <fei.qin@corigine.com>
> Signed-off-by: Louis Peens <louis.peens@corigine.com>

Thanks, I agree this is consistent with the review of v2 [1].
And addresses the documentation formatting problem of v3 [2].

Reviewed-by: Simon Horman <horms@kernel.org>

[1] https://lore.kernel.org/all/20240228075140.12085-2-louis.peens@corigine.com/
[2] https://lore.kernel.org/all/20240403145700.26881-2-louis.peens@corigine.com/

...

[PATCH net-next v4 2/4] nfp: update devlink device info output

[Louis Peens]

From: Fei Qin <fei.qin@corigine.com>

Newer NIC will introduce a new part number, now add it
into devlink device info.

This patch also updates the information of "board.id" in
nfp.rst to match the devlink-info.rst.

Signed-off-by: Fei Qin <fei.qin@corigine.com>
Signed-off-by: Louis Peens <louis.peens@corigine.com>
---
 Documentation/networking/devlink/nfp.rst         | 5 ++++-
 drivers/net/ethernet/netronome/nfp/nfp_devlink.c | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/devlink/nfp.rst b/Documentation/networking/devlink/nfp.rst
index a1717db0dfcc..a9bed03bf176 100644
--- a/Documentation/networking/devlink/nfp.rst
+++ b/Documentation/networking/devlink/nfp.rst
@@ -32,7 +32,7 @@ The ``nfp`` driver reports the following versions
      - Description
    * - ``board.id``
      - fixed
-     - Part number identifying the board design
+     - Identifier of the board design
    * - ``board.rev``
      - fixed
      - Revision of the board design
@@ -42,6 +42,9 @@ The ``nfp`` driver reports the following versions
    * - ``board.model``
      - fixed
      - Model name of the board design
+   * - ``board.part_number``
+     - fixed
+     - Part number of the board design
    * - ``fw.bundle_id``
      - stored, running
      - Firmware bundle id
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
index 635d33c0d6d3..ea75b9a06313 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
@@ -160,6 +160,7 @@ static const struct nfp_devlink_versions_simple {
 	{ DEVLINK_INFO_VERSION_GENERIC_BOARD_REV,	"assembly.revision", },
 	{ DEVLINK_INFO_VERSION_GENERIC_BOARD_MANUFACTURE, "assembly.vendor", },
 	{ "board.model", /* code name */		"assembly.model", },
+	{ DEVLINK_INFO_VERSION_GENERIC_BOARD_PART_NUMBER, "pn", },
 };
 
 static int
-- 
2.34.1

Re: [PATCH net-next v4 2/4] nfp: update devlink device info output

[Simon Horman]

On Fri, Apr 05, 2024 at 10:15:45AM +0200, Louis Peens wrote:
> From: Fei Qin <fei.qin@corigine.com>
> 
> Newer NIC will introduce a new part number, now add it
> into devlink device info.
> 
> This patch also updates the information of "board.id" in
> nfp.rst to match the devlink-info.rst.
> 
> Signed-off-by: Fei Qin <fei.qin@corigine.com>
> Signed-off-by: Louis Peens <louis.peens@corigine.com>

Thanks I agree this is consistent with review of v2 [1]

Reviewed-by: Simon Horman <horms@kernel.org>

[1] https://lore.kernel.org/all/20240228075140.12085-2-louis.peens@corigine.com/

Re: [PATCH net-next v4 2/4] nfp: update devlink device info output

[Jakub Kicinski]

On Fri,  5 Apr 2024 10:15:45 +0200 Louis Peens wrote:
> +   * - ``board.part_number``
> +     - fixed
> +     - Part number of the board design

Sorry, one more nit. Part number is not "of the board design"
Part number is a part number, AFAIU it's a customer-facing
identifier of a product. Let's drop the design or say "of the 
board and its components"? Since AFAIU part number may also
change if you replace a component even if you don't have to
redesign the PCB?

Re: [PATCH net-next v4 2/4] nfp: update devlink device info output

[Louis Peens]

On Mon, Apr 08, 2024 at 07:58:15PM -0700, Jakub Kicinski wrote:
> On Fri,  5 Apr 2024 10:15:45 +0200 Louis Peens wrote:
> > +   * - ``board.part_number``
> > +     - fixed
> > +     - Part number of the board design
> 
> Sorry, one more nit. Part number is not "of the board design"
> Part number is a part number, AFAIU it's a customer-facing
> identifier of a product. Let's drop the design or say "of the 
> board and its components"? Since AFAIU part number may also
> change if you replace a component even if you don't have to
> redesign the PCB?

I like "of the board and its components", will update, thanks.

[PATCH net-next v4 3/4] dim: introduce a specific dim profile for better latency

[Louis Peens]

From: Fei Qin <fei.qin@corigine.com>

The current profile is not well-adaptive to NFP NICs in
terms of latency, so introduce a specific profile for better
latency.

Signed-off-by: Fei Qin <fei.qin@corigine.com>
Signed-off-by: Louis Peens <louis.peens@corigine.com>
---
 include/linux/dim.h |  2 ++
 lib/dim/net_dim.c   | 18 ++++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/include/linux/dim.h b/include/linux/dim.h
index f343bc9aa2ec..edd6d7bceb28 100644
--- a/include/linux/dim.h
+++ b/include/linux/dim.h
@@ -119,11 +119,13 @@ struct dim {
  *
  * @DIM_CQ_PERIOD_MODE_START_FROM_EQE: Start counting from EQE
  * @DIM_CQ_PERIOD_MODE_START_FROM_CQE: Start counting from CQE (implies timer reset)
+ * @DIM_CQ_PERIOD_MODE_SPECIFIC_0: Specific mode to improve latency
  * @DIM_CQ_PERIOD_NUM_MODES: Number of modes
  */
 enum dim_cq_period_mode {
 	DIM_CQ_PERIOD_MODE_START_FROM_EQE = 0x0,
 	DIM_CQ_PERIOD_MODE_START_FROM_CQE = 0x1,
+	DIM_CQ_PERIOD_MODE_SPECIFIC_0 = 0x2,
 	DIM_CQ_PERIOD_NUM_MODES
 };
 
diff --git a/lib/dim/net_dim.c b/lib/dim/net_dim.c
index 4e32f7aaac86..2b5dccb6242c 100644
--- a/lib/dim/net_dim.c
+++ b/lib/dim/net_dim.c
@@ -33,6 +33,14 @@
 	{.usec = 64, .pkts = 64,}               \
 }
 
+#define NET_DIM_RX_SPECIFIC_0_PROFILES { \
+	{.usec = 0,   .pkts = 1,},   \
+	{.usec = 4,   .pkts = 32,},  \
+	{.usec = 64,  .pkts = 64,},  \
+	{.usec = 128, .pkts = 256,}, \
+	{.usec = 256, .pkts = 256,}  \
+}
+
 #define NET_DIM_TX_EQE_PROFILES { \
 	{.usec = 1,   .pkts = NET_DIM_DEFAULT_TX_CQ_PKTS_FROM_EQE,},  \
 	{.usec = 8,   .pkts = NET_DIM_DEFAULT_TX_CQ_PKTS_FROM_EQE,},  \
@@ -49,16 +57,26 @@
 	{.usec = 64, .pkts = 32,}   \
 }
 
+#define NET_DIM_TX_SPECIFIC_0_PROFILES { \
+	{.usec = 0,   .pkts = 1,},   \
+	{.usec = 4,   .pkts = 16,},  \
+	{.usec = 32,  .pkts = 64,},  \
+	{.usec = 64,  .pkts = 128,}, \
+	{.usec = 128, .pkts = 128,}  \
+}
+
 static const struct dim_cq_moder
 rx_profile[DIM_CQ_PERIOD_NUM_MODES][NET_DIM_PARAMS_NUM_PROFILES] = {
 	NET_DIM_RX_EQE_PROFILES,
 	NET_DIM_RX_CQE_PROFILES,
+	NET_DIM_RX_SPECIFIC_0_PROFILES,
 };
 
 static const struct dim_cq_moder
 tx_profile[DIM_CQ_PERIOD_NUM_MODES][NET_DIM_PARAMS_NUM_PROFILES] = {
 	NET_DIM_TX_EQE_PROFILES,
 	NET_DIM_TX_CQE_PROFILES,
+	NET_DIM_TX_SPECIFIC_0_PROFILES,
 };
 
 struct dim_cq_moder
-- 
2.34.1

Re: [PATCH net-next v4 3/4] dim: introduce a specific dim profile for better latency

[Simon Horman]

+ Tal Gilboa

On Fri, Apr 05, 2024 at 10:15:46AM +0200, Louis Peens wrote:
> From: Fei Qin <fei.qin@corigine.com>
> 
> The current profile is not well-adaptive to NFP NICs in
> terms of latency, so introduce a specific profile for better
> latency.
> 
> Signed-off-by: Fei Qin <fei.qin@corigine.com>
> Signed-off-by: Louis Peens <louis.peens@corigine.com>

Noting that this is consistent of review of v1 [1], this looks good to me.

Reviewed-by: Simon Horman <horms@kernel.org>

[1] https://lore.kernel.org/all/20240131085426.45374-3-louis.peens@corigine.com/

> ---
>  include/linux/dim.h |  2 ++
>  lib/dim/net_dim.c   | 18 ++++++++++++++++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/include/linux/dim.h b/include/linux/dim.h
> index f343bc9aa2ec..edd6d7bceb28 100644
> --- a/include/linux/dim.h
> +++ b/include/linux/dim.h
> @@ -119,11 +119,13 @@ struct dim {
>   *
>   * @DIM_CQ_PERIOD_MODE_START_FROM_EQE: Start counting from EQE
>   * @DIM_CQ_PERIOD_MODE_START_FROM_CQE: Start counting from CQE (implies timer reset)
> + * @DIM_CQ_PERIOD_MODE_SPECIFIC_0: Specific mode to improve latency
>   * @DIM_CQ_PERIOD_NUM_MODES: Number of modes
>   */
>  enum dim_cq_period_mode {
>  	DIM_CQ_PERIOD_MODE_START_FROM_EQE = 0x0,
>  	DIM_CQ_PERIOD_MODE_START_FROM_CQE = 0x1,
> +	DIM_CQ_PERIOD_MODE_SPECIFIC_0 = 0x2,
>  	DIM_CQ_PERIOD_NUM_MODES
>  };
>  
> diff --git a/lib/dim/net_dim.c b/lib/dim/net_dim.c
> index 4e32f7aaac86..2b5dccb6242c 100644
> --- a/lib/dim/net_dim.c
> +++ b/lib/dim/net_dim.c
> @@ -33,6 +33,14 @@
>  	{.usec = 64, .pkts = 64,}               \
>  }
>  
> +#define NET_DIM_RX_SPECIFIC_0_PROFILES { \
> +	{.usec = 0,   .pkts = 1,},   \
> +	{.usec = 4,   .pkts = 32,},  \
> +	{.usec = 64,  .pkts = 64,},  \
> +	{.usec = 128, .pkts = 256,}, \
> +	{.usec = 256, .pkts = 256,}  \
> +}
> +
>  #define NET_DIM_TX_EQE_PROFILES { \
>  	{.usec = 1,   .pkts = NET_DIM_DEFAULT_TX_CQ_PKTS_FROM_EQE,},  \
>  	{.usec = 8,   .pkts = NET_DIM_DEFAULT_TX_CQ_PKTS_FROM_EQE,},  \
> @@ -49,16 +57,26 @@
>  	{.usec = 64, .pkts = 32,}   \
>  }
>  
> +#define NET_DIM_TX_SPECIFIC_0_PROFILES { \
> +	{.usec = 0,   .pkts = 1,},   \
> +	{.usec = 4,   .pkts = 16,},  \
> +	{.usec = 32,  .pkts = 64,},  \
> +	{.usec = 64,  .pkts = 128,}, \
> +	{.usec = 128, .pkts = 128,}  \
> +}
> +
>  static const struct dim_cq_moder
>  rx_profile[DIM_CQ_PERIOD_NUM_MODES][NET_DIM_PARAMS_NUM_PROFILES] = {
>  	NET_DIM_RX_EQE_PROFILES,
>  	NET_DIM_RX_CQE_PROFILES,
> +	NET_DIM_RX_SPECIFIC_0_PROFILES,
>  };
>  
>  static const struct dim_cq_moder
>  tx_profile[DIM_CQ_PERIOD_NUM_MODES][NET_DIM_PARAMS_NUM_PROFILES] = {
>  	NET_DIM_TX_EQE_PROFILES,
>  	NET_DIM_TX_CQE_PROFILES,
> +	NET_DIM_TX_SPECIFIC_0_PROFILES,
>  };
>  
>  struct dim_cq_moder
> -- 
> 2.34.1
> 
> 

Re: [PATCH net-next v4 3/4] dim: introduce a specific dim profile for better latency

[Jakub Kicinski]

On Fri,  5 Apr 2024 10:15:46 +0200 Louis Peens wrote:
> From: Fei Qin <fei.qin@corigine.com>
> 
> The current profile is not well-adaptive to NFP NICs in
> terms of latency, so introduce a specific profile for better
> latency.

Let's drop the dim patches for now, please.
Adding random profiles to shared code doesn't look great and Heng Qi 
is re-working DIM to make it more configurable:
https://lore.kernel.org/all/1712547870-112976-2-git-send-email-hengqi@linux.alibaba.com/

Re: [PATCH net-next v4 3/4] dim: introduce a specific dim profile for better latency

[Louis Peens]

On Mon, Apr 08, 2024 at 07:55:35PM -0700, Jakub Kicinski wrote:
> On Fri,  5 Apr 2024 10:15:46 +0200 Louis Peens wrote:
> > From: Fei Qin <fei.qin@corigine.com>
> > 
> > The current profile is not well-adaptive to NFP NICs in
> > terms of latency, so introduce a specific profile for better
> > latency.
> 
> Let's drop the dim patches for now, please.
> Adding random profiles to shared code doesn't look great and Heng Qi 
> is re-working DIM to make it more configurable:
> https://lore.kernel.org/all/1712547870-112976-2-git-send-email-hengqi@linux.alibaba.com/
I nice, this looks like the kind of thing that was missing, will take a
closer look. And also drop the dim patches. Thanks.

[PATCH net-next v4 4/4] nfp: use new dim profiles for better latency

[Louis Peens]

From: Fei Qin <fei.qin@corigine.com>

Latency comparison between EQE profiles and SPECIFIC_0 profiles
for 5 different runs:

                                     Latency (us)
EQE profiles 	    |	132.85  136.32  131.31  131.37  133.51
SPECIFIC_0 profiles |	92.09   92.16   95.58   98.26   89.79

Signed-off-by: Fei Qin <fei.qin@corigine.com>
Signed-off-by: Louis Peens <louis.peens@corigine.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index 997cc4fcffdb..4c2dac1e1be4 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -1232,12 +1232,12 @@ static void nfp_net_open_stack(struct nfp_net *nn)
 
 		if (r_vec->rx_ring) {
 			INIT_WORK(&r_vec->rx_dim.work, nfp_net_rx_dim_work);
-			r_vec->rx_dim.mode = DIM_CQ_PERIOD_MODE_START_FROM_EQE;
+			r_vec->rx_dim.mode = DIM_CQ_PERIOD_MODE_SPECIFIC_0;
 		}
 
 		if (r_vec->tx_ring) {
 			INIT_WORK(&r_vec->tx_dim.work, nfp_net_tx_dim_work);
-			r_vec->tx_dim.mode = DIM_CQ_PERIOD_MODE_START_FROM_EQE;
+			r_vec->tx_dim.mode = DIM_CQ_PERIOD_MODE_SPECIFIC_0;
 		}
 
 		napi_enable(&r_vec->napi);
-- 
2.34.1

Re: [PATCH net-next v4 4/4] nfp: use new dim profiles for better latency

[Simon Horman]

On Fri, Apr 05, 2024 at 10:15:47AM +0200, Louis Peens wrote:
> From: Fei Qin <fei.qin@corigine.com>
> 
> Latency comparison between EQE profiles and SPECIFIC_0 profiles
> for 5 different runs:
> 
>                                      Latency (us)
> EQE profiles 	    |	132.85  136.32  131.31  131.37  133.51
> SPECIFIC_0 profiles |	92.09   92.16   95.58   98.26   89.79

Nice improvement :)

> Signed-off-by: Fei Qin <fei.qin@corigine.com>
> Signed-off-by: Louis Peens <louis.peens@corigine.com>

Reviewed-by: Simon Horman <horms@kernel.org>

...