diff mbox

[ovs-dev,3/4,v2] datapath-windows: return netlink error for read operation

Message ID 1443024933-47622-3-git-send-email-nithin@vmware.com
State Accepted
Headers show

Commit Message

Nithin Raju Sept. 23, 2015, 4:15 p.m. UTC 
The kernel datapath returns a NL error message upon any errors
during read operations, and returns STATUS_SUCCESS as the return
code. We reply on the input NL request to get the family ID, and the
PID. However, when the request is of type OVS_CTRL_CMD_EVENT_NOTIFY
and OVS_CTRL_CMD_READ_NOTIFY, there's no input buffer associated
with the request. So, we use a temporary input buffer to be able to
call the Netlink APIs for constructing the output NL error message.

Signed-off-by: Nithin Raju <nithin@vmware.com>
Acked-by: Sairam Venugopal <vsairam@vmware.com>
---
v2: addressed ALin's review comments
---
 datapath-windows/ovsext/Datapath.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Comments

Alin Serdean Sept. 23, 2015, 5:57 p.m. UTC | #1 
Acked-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com>


> -----Mesaj original-----

> De la: dev [mailto:dev-bounces@openvswitch.org] În numele Nithin Raju

> Trimis: Wednesday, September 23, 2015 7:16 PM

> Către: dev@openvswitch.org

> Subiect: [ovs-dev] [PATCH 3/4 v2] datapath-windows: return netlink error for

> read operation

> 

> The kernel datapath returns a NL error message upon any errors during read

> operations, and returns STATUS_SUCCESS as the return code. We reply on

> the input NL request to get the family ID, and the PID. However, when the

> request is of type OVS_CTRL_CMD_EVENT_NOTIFY and

> OVS_CTRL_CMD_READ_NOTIFY, there's no input buffer associated with the

> request. So, we use a temporary input buffer to be able to call the Netlink

> APIs for constructing the output NL error message.

> 

> Signed-off-by: Nithin Raju <nithin@vmware.com>

> Acked-by: Sairam Venugopal <vsairam@vmware.com>

> ---

> v2: addressed ALin's review comments

> ---

>  datapath-windows/ovsext/Datapath.c | 16 +++++++++++++++-

>  1 file changed, 15 insertions(+), 1 deletion(-)

> 

> diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-

> windows/ovsext/Datapath.c

> index 1e16604..b3dbd71 100644

> --- a/datapath-windows/ovsext/Datapath.c

> +++ b/datapath-windows/ovsext/Datapath.c

> @@ -1020,10 +1020,24 @@

> InvokeNetlinkCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,

>      if (status != STATUS_SUCCESS && status != STATUS_PENDING) {

>          if (usrParamsCtx->devOp != OVS_WRITE_DEV_OP && *replyLen == 0) {

>              NL_ERROR nlError = NlMapStatusToNlErr(status);

> -            POVS_MESSAGE msgIn = (POVS_MESSAGE)usrParamsCtx-

> >inputBuffer;

> +            OVS_MESSAGE msgInTmp = { 0 };

> +            POVS_MESSAGE msgIn = NULL;

>              POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)

>                  usrParamsCtx->outputBuffer;

> 

> +            if (usrParamsCtx->ovsMsg->genlMsg.cmd ==

> OVS_CTRL_CMD_EVENT_NOTIFY ||

> +                usrParamsCtx->ovsMsg->genlMsg.cmd ==

> OVS_CTRL_CMD_READ_NOTIFY) {

> +                /* There's no input buffer associated with such requests. */

> +                NL_BUFFER nlBuffer;

> +                msgIn = &msgInTmp;

> +                NlBufInit(&nlBuffer, (PCHAR)msgIn, sizeof *msgIn);

> +                NlFillNlHdr(&nlBuffer, nlFamilyOps->id, 0, 0,

> +                            usrParamsCtx->ovsInstance->pid);

> +            } else {

> +                msgIn = (POVS_MESSAGE)usrParamsCtx->inputBuffer;

> +            }

> +

> +            ASSERT(msgIn);

>              ASSERT(msgError);

>              NlBuildErrorMsg(msgIn, msgError, nlError);

>              *replyLen = msgError->nlMsg.nlmsgLen;

> --

> 1.8.5.6

> 

> _______________________________________________

> dev mailing list

> dev@openvswitch.org

> http://openvswitch.org/mailman/listinfo/dev
Ben Pfaff Sept. 30, 2015, 5:42 a.m. UTC | #2 
Applied, thanks everyone.

On Wed, Sep 23, 2015 at 05:57:51PM +0000, Alin Serdean wrote:
> Acked-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com>
> 
> > -----Mesaj original-----
> > De la: dev [mailto:dev-bounces@openvswitch.org] În numele Nithin Raju
> > Trimis: Wednesday, September 23, 2015 7:16 PM
> > Către: dev@openvswitch.org
> > Subiect: [ovs-dev] [PATCH 3/4 v2] datapath-windows: return netlink error for
> > read operation
> > 
> > The kernel datapath returns a NL error message upon any errors during read
> > operations, and returns STATUS_SUCCESS as the return code. We reply on
> > the input NL request to get the family ID, and the PID. However, when the
> > request is of type OVS_CTRL_CMD_EVENT_NOTIFY and
> > OVS_CTRL_CMD_READ_NOTIFY, there's no input buffer associated with the
> > request. So, we use a temporary input buffer to be able to call the Netlink
> > APIs for constructing the output NL error message.
> > 
> > Signed-off-by: Nithin Raju <nithin@vmware.com>
> > Acked-by: Sairam Venugopal <vsairam@vmware.com>
> > ---
> > v2: addressed ALin's review comments
> > ---
> >  datapath-windows/ovsext/Datapath.c | 16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> > 
> > diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-
> > windows/ovsext/Datapath.c
> > index 1e16604..b3dbd71 100644
> > --- a/datapath-windows/ovsext/Datapath.c
> > +++ b/datapath-windows/ovsext/Datapath.c
> > @@ -1020,10 +1020,24 @@
> > InvokeNetlinkCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
> >      if (status != STATUS_SUCCESS && status != STATUS_PENDING) {
> >          if (usrParamsCtx->devOp != OVS_WRITE_DEV_OP && *replyLen == 0) {
> >              NL_ERROR nlError = NlMapStatusToNlErr(status);
> > -            POVS_MESSAGE msgIn = (POVS_MESSAGE)usrParamsCtx-
> > >inputBuffer;
> > +            OVS_MESSAGE msgInTmp = { 0 };
> > +            POVS_MESSAGE msgIn = NULL;
> >              POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)
> >                  usrParamsCtx->outputBuffer;
> > 
> > +            if (usrParamsCtx->ovsMsg->genlMsg.cmd ==
> > OVS_CTRL_CMD_EVENT_NOTIFY ||
> > +                usrParamsCtx->ovsMsg->genlMsg.cmd ==
> > OVS_CTRL_CMD_READ_NOTIFY) {
> > +                /* There's no input buffer associated with such requests. */
> > +                NL_BUFFER nlBuffer;
> > +                msgIn = &msgInTmp;
> > +                NlBufInit(&nlBuffer, (PCHAR)msgIn, sizeof *msgIn);
> > +                NlFillNlHdr(&nlBuffer, nlFamilyOps->id, 0, 0,
> > +                            usrParamsCtx->ovsInstance->pid);
> > +            } else {
> > +                msgIn = (POVS_MESSAGE)usrParamsCtx->inputBuffer;
> > +            }
> > +
> > +            ASSERT(msgIn);
> >              ASSERT(msgError);
> >              NlBuildErrorMsg(msgIn, msgError, nlError);
> >              *replyLen = msgError->nlMsg.nlmsgLen;
> > --
> > 1.8.5.6
> > 
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
diff mbox

Patch

diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c
index 1e16604..b3dbd71 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -1020,10 +1020,24 @@  InvokeNetlinkCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
     if (status != STATUS_SUCCESS && status != STATUS_PENDING) {
         if (usrParamsCtx->devOp != OVS_WRITE_DEV_OP && *replyLen == 0) {
             NL_ERROR nlError = NlMapStatusToNlErr(status);
-            POVS_MESSAGE msgIn = (POVS_MESSAGE)usrParamsCtx->inputBuffer;
+            OVS_MESSAGE msgInTmp = { 0 };
+            POVS_MESSAGE msgIn = NULL;
             POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)
                 usrParamsCtx->outputBuffer;
 
+            if (usrParamsCtx->ovsMsg->genlMsg.cmd == OVS_CTRL_CMD_EVENT_NOTIFY ||
+                usrParamsCtx->ovsMsg->genlMsg.cmd == OVS_CTRL_CMD_READ_NOTIFY) {
+                /* There's no input buffer associated with such requests. */
+                NL_BUFFER nlBuffer;
+                msgIn = &msgInTmp;
+                NlBufInit(&nlBuffer, (PCHAR)msgIn, sizeof *msgIn);
+                NlFillNlHdr(&nlBuffer, nlFamilyOps->id, 0, 0,
+                            usrParamsCtx->ovsInstance->pid);
+            } else {
+                msgIn = (POVS_MESSAGE)usrParamsCtx->inputBuffer;
+            }
+
+            ASSERT(msgIn);
             ASSERT(msgError);
             NlBuildErrorMsg(msgIn, msgError, nlError);
             *replyLen = msgError->nlMsg.nlmsgLen;