Updated scope6.c locking patch...

gnn_at_freebsd.org
Date: 10/25/04

  • Next message: Don Bowman: "RE: Underutilisation of CPU --- am I PCI bus bandwidth limited?" 
    Date: Mon, 25 Oct 2004 22:04:32 +0900
To: rwatson@freebsd.org

    Hi Folks,

            Enclosed please find my updated patch for scope6 locking for
            the IPv6 code in FreeBSD-CURRENT. The major change was to
            remove the IFNET list locking because this actually needs to
            be done outside of the IPv6 code and to add address family
            (AF) data locking because that's something we can and should
            do within the scope6 code.

            I have tested this locally in the following way:

    1) Open a shell on the machine (shell1) and do:

    shell1> ifconfig lo1 create
    shell1> while 1
    shell1> scope6config -l 1 -s 2 -o 3 lo1
    shell1> end

    2) Open a second shell on the same machine (shell2) and do:

    shell2> ifconfig lo1 destroy
    shell2> ifconfig lo1 create

    etc.

    The messages in the shell1 window should switch between:

    lo1: 0, 6, 1, 0, 0, 2, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0

    and

    scope6config: ioctl(SIOCSSCOPE6): Device not configured

    and the kernel should not panic ;-)

    Later,
    George

    --- sys/netinet6/scope6.c.orig
    +++ sys/netinet6/scope6.c
    @@ -71,7 +71,6 @@
     scope6_ifattach(ifp)
             struct ifnet *ifp;
     {
    - int s = splnet();
             struct scope6_id *sid;
     
             sid = (struct scope6_id *)malloc(sizeof(*sid), M_IFADDR, M_WAITOK);
    @@ -89,7 +88,6 @@
             sid->s6id_list[IPV6_ADDR_SCOPE_ORGLOCAL] = 1;
     #endif
     
    - splx(s);
             return sid;
     }
     
    @@ -106,12 +104,17 @@
             struct ifnet *ifp;
             struct scope6_id *idlist;
     {
    - int i, s;
    + int i;
             int error = 0;
    - struct scope6_id *sid = SID(ifp);
    + struct scope6_id *sid = NULL;
    +
    + IF_AFDATA_LOCK(ifp);
    + sid = SID(ifp);
     
    - if (!sid) /* paranoid? */
    + if (!sid) { /* paranoid? */
    + IF_AFDATA_UNLOCK(ifp);
                     return (EINVAL);
    + }
     
             /*
              * XXX: We need more consistency checks of the relationship among
    @@ -123,8 +126,6 @@
              * interface addresses, routing table entries, PCB entries...
              */
     
    - s = splnet();
    -
             SCOPE6_LOCK();
             for (i = 0; i < 16; i++) {
                     if (idlist->s6id_list[i] &&
    @@ -135,7 +136,8 @@
                              */
                             if (i == IPV6_ADDR_SCOPE_INTFACELOCAL &&
                                 idlist->s6id_list[i] != ifp->if_index) {
    - splx(s);
    + IF_AFDATA_UNLOCK(ifp);
    + SCOPE6_UNLOCK();
                                     return (EINVAL);
                             }
     
    @@ -147,7 +149,8 @@
                                      * IDs, but we check the consistency for
                                      * safety in later use.
                                      */
    - splx(s);
    + IF_AFDATA_UNLOCK(ifp);
    + SCOPE6_UNLOCK();
                                     return (EINVAL);
                             }
     
    @@ -160,7 +163,7 @@
                     }
             }
             SCOPE6_UNLOCK();
    - splx(s);
    + IF_AFDATA_UNLOCK(ifp);
     
             return (error);
     }
    @@ -170,15 +173,20 @@
             struct ifnet *ifp;
             struct scope6_id *idlist;
     {
    + /* We only need to lock the interface's afdata for SID() to work. */
    + IF_AFDATA_LOCK(ifp);
             struct scope6_id *sid = SID(ifp);
     
    - if (sid == NULL) /* paranoid? */
    + if (sid == NULL) { /* paranoid? */
    + IF_AFDATA_UNLOCK(ifp);
                     return (EINVAL);
    + }
     
             SCOPE6_LOCK();
             *idlist = *sid;
             SCOPE6_UNLOCK();
     
    + IF_AFDATA_UNLOCK(ifp);
             return (0);
     }
     
    @@ -259,7 +267,11 @@
     {
             int scope;
             u_int32_t zoneid = 0;
    - struct scope6_id *sid = SID(ifp);
    + struct scope6_id *sid = NULL;
    +
    + IF_AFDATA_LOCK(ifp);
    +
    + sid = SID(ifp);
     
     #ifdef DIAGNOSTIC
             if (sid == NULL) { /* should not happen */
    @@ -277,10 +289,12 @@
              * interface.
              */
             if (IN6_IS_ADDR_LOOPBACK(addr)) {
    - if (!(ifp->if_flags & IFF_LOOPBACK))
    + if (!(ifp->if_flags & IFF_LOOPBACK)) {
    + IF_AFDATA_UNLOCK(ifp);
                             return (-1);
    - else {
    + } else {
                             *ret_id = 0; /* there's no ambiguity */
    + IF_AFDATA_UNLOCK(ifp);
                             return (0);
                     }
             }
    @@ -315,6 +329,9 @@
             SCOPE6_UNLOCK();
     
             *ret_id = zoneid;
    +
    + IF_AFDATA_UNLOCK(ifp);
    +
             return (0);
     }
     
    @@ -323,7 +340,7 @@
             struct ifnet *ifp; /* note that this might be NULL */
     {
             /*
    - * Currently, this function just set the default "interfaces"
    + * Currently, this function just sets the default "interfaces"
              * and "links" according to the given interface.
              * We might eventually have to separate the notion of "link" from
              * "interface" and provide a user interface to set the default.
    _______________________________________________
    freebsd-net@freebsd.org mailing list
    http://lists.freebsd.org/mailman/listinfo/freebsd-net
    To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org"

  • Next message: Don Bowman: "RE: Underutilisation of CPU --- am I PCI bus bandwidth limited?"