April 2024 - gerrit-log - lists.osmocom.org

[S] Change in libosmocore[master]: formalize log subsys stripping for vty

by neels

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 year, 2 months

1
0
0 0

[M] Change in libosmocore[master]: add osmo_stats_report_lock api

by neels

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 year, 2 months

1
0
0 0

[L] Change in osmo-hnbgw[master]: per-HNB GTP-U traffic counters via nft

by neels

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 year, 2 months

1
0
0 0

[L] Change in osmo-hnbgw[master]: per-HNB GTP-U traffic counters via nft

by pespin

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 year, 2 months

1
0
0 0

[L] Change in osmo-hnbgw[master]: per-HNB GTP-U traffic counters via nft

by pespin

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 year, 2 months

1
0
0 0

[S] Change in libosmocore[master]: add API logging_vty_subsys_strip_leading_char()

by fixeria

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 year, 2 months

1
0
0 0

[S] Change in libosmocore[master]: formalize log subsys stripping for vty

by fixeria

Attention is currently required from: neels. fixeria 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/6194be9b_913cbcc2 PS1, Line 432: int cat_idx probably a good idea to make it `unsigned` https://gerrit.osmocom.org/c/libosmocore/+/36545/comment/2b5f6379_831a452e PS1, Line 434: const char *name = log_info->cat[cat_idx].name; add `OSMO_ASSERT(cat_idx < log_info->num_cat)`? -- 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: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Tue, 09 Apr 2024 12:14:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 year, 2 months

1
0
0 0

[S] Change in osmo-ttcn3-hacks[master]: SIP_Emulation: Fix SIPEM_register when several conns are active

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36543?usp=email ) Change subject: SIP_Emulation: Fix SIPEM_register when several conns are active ...................................................................... SIP_Emulation: Fix SIPEM_register when several conns are active "Dynamic test case error: Port CLIENT_PROC has more than one active connections. Message can be sent on it only with explicit addressing.". Change-Id: Ibf868394ce2c495a78ab943ddec278a45bf71088 --- M library/SIP_Emulation.ttcn 1 file changed, 14 insertions(+), 2 deletions(-) Approvals: Jenkins Builder: Verified fixeria: Looks good to me, approved osmith: Looks good to me, but someone else must approve diff --git a/library/SIP_Emulation.ttcn b/library/SIP_Emulation.ttcn index 41e6975..5c00c4d 100644 --- a/library/SIP_Emulation.ttcn +++ b/library/SIP_Emulation.ttcn @@ -326,9 +326,9 @@ SIP.send(sip_resp); } - [] CLIENT_PROC.getcall(SIPEM_register:{?,?}) -> param(sip_to, vc_hdlr) { + [] CLIENT_PROC.getcall(SIPEM_register:{?,?}) -> param(sip_to, vc_hdlr) sender vc_conn { f_create_expect(sip_to, vc_hdlr); - CLIENT_PROC.reply(SIPEM_register:{sip_to, vc_hdlr}); + CLIENT_PROC.reply(SIPEM_register:{sip_to, vc_hdlr}) to vc_conn; } } -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36543?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: Ibf868394ce2c495a78ab943ddec278a45bf71088 Gerrit-Change-Number: 36543 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 year, 2 months

1
0
0 0

[S] Change in osmo-ttcn3-hacks[master]: SIP_Emulation: Match empty port as default port 5060

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36544?usp=email ) Change subject: SIP_Emulation: Match empty port as default port 5060 ...................................................................... SIP_Emulation: Match empty port as default port 5060 Change-Id: I8415571a5bdc99e8cc007bb4b57bcb73b7afd4fb --- M library/SIP_Emulation.ttcn 1 file changed, 12 insertions(+), 0 deletions(-) Approvals: Jenkins Builder: Verified osmith: Looks good to me, but someone else must approve fixeria: Looks good to me, approved diff --git a/library/SIP_Emulation.ttcn b/library/SIP_Emulation.ttcn index 5c00c4d..4aef536 100644 --- a/library/SIP_Emulation.ttcn +++ b/library/SIP_Emulation.ttcn @@ -380,6 +380,9 @@ } if (not ispresent(t_exp.hostPort.portField)) { t_exp.hostPort.portField := *; + } else if (valueof(t_exp.hostPort.portField) == 5060) { + /* if the port number is 5060, it may be omitted */ + t_exp.hostPort.portField := 5060 ifpresent; } if (not ispresent(t_exp.urlParameters)) { t_exp.urlParameters := *; -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36544?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: I8415571a5bdc99e8cc007bb4b57bcb73b7afd4fb Gerrit-Change-Number: 36544 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 year, 2 months

1
0
0 0

[S] Change in osmo-ttcn3-hacks[master]: SIP_Emulation: Match empty port as default port 5060

by fixeria

Attention is currently required from: pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36544?usp=email ) Change subject: SIP_Emulation: Match empty port as default port 5060 ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36544?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: I8415571a5bdc99e8cc007bb4b57bcb73b7afd4fb Gerrit-Change-Number: 36544 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 09 Apr 2024 10:36:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 2 months

1
0
0 0

2025

2024

2023

2022

gerrit-log April 2024