net/nfp: fix MTU configuration order

Message ID 20230308023318.3041151-1-chaoyong.he@corigine.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series net/nfp: fix MTU configuration order |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/github-robot: build success github build: passed
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-abi-testing success Testing PASS

Commit Message

Chaoyong He March 8, 2023, 2:33 a.m. UTC
  From: Peng Zhang <peng.zhang@corigine.com>

If rte_eth_dev_set_mtu() is called before rte_eth_rx_queue_setup() the
NFP driver setup fails. This is because the default values evaluated
when setting the MTU are initialized in the rte_eth_rx_queue_setup()
code path. Fix this by instead initializing the MTU default values in
the device initialization, in nfp_net_init() and the check also is
conducted in nfp_net_start(), so it doesn't influence the result.

This was found by using DPDK with OVS.

Fixes: dbad6f64f921 ("net/nfp: fix internal buffer size and MTU check")
Cc: stable@dpdk.org

Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
---
 drivers/net/nfp/flower/nfp_flower.c | 1 -
 drivers/net/nfp/nfp_common.c        | 4 ++--
 drivers/net/nfp/nfp_common.h        | 1 +
 drivers/net/nfp/nfp_ethdev.c        | 1 +
 drivers/net/nfp/nfp_ethdev_vf.c     | 1 +
 5 files changed, 5 insertions(+), 3 deletions(-)
  

Comments

Stephen Hemminger March 8, 2023, 2:44 a.m. UTC | #1
On Wed,  8 Mar 2023 10:33:18 +0800
Chaoyong He <chaoyong.he@corigine.com> wrote:

> diff --git a/drivers/net/nfp/nfp_common.c b/drivers/net/nfp/nfp_common.c
> index 5922bfea8e..5d92b476e2 100644
> --- a/drivers/net/nfp/nfp_common.c
> +++ b/drivers/net/nfp/nfp_common.c
> @@ -1126,9 +1126,9 @@ nfp_net_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
>  		return -EBUSY;
>  	}
>  
> -	/* MTU larger then current mbufsize not supported */
> +	/* MTU larger than current mbufsize not supported */
>  	if (mtu > hw->flbufsz) {
> -		PMD_DRV_LOG(ERR, "MTU (%u) larger then current mbufsize (%u) not supported",
> +		PMD_DRV_LOG(ERR, "MTU (%u) larger than current mbufsize (%u) not supported",
>  			    mtu, hw->flbufsz);
>  		return -ERANGE;
>  	}

Patch looks good but this looks like unrelated whitespace change.
  
Niklas Söderlund March 8, 2023, 8:19 a.m. UTC | #2
Hi Stephen,

Thanks for your feedback.

On 2023-03-07 18:44:09 -0800, Stephen Hemminger wrote:
> On Wed,  8 Mar 2023 10:33:18 +0800
> Chaoyong He <chaoyong.he@corigine.com> wrote:
> 
> > diff --git a/drivers/net/nfp/nfp_common.c b/drivers/net/nfp/nfp_common.c
> > index 5922bfea8e..5d92b476e2 100644
> > --- a/drivers/net/nfp/nfp_common.c
> > +++ b/drivers/net/nfp/nfp_common.c
> > @@ -1126,9 +1126,9 @@ nfp_net_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
> >  		return -EBUSY;
> >  	}
> >  
> > -	/* MTU larger then current mbufsize not supported */
> > +	/* MTU larger than current mbufsize not supported */
> >  	if (mtu > hw->flbufsz) {
> > -		PMD_DRV_LOG(ERR, "MTU (%u) larger then current mbufsize (%u) not supported",
> > +		PMD_DRV_LOG(ERR, "MTU (%u) larger than current mbufsize (%u) not supported",
> >  			    mtu, hw->flbufsz);
> >  		return -ERANGE;
> >  	}
> 
> Patch looks good but this looks like unrelated whitespace change.

It's a tad unrelated, but not a whitespace change. It fixes a spelling 
mistake related to setting the MTU,

    s/then/than/g
  
Ferruh Yigit March 8, 2023, 3:27 p.m. UTC | #3
On 3/8/2023 2:33 AM, Chaoyong He wrote:
> From: Peng Zhang <peng.zhang@corigine.com>
> 
> If rte_eth_dev_set_mtu() is called before rte_eth_rx_queue_setup() the
> NFP driver setup fails. This is because the default values evaluated
> when setting the MTU are initialized in the rte_eth_rx_queue_setup()
> code path. Fix this by instead initializing the MTU default values in
> the device initialization, in nfp_net_init() and the check also is
> conducted in nfp_net_start(), so it doesn't influence the result.
> 
> This was found by using DPDK with OVS.
> 
> Fixes: dbad6f64f921 ("net/nfp: fix internal buffer size and MTU check")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>

Applied to dpdk-next-net/main, thanks.
  

Patch

diff --git a/drivers/net/nfp/flower/nfp_flower.c b/drivers/net/nfp/flower/nfp_flower.c
index 2c797ae751..6f197396a4 100644
--- a/drivers/net/nfp/flower/nfp_flower.c
+++ b/drivers/net/nfp/flower/nfp_flower.c
@@ -24,7 +24,6 @@ 
 #include "nfp_flower_cmsg.h"
 
 #define CTRL_VNIC_NB_DESC 512
-#define DEFAULT_FLBUF_SIZE 9216
 
 static void
 nfp_pf_repr_enable_queues(struct rte_eth_dev *dev)
diff --git a/drivers/net/nfp/nfp_common.c b/drivers/net/nfp/nfp_common.c
index 5922bfea8e..5d92b476e2 100644
--- a/drivers/net/nfp/nfp_common.c
+++ b/drivers/net/nfp/nfp_common.c
@@ -1126,9 +1126,9 @@  nfp_net_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 		return -EBUSY;
 	}
 
-	/* MTU larger then current mbufsize not supported */
+	/* MTU larger than current mbufsize not supported */
 	if (mtu > hw->flbufsz) {
-		PMD_DRV_LOG(ERR, "MTU (%u) larger then current mbufsize (%u) not supported",
+		PMD_DRV_LOG(ERR, "MTU (%u) larger than current mbufsize (%u) not supported",
 			    mtu, hw->flbufsz);
 		return -ERANGE;
 	}
diff --git a/drivers/net/nfp/nfp_common.h b/drivers/net/nfp/nfp_common.h
index 49c89ac327..4486ffa72c 100644
--- a/drivers/net/nfp/nfp_common.h
+++ b/drivers/net/nfp/nfp_common.h
@@ -111,6 +111,7 @@  struct nfp_net_adapter;
 
 /* Maximum supported NFP frame size (MTU + layer 2 headers) */
 #define NFP_FRAME_SIZE_MAX	10048
+#define DEFAULT_FLBUF_SIZE        9216
 
 #include <linux/types.h>
 #include <rte_io.h>
diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 47d5dff16c..56fb8e8c73 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -603,6 +603,7 @@  nfp_net_init(struct rte_eth_dev *eth_dev)
 	hw->cap = nn_cfg_readl(hw, NFP_NET_CFG_CAP);
 	hw->max_mtu = nn_cfg_readl(hw, NFP_NET_CFG_MAX_MTU);
 	hw->mtu = RTE_ETHER_MTU;
+	hw->flbufsz = DEFAULT_FLBUF_SIZE;
 
 	/* VLAN insertion is incompatible with LSOv2 */
 	if (hw->cap & NFP_NET_CFG_CTRL_LSO2)
diff --git a/drivers/net/nfp/nfp_ethdev_vf.c b/drivers/net/nfp/nfp_ethdev_vf.c
index 7834b2ee0c..d69ac8cd37 100644
--- a/drivers/net/nfp/nfp_ethdev_vf.c
+++ b/drivers/net/nfp/nfp_ethdev_vf.c
@@ -365,6 +365,7 @@  nfp_netvf_init(struct rte_eth_dev *eth_dev)
 	hw->cap = nn_cfg_readl(hw, NFP_NET_CFG_CAP);
 	hw->max_mtu = nn_cfg_readl(hw, NFP_NET_CFG_MAX_MTU);
 	hw->mtu = RTE_ETHER_MTU;
+	hw->flbufsz = DEFAULT_FLBUF_SIZE;
 
 	/* VLAN insertion is incompatible with LSOv2 */
 	if (hw->cap & NFP_NET_CFG_CTRL_LSO2)