diff mbox

[dpdk-dev] examples: fix symmetric_mp, set NIC rx_drop_en bit

Message ID 1417625819-3024-1-git-send-email-bruce.richardson@intel.com (mailing list archive)
State Accepted, archived
Headers

Commit Message

Bruce Richardson Dec. 3, 2014, 4:56 p.m. UTC 
  The symmetric_mp example app is set up to allow two processes to
share a NIC port, with each pulling packets from one queue. In order
to have the app continue working when one of the process dies, the
drop_en bit should be set in the NIC configuration. Without this bit
set, the NIC will stall once any queue fills. With the bit set, once
a queue fills, all subsequent packets for that queue are discarded
allowing other queues to continue operating as normal.

This setting was missed when converting to use standardised defaults
in commit 81f7ecd9.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 examples/multi_process/symmetric_mp/main.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Thomas Monjalon Dec. 4, 2014, 1:37 p.m. UTC | #1 
2014-12-03 16:56, Bruce Richardson:
> The symmetric_mp example app is set up to allow two processes to
> share a NIC port, with each pulling packets from one queue. In order
> to have the app continue working when one of the process dies, the
> drop_en bit should be set in the NIC configuration. Without this bit
> set, the NIC will stall once any queue fills. With the bit set, once
> a queue fills, all subsequent packets for that queue are discarded
> allowing other queues to continue operating as normal.
> 
> This setting was missed when converting to use standardised defaults
> in commit 81f7ecd9.

I don't see rx_drop_en in commit 81f7ecd9.
Why do you say it was missed?
Bruce Richardson Dec. 4, 2014, 2:03 p.m. UTC | #2 
On Thu, Dec 04, 2014 at 02:37:15PM +0100, Thomas Monjalon wrote:
> 2014-12-03 16:56, Bruce Richardson:
> > The symmetric_mp example app is set up to allow two processes to
> > share a NIC port, with each pulling packets from one queue. In order
> > to have the app continue working when one of the process dies, the
> > drop_en bit should be set in the NIC configuration. Without this bit
> > set, the NIC will stall once any queue fills. With the bit set, once
> > a queue fills, all subsequent packets for that queue are discarded
> > allowing other queues to continue operating as normal.
> > 
> > This setting was missed when converting to use standardised defaults
> > in commit 81f7ecd9.
> 
> I don't see rx_drop_en in commit 81f7ecd9.
> Why do you say it was missed?
>
This is the one app that needs the drop_en bit set, and the bit is not set in the
defaults. Therefore, when this app was converted to use the defaults from the
driver, the drop_en bit should have been explicitly set as in this patch. [The 
fact of there being no drop_en bit in the commit is proof of its absense :-)].

/Bruce
Thomas Monjalon Dec. 4, 2014, 3:36 p.m. UTC | #3 
2014-12-04 14:03, Bruce Richardson:
> On Thu, Dec 04, 2014 at 02:37:15PM +0100, Thomas Monjalon wrote:
> > 2014-12-03 16:56, Bruce Richardson:
> > > The symmetric_mp example app is set up to allow two processes to
> > > share a NIC port, with each pulling packets from one queue. In order
> > > to have the app continue working when one of the process dies, the
> > > drop_en bit should be set in the NIC configuration. Without this bit
> > > set, the NIC will stall once any queue fills. With the bit set, once
> > > a queue fills, all subsequent packets for that queue are discarded
> > > allowing other queues to continue operating as normal.
> > > 
> > > This setting was missed when converting to use standardised defaults
> > > in commit 81f7ecd9.
> > 
> > I don't see rx_drop_en in commit 81f7ecd9.
> > Why do you say it was missed?
> >
> This is the one app that needs the drop_en bit set, and the bit is not set in the
> defaults. Therefore, when this app was converted to use the defaults from the
> driver, the drop_en bit should have been explicitly set as in this patch. [The 
> fact of there being no drop_en bit in the commit is proof of its absense :-)].

So you agree this bit was never been set for this app?
Therefore I should remove the last sentence of your commit log before applying?
Bruce Richardson Dec. 4, 2014, 4:06 p.m. UTC | #4 
On Thu, Dec 04, 2014 at 04:36:42PM +0100, Thomas Monjalon wrote:
> 2014-12-04 14:03, Bruce Richardson:
> > On Thu, Dec 04, 2014 at 02:37:15PM +0100, Thomas Monjalon wrote:
> > > 2014-12-03 16:56, Bruce Richardson:
> > > > The symmetric_mp example app is set up to allow two processes to
> > > > share a NIC port, with each pulling packets from one queue. In order
> > > > to have the app continue working when one of the process dies, the
> > > > drop_en bit should be set in the NIC configuration. Without this bit
> > > > set, the NIC will stall once any queue fills. With the bit set, once
> > > > a queue fills, all subsequent packets for that queue are discarded
> > > > allowing other queues to continue operating as normal.
> > > > 
> > > > This setting was missed when converting to use standardised defaults
> > > > in commit 81f7ecd9.
> > > 
> > > I don't see rx_drop_en in commit 81f7ecd9.
> > > Why do you say it was missed?
> > >
> > This is the one app that needs the drop_en bit set, and the bit is not set in the
> > defaults. Therefore, when this app was converted to use the defaults from the
> > driver, the drop_en bit should have been explicitly set as in this patch. [The 
> > fact of there being no drop_en bit in the commit is proof of its absense :-)].
> 
> So you agree this bit was never been set for this app?
> Therefore I should remove the last sentence of your commit log before applying?
> 
>
Oh, now I get your point. Looking at 1.7 release, yes, this bit was never set
but should really have been set. Therefore, please remove the last line as you
suggest.

/Bruce
Thomas Monjalon Dec. 5, 2014, 4:21 p.m. UTC | #5 
2014-12-03 16:56, Bruce Richardson:
> The symmetric_mp example app is set up to allow two processes to
> share a NIC port, with each pulling packets from one queue. In order
> to have the app continue working when one of the process dies, the
> drop_en bit should be set in the NIC configuration. Without this bit
> set, the NIC will stall once any queue fills. With the bit set, once
> a queue fills, all subsequent packets for that queue are discarded
> allowing other queues to continue operating as normal.
> 
[snip]
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

Applied

Thanks
diff mbox

Patch

diff --git a/examples/multi_process/symmetric_mp/main.c b/examples/multi_process/symmetric_mp/main.c
index 01faae9..2fc2acf 100644
--- a/examples/multi_process/symmetric_mp/main.c
+++ b/examples/multi_process/symmetric_mp/main.c
@@ -229,6 +229,7 @@  smp_port_init(uint8_t port, struct rte_mempool *mbuf_pool, uint16_t num_queues)
 			}
 	};
 	const uint16_t rx_rings = num_queues, tx_rings = num_queues;
+	struct rte_eth_dev_info info;
 	int retval;
 	uint16_t q;
 
@@ -241,6 +242,9 @@  smp_port_init(uint8_t port, struct rte_mempool *mbuf_pool, uint16_t num_queues)
 	printf("# Initialising port %u... ", (unsigned)port);
 	fflush(stdout);
 
+	rte_eth_dev_info_get(port, &info);
+	info.default_rxconf.rx_drop_en = 1;
+
 	retval = rte_eth_dev_configure(port, rx_rings, tx_rings, &port_conf);
 	if (retval < 0)
 		return retval;
@@ -248,7 +252,7 @@  smp_port_init(uint8_t port, struct rte_mempool *mbuf_pool, uint16_t num_queues)
 	for (q = 0; q < rx_rings; q ++) {
 		retval = rte_eth_rx_queue_setup(port, q, RX_RING_SIZE,
 				rte_eth_dev_socket_id(port),
-				NULL,
+				&info.default_rxconf,
 				mbuf_pool);
 		if (retval < 0)
 			return retval;