Change in osmo-mgw[master]: Inactive connection cleanup (disabled by default)
osmith has submitted this change and it was merged. (
https://gerrit.osmocom.org/12730 )
Change subject: Inactive connection cleanup (disabled by default)
..
Inactive connection cleanup (disabled by default)
Add a watchdog timer to connections, and close these connections when
the watchdog timer expires. Kick the watchdog whenever RTP messages or
the relevant MGCP messages arrive. Add the currently remaining timeout
to "show mgcp stats" in the VTY.
This feature is disabled by default, as it is incompatible with LCLS
(connections in LCLS state appear to be inactive). Enable it with the
new "conn-timeout" VTY setting. In general, this feature can be used to
work around interoperability problems causing connections to stay open
forever, and slowly exhausting all available ports. This happened for
various reasons already.
MDCX is the only relevant MGCP message:
- CRCX creates the conn and timer
- DLCX deletes the conn and timer
- MDCX is the only remaining supported MGCP message that indicates a CI
- Can't easily generically parse a CI for all MGCP messages, parsing is
done in handle_modify_con().
Related: OS#3429
Change-Id: I18886052e090466f73829133c24f011806cc1fe0
---
M include/osmocom/mgcp/mgcp.h
M include/osmocom/mgcp/mgcp_internal.h
M src/libosmo-mgcp/mgcp_conn.c
M src/libosmo-mgcp/mgcp_network.c
M src/libosmo-mgcp/mgcp_protocol.c
M src/libosmo-mgcp/mgcp_vty.c
M tests/mgcp/mgcp_test.c
7 files changed, 61 insertions(+), 0 deletions(-)
Approvals:
Jenkins Builder: Verified
Harald Welte: Looks good to me, approved
diff --git a/include/osmocom/mgcp/mgcp.h b/include/osmocom/mgcp/mgcp.h
index 034a780..7597af8 100644
--- a/include/osmocom/mgcp/mgcp.h
+++ b/include/osmocom/mgcp/mgcp.h
@@ -274,6 +274,9 @@
uint16_t osmux_dummy;
/* domain name of the media gateway */
char domain[255+1];
+
+ /* time after which inactive connections (CIs) get closed */
+ int conn_timeout;
};
/* config management */
diff --git a/include/osmocom/mgcp/mgcp_internal.h
b/include/osmocom/mgcp/mgcp_internal.h
index f75ae8b..14c5f11 100644
--- a/include/osmocom/mgcp/mgcp_internal.h
+++ b/include/osmocom/mgcp/mgcp_internal.h
@@ -231,6 +231,9 @@
/*! human readable name (vty, logging) */
char name[256];
+ /*! activity tracker (for cleaning up inactive connections) */
+ struct osmo_timer_list watchdog;
+
/*! union with connection description */
union {
struct mgcp_conn_rtp rtp;
@@ -328,3 +331,4 @@
#define PTYPE_UNDEFINED (-1)
void mgcp_get_local_addr(char *addr, struct mgcp_conn_rtp *conn);
+void mgcp_conn_watchdog_kick(struct mgcp_conn *conn);
diff --git a/src/libosmo-mgcp/mgcp_conn.c b/src/libosmo-mgcp/mgcp_conn.c
index fce8a78..a8341d6 100644
--- a/src/libosmo-mgcp/mgcp_conn.c
+++ b/src/libosmo-mgcp/mgcp_conn.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include
const static struct rate_ctr_group_desc rate_ctr_group_desc = {
@@ -125,6 +126,23 @@
rate_ctr_group_free(conn_rtp->rate_ctr_group);
}
+void mgcp_conn_watchdog_cb(void *data)
+{
+ struct mgcp_conn *conn = data;
+ LOGP(DLMGCP, LOGL_ERROR, "endpoint:0x%x CI:%s connection timed out!\n",
ENDPOINT_NUMBER(conn->endp), conn->id);
+ mgcp_conn_free(conn->endp, conn->id);
+}
+
+void mgcp_conn_watchdog_kick(struct mgcp_conn *conn)
+{
+ int timeout = conn->endp->cfg->conn_timeout;
+ if (!timeout)
+ return;
+
+ LOGP(DLMGCP, LOGL_DEBUG, "endpoint:0x%x CI:%s watchdog kicked\n",
ENDPOINT_NUMBER(conn->endp), conn->id);
+ osmo_timer_schedule(>watchdog, timeout, 0);
+}
+
/*! allocate a new connection list entry.
* \param[in] ctx talloc context
* \param[in] endp associated endpoint
@@ -167,6 +185,9 @@
OSMO_ASSERT(false);
}
+ /* Initialize watchdog */
+ osmo_timer_setup(>watchdog, mgcp_conn_watchdog_cb, conn);
+ mgcp_conn_watchdog_kick(conn);
llist_add(>entry, >conns);
return conn;
@@ -274,6 +295,7 @@
OSMO_ASSERT(false);
}
+ osmo_timer_del(>watchdog);
llist_del(>entry);
talloc_free(conn);
}
diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c
index 7af8e71..2c86f8f 100644
--- a/src/libosmo-mgcp/mgcp_network.c
+++ b/src/libosmo-mgcp/mgcp_network.c
@@ -1246,6 +1246,8 @@
if (len < 0)
return -1;
+ mgcp_conn_watchdog_kick(conn_src->conn);
+
/* Check if the connection is in loopback mode, if yes, just send the
* incoming data back to the origin */
if (conn_src->conn->mode == MGCP_CONN_LOOPBACK) {
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index f141485..9f95ea4 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -1141,6 +1141,8 @@
return
Change in osmo-mgw[master]: Inactive connection cleanup (disabled by default)
Harald Welte has posted comments on this change. ( https://gerrit.osmocom.org/12730 ) Change subject: Inactive connection cleanup (disabled by default) .. Patch Set 7: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/12730 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I18886052e090466f73829133c24f011806cc1fe0 Gerrit-Change-Number: 12730 Gerrit-PatchSet: 7 Gerrit-Owner: osmith Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Reviewer: daniel Gerrit-Reviewer: osmith Gerrit-Comment-Date: Wed, 06 Feb 2019 12:35:25 + Gerrit-HasComments: No Gerrit-HasLabels: Yes
Change in osmo-mgw[master]: Inactive connection cleanup (disabled by default)
osmith has posted comments on this change. ( https://gerrit.osmocom.org/12730 ) Change subject: Inactive connection cleanup (disabled by default) .. Patch Set 7: (1 comment) > I think this patch is missing to export the currently remaining timeout as > part of the "show mgcp [stats]" command. Or did I miss something? Right, I did not think about displaying the remaining timeout anywhere. It is implemented now, e.g. with "conn-timeout 60": OsmoMGW> show mgcp stats Virtual trunk 0 with 64 endpoints: Virtual trunk 0 endpoint rtpbridge/01: CONN: (138c/rtp, id:0xC4E6CC99, ip:192.168.1.37, rtp:16008 rtcp:16009) Currently remaining timeout (seconds): 59.990917 Packets Sent: 108 (4955 bytes total) Packets Received: 108 (4955 bytes total) ... And it even helped uncover a bug, I had parsed the conn-timeout value as hex number instead of decimal number. It's all fixed now, can you guys take another look? https://gerrit.osmocom.org/#/c/12730/6/src/libosmo-mgcp/mgcp_vty.c File src/libosmo-mgcp/mgcp_vty.c: https://gerrit.osmocom.org/#/c/12730/6/src/libosmo-mgcp/mgcp_vty.c@1342 PS6, Line 1342: This must be 10! -- To view, visit https://gerrit.osmocom.org/12730 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I18886052e090466f73829133c24f011806cc1fe0 Gerrit-Change-Number: 12730 Gerrit-PatchSet: 7 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Reviewer: daniel Gerrit-Reviewer: osmith Gerrit-CC: Harald Welte Gerrit-Comment-Date: Wed, 06 Feb 2019 11:05:57 + Gerrit-HasComments: Yes Gerrit-HasLabels: No
Change in osmo-mgw[master]: Inactive connection cleanup (disabled by default)
Hello Vadim Yanitskiy, daniel, Max, Neels Hofmeyr, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/12730 to look at the new patch set (#7). Change subject: Inactive connection cleanup (disabled by default) .. Inactive connection cleanup (disabled by default) Add a watchdog timer to connections, and close these connections when the watchdog timer expires. Kick the watchdog whenever RTP messages or the relevant MGCP messages arrive. Add the currently remaining timeout to "show mgcp stats" in the VTY. This feature is disabled by default, as it is incompatible with LCLS (connections in LCLS state appear to be inactive). Enable it with the new "conn-timeout" VTY setting. In general, this feature can be used to work around interoperability problems causing connections to stay open forever, and slowly exhausting all available ports. This happened for various reasons already. MDCX is the only relevant MGCP message: - CRCX creates the conn and timer - DLCX deletes the conn and timer - MDCX is the only remaining supported MGCP message that indicates a CI - Can't easily generically parse a CI for all MGCP messages, parsing is done in handle_modify_con(). Related: OS#3429 Change-Id: I18886052e090466f73829133c24f011806cc1fe0 --- M include/osmocom/mgcp/mgcp.h M include/osmocom/mgcp/mgcp_internal.h M src/libosmo-mgcp/mgcp_conn.c M src/libosmo-mgcp/mgcp_network.c M src/libosmo-mgcp/mgcp_protocol.c M src/libosmo-mgcp/mgcp_vty.c M tests/mgcp/mgcp_test.c 7 files changed, 61 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/30/12730/7 -- To view, visit https://gerrit.osmocom.org/12730 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I18886052e090466f73829133c24f011806cc1fe0 Gerrit-Change-Number: 12730 Gerrit-PatchSet: 7 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Reviewer: daniel Gerrit-Reviewer: osmith Gerrit-CC: Harald Welte
Change in osmo-mgw[master]: Inactive connection cleanup (disabled by default)
Harald Welte has posted comments on this change. ( https://gerrit.osmocom.org/12730 ) Change subject: Inactive connection cleanup (disabled by default) .. Patch Set 6: I think this patch is missing to export the currently remaining timeout as part of the "show mgcp [stats]" command. Or did I miss something? -- To view, visit https://gerrit.osmocom.org/12730 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I18886052e090466f73829133c24f011806cc1fe0 Gerrit-Change-Number: 12730 Gerrit-PatchSet: 6 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Reviewer: daniel Gerrit-Reviewer: osmith Gerrit-CC: Harald Welte Gerrit-Comment-Date: Wed, 06 Feb 2019 08:14:36 + Gerrit-HasComments: No Gerrit-HasLabels: No
Change in osmo-mgw[master]: Inactive connection cleanup (disabled by default)
Vadim Yanitskiy has posted comments on this change. ( https://gerrit.osmocom.org/12730 ) Change subject: Inactive connection cleanup (disabled by default) .. Patch Set 6: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/12730 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I18886052e090466f73829133c24f011806cc1fe0 Gerrit-Change-Number: 12730 Gerrit-PatchSet: 6 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Reviewer: daniel Gerrit-Reviewer: osmith Gerrit-Comment-Date: Tue, 05 Feb 2019 17:10:07 + Gerrit-HasComments: No Gerrit-HasLabels: Yes
Change in osmo-mgw[master]: Inactive connection cleanup (disabled by default)
Max has posted comments on this change. ( https://gerrit.osmocom.org/12730 ) Change subject: Inactive connection cleanup (disabled by default) .. Patch Set 6: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/12730 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I18886052e090466f73829133c24f011806cc1fe0 Gerrit-Change-Number: 12730 Gerrit-PatchSet: 6 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Reviewer: daniel Gerrit-Reviewer: osmith Gerrit-Comment-Date: Tue, 05 Feb 2019 17:03:53 + Gerrit-HasComments: No Gerrit-HasLabels: Yes
Change in osmo-mgw[master]: Inactive connection cleanup (disabled by default)
osmith has posted comments on this change. ( https://gerrit.osmocom.org/12730 ) Change subject: Inactive connection cleanup (disabled by default) .. Patch Set 5: (1 comment) https://gerrit.osmocom.org/#/c/12730/5/src/libosmo-mgcp/mgcp_conn.c File src/libosmo-mgcp/mgcp_conn.c: https://gerrit.osmocom.org/#/c/12730/5/src/libosmo-mgcp/mgcp_conn.c@138 PS5, Line 138: long > In 'mgcp.h' you're using 'unsigned int', so why it is 'long' here? […] Good point, changed it to use "int" everywhere to match osmo_timer_schedule(). -- To view, visit https://gerrit.osmocom.org/12730 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I18886052e090466f73829133c24f011806cc1fe0 Gerrit-Change-Number: 12730 Gerrit-PatchSet: 5 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Reviewer: daniel Gerrit-Reviewer: osmith Gerrit-Comment-Date: Tue, 05 Feb 2019 16:51:44 + Gerrit-HasComments: Yes Gerrit-HasLabels: No
Change in osmo-mgw[master]: Inactive connection cleanup (disabled by default)
Hello Vadim Yanitskiy, daniel, Max, Neels Hofmeyr, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/12730 to look at the new patch set (#6). Change subject: Inactive connection cleanup (disabled by default) .. Inactive connection cleanup (disabled by default) Add a watchdog timer to connections, and close these connections when the watchdog timer expires. Kick the watchdog whenever RTP messages or the relevant MGCP messages arrive. This feature is disabled by default, as it is incompatible with LCLS (connections in LCLS state appear to be inactive). Enable it with the new "conn-timeout" VTY setting. In general, this feature can be used to work around interoperability problems causing connections to stay open forever, and slowly exhausting all available ports. This happened for various reasons already. MDCX is the only relevant MGCP message: - CRCX creates the conn and timer - DLCX deletes the conn and timer - MDCX is the only remaining supported MGCP message that indicates a CI - Can't easily generically parse a CI for all MGCP messages, parsing is done in handle_modify_con(). Related: OS#3429 Change-Id: I18886052e090466f73829133c24f011806cc1fe0 --- M include/osmocom/mgcp/mgcp.h M include/osmocom/mgcp/mgcp_internal.h M src/libosmo-mgcp/mgcp_conn.c M src/libosmo-mgcp/mgcp_network.c M src/libosmo-mgcp/mgcp_protocol.c M src/libosmo-mgcp/mgcp_vty.c M tests/mgcp/mgcp_test.c 7 files changed, 54 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/30/12730/6 -- To view, visit https://gerrit.osmocom.org/12730 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I18886052e090466f73829133c24f011806cc1fe0 Gerrit-Change-Number: 12730 Gerrit-PatchSet: 6 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Reviewer: daniel Gerrit-Reviewer: osmith
Change in osmo-mgw[master]: Inactive connection cleanup (disabled by default)
Vadim Yanitskiy has posted comments on this change. ( https://gerrit.osmocom.org/12730 ) Change subject: Inactive connection cleanup (disabled by default) .. Patch Set 5: Code-Review+1 (1 comment) https://gerrit.osmocom.org/#/c/12730/5/src/libosmo-mgcp/mgcp_conn.c File src/libosmo-mgcp/mgcp_conn.c: https://gerrit.osmocom.org/#/c/12730/5/src/libosmo-mgcp/mgcp_conn.c@138 PS5, Line 138: long In 'mgcp.h' you're using 'unsigned int', so why it is 'long' here? Also, osmo_timer_schedule() is using 'int'. -- To view, visit https://gerrit.osmocom.org/12730 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I18886052e090466f73829133c24f011806cc1fe0 Gerrit-Change-Number: 12730 Gerrit-PatchSet: 5 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Reviewer: daniel Gerrit-Reviewer: osmith Gerrit-Comment-Date: Tue, 05 Feb 2019 16:31:16 + Gerrit-HasComments: Yes Gerrit-HasLabels: Yes
Change in osmo-mgw[master]: Inactive connection cleanup (disabled by default)
Max has posted comments on this change. ( https://gerrit.osmocom.org/12730 ) Change subject: Inactive connection cleanup (disabled by default) .. Patch Set 5: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/12730 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I18886052e090466f73829133c24f011806cc1fe0 Gerrit-Change-Number: 12730 Gerrit-PatchSet: 5 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Reviewer: osmith Gerrit-Comment-Date: Thu, 31 Jan 2019 15:38:52 + Gerrit-HasComments: No Gerrit-HasLabels: Yes
Change in osmo-mgw[master]: Inactive connection cleanup (disabled by default)
osmith has posted comments on this change. ( https://gerrit.osmocom.org/12730 ) Change subject: Inactive connection cleanup (disabled by default) .. Patch Set 5: (1 comment) https://gerrit.osmocom.org/#/c/12730/5/tests/mgcp/mgcp_test.c File tests/mgcp/mgcp_test.c: https://gerrit.osmocom.org/#/c/12730/5/tests/mgcp/mgcp_test.c@981 PS5, Line 981: endp.cfg = > Why adding watchdog requires this change? And how was it working before? This is needed, because mgcp_conn_watchdog_kick() looks up the timeout value in: conn->endp->cfg->conn_timeout These tests used to have no cfg in endp, which means dereferencing a NULL pointer at this point. It was working before, because nothing called by this test had accessed endp->cfg. -- To view, visit https://gerrit.osmocom.org/12730 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I18886052e090466f73829133c24f011806cc1fe0 Gerrit-Change-Number: 12730 Gerrit-PatchSet: 5 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Reviewer: osmith Gerrit-Comment-Date: Thu, 31 Jan 2019 15:37:16 + Gerrit-HasComments: Yes Gerrit-HasLabels: No
Change in osmo-mgw[master]: Inactive connection cleanup (disabled by default)
Max has posted comments on this change. ( https://gerrit.osmocom.org/12730 ) Change subject: Inactive connection cleanup (disabled by default) .. Patch Set 5: (1 comment) Looks good except for odd test change which does not result in any changes to output. Could you clarify why it's necessary? https://gerrit.osmocom.org/#/c/12730/5/tests/mgcp/mgcp_test.c File tests/mgcp/mgcp_test.c: https://gerrit.osmocom.org/#/c/12730/5/tests/mgcp/mgcp_test.c@981 PS5, Line 981: endp.cfg = Why adding watchdog requires this change? And how was it working before? -- To view, visit https://gerrit.osmocom.org/12730 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I18886052e090466f73829133c24f011806cc1fe0 Gerrit-Change-Number: 12730 Gerrit-PatchSet: 5 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Reviewer: osmith Gerrit-Comment-Date: Thu, 31 Jan 2019 15:31:49 + Gerrit-HasComments: Yes Gerrit-HasLabels: No
Change in osmo-mgw[master]: Inactive connection cleanup (disabled by default)
osmith has posted comments on this change. ( https://gerrit.osmocom.org/12730 ) Change subject: Inactive connection cleanup (disabled by default) .. Patch Set 4: (1 comment) https://gerrit.osmocom.org/#/c/12730/4/src/libosmo-mgcp/mgcp_vty.c File src/libosmo-mgcp/mgcp_vty.c: https://gerrit.osmocom.org/#/c/12730/4/src/libosmo-mgcp/mgcp_vty.c@1336 PS4, Line 1336: "conn-timeout <0-65534>", > Here we allow timeout 0 to be configured by user. Fixed, now 0 is not allowed anymore. -- To view, visit https://gerrit.osmocom.org/12730 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I18886052e090466f73829133c24f011806cc1fe0 Gerrit-Change-Number: 12730 Gerrit-PatchSet: 4 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Reviewer: osmith Gerrit-Comment-Date: Thu, 31 Jan 2019 14:55:27 + Gerrit-HasComments: Yes Gerrit-HasLabels: No
Change in osmo-mgw[master]: Inactive connection cleanup (disabled by default)
Hello Vadim Yanitskiy, Max, Neels Hofmeyr, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/12730 to look at the new patch set (#5). Change subject: Inactive connection cleanup (disabled by default) .. Inactive connection cleanup (disabled by default) Add a watchdog timer to connections, and close these connections when the watchdog timer expires. Kick the watchdog whenever RTP messages or the relevant MGCP messages arrive. This feature is disabled by default, as it is incompatible with LCLS (connections in LCLS state appear to be inactive). Enable it with the new "conn-timeout" VTY setting. In general, this feature can be used to work around interoperability problems causing connections to stay open forever, and slowly exhausting all available ports. This happened for various reasons already. MDCX is the only relevant MGCP message: - CRCX creates the conn and timer - DLCX deletes the conn and timer - MDCX is the only remaining supported MGCP message that indicates a CI - Can't easily generically parse a CI for all MGCP messages, parsing is done in handle_modify_con(). Related: OS#3429 Change-Id: I18886052e090466f73829133c24f011806cc1fe0 --- M include/osmocom/mgcp/mgcp.h M include/osmocom/mgcp/mgcp_internal.h M src/libosmo-mgcp/mgcp_conn.c M src/libosmo-mgcp/mgcp_network.c M src/libosmo-mgcp/mgcp_protocol.c M src/libosmo-mgcp/mgcp_vty.c M tests/mgcp/mgcp_test.c 7 files changed, 54 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/30/12730/5 -- To view, visit https://gerrit.osmocom.org/12730 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I18886052e090466f73829133c24f011806cc1fe0 Gerrit-Change-Number: 12730 Gerrit-PatchSet: 5 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Reviewer: osmith
Change in osmo-mgw[master]: Inactive connection cleanup (disabled by default)
Max has posted comments on this change. ( https://gerrit.osmocom.org/12730 ) Change subject: Inactive connection cleanup (disabled by default) .. Patch Set 4: Code-Review-1 (2 comments) I think we should permit only non-zero timeouts to avoid ambiguity. https://gerrit.osmocom.org/#/c/12730/4/src/libosmo-mgcp/mgcp_vty.c File src/libosmo-mgcp/mgcp_vty.c: https://gerrit.osmocom.org/#/c/12730/4/src/libosmo-mgcp/mgcp_vty.c@158 PS4, Line 158: if (g_cfg->conn_timeout) Here we treat 0 as "no timeout configured" indicator. https://gerrit.osmocom.org/#/c/12730/4/src/libosmo-mgcp/mgcp_vty.c@1336 PS4, Line 1336: "conn-timeout <0-65534>", Here we allow timeout 0 to be configured by user. -- To view, visit https://gerrit.osmocom.org/12730 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I18886052e090466f73829133c24f011806cc1fe0 Gerrit-Change-Number: 12730 Gerrit-PatchSet: 4 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Reviewer: osmith Gerrit-Comment-Date: Thu, 31 Jan 2019 14:25:33 + Gerrit-HasComments: Yes Gerrit-HasLabels: Yes
Change in osmo-mgw[master]: Inactive connection cleanup (disabled by default)
Hello Vadim Yanitskiy, Max, Neels Hofmeyr, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/12730 to look at the new patch set (#4). Change subject: Inactive connection cleanup (disabled by default) .. Inactive connection cleanup (disabled by default) Add a watchdog timer to connections, and close these connections when the watchdog timer expires. Kick the watchdog whenever RTP messages or the relevant MGCP messages arrive. This feature is disabled by default, as it is incompatible with LCLS (connections in LCLS state appear to be inactive). Enable it with the new "conn-timeout" VTY setting. In general, this feature can be used to work around interoperability problems causing connections to stay open forever, and slowly exhausting all available ports. This happened for various reasons already. MDCX is the only relevant MGCP message: - CRCX creates the conn and timer - DLCX deletes the conn and timer - MDCX is the only remaining supported MGCP message that indicates a CI - Can't easily generically parse a CI for all MGCP messages, parsing is done in handle_modify_con(). Related: OS#3429 Change-Id: I18886052e090466f73829133c24f011806cc1fe0 --- M include/osmocom/mgcp/mgcp.h M include/osmocom/mgcp/mgcp_internal.h M src/libosmo-mgcp/mgcp_conn.c M src/libosmo-mgcp/mgcp_network.c M src/libosmo-mgcp/mgcp_protocol.c M src/libosmo-mgcp/mgcp_vty.c M tests/mgcp/mgcp_test.c 7 files changed, 54 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/30/12730/4 -- To view, visit https://gerrit.osmocom.org/12730 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I18886052e090466f73829133c24f011806cc1fe0 Gerrit-Change-Number: 12730 Gerrit-PatchSet: 4 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Reviewer: osmith
Change in osmo-mgw[master]: Inactive connection cleanup (disabled by default)
osmith has posted comments on this change. ( https://gerrit.osmocom.org/12730 ) Change subject: Inactive connection cleanup (disabled by default) .. Patch Set 1: (2 comments) Good point with LCLS! > I agree that there should be vty switch for that. Also I think it should be > off by default. Done. > Would be nice to clarify in commit message what prompted this feature in more > details besides ticket link. Done. https://gerrit.osmocom.org/#/c/12730/1//COMMIT_MSG Commit Message: https://gerrit.osmocom.org/#/c/12730/1//COMMIT_MSG@12 PS1, Line 12: arrive. > elaborate why it makes sense only for MDCX: […] Done https://gerrit.osmocom.org/#/c/12730/1/include/osmocom/mgcp/mgcp_internal.h File include/osmocom/mgcp/mgcp_internal.h: https://gerrit.osmocom.org/#/c/12730/1/include/osmocom/mgcp/mgcp_internal.h@41 PS1, Line 41: #define MGCP_CONN_TIMEOUT 180 > MGCP_CONN_DEFAULT_TIMEOUT and make it configurable in VTY I've removed the variable, because it is disabled by default now. -- To view, visit https://gerrit.osmocom.org/12730 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I18886052e090466f73829133c24f011806cc1fe0 Gerrit-Change-Number: 12730 Gerrit-PatchSet: 1 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Reviewer: osmith Gerrit-Comment-Date: Thu, 31 Jan 2019 11:28:32 + Gerrit-HasComments: Yes Gerrit-HasLabels: No
Change in osmo-mgw[master]: Inactive connection cleanup (disabled by default)
Hello Vadim Yanitskiy, Max, Neels Hofmeyr, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/12730 to look at the new patch set (#3). Change subject: Inactive connection cleanup (disabled by default) .. Inactive connection cleanup (disabled by default) Add a watchdog timer to connections, and close these connections when the watchdog timer expires. Kick the watchdog whenever RTP messages or the relevant MGCP messages arrive. This feature is disabled by default, as it is incompatible with LCLS (connections in LCLS state appear to be inactive). Enable it with the new "conn-timeout" VTY setting. In general, this feature can be used to work around interoperability problems causing connections to stay open forever, and slowly exhausting all available ports. This happened for various reasons already. MDCX is the only relevant MGCP message: - CRCX creates the conn and timer - DLCX deletes the conn and timer - MDCX is the only remaining supported MGCP message that indicates a CI - Can't easily generically parse a CI for all MGCP messages, parsing is done in handle_modify_con(). Related: OS#3429 Change-Id: I18886052e090466f73829133c24f011806cc1fe0 --- M include/osmocom/mgcp/mgcp.h M include/osmocom/mgcp/mgcp_internal.h M src/libosmo-mgcp/mgcp_conn.c M src/libosmo-mgcp/mgcp_network.c M src/libosmo-mgcp/mgcp_protocol.c M src/libosmo-mgcp/mgcp_vty.c M tests/mgcp/mgcp_test.c 7 files changed, 55 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/30/12730/3 -- To view, visit https://gerrit.osmocom.org/12730 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I18886052e090466f73829133c24f011806cc1fe0 Gerrit-Change-Number: 12730 Gerrit-PatchSet: 3 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy
Change in osmo-mgw[master]: Inactive connection cleanup (disabled by default)
Hello Vadim Yanitskiy, Max, Neels Hofmeyr, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/12730 to look at the new patch set (#2). Change subject: Inactive connection cleanup (disabled by default) .. Inactive connection cleanup (disabled by default) Add a watchdog timer to connections, and close these connections when the watchdog timer expires. Kick the watchdog whenever RTP messages or the relevant MGCP messages arrive. This feature is disabled by default, as it is incompatible with LCLS (connections in LCLS state appear to be inactive). Enable it with the new "conn-timeout" VTY setting. MDCX is the only relevant MGCP message: - CRCX creates the conn and timer - DLCX deletes the conn and timer - MDCX is the only remaining supported MGCP message that indicates a CI - Can't easily generically parse a CI for all MGCP messages, parsing is done in handle_modify_con(). Related: OS#3429 Change-Id: I18886052e090466f73829133c24f011806cc1fe0 --- M include/osmocom/mgcp/mgcp.h M include/osmocom/mgcp/mgcp_internal.h M src/libosmo-mgcp/mgcp_conn.c M src/libosmo-mgcp/mgcp_network.c M src/libosmo-mgcp/mgcp_protocol.c M src/libosmo-mgcp/mgcp_vty.c M tests/mgcp/mgcp_test.c 7 files changed, 55 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/30/12730/2 -- To view, visit https://gerrit.osmocom.org/12730 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I18886052e090466f73829133c24f011806cc1fe0 Gerrit-Change-Number: 12730 Gerrit-PatchSet: 2 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy
