On 1/30/20 3:16 PM, Dumitru Ceara wrote:
> 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 zero out the pkt->md.ct_zone field when initializing
> metadata in pkt_metadata_init().
>
> CC: Darrell Ball <dlu...@gmail.com>
> Signed-off-by: Dumitru Ceara <dce...@redhat.com>
> ---
> lib/conntrack.c | 5 +++++
> lib/packets.h | 5 +++++
> 2 files changed, 10 insertions(+)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index ff5a894..e4d934a 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_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;
> diff --git a/lib/packets.h b/lib/packets.h
> index 5d7f82c..fae64bb 100644
> --- a/lib/packets.h
> +++ b/lib/packets.h
> @@ -161,6 +161,11 @@ pkt_metadata_init(struct pkt_metadata *md, odp_port_t
> port)
> */
> memset(md, 0, offsetof(struct pkt_metadata, ct_orig_tuple_ipv6));
>
> + /* Explicitly zero out ct_zone in order to be able to properly determine
> + * when a packet enters a new conntrack zone.
> + */
> + md->ct_zone = 0;
> +
> /* It can be expensive to zero out all of the tunnel metadata. However,
> * we can just zero out ip_dst and the rest of the data will never be
> * looked at. */
>
Hi,
Just a reminder about this patch.
OVN LB harpinning system tests on OVN master and OVN-20.03 with OVS
userspace datapath fail because of the issue this patch addresses.
Thanks,
Dumitru
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
