[ovs-dev] [PATCH] datapath-windows: Solved BSOD when handling flows

Nithin Raju nithin at vmware.com
Mon Aug 3 15:26:47 UTC 2015


> On Jul 1, 2015, at 10:28 AM, Sorin Vinturis <svinturis at cloudbasesolutions.com> wrote:
> 
> OvsPrepareFlow() returns an error only when the new flow allocation
> fails. In this case HandleFlowPut() should return error without trying
> to free the flow, thus avoiding the BSOD.
> 
> Signed-off-by: Sorin Vinturis <svinturis at cloudbasesolutions.com>
> Reported-by: Sorin Vinturis <svinturis at cloudbasesolutions.com>
> Reported-at: https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs-2Dissues_issues_91&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=1fAwiEJ0trEqEbnXCwc2WN_OdJ9ANd0-fuucgcbW9Bs&s=v7pTsmCFx-lu-RBhxiVv5cbM5jUrnSC6P6sP6dp-PRw&e= 
> ---
> This patch should be applied both on master and branch 2.4.
> ---
> datapath-windows/ovsext/Flow.c | 1 -
> 1 file changed, 1 deletion(-)
> 
> diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
> index 6fa10a3..b93f475 100644
> --- a/datapath-windows/ovsext/Flow.c
> +++ b/datapath-windows/ovsext/Flow.c
> @@ -2163,7 +2163,6 @@ HandleFlowPut(OvsFlowPut *put,
> 
>         status = OvsPrepareFlow(&KernelFlow, put, hash);
>         if (status != STATUS_SUCCESS) {
> -            FreeFlow(KernelFlow);
>             return STATUS_UNSUCCESSFUL;
>         }

I’m looking at OvsPrepareFlow(), and the only case where we the function can fail is if there’s a memory allocation failure at which point ‘KernelFlow’ would end up being NULL, so, FreeFlow() will actually ASSERT().

Moreover, I think it would be wrong for the caller to free up an object that is allocated within OvsPrepareFlow(). I believe that if the function OvsPrepareFlow() fails, it should take responsibility for cleaning the objects that were allocated in the context of the function, and not expect that caller to do so.

thanks,
-- Nithin


More information about the dev mailing list