- gerrit-log - lists.osmocom.org

[S] Change in osmo-ttcn3-hacks[master]: pgw: Always expect local TEIC when receiving DeleteSessionResponse
by osmith 10 Apr '24

10 Apr '24

Attention is currently required from: fixeria, lynxis lazus, pespin. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36534?usp=email ) Change subject: pgw: Always expect local TEIC when receiving DeleteSessionResponse ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36534?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I2086b4576a10c7b4ba8217be6e49c8f5f389bc4b Gerrit-Change-Number: 36534 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: osmith <osmith(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, 10 Apr 2024 09:02:25 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in libosmocore[master]: add osmo_stats_report_lock api
by pespin 10 Apr '24

10 Apr '24

Attention is currently required from: fixeria, laforge, neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36538?usp=email ) Change subject: add osmo_stats_report_lock api ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > pespin says use intermediate storage with mutexes in a comment at https://gerrit.osmocom.org/c/osmo-hnbgw/+/36385 For this one, my aim was to say that osmo-trx is doing it that way, not that you should do it that way. > nhofmeyr says directly write to rate_ctr, but still have a mutex around stats reporting, so that counter pairs are always in sync. If you plan on keep the mutex locked while you gather counters from nft, that's a no go for me, you are locking access to rate_ctr/stat for a long period of time, most probably blocking the main thread also for the same amount of time. If you plan to gather counters locally, then lock() then update/copy them to rate_ctr/stats then unlock(), this can be more acceptable and it's basically the option mentioned first quoted in this comment. Still, I think it's better going for osmo_itq to avoid problems accessing rate_ctr infrastructure (llist, etc.) which means you'd need to lock() unlock() everytime you want to update a metric, which is overkill. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36538?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ib335bea7d2a440ca284e6c439066f96456bf2c2d Gerrit-Change-Number: 36538 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Wed, 10 Apr 2024 08:38:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[L] Change in osmo-hnbgw[master]: per-HNB GTP-U traffic counters via nft
by pespin 10 Apr '24

10 Apr '24

Attention is currently required from: laforge, neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/36385?usp=email ) Change subject: per-HNB GTP-U traffic counters via nft ...................................................................... Patch Set 17: (1 comment) Patchset: PS3: > I think it makes a lot of sense to keep the patches separate as they are. […] osmo-trx: See trx_rate_ctr_timerfd_cb() and CommonLibs/trx_rate_ctr.cpp in general. See also dispatch_trx_rate_ctr_change(). osmo-trx is already heavily multithreaded so it's fine doing it that way since it's already complex (it could have been done with an osmo_itq too). For other programs not heavily multithread I'd avoid playing with mutexes and simply rely on message passing integrated in the main loop. Regarding own thread vs multithread: Anything running under the main loop which blocks the main loop for long periods of time while waiting for some procedure to finish is broken by design, be it 300ms, 500ms, 1s, 3s, 50000s. Fine if you want to keep it in 2 patches, but I wouldn't merge this one until the other one is ready and can be merged together. -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36385?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I35b7e97fd039e36633dfde1317170527c82f9f68 Gerrit-Change-Number: 36385 Gerrit-PatchSet: 17 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Wed, 10 Apr 2024 08:30:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in libosmocore[master]: add API logging_vty_subsys_strip_leading_char()
by neels 10 Apr '24

10 Apr '24

Attention is currently required from: fixeria. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36546?usp=email ) Change subject: add API logging_vty_subsys_strip_leading_char() ...................................................................... Patch Set 1: (1 comment) Commit Message: https://gerrit.osmocom.org/c/libosmocore/+/36546/comment/b7cf1756_e70598cf PS1, Line 17: So this allows an application to remove the odd 'D' from category names, : without any changes in any VTY configuration. > IIUC, the [only?] benefit of using this new API is that you can define a `struct log_info_cat` array […] (FYI, this is not sysmocom work, but a private itch) The way this character stripping is done in the VTY code introduces an inconsistency. The correct and consistent way would be for the VTY to use exactly the category name that is defined and also output in log output, i.e. including the leading D if the application has one. That way it is up to the application what names it chooses, and the names are the same everywhere. Since I cannot change this VTY behavior now (all osmo applications depend on that), i at least hope to offer a new application a consistent experience with category names on the VTY. An optional switch to change to consistent behavior. My motivation: I always found this leading D annoying, as apparently did the logging_vty.c implementer, unfortunately. I find it even more madly insane that, for example, DLMGCP now becomes "lmgcp" -- why strip the D and not the L? the whole thing about the D is very inelegant -- why print an extra meaningless character on each and every log line? But ok, say we want an extra "D" everywhere; then why not on the VTY??? I'm hacking on "osmo-gsm-shark" in my private time, and wanted to get rid of those Ds there. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36546?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I5faedf7d6525d744a734ebe54c185fcc904f763e Gerrit-Change-Number: 36546 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Wed, 10 Apr 2024 02:58:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in libosmocore[master]: formalize log subsys stripping for vty
by neels 10 Apr '24

10 Apr '24

Attention is currently required from: fixeria. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36545?usp=email ) Change subject: formalize log subsys stripping for vty ...................................................................... Patch Set 1: (2 comments) File src/core/logging.c: https://gerrit.osmocom.org/c/libosmocore/+/36545/comment/abd53216_0a83c47e PS1, Line 432: int cat_idx > probably a good idea to make it `unsigned` TL;DR: I'll move it to a private header. rambling on in case you're interested: it ended up this way because - intended for private API use only, i'd have liked it to be static, as the commit log explains. - at the very heart of the logging subsystem and should rather be lean. - When looking at the callers, it is clear that it's correct. Yet I did think about it quite a bit: we actually do have negative categories, the DL* ones. so i did ask myself, should it convert from one to the other? But: It is called only in places where the idx is already checked, and the local variable at all the callers is an int... So maybe in future it should be able to convert a negative category? Considering these back and forth I ended up at "good enough compromise, not important enough to bother"... But your remark makes me reconsider, I guess it should be less public? https://gerrit.osmocom.org/c/libosmocore/+/36545/comment/eb8fbfce_23f8762d PS1, Line 434: const char *name = log_info->cat[cat_idx].name; > add `OSMO_ASSERT(cat_idx < log_info->num_cat)`? (as above) -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36545?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I5f81343e8c7b714a4630e64ba654e391435c4244 Gerrit-Change-Number: 36545 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Wed, 10 Apr 2024 02:44:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in libosmocore[master]: add osmo_stats_report_lock api
by neels 10 Apr '24

10 Apr '24

Attention is currently required from: fixeria, laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36538?usp=email ) Change subject: add osmo_stats_report_lock api ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > And for the case at hand, I still belive it makes much more sense to simply use an osmo_itq to pass […] I'm in a bit of a communication dilemma: - laforge says don't use any mutexes and just directly write to rate_ctr from the other thread. - pespin says use an osmo_it_q here - pespin says use intermediate storage with mutexes in a comment at https://gerrit.osmocom.org/c/osmo-hnbgw/+/36385 - nhofmeyr says directly write to rate_ctr, but still have a mutex around stats reporting, so that counter pairs are always in sync. I interpret it so that laforge is radically "bare-metal" and simple, pespin radically thread-safe and elaborate, and i'm actually in the middle, a little more to laforge's side. Yet it seems you guys are missing the actual point for this patch here: A mutex lock around stats reporting is a very basic and very powerful tool. It allows a multi-threaded application to decide which stats updates will always be in sync. This patch adds this feature, in a completely optional way. It is up to the application how to make use of it, so let's discuss those details at the other patches. I fail to see any problem with this patch and don't know how we can reach a consensus on this. Neither of us seems to accept the others' arguments. At least I still think that I'm right... -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36538?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ib335bea7d2a440ca284e6c439066f96456bf2c2d Gerrit-Change-Number: 36538 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Wed, 10 Apr 2024 02:17:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[L] Change in osmo-hnbgw[master]: per-HNB GTP-U traffic counters via nft
by neels 10 Apr '24

10 Apr '24

Attention is currently required from: laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/36385?usp=email ) Change subject: per-HNB GTP-U traffic counters via nft ...................................................................... Patch Set 17: (1 comment) Patchset: PS3: > (I wrote the comment above this morning but actually saw just now I forgot to press the "submit" but […] I think it makes a lot of sense to keep the patches separate as they are. The patch wrapping a thread around existing code is very nicely orthogonal to the actual nft activity, and it is nice to see exactly where the mutexes etc are added, without the nft implementation along with it to clutter up the attention span. osmo-trx's use of multi-threaded counters: do you have a pointer? I was looking around and apparently hadn't found it... thanks! You say in osmo-trx "we do things a bit adhoc" and further below you say that's "how it should be done" -- IIUC an intermediate storage with mutexes. The underlying point is whether the occasional short blocking for HNB Register and HNB DeRegister (rare events) are noticeable enough to warrant further effort on that. If those blockings of the main thread are not desirable, it seems the best way is a queue fd (osmo_it_q?) because other than mutexes, it avoids all blocking by design. You say "it's broken", yet the patch is actually already being run successfully. I see no inconsistencies nor brokenness, I'd be glad to hear some technical detail to back up your claims. Scalability is an open point -- we are only now able to gather the empirical data on that. My experience is that nft below 1000 items is very fast, and only becomes prohibitively slow with higher numbers, with some exponentiality kicking in. I am not fully convinced that we even need a separate thread. In summary, am interested in how osmo-trx does it; otherwise do not agree / do not yet see the points that were claimed; instead, am engaging in scalability discussion. -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36385?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I35b7e97fd039e36633dfde1317170527c82f9f68 Gerrit-Change-Number: 36385 Gerrit-PatchSet: 17 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 10 Apr 2024 01:58:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[L] Change in osmo-hnbgw[master]: per-HNB GTP-U traffic counters via nft
by pespin 09 Apr '24

09 Apr '24

Attention is currently required from: laforge, neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/36385?usp=email ) Change subject: per-HNB GTP-U traffic counters via nft ...................................................................... Patch Set 17: (1 comment) Patchset: PS3: > Several pointers below: […] (I wrote the comment above this morning but actually saw just now I forgot to press the "submit" button). -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36385?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I35b7e97fd039e36633dfde1317170527c82f9f68 Gerrit-Change-Number: 36385 Gerrit-PatchSet: 17 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Tue, 09 Apr 2024 18:53:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[L] Change in osmo-hnbgw[master]: per-HNB GTP-U traffic counters via nft
by pespin 09 Apr '24

09 Apr '24

Attention is currently required from: laforge, neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/36385?usp=email ) Change subject: per-HNB GTP-U traffic counters via nft ...................................................................... Patch Set 17: (2 comments) Patchset: PS3: Several pointers below: - I think that definetly we want to merge all this done in a separate thread from first step. I don't think it even makes sense to have it separated in a first step where everything happens in the same thread, since it's broken and distracts the whole thing. > We don't have multi-threaded rate counters so far anywhere. - We do, in osmo-trx. In there since it's heavily multithreaded we do things a bit adhoc with separate structures and then copying them. > The other idea that does not need libosmocore changes is this: > have a separate "volatile" counter storage in the hnb_persistent struct; > in the nft thread, do all parsing and all hnb lookups, > and place parsed counters in that separate counter storage per hnbp. > inter-thread signal (it_q is nice because it integrates in the select()) > to tell the main thread to shovel all hnb volatile counters into the rate_ctr; > those volatile counters need a mutex (one global one, or maybe one per hnb). That's similar to how osmo-trx does it, and I think it's how it should be done. All other ways imho are calling for inconsistencies and stuff ending up broken. So my opinion is: I'm waiting to see a patch like the above described which I can review. Patchset: PS17: ree -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36385?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I35b7e97fd039e36633dfde1317170527c82f9f68 Gerrit-Change-Number: 36385 Gerrit-PatchSet: 17 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Tue, 09 Apr 2024 18:52:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Gerrit-MessageType: comment

1 0

[S] Change in libosmocore[master]: add API logging_vty_subsys_strip_leading_char()
by fixeria 09 Apr '24

09 Apr '24

Attention is currently required from: neels. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36546?usp=email ) Change subject: add API logging_vty_subsys_strip_leading_char() ...................................................................... Patch Set 1: (1 comment) Commit Message: https://gerrit.osmocom.org/c/libosmocore/+/36546/comment/29859f7b_d6a6079f PS1, Line 17: So this allows an application to remove the odd 'D' from category names, : without any changes in any VTY configuration. IIUC, the [only?] benefit of using this new API is that you can define a `struct log_info_cat` array with category names like `MAIN` instead of `DMAIN`. Is there anything else we could benefit with this API? Honestly, I think defining logging categories with the leading `D` is not a big deal... -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36546?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I5faedf7d6525d744a734ebe54c185fcc904f763e Gerrit-Change-Number: 36546 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Tue, 09 Apr 2024 12:20:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2026

2025

2024

2023

2022

gerrit-log