Attention is currently required from: osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27476 )
Change subject: Support forwarding messages with multiple BSCs ......................................................................
Patch Set 2:
(5 comments)
File include/osmocom/bsc_nat/bsc_nat.h:
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27476/comment/4c3504c2_55095343 PS2, Line 26: BSC_NAT_CN = 0, you probably lack a prefix there for the enum. BSC_NAT is the domain, and CN and RAN the elements. BSC_NAT_NET_CN BSC_NAT_NET_RAN?
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27476/comment/680b2c88_b95f3d9d PS2, Line 48: uint32_t subscr_conn_id_next[2]; /* for CN, RAN */ I'd simply have 2 variables here, looks easier to identify them in code, and I bet there's no real gain in having it as an array.
File include/osmocom/bsc_nat/subscr_conn.h:
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27476/comment/c4120d16_cf2b1bdb PS2, Line 29: uint32_t id[2]; /* conn_id for CN, RAN */ same, there's problably no good reason to have this as an array. Otherwise one needs to look whether 0 is CN or RAN, etc.
File src/osmo-bsc-nat/bsc.c:
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27476/comment/6621e8c6_83fcf79d PS2, Line 58: subscr_conn_free_by_bsc(bsc); This looks weird. I'd expect a subscr being passed here seeing the name of the function. Sounds more like the name should be "bsc_free_all_subscr_conn()" or bsc_free_subscr_conn_all()" Or perhaps into bsc_nat.c and have bsc_nat_free_subscr_conn_by_bsc().
File src/osmo-bsc-nat/msc.c:
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27476/comment/a683dc52_5664cc5e PS2, Line 77: subscr_conn_free_by_msc(msc); same: msc_free_subscr_conn_all()