Re: [PATCH net] net: do not bond/team netdevices which use ml_priv
Hangbin Liu <[email protected]>
| Newsgroups | org.kernel.vger.netdev |
|---|---|
| Message-ID | <aoet6aHyJI5lGNOX@fedora> |
Hi Oliver, On Thu, Aug 20, 2026 at 01:06:18PM +0200, Oliver Hartkopp wrote: > Hi Alexandra, > > > How would bonding modify dev->ml_priv? > > > > Could you give more information about this? > > I hope there is no issue for qeth_l2. There we use and rely on dev->ml_priv. > > > > I did some more investigation on all this. > > There are 5 drivers that are using ml_priv: > > - qeth > - ctcm > - cxgb2 > - i596 > - wilc1000 > > where qeth and i596 are using it to store a single pointer which can also be > done by adding this pointer to their netdev_priv structure. > > wilc1000 assigns ml_priv and never reads from it (development leftover). > > Only cxgb2 and ctcm use it in a more complex way that would make it tricky > to move its functionality into netdev_priv without having real hardware on > the desk. > > Either team and bonding do not fiddle with ml_priv on their own. But they > make assumptions that best fit to ethernet devices where they don't care > about nor copy any ml_priv pointers. > > 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. > > Long story short: > > The ml_priv assignment in wilc100 can be removed. > For ethernet devices like the qeth there's no problem AFAICS. > But I would think about making use of netdev_priv() there: [...] > > The final question (which is not really a ml_priv issue) is how to tell > team/bonding which netdevices are not capable to be used by them. To cover > e.g. your ctcm driver using ARPHRD_SLIP. > > The current check (bond_dev->type != slave_dev->type) would allow to join > two type-identical interfaces, which was at least not a good idea for CAN. > For that reason we already check (slave_dev->type == ARPHRD_CAN) there. > Other dev->types might follow. > > 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. Thanks Hangbin