This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
daniel gerrit-no-reply at lists.osmocom.orgdaniel has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/21960 ) Change subject: gbproxy: Implement TLLI cache and use it for SUSPEND/RESUME ...................................................................... Patch Set 4: (8 comments) https://gerrit.osmocom.org/c/osmo-sgsn/+/21960/3/include/osmocom/sgsn/gb_proxy.h File include/osmocom/sgsn/gb_proxy.h: https://gerrit.osmocom.org/c/osmo-sgsn/+/21960/3/include/osmocom/sgsn/gb_proxy.h@175 PS3, Line 175: struct gbproxy_tlli_cache { > cache_entry or cache_item would be more descriptive imho Done https://gerrit.osmocom.org/c/osmo-sgsn/+/21960/3/src/gbproxy/gb_proxy.c File src/gbproxy/gb_proxy.c: https://gerrit.osmocom.org/c/osmo-sgsn/+/21960/3/src/gbproxy/gb_proxy.c@896 PS3, Line 896: tlli = osmo_load32be(TLVP_VAL(&tp, BSSGP_IE_TLLI)); > would be great checking if TLLI is really there instead of directly de-referencing it. tlli is a mandatory field for SUSPEND/RESUME and in line 860 osmo_tlv_prot_parse would fail if any mandatory field isn't present. Other branches skip these kind of checks as well. https://gerrit.osmocom.org/c/osmo-sgsn/+/21960/3/src/gbproxy/gb_proxy.c@1164 PS3, Line 1164: uint32_t tlli = osmo_load32be(TLVP_VAL(&tp, BSSGP_IE_TLLI)); > same Done https://gerrit.osmocom.org/c/osmo-sgsn/+/21960/3/src/gbproxy/gb_proxy.c@1170 PS3, Line 1170: // FIXME > FIX what? Replaced with todo for counter https://gerrit.osmocom.org/c/osmo-sgsn/+/21960/3/src/gbproxy/gb_proxy.c@1400 PS3, Line 1400: void tlli_cache_cleanup(void *data) > static Done https://gerrit.osmocom.org/c/osmo-sgsn/+/21960/3/src/gbproxy/gb_proxy.c@1405 PS3, Line 1405: osmo_timer_schedule(&cfg->tlli_cache.timer, 2, 0); > Probably worth avoid reescheduling it if the tlli cache is empty, and reenabling it when first item […] Yeah, I thought about it as an optimization. I'll add a TODO for now https://gerrit.osmocom.org/c/osmo-sgsn/+/21960/3/src/gbproxy/gb_proxy_peer.c File src/gbproxy/gb_proxy_peer.c: https://gerrit.osmocom.org/c/osmo-sgsn/+/21960/3/src/gbproxy/gb_proxy_peer.c@270 PS3, Line 270: // Update the entry if it already exists > /* */ Done https://gerrit.osmocom.org/c/osmo-sgsn/+/21960/3/src/gbproxy/gb_proxy_peer.c@320 PS3, Line 320: expiry = now.tv_sec - cfg->tlli_cache.timeout; > I'd go or using timespec completely. […] We don't really need sub-second resolution anyway (and I changed the tlli_cache.timeout from timespec after a comment from Harald) https://gerrit.osmocom.org/c/osmo-sgsn/+/21960/1/include/osmocom/sgsn/gb_proxy.h#177 I agree that comparing with timespec would probably be nicer here. -- To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/21960 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sgsn Gerrit-Branch: master Gerrit-Change-Id: I42adf70f560d2bb358a9e1c7614281e8d2967568 Gerrit-Change-Number: 21960 Gerrit-PatchSet: 4 Gerrit-Owner: daniel <dwillmann at sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin at sysmocom.de> Gerrit-CC: laforge <laforge at osmocom.org> Gerrit-Comment-Date: Mon, 11 Jan 2021 04:04:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin at sysmocom.de> Gerrit-MessageType: comment -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210111/273f7284/attachment.htm>