[ovs-dev] datapath-windows: refactor code to setup dump start state (Was: Re: [PATCH 9/9 v3] datapath-windows: refactor code to setup dump start state)

[Ben Pfaff]

I'd appreciate it if you'd resend separately, with Samuel's ack
included.

Thanks,

Ben.

On Tue, Sep 09, 2014 at 07:43:17PM +0000, Nithin Raju wrote:
> Ben,
> This patch is good to go as well AS AN independent patch. It was marked earlier as [PATCH 9/9 v3], but it has since been broken off from the series, since Ankur sent out a new series, which have been committed.
> 
> Pls. let me know if you want me to send out a fresh patch for this.
> 
> thanks,
> Nithin
> 
> 
> On Sep 6, 2014, at 6:18 PM, Samuel Ghinet <sghinet at cloudbasesolutions.com>
>  wrote:
> 
> > Acked-by: Samuel Ghinet <sghinet at cloudbasesolutions.com>
> > ________________________________________
> > From: Samuel Ghinet
> > Sent: Sunday, September 07, 2014 2:54 AM
> > To: dev at openvswitch.org
> > Cc: Alin Serdean; nithin at vmware.com; ssaurabh at vmware.com; Ankur Sharma
> > Subject:  [PATCH 9/9 v3] datapath-windows: refactor code to         setup   dump start state
> > 
> > Looks good,
> > Sam
> > ________________________________________
> > 
> > Date: Fri, 29 Aug 2014 12:05:14 -0700
> > From: Ankur Sharma <ankursharma at vmware.com>
> > To: dev at openvswitch.org
> > Subject: [ovs-dev] [PATCH 9/9 v3] datapath-windows: refactor code to
> >        setup   dump start state
> > Message-ID: <1409339114-15838-1-git-send-email-ankursharma at vmware.com>
> > 
> > Per review comment, in this patch we refactor the code to create a
> > OvsSetupDumpStart() which can be leveraged by dump functions in the
> > future. I have not refactored the code that continues the dump
> > operation primarily since it is not final yet. Once the netlink set
> > APIs are in place, we can refactor that too.
> > 
> > Signed-off-by: Nithin Raju <nithin at vmware.com>
> > Signed-off-by: Ankur Sharma <ankursharma at vmware.com>
> > Acked-by: Ankur Sharma <ankursharma at vmware.com>
> > ---
> > Please attribute this to Nithin Raju <nithin at vmware.com>
> > ---
> > datapath-windows/ovsext/Datapath.c | 66 +++++++++++++++++++++++---------------
> > 1 file changed, 40 insertions(+), 26 deletions(-)
> > 
> > diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c
> > index ccb69e3..c145d00 100644
> > --- a/datapath-windows/ovsext/Datapath.c
> > +++ b/datapath-windows/ovsext/Datapath.c
> > @@ -110,8 +110,11 @@ static NTSTATUS OvsGetDpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
> > 
> > /* Netlink control family: this is a Windows specific family. */
> > NETLINK_CMD nlControlFamilyCmdOps[] = {
> > -    { OVS_CTRL_CMD_WIN_GET_PID, OvsGetPidCmdHandler,
> > -      OVS_TRANSACTION_DEV_OP, FALSE }
> > +    { .cmd             = OVS_CTRL_CMD_WIN_GET_PID,
> > +      .handler         = OvsGetPidCmdHandler,
> > +      .supportedDevOp  = OVS_TRANSACTION_DEV_OP,
> > +      .validateDpIndex = FALSE
> > +    }
> > };
> > 
> > NETLINK_FAMILY nlControlFamilyOps = {
> > @@ -125,8 +128,11 @@ NETLINK_FAMILY nlControlFamilyOps = {
> > 
> > /* Netlink datapath family. */
> > NETLINK_CMD nlDatapathFamilyCmdOps[] = {
> > -    { OVS_DP_CMD_GET, OvsGetDpCmdHandler,
> > -      OVS_WRITE_DEV_OP | OVS_READ_DEV_OP, FALSE }
> > +    { .cmd             = OVS_DP_CMD_GET,
> > +      .handler         = OvsGetDpCmdHandler,
> > +      .supportedDevOp  = OVS_WRITE_DEV_OP | OVS_READ_DEV_OP,
> > +      .validateDpIndex = FALSE
> > +    }
> > };
> > 
> > NETLINK_FAMILY nlDatapathFamilyOps = {
> > @@ -182,6 +188,7 @@ static NTSTATUS ValidateNetlinkCmd(UINT32 devOp,
> > static NTSTATUS InvokeNetlinkCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
> >                                         NETLINK_FAMILY *nlFamilyOps,
> >                                         UINT32 *replyLen);
> > +static NTSTATUS OvsSetupDumpStart(POVS_USER_PARAMS_CONTEXT usrParamsCtx);
> > 
> > 
> > /* Handles to the device object for communication with userspace. */
> > @@ -828,28 +835,8 @@ OvsGetDpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
> >         (POVS_OPEN_INSTANCE)usrParamsCtx->ovsInstance;
> > 
> >     if (usrParamsCtx->devOp == OVS_WRITE_DEV_OP) {
> > -        NTSTATUS status;
> > -
> > -        /* input buffer has been validated while validating write dev op. */
> > -        ASSERT(msgIn != NULL && usrParamsCtx->inputLength >= sizeof *msgIn);
> > -
> > -        /* A write operation that does not indicate dump start is invalid. */
> > -        if ((msgIn->nlMsg.nlmsgFlags & NLM_F_DUMP) != NLM_F_DUMP) {
> > -            return STATUS_INVALID_PARAMETER;
> > -        }
> > -        /* XXX: Handle other NLM_F_* flags in the future. */
> > -
> > -        /*
> > -         * This operation should be setting up the dump state. If there's any
> > -         * previous state, clear it up so as to set it up afresh.
> > -         */
> > -        if (instance->dumpState.ovsMsg != NULL) {
> > -            FreeUserDumpState(instance);
> > -        }
> > -        status = InitUserDumpState(instance, msgIn);
> > -        if (status != STATUS_SUCCESS) {
> > -            return STATUS_NO_MEMORY;
> > -        }
> > +        *replyLen = 0;
> > +        OvsSetupDumpStart(usrParamsCtx);
> >     } else {
> >         ASSERT(usrParamsCtx->devOp == OVS_READ_DEV_OP);
> > 
> > @@ -943,6 +930,33 @@ OvsGetDpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
> >     return STATUS_SUCCESS;
> > }
> > 
> > +static NTSTATUS
> > +OvsSetupDumpStart(POVS_USER_PARAMS_CONTEXT usrParamsCtx)
> > +{
> > +    POVS_MESSAGE msgIn = (POVS_MESSAGE)usrParamsCtx->inputBuffer;
> > +    POVS_OPEN_INSTANCE instance =
> > +        (POVS_OPEN_INSTANCE)usrParamsCtx->ovsInstance;
> > +
> > +    /* input buffer has been validated while validating write dev op. */
> > +    ASSERT(msgIn != NULL && usrParamsCtx->inputLength >= sizeof *msgIn);
> > +
> > +    /* A write operation that does not indicate dump start is invalid. */
> > +    if ((msgIn->nlMsg.nlmsgFlags & NLM_F_DUMP) != NLM_F_DUMP) {
> > +        return STATUS_INVALID_PARAMETER;
> > +    }
> > +    /* XXX: Handle other NLM_F_* flags in the future. */
> > +
> > +    /*
> > +     * This operation should be setting up the dump state. If there's any
> > +     * previous state, clear it up so as to set it up afresh.
> > +     */
> > +    if (instance->dumpState.ovsMsg != NULL) {
> > +        FreeUserDumpState(instance);
> > +    }
> > +
> > +    return InitUserDumpState(instance, msgIn);
> > +}
> > +
> > /*
> >  * --------------------------------------------------------------------------
> >  *  Utility function to map the output buffer in an IRP. The buffer is assumed
> > --
> > 1.8.3.2
>