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.
>
Do you mean we should change like below; I think your suggestion make sense, if
no one has objection, I will send a new version
diff --git a/lib/conntrack-tcp.c b/lib/conntrack-tcp.c
index 18a2aa7c7..b827eab62 100644
--- a/lib/conntrack-tcp.c
+++ b/lib/conntrack-tcp.c
@@ -338,21 +338,6 @@ tcp_conn_update(struct conntrack *ct, struct conn *conn_,
src->state = dst->state = CT_DPIF_TCPS_TIME_WAIT;
}
- if (src->state >= CT_DPIF_TCPS_FIN_WAIT_2
- && dst->state >= CT_DPIF_TCPS_FIN_WAIT_2) {
- conn_update_expiration(ct, &conn->up, CT_TM_TCP_CLOSED, now);
- } else if (src->state >= CT_DPIF_TCPS_CLOSING
- && dst->state >= CT_DPIF_TCPS_CLOSING) {
- conn_update_expiration(ct, &conn->up, CT_TM_TCP_FIN_WAIT, now);
- } else if (src->state < CT_DPIF_TCPS_ESTABLISHED
- || dst->state < CT_DPIF_TCPS_ESTABLISHED) {
- conn_update_expiration(ct, &conn->up, CT_TM_TCP_OPENING, now);
- } else if (src->state >= CT_DPIF_TCPS_CLOSING
- || dst->state >= CT_DPIF_TCPS_CLOSING) {
- conn_update_expiration(ct, &conn->up, CT_TM_TCP_CLOSING, now);
- } else {
- conn_update_expiration(ct, &conn->up, CT_TM_TCP_ESTABLISHED, now);
- }
} else if ((dst->state < CT_DPIF_TCPS_SYN_SENT
|| dst->state >= CT_DPIF_TCPS_FIN_WAIT_2
|| src->state >= CT_DPIF_TCPS_FIN_WAIT_2)
@@ -412,6 +397,22 @@ tcp_conn_update(struct conntrack *ct, struct conn *conn_,
return CT_UPDATE_INVALID;
}
+ if (src->state >= CT_DPIF_TCPS_FIN_WAIT_2
+ && dst->state >= CT_DPIF_TCPS_FIN_WAIT_2) {
+ conn_update_expiration(ct, &conn->up, CT_TM_TCP_CLOSED, now);
+ } else if (src->state >= CT_DPIF_TCPS_CLOSING
+ && dst->state >= CT_DPIF_TCPS_CLOSING) {
+ conn_update_expiration(ct, &conn->up, CT_TM_TCP_FIN_WAIT, now);
+ } else if (src->state < CT_DPIF_TCPS_ESTABLISHED
+ || dst->state < CT_DPIF_TCPS_ESTABLISHED) {
+ conn_update_expiration(ct, &conn->up, CT_TM_TCP_OPENING, now);
+ } else if (src->state >= CT_DPIF_TCPS_CLOSING
+ || dst->state >= CT_DPIF_TCPS_CLOSING) {
+ conn_update_expiration(ct, &conn->up, CT_TM_TCP_CLOSING, now);
+ } else {
+ conn_update_expiration(ct, &conn->up, CT_TM_TCP_ESTABLISHED, now);
+ }
+
return CT_UPDATE_VALID;
}
-Li
> 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?
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
