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').