Hi Simon,

Am 08.07.26 um 18:43 schrieb 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.
Full review at: https://sashiko.dev

Thanks for feedback. The remarks are valid, I agree. I also begin to better
understand the macvlan code so I'm a bit more confident now about which of
the existing "macvlan_passthru" checks need treatment. I'll send an updated
patch.

General question though: The initial posting triggered an AI review (I see
that now, I didn't know about https://sashiko.dev before). But it did not
trigger an email. Can you explain why? Because I accidentally dropped the RFC
tag from the mail subject?

Best regards.

---
macvlan: allow source mode devices along with passthru

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.

quoted

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index c40fa331836bb..d28f9d905a84d 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c

[ ... ]

quoted

@@ -1523,10 +1514,30 @@ 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;
+	}

[Severity: High]
While macvlan_common_newlink() prevents creating non-SOURCE devices on a
passthru port, macvlan_changelink() does not seem to have a similar check.

Can a user bypass this restriction by creating a SOURCE device and then using
changelink to modify its mode to VEPA, bypassing MAC isolation?

[Severity: High]
With source devices and passthru devices now coexisting on the same port,
functions like macvlan_open(), macvlan_stop(), and macvlan_sync_address()
check the port-wide macvlan_passthru(vlan->port) flag instead of the device
mode.

If a user brings down a SOURCE device on a passthru port, will macvlan_stop()
incorrectly take the passthru teardown path? This appears to skip dev_uc_del()
and call dev_set_promiscuity(..., -1) for a device that never incremented it.

quoted

+
+	/* 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);

[Severity: High]
If a PASSTHRU device is deleted while SOURCE devices still exist on the port,
macvlan_uninit() decrements port->count but does not clear MACVLAN_F_PASSTHRU.

Does this leave the port in a permanently broken state where new PASSTHRU
devices cannot be created, and remaining SOURCE devices continue to be
mistakenly treated as passthru?

quoted

 		eth_hw_addr_inherit(dev, lowerdev);

-- 
Thomas Martitz [off-list ref]
FRITZ! Technology GmbH, Berlin (Germany)