Attention is currently required from: fixeria, laforge.

lynxis lazus has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/39246?usp=email )

Change subject: gprs_gmm_util: add parsing of GMM Attach Requests ......................................................................

Patch Set 3:

(6 comments)

File src/sgsn/gprs_gmm_util.c:

https://gerrit.osmocom.org/c/osmo-sgsn/+/39246/comment/ac78f87d_cbcbb795?usp... : PS3, Line 93: l3 pointers must point to gmm

what does this even mean...

l3 pointer of msgb must point to the (start of) GMM

https://gerrit.osmocom.org/c/osmo-sgsn/+/39246/comment/2147b019_a681bd29?usp... : PS3, Line 94: rau_req

(copy-paste) `rau_req` -> `att_req`

Acknowledged

https://gerrit.osmocom.org/c/osmo-sgsn/+/39246/comment/2c77e7ed_0f9e7052?usp... : PS3, Line 97: struct msgb *msg

`const`? you're not modifying it, right?

Done

https://gerrit.osmocom.org/c/osmo-sgsn/+/39246/comment/c8c3a175_be17578d?usp... : PS3, Line 119: 21

where this magic number is coming from? do we have a define for it?

The problem here are the dynamic, but mandatory, IEs in form of LV in GMM. But there are multiple dynamic LV IEs in the Attach Req. So the GMM Attach Req can't be smaller then 25 bytes. After MS Network Capability come at least 21 bytes.

https://gerrit.osmocom.org/c/osmo-sgsn/+/39246/comment/c5a33036_62bf4f34?usp... : PS3, Line 122: if (len == 0)

"must be at least 2 bytes long" suggests `len < 2`?

Done

https://gerrit.osmocom.org/c/osmo-sgsn/+/39246/comment/8efafa3b_bd71876f?usp... : PS3, Line 143: 12

likewise

See the mention upwards. I wouldn't use a define for a value here.