The following reply was made to PR kern/176446; it has been noted by GNATS.
From: "Charbon, Julien" <jchar...@verisign.com>
To: John Baldwin <j...@freebsd.org>
Cc: bug-follo...@freebsd.org,
"De La Gueronniere, Marc" <mdelagueronni...@verisign.com>,
j...@freebsd.org
Subject: Re: kern/176446: [netinet] [patch] Concurrency in ixgbe driving
out-of-order
packet process and spurious RST
Date: Thu, 28 Feb 2013 20:10:39 +0100
On 2/28/13 4:57 PM, John Baldwin wrote:
> Can you try the fixes from
> http://svnweb.freebsd.org/base?view=revision&revision=240968?
Actually, Marc (I CC'ed him) did find the r240968 fix for concurrency
between ixgbe_msix_que() and ixgbe_handle_que(), and made a backport for
release-8.3.0 (see patch [1] below). However, the issue was still
reproducible, then Marc found another place for concurrency from
ixgbe_local_timer() and fix it (see patch [2]). But it was still not
enough, and he found a last place for concurrency due to
ixgbe_rearm_queues() call (see patch [3]). We all these patches
applied, we were not able to reproduce this issue.
If patch [1] and [2] seems clearly legitimates, patch [3] would need
more discussions/feedback I guess.
--
Julien
[1] Patch ixgbe (1/3): Backport r240968 in release-8.3.0
Index: sys/dev/ixgbe/ixgbe.c
===================================================================
--- sys/dev/ixgbe/ixgbe.c
+++ sys/dev/ixgbe/ixgbe.c
@@ -102,13 +102,15 @@
static int ixgbe_attach(device_t);
static int ixgbe_detach(device_t);
static int ixgbe_shutdown(device_t);
-static void ixgbe_start(struct ifnet *);
-static void ixgbe_start_locked(struct tx_ring *, struct ifnet *);
#if __FreeBSD_version >= 800000
static int ixgbe_mq_start(struct ifnet *, struct mbuf *);
static int ixgbe_mq_start_locked(struct ifnet *,
struct tx_ring *, struct mbuf *);
static void ixgbe_qflush(struct ifnet *);
+static void ixgbe_deferred_mq_start(void *, int);
+#else
+static void ixgbe_start(struct ifnet *);
+static void ixgbe_start_locked(struct tx_ring *, struct ifnet *);
#endif
static int ixgbe_ioctl(struct ifnet *, u_long, caddr_t);
static void ixgbe_init(void *);
@@ -645,6 +647,7 @@
{
struct adapter *adapter = device_get_softc(dev);
struct ix_queue *que = adapter->queues;
+ struct tx_ring *txr = adapter->tx_rings;
u32 ctrl_ext;
INIT_DEBUGOUT("ixgbe_detach: begin");
@@ -659,8 +662,11 @@
ixgbe_stop(adapter);
IXGBE_CORE_UNLOCK(adapter);
- for (int i = 0; i < adapter->num_tx_queues; i++, que++) {
+ for (int i = 0; i < adapter->num_tx_queues; i++, que++, txr++) {
if (que->tq) {
+#if __FreeBSD_version >= 800000
+ taskqueue_drain(que->tq, &txr->txq_task);
+#endif
taskqueue_drain(que->tq, &que->que_task);
taskqueue_free(que->tq);
}
@@ -722,6 +728,7 @@
}
+#if __FreeBSD_version < 800000
/*********************************************************************
* Transmit entry point
*
@@ -793,7 +800,7 @@
return;
}
-#if __FreeBSD_version >= 800000
+#else
/*
** Multiqueue Transmit driver
**
@@ -821,7 +828,7 @@
IXGBE_TX_UNLOCK(txr);
} else {
err = drbr_enqueue(ifp, txr->br, m);
- taskqueue_enqueue(que->tq, &que->que_task);
+ taskqueue_enqueue(que->tq, &txr->txq_task);
}
return (err);
@@ -887,6 +894,22 @@
}
/*
+ * Called from a taskqueue to drain queued transmit packets.
+ */
+static void
+ixgbe_deferred_mq_start(void *arg, int pending)
+{
+ struct tx_ring *txr = arg;
+ struct adapter *adapter = txr->adapter;
+ struct ifnet *ifp = adapter->ifp;
+
+ IXGBE_TX_LOCK(txr);
+ if (!drbr_empty(ifp, txr->br))
+ ixgbe_mq_start_locked(ifp, txr, NULL);
+ IXGBE_TX_UNLOCK(txr);
+}
+
+/*
** Flush all ring buffers
*/
static void
@@ -2210,6 +2233,9 @@
{
device_t dev = adapter->dev;
struct ix_queue *que = adapter->queues;
+#if __FreeBSD_version >= 800000
+ struct tx_ring *txr = adapter->tx_rings;
+#endif
int error, rid = 0;
/* MSI RID at 1 */
@@ -2229,6 +2255,9 @@
* Try allocating a fast interrupt and the associated deferred
* processing contexts.
*/
+#if __FreeBSD_version >= 800000
+ TASK_INIT(&txr->txq_task, 0, ixgbe_deferred_mq_start, txr);
+#endif
TASK_INIT(&que->que_task, 0, ixgbe_handle_que, que);
que->tq = taskqueue_create_fast("ixgbe_que", M_NOWAIT,
taskqueue_thread_enqueue, &que->tq);
@@ -2275,9 +2304,10 @@
{
device_t dev = adapter->dev;
struct ix_queue *que = adapter->queues;
+ struct tx_ring *txr = adapter->tx_rings;
int error, rid, vector = 0;
- for (int i = 0; i < adapter->num_tx_queues; i++, vector++, que++) {
+ for (int i = 0; i < adapter->num_tx_queues; i++, vector++, que++,
txr++) {
rid = vector + 1;
que->res = bus_alloc_resource_any(dev, SYS_RES_IRQ, &rid,
RF_SHAREABLE | RF_ACTIVE);
@@ -2307,6 +2337,9 @@
if (adapter->num_tx_queues > 1)
bus_bind_intr(dev, que->res, i);
+#if __FreeBSD_version >= 800000
+ TASK_INIT(&txr->txq_task, 0, ixgbe_deferred_mq_start, txr);
+#endif
TASK_INIT(&que->que_task, 0, ixgbe_handle_que, que);
que->tq = taskqueue_create_fast("ixgbe_que", M_NOWAIT,
taskqueue_thread_enqueue, &que->tq);
@@ -2555,12 +2588,13 @@
ifp->if_softc = adapter;
ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST;
ifp->if_ioctl = ixgbe_ioctl;
- ifp->if_start = ixgbe_start;
#if __FreeBSD_version >= 800000
ifp->if_transmit = ixgbe_mq_start;
ifp->if_qflush = ixgbe_qflush;
+#else
+ ifp->if_start = ixgbe_start;
+ IFQ_SET_MAXLEN(&ifp->if_snd, adapter->num_tx_desc - 2);
#endif
- ifp->if_snd.ifq_maxlen = adapter->num_tx_desc - 2;
ether_ifattach(ifp, adapter->hw.mac.addr);
Index: sys/dev/ixgbe/ixgbe.h
===================================================================
--- sys/dev/ixgbe/ixgbe.h
+++ sys/dev/ixgbe/ixgbe.h
@@ -298,6 +298,7 @@
char mtx_name[16];
#if __FreeBSD_version >= 800000
struct buf_ring *br;
+ struct task txq_task;
#endif
#ifdef IXGBE_FDIR
u16 atr_sample;
[2] Patch ixgbe (2/3): Do not schedule ixgbe_handle_que() from
ixgbe_local_timer().
Index: sys/dev/ixgbe/ixgbe.c
===================================================================
--- sys/dev/ixgbe/ixgbe.c
+++ sys/dev/ixgbe/ixgbe.c
@@ -2033,7 +2033,7 @@
if (txr->queue_status & IXGBE_QUEUE_DEPLETED)
++busy;
if ((txr->queue_status & IXGBE_QUEUE_IDLE) == 0)
- taskqueue_enqueue(que->tq, &que->que_task);
+ taskqueue_enqueue(que->tq, &txr->txq_task);
}
/* Only truely watchdog if all queues show hung */
if (hung == adapter->num_tx_queues)
[3] Patch ixgbe (3/3): ixgbe_rearm_queues() directly schedules an
interruption and drives not wanted concurrency, should we called it at all?
Index: sys/dev/ixgbe/ixgbe.c
===================================================================
--- sys/dev/ixgbe/ixgbe.c
+++ sys/dev/ixgbe/ixgbe.c
@@ -2046,7 +2046,7 @@
ifp->if_drv_flags &= ~IFF_DRV_OACTIVE;
out:
- ixgbe_rearm_queues(adapter, adapter->que_mask);
+ // ixgbe_rearm_queues(adapter, adapter->que_mask);
callout_reset(&adapter->timer, hz, ixgbe_local_timer, adapter);
return;
@@ -4674,7 +4674,7 @@
** Schedule another interrupt if so.
*/
if ((staterr & IXGBE_RXD_STAT_DD) != 0) {
- ixgbe_rearm_queues(adapter, (u64)(1 << que->msix));
+ // ixgbe_rearm_queues(adapter, (u64)(1 << que->msix));
return (TRUE);
}
_______________________________________________
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
