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

Chandran, Sugesh sugesh.chandran at intel.com
Fri Jun 23 11:00:17 UTC 2017


Hi Antonio,

The changes are LGTM.
Didn't find any issues with clang, gcc sparse builds and checkpatch.
Also in my testing didn't see any significant performance difference as such. 

Acked by: Sugesh Chandran <sugesh.chandran at intel.com>

Regards
_Sugesh


> -----Original Message-----
> From: ovs-dev-bounces at openvswitch.org [mailto:ovs-dev-
> bounces at openvswitch.org] On Behalf Of antonio.fischetti at intel.com
> Sent: Monday, June 19, 2017 11:12 AM
> To: dev at openvswitch.org
> Subject: [ovs-dev] [PATCH 3/4] conntrack: pass current time to
> conntrack_execute.
> 
> From: Antonio Fischetti <antonio.fischetti at intel.com>
> 
> Current time is passed to conntrack_execute so it doesn't have to recompute
> it again.
> 
> Signed-off-by: Antonio Fischetti <antonio.fischetti at intel.com>
> ---
>  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 90b154a..63ad273 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -839,11 +839,11 @@ 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;
> -    long long now = time_msec();
>      struct conn_lookup_ctx ctx;
> 
>      if (helper) {
> diff --git a/lib/conntrack.h b/lib/conntrack.h index 243aebb..763a68c 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 64a3cd4..1c22afd
> 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -5341,7 +5341,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
> f79a9fc..b0ba9ca 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 at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list