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 dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev