On Sun, May 28, 2017 at 04:06:22PM +0200, Martin Pieuchot wrote:
> Our original plan was to unlock the forwarding path without taking any
> lock. That's why I added some KERNEL_LOCK()/KERNEL_UNLOCK() around non
> MP-safe data structures.
>
> Now we're going to rely on the NET_LOCK() for the interface address and
> multicast list. So replace the KERNEL_LOCK()/KERNEL_UNLOCK() their.
>
> IPsec will still need the KERNEL_LOCK(). But taking and releasing it
> doesn't make sense. It's also wrong since tdb are not refcounted. So
> assert that the KERNEL_LOCK() is held. The idea is to run the softnet
> task under KERNEL_LOCK() if ipsec_in_use is true.
>
> ok?
OK bluhm@
>
> Index: netinet/in.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/in.c,v
> retrieving revision 1.138
> diff -u -p -u -4 -r1.138 in.c
> --- netinet/in.c 16 May 2017 12:24:01 -0000 1.138
> +++ netinet/in.c 28 May 2017 14:00:12 -0000
> @@ -797,10 +797,8 @@ in_addmulti(struct in_addr *ap, struct i
> {
> struct in_multi *inm;
> struct ifreq ifr;
>
> - NET_ASSERT_LOCKED();
> -
> /*
> * See if address already in list.
> */
> IN_LOOKUP_MULTI(*ap, ifp, inm);
> @@ -900,12 +898,10 @@ in_hasmulti(struct in_addr *ap, struct i
> {
> struct in_multi *inm;
> int joined;
>
> - KERNEL_LOCK();
> IN_LOOKUP_MULTI(*ap, ifp, inm);
> joined = (inm != NULL);
> - KERNEL_UNLOCK();
>
> return (joined);
> }
>
> Index: netinet/in_var.h
> ===================================================================
> RCS file: /cvs/src/sys/netinet/in_var.h,v
> retrieving revision 1.39
> diff -u -p -u -4 -r1.39 in_var.h
> --- netinet/in_var.h 15 Jun 2016 19:39:34 -0000 1.39
> +++ netinet/in_var.h 28 May 2017 14:01:33 -0000
> @@ -85,8 +85,9 @@ struct in_aliasreq {
> /* struct ifnet *ifp; */ \
> /* struct in_ifaddr *ia; */ \
> do { \
> struct ifaddr *ifa; \
> + NET_ASSERT_LOCKED(); \
> TAILQ_FOREACH(ifa, &(ifp)->if_addrlist, ifa_list) { \
> if (ifa->ifa_addr->sa_family == AF_INET) \
> break; \
> } \
> @@ -141,8 +142,9 @@ ifmatoinm(struct ifmaddr *ifma)
> do { \
> struct ifmaddr *ifma; \
> \
> (inm) = NULL; \
> + NET_ASSERT_LOCKED(); \
> TAILQ_FOREACH(ifma, &(ifp)->if_maddrlist, ifma_list) \
> if (ifma->ifma_addr->sa_family == AF_INET && \
> ifmatoinm(ifma)->inm_addr.s_addr == (addr).s_addr) {\
> (inm) = ifmatoinm(ifma); \
> Index: netinet/ip_input.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_input.c,v
> retrieving revision 1.306
> diff -u -p -u -4 -r1.306 ip_input.c
> --- netinet/ip_input.c 28 May 2017 12:22:54 -0000 1.306
> +++ netinet/ip_input.c 28 May 2017 13:56:09 -0000
> @@ -440,11 +440,11 @@ ipv4_input(struct mbuf *m)
> #ifdef IPSEC
> if (ipsec_in_use) {
> int rv;
>
> - KERNEL_LOCK();
> + KERNEL_ASSERT_LOCKED();
> +
> rv = ipsec_forward_check(m, hlen, AF_INET);
> - KERNEL_UNLOCK();
> if (rv != 0) {
> ipstat_inc(ips_cantforward);
> goto bad;
> }
> @@ -666,9 +666,9 @@ in_ouraddr(struct mbuf *m, struct ifnet
> * The check in the loop assumes you only rx a packet on an UP
> * interface, and that M_BCAST will only be set on a BROADCAST
> * interface.
> */
> - KERNEL_LOCK();
> + NET_ASSERT_LOCKED();
> TAILQ_FOREACH(ifa, &ifp->if_addrlist, ifa_list) {
> if (ifa->ifa_addr->sa_family != AF_INET)
> continue;
>
> @@ -677,9 +677,8 @@ in_ouraddr(struct mbuf *m, struct ifnet
> match = 1;
> break;
> }
> }
> - KERNEL_UNLOCK();
> }
>
> return (match);
> }
> Index: netinet/ip_output.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_output.c,v
> retrieving revision 1.339
> diff -u -p -u -4 -r1.339 ip_output.c
> --- netinet/ip_output.c 19 Apr 2017 15:21:54 -0000 1.339
> +++ netinet/ip_output.c 28 May 2017 13:58:03 -0000
> @@ -191,13 +191,11 @@ reroute:
> mtu = ifp->if_mtu;
> if (ip->ip_src.s_addr == INADDR_ANY) {
> struct in_ifaddr *ia;
>
> - KERNEL_LOCK();
> IFP_TO_IA(ifp, ia);
> if (ia != NULL)
> ip->ip_src = ia->ia_addr.sin_addr;
> - KERNEL_UNLOCK();
> }
> } else {
> struct in_ifaddr *ia;
>
> @@ -232,13 +230,12 @@ reroute:
> }
>
> #ifdef IPSEC
> if (ipsec_in_use || inp != NULL) {
> - KERNEL_LOCK();
> + KERNEL_ASSERT_LOCKED();
> /* Do we have any pending SAs to apply ? */
> tdb = ip_output_ipsec_lookup(m, hlen, &error, inp,
> ipsecflowinfo);
> - KERNEL_UNLOCK();
> if (error != 0) {
> /* Should silently drop packet */
> if (error == -EINVAL)
> error = 0;
> @@ -306,13 +303,11 @@ reroute:
> */
> if (ip->ip_src.s_addr == INADDR_ANY) {
> struct in_ifaddr *ia;
>
> - KERNEL_LOCK();
> IFP_TO_IA(ifp, ia);
> if (ia != NULL)
> ip->ip_src = ia->ia_addr.sin_addr;
> - KERNEL_UNLOCK();
> }
>
> if ((imo == NULL || imo->imo_loop) &&
> in_hasmulti(&ip->ip_dst, ifp)) {
> @@ -406,12 +401,11 @@ sendit:
> /*
> * Check if the packet needs encapsulation.
> */
> if (tdb != NULL) {
> - KERNEL_LOCK();
> + KERNEL_ASSERT_LOCKED();
> /* Callee frees mbuf */
> error = ip_output_ipsec_send(tdb, m, ifp, ro);
> - KERNEL_UNLOCK();
> goto done;
> }
> #endif /* IPSEC */
>
> Index: netinet6/ip6_input.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/ip6_input.c,v
> retrieving revision 1.190
> diff -u -p -u -4 -r1.190 ip6_input.c
> --- netinet6/ip6_input.c 28 May 2017 09:25:51 -0000 1.190
> +++ netinet6/ip6_input.c 28 May 2017 13:59:21 -0000
> @@ -473,11 +473,10 @@ ip6_input(struct mbuf *m)
> #ifdef IPSEC
> if (ipsec_in_use) {
> int rv;
>
> - KERNEL_LOCK();
> + KERNEL_ASSERT_LOCKED();
> rv = ipsec_forward_check(m, off, AF_INET6);
> - KERNEL_UNLOCK();
> if (rv != 0) {
> ip6stat_inc(ip6s_cantforward);
> goto bad;
> }
