On Wed, Nov 30, 2011 at 12:16:08PM -0800, Pravin B Shelar wrote:
> Following patch fixes memory leak in case there is ODP_FIT_ERROR
> on flow key.
The changes that match the summary look good to me. Thank you, those
are good catches.
This one I'm less sure about:
> @@ -2788,6 +2791,14 @@ update_stats(struct ofproto_dpif *p)
>
> fitness = odp_flow_key_to_flow(key, key_len, &flow);
> if (fitness == ODP_FIT_ERROR) {
> + struct ds s;
> +
> + ds_init(&s);
> + odp_flow_key_format(key, key_len, &s);
> + VLOG_ERR_RL(&rl, "unexpected flow from datapath %s",
> ds_cstr(&s));
> + ds_destroy(&s);
> +
> + dpif_flow_del(p->dpif, key, key_len, NULL);
> continue;
> }
It's much louder than the similar case in the "else" clause at the end
of the while loop. I think that these two cases should be treated in a
similar manner; both simply mean that there is a flow that we don't
expect in the datapath. I think that your new code is probably what we
really want these days. The appropriate log level looks to me like WARN
("A low-level operation failed, but higher-level subsystems may be able
to recover.").
It seems to me that this should be broken into two patches.
Thanks,
Ben.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
