Attention is currently required from: fixeria, lynxis lazus, pespin.
neels has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/37724?usp=email )
Change subject: gmm: Introduce interface GMMBSSGP
......................................................................
Patch Set 1:
(2 comments)
File include/osmocom/gprs/gmm/gmm_prim.h:
https://gerrit.osmocom.org/c/libosmo-gprs/+/37724/comment/149760f2_9c7fc81b… :
PS1, Line 369: /* Alloc primitive for GMMBSSGP SAP: */
> I really prefer getting this merged as is since it gives a clear view on the primitives being expect […]
objectively, the names are unusually long indeed.
It sounds like fixeria suggests a more maintainable public API.
I'm not entirely clear though, fixeria, on what exactly you mean by "let the user fill prim specific fields using designated initializers (or whatever they like)"
are we suggesting something like this?
```
my_prim = osmo_gmm_prim_gmmbssgp_alloc();
osmo_gmm_prim_set_bvci(my_prim, my_bvci);
osmo_gmm_prim_set_nsei(my_prim, my_nsei);
```
or like this?
```
struct osmo_gprs_gmm_prim_rcu_args {
uint16_t bvci;
uint16_t nsei;
};
my_rcu_args = (struct osmo_gprs_gmm_prim_rcu_args){...};
my_prim = osmo_gsmm_prim_alloc_rcu(&my_rcu_args);
```
Every way does have its own drawbacks and also we don't seem to have a clear culture of one way or the other in osmocom... so i'd let the patch submitter decide on this stuff, as long as it works.
For certain, objectively, these names are very very long. It's up in the same category with the osmo-msc logging of context that I wrote back in the days, almost breaching on the unmanageable.
File src/gmm/gmm_prim.c:
https://gerrit.osmocom.org/c/libosmo-gprs/+/37724/comment/e1d339e7_b5b94dd2… :
PS1, Line 353: gmm_prim->gmmbssgp.nsei = nsei;
in here it does look like a lot of repetition that could be more elegant with API like the pseudocode in my other comment -- pass in the OSMO_GPRS_GMM_ constants as arguments instead of adding a lot of binary objects for each constant.
--
To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/37724?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: libosmo-gprs
Gerrit-Branch: master
Gerrit-Change-Id: I39045833fd43cfe98cb1a3812fbce3fdcaae6dc6
Gerrit-Change-Number: 37724
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Comment-Date: Wed, 14 Aug 2024 01:11:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Attention is currently required from: neels.
fixeria has posted comments on this change by fixeria. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/37804?usp=email )
Change subject: erab_fsm: implement handling of GTP-U address
......................................................................
Patch Set 2:
(1 comment)
File src/erab_fsm.erl:
https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/37804/comment/9e0883a4_d4fc… :
PS2, Line 99: %% @param TA TEID and bind address indicated by the MME.
> btw often a TEID + IP address is called an F-TEID, a fully qualified TEID. […]
Good to know, thanks for letting me know! I'll consider renaming in a separate patch.
--
To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/37804?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: erlang/osmo-s1gw
Gerrit-Branch: master
Gerrit-Change-Id: Ifdc492c41266d07d0e951dcdcc1e4ba74da0b13b
Gerrit-Change-Number: 37804
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 14 Aug 2024 00:46:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Attention is currently required from: lynxis lazus.
neels has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/37812?usp=email )
Change subject: SGSN_Tests_NS: NS related tests: use the shutdown helper
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/37812?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: I08eee0d11ae04276ca1ad8fd58ebb93dd6d0066f
Gerrit-Change-Number: 37812
Gerrit-PatchSet: 1
Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Comment-Date: Wed, 14 Aug 2024 00:41:45 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: fixeria, laforge.
neels has posted comments on this change by fixeria. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/37795?usp=email )
Change subject: erab_fsm: include Network Instance IE in PDRs and FARs
......................................................................
Patch Set 4: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/37795?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: erlang/osmo-s1gw
Gerrit-Branch: master
Gerrit-Change-Id: I4dbe8e0b1d14eed5fdb3f9f3f2543c44fd543d22
Gerrit-Change-Number: 37795
Gerrit-PatchSet: 4
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 14 Aug 2024 00:39:19 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: fixeria.
neels has posted comments on this change by fixeria. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/37804?usp=email )
Change subject: erab_fsm: implement handling of GTP-U address
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
File src/erab_fsm.erl:
https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/37804/comment/198fb2d1_c2c6… :
PS2, Line 99: %% @param TA TEID and bind address indicated by the MME.
btw often a TEID + IP address is called an F-TEID, a fully qualified TEID. it's a short name useful for coding
--
To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/37804?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: erlang/osmo-s1gw
Gerrit-Branch: master
Gerrit-Change-Id: Ifdc492c41266d07d0e951dcdcc1e4ba74da0b13b
Gerrit-Change-Number: 37804
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 14 Aug 2024 00:37:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
