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 12:

(8 comments)

File include/osmocom/hnbgw/hnbgw.h:

https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/6665b6ba_668798f4
PS6, Line 402:          struct {
> yes i mean that too. […]
Done


File src/osmo-hnbgw/hnbgw_hnbap.c:

https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/7411b3df_81f63be8
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) […]
It really depends on the specific event you want to track here. I would only 
use the "connected/disconnected" wording for SCTP/TCP layer here. If you are 
actually tracking the registered state, then I'd for for "registered" or 
"active" or alike. Up to you.


File src/osmo-hnbgw/hnbgw_vty.c:

https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/8d040060_34f92616
PS9, Line 894:          vty_out(vty, "%% WARNING: nft configuration changes 
need a restart of osmo-hnbw%s", VTY_NEWLINE);
> hey! i fixed that!
Done


File src/osmo-hnbgw/nft_kpi.c:

https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/c0bb0fc3_540b6ff2
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 . […]
Done


https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/77d602fc_5b43f7d4
PS6, Line 415: static int update_ctr(struct rate_ctr_group *cg, int cgidx, 
uint64_t *last_val, uint64_t new_val)
> kk
Done


File src/osmo-hnbgw/nft_kpi.c:

https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/cbf89028_0749936e
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, t […]
Done


https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/ee81f850_2fa5823b
PS9, Line 700:  osmo_timer_setup(&g_hnbgw->nft_kpi.get_counters_timer, 
nft_kpi_get_counters_cb, NULL);
> possible idea for the future: keep interval tracking at the worker, use 
> osmo_timerfd_*() here, this  […]
Done


https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/bba5ff67_9b66038e
PS9, Line 994:          nft_kpi_get_counters_schedule();
> i believe this was the result of our discussion: to wait X34 time *between* 
> getting counters. […]
I really think we want to at least do as I described, which is really quick to 
implement (adding one "prev" timestamp and one timestmap subtraction).

This way logic conforms much more to what user would expect: If I configure X34 
to 10000, I get updated counters every 10 seconds, not every 10+X seconds.



--
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: 12
Gerrit-Owner: neels <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-Attention: neels <[email protected]>
Gerrit-Attention: laforge <[email protected]>
Gerrit-Comment-Date: Thu, 23 May 2024 12:24:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <[email protected]>
Comment-In-Reply-To: pespin <[email protected]>
Gerrit-MessageType: comment

Reply via email to