[ovs-dev] [VLAN splinters 03/16] ofproto-dpif: Factor NetFlow active timeouts out of flow expiration.

Ethan Jackson ethan at nicira.com
Tue Nov 22 23:19:37 UTC 2011


Looks good.

Ethan

On Tue, Nov 15, 2011 at 17:17, Ben Pfaff <blp at nicira.com> wrote:
> NetFlow active timeouts were only mixed in with flow expiration for
> convenience: both processes need to iterate all the facets.  But
> an upcoming commit will change flow expiration to work in terms of
> a new "subfacet" entity, so they will no longer fit together well.
>
> This change could be seen as an optimization, since NetFlow active
> timeouts don't ordinarily have to run as often as flow expiration,
> especially when the flow expiration rate is stepped up due to a
> large volume of flows.
> ---
>  ofproto/netflow.c      |   27 +++++++++++-
>  ofproto/netflow.h      |    4 +-
>  ofproto/ofproto-dpif.c |  105 ++++++++++++++++++++++++++++-------------------
>  3 files changed, 90 insertions(+), 46 deletions(-)
>
> diff --git a/ofproto/netflow.c b/ofproto/netflow.c
> index bf2e628..a128c5f 100644
> --- a/ofproto/netflow.c
> +++ b/ofproto/netflow.c
> @@ -27,6 +27,7 @@
>  #include "ofpbuf.h"
>  #include "ofproto.h"
>  #include "packets.h"
> +#include "poll-loop.h"
>  #include "socket-util.h"
>  #include "timeval.h"
>  #include "util.h"
> @@ -99,6 +100,7 @@ struct netflow {
>     uint32_t netflow_cnt;         /* Flow sequence number for NetFlow. */
>     struct ofpbuf packet;         /* NetFlow packet being accumulated. */
>     long long int active_timeout; /* Timeout for flows that are still active. */
> +    long long int next_timeout;   /* Next scheduled active timeout. */
>     long long int reconfig_time;  /* When we reconfigured the timeouts. */
>  };
>
> @@ -221,13 +223,33 @@ netflow_expire(struct netflow *nf, struct netflow_flow *nf_flow,
>     nf_flow->tcp_flags = 0;
>  }
>
> -void
> +/* Returns true if it's time to send out a round of NetFlow active timeouts,
> + * false otherwise. */
> +bool
>  netflow_run(struct netflow *nf)
>  {
>     if (nf->packet.size) {
>         collectors_send(nf->collectors, nf->packet.data, nf->packet.size);
>         nf->packet.size = 0;
>     }
> +
> +    if (nf->active_timeout && time_msec() >= nf->next_timeout) {
> +        nf->next_timeout = time_msec() + nf->active_timeout;
> +        return true;
> +    } else {
> +        return false;
> +    }
> +}
> +
> +void
> +netflow_wait(struct netflow *nf)
> +{
> +    if (nf->active_timeout) {
> +        poll_timer_wait_until(nf->next_timeout);
> +    }
> +    if (nf->packet.size) {
> +        poll_immediate_wake();
> +    }
>  }
>
>  int
> @@ -253,6 +275,7 @@ netflow_set_options(struct netflow *nf,
>     nf->active_timeout *= 1000;
>     if (old_timeout != nf->active_timeout) {
>         nf->reconfig_time = time_msec();
> +        nf->next_timeout = time_msec();
>     }
>
>     return error;
> @@ -261,7 +284,7 @@ netflow_set_options(struct netflow *nf,
>  struct netflow *
>  netflow_create(void)
>  {
> -    struct netflow *nf = xmalloc(sizeof *nf);
> +    struct netflow *nf = xzalloc(sizeof *nf);
>     nf->engine_type = 0;
>     nf->engine_id = 0;
>     nf->boot_time = time_msec();
> diff --git a/ofproto/netflow.h b/ofproto/netflow.h
> index bf5bf45..daabbac 100644
> --- a/ofproto/netflow.h
> +++ b/ofproto/netflow.h
> @@ -60,7 +60,9 @@ void netflow_destroy(struct netflow *);
>  int netflow_set_options(struct netflow *, const struct netflow_options *);
>  void netflow_expire(struct netflow *, struct netflow_flow *,
>                     struct ofexpired *);
> -void netflow_run(struct netflow *);
> +
> +bool netflow_run(struct netflow *);
> +void netflow_wait(struct netflow *);
>
>  void netflow_flow_init(struct netflow_flow *);
>  void netflow_flow_clear(struct netflow_flow *);
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 0cb33d4..cc02d52 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -422,6 +422,9 @@ static void handle_miss_upcalls(struct ofproto_dpif *,
>  /* Flow expiration. */
>  static int expire(struct ofproto_dpif *);
>
> +/* NetFlow. */
> +static void send_netflow_active_timeouts(struct ofproto_dpif *);
> +
>  /* Utilities. */
>  static int send_packet(struct ofproto_dpif *, uint32_t odp_port,
>                        const struct ofpbuf *packet);
> @@ -627,7 +630,9 @@ run(struct ofproto *ofproto_)
>     }
>
>     if (ofproto->netflow) {
> -        netflow_run(ofproto->netflow);
> +        if (netflow_run(ofproto->netflow)) {
> +            send_netflow_active_timeouts(ofproto);
> +        }
>     }
>     if (ofproto->sflow) {
>         dpif_sflow_run(ofproto->sflow);
> @@ -690,6 +695,9 @@ wait(struct ofproto *ofproto_)
>     HMAP_FOR_EACH (bundle, hmap_node, &ofproto->bundles) {
>         bundle_wait(bundle);
>     }
> +    if (ofproto->netflow) {
> +        netflow_wait(ofproto->netflow);
> +    }
>     mac_learning_wait(ofproto->ml);
>     stp_wait(ofproto);
>     if (ofproto->need_revalidate) {
> @@ -753,24 +761,6 @@ get_tables(struct ofproto *ofproto_, struct ofp_table_stats *ots)
>                        htonll(s.n_hit + ofproto->n_matches));
>  }
>
> -static int
> -set_netflow(struct ofproto *ofproto_,
> -            const struct netflow_options *netflow_options)
> -{
> -    struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
> -
> -    if (netflow_options) {
> -        if (!ofproto->netflow) {
> -            ofproto->netflow = netflow_create();
> -        }
> -        return netflow_set_options(ofproto->netflow, netflow_options);
> -    } else {
> -        netflow_destroy(ofproto->netflow);
> -        ofproto->netflow = NULL;
> -        return 0;
> -    }
> -}
> -
>  static struct ofport *
>  port_alloc(void)
>  {
> @@ -2568,36 +2558,12 @@ facet_max_idle(const struct ofproto_dpif *ofproto)
>  }
>
>  static void
> -facet_active_timeout(struct ofproto_dpif *ofproto, struct facet *facet)
> -{
> -    if (ofproto->netflow && !facet_is_controller_flow(facet) &&
> -        netflow_active_timeout_expired(ofproto->netflow, &facet->nf_flow)) {
> -        struct ofexpired expired;
> -
> -        if (facet->installed) {
> -            struct dpif_flow_stats stats;
> -
> -            facet_put__(ofproto, facet, facet->actions, facet->actions_len,
> -                        &stats);
> -            facet_update_stats(ofproto, facet, &stats);
> -        }
> -
> -        expired.flow = facet->flow;
> -        expired.packet_count = facet->packet_count;
> -        expired.byte_count = facet->byte_count;
> -        expired.used = facet->used;
> -        netflow_expire(ofproto->netflow, &facet->nf_flow, &expired);
> -    }
> -}
> -
> -static void
>  expire_facets(struct ofproto_dpif *ofproto, int dp_max_idle)
>  {
>     long long int cutoff = time_msec() - dp_max_idle;
>     struct facet *facet, *next_facet;
>
>     HMAP_FOR_EACH_SAFE (facet, next_facet, hmap_node, &ofproto->facets) {
> -        facet_active_timeout(ofproto, facet);
>         if (facet->used < cutoff) {
>             facet_remove(ofproto, facet);
>         }
> @@ -5098,6 +5064,26 @@ packet_out(struct ofproto *ofproto_, struct ofpbuf *packet,
>     }
>     return error;
>  }
> +
> +/* NetFlow. */
> +
> +static int
> +set_netflow(struct ofproto *ofproto_,
> +            const struct netflow_options *netflow_options)
> +{
> +    struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
> +
> +    if (netflow_options) {
> +        if (!ofproto->netflow) {
> +            ofproto->netflow = netflow_create();
> +        }
> +        return netflow_set_options(ofproto->netflow, netflow_options);
> +    } else {
> +        netflow_destroy(ofproto->netflow);
> +        ofproto->netflow = NULL;
> +        return 0;
> +    }
> +}
>
>  static void
>  get_netflow_ids(const struct ofproto *ofproto_,
> @@ -5107,6 +5093,39 @@ get_netflow_ids(const struct ofproto *ofproto_,
>
>     dpif_get_netflow_ids(ofproto->dpif, engine_type, engine_id);
>  }
> +
> +static void
> +send_active_timeout(struct ofproto_dpif *ofproto, struct facet *facet)
> +{
> +    if (!facet_is_controller_flow(facet) &&
> +        netflow_active_timeout_expired(ofproto->netflow, &facet->nf_flow)) {
> +        struct ofexpired expired;
> +
> +        if (facet->installed) {
> +            struct dpif_flow_stats stats;
> +
> +            facet_put__(ofproto, facet, facet->actions, facet->actions_len,
> +                        &stats);
> +            facet_update_stats(ofproto, facet, &stats);
> +        }
> +
> +        expired.flow = facet->flow;
> +        expired.packet_count = facet->packet_count;
> +        expired.byte_count = facet->byte_count;
> +        expired.used = facet->used;
> +        netflow_expire(ofproto->netflow, &facet->nf_flow, &expired);
> +    }
> +}
> +
> +static void
> +send_netflow_active_timeouts(struct ofproto_dpif *ofproto)
> +{
> +    struct facet *facet;
> +
> +    HMAP_FOR_EACH (facet, hmap_node, &ofproto->facets) {
> +        send_active_timeout(ofproto, facet);
> +    }
> +}
>
>  static struct ofproto_dpif *
>  ofproto_dpif_lookup(const char *name)
> --
> 1.7.4.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>



More information about the dev mailing list