Attention is currently required from: jolly.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34987?usp=email )
Change subject: LAPDm: Add an extra queue for UI frames
......................................................................
Patch Set 8:
(2 comments)
File include/osmocom/gsm/lapdm.h:
https://gerrit.osmocom.org/c/libosmocore/+/34987/comment/94492213_f093e230
PS8, Line 37: struct llist_head tx_ui_queue; /*!< UI frames to L1 */
TODO-RELEASE due to ABI breakage
File src/gsm/lapdm.c:
https://gerrit.osmocom.org/c/libosmocore/+/34987/comment/4d7929af_b1a62151
PS8, Line 295: while ((msg = msgb_dequeue(&dl->tx_ui_queue)))
this function reimplements the msgb_queue_free() function which we already have in msgb.h of libosmocore. We should avoid duplicating code.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34987?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I00c8ee73be8b7c564a4dee3fca3e893484f567da
Gerrit-Change-Number: 34987
Gerrit-PatchSet: 8
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-Comment-Date: Mon, 13 Nov 2023 18:30:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: jolly.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34986?usp=email )
Change subject: LAPDm: Add support for RTS based polling
......................................................................
Patch Set 8:
(4 comments)
File include/osmocom/gsm/lapdm.h:
https://gerrit.osmocom.org/c/libosmocore/+/34986/comment/b0832641_9986bde9
PS8, Line 38: uint32_t t200_timeout; /*!< T200 timeout frame number */
also breaks ABI, see my comment regarding TODO-RELEASE in previous patch
File src/gsm/lapdm.c:
https://gerrit.osmocom.org/c/libosmocore/+/34986/comment/c282cb8a_3c223707
PS8, Line 355: uint8_t chan_nr, uint8_t link_id, uint8_t pad)
unrelated cosmetic change?
https://gerrit.osmocom.org/c/libosmocore/+/34986/comment/f906a1a3_8388f7bd
PS8, Line 1617: void lapdm_entity_set_t200_fn(struct lapdm_entity *le, int *t200_fn)
this misses explanation of what the int t200_fn array is supposed to contain. Also: why is it signed and not unsigned? can thre ever be negative T200?
https://gerrit.osmocom.org/c/libosmocore/+/34986/comment/a154dd2e_20029181
PS8, Line 1625: T200 FN timer
The reader should be educated what is a "T200 FN timer"? What are the expected argument types? In which unit? Can they be NULL? ...
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34986?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I6ebe83f829d7751ea9de1d90eb478c7a628db64c
Gerrit-Change-Number: 34986
Gerrit-PatchSet: 8
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-Comment-Date: Mon, 13 Nov 2023 18:28:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: jolly.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34985?usp=email )
Change subject: LAPD: Add support for RTS based polling and T200
......................................................................
Patch Set 5:
(3 comments)
File include/osmocom/isdn/lapd_core.h:
https://gerrit.osmocom.org/c/libosmocore/+/34985/comment/d3629140_d95fb0c8
PS5, Line 155: unsigned
you are modifying a public data structure, which is ABI breakage. Old applications must not link against new lib and vice-versa. This must be documented in TODO-RELEASE as it requires a libversion bump.
File src/isdn/lapd_core.c:
https://gerrit.osmocom.org/c/libosmocore/+/34985/comment/72c0d46c_161a9fcd
PS5, Line 220: /
handle how? This requires more ddocumentation. I know the original code is not very good in terms of doxygen aPI documentation, but new functions could start with it right away.
The library API must document how it is used by an application program.
https://gerrit.osmocom.org/c/libosmocore/+/34985/comment/50e7c2ca_89a4d743
PS5, Line 399: dl->flags = flags;
I guess it would leads to undefined behaviour if somebody called lapd_dl_set_flags to enable RTS mode at a random *later* time after already having started to use the non-RTS mode. We should protect against this here. I guess the RTS flag should only be modifiable during an initial state before any timers are started?
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34985?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ib961b5a44911b99b0487641533301749c0286995
Gerrit-Change-Number: 34985
Gerrit-PatchSet: 5
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-Comment-Date: Mon, 13 Nov 2023 18:24:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: jolly.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34984?usp=email )
Change subject: LAPD: Always update N(R) in pending TX frames if V(R) is incremented
......................................................................
Patch Set 4: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34984?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I71676c709878105bfd18b9370fecc61b92796a6f
Gerrit-Change-Number: 34984
Gerrit-PatchSet: 4
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-Comment-Date: Mon, 13 Nov 2023 18:19:21 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: jolly.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34982?usp=email )
Change subject: LAPD: Prepare lapd_send_i() for RTS support
......................................................................
Patch Set 4: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34982?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I3109b7aa15c0f75f4a7458fc1c5d0ce633100f76
Gerrit-Change-Number: 34982
Gerrit-PatchSet: 4
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-Comment-Date: Mon, 13 Nov 2023 18:18:29 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: jolly.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/35007?usp=email )
Change subject: Use polling based LAPDm with frame numbers
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
commit message is missing Depends: libosmocore.git ChangeID ... lines for the libosmocore patches.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/35007?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Ic6d7902b13cf491daaa8752db78f9875387aeffd
Gerrit-Change-Number: 35007
Gerrit-PatchSet: 2
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-Comment-Date: Mon, 13 Nov 2023 18:17:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: jolly.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/35008?usp=email )
Change subject: ASCI: Repeat notification after assigning MS to VGCS/VBS channel
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/35008?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ife568b8c2756be332c0b8de21111f66f6e537c4d
Gerrit-Change-Number: 35008
Gerrit-PatchSet: 1
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-Comment-Date: Mon, 13 Nov 2023 18:13:22 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: jolly.
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bts/+/35007?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Verified-1 by Jenkins Builder
Change subject: Use polling based LAPDm with frame numbers
......................................................................
Use polling based LAPDm with frame numbers
Osmo-bts uses the new polling based LAPDm implementation.
The OML message NM_ATT_T200 is ignored, because T200 timeouts are set to
the minimal response time. Longer timeouts would cause lower throughput
in case of lost frames. Shorter timeouts would cause LAPDm to fail.
Related: OS#4074
Change-Id: Ic6d7902b13cf491daaa8752db78f9875387aeffd
---
M include/osmo-bts/bts.h
M include/osmo-bts/oml.h
M src/common/bts.c
M src/common/l1sap.c
M src/common/lchan.c
M src/common/oml.c
6 files changed, 100 insertions(+), 72 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/07/35007/2
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/35007?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Ic6d7902b13cf491daaa8752db78f9875387aeffd
Gerrit-Change-Number: 35007
Gerrit-PatchSet: 2
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-MessageType: newpatchset