[PATCH 3/3] Fix cli socket close due to io buffer full
The pointer rpos in the socket io module is only restored if the rx_hook callback returns 1. Previously the cli_rx hook always returned 0. Guarantee by design that we return 1 if we are at the end of the socket buffer. Signed-off-by: Kenth Eriksson <kenth.eriksson@infinera.com> --- sysdep/unix/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sysdep/unix/main.c b/sysdep/unix/main.c index cf5f4a3c..6a6d1d2b 100644 --- a/sysdep/unix/main.c +++ b/sysdep/unix/main.c @@ -422,7 +422,7 @@ static int cli_rx(sock *s, uint size UNUSED) { cli_kick(s->data); - return 0; + return s->rbuf+s->rbsize==s->rpos? 1 : 0; } static void -- 2.21.0
On Tue, Jun 11, 2019 at 12:09:03PM +0200, Kenth Eriksson wrote:
The pointer rpos in the socket io module is only restored if the rx_hook callback returns 1. Previously the cli_rx hook always returned 0. Guarantee by design that we return 1 if we are at the end of the socket buffer.
I don't think this is correct, as that would reset buffer before its content is processed by cli_event(). Also, the issue is already fixed by patch 2/3.
Signed-off-by: Kenth Eriksson <kenth.eriksson@infinera.com> --- sysdep/unix/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sysdep/unix/main.c b/sysdep/unix/main.c index cf5f4a3c..6a6d1d2b 100644 --- a/sysdep/unix/main.c +++ b/sysdep/unix/main.c @@ -422,7 +422,7 @@ static int cli_rx(sock *s, uint size UNUSED) { cli_kick(s->data); - return 0; + return s->rbuf+s->rbsize==s->rpos? 1 : 0; }
static void -- 2.21.0
-- Elen sila lumenn' omentielvo Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
On Tue, 2019-06-11 at 17:20 +0200, Ondrej Zajicek wrote:
CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
On Tue, Jun 11, 2019 at 12:09:03PM +0200, Kenth Eriksson wrote:
The pointer rpos in the socket io module is only restored if the rx_hook callback returns 1. Previously the cli_rx hook always returned 0. Guarantee by design that we return 1 if we are at the end of the socket buffer.
I don't think this is correct, as that would reset buffer before its content is processed by cli_event().
We can remove patch 3/3, but only if we have patch 1/3, else the socket may be closed by the event loop reading 0 bytes. The socket read (sk_read) happens in an inner while loop and the event list (ev_run_list) in the outer while loop.
Also, the issue is already fixed by patch 2/3.
Well, patch 2/3 fixes the bug, but due to the asynchronous design where io may happen before events I see this as robustness improvement. But again, if we include patch 1/3 we can skip patch 3/3.
Signed-off-by: Kenth Eriksson <kenth.eriksson@infinera.com> --- sysdep/unix/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sysdep/unix/main.c b/sysdep/unix/main.c index cf5f4a3c..6a6d1d2b 100644 --- a/sysdep/unix/main.c +++ b/sysdep/unix/main.c @@ -422,7 +422,7 @@ static int cli_rx(sock *s, uint size UNUSED) { cli_kick(s->data); - return 0; + return s->rbuf+s->rbsize==s->rpos? 1 : 0; }
static void -- 2.21.0
-- Elen sila lumenn' omentielvo
Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
participants (3)
-
Kenth Eriksson -
Kenth Eriksson
-
Ondrej Zajicek