Push kernel lock within rtable_add(9) and rework it to return 0 in the

case when requested table is already exists.

Except initialization time, route_output() and if_createrdomain() are the
only paths where we call rtable_add(9). We check requested table existence
by rtable_exists(9) and it's not the error condition if the table exists.
Otherwise we are trying to create requested table by rtable_add(9). Those
paths are kernel locked so concurrent thread can't create requested table
just after rtable_exists(9) check. Also rtable_add(9) has internal
rtable_exists(9) check and in this case the table existence assumed as
EEXIST error. This error path is never reached.

We are going to unlock PF_ROUTE sockets. This means route_output() will
not be serialized with if_createrdomain() and concurrent thread could
create requested table. Table existence check and creation should be
serialized and it makes sense to do this within rtable_add(9). This time
kernel lock is used for this so it pushed down to rtable_add(9). The
internal rtable_exists(9) check was modified and table existence is not
error now.

Since the external rtable_exists(9) check is useless it was removed from
if_createrdomain(). It still exists in route_output() path because the
logic is more complicated here.

ok mpi@

diff --git a/share/man/man9/rtable_add.9 b/share/man/man9/rtable_add.9
index f82b22a..05d91cc 100644 (file)
--- a/share/man/man9/rtable_add.9
+++ b/share/man/man9/rtable_add.9
@@ -1,4 +1,4 @@
-.\"     $OpenBSD: rtable_add.9,v 1.8 2016/11/14 10:32:46 mpi Exp $
+.\"     $OpenBSD: rtable_add.9,v 1.9 2021/03/26 22:41:06 mvs Exp $
 .\"
 .\" Copyright (c) 2011 Bret S. Lambert <blambert@openbsd.org>
 .\" All rights reserved.
@@ -15,7 +15,7 @@
 .\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 .\"
-.Dd $Mdocdate: November 14 2016 $
+.Dd $Mdocdate: March 26 2021 $
 .Dt RTABLE_ADD 9
 .Os
 .Sh NAME
@@ -83,10 +83,6 @@ can be called during autoconf, from process context, or from interrupt context.
 may fail with:
 .Pp
 .Bl -tag -width Er -compact
-.It Bq Er EEXIST
-A routing table with ID of
-.Fa id
-already exists.
 .It Bq Er ENOMEM
 Memory could not be allocated to extend the list of routing domains.
 .El

diff --git a/sys/net/if.c b/sys/net/if.c
index e5c5e65..c11710b 100644 (file)
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if.c,v 1.639 2021/03/20 17:08:57 florian Exp $        */
+/*     $OpenBSD: if.c,v 1.640 2021/03/26 22:41:06 mvs Exp $    */
 /*     $NetBSD: if.c,v 1.35 1996/05/07 05:26:04 thorpej Exp $  */
 
 /*
@@ -1780,7 +1780,7 @@ if_createrdomain(int rdomain, struct ifnet *ifp)
        char loifname[IFNAMSIZ];
        unsigned int unit = rdomain;
 
-       if (!rtable_exists(rdomain) && (error = rtable_add(rdomain)) != 0)
+       if ((error = rtable_add(rdomain)) != 0)
                return (error);
        if (!rtable_empty(rdomain))
                return (EEXIST);

diff --git a/sys/net/rtable.c b/sys/net/rtable.c
index d88544e..4d00cd1 100644 (file)
--- a/sys/net/rtable.c
+++ b/sys/net/rtable.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: rtable.c,v 1.73 2021/03/10 10:21:48 jsg Exp $ */
+/*     $OpenBSD: rtable.c,v 1.74 2021/03/26 22:41:06 mvs Exp $ */
 
 /*
  * Copyright (c) 2014-2016 Martin Pieuchot
@@ -195,15 +195,15 @@ rtable_add(unsigned int id)
        struct dommp    *dmm;
        sa_family_t      af;
        unsigned int     off, alen;
-       int              i;
-
-       KERNEL_ASSERT_LOCKED();
+       int              i, error = 0;
 
        if (id > RT_TABLEID_MAX)
                return (EINVAL);
 
+       KERNEL_LOCK();
+
        if (rtable_exists(id))
-               return (EEXIST);
+               goto out;
 
        for (i = 0; (dp = domains[i]) != NULL; i++) {
                if (dp->dom_rtoffset == 0)
@@ -217,8 +217,10 @@ rtable_add(unsigned int id)
                        rtmap_grow(id + 1, af);
 
                tbl = rtable_alloc(id, alen, off);
-               if (tbl == NULL)
-                       return (ENOMEM);
+               if (tbl == NULL) {
+                       error = ENOMEM;
+                       goto out;
+               }
 
                map = srp_get_locked(&afmap[af2idx[af]]);
                map->tbl[id] = tbl;
@@ -233,8 +235,10 @@ rtable_add(unsigned int id)
        /* Use main rtable/rdomain by default. */
        dmm = srp_get_locked(&afmap[0]);
        dmm->value[id] = 0;
+out:
+       KERNEL_UNLOCK();
 
-       return (0);
+       return (error);
 }
 
 void *