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

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

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> Sent: Tuesday, January 14, 2020 11:34 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 v2] 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
> 
> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>

Please add the change log when sending the incremental versions.

v2:

# Fix the following reported compilation error.
http://mails.dpdk.org/archives/dev/2020-January/155161.html

Applied to dpdk-next-eventdev/master. Thanks.

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

[Jerin Jacob Kollanukkaran]

> -----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

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

[Mattias Rönnblom]

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

Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
---
 drivers/event/dsw/dsw_event.c | 38 ++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/drivers/event/dsw/dsw_event.c b/drivers/event/dsw/dsw_event.c
index 1641c2d06..eae53b240 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;
@@ -1047,13 +1047,10 @@ dsw_event_enqueue_burst_generic(void *port, const struct rte_event events[],
 	 */
 	if (unlikely(events_len == 0)) {
 		dsw_port_note_op(source_port, DSW_MAX_PORT_OPS_PER_BG_TASK);
-		dsw_port_flush_out_buffers(dsw, port);
+		dsw_port_flush_out_buffers(dsw, source_port);
 		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)
@@ -1109,24 +1106,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