Re: Instantiating rmnet devices for data ports on QRTR-based modems
Hi, Il giorno lun 19 ott 2020 alle ore 09:57 Aleksander Morgado ha scritto: > > Hey, > > > > > I seem we have done for "QMI based on QTRT" in libqmi. > > > > For QUALCOMM's data services, there are lots of QMI library, > > > > and libqmi used a more simply solution. (in fact, if just send QMI, > > > > QTRT is also not a good idea) > > > > > > Why do you say QRTR is not a good idea? Isn't QMI over QRTR the only way > > > to > > > handle the communication in certain setups? > > [carl.yin] 1. QRTR codes is much complex than direct send QMI message via > > QMI channel like /dev/cdc-wdm0 > > And it is not easy to find real hardware device for the > > QTRT node. > > No matter the hardware interface between AP and Qualcomm > > modem is USB/PCIE/SMD, > > (SMD is for QUALCOMM inter-AP, like MDM, APQ,MSM chipset.) > > There always have QMI channel can be used to send QMI > > message. > > If use these QMI channel, very few modifies is need apply > > to libqmi and MM > > I cannot comment on this, probably @Eric Caruso has a better point of view. > > > > This logic you're suggesting, except for step 5 I think, is what we > > > already had in > > > mind more or less. I think we could have that done by ModemManager, and > > > connection managers like NetworkManager will get that working > > > automatically, > > > but now I also fear that if we change the default QMI modem behavior > > > w.r.t. > > > which is the connected network interface, we may be breaking user setups > > > out > > > there that rely on fixed interface names (e.g. for iptables rules). > > > > > > > rmnet_dataX is not a good name, there may be rment_usb0, > > > rment_mhi0 exist at the same time, so can named as rment_usb0_X. > > > > > > In qmi_wwan, the muxed interfaces are named qmimux0, qmimux1 ... And if > > > you > > > have 5 different modems, each of them instantiating muxed virtual > > > interfaces, > > > they'll just get assigned sequential interface names, without any > > > reference in the > > > interface name to which physical interface they're mapped to. I don't > > > think the > > > name of the interface matters much at this point, as long as we can know > > > which > > > virtual interface maps to which physical interface. > > > > > > E.g. qmi_wwan adds a "lower_wwan0" file in the virtual interface sysfs > > > contents > > > pointing to the physical interface: > > > # ls -ltr /sys/class/net/qmimux0/lower_wwan0 > > > lrwxrwxrwx1 root root 0 Oct 17 20:52 > > > /sys/class/net/qmimux0/lower_wwan0 -> > > > ../../../pci:00/:00:16.0/usb1/1-1/1-1.2/1-1.2:1.8/net/wwan0 > > > > > > And it adds a "upper_qmimux0" file in the physical interface sysfs > > > pointing to the > > > virtual device: > > > # ls -ltr /sys/class/net/wwan0/upper_qmimux0 > > > lrwxrwxrwx1 root root 0 Oct 17 20:53 > > > /sys/class/net/wwan0/upper_qmimux0 -> > > > ../../../../../../../../virtual/net/qmimux0 > > > > > > As long as we have this kind of relationship between the virtual and > > > physical > > > interfaces, we're good to go, regardless of the actual naming of the > > > interface. > > [carl.yin] developer can find the relationship of wwanX and qmimuxX. > > But for the end customers, maybe it is difficult, maybe he > > want to setup data call on PDN-x on modem y. > > It will be Very intuitive if we can make PDN-x to muxid-X > > to rmnet_modemY_X. > > I truly don't have a strong opinion on that, kernel devs probably have > a better idea on what the naming should look like. Either way, it is > true that the current kernel implementation for the muxing in qmi_wwan > already relies on that naming mechanism, so not sure if that could be > changed right now without breaking the API. > Maybe a possibility to align things is to add rmnet support to qmi_wwan? In the past there was an attempt for this (see https://www.mail-archive.com/netdev@vger.kernel.org/msg239867.html), but not completed. Regards, Daniele > -- > Aleksander > https://aleksander.es > ___ > ModemManager-devel mailing list > ModemManager-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: Instantiating rmnet devices for data ports on QRTR-based modems
Hey, > > > I seem we have done for "QMI based on QTRT" in libqmi. > > > For QUALCOMM's data services, there are lots of QMI library, > > > and libqmi used a more simply solution. (in fact, if just send QMI, > > > QTRT is also not a good idea) > > > > Why do you say QRTR is not a good idea? Isn't QMI over QRTR the only way to > > handle the communication in certain setups? > [carl.yin] 1. QRTR codes is much complex than direct send QMI message via QMI > channel like /dev/cdc-wdm0 > And it is not easy to find real hardware device for the QTRT > node. > No matter the hardware interface between AP and Qualcomm > modem is USB/PCIE/SMD, > (SMD is for QUALCOMM inter-AP, like MDM, APQ,MSM chipset.) > There always have QMI channel can be used to send QMI message. > If use these QMI channel, very few modifies is need apply to > libqmi and MM I cannot comment on this, probably @Eric Caruso has a better point of view. > > This logic you're suggesting, except for step 5 I think, is what we already > > had in > > mind more or less. I think we could have that done by ModemManager, and > > connection managers like NetworkManager will get that working automatically, > > but now I also fear that if we change the default QMI modem behavior w.r.t. > > which is the connected network interface, we may be breaking user setups out > > there that rely on fixed interface names (e.g. for iptables rules). > > > > > rmnet_dataX is not a good name, there may be rment_usb0, > > rment_mhi0 exist at the same time, so can named as rment_usb0_X. > > > > In qmi_wwan, the muxed interfaces are named qmimux0, qmimux1 ... And if you > > have 5 different modems, each of them instantiating muxed virtual > > interfaces, > > they'll just get assigned sequential interface names, without any reference > > in the > > interface name to which physical interface they're mapped to. I don't think > > the > > name of the interface matters much at this point, as long as we can know > > which > > virtual interface maps to which physical interface. > > > > E.g. qmi_wwan adds a "lower_wwan0" file in the virtual interface sysfs > > contents > > pointing to the physical interface: > > # ls -ltr /sys/class/net/qmimux0/lower_wwan0 > > lrwxrwxrwx1 root root 0 Oct 17 20:52 > > /sys/class/net/qmimux0/lower_wwan0 -> > > ../../../pci:00/:00:16.0/usb1/1-1/1-1.2/1-1.2:1.8/net/wwan0 > > > > And it adds a "upper_qmimux0" file in the physical interface sysfs pointing > > to the > > virtual device: > > # ls -ltr /sys/class/net/wwan0/upper_qmimux0 > > lrwxrwxrwx1 root root 0 Oct 17 20:53 > > /sys/class/net/wwan0/upper_qmimux0 -> > > ../../../../../../../../virtual/net/qmimux0 > > > > As long as we have this kind of relationship between the virtual and > > physical > > interfaces, we're good to go, regardless of the actual naming of the > > interface. > [carl.yin] developer can find the relationship of wwanX and qmimuxX. > But for the end customers, maybe it is difficult, maybe he > want to setup data call on PDN-x on modem y. > It will be Very intuitive if we can make PDN-x to muxid-X to > rmnet_modemY_X. I truly don't have a strong opinion on that, kernel devs probably have a better idea on what the naming should look like. Either way, it is true that the current kernel implementation for the muxing in qmi_wwan already relies on that naming mechanism, so not sure if that could be changed right now without breaking the API. -- Aleksander https://aleksander.es ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: Instantiating rmnet devices for data ports on QRTR-based modems
Hey, > > > So the process can next: > > > 1. rmnet physic driver probe, create netcard > > > rmnet_usb0/rmnet_mhi0/rmnet_ipa0, and call register_rmnet_data > > > 2. rmnet data driver create rmnet_data0 > > > > Are you suggesting that there is always a virtual network interface > > created for the physical network interface? In terms of qmi_wwan, are > > you suggesting a virtual qmimux0 is always created when the physical > > wwan0 interface is exposed? What would be the benefit of doing that? > > > > One of the benefits is dl throughput for high-cat modems, since > enabling data aggregation is mandatory to get the most out of the > modem. > That's a good point. > > > 3. MM query qmap-version, ep-type, iface-id, > > > dll_data_aggregation_max_size from rmnet physic driver > > > 4. MM send QMIWDS_ADMIN_SET_DATA_FORMAT_REQ to modem. > > > 5. MM get ul_data_aggregation_max_size from > > > QMIWDS_ADMIN_SET_DATA_FORMAT_RESP, and send to rmnet physic driver. > > > > @Bjørn Mork @Daniele Palmas is that step 5 something we're not > > currently doing and we may need to do? Does the physical wwan > > interface truly need to know the ul_data_aggregation_max_size? > > > > I can't find this WDS request, or do you mean QMI_WDA_SET_DATA_FORMAT? Yes, that's what he refers to. > Anyway, the driver is not dealing with ul values: so far I did not > receive complaints on upload performance, even with high-cat modems, > so I'm not sure this is something that should be fixed at the moment. > I'm more concerned about the dl aggregation, whose setting is still a > manual process. > Carl, could you elaborate on why the kernel driver would need to know about ul_data_aggregation_max_size? What is the downside of not implementing it? > By the way, not strictly related to this thread, but there is a very > good effort ongoing for upstreaming mhi driver, mainly targeted on > SDX55, see for example > https://patchwork.kernel.org/project/linux-arm-msm/patch/1602258664-9980-1-git-send-email-loic.poul...@linaro.org/ > and https://www.spinics.net/lists/netdev/msg692557.html > > The uci driver is still missing, but I think it will be just a matter > of time: the interesting part is that with uci a char qmi device is > exposed that, according to my current tests, is behaving at the high > level just like the cdc-wdm device. I believe Carl is already testing a setup like that one as well. https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/issues/254 -- Aleksander https://aleksander.es ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: Instantiating rmnet devices for data ports on QRTR-based modems
Hi all, Il giorno sab 17 ott 2020 alle ore 23:05 Aleksander Morgado ha scritto: > > Hey Carl, > > > Some ideas from me. > > Thanks for your comments, very much appreciated. > > > The discussion is based on QUALCOMM's rmnet driver and his data > > services. > > We are discussing how porting these software to MM and libqmi. > > But I think QUALLCOMM's solution is too complex, porting is > > impossible. > > What we have to do is re-implementation. > > > > I seem we have done for "QMI based on QTRT" in libqmi. > > For QUALCOMM's data services, there are lots of QMI library, and > > libqmi used a more simply solution. (in fact, if just send QMI, QTRT is > > also not a good idea) > > Why do you say QRTR is not a good idea? Isn't QMI over QRTR the only > way to handle the communication in certain setups? > > > > > So, others part of QUALCOMM's data services and QUALCOMM's rmnet > > drvier, we also can simplify. > > > > I'm all for simplifying :D > > > Take rmnet driver for example: > > QUALCOMM;s rmnet driver consist of two parts. > > 1. physic netcard part, name as > > rmnet_usb.c/rmnet_mhi.c/rmnet_ipa.c,, depend on the hardware interface used. > > this driver response for transfer QMAP packet to modem. > > 2. virtual nectar part, name as rmnet_data.c, this driver response > > for: rx IP packets from tcp/ip stack, and add QMAP header to IP packet, and > > sent to rmnet physic driver. > > > > Ok, understood. > > > So the process can next: > > 1. rmnet physic driver probe, create netcard > > rmnet_usb0/rmnet_mhi0/rmnet_ipa0, and call register_rmnet_data > > 2. rmnet data driver create rmnet_data0 > > Are you suggesting that there is always a virtual network interface > created for the physical network interface? In terms of qmi_wwan, are > you suggesting a virtual qmimux0 is always created when the physical > wwan0 interface is exposed? What would be the benefit of doing that? > One of the benefits is dl throughput for high-cat modems, since enabling data aggregation is mandatory to get the most out of the modem. > > 3. MM query qmap-version, ep-type, iface-id, > > dll_data_aggregation_max_size from rmnet physic driver > > 4. MM send QMIWDS_ADMIN_SET_DATA_FORMAT_REQ to modem. > > 5. MM get ul_data_aggregation_max_size from > > QMIWDS_ADMIN_SET_DATA_FORMAT_RESP, and send to rmnet physic driver. > > @Bjørn Mork @Daniele Palmas is that step 5 something we're not > currently doing and we may need to do? Does the physical wwan > interface truly need to know the ul_data_aggregation_max_size? > I can't find this WDS request, or do you mean QMI_WDA_SET_DATA_FORMAT? Anyway, the driver is not dealing with ul values: so far I did not receive complaints on upload performance, even with high-cat modems, so I'm not sure this is something that should be fixed at the moment. I'm more concerned about the dl aggregation, whose setting is still a manual process. > > 6. MM want to setup data call on PDN-x. MM send muxid-X to rmnet > > physic driver, rmnet physic driver tell rmnet data driver to create > > rmnet_dataX. > > > > This logic you're suggesting, except for step 5 I think, is what we > already had in mind more or less. I think we could have that done by > ModemManager, and connection managers like NetworkManager will get > that working automatically, but now I also fear that if we change the > default QMI modem behavior w.r.t. which is the connected network > interface, we may be breaking user setups out there that rely on fixed > interface names (e.g. for iptables rules). > > > rmnet_dataX is not a good name, there may be rment_usb0, rment_mhi0 > > exist at the same time, so can named as rment_usb0_X. > > In qmi_wwan, the muxed interfaces are named qmimux0, qmimux1 ... And > if you have 5 different modems, each of them instantiating muxed > virtual interfaces, they'll just get assigned sequential interface > names, without any reference in the interface name to which physical > interface they're mapped to. I don't think the name of the interface > matters much at this point, as long as we can know which virtual > interface maps to which physical interface. > > E.g. qmi_wwan adds a "lower_wwan0" file in the virtual interface sysfs > contents pointing to the physical interface: > # ls -ltr /sys/class/net/qmimux0/lower_wwan0 > lrwxrwxrwx1 root root 0 Oct 17 20:52 > /sys/class/net/qmimux0/lower_wwan0 -> > ../../../pci:00/:00:16.0/usb1/1-1/1-1.2/1-1.2:1.8/net/wwan0 > > And it adds a "upper_qmimux0" file in the physical interface sysfs > pointing to the virtual device: > # ls -ltr /sys/class/net/wwan0/upper_qmimux0 > lrwxrwxrwx1 root root 0 Oct 17 20:53 > /sys/class/net/wwan0/upper_qmimux0 -> > ../../../../../../../../virtual/net/qmimux0 > > As long as we have this kind of relationship
Re: Instantiating rmnet devices for data ports on QRTR-based modems
Hey Carl, > Some ideas from me. Thanks for your comments, very much appreciated. > The discussion is based on QUALCOMM's rmnet driver and his data > services. > We are discussing how porting these software to MM and libqmi. > But I think QUALLCOMM's solution is too complex, porting is > impossible. > What we have to do is re-implementation. > > I seem we have done for "QMI based on QTRT" in libqmi. > For QUALCOMM's data services, there are lots of QMI library, and > libqmi used a more simply solution. (in fact, if just send QMI, QTRT is also > not a good idea) Why do you say QRTR is not a good idea? Isn't QMI over QRTR the only way to handle the communication in certain setups? > > So, others part of QUALCOMM's data services and QUALCOMM's rmnet > drvier, we also can simplify. > I'm all for simplifying :D > Take rmnet driver for example: > QUALCOMM;s rmnet driver consist of two parts. > 1. physic netcard part, name as rmnet_usb.c/rmnet_mhi.c/rmnet_ipa.c,, > depend on the hardware interface used. > this driver response for transfer QMAP packet to modem. > 2. virtual nectar part, name as rmnet_data.c, this driver response > for: rx IP packets from tcp/ip stack, and add QMAP header to IP packet, and > sent to rmnet physic driver. > Ok, understood. > So the process can next: > 1. rmnet physic driver probe, create netcard > rmnet_usb0/rmnet_mhi0/rmnet_ipa0, and call register_rmnet_data > 2. rmnet data driver create rmnet_data0 Are you suggesting that there is always a virtual network interface created for the physical network interface? In terms of qmi_wwan, are you suggesting a virtual qmimux0 is always created when the physical wwan0 interface is exposed? What would be the benefit of doing that? > 3. MM query qmap-version, ep-type, iface-id, > dll_data_aggregation_max_size from rmnet physic driver > 4. MM send QMIWDS_ADMIN_SET_DATA_FORMAT_REQ to modem. > 5. MM get ul_data_aggregation_max_size from > QMIWDS_ADMIN_SET_DATA_FORMAT_RESP, and send to rmnet physic driver. @Bjørn Mork @Daniele Palmas is that step 5 something we're not currently doing and we may need to do? Does the physical wwan interface truly need to know the ul_data_aggregation_max_size? > 6. MM want to setup data call on PDN-x. MM send muxid-X to rmnet > physic driver, rmnet physic driver tell rmnet data driver to create > rmnet_dataX. > This logic you're suggesting, except for step 5 I think, is what we already had in mind more or less. I think we could have that done by ModemManager, and connection managers like NetworkManager will get that working automatically, but now I also fear that if we change the default QMI modem behavior w.r.t. which is the connected network interface, we may be breaking user setups out there that rely on fixed interface names (e.g. for iptables rules). > rmnet_dataX is not a good name, there may be rment_usb0, rment_mhi0 > exist at the same time, so can named as rment_usb0_X. In qmi_wwan, the muxed interfaces are named qmimux0, qmimux1 ... And if you have 5 different modems, each of them instantiating muxed virtual interfaces, they'll just get assigned sequential interface names, without any reference in the interface name to which physical interface they're mapped to. I don't think the name of the interface matters much at this point, as long as we can know which virtual interface maps to which physical interface. E.g. qmi_wwan adds a "lower_wwan0" file in the virtual interface sysfs contents pointing to the physical interface: # ls -ltr /sys/class/net/qmimux0/lower_wwan0 lrwxrwxrwx1 root root 0 Oct 17 20:52 /sys/class/net/qmimux0/lower_wwan0 -> ../../../pci:00/:00:16.0/usb1/1-1/1-1.2/1-1.2:1.8/net/wwan0 And it adds a "upper_qmimux0" file in the physical interface sysfs pointing to the virtual device: # ls -ltr /sys/class/net/wwan0/upper_qmimux0 lrwxrwxrwx1 root root 0 Oct 17 20:53 /sys/class/net/wwan0/upper_qmimux0 -> ../../../../../../../../virtual/net/qmimux0 As long as we have this kind of relationship between the virtual and physical interfaces, we're good to go, regardless of the actual naming of the interface. It is true, though, that with the current qmi_wwan logic we cannot force Qualcomm's relationship between mux ID and the name of the interface being ID-1, because the qmimux names are given sequentially as they're allocated. Not a big deal, though. -- Aleksander https://aleksander.es ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: Instantiating rmnet devices for data ports on QRTR-based modems
Hey, > > > And an additional question I have; what if we start using QMAP also > > > for qmi_wwan based modems that support it? I think it would be very > > > similar to your current needs with QRTR/IPA, right? See > > > https://lists.freedesktop.org/archives/libqmi-devel/2018-July/002935.html > > > Maybe we can setup a common API in libqmi that brings up the muxed > > > network interfaces for both cases in a common way? > > > > Yes, that exact thought stroke me as well while skimming through this > > discussion. It would be nice if the API could abstract away the > > differences wrt adding network devices etc, and just present a common > > muxing API. > > I haven't personally dealt with QMAP so I'm not really sure what's > going on there. The linked discussion does suggest a similar > architecture of one "real" net interface (wwan0 for QMAP, rmnet_ipa0 > for QRTR/IPA) over which several "virtual" net ports are overlaid > (qmimuxN for QMAP, rmnet_dataN for QRTR/IPA). > > Neither case would deal with QMI directly outside of the Bind Mux Data > Port message though, as far as I can tell. So I'm not sure if this is > an argument for keeping it in libqmi or not. > At this point, I start to prefer the idea of keeping it in libqmi, but the logic should not be automatic on QmiDevice open. libqmi could provide a common method to add/delete the muxed interfaces, and that method would be implemented differently depending on what kernel driver is in use. Then, upper layers like MM or other programs using qmicli could trigger the instantiation of the virtual net devices as part of the connection attempt procedure. -- Aleksander https://aleksander.es ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: Instantiating rmnet devices for data ports on QRTR-based modems
Aleksander Morgado writes: > And an additional question I have; what if we start using QMAP also > for qmi_wwan based modems that support it? I think it would be very > similar to your current needs with QRTR/IPA, right? See > https://lists.freedesktop.org/archives/libqmi-devel/2018-July/002935.html > Maybe we can setup a common API in libqmi that brings up the muxed > network interfaces for both cases in a common way? Yes, that exact thought stroke me as well while skimming through this discussion. It would be nice if the API could abstract away the differences wrt adding network devices etc, and just present a common muxing API. Bjørn ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: Instantiating rmnet devices for data ports on QRTR-based modems
Hey Eric, And an additional question I have; what if we start using QMAP also for qmi_wwan based modems that support it? I think it would be very similar to your current needs with QRTR/IPA, right? See https://lists.freedesktop.org/archives/libqmi-devel/2018-July/002935.html Maybe we can setup a common API in libqmi that brings up the muxed network interfaces for both cases in a common way? -- Aleksander https://aleksander.es ___ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
Re: Instantiating rmnet devices for data ports on QRTR-based modems
Hey,
> > > It does mean that MM will have to instantiate net ports and
> > > then assign the mux ID to the QmiDevice, and then set it again in the
> > > Bind Mux Data Port message, which leaves room for those potentially
> > > getting out of sync.
> >
> > Why would this not be a problem in the case of having everything in
> > libqmi? because you already know the mux ID in the QmiDevice and it
> > would be "automatically" set in the Bind Mux Data Port TLV? Not sure I
> > like that, and not sure if I'm understanding that flow correctly. The
> > potential out of sync operation would also happen in the case of
> > having everything in libqmi. But I don't think that's a great issue
> > really, we can probably have proper logic to handle all that.
>
> libqmi wouldn't do any magic here, but all MM would have to do in the
> other case is ask the QmiDevice for its mux ID, which the device
> instantiated automatically for itself (or got from the command line,
> potentially, if it's qmicli) and then plug that number into the WDS
> message. This isn't a huge concern at all; it's just something that I
> figure could get lost in the noise and in general I'd like to have as
> few points of "redundant" configuration like this as possible.
>
I see.
> > > It also means that under more generalized
> > > operation (where we don't necessarily follow the rmnet_dataX = mux ID
> > > X+1 convention[3]) we'd still need to use netlink from libqmi to fetch
> > > all of the rmnet interfaces and inspect their mux ID properties to
> > > find a match when performing reload_wwan_iface.
> >
> > The need to have the proper network interface loaded by
> > reload_wwan_iface(), if not using sysfs, is low. Yes, we do have that
> > API in the library, and yes that API currently works perfectly with
> > qmi_wwan and sysfs, but if you ask me, there is not much purpose for
> > that netlink based generic logic otherwise.
>
> Mostly this is for robustness -- if we assume the convention *always*
> holds, then no such netlink work is necessary; we just do
> sprintf("rmnet_data%u", mux_id - 1) and call it a day.
Ok, so in the case of implementing all this as part of the QmiDevice
logic, libqmi would take care of the mux_id vs net interface mapping,
it would expose which mux id to use during the connection attempt, and
it would also take care of bringing up the network interface
associated to that mux id. Right?
> But something
> outside of MM might mess with this, and the bugs that would cause
> might be incredibly hard to find in MM logs alone, I think.
>
Oh, this is something that, while it may happen, shouldn't be our main
focus. We shouldn't e.g. over-complicate the solution just because
there may be something out there messing around. At the end, even
malloc() may fail. If we provide a solution that makes all this
"automagically work", it should be fine to assume that no other
software will try to mess around. Doesn't your MR already assume that
rmnet_dataX maps to X-1 mux id?
> One way or the other we are going to need to use netlink to find an
> unused net interface name, so I don't think implementing the generic
> logic is much harder.
>
That is true.
> >
> > > (On an abstract level,
> > > this seems as valid a use for netlink for libqmi as it is valid for
> > > libqmi to crawl sysfs for net interface details. But it does increase
> > > the total complexity of the code, since MM and libqmi are essentially
> > > exchanging information through the kernel.)
> > >
> >
> > Your point comparing the sysfs crawling vs netlink makes sense. It's
> > one of the only places where libqmi does something other than plain
> > QMI protocol operations, and we did that to allow nicer qmicli
> > actions. In the case of MM, we're using that sysfs crawling inside
> > libqmi only when MM calls qmi_device_set_expected_data_format()
> > though, that is the only place where reload_wwan_iface_name() is run.
> > From MM's point of view there is no need to find what net interface
> > applies to a given QMI control port, but we already do the opposite,
> > looking for what control port applies to a given net port. If MM is
> > going to instantiate the network interfaces itself, there's also not
> > much point on having libqmi look for the interface associated to a
> > QmiDevice; as we already know it in advance.
>
> Actually, fussing around with the expected data format is not even
> necessary for QRTR-based modems as far as I can tell. I think they all
> use raw IP and there's nowhere equivalent to the sysfs nodes to even
> work with expected data format.
>
Yes yes, that was just an example. I do assume we're going to work
with raw-ip only from now on.
> > > One thing that we'll have to consider in the future, though not right
> > > now, is how each design works with multiple PDN operation -- I think
> > > the design implied by the libqmi MR[1] makes this difficult, but
> > > libqmi also seems to make the assumption that we only associate each
> > > device with
Re: Instantiating rmnet devices for data ports on QRTR-based modems
Thanks for taking a look, see responses inline.
On Thu, Oct 15, 2020 at 12:35 PM Aleksander Morgado
wrote:
>
> > However, delegating it to MM also works. I think that the conceptual
> > design is cleaner here as interacting with netlink to create
> > interfaces isn't really something that makes a ton of sense to leave
> > in libqmi.
>
> Yes, that is something I suggested we should avoid.
>
> > It does mean that MM will have to instantiate net ports and
> > then assign the mux ID to the QmiDevice, and then set it again in the
> > Bind Mux Data Port message, which leaves room for those potentially
> > getting out of sync.
>
> Why would this not be a problem in the case of having everything in
> libqmi? because you already know the mux ID in the QmiDevice and it
> would be "automatically" set in the Bind Mux Data Port TLV? Not sure I
> like that, and not sure if I'm understanding that flow correctly. The
> potential out of sync operation would also happen in the case of
> having everything in libqmi. But I don't think that's a great issue
> really, we can probably have proper logic to handle all that.
libqmi wouldn't do any magic here, but all MM would have to do in the
other case is ask the QmiDevice for its mux ID, which the device
instantiated automatically for itself (or got from the command line,
potentially, if it's qmicli) and then plug that number into the WDS
message. This isn't a huge concern at all; it's just something that I
figure could get lost in the noise and in general I'd like to have as
few points of "redundant" configuration like this as possible.
> > It also means that under more generalized
> > operation (where we don't necessarily follow the rmnet_dataX = mux ID
> > X+1 convention[3]) we'd still need to use netlink from libqmi to fetch
> > all of the rmnet interfaces and inspect their mux ID properties to
> > find a match when performing reload_wwan_iface.
>
> The need to have the proper network interface loaded by
> reload_wwan_iface(), if not using sysfs, is low. Yes, we do have that
> API in the library, and yes that API currently works perfectly with
> qmi_wwan and sysfs, but if you ask me, there is not much purpose for
> that netlink based generic logic otherwise.
Mostly this is for robustness -- if we assume the convention *always*
holds, then no such netlink work is necessary; we just do
sprintf("rmnet_data%u", mux_id - 1) and call it a day. But something
outside of MM might mess with this, and the bugs that would cause
might be incredibly hard to find in MM logs alone, I think.
One way or the other we are going to need to use netlink to find an
unused net interface name, so I don't think implementing the generic
logic is much harder.
>
> > (On an abstract level,
> > this seems as valid a use for netlink for libqmi as it is valid for
> > libqmi to crawl sysfs for net interface details. But it does increase
> > the total complexity of the code, since MM and libqmi are essentially
> > exchanging information through the kernel.)
> >
>
> Your point comparing the sysfs crawling vs netlink makes sense. It's
> one of the only places where libqmi does something other than plain
> QMI protocol operations, and we did that to allow nicer qmicli
> actions. In the case of MM, we're using that sysfs crawling inside
> libqmi only when MM calls qmi_device_set_expected_data_format()
> though, that is the only place where reload_wwan_iface_name() is run.
> From MM's point of view there is no need to find what net interface
> applies to a given QMI control port, but we already do the opposite,
> looking for what control port applies to a given net port. If MM is
> going to instantiate the network interfaces itself, there's also not
> much point on having libqmi look for the interface associated to a
> QmiDevice; as we already know it in advance.
Actually, fussing around with the expected data format is not even
necessary for QRTR-based modems as far as I can tell. I think they all
use raw IP and there's nowhere equivalent to the sysfs nodes to even
work with expected data format.
> > One thing that we'll have to consider in the future, though not right
> > now, is how each design works with multiple PDN operation -- I think
> > the design implied by the libqmi MR[1] makes this difficult, but
> > libqmi also seems to make the assumption that we only associate each
> > device with one net port anyway, so we may have to unwind that
> > assumption no matter what. The design where MM instantiates net ports
> > might not run into this issue in so many places.
> >
>
> The connection logic in MM is always based on looking for an "unused
> net port" and then looking for the matching control port that can get
> that net port connected. In the case of QRTR/IPA, there is a "master"
> network interface already and we instantiate new network interfaces on
> demand for each new connection that we want to make. I think this
> instantiation would make sense inside MM; but see my last comment.
>
> > [1]
>
Re: Instantiating rmnet devices for data ports on QRTR-based modems
Hey! > > As part of the QRTR integration into MM we're trying to figure out > whose responsibility it is to make rmnet_data net interfaces for a > QRTR/IPA-based modem. I had started an MR[1] that gave this > responsibility to libqmi, but it's not clear this is the right place > for this to go. > Thanks for bringing this to the list, I hope we can get several points of view on the topic. > If we leave it in libqmi, then ModemManager has to do very little > extra setup: it can just grab the mux ID from the QmiDevice and push > it to the modem via Bind Mux Data Port in the connect step[2]. The > QmiDevice can also cache the interface name it selected when creating > the net port to return from reload_wwan_iface. > Ok. > However, delegating it to MM also works. I think that the conceptual > design is cleaner here as interacting with netlink to create > interfaces isn't really something that makes a ton of sense to leave > in libqmi. Yes, that is something I suggested we should avoid. > It does mean that MM will have to instantiate net ports and > then assign the mux ID to the QmiDevice, and then set it again in the > Bind Mux Data Port message, which leaves room for those potentially > getting out of sync. Why would this not be a problem in the case of having everything in libqmi? because you already know the mux ID in the QmiDevice and it would be "automatically" set in the Bind Mux Data Port TLV? Not sure I like that, and not sure if I'm understanding that flow correctly. The potential out of sync operation would also happen in the case of having everything in libqmi. But I don't think that's a great issue really, we can probably have proper logic to handle all that. > It also means that under more generalized > operation (where we don't necessarily follow the rmnet_dataX = mux ID > X+1 convention[3]) we'd still need to use netlink from libqmi to fetch > all of the rmnet interfaces and inspect their mux ID properties to > find a match when performing reload_wwan_iface. The need to have the proper network interface loaded by reload_wwan_iface(), if not using sysfs, is low. Yes, we do have that API in the library, and yes that API currently works perfectly with qmi_wwan and sysfs, but if you ask me, there is not much purpose for that netlink based generic logic otherwise. > (On an abstract level, > this seems as valid a use for netlink for libqmi as it is valid for > libqmi to crawl sysfs for net interface details. But it does increase > the total complexity of the code, since MM and libqmi are essentially > exchanging information through the kernel.) > Your point comparing the sysfs crawling vs netlink makes sense. It's one of the only places where libqmi does something other than plain QMI protocol operations, and we did that to allow nicer qmicli actions. In the case of MM, we're using that sysfs crawling inside libqmi only when MM calls qmi_device_set_expected_data_format() though, that is the only place where reload_wwan_iface_name() is run. >From MM's point of view there is no need to find what net interface applies to a given QMI control port, but we already do the opposite, looking for what control port applies to a given net port. If MM is going to instantiate the network interfaces itself, there's also not much point on having libqmi look for the interface associated to a QmiDevice; as we already know it in advance. > One thing that we'll have to consider in the future, though not right > now, is how each design works with multiple PDN operation -- I think > the design implied by the libqmi MR[1] makes this difficult, but > libqmi also seems to make the assumption that we only associate each > device with one net port anyway, so we may have to unwind that > assumption no matter what. The design where MM instantiates net ports > might not run into this issue in so many places. > The connection logic in MM is always based on looking for an "unused net port" and then looking for the matching control port that can get that net port connected. In the case of QRTR/IPA, there is a "master" network interface already and we instantiate new network interfaces on demand for each new connection that we want to make. I think this instantiation would make sense inside MM; but see my last comment. > [1] > https://gitlab.freedesktop.org/mobile-broadband/libqmi/-/merge_requests/135 > [2] > https://chromium.googlesource.com/chromiumos/third_party/modemmanager-next/+/dbee3f23a9c27d8bf3f5e634dabb41ef856867f8 > [3] This convention was alluded to by QC, but there's no magic in it > and a user could easily break this with two invocations of the "ip" > command, which might cause some tricky bugs. So I don't know that we > should try to keep the convention, and instead just find any open > rmnet_dataX and mux ID and link those together. > If it doesn't cost us much, I don't see why we wouldn't follow the same convention, although not a strong opinion. So overall, my opinion is that instantiating the
