From f6e0c471691e0005e450ec968a9fa3b5346a3c45 Mon Sep 17 00:00:00 2001 From: Lutz Donnerhacke Date: Tue, 9 Feb 2021 14:28:48 +0100 Subject: [PATCH] netgraph/ng_bridge: move MACs via control message Use the new control message to move ethernet addresses from a link to a new link in ng_bridge(4). Send this message instead of doing the work directly requires to move the loop detection into the control message processing. This will delay the loop detection by a few frames. This decouples the read-only activity from the modification under a more strict writer lock. Reviewed by: manpages (gbe) MFC after: 2 weeks Differential Revision: https://reviews.freebsd.org/D28559 --- share/man/man4/ng_bridge.4 | 13 ++++- sys/netgraph/ng_bridge.c | 112 ++++++++++++++++--------------------- 2 files changed, 59 insertions(+), 66 deletions(-) diff --git a/share/man/man4/ng_bridge.4 b/share/man/man4/ng_bridge.4 index 216dea3c392a..f83bacecce40 100644 --- a/share/man/man4/ng_bridge.4 +++ b/share/man/man4/ng_bridge.4 @@ -34,7 +34,7 @@ .\" .\" $FreeBSD$ .\" -.Dd February 17, 2021 +.Dd May 13, 2021 .Dt NG_BRIDGE 4 .Os .Sh NAME @@ -221,12 +221,19 @@ as an argument. It assigns the MAC .Va addr to the -.Va hook , -which must not be assigned yet. +.Va hook . If the .Va hook is the empty string, the incoming hook of the control message is used as fallback. +.Pp +If necessary, the MAC is removed from the currently assigned hook and +moved to the new one. +If the MAC is moved faster than +.Va minStableAge , +the hook is considered as a loop and will block traffic for +.Va loopTimeout +seconds. .Bd -literal -offset 0n struct ng_bridge_move_host { u_char addr[ETHER_ADDR_LEN]; /* ethernet address */ diff --git a/sys/netgraph/ng_bridge.c b/sys/netgraph/ng_bridge.c index 915a18550cba..ecb00f9ba29f 100644 --- a/sys/netgraph/ng_bridge.c +++ b/sys/netgraph/ng_bridge.c @@ -103,7 +103,7 @@ struct ng_bridge_link_kernel_stats { counter_u64_t xmitMulticasts; /* multicast pkts xmit'd on link */ counter_u64_t xmitBroadcasts; /* broadcast pkts xmit'd on link */ counter_u64_t loopDrops; /* pkts dropped due to loopback */ - counter_u64_t loopDetects; /* number of loop detections */ + u_int64_t loopDetects; /* number of loop detections */ counter_u64_t memoryFailures; /* times couldn't get mem or mbuf */ }; @@ -420,7 +420,6 @@ ng_bridge_newhook(node_p node, hook_p hook, const char *name) link->stats.xmitMulticasts = counter_u64_alloc(M_WAITOK); link->stats.xmitBroadcasts = counter_u64_alloc(M_WAITOK); link->stats.loopDrops = counter_u64_alloc(M_WAITOK); - link->stats.loopDetects = counter_u64_alloc(M_WAITOK); link->stats.memoryFailures = counter_u64_alloc(M_WAITOK); link->hook = hook; @@ -456,7 +455,7 @@ static void ng_bridge_clear_link_stats(struct ng_bridge_link_kernel_stats * p) counter_u64_zero(p->xmitMulticasts); counter_u64_zero(p->xmitBroadcasts); counter_u64_zero(p->loopDrops); - counter_u64_zero(p->loopDetects); + p->loopDetects = 0; counter_u64_zero(p->memoryFailures); }; @@ -573,7 +572,7 @@ ng_bridge_rcvmsg(node_p node, item_p item, hook_p lasthook) FETCH(xmitMulticasts); FETCH(xmitBroadcasts); FETCH(loopDrops); - FETCH(loopDetects); + rs->loopDetects = link->stats.loopDetects; FETCH(memoryFailures); #undef FETCH } @@ -619,7 +618,6 @@ ng_bridge_rcvmsg(node_p node, item_p item, hook_p lasthook) { struct ng_bridge_move_host *mh; hook_p hook; - struct ng_bridge_host *host; if (msg->header.arglen < sizeof(*mh)) { error = EINVAL; @@ -633,11 +631,6 @@ ng_bridge_rcvmsg(node_p node, item_p item, hook_p lasthook) error = ENOENT; break; } - host = ng_bridge_get(priv, mh->addr); - if (host != NULL) { - error = EADDRINUSE; - break; - } error = ng_bridge_put(priv, mh->addr, NG_HOOK_PRIVATE(hook)); break; } @@ -784,7 +777,7 @@ ng_bridge_rcvdata(hook_p hook, item_p item) counter_u64_add(ctx.incoming->stats.loopDrops, 1); NG_FREE_ITEM(item); NG_FREE_M(ctx.m); - return (ELOOP); /* XXX is this an appropriate error? */ + return (ELOOP); } /* Update stats */ @@ -799,56 +792,15 @@ ng_bridge_rcvdata(hook_p hook, item_p item) } /* Look up packet's source Ethernet address in hashtable */ - if ((host = ng_bridge_get(priv, eh->ether_shost)) != NULL) { + if ((host = ng_bridge_get(priv, eh->ether_shost)) != NULL) /* Update time since last heard from this host. * This is safe without locking, because it's * the only operation during shared access. */ host->staleness = 0; - /* Did host jump to a different link? */ - if (host->link != ctx.incoming) { - /* - * If the host's old link was recently established - * on the old link and it's already jumped to a new - * link, declare a loopback condition. - */ - if (host->age < priv->conf.minStableAge) { - /* Log the problem */ - if (priv->conf.debugLevel >= 2) { - struct ifnet *ifp = ctx.m->m_pkthdr.rcvif; - char suffix[32]; - - if (ifp != NULL) - snprintf(suffix, sizeof(suffix), - " (%s)", ifp->if_xname); - else - *suffix = '\0'; - log(LOG_WARNING, "ng_bridge: %s:" - " loopback detected on %s%s\n", - ng_bridge_nodename(node), - NG_HOOK_NAME(hook), suffix); - } - - /* Mark link as linka non grata */ - ctx.incoming->loopCount = priv->conf.loopTimeout; - counter_u64_add(ctx.incoming->stats.loopDetects, 1); - - /* Forget all hosts on this link */ - ng_bridge_remove_hosts(priv, ctx.incoming); - - /* Drop packet */ - counter_u64_add(ctx.incoming->stats.loopDrops, 1); - NG_FREE_ITEM(item); - NG_FREE_M(ctx.m); - return (ELOOP); /* XXX appropriate? */ - } - - /* Move host over to new link */ - host->link = ctx.incoming; - host->age = 0; - } - } else if (ctx.incoming->learnMac) { + if ((host == NULL && ctx.incoming->learnMac) || + (host != NULL && host->link != ctx.incoming)) { struct ng_mesg *msg; struct ng_bridge_move_host *mh; int error = 0; @@ -871,6 +823,16 @@ ng_bridge_rcvdata(hook_p hook, item_p item) counter_u64_add(ctx.incoming->stats.memoryFailures, 1); } + if (host != NULL && host->link != ctx.incoming) { + if (host->age < priv->conf.minStableAge) { + /* Drop packet on instable links */ + counter_u64_add(ctx.incoming->stats.loopDrops, 1); + NG_FREE_ITEM(item); + NG_FREE_M(ctx.m); + return (ELOOP); + } + } + /* Run packet through ipfw processing, if enabled */ #if 0 if (priv->conf.ipfw[linkNum] && V_fw_enable && V_ip_fw_chk_ptr != NULL) { @@ -970,7 +932,6 @@ ng_bridge_disconnect(hook_p hook) counter_u64_free(link->stats.xmitMulticasts); counter_u64_free(link->stats.xmitBroadcasts); counter_u64_free(link->stats.loopDrops); - counter_u64_free(link->stats.loopDetects); counter_u64_free(link->stats.memoryFailures); free(link, M_NETGRAPH_BRIDGE); priv->numLinks--; @@ -1012,8 +973,8 @@ ng_bridge_get(priv_cp priv, const u_char *addr) } /* - * Add a new host entry to the table. This assumes the host doesn't - * already exist in the table. Returns 0 on success. + * Add a host entry to the table. If it already exists, move it + * to the new link. Returns 0 on success. */ static int ng_bridge_put(priv_p priv, const u_char *addr, link_p link) @@ -1021,11 +982,36 @@ ng_bridge_put(priv_p priv, const u_char *addr, link_p link) const int bucket = HASH(addr, priv->hashMask); struct ng_bridge_host *host; -#ifdef INVARIANTS - /* Assert that entry does not already exist in hashtable */ - KASSERT(ng_bridge_get(priv, addr) == NULL, - ("%s: entry %6D exists in table", __func__, addr, ":")); -#endif + if ((host = ng_bridge_get(priv, addr)) != NULL) { + /* Host already on the correct link? */ + if (host->link == link) + return 0; + + /* Move old host over to new link */ + if (host->age >= priv->conf.minStableAge) { + host->link = link; + host->age = 0; + return (0); + } + /* + * If the host was recently moved to the old link and + * it's now jumping to a new link, declare a loopback + * condition. + */ + if (priv->conf.debugLevel >= 2) + log(LOG_WARNING, "ng_bridge: %s:" + " loopback detected on %s\n", + ng_bridge_nodename(priv->node), + NG_HOOK_NAME(link->hook)); + + /* Mark link as linka non grata */ + link->loopCount = priv->conf.loopTimeout; + link->stats.loopDetects++; + + /* Forget all hosts on this link */ + ng_bridge_remove_hosts(priv, link); + return (ELOOP); + } /* Allocate and initialize new hashtable entry */ host = malloc(sizeof(*host), M_NETGRAPH_BRIDGE, M_NOWAIT);