Attention is currently required from: fixeria, laforge.
7 comments:
File config/sys.config:
Patch Set #8, Line 24: %% {enft_kpi_enable, true}, %% whether to enable the NFT KPI module (default: false)
where does the "e" from "enft" come from? At first sight looks like "nft" would be much more comprehensive for users.
File include/s1gw_metrics.hrl:
Patch Set #8, Line 40: %% NOTE: these counters shall not be listed in ?S1GW_COUNTERS,
So simply this can be announced as "Per-eNB counters"?
File rebar.config:
Patch Set #8, Line 13: {git, "https://gitea.osmocom.org/vyanitskiy/enftables.git", {branch, "fixeria/json"}}},
can we have this in a "osmocom/master" or "master" branch instead?
File src/enft_kpi.erl:
Patch Set #8, Line 142: R1 = [nft_expr_match_ip_proto("udp", ?OP_NEQ), nft_expr_accept()],
sounds like you may want to introduce atoms in the nft lib to express OP_NEQ and OP_EQ instead.
Patch Set #8, Line 145: nft_cmd_add_chain(TName, "gtpu-ul", "prerouting"),
may make sense to have this strings as defines, to avoid potential typos.
Patch Set #8, Line 341: case enb_add_nft_counters(ES0#{addr => Addr}, Cfg) of
fyi this can take some time, and afaict you are doing it synchronously (handle_call), which means the caller (probably handling SCTP traffic) will become stuck for a while waiting for this.
It's fine for now but can become a problem later. Also it's fine if the the caller of this call is per-session I guess.
File src/osmo_s1gw_sup.erl:
Patch Set #8, Line 81: EnftKpi = {enft_kpi, {enft_kpi, start_link, [enft_kpi_cfg()]},
I?m still wondering why this is called "enft" but above it's called "Pfcp" and not "Epfcp" or "Sctp" vs "Esctp".
To view, visit change 40281. To unsubscribe, or for help writing mail filters, visit settings.