pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/27262 )
Change subject: Revert "sgsn: Handle different levels of QoS" ......................................................................
Revert "sgsn: Handle different levels of QoS"
This reverts commit 4bd931f96d75d3e71b73a06e67f84ffdcab9caf6.
The commit was wrong, and previous code is correct. Relevant specs: * TS 29.060 7.7.34 Quality of Service (QoS) Profile * TS 24.008 10.5.6.5 Quality of service
As can be seen in TS 24.008 10.5.6.5, OSMO_IE_GSM_REQ_QOS never comes with the the ARP byte prepended. This is actually always prepended when sending the GTP message, as explained in TS 29.060 7.7.34.
As a result, the Qos Service sent in Create PDP Context Request sent to the GGSN contained wrongly formatted Qos Profile IE, which was observed checking wireshark with a real phone. This was found due to open5gs-smfd being more strict about the possible lengths of the IE, since the wrongly formatted IE send in GTP had length=14, which is incorrect due to folllowing TS 24.008 10.5.6.5 wording: "Octets 15-22 are optional. If octet 15 is included, then octet 16 shall also be included, and octets 17-22may be included." In this case, due to the wrong format it was seen as including octet 15 but not 16.
Change-Id: I4fc5ab823a27d27482858a7459337a2f8ae593c3 Related: SYS#5793 --- M src/sgsn/sgsn_libgtp.c 1 file changed, 5 insertions(+), 12 deletions(-)
Approvals: Jenkins Builder: Verified fixeria: Looks good to me, but someone else must approve laforge: Looks good to me, approved
diff --git a/src/sgsn/sgsn_libgtp.c b/src/sgsn/sgsn_libgtp.c index 1ac3b44..6fb3adc 100644 --- a/src/sgsn/sgsn_libgtp.c +++ b/src/sgsn/sgsn_libgtp.c @@ -228,18 +228,11 @@ qos = TLVP_VAL(tp, OSMO_IE_GSM_REQ_QOS); }
- if (qos_len <= 3) { - pdp->qos_req.l = qos_len + 1; - if (pdp->qos_req.l > sizeof(pdp->qos_req.v)) - pdp->qos_req.l = sizeof(pdp->qos_req.v); - pdp->qos_req.v[0] = 0; /* Allocation/Retention policy */ - memcpy(&pdp->qos_req.v[1], qos, pdp->qos_req.l - 1); - } else { - pdp->qos_req.l = qos_len; - if (pdp->qos_req.l > sizeof(pdp->qos_req.v)) - pdp->qos_req.l = sizeof(pdp->qos_req.v); - memcpy(pdp->qos_req.v, qos, pdp->qos_req.l); - } + pdp->qos_req.l = qos_len + 1; + if (pdp->qos_req.l > sizeof(pdp->qos_req.v)) + pdp->qos_req.l = sizeof(pdp->qos_req.v); + pdp->qos_req.v[0] = 0; /* Allocation/Retention policy */ + memcpy(&pdp->qos_req.v[1], qos, pdp->qos_req.l - 1);
/* charging characteristics if present */ if (TLVP_LEN(tp, OSMO_IE_GSM_CHARG_CHAR) >= sizeof(pdp->cch_pdp))
-
pespin