On 8/20/2020 2:42 AM, Wei Hu (Xavier) wrote:
> From: Chengchang Tang <tangchengch...@huawei.com>
> 
> To set Tx vlan offloads, it is required to stop port firstly. But before
> checking whether the port is stopped, the port id entered by the user
> is not checked for validity. When the port id is illegal, it would lead
> to a segmentation fault since it attempts to access a member of
> non-existent port.
> 
> This patch adds verification of port id in tx vlan offloads.
> 
> Fixes: 597f9fafe13b ("app/testpmd: convert to new Tx offloads API")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Chengchang Tang <tangchengch...@huawei.com>
> Signed-off-by: Wei Hu (Xavier) <xavier.hu...@huawei.com>
> ---
>  app/test-pmd/cmdline.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 0a6ed85f3..8377f8401 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -4268,6 +4268,9 @@ cmd_tx_vlan_set_parsed(void *parsed_result,
>  {
>       struct cmd_tx_vlan_set_result *res = parsed_result;
>  
> +     if (port_id_is_invalid(res->port_id, ENABLED_WARN))
> +             return;
> +
>       if (!port_is_stopped(res->port_id)) {
>               printf("Please stop port %d first\n", res->port_id);
>               return;

These functions are wrappers to some testpmd functions, and those
functions already have invalid port check, like 'tx_vlan_set()'

Agree that check should be in these functions, but to prevent the
duplicated checks, what do you think to remove checks from internal
functions? ('tx_vlan_set', 'tx_qinq_set' & 'tx_vlan_reset').

Reply via email to