[review] Support connection checks in DirectIP (sierra-legacy plugin)

[Bjørn Mork]

Aleksander Morgado <aleksander at aleksander.es> writes:

> Hey!
>
>> > Devices like the AC313U require explicit context monitoring with !SCACT?.
>> > Otherwise, we may end up with the modem reported as connected in MM, with
>> > IP settings in the network interface and all, but without a real connection
>> > inside the modem.
>>
>> Really?  I must admit that it's been 5 years or more since the last time
>> I tested DirectIP, but reading the driver suggests that it should react
>> to link down events from the firmware:
>>
>>
>> static void sierra_net_handle_lsi(struct usbnet *dev, char *data,
>>                 struct hip_hdr  *hh)
>> {
>>         struct sierra_net_data *priv = sierra_net_get_private(dev);
>>         int link_up;
>>
>>         link_up = sierra_net_parse_lsi(dev, data + hh->hdrlen,
>>                                         hh->payload_len.word);
>>         if (link_up < 0) {
>>                 netdev_err(dev->net, "Invalid LSI\n");
>>                 return;
>>         }
>>         if (link_up) {
>>                 sierra_net_set_ctx_index(priv, hh->msgspecific.byte);
>>                 priv->link_up = 1;
>>         } else {
>>                 priv->link_up = 0;
>>         }
>>         usbnet_link_change(dev, link_up, 0);
>> }
>>
>>
>> Doesn't this work? If so, is the event missing or do we fail to parse
>> it?  Or is the problem that MM doesn't consider the netdev link state?
>>
>
> I've seen several "Invalid LSI" errors reported, although not sure if
> that would be totally related to the disconnection management here.

Might be.  I've seen those as well. I guess i can mean that we fail to
parse a link down event from the firmware.

The driver will set the link state down if the LSI "session_state" is
SIERRA_NET_SESSION_IDLE (0) or "coverage" is SIERRA_NET_COVERAGE_NONE
(0) or SIERRA_NET_COVERAGE_NOPACKET (1). Which I'll just have to assume
makes sense since I dont' have any DirectIP docs.  But the latter test
is only done after validating "link_type" as either IPv4 or IPv6 for
single stack sessions.  And that smells fishy.

I know we've seen a number of "Invalid LSI" errors paired with "Link
type unsupported: 0xff".  I suspect we might get all link down events
with "link_type" == 0xff.  There is no reason to assume a valid link
type when the session is down...

> What I'm sure is that I've seen the AC313U reported as connected in
> MM, with the wwan interface up and with IP settings, and when querying
> SCACT? it would tell me that no context was connected (and of course
> there was no traffic).
>
> I fail to see if this issue is only seen in the AC313U (this is where
> I've seen it) or in all DirectIP modems, so just added it for all.
>
> I do *not* have a AC313U myself, so I'm very limited to what I can
> test or try really. I'll try to get one off ebay.

I do have an MC7710 somewhere, which should still be able to do
DirectIP.  But I don't know if there is any point in bothering.  If any
version of the driver fails to parse the link down events for any modem,
then I guess MM has to deal with that anyway. And then it might as well
use the AT method everywhere.

Go ahead! Sorry for the confusion.


Bjørn