Attention is currently required from: laforge, pespin, msuraev.

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-hnbgw/+/31431 )

Change subject: context map: introduce RUA and SCCP FSMs to fix leaks
......................................................................


Patch Set 6:

(5 comments)

File include/osmocom/hnbgw/context_map.h:

https://gerrit.osmocom.org/c/osmo-hnbgw/+/31431/comment/48b49e44_1257e896
PS6, Line 18:  * - For events that may send a RANAP message towards CN via 
SCCP, the msgb shall have reserved headrom to fit a struct
> typo: headroom
lol head-ROM =)


File src/osmo-hnbgw/context_map_rua.c:

https://gerrit.osmocom.org/c/osmo-hnbgw/+/31431/comment/25581165_5e5caf15
PS6, Line 57:  * Assumes local variable fi exists. */
> That's very confusing - better make no assumptions and pass all variables 
> explicitly. […]
I agree with the principle but this one is the exception; 'fi' is literally 
everywhere in an fsm implementation .c file. We use a macro like this in pretty 
much all osmo_fsm implementation .c files.


File src/osmo-hnbgw/context_map_sccp.c:

https://gerrit.osmocom.org/c/osmo-hnbgw/+/31431/comment/0054f0c6_281a465d
PS6, Line 63:  * Assumes local variable fi exists. */
> Same here.
Done


File src/osmo-hnbgw/hnbgw.c:

https://gerrit.osmocom.org/c/osmo-hnbgw/+/31431/comment/058fe4fa_a38f7bce
PS6, Line 862:  ranap_msg = msgb_alloc_c(OTC_SELECT, sizeof(struct 
osmo_scu_prim) + 1500, name);
> Where does 1500 comes from?
that's a good question, I took it from prior code. Actually, before, osmo-hnbgw 
would reserve 1500 in *total*, and would even subtract from that the headroom 
required for osmo_scu_prim; so apparently all RANAP messages that osmo-hnbgw 
processed so far are way shorter than that.
Maybe it comes from the common MTU = 1500?


File src/osmo-hnbgw/hnbgw_rua.c:

https://gerrit.osmocom.org/c/osmo-hnbgw/+/31431/comment/0082d02a_a53af90a
PS6, Line 230:  msg = msgb_alloc(1500, "rua_to_sccp");
(this is where I took 1500 from; note that it first puts the prim, and then the 
ranap in l2h below)



--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/31431
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: I6ff7e36532ff57c6f2d3e7e419dd22ef27dafd19
Gerrit-Change-Number: 31431
Gerrit-PatchSet: 6
Gerrit-Owner: neels <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-CC: msuraev <[email protected]>
Gerrit-CC: pespin <[email protected]>
Gerrit-Attention: laforge <[email protected]>
Gerrit-Attention: pespin <[email protected]>
Gerrit-Attention: msuraev <[email protected]>
Gerrit-Comment-Date: Fri, 24 Feb 2023 14:19:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: msuraev <[email protected]>
Gerrit-MessageType: comment

Reply via email to