Attention is currently required from: pespin, keith, dexter. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-abis/+/32374 ) Change subject: e1d: reconnect to osmo-e1d after connection loss ...................................................................... Patch Set 11: (5 comments) File src/input/e1d.c: https://gerrit.osmocom.org/c/libosmo-abis/+/32374/comment/adeb34c6_6888d863 PS11, Line 340: osmo_fsm the question to me is if we really want to call this on every "short read", or just in case of ret <= 0. If *some* amount of data was read, the socket is not dead yet. In osmo-e1d, the socket in E1_TS_MODE_RAW is a SOCK_STREAM socket, so in theory, depending on operating system scheduling, you could receive any number of bytes (up to D_TSX_ALLOC_SIZE), and it would be perfectly legal. https://gerrit.osmocom.org/c/libosmo-abis/+/32374/comment/fd09ffa0_a1e8bcdf PS11, Line 391: osmo_fsm likewise here, a SOCK_STREAM socket might in theory do a successful write of any size, if the socket buffer/queue is full. "<= 0" should be the condition for connection loss, not "< msg->len" https://gerrit.osmocom.org/c/libosmo-abis/+/32374/comment/412acc5a_48e8435c PS11, Line 414: osmo_fsm same as above https://gerrit.osmocom.org/c/libosmo-abis/+/32374/comment/943d6248_e266ac18 PS11, Line 451: osmo_fsm here I think the existing implementation is OK, as a HDLC socket is an E1_TS_MODE_HDLCFCS socket, which is SOCK_SEQPACKET and not SOCK_STREAM. https://gerrit.osmocom.org/c/libosmo-abis/+/32374/comment/39f1daca_b3ddea3d PS11, Line 476: osmo_fsm_inst_dispatch(g_e1d_fsm_inst, EV_CONN_LOST, line); I'm not sure about the exact semantics of SOCK_SEQPACKET here, but I would assume that a zero-length read (after select/poll has told us data is available) is a "socket closed" indication, so it should be "<= 0" ? -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/32374 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: Iaf4d42c2f009b1d7666e319fabdeb2598aa0b338 Gerrit-Change-Number: 32374 Gerrit-PatchSet: 11 Gerrit-Owner: dexter &lt;pmaier(a)sysmocom.de&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-CC: keith &lt;keith(a)rhizomatica.org&gt; Gerrit-CC: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-CC: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-CC: tnt &lt;tnt(a)246tNt.com&gt; Gerrit-Attention: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-Attention: keith &lt;keith(a)rhizomatica.org&gt; Gerrit-Attention: dexter &lt;pmaier(a)sysmocom.de&gt; Gerrit-Comment-Date: Tue, 09 May 2023 23:34:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment