On Tue, Dec 29, 2020 at 11:18:26PM +0100, Claudio Jeker wrote:
> Generally I would prefer to go for direct dispatch and not use netisr.
> This removes a queue and a scheduling point and should help reduce the
> latency in processing pppoe packages.
>  
> This does not mean that I'm against this change. I just think it may be
> benefitial to move one step further.
Here's a diff that removes the kernel lock and calls input routines
directly instead of (de)queuing through netisr.

Previously, if_netisr() handled the net lock around those calls, now
if_input_process() does it before calling ether_input(), so no need to
add or remove NET_*LOCK() anywhere.

I'm running this on my home router without any regression so far.

Feedback? Objections? OK?
NB: I want to commit this separately modulo the previous diff.


Index: if.c
===================================================================
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.621
diff -u -p -r1.621 if.c
--- if.c        15 Dec 2020 03:43:34 -0000      1.621
+++ if.c        3 Jan 2021 21:38:57 -0000
@@ -68,7 +68,6 @@
 #include "pf.h"
 #include "pfsync.h"
 #include "ppp.h"
-#include "pppoe.h"
 #include "switch.h"
 #include "if_wg.h"
 
@@ -902,13 +901,6 @@ if_netisr(void *unused)
                if (n & (1 << NETISR_SWITCH)) {
                        KERNEL_LOCK();
                        switchintr();
-                       KERNEL_UNLOCK();
-               }
-#endif
-#if NPPPOE > 0
-               if (n & (1 << NETISR_PPPOE)) {
-                       KERNEL_LOCK();
-                       pppoeintr();
                        KERNEL_UNLOCK();
                }
 #endif
Index: if_ethersubr.c
===================================================================
RCS file: /cvs/src/sys/net/if_ethersubr.c,v
retrieving revision 1.267
diff -u -p -r1.267 if_ethersubr.c
--- if_ethersubr.c      1 Oct 2020 05:14:10 -0000       1.267
+++ if_ethersubr.c      3 Jan 2021 21:41:17 -0000
@@ -532,9 +532,9 @@ ether_input(struct ifnet *ifp, struct mb
                }
 #endif
                if (etype == ETHERTYPE_PPPOEDISC)
-                       niq_enqueue(&pppoediscinq, m);
+                       pppoe_disc_input(m);
                else
-                       niq_enqueue(&pppoeinq, m);
+                       pppoe_data_input(m);
                return;
 #endif
 #ifdef MPLS
Index: if_pppoe.c
===================================================================
RCS file: /cvs/src/sys/net/if_pppoe.c,v
retrieving revision 1.75
diff -u -p -r1.75 if_pppoe.c
--- if_pppoe.c  30 Dec 2020 13:18:07 -0000      1.75
+++ if_pppoe.c  4 Jan 2021 00:14:30 -0000
@@ -143,14 +143,8 @@ struct pppoe_softc {
        struct timeval sc_session_time; /* [N] time the session was established 
*/
 };
 
-/* incoming traffic will be queued here */
-struct niqueue pppoediscinq = NIQUEUE_INITIALIZER(IFQ_MAXLEN, NETISR_PPPOE);
-struct niqueue pppoeinq = NIQUEUE_INITIALIZER(IFQ_MAXLEN, NETISR_PPPOE);
-
 /* input routines */
-static void pppoe_disc_input(struct mbuf *);
 static void pppoe_dispatch_disc_pkt(struct mbuf *);
-static void pppoe_data_input(struct mbuf *);
 
 /* management routines */
 void pppoeattach(int);
@@ -341,21 +335,6 @@ pppoe_find_softc_by_hunique(u_int8_t *to
        return (sc);
 }
 
-/* Interface interrupt handler routine. */
-void
-pppoeintr(void)
-{
-       struct mbuf *m;
-
-       NET_ASSERT_LOCKED();
-
-       while ((m = niq_dequeue(&pppoediscinq)) != NULL)
-               pppoe_disc_input(m);
-
-       while ((m = niq_dequeue(&pppoeinq)) != NULL)
-               pppoe_data_input(m);
-}
-
 /* Analyze and handle a single received packet while not in session state. */
 static void
 pppoe_dispatch_disc_pkt(struct mbuf *m)
@@ -649,7 +628,7 @@ done:
 }
 
 /* Input function for discovery packets. */
-static void
+void
 pppoe_disc_input(struct mbuf *m)
 {
        /* avoid error messages if there is not a single pppoe instance */
@@ -661,7 +640,7 @@ pppoe_disc_input(struct mbuf *m)
 }
 
 /* Input function for data packets */
-static void
+void
 pppoe_data_input(struct mbuf *m)
 {
        struct pppoe_softc *sc;
Index: if_pppoe.h
===================================================================
RCS file: /cvs/src/sys/net/if_pppoe.h,v
retrieving revision 1.6
diff -u -p -r1.6 if_pppoe.h
--- if_pppoe.h  10 Apr 2015 13:58:20 -0000      1.6
+++ if_pppoe.h  3 Jan 2021 21:46:11 -0000
@@ -66,10 +66,8 @@ struct pppoeconnectionstate {
 
 #ifdef _KERNEL
 
-extern struct niqueue pppoediscinq;
-extern struct niqueue pppoeinq;
-
-void pppoeintr(void);
+void pppoe_disc_input(struct mbuf *);
+void pppoe_data_input(struct mbuf *);
 
 #endif /* _KERNEL */
 #endif /* _NET_IF_PPPOE_H_ */
Index: netisr.h
===================================================================
RCS file: /cvs/src/sys/net/netisr.h,v
retrieving revision 1.53
diff -u -p -r1.53 netisr.h
--- netisr.h    6 Aug 2020 12:00:46 -0000       1.53
+++ netisr.h    3 Jan 2021 21:38:37 -0000
@@ -45,7 +45,6 @@
 #define        NETISR_ARP      18              /* same as AF_LINK */
 #define        NETISR_PPP      28              /* for PPP processing */
 #define        NETISR_BRIDGE   29              /* for bridge processing */
-#define        NETISR_PPPOE    30              /* for pppoe processing */
 #define        NETISR_SWITCH   31              /* for switch dataplane */
 
 #ifndef _LOCORE

Reply via email to