Re: [dpdk-stable] [PATCH 16.11 1/3] app/testpmd: fix vdev socket initialization

["Phil Yang (Arm Technology China)" <Phil.Yang@arm.com>]

> -----Original Message-----
> From: Luca Boccassi <bluca@debian.org>
> Sent: Friday, November 2, 2018 6:43 PM
> To: Phil Yang (Arm Technology China) <Phil.Yang@arm.com>; stable@dpdk.org
> Subject: Re: [dpdk-stable] [PATCH 16.11 1/3] app/testpmd: fix vdev socket
> initialization
>
> On Fri, 2018-11-02 at 15:40 +0800, Phil Yang wrote:
> > [ backported from upstream commit
> > 29841336438400ce040e394c0c00040c21644727 ]
> >
> > The cmdline settings of port-numa-config and rxring-numa-config have
> > been flushed by the following init_config. If we don't configure the
> > port-numa-config, the virtual device will allocate the device ports to
> > socket 0. It will cause failure when the socket 0 is unavailable.
> >
> > eg:
> > testpmd -l <cores from socket 1> --vdev net_pcap0,iface=lo
> > --socket-mem=64 -- --numa --port-numa-config="(0,1)"
> > --ring-numa-config="(0,1,1),(0,2,1)" -i
> >
> > ...
> > Configuring Port 0 (socket 0)
> > Failed to setup RX queue:No mempool allocation on the socket 0
> > EAL: Error - exiting with code: 1
> >   Cause: Start ports failed
> >
> > Fix by allocate the devices port to the first available socket or the
> > socket configured in port-numa-config.
> >
> > Fixes: 487f9a5 ("app/testpmd: fix NUMA structures initialization")
> > Fixes: 20a0286fd2c0 ("app/testpmd: check socket id validity")
> >
> > Signed-off-by: Phil Yang <phil.yang@arm.com>
> > Reviewed-by: Gavin Hu <gavin.hu@arm.com>
> > Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > ---
> >  app/test-pmd/testpmd.c | 29 +++++++++++++++++------------
> >  1 file changed, 17 insertions(+), 12 deletions(-)
> >
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > c3ab448..8984d0b 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -582,12 +582,6 @@ init_config(void)
> >
> >  memset(port_per_socket,0,RTE_MAX_NUMA_NODES);
> >
> > -if (numa_support) {
> > -memset(port_numa, NUMA_NO_CONFIG,
> RTE_MAX_ETHPORTS);
> > -memset(rxring_numa, NUMA_NO_CONFIG,
> > RTE_MAX_ETHPORTS);
> > -memset(txring_numa, NUMA_NO_CONFIG,
> > RTE_MAX_ETHPORTS);
> > -}
> > -
> >  /* Configuration of logical cores. */
> >  fwd_lcores = rte_zmalloc("testpmd: fwd_lcores",
> >  sizeof(struct fwd_lcore *) *
> > nb_lcores,
> > @@ -617,9 +611,12 @@ init_config(void)
> >  else {
> >  uint32_t socket_id =
> > rte_eth_dev_socket_id(pid);
> >
> > -/* if socket_id is invalid, set to 0
> > */
> > +/*
> > + * if socket_id is invalid,
> > + * set to the first available
> > socket.
> > + */
> >  if (check_socket_id(socket_id) < 0)
> > -socket_id = 0;
> > +socket_id = socket_ids[0];
> >  port_per_socket[socket_id]++;
> >  }
> >  }
> > @@ -753,9 +750,12 @@ init_fwd_streams(void)
> >  else {
> >  port->socket_id =
> > rte_eth_dev_socket_id(pid);
> >
> > -/* if socket_id is invalid, set to 0
> > */
> > +/*
> > + * if socket_id is invalid,
> > + * set to the first available
> > socket.
> > + */
> >  if (check_socket_id(port->socket_id) < 0)
> > -port->socket_id = 0;
> > +port->socket_id =
> > socket_ids[0];
> >  }
> >  }
> >  else {
> > @@ -1774,9 +1774,9 @@ attach_port(char *identifier)
> >  return;
> >
> >  socket_id = (unsigned)rte_eth_dev_socket_id(pi);
> > -/* if socket_id is invalid, set to 0 */
> > +/* if socket_id is invalid, set to the first available
> > socket. */
> >  if (check_socket_id(socket_id) < 0)
> > -socket_id = 0;
> > +socket_id = socket_ids[0];
> >  reconfig(pi, socket_id);
> >  rte_eth_promiscuous_enable(pi);
> >
> > @@ -2325,6 +2325,11 @@ init_port(void)
> >  "rte_zmalloc(%d struct rte_port)
> > failed\n",
> >  RTE_MAX_ETHPORTS);
> >  }
> > +
> > +/* Initialize ports NUMA structures */
> > +memset(port_numa, NUMA_NO_CONFIG, RTE_MAX_ETHPORTS);
> > +memset(rxring_numa, NUMA_NO_CONFIG, RTE_MAX_ETHPORTS);
> > +memset(txring_numa, NUMA_NO_CONFIG, RTE_MAX_ETHPORTS);
> >  }
> >
> >  static void
>
> Hi Phil,
>
> Thanks for sending the series, but I just tried to apply it and there's conflicts in
> the first patch. Once solved, it fails to build:
>
> /home/bluca/git/dpdk/app/test-pmd/testpmd.c: In function ‘init_config’:
> /home/bluca/git/dpdk/app/test-pmd/testpmd.c:537:18: error: ‘socket_ids’
> undeclared (first use in this function)
>       socket_id = socket_ids[0];
>                   ^~~~~~~~~~
> /home/bluca/git/dpdk/app/test-pmd/testpmd.c:537:18: note: each undeclared
> identifier is reported only once for each function it appears in
> /home/bluca/git/dpdk/app/test-pmd/testpmd.c: In function ‘init_fwd_streams’:
> /home/bluca/git/dpdk/app/test-pmd/testpmd.c:652:24: error: ‘socket_ids’
> undeclared (first use in this function)
>       port->socket_id = socket_ids[0];
>                         ^~~~~~~~~~
> /home/bluca/git/dpdk/app/test-pmd/testpmd.c: In function ‘attach_port’:
> /home/bluca/git/dpdk/app/test-pmd/testpmd.c:1604:15: error: ‘socket_ids’
> undeclared (first use in this function)
>    socket_id = socket_ids[0];
>
>
> Have you tested this on the latest tip of 16.11? As a reminder, you can get it
> from here:
>
> https://git.dpdk.org/dpdk-stable/log/?h=16.11
>

Hi Luca,

It's my fault. I had put the patchset on the top of master branch. It is workable on the latest tip of master.

I think I need to rework these patches for 16.11 branch.

Thanks for your reminder.

> --
> Kind regards,
> Luca Boccassi
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.