Fix epoch handling in if_oce

[tuexen]

Summary

When calling tcp_lro_flush_all(), the caller must enter the network epoch. This patch ensured this for the oce driver.
This issue was reported in PR 260330

[gallatin]

Why not just fix the driver for epoch for real so that the normal, non LRO path holds epoch and we don't need to repeatedly enter it in ether_input()? I've tried to upload an alternate patch..

t.diff1 KBDownload

[gallatin]

In D33395#756461, @gallatin wrote:

Why not just fix the driver for epoch for real so that the normal, non LRO path holds epoch and we don't need to repeatedly enter it in ether_input()? I've tried to upload an alternate patch..

t.diff1 KBDownload

diff --git a/sys/dev/oce/oce_if.c b/sys/dev/oce/oce_if.c
index 271c35015a9..1127565282c 100644
--- a/sys/dev/oce/oce_if.c
+++ b/sys/dev/oce/oce_if.c
@@ -2039,14 +2039,17 @@ oce_rq_handler_lro(void *arg)
 uint16_t
 oce_rq_handler(void *arg)
 {
+       struct epoch_tracker et;
        struct oce_rq *rq = (struct oce_rq *)arg;
        struct oce_cq *cq = rq->cq;
        POCE_SOFTC sc = rq->parent;
        struct oce_nic_rx_cqe *cqe;
        int num_cqes = 0;
 
+       NET_EPOCH_ENTER(et);
        if(rq->islro) {
                oce_rq_handler_lro(arg);
+               NET_EPOCH_EXIT(et);
                return 0;
        }
        LOCK(&rq->rx_lock);
@@ -2090,6 +2093,7 @@ oce_rq_handler(void *arg)
 
        oce_check_rx_bufs(sc, num_cqes, rq);
        UNLOCK(&rq->rx_lock);
+       NET_EPOCH_EXIT(et);
        return 0;
 
 }
@@ -2110,7 +2114,7 @@ oce_attach_ifp(POCE_SOFTC sc)
        ifmedia_add(&sc->media, IFM_ETHER | IFM_AUTO, 0, NULL);
        ifmedia_set(&sc->media, IFM_ETHER | IFM_AUTO);
 
-       sc->ifp->if_flags = IFF_BROADCAST | IFF_MULTICAST;
+       sc->ifp->if_flags = IFF_BROADCAST | IFF_MULTICAST | IFF_KNOWSEPOCH;
        sc->ifp->if_ioctl = oce_ioctl;
        sc->ifp->if_start = oce_start;
        sc->ifp->if_init = oce_init;

[tuexen]

I like your approach much better than mine. Does your patch already handle the calling sequence:

oce_if_deactivate()
  oce_stop_rx()
    oce_rx_cq_clean()
      oce_rx_flush_lro()
        tcp_lro_flush_all()

Or don't we need to cover that?

[gallatin]

In D33395#756468, @tuexen wrote:

I like your approach much better than mine. Does your patch already handle the calling sequence:

oce_if_deactivate()
  oce_stop_rx()
    oce_rx_cq_clean()
      oce_rx_flush_lro()
        tcp_lro_flush_all()

Or don't we need to cover that?

Indeed, I think it misses that case. I've updated the patch...

t.diff2 KBDownload

diff --git a/sys/dev/oce/oce_if.c b/sys/dev/oce/oce_if.c
index 271c35015a9..1127565282c 100644
--- a/sys/dev/oce/oce_if.c
+++ b/sys/dev/oce/oce_if.c
@@ -2039,14 +2039,17 @@ oce_rq_handler_lro(void *arg)
 uint16_t
 oce_rq_handler(void *arg)
 {
+	struct epoch_tracker et;
 	struct oce_rq *rq = (struct oce_rq *)arg;
 	struct oce_cq *cq = rq->cq;
 	POCE_SOFTC sc = rq->parent;
 	struct oce_nic_rx_cqe *cqe;
 	int num_cqes = 0;
 
+	NET_EPOCH_ENTER(et);
 	if(rq->islro) {
 		oce_rq_handler_lro(arg);
+		NET_EPOCH_EXIT(et);
 		return 0;
 	}
 	LOCK(&rq->rx_lock);
@@ -2090,6 +2093,7 @@ oce_rq_handler(void *arg)
 
 	oce_check_rx_bufs(sc, num_cqes, rq);
 	UNLOCK(&rq->rx_lock);
+	NET_EPOCH_EXIT(et);
 	return 0;
 
 }
@@ -2110,7 +2114,7 @@ oce_attach_ifp(POCE_SOFTC sc)
 	ifmedia_add(&sc->media, IFM_ETHER | IFM_AUTO, 0, NULL);
 	ifmedia_set(&sc->media, IFM_ETHER | IFM_AUTO);
 
-	sc->ifp->if_flags = IFF_BROADCAST | IFF_MULTICAST;
+	sc->ifp->if_flags = IFF_BROADCAST | IFF_MULTICAST | IFF_KNOWSEPOCH;
 	sc->ifp->if_ioctl = oce_ioctl;
 	sc->ifp->if_start = oce_start;
 	sc->ifp->if_init = oce_init;
diff --git a/sys/dev/oce/oce_if.h b/sys/dev/oce/oce_if.h
index 8938dd86ecc..4ddce2aab59 100644
--- a/sys/dev/oce/oce_if.h
+++ b/sys/dev/oce/oce_if.h
@@ -43,6 +43,7 @@
 #include <sys/param.h>
 #include <sys/endian.h>
 #include <sys/eventhandler.h>
+#include <sys/epoch.h>
 #include <sys/malloc.h>
 #include <sys/module.h>
 #include <sys/kernel.h>
diff --git a/sys/dev/oce/oce_queue.c b/sys/dev/oce/oce_queue.c
index 04c92a5f397..1308ca1ab70 100644
--- a/sys/dev/oce/oce_queue.c
+++ b/sys/dev/oce/oce_queue.c
@@ -1233,12 +1233,14 @@ oce_rx_cq_clean(struct oce_rq *rq)
 void
 oce_stop_rx(POCE_SOFTC sc)
 {
+	struct epoch_tracker et;
         struct oce_mbx mbx;
         struct mbx_delete_nic_rq *fwcmd;
         struct mbx_delete_nic_rq_v1 *fwcmd1;
         struct oce_rq *rq;
         int i = 0;
 
+	NET_EPOCH_ENTER(et);
        /* before deleting disable hwlro */
 	if(sc->enable_hwlro)
         	oce_mbox_nic_set_iface_lro_config(sc, 0);
@@ -1274,6 +1276,7 @@ oce_stop_rx(POCE_SOFTC sc)
 			UNLOCK(&rq->rx_lock);
                 }
         }
+	NET_EPOCH_EXIT(et);
 }
 
 int

[tuexen]

I like the second patch you suggested. How to proceed? Would you like to create a new review for your patch and I approve it or would you like me to update this review with your patch?

[gallatin]

In D33395#756733, @tuexen wrote:

I like the second patch you suggested. How to proceed? Would you like to create a new review for your patch and I approve it or would you like me to update this review with your patch?

If you'd like to update w/my patch, that would be great. I don't have one of these NICs, so I can't test..

[tuexen]

Use patch suggested by Drew.

[tuexen]

In D33395#756787, @gallatin wrote:

In D33395#756733, @tuexen wrote:

I like the second patch you suggested. How to proceed? Would you like to create a new review for your patch and I approve it or would you like me to update this review with your patch?

If you'd like to update w/my patch, that would be great. I don't have one of these NICs, so I can't test..

I updated the patch. Since I also don't have the hardware, I asked the reporter of the bug to test the patch.

[glebius]

P.S. I should get back to where we left over a year ago and make IFF_KNOWSEPOCH a default thing.