Hi,

For parallel IP forwarding I had put kernel lock around arpresolve()
as a quick workaround for crashes.  Moving the kernel lock inside
the function makes the hot path lock free.  I see slight prerformace
increase in my test and no lock contention in kstack flamegraph.

http://bluhm.genua.de/perform/results/latest/2022-05-16T00%3A00%3A00Z/btrace/tcpbench_-S1000000_-t10_-n100_10.3.45.35-btrace-kstack.0.svg
http://bluhm.genua.de/perform/results/latest/patch-sys-arpresolve-kernel-lock.1/btrace/tcpbench_-S1000000_-t10_-n100_10.3.45.35-btrace-kstack.0.svg

Search for kernel_lock.  Matched goes from 0.6% to 0.2%

We are running such a diff in our genua code for a while.  I think
route flags need more love.  I doubt that all flags and fields are
consistent when run on multiple CPU.  But this diff does not make
it worse and increases MP pressure.

ok?

bluhm

Index: net/if_ethersubr.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_ethersubr.c,v
retrieving revision 1.279
diff -u -p -r1.279 if_ethersubr.c
--- net/if_ethersubr.c  22 Apr 2022 12:10:57 -0000      1.279
+++ net/if_ethersubr.c  17 May 2022 22:26:22 -0000
@@ -221,10 +221,7 @@ ether_resolve(struct ifnet *ifp, struct 
 
        switch (af) {
        case AF_INET:
-               KERNEL_LOCK();
-               /* XXXSMP there is a MP race in arpresolve() */
                error = arpresolve(ifp, rt, m, dst, eh->ether_dhost);
-               KERNEL_UNLOCK();
                if (error)
                        return (error);
                eh->ether_type = htons(ETHERTYPE_IP);
@@ -285,10 +282,7 @@ ether_resolve(struct ifnet *ifp, struct 
                        break;
 #endif
                case AF_INET:
-                       KERNEL_LOCK();
-                       /* XXXSMP there is a MP race in arpresolve() */
                        error = arpresolve(ifp, rt, m, dst, eh->ether_dhost);
-                       KERNEL_UNLOCK();
                        if (error)
                                return (error);
                        break;
Index: netinet/if_ether.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/if_ether.c,v
retrieving revision 1.248
diff -u -p -r1.248 if_ether.c
--- netinet/if_ether.c  28 Apr 2021 21:21:44 -0000      1.248
+++ netinet/if_ether.c  17 May 2022 22:26:22 -0000
@@ -347,8 +347,7 @@ arpresolve(struct ifnet *ifp, struct rte
                log(LOG_DEBUG, "%s: %s: route contains no arp information\n",
                    __func__, inet_ntop(AF_INET, &satosin(rt_key(rt))->sin_addr,
                    addr, sizeof(addr)));
-               m_freem(m);
-               return (EINVAL);
+               goto bad;
        }
 
        sdl = satosdl(rt->rt_gateway);
@@ -386,6 +385,15 @@ arpresolve(struct ifnet *ifp, struct rte
        if (ifp->if_flags & (IFF_NOARP|IFF_STATICARP))
                goto bad;
 
+       KERNEL_LOCK();
+       /* re-check since we grab the KERNEL lock after the first check */
+       if (!ISSET(rt->rt_flags, RTF_LLINFO)) {
+               KERNEL_UNLOCK();
+               goto bad;
+       }
+       la = (struct llinfo_arp *)rt->rt_llinfo;
+       KASSERT(la != NULL);
+
        /*
         * There is an arptab entry, but no ethernet address
         * response yet. Insert mbuf in hold queue if below limit
@@ -430,6 +438,7 @@ arpresolve(struct ifnet *ifp, struct rte
                }
        }
 
+       KERNEL_UNLOCK();
        return (EAGAIN);
 
 bad:

Reply via email to