diff mbox series

[ovs-dev,v3,3/3] Increase speed/decrease cost of detecting dead connections

Message ID 20200226115239.3192-3-anton.ivanov@cambridgegreys.com
State Changes Requested
Headers show
Series [ovs-dev,v3,1/3] Add file descriptor persistence where possible | expand

Commit Message

Anton Ivanov Feb. 26, 2020, 11:52 a.m. UTC
From: Anton Ivanov <anton.ivanov@cambridgegreys.com>

Make persistent dead connections immeditely return an
EPIPE to upper layers.

Signed-off-by: Anton Ivanov <anton.ivanov@cambridgegreys.com>
---
 lib/stream-fd.c  | 11 ++++++++++-
 lib/stream-ssl.c |  8 +++++++-
 2 files changed, 17 insertions(+), 2 deletions(-)

Comments

0-day Robot Feb. 26, 2020, 12:13 p.m. UTC | #1
Bleep bloop.  Greetings Anton Ivanov, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line lacks whitespace around operator
#30 FILE: lib/stream-fd.c:119:
        if (stream->hint->revents & (POLLHUP|POLLNVAL)) {

WARNING: Line has non-spaces leading whitespace
WARNING: Line has trailing whitespace
#42 FILE: lib/stream-fd.c:161:
    

WARNING: Line lacks whitespace around operator
#44 FILE: lib/stream-fd.c:163:
        if (stream->hint->revents & (POLLHUP|POLLNVAL)) {

WARNING: Line lacks whitespace around operator
#59 FILE: lib/stream-ssl.c:710:
        if (stream->hint->revents & (POLLHUP|POLLNVAL)) {

WARNING: Line lacks whitespace around operator
#70 FILE: lib/stream-ssl.c:761:
        if (stream->hint->revents & (POLLHUP|POLLNVAL)) {

Lines checked: 78, Warnings: 6, Errors: 0


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot
Anton Ivanov Feb. 26, 2020, 12:58 p.m. UTC | #2
This one breaks tests 1720 and 1721, I will revisit it later.

1 and 2 are OK.

A.

On 26/02/2020 11:52, anton.ivanov@cambridgegreys.com wrote:
> From: Anton Ivanov <anton.ivanov@cambridgegreys.com>
>
> Make persistent dead connections immeditely return an
> EPIPE to upper layers.
>
> Signed-off-by: Anton Ivanov <anton.ivanov@cambridgegreys.com>
> ---
>   lib/stream-fd.c  | 11 ++++++++++-
>   lib/stream-ssl.c |  8 +++++++-
>   2 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/lib/stream-fd.c b/lib/stream-fd.c
> index 791bb96f6..3ac01e2dd 100644
> --- a/lib/stream-fd.c
> +++ b/lib/stream-fd.c
> @@ -110,12 +110,17 @@ fd_recv(struct stream *stream, void *buffer, size_t n)
>       ssize_t retval;
>       int error;
>   
> +
>       if (stream->persist && stream->hint) {
>           /* poll-loop is providing us with hints for IO. If we got a HUP/NVAL we skip straight
>            * to the read which should return 0 if the HUP is a real one, if not we clear it
>            * for all other cases we belive what (e)poll has fed us.
>            */
> -        if ((!(stream->hint->revents & (POLLHUP|POLLNVAL))) && (!stream->rx_ready)) {
> +        if (stream->hint->revents & (POLLHUP|POLLNVAL)) {
> +                return -EPIPE;
> +        }
> +
> +        if (!stream->rx_ready) {
>               if (!(stream->hint->revents & POLLIN)) {
>                   return -EAGAIN;
>               } else {
> @@ -153,7 +158,11 @@ fd_send(struct stream *stream, const void *buffer, size_t n)
>       ssize_t retval;
>       int error;
>   
> +
>       if (stream->persist && stream->hint) {
> +        if (stream->hint->revents & (POLLHUP|POLLNVAL)) {
> +                return -EPIPE;
> +        }
>           /* poll-loop is providing us with hints for IO */
>           if (!stream->tx_ready) {
>               if (!(stream->hint->revents & POLLOUT)) {
> diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
> index 14abacf4a..b3ae46047 100644
> --- a/lib/stream-ssl.c
> +++ b/lib/stream-ssl.c
> @@ -707,7 +707,10 @@ ssl_recv(struct stream *stream, void *buffer, size_t n)
>            * to the read which should return 0 if the HUP is a real one, if not we clear it
>            * for all other cases we belive what (e)poll has fed us.
>            */
> -        if ((!(stream->hint->revents & (POLLHUP|POLLNVAL))) && (sslv->rx_want == SSL_READING)) {
> +        if (stream->hint->revents & (POLLHUP|POLLNVAL)) {
> +                return -EPIPE;
> +        }
> +        if (sslv->rx_want == SSL_READING) {
>               if (!(stream->hint->revents & POLLIN)) {
>                   return -EAGAIN;
>               } else {
> @@ -755,6 +758,9 @@ ssl_do_tx(struct stream *stream)
>       struct ssl_stream *sslv = ssl_stream_cast(stream);
>   
>        if (stream->persist && stream->hint) {
> +        if (stream->hint->revents & (POLLHUP|POLLNVAL)) {
> +                return -EPIPE;
> +        }
>           /* poll-loop is providing us with hints for IO */
>           if (sslv->tx_want == SSL_WRITING) {
>               if (!(stream->hint->revents & POLLOUT)) {
Ilya Maximets Feb. 26, 2020, 1:05 p.m. UTC | #3
On 2/26/20 1:58 PM, Anton Ivanov wrote:
> This one breaks tests 1720 and 1721, I will revisit it later.
> 
> 1 and 2 are OK.
> 
> A.

Could you, please, also run utilities/checkpatch.py script before sending
patches to the list.  To limit the noise from the 0-day robot and reduce
number of re-sending iterations.

Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/lib/stream-fd.c b/lib/stream-fd.c
index 791bb96f6..3ac01e2dd 100644
--- a/lib/stream-fd.c
+++ b/lib/stream-fd.c
@@ -110,12 +110,17 @@  fd_recv(struct stream *stream, void *buffer, size_t n)
     ssize_t retval;
     int error;
 
+
     if (stream->persist && stream->hint) {
         /* poll-loop is providing us with hints for IO. If we got a HUP/NVAL we skip straight
          * to the read which should return 0 if the HUP is a real one, if not we clear it
          * for all other cases we belive what (e)poll has fed us.
          */
-        if ((!(stream->hint->revents & (POLLHUP|POLLNVAL))) && (!stream->rx_ready)) {
+        if (stream->hint->revents & (POLLHUP|POLLNVAL)) {
+                return -EPIPE;
+        }
+
+        if (!stream->rx_ready) {
             if (!(stream->hint->revents & POLLIN)) {
                 return -EAGAIN;
             } else {
@@ -153,7 +158,11 @@  fd_send(struct stream *stream, const void *buffer, size_t n)
     ssize_t retval;
     int error;
 
+    
     if (stream->persist && stream->hint) {
+        if (stream->hint->revents & (POLLHUP|POLLNVAL)) {
+                return -EPIPE;
+        }
         /* poll-loop is providing us with hints for IO */
         if (!stream->tx_ready) {
             if (!(stream->hint->revents & POLLOUT)) {
diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
index 14abacf4a..b3ae46047 100644
--- a/lib/stream-ssl.c
+++ b/lib/stream-ssl.c
@@ -707,7 +707,10 @@  ssl_recv(struct stream *stream, void *buffer, size_t n)
          * to the read which should return 0 if the HUP is a real one, if not we clear it
          * for all other cases we belive what (e)poll has fed us.
          */
-        if ((!(stream->hint->revents & (POLLHUP|POLLNVAL))) && (sslv->rx_want == SSL_READING)) {
+        if (stream->hint->revents & (POLLHUP|POLLNVAL)) {
+                return -EPIPE;
+        }
+        if (sslv->rx_want == SSL_READING) {
             if (!(stream->hint->revents & POLLIN)) {
                 return -EAGAIN;
             } else {
@@ -755,6 +758,9 @@  ssl_do_tx(struct stream *stream)
     struct ssl_stream *sslv = ssl_stream_cast(stream);
 
      if (stream->persist && stream->hint) {
+        if (stream->hint->revents & (POLLHUP|POLLNVAL)) {
+                return -EPIPE;
+        }
         /* poll-loop is providing us with hints for IO */
         if (sslv->tx_want == SSL_WRITING) {
             if (!(stream->hint->revents & POLLOUT)) {