Date: Tue, 15 Jul 2014 10:22:42 +0200 From: Stefano Garzarella <stefanogarzarella@gmail.com> To: Borja Marcos <borjam@sarenet.es> Cc: "freebsd-net@freebsd.org" <freebsd-net@freebsd.org>, freebsd-current <freebsd-current@freebsd.org>, Luigi Rizzo <rizzo@iet.unipi.it>, Xin LI <d@delphij.net> Subject: Re: Fix Emulex "oce" driver in CURRENT Message-ID: <CAO0mX5bmXnQGuEgSYoBxy8bQK5i3B2MG0LBmJ7A178W1B5sqDw@mail.gmail.com> In-Reply-To: <6C8CF68D-68E2-4168-AA0A-6A629D363371@sarenet.es> References: <CA%2BhQ2%2BimE=%2BncZwpHGhWb175mYiAKV78MV=Dfc1GJf=3XYciPQ@mail.gmail.com> <453BA9EC-BB63-4258-8141-847F41315E1E@sarenet.es> <CA%2BhQ2%2BjaP2fuMaCoorLpGu=uWDPgHy3at5UdtLAOXM2d6uoWkg@mail.gmail.com> <6C8CF68D-68E2-4168-AA0A-6A629D363371@sarenet.es>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi,
I found other problems in the "oce" driver during some experiments with
netmap in emulation mode.
In details:
- missing locking:
- in some functions there are write accesses on the wq struct (tx queue
descriptor)
without acquire LOCK on the queue, particularly in oce_wq_handler() that is
invoked
in the interrupt routine. For this reason there may be race conditions.
- tx cleanup:
- in oce_if_deactivate() the wq queues are drained but some still pending
mbufs are not freed.
For this reason, I added the oce_tx_clean() that releases any pending mbufs.
I also tried experimenting with iperf3 using the same Borja environment and
I don't have panic.
Can you try this patch? Do you still have the panic?
Cheers,
Stefano Garzarella
diff --git a/sys/dev/oce/oce_if.c b/sys/dev/oce/oce_if.c
index af57491..33b35b4 100644
--- a/sys/dev/oce/oce_if.c
+++ b/sys/dev/oce/oce_if.c
@@ -142,6 +142,7 @@ static int oce_tx(POCE_SOFTC sc, struct mbuf **mpp,
int wq_index);
static void oce_tx_restart(POCE_SOFTC sc, struct oce_wq *wq);
static void oce_tx_complete(struct oce_wq *wq, uint32_t wqe_idx,
uint32_t status);
+static void oce_tx_clean(POCE_SOFTC sc);
static int oce_multiq_transmit(struct ifnet *ifp, struct mbuf *m,
struct oce_wq *wq);
@@ -585,8 +586,10 @@ oce_multiq_flush(struct ifnet *ifp)
int i = 0;
for (i = 0; i < sc->nwqs; i++) {
+ LOCK(&sc->wq[i]->tx_lock);
while ((m = buf_ring_dequeue_sc(sc->wq[i]->br)) != NULL)
m_freem(m);
+ UNLOCK(&sc->wq[i]->tx_lock);
}
if_qflush(ifp);
}
@@ -1052,6 +1055,19 @@ oce_tx_complete(struct oce_wq *wq, uint32_t wqe_idx,
uint32_t status)
}
}
+static void
+oce_tx_clean(POCE_SOFTC sc) {
+ int i = 0;
+ struct oce_wq *wq;
+
+ for_all_wq_queues(sc, wq, i) {
+ LOCK(&wq->tx_lock);
+ while (wq->pkt_desc_tail != wq->pkt_desc_head) {
+ oce_tx_complete(wq, 0, 0);
+ }
+ UNLOCK(&wq->tx_lock);
+ }
+}
static void
oce_tx_restart(POCE_SOFTC sc, struct oce_wq *wq)
@@ -1213,6 +1229,8 @@ oce_wq_handler(void *arg)
struct oce_nic_tx_cqe *cqe;
int num_cqes = 0;
+ LOCK(&wq->tx_lock);
+
bus_dmamap_sync(cq->ring->dma.tag,
cq->ring->dma.map, BUS_DMASYNC_POSTWRITE);
cqe = RING_GET_CONSUMER_ITEM_VA(cq->ring, struct oce_nic_tx_cqe);
@@ -1237,6 +1255,8 @@ oce_wq_handler(void *arg)
if (num_cqes)
oce_arm_cq(sc, cq->cq_id, num_cqes, FALSE);
+ UNLOCK(&wq->tx_lock);
+
return 0;
}
@@ -2087,6 +2107,9 @@ oce_if_deactivate(POCE_SOFTC sc)
/* Delete RX queue in card with flush param */
oce_stop_rx(sc);
+ /* Flush the mbufs that are still in TX queues */
+ oce_tx_clean(sc);
+
/* Invalidate any pending cq and eq entries*/
for_all_evnt_queues(sc, eq, i)
oce_drain_eq(eq);
diff --git a/sys/dev/oce/oce_queue.c b/sys/dev/oce/oce_queue.c
index 308c16d..161011b 100644
--- a/sys/dev/oce/oce_queue.c
+++ b/sys/dev/oce/oce_queue.c
@@ -969,7 +969,9 @@ oce_start_rq(struct oce_rq *rq)
int
oce_start_wq(struct oce_wq *wq)
{
+ LOCK(&wq->tx_lock); /* XXX: maybe not necessary */
oce_arm_cq(wq->parent, wq->cq->cq_id, 0, TRUE);
+ UNLOCK(&wq->tx_lock);
return 0;
}
@@ -1076,6 +1078,8 @@ oce_drain_wq_cq(struct oce_wq *wq)
struct oce_nic_tx_cqe *cqe;
int num_cqes = 0;
+ LOCK(&wq->tx_lock); /* XXX: maybe not necessary */
+
bus_dmamap_sync(cq->ring->dma.tag, cq->ring->dma.map,
BUS_DMASYNC_POSTWRITE);
@@ -1093,6 +1097,7 @@ oce_drain_wq_cq(struct oce_wq *wq)
oce_arm_cq(sc, cq->cq_id, num_cqes, FALSE);
+ UNLOCK(&wq->tx_lock);
}
2014-07-07 13:57 GMT+02:00 Borja Marcos <borjam@sarenet.es>:
>
> On Jul 7, 2014, at 1:23 PM, Luigi Rizzo wrote:
>
> > On Mon, Jul 7, 2014 at 1:03 PM, Borja Marcos <borjam@sarenet.es> wrote:
> > we'll try to investigate, can you tell us more about the environment you
> use ?
> > (FreeBSD version, card model (PCI id perhaps), iperf3 invocation line,
> > interface configuration etc.)
> >
> > The main differences between 10.0.747.0 and the code in head (after
> > our fix) is the use
> > of drbr_enqueue/dequeue versus the peek/putback in the transmit routine.
> >
> >
> > Both drivers still have issues when the link flaps because the
> > transmit queue is not cleaned
> > up properly (unlike what happens in the linux driver and all FreeBSD
> > drivers for different
> > hardware), so it might well be that you are seeing some side effect of
> > that or other
> > problem which manifests itself differently depending on the environment.
> >
> > 'instant panic' by itself does not tell us anything about what could
> > be the problem you experience (and we do not see it with either driver).
>
> The environment details are here:
>
> https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=183391
>
> The way I produce an instant panic is:
>
> 1) Connect to another machine (cross connect cable)
>
> 2) iperf3 -s on the other machine
> (The other machine is different, it has an "ix" card)
>
> 3) iperf3 -t 30 -P 4 -c 10.0.0.1 -N
>
> In less than 30 seconds, panic.
>
>
>
> mierda dumped core - see /var/crash/vmcore.0
>
> Mon Jul 7 13:06:44 CEST 2014
>
> FreeBSD mierda 10.0-STABLE FreeBSD 10.0-STABLE #2: Mon Jul 7 11:41:45
> CEST 2014 root@mierda:/usr/obj/usr/src/sys/GENERIC amd64
>
> panic: sbsndptr: sockbuf 0xfffff800a70489b0 and mbuf 0xfffff801a3326e00
> clashing
>
> GNU gdb 6.1.1 [FreeBSD]
> Copyright 2004 Free Software Foundation, Inc.
> GDB is free software, covered by the GNU General Public License, and you
> are
> welcome to change it and/or distribute copies of it under certain
> conditions.
> Type "show copying" to see the conditions.
> There is absolutely no warranty for GDB. Type "show warranty" for details.
> This GDB was configured as "amd64-marcel-freebsd"...
>
> Unread portion of the kernel message buffer:
> panic: sbsndptr: sockbuf 0xfffff800a70489b0 and mbuf 0xfffff801a3326e00
> clashing
> cpuid = 12
> KDB: stack backtrace:
> #0 0xffffffff8092a470 at kdb_backtrace+0x60
> #1 0xffffffff808ef9c5 at panic+0x155
> #2 0xffffffff80962710 at sbdroprecord_locked+0
> #3 0xffffffff80a8ba8c at tcp_output+0xdbc
> #4 0xffffffff80a8987f at tcp_do_segment+0x30ff
> #5 0xffffffff80a85b34 at tcp_input+0xd04
> #6 0xffffffff80a1af57 at ip_input+0x97
> #7 0xffffffff809ba512 at netisr_dispatch_src+0x62
> #8 0xffffffff809b1ae6 at ether_demux+0x126
> #9 0xffffffff809b278e at ether_nh_input+0x35e
> #10 0xffffffff809ba512 at netisr_dispatch_src+0x62
> #11 0xffffffff81c19ab9 at oce_rx+0x3c9
> #12 0xffffffff81c19536 at oce_rq_handler+0xb6
> #13 0xffffffff81c1bb1c at oce_intr+0xdc
> #14 0xffffffff80938b35 at taskqueue_run_locked+0xe5
> #15 0xffffffff809395c8 at taskqueue_thread_loop+0xa8
> #16 0xffffffff808c057a at fork_exit+0x9a
> #17 0xffffffff80ccb51e at fork_trampoline+0xe
> Uptime: 51m20s
>
>
>
>
>
>
>
>
>
>
>
>
>
> Borja.
>
> _______________________________________________
> freebsd-current@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org"
>
--
Stefano Garzarella
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAO0mX5bmXnQGuEgSYoBxy8bQK5i3B2MG0LBmJ7A178W1B5sqDw>