On Thu, 2011-05-19 at 18:28 -0400, Nathan Williams wrote: > On Thu, May 19, 2011 at 4:54 PM, Dan Williams <d...@redhat.com> wrote: > > On Thu, 2011-05-12 at 19:02 -0400, Nathan Williams wrote: > >> In mm-generic-gsm.c, if an APN is not provided as a property to > >> simple_connect(), priv->cid is never set and remains at -1. > >> The Icera modem plugin has a _get_cid() wrapper call which checks for > >> this and returns 0 if the stored CID is -1. There are two problems > >> with this: > >> * 0 is not a valid CID; according to 3GPP 27.007, CIDs have a > >> minimum value of 1. Thus, if an APN was not provided, the plugin > >> issues commands like %IPDPCFG and %IPDPACT with CID=0, which the modem > >> rejects. > >> * connection_enabled() uses the priv->cid value rather than the > >> _get_cid() wrapped value, and bails out if it's -1, so a connection is > >> never acknowledged and the modem remains in "connecting" state. > > > > So we should never be using an invalid CID here, it should always be 1 > > or higher. I think the -1 was used as an error value that we should > > never hit, and there used to be assertions for that in the code. I > > think the whole dance here was just to be paranoid since we never ever > > want to send -1 to the modem, which happened before in some corner cases > > which have since been fixed. > > > > So we basically have to deal with the corner case of what happens if the > > code somehow screws up and we for whatever reason can't figure out the > > CID of the PDP context that we're trying to disconnect. I think > > returning "1" here should only happen for the disconnect case, since in > > all other cases, there would be a logic error to have an invalid CID and > > we should fail out, because that's a bug we need to fix. > > I'm a little puzzled. The issue I see is at connect time, if an APN > hasn't been specified. Is it a logic bug that we're not selecting > (perhaps arbitrarily) a CID in this case (in simple_state_machine)? > I'm not sure where the responsibility lies here.
I think the assumption is that an APN *has* been supplied, at least that's been the assumption all along. Marc Herbert and I had a discussion a while back about the "default" APN, where you simply pass nothing for the AP and create a PDP context with a blank APN and hope it works, which is what you might be expecting to happen here. At the moment everything expects an APN and if it half-works that's by coincidence. I did some testing last month with default APNs without ModemManager, just trying to see if they worked with the providers I have, and found that they did; I'm not sure if that's happened (at least for T-Mobile) in the past few years or what, but I could swear it didn't used to, and I'm sure some providers simply don't set up the default APN in their HLR. But Marc is right and we should allow a blank APN (or really, just don't add the APN to the Simple.Connect properties hash at all) during the connect attempt. If no APN is supplied, then MM would check for an existing PDP context with a blank APN, and if none was present, create one, and then dial using that CID. And then we just hope it works, and if not, hope the modem gives us a usable error code. Dan > - Nathan > > So I don't > > think the patch as sent is the right approach; perhaps we should > > g_assert(cid > 1) in the other cases, and be more relaxed for > > disconnect? Or in disconnect if we dont' get a valid CID, we just send > > "ATH" as a desperate attempt to kill everything since something went > > wrong... > > > > Dan > > > >> > >> I've attached a patch that (I think) fixes this, by having _get_cid() > >> return 1 and having connection_enabled() use it instead of > >> mm_generic_gsm_get_cid(); however, I have a feeling that the way it > >> works now is deliberate and I'm missing something about how the no-APN > >> case is supposed to work. > >> Thoughts? Or, if this analysis seems correct, the patch should be > >> ready to go. > >> > >> > >> - Nathan > >> > >> > >> _______________________________________________ > >> networkmanager-list mailing list > >> networkmanager-list@gnome.org > >> http://mail.gnome.org/mailman/listinfo/networkmanager-list > > > > > > _______________________________________________ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
