[ovs-dev] [PATCH v7 1/4] conntrack: handle already natted packets

Dumitru Ceara dceara at redhat.com
Thu Jun 24 10:55:56 UTC 2021


On 6/21/21 12:06 PM, Paolo Valerio wrote:
> when a packet gets dnatted and then recirculated, it could be possible
> that it matches another rule that performs another nat action.
> The kernel datapath handles this situation turning to a no-op the
> second nat action, so natting only once the packet.  In the userspace
> datapath instead, when the ct action gets executed, an initial lookup
> of the translated packet fails to retrieve the connection related to
> the packet, leading to the creation of a new entry in ct for the src
> nat action with a subsequent failure of the connection establishment.
> 
> with the following flows:
> 
> table=0,priority=30,in_port=1,ip,nw_dst=192.168.2.100,actions=ct(commit,nat(dst=10.1.1.2:80),table=1)
> table=0,priority=20,in_port=2,ip,actions=ct(nat,table=1)
> table=0,priority=10,ip,actions=resubmit(,2)
> table=0,priority=10,arp,actions=NORMAL
> table=0,priority=0,actions=drop
> table=1,priority=5,ip,actions=ct(commit,nat(src=10.1.1.240),table=2)
> table=2,in_port=ovs-l0,actions=2
> table=2,in_port=ovs-r0,actions=1
> 
> establishing a connection from 10.1.1.1 to 192.168.2.100 the outcome is:
> 
> tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=4000,dport=80),reply=(src=10.1.1.2,dst=10.1.1.240,sport=80,dport=4000),protoinfo=(state=ESTABLISHED)
> tcp,orig=(src=10.1.1.1,dst=192.168.2.100,sport=4000,dport=80),reply=(src=10.1.1.2,dst=10.1.1.1,sport=80,dport=4000),protoinfo=(state=ESTABLISHED)
> 
> with this patch applied the outcome is:
> 
> tcp,orig=(src=10.1.1.1,dst=192.168.2.100,sport=4000,dport=80),reply=(src=10.1.1.2,dst=10.1.1.1,sport=80,dport=4000),protoinfo=(state=ESTABLISHED)
> 
> The patch performs, for already natted packets, a lookup of the
> reverse key in order to retrieve the related entry, it also adds a
> test case that besides testing the scenario ensures that the other ct
> actions are executed.
> 
> Reported-by: Dumitru Ceara <dceara at redhat.com>
> Signed-off-by: Paolo Valerio <pvalerio at redhat.com>
> ---

Hi Paolo,

Thanks for the patch!  I tested it and it works fine for OVN.  I have a
few comments/questions below.

>  lib/conntrack.c         |   30 +++++++++++++++++++++++++++++-
>  tests/system-traffic.at |   35 +++++++++++++++++++++++++++++++++++
>  2 files changed, 64 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 99198a601..7e8b16a3e 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -1281,6 +1281,33 @@ process_one_fast(uint16_t zone, const uint32_t *setmark,
>      }
>  }
>  
> +static void
> +initial_conn_lookup(struct conntrack *ct, struct conn_lookup_ctx *ctx,
> +         long long now, bool natted)

Nit: indentation.

> +{
> +    bool found;
> +
> +    if (natted) {
> +        /* if the packet has been already natted (e.g. a previous
> +         * action took place), retrieve it performing a lookup of its
> +         * reverse key. */
> +        conn_key_reverse(&ctx->key);
> +    }
> +
> +    found = conn_key_lookup(ct, &ctx->key, ctx->hash,
> +                            now, &ctx->conn, &ctx->reply);
> +    if (natted) {
> +        if (OVS_LIKELY(found)) {
> +            ctx->reply = !ctx->reply;
> +            ctx->key = ctx->reply ? ctx->conn->rev_key : ctx->conn->key;
> +            ctx->hash = conn_key_hash(&ctx->key, ct->hash_basis);
> +        } else {
> +            /* in case of failure restore the initial key. */
> +            conn_key_reverse(&ctx->key);

Can the lookup actually fail?  I mean, if the packet was natted, there
must have been a connection on which it got natted.  Anyway, I think we
should probably also increment a coverage counter.  I guess dropping
such packets would be hard, right?

Thanks,
Dumitru



More information about the dev mailing list