Re: [dpdk-dev] [EXT] [PATCH] event/dsw: avoid credit leak on oversized enqueue bursts

[Jerin Jacob Kollanukkaran <jerinj@marvell.com>]

> -----Original Message-----
> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> Sent: Friday, January 3, 2020 5:02 PM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> Cc: dev@dpdk.org; Mattias Rönnblom <mattias.ronnblom@ericsson.com>;
> stable@dpdk.org
> Subject: [EXT] [PATCH] event/dsw: avoid credit leak on oversized enqueue
> bursts
> 
> If an application issues rte_event_enqueue_new_burst() or
> rte_event_enqueue_forward_burst() call with a burst of events longer than the
> configured max enqueue burst size, DSW allocates credits not only for events
> actually enqueued, but for the complete burst. If this process is repeated,
> enough credits will have leaked to cause the event device to backpressure (i.e.
> disallow) any new enqueue operations.
> 
> In addition, the port-level enqueue xstats will log the wrong number of events
> enqueued for oversized enqueues.
> 
> This patch makes DSW gracefully handle oversized enqueue bursts.
> 
> Fixes: 1c8e3caa3bfb ("event/dsw: add event scheduling and device start/stop")
> Cc: stable@dpdk.org


This patch has following compilation issue when applying to dpdk-next-eventdev. Please send v2.

ninja: Entering directory `build'
[1613/2012] Compiling C object 'drivers/a715181@@tmp_rte_pmd_dsw_event@sta/event_dsw_dsw_event.c.o'.
FAILED: drivers/a715181@@tmp_rte_pmd_dsw_event@sta/event_dsw_dsw_event.c.o
ccache cc -Idrivers/a715181@@tmp_rte_pmd_dsw_event@sta -Idrivers -I../drivers -Idrivers/event/dsw -I../drivers/event/dsw -Ilib/librte_eventdev -I../lib/librte_eventdev -I. -I../ -Iconfig -I../config -Ilib/librte_eal/common/include -I../lib/
librte_eal/common/include -I../lib/librte_eal/linux/eal/include -Ilib/librte_eal/common -I../lib/librte_eal/common -Ilib/librte_eal/common/include/arch/x86 -I../lib/librte_eal/common/include/arch/x86 -Ilib/librte_eal -I../lib/librte_eal -Il
ib/librte_kvargs -I../lib/librte_kvargs -Ilib/librte_ring -I../lib/librte_ring -Ilib/librte_ethdev -I../lib/librte_ethdev -Ilib/librte_net -I../lib/librte_net -Ilib/librte_mbuf -I../lib/librte_mbuf -Ilib/librte_mempool -I../lib/librte_mempo
ol -Ilib/librte_meter -I../lib/librte_meter -Ilib/librte_hash -I../lib/librte_hash -Ilib/librte_timer -I../lib/librte_timer -Ilib/librte_cryptodev -I../lib/librte_cryptodev -Idrivers/bus/vdev -I../drivers/bus/vdev -fdiagnostics-color=always
 -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -O3 -include rte_config.h -Wextra -Wcast-qual -Wdeprecated -Wformat-nonliteral -Wformat-security -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer
-arith -Wsign-compare -Wstrict-prototypes -Wundef -Wwrite-strings -Wno-address-of-packed-member -Wno-packed-not-aligned -Wno-missing-field-initializers -D_GNU_SOURCE -fPIC -march=native -Wno-format-truncation -Wno-format-nonliteral -MD -MQ
'drivers/a715181@@tmp_rte_pmd_dsw_event@sta/event_dsw_dsw_event.c.o' -MF 'drivers/a715181@@tmp_rte_pmd_dsw_event@sta/event_dsw_dsw_event.c.o.d' -o 'drivers/a715181@@tmp_rte_pmd_dsw_event@sta/event_dsw_dsw_event.c.o' -c ../drivers/event/dsw/
dsw_event.c
../drivers/event/dsw/dsw_event.c: In function ‘dsw_event_enqueue_burst_generic’:
../drivers/event/dsw/dsw_event.c:1050:35: error: ‘port’ undeclared (first use in this function)
 1050 |   dsw_port_flush_out_buffers(dsw, port);
      |                                   ^~~~
../drivers/event/dsw/dsw_event.c:1050:35: note: each undeclared identifier is reported only once for each function it appears in
[1668/2012] Compiling C object 'drivers/a715181@@tmp_rte_pmd_octeontx_crypto@sta/crypto_octeontx_otx_cryptodev_ops.c.o'.^C
ninja: build stopped: interrupted by user.


[master]dell[dpdk-next-eventdev] $ gcc -v
Using built-in specs.
COLLECT_GCC=/usr/bin/gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /build/gcc/src/gcc/configure --prefix=/usr --libdir=/usr/lib --libexecdir=/usr/lib --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=https://bugs.archlinux.org/ --enable-languages=c,c++,ada,fortran,go,lto,objc
,obj-c++,d --enable-shared --enable-threads=posix --with-system-zlib --with-isl --enable-__cxa_atexit --disable-libunwind-exceptions --enable-clocale=gnu --disable-libstdcxx-pch --disable-libssp --enable-gnu-unique-object --enable-linker-bu
ild-id --enable-lto --enable-plugin --enable-install-libiberty --with-linker-hash-style=gnu --enable-gnu-indirect-function --enable-multilib --disable-werror --enable-checking=release --enable-default-pie --enable-default-ssp --enable-cet=a
uto gdc_include_dir=/usr/include/dlang/gdc
Thread model: posix
gcc version 9.2.0 (GCC)

> 
> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> ---
>  drivers/event/dsw/dsw_event.c | 36 ++++++++++++++++++++++++-----------
>  1 file changed, 25 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/event/dsw/dsw_event.c b/drivers/event/dsw/dsw_event.c
> index 61a66fabf..b919244c6 100644
> --- a/drivers/event/dsw/dsw_event.c
> +++ b/drivers/event/dsw/dsw_event.c
> @@ -1018,12 +1018,12 @@ dsw_event_enqueue(void *port, const struct
> rte_event *ev)  }
> 
>  static __rte_always_inline uint16_t
> -dsw_event_enqueue_burst_generic(void *port, const struct rte_event events[],
> +dsw_event_enqueue_burst_generic(struct dsw_port *source_port,
> +				const struct rte_event events[],
>  				uint16_t events_len, bool op_types_known,
>  				uint16_t num_new, uint16_t num_release,
>  				uint16_t num_non_release)
>  {
> -	struct dsw_port *source_port = port;
>  	struct dsw_evdev *dsw = source_port->dsw;
>  	bool enough_credits;
>  	uint16_t i;
> @@ -1050,9 +1050,6 @@ dsw_event_enqueue_burst_generic(void *port,
> const struct rte_event events[],
>  		return 0;
>  	}
> 
> -	if (unlikely(events_len > source_port->enqueue_depth))
> -		events_len = source_port->enqueue_depth;
> -
>  	dsw_port_note_op(source_port, events_len);
> 
>  	if (!op_types_known)
> @@ -1108,24 +1105,41 @@ uint16_t
>  dsw_event_enqueue_burst(void *port, const struct rte_event events[],
>  			uint16_t events_len)
>  {
> -	return dsw_event_enqueue_burst_generic(port, events, events_len,
> false,
> -					       0, 0, 0);
> +	struct dsw_port *source_port = port;
> +
> +	if (unlikely(events_len > source_port->enqueue_depth))
> +		events_len = source_port->enqueue_depth;
> +
> +	return dsw_event_enqueue_burst_generic(source_port, events,
> +					       events_len, false, 0, 0, 0);
>  }
> 
>  uint16_t
>  dsw_event_enqueue_new_burst(void *port, const struct rte_event events[],
>  			    uint16_t events_len)
>  {
> -	return dsw_event_enqueue_burst_generic(port, events, events_len,
> true,
> -					       events_len, 0, events_len);
> +	struct dsw_port *source_port = port;
> +
> +	if (unlikely(events_len > source_port->enqueue_depth))
> +		events_len = source_port->enqueue_depth;
> +
> +	return dsw_event_enqueue_burst_generic(source_port, events,
> +					       events_len, true, events_len,
> +					       0, events_len);
>  }
> 
>  uint16_t
>  dsw_event_enqueue_forward_burst(void *port, const struct rte_event
> events[],
>  				uint16_t events_len)
>  {
> -	return dsw_event_enqueue_burst_generic(port, events, events_len,
> true,
> -					       0, 0, events_len);
> +	struct dsw_port *source_port = port;
> +
> +	if (unlikely(events_len > source_port->enqueue_depth))
> +		events_len = source_port->enqueue_depth;
> +
> +	return dsw_event_enqueue_burst_generic(source_port, events,
> +					       events_len, true, 0, 0,
> +					       events_len);
>  }
> 
>  uint16_t
> --
> 2.17.1