[PATCH v4 0/1] macvlan: allow source mode devices along with passthru

[PATCH v4 0/1] macvlan: allow source mode devices along with passthru

[Thomas Martitz]

Hello,

we're trying to solve a use case on our devices where two SoC are
connected on the same board, using the only available high-speed interface.

One SoC runs the main Linux system including the full routing stack
(FRITZ!OS) and the other SoC implements most of the GPON ONT side.

The high-speed interface is of course also used for the user traffic.
Therefore we must tell the inter-SoC traffic apart from the user traffic.

We achieve this by matching the well-known MAC address of the ONT SoC.
The user traffic passes through the ONT SoC without modifying MAC headers.
Now we would like to use macvlan (with source mode devices) on the main
SoC side for this but our routing stack requires the rx_handler to be
available. Therefore macvlan is currently not an option.

With this patch macvlan becomes an option because the current limitation
of either "one passthru device" or "any other configuration" is relaxed
for the combination of passthru and any number of source mode devices.

This allows us to configure a source mode device for the other SoC and
register an rx_handler for further processing on the passthru device.

Thanks in advance!

---
Changes in v4
- Prevent macvlan_restore_mac() from changing the MAC address
  of remaining source mode interfaces.
- Set MACVLAN_F_PASSTHRU on the macvlan_port only after adding
  the passthru interface truly succeeds.
- Changing existing interfaces to passthru mode shouldn't become
  allowed.

Changes in v3
- fix passthru port removal caused by passing the wrong 
  device to macvlan_port_release_mac(). This was also
  detected by syzbot.
- macvlan_port_release_mac() is now named macvlan_restore_mac()
  and gets passed a "struct macvlan_port" directly.
- Link to v2: https://lore.kernel.org/netdev/20260709100512.1383421-1-t.martitz@fritz.com/

Changes in v2:
- changed several port-wide checks (macvlan_passthru()) to 
  per-interface checks (vlan->mode == vlan->mode == MACVLAN_MODE_PASSTHRU)
- correctly handle removing the passthru interface when there are still
  source interfaces
- Link to initial posting: https://lore.kernel.org/netdev/20260612092345.2352255-1-t.martitz@fritz.com/
---

Thomas Martitz (1):
  macvlan: allow source mode devices along with passthru

 drivers/net/macvlan.c | 107 ++++++++++++++++++++++++++++--------------
 1 file changed, 73 insertions(+), 34 deletions(-)

-- 
2.54.0

[PATCH v4 1/1] macvlan: allow source mode devices along with passthru

[Thomas Martitz]

This allows for configurations where there are a few
known senders in the system (e.g. multiple SoCs on the same
board) along with unlimited external senders.

The source mode devices represent the known senders while
all external senders terminate on passthru device.

Although you can still receive packets on the lower device
without the need for the passthru vlan device, there
are use cases where you need additional packet processing
in the pipeline that hooks via rx_handler. With this the
rx_handler can be attached to the passthru device while
macvlan itself remains attached to the lower device.

We use this to use the same physical link for inter-SoC
networking and external networking. Some of our chips
have no other viable link for inter-SoC traffic.

Signed-off-by: Thomas Martitz <t.martitz@fritz.com>
---
 drivers/net/macvlan.c | 118 ++++++++++++++++++++++++++++--------------
 1 file changed, 79 insertions(+), 39 deletions(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 9a4bc99dbf53b..405f2cc0e5363 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -83,6 +83,11 @@ static inline void macvlan_set_passthru(struct macvlan_port *port)
 	port->flags |= MACVLAN_F_PASSTHRU;
 }
 
+static inline void macvlan_clear_passthru(struct macvlan_port *port)
+{
+	port->flags &= ~MACVLAN_F_PASSTHRU;
+}
+
 static inline bool macvlan_addr_change(const struct macvlan_port *port)
 {
 	return port->flags & MACVLAN_F_ADDRCHANGE;
@@ -637,7 +642,7 @@ static int macvlan_open(struct net_device *dev)
 	struct net_device *lowerdev = vlan->lowerdev;
 	int err;
 
-	if (macvlan_passthru(vlan->port)) {
+	if (vlan->mode == MACVLAN_MODE_PASSTHRU) {
 		if (!(vlan->flags & MACVLAN_FLAG_NOPROMISC)) {
 			err = dev_set_promiscuity(lowerdev, 1);
 			if (err < 0)
@@ -712,7 +717,7 @@ static int macvlan_stop(struct net_device *dev)
 	dev_uc_unsync(lowerdev, dev);
 	dev_mc_unsync(lowerdev, dev);
 
-	if (macvlan_passthru(vlan->port)) {
+	if (vlan->mode == MACVLAN_MODE_PASSTHRU) {
 		if (!(vlan->flags & MACVLAN_FLAG_NOPROMISC))
 			dev_set_promiscuity(lowerdev, -1);
 		goto hash_del;
@@ -968,6 +973,21 @@ static int macvlan_init(struct net_device *dev)
 	return 0;
 }
 
+static void macvlan_restore_mac(struct macvlan_port *port)
+{
+	/* If the lower device address has been changed by passthru
+	 * macvlan, put it back.
+	 */
+	if (macvlan_passthru(port) &&
+	    !ether_addr_equal(port->dev->dev_addr, port->perm_addr)) {
+		struct sockaddr_storage ss;
+
+		ss.ss_family = port->dev->type;
+		memcpy(&ss.__data, port->perm_addr, port->dev->addr_len);
+		dev_set_mac_address(port->dev, &ss, NULL);
+	}
+}
+
 static void macvlan_uninit(struct net_device *dev)
 {
 	struct macvlan_dev *vlan = netdev_priv(dev);
@@ -977,8 +997,19 @@ static void macvlan_uninit(struct net_device *dev)
 
 	macvlan_flush_sources(port, vlan);
 	port->count -= 1;
-	if (!port->count)
-		macvlan_port_destroy(port->dev);
+	if (port->count) {
+		/* In case of remaining source interfaces undo
+		 * passthru-specific properties.
+		 */
+		if (vlan->mode == MACVLAN_MODE_PASSTHRU) {
+			/* Order is important, do not trigger macvlan_device_event(). */
+			macvlan_clear_passthru(port);
+			macvlan_restore_mac(port);
+		}
+		return;
+	}
+
+	macvlan_port_destroy(port->dev);
 }
 
 static void macvlan_dev_get_stats64(struct net_device *dev,
@@ -1052,7 +1083,7 @@ static int macvlan_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
 	/* Support unicast filter only on passthru devices.
 	 * Multicast filter should be allowed on all devices.
 	 */
-	if (!macvlan_passthru(vlan->port) && is_unicast_ether_addr(addr))
+	if (vlan->mode != MACVLAN_MODE_PASSTHRU && is_unicast_ether_addr(addr))
 		return -EOPNOTSUPP;
 
 	if (flags & NLM_F_REPLACE)
@@ -1077,7 +1108,7 @@ static int macvlan_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
 	/* Support unicast filter only on passthru devices.
 	 * Multicast filter should be allowed on all devices.
 	 */
-	if (!macvlan_passthru(vlan->port) && is_unicast_ether_addr(addr))
+	if (vlan->mode != MACVLAN_MODE_PASSTHRU && is_unicast_ether_addr(addr))
 		return -EOPNOTSUPP;
 
 	if (is_unicast_ether_addr(addr))
@@ -1308,17 +1339,7 @@ static void macvlan_port_destroy(struct net_device *dev)
 		kfree_skb(skb);
 	}
 
-	/* If the lower device address has been changed by passthru
-	 * macvlan, put it back.
-	 */
-	if (macvlan_passthru(port) &&
-	    !ether_addr_equal(port->dev->dev_addr, port->perm_addr)) {
-		struct sockaddr_storage ss;
-
-		ss.ss_family = port->dev->type;
-		memcpy(&ss.__data, port->perm_addr, port->dev->addr_len);
-		dev_set_mac_address(port->dev, &ss, NULL);
-	}
+	macvlan_restore_mac(port);
 
 	kfree(port);
 }
@@ -1506,15 +1527,6 @@ int macvlan_common_newlink(struct net_device *dev,
 	}
 	port = macvlan_port_get_rtnl(lowerdev);
 
-	/* Only 1 macvlan device can be created in passthru mode */
-	if (macvlan_passthru(port)) {
-		/* The macvlan port must be not created this time,
-		 * still goto destroy_macvlan_port for readability.
-		 */
-		err = -EINVAL;
-		goto destroy_macvlan_port;
-	}
-
 	vlan->lowerdev = lowerdev;
 	vlan->dev      = dev;
 	vlan->port     = port;
@@ -1527,12 +1539,31 @@ int macvlan_common_newlink(struct net_device *dev,
 	if (data && data[IFLA_MACVLAN_FLAGS])
 		vlan->flags = nla_get_u16(data[IFLA_MACVLAN_FLAGS]);
 
+	/* Only 1 macvlan device can be created in passthru mode. There may be
+	 * additional source mode devices but nothing else at the moment.
+	 *
+	 * First check if adding a source mode device to an existing passthru vlan.
+	 */
+	if (macvlan_passthru(port) && vlan->mode != MACVLAN_MODE_SOURCE) {
+		/* The macvlan port must be not created this time,
+		 * still goto destroy_macvlan_port for readability.
+		 */
+		err = -EINVAL;
+		goto destroy_macvlan_port;
+	}
+
+	/* Now check if adding a passthru device to an existing set of source mode
+	 * devices.
+	 */
 	if (vlan->mode == MACVLAN_MODE_PASSTHRU) {
-		if (port->count) {
-			err = -EINVAL;
-			goto destroy_macvlan_port;
+		struct macvlan_dev *p;
+
+		list_for_each_entry(p, &port->vlans, list) {
+			if (p->mode != MACVLAN_MODE_SOURCE) {
+				err = -EINVAL;
+				goto destroy_macvlan_port;
+			}
 		}
-		macvlan_set_passthru(port);
 		eth_hw_addr_inherit(dev, lowerdev);
 	}
 
@@ -1564,7 +1595,12 @@ int macvlan_common_newlink(struct net_device *dev,
 	if (err)
 		goto unregister_netdev;
 
-	list_add_tail_rcu(&vlan->list, &port->vlans);
+	/* macvlan_handle_frame expects the (one and only) passthru device first. */
+	if (vlan->mode == MACVLAN_MODE_PASSTHRU) {
+		macvlan_set_passthru(port);
+		list_add_rcu(&vlan->list, &port->vlans);
+	} else
+		list_add_tail_rcu(&vlan->list, &port->vlans);
 	update_port_bc_queue_len(vlan->port);
 	netif_stacked_transfer_operstate(lowerdev, dev);
 	linkwatch_fire_event(dev);
@@ -1627,19 +1663,23 @@ static int macvlan_changelink(struct net_device *dev,
 	if (data && data[IFLA_MACVLAN_MODE]) {
 		set_mode = true;
 		mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
-		/* Passthrough mode can't be set or cleared dynamically */
-		if ((mode == MACVLAN_MODE_PASSTHRU) !=
-		    (vlan->mode == MACVLAN_MODE_PASSTHRU))
-			return -EINVAL;
-		if (vlan->mode == MACVLAN_MODE_SOURCE &&
-		    vlan->mode != mode)
-			macvlan_flush_sources(vlan->port, vlan);
+		if (mode != vlan->mode) {
+			/* Passthrough mode can't be set or cleared dynamically,
+			 * regardless of existing source interfaces. Furthermore, source
+			 * interfaces can't switch modes within a passhtrough port.
+			 */
+			if (vlan->mode == MACVLAN_MODE_PASSTHRU ||
+			    macvlan_passthru(vlan->port))
+				return -EINVAL;
+			if (vlan->mode == MACVLAN_MODE_SOURCE)
+				macvlan_flush_sources(vlan->port, vlan);
+		}
 	}
 
 	if (data && data[IFLA_MACVLAN_FLAGS]) {
 		__u16 flags = nla_get_u16(data[IFLA_MACVLAN_FLAGS]);
 		bool promisc = (flags ^ vlan->flags) & MACVLAN_FLAG_NOPROMISC;
-		if (macvlan_passthru(vlan->port) && promisc) {
+		if (vlan->mode == MACVLAN_MODE_PASSTHRU && promisc) {
 			int err;
 
 			if (flags & MACVLAN_FLAG_NOPROMISC)
-- 
2.54.0

Re: [PATCH v4 1/1] macvlan: allow source mode devices along with passthru

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260713112414.2842803-2-t.martitz@fritz.com?part=1
---
macvlan: allow source mode devices along with passthru

This change permits combining one passthru macvlan with one or more
source-mode macvlans on the same lower device, so that an rx_handler can
be attached to the passthru device while source-mode devices continue to
represent known senders.

> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> index c40fa331836b..635ed01abeb6 100644
> --- a/drivers/net/macvlan.c
> +++ b/drivers/net/macvlan.c

[ ... ]

> @@ -964,6 +969,21 @@ static int macvlan_init(struct net_device *dev)
>  	return 0;
>  }
>  
> +static void macvlan_restore_mac(struct macvlan_port *port)
> +{
> +	/* If the lower device address has been changed by passthru
> +	 * macvlan, put it back.
> +	 */
> +	if (macvlan_passthru(port) &&
> +	    !ether_addr_equal(port->dev->dev_addr, port->perm_addr)) {
> +		struct sockaddr_storage ss;
> +
> +		ss.ss_family = port->dev->type;
> +		memcpy(&ss.__data, port->perm_addr, port->dev->addr_len);
> +		dev_set_mac_address(port->dev, &ss, NULL);
> +	}
> +}
> +
>  static void macvlan_uninit(struct net_device *dev)
>  {
>  	struct macvlan_dev *vlan = netdev_priv(dev);
> @@ -973,8 +993,19 @@ static void macvlan_uninit(struct net_device *dev)
>  
>  	macvlan_flush_sources(port, vlan);
>  	port->count -= 1;
> -	if (!port->count)
> -		macvlan_port_destroy(port->dev);
> +	if (port->count) {
> +		/* In case of remaining source interfaces undo
> +		 * passthru-specific properties.
> +		 */
> +		if (vlan->mode == MACVLAN_MODE_PASSTHRU) {
> +			/* Order is important, do not trigger macvlan_device_event(). */
> +			macvlan_clear_passthru(port);
> +			macvlan_restore_mac(port);

[High]
When the last passthru macvlan is removed while source-mode siblings
remain, does macvlan_restore_mac() actually restore the lower device's
MAC in this ordering?

macvlan_clear_passthru() is called first, which drops MACVLAN_F_PASSTHRU:

    static inline void macvlan_clear_passthru(struct macvlan_port *port)
    {
        port->flags &= ~MACVLAN_F_PASSTHRU;
    }

Then macvlan_restore_mac() runs, but its body is gated on
macvlan_passthru(port):

    static void macvlan_restore_mac(struct macvlan_port *port)
    {
        if (macvlan_passthru(port) &&
            !ether_addr_equal(port->dev->dev_addr, port->perm_addr)) {
            ...
            dev_set_mac_address(port->dev, &ss, NULL);
        }
    }

Since the flag has just been cleared, does the guard evaluate to false
here, causing dev_set_mac_address() to be skipped and the lower device
to retain the passthru MAC instead of port->perm_addr?

The pre-patch macvlan_port_destroy() path did not clear the flag first,
so the restore actually took effect there.

>  		}
> +		return;
> +	}
> +
> +	macvlan_port_destroy(port->dev);
>  }
>  
>  static void macvlan_dev_get_stats64(struct net_device *dev,

[ ... ]

> @@ -1623,19 +1659,23 @@ static int macvlan_changelink(struct net_device *dev,
>  	if (data && data[IFLA_MACVLAN_MODE]) {
>  		set_mode = true;
>  		mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
> -		/* Passthrough mode can't be set or cleared dynamically */
> -		if ((mode == MACVLAN_MODE_PASSTHRU) !=
> -		    (vlan->mode == MACVLAN_MODE_PASSTHRU))
> -			return -EINVAL;
> -		if (vlan->mode == MACVLAN_MODE_SOURCE &&
> -		    vlan->mode != mode)
> -			macvlan_flush_sources(vlan->port, vlan);
> +		if (mode != vlan->mode) {
> +			/* Passthrough mode can't be set or cleared dynamically,
> +			 * regardless of existing source interfaces. Furthermore, source
> +			 * interfaces can't switch modes within a passhtrough port.
> +			 */
> +			if (vlan->mode == MACVLAN_MODE_PASSTHRU ||
> +			    macvlan_passthru(vlan->port))
> +				return -EINVAL;
> +			if (vlan->mode == MACVLAN_MODE_SOURCE)
> +				macvlan_flush_sources(vlan->port, vlan);
> +		}
>  	}

[High]
Does this new check still reject setting an existing non-passthru vlan
to MACVLAN_MODE_PASSTHRU when the port has no passthru vlan yet?

The comment still says "Passthrough mode can't be set or cleared
dynamically", but the condition:

    if (vlan->mode == MACVLAN_MODE_PASSTHRU ||
        macvlan_passthru(vlan->port))
        return -EINVAL;

only catches clearing passthru on a passthru vlan, and any mode change
on a vlan whose port already has passthru.

If vlan->mode is one of VEPA/BRIDGE/PRIVATE/SOURCE and the request is
mode == MACVLAN_MODE_PASSTHRU on a port that has no passthru vlan yet,
does execution fall through? Later in the same function:

    if (set_mode)
        vlan->mode = mode;

promotes vlan->mode to PASSTHRU, but nothing calls
macvlan_set_passthru(port), nothing performs eth_hw_addr_inherit(dev,
lowerdev), and no passthru promiscuity accounting is done.

After this, does the vlan report mode == PASSTHRU while the port flag
MACVLAN_F_PASSTHRU stays clear? Downstream code splits between the two
predicates:

  - macvlan_open, macvlan_stop, macvlan_fdb_add, macvlan_fdb_del, and
    the IFLA_MACVLAN_FLAGS branch here read vlan->mode ==
    MACVLAN_MODE_PASSTHRU.

  - macvlan_device_event NETDEV_CHANGEADDR, macvlan_addr_busy,
    macvlan_change_rx_flags, macvlan_sync_address and
    macvlan_handle_frame read macvlan_passthru(port).

Does this then produce visible symptoms such as:

  - macvlan_open calling dev_set_promiscuity(lowerdev, 1) while the
    lower device's MAC was never inherited to the vlan;
  - macvlan_addr_busy no longer treating the port as passthru, allowing
    a sibling with the lowerdev's address;
  - macvlan_handle_frame not delivering to this vlan via
    list_first_or_null_rcu(&port->vlans) because the port flag is
    clear?

Would a condition that also rejects the direction into passthru be more
consistent with the comment, e.g.:

    if (vlan->mode == MACVLAN_MODE_PASSTHRU ||
        mode == MACVLAN_MODE_PASSTHRU ||
        macvlan_passthru(vlan->port))
        return -EINVAL;

>  
>  	if (data && data[IFLA_MACVLAN_FLAGS]) {
>  		__u16 flags = nla_get_u16(data[IFLA_MACVLAN_FLAGS]);
>  		bool promisc = (flags ^ vlan->flags) & MACVLAN_FLAG_NOPROMISC;
> -		if (macvlan_passthru(vlan->port) && promisc) {
> +		if (vlan->mode == MACVLAN_MODE_PASSTHRU && promisc) {
>  			int err;
>  
>  			if (flags & MACVLAN_FLAG_NOPROMISC)