[ovs-dev,v3,3/4] conntrack: pass current time to conntrack_execute.

Message ID 1502466766-17370-4-git-send-email-antonio.fischetti@intel.com
State Accepted
Delegated to: Darrell Ball
Headers show

Commit Message

Fischetti, Antonio Aug. 11, 2017, 3:52 p.m.
Current time is passed to conntrack_execute so it doesn't have
to recompute it again.

Signed-off-by: Antonio Fischetti <antonio.fischetti@intel.com>
Acked by: Sugesh Chandran <sugesh.chandran@intel.com>
---
In a firewall testbench set up with

 table=0, priority=1 actions=drop
 table=0, priority=10,arp actions=NORMAL
 table=0, priority=100,ct_state=-trk,ip actions=ct(table=1)
 table=1, ct_state=+new+trk,ip,in_port=1 actions=ct(commit),output:2
 table=1, ct_state=+est+trk,ip,in_port=1 actions=output:2
 table=1, ct_state=+new+trk,ip,in_port=2 actions=drop
 table=1, ct_state=+est+trk,ip,in_port=2 actions=output:1

I measured packet Rx rate in Mpps (regardless of packet loss).
Bidirectional test with 64B UDP packets.

2 PMDs, 3 Tx q.

I collected the following performance figures.

Orig OvS-DPDK means Commit ID:
  6b1babacc3ca0488e07596bf822fe356c9bab646

                +-------------------+----------------+
        UDP     | Orig OvS-DPDK +   | Previous case  |
    connections | patches #1,2      | + this patch   |
  --------------+-------------------+----------------+
         1      |   1.42, 1.42      |   1.82, 1.76   |
        10      |   2.35, 2.35      |   2.35, 2.35   |
        50      |   2.38, 2.42      |   2.41, 2.43   |
       100      |   2.49, 2.49      |   2.50, 2.52   |
       500      |   2.11, 2.10      |   2.21, 2.22   |
      1000      |   2.02, 2.00      |   2.09, 2.11   |
      5000      |   1.94, 1.87      |   1.93, 1.89   |
  --------------+-------------------+----------------+
---
 lib/conntrack.c        | 4 ++--
 lib/conntrack.h        | 3 ++-
 lib/dpif-netdev.c      | 2 +-
 tests/test-conntrack.c | 8 +++++---
 4 files changed, 10 insertions(+), 7 deletions(-)

Comments

Darrell Ball Aug. 14, 2017, 6:33 a.m. | #1
I did not try it yet, but seems reasonable

-----Original Message-----
From: <ovs-dev-bounces@openvswitch.org> on behalf of "antonio.fischetti@intel.com" <antonio.fischetti@intel.com>
Date: Friday, August 11, 2017 at 8:52 AM
To: "dev@openvswitch.org" <dev@openvswitch.org>
Subject: [ovs-dev] [PATCH v3 3/4] conntrack: pass current time to	conntrack_execute.

    Current time is passed to conntrack_execute so it doesn't have
    to recompute it again.
    
    Signed-off-by: Antonio Fischetti <antonio.fischetti@intel.com>
    Acked by: Sugesh Chandran <sugesh.chandran@intel.com>
    ---
    In a firewall testbench set up with
    
     table=0, priority=1 actions=drop
     table=0, priority=10,arp actions=NORMAL
     table=0, priority=100,ct_state=-trk,ip actions=ct(table=1)
     table=1, ct_state=+new+trk,ip,in_port=1 actions=ct(commit),output:2
     table=1, ct_state=+est+trk,ip,in_port=1 actions=output:2
     table=1, ct_state=+new+trk,ip,in_port=2 actions=drop
     table=1, ct_state=+est+trk,ip,in_port=2 actions=output:1
    
    I measured packet Rx rate in Mpps (regardless of packet loss).
    Bidirectional test with 64B UDP packets.
    
    2 PMDs, 3 Tx q.
    
    I collected the following performance figures.
    
    Orig OvS-DPDK means Commit ID:
      6b1babacc3ca0488e07596bf822fe356c9bab646
    
                    +-------------------+----------------+
            UDP     | Orig OvS-DPDK +   | Previous case  |
        connections | patches #1,2      | + this patch   |
      --------------+-------------------+----------------+
             1      |   1.42, 1.42      |   1.82, 1.76   |
            10      |   2.35, 2.35      |   2.35, 2.35   |
            50      |   2.38, 2.42      |   2.41, 2.43   |
           100      |   2.49, 2.49      |   2.50, 2.52   |
           500      |   2.11, 2.10      |   2.21, 2.22   |
          1000      |   2.02, 2.00      |   2.09, 2.11   |
          5000      |   1.94, 1.87      |   1.93, 1.89   |
      --------------+-------------------+----------------+
    ---
     lib/conntrack.c        | 4 ++--
     lib/conntrack.h        | 3 ++-
     lib/dpif-netdev.c      | 2 +-
     tests/test-conntrack.c | 8 +++++---
     4 files changed, 10 insertions(+), 7 deletions(-)
    
    diff --git a/lib/conntrack.c b/lib/conntrack.c
    index 1c0e023..c61bcd6 100644
    --- a/lib/conntrack.c
    +++ b/lib/conntrack.c
    @@ -1136,13 +1136,13 @@ conntrack_execute(struct conntrack *ct, struct dp_packet_batch *pkt_batch,
                       const uint32_t *setmark,
                       const struct ovs_key_ct_labels *setlabel,
                       const char *helper,
    -                  const struct nat_action_info_t *nat_action_info)
    +                  const struct nat_action_info_t *nat_action_info,
    +                  long long now)
     {
     
         struct dp_packet **pkts = pkt_batch->packets;
         size_t cnt = pkt_batch->count;
         struct conn_lookup_ctx ctx;
    -    long long now = time_msec();
     
         for (size_t i = 0; i < cnt; i++) {
             if (!conn_key_extract(ct, pkts[i], dl_type, &ctx, zone)) {
    diff --git a/lib/conntrack.h b/lib/conntrack.h
    index 272d2d5..fbeef1c 100644
    --- a/lib/conntrack.h
    +++ b/lib/conntrack.h
    @@ -95,7 +95,8 @@ int conntrack_execute(struct conntrack *, struct dp_packet_batch *,
                           uint16_t zone, const uint32_t *setmark,
                           const struct ovs_key_ct_labels *setlabel,
                           const char *helper,
    -                      const struct nat_action_info_t *nat_action_info);
    +                      const struct nat_action_info_t *nat_action_info,
    +                      long long now);
     
     struct conntrack_dump {
         struct conntrack *ct;
    diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
    index 0db6f83..6aeedf8 100644
    --- a/lib/dpif-netdev.c
    +++ b/lib/dpif-netdev.c
    @@ -5471,7 +5471,7 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_,
     
             conntrack_execute(&dp->conntrack, packets_, aux->flow->dl_type, force,
                               commit, zone, setmark, setlabel, helper,
    -                          nat_action_info_ref);
    +                          nat_action_info_ref, now);
             break;
         }
     
    diff --git a/tests/test-conntrack.c b/tests/test-conntrack.c
    index 6a77db8..b27d181 100644
    --- a/tests/test-conntrack.c
    +++ b/tests/test-conntrack.c
    @@ -84,12 +84,13 @@ ct_thread_main(void *aux_)
         struct dp_packet_batch *pkt_batch;
         ovs_be16 dl_type;
         size_t i;
    +    long long now = time_msec();
     
         pkt_batch = prepare_packets(batch_size, change_conn, aux->tid, &dl_type);
         ovs_barrier_block(&barrier);
         for (i = 0; i < n_pkts; i += batch_size) {
             conntrack_execute(&ct, pkt_batch, dl_type, false, true, 0, NULL, NULL,
    -                          NULL, NULL);
    +                          NULL, NULL, now);
         }
         ovs_barrier_block(&barrier);
         destroy_packets(pkt_batch);
    @@ -154,6 +155,7 @@ pcap_batch_execute_conntrack(struct conntrack *ct,
     {
         struct dp_packet_batch new_batch;
         ovs_be16 dl_type = htons(0);
    +    long long now = time_msec();
     
         dp_packet_batch_init(&new_batch);
     
    @@ -172,7 +174,7 @@ pcap_batch_execute_conntrack(struct conntrack *ct,
     
             if (flow.dl_type != dl_type) {
                 conntrack_execute(ct, &new_batch, dl_type, false, true, 0,
    -                              NULL, NULL, NULL, NULL);
    +                              NULL, NULL, NULL, NULL, now);
                 dp_packet_batch_init(&new_batch);
             }
             new_batch.packets[new_batch.count++] = packet;;
    @@ -180,7 +182,7 @@ pcap_batch_execute_conntrack(struct conntrack *ct,
     
         if (!dp_packet_batch_is_empty(&new_batch)) {
             conntrack_execute(ct, &new_batch, dl_type, false, true, 0, NULL, NULL,
    -                          NULL, NULL);
    +                          NULL, NULL, now);
         }
     
     }
    -- 
    2.4.11
    
    _______________________________________________
    dev mailing list
    dev@openvswitch.org
    https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=qkVYSe2nXrmLDEmIGFzyiHryMudtfscdCk2YOvOxlgQ&s=KjURcHI4LDMd760v9bMg2bmXQ6LjbUx1PIsi20vc8Wk&e=

Patch

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 1c0e023..c61bcd6 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -1136,13 +1136,13 @@  conntrack_execute(struct conntrack *ct, struct dp_packet_batch *pkt_batch,
                   const uint32_t *setmark,
                   const struct ovs_key_ct_labels *setlabel,
                   const char *helper,
-                  const struct nat_action_info_t *nat_action_info)
+                  const struct nat_action_info_t *nat_action_info,
+                  long long now)
 {
 
     struct dp_packet **pkts = pkt_batch->packets;
     size_t cnt = pkt_batch->count;
     struct conn_lookup_ctx ctx;
-    long long now = time_msec();
 
     for (size_t i = 0; i < cnt; i++) {
         if (!conn_key_extract(ct, pkts[i], dl_type, &ctx, zone)) {
diff --git a/lib/conntrack.h b/lib/conntrack.h
index 272d2d5..fbeef1c 100644
--- a/lib/conntrack.h
+++ b/lib/conntrack.h
@@ -95,7 +95,8 @@  int conntrack_execute(struct conntrack *, struct dp_packet_batch *,
                       uint16_t zone, const uint32_t *setmark,
                       const struct ovs_key_ct_labels *setlabel,
                       const char *helper,
-                      const struct nat_action_info_t *nat_action_info);
+                      const struct nat_action_info_t *nat_action_info,
+                      long long now);
 
 struct conntrack_dump {
     struct conntrack *ct;
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 0db6f83..6aeedf8 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -5471,7 +5471,7 @@  dp_execute_cb(void *aux_, struct dp_packet_batch *packets_,
 
         conntrack_execute(&dp->conntrack, packets_, aux->flow->dl_type, force,
                           commit, zone, setmark, setlabel, helper,
-                          nat_action_info_ref);
+                          nat_action_info_ref, now);
         break;
     }
 
diff --git a/tests/test-conntrack.c b/tests/test-conntrack.c
index 6a77db8..b27d181 100644
--- a/tests/test-conntrack.c
+++ b/tests/test-conntrack.c
@@ -84,12 +84,13 @@  ct_thread_main(void *aux_)
     struct dp_packet_batch *pkt_batch;
     ovs_be16 dl_type;
     size_t i;
+    long long now = time_msec();
 
     pkt_batch = prepare_packets(batch_size, change_conn, aux->tid, &dl_type);
     ovs_barrier_block(&barrier);
     for (i = 0; i < n_pkts; i += batch_size) {
         conntrack_execute(&ct, pkt_batch, dl_type, false, true, 0, NULL, NULL,
-                          NULL, NULL);
+                          NULL, NULL, now);
     }
     ovs_barrier_block(&barrier);
     destroy_packets(pkt_batch);
@@ -154,6 +155,7 @@  pcap_batch_execute_conntrack(struct conntrack *ct,
 {
     struct dp_packet_batch new_batch;
     ovs_be16 dl_type = htons(0);
+    long long now = time_msec();
 
     dp_packet_batch_init(&new_batch);
 
@@ -172,7 +174,7 @@  pcap_batch_execute_conntrack(struct conntrack *ct,
 
         if (flow.dl_type != dl_type) {
             conntrack_execute(ct, &new_batch, dl_type, false, true, 0,
-                              NULL, NULL, NULL, NULL);
+                              NULL, NULL, NULL, NULL, now);
             dp_packet_batch_init(&new_batch);
         }
         new_batch.packets[new_batch.count++] = packet;;
@@ -180,7 +182,7 @@  pcap_batch_execute_conntrack(struct conntrack *ct,
 
     if (!dp_packet_batch_is_empty(&new_batch)) {
         conntrack_execute(ct, &new_batch, dl_type, false, true, 0, NULL, NULL,
-                          NULL, NULL);
+                          NULL, NULL, now);
     }
 
 }