Instantiating rmnet devices for data ports on QRTR-based modems

[Eric Caruso]

Thanks for taking a look, see responses inline.

On Thu, Oct 15, 2020 at 12:35 PM Aleksander Morgado
<aleksander at aleksander.es> 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] 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 network interfaces on
> demand by ModemManager would make most sense, especially since it
> involves adding a new dependency on librmnetctl. I don't know if this
> librmnetctl has a "cli" though, something qmicli users could use to
> perform the same instantiation required to bring up the connection
> when MM is not used; is there something like that already? If there is
> some cli already, I would definitely suggest to take the MM
> integration.

The librmnetctl dependency is actually not necessary as it is a pretty
small wrapper around netlink. I wrote raw netlink in my MR to avoid
this dependency since I didn't know how easy it would be for users to
get a hold of the library, or if we might have licensing issues.

There is an associated cli[1] which might be useful for debugging, but
I've never used it. Utilities for working with rmnet devices are
actually also available as part of iproute2 as I mentioned before.
Bjorn has used these heavily in the past and can speak to how it works
and how users can set that up. If we choose not to use librmnetcl at
all, the relevant bits can be moved out of both MM and libqmi into a
separate, small library that can be used by both MM and qmicli if you
want to make those operations available from either flow, but the fact
that rmnet ports can be set up via iproute2 means this isn't strictly
necessary in my opinion.

> If there is no cli that qmicli users can use to instantiate the
> network interfaces, maybe the integration within libqmi would make
> sense, to be able to provide qmicli users with a complete solution.
> Although extending librmnetctl to implement a proper CLI there could
> also make more sense. My feel is that libqmi should be the last
> option, if there are other viable ones.
>
> One last thing regarding this integration, is that it looks like there
> is no "official" single librmnetctl release. That is something to
> solve out of MM/libqmi, unless we want to "incorporate" such a library
> within our sources and just use some given snapshot of it. Or just run
> the netlink operations ourselves.

This does seem like an issue with depending on librmnetctl. Again,
it's very small, so replicating its functionality shouldn't be too
hard (and I think most of it is already written out in the MR anyway).

>
> --
> Aleksander
> https://aleksander.es

[1] https://source.codeaurora.org/quic/la/platform/vendor/qcom-opensource/dataservices/tree/rmnetctl/cli

-Eric