diff mbox series

[ovs-dev,v3,4/4] conntrack: Optimize recirculations.

Message ID 1543814239-89269-5-git-send-email-dlu998@gmail.com
State Superseded
Headers show
Series conntrack: Optimize and refactor. | expand

Commit Message

Darrell Ball Dec. 3, 2018, 5:17 a.m. UTC
In most cases, recirculations through conntrack can be much less costly.

If conntrack entry keys are changed outside of conntrack, the user must
call the pre-existing API, 'conntrack_clear()', which is modified in
this patch.

An implicit assumption is that a packet is processed to completion by a
PMD before returning to it's poll loop.

Signed-off-by: Darrell Ball <dlu998@gmail.com>
---
 lib/conntrack.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
 lib/packets.h   |  4 ++++
 2 files changed, 48 insertions(+), 2 deletions(-)

Comments

Darrell Ball Dec. 3, 2018, 6:26 p.m. UTC | #1
typo noted inline

On Sun, Dec 2, 2018 at 9:18 PM Darrell Ball <dlu998@gmail.com> wrote:

> In most cases, recirculations through conntrack can be much less costly.
>
> If conntrack entry keys are changed outside of conntrack, the user must
> call the pre-existing API, 'conntrack_clear()', which is modified in
> this patch.
>
> An implicit assumption is that a packet is processed to completion by a
> PMD before returning to it's poll loop.
>
> Signed-off-by: Darrell Ball <dlu998@gmail.com>
> ---
>  lib/conntrack.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
>  lib/packets.h   |  4 ++++
>  2 files changed, 48 insertions(+), 2 deletions(-)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 3168557..6f926c6 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -1200,6 +1200,39 @@ process_one(struct dp_packet *pkt, struct
> conn_lookup_ctx *ctx, uint16_t zone,
>      }
>
>      handle_alg_ctl(ctx, pkt, ct_alg_ctl, conn, now, !!nat_action_info);
> +
> +    if (nat_action_info && ct_alg_ctl == CT_ALG_CTL_NONE) {
>

above check should have been:

+    if (!nat_action_info && ct_alg_ctl == CT_ALG_CTL_NONE) {


> +        pkt->md.conn = conn;
> +        pkt->md.reply = ctx->reply;
> +        pkt->md.icmp_related = ctx->icmp_related;
> +    } else {
> +        pkt->md.conn = NULL;
> +    }
> +}
> +
> +static inline void
> +process_one_fast(struct dp_packet *pkt, uint16_t zone,
> +                 const uint32_t *setmark,
> +                 const struct ovs_key_ct_labels *setlabel,
> +                 const struct nat_action_info_t *nat_action_info,
> +                 struct conn *conn)
> +{
> +    if (nat_action_info) {
> +        handle_nat(pkt, conn, zone, pkt->md.reply, pkt->md.icmp_related);
> +        pkt->md.conn = NULL;
> +    }
> +
> +    pkt->md.ct_zone = zone;
> +    pkt->md.ct_mark = conn->mark;
> +    pkt->md.ct_label = conn->label;
> +
> +    if (setmark) {
> +        set_mark(pkt, conn, setmark[0], setmark[1]);
> +    }
> +
> +    if (setlabel) {
> +        set_label(pkt, conn, &setlabel[0], &setlabel[1]);
> +    }
>  }
>
>  /* Sends the packets in '*pkt_batch' through the connection tracker
> 'ct'.  All
> @@ -1223,8 +1256,16 @@ conntrack_execute(struct dp_packet_batch
> *pkt_batch, ovs_be16 dl_type,
>      struct conn_lookup_ctx ctx;
>
>      DP_PACKET_BATCH_FOR_EACH (i, packet, pkt_batch) {
> -        if (packet->md.ct_state == CS_INVALID
> -            || !conn_key_extract(packet, dl_type, &ctx, zone)) {
> +        struct conn *conn = packet->md.conn;
> +        if (OVS_UNLIKELY(packet->md.ct_state == CS_INVALID)) {
> +            write_ct_md(packet, zone, NULL, NULL, NULL);
> +                continue;
> +        } else if (conn && !force && !commit && conn->key.zone == zone) {
> +            process_one_fast(packet, zone, setmark, setlabel,
> nat_action_info,
> +                             packet->md.conn);
> +            continue;
> +        } else if (OVS_UNLIKELY(!conn_key_extract(packet, dl_type, &ctx,
> +                                zone))) {
>              packet->md.ct_state = CS_INVALID;
>              write_ct_md(packet, zone, NULL, NULL, NULL);
>              continue;
> @@ -1242,6 +1283,7 @@ conntrack_clear(struct dp_packet *packet)
>      /* According to pkt_metadata_init(), ct_state == 0 is enough to make
> all of
>       * the conntrack fields invalid. */
>      packet->md.ct_state = 0;
> +    packet->md.conn = NULL;
>  }
>
>  static void
> diff --git a/lib/packets.h b/lib/packets.h
> index 09a0ac3..a88d1ad 100644
> --- a/lib/packets.h
> +++ b/lib/packets.h
> @@ -108,6 +108,9 @@ PADDED_MEMBERS_CACHELINE_MARKER(CACHE_LINE_SIZE,
> cacheline0,
>      uint32_t ct_mark;           /* Connection mark. */
>      ovs_u128 ct_label;          /* Connection label. */
>      union flow_in_port in_port; /* Input port. */
> +    void *conn;
> +    bool reply;
> +    bool icmp_related;
>  );
>
>  PADDED_MEMBERS_CACHELINE_MARKER(CACHE_LINE_SIZE, cacheline1,
> @@ -157,6 +160,7 @@ pkt_metadata_init(struct pkt_metadata *md, odp_port_t
> port)
>      md->tunnel.ip_dst = 0;
>      md->tunnel.ipv6_dst = in6addr_any;
>      md->in_port.odp_port = port;
> +    md->conn = NULL;
>  }
>
>  /* This function prefetches the cachelines touched by pkt_metadata_init()
> --
> 1.9.1
>
>
diff mbox series

Patch

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 3168557..6f926c6 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -1200,6 +1200,39 @@  process_one(struct dp_packet *pkt, struct conn_lookup_ctx *ctx, uint16_t zone,
     }
 
     handle_alg_ctl(ctx, pkt, ct_alg_ctl, conn, now, !!nat_action_info);
+
+    if (nat_action_info && ct_alg_ctl == CT_ALG_CTL_NONE) {
+        pkt->md.conn = conn;
+        pkt->md.reply = ctx->reply;
+        pkt->md.icmp_related = ctx->icmp_related;
+    } else {
+        pkt->md.conn = NULL;
+    }
+}
+
+static inline void
+process_one_fast(struct dp_packet *pkt, uint16_t zone,
+                 const uint32_t *setmark,
+                 const struct ovs_key_ct_labels *setlabel,
+                 const struct nat_action_info_t *nat_action_info,
+                 struct conn *conn)
+{
+    if (nat_action_info) {
+        handle_nat(pkt, conn, zone, pkt->md.reply, pkt->md.icmp_related);
+        pkt->md.conn = NULL;
+    }
+
+    pkt->md.ct_zone = zone;
+    pkt->md.ct_mark = conn->mark;
+    pkt->md.ct_label = conn->label;
+
+    if (setmark) {
+        set_mark(pkt, conn, setmark[0], setmark[1]);
+    }
+
+    if (setlabel) {
+        set_label(pkt, conn, &setlabel[0], &setlabel[1]);
+    }
 }
 
 /* Sends the packets in '*pkt_batch' through the connection tracker 'ct'.  All
@@ -1223,8 +1256,16 @@  conntrack_execute(struct dp_packet_batch *pkt_batch, ovs_be16 dl_type,
     struct conn_lookup_ctx ctx;
 
     DP_PACKET_BATCH_FOR_EACH (i, packet, pkt_batch) {
-        if (packet->md.ct_state == CS_INVALID
-            || !conn_key_extract(packet, dl_type, &ctx, zone)) {
+        struct conn *conn = packet->md.conn;
+        if (OVS_UNLIKELY(packet->md.ct_state == CS_INVALID)) {
+            write_ct_md(packet, zone, NULL, NULL, NULL);
+                continue;
+        } else if (conn && !force && !commit && conn->key.zone == zone) {
+            process_one_fast(packet, zone, setmark, setlabel, nat_action_info,
+                             packet->md.conn);
+            continue;
+        } else if (OVS_UNLIKELY(!conn_key_extract(packet, dl_type, &ctx,
+                                zone))) {
             packet->md.ct_state = CS_INVALID;
             write_ct_md(packet, zone, NULL, NULL, NULL);
             continue;
@@ -1242,6 +1283,7 @@  conntrack_clear(struct dp_packet *packet)
     /* According to pkt_metadata_init(), ct_state == 0 is enough to make all of
      * the conntrack fields invalid. */
     packet->md.ct_state = 0;
+    packet->md.conn = NULL;
 }
 
 static void
diff --git a/lib/packets.h b/lib/packets.h
index 09a0ac3..a88d1ad 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -108,6 +108,9 @@  PADDED_MEMBERS_CACHELINE_MARKER(CACHE_LINE_SIZE, cacheline0,
     uint32_t ct_mark;           /* Connection mark. */
     ovs_u128 ct_label;          /* Connection label. */
     union flow_in_port in_port; /* Input port. */
+    void *conn;
+    bool reply;
+    bool icmp_related;
 );
 
 PADDED_MEMBERS_CACHELINE_MARKER(CACHE_LINE_SIZE, cacheline1,
@@ -157,6 +160,7 @@  pkt_metadata_init(struct pkt_metadata *md, odp_port_t port)
     md->tunnel.ip_dst = 0;
     md->tunnel.ipv6_dst = in6addr_any;
     md->in_port.odp_port = port;
+    md->conn = NULL;
 }
 
 /* This function prefetches the cachelines touched by pkt_metadata_init()