Project

General

Profile

Actions

Bug #3426

closed

gprs_llc_process_xid_ind() does not echo L3_PAR, breaks PDP activation for Motorola KRZR

Added by keith over 5 years ago. Updated over 3 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
-
Target version:
-
Start date:
07/27/2018
Due date:
% Done:

100%

Spec Reference:

Description

In gprs_llc_process_xid_ind()
(at the time of writing) http://cgit.osmocom.org/osmo-sgsn/tree/src/gprs/gprs_llc.c#n227

We check if (xid_field->type != GPRS_LLC_XID_T_L3_PAR)
before echoing the XID to the MS

The Motorola KRZR responds by sending the XID list again and repeats 4 times before giving up and sending PDP deactivate context, with a cause 25 "LLC or SNDCP Failure"

Removing the check for xid_field->type != GPRS_LLC_XID_T_L3_PAR and echoing the L3 parameter back results in PDP activation success and GPRS session works.

Why this check? Why do we not echo all the xid fields back? Did it break with some other phone?


Files

krzr_llc.pre_patch.pcap krzr_llc.pre_patch.pcap 1.6 KB keith, 04/29/2019 05:44 PM
krzr_llc.post_patch.pcap krzr_llc.post_patch.pcap 1.11 KB keith, 04/29/2019 05:44 PM
Actions #1

Updated by laforge over 5 years ago

I purchased a KRZR 3 for the sysmocom lab. Not sure when somebody will be able to try to reproduce, but at least we should be able to do now.

Actions #2

Updated by keith over 5 years ago

I've been running SGSN now for some weeks with this patch in place to allow the KRZR to attach and I have not seen any related problems. (with any other phone)

Actions #3

Updated by keith over 5 years ago

TS 04.06 [EDIT correction TS 04.64 6.4.1.6 ] is kind of confusing here.

On the one had it says:
As an optimisation, parameters confirming the requested values may be omitted from the XID response.

also:

The responding side may respond with parameters that were not included in the XID command. A parameter that was
not included in the XID command shall in this case be treated as if the current value of the parameter was included in
the XID command. The responding side shall include such a parameter in every XID response until the parameter has
been explicitly negotiated
, either by responding to an XID command that included the parameter, or by explicitly
including the parameter the next time an XID command is transmitted.

also:

Negotiated XID parameters shall apply to the LLE identified by the DLCI of the XID frames used, except Version,
Reset, and IOV-UI that applies to an LLME (i.e., a TLLI), and except Layer-3 Parameters that apply to the layer 3
above the LLE
.

So from that I understand that while we may not actually be applying this L3 param to the LLE identified by the DLCI (to be honest, I haven't even looked up what the dlci is), we still MUST include it in the response.

Maybe this is a mis understanding that is the origin of this

if (xid_field->type != GPRS_LLC_XID_T_L3_PAR)

in the code?

Ref: https://www.etsi.org/deliver/etsi_ts/101300_101399/101351/08.07.00_60/ts_101351v080700p.pdf#page=29

Actions #4

Updated by keith almost 5 years ago

  • Assignee set to dexter
Actions #5

Updated by keith almost 5 years ago

note to self: attach patch and .pcap

Actions #6

Updated by laforge almost 5 years ago

The point is that the L3 parameters are handled in SNDCP, and not in LLC. So the LLC code is correct to only handle the non-L3. The behavioral difference (and possible bug) hence is in the SNDCP XID handling.

What we need is a pcap file showing request and response for both the modified and the unmodified code.

Actions #7

Updated by laforge almost 5 years ago

Actions #8

Updated by lynxis almost 5 years ago

The interesting function might be the sndcp_sn_xid_ind() in the sgsn.

Actions #10

Updated by keith almost 5 years ago

  • Assignee changed from dexter to keith
Actions #11

Updated by laforge almost 5 years ago

  • Status changed from New to In Progress

I'm writing a testcase for this. It should be reasonably simple to send a MO XID with empty L3, and check the response.

note: We don't really have reasonable XID related tests yet.

Actions #12

Updated by keith over 3 years ago

  • Status changed from In Progress to Feedback
  • % Done changed from 0 to 90

Hi laforge Was that test case ever merged? Maybe we can reference it and then close this.

Actions #13

Updated by laforge over 3 years ago

  • Status changed from Feedback to In Progress

keith wrote:

Hi laforge Was that test case ever merged? Maybe we can reference it and then close this.

commit 2aaac1b1d1655732055e6ad5da5965fe94af33e5
Author: Harald Welte <laforge@gnumonks.org>
Date:   Thu May 2 10:02:53 2019 +0200

    sgsn: Add initial XID handshake related tests

    Change-Id: I5d4b3cba2fe05dffe10c843f16cfec343bc67b5f

That commit includes a case with empty L3: TC_xid_empty_l3

Actions #14

Updated by laforge over 3 years ago

  • Status changed from In Progress to Resolved
  • % Done changed from 90 to 100
Actions

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 48.8 MB)