[ovs-dev] [PATCH][v2] conntrack: Fix conntrack tw expiration

[Aaron Conole]

Li RongQing <lirongqing at baidu.com> writes:

> a connection will enter timewait status when a reset packet
> reached after a fin is received/sent, But the expiration
> time is not updated, still is the previous expiration
> time. this maybe causes connection table overflow due to long
> expiration time
>
> Fixes: a489b16854b59 ("conntrack: New userspace connection tracker.")
> Signed-off-by: Li RongQing <lirongqing at baidu.com>
> Co-authored-by: Mao YingMing <maoyingming at baidu.com>
> Signed-off-by: Mao YingMing <maoyingming at baidu.com>
> ---
> resend with maoyingming signature
>
>  lib/conntrack-tcp.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/lib/conntrack-tcp.c b/lib/conntrack-tcp.c
> index 18a2aa7c7..f1595af7a 100644
> --- a/lib/conntrack-tcp.c
> +++ b/lib/conntrack-tcp.c
> @@ -406,6 +406,7 @@ tcp_conn_update(struct conntrack *ct, struct conn *conn_,
>  
>          if (tcp_flags & TCP_RST) {
>              src->state = dst->state = CT_DPIF_TCPS_TIME_WAIT;
> +            conn_update_expiration(ct, &conn->up, CT_TM_TCP_CLOSED, now);
>          }
>      } else {
>          COVERAGE_INC(conntrack_tcp_seq_chk_failed);

Does it make more sense to move the entire conn_update_expiration block
in the previous condition to a common place after the 3 cases here
(seq_chk, shotgun-syn, and in-window blocks)?

That should cover this case, and the fin assignment as well.  Then
again, there are only two assignments in this block.

Alternatively, I think the fin check earlier will need a similar
conn_update_expiration - do we have a case where FIN|ACK on learning an
already established connection could keep us with an older timeout?