| Message ID | 20210107170523.26531-18-m.chetan.kumar@intel.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: iosm: PCIe Driver for Intel M.2 Modem | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | fail | Series longer than 15 patches |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | warning | 2 maintainers not CCed: kuba@kernel.org davem@davemloft.net |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 1 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
> diff --git a/drivers/net/wwan/iosm/README b/drivers/net/wwan/iosm/README Please convert this to rst, and put it somewhere under Documentation. > +Multiplexed IP sessions (IPS) > +----------------------------- > +IOSM driver allows multiplexing of several IP sessions over the single network > +device of type wwan0. IOSM driver models such IP sessions as 802.1q VLAN > +subdevices of the master wwanY device, mapping MBIM IP session M to VLAN ID M > +for all values of M greater than 0. > + > +The userspace management application is responsible for adding new VLAN links > +prior to establishing MBIM IP sessions where the SessionId is greater than 0. > +These links can be added by using the normal VLAN kernel interfaces. > + > +For example, adding a link for a MBIM IP session with SessionId 5: > + > + ip link add link wwan0 name wwan0.<name> type vlan id 5 So, this is what all the Ethernet nonsense is all about. You have a session ID you need to somehow represent to user space. And you decided to use VLANs. But to use VLANs, you need an Ethernet header. So you added a bogus Ethernet header. Is any of this VLAN stuff required by MBIM? I suggest you throw away the pretence this is an Ethernet device. It is not. Linux allows you to dynamically create/destroy network interfaces. So you want to do something like ip link add link wwan0 name wwan42 type mbim id 42 Which will create a new mbim netdev interface using session id 42 on top of the device which provides wwan0. I don't actually like this last bit, but you somehow need to indicate on which MBIM transport you want to create the new session, since you could have multiple bits of hardware providing MBIM services. Andrew
Hi Andrew, all, > > +For example, adding a link for a MBIM IP session with SessionId 5: > > + > > + ip link add link wwan0 name wwan0.<name> type vlan id 5 > > So, this is what all the Ethernet nonsense is all about. You have a > session ID you need to somehow represent to user space. And you > decided to use VLANs. But to use VLANs, you need an Ethernet > header. So you added a bogus Ethernet header. So yeah, I don't think anyone likes that. I had half-heartedly started working on a replacement framework (*1), but then things happened and I didn't really have much time, and you also reviewed it and had some comments but when I looked the component framework really didn't seem appropriate, but didn't really have time to do anything on this either. (*1) https://lore.kernel.org/netdev/20200225100053.16385-1-johannes@sipsolutions.net/ In the mean time, the team doing this driver (I'm not directly involved, just helping them out with upstream processes) really needed/wanted to continue on this, and this is what they had already, more or less. Now, the question here at this point of course is they already had it that way. But that's easily explained - that's how it works upstream today, unfortunately, cf. for example drivers/net/usb/cdc_mbim.c. Now, granted, some of the newer ones such as drivers/net/ipa/ _don't_ things that way and come out with ARPHRD_RAWIP, but that requires userspace to actually be aware of this, and know how to create the necessary channels etc. For IPA this is handled by 'rmnet', but rmnet is just Qualcomm's proprietary protocol exposed as an rtnetlink type, so is rather unsuitable for this driver. Hence originally the thought we could come up with a generic framework to handle this all. Unfortunately, I never had the time to follow up on everything there. T be honest I also lost interest when IPA got merged without any thoughts given to unifying this, despite my involvement in the reviews and time spent on trying to make a suitable framework that would serve both IPA and this IOSM driver. > Is any of this VLAN stuff required by MBIM? Yes and no. It's not required to do _VLAN_ stuff, but that's one of the few ways that userspace currently knows of. Note that as far as I can tell Qualcomm (with rmnet/IPA etc.) has basically "reinvented" the world here - requiring the use of either their proprietary modem stack, or libqmi that knows specifically how to drive their modems. This was something we wanted to avoid (unless perhaps we could arrive at a standardised solution, see above) - thus being left with the VLAN method that's used elsewhere in the kernel. > Linux allows you to dynamically create/destroy network > interfaces. So you want to do something like > > ip link add link wwan0 name wwan42 type mbim id 42 > > Which will create a new mbim netdev interface using session id 42 on > top of the device which provides wwan0. I don't actually like this > last bit, but you somehow need to indicate on which MBIM transport you > want to create the new session, since you could have multiple bits of > hardware providing MBIM services. I don't even like the fact that 'wwan0' exists there in the first place (or how it exists in this driver), because it cannot ever actually transport traffic since it's just the root device of sorts. Hence the proposal to have - similar what we do in wifi - a separate abstraction of what a modem device is, and then just allow channels to be created on it, and those channels are exposed as netdevs. In any case - I'm not sure how we resolve this. On the one hand, as a technical person going for the most technically correct solution, I'd say you're completely right and this should expose pure IP netdevs, and have a (custom or not) way to configure channels. That still leaves the "dead" wwan0 interface that can't do anything, but at least it's better for the channel netdevs. Perhaps like with the framework I was trying to do. We could even initially side-step the issue with the component framework and simply not allow that in the framework from the start. However, I'm not sure of the value of this. Qualcomm's newer stuff is already locked in to their custom APIs in rmnet and IPA, with QMI etc. If we're honest with ourselves, older stuff that exists in the kernel today is highly unlikely to be converted since it works now and very few people really care about anything else. Which basically leaves only this driver - either doing some old-fashioned way like it is now, or - doing its own custom way like rmnet/IPA, or - coming with a framework that pretends to be more general than rmnet but really is only used for this driver. The later two choices both require significant investment on the userspace side, so I don't think it's any wonder the first is what the driver chose, especially after my more or less failed attempt at getting traction for the common framework (before IPA got merged, after all.) Also, non-technically speaking, I'm really not sure as to what we can and should require from a single driver like this in terms of "cleaning up the ecosystem". Yes, having a common framework would be nice, but if nobody's going to use it, what's the point? And we didn't require such from IPA. Now, granted, IPA already ships with a slightly better way of doing things than ethernet+802.1q, but there's precedent for that as well... johannes
Sorry about being much too late into this discussion. I'm not having the bandwidth to read netdev anymore, and just stumbled across this now. Andrew Lunn <andrew@lunn.ch> writes: > So, this is what all the Ethernet nonsense is all about. You have a > session ID you need to somehow represent to user space. And you > decided to use VLANs. But to use VLANs, you need an Ethernet > header. So you added a bogus Ethernet header. Actually, the original reasoning was the other way around. The bogus ethernet header was added because I had seen the 3G modem vendors do that for a few years already, in the modem firmware. And I didn't think enough about it to realize that it was a really bad idea, or even that it was something I could change. Or should change. I cannot blame the MBIM sesison to VLAN mapping idea on anyone else. As far as I can remember, that was just something that popped up in my head while working on the cdc_mbim driver. But it came as a consequence of already having the bogus ethernet header. And I didn't really understand that I could define a new wwan subsystem with new device types. I thought I had to use whatever was there already. I was young and stupid. Now I'm not that young anymore ;-) Never ever imagined that this would be replicated in another driver, though. That doesn't really make much sense. We have learned by now, haven't we? This subject has been discussed a few times in the past, and Johannes summary is my understanding as well: "I don't think anyone likes that" The DSS mapping sucks even more that the IPS mapping, BTW. I don't think there are any real users? Not that I know of, at least. DSS is much better implmeneted as some per-session character device, as requested by numerous people for years. Sorry for not listening. Looks like it is too late now. > Is any of this VLAN stuff required by MBIM? No. It's my fault and mine alone. > I suggest you throw away the pretence this is an Ethernet device. It > is not. I completely agree. I wish I had gone for simple raw-ip devices both in the qmi_wwan and cdc_mbim. But qmi_wwan got them later, so there is already support for such things in wwan userspace. Bjørn
On Sun, Jan 17, 2021 at 06:26:54PM +0100, Bjørn Mork wrote: > I was young and stupid. Now I'm not that young anymore ;-) We all make mistakes, when we don't have the knowledge there are other ways. That is partially what code review is about. > Never ever imagined that this would be replicated in another driver, > though. That doesn't really make much sense. We have learned by now, > haven't we? This subject has been discussed a few times in the past, > and Johannes summary is my understanding as well: > "I don't think anyone likes that" So there seems to be agreement there. But what is not clear, is anybody willing to do the work to fix this, and is there enough ROI. Do we expect more devices like this? Will 6G, 7G modems look very different? Be real network devices and not need any of this odd stuff? Or will they be just be incrementally better but mostly the same? I went into the review thinking it was an Ethernet driver, and kept having WTF moments. Now i know it is not an Ethernet driver, i can say it is not my domain, i don't know the field well enough to say if all these hacks are acceptable or not. It probably needs David and Jakub to set the direction to be followed. Andrew
On Wed, 20 Jan 2021 20:34:51 +0100 Andrew Lunn wrote: > On Sun, Jan 17, 2021 at 06:26:54PM +0100, Bjørn Mork wrote: > > I was young and stupid. Now I'm not that young anymore ;-) > > We all make mistakes, when we don't have the knowledge there are other > ways. That is partially what code review is about. > > > Never ever imagined that this would be replicated in another driver, > > though. That doesn't really make much sense. We have learned by now, > > haven't we? This subject has been discussed a few times in the past, > > and Johannes summary is my understanding as well: > > "I don't think anyone likes that" > > So there seems to be agreement there. But what is not clear, is > anybody willing to do the work to fix this, and is there enough ROI. > > Do we expect more devices like this? Will 6G, 7G modems look very > different? Didn't Intel sell its 5G stuff off to Apple? > Be real network devices and not need any of this odd stuff? > Or will they be just be incrementally better but mostly the same? > > I went into the review thinking it was an Ethernet driver, and kept > having WTF moments. Now i know it is not an Ethernet driver, i can say > it is not my domain, i don't know the field well enough to say if all > these hacks are acceptable or not. > > It probably needs David and Jakub to set the direction to be followed. AFAIU all those cellar modems are relatively slow and FW-heavy, so the ideal solution IMO is not even a common kernel interface but actually a common device interface, like NVMe (or virtio for lack of better examples).
On Wed, 2021-01-20 at 15:32 -0800, Jakub Kicinski wrote: > On Wed, 20 Jan 2021 20:34:51 +0100 Andrew Lunn wrote: > > On Sun, Jan 17, 2021 at 06:26:54PM +0100, Bjørn Mork wrote: > > > I was young and stupid. Now I'm not that young anymore ;-) > > > > We all make mistakes, when we don't have the knowledge there are > > other > > ways. That is partially what code review is about. > > > > > Never ever imagined that this would be replicated in another > > > driver, > > > though. That doesn't really make much sense. We have learned by > > > now, > > > haven't we? This subject has been discussed a few times in the > > > past, > > > and Johannes summary is my understanding as well: > > > "I don't think anyone likes that" > > > > So there seems to be agreement there. But what is not clear, is > > anybody willing to do the work to fix this, and is there enough > > ROI. > > > > Do we expect more devices like this? Will 6G, 7G modems look very > > different? > > Didn't Intel sell its 5G stuff off to Apple? Yes, but they kept the ability to continue with 3G/4G hardware and other stuff. > > Be real network devices and not need any of this odd stuff? > > Or will they be just be incrementally better but mostly the same? > > > > I went into the review thinking it was an Ethernet driver, and kept > > having WTF moments. Now i know it is not an Ethernet driver, i can > > say > > it is not my domain, i don't know the field well enough to say if > > all > > these hacks are acceptable or not. > > > > It probably needs David and Jakub to set the direction to be > > followed. > > AFAIU all those cellar modems are relatively slow and FW-heavy, so > the > ideal solution IMO is not even a common kernel interface but actually > a common device interface, like NVMe (or virtio for lack of better > examples). That was supposed to be MBIM, but unfortunately those involved didn't iterate and MBIM got stuck. I don't think we'll see a standard as long as some vendors are dominant and see no need for it. Dan
On Wed, Jan 20, 2021 at 07:34:48PM -0600, Dan Williams wrote: > On Wed, 2021-01-20 at 15:32 -0800, Jakub Kicinski wrote: > > On Wed, 20 Jan 2021 20:34:51 +0100 Andrew Lunn wrote: > > > On Sun, Jan 17, 2021 at 06:26:54PM +0100, Bjørn Mork wrote: > > > > I was young and stupid. Now I'm not that young anymore ;-) > > > > > > We all make mistakes, when we don't have the knowledge there are > > > other > > > ways. That is partially what code review is about. > > > > > > > Never ever imagined that this would be replicated in another > > > > driver, > > > > though. That doesn't really make much sense. We have learned by > > > > now, > > > > haven't we? This subject has been discussed a few times in the > > > > past, > > > > and Johannes summary is my understanding as well: > > > > "I don't think anyone likes that" > > > > > > So there seems to be agreement there. But what is not clear, is > > > anybody willing to do the work to fix this, and is there enough > > > ROI. > > > > > > Do we expect more devices like this? Will 6G, 7G modems look very > > > different? > > > > Didn't Intel sell its 5G stuff off to Apple? > > Yes, but they kept the ability to continue with 3G/4G hardware and > other stuff. But we can expect 6G in what, 2030? And 7G in 2040? Are they going to look different? Or is it going to be more of the same, meaningless ethernet headers, VLANs where VLANs make little sense? > > > Be real network devices and not need any of this odd stuff? > > > Or will they be just be incrementally better but mostly the same? > > > > > > I went into the review thinking it was an Ethernet driver, and kept > > > having WTF moments. Now i know it is not an Ethernet driver, i can > > > say > > > it is not my domain, i don't know the field well enough to say if > > > all > > > these hacks are acceptable or not. > > > > > > It probably needs David and Jakub to set the direction to be > > > followed. > > > > AFAIU all those cellar modems are relatively slow and FW-heavy, so > > the > > ideal solution IMO is not even a common kernel interface but actually > > a common device interface, like NVMe (or virtio for lack of better > > examples). > > That was supposed to be MBIM, but unfortunately those involved didn't > iterate and MBIM got stuck. I don't think we'll see a standard as long > as some vendors are dominant and see no need for it. We the kernel community need to decide, we are happy for this broken architecture to live on, and we should give suggest how to make this submission better. Or we need to push back and say for the long term good, this driver is not going to be accepted, use a more sensible architecture. Andrew
diff --git a/drivers/net/wwan/iosm/README b/drivers/net/wwan/iosm/README new file mode 100644 index 000000000000..4a489177ad96 --- /dev/null +++ b/drivers/net/wwan/iosm/README @@ -0,0 +1,126 @@ +IOSM Driver for PCIe based Intel M.2 Modems +================================================ +The IOSM (IPC over Shared Memory) driver is a PCIe host driver implemented +for linux or chrome platform for data exchange over PCIe interface between +Host platform & Intel M.2 Modem. The driver exposes interface conforming to the +MBIM protocol [1]. Any front end application ( eg: Modem Manager) could easily +manage the MBIM interface to enable data communication towards WWAN. + +Basic usage +=========== +MBIM functions are inactive when unmanaged. The IOSM driver only +provides a userspace interface of a character device representing +MBIM control channel and does not play any role in managing the +functionality. It is the job of a userspace application to enumerate +the port appropriately and enable MBIM functionality. + +Examples of few such userspace application are: + - mbimcli (included with the libmbim [2] library), and + - ModemManager [3] + +For establishing an MBIM IP session at least these actions are required by the +management application: + - open the control channel + - configure network connection settings + - connect to network + - configure IP interface + +Management application development +---------------------------------- +The driver and userspace interfaces are described below. The MBIM +control channel protocol is described in [1]. + +MBIM control channel userspace ABI +================================== + +/dev/wwanctrl character device +------------------------------ +The driver exposes an interface to the MBIM function control channel using char +driver as a subdriver. The userspace end of the control channel pipe is a +/dev/wwanctrl character device. + +The /dev/wwanctrl device is created as a subordinate character device under +IOSM driver. The character device associated with a specific MBIM function +can be looked up using sysfs with matching the above device name. + +Control channel configuration +----------------------------- +The wMaxControlMessage field of the MBIM functional descriptor +limits the maximum control message size. The management application needs to +negotiate the control message size as per the requirements. +See also the ioctl documentation below. + +Fragmentation +------------- +The userspace application is responsible for all control message +fragmentation and defragmentation as per MBIM. + +/dev/wwanctrl write() +--------------------- +The MBIM control messages from the management application must not +exceed the negotiated control message size. + +/dev/wwanctrl read() +-------------------- +The management application must accept control messages of up the +negotiated control message size. + +/dev/wwanctrl ioctl() +-------------------- +IOCTL_WDM_MAX_COMMAND: Get Maximum Command Size +This IOCTL command could be used by applications to fetch the Maximum Command +buffer length supported by the driver which is restricted to 4096 bytes. + + #include <stdio.h> + #include <fcntl.h> + #include <sys/ioctl.h> + #include <linux/types.h> + int main() + { + __u16 max; + int fd = open("/dev/wwanctrl", O_RDWR); + if (!ioctl(fd, IOCTL_WDM_MAX_COMMAND, &max)) + printf("wMaxControlMessage is %d\n", max); + } + +MBIM data channel userspace ABI +=============================== + +wwanY network device +-------------------- +The IOSM driver represents the MBIM data channel as a single +network device of the "wwan0" type. This network device is initially +mapped to MBIM IP session 0. + +Multiplexed IP sessions (IPS) +----------------------------- +IOSM driver allows multiplexing of several IP sessions over the single network +device of type wwan0. IOSM driver models such IP sessions as 802.1q VLAN +subdevices of the master wwanY device, mapping MBIM IP session M to VLAN ID M +for all values of M greater than 0. + +The userspace management application is responsible for adding new VLAN links +prior to establishing MBIM IP sessions where the SessionId is greater than 0. +These links can be added by using the normal VLAN kernel interfaces. + +For example, adding a link for a MBIM IP session with SessionId 5: + + ip link add link wwan0 name wwan0.<name> type vlan id 5 + +The driver will automatically map the "wwan0.<name>" network device to MBIM +IP session 5. + +References +========== + +[1] "MBIM (Mobile Broadband Interface Model) Registry" + - http://compliance.usb.org/mbim/ + +[2] libmbim - "a glib-based library for talking to WWAN modems and + devices which speak the Mobile Interface Broadband Model (MBIM) + protocol" + - http://www.freedesktop.org/wiki/Software/libmbim/ + +[3] ModemManager - "a DBus-activated daemon which controls mobile + broadband (2G/3G/4G) devices and connections" + - http://www.freedesktop.org/wiki/Software/ModemManager/ \ No newline at end of file
Documents IOSM Driver interface usage. Signed-off-by: M Chetan Kumar <m.chetan.kumar@intel.com> --- drivers/net/wwan/iosm/README | 126 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 drivers/net/wwan/iosm/README