[src/trunk]: src/sys/net/lagg Fix to call lacp_linkstate with IFNET_LOCK held

[yamaguchi <yamaguchi%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/dfa29ee42ab5
branches:  trunk
changeset: 359594:dfa29ee42ab5
user:      yamaguchi <yamaguchi%NetBSD.org@localhost>
date:      Wed Jan 12 08:23:53 2022 +0000

description:
Fix to call lacp_linkstate with IFNET_LOCK held

Network stack calls lacp_linkstate through lagg_port_ioctl when
doing "ifconfig up" or "ifconfig down" to an interface that is
a member of lagg(4). And IFNET_LOCK in the member interface
is held while the ioctl.
Therefore, lacp_linkstate is renamed to
lacp_linkstate_ifnet_locked, and always called with IFNET_LOCK
held. It avoids locking agains myself.

diffstat:

 sys/net/lagg/if_lagg.c      |  13 ++++++++++---
 sys/net/lagg/if_lagg_lacp.c |  28 +++++++++++++++++++---------
 sys/net/lagg/if_laggproto.h |   6 +++---
 3 files changed, 32 insertions(+), 15 deletions(-)

diffs (179 lines):

diff -r 2dfa450efd77 -r dfa29ee42ab5 sys/net/lagg/if_lagg.c
--- a/sys/net/lagg/if_lagg.c    Wed Jan 12 01:21:36 2022 +0000
+++ b/sys/net/lagg/if_lagg.c    Wed Jan 12 08:23:53 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_lagg.c,v 1.29 2022/01/12 01:21:36 riastradh Exp $   */
+/*     $NetBSD: if_lagg.c,v 1.30 2022/01/12 08:23:53 yamaguchi Exp $   */
 
 /*
  * Copyright (c) 2005, 2006 Reyk Floeter <reyk%openbsd.org@localhost>
@@ -20,7 +20,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_lagg.c,v 1.29 2022/01/12 01:21:36 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_lagg.c,v 1.30 2022/01/12 08:23:53 yamaguchi Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -102,7 +102,7 @@
                .pr_stopport = lacp_stopport,
                .pr_protostat = lacp_protostat,
                .pr_portstat = lacp_portstat,
-               .pr_linkstate = lacp_linkstate,
+               .pr_linkstate = lacp_linkstate_ifnet_locked,
                .pr_ioctl = lacp_ioctl,
        },
        [LAGG_PROTO_FAILOVER] = {
@@ -1490,6 +1490,8 @@
        lagg_proto pr;
        int bound;
 
+       KASSERT(IFNET_LOCKED(lp->lp_ifp));
+
        bound = curlwp_bind();
        var = lagg_variant_getref(sc, &psref);
 
@@ -2692,6 +2694,8 @@
                goto fallback;
        }
 
+       KASSERT(IFNET_LOCKED(lp->lp_ifp));
+
        switch (cmd) {
        case SIOCSIFCAP:
        case SIOCSIFMTU:
@@ -2817,7 +2821,10 @@
        }
        pserialize_read_exit(s);
 
+       IFNET_LOCK(lp->lp_ifp);
        lagg_proto_linkstate(lp->lp_softc, lp);
+       IFNET_UNLOCK(lp->lp_ifp);
+
        lagg_port_putref(lp, &psref);
        curlwp_bindx(bound);
 }
diff -r 2dfa450efd77 -r dfa29ee42ab5 sys/net/lagg/if_lagg_lacp.c
--- a/sys/net/lagg/if_lagg_lacp.c       Wed Jan 12 01:21:36 2022 +0000
+++ b/sys/net/lagg/if_lagg_lacp.c       Wed Jan 12 08:23:53 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_lagg_lacp.c,v 1.11 2022/01/06 12:09:42 riastradh Exp $      */
+/*     $NetBSD: if_lagg_lacp.c,v 1.12 2022/01/12 08:23:53 yamaguchi Exp $      */
 
 /*-
  * SPDX-License-Identifier: BSD-2-Clause-NetBSD
@@ -31,7 +31,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_lagg_lacp.c,v 1.11 2022/01/06 12:09:42 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_lagg_lacp.c,v 1.12 2022/01/12 08:23:53 yamaguchi Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_lagg.h"
@@ -212,8 +212,6 @@
 #define LACP_LOCK(_sc)         mutex_enter(&(_sc)->lsc_lock)
 #define LACP_UNLOCK(_sc)       mutex_exit(&(_sc)->lsc_lock)
 #define LACP_LOCKED(_sc)       mutex_owned(&(_sc)->lsc_lock)
-#define LACP_LINKSTATE(_sc, _lp)                       \
-       lacp_linkstate((struct lagg_proto_softc *)(_sc), (_lp))
 #define LACP_TIMER_ARM(_lacpp, _timer, _val)           \
        (_lacpp)->lp_timer[(_timer)] = (_val)
 #define LACP_TIMER_DISARM(_lacpp, _timer)              \
@@ -242,6 +240,7 @@
 
 static void    lacp_tick(void *);
 static void    lacp_tick_work(struct lagg_work *, void *);
+static void    lacp_linkstate(struct lagg_proto_softc *, struct lagg_port *);
 static uint32_t        lacp_ifmedia2lacpmedia(u_int);
 static void    lacp_port_disable(struct lacp_softc *, struct lacp_port *);
 static void    lacp_port_enable(struct lacp_softc *, struct lacp_port *);
@@ -673,7 +672,7 @@
 
        lp->lp_proto_ctx = (void *)lacpp;
        lp->lp_prio = ntohs(lacpp->lp_actor.lpi_portprio);
-       LACP_LINKSTATE(lsc, lp);
+       lacp_linkstate(xlsc, lp);
 
        return 0;
 }
@@ -689,7 +688,7 @@
        prio = (uint16_t)MIN(lp->lp_prio, UINT16_MAX);
        lacpp->lp_actor.lpi_portprio = htons(prio);
 
-       LACP_LINKSTATE((struct lacp_softc *)xlsc, lp);
+       lacp_linkstate(xlsc, lp);
 }
 
 void
@@ -821,7 +820,7 @@
 }
 
 void
-lacp_linkstate(struct lagg_proto_softc *xlsc, struct lagg_port *lp)
+lacp_linkstate_ifnet_locked(struct lagg_proto_softc *xlsc, struct lagg_port *lp)
 {
        struct lacp_softc *lsc;
        struct lacp_port *lacpp;
@@ -831,6 +830,8 @@
        uint32_t media, old_media;
        int error;
 
+       KASSERT(IFNET_LOCKED(lp->lp_ifp));
+
        lsc = (struct lacp_softc *)xlsc;
 
        ifp_port = lp->lp_ifp;
@@ -839,9 +840,7 @@
 
        memset(&ifmr, 0, sizeof(ifmr));
        ifmr.ifm_count = 0;
-       IFNET_LOCK(ifp_port);
        error = if_ioctl(ifp_port, SIOCGIFMEDIA, (void *)&ifmr);
-       IFNET_UNLOCK(ifp_port);
        if (error == 0) {
                media = lacp_ifmedia2lacpmedia(ifmr.ifm_active);
        } else if (error != ENOTTY){
@@ -2644,3 +2643,14 @@
 
        return rv;
 }
+
+static void
+lacp_linkstate(struct lagg_proto_softc *xlsc, struct lagg_port *lp)
+{
+
+       IFNET_ASSERT_UNLOCKED(lp->lp_ifp);
+
+       IFNET_LOCK(lp->lp_ifp);
+       lacp_linkstate_ifnet_locked(xlsc, lp);
+       IFNET_UNLOCK(lp->lp_ifp);
+}
diff -r 2dfa450efd77 -r dfa29ee42ab5 sys/net/lagg/if_laggproto.h
--- a/sys/net/lagg/if_laggproto.h       Wed Jan 12 01:21:36 2022 +0000
+++ b/sys/net/lagg/if_laggproto.h       Wed Jan 12 08:23:53 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_laggproto.h,v 1.9 2021/10/19 07:52:33 yamaguchi Exp $       */
+/*     $NetBSD: if_laggproto.h,v 1.10 2022/01/12 08:23:53 yamaguchi Exp $      */
 
 /*
  * Copyright (c) 2021 Internet Initiative Japan Inc.
@@ -199,8 +199,8 @@
  * - IFNET_LOCK(sc_if) -> LAGG_LOCK -> ETHER_LOCK(sc_if) -> a lock in
  *   struct lagg_port_softc
  * - IFNET_LOCK(sc_if) -> LAGG_LOCK -> IFNET_LOCK(lp_ifp)
+ * - IFNET_LOCK(lp_ifp) -> a lock in struct lagg_proto_softc
  * - Currently, there is no combination of following locks
- *   - IFNET_LOCK(lp_ifp) and a lock in struct lagg_proto_softc
  *   - IFNET_LOCK(lp_ifp) and ETHER_LOCK(sc_if)
  */
 #define LAGG_LOCK(_sc)         mutex_enter(&(_sc)->sc_lock)
@@ -320,6 +320,6 @@
                    struct laggreqproto *);
 void           lacp_portstat(struct lagg_proto_softc *, struct lagg_port *,
                    struct laggreqport *);
-void           lacp_linkstate(struct lagg_proto_softc *, struct lagg_port *);
+void           lacp_linkstate_ifnet_locked(struct lagg_proto_softc *, struct lagg_port *);
 int            lacp_ioctl(struct lagg_proto_softc *, struct laggreqproto *);
 #endif