Re: [PATCH net-next v2 0/3] net: Fix protodown with macvlan

[Ido Schimmel <idosch@nvidia.com>]

On Tue, May 05, 2026 at 11:16:52AM +0300, Ido Schimmel wrote:
> When protodown is enabled on a macvlan, two bugs cause the macvlan to
> incorrectly report an UP operational state:
> 
> 1. Toggling the lower device's carrier while protodown is enabled on the
> macvlan causes the macvlan to inherit the UP operational state,
> effectively bypassing the protodown mechanism.
> 
> 2. Toggling protodown on and then off on the macvlan while the lower
> device has no carrier causes the macvlan to report UP instead of
> LOWERLAYERDOWN, since netif_change_proto_down() unconditionally turns
> the carrier on.
> 
> Patch #1 solves the first problem by making
> netif_stacked_transfer_operstate() return early when protodown is on.
> 
> Patch #2 solves the second problem by calling
> netif_stacked_transfer_operstate() instead of netif_carrier_on() when
> protodown is disabled on a net device that has a linked net device.
> 
> Patch #3 adds a selftest covering both bugs and the basic protodown
> functionality.
> 
> Targeting at net-next since these are not regressions (i.e., never
> worked).
> 
> Note that while these changes are in the core, they should only affect
> macvlan as protodown is only supported by macvlan and vxlan and only the
> former has a linked net device.
> 
> v2:
> - Move protodown handling away from drivers to the core (Jakub).
> - Add a new test case for vxlan.
> v1: https://lore.kernel.org/netdev/20260429124624.835335-1-idosch@nvidia.com/

I thought about this again and since protodown is only about carrier, I
think it's best to avoid netif_stacked_transfer_operstate(). Instead,
make netif_carrier_on() a NOP when protodown is turned on and when
protodown is turned off, only turn on the carrier if the linked net
device (assuming it exists) also has a carrier.

IOW, I will make these changes for v3:

diff --git a/net/core/dev.c b/net/core/dev.c
index 46f8a2efd982..0c272f6e9aaa 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10169,10 +10169,8 @@ int netif_change_proto_down(struct net_device *dev, bool proto_down)
 	WRITE_ONCE(dev->proto_down, proto_down);
 	if (proto_down)
 		netif_carrier_off(dev);
-	else if (dev == iflink_dev)
+	else if (dev == iflink_dev || netif_carrier_ok(iflink_dev))
 		netif_carrier_on(dev);
-	else
-		netif_stacked_transfer_operstate(iflink_dev, dev);
 	return 0;
 }
 
@@ -11134,9 +11132,6 @@ EXPORT_SYMBOL(netdev_change_features);
 void netif_stacked_transfer_operstate(const struct net_device *rootdev,
 					struct net_device *dev)
 {
-	if (dev->proto_down)
-		return;
-
 	if (rootdev->operstate == IF_OPER_DORMANT)
 		netif_dormant_on(dev);
 	else
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index a93321db8fd7..05c250c483f0 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -609,6 +609,9 @@ static void netdev_watchdog_down(struct net_device *dev)
  */
 void netif_carrier_on(struct net_device *dev)
 {
+	if (READ_ONCE(dev->proto_down))
+		return;
+
 	if (test_and_clear_bit(__LINK_STATE_NOCARRIER, &dev->state)) {
 		if (dev->reg_state == NETREG_UNINITIALIZED)
 			return;