[ovs-dev] [PATCH] ofproto-dpif: Fix memory leak.

[Pravin Shelar]

On Wed, Nov 30, 2011 at 12:21 PM, Ben Pfaff <blp at 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.