Re: [dpdk-dev] [PATCH v7 1/2] examples/multi_process/client_server_mp: check port validity

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue, 6 Aug 2019 12:07:25 +0000
Matan Azrad <matan@mellanox.com> wrote:

> Hi
> 
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Stephen Hemminger
> > Sent: Monday, August 5, 2019 7:38 PM
> > To: dev@dpdk.org
> > Cc: Stephen Hemminger <sthemmin@microsoft.com>
> > Subject: [dpdk-dev] [PATCH v7 1/2]
> > examples/multi_process/client_server_mp: check port validity
> > 
> > From: Stephen Hemminger <sthemmin@microsoft.com>
> > 
> > The mp_server incorrectly allows a port mask that included hidden ports and
> > which later caused either lost packets or failed initialization.
> > 
> > This fixes explicitly checking that each bit in portmask is a valid port before
> > using it.
> > 
> > Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
> > Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> > ---
> >  .../client_server_mp/mp_server/args.c         | 40 ++++++++++---------
> >  .../client_server_mp/mp_server/args.h         |  2 +-
> >  .../client_server_mp/mp_server/init.c         |  7 +---
> >  3 files changed, 25 insertions(+), 24 deletions(-)
> > 
> > diff --git a/examples/multi_process/client_server_mp/mp_server/args.c
> > b/examples/multi_process/client_server_mp/mp_server/args.c
> > index b0d8d7665c85..3c2ca266b096 100644
> > --- a/examples/multi_process/client_server_mp/mp_server/args.c
> > +++ b/examples/multi_process/client_server_mp/mp_server/args.c
> > @@ -10,6 +10,7 @@
> >  #include <errno.h>
> > 
> >  #include <rte_memory.h>
> > +#include <rte_ethdev.h>
> >  #include <rte_string_fns.h>
> > 
> >  #include "common.h"
> > @@ -41,31 +42,34 @@ usage(void)
> >   * array variable
> >   */
> >  static int
> > -parse_portmask(uint8_t max_ports, const char *portmask)
> > +parse_portmask(const char *portmask)
> >  {
> >  	char *end = NULL;
> > -	unsigned long pm;
> > -	uint16_t count = 0;
> > +	unsigned long long pm;
> > +	uint16_t id;
> > 
> >  	if (portmask == NULL || *portmask == '\0')
> >  		return -1;
> > 
> >  	/* convert parameter to a number and verify */
> > -	pm = strtoul(portmask, &end, 16);
> > -	if (end == NULL || *end != '\0' || pm == 0)
> > +	errno = 0;
> > +	pm = strtoull(portmask, &end, 16);
> > +	if (errno != 0 || end == NULL || *end != '\0')
> >  		return -1;
> >   
> Please Continue discussion on this on V5 thread.

The V5 thread degenerated into "applications should not use portmask".
That is a valid discussion but out of scope for this patch which is a bug
fix for users.