On Wed, Nov 30, 2011 at 12:21 PM, Ben Pfaff <b...@nicira.com> wrote:
> 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.
ok, I will send separate patch for this.
Thanks,
Pravin.
>
> Thanks,
>
> Ben.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
