diff mbox series

[v2] net/ixgbe: initialize port even if mtu config fails

Message ID 1634753626-84056-1-git-send-email-tudor.cornea@gmail.com (mailing list archive)
State Accepted, archived
Delegated to: Qi Zhang
Headers
Series [v2] net/ixgbe: initialize port even if mtu config fails |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/iol-testing warning apply patch failure
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS

Commit Message

Tudor Cornea Oct. 20, 2021, 6:13 p.m. UTC 
  On a VMware ESXi 6.0 setup with an Intel 82599 NIC the ports don't
seem to initialize anymore, while running testpmd.

Configuring Port 0 (socket 0)
ixgbevf_dev_rx_init(): Set max packet length to 1518 failed.
ixgbevf_dev_start(): Unable to initialize RX hardware (-22)
Fail to start port 0: Invalid argument
Configuring Port 1 (socket 0)
ixgbevf_dev_rx_init(): Set max packet length to 1518 failed.
ixgbevf_dev_start(): Unable to initialize RX hardware (-22)
Fail to start port 1: Invalid argument
Please stop the ports first

If the call to ixgbevf_rlpml_set_vf fails and we return prematurely,
we will not be able to initialize the ports correctly.

The behavior seems to have changed since the following commit:

commit c77866a16904 ("net/ixgbe: detect failed VF MTU set")

We can make this particular use case work correctly if we don't
return an error, which seems to be consistent with the overall
kernel ixgbevf implementation.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c#n2015

Signed-off-by: Tudor Cornea <tudor.cornea@gmail.com>

---
v2:
* Change title
* Remove max_rx_pkt_len fix in ixgbe_ethdev.c
  It's already fixed as part of Ferruh's changes in next-net branch,
  so this part should be redundant, now
---
 drivers/net/ixgbe/ixgbe_rxtx.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Wang, Haiyue Oct. 21, 2021, 1:14 a.m. UTC | #1 
> -----Original Message-----
> From: Tudor Cornea <tudor.cornea@gmail.com>
> Sent: Thursday, October 21, 2021 02:14
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: Wang, Haiyue <haiyue.wang@intel.com>; Zhang, AlvinX <alvinx.zhang@intel.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>; dev@dpdk.org; Tudor Cornea <tudor.cornea@gmail.com>
> Subject: [PATCH v2] net/ixgbe: initialize port even if mtu config fails
> 
> On a VMware ESXi 6.0 setup with an Intel 82599 NIC the ports don't
> seem to initialize anymore, while running testpmd.
> 
> Configuring Port 0 (socket 0)
> ixgbevf_dev_rx_init(): Set max packet length to 1518 failed.
> ixgbevf_dev_start(): Unable to initialize RX hardware (-22)
> Fail to start port 0: Invalid argument
> Configuring Port 1 (socket 0)
> ixgbevf_dev_rx_init(): Set max packet length to 1518 failed.
> ixgbevf_dev_start(): Unable to initialize RX hardware (-22)
> Fail to start port 1: Invalid argument
> Please stop the ports first
> 
> If the call to ixgbevf_rlpml_set_vf fails and we return prematurely,
> we will not be able to initialize the ports correctly.
> 
> The behavior seems to have changed since the following commit:
> 
> commit c77866a16904 ("net/ixgbe: detect failed VF MTU set")
> 
> We can make this particular use case work correctly if we don't
> return an error, which seems to be consistent with the overall
> kernel ixgbevf implementation.
> 
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/intel/ixg
> bevf/ixgbevf_main.c#n2015
> 
> Signed-off-by: Tudor Cornea <tudor.cornea@gmail.com>
> 
> ---
> v2:
> * Change title
> * Remove max_rx_pkt_len fix in ixgbe_ethdev.c
>   It's already fixed as part of Ferruh's changes in next-net branch,
>   so this part should be redundant, now
> ---
>  drivers/net/ixgbe/ixgbe_rxtx.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 

Thanks!

Acked-by: Haiyue Wang <haiyue.wang@intel.com>


> --
> 2.7.4
Qi Zhang Oct. 21, 2021, 2:54 a.m. UTC | #2 
> -----Original Message-----
> From: Wang, Haiyue <haiyue.wang@intel.com>
> Sent: Thursday, October 21, 2021 9:14 AM
> To: Tudor Cornea <tudor.cornea@gmail.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Cc: Zhang, AlvinX <alvinx.zhang@intel.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>; dev@dpdk.org
> Subject: RE: [PATCH v2] net/ixgbe: initialize port even if mtu config fails
> 
> > -----Original Message-----
> > From: Tudor Cornea <tudor.cornea@gmail.com>
> > Sent: Thursday, October 21, 2021 02:14
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>
> > Cc: Wang, Haiyue <haiyue.wang@intel.com>; Zhang, AlvinX
> > <alvinx.zhang@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> > dev@dpdk.org; Tudor Cornea <tudor.cornea@gmail.com>
> > Subject: [PATCH v2] net/ixgbe: initialize port even if mtu config
> > fails
> >
> > On a VMware ESXi 6.0 setup with an Intel 82599 NIC the ports don't
> > seem to initialize anymore, while running testpmd.
> >
> > Configuring Port 0 (socket 0)
> > ixgbevf_dev_rx_init(): Set max packet length to 1518 failed.
> > ixgbevf_dev_start(): Unable to initialize RX hardware (-22) Fail to
> > start port 0: Invalid argument Configuring Port 1 (socket 0)
> > ixgbevf_dev_rx_init(): Set max packet length to 1518 failed.
> > ixgbevf_dev_start(): Unable to initialize RX hardware (-22) Fail to
> > start port 1: Invalid argument Please stop the ports first
> >
> > If the call to ixgbevf_rlpml_set_vf fails and we return prematurely,
> > we will not be able to initialize the ports correctly.
> >
> > The behavior seems to have changed since the following commit:
> >
> > commit c77866a16904 ("net/ixgbe: detect failed VF MTU set")
> >
> > We can make this particular use case work correctly if we don't return
> > an error, which seems to be consistent with the overall kernel ixgbevf
> > implementation.
> >
> > [1]
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tre
> > e/drivers/net/ethernet/intel/ixg
> > bevf/ixgbevf_main.c#n2015
> >
> > Signed-off-by: Tudor Cornea <tudor.cornea@gmail.com>
> >
> > ---
> > v2:
> > * Change title
> > * Remove max_rx_pkt_len fix in ixgbe_ethdev.c
> >   It's already fixed as part of Ferruh's changes in next-net branch,
> >   so this part should be redundant, now
> > ---
> >  drivers/net/ixgbe/ixgbe_rxtx.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> 
> Thanks!
> 
> Acked-by: Haiyue Wang <haiyue.wang@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi
> 
> 
> > --
> > 2.7.4
>
Ferruh Yigit Oct. 21, 2021, 3:33 p.m. UTC | #3 
On 10/20/2021 7:13 PM, Tudor Cornea wrote:
> On a VMware ESXi 6.0 setup with an Intel 82599 NIC the ports don't
> seem to initialize anymore, while running testpmd.
> 
> Configuring Port 0 (socket 0)
> ixgbevf_dev_rx_init(): Set max packet length to 1518 failed.
> ixgbevf_dev_start(): Unable to initialize RX hardware (-22)
> Fail to start port 0: Invalid argument
> Configuring Port 1 (socket 0)
> ixgbevf_dev_rx_init(): Set max packet length to 1518 failed.
> ixgbevf_dev_start(): Unable to initialize RX hardware (-22)
> Fail to start port 1: Invalid argument
> Please stop the ports first
> 
> If the call to ixgbevf_rlpml_set_vf fails and we return prematurely,
> we will not be able to initialize the ports correctly.
> 
> The behavior seems to have changed since the following commit:
> 
> commit c77866a16904 ("net/ixgbe: detect failed VF MTU set")
> 

Hi Tudor,

We document this with explicit 'Fixes' tag, this also helps up to
manage backporting patches to LTS releases, so updating as:

     Fixes: 3a6bfc37eaf4 ("net/ice: support QoS config VF bandwidth in DCF")
     Cc: stable@dpdk.org

Also we use 'fix' verb in the patch title almost as keyword, again
to help deciding which patch to backport, also to clarify impact of
the patch, so will update patch title as:

     net/ixgbe: fix port initialization if MTU config fails


For more details please check contribution guide:
https://doc.dpdk.org/guides/contributing/patches.html

Thanks,
ferruh

> We can make this particular use case work correctly if we don't
> return an error, which seems to be consistent with the overall
> kernel ixgbevf implementation.
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c#n2015
> 

The code that this link references can change by time as code changes,
I will update it as following to bind it a specific version (v5.14):
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c?h=v5.14#n2015

> Signed-off-by: Tudor Cornea <tudor.cornea@gmail.com>
> 
> ---
> v2:
> * Change title
> * Remove max_rx_pkt_len fix in ixgbe_ethdev.c
>    It's already fixed as part of Ferruh's changes in next-net branch,
>    so this part should be redundant, now
> ---
>   drivers/net/ixgbe/ixgbe_rxtx.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index b263dfe..a51450f 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -5673,11 +5673,9 @@ ixgbevf_dev_rx_init(struct rte_eth_dev *dev)
>   	 * ixgbevf_rlpml_set_vf even if jumbo frames are not used. This way,
>   	 * VF packets received can work in all cases.
>   	 */
> -	if (ixgbevf_rlpml_set_vf(hw, frame_size) != 0) {
> +	if (ixgbevf_rlpml_set_vf(hw, frame_size) != 0)
>   		PMD_INIT_LOG(ERR, "Set max packet length to %d failed.",
>   			     frame_size);
> -		return -EINVAL;
> -	}
>   
>   	/*
>   	 * Assume no header split and no VLAN strip support
>
Tudor Cornea Oct. 21, 2021, 4:40 p.m. UTC | #4 
On Thu, 21 Oct 2021 at 18:33, Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 10/20/2021 7:13 PM, Tudor Cornea wrote:
> > On a VMware ESXi 6.0 setup with an Intel 82599 NIC the ports don't
> > seem to initialize anymore, while running testpmd.
> >
> > Configuring Port 0 (socket 0)
> > ixgbevf_dev_rx_init(): Set max packet length to 1518 failed.
> > ixgbevf_dev_start(): Unable to initialize RX hardware (-22)
> > Fail to start port 0: Invalid argument
> > Configuring Port 1 (socket 0)
> > ixgbevf_dev_rx_init(): Set max packet length to 1518 failed.
> > ixgbevf_dev_start(): Unable to initialize RX hardware (-22)
> > Fail to start port 1: Invalid argument
> > Please stop the ports first
> >
> > If the call to ixgbevf_rlpml_set_vf fails and we return prematurely,
> > we will not be able to initialize the ports correctly.
> >
> > The behavior seems to have changed since the following commit:
> >
> > commit c77866a16904 ("net/ixgbe: detect failed VF MTU set")
> >
>
> Hi Tudor,
>
> We document this with explicit 'Fixes' tag, this also helps up to
> manage backporting patches to LTS releases, so updating as:
>
>      Fixes: 3a6bfc37eaf4 ("net/ice: support QoS config VF bandwidth in
> DCF")
>      Cc: stable@dpdk.org
>
> Also we use 'fix' verb in the patch title almost as keyword, again
> to help deciding which patch to backport, also to clarify impact of
> the patch, so will update patch title as:
>
>      net/ixgbe: fix port initialization if MTU config fails
>
>
> For more details please check contribution guide:
> https://doc.dpdk.org/guides/contributing/patches.html
>
> Thanks,
> ferruh
>
> > We can make this particular use case work correctly if we don't
> > return an error, which seems to be consistent with the overall
> > kernel ixgbevf implementation.
> >
> > [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c#n2015
> >
>
> The code that this link references can change by time as code changes,
> I will update it as following to bind it a specific version (v5.14):
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c?h=v5.14#n2015
>
> > Signed-off-by: Tudor Cornea <tudor.cornea@gmail.com>
> >
> > ---
> > v2:
> > * Change title
> > * Remove max_rx_pkt_len fix in ixgbe_ethdev.c
> >    It's already fixed as part of Ferruh's changes in next-net branch,
> >    so this part should be redundant, now
> > ---
> >   drivers/net/ixgbe/ixgbe_rxtx.c | 4 +---
> >   1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c
> b/drivers/net/ixgbe/ixgbe_rxtx.c
> > index b263dfe..a51450f 100644
> > --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> > +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> > @@ -5673,11 +5673,9 @@ ixgbevf_dev_rx_init(struct rte_eth_dev *dev)
> >        * ixgbevf_rlpml_set_vf even if jumbo frames are not used. This
> way,
> >        * VF packets received can work in all cases.
> >        */
> > -     if (ixgbevf_rlpml_set_vf(hw, frame_size) != 0) {
> > +     if (ixgbevf_rlpml_set_vf(hw, frame_size) != 0)
> >               PMD_INIT_LOG(ERR, "Set max packet length to %d failed.",
> >                            frame_size);
> > -             return -EINVAL;
> > -     }
> >
> >       /*
> >        * Assume no header split and no VLAN strip support
> >
>
>
Hi Ferruh,

Thanks a lot for the good observations and for pointing me in the right
direction !

Tudor
diff mbox series

Patch

diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index b263dfe..a51450f 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -5673,11 +5673,9 @@  ixgbevf_dev_rx_init(struct rte_eth_dev *dev)
 	 * ixgbevf_rlpml_set_vf even if jumbo frames are not used. This way,
 	 * VF packets received can work in all cases.
 	 */
-	if (ixgbevf_rlpml_set_vf(hw, frame_size) != 0) {
+	if (ixgbevf_rlpml_set_vf(hw, frame_size) != 0)
 		PMD_INIT_LOG(ERR, "Set max packet length to %d failed.",
 			     frame_size);
-		return -EINVAL;
-	}
 
 	/*
 	 * Assume no header split and no VLAN strip support