Attention is currently required from: laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539?usp=email ) Change subject: per-HNB GTP-U traffic counters via nft ...................................................................... Patch Set 10: (8 comments) File include/osmocom/hnbgw/hnbgw.h: https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/a936ea4a_eeddd3d8 PS6, Line 402: struct { yes i mean that too. i wouldn't do it here because nothing points to it, but my opinion is not strong. to keep it as is, resolve. to change it, say "change it please". File src/osmo-hnbgw/hnbgw_hnbap.c: https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/056856c9_7e254a03 PS6, Line 571: hnb_persistent_connected(ctx->persistent); yes, but a hnb can be connected without being registered. (that is for a different redmine ticket) so do you prefer 'active' and 'inactive', or is 'connect' and 'disconnect' good enough. File src/osmo-hnbgw/hnbgw_vty.c: https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/040c3b3d_0b639b97 PS9, Line 894: vty_out(vty, "%% WARNING: nft configuration changes need a restart of osmo-hnbw%s", VTY_NEWLINE); hey! i fixed that! File src/osmo-hnbgw/nft_kpi.c: https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/47a5b1fd_bb91d2cc PS6, Line 86: static __thread struct nft_thread *g_nft_thread = NULL; Done https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/9fea60c5_2b46deb8 PS6, Line 193: static const char * const nft_thread_req_type_name[] = { i want to avoid iteration ala value_string, because all callers are entirely constants within this .c file https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/a84eb397_87f47076 PS6, Line 415: static int update_ctr(struct rate_ctr_group *cg, int cgidx, uint64_t *last_val, uint64_t new_val) kk File src/osmo-hnbgw/nft_kpi.c: https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/db2f120f_9e4ea83d PS9, Line 626: /* From here on, until we receive the next NFT_THREAD_GET_COUNTERS in this thread, the if ever the time it takes to retrieve counters does overlap the wait time configured in timer X34, then we would simply add another nft_counters2 thread, submitting GET_COUNTERS requests round robin between them. repeat to taste. (but i don't expect that to be necessary.) https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/fc2fbb4d_735d293d PS9, Line 994: nft_kpi_get_counters_schedule(); i believe this was the result of our discussion: to wait X34 time *between* getting counters. This is nice because it is simple and avoids all such races you describe. What I don't like about it is that the metric from the user perspective should be "get timers every N seconds" -- when we wait in-between, we will "drift", elongating the period by the time it takes to retrieve counters. This is not very long really, but i'd prefer scheduling to start counters every N seconds, not wait N seconds in-between. Doing so means that we need to ensure avoiding races, making the code more complex. So how ambitious are we? I think I am fine with waiting X34 seconds *between* getting counters. it is good enough as i see it. Other opinions? -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539?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: Ib2f0a9252715ea4b2fe9c367aa65f771357768ca Gerrit-Change-Number: 36539 Gerrit-PatchSet: 10 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 22 May 2024 22:21:12 +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