On 8/20/2020 2:42 AM, Wei Hu (Xavier) wrote:
> From: Chengchang Tang <[email protected]>
>
> 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: [email protected]
>
> Signed-off-by: Chengchang Tang <[email protected]>
> Signed-off-by: Wei Hu (Xavier) <[email protected]>
> ---
> 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').