Change in osmo-sgsn[master]: LLC: Store the XID inside the LLC Entity, not LLC Mgmg Entity

Harald Welte gerrit-no-reply at lists.osmocom.org
Thu Apr 25 19:59:57 UTC 2019


Harald Welte has submitted this change and it was merged. ( https://gerrit.osmocom.org/13623 )

Change subject: LLC: Store the XID inside the LLC Entity, not LLC Mgmg Entity
......................................................................

LLC: Store the XID inside the LLC Entity, not LLC Mgmg Entity

For every logical session between a MS and the SGSN, there is one LLME
(LLC Management Entity) and a set of LLEs (Logical Link Entities): One
for each SAPI.

The XID procedure used to establish LLC configuration values such as
N201 (MTU) parameters happens on each LLE separately. The negotiated
parameters only affect that one LLE (SAPI) and are not global.

Still, the OsmoSGSN LLC code has the "struct llist_head *xid" member as
part of the gprs_llc_llme, and not as part of the gprs_llc_lle. This
list is a cache of the XID fields we have sent with the last XID
request, which is used in processing the response from the MS.

If two XID handshakes were to occur concurrently on two LLEs, the state
between them would get messed up. It must be maintained separately for
each LLE.

Closes: OS#3955
Change-Id: Iaeb54ca5ac58391be45e56c2e721f531969f3a9e
---
M include/osmocom/sgsn/gprs_llc.h
M src/gprs/gprs_llc.c
M src/gprs/gprs_sndcp.c
3 files changed, 25 insertions(+), 26 deletions(-)

Approvals:
  Jenkins Builder: Verified
  Harald Welte: Looks good to me, approved
  Pau Espin Pedrol: Looks good to me, approved



diff --git a/include/osmocom/sgsn/gprs_llc.h b/include/osmocom/sgsn/gprs_llc.h
index 376ae9a..711bcd6 100644
--- a/include/osmocom/sgsn/gprs_llc.h
+++ b/include/osmocom/sgsn/gprs_llc.h
@@ -145,6 +145,13 @@
 	unsigned int retrans_ctr;
 
 	struct gprs_llc_params params;
+
+	/* Copy of the XID fields we have sent with the last
+	 * network originated XID-Request. Since the phone
+	 * may strip the optional fields in the confirmation
+	 * we need to remeber those fields in order to be
+	 * able to create the compression entity. */
+	struct llist_head *xid;
 };
 
 #define NUM_SAPIS	16
@@ -169,13 +176,6 @@
 	uint16_t nsei;
 	struct gprs_llc_lle lle[NUM_SAPIS];
 
-	/* Copy of the XID fields we have sent with the last
-	 * network originated XID-Request. Since the phone
-	 * may strip the optional fields in the confirmation
-	 * we need to remeber those fields in order to be
-	 * able to create the compression entity. */
-	struct llist_head *xid;
-
 	/* Compression entities */
 	struct {
 		/* In these two list_heads we will store the
diff --git a/src/gprs/gprs_llc.c b/src/gprs/gprs_llc.c
index 2ec7100..acf4b54 100644
--- a/src/gprs/gprs_llc.c
+++ b/src/gprs/gprs_llc.c
@@ -53,7 +53,7 @@
 /* Generate XID message */
 static int gprs_llc_generate_xid(uint8_t *bytes, int bytes_len,
 				 struct gprs_llc_xid_field *l3_xid_field,
-				 struct gprs_llc_llme *llme)
+				 struct gprs_llc_lle *lle)
 {
 	/* Note: Called by gprs_ll_xid_req() */
 
@@ -90,8 +90,8 @@
 	}
 
 	/* Store generated XID for later reference */
-	talloc_free(llme->xid);
-	llme->xid = gprs_llc_copy_xid(llme, &xid_fields);
+	talloc_free(lle->xid);
+	lle->xid = gprs_llc_copy_xid(lle->llme, &xid_fields);
 
 	return gprs_llc_compile_xid(bytes, bytes_len, &xid_fields);
 }
@@ -99,7 +99,7 @@
 /* Generate XID message that will cause the GMM to reset */
 static int gprs_llc_generate_xid_for_gmm_reset(uint8_t *bytes,
 					       int bytes_len, uint32_t iov_ui,
-					       struct gprs_llc_llme *llme)
+					       struct gprs_llc_lle *lle)
 {
 	/* Called by gprs_llgmm_reset() and
 	 * gprs_llgmm_reset_oldmsg() */
@@ -124,8 +124,8 @@
 	llist_add(&xid_reset.list, &xid_fields);
 
 	/* Store generated XID for later reference */
-	talloc_free(llme->xid);
-	llme->xid = gprs_llc_copy_xid(llme, &xid_fields);
+	talloc_free(lle->xid);
+	lle->xid = gprs_llc_copy_xid(lle->llme, &xid_fields);
 
 	return gprs_llc_compile_xid(bytes, bytes_len, &xid_fields);
 }
@@ -144,8 +144,8 @@
 	struct gprs_llc_xid_field *xid_field_request_l3 = NULL;
 
 	/* Pick layer3 XID from the XID request we have sent last */
-	if (lle->llme->xid) {
-		llist_for_each_entry(xid_field_request, lle->llme->xid, list) {
+	if (lle->xid) {
+		llist_for_each_entry(xid_field_request, lle->xid, list) {
 			if (xid_field_request->type == GPRS_LLC_XID_T_L3_PAR)
 				xid_field_request_l3 = xid_field_request;
 		}
@@ -189,8 +189,8 @@
 	}
 
 	/* Flush pending XID fields */
-	talloc_free(lle->llme->xid);
-	lle->llme->xid = NULL;
+	talloc_free(lle->xid);
+	lle->xid = NULL;
 
 	return 0;
 }
@@ -325,8 +325,7 @@
 
 	/* Generate XID */
 	xid_bytes_len =
-	    gprs_llc_generate_xid(xid_bytes, sizeof(xid_bytes),
-				  l3_xid_field, lle->llme);
+	    gprs_llc_generate_xid(xid_bytes, sizeof(xid_bytes), l3_xid_field, lle);
 
 	/* Only perform XID sending if the XID message contains something */
 	if (xid_bytes_len > 0) {
@@ -577,7 +576,6 @@
 {
 	gprs_sndcp_comp_free(llme->comp.proto);
 	gprs_sndcp_comp_free(llme->comp.data);
-	talloc_free(llme->xid);
 	llist_del(&llme->list);
 	talloc_free(llme);
 }
@@ -1114,8 +1112,8 @@
 	}
 
 	/* Generate XID message */
-	xid_bytes_len = gprs_llc_generate_xid_for_gmm_reset(xid_bytes,
-					sizeof(xid_bytes),llme->iov_ui,llme);
+	xid_bytes_len = gprs_llc_generate_xid_for_gmm_reset(xid_bytes, sizeof(xid_bytes),
+							    llme->iov_ui, lle);
 	if (xid_bytes_len < 0)
 		return -EINVAL;
 	xid = msgb_put(msg, xid_bytes_len);
@@ -1135,6 +1133,7 @@
 			    struct gprs_llc_llme *llme)
 {
 	struct msgb *msg = msgb_alloc_headroom(4096, 1024, "LLC_XID");
+	struct gprs_llc_lle *lle = &llme->lle[sapi];
 	uint8_t xid_bytes[1024];
 	int xid_bytes_len, rc;
 	uint8_t *xid;
@@ -1148,8 +1147,8 @@
 	}
 
 	/* Generate XID message */
-	xid_bytes_len = gprs_llc_generate_xid_for_gmm_reset(xid_bytes,
-					sizeof(xid_bytes),llme->iov_ui,llme);
+	xid_bytes_len = gprs_llc_generate_xid_for_gmm_reset(xid_bytes, sizeof(xid_bytes),
+							    llme->iov_ui, lle);
 	if (xid_bytes_len < 0)
 		return -EINVAL;
 	xid = msgb_put(msg, xid_bytes_len);
diff --git a/src/gprs/gprs_sndcp.c b/src/gprs/gprs_sndcp.c
index f0239cb..23d1e9a 100644
--- a/src/gprs/gprs_sndcp.c
+++ b/src/gprs/gprs_sndcp.c
@@ -989,8 +989,8 @@
 	gprs_sndcp_comp_free(lle->llme->comp.data);
 	lle->llme->comp.proto = gprs_sndcp_comp_alloc(lle->llme);
 	lle->llme->comp.data = gprs_sndcp_comp_alloc(lle->llme);
-	talloc_free(lle->llme->xid);
-	lle->llme->xid = NULL;
+	talloc_free(lle->xid);
+	lle->xid = NULL;
 
 	/* Generate compression parameter bytestream */
 	xid_len = gprs_llc_gen_sndcp_xid(l3params, sizeof(l3params), nsapi);

-- 
To view, visit https://gerrit.osmocom.org/13623
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iaeb54ca5ac58391be45e56c2e721f531969f3a9e
Gerrit-Change-Number: 13623
Gerrit-PatchSet: 4
Gerrit-Owner: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190425/2b33632b/attachment.html>


More information about the gerrit-log mailing list