[V3,6/7] app/testpmd: add forwarding config in start port

Message ID 1618909266-17584-7-git-send-email-lihuisong@huawei.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series modifications about DCB forwarding configuration |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

lihuisong (C) April 20, 2021, 9:01 a.m. UTC
  Most operations in testpmd that need to update the forwarding streams
in testpmd call fwd_config_setup(). In some scenarios, eg, dev_configure
is called again, the forwarding streams may not be updated. As a result,
the actual forwarding streams cannot be queried by "show config fwd" cmd.

The procedure is as follows:
set nbcore 4
port stop all
port config 0 dcb vt off 4 pfc on
port start all
show config fwd

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Lijun Ou <oulijun@huawei.com>
---
 app/test-pmd/testpmd.c | 6 ++++++
 1 file changed, 6 insertions(+)
  

Comments

Li, Xiaoyun April 27, 2021, 11:43 a.m. UTC | #1
> -----Original Message-----
> From: Huisong Li <lihuisong@huawei.com>
> Sent: Tuesday, April 20, 2021 17:01
> To: dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>;
> linuxarm@openeuler.org; lihuisong@huawei.com
> Subject: [PATCH V3 6/7] app/testpmd: add forwarding config in start port
> 
> Most operations in testpmd that need to update the forwarding streams in
> testpmd call fwd_config_setup(). In some scenarios, eg, dev_configure is called
> again, the forwarding streams may not be updated. As a result, the actual
> forwarding streams cannot be queried by "show config fwd" cmd.

I don't agree on this.
Fwd config should be only changed after the user change something like nb-cores, queue number, eth-peer in non-dcb mode. These are already done in those commands.

You should do fwd_config_setup at the end of cmd_config_dcb_parsed(), I agree on this.
But doing it in start port seems to do redundant times of fwd_setup. It's not really needed.

> 
> The procedure is as follows:
> set nbcore 4
> port stop all
> port config 0 dcb vt off 4 pfc on
> port start all
> show config fwd
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Lijun Ou <oulijun@huawei.com>
> ---
>  app/test-pmd/testpmd.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> abcbdaa..f8052b6 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -2678,6 +2678,12 @@ start_port(portid_t pid)
>  			}
>  		}
>  	}
> +	/*
> +	 * In some scenarios, eg, dev_configure is called again, the forwarding
> +	 * streams may not be updated. As a result, the actual forwarding
> +	 * streams cannot be queried by "show config fwd" command.
> +	 */
> +	fwd_config_setup();
> 
>  	printf("Done\n");
>  	return 0;
> --
> 2.7.4
  
lihuisong (C) April 28, 2021, 2:14 a.m. UTC | #2
在 2021/4/27 19:43, Li, Xiaoyun 写道:
>> -----Original Message-----
>> From: Huisong Li <lihuisong@huawei.com>
>> Sent: Tuesday, April 20, 2021 17:01
>> To: dev@dpdk.org
>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>;
>> linuxarm@openeuler.org; lihuisong@huawei.com
>> Subject: [PATCH V3 6/7] app/testpmd: add forwarding config in start port
>>
>> Most operations in testpmd that need to update the forwarding streams in
>> testpmd call fwd_config_setup(). In some scenarios, eg, dev_configure is called
>> again, the forwarding streams may not be updated. As a result, the actual
>> forwarding streams cannot be queried by "show config fwd" cmd.
> I don't agree on this.
> Fwd config should be only changed after the user change something like nb-cores, queue number, eth-peer in non-dcb mode. These are already done in those commands.
>
> You should do fwd_config_setup at the end of cmd_config_dcb_parsed(), I agree on this.
> But doing it in start port seems to do redundant times of fwd_setup. It's not really needed.
After moving the check of dcb test to fwd_config_setup() in this patch 
set, doing fwd_config_setup at the end of cmd_config_dcb_parsed() does 
make more sense.
In this way, similar commands can maintain the same design.  And I 
tested it, and it was OK. I will fix it in next version, thanks.
>> The procedure is as follows:
>> set nbcore 4
>> port stop all
>> port config 0 dcb vt off 4 pfc on
>> port start all
>> show config fwd
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> Signed-off-by: Lijun Ou <oulijun@huawei.com>
>> ---
>>   app/test-pmd/testpmd.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
>> abcbdaa..f8052b6 100644
>> --- a/app/test-pmd/testpmd.c
>> +++ b/app/test-pmd/testpmd.c
>> @@ -2678,6 +2678,12 @@ start_port(portid_t pid)
>>   			}
>>   		}
>>   	}
>> +	/*
>> +	 * In some scenarios, eg, dev_configure is called again, the forwarding
>> +	 * streams may not be updated. As a result, the actual forwarding
>> +	 * streams cannot be queried by "show config fwd" command.
>> +	 */
>> +	fwd_config_setup();
>>
>>   	printf("Done\n");
>>   	return 0;
>> --
>> 2.7.4
> .
  

Patch

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index abcbdaa..f8052b6 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2678,6 +2678,12 @@  start_port(portid_t pid)
 			}
 		}
 	}
+	/*
+	 * In some scenarios, eg, dev_configure is called again, the forwarding
+	 * streams may not be updated. As a result, the actual forwarding
+	 * streams cannot be queried by "show config fwd" command.
+	 */
+	fwd_config_setup();
 
 	printf("Done\n");
 	return 0;