[M] Change in libosmocore[master]: LAPD: Add support for RTS based polling and T200
laforge has submitted this change. (
https://gerrit.osmocom.org/c/libosmocore/+/34985?usp=email )
Change subject: LAPD: Add support for RTS based polling and T200
..
LAPD: Add support for RTS based polling and T200
The T200 timer is started when the current frame is polled at
PH-READY-TO-SEND event.
A flag is used to enable this feature. The user of LAPD core must track
frame numbers to check the timeout condition. Then it must call the
external timeout function.
Related: OS#4074
Change-Id: Ib961b5a44911b99b0487641533301749c0286995
---
M TODO-RELEASE
M include/osmocom/isdn/lapd_core.h
M src/isdn/lapd_core.c
M src/isdn/libosmoisdn.map
4 files changed, 135 insertions(+), 14 deletions(-)
Approvals:
laforge: Looks good to me, approved
Jenkins Builder: Verified
daniel: Looks good to me, but someone else must approve
diff --git a/TODO-RELEASE b/TODO-RELEASE
index b67161d..1d56d45 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -9,3 +9,4 @@
#library whatdescription / commit summary line
core ADD osmo_sock_multiaddr_{add,del}_local_addr()
core ADD gsmtap_inst_fd2() core, DEPRECATE gsmtap_inst_fd()
+isdn ABI change add states and flags for external T200
handling
diff --git a/include/osmocom/isdn/lapd_core.h b/include/osmocom/isdn/lapd_core.h
index e4e8b46..776d4f4 100644
--- a/include/osmocom/isdn/lapd_core.h
+++ b/include/osmocom/isdn/lapd_core.h
@@ -84,6 +84,16 @@
LAPD_STATE_TIMER_RECOV,
};
+/*! lapd_flags */
+#define LAPD_F_RTS 0x0001
+
+/*! LAPD T200 state in RTS mode */
+enum lapd_t200_rts {
+ LAPD_T200_RTS_OFF = 0,
+ LAPD_T200_RTS_PENDING,
+ LAPD_T200_RTS_RUNNING,
+};
+
/*! LAPD message format (I / S / U) */
enum lapd_format {
LAPD_FORM_UKN = 0,
@@ -133,6 +143,7 @@
struct lapd_cr_ent rem2loc;
} cr;
enum lapd_mode mode; /*!< current mode of link */
+ unsigned int lapd_flags; /*!< \ref lapd_flags to change processing */
int use_sabme; /*!< use SABME instead of SABM */
int reestablish; /*!< enable reestablish support */
int n200, n200_est_rel; /*!< number of retranmissions */
@@ -149,6 +160,7 @@
uint8_t peer_busy; /*!< receiver busy on remote side */
int t200_sec, t200_usec; /*!< retry timer (default 1 sec) */
int t203_sec, t203_usec; /*!< retry timer (default 10 secs) */
+ enum lapd_t200_rts t200_rts; /*!< state of T200 in RTS mode */
struct osmo_timer_list t200; /*!< T200 timer */
struct osmo_timer_list t203; /*!< T203 timer */
uint8_t retrans_ctr; /*!< re-transmission counter */
@@ -169,8 +181,11 @@
void lapd_dl_set_name(struct lapd_datalink *dl, const char *name);
void lapd_dl_exit(struct lapd_datalink *dl);
void lapd_dl_reset(struct lapd_datalink *dl);
+int lapd_dl_set_flags(struct lapd_datalink *dl, unsigned int flags);
int lapd_set_mode(struct lapd_datalink *dl, enum lapd_mode mode);
int lapd_ph_data_ind(struct msgb *msg, struct lapd_msg_ctx *lctx);
+int lapd_ph_rts_ind(struct lapd_msg_ctx *lctx);
int lapd_recv_dlsap(struct osmo_dlsap_prim *dp, struct lapd_msg_ctx *lctx);
+int lapd_t200_timeout(struct lapd_datalink *dl);
/*! @} */
diff --git a/src/isdn/lapd_core.c b/src/isdn/lapd_core.c
index 57a0225..34748a4 100644
--- a/src/isdn/lapd_core.c
+++ b/src/isdn/lapd_core.c
@@ -204,11 +204,35 @@
static void lapd_start_t200(struct lapd_datalink *dl)
{
- if (osmo_timer_pending(>t200))
- return;
- LOGDL(dl, LOGL_INFO, "start T200 (timeout=%d.%06ds)\n",
- dl->t200_sec, dl->t200_usec);
- osmo_timer_schedule(>t200, dl->t200_sec, dl->t200_usec);
+ if ((dl->lapd_flags & LAPD_F_RTS)) {
+ if (dl->t200_rts != LAPD_T200_RTS_OFF)
+ return;
+ LOGDL(dl, LOGL_INFO, "Start T200. (pending until triggered by
RTS)\n");
+ dl->t200_rts = LAPD_T200_RTS_PENDING;
+ } else {
+ if (osmo_timer_pending(>t200))
+ return;
+ LOGDL(dl, LOGL_INFO, "Start T200 (timeout=%d.%06ds).\n",
dl->t200_sec, dl->t200_usec);
+ osmo_timer_schedule(>t200, dl->t200_sec, dl->t200_usec);
+ }
+}
+
+/*! Handle timeout condition of T200 in RTS mode.
+ * The caller (LAPDm code) implements the T200 timer and must detect timeout
condition.
+ * The function gets called by LAPDm code when it detects a timeout of T200.
+ * \param[in] dl caller-allocated datalink structure */
+int lapd_t200_timeout(struct lapd_datalink *dl)
+{
+ OSMO_ASSERT((dl->lapd_flags & LAPD_F_RTS));
+
+ if (dl->t200_rts != LAPD_T200_RTS_RUNNING)
+ return -EINVAL;
+
+ dl->t200_rts = LAPD_T200_RTS_OFF;
+
+ lapd_t200_cb(dl);
+
+ return 0;
}
static void lapd_start_t203(struct lapd_datalink *dl)
@@ -221,10 +245,24 @@
static void
[M] Change in libosmocore[master]: LAPD: Add support for RTS based polling and T200
Attention is currently required from: jolly, neels. 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 9: Code-Review+2 -- 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: 9 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Attention: jolly Gerrit-Attention: neels Gerrit-Comment-Date: Mon, 27 Nov 2023 16:25:49 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in libosmocore[master]: LAPD: Add support for RTS based polling and T200
Attention is currently required from: jolly, neels.
daniel 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 9: Code-Review+1
(1 comment)
File src/isdn/lapd_core.c:
https://gerrit.osmocom.org/c/libosmocore/+/34985/comment/2c4b40bd_c3dea4e3
PS9, Line 1939: if ((dl->lapd_flags & LAPD_F_RTS) &&
!llist_empty(>tx_queue)) {
> This function is called not only from lapd_ph_rts_ind, but from various
> locations of the LAPD proces […]
Thanks for the explanation
--
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: 9
Gerrit-Owner: jolly
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel
Gerrit-Reviewer: laforge
Gerrit-Reviewer: neels
Gerrit-Attention: jolly
Gerrit-Attention: neels
Gerrit-Comment-Date: Mon, 27 Nov 2023 12:52:05 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: jolly
Comment-In-Reply-To: daniel
Gerrit-MessageType: comment
[M] Change in libosmocore[master]: LAPD: Add support for RTS based polling and T200
Attention is currently required from: daniel, neels.
jolly 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 9:
(1 comment)
File src/isdn/lapd_core.c:
https://gerrit.osmocom.org/c/libosmocore/+/34985/comment/5214f616_13aa8ff8
PS9, Line 1939: if ((dl->lapd_flags & LAPD_F_RTS) &&
!llist_empty(>tx_queue)) {
> This looks a bit weird. […]
This function is called not only from lapd_ph_rts_ind, but from various
locations of the LAPD process. The !llist_empty() checks if there is already
some message in the queue. (RTS mode can only handle one message at a time.)
--
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: 9
Gerrit-Owner: jolly
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge
Gerrit-Reviewer: neels
Gerrit-CC: daniel
Gerrit-Attention: neels
Gerrit-Attention: daniel
Gerrit-Comment-Date: Mon, 27 Nov 2023 09:35:19 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: daniel
Gerrit-MessageType: comment
[M] Change in libosmocore[master]: LAPD: Add support for RTS based polling and T200
Attention is currently required from: jolly, neels.
daniel 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 9:
(2 comments)
File src/isdn/lapd_core.c:
https://gerrit.osmocom.org/c/libosmocore/+/34985/comment/956b985c_79614f74
PS9, Line 1939: if ((dl->lapd_flags & LAPD_F_RTS) &&
!llist_empty(>tx_queue)) {
This looks a bit weird. You return here if !llist_empty() (and RTS), but in
lapd_ph_rts_ind() you call this function only if llist_empty().
I might be missing something, I'm not really familiar with this code.
https://gerrit.osmocom.org/c/libosmocore/+/34985/comment/a2231539_1c716b12
PS9, Line 1945: next_frame:
Irrelevant for this patch, but this label really should be unindented.
--
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: 9
Gerrit-Owner: jolly
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge
Gerrit-Reviewer: neels
Gerrit-CC: daniel
Gerrit-Attention: jolly
Gerrit-Attention: neels
Gerrit-Comment-Date: Fri, 24 Nov 2023 10:09:32 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
[M] Change in libosmocore[master]: LAPD: Add support for RTS based polling and T200
Attention is currently required from: jolly, neels. 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 9: Code-Review+1 -- 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: 9 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Attention: jolly Gerrit-Attention: neels Gerrit-Comment-Date: Fri, 24 Nov 2023 08:36:52 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in libosmocore[master]: LAPD: Add support for RTS based polling and T200
Attention is currently required from: laforge, neels. Hello Jenkins Builder, laforge, neels, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/34985?usp=email to look at the new patch set (#9). The following approvals got outdated and were removed: Code-Review+1 by laforge, Code-Review+1 by neels, Verified+1 by Jenkins Builder Change subject: LAPD: Add support for RTS based polling and T200 .. LAPD: Add support for RTS based polling and T200 The T200 timer is started when the current frame is polled at PH-READY-TO-SEND event. A flag is used to enable this feature. The user of LAPD core must track frame numbers to check the timeout condition. Then it must call the external timeout function. Related: OS#4074 Change-Id: Ib961b5a44911b99b0487641533301749c0286995 --- M TODO-RELEASE M include/osmocom/isdn/lapd_core.h M src/isdn/lapd_core.c M src/isdn/libosmoisdn.map 4 files changed, 135 insertions(+), 14 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/85/34985/9 -- 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: 9 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-MessageType: newpatchset
[M] Change in libosmocore[master]: LAPD: Add support for RTS based polling and T200
Attention is currently required from: neels. jolly 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 8: (4 comments) File include/osmocom/isdn/lapd_core.h: https://gerrit.osmocom.org/c/libosmocore/+/34985/comment/215d440d_f2511945 PS8, Line 88: #define LAPD_F_RTS0x0001 > (an enum would be nicer IMHO) There is another flag in a later patch. https://gerrit.osmocom.org/c/libosmocore/+/35015 File src/isdn/lapd_core.c: https://gerrit.osmocom.org/c/libosmocore/+/34985/comment/437e7a58_4f75489a PS8, Line 220: Function to > (just drop the words "Function to", rationale: it is obvious that it is a > function) Done https://gerrit.osmocom.org/c/libosmocore/+/34985/comment/4aeccd7b_30b7fa62 PS8, Line 400: \ref > '\ref' is not valid doxygen here, see https://osmocom. […] Done https://gerrit.osmocom.org/c/libosmocore/+/34985/comment/033fbb3f_b994b69e PS8, Line 1785: Function call when a LAPD frame is ready to sent. > this is hard to understand, could you make this an "imperative form" > description of what the functio […] Done -- 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: 8 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Attention: neels Gerrit-Comment-Date: Thu, 23 Nov 2023 10:46:53 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Gerrit-MessageType: comment
[M] Change in libosmocore[master]: LAPD: Add support for RTS based polling and T200
Attention is currently required from: jolly. neels 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 8: Code-Review+1 (4 comments) File include/osmocom/isdn/lapd_core.h: https://gerrit.osmocom.org/c/libosmocore/+/34985/comment/096a5c7e_e9c0ff67 PS8, Line 88: #define LAPD_F_RTS0x0001 (an enum would be nicer IMHO) File src/isdn/lapd_core.c: https://gerrit.osmocom.org/c/libosmocore/+/34985/comment/a08beb95_979c4df4 PS8, Line 220: Function to (just drop the words "Function to", rationale: it is obvious that it is a function) https://gerrit.osmocom.org/c/libosmocore/+/34985/comment/37864d12_f292a897 PS8, Line 400: \ref '\ref' is not valid doxygen here, see https://osmocom.org/projects/cellular-infrastructure/wiki/Guidelines_for_API_documentation#Parameters https://gerrit.osmocom.org/c/libosmocore/+/34985/comment/8dbf6784_2d0d40c0 PS8, Line 1785: Function call when a LAPD frame is ready to sent. this is hard to understand, could you make this an "imperative form" description of what the function does, like Send the next LAPD frame. (if that is correct). That is the shortest and clearest form of doc. thanks -- 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: 8 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Attention: jolly Gerrit-Comment-Date: Wed, 22 Nov 2023 22:17:38 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in libosmocore[master]: LAPD: Add support for RTS based polling and T200
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 7: Code-Review+1 -- 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: 7 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Attention: jolly Gerrit-Comment-Date: Fri, 17 Nov 2023 14:16:04 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in libosmocore[master]: LAPD: Add support for RTS based polling and T200
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 6: Code-Review+1 -- 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: 6 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Attention: jolly Gerrit-Comment-Date: Wed, 15 Nov 2023 21:27:22 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in libosmocore[master]: LAPD: Add support for RTS based polling and T200
Attention is currently required from: laforge. jolly 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 6: (3 comments) File include/osmocom/isdn/lapd_core.h: https://gerrit.osmocom.org/c/libosmocore/+/34985/comment/434a3ec9_3d1e59da PS5, Line 155: unsigned > you are modifying a public data structure, which is ABI breakage. […] Done File src/isdn/lapd_core.c: https://gerrit.osmocom.org/c/libosmocore/+/34985/comment/7e619430_778d63ce PS5, Line 220: / > handle how? This requires more ddocumentation. […] Done https://gerrit.osmocom.org/c/libosmocore/+/34985/comment/f67e0606_6343fc1c PS5, Line 399: dl->flags = flags; > I guess it would leads to undefined behaviour if somebody called > lapd_dl_set_flags to enable RTS mod […] done, also moved the timer state to an enum, so it is not mixed with the flags that define behavior of LAPD. -- 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: 6 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge Gerrit-Attention: laforge Gerrit-Comment-Date: Wed, 15 Nov 2023 13:58:22 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge Gerrit-MessageType: comment
[M] Change in libosmocore[master]: LAPD: Add support for RTS based polling and T200
Attention is currently required from: jolly. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/34985?usp=email to look at the new patch set (#6). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder Change subject: LAPD: Add support for RTS based polling and T200 .. LAPD: Add support for RTS based polling and T200 The T200 timer is started when the current frame is polled at PH-READY-TO-SEND event. A flag is used to enable this feature. The user of LAPD core must track frame numbers to check the timeout condition. Then it must call the external timeout function. Related: OS#4074 Change-Id: Ib961b5a44911b99b0487641533301749c0286995 --- M TODO-RELEASE M include/osmocom/isdn/lapd_core.h M src/isdn/lapd_core.c M src/isdn/libosmoisdn.map 4 files changed, 135 insertions(+), 14 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/85/34985/6 -- 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: 6 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge Gerrit-Attention: jolly Gerrit-MessageType: newpatchset
[M] Change in libosmocore[master]: LAPD: Add support for RTS based polling and T200
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 Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge Gerrit-Attention: jolly Gerrit-Comment-Date: Mon, 13 Nov 2023 18:24:06 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[M] Change in libosmocore[master]: LAPD: Add support for RTS based polling and T200
jolly 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 .. Set Ready For Review -- 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 Gerrit-Reviewer: Jenkins Builder Gerrit-Comment-Date: Mon, 13 Nov 2023 11:56:08 + Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
