DPDK-dev Archive on lore.kernel.org
 help / color / Atom feed
* [dpdk-dev] [PATCH] event/dsw: avoid credit leak on oversized enqueue bursts
@ 2020-01-03 11:31 Mattias Rönnblom
  2020-01-14 17:32 ` [dpdk-dev] [EXT] " Jerin Jacob Kollanukkaran
  0 siblings, 1 reply; 3+ messages in thread
From: Mattias Rönnblom @ 2020-01-03 11:31 UTC (permalink / raw)
  To: jerinj; +Cc: dev, Mattias Rönnblom, stable

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [dpdk-dev] [EXT] [PATCH] event/dsw: avoid credit leak on oversized enqueue bursts
  2020-01-03 11:31 [dpdk-dev] [PATCH] event/dsw: avoid credit leak on oversized enqueue bursts Mattias Rönnblom
@ 2020-01-14 17:32 ` " Jerin Jacob Kollanukkaran
  2020-01-14 18:03   ` [dpdk-dev] [PATCH v2] " Mattias Rönnblom
  0 siblings, 1 reply; 3+ messages in thread
From: Jerin Jacob Kollanukkaran @ 2020-01-14 17:32 UTC (permalink / raw)
  To: Mattias Rönnblom; +Cc: dev, stable

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [dpdk-dev] [PATCH v2] event/dsw: avoid credit leak on oversized enqueue bursts
  2020-01-14 17:32 ` [dpdk-dev] [EXT] " Jerin Jacob Kollanukkaran
@ 2020-01-14 18:03   ` " Mattias Rönnblom
  0 siblings, 0 replies; 3+ messages in thread
From: Mattias Rönnblom @ 2020-01-14 18:03 UTC (permalink / raw)
  To: jerinj; +Cc: dev, Mattias Rönnblom, stable

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-03 11:31 [dpdk-dev] [PATCH] event/dsw: avoid credit leak on oversized enqueue bursts Mattias Rönnblom
2020-01-14 17:32 ` [dpdk-dev] [EXT] " Jerin Jacob Kollanukkaran
2020-01-14 18:03   ` [dpdk-dev] [PATCH v2] " Mattias Rönnblom

DPDK-dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dpdk-dev/0 dpdk-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dpdk-dev dpdk-dev/ https://lore.kernel.org/dpdk-dev \
		dev@dpdk.org
	public-inbox-index dpdk-dev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git