git: a56e5ad69030 - main - netgraph/ng_bridge: Handle send errors during loop handling

[Lutz Donnerhacke]

The branch main has been updated by donner:

URL: https://cgit.FreeBSD.org/src/commit/?id=a56e5ad6903037861457da754574b4903dcb0e7e

commit a56e5ad6903037861457da754574b4903dcb0e7e
Author:     Lutz Donnerhacke <donner at FreeBSD.org>
AuthorDate: 2021-04-27 07:49:50 +0000
Commit:     Lutz Donnerhacke <donner at FreeBSD.org>
CommitDate: 2021-05-13 19:49:20 +0000

    netgraph/ng_bridge: Handle send errors during loop handling
    
    If sending out a packet fails during the loop over all links, the
    allocated memory is leaked and not all links receive a copy.  This
    patch fixes those problems, clarifies a premature abort of the loop,
    and fixes a minory style(9) bug.
    
    PR:             255430
    Submitted by:   Dancho Penev
    Tested by:      Dancho Penev
    MFC after:      1 week
    Differential Revision: https://reviews.freebsd.org/D30008
---
 sys/netgraph/ng_bridge.c | 55 ++++++++++++++++++++++++++----------------------
 1 file changed, 30 insertions(+), 25 deletions(-)

diff --git a/sys/netgraph/ng_bridge.c b/sys/netgraph/ng_bridge.c
index 820c93515490..f77012d42d1c 100644
--- a/sys/netgraph/ng_bridge.c
+++ b/sys/netgraph/ng_bridge.c
@@ -663,22 +663,28 @@ ng_bridge_send_data(link_cp dst, int manycast, struct mbuf *m, item_p item) {
 	else
 		NG_SEND_DATA_ONLY(error, dst->hook, m);
 
-	if (error == 0) {
-		counter_u64_add(dst->stats.xmitPackets, 1);
-		counter_u64_add(dst->stats.xmitOctets, len);
-		switch (manycast) {
-		default:		       /* unknown unicast */
-			break;
-		case 1:			       /* multicast */
-			counter_u64_add(dst->stats.xmitMulticasts, 1);
-			break;
-		case 2:			       /* broadcast */
-			counter_u64_add(dst->stats.xmitBroadcasts, 1);
-			break;
-		}
+	if (error) {
+		/* The packet is still ours */
+		if (item != NULL)
+			NG_FREE_ITEM(item);
+		if (m != NULL)
+			NG_FREE_M(m);
+		return (error);
 	}
 
-	return (error);
+	counter_u64_add(dst->stats.xmitPackets, 1);
+	counter_u64_add(dst->stats.xmitOctets, len);
+	switch (manycast) {
+	default:		       /* unknown unicast */
+		break;
+	case 1:			       /* multicast */
+		counter_u64_add(dst->stats.xmitMulticasts, 1);
+		break;
+	case 2:			       /* broadcast */
+		counter_u64_add(dst->stats.xmitBroadcasts, 1);
+		break;
+	}
+	return (0);
 }
 
 /*
@@ -716,16 +722,16 @@ ng_bridge_send_ctx(hook_p dst, void *arg)
 	 * It's usable link but not the reserved (first) one.
 	 * Copy mbuf info for sending.
 	 */
-	m2 = m_dup(ctx->m, M_NOWAIT);	/* XXX m_copypacket() */
+	m2 = m_dup(ctx->m, M_NOWAIT);
 	if (m2 == NULL) {
 		counter_u64_add(ctx->incoming->stats.memoryFailures, 1);
 		ctx->error = ENOBUFS;
-		return (0);	       /* abort loop */
+		return (0);	       /* abort loop, do not try again and again */
 	}
 
 	/* Send packet */
 	error = ng_bridge_send_data(destLink, ctx->manycast, m2, NULL);
-	if(error)
+	if (error)
 	  ctx->error = error;
 	return (1);
 }
@@ -859,18 +865,17 @@ ng_bridge_rcvdata(hook_p hook, item_p item)
 	/* Distribute unknown, multicast, broadcast pkts to all other links */
 	NG_NODE_FOREACH_HOOK(node, ng_bridge_send_ctx, &ctx, ret);
 
-	/* If we never saw a good link, leave. */
-	if (ctx.foundFirst == NULL || ctx.error != 0) {
+	/* Finally send out on the first link found */
+	if (ctx.foundFirst != NULL) {
+		int error = ng_bridge_send_data(ctx.foundFirst, ctx.manycast, ctx.m, item);
+		if (error)
+			ctx.error = error;
+	} else {		       /* nothing to send at all */
 		NG_FREE_ITEM(item);
 		NG_FREE_M(ctx.m);
-		return (ctx.error);
 	}
 
-	/*
-	 * If we've sent all the others, send the original
-	 * on the first link we found.
-	 */
-	return (ng_bridge_send_data(ctx.foundFirst, ctx.manycast, ctx.m, item));
+	return (ctx.error);
 }
 
 /*