Attention is currently required from: laforge, neels. pespin 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 9: (16 comments) File include/osmocom/hnbgw/hnbgw.h: https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/84344d4d_b07e3a98 PS6, Line 384: /* When rules to count traffic to and from this hNodeB are present, this reflects the state in nftables ACK. https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/ac15f67a_9120ed54 PS6, Line 394: struct osmo_sockaddr_str addr_remote; Done https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/f00243b2_56949943 PS6, Line 402: struct { Oh I didn't mean joining the dl and ul. My proposal was to simply make the struct named so it can be declared one, used twice. That also makes it possible to pass a pointer to it. File include/osmocom/hnbgw/nft_kpi.h: https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/2d63fa0a_4f91520d PS6, Line 21: int nft_kpi_hnb_start(struct hnb_persistent *hnbp, const struct osmo_sockaddr_str *gtpu_remote); Done File src/osmo-hnbgw/hnbgw.c: https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/09ce5455_93fb3768 PS6, Line 346: hnb_persistent_disconnected(ctx->persistent); Yeah, whatever, just give it a second though :) https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/d495300f_44ccb1b7 PS6, Line 607: { Ack File src/osmo-hnbgw/hnbgw_hnbap.c: https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/2b5a2a59_fccd98c0 PS6, Line 571: hnb_persistent_connected(ctx->persistent); Can actually an HNB be registered without being connected? I don't think so. My understanding is that a HNBAP link disconnect makes the HNB implicitly become "unregistered". File src/osmo-hnbgw/hnbgw_vty.c: https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/f3192d20_704f761c PS6, Line 884: DEFUN(cfg_hnbgw_nft_kpi, cfg_hnbgw_nft_kpi_cmd, Ah then it's probably the default. DONE. File src/osmo-hnbgw/nft_kpi.c: https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/7fd077cd_6b1f1569 PS6, Line 86: static __thread struct nft_thread *g_nft_thread = NULL; Ok, I'm not really a fan of having this sort of generic thread which may or may not do stuff, makes the architecture more open for no good reason, but fine. I'd prefer having 2 (actually 3 counting the main thread) distinct threads doing specific stuff with specific context, but if you like it the other way we can keep it. https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/16d564ff_9dfe2e27 PS6, Line 120: LOGP(DNFT, LOGL_ERROR, "error running nft cmd %s#%u: rc=%d cmd=%s\n", Done https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/9c8b547a_5f808d31 PS6, Line 182: enum nft_thread_req_type { Done https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/68d34dcd_2c071dcd PS6, Line 207: struct { Done https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/7ade3ea8_0eef8f10 PS6, Line 251: OSMO_ASSERT(g_nft_thread); Done https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/6601dced_9cada900 PS6, Line 297: static void nft_t2m_enqueue(struct nft_thread *t, struct nft_thread_req *req) Done https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/0e333f77_964f8f30 PS6, Line 415: static int update_ctr(struct rate_ctr_group *cg, int cgidx, uint64_t *last_val, uint64_t new_val) Yes, after looking at the surrounding code. Just sharing that we use "ctrg" in lots of places already, so it's easier for osmocom users to understand right away. https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/27662d54_da2b328d PS6, Line 452: size_t want = g_nft_thread->counters_len + 64; Done -- 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: 9 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: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Wed, 22 May 2024 09:20:51 +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