May 2023 - gerrit-log - lists.osmocom.org

[S] Change in osmo-hnbgw[master]: add doc/examples/osmo-hnbgw/osmo-hnbgw-cs7.cfg

by neels

neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/32989 ) Change subject: add doc/examples/osmo-hnbgw/osmo-hnbgw-cs7.cfg ...................................................................... add doc/examples/osmo-hnbgw/osmo-hnbgw-cs7.cfg We have not a single example yet showing point code and SCCP address configuration. Add this example, which will also show how the config file syntax changes while introducing CN pooling in upcoming patches. Related: SYS#6412 Change-Id: I42c3b434a7339cc3efb27b43c893cfb734de9ca4 --- A doc/examples/osmo-hnbgw/osmo-hnbgw-cs7.cfg 1 file changed, 41 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/89/32989/1 diff --git a/doc/examples/osmo-hnbgw/osmo-hnbgw-cs7.cfg b/doc/examples/osmo-hnbgw/osmo-hnbgw-cs7.cfg new file mode 100644 index 0000000..5e78947 --- /dev/null +++ b/doc/examples/osmo-hnbgw/osmo-hnbgw-cs7.cfg @@ -0,0 +1,27 @@ +cs7 instance 0 + point-code 0.3.0 + asp asp-clnt-hnbgw-0 2905 0 m3ua + remote-ip 127.0.0.1 + sccp-address my-msc + point-code 1.2.3 + sccp-address my-sgsn + point-code 1.4.2 + +hnbgw + iuh + local-ip 0.0.0.0 + hnbap-allow-tmsi 1 + iucs + remote-addr my-msc + iups + remote-addr my-sgsn + +log stderr + logging filter all 1 + logging color 1 + logging print level 1 + logging print category 1 + logging print category-hex 0 + logging print file basename last + logging print extended-timestamp 1 + logging level set-all notice -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/32989 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I42c3b434a7339cc3efb27b43c893cfb734de9ca4 Gerrit-Change-Number: 32989 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 1 month

1
0
0 0

[L] Change in osmo-hnbgw[master]: cnpool: allow separate cs7 for IuPS and IuCS

by neels

Attention is currently required from: neels, msuraev. Hello Jenkins Builder, laforge, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/32323 to look at the new patch set (#6). Change subject: cnpool: allow separate cs7 for IuPS and IuCS ...................................................................... cnpool: allow separate cs7 for IuPS and IuCS Prepare for CN pooling; allow using separate SS7 instances for IuCS and IuPS. New struct hnbgw_sccp_user describes a RANAP SCCP user, one per cs7. Limit struct hnbgw_cnlink to describing a CN peer, using whichever SCCP instance that is indicated by hnbgw_sccp_user. Chart sccp.dot shows the changes made. Related: SYS#6412 Change-Id: Iea1824f1c586723d989c80a909bae16bd2866e08 --- M doc/charts/sccp.dot M include/osmocom/hnbgw/context_map.h M include/osmocom/hnbgw/hnbgw.h M include/osmocom/hnbgw/hnbgw_cn.h M src/osmo-hnbgw/context_map.c M src/osmo-hnbgw/context_map_sccp.c M src/osmo-hnbgw/hnbgw.c M src/osmo-hnbgw/hnbgw_cn.c M src/osmo-hnbgw/hnbgw_rua.c M src/osmo-hnbgw/hnbgw_vty.c M src/osmo-hnbgw/osmo_hnbgw_main.c M tests/config/defaults.vty M tests/config/one_cs7.vty M tests/config/one_cs7_with_addrs.vty M tests/config/one_cs7_with_iucs_addr.vty M tests/config/one_cs7_with_iups_addr.vty A tests/config/two_cs7.cfg A tests/config/two_cs7.vty A tests/config/two_cs7_with_addrs.cfg A tests/config/two_cs7_with_addrs.vty 20 files changed, 667 insertions(+), 298 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/23/32323/6 -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/32323 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: Iea1824f1c586723d989c80a909bae16bd2866e08 Gerrit-Change-Number: 32323 Gerrit-PatchSet: 6 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: msuraev <msuraev(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-MessageType: newpatchset

2 years, 1 month

1
0
0 0

[M] Change in osmo-hnbgw[master]: add startup config tests

by neels

neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/32991 ) Change subject: add startup config tests ...................................................................... add startup config tests Get a handle on how osmo-hnbgw currently auto-configures itself, by running vty tests with a couple of config files that have various levels of omitted config items that are to be auto-configured. These tests serve to ensure continuity and compatibility by upcoming cnpool patches. They will show how the configuration changes / doesn't change in all the right ways. Uncover one bug in osmo-hnbgw: local point-code setting is discarded by osmo-hnbgw startup when there are no SCCP address book entries in use. The error is fixed as side effect of an upcoming patch: Iea1824f1c586723d989c80a909bae16bd2866e08 Related: SYS#6412 Change-Id: Ic8bb30e1dd73753c2ff255566382e241918414f7 --- M tests/Makefile.am A tests/config/defaults.cfg A tests/config/defaults.vty A tests/config/one_cs7.cfg A tests/config/one_cs7.vty A tests/config/one_cs7_with_addrs.cfg A tests/config/one_cs7_with_addrs.vty A tests/config/one_cs7_with_iucs_addr.cfg A tests/config/one_cs7_with_iucs_addr.vty A tests/config/one_cs7_with_iups_addr.cfg A tests/config/one_cs7_with_iups_addr.vty 11 files changed, 244 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/91/32991/1 diff --git a/tests/Makefile.am b/tests/Makefile.am index 58d6592..770c072 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -26,6 +26,8 @@ $(TESTSUITE) \ osmo-hnbgw-vty-test.cfg \ osmo-hnbgw.vty \ + config/*.cfg \ + config/*.vty \ $(NULL) TESTSUITE = $(srcdir)/testsuite @@ -37,6 +39,7 @@ if ENABLE_EXT_TESTS python-tests: $(MAKE) vty-test + $(MAKE) config-tests osmotestvty.py -p $(abs_top_srcdir) -w $(abs_top_builddir) -v osmotestconfig.py -p $(abs_top_srcdir) -w $(abs_top_builddir) -v else @@ -57,6 +60,13 @@ -c $(srcdir)/osmo-hnbgw-vty-test.cfg" \ $(U) $(srcdir)/$(VTY_TEST) +# Test each config/*.cfg file with the corresponding config/*.vty transcript test +.PHONY: %.config-test +%.config-test: $(srcdir)/config/%.vty + osmo_verify_transcript_vty.py -v -n OsmoHNBGW -p 4261 -r "$(top_builddir)/src/osmo-hnbgw/osmo-hnbgw -c $(patsubst %.vty,%.cfg,$<)" $< $U +.PHONY: config-tests +config-tests: $(patsubst $(srcdir)/config/%.vty,%.config-test,$(wildcard $(srcdir)/config/*.vty)) + check-local: atconfig $(TESTSUITE) $(SHELL) '$(TESTSUITE)' $(TESTSUITEFLAGS) $(MAKE) $(AM_MAKEFLAGS) python-tests diff --git a/tests/config/defaults.cfg b/tests/config/defaults.cfg new file mode 100644 index 0000000..b4e37fb --- /dev/null +++ b/tests/config/defaults.cfg @@ -0,0 +1 @@ +# configure nothing, to get purely the implicit default config diff --git a/tests/config/defaults.vty b/tests/config/defaults.vty new file mode 100644 index 0000000..282c080 --- /dev/null +++ b/tests/config/defaults.vty @@ -0,0 +1,31 @@ +OsmoHNBGW> ### Config file was empty, everything we see here is the default auto configuration + +OsmoHNBGW> enable + +OsmoHNBGW# show cs7 config +cs7 instance 0 + point-code 0.23.5 + asp asp-clnt-OsmoHNBGW 2905 0 m3ua + local-ip localhost + remote-ip localhost + sctp-role client + as as-clnt-OsmoHNBGW m3ua + asp asp-clnt-OsmoHNBGW + routing-key 0 0.23.5 + +OsmoHNBGW# show cnlink +IuCS: OsmoHNBGW:RI=SSN_PC,PC=0.23.5,SSN=RANAP <-> RI=SSN_PC,PC=0.23.1,SSN=RANAP + SS7 route: pc=0=0.0.0 mask=0x0=0.0.0 via AS as-clnt-OsmoHNBGW proto=m3ua ASP asp-clnt-OsmoHNBGW +IuPS: OsmoHNBGW:RI=SSN_PC,PC=0.23.5,SSN=RANAP <-> RI=SSN_PC,PC=0.23.4,SSN=RANAP + SS7 route: pc=0=0.0.0 mask=0x0=0.0.0 via AS as-clnt-OsmoHNBGW proto=m3ua ASP asp-clnt-OsmoHNBGW + +OsmoHNBGW# show running-config +... +cs7 instance 0 + point-code 0.23.5 +hnbgw + rnc-id 23 + log-prefix hnb-id + iuh + mgw 0 +... diff --git a/tests/config/one_cs7.cfg b/tests/config/one_cs7.cfg new file mode 100644 index 0000000..83862d8 --- /dev/null +++ b/tests/config/one_cs7.cfg @@ -0,0 +1,3 @@ +# auto-configuration will pick up cs7 instance 0. Configure only the local point-code. +cs7 instance 0 + point-code 1.1.1 diff --git a/tests/config/one_cs7.vty b/tests/config/one_cs7.vty new file mode 100644 index 0000000..7011c83 --- /dev/null +++ b/tests/config/one_cs7.vty @@ -0,0 +1,28 @@ +OsmoHNBGW> enable + +OsmoHNBGW# ### ERROR: 'one_cs7.cfg' sets 'cs7 instance 0' / 'point-code 1.1.1', but osmo-hnbgw uses 0.23.5 +OsmoHNBGW# show cs7 config +cs7 instance 0 + point-code 0.23.5 + asp asp-clnt-OsmoHNBGW 2905 0 m3ua + local-ip localhost + remote-ip localhost + sctp-role client + as as-clnt-OsmoHNBGW m3ua + asp asp-clnt-OsmoHNBGW + routing-key 0 0.23.5 + +OsmoHNBGW# ### ERROR: 'one_cs7.cfg' sets 'cs7 instance 0' / 'point-code 1.1.1', but osmo-hnbgw uses 0.23.5 +OsmoHNBGW# show cnlink +IuCS: OsmoHNBGW:RI=SSN_PC,PC=0.23.5,SSN=RANAP <-> RI=SSN_PC,PC=0.23.1,SSN=RANAP + SS7 route: pc=0=0.0.0 mask=0x0=0.0.0 via AS as-clnt-OsmoHNBGW proto=m3ua ASP asp-clnt-OsmoHNBGW +IuPS: OsmoHNBGW:RI=SSN_PC,PC=0.23.5,SSN=RANAP <-> RI=SSN_PC,PC=0.23.4,SSN=RANAP + SS7 route: pc=0=0.0.0 mask=0x0=0.0.0 via AS as-clnt-OsmoHNBGW proto=m3ua ASP asp-clnt-OsmoHNBGW + +OsmoHNBGW# ### ERROR: 'one_cs7.cfg' sets 'cs7 instance 0' / 'point-code 1.1.1', but osmo-hnbgw uses 0.23.5 +OsmoHNBGW# show running-config +... +cs7 instance 0 + point-code 0.23.5 +hnbgw +... diff --git a/tests/config/one_cs7_with_addrs.cfg b/tests/config/one_cs7_with_addrs.cfg new file mode 100644 index 0000000..0369bd4 --- /dev/null +++ b/tests/config/one_cs7_with_addrs.cfg @@ -0,0 +1,12 @@ +# explicitly point at cs7 instance 0 via two address book entries for CS and PS. Leave ASP and AS to auto-configuration. +cs7 instance 0 + point-code 1.1.1 + sccp-address my-msc + point-code 1.2.3 + sccp-address my-sgsn + point-code 1.4.2 +hnbgw + iucs + remote-addr my-msc + iups + remote-addr my-sgsn diff --git a/tests/config/one_cs7_with_addrs.vty b/tests/config/one_cs7_with_addrs.vty new file mode 100644 index 0000000..59b1b76 --- /dev/null +++ b/tests/config/one_cs7_with_addrs.vty @@ -0,0 +1,46 @@ +OsmoHNBGW> enable + +OsmoHNBGW# ### ASP and AS were auto-configured +OsmoHNBGW# show cs7 config +cs7 instance 0 + point-code 1.1.1 + asp asp-clnt-OsmoHNBGW 2905 0 m3ua + local-ip localhost + remote-ip localhost + sctp-role client + as as-clnt-OsmoHNBGW m3ua + asp asp-clnt-OsmoHNBGW + routing-key 0 1.1.1 + sccp-address my-msc + routing-indicator PC + point-code 1.2.3 + sccp-address my-sgsn + routing-indicator PC + point-code 1.4.2 + +OsmoHNBGW# ### IuCS and IuPS were explicitly pointed to specific remote point-codes, using cs7 instance 0 addresses +OsmoHNBGW# show cnlink +IuCS: OsmoHNBGW:RI=SSN_PC,PC=1.1.1,SSN=RANAP <-> my-msc:RI=SSN_PC,PC=1.2.3,SSN=RANAP + SS7 route: pc=0=0.0.0 mask=0x0=0.0.0 via AS as-clnt-OsmoHNBGW proto=m3ua ASP asp-clnt-OsmoHNBGW +IuPS: OsmoHNBGW:RI=SSN_PC,PC=1.1.1,SSN=RANAP <-> my-sgsn:RI=SSN_PC,PC=1.4.2,SSN=RANAP + SS7 route: pc=0=0.0.0 mask=0x0=0.0.0 via AS as-clnt-OsmoHNBGW proto=m3ua ASP asp-clnt-OsmoHNBGW + +OsmoHNBGW# show running-config +... +cs7 instance 0 + point-code 1.1.1 + sccp-address my-msc + routing-indicator PC + point-code 1.2.3 + sccp-address my-sgsn + routing-indicator PC + point-code 1.4.2 +hnbgw + rnc-id 23 + log-prefix hnb-id + iuh + iucs + remote-addr my-msc + iups + remote-addr my-sgsn +... diff --git a/tests/config/one_cs7_with_iucs_addr.cfg b/tests/config/one_cs7_with_iucs_addr.cfg new file mode 100644 index 0000000..9e0207e --- /dev/null +++ b/tests/config/one_cs7_with_iucs_addr.cfg @@ -0,0 +1,8 @@ +# only set an explicit remote address for IuCS, let IuPS be auto-configured +cs7 instance 0 + point-code 1.1.1 + sccp-address my-msc + point-code 1.2.3 +hnbgw + iucs + remote-addr my-msc diff --git a/tests/config/one_cs7_with_iucs_addr.vty b/tests/config/one_cs7_with_iucs_addr.vty new file mode 100644 index 0000000..2e57ea1 --- /dev/null +++ b/tests/config/one_cs7_with_iucs_addr.vty @@ -0,0 +1,37 @@ +OsmoHNBGW> enable + +OsmoHNBGW# show cs7 config +cs7 instance 0 + point-code 1.1.1 + asp asp-clnt-OsmoHNBGW 2905 0 m3ua + local-ip localhost + remote-ip localhost + sctp-role client + as as-clnt-OsmoHNBGW m3ua + asp asp-clnt-OsmoHNBGW + routing-key 0 1.1.1 + sccp-address my-msc + routing-indicator PC + point-code 1.2.3 + +OsmoHNBGW# ### only IuCS was configured to go to remote-PC = 1.2.3, IuPS auto-configured to remote-PC = 0.23.4 +OsmoHNBGW# show cnlink +IuCS: OsmoHNBGW:RI=SSN_PC,PC=1.1.1,SSN=RANAP <-> my-msc:RI=SSN_PC,PC=1.2.3,SSN=RANAP + SS7 route: pc=0=0.0.0 mask=0x0=0.0.0 via AS as-clnt-OsmoHNBGW proto=m3ua ASP asp-clnt-OsmoHNBGW +IuPS: OsmoHNBGW:RI=SSN_PC,PC=1.1.1,SSN=RANAP <-> RI=SSN_PC,PC=0.23.4,SSN=RANAP + SS7 route: pc=0=0.0.0 mask=0x0=0.0.0 via AS as-clnt-OsmoHNBGW proto=m3ua ASP asp-clnt-OsmoHNBGW + +OsmoHNBGW# show running-config +... +cs7 instance 0 + point-code 1.1.1 + sccp-address my-msc + routing-indicator PC + point-code 1.2.3 +hnbgw + rnc-id 23 + log-prefix hnb-id + iuh + iucs + remote-addr my-msc +... diff --git a/tests/config/one_cs7_with_iups_addr.cfg b/tests/config/one_cs7_with_iups_addr.cfg new file mode 100644 index 0000000..96e7afa --- /dev/null +++ b/tests/config/one_cs7_with_iups_addr.cfg @@ -0,0 +1,8 @@ +# only set an explicit remote address for IuPS, let IuCS be auto-configured +cs7 instance 0 + point-code 1.1.1 + sccp-address my-sgsn + point-code 1.4.2 +hnbgw + iups + remote-addr my-sgsn diff --git a/tests/config/one_cs7_with_iups_addr.vty b/tests/config/one_cs7_with_iups_addr.vty new file mode 100644 index 0000000..1e4346f --- /dev/null +++ b/tests/config/one_cs7_with_iups_addr.vty @@ -0,0 +1,37 @@ +OsmoHNBGW> enable + +OsmoHNBGW# show cs7 config +cs7 instance 0 + point-code 1.1.1 + asp asp-clnt-OsmoHNBGW 2905 0 m3ua + local-ip localhost + remote-ip localhost + sctp-role client + as as-clnt-OsmoHNBGW m3ua + asp asp-clnt-OsmoHNBGW + routing-key 0 1.1.1 + sccp-address my-sgsn + routing-indicator PC + point-code 1.4.2 + +OsmoHNBGW# ### only IuPS was configured to go to remote-PC = 1.4.2, IuCS auto-configured to remote-PC = 0.23.1 +OsmoHNBGW# show cnlink +IuCS: OsmoHNBGW:RI=SSN_PC,PC=1.1.1,SSN=RANAP <-> RI=SSN_PC,PC=0.23.1,SSN=RANAP + SS7 route: pc=0=0.0.0 mask=0x0=0.0.0 via AS as-clnt-OsmoHNBGW proto=m3ua ASP asp-clnt-OsmoHNBGW +IuPS: OsmoHNBGW:RI=SSN_PC,PC=1.1.1,SSN=RANAP <-> my-sgsn:RI=SSN_PC,PC=1.4.2,SSN=RANAP + SS7 route: pc=0=0.0.0 mask=0x0=0.0.0 via AS as-clnt-OsmoHNBGW proto=m3ua ASP asp-clnt-OsmoHNBGW + +OsmoHNBGW# show running-config +... +cs7 instance 0 + point-code 1.1.1 + sccp-address my-sgsn + routing-indicator PC + point-code 1.4.2 +hnbgw + rnc-id 23 + log-prefix hnb-id + iuh + iups + remote-addr my-sgsn +... -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/32991 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: Ic8bb30e1dd73753c2ff255566382e241918414f7 Gerrit-Change-Number: 32991 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 1 month

1
0
0 0

[S] Change in osmo-bts[master]: all models, HR1 codec: accept both TS101318 and RFC5993 formats

by falconia

Attention is currently required from: laforge, pespin, dexter. falconia has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32968 ) Change subject: all models, HR1 codec: accept both TS101318 and RFC5993 formats ...................................................................... Patch Set 2: (2 comments) File src/common/l1sap.c: https://gerrit.osmocom.org/c/osmo-bts/+/32968/comment/6ee397ff_481526d5 PS1, Line 1264: static bool rtppayload_validate_hr(struct msgb *msg) > The same is already the case for rtppayload_validate_{fr,efr} - those functions call osmo_{fr,efr}_s […] Done https://gerrit.osmocom.org/c/osmo-bts/+/32968/comment/2522598a_4f057f2d PS1, Line 1279: memmove(msg->data, msg->data + 1, GSM_HR_BYTES); : msgb_get(msg, 1); > Please read the long C comment on the 7 lines immediately preceding your highlight. […] In my new patch series I was able to eliminate memmove and convert to pointer manipulation only in the case of plain RTP input, i.e., not Osmux. In the case of Osmux I am not able to find a way to avoid memmove for the header octet stripping operation. Can we just live with it and declare the combination of Osmux + HR1 codec + RFC 5993 input + slow CPU on the BTS to be non-recommended? Please note that at least on sysmoBTS ARM926, the effect of misaligned l1sap header being pushed on top of a shifted-by-1 payload is much much worse than the memmove penalty - when I tried the msgb_pull() approach, the BTS is immediately brought into totally unusable state with just one active TCH, whereas a memmove approach like what I had here originally (see falconia/rtp_traulike branch) works just fine on my sysmoBTS. -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32968 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I702e26c3ad5b9d8347e73c6cd23efa38a3a3407e Gerrit-Change-Number: 32968 Gerrit-PatchSet: 2 Gerrit-Owner: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 24 May 2023 23:03:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: falconia <falcon(a)freecalypso.org> Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

[M] Change in osmo-bts[master]: all models, HR1 codec: select RTP output format via vty option

by falconia

Attention is currently required from: laforge, pespin, dexter. falconia has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32970 ) Change subject: all models, HR1 codec: select RTP output format via vty option ...................................................................... Patch Set 2: (3 comments) File src/common/bts.c: https://gerrit.osmocom.org/c/osmo-bts/+/32970/comment/55f41ae9_9ce7aa75 PS1, Line 333: bts->emit_hr_rfc5993 = true; > I am not sure if it is wise to have a static default. I am a bit worried about existing deployments. […] The new patch implements defaults that match previous model-specific behavior. However, I am not using BTS feature flags, instead I have just one switch statement based on bts->variant in just one place where the default is set. Rationale: with the current code there is no inherent property in any BTS model that makes one format or the other more native for that model - instead the internal native format is always TS 101 318 for all models, while RFC 5993 remains an external format only. The "thing" being tested for the purpose of the default setting is not inherent properties of this or that BTS model, but existing behavior deployed in the field. Hence feature flags do not seem appropriate. File src/common/l1sap.c: https://gerrit.osmocom.org/c/osmo-bts/+/32970/comment/fc7bc115_36fd830e PS1, Line 1705: send_rtp_rfc5993(lchan, fn, msg->data, msg->len); > Good point. […] Done File src/common/vty.c: https://gerrit.osmocom.org/c/osmo-bts/+/32970/comment/23e2a037_da8ed4de PS1, Line 802: DEFUN(cfg_bts_rtp_hr_format, > Point taken - I shall look into doing what you suggest. Done -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32970 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I168191874a062429a57904511a1e89e3f588732e Gerrit-Change-Number: 32970 Gerrit-PatchSet: 2 Gerrit-Owner: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: dexter <pmaier(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 24 May 2023 22:52:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: falconia <falcon(a)freecalypso.org> Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

[S] Change in osmo-bts[master]: all models, HR1 codec: accept both TS101318 and RFC5993 formats

by falconia

Attention is currently required from: laforge, pespin, dexter. Hello Jenkins Builder, dexter, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bts/+/32968 to look at the new patch set (#2). Change subject: all models, HR1 codec: accept both TS101318 and RFC5993 formats ...................................................................... all models, HR1 codec: accept both TS101318 and RFC5993 formats There exist two different RTP encoding formats for HR1 codec: one "simple" format defined in ETSI TS 101 318, and an extended format (adding a ToC octet) defined in IETF RFC 5993. Following the liberal-accept clause of Postel's law, we would like to accept both formats. Implement this feature by first converting all HR1 RTP inputs to the more basic TS 101 318 format at the point of input preening, and then consistently using this basic format internally. Related: OS#5688 Depends: I13eaad366f9f68615b9e9e4a5f87396a0e9dea0f (libosmocore) Change-Id: I702e26c3ad5b9d8347e73c6cd23efa38a3a3407e --- M TODO-RELEASE M src/common/rtp_input_preen.c 2 files changed, 39 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/68/32968/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32968 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I702e26c3ad5b9d8347e73c6cd23efa38a3a3407e Gerrit-Change-Number: 32968 Gerrit-PatchSet: 2 Gerrit-Owner: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: newpatchset

2 years, 1 month

1
0
0 0

[M] Change in osmo-bts[master]: all models, HR1 codec: select RTP output format via vty option

by falconia

Attention is currently required from: laforge, pespin, dexter. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bts/+/32970 to look at the new patch set (#2). Change subject: all models, HR1 codec: select RTP output format via vty option ...................................................................... all models, HR1 codec: select RTP output format via vty option The new vty option "rtp hr-format (rfc5993|ts101318)" selects the RTP output format to be used for HR1 codec, consistently across all models. The default is set to match legacy behavior: ts101318 on osmo-bts-{lc15,oc2g,sysmo} and rfc5993 on osmo-bts-trx. On models where no legacy behavior is applicable, the default is set to rfc5993 as a forward-looking measure - see OS#6036. Closes: OS#5688 Change-Id: I168191874a062429a57904511a1e89e3f588732e --- M include/osmo-bts/bts.h M src/common/bts.c M src/common/l1sap.c M src/common/vty.c M tests/osmo-bts.vty 5 files changed, 80 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/70/32970/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32970 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I168191874a062429a57904511a1e89e3f588732e Gerrit-Change-Number: 32970 Gerrit-PatchSet: 2 Gerrit-Owner: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: dexter <pmaier(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: newpatchset

2 years, 1 month

1
0
0 0

[L] Change in osmo-bts[master]: refactor: replace rtppayload_is_valid() with preening before enqueue

by falconia

falconia has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bts/+/32988 ) Change subject: refactor: replace rtppayload_is_valid() with preening before enqueue ...................................................................... refactor: replace rtppayload_is_valid() with preening before enqueue Up until now, our approach to validating incoming RTP payloads and dropping invalid ones has been to apply the preening function inside l1sap_tch_rts_ind(), at the point of dequeueing from the DL input queue. However, there are some RTP formats where we need to strip one byte of header from the payload before passing the rest to our innards: there is RFC 5993 for HR codec, and there also exists a non-standard extension (rtp_traulike) that does a similar deal for FR and EFR. Because of alignment issues, it will be much efficient (avoids memmove) if we can do this header octet stripping before we copy the payload into msgb - but doing so requires that we move this preening logic to the point of RTP input before enqueueing. Make this change. Related: OS#5688 Change-Id: I7fc99aeecba8303b56d397b8952de5eea82b301e --- M include/osmo-bts/Makefile.am A include/osmo-bts/rtp_input_preen.h M src/common/Makefile.am M src/common/l1sap.c M src/common/osmux.c A src/common/rtp_input_preen.c 6 files changed, 216 insertions(+), 72 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/88/32988/1 diff --git a/include/osmo-bts/Makefile.am b/include/osmo-bts/Makefile.am index f24da53..0e45d70 100644 --- a/include/osmo-bts/Makefile.am +++ b/include/osmo-bts/Makefile.am @@ -12,6 +12,7 @@ oml.h \ paging.h \ rsl.h \ + rtp_input_preen.h \ signal.h \ vty.h \ amr.h \ diff --git a/include/osmo-bts/rtp_input_preen.h b/include/osmo-bts/rtp_input_preen.h new file mode 100644 index 0000000..3847845 --- /dev/null +++ b/include/osmo-bts/rtp_input_preen.h @@ -0,0 +1,20 @@ +/* + * A function common to RTP and Osmux input paths: validates incoming RTP + * payloads, makes the accept-or-drop decision, and for some codecs signals + * additional required actions such as dropping one header octet. + */ + +#pragma once + +#include <stdint.h> +#include <osmo-bts/lchan.h> + +enum pl_input_decision { + PL_DECISION_DROP, + PL_DECISION_ACCEPT_ASIS, + PL_DECISION_STRIP_HDR_OCTET, +}; + +enum pl_input_decision +rtp_payload_input_preen(struct gsm_lchan *lchan, const uint8_t *rtp_pl, + unsigned rtp_pl_len); diff --git a/src/common/Makefile.am b/src/common/Makefile.am index 830f940..32f644c 100644 --- a/src/common/Makefile.am +++ b/src/common/Makefile.am @@ -36,6 +36,7 @@ bts_sm.c \ bts_trx.c \ rsl.c \ + rtp_input_preen.c \ vty.c \ paging.c \ measurement.c \ diff --git a/src/common/l1sap.c b/src/common/l1sap.c index f24bc2f..8142a86 100644 --- a/src/common/l1sap.c +++ b/src/common/l1sap.c @@ -54,6 +54,7 @@ #include <osmo-bts/bts_model.h> #include <osmo-bts/handover.h> #include <osmo-bts/msg_utils.h> +#include <osmo-bts/rtp_input_preen.h> #include <osmo-bts/pcuif_proto.h> #include <osmo-bts/cbch.h> @@ -1216,83 +1217,23 @@ return 1; } -static bool rtppayload_is_octet_aligned(const uint8_t *rtp_pl, uint8_t payload_len) +/* This helper function for l1sap_tch_rts_ind() preens incoming RTP frames + * for FR/EFR SID: if the received frame is a valid SID per the rules of + * GSM 06.31/06.81 section 6.1.1, it needs to be rejuvenated by clearing + * all reserved bits and resetting the SID code word to error-free 95 zeros + * for FR or 95 ones for EFR, and if the received frame is an invalid SID + * per the same rules, then we need to drop it. We return false if + * l1sap_tch_rts_ind() needs to drop this frame, otherwise true. */ +static bool rtppayload_sid_preen(struct gsm_lchan *lchan, struct msgb *msg) { - /* - * Logic: If 1st bit padding is not zero, packet is either: - * - bandwidth-efficient AMR payload. - * - malformed packet. - * However, Bandwidth-efficient AMR 4,75 frame last in payload(F=0, FT=0) - * with 4th,5ht,6th AMR payload to 0 matches padding==0. - * Furthermore, both AMR 4,75 bw-efficient and octet alignment are 14 bytes long (AMR 4,75 encodes 95b): - * bw-efficient: 95b, + 4b hdr + 6b ToC = 105b, + padding = 112b = 14B. - * octet-aligned: 1B hdr + 1B ToC + 95b = 111b, + padding = 112b = 14B. - * We cannot use other fields to match since they are inside the AMR - * payload bits which are unknown. - * As a result, this function may return false positive (true) for some AMR - * 4,75 AMR frames, but given the length, CMR and FT read is the same as a - * consequence, the damage in here is harmless other than being unable to - * decode the audio at the other side. - */ - #define AMR_PADDING1(rtp_pl) (rtp_pl[0] & 0x0f) - #define AMR_PADDING2(rtp_pl) (rtp_pl[1] & 0x03) - - if (payload_len < 2 || AMR_PADDING1(rtp_pl) || AMR_PADDING2(rtp_pl)) - return false; - - return true; -} - -static bool rtppayload_validate_fr(struct msgb *msg) -{ - if (msg->len != GSM_FR_BYTES) - return false; - if ((msg->data[0] & 0xF0) != 0xD0) - return false; - return osmo_fr_sid_preen(msg->data); -} - -static bool rtppayload_validate_efr(struct msgb *msg) -{ - if (msg->len != GSM_EFR_BYTES) - return false; - if ((msg->data[0] & 0xF0) != 0xC0) - return false; - return osmo_efr_sid_preen(msg->data); -} - -static bool rtppayload_is_valid(struct gsm_lchan *lchan, struct msgb *resp_msg) -{ - /* If rtp continuous-streaming is enabled, we shall emit RTP packets - * with zero-length payloads as BFI markers. In a TrFO scenario such - * RTP packets sent by call leg A will be received by call leg B, - * hence we need to handle them gracefully. For the purposes of a BTS - * that runs on its own TDMA timing and does not need timing ticks from - * an incoming RTP stream, the correct action upon receiving such - * timing-tick-only RTP packets should be the same as when receiving - * no RTP packet at all. The simplest way to produce that behavior - * is to treat zero-length RTP payloads as invalid. */ - if (resp_msg->len == 0) - return false; - switch (lchan->tch_mode) { case GSM48_CMODE_SPEECH_V1: if (lchan->type == GSM_LCHAN_TCH_F) - return rtppayload_validate_fr(resp_msg); + return osmo_fr_sid_preen(msg->data); else - return true; /* FIXME: implement preening for HR1 */ + return true; /* FIXME: see OS#6036 */ case GSM48_CMODE_SPEECH_EFR: - return rtppayload_validate_efr(resp_msg); - case GSM48_CMODE_SPEECH_AMR: - /* Avoid forwarding bw-efficient AMR to lower layers, - * most bts models don't support it. */ - if (!rtppayload_is_octet_aligned(resp_msg->data, resp_msg->len)) { - LOGPLCHAN(lchan, DL1P, LOGL_NOTICE, - "RTP->L1: Dropping unexpected AMR encoding (bw-efficient?) %s\n", - osmo_hexdump(resp_msg->data, resp_msg->len)); - return false; - } - return true; + return osmo_efr_sid_preen(msg->data); default: return true; } @@ -1337,7 +1278,7 @@ if (!resp_msg) { LOGPLCGT(lchan, &g_time, DL1P, LOGL_DEBUG, "DL TCH Tx queue underrun\n"); resp_l1sap = &empty_l1sap; - } else if (!rtppayload_is_valid(lchan, resp_msg)) { + } else if (!rtppayload_sid_preen(lchan, resp_msg)) { msgb_free(resp_msg); resp_msg = NULL; resp_l1sap = &empty_l1sap; @@ -1943,6 +1884,20 @@ if (lchan->loopback) return; + /* initial preen */ + switch (rtp_payload_input_preen(lchan, rtp_pl, rtp_pl_len)) { + case PL_DECISION_DROP: + return; + case PL_DECISION_ACCEPT_ASIS: + break; + case PL_DECISION_STRIP_HDR_OCTET: + rtp_pl++; + rtp_pl_len--; + break; + default: + OSMO_ASSERT(0); + } + msg = l1sap_msgb_alloc(rtp_pl_len); if (!msg) return; diff --git a/src/common/osmux.c b/src/common/osmux.c index 8ca2b44..701e507 100644 --- a/src/common/osmux.c +++ b/src/common/osmux.c @@ -38,6 +38,7 @@ #include <osmo-bts/osmux.h> #include <osmo-bts/lchan.h> #include <osmo-bts/msg_utils.h> +#include <osmo-bts/rtp_input_preen.h> #include <osmo-bts/l1sap.h> /* Bitmask containing Allocated Osmux circuit ID. +7 to round up to 8 bit boundary. */ @@ -409,6 +410,29 @@ */ msgb_pull(msg, sizeof(struct osmo_phsap_prim)); + /* We have to apply the same checks as in l1sap_rtp_rx_cb(), in case + * we got an invalid payload or a codec payload where we have to + * strip off one header byte. */ + switch (rtp_payload_input_preen(lchan, msg->data, msg->len)) { + case PL_DECISION_DROP: + msgb_free(msg); + return; + case PL_DECISION_ACCEPT_ASIS: + break; + case PL_DECISION_STRIP_HDR_OCTET: + /* This case is messy: we have no choice other than memmove; + * if we were to do a simple msgb_pull(), the alignment will + * be off, and when l1sap_tch_rts_ind() later does its + * msgb_push() of l1sap header, that header will be + * misaligned, causing severe crippling effects at least + * on sysmoBTS ARM926. */ + memmove(msg->data, msg->data + 1, msg->len - 1); + msgb_get(msg, 1); + break; + default: + OSMO_ASSERT(0); + } + /* enqueue making sure the queue doesn't get too long */ lchan_dl_tch_queue_enqueue(lchan, msg, 16); } diff --git a/src/common/rtp_input_preen.c b/src/common/rtp_input_preen.c new file mode 100644 index 0000000..bb0ac11 --- /dev/null +++ b/src/common/rtp_input_preen.c @@ -0,0 +1,121 @@ +/* + * This module implements a function common to RTP and Osmux input paths: + * validates incoming RTP payloads, makes the accept-or-drop decision, + * and for some codecs signals additional required actions such as + * dropping one header octet. + * + * Author: Mychaela N. Falconia <falcon(a)freecalypso.org>, 2023 - however, + * Mother Mychaela's contributions are NOT subject to copyright. + * No rights reserved, all rights relinquished. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include <stdbool.h> +#include <stdint.h> + +#include <osmocom/core/logging.h> +#include <osmocom/core/utils.h> + +#include <osmocom/codec/codec.h> + +#include <osmo-bts/lchan.h> +#include <osmo-bts/logging.h> +#include <osmo-bts/rtp_input_preen.h> + +static bool rtppayload_is_octet_aligned(const uint8_t *rtp_pl, unsigned payload_len) +{ + /* + * Logic: If 1st bit padding is not zero, packet is either: + * - bandwidth-efficient AMR payload. + * - malformed packet. + * However, Bandwidth-efficient AMR 4,75 frame last in payload(F=0, FT=0) + * with 4th,5ht,6th AMR payload to 0 matches padding==0. + * Furthermore, both AMR 4,75 bw-efficient and octet alignment are 14 bytes long (AMR 4,75 encodes 95b): + * bw-efficient: 95b, + 4b hdr + 6b ToC = 105b, + padding = 112b = 14B. + * octet-aligned: 1B hdr + 1B ToC + 95b = 111b, + padding = 112b = 14B. + * We cannot use other fields to match since they are inside the AMR + * payload bits which are unknown. + * As a result, this function may return false positive (true) for some AMR + * 4,75 AMR frames, but given the length, CMR and FT read is the same as a + * consequence, the damage in here is harmless other than being unable to + * decode the audio at the other side. + */ + #define AMR_PADDING1(rtp_pl) (rtp_pl[0] & 0x0f) + #define AMR_PADDING2(rtp_pl) (rtp_pl[1] & 0x03) + + if (payload_len < 2 || AMR_PADDING1(rtp_pl) || AMR_PADDING2(rtp_pl)) + return false; + + return true; +} + +static enum pl_input_decision +input_preen_fr(const uint8_t *rtp_pl, unsigned rtp_pl_len) +{ + if (rtp_pl_len != GSM_FR_BYTES) + return PL_DECISION_DROP; + if ((rtp_pl[0] & 0xF0) != 0xD0) + return PL_DECISION_DROP; + return PL_DECISION_ACCEPT_ASIS; +} + +static enum pl_input_decision +input_preen_efr(const uint8_t *rtp_pl, unsigned rtp_pl_len) +{ + if (rtp_pl_len != GSM_EFR_BYTES) + return PL_DECISION_DROP; + if ((rtp_pl[0] & 0xF0) != 0xC0) + return PL_DECISION_DROP; + return PL_DECISION_ACCEPT_ASIS; +} + +enum pl_input_decision +rtp_payload_input_preen(struct gsm_lchan *lchan, const uint8_t *rtp_pl, + unsigned rtp_pl_len) +{ + /* If rtp continuous-streaming is enabled, we shall emit RTP packets + * with zero-length payloads as BFI markers. In a TrFO scenario such + * RTP packets sent by call leg A will be received by call leg B, + * hence we need to handle them gracefully. For the purposes of a BTS + * that runs on its own TDMA timing and does not need timing ticks from + * an incoming RTP stream, the correct action upon receiving such + * timing-tick-only RTP packets should be the same as when receiving + * no RTP packet at all. The simplest way to produce that behavior + * is to treat zero-length RTP payloads as invalid. */ + if (rtp_pl_len == 0) + return PL_DECISION_DROP; + + switch (lchan->tch_mode) { + case GSM48_CMODE_SPEECH_V1: + if (lchan->type == GSM_LCHAN_TCH_F) + return input_preen_fr(rtp_pl, rtp_pl_len); + else + return PL_DECISION_ACCEPT_ASIS; /* FIXME: next patch in the series */ + case GSM48_CMODE_SPEECH_EFR: + return input_preen_efr(rtp_pl, rtp_pl_len); + case GSM48_CMODE_SPEECH_AMR: + /* Avoid forwarding bw-efficient AMR to lower layers, + * most bts models don't support it. */ + if (!rtppayload_is_octet_aligned(rtp_pl, rtp_pl_len)) { + LOGPLCHAN(lchan, DL1P, LOGL_NOTICE, + "RTP->L1: Dropping unexpected AMR encoding (bw-efficient?) %s\n", + osmo_hexdump(rtp_pl, rtp_pl_len)); + return PL_DECISION_DROP; + } + return PL_DECISION_ACCEPT_ASIS; + default: + return PL_DECISION_ACCEPT_ASIS; + } +} -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32988 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I7fc99aeecba8303b56d397b8952de5eea82b301e Gerrit-Change-Number: 32988 Gerrit-PatchSet: 1 Gerrit-Owner: falconia <falcon(a)freecalypso.org> Gerrit-MessageType: newchange

2 years, 1 month

1
0
0 0

[M] Change in osmo-hnbgw[master]: drop dead code: cnlink.T_RafC

by neels

Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/32911 ) Change subject: drop dead code: cnlink.T_RafC ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > Id3eefdea889a736fd5957b80280fa45b9547b792 shows nothing in gerrit, waiting to see it and be submitte […] it's going to come soon, some way down in the branch. I'm about to submit all the patches as they are ready, but until then, you can find an "early version" here: https://gerrit.osmocom.org/plugins/gitiles/osmo-hnbgw/+/ff570eddac676987b03… A lot of other changes need to happen first, and I wanted this unused code out of the way early, for the sake of those patches between now and the new RESET implementation. -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/32911 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I7cacaec631051cf5420202f2f0dd9665a5565b17 Gerrit-Change-Number: 32911 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 24 May 2023 21:43:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

[M] Change in osmo-hnbgw[master]: use new osmo_sccp_instance_next_conn_id()

by neels

neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/32987 ) Change subject: use new osmo_sccp_instance_next_conn_id() ...................................................................... use new osmo_sccp_instance_next_conn_id() Change-Id: I0fc6e486ce5d68b06d0bc9869292f082ec7a67f0 --- M include/osmocom/hnbgw/hnbgw.h M src/osmo-hnbgw/context_map.c M src/osmo-hnbgw/hnbgw_cn.c 3 files changed, 11 insertions(+), 44 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/87/32987/1 diff --git a/include/osmocom/hnbgw/hnbgw.h b/include/osmocom/hnbgw/hnbgw.h index e67a86a..6570bdf 100644 --- a/include/osmocom/hnbgw/hnbgw.h +++ b/include/osmocom/hnbgw/hnbgw.h @@ -67,7 +67,6 @@ /* reference to the SCCP User SAP by which we communicate */ struct osmo_sccp_instance *sccp; struct osmo_sccp_user *sccp_user; - uint32_t next_conn_id; /* linked list of hnbgw_context_map */ struct llist_head map_list; diff --git a/src/osmo-hnbgw/context_map.c b/src/osmo-hnbgw/context_map.c index f610087..f8d0ecf 100644 --- a/src/osmo-hnbgw/context_map.c +++ b/src/osmo-hnbgw/context_map.c @@ -52,47 +52,6 @@ return MAP_S_CONNECTING; } -/* is a given SCCP USER SAP Connection ID in use for a given CN link? */ -static int cn_id_in_use(struct hnbgw_cnlink *cn, uint32_t id) -{ - struct hnbgw_context_map *map; - - llist_for_each_entry(map, &cn->map_list, cn_list) { - if (map->scu_conn_id == id) - return 1; - } - return 0; -} - -/* try to allocate a new SCCP User SAP Connection ID */ -static int alloc_cn_conn_id(struct hnbgw_cnlink *cn, uint32_t *id_out) -{ - uint32_t i; - uint32_t id; - - /* SUA: RFC3868 sec 3.10.4: - * The source reference number is a 4 octet long integer. - * This is allocated by the source SUA instance. - * M3UA/SCCP: ITU-T Q.713 sec 3.3: - * The "source local reference" parameter field is a three-octet field containing a - * reference number which is generated and used by the local node to identify the - * connection section after the connection section is set up. - * The coding "all ones" is reserved for future use. - * Hence, let's simply use 24 bit ids to fit all link types (excluding 0x00ffffff). - */ - - for (i = 0; i < 0x00ffffff; i++) { - id = cn->next_conn_id++; - if (cn->next_conn_id == 0x00ffffff) - cn->next_conn_id = 0; - if (!cn_id_in_use(cn, id)) { - *id_out = id; - return 1; - } - } - return -1; -} - /* Map from a HNB + ContextID to the SCCP-side Connection ID */ struct hnbgw_context_map * context_map_alloc_by_hnb(struct hnb_context *hnb, uint32_t rua_ctx_id, @@ -118,7 +77,8 @@ } } - if (alloc_cn_conn_id(cn_if_new, &new_scu_conn_id) < 0) { + new_scu_conn_id = osmo_sccp_instance_next_conn_id(map->cn_link->sccp); + if (new_scu_conn_id < 0) { LOGHNB(hnb, DMAIN, LOGL_ERROR, "Unable to allocate CN connection ID\n"); return NULL; } diff --git a/src/osmo-hnbgw/hnbgw_cn.c b/src/osmo-hnbgw/hnbgw_cn.c index d9e35af..33ec0f6 100644 --- a/src/osmo-hnbgw/hnbgw_cn.c +++ b/src/osmo-hnbgw/hnbgw_cn.c @@ -495,7 +495,6 @@ cnlink = talloc_zero(g_hnbgw, struct hnbgw_cnlink); INIT_LLIST_HEAD(&cnlink->map_list); - cnlink->next_conn_id = 1000; cnlink->sccp_user = osmo_sccp_user_bind_pc(g_hnbgw->sccp.client, "OsmoHNBGW", sccp_sap_up, OSMO_SCCP_SSN_RANAP, g_hnbgw->sccp.local_addr.pc); -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/32987 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I0fc6e486ce5d68b06d0bc9869292f082ec7a67f0 Gerrit-Change-Number: 32987 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 1 month

1
0
0 0

2025

2024

2023

2022

gerrit-log May 2023