Re: [PATCH v2 1/8] event/opdl: add the opdl ring infrastructure library

["Ma, Liang" <liang.j.ma@intel.com>]

On 15 Dec 16:23, Neil Horman wrote:
> On Fri, Dec 15, 2017 at 01:50:41PM +0000, Ma, Liang wrote:
> > On 15 Dec 07:38, Neil Horman wrote:
> > > On Fri, Dec 15, 2017 at 11:26:22AM +0000, Liang Ma wrote:
> > > > OPDL ring is the core infrastructure of OPDL PMD. OPDL ring library
> > > > provide the core data structure and core helper function set. The Ring
> > > > implements a single ring multi-port/stage pipelined packet distribution
> > > > mechanism. This mechanism has the following characteristics:
> > > > 
> > > > • No multiple queue cost, therefore, latency is significant reduced.
> > > > • Fixed dependencies between queue/ports is more suitable for complex.
> > > >   fixed pipelines of stateless packet processing (static pipeline).
> > > > • Has decentralized distribution (no scheduling core).
> > > > • Packets remain in order (no reorder core(s)).
> > > > 
> > > > Signed-off-by: Liang Ma <liang.j.ma@intel.com>
> > > > Signed-off-by: Peter, Mccarthy <peter.mccarthy@intel.com>
> > > > ---
> > > >  drivers/event/opdl/Makefile                       |   66 ++
> > > >  drivers/event/opdl/opdl_ring.c                    | 1232 +++++++++++++++++++++
> > > >  drivers/event/opdl/opdl_ring.h                    |  601 ++++++++++
> > > >  drivers/event/opdl/rte_pmd_evdev_opdl_version.map |    3 +
> > > >  4 files changed, 1902 insertions(+)
> > > >  create mode 100644 drivers/event/opdl/Makefile
> > > >  create mode 100644 drivers/event/opdl/opdl_ring.c
> > > >  create mode 100644 drivers/event/opdl/opdl_ring.h
> > > >  create mode 100644 drivers/event/opdl/rte_pmd_evdev_opdl_version.map
> > > > 
> > > ><snip>
> > > 
> > > > +
> > > > +#endif  /* _OPDL_H_ */
> > > > diff --git a/drivers/event/opdl/rte_pmd_evdev_opdl_version.map b/drivers/event/opdl/rte_pmd_evdev_opdl_version.map
> > > > new file mode 100644
> > > > index 0000000..5352e7e
> > > > --- /dev/null
> > > > +++ b/drivers/event/opdl/rte_pmd_evdev_opdl_version.map
> > > > @@ -0,0 +1,3 @@
> > > > +DPDK_17.05 {
> > > > +	local: *;
> > > > +};
> > > you need to ennumerate the functions you want to globally export here, or this won't work
> > > as a shared library.  This also suggests you haven't tested this as a DSO yet,
> > > please do so.
> > > 
> > > Neil
> > FYI, the subject of email has indicated that's part of the PMD.
> I apologize, I didn't realize it was its own pmd, rather than part of the other
> pmds.  I missed the pathing and thought you were creating a separate library for
> others to consume, rather than a PMD to be accessed via the ethdev api
> 
> > there is no need to export any function globally.
> > you can reference 
> > drivers/event/octeontx/rte_pmd_octeontx_ssovf_version.map
> > /drivers/event/dpaa2/rte_pmd_dpaa2_event_version.map
> > drivers/event/sw/rte_pmd_sw_event_version.map
> > 
> > BTW: I do test with shared library build. 
> What did you test with?  I ask because with gcc version 7.2.1, the build throws
> several warnings that error the build out:
I test with Gcc 6.3.1 and some older version, no warning report. 
gcc 7.2.1 is not in our test list rightnow, I will investigate the issue. 
> 
> /home/nhorman/git/dpdk/drivers/event/opdl/opdl_evdev_init.c: In function ‘create_queues_and_rings’:
> /home/nhorman/git/dpdk/drivers/event/opdl/opdl_evdev_init.c:570:17: error: ‘%s’ directive writing up to 63 bytes into a region of size 32 [-Werror=format-overflow=]
>   sprintf(name, "%s_%u", device->service_name, device->nb_opdls);
>                  ^~
> /home/nhorman/git/dpdk/drivers/event/opdl/opdl_evdev_init.c:570:2: note: ‘sprintf’ output between 3 and 75 bytes into a destination of size 32
>   sprintf(name, "%s_%u", device->service_name, device->nb_opdls);
>   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> /home/nhorman/git/dpdk/drivers/event/opdl/opdl_evdev_init.c:570:17: error: ‘%s’ directive writing up to 63 bytes into a region of size 32 [-Werror=format-overflow=]
>   sprintf(name, "%s_%u", device->service_name, device->nb_opdls);
>                  ^~
> /home/nhorman/git/dpdk/drivers/event/opdl/opdl_evdev_init.c:570:2: note: ‘sprintf’ output between 3 and 75 bytes into a destination of size 32
>   sprintf(name, "%s_%u", device->service_name, device->nb_opdls);
>   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> 
> 
> It looks like you're trying to write a string to a 32 byte array, but the first
> component of that string (service_name) can be as long as 63 bytes, and the
> nb_opdls can be up to 75 bytes long. I'm guessing you want to use snprintf there
> rather than sprintf with a limit of RTE_MEMZONE_NAMESIZE - LIB_NAME (to account
> for the added characters in opdl_ring_create?
> 
> Neil
> 
> > > 
> > > > -- 
> > > > 2.7.5
> > > > 
> > > > --------------------------------------------------------------
> > > > Intel Research and Development Ireland Limited
> > > > Registered in Ireland
> > > > Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
> > > > Registered Number: 308263
> > > > 
> > > > 
> > > > This e-mail and any attachments may contain confidential material for the sole
> > > > use of the intended recipient(s). Any review or distribution by others is
> > > > strictly prohibited. If you are not the intended recipient, please contact the
> > > > sender and delete all copies.
> >