[dpdk-dev] [PATCH] net/liquidio:Fix Unable to update lio_dev->linfo.link var

[dpdk-dev] [PATCH] net/liquidio:Fix Unable to update lio_dev->linfo.link var

[yaochuhong]

When I was using VPP +dpdk-18.02+liqudio CN23xx, I encountered such a bug. \n
When VPP called dpdk_device_start to initialize DPDK liqudio drive, I found that initialization failed. \n
The reason for the failure is that VF MTU > PF MTU, but PF MTU has been modified to 9600 (> VF MTU). \n
Finally, I am location that DPDK liqudio drive cannot get the correct PF driver to liqudio network card. \n
It is due to the fact that when VPP calls dpdk_device_start to initialize DPDK liqudio drive,  \n
this time, lio_dev->linfo. Link var already exists in the old value, not empty. \n
Cause lio_dev - > linfo. Link. Link_status64 != 0 statement is set up,  \n
and the link info is stopped directly to liqudio card, resulting in no get accurate pf mtu. \n
I did a test model to reproduce the bug, which is to add rte_eth_dev_set_mtu(portid, vf_mtu)  \n
to the rte_eth_dev_start function when using dpdk-18.02+liqudio CN23xx+l2fwd. \n
You need to make sure that 1500 < vf_mtu < pf_mtu will be available.  \n
At this time, you will have net_liovf[04:00.3]ERROR: lio_dev_mtu_set() VF MTU should be >= 68 and <= 1500. Such a mistake. \n
ps:I am a novice git. Please understand the trouble caused by this.

Signed-off-by: yaochuhong <ych@panath.cn>
---
 drivers/net/liquidio/lio_ethdev.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/liquidio/lio_ethdev.c b/drivers/net/liquidio/lio_ethdev.c
index 0e0b5d8..50743c7 100644
--- a/drivers/net/liquidio/lio_ethdev.c
+++ b/drivers/net/liquidio/lio_ethdev.c
@@ -1405,6 +1405,9 @@ struct rte_lio_xstats_name_off {
 	/* Configure RSS if device configured with multiple RX queues. */
 	lio_dev_mq_rx_configure(eth_dev);
 
+	/* Before update the link info, must set linfo.link.link_status64 to 0. */
+	lio_dev->linfo.link.link_status64 = 0;
+
 	/* start polling for lsc */
 	ret = rte_eal_alarm_set(LIO_LSC_TIMEOUT,
 				lio_sync_link_state_check,
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH] net/liquidio:Fix Unable to update lio_dev->linfo.link var

[Shijith Thotton]

On Mon, May 21, 2018 at 08:58:15PM +0800, yaochuhong wrote:
> When I was using VPP +dpdk-18.02+liqudio CN23xx, I encountered such a bug. \n
> When VPP called dpdk_device_start to initialize DPDK liqudio drive, I found that initialization failed. \n
> The reason for the failure is that VF MTU > PF MTU, but PF MTU has been modified to 9600 (> VF MTU). \n
> Finally, I am location that DPDK liqudio drive cannot get the correct PF driver to liqudio network card. \n
> It is due to the fact that when VPP calls dpdk_device_start to initialize DPDK liqudio drive,  \n
> this time, lio_dev->linfo. Link var already exists in the old value, not empty. \n
> Cause lio_dev - > linfo. Link. Link_status64 != 0 statement is set up,  \n
> and the link info is stopped directly to liqudio card, resulting in no get accurate pf mtu. \n
> I did a test model to reproduce the bug, which is to add rte_eth_dev_set_mtu(portid, vf_mtu)  \n
> to the rte_eth_dev_start function when using dpdk-18.02+liqudio CN23xx+l2fwd. \n
> You need to make sure that 1500 < vf_mtu < pf_mtu will be available.  \n
> At this time, you will have net_liovf[04:00.3]ERROR: lio_dev_mtu_set() VF MTU should be >= 68 and <= 1500. Such a mistake. \n
> ps:I am a novice git. Please understand the trouble caused by this.
> 
> Signed-off-by: yaochuhong <ych@panath.cn>
> ---
>  drivers/net/liquidio/lio_ethdev.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/liquidio/lio_ethdev.c b/drivers/net/liquidio/lio_ethdev.c
> index 0e0b5d8..50743c7 100644
> --- a/drivers/net/liquidio/lio_ethdev.c
> +++ b/drivers/net/liquidio/lio_ethdev.c
> @@ -1405,6 +1405,9 @@ struct rte_lio_xstats_name_off {
>  	/* Configure RSS if device configured with multiple RX queues. */
>  	lio_dev_mq_rx_configure(eth_dev);
>  
> +	/* Before update the link info, must set linfo.link.link_status64 to 0. */
> +	lio_dev->linfo.link.link_status64 = 0;
> +
>  	/* start polling for lsc */
>  	ret = rte_eal_alarm_set(LIO_LSC_TIMEOUT,
>  				lio_sync_link_state_check,
> -- 
> 1.8.3.1
> 

Hi Yao,

Thanks for the catch! Please send a v2 with corrections to commit log.

Acked-by: Shijith Thotton <shijith.thotton@caviumnetworks.com>

Re: [dpdk-dev] [PATCH] net/liquidio:Fix Unable to update lio_dev->linfo.link var

[Yao chuhong]

hi, shijith


I have reissued v2 patch and corrected the commit log. Please check it. Thank you.
 
------------------ Original ------------------
From:  "Shijith Thotton"<shijith.thotton@caviumnetworks.com>;
Date:  Tue, May 22, 2018 10:19 PM
To:  "yaochuhong"<ych@panath.cn>; 
Cc:  "shijith.thotton"<shijith.thotton@cavium.com>; "dev"<dev@dpdk.org>; 
Subject:  Re: [PATCH] net/liquidio:Fix Unable to update lio_dev->linfo.link var

 
On Mon, May 21, 2018 at 08:58:15PM +0800, yaochuhong wrote:
> When I was using VPP +dpdk-18.02+liqudio CN23xx, I encountered such a bug. \n
> When VPP called dpdk_device_start to initialize DPDK liqudio drive, I found that initialization failed. \n
> The reason for the failure is that VF MTU > PF MTU, but PF MTU has been modified to 9600 (> VF MTU). \n
> Finally, I am location that DPDK liqudio drive cannot get the correct PF driver to liqudio network card. \n
> It is due to the fact that when VPP calls dpdk_device_start to initialize DPDK liqudio drive,  \n
> this time, lio_dev->linfo. Link var already exists in the old value, not empty. \n
> Cause lio_dev - > linfo. Link. Link_status64 != 0 statement is set up,  \n
> and the link info is stopped directly to liqudio card, resulting in no get accurate pf mtu. \n
> I did a test model to reproduce the bug, which is to add rte_eth_dev_set_mtu(portid, vf_mtu)  \n
> to the rte_eth_dev_start function when using dpdk-18.02+liqudio CN23xx+l2fwd. \n
> You need to make sure that 1500 < vf_mtu < pf_mtu will be available.  \n
> At this time, you will have net_liovf[04:00.3]ERROR: lio_dev_mtu_set() VF MTU should be >= 68 and <= 1500. Such a mistake. \n
> ps:I am a novice git. Please understand the trouble caused by this.
> 
> Signed-off-by: yaochuhong <ych@panath.cn>
> ---
>  drivers/net/liquidio/lio_ethdev.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/liquidio/lio_ethdev.c b/drivers/net/liquidio/lio_ethdev.c
> index 0e0b5d8..50743c7 100644
> --- a/drivers/net/liquidio/lio_ethdev.c
> +++ b/drivers/net/liquidio/lio_ethdev.c
> @@ -1405,6 +1405,9 @@ struct rte_lio_xstats_name_off {
>  	/* Configure RSS if device configured with multiple RX queues. */
>  	lio_dev_mq_rx_configure(eth_dev);
>  
> +	/* Before update the link info, must set linfo.link.link_status64 to 0. */
> +	lio_dev->linfo.link.link_status64 = 0;
> +
>  	/* start polling for lsc */
>  	ret = rte_eal_alarm_set(LIO_LSC_TIMEOUT,
>  				lio_sync_link_state_check,
> -- 
> 1.8.3.1
> 

Hi Yao,

Thanks for the catch! Please send a v2 with corrections to commit log.

Acked-by: Shijith Thotton <shijith.thotton@caviumnetworks.com>




搜索

复制

[dpdk-dev] [PATCH] net/liquidio:fix Unable to update lio_dev->linfo.link var

[yaochuhong]

Signed-off-by: yaochuhong <ych@panath.cn>
---
 drivers/net/liquidio/lio_ethdev.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/liquidio/lio_ethdev.c b/drivers/net/liquidio/lio_ethdev.c
index 0e0b5d8..ba39d20 100644
--- a/drivers/net/liquidio/lio_ethdev.c
+++ b/drivers/net/liquidio/lio_ethdev.c
@@ -1405,6 +1405,9 @@ struct rte_lio_xstats_name_off {
 	/* Configure RSS if device configured with multiple RX queues. */
 	lio_dev_mq_rx_configure(eth_dev);
 
+	/* Before update the link info, must set linfo.link.link_status64 to 0. */
+	lio_dev->linfo.link.link_status64 = 0;
+    
 	/* start polling for lsc */
 	ret = rte_eal_alarm_set(LIO_LSC_TIMEOUT,
 				lio_sync_link_state_check,
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH] net/liquidio:fix Unable to update lio_dev->linfo.link var

[Ferruh Yigit]

On 5/21/2018 11:37 AM, yaochuhong wrote:
> Signed-off-by: yaochuhong <ych@panath.cn>
> ---
>  drivers/net/liquidio/lio_ethdev.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/liquidio/lio_ethdev.c b/drivers/net/liquidio/lio_ethdev.c
> index 0e0b5d8..ba39d20 100644
> --- a/drivers/net/liquidio/lio_ethdev.c
> +++ b/drivers/net/liquidio/lio_ethdev.c
> @@ -1405,6 +1405,9 @@ struct rte_lio_xstats_name_off {
>  	/* Configure RSS if device configured with multiple RX queues. */
>  	lio_dev_mq_rx_configure(eth_dev);
>  
> +	/* Before update the link info, must set linfo.link.link_status64 to 0. */
> +	lio_dev->linfo.link.link_status64 = 0;

Initial link_status64 value should be 0 (memory allocated with
rte_zmalloc_socket()) and lio_dev_stop() sets it to 0. So there should be no
need to set zero here.

Are you observing cases this value is not zero? Can you please add more
information to commit log about what/how you are observing and what is the
result of it?

> +    
>  	/* start polling for lsc */
>  	ret = rte_eal_alarm_set(LIO_LSC_TIMEOUT,
>  				lio_sync_link_state_check,
>