Re: [PATCH net] net: do not bond/team netdevices which use ml_priv

On 21.08.26 03:46, Hangbin Liu wrote:

>> This caused a problem on CAN devices that were not created by the CAN driver
>> infrastructure (creating proper ml_priv content). When TUN/TAP set the
>> dev->type of an ethernet device to ARPHRD_CAN the CAN ml_priv is NULL (not
>> initialized).
> 
> I don't know why a user change the tun/tap dev->type to CAN. Can they work
> together? If it's a miss config, I think we can just leave it since we already
> block CAN slave.

Correct. We currently block ARPHRD_CAN in bond_main.c

>> Not sure if collecting a bunch of ARPHRD values is the right approach or
>> whether team/bonding should check required features and settings (like IFF
>> flags, e.g. IFF_ARP or specific address length)?
> 
> Bond supports none arp devices. It also supports infiniband devices. So we
> can't check it with IFF_ARP or address length.
> 
>  From my perspective, we can keep the existing check as it only causes issues
> with CAN devices. We can work out a better solution if more incompatible
> devices are found under bond/team.

I've checked some whitelisting ideas for bond and team which did not 
really work and turned out to be risky.

In the end the V2 patch from Jiale Yao testing for ARPHRD_CAN and 
ARPHRD_IEEE802154 / ARPHRD_IEEE802154_MONITOR (suggested by 
Gemini/Jakub) seems to be the best idea!

https://lore.kernel.org/netdev/[email protected]/

ARPHRD_IEEE802154 / ARPHRD_IEEE802154_MONITOR do similar things like the 
CAN dev->ml_priv approach but with dev->ieee802154_ptr :-/

diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
index feaa75fbf8fc..d8c46105fc8b 100644
--- a/drivers/net/team/team_core.c
+++ b/drivers/net/team/team_core.c
@@ -1224,6 +1224,17 @@ static int team_port_add(struct team *team, 
struct net_device *port_dev,
  		return -EINVAL;
  	}

+		if (port_dev->type == ARPHRD_CAN ||
+		    port_dev->type == ARPHRD_IEEE802154 ||
+		    port_dev->type == ARPHRD_IEEE802154_MONITOR) {
+			NL_SET_ERR_MSG(extack,
+				       "CAN and IEEE 802.15.4 devices can't be added as a team port");
+			netdev_err(dev, "Device %s is CAN or IEEE 802.15.4. These device 
types can't be added as a team port\n",
+				   portname);
+			return -EINVAL;
+		}
+	}
+

And for the same reason the same checks for ARPHRD_IEEE802154 / 
ARPHRD_IEEE802154_MONITOR need to be added in bond_main.c where 
currently only ARPHRD_CAN is blocked.

And I would suggest to have two separate patches for team/bond and not 
create a common function for now. Also to provide proper Fixes tags.

@Jiale Yao: Would you like to pick up the bond patch too, so that your 
team V2 patch together with the new bond patch will solve this entire issue?

Finally this will fix these three users of "ml_priv"-style non-ethernet 
device types.

The general discussion about the ml_priv usage in

- qeth
- ctcm
- cxgb2
- i596
- wilc1000

gets relaxed after drilling down the shown critical cases. The use of 
ml_priv is simply legacy code and we can make some cleanups/enhancements 
there without pressure.

Best regards,
Oliver