Hello Eli,

Eli Britstein via dev <[email protected]> writes:

> A connection is established if we see packets from both directions.
> The cited commit [1] fixed the issue of sending twice in one direction,
> but still an issue if more than that.
> Fix it.
>

The patch LGTM.
Just a very minor nit: I guess "[1]" could be removed from the
description. "The cited commit" seems enough.

In any case,

Acked-by: Paolo Valerio <[email protected]>

> Fixes: a867c010ee91 ("conntrack: Fix conntrack new state")
> Signed-off-by: Eli Britstein <[email protected]>
> ---
>  lib/conntrack-other.c   | 7 ++++---
>  tests/system-traffic.at | 9 +++++++++
>  2 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/lib/conntrack-other.c b/lib/conntrack-other.c
> index d3b4601858..7f3e63c384 100644
> --- a/lib/conntrack-other.c
> +++ b/lib/conntrack-other.c
> @@ -48,18 +48,19 @@ other_conn_update(struct conntrack *ct, struct conn 
> *conn_,
>                    struct dp_packet *pkt OVS_UNUSED, bool reply, long long 
> now)
>  {
>      struct conn_other *conn = conn_other_cast(conn_);
> -    enum ct_update_res ret = CT_UPDATE_VALID;
>  
>      if (reply && conn->state != OTHERS_BIDIR) {
>          conn->state = OTHERS_BIDIR;
>      } else if (conn->state == OTHERS_FIRST) {
>          conn->state = OTHERS_MULTIPLE;
> -        ret = CT_UPDATE_VALID_NEW;
>      }
>  
>      conn_update_expiration(ct, &conn->up, other_timeouts[conn->state], now);
>  
> -    return ret;
> +    if (conn->state == OTHERS_BIDIR) {
> +        return CT_UPDATE_VALID;
> +    }
> +    return CT_UPDATE_VALID_NEW;
>  }
>  
>  static bool
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index 89107ab624..182a78847e 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -3078,6 +3078,15 @@ NXST_FLOW reply:
>   table=1, priority=100,ct_state=+est+trk,in_port=1 actions=output:2
>  ])
>  
> +dnl Send a 3rd UDP packet on port 1
> +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 
> packet=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000
>  actions=resubmit(,0)"])
> +
> +dnl There still should not be any packet that matches the established 
> ct_state.
> +AT_CHECK([ovs-ofctl dump-flows br0 "table=1 in_port=1,ct_state=+trk+est" | 
> ofctl_strip], [0], [dnl
> +NXST_FLOW reply:
> + table=1, priority=100,ct_state=+est+trk,in_port=1 actions=output:2
> +])
> +
>  OVS_TRAFFIC_VSWITCHD_STOP
>  AT_CLEANUP
>  
> -- 
> 2.26.2.1730.g385c171
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to