Change in osmo-sgsn[master]: gprs_gmm: send Service Reject when no PDP ctxs are available.

Pau Espin Pedrol gerrit-no-reply at lists.osmocom.org
Tue Apr 23 19:15:12 UTC 2019


Pau Espin Pedrol has posted comments on this change. ( https://gerrit.osmocom.org/13744 )

Change subject: gprs_gmm: send Service Reject when no PDP ctxs are available.
......................................................................


Patch Set 2:

(5 comments)

https://gerrit.osmocom.org/#/c/13744/2/src/gprs/gprs_gmm.c
File src/gprs/gprs_gmm.c:

https://gerrit.osmocom.org/#/c/13744/2/src/gprs/gprs_gmm.c@1620
PS2, Line 1620: static bool all_ms_ctx_present_on_sgsn(struct sgsn_mm_ctx *mmctx,
You probably want to add spec chapter and section in this comment too.


https://gerrit.osmocom.org/#/c/13744/2/src/gprs/gprs_gmm.c@1625
PS2, Line 1625: 	for (pdp_nsapi = 0; pdp_nsapi < 16; pdp_nsapi++) {
where does this 16 come from? Can we please have a define (if we don't have already) for it?


https://gerrit.osmocom.org/#/c/13744/2/src/gprs/gprs_gmm.c@1628
PS2, Line 1628: 			active = (1 << pdp_nsapi) & pdp_status[0];
While fine at runtime, you are actually assigning an integer value (bitmask) to a bool here, that's weird. Either have active an uint16_t or add some " != 0" at the end.


https://gerrit.osmocom.org/#/c/13744/2/src/gprs/gprs_gmm.c@1630
PS2, Line 1630: 			active = (1 << (pdp_nsapi - 8)) & pdp_status[1];
Can we actually have something like:

for (pdp_nsapi = 0; pdp_nsapi < 16; pdp_nsapi++) {

if (!(pdp_status[pdp_nsapi/8] & (1 << pdp_nsapi)))
   continue; /* not active in MS */

if (!sgsn_pdp_ctx_by_nsapi(mmctx, pdp_nsapi))
   return false; /* active in MS but not in sgsn */
}

return true;


https://gerrit.osmocom.org/#/c/13744/2/src/gprs/gprs_gmm.c@1920
PS2, Line 1920: 	/* Look at PDP Context Status IE and see if MS's view of
You probably need to drop this comment too right?



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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If610cbef17c25ec44e65d4f1b2340d102c560437
Gerrit-Change-Number: 13744
Gerrit-PatchSet: 2
Gerrit-Owner: Mykola Shchetinin <mykola at pentonet.com>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Mykola Shchetinin <mykola at pentonet.com>
Gerrit-Reviewer: lynxis lazus <lynxis at fe80.eu>
Gerrit-CC: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Comment-Date: Tue, 23 Apr 2019 19:15:12 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190423/60b16a78/attachment.html>


More information about the gerrit-log mailing list