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" ?