This not only improves logging, it's also a nice cleanup. Thank you, I
didn't expect that part.
I see one nit: in the update_stats() case where we log, please use
VLOG_DROP_WARN() followed by plain VLOG_WARN() to avoid formatting the
log message in the case where it is just going to get discarded.
Thanks,
Ben.
On Wed, Nov 30, 2011 at 01:34:50PM -0800, Pravin B Shelar wrote:
> Fixed according to comments from Ben.
>
> --8<--------------------------cut here-------------------------->8--
>
> Treat unexpected and un-fit flow in a similar manner.
>
> ---
> ofproto/ofproto-dpif.c | 31 ++++++++++++++++++-------------
> 1 files changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 14b5447..853a170 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -373,8 +373,7 @@ static struct subfacet *subfacet_create(struct
> ofproto_dpif *, struct facet *,
> const struct nlattr *key,
> size_t key_len, ovs_be16
> initial_tci);
> static struct subfacet *subfacet_find(struct ofproto_dpif *,
> - const struct nlattr *key, size_t
> key_len,
> - const struct flow *);
> + const struct nlattr *key, size_t
> key_len);
> static void subfacet_destroy(struct ofproto_dpif *, struct subfacet *);
> static void subfacet_destroy__(struct ofproto_dpif *, struct subfacet *);
> static void subfacet_reset_dp_stats(struct subfacet *,
> @@ -2782,16 +2781,9 @@ update_stats(struct ofproto_dpif *p)
>
> dpif_flow_dump_start(&dump, p->dpif);
> while (dpif_flow_dump_next(&dump, &key, &key_len, NULL, NULL, &stats)) {
> - enum odp_key_fitness fitness;
> struct subfacet *subfacet;
> - struct flow flow;
>
> - fitness = odp_flow_key_to_flow(key, key_len, &flow);
> - if (fitness == ODP_FIT_ERROR) {
> - continue;
> - }
> -
> - subfacet = subfacet_find(p, key, key_len, &flow);
> + subfacet = subfacet_find(p, key, key_len);
> if (subfacet && subfacet->installed) {
> struct facet *facet = subfacet->facet;
>
> @@ -2815,8 +2807,15 @@ update_stats(struct ofproto_dpif *p)
> facet_account(p, facet);
> facet_push_stats(facet);
> } else {
> + struct ds s;
> +
> /* There's a flow in the datapath that we know nothing about, or
> a
> * flow that shouldn't be installed but was anyway. Delete it.
> */
> + ds_init(&s);
> + odp_flow_key_format(key, key_len, &s);
> + VLOG_WARN_RL(&rl, "unexpected flow from datapath %s",
> ds_cstr(&s));
> + ds_destroy(&s);
> +
> COVERAGE_INC(facet_unexpected);
> dpif_flow_del(p->dpif, key, key_len, NULL);
> }
> @@ -3508,12 +3507,18 @@ subfacet_create(struct ofproto_dpif *ofproto, struct
> facet *facet,
> * 'flow'. Returns the subfacet if one exists, otherwise NULL. */
> static struct subfacet *
> subfacet_find(struct ofproto_dpif *ofproto,
> - const struct nlattr *key, size_t key_len,
> - const struct flow *flow)
> + const struct nlattr *key, size_t key_len)
> {
> uint32_t key_hash = odp_flow_key_hash(key, key_len);
> + enum odp_key_fitness fitness;
> + struct flow flow;
> +
> + fitness = odp_flow_key_to_flow(key, key_len, &flow);
> + if (fitness == ODP_FIT_ERROR) {
> + return NULL;
> + }
>
> - return subfacet_find__(ofproto, key, key_len, key_hash, flow);
> + return subfacet_find__(ofproto, key, key_len, key_hash, &flow);
> }
>
> /* Uninstalls 'subfacet' from the datapath, if it is installed, removes it
> from
> --
> 1.7.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
