From 51772eb6ca0378a11bd1f4965abf57cb7fb874bc Mon Sep 17 00:00:00 2001 From: mvs Date: Fri, 26 Mar 2021 22:41:06 +0000 Subject: [PATCH] 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@ --- share/man/man9/rtable_add.9 | 8 ++------ sys/net/if.c | 4 ++-- sys/net/rtable.c | 20 ++++++++++++-------- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/share/man/man9/rtable_add.9 b/share/man/man9/rtable_add.9 index f82b22a3491..05d91ccb44d 100644 --- 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 .\" 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 e5c5e65129d..c11710b75fc 100644 --- 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 d88544e2a22..4d00cd101f7 100644 --- 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 * -- 2.20.1