Attention is currently required from: pespin.
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmo-gprs/+/31075
to look at the new patch set (#2).
Change subject: rlcmac: Support extending log categories in the future
......................................................................
rlcmac: Support extending log categories in the future
An initial header structure and LOG defines is created in a similar way
to those already existing for libosmo-gprs-llc and sndcp.
Related: OS#5500
Change-Id: Ic9ba207cafeada1b174f1b085e0d1d9a38c66b0a
---
M include/osmocom/gprs/rlcmac/Makefile.am
M include/osmocom/gprs/rlcmac/gprs_rlcmac.h
A include/osmocom/gprs/rlcmac/rlcmac.h
A include/osmocom/gprs/rlcmac/rlcmac_private.h
M src/rlcmac/misc.c
M src/rlcmac/ts_24_008.c
M src/rlcmac/ts_44_018.c
M src/rlcmac/ts_44_060.c
8 files changed, 84 insertions(+), 59 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-gprs refs/changes/75/31075/2
--
To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/31075
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-gprs
Gerrit-Branch: master
Gerrit-Change-Id: Ic9ba207cafeada1b174f1b085e0d1d9a38c66b0a
Gerrit-Change-Number: 31075
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/31075 )
Change subject: rlcmac: Support extending log categories in the future
......................................................................
Patch Set 1:
(1 comment)
File src/rlcmac/misc.c:
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-3023):
https://gerrit.osmocom.org/c/libosmo-gprs/+/31075/comment/27accc99_49c6dd94
PS1, Line 30: }
adding a line without newline at end of file
--
To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/31075
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-gprs
Gerrit-Branch: master
Gerrit-Change-Id: Ic9ba207cafeada1b174f1b085e0d1d9a38c66b0a
Gerrit-Change-Number: 31075
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-CC: Jenkins Builder
Gerrit-Comment-Date: Wed, 25 Jan 2023 18:59:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: pespin.
msuraev has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31071 )
Change subject: netdev: Fix compilation building with --disable-libmnl
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31071
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I62bdea075afb9e0cc2bbcec6dd3a930e8f7bbc40
Gerrit-Change-Number: 31071
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 25 Jan 2023 18:09:37 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: laforge, daniel, lynxis lazus.
Hello Jenkins Builder, daniel, lynxis lazus,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmocore/+/31069
to look at the new patch set (#5).
Change subject: gprs_ns2_fr: use osmo_netdev to monitor and operate network device
......................................................................
gprs_ns2_fr: use osmo_netdev to monitor and operate network device
As a result libosmogb doesn't depend directly on libmnl anymore, but
through libosmocore.
Change-Id: Ib0e499e09c50135a5c4a361332d6120f660a1a45
---
M src/core/netdev.c
M src/gb/Makefile.am
M src/gb/gprs_ns2_fr.c
M src/gb/gprs_ns2_internal.h
M tests/Makefile.am
5 files changed, 65 insertions(+), 141 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/69/31069/5
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31069
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ib0e499e09c50135a5c4a361332d6120f660a1a45
Gerrit-Change-Number: 31069
Gerrit-PatchSet: 5
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-MessageType: newpatchset
Attention is currently required from: laforge, daniel, lynxis lazus.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31069 )
Change subject: gprs_ns2_fr: use osmo_netdev to montior and operate network device
......................................................................
Patch Set 4:
This change is ready for review.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31069
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ib0e499e09c50135a5c4a361332d6120f660a1a45
Gerrit-Change-Number: 31069
Gerrit-PatchSet: 4
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Comment-Date: Wed, 25 Jan 2023 16:53:54 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-e1d/+/31073 )
Change subject: Increased the size of the FIFO/RIFO to 1600 frames (instead of 800) and changed the RIFO tests in such a manner that they do not hardcode the FIFO/RIFO size.
......................................................................
Patch Set 1:
(6 comments)
File tests/rifo/rifo_test.c:
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-3019):
https://gerrit.osmocom.org/c/osmo-e1d/+/31073/comment/3ab315ab_729e15fd
PS1, Line 18: (((int) (depth)) - (FRAMES_PER_FIFO - 1)))
code indent should use tabs where possible
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-3019):
https://gerrit.osmocom.org/c/osmo-e1d/+/31073/comment/1ce9c4f8_b8aea8ee
PS1, Line 18: (((int) (depth)) - (FRAMES_PER_FIFO - 1)))
please, no space before tabs
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-3019):
https://gerrit.osmocom.org/c/osmo-e1d/+/31073/comment/5d539fe1_9ab164fa
PS1, Line 18: (((int) (depth)) - (FRAMES_PER_FIFO - 1)))
please, no spaces at the start of a line
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-3019):
https://gerrit.osmocom.org/c/osmo-e1d/+/31073/comment/6bf6201c_8bdeb960
PS1, Line 20: ("FRAMES_PER_FIFO - 1 + "))
code indent should use tabs where possible
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-3019):
https://gerrit.osmocom.org/c/osmo-e1d/+/31073/comment/481811d6_bfe652be
PS1, Line 20: ("FRAMES_PER_FIFO - 1 + "))
please, no space before tabs
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-3019):
https://gerrit.osmocom.org/c/osmo-e1d/+/31073/comment/8c45f9db_aa0a1e2f
PS1, Line 20: ("FRAMES_PER_FIFO - 1 + "))
please, no spaces at the start of a line
--
To view, visit https://gerrit.osmocom.org/c/osmo-e1d/+/31073
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-e1d
Gerrit-Branch: master
Gerrit-Change-Id: Iebfe10aaf5244901c6ac0f4f84ac75e7947b57b8
Gerrit-Change-Number: 31073
Gerrit-PatchSet: 1
Gerrit-Owner: Christoph Lauter <christoph.lauter(a)christoph-lauter.org>
Gerrit-CC: Jenkins Builder
Gerrit-Comment-Date: Wed, 25 Jan 2023 16:43:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: pespin.
daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31071 )
Change subject: netdev: Fix compilation building with --disable-libmnl
......................................................................
Patch Set 2:
(1 comment)
File src/core/netdev.c:
https://gerrit.osmocom.org/c/libosmocore/+/31071/comment/9575831b_218ae49c
PS1, Line 845:
> Now yes, but I'd need to do that anyway for instance if an ioctl() based implementation is added (I […]
Understood
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31071
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I62bdea075afb9e0cc2bbcec6dd3a930e8f7bbc40
Gerrit-Change-Number: 31071
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 25 Jan 2023 16:28:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: daniel.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31071 )
Change subject: netdev: Fix compilation building with --disable-libmnl
......................................................................
Patch Set 2:
(1 comment)
File src/core/netdev.c:
https://gerrit.osmocom.org/c/libosmocore/+/31071/comment/c5d72226_a61b181c
PS1, Line 845:
> But you're only doing the netns enter/exit to (not) call the mnl function? […]
Now yes, but I'd need to do that anyway for instance if an ioctl() based implementation is added (I even already had an ioctl() function in a previous patch adding osmo_netdev).
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31071
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I62bdea075afb9e0cc2bbcec6dd3a930e8f7bbc40
Gerrit-Change-Number: 31071
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 25 Jan 2023 16:20:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de>
Gerrit-MessageType: comment
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31072 )
Change subject: configure --enable-libmnl: Add libmnl to libosmocore.pc.in Requires
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
The data types are exported publicly in struct osmo_mnl, hence the libmnl APIs may be used by apps using libosmocore. That's why I didn't add it to the .private one.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31072
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I8ad8418ada95e79bb6079f34c6b57817c6f6ab11
Gerrit-Change-Number: 31072
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-CC: Jenkins Builder
Gerrit-Comment-Date: Wed, 25 Jan 2023 16:19:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: pespin.
daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31071 )
Change subject: netdev: Fix compilation building with --disable-libmnl
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
File src/core/netdev.c:
https://gerrit.osmocom.org/c/libosmocore/+/31071/comment/8630c8d1_68ce5e43
PS1, Line 845:
> I would prefer to keep them below, since the netns stuff is totally orthogonal to using ioctl or lib […]
But you're only doing the netns enter/exit to (not) call the mnl function?
In any case fine with me.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31071
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I62bdea075afb9e0cc2bbcec6dd3a930e8f7bbc40
Gerrit-Change-Number: 31071
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 25 Jan 2023 16:18:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: daniel.
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmocore/+/31071
to look at the new patch set (#2).
Change subject: netdev: Fix compilation building with --disable-libmnl
......................................................................
netdev: Fix compilation building with --disable-libmnl
netdev.h doesn't expose its use of libmnl publicly. It could actually be
implemented using other subsystems internally, such as ioctl() or other
OS-dependent APIs.
Hence, if --disable-libmnl is used, still make the API available but
make it fail with -ENOTSUP on functionalities which are only implemented
through libmnl so far.
Change-Id: I62bdea075afb9e0cc2bbcec6dd3a930e8f7bbc40
---
M src/core/netdev.c
1 file changed, 31 insertions(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/71/31071/2
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31071
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I62bdea075afb9e0cc2bbcec6dd3a930e8f7bbc40
Gerrit-Change-Number: 31071
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: daniel <dwillmann(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: osmith, Hoernchen, neels, laforge, fixeria, daniel.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 )
Change subject: Add osmo_io with initial poll backend
......................................................................
Patch Set 4:
(4 comments)
File src/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/4f24affb_91ab9c56
PS3, Line 50: struct iofd_backend_ops g_iofd_ops;
> This should be resolved if we end up introducing a . […]
if a map file is introduced, this symbol will still be exported in order to be used by tests AFAIU. And what we want to avoid is exporting symbols not prefixed with osmo_ which would collide with other libs.
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/64a7ffd2_9590d754
PS3, Line 111: talloc_free(msghdr);
> For io_uring the msghdr will also be used for read requests and then the msg might continue to live […]
Better describe it in the code then.
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/9cc09570_70a0a0c3
PS3, Line 225: msg_pending = iofd_msgb_alloc(iofd);
> This path is chosen if we receive more than one message. […]
Ack. Better describe this in the code.
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/3332e966_daeee86f
PS3, Line 228: iofd->pending = msg_pending;
you may want to add OSMO_ASSERT(iofd->pending == NULL) perhaps.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0
Gerrit-Change-Number: 30934
Gerrit-PatchSet: 4
Gerrit-Owner: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 25 Jan 2023 16:17:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: daniel.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31071 )
Change subject: netdev: Fix compilation building with --disable-libmnl
......................................................................
Patch Set 1:
(2 comments)
File src/core/netdev.c:
https://gerrit.osmocom.org/c/libosmocore/+/31071/comment/287bd655_f7eff6f0
PS1, Line 845:
> You could actually have the ifdef here and log/return -ENOTSUP. […]
I would prefer to keep them below, since the netns stuff is totally orthogonal to using ioctl or libmnl or whatever.
https://gerrit.osmocom.org/c/libosmocore/+/31071/comment/5ab526c4_4a321c60
PS1, Line 855: LOGNETDEV(netdev, LOGL_NOTICE, "%s: NOT SUPPORTED. Build libosmocore with --enable-libmnl.\n", __func__);
> I'm torn whether these should actually abort the program. […]
I'll move it to ERROR.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31071
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I62bdea075afb9e0cc2bbcec6dd3a930e8f7bbc40
Gerrit-Change-Number: 31071
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: daniel <dwillmann(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 25 Jan 2023 16:04:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: osmith, Hoernchen, neels, laforge, pespin, fixeria.
daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 )
Change subject: Add osmo_io with initial poll backend
......................................................................
Patch Set 4:
(11 comments)
This change is ready for review.
Patchset:
PS3:
> In general I think we need to think about naming / namespace pollution. […]
Are you talking about a map file for all of libosmocore or just a libosmoio map file?
File src/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/8c9b0167_87a37287
PS3, Line 50: struct iofd_backend_ops g_iofd_ops;
> then better name it with an osmo_ prefix.
This should be resolved if we end up introducing a .map file, right?
File src/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/cdabab1f_b2c19701
PS4, Line 228: msg_pending = iofd_msgb_alloc(iofd);
This function now sets pending_out to the remaining msgb and iofd_handle_segmented_read is responsible for setting this. This also saves us one msgb_pending call in the handle_more case below.
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/0a9f73e5_028dca84
PS4, Line 299: * \returns 0 in case of success; a negative value in case of error
Even though almost no code uses it I guess you're right.
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/1115e0e0_632590fc
PS4, Line 356:
: struct os
Done
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/b7a0d6ae_03597afb
PS4, Line 367: iofd->io_ops = *ioops;
I agree, the name should be mandatory.
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/8471d7a4_6109cd5a
PS4, Line 414: * \param[in] headroom the headroom of the msgb when receiving data
Ack
File src/osmo_io_internal.h:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/8e8e3792_75bbe29d
PS3, Line 17: in
> I'm wondering if it had any advantage to have only a single function with an argument stating the "o […]
I'll keep it as it is
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/337a22b0_b3a229c7
PS3, Line 72: struct {
: bool read_enabled;
: bool read_pending;
: bool write_pending;
: bool write_enabled;
: /* TODO: index into array of registered fd's? */
: } uring;
> Ack
Done
File src/osmo_io_poll.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/66e76cf0_7c9f532a
PS4, Line 104: struct osmo_sockaddr saddr;
Done
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/f3a3dc66_965873ff
PS4, Line 128: if (rc < 0) {
Does the kernel use addrlen for more than just checking if it's long enough for the variant? In any case I guess we only want to copy to the kernel what we really need.
I think should be a helper function in libosmocore like osmo_sockaddr_size() that returns the size depending on the family that way we can reuse it elsewhere.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0
Gerrit-Change-Number: 30934
Gerrit-PatchSet: 4
Gerrit-Owner: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 25 Jan 2023 16:03:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: pespin.
daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31071 )
Change subject: netdev: Fix compilation building with --disable-libmnl
......................................................................
Patch Set 1:
(2 comments)
File src/core/netdev.c:
https://gerrit.osmocom.org/c/libosmocore/+/31071/comment/eebc3667_46954c13
PS1, Line 845:
You could actually have the ifdef here and log/return -ENOTSUP. Then there's no message about bringing an interface up/down that we know is wrong.
https://gerrit.osmocom.org/c/libosmocore/+/31071/comment/9a4c71d3_db4982bd
PS1, Line 855: LOGNETDEV(netdev, LOGL_NOTICE, "%s: NOT SUPPORTED. Build libosmocore with --enable-libmnl.\n", __func__);
I'm torn whether these should actually abort the program. This should at least be LOGL_ERROR, though.
Same on the other hunks
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31071
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I62bdea075afb9e0cc2bbcec6dd3a930e8f7bbc40
Gerrit-Change-Number: 31071
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: daniel <dwillmann(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 25 Jan 2023 16:02:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31063 )
Change subject: Introduce tundev API
......................................................................
Introduce tundev API
The data structre is held private so that it can be easily extended in
the future.
Change-Id: I6f8324da9ba49b9249682e2ec5b45297f18dd8c2
---
M TODO-RELEASE
M include/osmocom/core/Makefile.am
A include/osmocom/core/tun.h
M src/core/Makefile.am
A src/core/tun.c
5 files changed, 617 insertions(+), 0 deletions(-)
Approvals:
Jenkins Builder: Verified
pespin: Looks good to me, approved
diff --git a/TODO-RELEASE b/TODO-RELEASE
index 80e6bc1..7a8833d 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -11,3 +11,4 @@
libosmocore ABI breakage OSMO_NUM_DLIB change affecting internal_cat[]
libosmocore new API osmo_netns_*()
libosmocore new API osmo_netdev_*()
+libosmocore new API osmo_tundev_*()
diff --git a/include/osmocom/core/Makefile.am b/include/osmocom/core/Makefile.am
index 3b2ef30..e1cd92a 100644
--- a/include/osmocom/core/Makefile.am
+++ b/include/osmocom/core/Makefile.am
@@ -55,6 +55,7 @@
thread.h \
timer.h \
timer_compat.h \
+ tun.h \
utils.h \
write_queue.h \
sockaddr_str.h \
diff --git a/include/osmocom/core/tun.h b/include/osmocom/core/tun.h
new file mode 100644
index 0000000..86bd8df
--- /dev/null
+++ b/include/osmocom/core/tun.h
@@ -0,0 +1,43 @@
+/*! \file tun.h
+ * tunnel network device convenience functions. */
+
+#pragma once
+#if (!EMBEDDED)
+
+#include <stddef.h>
+#include <stdint.h>
+
+#include <osmocom/core/msgb.h>
+#include <osmocom/core/socket.h>
+#include <osmocom/core/netdev.h>
+
+struct osmo_tundev;
+
+/* callback user gets ownership of the msgb and is expected to free it. */
+typedef int (*osmo_tundev_data_ind_cb_t)(struct osmo_tundev *tun, struct msgb *msg);
+
+struct osmo_tundev *osmo_tundev_alloc(void *ctx, const char *name);
+void osmo_tundev_free(struct osmo_tundev *tundev);
+int osmo_tundev_open(struct osmo_tundev *tundev);
+int osmo_tundev_close(struct osmo_tundev *tundev);
+bool osmo_tundev_is_open(struct osmo_tundev *tundev);
+
+void osmo_tundev_set_priv_data(struct osmo_tundev *tundev, void *priv_data);
+void *osmo_tundev_get_priv_data(struct osmo_tundev *tundev);
+
+void osmo_tundev_set_data_ind_cb(struct osmo_tundev *tundev, osmo_tundev_data_ind_cb_t data_ind_cb);
+
+const char *osmo_tundev_get_name(const struct osmo_tundev *tundev);
+
+int osmo_tundev_set_dev_name(struct osmo_tundev *tundev, const char *dev_name);
+const char *osmo_tundev_get_dev_name(const struct osmo_tundev *tundev);
+
+int osmo_tundev_set_netns_name(struct osmo_tundev *tundev, const char *netns);
+const char *osmo_tundev_get_netns_name(const struct osmo_tundev *tundev);
+
+struct osmo_netdev *osmo_tundev_get_netdev(struct osmo_tundev *tundev);
+
+int osmo_tundev_send(struct osmo_tundev *tundev, struct msgb *msg);
+
+#endif /* (!EMBEDDED) */
+/*! @} */
diff --git a/src/core/Makefile.am b/src/core/Makefile.am
index 04201a8..e942a49 100644
--- a/src/core/Makefile.am
+++ b/src/core/Makefile.am
@@ -71,6 +71,7 @@
timer.c \
timer_gettimeofday.c \
timer_clockgettime.c \
+ tun.c \
use_count.c \
utils.c \
write_queue.c \
diff --git a/src/core/tun.c b/src/core/tun.c
new file mode 100644
index 0000000..7f61183
--- /dev/null
+++ b/src/core/tun.c
@@ -0,0 +1,571 @@
+
+/* TUN interface functions.
+ * (C) 2023 by sysmocom - s.m.f.c. GmbH <info(a)sysmocom.de>
+ * Author: Pau Espin Pedrol <pespin(a)sysmocom.de>
+ *
+ * All Rights Reserved
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 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.
+ *
+ */
+
+#include "config.h"
+
+/*! \addtogroup tun
+ * @{
+ * tun network device (interface) convenience functions
+ *
+ * \file tundev.c
+ *
+ * Example lifecycle use of the API:
+ *
+ * struct osmo_sockaddr_str osa_str = {};
+ * struct osmo_sockaddr osa = {};
+ *
+ * // Allocate object:
+ * struct osmo_tundev *tundev = osmo_tundev_alloc(parent_talloc_ctx, name);
+ * OSMO_ASSERT(tundev);
+ *
+ * // Configure object (before opening):
+ * osmo_tundev_set_data_ind_cb(tun, tun_data_ind_cb);
+ * rc = osmo_tundev_set_dev_name(tun, "mytunnel0");
+ * rc = osmo_tundev_set_netns_name(tun, "some_netns_name_or_null");
+ *
+ * // Open the tundev object:
+ * rc = osmo_tundev_open(tundev);
+ * // The tunnel device is now created and an associatd netdev object
+ * // is available to operate the device:
+ * struct osmo_netdev *netdev = osmo_tundev_get_netdev(tundev);
+ * OSMO_ASSERT(netdev);
+ *
+ * // Add a local IPv4 address:
+ * rc = osmo_sockaddr_str_from_str2(&osa_str, "192.168.200.1");
+ * rc = osmo_sockaddr_str_to_sockaddr(&osa_str, &osa.u.sas);
+ * rc = osmo_netdev_add_addr(netdev, &osa, 24);
+ *
+ * // Bring network interface up:
+ * rc = osmo_netdev_ifupdown(netdev, true);
+ *
+ * // Add default route (0.0.0.0/0):
+ * rc = osmo_sockaddr_str_from_str2(&osa_str, "0.0.0.0");
+ * rc = osmo_sockaddr_str_to_sockaddr(&osa_str, &osa.u.sas);
+ * rc = osmo_netdev_add_route(netdev, &osa, 0, NULL);
+ *
+ * // Close the tunnel (asssociated netdev object becomes unavailable)
+ * rc = osmo_tundev_close(tundev);
+ * // Free the object:
+ * osmo_tundev_free(tundev);
+ */
+
+#if (!EMBEDDED)
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <errno.h>
+#include <ifaddrs.h>
+#include <sys/types.h>
+#include <sys/time.h>
+#include <sys/ioctl.h>
+#include <sys/socket.h>
+#include <sys/stat.h>
+#include <sys/time.h>
+#include <net/if.h>
+
+#if defined(__linux__)
+#include <linux/if_tun.h>
+#else
+#error "Unknown platform!"
+#endif
+
+#include <osmocom/core/utils.h>
+#include <osmocom/core/select.h>
+#include <osmocom/core/linuxlist.h>
+#include <osmocom/core/logging.h>
+#include <osmocom/core/write_queue.h>
+#include <osmocom/core/socket.h>
+#include <osmocom/core/netns.h>
+#include <osmocom/core/netdev.h>
+#include <osmocom/core/tun.h>
+
+#define TUN_DEV_PATH "/dev/net/tun"
+#define TUN_PACKET_MAX 8196
+
+#define LOGTUN(tun, lvl, fmt, args ...) \
+ LOGP(DLGLOBAL, lvl, "TUN(%s,if=%s/%u,ns=%s): " fmt, \
+ (tun)->name, (tun)->dev_name ? : "", \
+ (tun)->ifindex, (tun)->netns_name ? : "", ## args)
+
+struct osmo_tundev {
+ /* Name used to identify the osmo_tundev */
+ char *name;
+
+ /* netdev managing the tun interface: */
+ struct osmo_netdev *netdev;
+
+ /* ifindiex of the currently opened tunnel interface */
+ unsigned int ifindex;
+
+ /* Network interface name to use when setting up the tun device.
+ * NULL = let the system pick one. */
+ char *dev_name;
+ /* Whether dev_name is set by user or dynamically allocated by system */
+ bool dev_name_dynamic;
+
+ /* Write queue used since tun fd is set non-blocking */
+ struct osmo_wqueue wqueue;
+
+ /* netns name where the tun interface is created (NULL = default netns) */
+ char *netns_name;
+
+ /* API user private data */
+ void *priv_data;
+
+ /* Called by tundev each time a new packet is received on the tun interface. Can be NULL. */
+ osmo_tundev_data_ind_cb_t data_ind_cb;
+
+ /* Whether the tundev is in opened state (managing the tun interface) */
+ bool opened;
+};
+
+/* A new pkt arrived from the tun device, dispatch it to the API user */
+static int tundev_decaps(struct osmo_tundev *tundev)
+{
+ struct msgb *msg;
+ int rc;
+
+ msg = msgb_alloc(TUN_PACKET_MAX, "tundev_rx");
+
+ if ((rc = read(tundev->wqueue.bfd.fd, msgb_data(msg), TUN_PACKET_MAX)) <= 0) {
+ LOGTUN(tundev, LOGL_ERROR, "read() failed: %s (%d)\n", strerror(errno), errno);
+ msgb_free(msg);
+ return -1;
+ }
+ msgb_put(msg, rc);
+
+ if (tundev->data_ind_cb)
+ return tundev->data_ind_cb(tundev, msg);
+
+ msgb_free(msg);
+ return 0;
+}
+
+/* callback for tun device osmocom select loop integration */
+static int tundev_read_cb(struct osmo_fd *fd)
+{
+ struct osmo_tundev *tundev = fd->data;
+ return tundev_decaps(tundev);
+}
+
+/* callback for tun device osmocom select loop integration */
+static int tundev_write_cb(struct osmo_fd *fd, struct msgb *msg)
+{
+ struct osmo_tundev *tundev = fd->data;
+ size_t pkt_len = msgb_length(msg);
+
+ int rc;
+ rc = write(tundev->wqueue.bfd.fd, msgb_data(msg), pkt_len);
+ if (rc < 0)
+ LOGTUN(tundev, LOGL_ERROR, "write() failed: %s (%d)\n", strerror(errno), errno);
+ else if (rc < pkt_len)
+ LOGTUN(tundev, LOGL_ERROR, "short write() %d < %zu\n", rc, pkt_len);
+ return rc;
+}
+
+static int tundev_ifupdown_ind_cb(struct osmo_netdev *netdev, bool ifupdown)
+{
+ struct osmo_tundev *tundev = osmo_netdev_get_priv_data(netdev);
+ LOGTUN(tundev, LOGL_NOTICE, "Physical link state changed: %s\n",
+ ifupdown ? "UP" : "DOWN");
+
+ /* free any backlog, both on IFUP and IFDOWN. Keep the LMI, as it makes
+ * sense to get one out of the door ASAP. */
+ osmo_wqueue_clear(&tundev->wqueue);
+ return 0;
+}
+
+static int tundev_dev_name_chg_cb(struct osmo_netdev *netdev, const char *new_dev_name)
+{
+ struct osmo_tundev *tundev = osmo_netdev_get_priv_data(netdev);
+ LOGTUN(tundev, LOGL_NOTICE, "netdev changed name: %s -> %s\n",
+ osmo_netdev_get_dev_name(netdev), new_dev_name);
+
+ if (tundev->dev_name_dynamic) {
+ osmo_talloc_replace_string(tundev, &tundev->dev_name, new_dev_name);
+ } else {
+ /* TODO: in here we probably want to force the iface name back
+ * to tundev->dev_name one we have a osmo_netdev_set_ifname() API */
+ osmo_talloc_replace_string(tundev, &tundev->dev_name, new_dev_name);
+ }
+
+ return 0;
+}
+
+static int tundev_mtu_chg_cb(struct osmo_netdev *netdev, uint32_t new_mtu)
+{
+ struct osmo_tundev *tundev = osmo_netdev_get_priv_data(netdev);
+ LOGTUN(tundev, LOGL_NOTICE, "netdev changed MTU: %u\n", new_mtu);
+
+ return 0;
+}
+
+/*! Allocate a new tundev object.
+ * \param[in] ctx talloc context to use as a parent when allocating the tundev object
+ * \param[in] name A name providen to identify the tundev object
+ * \returns newly allocated tundev object on success; NULL on error
+ */
+struct osmo_tundev *osmo_tundev_alloc(void *ctx, const char *name)
+{
+ struct osmo_tundev *tundev;
+
+ tundev = talloc_zero(ctx, struct osmo_tundev);
+ if (!tundev)
+ return NULL;
+
+ tundev->netdev = osmo_netdev_alloc(tundev, name);
+ if (!tundev->netdev) {
+ talloc_free(tundev);
+ return NULL;
+ }
+ osmo_netdev_set_priv_data(tundev->netdev, tundev);
+ osmo_netdev_set_ifupdown_ind_cb(tundev->netdev, tundev_ifupdown_ind_cb);
+ osmo_netdev_set_dev_name_chg_cb(tundev->netdev, tundev_dev_name_chg_cb);
+ osmo_netdev_set_mtu_chg_cb(tundev->netdev, tundev_mtu_chg_cb);
+
+ tundev->name = talloc_strdup(tundev, name);
+ osmo_wqueue_init(&tundev->wqueue, 1000);
+ osmo_fd_setup(&tundev->wqueue.bfd, -1, OSMO_FD_READ, osmo_wqueue_bfd_cb, tundev, 0);
+ tundev->wqueue.read_cb = tundev_read_cb;
+ tundev->wqueue.write_cb = tundev_write_cb;
+
+ return tundev;
+}
+
+/*! Free an allocated tundev object.
+ * \param[in] tundev The tundev object to free
+ */
+void osmo_tundev_free(struct osmo_tundev *tundev)
+{
+ if (!tundev)
+ return;
+ osmo_tundev_close(tundev);
+ osmo_netdev_free(tundev->netdev);
+ talloc_free(tundev);
+}
+
+/*! Open and configure fd of the tunnel device.
+ * \param[in] tundev The tundev object whose tunnel interface to open
+ * \param[in] flags internal linux flags to pass when creating the device (not used yet)
+ * \returns 0 on success; negative on error
+ */
+static int tundev_open_fd(struct osmo_tundev *tundev, int flags)
+{
+ struct ifreq ifr;
+ int fd, rc;
+
+ fd = open(TUN_DEV_PATH, O_RDWR);
+ if (fd < 0) {
+ LOGTUN(tundev, LOGL_ERROR, "Cannot open " TUN_DEV_PATH ": %s\n", strerror(errno));
+ return fd;
+ }
+
+ memset(&ifr, 0, sizeof(ifr));
+ ifr.ifr_flags = IFF_TUN | IFF_NO_PI | flags;
+ if (tundev->dev_name) {
+ /* if a TUN interface name was specified, put it in the structure; otherwise,
+ the kernel will try to allocate the "next" device of the specified type */
+ osmo_strlcpy(ifr.ifr_name, tundev->dev_name, IFNAMSIZ);
+ }
+
+ /* try to create the device */
+ rc = ioctl(fd, TUNSETIFF, (void *) &ifr);
+ if (rc < 0)
+ goto close_ret;
+
+ /* Read name back from device */
+ if (!tundev->dev_name) {
+ ifr.ifr_name[IFNAMSIZ - 1] = '\0';
+ tundev->dev_name = talloc_strdup(tundev, ifr.ifr_name);
+ tundev->dev_name_dynamic = true;
+ }
+
+ /* Store interface index:
+ * (Note: there's a potential race condition here between creating the
+ * iface with a given name above and attempting to retrieve its ifindex based
+ * on that name. Someone (ie udev) could have the iface renamed in
+ * between here. It's a pity that TUNSETIFF doesn't copy back to us the
+ * newly allocated ifinidex as it does with ifname)
+ */
+ tundev->ifindex = if_nametoindex(tundev->dev_name);
+ if (tundev->ifindex == 0) {
+ LOGTUN(tundev, LOGL_ERROR, "Unable to find ifinidex for dev %s\n",
+ tundev->dev_name);
+ rc = -ENODEV;
+ goto close_ret;
+ }
+
+ LOGTUN(tundev, LOGL_INFO, "TUN device created\n");
+
+ /* set non-blocking: */
+ rc = fcntl(fd, F_GETFL);
+ if (rc < 0) {
+ LOGTUN(tundev, LOGL_ERROR, "fcntl(F_GETFL) failed: %s (%d)\n",
+ strerror(errno), errno);
+ goto close_ret;
+ }
+ rc = fcntl(fd, F_SETFL, rc | O_NONBLOCK);
+ if (rc < 0) {
+ LOGTUN(tundev, LOGL_ERROR, "fcntl(F_SETFL, O_NONBLOCK) failed: %s (%d)\n",
+ strerror(errno), errno);
+ goto close_ret;
+ }
+ return fd;
+
+close_ret:
+ close(fd);
+ return rc;
+}
+
+/*! Open the tunnel device owned by the tundev object.
+ * \param[in] tundev The tundev object to open
+ * \returns 0 on success; negative on error
+ */
+int osmo_tundev_open(struct osmo_tundev *tundev)
+{
+ struct osmo_netns_switch_state switch_state;
+ int rc;
+ int netns_fd = -1;
+
+ if (tundev->opened)
+ return -EALREADY;
+
+ /* temporarily switch to specified namespace to create tun device */
+ if (tundev->netns_name) {
+ LOGTUN(tundev, LOGL_INFO, "Open tun: Switch to netns '%s'\n",
+ tundev->netns_name);
+ netns_fd = osmo_netns_open_fd(tundev->netns_name);
+ if (netns_fd < 0) {
+ LOGP(DLGLOBAL, LOGL_ERROR, "Open tun: Cannot switch to netns '%s': %s (%d)\n",
+ tundev->netns_name, strerror(errno), errno);
+ return netns_fd;
+ }
+ rc = osmo_netns_switch_enter(netns_fd, &switch_state);
+ if (rc < 0) {
+ LOGTUN(tundev, LOGL_ERROR, "Open tun: Cannot switch to netns '%s': %s (%d)\n",
+ tundev->netns_name, strerror(errno), errno);
+ goto err_close_netns_fd;
+ }
+ }
+
+ tundev->wqueue.bfd.fd = tundev_open_fd(tundev, 0);
+ if (tundev->wqueue.bfd.fd < 0) {
+ LOGTUN(tundev, LOGL_ERROR, "Cannot open TUN device: %s\n", strerror(errno));
+ rc = -ENODEV;
+ goto err_restore_ns;
+ }
+
+ /* switch back to default namespace */
+ if (tundev->netns_name) {
+ rc = osmo_netns_switch_exit(&switch_state);
+ if (rc < 0) {
+ LOGTUN(tundev, LOGL_ERROR, "Open tun: Cannot switch back from netns '%s': %s\n",
+ tundev->netns_name, strerror(errno));
+ goto err_close_tun;
+ }
+ LOGTUN(tundev, LOGL_INFO, "Open tun: Back from netns '%s'\n",
+ tundev->netns_name);
+ }
+
+ rc = osmo_netdev_set_netns_name(tundev->netdev, tundev->netns_name);
+ if (rc < 0)
+ goto err_close_tun;
+ rc = osmo_netdev_set_ifindex(tundev->netdev, tundev->ifindex);
+ if (rc < 0)
+ goto err_close_tun;
+
+ rc = osmo_netdev_register(tundev->netdev);
+ if (rc < 0)
+ goto err_close_tun;
+
+ osmo_fd_register(&tundev->wqueue.bfd);
+ tundev->opened = true;
+ return 0;
+
+err_close_tun:
+ close(tundev->wqueue.bfd.fd);
+ tundev->wqueue.bfd.fd = -1;
+err_restore_ns:
+ osmo_netns_switch_exit(&switch_state);
+err_close_netns_fd:
+ if (netns_fd >= 0)
+ close(netns_fd);
+ return rc;
+}
+
+/*! Close the tunnel device owned by the tundev object.
+ * \param[in] tundev The tundev object to close
+ * \returns 0 on success; negative on error
+ */
+int osmo_tundev_close(struct osmo_tundev *tundev)
+{
+ if (!tundev->opened)
+ return -EALREADY;
+
+ osmo_wqueue_clear(&tundev->wqueue);
+ if (tundev->wqueue.bfd.fd != -1) {
+ osmo_fd_unregister(&tundev->wqueue.bfd);
+ close(tundev->wqueue.bfd.fd);
+ tundev->wqueue.bfd.fd = -1;
+ }
+
+ osmo_netdev_unregister(tundev->netdev);
+ if (tundev->dev_name_dynamic) {
+ TALLOC_FREE(tundev->dev_name);
+ tundev->dev_name_dynamic = false;
+ }
+ tundev->opened = false;
+ return 0;
+}
+
+/*! Retrieve whether the tundev object is in "opened" state.
+ * \param[in] tundev The tundev object to check
+ * \returns true if in state "opened"; false otherwise
+ */
+bool osmo_tundev_is_open(struct osmo_tundev *tundev)
+{
+ return tundev->opened;
+}
+
+/*! Set private user data pointer on the tundev object.
+ * \param[in] tundev The tundev object where the field is set
+ */
+void osmo_tundev_set_priv_data(struct osmo_tundev *tundev, void *priv_data)
+{
+ tundev->priv_data = priv_data;
+}
+
+/*! Get private user data pointer from the tundev object.
+ * \param[in] tundev The tundev object from where to retrieve the field
+ * \returns The current value of the priv_data field.
+ */
+void *osmo_tundev_get_priv_data(struct osmo_tundev *tundev)
+{
+ return tundev->priv_data;
+}
+
+/*! Set data_ind_cb callback, called when a new packet is received on the tun interface.
+ * \param[in] tundev The tundev object where the field is set
+ * \param[in] data_ind_cb the user provided function to be called when a new packet is received
+ */
+void osmo_tundev_set_data_ind_cb(struct osmo_tundev *tundev, osmo_tundev_data_ind_cb_t data_ind_cb)
+{
+ tundev->data_ind_cb = data_ind_cb;
+}
+
+/*! Get name used to identify the tundev object.
+ * \param[in] tundev The tundev object from where to retrieve the field
+ * \returns The current value of the name used to identify the tundev object
+ */
+const char *osmo_tundev_get_name(const struct osmo_tundev *tundev)
+{
+ return tundev->name;
+}
+
+/*! Set name used to name the tunnel interface created by the tundev object.
+ * \param[in] tundev The tundev object where the field is set
+ * \param[in] dev_name The tunnel interface name to use
+ * \returns 0 on success; negative on error
+ *
+ * This is used during osmo_tundev_open() time, and hence shouldn't be changed
+ * when the tundev object is in "opened" state.
+ * If left as NULL (default), the system will pick a suitable name during
+ * osmo_tundev_open(), and the field will be updated to the system-selected
+ * name, which can be retrieved later with osmo_tundev_get_dev_name().
+ */
+int osmo_tundev_set_dev_name(struct osmo_tundev *tundev, const char *dev_name)
+{
+ if (tundev->opened)
+ return -EALREADY;
+ osmo_talloc_replace_string(tundev, &tundev->dev_name, dev_name);
+ tundev->dev_name_dynamic = false;
+ return 0;
+}
+
+/*! Get name used to name the tunnel interface created by the tundev object
+ * \param[in] tundev The tundev object from where to retrieve the field
+ * \returns The current value of the configured tunnel interface name to use
+ */
+const char *osmo_tundev_get_dev_name(const struct osmo_tundev *tundev)
+{
+ return tundev->dev_name;
+}
+
+/*! Set name of the network namespace to use when opening the tunnel interface
+ * \param[in] tundev The tundev object where the field is set
+ * \param[in] netns_name The network namespace to use during tunnel interface creation
+ * \returns 0 on success; negative on error
+ *
+ * This is used during osmo_tundev_open() time, and hence shouldn't be changed
+ * when the tundev object is in "opened" state.
+ * If left as NULL (default), the system will stay in the current network namespace.
+ */
+int osmo_tundev_set_netns_name(struct osmo_tundev *tundev, const char *netns_name)
+{
+ if (tundev->opened)
+ return -EALREADY;
+ osmo_talloc_replace_string(tundev, &tundev->netns_name, netns_name);
+ return 0;
+}
+
+/*! Get name of network namespace used when opening the tunnel interface
+ * \param[in] tundev The tundev object from where to retrieve the field
+ * \returns The current value of the configured network namespace
+ */
+const char *osmo_tundev_get_netns_name(const struct osmo_tundev *tundev)
+{
+ return tundev->netns_name;
+}
+
+/*! Get netdev managing the tunnel interface of tundev
+ * \param[in] tundev The tundev object from where to retrieve the field
+ * \returns The netdev objet managing the tun interface
+ */
+struct osmo_netdev *osmo_tundev_get_netdev(struct osmo_tundev *tundev)
+{
+ return tundev->netdev;
+}
+
+/*! Submit a packet to the tunnel device managed by the tundev object
+ * \param[in] tundev The tundev object owning the tunnel device where to inject the packet
+ * \param[in] msg The msgb containg the packet to transfer
+ * \returns The current value of the configured network namespace
+ *
+ * This function takes the ownership of msg in all cases.
+ */
+int osmo_tundev_send(struct osmo_tundev *tundev, struct msgb *msg)
+{
+ int rc = osmo_wqueue_enqueue(&tundev->wqueue, msg);
+ if (rc < 0) {
+ LOGTUN(tundev, LOGL_ERROR, "Failed to enqueue the packet\n");
+ msgb_free(msg);
+ return rc;
+ }
+ return rc;
+}
+
+
+#endif /* (!EMBEDDED) */
+
+/*! @} */
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31063
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I6f8324da9ba49b9249682e2ec5b45297f18dd8c2
Gerrit-Change-Number: 31063
Gerrit-PatchSet: 5
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
Attention is currently required from: pespin.
Hello osmith, Jenkins Builder, laforge,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmocore/+/31063
to look at the new patch set (#5).
Change subject: Introduce tundev API
......................................................................
Introduce tundev API
The data structre is held private so that it can be easily extended in
the future.
Change-Id: I6f8324da9ba49b9249682e2ec5b45297f18dd8c2
---
M TODO-RELEASE
M include/osmocom/core/Makefile.am
A include/osmocom/core/tun.h
M src/core/Makefile.am
A src/core/tun.c
5 files changed, 617 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/63/31063/5
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31063
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I6f8324da9ba49b9249682e2ec5b45297f18dd8c2
Gerrit-Change-Number: 31063
Gerrit-PatchSet: 5
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: osmith.
Hello osmith, Jenkins Builder, laforge,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmocore/+/31062
to look at the new patch set (#6).
Change subject: Introduce netdev API
......................................................................
Introduce netdev API
This module provides several operations on network devices
(interfaces), like monitoring changes, setting addresses, routes, link
state, etc.
It also supports managing network interfaces on several different netns
concurrently.
These functionalitites will be used by the tun module included in a
follow-up patch.
Change-Id: I7a00c0445a89e088676a4897061b65196d9197f1
---
M TODO-RELEASE
M include/osmocom/core/Makefile.am
A include/osmocom/core/netdev.h
M src/core/Makefile.am
A src/core/netdev.c
5 files changed, 957 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/62/31062/6
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31062
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I7a00c0445a89e088676a4897061b65196d9197f1
Gerrit-Change-Number: 31062
Gerrit-PatchSet: 6
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-MessageType: newpatchset
neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/31016 )
Change subject: fix possible leak of ue_context on UE REGISTER error
......................................................................
fix possible leak of ue_context on UE REGISTER error
Deallocate a recently allocated UE context if the UE REGISTER procedure
fails internally, in two places.
The UE REGISTER error is rather unlikely to happen in practice: it only
occurs when encoding the HNBAP response fails, which only gets checked
input and thus is unlikely to fail.
The same code paths also possibly leak asn1c allocations -- leave those
for another patch.
Related: SYS#6297
Change-Id: Icf4b82f9a904d56332c567f7ccfb24231ee66270
---
M src/osmo-hnbgw/hnbgw_hnbap.c
1 file changed, 18 insertions(+), 4 deletions(-)
Approvals:
Jenkins Builder: Verified
dexter: Looks good to me, but someone else must approve
laforge: Looks good to me, approved
diff --git a/src/osmo-hnbgw/hnbgw_hnbap.c b/src/osmo-hnbgw/hnbgw_hnbap.c
index 23d1f48..4aefe2b 100644
--- a/src/osmo-hnbgw/hnbgw_hnbap.c
+++ b/src/osmo-hnbgw/hnbgw_hnbap.c
@@ -284,6 +284,7 @@
uint32_t ctx_id;
uint32_t tmsi = 0;
struct ue_context *ue;
+ struct ue_context *ue_allocated = NULL;
int rc;
memset(&accept, 0, sizeof(accept));
@@ -331,14 +332,19 @@
ue = ue_context_by_tmsi(hnb->gw, tmsi);
if (!ue)
- ue = ue_context_alloc(hnb, NULL, tmsi);
+ ue = ue_allocated = ue_context_alloc(hnb, NULL, tmsi);
asn1_u24_to_bitstring(&accept.context_ID, &ctx_id, ue->context_id);
memset(&accept_out, 0, sizeof(accept_out));
rc = hnbap_encode_ueregisteraccepties(&accept_out, &accept);
- if (rc < 0)
+ if (rc < 0) {
+ /* If we allocated the UE context but the UE REGISTER fails, get rid of it again: there will likely
+ * never be a UE DE-REGISTER for this UE from the HNB, and the ue_context would linger forever. */
+ if (ue_allocated)
+ ue_context_free(ue_allocated);
return rc;
+ }
msg = hnbap_generate_successful_outcome(HNBAP_ProcedureCode_id_UERegister,
HNBAP_Criticality_reject,
@@ -475,6 +481,7 @@
{
HNBAP_UERegisterRequestIEs_t ies;
struct ue_context *ue;
+ struct ue_context *ue_allocated = NULL;
char imsi[16];
int rc;
@@ -516,11 +523,18 @@
ue = ue_context_by_imsi(ctx->gw, imsi);
if (!ue)
- ue = ue_context_alloc(ctx, imsi, 0);
+ ue = ue_allocated = ue_context_alloc(ctx, imsi, 0);
hnbap_free_ueregisterrequesties(&ies);
/* Send UERegisterAccept */
- return hnbgw_tx_ue_register_acc(ue);
+ rc = hnbgw_tx_ue_register_acc(ue);
+ if (rc < 0) {
+ /* If we allocated the UE context but the UE REGISTER fails, get rid of it again: there will likely
+ * never be a UE DE-REGISTER for this UE from the HNB, and the ue_context would linger forever. */
+ if (ue_allocated)
+ ue_context_free(ue_allocated);
+ }
+ return rc;
}
static int hnbgw_rx_ue_deregister(struct hnb_context *ctx, ANY_t *in)
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/31016
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: Icf4b82f9a904d56332c567f7ccfb24231ee66270
Gerrit-Change-Number: 31016
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/31028 )
Change subject: fix SCCP conn leak on non-graceful HNB shutdown
......................................................................
fix SCCP conn leak on non-graceful HNB shutdown
Clean up SCCP connections when a HNB disconnects.
When a HNB disconnects, we clean up all RUA <-> SCCP connection state
for that HNB. In that cleanup, discarding the SCCP connection is so far
missing.
Add a flag indicating true between SCCP CC and DISCONNECT. Hence we can
tell during context_map_deactivate() whether the cleanup is graceful
(DISCONNECT already sent) or non-graceful (need to DISCONNECT).
Change-Id: Icc2db9f6c0b2d0a814ff1110ffbe5e8f7f629222
---
M include/osmocom/hnbgw/context_map.h
M src/osmo-hnbgw/context_map.c
M src/osmo-hnbgw/hnbgw_cn.c
M src/osmo-hnbgw/hnbgw_rua.c
4 files changed, 26 insertions(+), 1 deletion(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, but someone else must approve
pespin: Looks good to me, approved
diff --git a/include/osmocom/hnbgw/context_map.h b/include/osmocom/hnbgw/context_map.h
index 99efe8a..fe09da0 100644
--- a/include/osmocom/hnbgw/context_map.h
+++ b/include/osmocom/hnbgw/context_map.h
@@ -43,6 +43,10 @@
bool is_ps;
/* SCCP User SAP connection ID */
uint32_t scu_conn_id;
+ /* Set to true on SCCP Conn Conf, set to false when an OSMO_SCU_PRIM_N_DISCONNECT has been sent for the SCCP
+ * User SAP conn. Useful to avoid leaking SCCP connections: guarantee that an OSMO_SCU_PRIM_N_DISCONNECT gets
+ * sent, even when RUA fails to gracefully disconnect. */
+ bool scu_conn_active;
/* Pending data to be sent: when we send an "empty" SCCP CR first, the initial RANAP message will be sent in a
* separate DT once the CR is confirmed. This caches the initial RANAP message. */
struct msgb *cached_msg;
diff --git a/src/osmo-hnbgw/context_map.c b/src/osmo-hnbgw/context_map.c
index 98dffad..31ba032 100644
--- a/src/osmo-hnbgw/context_map.c
+++ b/src/osmo-hnbgw/context_map.c
@@ -26,6 +26,8 @@
#include <osmocom/core/timer.h>
+#include <osmocom/sigtran/sccp_helpers.h>
+
#include <osmocom/hnbgw/hnbgw.h>
#include <osmocom/hnbgw/hnbgw_rua.h>
#include <osmocom/hnbgw/context_map.h>
@@ -174,6 +176,12 @@
if (map->state != MAP_S_RESERVED2)
map->state = MAP_S_RESERVED1;
+ /* Is SCCP still active and needs to be disconnected ungracefully? */
+ if (map->scu_conn_active) {
+ osmo_sccp_tx_disconn(map->hnb_ctx->gw->sccp.cnlink->sccp_user, map->scu_conn_id, NULL, 0);
+ map->scu_conn_active = false;
+ }
+
/* a possibly still existing MGW FSM must be terminated when the context
* map is deactivated. (this is a cornercase) */
if (map->mgw_fi) {
diff --git a/src/osmo-hnbgw/hnbgw_cn.c b/src/osmo-hnbgw/hnbgw_cn.c
index 7cbfcec..c4189e8 100644
--- a/src/osmo-hnbgw/hnbgw_cn.c
+++ b/src/osmo-hnbgw/hnbgw_cn.c
@@ -337,6 +337,8 @@
struct osmo_prim_hdr *oph)
{
struct osmo_ss7_instance *ss7 = osmo_sccp_get_ss7(cnlink->gw->sccp.client);
+ struct hnbgw_context_map *map;
+
LOGP(DMAIN, LOGL_DEBUG, "handle_cn_conn_conf() conn_id=%d, addrs: called=%s calling=%s responding=%s\n",
param->conn_id,
osmo_sccp_addr_to_str_c(OTC_SELECT, ss7, ¶m->called_addr),
@@ -346,9 +348,17 @@
/* Nothing needs to happen for RUA, RUA towards the HNB doesn't seem to know any confirmations to its CONNECT
* operation. */
+ map = context_map_by_cn(cnlink, param->conn_id);
+ if (!map)
+ return 0;
+
+ /* SCCP connection is confirmed. Mark conn as active, i.e. requires a DISCONNECT to clean up the SCCP
+ * connection. */
+ map->scu_conn_active = true;
+
/* If our initial SCCP CR was sent without data payload, then the initial RANAP message is cached and waiting to
* be sent as soon as the SCCP connection is confirmed. See if that is the case, send cached data. */
- context_map_send_cached_msg(context_map_by_cn(cnlink, param->conn_id));
+ context_map_send_cached_msg(map);
return 0;
}
diff --git a/src/osmo-hnbgw/hnbgw_rua.c b/src/osmo-hnbgw/hnbgw_rua.c
index f4e24d7..215379a 100644
--- a/src/osmo-hnbgw/hnbgw_rua.c
+++ b/src/osmo-hnbgw/hnbgw_rua.c
@@ -252,6 +252,9 @@
prim->u.disconnect.conn_id = map->scu_conn_id;
prim->u.disconnect.cause = cause;
release_context_map = true;
+ /* Mark SCCP conn as gracefully disconnected */
+ if (map)
+ map->scu_conn_active = false;
break;
case OSMO_SCU_PRIM_N_UNITDATA:
prim->u.unitdata.called_addr = *remote_addr;
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/31028
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: Icc2db9f6c0b2d0a814ff1110ffbe5e8f7f629222
Gerrit-Change-Number: 31028
Gerrit-PatchSet: 2
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
Attention is currently required from: laforge.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/31067 )
Change subject: ts_51_011, ts_31_102: point to proper EF_EXTn file
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
Patchset:
PS2:
I guess someone already thought about this, but I'd find intersting to have some sort of unit tests here, at least encoding/decoding some buffers or filesystems under test/
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/31067
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I5301d41225266d35c05e41588811502e5595520d
Gerrit-Change-Number: 31067
Gerrit-PatchSet: 2
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Wed, 25 Jan 2023 13:08:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: laforge.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/31066 )
Change subject: ts_51_011: Implement Extended BCD Coding
......................................................................
Patch Set 2:
(2 comments)
File pySim/ts_51_011.py:
https://gerrit.osmocom.org/c/pysim/+/31066/comment/de70b284_cf61cc9e
PS2, Line 347: # we only translate a=* / b=# as they habe a clear representation
typo: have
https://gerrit.osmocom.org/c/pysim/+/31066/comment/5750772b_0aebd402
PS2, Line 366: 'dialing_nr'/ExtendedBcdAdapter(BcdAdapter(Rpad(Bytes(10)))),
Just to be sure: Shouldn't this be:
ExtendedBcdAdapter(Rpad(Bytes(10)))
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/31066
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: Ifcec13e9b296dba7bec34b7872192b7ce185c23c
Gerrit-Change-Number: 31066
Gerrit-PatchSet: 2
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Wed, 25 Jan 2023 13:06:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: laforge.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/31064 )
Change subject: ts_51_011: Fix bit-order in EF.VGCSS and EF.VBSS
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/31064
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I77674c23823aae71c9504b1a85cd75266edadc6f
Gerrit-Change-Number: 31064
Gerrit-PatchSet: 2
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Wed, 25 Jan 2023 13:03:56 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: arehbein, laforge.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31029 )
Change subject: osmo-bsc: Fix 'apply-config-file' CTRL command
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
File src/osmo-bsc/bsc_ctrl.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/31029/comment/5a93abad_b632990f
PS2, Line 89: cmd->reply = talloc_asprintf(cmd, "Errors in neighbor configuration");
> why is it too specific? The error message is only printed in case neighbors_check_cfg() is failing. […]
Ah I misread the function being called, I thought the function validating the whole BTS config was being called here, which checks several things.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31029
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I24ae8cd7e5e0d15eab9fd04b1858072bf0bad36a
Gerrit-Change-Number: 31029
Gerrit-PatchSet: 2
Gerrit-Owner: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: arehbein <arehbein(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Wed, 25 Jan 2023 13:02:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment