Johannes Berg <johan...@sipsolutions.net> writes: > Hi all, > > I've been looking at modem drivers, to see what the APIs are to interact > with them, and while I originally thought I had the story sorted out ... > not at all.
Thanks a lot for doing this! Being responsible for most of the issues you point out, I can only say that you have my full support if you want to change any of it. My pathetic excuses below are just meant to clarify why things are the way they are. They are not a defense for status quo ;-) > Here's the current things we seem to be doing: > > (1) Channels are created/encoded as VLANs (cdc_mbim) > > This is ... strange at best, it requires creating fake ethernet > headers on the frames, just to be able to have a VLAN tag. If you > could rely on VLAN acceleration it wouldn't be _so_ bad, but of > course you can't, so you have to detect an in-band VLAN tag and > decode/remove it, before taking the VLAN ID into the virtual > channel number. No, the driver sets NETIF_F_HW_VLAN_CTAG_TX. There is no in-band VLAN tag for any normal use. The tag is taken directly from skb metadata and mapped to the appropriate MBIM session ID. But this failed when cooking raw frames with an in-line tag using packet sockets, so I added a fallback to in-line tags for that use case. > Creating channels is hooked on VLAN operations, which is about the > only thing that makes sense here? Well, that was why I did this, to avoid requiring som new set of userspace tools to manage these links. I looked for some existing tools for adding virtual netdevs, and I thought I could make VLANs fit the scheme. In hindsight, I should have created a new netlink based API for cellular modem virtual links instead. But I don't think it ever struck me as a choice I had at the time. I just wasn't experienced enough to realize how the Linux kernel APIs are developed ;-) > (2) Channels are created using sysfs (qmi_wwan) > > This feels almost worse - channels are created using sysfs and > just *bam* new netdev shows up, no networking APIs are used to > create them at all, and I suppose you can't even query the channel > ID for each netdev if you rename them or so. Actually, maybe you > can in sysfs, not sure I understand the code fully. This time I was, and I tried to learn from the MBIM mistake. So I asked the users (ModemManager developers++), proposing a netlink API as a possible solution: https://lists.freedesktop.org/archives/libqmi-devel/2017-January/001900.html The options I presented were those I saw at the time: VLANs like cdc_mbim, a new netlink API, or sysfs. There wasn't much feedback, but sysfs "won". So this was a decision made by the users of the API, FWIW. > (3) Channels are created using a new link type (rmnet) > > To me this sort of feels the most natural, but this particular > implementation has at least two issues: > > (a) The implementation is basically driver-specific now, the link > type is called 'rmnet' etc. > (b) The bridge enslave thing there is awful. This driver showed up right after the sysfs based implementation in qmi_wwan. Too bad we didn't know about this work then. I don't think anyone would have been interested in the qmi_wwan sysfs thing if we had known about the plans for this driver. But what's done is done. > It seems to me that there really is space here for some common > framework, probably modelled on rmnet - that seems the most reasonable > approach of all three. > > The only question I have there is whether the 'netdev model' they all > have actually makes sense. What I mean by that is that they all assume > they have a default channel (using untagged frames, initial netdev, > initial netdev respectively for (1) - (3)). Good question. I guess the main argument for the 'netdev model' is that it makes the device directly usable with no extra setup or tools. Most users won't ever want or need more than one channel anyway. They use the modem for a single IP session. There is a also an advantage for QMI/RMNET where you can drop the muxing header when using the default channel only. > In 802.11, we don't have such a default channel - you can add/remove > virtual netdevs on the fly. But if you want to do that, then you can't > use IFLA_LINK and the normal link type, which means custom netlink and > custom userspace etc. which, while we do it in wifi, is bothersome. Yes, some of the feedback I've got from the embedded users is that they don't want any more custom userspace tools. But I'm sure you've heard that a few times too :-) > Here I guess the question would be whether it makes sense to even remove > the default channel, or retag it, or something like that. If no, then to > me it all makes sense to just model rmnet. And even if it *is* something > that could theoretically be done, it seems well possible to me that the > benefits (using rtnl_link_register() etc.) outweigh the deficits of the > approach. > > > I'm tempted to take a stab at breaking out rmnet_link_ops from the rmnet > driver, somehow giving it an alias of 'wwan-channel' or something like > that, and putting it into some sort of small infrastructure. > > Anyone else have any thoughts? I've added Aleksander (ModemManager) and Daniele (qmi_wwan muxing user and developer) to the CC list. They are the ones who wold end up using a possible new API, so they should definitely be part of the discussion Bjørn
