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 <[email protected]>

Regards
_Sugesh


> -----Original Message-----
> From: [email protected] [mailto:ovs-dev-
> [email protected]] On Behalf Of [email protected]
> Sent: Monday, June 19, 2017 11:12 AM
> To: [email protected]
> Subject: [ovs-dev] [PATCH 3/4] conntrack: pass current time to
> conntrack_execute.
> 
> From: Antonio Fischetti <[email protected]>
> 
> Current time is passed to conntrack_execute so it doesn't have to recompute
> it again.
> 
> Signed-off-by: Antonio Fischetti <[email protected]>
> ---
>  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
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to