When a new conntrack zone is entered, the ct_state field is zeroed in order to avoid using state information from different zones.
One such scenario is when a packet is double NATed. Assuming two zones and 3 flows performing the following actions in order on the packet: 1. ct(zone=5,nat), recirc 2. ct(zone=1), recirc 3. ct(zone=1,nat) If at step #1 the packet matches an existing NAT entry, it will get translated and pkt->md.ct_state is set to CS_DST_NAT or CS_SRC_NAT. At step #2 the new tuple might match an existing connection and pkt->md.ct_zone is set to 1. If at step #3 the packet matches an existing NAT entry in zone 1, handle_nat() will be called to perform the translation but it will return early because the packet's zone matches the conntrack zone and the ct_state field still contains CS_DST_NAT or CS_SRC_NAT from the translations in zone 5. In order to reliably detect when a packet enters a new conntrack zone we also need to make sure that the pkt->md.ct_zone is properly initialized if pkt->md.ct_state is non-zero. This already happens for most cases. The only exception is when matched conntrack connection is of type CT_CONN_TYPE_UN_NAT and the master connection is missing. To cover this path we now call write_ct_md() in that case too. Remove setting the CS_TRACKED flag as in this case as it will be done by the new call to write_ct_md(). Also, the error path above seems hard to hit as it would've caused a crash due to dereferencing a NULL pointer when calling: 'ct_print_conn_info(conn, log_msg, VLL_INFO, true, true)'. This patch changes the call to log the 'rev_conn' info instead. CC: Darrell Ball <dlu...@gmail.com> Fixes: 967bb5c5cd90 ("conntrack: Add rcu support.") Signed-off-by: Dumitru Ceara <dce...@redhat.com> --- V2: - Address Ilya's comments: - revert changes to pkt_metadata_init(). - update ct_state in process_one() only if ct_state is already non-zero. - Make sure pkt->md.ct_zone is always initialized when pkt->md.ct_state is non-zero. - Fix NULL pointer dereference in process_one() if conn_type is CT_CONN_TYPE_UN_NAT and master conn is not found. --- lib/conntrack.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/conntrack.c b/lib/conntrack.c index ff5a894..0cbc8f6 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -1277,6 +1277,11 @@ process_one(struct conntrack *ct, struct dp_packet *pkt, const struct nat_action_info_t *nat_action_info, ovs_be16 tp_src, ovs_be16 tp_dst, const char *helper) { + /* Reset ct_state whenever entering a new zone. */ + if (pkt->md.ct_state && pkt->md.ct_zone != zone) { + pkt->md.ct_state = 0; + } + bool create_new_conn = false; conn_key_lookup(ct, &ctx->key, ctx->hash, now, &ctx->conn, &ctx->reply); struct conn *conn = ctx->conn; @@ -1300,9 +1305,10 @@ process_one(struct conntrack *ct, struct dp_packet *pkt, conn_key_lookup(ct, &ctx->key, hash, now, &conn, &ctx->reply); if (!conn) { - pkt->md.ct_state |= CS_TRACKED | CS_INVALID; + pkt->md.ct_state |= CS_INVALID; + write_ct_md(pkt, zone, NULL, NULL, NULL); char *log_msg = xasprintf("Missing master conn %p", rev_conn); - ct_print_conn_info(conn, log_msg, VLL_INFO, true, true); + ct_print_conn_info(rev_conn, log_msg, VLL_INFO, true, true); free(log_msg); return; } -- 1.8.3.1 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev