[ovs-dev] conntrack: Fix TCP conntrack state
diff mbox series

Message ID 1581116106-24519-1-git-send-email-yihung.wei@gmail.com
State New
Headers show
Series
  • [ovs-dev] conntrack: Fix TCP conntrack state
Related show

Commit Message

Yi-Hung Wei Feb. 7, 2020, 10:55 p.m. UTC
If a TCP connection is in SYN_SENT state, receiving another SYN packet
would just renew the timeout of that conntrack entry rather than create
a new one.  Thus, tcp_conn_update() should return CT_UPDATE_VALID_NEW.

This also fixes regressions of a couple of  OVN system tests.

Fixes: a867c010ee91 ("conntrack: Fix conntrack new state")
Reported-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
---
Please backport to branch 2.13.

---
 lib/conntrack-tcp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Dumitru Ceara Feb. 11, 2020, 3:07 p.m. UTC | #1
On 2/7/20 11:55 PM, Yi-Hung Wei wrote:
> If a TCP connection is in SYN_SENT state, receiving another SYN packet
> would just renew the timeout of that conntrack entry rather than create
> a new one.  Thus, tcp_conn_update() should return CT_UPDATE_VALID_NEW.
> 
> This also fixes regressions of a couple of  OVN system tests.
> 
> Fixes: a867c010ee91 ("conntrack: Fix conntrack new state")
> Reported-by: Dumitru Ceara <dceara@redhat.com>
> Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>

Hi Yi-Hung,

The changes look good to me but I'll let userspace conntrack reviewers
formally ack the patch.

I did try it out and it works fine.

Regards,
Dumitru

Tested-by: Dumitru Ceara <dceara@redhat.com>

> ---
> Please backport to branch 2.13.
> 
> ---
>  lib/conntrack-tcp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/conntrack-tcp.c b/lib/conntrack-tcp.c
> index 416cb769d22f..47261c7551d1 100644
> --- a/lib/conntrack-tcp.c
> +++ b/lib/conntrack-tcp.c
> @@ -189,7 +189,7 @@ tcp_conn_update(struct conntrack *ct, struct conn *conn_,
>          } else if (src->state <= CT_DPIF_TCPS_SYN_SENT) {
>              src->state = CT_DPIF_TCPS_SYN_SENT;
>              conn_update_expiration(ct, &conn->up, CT_TM_TCP_FIRST_PACKET, now);
> -            return CT_UPDATE_NEW;
> +            return CT_UPDATE_VALID_NEW;
>          }
>      }
>  
>
William Tu Feb. 18, 2020, 11:03 p.m. UTC | #2
On Tue, Feb 11, 2020 at 04:07:53PM +0100, Dumitru Ceara wrote:
> On 2/7/20 11:55 PM, Yi-Hung Wei wrote:
> > If a TCP connection is in SYN_SENT state, receiving another SYN packet
> > would just renew the timeout of that conntrack entry rather than create
> > a new one.  Thus, tcp_conn_update() should return CT_UPDATE_VALID_NEW.
> > 
> > This also fixes regressions of a couple of  OVN system tests.
> > 
> > Fixes: a867c010ee91 ("conntrack: Fix conntrack new state")
> > Reported-by: Dumitru Ceara <dceara@redhat.com>
> > Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
> 
> Hi Yi-Hung,
> 
> The changes look good to me but I'll let userspace conntrack reviewers
> formally ack the patch.
> 
> I did try it out and it works fine.
> 
> Regards,
> Dumitru
> 
> Tested-by: Dumitru Ceara <dceara@redhat.com>
Thanks
I applied to master and branch 2.13.

William

Patch
diff mbox series

diff --git a/lib/conntrack-tcp.c b/lib/conntrack-tcp.c
index 416cb769d22f..47261c7551d1 100644
--- a/lib/conntrack-tcp.c
+++ b/lib/conntrack-tcp.c
@@ -189,7 +189,7 @@  tcp_conn_update(struct conntrack *ct, struct conn *conn_,
         } else if (src->state <= CT_DPIF_TCPS_SYN_SENT) {
             src->state = CT_DPIF_TCPS_SYN_SENT;
             conn_update_expiration(ct, &conn->up, CT_TM_TCP_FIRST_PACKET, now);
-            return CT_UPDATE_NEW;
+            return CT_UPDATE_VALID_NEW;
         }
     }