netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] macvlan: Allow some packets to bypass broadcast queue
@ 2023-03-28  2:56 Herbert Xu
  2023-03-28  2:57 ` [PATCH 1/2] macvlan: Skip broadcast queue if multicast with single receiver Herbert Xu
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Herbert Xu @ 2023-03-28  2:56 UTC (permalink / raw)
  To: netdev

This patch series allows some packets to bypass the broadcast
queue on receive.  Currently all multicast packets are queued
on receive and then processed in a work queue.  This is to avoid
an unbounded amount of work occurring in the receive path, as
one broadcast packet could easily translate into 4,000 packets.

However, for multicast packets with just one receiver (possible
for IPv6 ND), this introduces unnecessary latency as the packet
will go to exactly one device.

This series allows such multicast packets to be processed inline.
It also adds a toggle which lets the admin control what threshold
to set between queueing and not queueing.  A follow-up patch for
iproute will be posted.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2] macvlan: Skip broadcast queue if multicast with single receiver
  2023-03-28  2:56 [PATCH 0/2] macvlan: Allow some packets to bypass broadcast queue Herbert Xu
@ 2023-03-28  2:57 ` Herbert Xu
  2023-03-28  2:57 ` [PATCH 2/2] macvlan: Add netlink attribute for broadcast cutoff Herbert Xu
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Herbert Xu @ 2023-03-28  2:57 UTC (permalink / raw)
  To: netdev

As it stands all broadcast and multicast packets are queued and
processed in a work queue.  This is so that we don't overwhelm
the receive softirq path by generating thousands of packets or
more (see commit 412ca1550cbe "macvlan: Move broadcasts into a
work queue").

As such all multicast packets will be delayed, even if they will
be received by a single macvlan device.  As using a workqueue
is not free in terms of latency, we should avoid this where possible.

This patch adds a new filter to determine which addresses should
be delayed and which ones won't.  This is done using a crude
counter of how many times an address has been added to the macvlan
port (ha->synced).  For now if an address has been added more than
once, then it will be considered to be broadcast.  This could be
tuned further by making this threshold configurable.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 drivers/net/macvlan.c |   74 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 46 insertions(+), 28 deletions(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 99a971929c8e..62b4748d3836 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -50,6 +50,7 @@ struct macvlan_port {
 	u32			flags;
 	int			count;
 	struct hlist_head	vlan_source_hash[MACVLAN_HASH_SIZE];
+	DECLARE_BITMAP(bc_filter, MACVLAN_MC_FILTER_SZ);
 	DECLARE_BITMAP(mc_filter, MACVLAN_MC_FILTER_SZ);
 	unsigned char           perm_addr[ETH_ALEN];
 };
@@ -291,6 +292,31 @@ static void macvlan_broadcast(struct sk_buff *skb,
 	}
 }
 
+static void macvlan_multicast_rx(const struct macvlan_port *port,
+				 const struct macvlan_dev *src,
+				 struct sk_buff *skb)
+{
+	if (!src)
+		/* frame comes from an external address */
+		macvlan_broadcast(skb, port, NULL,
+				  MACVLAN_MODE_PRIVATE |
+				  MACVLAN_MODE_VEPA    |
+				  MACVLAN_MODE_PASSTHRU|
+				  MACVLAN_MODE_BRIDGE);
+	else if (src->mode == MACVLAN_MODE_VEPA)
+		/* flood to everyone except source */
+		macvlan_broadcast(skb, port, src->dev,
+				  MACVLAN_MODE_VEPA |
+				  MACVLAN_MODE_BRIDGE);
+	else
+		/*
+		 * flood only to VEPA ports, bridge ports
+		 * already saw the frame on the way out.
+		 */
+		macvlan_broadcast(skb, port, src->dev,
+				  MACVLAN_MODE_VEPA);
+}
+
 static void macvlan_process_broadcast(struct work_struct *w)
 {
 	struct macvlan_port *port = container_of(w, struct macvlan_port,
@@ -308,27 +334,7 @@ static void macvlan_process_broadcast(struct work_struct *w)
 		const struct macvlan_dev *src = MACVLAN_SKB_CB(skb)->src;
 
 		rcu_read_lock();
-
-		if (!src)
-			/* frame comes from an external address */
-			macvlan_broadcast(skb, port, NULL,
-					  MACVLAN_MODE_PRIVATE |
-					  MACVLAN_MODE_VEPA    |
-					  MACVLAN_MODE_PASSTHRU|
-					  MACVLAN_MODE_BRIDGE);
-		else if (src->mode == MACVLAN_MODE_VEPA)
-			/* flood to everyone except source */
-			macvlan_broadcast(skb, port, src->dev,
-					  MACVLAN_MODE_VEPA |
-					  MACVLAN_MODE_BRIDGE);
-		else
-			/*
-			 * flood only to VEPA ports, bridge ports
-			 * already saw the frame on the way out.
-			 */
-			macvlan_broadcast(skb, port, src->dev,
-					  MACVLAN_MODE_VEPA);
-
+		macvlan_multicast_rx(port, src, skb);
 		rcu_read_unlock();
 
 		if (src)
@@ -476,8 +482,10 @@ static rx_handler_result_t macvlan_handle_frame(struct sk_buff **pskb)
 		}
 
 		hash = mc_hash(NULL, eth->h_dest);
-		if (test_bit(hash, port->mc_filter))
+		if (test_bit(hash, port->bc_filter))
 			macvlan_broadcast_enqueue(port, src, skb);
+		else if (test_bit(hash, port->mc_filter))
+			macvlan_multicast_rx(port, src, skb);
 
 		return RX_HANDLER_PASS;
 	}
@@ -780,20 +788,27 @@ static void macvlan_change_rx_flags(struct net_device *dev, int change)
 
 static void macvlan_compute_filter(unsigned long *mc_filter,
 				   struct net_device *dev,
-				   struct macvlan_dev *vlan)
+				   struct macvlan_dev *vlan, int cutoff)
 {
 	if (dev->flags & (IFF_PROMISC | IFF_ALLMULTI)) {
-		bitmap_fill(mc_filter, MACVLAN_MC_FILTER_SZ);
+		if (cutoff >= 0)
+			bitmap_fill(mc_filter, MACVLAN_MC_FILTER_SZ);
+		else
+			bitmap_zero(mc_filter, MACVLAN_MC_FILTER_SZ);
 	} else {
-		struct netdev_hw_addr *ha;
 		DECLARE_BITMAP(filter, MACVLAN_MC_FILTER_SZ);
+		struct netdev_hw_addr *ha;
 
 		bitmap_zero(filter, MACVLAN_MC_FILTER_SZ);
 		netdev_for_each_mc_addr(ha, dev) {
+			if (cutoff >= 0 && ha->synced <= cutoff)
+				continue;
+
 			__set_bit(mc_hash(vlan, ha->addr), filter);
 		}
 
-		__set_bit(mc_hash(vlan, dev->broadcast), filter);
+		if (cutoff >= 0)
+			__set_bit(mc_hash(vlan, dev->broadcast), filter);
 
 		bitmap_copy(mc_filter, filter, MACVLAN_MC_FILTER_SZ);
 	}
@@ -803,7 +818,7 @@ static void macvlan_set_mac_lists(struct net_device *dev)
 {
 	struct macvlan_dev *vlan = netdev_priv(dev);
 
-	macvlan_compute_filter(vlan->mc_filter, dev, vlan);
+	macvlan_compute_filter(vlan->mc_filter, dev, vlan, 0);
 
 	dev_uc_sync(vlan->lowerdev, dev);
 	dev_mc_sync(vlan->lowerdev, dev);
@@ -821,7 +836,10 @@ static void macvlan_set_mac_lists(struct net_device *dev)
 	 * The solution is to maintain a list of broadcast addresses like
 	 * we do for uc/mc, if you care.
 	 */
-	macvlan_compute_filter(vlan->port->mc_filter, vlan->lowerdev, NULL);
+	macvlan_compute_filter(vlan->port->mc_filter, vlan->lowerdev, NULL,
+			       0);
+	macvlan_compute_filter(vlan->port->bc_filter, vlan->lowerdev, NULL,
+			       1);
 }
 
 static int macvlan_change_mtu(struct net_device *dev, int new_mtu)

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] macvlan: Add netlink attribute for broadcast cutoff
  2023-03-28  2:56 [PATCH 0/2] macvlan: Allow some packets to bypass broadcast queue Herbert Xu
  2023-03-28  2:57 ` [PATCH 1/2] macvlan: Skip broadcast queue if multicast with single receiver Herbert Xu
@ 2023-03-28  2:57 ` Herbert Xu
  2023-03-28  3:02 ` [PATCH iproute2-next] macvlan: Add bclim parameter Herbert Xu
  2023-03-29  8:10 ` [PATCH 0/2] macvlan: Allow some packets to bypass broadcast queue patchwork-bot+netdevbpf
  3 siblings, 0 replies; 10+ messages in thread
From: Herbert Xu @ 2023-03-28  2:57 UTC (permalink / raw)
  To: netdev

Make the broadcast cutoff configurable through netlink.  Note
that macvlan is weird because there is no central device for
us to configure (the lowerdev could be anything).  So all the
options are duplicated over what could be thousands of child
devices.

IFLA_MACVLAN_BC_QUEUE_LEN took the approach of taking the maximum
of all child device settings.  This is unnecessary as we could
simply store the option in the port device and take the last
child device that gets updated as the value to use.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 drivers/net/macvlan.c              |   31 +++++++++++++++++++++++++++++--
 include/uapi/linux/if_link.h       |    1 +
 tools/include/uapi/linux/if_link.h |    1 +
 3 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 62b4748d3836..4215106adc40 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -47,6 +47,7 @@ struct macvlan_port {
 	struct sk_buff_head	bc_queue;
 	struct work_struct	bc_work;
 	u32			bc_queue_len_used;
+	int			bc_cutoff;
 	u32			flags;
 	int			count;
 	struct hlist_head	vlan_source_hash[MACVLAN_HASH_SIZE];
@@ -814,6 +815,12 @@ static void macvlan_compute_filter(unsigned long *mc_filter,
 	}
 }
 
+static void macvlan_recompute_bc_filter(struct macvlan_dev *vlan)
+{
+	macvlan_compute_filter(vlan->port->bc_filter, vlan->lowerdev, NULL,
+			       vlan->port->bc_cutoff);
+}
+
 static void macvlan_set_mac_lists(struct net_device *dev)
 {
 	struct macvlan_dev *vlan = netdev_priv(dev);
@@ -838,8 +845,16 @@ static void macvlan_set_mac_lists(struct net_device *dev)
 	 */
 	macvlan_compute_filter(vlan->port->mc_filter, vlan->lowerdev, NULL,
 			       0);
-	macvlan_compute_filter(vlan->port->bc_filter, vlan->lowerdev, NULL,
-			       1);
+	macvlan_recompute_bc_filter(vlan);
+}
+
+static void update_port_bc_cutoff(struct macvlan_dev *vlan, int cutoff)
+{
+	if (vlan->port->bc_cutoff == cutoff)
+		return;
+
+	vlan->port->bc_cutoff = cutoff;
+	macvlan_recompute_bc_filter(vlan);
 }
 
 static int macvlan_change_mtu(struct net_device *dev, int new_mtu)
@@ -1254,6 +1269,7 @@ static int macvlan_port_create(struct net_device *dev)
 		INIT_HLIST_HEAD(&port->vlan_source_hash[i]);
 
 	port->bc_queue_len_used = 0;
+	port->bc_cutoff = 1;
 	skb_queue_head_init(&port->bc_queue);
 	INIT_WORK(&port->bc_work, macvlan_process_broadcast);
 
@@ -1527,6 +1543,10 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
 	if (data && data[IFLA_MACVLAN_BC_QUEUE_LEN])
 		vlan->bc_queue_len_req = nla_get_u32(data[IFLA_MACVLAN_BC_QUEUE_LEN]);
 
+	if (data && data[IFLA_MACVLAN_BC_CUTOFF])
+		update_port_bc_cutoff(
+			vlan, nla_get_s32(data[IFLA_MACVLAN_BC_CUTOFF]));
+
 	err = register_netdevice(dev);
 	if (err < 0)
 		goto destroy_macvlan_port;
@@ -1623,6 +1643,10 @@ static int macvlan_changelink(struct net_device *dev,
 		update_port_bc_queue_len(vlan->port);
 	}
 
+	if (data && data[IFLA_MACVLAN_BC_CUTOFF])
+		update_port_bc_cutoff(
+			vlan, nla_get_s32(data[IFLA_MACVLAN_BC_CUTOFF]));
+
 	if (set_mode)
 		vlan->mode = mode;
 	if (data && data[IFLA_MACVLAN_MACADDR_MODE]) {
@@ -1703,6 +1727,9 @@ static int macvlan_fill_info(struct sk_buff *skb,
 		goto nla_put_failure;
 	if (nla_put_u32(skb, IFLA_MACVLAN_BC_QUEUE_LEN_USED, port->bc_queue_len_used))
 		goto nla_put_failure;
+	if (port->bc_cutoff != 1 &&
+	    nla_put_s32(skb, IFLA_MACVLAN_BC_CUTOFF, port->bc_cutoff))
+		goto nla_put_failure;
 	return 0;
 
 nla_put_failure:
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 57ceb788250f..8d679688efe0 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -635,6 +635,7 @@ enum {
 	IFLA_MACVLAN_MACADDR_COUNT,
 	IFLA_MACVLAN_BC_QUEUE_LEN,
 	IFLA_MACVLAN_BC_QUEUE_LEN_USED,
+	IFLA_MACVLAN_BC_CUTOFF,
 	__IFLA_MACVLAN_MAX,
 };
 
diff --git a/tools/include/uapi/linux/if_link.h b/tools/include/uapi/linux/if_link.h
index 901d98b865a1..39e659c83cfd 100644
--- a/tools/include/uapi/linux/if_link.h
+++ b/tools/include/uapi/linux/if_link.h
@@ -605,6 +605,7 @@ enum {
 	IFLA_MACVLAN_MACADDR_COUNT,
 	IFLA_MACVLAN_BC_QUEUE_LEN,
 	IFLA_MACVLAN_BC_QUEUE_LEN_USED,
+	IFLA_MACVLAN_BC_CUTOFF,
 	__IFLA_MACVLAN_MAX,
 };
 

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH iproute2-next] macvlan: Add bclim parameter
  2023-03-28  2:56 [PATCH 0/2] macvlan: Allow some packets to bypass broadcast queue Herbert Xu
  2023-03-28  2:57 ` [PATCH 1/2] macvlan: Skip broadcast queue if multicast with single receiver Herbert Xu
  2023-03-28  2:57 ` [PATCH 2/2] macvlan: Add netlink attribute for broadcast cutoff Herbert Xu
@ 2023-03-28  3:02 ` Herbert Xu
  2023-03-29 14:51   ` David Ahern
  2023-03-29  8:10 ` [PATCH 0/2] macvlan: Allow some packets to bypass broadcast queue patchwork-bot+netdevbpf
  3 siblings, 1 reply; 10+ messages in thread
From: Herbert Xu @ 2023-03-28  3:02 UTC (permalink / raw)
  To: netdev, David Ahern

This patch adds support for setting the broadcast queueing threshold
on macvlan devices.  This controls which multicast packets will be
processed in a workqueue instead of inline.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index d61bd32d..71ddffc6 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -633,6 +633,7 @@ enum {
 	IFLA_MACVLAN_MACADDR_COUNT,
 	IFLA_MACVLAN_BC_QUEUE_LEN,
 	IFLA_MACVLAN_BC_QUEUE_LEN_USED,
+	IFLA_MACVLAN_BC_CUTOFF,
 	__IFLA_MACVLAN_MAX,
 };
 
diff --git a/ip/iplink_macvlan.c b/ip/iplink_macvlan.c
index 0f13637d..29a9112e 100644
--- a/ip/iplink_macvlan.c
+++ b/ip/iplink_macvlan.c
@@ -26,13 +26,14 @@
 static void print_explain(struct link_util *lu, FILE *f)
 {
 	fprintf(f,
-		"Usage: ... %s mode MODE [flag MODE_FLAG] MODE_OPTS [bcqueuelen BC_QUEUE_LEN]\n"
+		"Usage: ... %s mode MODE [flag MODE_FLAG] MODE_OPTS [bcqueuelen BC_QUEUE_LEN] [bclim BCLIM]\n"
 		"\n"
 		"MODE: private | vepa | bridge | passthru | source\n"
 		"MODE_FLAG: null | nopromisc | nodst\n"
 		"MODE_OPTS: for mode \"source\":\n"
 		"\tmacaddr { { add | del } <macaddr> | set [ <macaddr> [ <macaddr>  ... ] ] | flush }\n"
-		"BC_QUEUE_LEN: Length of the rx queue for broadcast/multicast: [0-4294967295]\n",
+		"BC_QUEUE_LEN: Length of the rx queue for broadcast/multicast: [0-4294967295]\n"
+		"BCLIM: Threshold for broadcast queueing: 32-bit integer\n",
 		lu->id
 	);
 }
@@ -67,6 +68,12 @@ static int bc_queue_len_arg(const char *arg)
 	return -1;
 }
 
+static int bclim_arg(const char *arg)
+{
+	fprintf(stderr, "Error: illegal value for \"bclen\": \"%s\"\n", arg);
+	return -1;
+}
+
 static int macvlan_parse_opt(struct link_util *lu, int argc, char **argv,
 			  struct nlmsghdr *n)
 {
@@ -168,6 +175,15 @@ static int macvlan_parse_opt(struct link_util *lu, int argc, char **argv,
 				return bc_queue_len_arg(*argv);
 			}
 			addattr32(n, 1024, IFLA_MACVLAN_BC_QUEUE_LEN, bc_queue_len);
+		} else if (matches(*argv, "bclim") == 0) {
+			__s32 bclim;
+			NEXT_ARG();
+
+			if (get_s32(&bclim, *argv, 0)) {
+				return bclim_arg(*argv);
+			}
+			addattr_l(n, 1024, IFLA_MACVLAN_BC_CUTOFF,
+				  &bclim, sizeof(bclim));
 		} else if (matches(*argv, "help") == 0) {
 			explain(lu);
 			return -1;
@@ -245,6 +261,12 @@ static void macvlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[]
 		print_luint(PRINT_ANY, "usedbcqueuelen", "usedbcqueuelen %lu ", bc_queue_len);
 	}
 
+	if (tb[IFLA_MACVLAN_BC_CUTOFF] &&
+		RTA_PAYLOAD(tb[IFLA_MACVLAN_BC_CUTOFF]) >= sizeof(__s32)) {
+		__s32 bclim = rta_getattr_s32(tb[IFLA_MACVLAN_BC_CUTOFF]);
+		print_int(PRINT_ANY, "bclim", "bclim %d ", bclim);
+	}
+
 	/* in source mode, there are more options to print */
 
 	if (mode != MACVLAN_MODE_SOURCE)
diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index c8c65657..bec1b78b 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -1479,6 +1479,7 @@ the following additional arguments are supported:
 .BR mode " { " private " | " vepa " | " bridge " | " passthru
 .RB " [ " nopromisc " ] | " source " [ " nodst " ] } "
 .RB " [ " bcqueuelen " { " LENGTH " } ] "
+.RB " [ " bclim " " LIMIT " ] "
 
 .in +8
 .sp
@@ -1537,6 +1538,13 @@ will be the maximum length that any macvlan interface has requested.
 When listing device parameters both the bcqueuelen parameter
 as well as the actual used bcqueuelen are listed to better help
 the user understand the setting.
+
+.BR bclim " " LIMIT
+- Set the threshold for broadcast queueing.
+.BR LIMIT " must be a 32-bit integer."
+Setting this to -1 disables broadcast queueing altogether.  Otherwise
+a multicast address will be queued as broadcast if the number of devices
+using it is greater than the given value.
 .in -8
 
 .TP
@@ -2699,6 +2707,9 @@ Update the broadcast/multicast queue length.
 [
 .BI bcqueuelen "  LENGTH  "
 ]
+[
+.BI bclim " LIMIT "
+]
 
 .in +8
 .BI bcqueuelen " LENGTH "
@@ -2712,6 +2723,13 @@ will be the maximum length that any macvlan interface has requested.
 When listing device parameters both the bcqueuelen parameter
 as well as the actual used bcqueuelen are listed to better help
 the user understand the setting.
+
+.BI bclim " LIMIT "
+- Set the threshold for broadcast queueing.
+.IR LIMIT " must be a 32-bit integer."
+Setting this to -1 disables broadcast queueing altogether.  Otherwise
+a multicast address will be queued as broadcast if the number of devices
+using it is greater than the given value.
 .in -8
 
 .TP
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/2] macvlan: Allow some packets to bypass broadcast queue
  2023-03-28  2:56 [PATCH 0/2] macvlan: Allow some packets to bypass broadcast queue Herbert Xu
                   ` (2 preceding siblings ...)
  2023-03-28  3:02 ` [PATCH iproute2-next] macvlan: Add bclim parameter Herbert Xu
@ 2023-03-29  8:10 ` patchwork-bot+netdevbpf
  2023-03-29  8:45   ` [PATCH] macvlan: Fix mc_filter calculation Herbert Xu
  3 siblings, 1 reply; 10+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-03-29  8:10 UTC (permalink / raw)
  To: Herbert Xu; +Cc: netdev

Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Tue, 28 Mar 2023 10:56:57 +0800 you wrote:
> This patch series allows some packets to bypass the broadcast
> queue on receive.  Currently all multicast packets are queued
> on receive and then processed in a work queue.  This is to avoid
> an unbounded amount of work occurring in the receive path, as
> one broadcast packet could easily translate into 4,000 packets.
> 
> However, for multicast packets with just one receiver (possible
> for IPv6 ND), this introduces unnecessary latency as the packet
> will go to exactly one device.
> 
> [...]

Here is the summary with links:
  - [1/2] macvlan: Skip broadcast queue if multicast with single receiver
    https://git.kernel.org/netdev/net-next/c/d45276e75e90
  - [2/2] macvlan: Add netlink attribute for broadcast cutoff
    https://git.kernel.org/netdev/net-next/c/954d1fa1ac93

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] macvlan: Fix mc_filter calculation
  2023-03-29  8:10 ` [PATCH 0/2] macvlan: Allow some packets to bypass broadcast queue patchwork-bot+netdevbpf
@ 2023-03-29  8:45   ` Herbert Xu
  2023-03-31  8:00     ` patchwork-bot+netdevbpf
  0 siblings, 1 reply; 10+ messages in thread
From: Herbert Xu @ 2023-03-29  8:45 UTC (permalink / raw)
  To: patchwork-bot+netdevbpf; +Cc: netdev

On Wed, Mar 29, 2023 at 08:10:26AM +0000, patchwork-bot+netdevbpf@kernel.org wrote:
> 
> Here is the summary with links:
>   - [1/2] macvlan: Skip broadcast queue if multicast with single receiver
>     https://git.kernel.org/netdev/net-next/c/d45276e75e90
>   - [2/2] macvlan: Add netlink attribute for broadcast cutoff
>     https://git.kernel.org/netdev/net-next/c/954d1fa1ac93

Sorry, I made an error and posted my patches from an earlier
revision so a follow-up fix was missing:

---8<---
The bc_cutoff patch broke the calculation of mc_filter causing
some multicast packets to not make it through to the targeted
device.

Fix this by checking whether vlan is set instead of cutoff >= 0.

Also move the cutoff < 0 logic into macvlan_recompute_bc_filter
so that it doesn't change the mc_filter at all.

Fixes: d45276e75e90 ("macvlan: Skip broadcast queue if multicast with single receiver")
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 drivers/net/macvlan.c |   15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 4215106adc40..4a53debf9d7c 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -792,24 +792,20 @@ static void macvlan_compute_filter(unsigned long *mc_filter,
 				   struct macvlan_dev *vlan, int cutoff)
 {
 	if (dev->flags & (IFF_PROMISC | IFF_ALLMULTI)) {
-		if (cutoff >= 0)
-			bitmap_fill(mc_filter, MACVLAN_MC_FILTER_SZ);
-		else
-			bitmap_zero(mc_filter, MACVLAN_MC_FILTER_SZ);
+		bitmap_fill(mc_filter, MACVLAN_MC_FILTER_SZ);
 	} else {
 		DECLARE_BITMAP(filter, MACVLAN_MC_FILTER_SZ);
 		struct netdev_hw_addr *ha;
 
 		bitmap_zero(filter, MACVLAN_MC_FILTER_SZ);
 		netdev_for_each_mc_addr(ha, dev) {
-			if (cutoff >= 0 && ha->synced <= cutoff)
+			if (!vlan && ha->synced <= cutoff)
 				continue;
 
 			__set_bit(mc_hash(vlan, ha->addr), filter);
 		}
 
-		if (cutoff >= 0)
-			__set_bit(mc_hash(vlan, dev->broadcast), filter);
+		__set_bit(mc_hash(vlan, dev->broadcast), filter);
 
 		bitmap_copy(mc_filter, filter, MACVLAN_MC_FILTER_SZ);
 	}
@@ -817,6 +813,11 @@ static void macvlan_compute_filter(unsigned long *mc_filter,
 
 static void macvlan_recompute_bc_filter(struct macvlan_dev *vlan)
 {
+	if (vlan->port->bc_cutoff < 0) {
+		bitmap_zero(vlan->port->bc_filter, MACVLAN_MC_FILTER_SZ);
+		return;
+	}
+
 	macvlan_compute_filter(vlan->port->bc_filter, vlan->lowerdev, NULL,
 			       vlan->port->bc_cutoff);
 }
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH iproute2-next] macvlan: Add bclim parameter
  2023-03-28  3:02 ` [PATCH iproute2-next] macvlan: Add bclim parameter Herbert Xu
@ 2023-03-29 14:51   ` David Ahern
  2023-03-30  3:07     ` [v2 PATCH " Herbert Xu
  0 siblings, 1 reply; 10+ messages in thread
From: David Ahern @ 2023-03-29 14:51 UTC (permalink / raw)
  To: Herbert Xu, netdev

On 3/27/23 9:02 PM, Herbert Xu wrote:
> @@ -67,6 +68,12 @@ static int bc_queue_len_arg(const char *arg)
>  	return -1;
>  }
>  
> +static int bclim_arg(const char *arg)
> +{
> +	fprintf(stderr, "Error: illegal value for \"bclen\": \"%s\"\n", arg);

s/bclen/bclim/?

> +	return -1;
> +}
> +
>  static int macvlan_parse_opt(struct link_util *lu, int argc, char **argv,
>  			  struct nlmsghdr *n)
>  {
> @@ -168,6 +175,15 @@ static int macvlan_parse_opt(struct link_util *lu, int argc, char **argv,
>  				return bc_queue_len_arg(*argv);
>  			}
>  			addattr32(n, 1024, IFLA_MACVLAN_BC_QUEUE_LEN, bc_queue_len);
> +		} else if (matches(*argv, "bclim") == 0) {

we stopped accepting new uses of `matches`; make this strcmp.




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [v2 PATCH iproute2-next] macvlan: Add bclim parameter
  2023-03-29 14:51   ` David Ahern
@ 2023-03-30  3:07     ` Herbert Xu
  2023-03-30 16:01       ` David Ahern
  0 siblings, 1 reply; 10+ messages in thread
From: Herbert Xu @ 2023-03-30  3:07 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev

v2 fixes a typo on bclen/bclim and switches matches to strcmp.

---8<---
This patch adds support for setting the broadcast queueing threshold
on macvlan devices.  This controls which multicast packets will be
processed in a workqueue instead of inline.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 include/uapi/linux/if_link.h |    1 +
 ip/iplink_macvlan.c          |   26 ++++++++++++++++++++++++--
 man/man8/ip-link.8.in        |   18 ++++++++++++++++++
 3 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index d61bd32d..71ddffc6 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -633,6 +633,7 @@ enum {
 	IFLA_MACVLAN_MACADDR_COUNT,
 	IFLA_MACVLAN_BC_QUEUE_LEN,
 	IFLA_MACVLAN_BC_QUEUE_LEN_USED,
+	IFLA_MACVLAN_BC_CUTOFF,
 	__IFLA_MACVLAN_MAX,
 };
 
diff --git a/ip/iplink_macvlan.c b/ip/iplink_macvlan.c
index 0f13637d..6bdc76d1 100644
--- a/ip/iplink_macvlan.c
+++ b/ip/iplink_macvlan.c
@@ -26,13 +26,14 @@
 static void print_explain(struct link_util *lu, FILE *f)
 {
 	fprintf(f,
-		"Usage: ... %s mode MODE [flag MODE_FLAG] MODE_OPTS [bcqueuelen BC_QUEUE_LEN]\n"
+		"Usage: ... %s mode MODE [flag MODE_FLAG] MODE_OPTS [bcqueuelen BC_QUEUE_LEN] [bclim BCLIM]\n"
 		"\n"
 		"MODE: private | vepa | bridge | passthru | source\n"
 		"MODE_FLAG: null | nopromisc | nodst\n"
 		"MODE_OPTS: for mode \"source\":\n"
 		"\tmacaddr { { add | del } <macaddr> | set [ <macaddr> [ <macaddr>  ... ] ] | flush }\n"
-		"BC_QUEUE_LEN: Length of the rx queue for broadcast/multicast: [0-4294967295]\n",
+		"BC_QUEUE_LEN: Length of the rx queue for broadcast/multicast: [0-4294967295]\n"
+		"BCLIM: Threshold for broadcast queueing: 32-bit integer\n",
 		lu->id
 	);
 }
@@ -67,6 +68,12 @@ static int bc_queue_len_arg(const char *arg)
 	return -1;
 }
 
+static int bclim_arg(const char *arg)
+{
+	fprintf(stderr, "Error: illegal value for \"bclim\": \"%s\"\n", arg);
+	return -1;
+}
+
 static int macvlan_parse_opt(struct link_util *lu, int argc, char **argv,
 			  struct nlmsghdr *n)
 {
@@ -168,6 +175,15 @@ static int macvlan_parse_opt(struct link_util *lu, int argc, char **argv,
 				return bc_queue_len_arg(*argv);
 			}
 			addattr32(n, 1024, IFLA_MACVLAN_BC_QUEUE_LEN, bc_queue_len);
+		} else if (!strcmp(*argv, "bclim")) {
+			__s32 bclim;
+			NEXT_ARG();
+
+			if (get_s32(&bclim, *argv, 0)) {
+				return bclim_arg(*argv);
+			}
+			addattr_l(n, 1024, IFLA_MACVLAN_BC_CUTOFF,
+				  &bclim, sizeof(bclim));
 		} else if (matches(*argv, "help") == 0) {
 			explain(lu);
 			return -1;
@@ -245,6 +261,12 @@ static void macvlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[]
 		print_luint(PRINT_ANY, "usedbcqueuelen", "usedbcqueuelen %lu ", bc_queue_len);
 	}
 
+	if (tb[IFLA_MACVLAN_BC_CUTOFF] &&
+		RTA_PAYLOAD(tb[IFLA_MACVLAN_BC_CUTOFF]) >= sizeof(__s32)) {
+		__s32 bclim = rta_getattr_s32(tb[IFLA_MACVLAN_BC_CUTOFF]);
+		print_int(PRINT_ANY, "bclim", "bclim %d ", bclim);
+	}
+
 	/* in source mode, there are more options to print */
 
 	if (mode != MACVLAN_MODE_SOURCE)
diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index c8c65657..bec1b78b 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -1479,6 +1479,7 @@ the following additional arguments are supported:
 .BR mode " { " private " | " vepa " | " bridge " | " passthru
 .RB " [ " nopromisc " ] | " source " [ " nodst " ] } "
 .RB " [ " bcqueuelen " { " LENGTH " } ] "
+.RB " [ " bclim " " LIMIT " ] "
 
 .in +8
 .sp
@@ -1537,6 +1538,13 @@ will be the maximum length that any macvlan interface has requested.
 When listing device parameters both the bcqueuelen parameter
 as well as the actual used bcqueuelen are listed to better help
 the user understand the setting.
+
+.BR bclim " " LIMIT
+- Set the threshold for broadcast queueing.
+.BR LIMIT " must be a 32-bit integer."
+Setting this to -1 disables broadcast queueing altogether.  Otherwise
+a multicast address will be queued as broadcast if the number of devices
+using it is greater than the given value.
 .in -8
 
 .TP
@@ -2699,6 +2707,9 @@ Update the broadcast/multicast queue length.
 [
 .BI bcqueuelen "  LENGTH  "
 ]
+[
+.BI bclim " LIMIT "
+]
 
 .in +8
 .BI bcqueuelen " LENGTH "
@@ -2712,6 +2723,13 @@ will be the maximum length that any macvlan interface has requested.
 When listing device parameters both the bcqueuelen parameter
 as well as the actual used bcqueuelen are listed to better help
 the user understand the setting.
+
+.BI bclim " LIMIT "
+- Set the threshold for broadcast queueing.
+.IR LIMIT " must be a 32-bit integer."
+Setting this to -1 disables broadcast queueing altogether.  Otherwise
+a multicast address will be queued as broadcast if the number of devices
+using it is greater than the given value.
 .in -8
 
 .TP

-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [v2 PATCH iproute2-next] macvlan: Add bclim parameter
  2023-03-30  3:07     ` [v2 PATCH " Herbert Xu
@ 2023-03-30 16:01       ` David Ahern
  0 siblings, 0 replies; 10+ messages in thread
From: David Ahern @ 2023-03-30 16:01 UTC (permalink / raw)
  To: Herbert Xu; +Cc: netdev

On 3/29/23 9:07 PM, Herbert Xu wrote:
> v2 fixes a typo on bclen/bclim and switches matches to strcmp.
> 
> ---8<---
> This patch adds support for setting the broadcast queueing threshold
> on macvlan devices.  This controls which multicast packets will be
> processed in a workqueue instead of inline.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> ---
> 
>  include/uapi/linux/if_link.h |    1 +
>  ip/iplink_macvlan.c          |   26 ++++++++++++++++++++++++--
>  man/man8/ip-link.8.in        |   18 ++++++++++++++++++
>  3 files changed, 43 insertions(+), 2 deletions(-)
> 

applied to iproute2-next.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] macvlan: Fix mc_filter calculation
  2023-03-29  8:45   ` [PATCH] macvlan: Fix mc_filter calculation Herbert Xu
@ 2023-03-31  8:00     ` patchwork-bot+netdevbpf
  0 siblings, 0 replies; 10+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-03-31  8:00 UTC (permalink / raw)
  To: Herbert Xu; +Cc: patchwork-bot+netdevbpf, netdev

Hello:

This patch was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Wed, 29 Mar 2023 16:45:33 +0800 you wrote:
> On Wed, Mar 29, 2023 at 08:10:26AM +0000, patchwork-bot+netdevbpf@kernel.org wrote:
> >
> > Here is the summary with links:
> >   - [1/2] macvlan: Skip broadcast queue if multicast with single receiver
> >     https://git.kernel.org/netdev/net-next/c/d45276e75e90
> >   - [2/2] macvlan: Add netlink attribute for broadcast cutoff
> >     https://git.kernel.org/netdev/net-next/c/954d1fa1ac93
> 
> [...]

Here is the summary with links:
  - macvlan: Fix mc_filter calculation
    https://git.kernel.org/netdev/net-next/c/ae63ad9b2cc7

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-03-31  8:00 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-28  2:56 [PATCH 0/2] macvlan: Allow some packets to bypass broadcast queue Herbert Xu
2023-03-28  2:57 ` [PATCH 1/2] macvlan: Skip broadcast queue if multicast with single receiver Herbert Xu
2023-03-28  2:57 ` [PATCH 2/2] macvlan: Add netlink attribute for broadcast cutoff Herbert Xu
2023-03-28  3:02 ` [PATCH iproute2-next] macvlan: Add bclim parameter Herbert Xu
2023-03-29 14:51   ` David Ahern
2023-03-30  3:07     ` [v2 PATCH " Herbert Xu
2023-03-30 16:01       ` David Ahern
2023-03-29  8:10 ` [PATCH 0/2] macvlan: Allow some packets to bypass broadcast queue patchwork-bot+netdevbpf
2023-03-29  8:45   ` [PATCH] macvlan: Fix mc_filter calculation Herbert Xu
2023-03-31  8:00     ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).