September 2022 - gerrit-log - lists.osmocom.org

Change in libosmo-netif[master]: stream: Set sctp_ppid and sctp_stream when sctp notifciation is received

by neels

Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29543 ) Change subject: stream: Set sctp_ppid and sctp_stream when sctp notifciation is received ...................................................................... Patch Set 1: Code-Review+1 (1 comment) Patchset: PS1: would be good to read context in the commit log, whether it fixes a bug / motivation for the patch etc -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29543 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I99f6098d8d9fc1c06bc28373bf7ee76f15d5f525 Gerrit-Change-Number: 29543 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Fri, 30 Sep 2022 13:59:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 8 months

1
0
0 0

Change in libosmo-netif[master]: stream: Remove unneeded break statement

by neels

Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29544 ) Change subject: stream: Remove unneeded break statement ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29544 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: Iaa36a3661fc3d049f656342ee6dc3aafd45498bb Gerrit-Change-Number: 29544 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Fri, 30 Sep 2022 13:56:51 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 8 months

1
0
0 0

Change in libosmo-netif[master]: stream: Return 0 when receiving sctp notification SCTP_COMM_LOST

by neels

Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29545 ) Change subject: stream: Return 0 when receiving sctp notification SCTP_COMM_LOST ...................................................................... Patch Set 1: Code-Review+1 (2 comments) Commit Message: https://gerrit.osmocom.org/c/libosmo-netif/+/29545/comment/5ea3ada9_5e8c7cb0 PS1, Line 9: kernel probably the *Linux* kernel stack? https://gerrit.osmocom.org/c/libosmo-netif/+/29545/comment/eccf3b33_5269914c PS1, Line 23: death dead -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29545 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: If94d44f25b76a96a5ea402fec9fc14c4e6296ba3 Gerrit-Change-Number: 29545 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Fri, 30 Sep 2022 13:55:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 8 months

1
0
0 0

Change in osmo-hnbgw[master]: hnb_read_cb(): -EBADF must be returned if conn is freed to avoid use-...

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/29549 ) Change subject: hnb_read_cb(): -EBADF must be returned if conn is freed to avoid use-after-free ...................................................................... hnb_read_cb(): -EBADF must be returned if conn is freed to avoid use-after-free Otherwise the libosmo-netif stream API may continue accessing the conn after returning if the socket has the WRITE flag active in the same main loop iteration. Change-Id: I628c59a88d94d299f432f405b37fbe602381d47e --- M src/osmo-hnbgw/hnbgw.c 1 file changed, 4 insertions(+), 3 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/49/29549/1 diff --git a/src/osmo-hnbgw/hnbgw.c b/src/osmo-hnbgw/hnbgw.c index b824dd3..38d33eb 100644 --- a/src/osmo-hnbgw/hnbgw.c +++ b/src/osmo-hnbgw/hnbgw.c @@ -264,7 +264,7 @@ "sctp_recvmsg(%s) = SCTP_COMM_LOST, closing conn\n", osmo_sock_get_name2(ofd->fd)); osmo_stream_srv_destroy(conn); - rc = -1; + rc = -EBADF; break; case SCTP_RESTART: LOGHNB(hnb, DMAIN, LOGL_NOTICE, "HNB SCTP conn RESTARTed, marking as HNBAP-unregistered\n"); @@ -277,7 +277,7 @@ "sctp_recvmsg(%s) = SCTP_SHUTDOWN_EVENT, closing conn\n", osmo_sock_get_name2(ofd->fd)); osmo_stream_srv_destroy(conn); - rc = -1; + rc = -EBADF; break; } goto out; @@ -291,12 +291,13 @@ LOGHNB(hnb, DMAIN, LOGL_ERROR, "Error during sctp_recvmsg(%s)\n", osmo_sock_get_name2(ofd->fd)); osmo_stream_srv_destroy(conn); + rc = -EBADF; goto out; } else if (rc == 0) { LOGHNB(hnb, DMAIN, LOGL_NOTICE, "Connection closed sctp_recvmsg(%s) = 0\n", osmo_sock_get_name2(ofd->fd)); osmo_stream_srv_destroy(conn); - rc = -1; + rc = -EBADF; goto out; } else { msgb_put(msg, rc); -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/29549 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I628c59a88d94d299f432f405b37fbe602381d47e Gerrit-Change-Number: 29549 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 8 months

1
0
0 0

Change in osmo-hnbgw[master]: hnb_read_cb: Store ofd in variable

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/29548 ) Change subject: hnb_read_cb: Store ofd in variable ...................................................................... hnb_read_cb: Store ofd in variable Change-Id: Ic7058b5a05b0d34b80617006d4e929a523212221 --- M src/osmo-hnbgw/hnbgw.c 1 file changed, 5 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/48/29548/1 diff --git a/src/osmo-hnbgw/hnbgw.c b/src/osmo-hnbgw/hnbgw.c index dd0999c..b824dd3 100644 --- a/src/osmo-hnbgw/hnbgw.c +++ b/src/osmo-hnbgw/hnbgw.c @@ -239,6 +239,7 @@ static int hnb_read_cb(struct osmo_stream_srv *conn) { struct hnb_context *hnb = osmo_stream_srv_get_data(conn); + struct osmo_fd *ofd = osmo_stream_srv_get_ofd(conn); struct msgb *msg = msgb_alloc(IUH_MSGB_SIZE, "Iuh rx"); int rc; @@ -261,7 +262,7 @@ case SCTP_COMM_LOST: LOGHNB(hnb, DMAIN, LOGL_NOTICE, "sctp_recvmsg(%s) = SCTP_COMM_LOST, closing conn\n", - osmo_sock_get_name2(osmo_stream_srv_get_ofd(conn)->fd)); + osmo_sock_get_name2(ofd->fd)); osmo_stream_srv_destroy(conn); rc = -1; break; @@ -274,7 +275,7 @@ case SCTP_SHUTDOWN_EVENT: LOGHNB(hnb, DMAIN, LOGL_NOTICE, "sctp_recvmsg(%s) = SCTP_SHUTDOWN_EVENT, closing conn\n", - osmo_sock_get_name2(osmo_stream_srv_get_ofd(conn)->fd)); + osmo_sock_get_name2(ofd->fd)); osmo_stream_srv_destroy(conn); rc = -1; break; @@ -288,12 +289,12 @@ goto out; } else if (rc < 0) { LOGHNB(hnb, DMAIN, LOGL_ERROR, "Error during sctp_recvmsg(%s)\n", - osmo_sock_get_name2(osmo_stream_srv_get_ofd(conn)->fd)); + osmo_sock_get_name2(ofd->fd)); osmo_stream_srv_destroy(conn); goto out; } else if (rc == 0) { LOGHNB(hnb, DMAIN, LOGL_NOTICE, "Connection closed sctp_recvmsg(%s) = 0\n", - osmo_sock_get_name2(osmo_stream_srv_get_ofd(conn)->fd)); + osmo_sock_get_name2(ofd->fd)); osmo_stream_srv_destroy(conn); rc = -1; goto out; -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/29548 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: Ic7058b5a05b0d34b80617006d4e929a523212221 Gerrit-Change-Number: 29548 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 8 months

1
0
0 0

Change in osmo-hnbgw[master]: Close conn when receiving SCTP_ASSOC_CHANGE notification

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/29547 ) Change subject: Close conn when receiving SCTP_ASSOC_CHANGE notification ...................................................................... Close conn when receiving SCTP_ASSOC_CHANGE notification It was seen on a real pcap trace (sctp & gsmtap_log) that the kernel stack may decide to kill the connection (sending an ABORT) if it fails to transmit some data after a while: ABORT Cause code: "Protocol violation (0x000d)", Cause Information: "Association exceeded its max_retrans count". When this occurs, the kernel sends the MSG_NOTIFICATION,SCTP_ASSOC_CHANGE,SCTP_COMM_LOST notification when reading from the socket with sctp_recvmsg(). This basically signals that the socket conn is death, and subsequent writes to it will result in send() failures (and receive SCTP_SEND_FAILED notification upon follow up reads). It's important to notice that after those events, there's no other sort of different event like SHUTDOWN coming in, so that's the time at which we must tell the user to close the socket. Hence, let's signal the caller that the socket is death by returning 0, to comply with usual recv() API. Related: SYS#6113 Change-Id: If35efd404405f926a4a6cc45862eeadd1b04e08c --- M src/osmo-hnbgw/hnbgw.c 1 file changed, 7 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/47/29547/1 diff --git a/src/osmo-hnbgw/hnbgw.c b/src/osmo-hnbgw/hnbgw.c index 0c59c7d..dd0999c 100644 --- a/src/osmo-hnbgw/hnbgw.c +++ b/src/osmo-hnbgw/hnbgw.c @@ -258,6 +258,13 @@ switch (notif->sn_header.sn_type) { case SCTP_ASSOC_CHANGE: switch (notif->sn_assoc_change.sac_state) { + case SCTP_COMM_LOST: + LOGHNB(hnb, DMAIN, LOGL_NOTICE, + "sctp_recvmsg(%s) = SCTP_COMM_LOST, closing conn\n", + osmo_sock_get_name2(osmo_stream_srv_get_ofd(conn)->fd)); + osmo_stream_srv_destroy(conn); + rc = -1; + break; case SCTP_RESTART: LOGHNB(hnb, DMAIN, LOGL_NOTICE, "HNB SCTP conn RESTARTed, marking as HNBAP-unregistered\n"); hnb->hnb_registered = false; -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/29547 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: If35efd404405f926a4a6cc45862eeadd1b04e08c Gerrit-Change-Number: 29547 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 8 months

1
0
0 0

Change in osmo-hnbgw[master]: Fix handling of sctp SCTP_SHUTDOWN_EVENT notification

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/29546 ) Change subject: Fix handling of sctp SCTP_SHUTDOWN_EVENT notification ...................................................................... Fix handling of sctp SCTP_SHUTDOWN_EVENT notification SCTP_SHUTDOWN_EVENT is a first class event, and not a subtype of SCTP_ASSOC_CHANGE (such as SCTP_SHUTDOWN_COMP). Related: SYS#6113 Change-Id: I7fa648142c07f63c55091d2a15b9d7312bcd4cec --- M src/osmo-hnbgw/hnbgw.c 1 file changed, 7 insertions(+), 7 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/46/29546/1 diff --git a/src/osmo-hnbgw/hnbgw.c b/src/osmo-hnbgw/hnbgw.c index 6d75525..0c59c7d 100644 --- a/src/osmo-hnbgw/hnbgw.c +++ b/src/osmo-hnbgw/hnbgw.c @@ -262,15 +262,15 @@ LOGHNB(hnb, DMAIN, LOGL_NOTICE, "HNB SCTP conn RESTARTed, marking as HNBAP-unregistered\n"); hnb->hnb_registered = false; break; - case SCTP_SHUTDOWN_EVENT: - LOGHNB(hnb, DMAIN, LOGL_NOTICE, - "sctp_recvmsg(%s) = SCTP_SHUTDOWN_EVENT, closing conn\n", - osmo_sock_get_name2(osmo_stream_srv_get_ofd(conn)->fd)); - osmo_stream_srv_destroy(conn); - rc = -1; - break; } break; + case SCTP_SHUTDOWN_EVENT: + LOGHNB(hnb, DMAIN, LOGL_NOTICE, + "sctp_recvmsg(%s) = SCTP_SHUTDOWN_EVENT, closing conn\n", + osmo_sock_get_name2(osmo_stream_srv_get_ofd(conn)->fd)); + osmo_stream_srv_destroy(conn); + rc = -1; + break; } goto out; } else if (rc == -EAGAIN) { -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/29546 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I7fa648142c07f63c55091d2a15b9d7312bcd4cec Gerrit-Change-Number: 29546 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 8 months

1
0
0 0

Change in libosmo-netif[master]: stream: Return 0 when receiving sctp notification SCTP_COMM_LOST

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29545 ) Change subject: stream: Return 0 when receiving sctp notification SCTP_COMM_LOST ...................................................................... stream: Return 0 when receiving sctp notification SCTP_COMM_LOST It was seen on a real pcap trace (sctp & gsmtap_log) that the kernel stack may decide to kill the connection (sending an ABORT) if it fails to transmit some data after a while: ABORT Cause code: "Protocol violation (0x000d)", Cause Information: "Association exceeded its max_retrans count". When this occurs, the kernel sends the MSG_NOTIFICATION,SCTP_ASSOC_CHANGE,SCTP_COMM_LOST notification when reading from the socket with sctp_recvmsg(). This basically signals that the socket conn is death, and subsequent writes to it will result in send() failures (and receive SCTP_SEND_FAILED notification upon follow up reads). It's important to notice that after those events, there's no other sort of different event like SHUTDOWN coming in, so that's the time at which we must tell the user to close the socket. Hence, let's signal the caller that the socket is death by returning 0, to comply with usual recv() API. Related: SYS#6113 Change-Id: If94d44f25b76a96a5ea402fec9fc14c4e6296ba3 --- M src/stream.c 1 file changed, 2 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/45/29545/1 diff --git a/src/stream.c b/src/stream.c index 5e15142..38d24fe 100644 --- a/src/stream.c +++ b/src/stream.c @@ -1490,7 +1490,8 @@ break; case SCTP_COMM_LOST: LOGPC(DLINP, LOGL_DEBUG, " LOST\n"); - break; + /* Handle this like a regular disconnect */ + return 0; case SCTP_RESTART: LOGPC(DLINP, LOGL_DEBUG, " RESTART\n"); break; -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29545 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: If94d44f25b76a96a5ea402fec9fc14c4e6296ba3 Gerrit-Change-Number: 29545 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 8 months

1
0
0 0

Change in libosmo-netif[master]: stream: Remove unneeded break statement

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29544 ) Change subject: stream: Remove unneeded break statement ...................................................................... stream: Remove unneeded break statement Change-Id: Iaa36a3661fc3d049f656342ee6dc3aafd45498bb --- M src/stream.c 1 file changed, 0 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/44/29544/1 diff --git a/src/stream.c b/src/stream.c index 604955b..5e15142 100644 --- a/src/stream.c +++ b/src/stream.c @@ -1512,7 +1512,6 @@ LOGP(DLINP, LOGL_DEBUG, "===> SHUTDOWN EVT\n"); /* Handle this like a regular disconnect */ return 0; - break; } return -EAGAIN; } -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29544 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: Iaa36a3661fc3d049f656342ee6dc3aafd45498bb Gerrit-Change-Number: 29544 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 8 months

1
0
0 0

Change in libosmo-netif[master]: stream: Log rx of sctp notification SCTP_SEND_FAILED

by pespin

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-netif/+/29542 to look at the new patch set (#2). Change subject: stream: Log rx of sctp notification SCTP_SEND_FAILED ...................................................................... stream: Log rx of sctp notification SCTP_SEND_FAILED Change-Id: I16d3ed950d4da3b13cca25bcd8f29af0d880c23e --- M src/stream.c 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/42/29542/2 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29542 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I16d3ed950d4da3b13cca25bcd8f29af0d880c23e Gerrit-Change-Number: 29542 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newpatchset

2 years, 8 months

1
0
0 0

2025

2024

2023

2022

gerrit-log September 2022