Hi Nithin,
It may be out of the scope of this change but for consistency reason all NL 
function should return BOOLEAN rather than NTSTATUS.
This should apply NlFillOvsMsg() and NlFillNlHdr() too.

    BOOLEAN ok;
+    ok = NlFillOvsMsg(&nlBuffer, msgIn->nlMsg.nlmsgType, NLM_F_MULTI,

But the function signature is 
NTSTATUS NlFillOvsMsg(PNL_BUFFER nlBuf, UINT16 nlmsgType,:

Besides that LG.
Thanks,
Eitan

-----Original Message-----
From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Nithin Raju
Sent: Wednesday, December 03, 2014 7:56 AM
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH 3/3] datapath-windows: refactor 
BuildReplyMsgFromMsgIn & BuildErrorMsg

In this patch, we consolidate code in Netlink.c.

Signed-off-by: Nithin Raju <nit...@vmware.com>
---
 datapath-windows/ovsext/Datapath.c        |    2 +-
 datapath-windows/ovsext/Flow.c            |    4 +-
 datapath-windows/ovsext/Netlink/Netlink.c |   40 ++++++++---------------------
 datapath-windows/ovsext/Netlink/Netlink.h |    6 +---
 datapath-windows/ovsext/User.c            |    2 +-
 datapath-windows/ovsext/Vport.c           |   28 +++++++++-----------
 6 files changed, 30 insertions(+), 52 deletions(-)

diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c
index f6e6e50..af8d818 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -1324,7 +1324,7 @@ cleanup:
         POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)
             usrParamsCtx->outputBuffer;
 
-        BuildErrorMsg(msgIn, msgError, nlError);
+        NlBuildErrorMsg(msgIn, msgError, nlError);
         *replyLen = msgError->nlMsg.nlmsgLen;
     }
 
diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c 
index f47d469..f50bb39 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -355,7 +355,7 @@ done:
     if (nlError != NL_ERROR_SUCCESS) {
         POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)
                                        usrParamsCtx->outputBuffer;
-        BuildErrorMsg(msgIn, msgError, nlError);
+        NlBuildErrorMsg(msgIn, msgError, nlError);
         *replyLen = msgError->nlMsg.nlmsgLen;
         rc = STATUS_SUCCESS;
     }
@@ -387,7 +387,7 @@ OvsFlowNlGetCmdHandler(POVS_USER_PARAMS_CONTEXT 
usrParamsCtx,
             (usrParamsCtx->outputBuffer)) {
             POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)
                                            usrParamsCtx->outputBuffer;
-            BuildErrorMsg(msgIn, msgError, nlError);
+            NlBuildErrorMsg(msgIn, msgError, nlError);
             *replyLen = msgError->nlMsg.nlmsgLen;
             status = STATUS_SUCCESS;
             goto done;
diff --git a/datapath-windows/ovsext/Netlink/Netlink.c 
b/datapath-windows/ovsext/Netlink/Netlink.c
index 7633f2f..0d48d08 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.c
+++ b/datapath-windows/ovsext/Netlink/Netlink.c
@@ -102,41 +102,23 @@ NlFillNlHdr(PNL_BUFFER nlBuf, UINT16 nlmsgType,
     return writeOk ? STATUS_SUCCESS : STATUS_INVALID_BUFFER_SIZE;  }
 
-static VOID
-BuildMsgOut(POVS_MESSAGE msgIn, POVS_MESSAGE msgOut, UINT16 type,
-            UINT32 length, UINT16 flags)
-{
-    msgOut->nlMsg.nlmsgType = type;
-    msgOut->nlMsg.nlmsgFlags = flags;
-    msgOut->nlMsg.nlmsgSeq = msgIn->nlMsg.nlmsgSeq;
-    msgOut->nlMsg.nlmsgPid = msgIn->nlMsg.nlmsgPid;
-    msgOut->nlMsg.nlmsgLen = length;
-
-    msgOut->genlMsg.cmd = msgIn->genlMsg.cmd;
-    msgOut->genlMsg.version = msgIn->genlMsg.version;
-    msgOut->genlMsg.reserved = 0;
-}
-
 /*
- * XXX: should move out these functions to a Netlink.c or to a OvsMessage.c
- * or even make them inlined functions in Datapath.h. Can be done after the
- * first sprint once we have more code to refactor.
+ * 
+ ----------------------------------------------------------------------
+ -----
+ * Prepare a 'OVS_MESSAGE_ERROR' message.
+ * 
+ ----------------------------------------------------------------------
+ -----
  */
 VOID
-BuildReplyMsgFromMsgIn(POVS_MESSAGE msgIn, POVS_MESSAGE msgOut, UINT16 flags)
+NlBuildErrorMsg(POVS_MESSAGE msgIn, POVS_MESSAGE_ERROR msgError, UINT 
+errorCode)
 {
-    BuildMsgOut(msgIn, msgOut, msgIn->nlMsg.nlmsgType, sizeof(OVS_MESSAGE),
-                flags);
-}
+    NL_BUFFER nlBuffer;
 
-VOID
-BuildErrorMsg(POVS_MESSAGE msgIn, POVS_MESSAGE_ERROR msgOut, UINT errorCode) -{
-    BuildMsgOut(msgIn, (POVS_MESSAGE)msgOut, NLMSG_ERROR,
-                sizeof(OVS_MESSAGE_ERROR), 0);
+    NlBufInit(&nlBuffer, (PCHAR)msgError, sizeof *msgError);
+    NlFillNlHdr(&nlBuffer, NLMSG_ERROR, 0,
+                msgIn->nlMsg.nlmsgSeq, msgIn->nlMsg.nlmsgPid);
 
-    msgOut->errorMsg.error = errorCode;
-    msgOut->errorMsg.nlMsg = msgIn->nlMsg;
+    msgError->errorMsg.error = errorCode;
+    msgError->errorMsg.nlMsg = msgIn->nlMsg;
+    msgError->nlMsg.nlmsgLen = sizeof(OVS_MESSAGE_ERROR);
 }
 
 /*
diff --git a/datapath-windows/ovsext/Netlink/Netlink.h 
b/datapath-windows/ovsext/Netlink/Netlink.h
index 18e40b1..a7a53e0 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.h
+++ b/datapath-windows/ovsext/Netlink/Netlink.h
@@ -94,10 +94,8 @@ NTSTATUS NlFillNlHdr(PNL_BUFFER nlBuf,
                      UINT16 nlmsgType, UINT16 nlmsgFlags,
                      UINT32 nlmsgSeq, UINT32 nlmsgPid);
 
-VOID BuildReplyMsgFromMsgIn(POVS_MESSAGE msgIn, POVS_MESSAGE msgOut,
-                            UINT16 flags);
-VOID BuildErrorMsg(POVS_MESSAGE msgIn, POVS_MESSAGE_ERROR msgOut,
-                   UINT errorCode);
+VOID NlBuildErrorMsg(POVS_MESSAGE msgIn, POVS_MESSAGE_ERROR msgOut,
+                     UINT errorCode);
 
 /* Netlink message accessing the payload */  PVOID NlMsgAt(const PNL_MSG_HDR 
nlh, UINT32 offset); diff --git a/datapath-windows/ovsext/User.c 
b/datapath-windows/ovsext/User.c index 8473f98..70091cb 100644
--- a/datapath-windows/ovsext/User.c
+++ b/datapath-windows/ovsext/User.c
@@ -399,7 +399,7 @@ OvsNlExecuteCmdHandler(POVS_USER_PARAMS_CONTEXT 
usrParamsCtx,
 
             POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)
                                            usrParamsCtx->outputBuffer;
-            BuildErrorMsg(msgIn, msgError, nlError);
+            NlBuildErrorMsg(msgIn, msgError, nlError);
             *replyLen = msgError->nlMsg.nlmsgLen;
             status = STATUS_SUCCESS;
             goto done;
diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c 
index 35f95bc..2efb363 100644
--- a/datapath-windows/ovsext/Vport.c
+++ b/datapath-windows/ovsext/Vport.c
@@ -1535,7 +1535,7 @@ cleanup:
         POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)
             usrParamsCtx->outputBuffer;
 
-        BuildErrorMsg(msgIn, msgError, nlError);
+        NlBuildErrorMsg(msgIn, msgError, nlError);
         *replyLen = msgError->nlMsg.nlmsgLen;
     }
 
@@ -1558,16 +1558,15 @@ CreateNetlinkMesgForNetdev(POVS_VPORT_EXT_INFO info,  {
     NL_BUFFER nlBuffer;
     BOOLEAN ok;
-    OVS_MESSAGE msgOut;
     PNL_MSG_HDR nlMsg;
     UINT32 netdevFlags = 0;
 
     NlBufInit(&nlBuffer, outBuffer, outBufLen);
 
-    BuildReplyMsgFromMsgIn(msgIn, &msgOut, 0);
-    msgOut.ovsHdr.dp_ifindex = dpIfIndex;
-
-    ok = NlMsgPutHead(&nlBuffer, (PCHAR)&msgOut, sizeof msgOut);
+    ok = NlFillOvsMsg(&nlBuffer, msgIn->nlMsg.nlmsgType, NLM_F_MULTI,
+                      msgIn->nlMsg.nlmsgSeq, msgIn->nlMsg.nlmsgPid,
+                      msgIn->genlMsg.cmd, msgIn->genlMsg.version,
+                      dpIfIndex) == STATUS_SUCCESS ? TRUE : FALSE;
     if (!ok) {
         return STATUS_INSUFFICIENT_RESOURCES;
     }
@@ -1641,15 +1640,14 @@ OvsCreateMsgFromVport(POVS_VPORT_ENTRY vport,
     NL_BUFFER nlBuffer;
     OVS_VPORT_FULL_STATS vportStats;
     BOOLEAN ok;
-    OVS_MESSAGE msgOut;
     PNL_MSG_HDR nlMsg;
 
     NlBufInit(&nlBuffer, outBuffer, outBufLen);
 
-    BuildReplyMsgFromMsgIn(msgIn, &msgOut, NLM_F_MULTI);
-    msgOut.ovsHdr.dp_ifindex = dpIfIndex;
-
-    ok = NlMsgPutHead(&nlBuffer, (PCHAR)&msgOut, sizeof msgOut);
+    ok = NlFillOvsMsg(&nlBuffer, msgIn->nlMsg.nlmsgType, NLM_F_MULTI,
+                      msgIn->nlMsg.nlmsgSeq, msgIn->nlMsg.nlmsgPid,
+                      msgIn->genlMsg.cmd, msgIn->genlMsg.version,
+                      dpIfIndex) == STATUS_SUCCESS ? TRUE : FALSE;
     if (!ok) {
         return STATUS_INSUFFICIENT_RESOURCES;
     }
@@ -1901,7 +1899,7 @@ Cleanup:
         POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)
             usrParamsCtx->outputBuffer;
 
-        BuildErrorMsg(msgIn, msgError, nlError);
+        NlBuildErrorMsg(msgIn, msgError, nlError);
         *replyLen = msgError->nlMsg.nlmsgLen;
     }
 
@@ -2134,7 +2132,7 @@ Cleanup:
             OvsFreeMemory(vport);
         }
 
-        BuildErrorMsg(msgIn, msgError, nlError);
+        NlBuildErrorMsg(msgIn, msgError, nlError);
         *replyLen = msgError->nlMsg.nlmsgLen;
     }
 
@@ -2246,7 +2244,7 @@ Cleanup:
         POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)
             usrParamsCtx->outputBuffer;
 
-        BuildErrorMsg(msgIn, msgError, nlError);
+        NlBuildErrorMsg(msgIn, msgError, nlError);
         *replyLen = msgError->nlMsg.nlmsgLen;
     }
 
@@ -2330,7 +2328,7 @@ Cleanup:
         POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)
             usrParamsCtx->outputBuffer;
 
-        BuildErrorMsg(msgIn, msgError, nlError);
+        NlBuildErrorMsg(msgIn, msgError, nlError);
         *replyLen = msgError->nlMsg.nlmsgLen;
     }
 
--
1.7.4.1

_______________________________________________
dev mailing list
dev@openvswitch.org
https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailman_listinfo_dev&d=AAIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=uV1ziCRTbQ2ywCwaGRIn51SpQrftiGFTfg1j9QMLsO8&s=jTGQSYmDNTQmSZGjG19ETB8jww9uXdQyc8w5d6F6tRY&e=
 
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to