[ovs-dev] [PATCH] WMI Script that updates Hyper-V friendly port names

2014-09-25 Thread Alin Serdean
The following script leverage's the advantages of WMI infrastructure
offered in Hyper-V.

This scripts allows the user to change the
Msvm_EthernetPortAllocationSettingData property of a VM network adapter
connected to a Hyper-V Virtual Switch.

Usage:
import-module .\OVS.psm1
$vnic = Get-VMNetworkAdapter VM1
Connect-VMNetworkAdapter -VMNetworkAdapter $vnic -SwitchName external
$vnic | Set-VMNetworkAdapterOVSPort -OVSPortName ovs-port-1

VM1 - is a VM on top of a Hyper-V
external - is a Hyper-V Virtual Switch

Signed-off-by: Alessandro Pilotti apilo...@cloudbasesolutions.com
Signed-off-by: Alin Gabriel Serdean aserd...@cloudbasesolutions.com
---
 datapath-windows/misc/OVS.psm1 | 76 ++
 1 file changed, 76 insertions(+)
 create mode 100644 datapath-windows/misc/OVS.psm1

diff --git a/datapath-windows/misc/OVS.psm1 b/datapath-windows/misc/OVS.psm1
new file mode 100644
index 000..52ed3ba
--- /dev/null
+++ b/datapath-windows/misc/OVS.psm1
@@ -0,0 +1,76 @@
+#
+Copyright 2014 Cloudbase Solutions Srl
+
+Licensed under the Apache License, Version 2.0 (the License);
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an AS IS BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+#
+
+$hvassembly = 
[System.Reflection.Assembly]::LoadWithPartialName(Microsoft.HyperV.PowerShell)
+
+function Set-VMNetworkAdapterOVSPort
+{
+[CmdletBinding()]
+param
+(
+[parameter(Mandatory=$true, ValueFromPipeline=$true)]
+[Microsoft.HyperV.PowerShell.VMNetworkAdapter]$VMNetworkAdapter,
+
+[parameter(Mandatory=$true)]
+[string]$OVSPortName
+)
+process
+{
+$ns = root\virtualization\v2
+$EscapedId = $VMNetworkAdapter.Id.Replace('\', '\\')
+$sd = gwmi -namespace $ns -class 
Msvm_EthernetPortAllocationSettingData -Filter InstanceId like '$EscapedId%'
+
+if($sd)
+{
+$sd.ElementName = $OVSPortName
+
+$vsms = gwmi -namespace $ns -class 
Msvm_VirtualSystemManagementService
+$retVal = $vsms.ModifyResourceSettings(@($sd.GetText(1)))
+try
+{
+Check-WMIReturnValue $retVal
+}
+catch
+{
+throw Assigning OVS port '$OVSPortName' failed
+}
+}
+}
+}
+
+function Check-WMIReturnValue($retVal)
+{
+if ($retVal.ReturnValue -ne 0)
+{
+if ($retVal.ReturnValue -eq 4096)
+{
+do
+{
+$job = [wmi]$retVal.Job
+}
+while ($job.JobState -eq 4)
+
+if ($job.JobState -ne 7)
+{
+throw Job Failed
+}
+}
+else
+{
+throw Job Failed
+}
+}
+}
-- 
1.9.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH v2 01/10] datapath-windows: move OVS_MESSAGE to Netlink.h

2014-09-25 Thread Ankur Sharma
Moved the structure OVS_MESSAGE to Netlink.h.
This change is done for following reasons.

a. Patch 2 in this series provides a generic API in Netlink.c
for creating netlink message. That API needs OVS_MESSAGE.
Including Datapath.h in Netlink.c/h gives compilation error.

b. OVS_MESSAGE defines netlink messages hence moving it to
Netlink.h looked fine to me.

Signed-off-by: Ankur Sharma ankursha...@vmware.com
Acked-by: Alin Gabriel Serdean aserd...@cloudbasesolutions.com
Acked-by: Eitan Eliahu elia...@vmware.com
---
 datapath-windows/ovsext/Datapath.h| 10 --
 datapath-windows/ovsext/Netlink/Netlink.h | 11 +++
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/datapath-windows/ovsext/Datapath.h 
b/datapath-windows/ovsext/Datapath.h
index 4e6c9b1..f77c1e2 100644
--- a/datapath-windows/ovsext/Datapath.h
+++ b/datapath-windows/ovsext/Datapath.h
@@ -28,16 +28,6 @@
 #ifndef __DATAPATH_H_
 #define __DATAPATH_H_ 1
 
-/*
- * Structure of any message passed between userspace and kernel.
- */
-typedef struct _OVS_MESSAGE {
-NL_MSG_HDR nlMsg;
-GENL_MSG_HDR genlMsg;
-OVS_HDR ovsHdr;
-/* Variable length nl_attrs follow. */
-} OVS_MESSAGE, *POVS_MESSAGE;
-
 typedef struct _OVS_DEVICE_EXTENSION {
 INT numberOpenInstance;
 INT pidCount;
diff --git a/datapath-windows/ovsext/Netlink/Netlink.h 
b/datapath-windows/ovsext/Netlink/Netlink.h
index 6ecbdc5..b036723 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.h
+++ b/datapath-windows/ovsext/Netlink/Netlink.h
@@ -20,6 +20,17 @@
 #include Types.h
 #include NetlinkProto.h
 #include NetlinkBuf.h
+#include ..\..\include\OvsDpInterface.h
+
+/*
+ * Structure of any message passed between userspace and kernel.
+ */
+typedef struct _OVS_MESSAGE {
+NL_MSG_HDR nlMsg;
+GENL_MSG_HDR genlMsg;
+OVS_HDR ovsHdr;
+/* Variable length nl_attrs follow. */
+} OVS_MESSAGE, *POVS_MESSAGE;
 
 /* Netlink attribute types. */
 typedef enum
-- 
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH v2 02/10] datapath-windows/Netlink: Add NlFillOvsMsg API for creating Netlink message headers.

2014-09-25 Thread Ankur Sharma
Added NlFillOvsMsg API in Netlink.c This API will be used to populate
netlink message headers.

Signed-off-by: Ankur Sharma ankursha...@vmware.com
Acked-by: Alin Gabriel Serdean aserd...@cloudbasesolutions.com
Acked-by: Eitan Eliahu elia...@vmware.com
---
 datapath-windows/ovsext/Netlink/Netlink.c | 38 +++
 datapath-windows/ovsext/Netlink/Netlink.h |  6 +
 2 files changed, 44 insertions(+)

diff --git a/datapath-windows/ovsext/Netlink/Netlink.c 
b/datapath-windows/ovsext/Netlink/Netlink.c
index c286c2f..a2829a5 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.c
+++ b/datapath-windows/ovsext/Netlink/Netlink.c
@@ -34,6 +34,44 @@
 
 /*
  * ---
+ * Prepare netlink message headers. Attributes should be added by caller.
+ * ---
+ */
+NTSTATUS
+NlFillOvsMsg(POVS_MESSAGE msgOut, PNL_BUFFER nlBuf,
+ UINT16 nlmsgType, UINT16 nlmsgFlags,
+ UINT32 nlmsgSeq, UINT32 nlmsgPid,
+ UINT8 genlCmd, UINT8 genlVer, UINT32 dpNo)
+{
+BOOLEAN writeOk;
+PNL_MSG_HDR nlMsg;
+
+/* XXX: Add API for nlBuf-bufRemLen. */
+ASSERT(NlBufAt(nlBuf, 0, 0) != 0 
+   nlBuf-bufRemLen = sizeof (struct _OVS_MESSAGE));
+
+msgOut-nlMsg.nlmsgType = nlmsgType;
+msgOut-nlMsg.nlmsgFlags = nlmsgFlags;
+msgOut-nlMsg.nlmsgSeq = nlmsgSeq;
+msgOut-nlMsg.nlmsgPid = nlmsgPid;
+
+msgOut-genlMsg.cmd = genlCmd;
+msgOut-genlMsg.version = genlVer;
+msgOut-genlMsg.reserved = 0;
+
+msgOut-ovsHdr.dp_ifindex = dpNo;
+
+writeOk = NlMsgPutHead(nlBuf, (PCHAR)msgOut,
+   sizeof (struct _OVS_MESSAGE));
+
+nlMsg = (PNL_MSG_HDR)NlBufAt(nlBuf, 0, 0);
+nlMsg-nlmsgLen = NlBufSize(nlBuf);
+
+return writeOk ? STATUS_SUCCESS : STATUS_INVALID_BUFFER_SIZE;
+}
+
+/*
+ * ---
  * Adds Netlink Header to the NL_BUF.
  * ---
  */
diff --git a/datapath-windows/ovsext/Netlink/Netlink.h 
b/datapath-windows/ovsext/Netlink/Netlink.h
index b036723..774faf4 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.h
+++ b/datapath-windows/ovsext/Netlink/Netlink.h
@@ -78,6 +78,12 @@ typedef struct _NL_POLICY
 #define NL_ATTR_GET_AS(NLA, TYPE) \
 (*(TYPE*) NlAttrGetUnspec(nla, sizeof(TYPE)))
 
+NTSTATUS
+NlFillOvsMsg(POVS_MESSAGE msgOut, PNL_BUFFER nlBuf,
+ UINT16 nlmsgType, UINT16 nlmsgFlags,
+ UINT32 nlmsgSeq, UINT32 nlmsgPid,
+ UINT8 genlCmd, UINT8 genlVer, UINT32 dpNo);
+
 /* Netlink message accessing the payload */
 PVOID NlMsgAt(const PNL_MSG_HDR nlh, UINT32 offset);
 UINT32 NlMsgSize(const PNL_MSG_HDR nlh);
-- 
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH v2 03/10] datapath-windows/Netlink: Added NlAttrLen API

2014-09-25 Thread Ankur Sharma
Added an API to retrieve the attribute length.
Added 2 more API for BE16 and BE8 attribute parsing.
Fixed a trailing whitespace issue.

Signed-off-by: Ankur Sharma ankursha...@vmware.com
Acked-by: Alin Gabriel Serdean aserd...@cloudbasesolutions.com
Acked-by: Eitan Eliahu elia...@vmware.com
---
 datapath-windows/ovsext/Netlink/Netlink.c | 28 +++-
 datapath-windows/ovsext/Netlink/Netlink.h | 13 -
 datapath-windows/ovsext/Types.h   |  1 +
 3 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/datapath-windows/ovsext/Netlink/Netlink.c 
b/datapath-windows/ovsext/Netlink/Netlink.c
index a2829a5..8f3b7ae 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.c
+++ b/datapath-windows/ovsext/Netlink/Netlink.c
@@ -585,7 +585,7 @@ NlMsgAttrs(const PNL_MSG_HDR nlh)
  * Returns size of to nlmsg attributes.
  * ---
  */
-INT
+UINT32
 NlMsgAttrLen(const PNL_MSG_HDR nlh)
 {
 return NlMsgPayloadLen(nlh) - GENL_HDRLEN - OVS_HDRLEN;
@@ -788,6 +788,32 @@ NlAttrGetBe32(const PNL_ATTR nla)
 
 /*
  * ---
+ * Returns the 16-bit network byte order value in 'nla''s payload.
+ *
+ * Asserts that 'nla''s payload is at least 2 bytes long.
+ * ---
+ */
+BE16
+NlAttrGetBe16(const PNL_ATTR nla)
+{
+return NL_ATTR_GET_AS(nla, BE16);
+}
+
+/*
+ * ---
+ * Returns the 8-bit network byte order value in 'nla''s payload.
+ *
+ * Asserts that 'nla''s payload is at least 1 byte long.
+ * ---
+ */
+BE8
+NlAttrGetBe8(const PNL_ATTR nla)
+{
+return NL_ATTR_GET_AS(nla, BE8);
+}
+
+/*
+ * ---
  * Returns the 8-bit value in 'nla''s payload.
  * ---
  */
diff --git a/datapath-windows/ovsext/Netlink/Netlink.h 
b/datapath-windows/ovsext/Netlink/Netlink.h
index 774faf4..6494a59 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.h
+++ b/datapath-windows/ovsext/Netlink/Netlink.h
@@ -90,7 +90,7 @@ UINT32 NlMsgSize(const PNL_MSG_HDR nlh);
 PCHAR NlMsgPayload(const PNL_MSG_HDR nlh);
 UINT32 NlMsgPayloadLen(const PNL_MSG_HDR nlh);
 PNL_ATTR NlMsgAttrs(const PNL_MSG_HDR nlh);
-INT NlMsgAttrLen(const PNL_MSG_HDR nlh);
+UINT32 NlMsgAttrLen(const PNL_MSG_HDR nlh);
 
 /* Netlink message parse */
 PNL_MSG_HDR NlMsgNext(const PNL_MSG_HDR nlh);
@@ -122,6 +122,17 @@ BOOLEAN NlAttrParse(const PNL_MSG_HDR nlMsg, UINT32 
attrOffset,
 BOOLEAN NlParseNested(const PNL_ATTR, const NL_POLICY policy[],
   PNL_ATTR attrs[], UINT32 n_attrs);
 
+/*
+ * --
+ * Returns the length of attribute.
+ * --
+ */
+static __inline UINT16
+NlAttrLen(const PNL_ATTR nla)
+{
+return nla-nlaLen;
+}
+
 /* Netlink attribute validation */
 BOOLEAN NlAttrValidate(const PNL_ATTR, const PNL_POLICY);
 
diff --git a/datapath-windows/ovsext/Types.h b/datapath-windows/ovsext/Types.h
index b2ef48c..5d2744b 100644
--- a/datapath-windows/ovsext/Types.h
+++ b/datapath-windows/ovsext/Types.h
@@ -31,6 +31,7 @@ typedef uint8 __u8;
 
 /* Defines the  userspace specific data types for file
  * included within kernel only. */
+typedef UINT8 BE8;
 typedef UINT16 BE16;
 typedef UINT32 BE32;
 typedef UINT64 BE64;
-- 
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH v2 06/10] datapath-windows/Flow.c : Basic support for add-flow.

2014-09-25 Thread Ankur Sharma
This patch covers basic changes in registering add flow handler.
And declaring FLOW related attribute parsing policies.

Signed-off-by: Ankur Sharma ankursha...@vmware.com
Acked-by: Alin Gabriel Serdean aserd...@cloudbasesolutions.com
Acked-by: Eitan Eliahu elia...@vmware.com
---
 datapath-windows/ovsext/Datapath.c |  18 -
 datapath-windows/ovsext/Flow.c | 152 +
 datapath-windows/ovsext/Flow.h |   5 ++
 3 files changed, 171 insertions(+), 4 deletions(-)

diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c
index ffb7d44..9934a9a 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -191,14 +191,22 @@ NETLINK_FAMILY nlVportFamilyOps = {
 };
 
 /* Netlink flow family. */
-/* XXX: Add commands here. */
+
+NETLINK_CMD nlFlowFamilyCmdOps[] = {
+{ .cmd  = OVS_FLOW_CMD_NEW,
+  .handler  = OvsFlowNlNewCmdHandler,
+  .supportedDevOp   = OVS_TRANSACTION_DEV_OP,
+  .validateDpIndex  = FALSE
+}
+};
+
 NETLINK_FAMILY nlFLowFamilyOps = {
 .name = OVS_FLOW_FAMILY,
 .id   = OVS_WIN_NL_FLOW_FAMILY_ID,
 .version  = OVS_FLOW_VERSION,
 .maxAttr  = OVS_FLOW_ATTR_MAX,
-.cmds = NULL, /* XXX: placeholder. */
-.opsCount = 0
+.cmds = nlFlowFamilyCmdOps,
+.opsCount = ARRAY_SIZE(nlFlowFamilyCmdOps)
 };
 
 static NTSTATUS MapIrpOutputBuffer(PIRP irp,
@@ -689,8 +697,10 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject,
 case OVS_WIN_NL_DATAPATH_FAMILY_ID:
 nlFamilyOps = nlDatapathFamilyOps;
 break;
-case OVS_WIN_NL_PACKET_FAMILY_ID:
 case OVS_WIN_NL_FLOW_FAMILY_ID:
+nlFamilyOps = nlFLowFamilyOps;
+break;
+case OVS_WIN_NL_PACKET_FAMILY_ID:
 case OVS_WIN_NL_VPORT_FAMILY_ID:
 status = STATUS_NOT_IMPLEMENTED;
 goto done;
diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
index dae1dca..25b39c1 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -51,6 +51,158 @@ static VOID __inline *GetStartAddrNBL(const NET_BUFFER_LIST 
*_pNB);
 #define OVS_FLOW_TABLE_MASK (OVS_FLOW_TABLE_SIZE -1)
 #define HASH_BUCKET(hash) ((hash)  OVS_FLOW_TABLE_MASK)
 
+/* Flow family related netlink policies */
+
+/* For Parsing attributes in FLOW_* commands */
+static const NL_POLICY nlFlowPolicy[] = {
+[OVS_FLOW_ATTR_KEY] = {.type = NL_A_NESTED, .optional = TRUE},
+[OVS_FLOW_ATTR_MASK] = {.type = NL_A_NESTED, .optional = TRUE},
+[OVS_FLOW_ATTR_ACTIONS] = {.type = NL_A_NESTED, .optional = TRUE},
+[OVS_FLOW_ATTR_STATS] = {.type = NL_A_UNSPEC,
+ .minLen = sizeof(struct ovs_flow_stats),
+ .maxLen = sizeof(struct ovs_flow_stats),
+ .optional = TRUE},
+[OVS_FLOW_ATTR_TCP_FLAGS] = {NL_A_U8, .optional = TRUE},
+[OVS_FLOW_ATTR_USED] = {NL_A_U64, .optional = TRUE}
+};
+
+/* For Parsing nested OVS_FLOW_ATTR_KEY attributes */
+static const NL_POLICY nlFlowKeyPolicy[] = {
+[OVS_KEY_ATTR_ENCAP] = {.type = NL_A_VAR_LEN, .optional = TRUE},
+[OVS_KEY_ATTR_PRIORITY] = {.type = NL_A_UNSPEC, .minLen = 4,
+   .maxLen = 4, .optional = TRUE},
+[OVS_KEY_ATTR_IN_PORT] = {.type = NL_A_UNSPEC, .minLen = 4,
+  .maxLen = 4, .optional = TRUE},
+[OVS_KEY_ATTR_ETHERNET] = {.type = NL_A_UNSPEC,
+   .minLen = sizeof(struct ovs_key_ethernet),
+   .maxLen = sizeof(struct ovs_key_ethernet),
+   .optional = TRUE},
+[OVS_KEY_ATTR_VLAN] = {.type = NL_A_UNSPEC, .minLen = 2,
+   .maxLen = 2, .optional = TRUE},
+[OVS_KEY_ATTR_ETHERTYPE] = {.type = NL_A_UNSPEC, .minLen = 2,
+.maxLen = 2, .optional = TRUE},
+[OVS_KEY_ATTR_IPV4] = {.type = NL_A_UNSPEC,
+   .minLen = sizeof(struct ovs_key_ipv4),
+   .maxLen = sizeof(struct ovs_key_ipv4),
+   .optional = TRUE},
+[OVS_KEY_ATTR_IPV6] = {.type = NL_A_UNSPEC,
+   .minLen = sizeof(struct ovs_key_ipv6),
+   .maxLen = sizeof(struct ovs_key_ipv6),
+   .optional = TRUE},
+[OVS_KEY_ATTR_TCP] = {.type = NL_A_UNSPEC,
+  .minLen = sizeof(struct ovs_key_tcp),
+  .maxLen = sizeof(struct ovs_key_tcp),
+  .optional = TRUE},
+[OVS_KEY_ATTR_UDP] = {.type = NL_A_UNSPEC,
+  .minLen = sizeof(struct ovs_key_udp),
+  .maxLen = sizeof(struct ovs_key_udp),
+  .optional = TRUE},
+[OVS_KEY_ATTR_ICMP] = {.type = NL_A_UNSPEC,
+   .minLen = sizeof(struct ovs_key_icmp),
+   .maxLen = sizeof(struct 

[ovs-dev] [PATCH v2 05/10] datapath-windows/Netlink: Fixed NlAttrParseNested

2014-09-25 Thread Ankur Sharma
NlAttrParseNested was using the whole netlink payload for iteration.
This is not correct, as it would lead to exceeding the
nested attribute boundries. Fixed the same in this patch.

Signed-off-by: Ankur Sharma ankursha...@vmware.com
Acked-by: Alin Gabriel Serdean aserd...@cloudbasesolutions.com
Acked-by: Eitan Eliahu elia...@vmware.com
---
 datapath-windows/ovsext/Datapath.c|  4 +++-
 datapath-windows/ovsext/Netlink/Netlink.c | 15 ---
 datapath-windows/ovsext/Netlink/Netlink.h |  8 
 3 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c
index 0dfdd57..ffb7d44 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -949,7 +949,8 @@ OvsSubscribeEventCmdHandler(POVS_USER_PARAMS_CONTEXT 
usrParamsCtx,
 (POVS_OPEN_INSTANCE)usrParamsCtx-ovsInstance;
 POVS_MESSAGE msgIn = (POVS_MESSAGE)usrParamsCtx-inputBuffer;
 
-rc = NlAttrParse(msgIn-nlMsg, sizeof (*msgIn),policy, attrs, 2);
+rc = NlAttrParse(msgIn-nlMsg, sizeof (*msgIn),
+ NlMsgAttrLen((PNL_MSG_HDR)msgIn), policy, attrs, 2);
 if (!rc) {
 status = STATUS_INVALID_PARAMETER;
 goto done;
@@ -1107,6 +1108,7 @@ HandleDpTransaction(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
 if (usrParamsCtx-ovsMsg-genlMsg.cmd == OVS_DP_CMD_SET) {
 if (!NlAttrParse((PNL_MSG_HDR)msgIn,
 NLMSG_HDRLEN + GENL_HDRLEN + OVS_HDRLEN,
+NlMsgAttrLen((PNL_MSG_HDR)msgIn),
 ovsDatapathSetPolicy, dpAttrs, ARRAY_SIZE(dpAttrs))) {
 return STATUS_INVALID_PARAMETER;
 }
diff --git a/datapath-windows/ovsext/Netlink/Netlink.c 
b/datapath-windows/ovsext/Netlink/Netlink.c
index bb3d603..5bac4b5 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.c
+++ b/datapath-windows/ovsext/Netlink/Netlink.c
@@ -969,6 +969,7 @@ NlAttrFindNested(const PNL_ATTR nla, UINT16 type)
  */
 BOOLEAN
 NlAttrParse(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
+UINT32 attrLen,
 const NL_POLICY policy[],
 PNL_ATTR attrs[], UINT32 n_attrs)
 {
@@ -979,14 +980,21 @@ NlAttrParse(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
 
 RtlZeroMemory(attrs, n_attrs * sizeof *attrs);
 
-if ((NlMsgSize(nlMsg)  attrOffset) || (!(NlMsgAttrLen(nlMsg {
+
+/* There is nothing to parse */
+if (!(NlMsgAttrLen(nlMsg))) {
+ret = TRUE;
+goto done;
+}
+
+if ((NlMsgSize(nlMsg)  attrOffset)) {
 OVS_LOG_WARN(No attributes in nlMsg: %p at offset: %d,
  nlMsg, attrOffset);
 goto done;
 }
 
 NL_ATTR_FOR_EACH (nla, left, NlMsgAt(nlMsg, attrOffset),
-  NlMsgSize(nlMsg) - attrOffset)
+  attrLen)
 {
 UINT16 type = NlAttrType(nla);
 if (type  n_attrs  policy[type].type != NL_A_NO_ATTR) {
@@ -1035,9 +1043,10 @@ done:
  */
 BOOLEAN
 NlAttrParseNested(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
+  UINT32 attrLen,
   const NL_POLICY policy[],
   PNL_ATTR attrs[], UINT32 n_attrs)
 {
 return NlAttrParse(nlMsg, attrOffset + NLA_HDRLEN,
-   policy, attrs, n_attrs);
+   attrLen - NLA_HDRLEN, policy, attrs, n_attrs);
 }
diff --git a/datapath-windows/ovsext/Netlink/Netlink.h 
b/datapath-windows/ovsext/Netlink/Netlink.h
index 6494a59..57fc15f 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.h
+++ b/datapath-windows/ovsext/Netlink/Netlink.h
@@ -117,11 +117,11 @@ const PNL_ATTR NlAttrFind__(const PNL_ATTR attrs,
 const PNL_ATTR NlAttrFindNested(const PNL_ATTR nla,
 UINT16 type);
 BOOLEAN NlAttrParse(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
-const NL_POLICY policy[],
+UINT32 attrLen, const NL_POLICY policy[],
 PNL_ATTR attrs[], UINT32 n_attrs);
-BOOLEAN NlParseNested(const PNL_ATTR, const NL_POLICY policy[],
-  PNL_ATTR attrs[], UINT32 n_attrs);
-
+BOOLEAN NlAttrParseNested(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
+  UINT32 attrLen, const NL_POLICY policy[],
+  PNL_ATTR attrs[], UINT32 n_attrs);
 /*
  * --
  * Returns the length of attribute.
-- 
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH v2 04/10] datapath-windows/Netlink: Allow support for NESTED Attributes in NlAttrValidate

2014-09-25 Thread Ankur Sharma
Signed-off-by: Ankur Sharma ankursha...@vmware.com
Acked-by: Alin Gabriel Serdean aserd...@cloudbasesolutions.com
Acked-by: Eitan Eliahu elia...@vmware.com

---
 datapath-windows/ovsext/Netlink/Netlink.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/datapath-windows/ovsext/Netlink/Netlink.c 
b/datapath-windows/ovsext/Netlink/Netlink.c
index 8f3b7ae..bb3d603 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.c
+++ b/datapath-windows/ovsext/Netlink/Netlink.c
@@ -870,12 +870,13 @@ NlAttrValidate(const PNL_ATTR nla, const PNL_POLICY 
policy)
 UINT32 minLen;
 UINT32 maxLen;
 UINT32 len;
-BOOLEAN ret = TRUE;
+BOOLEAN ret = FALSE;
 
 if ((policy-type == NL_A_NO_ATTR) ||
-(policy-type == NL_A_VAR_LEN)) {
+(policy-type == NL_A_VAR_LEN) ||
+(policy-type == NL_A_NESTED)) {
 /* Do not validate anything for attributes of type var length */
-ret = FALSE;
+ret = TRUE;
 goto done;
 }
 
@@ -894,7 +895,6 @@ NlAttrValidate(const PNL_ATTR nla, const PNL_POLICY policy)
 if (len  minLen || len  maxLen) {
 OVS_LOG_WARN(Attribute: %p, len: %d, not in valid range, 
  min: %d, max: %d, nla, len, minLen, maxLen);
-ret = FALSE;
 goto done;
 }
 
@@ -902,17 +902,17 @@ NlAttrValidate(const PNL_ATTR nla, const PNL_POLICY 
policy)
 if (policy-type == NL_A_STRING) {
 if (((PCHAR) nla)[nla-nlaLen - 1]) {
 OVS_LOG_WARN(Attributes %p lacks null at the end, nla);
-ret = FALSE;
 goto done;
 }
 
 if (memchr(nla + 1, '\0', len - 1) != NULL) {
 OVS_LOG_WARN(Attributes %p has bad length, nla);
-ret = FALSE;
 goto done;
 }
 }
 
+ret = TRUE;
+
 done:
 return ret;
 }
-- 
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH v2 07/10] datapath-windows/Flow.c: FLOW_NEW command handler.

2014-09-25 Thread Ankur Sharma
This patch covers the changes needed to support FLOW_NEW command.
API _OvsFlowMapNlToFlowPutFlags has a bug, which will be fixed
with the patches for FLOW_DEL.

Signed-off-by: Ankur Sharma ankursha...@vmware.com
Acked-by: Alin Gabriel Serdean aserd...@cloudbasesolutions.com
Acked-by: Eitan Eliahu elia...@vmware.com
---
 datapath-windows/include/OvsPub.h |   2 +-
 datapath-windows/ovsext/Flow.c| 375 +++---
 datapath-windows/ovsext/Flow.h|   6 +-
 3 files changed, 357 insertions(+), 26 deletions(-)

diff --git a/datapath-windows/include/OvsPub.h 
b/datapath-windows/include/OvsPub.h
index 36814c4..fa1d6d4 100644
--- a/datapath-windows/include/OvsPub.h
+++ b/datapath-windows/include/OvsPub.h
@@ -420,7 +420,7 @@ typedef struct OvsFlowPut {
 uint32_t actionsLen;
 OvsFlowKey key;
 uint32_t flags;
-NL_ATTR  actions[0];  /* Variable length indicated by actionsLen. */
+PNL_ATTR  actions;
 } OvsFlowPut;
 
 #define OVS_MIN_PACKET_SIZE 60
diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
index 25b39c1..832cf3e 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -46,6 +46,20 @@ static VOID DeleteAllFlows(OVS_DATAPATH *datapath);
 static NTSTATUS AddFlow(OVS_DATAPATH *datapath, OvsFlow *flow);
 static VOID FreeFlow(OvsFlow *flow);
 static VOID __inline *GetStartAddrNBL(const NET_BUFFER_LIST *_pNB);
+static NTSTATUS _OvsFlowMapNlToFlowPut(POVS_MESSAGE msgIn, PNL_ATTR keyAttr,
+   PNL_ATTR actionAttr,
+   PNL_ATTR flowAttrClear,
+   OvsFlowPut *mappedFlow);
+static VOID _OvsFlowMapKeyAttrToFlowPut(PNL_ATTR *keyAttrs,
+PNL_ATTR *tunnelAttrs,
+OvsFlowPut *mappedFlow);
+
+static VOID _OvsFlowMapTunAttrToFlowPut(PNL_ATTR *keyAttrs,
+PNL_ATTR *tunnelAttrs,
+OvsFlowKey *destKey);
+static VOID _OvsFlowMapNlToFlowPutFlags(PGENL_MSG_HDR genlMsgHdr,
+PNL_ATTR flowAttrClear,
+OvsFlowPut *mappedFlow);
 
 #define OVS_FLOW_TABLE_SIZE 2048
 #define OVS_FLOW_TABLE_MASK (OVS_FLOW_TABLE_SIZE -1)
@@ -191,20 +205,356 @@ static const NL_POLICY nlFlowActionPolicy[] = {
  * Netlink interface for flow commands.
  *
  */
+
+/*
+ *
+ *  OvsFlowNlNewCmdHandler --
+ *Handler for OVS_FLOW_CMD_NEW command.
+ *
+ */
 NTSTATUS
 OvsFlowNlNewCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
UINT32 *replyLen)
 {
 NTSTATUS rc = STATUS_SUCCESS;
+POVS_MESSAGE msgIn = (POVS_MESSAGE)usrParamsCtx-inputBuffer;
+POVS_MESSAGE msgOut = (POVS_MESSAGE)usrParamsCtx-outputBuffer;
+PNL_MSG_HDR nlMsgHdr = (msgIn-nlMsg);
+PGENL_MSG_HDR genlMsgHdr = (msgIn-genlMsg);
+POVS_HDR ovsHdr = (msgIn-ovsHdr);
+PNL_ATTR nlAttrs[__OVS_FLOW_ATTR_MAX];
+UINT32 attrOffset = NLMSG_HDRLEN + GENL_HDRLEN + OVS_HDRLEN;
+OvsFlowPut mappedFlow;
+OvsFlowStats stats;
+struct ovs_flow_stats replyStats;
+
+NL_BUFFER nlBuf;
+
+RtlZeroMemory(mappedFlow, sizeof(OvsFlowPut));
+RtlZeroMemory(stats, sizeof(stats));
+RtlZeroMemory(replyStats, sizeof(replyStats));
+
+*replyLen = 0;
+
+/* Get all the top level Flow attributes */
+if ((NlAttrParse(nlMsgHdr, attrOffset, NlMsgAttrLen(nlMsgHdr),
+ nlFlowPolicy, nlAttrs, ARRAY_SIZE(nlAttrs)))
+ != TRUE) {
+OVS_LOG_ERROR(Attr Parsing failed for msg: %p,
+   nlMsgHdr);
+rc = STATUS_UNSUCCESSFUL;
+goto done;
+}
+
+if ((_OvsFlowMapNlToFlowPut(msgIn, nlAttrs[OVS_FLOW_ATTR_KEY],
+ nlAttrs[OVS_FLOW_ATTR_ACTIONS], nlAttrs[OVS_FLOW_ATTR_CLEAR],
+ mappedFlow))
+!= STATUS_SUCCESS) {
+OVS_LOG_ERROR(Conversion to OvsFlowPut failed);
+goto done;
+}
+
+rc = OvsPutFlowIoctl(mappedFlow, sizeof (struct OvsFlowPut),
+ stats);
+if (rc != STATUS_SUCCESS) {
+OVS_LOG_ERROR(OvsFlowPut failed.);
+goto done;
+}
+
+if (!(usrParamsCtx-outputBuffer)) {
+/* No output buffer */
+OVS_LOG_ERROR(outputBuffer NULL.);
+goto done;
+}
 
-UNREFERENCED_PARAMETER(usrParamsCtx);
-UNREFERENCED_PARAMETER(replyLen);
+replyStats.n_packets = stats.packetCount;
+replyStats.n_bytes = stats.byteCount;
+
+/* So far so good. Prepare the reply for userspace */
+NlBufInit(nlBuf, usrParamsCtx-outputBuffer,
+  usrParamsCtx-outputLength);
+
+/* Prepare nl Msg headers */
+rc = 

[ovs-dev] [PATCH v2 09/10] datapath-windows/Flow.c: FLOW_DEL command handler.

2014-09-25 Thread Ankur Sharma
Registered FLOW_DEL command handler. The same command
handler as FLOW_ADD is good enough to handle FLOW_DEL
case as well with minor changes for checking to action
attribute.

Signed-off-by: Ankur Sharma ankursha...@vmware.com
Acked-by: Alin Gabriel Serdean aserd...@cloudbasesolutions.com
Acked-by: Eitan Eliahu elia...@vmware.com
---
 datapath-windows/ovsext/Datapath.c | 5 +
 datapath-windows/ovsext/Flow.c | 7 +--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c
index 5008aab..5377f09 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -202,6 +202,11 @@ NETLINK_CMD nlFlowFamilyCmdOps[] = {
   .handler  = OvsFlowNlNewCmdHandler,
   .supportedDevOp   = OVS_TRANSACTION_DEV_OP,
   .validateDpIndex  = FALSE
+},
+{ .cmd  = OVS_FLOW_CMD_DEL,
+  .handler  = OvsFlowNlNewCmdHandler,
+  .supportedDevOp   = OVS_TRANSACTION_DEV_OP,
+  .validateDpIndex  = FALSE
 }
 };
 
diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
index 832cf3e..82c1c93 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -349,8 +349,11 @@ _OvsFlowMapNlToFlowPut(POVS_MESSAGE msgIn, PNL_ATTR 
keyAttr,
 mappedFlow);
 
 /* Map the action */
-mappedFlow-actionsLen = NlAttrGetSize(actionAttr);
-mappedFlow-actions = NlAttrGet(actionAttr);
+if (actionAttr) {
+mappedFlow-actionsLen = NlAttrGetSize(actionAttr);
+mappedFlow-actions = NlAttrGet(actionAttr);
+}
+
 mappedFlow-dpNo = ovsHdr-dp_ifindex;
 
 _OvsFlowMapNlToFlowPutFlags(genlMsgHdr, flowAttrClear,
-- 
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH v2 08/10] datapath-windows/Flow.c: FLOW_SET command handler.

2014-09-25 Thread Ankur Sharma
Registered FLOW_SET command handler. The same command
handler as FLOW_ADD is good enough to handle FLOW_SET
case as well.

Signed-off-by: Ankur Sharma ankursha...@vmware.com
Acked-by: Alin Gabriel Serdean aserd...@cloudbasesolutions.com
Acked-by: Eitan Eliahu elia...@vmware.com
---
 datapath-windows/ovsext/Datapath.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c
index 9934a9a..5008aab 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -197,6 +197,11 @@ NETLINK_CMD nlFlowFamilyCmdOps[] = {
   .handler  = OvsFlowNlNewCmdHandler,
   .supportedDevOp   = OVS_TRANSACTION_DEV_OP,
   .validateDpIndex  = FALSE
+},
+{ .cmd  = OVS_FLOW_CMD_SET,
+  .handler  = OvsFlowNlNewCmdHandler,
+  .supportedDevOp   = OVS_TRANSACTION_DEV_OP,
+  .validateDpIndex  = FALSE
 }
 };
 
-- 
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH v2 10/10] datapath-windows/Flow.c: DEL_FLOWS command handler.

2014-09-25 Thread Ankur Sharma
Added changes to handle DEL_FLOWS (FLUSH) scenario.

Signed-off-by: Ankur Sharma ankursha...@vmware.com
Acked-by: Alin Gabriel Serdean aserd...@cloudbasesolutions.com
Acked-by: Eitan Eliahu elia...@vmware.com
---
 datapath-windows/ovsext/Flow.c | 19 ++-
 datapath-windows/ovsext/Flow.h |  2 +-
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
index 82c1c93..e374e3f 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -209,7 +209,8 @@ static const NL_POLICY nlFlowActionPolicy[] = {
 /*
  *
  *  OvsFlowNlNewCmdHandler --
- *Handler for OVS_FLOW_CMD_NEW command.
+ *Handler for OVS_FLOW_CMD_NEW/SET/DEL command.
+ *It also handles FLUSH case (DEL w/o any key in input)
  *
  */
 NTSTATUS
@@ -246,6 +247,13 @@ OvsFlowNlNewCmdHandler(POVS_USER_PARAMS_CONTEXT 
usrParamsCtx,
 goto done;
 }
 
+/* FLOW_DEL command w/o any key input is a flush case. */
+if ((genlMsgHdr-cmd == OVS_FLOW_CMD_DEL) 
+(!(nlAttrs[OVS_FLOW_ATTR_KEY]))) {
+rc = OvsFlushFlowIoctl(ovsHdr-dp_ifindex);
+goto done;
+}
+
 if ((_OvsFlowMapNlToFlowPut(msgIn, nlAttrs[OVS_FLOW_ATTR_KEY],
  nlAttrs[OVS_FLOW_ATTR_ACTIONS], nlAttrs[OVS_FLOW_ATTR_CLEAR],
  mappedFlow))
@@ -1432,19 +1440,12 @@ unlock:
 }
 
 NTSTATUS
-OvsFlushFlowIoctl(PVOID inputBuffer,
-  UINT32 inputLength)
+OvsFlushFlowIoctl(UINT32 dpNo)
 {
 NTSTATUS status = STATUS_SUCCESS;
 OVS_DATAPATH *datapath = NULL;
-UINT32 dpNo;
 LOCK_STATE_EX dpLockState;
 
-if (inputLength != sizeof(UINT32) || inputBuffer == NULL) {
-return STATUS_INFO_LENGTH_MISMATCH;
-}
-
-dpNo = *(UINT32 *)inputBuffer;
 NdisAcquireSpinLock(gOvsCtrlLock);
 if (gOvsSwitchContext == NULL ||
 gOvsSwitchContext-dpNo != dpNo) {
diff --git a/datapath-windows/ovsext/Flow.h b/datapath-windows/ovsext/Flow.h
index e62ba40..10ef62b 100644
--- a/datapath-windows/ovsext/Flow.h
+++ b/datapath-windows/ovsext/Flow.h
@@ -68,7 +68,7 @@ NTSTATUS OvsPutFlowIoctl(PVOID inputBuffer, UINT32 
inputLength,
 NTSTATUS OvsGetFlowIoctl(PVOID inputBuffer, UINT32 inputLength,
  PVOID outputBuffer, UINT32 outputLength,
  UINT32 *replyLen);
-NTSTATUS OvsFlushFlowIoctl(PVOID inputBuffer, UINT32 inputLength);
+NTSTATUS OvsFlushFlowIoctl(UINT32 dpNo);
 
 NTSTATUS OvsFlowNlNewCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
 UINT32 *replyLen);
-- 
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2 01/10] datapath-windows: move OVS_MESSAGE to Netlink.h

2014-09-25 Thread Nithin Raju
On Sep 24, 2014, at 11:56 PM, Ankur Sharma ankursha...@vmware.com
 wrote:

 Moved the structure OVS_MESSAGE to Netlink.h.
 This change is done for following reasons.
 
 a. Patch 2 in this series provides a generic API in Netlink.c
 for creating netlink message. That API needs OVS_MESSAGE.
 Including Datapath.h in Netlink.c/h gives compilation error.
 
 b. OVS_MESSAGE defines netlink messages hence moving it to
 Netlink.h looked fine to me.
 
 Signed-off-by: Ankur Sharma ankursha...@vmware.com
 Acked-by: Alin Gabriel Serdean aserd...@cloudbasesolutions.com
 Acked-by: Eitan Eliahu elia...@vmware.com

Acked-by: Nithin Raju nit...@vmware.com
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2 02/10] datapath-windows/Netlink: Add NlFillOvsMsg API for creating Netlink message headers.

2014-09-25 Thread Nithin Raju
On Sep 24, 2014, at 11:57 PM, Ankur Sharma ankursha...@vmware.com
 wrote:

 Added NlFillOvsMsg API in Netlink.c This API will be used to populate
 netlink message headers.
 
 Signed-off-by: Ankur Sharma ankursha...@vmware.com
 Acked-by: Alin Gabriel Serdean aserd...@cloudbasesolutions.com
 Acked-by: Eitan Eliahu elia...@vmware.com
 ---
 datapath-windows/ovsext/Netlink/Netlink.c | 38 +++
 datapath-windows/ovsext/Netlink/Netlink.h |  6 +
 2 files changed, 44 insertions(+)
 
 diff --git a/datapath-windows/ovsext/Netlink/Netlink.c 
 b/datapath-windows/ovsext/Netlink/Netlink.c
 index c286c2f..a2829a5 100644
 --- a/datapath-windows/ovsext/Netlink/Netlink.c
 +++ b/datapath-windows/ovsext/Netlink/Netlink.c
 @@ -34,6 +34,44 @@
 
 /*
  * ---
 + * Prepare netlink message headers. Attributes should be added by caller.
 + * 
 ---
 + */
 +NTSTATUS
 +NlFillOvsMsg(POVS_MESSAGE msgOut, PNL_BUFFER nlBuf,
 + UINT16 nlmsgType, UINT16 nlmsgFlags,
 + UINT32 nlmsgSeq, UINT32 nlmsgPid,
 + UINT8 genlCmd, UINT8 genlVer, UINT32 dpNo)

We don't need msgOut and nlBuf both as parameters. One of them will suffice.
You can declare a local variable msgOut = NlBufAt(nlBuf, 0, sizeof *msgOut);

LG otherwise.

Acked-by: Nithin Raju nit...@vmware.com

-- Nithin
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2 03/10] datapath-windows/Netlink: Added NlAttrLen API

2014-09-25 Thread Nithin Raju

On Sep 24, 2014, at 11:57 PM, Ankur Sharma ankursha...@vmware.com wrote:

 Added an API to retrieve the attribute length.
 Added 2 more API for BE16 and BE8 attribute parsing.
 Fixed a trailing whitespace issue.
 
 Signed-off-by: Ankur Sharma ankursha...@vmware.com
 Acked-by: Alin Gabriel Serdean aserd...@cloudbasesolutions.com
 Acked-by: Eitan Eliahu elia...@vmware.com

Acked-by: Nithin Raju nit...@vmware.com

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2 04/10] datapath-windows/Netlink: Allow support for NESTED Attributes in NlAttrValidate

2014-09-25 Thread Nithin Raju

On Sep 24, 2014, at 11:57 PM, Ankur Sharma ankursha...@vmware.com wrote:

 Signed-off-by: Ankur Sharma ankursha...@vmware.com
 Acked-by: Alin Gabriel Serdean aserd...@cloudbasesolutions.com
 Acked-by: Eitan Eliahu elia...@vmware.com

Acked-by: Nithin Raju nit...@vmware.com
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2 05/10] datapath-windows/Netlink: Fixed NlAttrParseNested

2014-09-25 Thread Nithin Raju

On Sep 24, 2014, at 11:57 PM, Ankur Sharma ankursha...@vmware.com wrote:

 NlAttrParseNested was using the whole netlink payload for iteration.
 This is not correct, as it would lead to exceeding the
 nested attribute boundries. Fixed the same in this patch.
 
 Signed-off-by: Ankur Sharma ankursha...@vmware.com
 Acked-by: Alin Gabriel Serdean aserd...@cloudbasesolutions.com
 Acked-by: Eitan Eliahu elia...@vmware.com

Acked-by: Nithin Raju nit...@vmware.com

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2 06/10] datapath-windows/Flow.c : Basic support for add-flow.

2014-09-25 Thread Nithin Raju
On Sep 24, 2014, at 11:57 PM, Ankur Sharma ankursha...@vmware.com wrote:

 This patch covers basic changes in registering add flow handler.
 And declaring FLOW related attribute parsing policies.
 
 Signed-off-by: Ankur Sharma ankursha...@vmware.com
 Acked-by: Alin Gabriel Serdean aserd...@cloudbasesolutions.com
 Acked-by: Eitan Eliahu elia...@vmware.com

LG. Only thing is, I'd add a command saying some of the actions/key fields 
related to recirc and MPLS are not supported yet.

Acked-by: Nithin Raju nit...@vmware.com

Thanks,
-- Nithin

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH v2] datapath-windows Event read handler

2014-09-25 Thread Eitan Eliahu
The Read event handler is executed when user mode issues a socket receive on
an MC socket associated with the event queue. A new IOCTL READ command is
used to differentiate between transaction based and packet miss sockets.
An entry for the handler will be added once the Vport family implementation
checked in.
User mode code for setting the socket type will follow

Signed-off-by: Eitan Eliahu elia...@vmware.com
---
 build-aux/extract-odp-netlink-windows-dp-h   |   4 +
 datapath-windows/include/OvsDpInterfaceExt.h |  10 +-
 datapath-windows/ovsext/Datapath.c   | 155 ++-
 datapath-windows/ovsext/Event.c  |  42 
 datapath-windows/ovsext/Event.h  |   2 +
 5 files changed, 210 insertions(+), 3 deletions(-)

diff --git a/build-aux/extract-odp-netlink-windows-dp-h 
b/build-aux/extract-odp-netlink-windows-dp-h
index 70514cb..82d95f1 100755
--- a/build-aux/extract-odp-netlink-windows-dp-h
+++ b/build-aux/extract-odp-netlink-windows-dp-h
@@ -23,3 +23,7 @@ s,#include linux/if_ether\.h,\n#ifndef ETH_ADDR_LEN \
 
 # Use OVS's own ETH_ADDR_LEN instead of Linux-specific ETH_ALEN.
 s/ETH_ALEN/ETH_ADDR_LEN/
+
+s/OVS_VPORT_CMD_GET,/OVS_VPORT_CMD_GET,\
+   OVS_VPORT_CMD_NOTIFY,/
+
diff --git a/datapath-windows/include/OvsDpInterfaceExt.h 
b/datapath-windows/include/OvsDpInterfaceExt.h
index 64fd20e..be1e49f 100644
--- a/datapath-windows/include/OvsDpInterfaceExt.h
+++ b/datapath-windows/include/OvsDpInterfaceExt.h
@@ -34,11 +34,17 @@
 #define OVS_IOCTL_READ \
 CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x0, METHOD_OUT_DIRECT,\
   FILE_READ_ACCESS)
+#define OVS_IOCTL_READ_PACKET \
+CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x1, METHOD_OUT_DIRECT, 
\
+  FILE_READ_ACCESS)
+#define OVS_IOCTL_READ_EVENT \
+CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x2, METHOD_OUT_DIRECT, 
\
+  FILE_READ_ACCESS)
 #define OVS_IOCTL_WRITE \
-CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x1, METHOD_IN_DIRECT,\
+CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x3, METHOD_IN_DIRECT,\
   FILE_READ_ACCESS)
 #define OVS_IOCTL_TRANSACT \
-CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x2, METHOD_OUT_DIRECT,\
+CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x4, METHOD_OUT_DIRECT,\
   FILE_WRITE_ACCESS)
 
 /*
diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c
index 0dfdd57..4954712 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -99,7 +99,8 @@ static NetlinkCmdHandler OvsGetPidCmdHandler,
  OvsGetDpCmdHandler,
  OvsPendEventCmdHandler,
  OvsSubscribeEventCmdHandler,
- OvsSetDpCmdHandler;
+ OvsSetDpCmdHandler,
+ OvsReadEventCmdHandler;
 
 static NTSTATUS HandleGetDpTransaction(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
UINT32 *replyLen);
@@ -620,6 +621,30 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject,
 devOp = OVS_TRANSACTION_DEV_OP;
 break;
 
+case OVS_IOCTL_READ_EVENT:
+/* This IOCTL is used to read events */
+if (outputBufferLen != 0) {
+status = MapIrpOutputBuffer(irp, outputBufferLen,
+sizeof *ovsMsg, outputBuffer);
+if (status != STATUS_SUCCESS) {
+goto done;
+}
+ASSERT(outputBuffer);
+}
+else {
+status = STATUS_NDIS_INVALID_LENGTH;
+goto done;
+}
+inputBuffer = NULL;
+inputBufferLen = 0;
+
+ovsMsg = ovsMsgReadOp;
+ovsMsg-nlMsg.nlmsgType = OVS_WIN_NL_VPORT_FAMILY_ID;
+/* An artificial command so we can use NL family function table*/
+ovsMsg-genlMsg.cmd = OVS_VPORT_CMD_NOTIFY;
+devOp = OVS_READ_DEV_OP;
+break;
+
 case OVS_IOCTL_READ:
 /* Output buffer is mandatory. */
 if (outputBufferLen != 0) {
@@ -924,6 +949,7 @@ OvsPendEventCmdHandler(POVS_USER_PARAMS_CONTEXT 
usrParamsCtx,
 return status;
 }
 
+
 /*
  * --
  *  Handler for the subscription for the event queue
@@ -1214,4 +1240,131 @@ MapIrpOutputBuffer(PIRP irp,
 return STATUS_SUCCESS;
 }
 
+
+
+/*
+ * --
+ * Utility function to fill up information about the state of a port in a reply
+ * to* userspace.
+ * Assumes that 'gOvsCtrlLock' lock is acquired.
+ * --
+ */
+static NTSTATUS
+OvsPortFillInfo(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
+POVS_EVENT_ENTRY eventEntry,
+PNL_BUFFER nlBuf)
+{
+NTSTATUS status;
+BOOLEAN rc;
+OVS_MESSAGE msgOutTmp;

[ovs-dev] L7-filter and Openvswitch

2014-09-25 Thread Gustavo Miotto
Hi all,

 I need to filter packets based on Layer 7 inside of ovs flow tables for my
graduation work. Doing some research on internet I've found l7-filter
http://l7-filter.sourceforge.net/

Does anyone know if it is possible to use this software inside the OVS code?

Thanks in advance.

Best regards,
 Gustavo Miotto
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [ovs-discuss] checking load on OVS

2014-09-25 Thread Gauri Bhutkar
I want to check *Statistics : load_average *of openvswitch*, *I have gone
throgh this link http://openvswitch.org/ovs-vswitchd.conf.db.5.pdf
but I am not getting how to extract Statistics : load_average  field from
Open_vSwitch table.
Is there any command for this?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v1 01/10] datapath-windows: move OVS_MESSAGE to Netlink.h

2014-09-25 Thread Samuel Ghinet
Acked-by: Samuel Ghinet sghi...@cloudbasesolutions.com

Date: Wed, 24 Sep 2014 00:15:35 -0700
From: Ankur Sharma ankursha...@vmware.com
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH v1 01/10] datapath-windows: move OVS_MESSAGE
to  Netlink.h
Message-ID: 1411542944-19374-1-git-send-email-ankursha...@vmware.com

Moved the structure OVS_MESSAGE to Netlink.h.
This change is done for following reasons.

a. Patch 2 in this series provides a generic API in Netlink.c
for creating netlink message. That API needs OVS_MESSAGE.
Including Datapath.h in Netlink.c/h gives compilation error.

b. OVS_MESSAGE defines netlink messages hence moving it to
Netlink.h looked fine to me.
---
 datapath-windows/ovsext/Datapath.h| 10 --
 datapath-windows/ovsext/Netlink/Netlink.h | 11 +++
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/datapath-windows/ovsext/Datapath.h 
b/datapath-windows/ovsext/Datapath.h
index 4e6c9b1..f77c1e2 100644
--- a/datapath-windows/ovsext/Datapath.h
+++ b/datapath-windows/ovsext/Datapath.h
@@ -28,16 +28,6 @@
 #ifndef __DATAPATH_H_
 #define __DATAPATH_H_ 1

-/*
- * Structure of any message passed between userspace and kernel.
- */
-typedef struct _OVS_MESSAGE {
-NL_MSG_HDR nlMsg;
-GENL_MSG_HDR genlMsg;
-OVS_HDR ovsHdr;
-/* Variable length nl_attrs follow. */
-} OVS_MESSAGE, *POVS_MESSAGE;
-
 typedef struct _OVS_DEVICE_EXTENSION {
 INT numberOpenInstance;
 INT pidCount;
diff --git a/datapath-windows/ovsext/Netlink/Netlink.h 
b/datapath-windows/ovsext/Netlink/Netlink.h
index 6ecbdc5..b036723 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.h
+++ b/datapath-windows/ovsext/Netlink/Netlink.h
@@ -20,6 +20,17 @@
 #include Types.h
 #include NetlinkProto.h
 #include NetlinkBuf.h
+#include ..\..\include\OvsDpInterface.h
+
+/*
+ * Structure of any message passed between userspace and kernel.
+ */
+typedef struct _OVS_MESSAGE {
+NL_MSG_HDR nlMsg;
+GENL_MSG_HDR genlMsg;
+OVS_HDR ovsHdr;
+/* Variable length nl_attrs follow. */
+} OVS_MESSAGE, *POVS_MESSAGE;

 /* Netlink attribute types. */
 typedef enum
--
1.9.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v1 02/10] datapath-windows/Netlink: Add NlFillOvsMsg API for creating Netlink message headers.

2014-09-25 Thread Samuel Ghinet
Ankur,

The NlFillOvsMsg function you add looks very similar to my versions:
BuildReplyMsgFromMsgIn and BuildMsgOut.

Please look on my latest patch on vport dump to see how I defined 
OvsCreateMsgFromVport. This patch of mine is not merged yet.
We could use both versions, and refactor later. Or you may wish to make some 
use of my functions as well.

Either way it's fine by me.
Acked-by: Samuel Ghinet sghi...@cloudbasesolutions.com

Date: Wed, 24 Sep 2014 00:15:36 -0700
From: Ankur Sharma ankursha...@vmware.com
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH v1 02/10] datapath-windows/Netlink: Add
NlFillOvsMsg API for creating Netlink message headers.
Message-ID: 1411542944-19374-2-git-send-email-ankursha...@vmware.com

Added NlFillOvsMsg API in Netlink.c This API will be used to populate
netlink message headers.
---
 datapath-windows/ovsext/Netlink/Netlink.c | 38 +++
 datapath-windows/ovsext/Netlink/Netlink.h | 14 
 2 files changed, 52 insertions(+)

diff --git a/datapath-windows/ovsext/Netlink/Netlink.c 
b/datapath-windows/ovsext/Netlink/Netlink.c
index c286c2f..efaba90 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.c
+++ b/datapath-windows/ovsext/Netlink/Netlink.c
@@ -34,6 +34,44 @@

 /*
  * ---
+ * Prepare netlink message headers. Attributes should be added by caller.
+ * ---
+ */
+NTSTATUS
+NlFillOvsMsg(POVS_MESSAGE msgOut, PNL_BUFFER nlBuf,
+ UINT16 nlmsgType, UINT16 nlmsgFlags,
+ UINT32 nlmsgSeq, UINT32 nlmsgPid,
+ UINT8 genlCmd, UINT8 genlVer, UINT32 dpNo)
+{
+BOOLEAN writeOk;
+PNL_MSG_HDR nlMsg;
+
+/* XXX: Add API for nlBuf-bufRemLen. */
+ASSERT(NlBufAt(nlBuf, 0, 0) != 0 
+   nlBuf-bufRemLen = sizeof (struct _OVS_MESSAGE));
+
+msgOut-nlMsg.nlmsgType = nlmsgType;
+msgOut-nlMsg.nlmsgFlags = nlmsgFlags;
+msgOut-nlMsg.nlmsgSeq = nlmsgSeq;
+msgOut-nlMsg.nlmsgPid = nlmsgPid;
+
+msgOut-genlMsg.cmd = genlCmd;
+msgOut-genlMsg.version = genlVer;
+msgOut-genlMsg.reserved = 0;
+
+msgOut-ovsHdr.dp_ifindex = dpNo;
+
+writeOk = NlMsgPutHead(nlBuf, (PCHAR)msgOut,
+   sizeof (struct _OVS_MESSAGE));
+
+nlMsg = (PNL_MSG_HDR)NlBufAt(nlBuf, 0, 0);
+nlMsg-nlmsgLen = NLMSG_ALIGN(NlBufSize(nlBuf));
+
+return writeOk ? STATUS_SUCCESS : STATUS_INVALID_BUFFER_SIZE;
+}
+
+/*
+ * ---
  * Adds Netlink Header to the NL_BUF.
  * ---
  */
diff --git a/datapath-windows/ovsext/Netlink/Netlink.h 
b/datapath-windows/ovsext/Netlink/Netlink.h
index b036723..8f30800 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.h
+++ b/datapath-windows/ovsext/Netlink/Netlink.h
@@ -61,6 +61,14 @@ typedef struct _NL_POLICY
 BOOLEAN optional;
 } NL_POLICY, *PNL_POLICY;

+/* Structure to hold arguments needed by NlFillOvsMsg */
+typedef struct _NL_FILL_ARGS {
+UINT16 nlmsgType;
+UINT8 genlCmd;
+UINT8 version;
+UINT32 dpNo;
+} NL_FILL_ARGS, *PNL_FILL_ARGS;
+
 /* This macro is careful to check for attributes with bad lengths. */
 #define NL_ATTR_FOR_EACH(ITER, LEFT, ATTRS, ATTRS_LEN)  \
 for ((ITER) = (ATTRS), (LEFT) = (ATTRS_LEN);\
@@ -78,6 +86,12 @@ typedef struct _NL_POLICY
 #define NL_ATTR_GET_AS(NLA, TYPE) \
 (*(TYPE*) NlAttrGetUnspec(nla, sizeof(TYPE)))

+NTSTATUS
+NlFillOvsMsg(POVS_MESSAGE msgOut, PNL_BUFFER nlBuf,
+ UINT16 nlmsgType, UINT16 nlmsgFlags,
+ UINT32 nlmsgSeq, UINT32 nlmsgPid,
+ UINT8 genlCmd, UINT8 genlVer, UINT32 dpNo);
+
 /* Netlink message accessing the payload */
 PVOID NlMsgAt(const PNL_MSG_HDR nlh, UINT32 offset);
 UINT32 NlMsgSize(const PNL_MSG_HDR nlh);
--
1.9.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v1 03/10] datapath-windows/Netlink: Added NlAttrLen API

2014-09-25 Thread Samuel Ghinet
One very minor thing:
I believe the titles of the commit messages are normally put like Add 
NlAttrLen API, not Added NlAttrLen API.

Acked-by: Samuel Ghinet sghi...@cloudbasesolutions.com

Date: Wed, 24 Sep 2014 00:15:37 -0700
From: Ankur Sharma ankursha...@vmware.com
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH v1 03/10] datapath-windows/Netlink: Added
NlAttrLen API
Message-ID: 1411542944-19374-3-git-send-email-ankursha...@vmware.com

Added an API to retrieve the attribute length.
Added 2 more API for BE16 and BE8 attribute parsing.
Fixed a trailing whitespace issue.
---
 datapath-windows/ovsext/Netlink/Netlink.c | 28 +++-
 datapath-windows/ovsext/Netlink/Netlink.h | 13 -
 datapath-windows/ovsext/Types.h   |  1 +
 3 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/datapath-windows/ovsext/Netlink/Netlink.c 
b/datapath-windows/ovsext/Netlink/Netlink.c
index efaba90..5f07451 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.c
+++ b/datapath-windows/ovsext/Netlink/Netlink.c
@@ -585,7 +585,7 @@ NlMsgAttrs(const PNL_MSG_HDR nlh)
  * Returns size of to nlmsg attributes.
  * ---
  */
-INT
+UINT32
 NlMsgAttrLen(const PNL_MSG_HDR nlh)
 {
 return NlMsgPayloadLen(nlh) - GENL_HDRLEN - OVS_HDRLEN;
@@ -788,6 +788,32 @@ NlAttrGetBe32(const PNL_ATTR nla)

 /*
  * ---
+ * Returns the 16-bit network byte order value in 'nla''s payload.
+ *
+ * Asserts that 'nla''s payload is at least 2 bytes long.
+ * ---
+ */
+BE16
+NlAttrGetBe16(const PNL_ATTR nla)
+{
+return NL_ATTR_GET_AS(nla, BE16);
+}
+
+/*
+ * ---
+ * Returns the 8-bit network byte order value in 'nla''s payload.
+ *
+ * Asserts that 'nla''s payload is at least 1 byte long.
+ * ---
+ */
+BE8
+NlAttrGetBe8(const PNL_ATTR nla)
+{
+return NL_ATTR_GET_AS(nla, BE8);
+}
+
+/*
+ * ---
  * Returns the 8-bit value in 'nla''s payload.
  * ---
  */
diff --git a/datapath-windows/ovsext/Netlink/Netlink.h 
b/datapath-windows/ovsext/Netlink/Netlink.h
index 8f30800..80f98dd 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.h
+++ b/datapath-windows/ovsext/Netlink/Netlink.h
@@ -98,7 +98,7 @@ UINT32 NlMsgSize(const PNL_MSG_HDR nlh);
 PCHAR NlMsgPayload(const PNL_MSG_HDR nlh);
 UINT32 NlMsgPayloadLen(const PNL_MSG_HDR nlh);
 PNL_ATTR NlMsgAttrs(const PNL_MSG_HDR nlh);
-INT NlMsgAttrLen(const PNL_MSG_HDR nlh);
+UINT32 NlMsgAttrLen(const PNL_MSG_HDR nlh);

 /* Netlink message parse */
 PNL_MSG_HDR NlMsgNext(const PNL_MSG_HDR nlh);
@@ -130,6 +130,17 @@ BOOLEAN NlAttrParse(const PNL_MSG_HDR nlMsg, UINT32 
attrOffset,
 BOOLEAN NlParseNested(const PNL_ATTR, const NL_POLICY policy[],
   PNL_ATTR attrs[], UINT32 n_attrs);

+/*
+ * --
+ * Returns the length of attribute.
+ * --
+ */
+static __inline UINT16
+NlAttrLen(const PNL_ATTR nla)
+{
+return nla-nlaLen;
+}
+
 /* Netlink attribute validation */
 BOOLEAN NlAttrValidate(const PNL_ATTR, const PNL_POLICY);

diff --git a/datapath-windows/ovsext/Types.h b/datapath-windows/ovsext/Types.h
index b2ef48c..5d2744b 100644
--- a/datapath-windows/ovsext/Types.h
+++ b/datapath-windows/ovsext/Types.h
@@ -31,6 +31,7 @@ typedef uint8 __u8;

 /* Defines the  userspace specific data types for file
  * included within kernel only. */
+typedef UINT8 BE8;
 typedef UINT16 BE16;
 typedef UINT32 BE32;
 typedef UINT64 BE64;
--
1.9.1


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v1 05/10] datapath-windows/Netlink: Fixed NlAttrParseNested

2014-09-25 Thread Samuel Ghinet
Hello Ankur,

This code confused me a little.
It looks correct, but I would have a few suggestions:

o)
 BOOLEAN
 NlAttrParse(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
 UINT32 attrLen,
the attrLen field gives the impression that it is the length of one single 
attribute.
You could try attributesLen, or totalAttrLen or smth similar, perhaps.

o) I find the functions NlMsgAttrLen and NlMsgPayloadLen a bit confusing:
[CODE]
UINT32
NlMsgAttrLen(const PNL_MSG_HDR nlh)
{
return NlMsgPayloadLen(nlh) - GENL_HDRLEN - OVS_HDRLEN;
}
[/CODE]

NlMsgPayloadLen: when you hear payload, you think about the buffer, without 
NL_MSG_HDR, HDRLEN and HDRLEN.
NlMsgAttrLen: I tended to read it like Netlink Message Attribute Length, and 
was thinking, it computes the length of the first netlink attribute? Perhaps a 
name like OvsMsgPayloadLength() would be more intuitive.

o) I find it a bit odd that a function like NlAttrParse, which parses 
attributes, would require: the message header, the ovs message payload length 
and the offset.
Perhaps it would have been simpler if NlAttrParse had assumed that the pointer 
provided is the beginning of the first netlink attribute, and not validate the 
message itself.

It is possible such changes would make code clearer a bit.
However, there is existing code that relies on the current format of 
NlAttrParse, so perhaps on a new patch.

Acked-by: Samuel Ghinet sghi...@cloudbasesolutions.com


Date: Wed, 24 Sep 2014 00:15:39 -0700
From: Ankur Sharma ankursha...@vmware.com
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH v1 05/10] datapath-windows/Netlink: Fixed
NlAttrParseNested
Message-ID: 1411542944-19374-5-git-send-email-ankursha...@vmware.com

NlAttrParseNested was using the whole netlink payload for iteration.
This is not correct, as it would lead to exceeding the
nested attribute boundries. Fixed the same in this patch.
---
 datapath-windows/ovsext/Datapath.c|  4 +++-
 datapath-windows/ovsext/Netlink/Netlink.c | 15 ---
 datapath-windows/ovsext/Netlink/Netlink.h |  8 
 3 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c
index 0dfdd57..ffb7d44 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -949,7 +949,8 @@ OvsSubscribeEventCmdHandler(POVS_USER_PARAMS_CONTEXT 
usrParamsCtx,
 (POVS_OPEN_INSTANCE)usrParamsCtx-ovsInstance;
 POVS_MESSAGE msgIn = (POVS_MESSAGE)usrParamsCtx-inputBuffer;

-rc = NlAttrParse(msgIn-nlMsg, sizeof (*msgIn),policy, attrs, 2);
+rc = NlAttrParse(msgIn-nlMsg, sizeof (*msgIn),
+ NlMsgAttrLen((PNL_MSG_HDR)msgIn), policy, attrs, 2);
 if (!rc) {
 status = STATUS_INVALID_PARAMETER;
 goto done;
@@ -1107,6 +1108,7 @@ HandleDpTransaction(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
 if (usrParamsCtx-ovsMsg-genlMsg.cmd == OVS_DP_CMD_SET) {
 if (!NlAttrParse((PNL_MSG_HDR)msgIn,
 NLMSG_HDRLEN + GENL_HDRLEN + OVS_HDRLEN,
+NlMsgAttrLen((PNL_MSG_HDR)msgIn),
 ovsDatapathSetPolicy, dpAttrs, ARRAY_SIZE(dpAttrs))) {
 return STATUS_INVALID_PARAMETER;
 }
diff --git a/datapath-windows/ovsext/Netlink/Netlink.c 
b/datapath-windows/ovsext/Netlink/Netlink.c
index 5c74ec0..a72d846 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.c
+++ b/datapath-windows/ovsext/Netlink/Netlink.c
@@ -969,6 +969,7 @@ NlAttrFindNested(const PNL_ATTR nla, UINT16 type)
  */
 BOOLEAN
 NlAttrParse(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
+UINT32 attrLen,
 const NL_POLICY policy[],
 PNL_ATTR attrs[], UINT32 n_attrs)
 {
@@ -979,14 +980,21 @@ NlAttrParse(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,

 RtlZeroMemory(attrs, n_attrs * sizeof *attrs);

-if ((NlMsgSize(nlMsg)  attrOffset) || (!(NlMsgAttrLen(nlMsg {
+
+/* There is nothing to parse */
+if (!(NlMsgAttrLen(nlMsg))) {
+ret = TRUE;
+goto done;
+}
+
+if ((NlMsgSize(nlMsg)  attrOffset)) {
 OVS_LOG_WARN(No attributes in nlMsg: %p at offset: %d,
  nlMsg, attrOffset);
 goto done;
 }

 NL_ATTR_FOR_EACH (nla, left, NlMsgAt(nlMsg, attrOffset),
-  NlMsgSize(nlMsg) - attrOffset)
+  attrLen)
 {
 UINT16 type = NlAttrType(nla);
 if (type  n_attrs  policy[type].type != NL_A_NO_ATTR) {
@@ -1035,9 +1043,10 @@ done:
  */
 BOOLEAN
 NlAttrParseNested(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
+  UINT32 attrLen,
   const NL_POLICY policy[],
   PNL_ATTR attrs[], UINT32 n_attrs)
 {
 return NlAttrParse(nlMsg, attrOffset + NLA_HDRLEN,
-   policy, attrs, n_attrs);
+   attrLen - NLA_HDRLEN, policy, attrs, n_attrs);
 }
diff --git 

Re: [ovs-dev] [PATCH v2 07/10] datapath-windows/Flow.c: FLOW_NEW command handler.

2014-09-25 Thread Nithin Raju

On Sep 24, 2014, at 11:58 PM, Ankur Sharma ankursha...@vmware.com wrote:

 This patch covers the changes needed to support FLOW_NEW command.
 API _OvsFlowMapNlToFlowPutFlags has a bug, which will be fixed
 with the patches for FLOW_DEL.
 
 Signed-off-by: Ankur Sharma ankursha...@vmware.com
 Acked-by: Alin Gabriel Serdean aserd...@cloudbasesolutions.com
 Acked-by: Eitan Eliahu elia...@vmware.com

Acked-by: Nithin Raju nit...@vmware.com
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v1 06/10] datapath-windows/Flow.c : Basic support for add-flow.

2014-09-25 Thread Samuel Ghinet
Hey Ankur,

A few notes:
 +NETLINK_CMD nlFlowFamilyCmdOps[] = {
 +{ .cmd  = OVS_FLOW_CMD_NEW,
 +  .handler  = OvsFlowNlNewCmdHandler,
 +  .supportedDevOp   = OVS_TRANSACTION_DEV_OP,
 +  .validateDpIndex  = FALSE
 +}
 +};

It is possible that we need to have validateDpIndex  = TRUE here.

Regarding the policies, several optionals for them are wrong, or perhaps 
depending on context:
 +[OVS_FLOW_ATTR_KEY] = {.type = NL_A_NESTED, .optional = TRUE},
For Flow New, Set, Get, the KEY is always required.
For Flow Delete, if KEY is not provided it means delete all. Dump does not 
require it.

 +[OVS_KEY_ATTR_IN_PORT] = {.type = NL_A_UNSPEC, .minLen = 4,
 +  .maxLen = 4, .optional = TRUE},
I believe the input port is always required: all flows must have an input port 
non-masked, as I remember.
The same goes for ethernet type.
If we have a flow/key/tunnel, tunnel destination ipv4 address is required 
(non-masked).

Perhaps it would be best if you set the option values depending on context.

Regards,
Sam


Date: Wed, 24 Sep 2014 00:15:40 -0700
From: Ankur Sharma ankursha...@vmware.com
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH v1 06/10] datapath-windows/Flow.c : Basic
support for add-flow.
Message-ID: 1411542944-19374-6-git-send-email-ankursha...@vmware.com

This patch covers basic changes in registering add flow handler.
And declaring FLOW related attribute parsing policies.
---
 datapath-windows/ovsext/Datapath.c |  18 -
 datapath-windows/ovsext/Flow.c | 152 +
 datapath-windows/ovsext/Flow.h |   5 ++
 3 files changed, 171 insertions(+), 4 deletions(-)

diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c
index ffb7d44..7b064f9 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -191,14 +191,22 @@ NETLINK_FAMILY nlVportFamilyOps = {
 };

 /* Netlink flow family. */
-/* XXX: Add commands here. */
+
+NETLINK_CMD nlFlowFamilyCmdOps[] = {
+{ .cmd  = OVS_FLOW_CMD_NEW,
+  .handler  = OvsFlowNlNewCmdHandler,
+  .supportedDevOp   = OVS_TRANSACTION_DEV_OP,
+  .validateDpIndex  = FALSE
+}
+};
+
 NETLINK_FAMILY nlFLowFamilyOps = {
 .name = OVS_FLOW_FAMILY,
 .id   = OVS_WIN_NL_FLOW_FAMILY_ID,
 .version  = OVS_FLOW_VERSION,
 .maxAttr  = OVS_FLOW_ATTR_MAX,
-.cmds = NULL, /* XXX: placeholder. */
-.opsCount = 0
+.cmds = nlFlowFamilyCmdOps,
+.opsCount = ARRAY_SIZE(nlFlowFamilyCmdOps)
 };

 static NTSTATUS MapIrpOutputBuffer(PIRP irp,
@@ -689,8 +697,10 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject,
 case OVS_WIN_NL_DATAPATH_FAMILY_ID:
 nlFamilyOps = nlDatapathFamilyOps;
 break;
-case OVS_WIN_NL_PACKET_FAMILY_ID:
 case OVS_WIN_NL_FLOW_FAMILY_ID:
+nlFamilyOps = nlFLowFamilyOps;
+break;
+case OVS_WIN_NL_PACKET_FAMILY_ID:
 case OVS_WIN_NL_VPORT_FAMILY_ID:
 status = STATUS_NOT_IMPLEMENTED;
 goto done;
diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
index dae1dca..25b39c1 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -51,6 +51,158 @@ static VOID __inline *GetStartAddrNBL(const NET_BUFFER_LIST 
*_pNB);
 #define OVS_FLOW_TABLE_MASK (OVS_FLOW_TABLE_SIZE -1)
 #define HASH_BUCKET(hash) ((hash)  OVS_FLOW_TABLE_MASK)

+/* Flow family related netlink policies */
+
+/* For Parsing attributes in FLOW_* commands */
+static const NL_POLICY nlFlowPolicy[] = {
+[OVS_FLOW_ATTR_KEY] = {.type = NL_A_NESTED, .optional = TRUE},
+[OVS_FLOW_ATTR_MASK] = {.type = NL_A_NESTED, .optional = TRUE},
+[OVS_FLOW_ATTR_ACTIONS] = {.type = NL_A_NESTED, .optional = TRUE},
+[OVS_FLOW_ATTR_STATS] = {.type = NL_A_UNSPEC,
+ .minLen = sizeof(struct ovs_flow_stats),
+ .maxLen = sizeof(struct ovs_flow_stats),
+ .optional = TRUE},
+[OVS_FLOW_ATTR_TCP_FLAGS] = {NL_A_U8, .optional = TRUE},
+[OVS_FLOW_ATTR_USED] = {NL_A_U64, .optional = TRUE}
+};
+
+/* For Parsing nested OVS_FLOW_ATTR_KEY attributes */
+static const NL_POLICY nlFlowKeyPolicy[] = {
+[OVS_KEY_ATTR_ENCAP] = {.type = NL_A_VAR_LEN, .optional = TRUE},
+[OVS_KEY_ATTR_PRIORITY] = {.type = NL_A_UNSPEC, .minLen = 4,
+   .maxLen = 4, .optional = TRUE},
+[OVS_KEY_ATTR_IN_PORT] = {.type = NL_A_UNSPEC, .minLen = 4,
+  .maxLen = 4, .optional = TRUE},
+[OVS_KEY_ATTR_ETHERNET] = {.type = NL_A_UNSPEC,
+   .minLen = sizeof(struct ovs_key_ethernet),
+   .maxLen = sizeof(struct ovs_key_ethernet),
+   .optional = TRUE},
+[OVS_KEY_ATTR_VLAN] = {.type = NL_A_UNSPEC, .minLen = 2,
+   

Re: [ovs-dev] [PATCH v2 08/10] datapath-windows/Flow.c: FLOW_SET command handler.

2014-09-25 Thread Nithin Raju

On Sep 24, 2014, at 11:58 PM, Ankur Sharma ankursha...@vmware.com wrote:

 Registered FLOW_SET command handler. The same command
 handler as FLOW_ADD is good enough to handle FLOW_SET
 case as well.
 
 Signed-off-by: Ankur Sharma ankursha...@vmware.com
 Acked-by: Alin Gabriel Serdean aserd...@cloudbasesolutions.com
 Acked-by: Eitan Eliahu elia...@vmware.com
 ---
 datapath-windows/ovsext/Datapath.c | 5 +
 1 file changed, 5 insertions(+)
 
 diff --git a/datapath-windows/ovsext/Datapath.c 
 b/datapath-windows/ovsext/Datapath.c
 index 9934a9a..5008aab 100644
 --- a/datapath-windows/ovsext/Datapath.c
 +++ b/datapath-windows/ovsext/Datapath.c
 @@ -197,6 +197,11 @@ NETLINK_CMD nlFlowFamilyCmdOps[] = {
   .handler  = OvsFlowNlNewCmdHandler,
   .supportedDevOp   = OVS_TRANSACTION_DEV_OP,
   .validateDpIndex  = FALSE
 +},
 +{ .cmd  = OVS_FLOW_CMD_SET,
 +  .handler  = OvsFlowNlNewCmdHandler,
 +  .supportedDevOp   = OVS_TRANSACTION_DEV_OP,
 +  .validateDpIndex  = FALSE
 }
 };

validateDpIndex should be TRUE for both the commands. But, LG otherwise.

Acked-by: Nithin Raju nit...@vmware.com

-- Nithin
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [ovs-discuss] checking load on OVS

2014-09-25 Thread Ben Pfaff
On Thu, Sep 25, 2014 at 07:39:23AM -0700, Gauri Bhutkar wrote:
 I want to check *Statistics : load_average *of openvswitch*, *I have gone
 throgh this link http://openvswitch.org/ovs-vswitchd.conf.db.5.pdf
 but I am not getting how to extract Statistics : load_average  field from
 Open_vSwitch table.
 Is there any command for this?

It's no different from any other field.  Read ovs-vsctl(8).
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] rstp.at: Fix intermittent test failure.

2014-09-25 Thread Alex Wang
Sub-test RSTP - dummy interface checks the ovs-vswitchd.log
output immediately after command execution.  The check may
fail if the write of new log is delayed by the IO thread.

This commit fixes the above issue by waiting for the
ovs-vswitchd.log output.

Signed-off-by: Alex Wang al...@nicira.com
---
 tests/rstp.at |2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/rstp.at b/tests/rstp.at
index 3f56931..5caa84f 100644
--- a/tests/rstp.at
+++ b/tests/rstp.at
@@ -213,6 +213,7 @@ AT_CHECK([ovs-ofctl add-flow br1 in_port=2 icmp 
actions=8])
 
 AT_CHECK([ovs-vsctl set port p2 other_config:rstp-enable=true])
 
+OVS_WAIT_UNTIL([test `cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY | wc -l` -ge 
2])
 AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY], [0], [dnl
 port p1: RSTP state changed from Disabled to Discarding
 port p2: RSTP state changed from Disabled to Discarding
@@ -232,6 +233,7 @@ AT_CHECK([ovs-appctl netdev-dummy/set-admin-state p2 up], 
[], [dnl
 OK
 ])
 
+OVS_WAIT_UNTIL([test `cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY | wc -l` -ge 
4])
 AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY], [0], [dnl
 port p1: RSTP state changed from Disabled to Discarding
 port p2: RSTP state changed from Disabled to Discarding
-- 
1.7.9.5

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2 09/10] datapath-windows/Flow.c: FLOW_DEL command handler.

2014-09-25 Thread Nithin Raju

On Sep 24, 2014, at 11:58 PM, Ankur Sharma ankursha...@vmware.com wrote:

 Registered FLOW_DEL command handler. The same command
 handler as FLOW_ADD is good enough to handle FLOW_DEL
 case as well with minor changes for checking to action
 attribute.
 
 Signed-off-by: Ankur Sharma ankursha...@vmware.com
 Acked-by: Alin Gabriel Serdean aserd...@cloudbasesolutions.com
 Acked-by: Eitan Eliahu elia...@vmware.com
 ---
 datapath-windows/ovsext/Datapath.c | 5 +
 datapath-windows/ovsext/Flow.c | 7 +--
 2 files changed, 10 insertions(+), 2 deletions(-)
 
 diff --git a/datapath-windows/ovsext/Datapath.c 
 b/datapath-windows/ovsext/Datapath.c
 index 5008aab..5377f09 100644
 --- a/datapath-windows/ovsext/Datapath.c
 +++ b/datapath-windows/ovsext/Datapath.c
 @@ -202,6 +202,11 @@ NETLINK_CMD nlFlowFamilyCmdOps[] = {
   .handler  = OvsFlowNlNewCmdHandler,
   .supportedDevOp   = OVS_TRANSACTION_DEV_OP,
   .validateDpIndex  = FALSE
 +},
 +{ .cmd  = OVS_FLOW_CMD_DEL,
 +  .handler  = OvsFlowNlNewCmdHandler,
 +  .supportedDevOp   = OVS_TRANSACTION_DEV_OP,
 +  .validateDpIndex  = FALSE

LG. Maybe we should call the function as OvsFlowNlCmdHandler, and get rid of 
the 'New'.

.validateDpindex should be TRUE.

-- Nithin
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2 10/10] datapath-windows/Flow.c: DEL_FLOWS command handler.

2014-09-25 Thread Nithin Raju
On Sep 24, 2014, at 11:58 PM, Ankur Sharma ankursha...@vmware.com wrote:

 Added changes to handle DEL_FLOWS (FLUSH) scenario.
 
 Signed-off-by: Ankur Sharma ankursha...@vmware.com
 Acked-by: Alin Gabriel Serdean aserd...@cloudbasesolutions.com
 Acked-by: Eitan Eliahu elia...@vmware.com

LG. Thanks for the feature.

Acked-by: Nithin Raju nit...@vmware.com

Thanks,
-- Nithin
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v1 07/10] datapath-windows/Flow.c: FLOW_NEW command handler.

2014-09-25 Thread Samuel Ghinet
Hey Ankur,

 +if (keyAttrs[OVS_KEY_ATTR_TUNNEL]) {
 +destKey-tunKey.tunnelId = NlAttrGetU64
 +   (tunAttrs[OVS_TUNNEL_KEY_ATTR_ID]);
 +destKey-tunKey.dst = NlAttrGetU32
 +  (tunAttrs[OVS_TUNNEL_KEY_ATTR_IPV4_DST]);
 +destKey-tunKey.src = NlAttrGetU32
 +  (tunAttrs[OVS_TUNNEL_KEY_ATTR_IPV4_SRC]);
 +
 +/* XXX: There are no attributes to specify tunnel flags.
 + * Also, i do not see these flags being used anywhere in
 + * flowPut code. */
 +destKey-tunKey.flags = 0;
 +destKey-tunKey.tos = NlAttrGetU8

Those flags are actually necessary.
The flag attributes are (OvsDpInterface.h):
[CODE]
OVS_TUNNEL_KEY_ATTR_DONT_FRAGMENT,  /* No argument, set DF. */
OVS_TUNNEL_KEY_ATTR_CSUM,   /* No argument. CSUM packet. */
OVS_TUNNEL_KEY_ATTR_OAM,/* No argument, OAM frame. */
[/CODE]
The tunnelKey must OR together the flags, based on the attributes.
Each attribute that is a flag does not have a length, nor payload, as I 
remember. If the attribute flag appears, it means you must take it as set. If 
the flag attribute does not appear, you must take it as not set.

The corresponding flags are:
[CODE]
/* Flags for tunneling */
#define OVS_TNL_F_DONT_FRAGMENT (1  0)
#define OVS_TNL_F_CSUM  (1  1)
#define OVS_TNL_F_KEY   (1  2)
[/CODE]

Also,
_OvsFlowMapTunAttrToFlowPut:
 +destKey-tunKey.tunnelId = NlAttrGetU64
 +   (tunAttrs[OVS_TUNNEL_KEY_ATTR_ID]);
This will crash if the userspace did not give you a tunnel attribute tunnel 
id. And you also have it specified as optional in the policy.

OvsFlowMapKeyAttrToFlowPut:
 destKey-l2.inPort = NlAttrGetU32(keyAttrs[OVS_KEY_ATTR_IN_PORT]);
The same here.
 eth_key = NlAttrGet(keyAttrs[OVS_KEY_ATTR_ETHERNET]);
and here. It is possible that the eth key is required. Or that it is optional 
(I think in ovs 1.11 was required, but in 2.3, when masks were allowed, was 
optional - experience with our implementation of the driver)

 case ETH_TYPE_ARP:
 case ETH_TYPE_RARP: {
I really believe RARP is no longer used, since some long time ago.

 case ETH_TYPE_IPV4: {
 const struct ovs_key_ipv4 *ipv4Key;
 const struct ovs_key_tcp *tcpKey;
 
 ipv4Key = NlAttrGet(keyAttrs[OVS_KEY_ATTR_IPV4]);
It is possible that the eth type attribute was given ipv4 but the ipv4 key 
was not given. When key masks are allowed, this can happen if the ipv4 key is 
considered to be wildcard so it is not given as a key, nor mask, at all.

 if (keyAttrs[OVS_KEY_ATTR_TCP]) {
tcpKey = NlAttrGet(keyAttrs[OVS_KEY_ATTR_TCP]);
ipv4FlowPutKey-l4.tpSrc = tcpKey-tcp_src;
ipv4FlowPutKey-l4.tpDst = tcpKey-tcp_dst;
}
Where is tcp flags set?

 arpFlowPutKey-pad[0] = 0;
arpFlowPutKey-pad[1] = 0;
arpFlowPutKey-pad[2] = 0;
destKey-l2.keyLen += OVS_ARP_KEY_SIZE;
What I particularly don't like about the old approach with flows is this 
somewhat crypted text.
I wished there was at least some documentation on structs like this:
[CODE]
typedef struct L2Key {
uint32_t inPort; /* Port number of input port. */
union {
struct {
uint16_t offset;
uint16_t keyLen;
};
uint32_t val;
};
[/CODE]
on what the fields are and how they are used.

Also,
_OvsFlowMapKeyAttrToFlowPut,
_OvsFlowMapTunAttrToFlowPut,
_OvsFlowMapNlToFlowPutFlags,
_OvsFlowMapNlToFlowPut
The names are very long and very similar to one another. It is very easy to 
confuse one with the other, when reading.
Perhaps something like _KeyMapToFlowPut, _FlowMapToFlowPut, _FlowMapToFlags.
Or perhaps use a FP for FlowPut, and document this shorthand.

 eth_key = NlAttrGet(keyAttrs[OVS_KEY_ATTR_ETHERNET]);
 memcpy(destKey-l2.dlSrc, eth_key-eth_src, ETH_ADDR_LEN);
memcpy(destKey-l2.dlDst, eth_key-eth_dst, ETH_ADDR_LEN);
For those who prefer the Rtl* style, there is RtlCopyMemory :)

Regards,
Sam

Date: Wed, 24 Sep 2014 00:15:41 -0700
From: Ankur Sharma ankursha...@vmware.com
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH v1 07/10] datapath-windows/Flow.c: FLOW_NEW
command handler.
Message-ID: 1411542944-19374-7-git-send-email-ankursha...@vmware.com

This patch covers the changes needed to support FLOW_NEW command.
API _OvsFlowMapNlToFlowPutFlags has a bug, which will be fixed
with the patches for FLOW_DEL.
---
 datapath-windows/include/OvsPub.h |   2 +-
 datapath-windows/ovsext/Flow.c| 384 +++---
 datapath-windows/ovsext/Flow.h|   6 +-
 3 files changed, 366 insertions(+), 26 deletions(-)

diff --git a/datapath-windows/include/OvsPub.h 
b/datapath-windows/include/OvsPub.h
index 36814c4..fa1d6d4 100644
--- 

Re: [ovs-dev] [PATCH v1 08/10] datapath-windows/Flow.c: FLOW_SET command handler.

2014-09-25 Thread Samuel Ghinet
Hey Ankur,

There are a few differences between flow new and flow set, with regard to the 
transactional errors that will need to be implemented:
Flow New: can do NEW or SET. 
If netlink command flow new is issued, with netlink flags create and 
exclusive, and the flow exists, it must return EEXIST.

Flow Set: can do SET only: if the flow does not exist, it must return ENOENT.

Also, Flow Set has actions optional, while for Flow New the actions are 
required.

Regards,
Sam


Date: Wed, 24 Sep 2014 00:15:42 -0700
From: Ankur Sharma ankursha...@vmware.com
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH v1 08/10] datapath-windows/Flow.c: FLOW_SET
command handler.
Message-ID: 1411542944-19374-8-git-send-email-ankursha...@vmware.com

Registered FLOW_SET command handler. The same command
handler as FLOW_ADD is good enough to handle FLOW_SET
case as well.
---
 datapath-windows/ovsext/Datapath.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c
index 7b064f9..5008aab 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -193,7 +193,12 @@ NETLINK_FAMILY nlVportFamilyOps = {
 /* Netlink flow family. */

 NETLINK_CMD nlFlowFamilyCmdOps[] = {
-{ .cmd  = OVS_FLOW_CMD_NEW,
+{ .cmd  = OVS_FLOW_CMD_NEW,
+  .handler  = OvsFlowNlNewCmdHandler,
+  .supportedDevOp   = OVS_TRANSACTION_DEV_OP,
+  .validateDpIndex  = FALSE
+},
+{ .cmd  = OVS_FLOW_CMD_SET,
   .handler  = OvsFlowNlNewCmdHandler,
   .supportedDevOp   = OVS_TRANSACTION_DEV_OP,
   .validateDpIndex  = FALSE
--
1.9.1


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2 09/10] datapath-windows/Flow.c: FLOW_DEL command handler.

2014-09-25 Thread Nithin Raju
 LG. Maybe we should call the function as OvsFlowNlCmdHandler, and get rid of 
 the 'New'.
 
 .validateDpindex should be TRUE.

Acked-by: Nithin Raju nit...@vmware.com

-- Nithin
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] rstp.at: Fix intermittent test failure.

2014-09-25 Thread Gurucharan Shetty
On Thu, Sep 25, 2014 at 8:51 AM, Alex Wang al...@nicira.com wrote:
 Sub-test RSTP - dummy interface checks the ovs-vswitchd.log
 output immediately after command execution.  The check may
 fail if the write of new log is delayed by the IO thread.

 This commit fixes the above issue by waiting for the
 ovs-vswitchd.log output.

 Signed-off-by: Alex Wang al...@nicira.com
 ---
  tests/rstp.at |2 ++
  1 file changed, 2 insertions(+)

 diff --git a/tests/rstp.at b/tests/rstp.at
 index 3f56931..5caa84f 100644
 --- a/tests/rstp.at
 +++ b/tests/rstp.at
 @@ -213,6 +213,7 @@ AT_CHECK([ovs-ofctl add-flow br1 in_port=2 icmp 
 actions=8])

  AT_CHECK([ovs-vsctl set port p2 other_config:rstp-enable=true])

 +OVS_WAIT_UNTIL([test `cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY | wc -l` 
 -ge 2])

The test failures that I see create 3 entries instead of 2. So the
above test won't fix it.
You can reproduce it, if you put a 'sleep 5' after the below command.
AT_CHECK([ovs-appctl netdev-dummy/set-admin-state p1 up], [], [dnl
 OK
 ])

  AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY], [0], [dnl
  port p1: RSTP state changed from Disabled to Discarding
  port p2: RSTP state changed from Disabled to Discarding
 @@ -232,6 +233,7 @@ AT_CHECK([ovs-appctl netdev-dummy/set-admin-state p2 up], 
 [], [dnl
  OK
  ])

 +OVS_WAIT_UNTIL([test `cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY | wc -l` 
 -ge 4])
  AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY], [0], [dnl
  port p1: RSTP state changed from Disabled to Discarding
  port p2: RSTP state changed from Disabled to Discarding
 --
 1.7.9.5

 ___
 dev mailing list
 dev@openvswitch.org
 http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v1 10/10] datapath-windows/Flow.c: DEL_FLOWS command handler.

2014-09-25 Thread Samuel Ghinet
Oh, now I see, flow flush is handled :)


Date: Wed, 24 Sep 2014 00:15:44 -0700
From: Ankur Sharma ankursha...@vmware.com
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH v1 10/10] datapath-windows/Flow.c: DEL_FLOWS
command handler.
Message-ID:
1411542944-19374-10-git-send-email-ankursha...@vmware.com

Added changes to handle DEL_FLOWS (FLUSH) scenario.
---
 datapath-windows/ovsext/Flow.c | 19 ++-
 datapath-windows/ovsext/Flow.h |  2 +-
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
index b95f69b..0b4a7fa 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -209,7 +209,8 @@ static const NL_POLICY nlFlowActionPolicy[] = {
 /*
  *
  *  OvsFlowNlNewCmdHandler --
- *Handler for OVS_FLOW_CMD_NEW command.
+ *Handler for OVS_FLOW_CMD_NEW/SET/DEL command.
+ *It also handles FLUSH case (DEL w/o any key in input)
  *
  */
 NTSTATUS
@@ -252,6 +253,13 @@ OvsFlowNlNewCmdHandler(POVS_USER_PARAMS_CONTEXT 
usrParamsCtx,
 goto done;
 }

+/* FLOW_DEL command w/o any key input is a flush case. */
+if ((genlMsgHdr-cmd == OVS_FLOW_CMD_DEL) 
+(!(nlAttrs[OVS_FLOW_ATTR_KEY]))) {
+rc = OvsFlushFlowIoctl(ovsHdr-dp_ifindex);
+goto done;
+}
+
 if ((_OvsFlowMapNlToFlowPut(msgIn, nlAttrs[OVS_FLOW_ATTR_KEY],
  nlAttrs[OVS_FLOW_ATTR_ACTIONS], nlAttrs[OVS_FLOW_ATTR_CLEAR],
  mappedFlow))
@@ -1441,19 +1449,12 @@ unlock:
 }

 NTSTATUS
-OvsFlushFlowIoctl(PVOID inputBuffer,
-  UINT32 inputLength)
+OvsFlushFlowIoctl(UINT32 dpNo)
 {
 NTSTATUS status = STATUS_SUCCESS;
 OVS_DATAPATH *datapath = NULL;
-UINT32 dpNo;
 LOCK_STATE_EX dpLockState;

-if (inputLength != sizeof(UINT32) || inputBuffer == NULL) {
-return STATUS_INFO_LENGTH_MISMATCH;
-}
-
-dpNo = *(UINT32 *)inputBuffer;
 NdisAcquireSpinLock(gOvsCtrlLock);
 if (gOvsSwitchContext == NULL ||
 gOvsSwitchContext-dpNo != dpNo) {
diff --git a/datapath-windows/ovsext/Flow.h b/datapath-windows/ovsext/Flow.h
index e62ba40..10ef62b 100644
--- a/datapath-windows/ovsext/Flow.h
+++ b/datapath-windows/ovsext/Flow.h
@@ -68,7 +68,7 @@ NTSTATUS OvsPutFlowIoctl(PVOID inputBuffer, UINT32 
inputLength,
 NTSTATUS OvsGetFlowIoctl(PVOID inputBuffer, UINT32 inputLength,
  PVOID outputBuffer, UINT32 outputLength,
  UINT32 *replyLen);
-NTSTATUS OvsFlushFlowIoctl(PVOID inputBuffer, UINT32 inputLength);
+NTSTATUS OvsFlushFlowIoctl(UINT32 dpNo);

 NTSTATUS OvsFlowNlNewCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
 UINT32 *replyLen);
--
1.9.1


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] datapath-windows Event read handler

2014-09-25 Thread Samuel Ghinet
Hey Eitan,

   */
  #if defined OVS_USE_NL_INTERFACE  OVS_USE_NL_INTERFACE == 1
 
 +#pragma warning( push )
 +#pragma warning( disable:4127 )
 +
  #include precomp.h
Could you put some doc comment saying what warning you are disabling, and 
perhaps, why? :)

You've got a few else-s like this:
 }
 else {
 status = STATUS_NDIS_INVALID_LENGTH;
 goto done;
 }
And an added new line, I see.

 msgOutTmp.ovsHdr.dp_ifindex = gOvsSwitchContext-dpNo;
 
 do {
 
 rc = NlMsgPutHead(nlBuf, (PCHAR)msgOutTmp, sizeof msgOutTmp);
 if (!rc) {
 status = STATUS_INVALID_BUFFER_SIZE;
 break;
 }
 
 vport = OvsFindVportByPortNo(gOvsSwitchContext, eventEntry-portNo);
 if (!vport) {
 status = STATUS_DEVICE_DOES_NOT_EXIST;
 break;
 }
 
 rc = NlMsgPutTailU32(nlBuf, OVS_VPORT_ATTR_PORT_NO, 
 eventEntry-portNo) ||
  NlMsgPutTailU32(nlBuf, OVS_VPORT_ATTR_TYPE, vport-ovsType) ||
  NlMsgPutTailString(nlBuf, OVS_VPORT_ATTR_NAME, vport-ovsName);
 if (!rc) {
 status = STATUS_INVALID_BUFFER_SIZE;
 break;
 }
 
 /*  Should we add the port stats attributes?*/
 nlMsg = (PNL_MSG_HDR)NlBufAt(nlBuf, 0, 0);
 nlMsg-nlmsgLen = NlBufSize(nlBuf);
 rc = STATUS_SUCCESS;
 } while (FALSE);
1. What does rc stand for?
2. will it not be a problem if it would fail at the second break, so that you 
put a message header (what length will it have?), and then you're out of buffer 
space?

How many events do you expect to be put there?
Also, I think the proper method would be to put as many as we can, and return 
success, not put as many as we can, and return STATUS_INVALID_BUFFER_SIZE. Am I 
missing something?
Also, what would be the case when rc will be STATUS_SUCCESS and it will break 
out of the loop?

Also, note that NlMsgPutTailU32 returns BOOLEAN: FALSE means error. If you 
break from the loop it will return rc == FALSE == STATUS_SUCCESS.

Also,
 case OVS_IOCTL_READ_EVENT:
 /* This IOCTL is used to read events */
 if (outputBufferLen != 0) {
I see it shares some code with case OVS_IOCTL_READ:
An idea would be to extract a common function of the two.

Regards,
Sam


Message: 1
Date: Wed, 24 Sep 2014 16:36:28 -0700
From: Eitan Eliahu elia...@vmware.com
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH] datapath-windows Event read handler
Message-ID: 1411601788-2024-1-git-send-email-elia...@vmware.com

The Read event handler is executed when user mode issues a socket receive on
an MC socket associated with the event queue. A new IOCTL READ command is
used to differentiate between transaction based and packet miss sockets.
An entry for the handler will be added once the Vport family implementation
checked in.
User mode code for setting the socket type will follow

Signed-off-by: Eitan Eliahu elia...@vmware.com
---
 build-aux/extract-odp-netlink-windows-dp-h   |   4 +
 datapath-windows/include/OvsDpInterfaceExt.h |  10 +-
 datapath-windows/ovsext/Datapath.c   | 164 ++-
 datapath-windows/ovsext/Event.c  |  42 +++
 datapath-windows/ovsext/Event.h  |   2 +
 5 files changed, 219 insertions(+), 3 deletions(-)

diff --git a/build-aux/extract-odp-netlink-windows-dp-h 
b/build-aux/extract-odp-netlink-windows-dp-h
index 70514cb..82d95f1 100755
--- a/build-aux/extract-odp-netlink-windows-dp-h
+++ b/build-aux/extract-odp-netlink-windows-dp-h
@@ -23,3 +23,7 @@ s,#include linux/if_ether\.h,\n#ifndef ETH_ADDR_LEN \

 # Use OVS's own ETH_ADDR_LEN instead of Linux-specific ETH_ALEN.
 s/ETH_ALEN/ETH_ADDR_LEN/
+
+s/OVS_VPORT_CMD_GET,/OVS_VPORT_CMD_GET,\
+   OVS_VPORT_CMD_NOTIFY,/
+
diff --git a/datapath-windows/include/OvsDpInterfaceExt.h 
b/datapath-windows/include/OvsDpInterfaceExt.h
index 64fd20e..be1e49f 100644
--- a/datapath-windows/include/OvsDpInterfaceExt.h
+++ b/datapath-windows/include/OvsDpInterfaceExt.h
@@ -34,11 +34,17 @@
 #define OVS_IOCTL_READ \
 CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x0, METHOD_OUT_DIRECT,\
   FILE_READ_ACCESS)
+#define OVS_IOCTL_READ_PACKET \
+CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x1, METHOD_OUT_DIRECT, 
\
+  FILE_READ_ACCESS)
+#define OVS_IOCTL_READ_EVENT \
+CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x2, METHOD_OUT_DIRECT, 
\
+  FILE_READ_ACCESS)
 #define OVS_IOCTL_WRITE \
-CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x1, METHOD_IN_DIRECT,\
+CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x3, METHOD_IN_DIRECT,\
   FILE_READ_ACCESS)
 #define OVS_IOCTL_TRANSACT \
-CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x2, METHOD_OUT_DIRECT,\
+CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x4, METHOD_OUT_DIRECT,\
   FILE_WRITE_ACCESS)

 

Re: [ovs-dev] [PATCH v2] datapath-windows Event read handler

2014-09-25 Thread Samuel Ghinet
I now see this new version.
The while has been removed I see, along with the issues I had pointed out with 
the values returned.

I will write my comments here, of the things that still remained :)

 }
 
 +
  /*
   * --
   *  Handler for the subscription for the event queue
 @@ -1214,4 +1240,131 @@ MapIrpOutputBuffer(PIRP irp
I believe we don't need that blank line :)

You've got a few else-s like this:
 }
 else {
 status = STATUS_NDIS_INVALID_LENGTH;
 goto done;
 }

I see that you now always returns status, and no longer rc.
Still, what does rc stand for?

And, 
 case OVS_IOCTL_READ_EVENT:
 /* This IOCTL is used to read events */
 if (outputBufferLen != 0) {
I see it shares some code with case OVS_IOCTL_READ:
An idea would be to extract a common function of the two. But this is very 
minor.

Regards,
Sam

Date: Thu, 25 Sep 2014 13:21:12 -0700
From: Eitan Eliahu elia...@vmware.com
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH v2] datapath-windows Event read handler
Message-ID: 1411676472-3644-1-git-send-email-elia...@vmware.com

The Read event handler is executed when user mode issues a socket receive on
an MC socket associated with the event queue. A new IOCTL READ command is
used to differentiate between transaction based and packet miss sockets.
An entry for the handler will be added once the Vport family implementation
checked in.
User mode code for setting the socket type will follow

Signed-off-by: Eitan Eliahu elia...@vmware.com
---
 build-aux/extract-odp-netlink-windows-dp-h   |   4 +
 datapath-windows/include/OvsDpInterfaceExt.h |  10 +-
 datapath-windows/ovsext/Datapath.c   | 155 ++-
 datapath-windows/ovsext/Event.c  |  42 
 datapath-windows/ovsext/Event.h  |   2 +
 5 files changed, 210 insertions(+), 3 deletions(-)

diff --git a/build-aux/extract-odp-netlink-windows-dp-h 
b/build-aux/extract-odp-netlink-windows-dp-h
index 70514cb..82d95f1 100755
--- a/build-aux/extract-odp-netlink-windows-dp-h
+++ b/build-aux/extract-odp-netlink-windows-dp-h
@@ -23,3 +23,7 @@ s,#include linux/if_ether\.h,\n#ifndef ETH_ADDR_LEN \

 # Use OVS's own ETH_ADDR_LEN instead of Linux-specific ETH_ALEN.
 s/ETH_ALEN/ETH_ADDR_LEN/
+
+s/OVS_VPORT_CMD_GET,/OVS_VPORT_CMD_GET,\
+   OVS_VPORT_CMD_NOTIFY,/
+
diff --git a/datapath-windows/include/OvsDpInterfaceExt.h 
b/datapath-windows/include/OvsDpInterfaceExt.h
index 64fd20e..be1e49f 100644
--- a/datapath-windows/include/OvsDpInterfaceExt.h
+++ b/datapath-windows/include/OvsDpInterfaceExt.h
@@ -34,11 +34,17 @@
 #define OVS_IOCTL_READ \
 CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x0, METHOD_OUT_DIRECT,\
   FILE_READ_ACCESS)
+#define OVS_IOCTL_READ_PACKET \
+CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x1, METHOD_OUT_DIRECT, 
\
+  FILE_READ_ACCESS)
+#define OVS_IOCTL_READ_EVENT \
+CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x2, METHOD_OUT_DIRECT, 
\
+  FILE_READ_ACCESS)
 #define OVS_IOCTL_WRITE \
-CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x1, METHOD_IN_DIRECT,\
+CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x3, METHOD_IN_DIRECT,\
   FILE_READ_ACCESS)
 #define OVS_IOCTL_TRANSACT \
-CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x2, METHOD_OUT_DIRECT,\
+CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x4, METHOD_OUT_DIRECT,\
   FILE_WRITE_ACCESS)

 /*
diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c
index 0dfdd57..4954712 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -99,7 +99,8 @@ static NetlinkCmdHandler OvsGetPidCmdHandler,
  OvsGetDpCmdHandler,
  OvsPendEventCmdHandler,
  OvsSubscribeEventCmdHandler,
- OvsSetDpCmdHandler;
+ OvsSetDpCmdHandler,
+ OvsReadEventCmdHandler;

 static NTSTATUS HandleGetDpTransaction(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
UINT32 *replyLen);
@@ -620,6 +621,30 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject,
 devOp = OVS_TRANSACTION_DEV_OP;
 break;

+case OVS_IOCTL_READ_EVENT:
+/* This IOCTL is used to read events */
+if (outputBufferLen != 0) {
+status = MapIrpOutputBuffer(irp, outputBufferLen,
+sizeof *ovsMsg, outputBuffer);
+if (status != STATUS_SUCCESS) {
+goto done;
+}
+ASSERT(outputBuffer);
+}
+else {
+status = STATUS_NDIS_INVALID_LENGTH;
+goto done;
+}
+inputBuffer = NULL;
+inputBufferLen = 0;
+
+ovsMsg = ovsMsgReadOp;

Re: [ovs-dev] [PATCH v1 04/10] datapath-windows/Netlink: Allow support for NESTED Attributes in NlAttrValidate

2014-09-25 Thread Samuel Ghinet
In the future it might be useful if we could do recursive validation checks in 
NlAttrValidate when we have nested attributes.
Because I am not sure we can currently validate the netlink attributes nested 
in parent netlink attributes, using functions like NlAttrValidate.

Acked-by: Samuel Ghinet sghi...@cloudbasesolutions.com

Date: Wed, 24 Sep 2014 00:15:38 -0700
From: Ankur Sharma ankursha...@vmware.com
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH v1 04/10] datapath-windows/Netlink: Allow
support for NESTED Attributes in NlAttrValidate
Message-ID: 1411542944-19374-4-git-send-email-ankursha...@vmware.com

---
 datapath-windows/ovsext/Netlink/Netlink.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/datapath-windows/ovsext/Netlink/Netlink.c 
b/datapath-windows/ovsext/Netlink/Netlink.c
index 5f07451..5c74ec0 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.c
+++ b/datapath-windows/ovsext/Netlink/Netlink.c
@@ -870,12 +870,13 @@ NlAttrValidate(const PNL_ATTR nla, const PNL_POLICY 
policy)
 UINT32 minLen;
 UINT32 maxLen;
 UINT32 len;
-BOOLEAN ret = TRUE;
+BOOLEAN ret = FALSE;

 if ((policy-type == NL_A_NO_ATTR) ||
-(policy-type == NL_A_VAR_LEN)) {
+(policy-type == NL_A_VAR_LEN) ||
+(policy-type == NL_A_NESTED)) {
 /* Do not validate anything for attributes of type var length */
-ret = FALSE;
+ret = TRUE;
 goto done;
 }

@@ -894,7 +895,6 @@ NlAttrValidate(const PNL_ATTR nla, const PNL_POLICY policy)
 if (len  minLen || len  maxLen) {
 OVS_LOG_WARN(Attribute: %p, len: %d, not in valid range, 
  min: %d, max: %d, nla, len, minLen, maxLen);
-ret = FALSE;
 goto done;
 }

@@ -902,17 +902,17 @@ NlAttrValidate(const PNL_ATTR nla, const PNL_POLICY 
policy)
 if (policy-type == NL_A_STRING) {
 if (((PCHAR) nla)[nla-nlaLen - 1]) {
 OVS_LOG_WARN(Attributes %p lacks null at the end, nla);
-ret = FALSE;
 goto done;
 }

 if (memchr(nla + 1, '\0', len - 1) != NULL) {
 OVS_LOG_WARN(Attributes %p has bad length, nla);
-ret = FALSE;
 goto done;
 }
 }

+ret = TRUE;
+
 done:
 return ret;
 }
--
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v1 09/10] datapath-windows/Flow.c: FLOW_DEL command handler.

2014-09-25 Thread Samuel Ghinet
Hey Ankur,

A problem I see here with flow delete is that Flow delete requires:
- no attributes (i.e. no key): if flow flush is requested
- key only: if a specific flow key is to be deleted.

When / if masks will be allowed for flows, the mask is expected not to exist.

How does the current code behave if the userspace gives it a flow delete with 
actions (which is invalid)? Will it do a flow set then a flow delete?
As about flow flush, I think that by the current implementation it will crash.

Regards,
Sam

Date: Wed, 24 Sep 2014 00:15:43 -0700
From: Ankur Sharma ankursha...@vmware.com
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH v1 09/10] datapath-windows/Flow.c: FLOW_DEL
command handler.
Message-ID: 1411542944-19374-9-git-send-email-ankursha...@vmware.com

Registered FLOW_DEL command handler. The same command
handler as FLOW_ADD is good enough to handle FLOW_DEL
case as well with minor changes for checking to action
attribute.
---
 datapath-windows/ovsext/Datapath.c | 5 +
 datapath-windows/ovsext/Flow.c | 7 +--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c
index 5008aab..5377f09 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -202,6 +202,11 @@ NETLINK_CMD nlFlowFamilyCmdOps[] = {
   .handler  = OvsFlowNlNewCmdHandler,
   .supportedDevOp   = OVS_TRANSACTION_DEV_OP,
   .validateDpIndex  = FALSE
+},
+{ .cmd  = OVS_FLOW_CMD_DEL,
+  .handler  = OvsFlowNlNewCmdHandler,
+  .supportedDevOp   = OVS_TRANSACTION_DEV_OP,
+  .validateDpIndex  = FALSE
 }
 };

diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
index e170de6..b95f69b 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -358,8 +358,11 @@ _OvsFlowMapNlToFlowPut(POVS_MESSAGE msgIn, PNL_ATTR 
keyAttr,
 mappedFlow);

 /* Map the action */
-mappedFlow-actionsLen = NlAttrGetSize(actionAttr);
-mappedFlow-actions = NlAttrGet(actionAttr);
+if (actionAttr) {
+mappedFlow-actionsLen = NlAttrGetSize(actionAttr);
+mappedFlow-actions = NlAttrGet(actionAttr);
+}
+
 mappedFlow-dpNo = ovsHdr-dp_ifindex;

 _OvsFlowMapNlToFlowPutFlags(genlMsgHdr, flowAttrClear,
--
1.9.1


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] netdev: Fix a typo.

2014-09-25 Thread Alex Wang
Reported-by: Daniel Badea daniel.ba...@windriver.com
Signed-off-by: Alex Wang al...@nicira.com
---
 lib/netdev.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/netdev.c b/lib/netdev.c
index 1fd5121..a9ad7d1 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -682,7 +682,7 @@ netdev_set_multiq(struct netdev *netdev, unsigned int n_txq,
 MAX(n_rxq, 1))
  : EOPNOTSUPP);
 
-if (error != EOPNOTSUPP) {
+if (error  error != EOPNOTSUPP) {
 VLOG_DBG_RL(rl, failed to set tx/rx queue for network device %s:
 %s, netdev_get_name(netdev), ovs_strerror(error));
 }
-- 
1.7.9.5

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 2/3] datapath-windows: Add Netlink vport command get

2014-09-25 Thread Nithin Raju
hi Samuel,
I had some minor comments. Looks good otherwise.

Acked-by: Nithin Raju nit...@vmware.com

On Sep 24, 2014, at 8:42 AM, Samuel Ghinet sghi...@cloudbasesolutions.com 
wrote:

 The transactional get vport command.
 This command uses the netlink transactional errors.
 
 Signed-off-by: Samuel Ghinet sghi...@cloudbasesolutions.com
 ---
 datapath-windows/ovsext/Datapath.c | 83 ++
 1 file changed, 83 insertions(+)
 
 diff --git a/datapath-windows/ovsext/Datapath.c 
 b/datapath-windows/ovsext/Datapath.c
 index 05c06b6..8a8c542 100644
 --- a/datapath-windows/ovsext/Datapath.c
 +++ b/datapath-windows/ovsext/Datapath.c
 @@ -1425,6 +1425,86 @@ OvsGetVportDumpNext(POVS_USER_PARAMS_CONTEXT 
 usrParamsCtx,
 return STATUS_SUCCESS;
 }
 
 +static NTSTATUS
 +OvsGetVport(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
 +UINT32 *replyLen)

All of the vport commands are implemented in Vport.c. Pls. move this to that 
file. Perhaps even the Vport dump. You can move all of them at once in a 
subsequent commit.

 +{
 +NDIS_STATUS status = STATUS_SUCCESS;

NDIS_STATUS = NTSTATUS.

 +LOCK_STATE_EX lockState;
 +
 +POVS_MESSAGE msgIn = (POVS_MESSAGE)usrParamsCtx-inputBuffer;
 +POVS_MESSAGE msgOut = (POVS_MESSAGE)usrParamsCtx-outputBuffer;
 +POVS_VPORT_ENTRY vport = NULL;
 +NL_ERROR nlError = NL_ERROR_SUCCESS;
 +
 +static const NL_POLICY ovsVportPolicy[] = {
 +[OVS_VPORT_ATTR_PORT_NO] = { .type = NL_A_U32 },
 +[OVS_VPORT_ATTR_TYPE] = { .type = NL_A_U32 },
 +[OVS_VPORT_ATTR_NAME] = { .type = NL_A_STRING, .maxLen = IFNAMSIZ },
 +[OVS_VPORT_ATTR_UPCALL_PID] = { .type = NL_A_UNSPEC },
 +[OVS_VPORT_ATTR_STATS] = { .type = NL_A_UNSPEC,
 +   .minLen = sizeof(OVS_VPORT_FULL_STATS),
 +   .maxLen = sizeof(OVS_VPORT_FULL_STATS)
 +},
 +[OVS_VPORT_ATTR_OPTIONS] = { .type = NL_A_NESTED, .optional = TRUE },

Are all the attributes mandatory? I'd think that one of OVS_VPORT_ATTR_PORT_NO 
or OVS_VPORT_ATTR_NAME are mandatory. Rest are optional.

You can see an example here:
dpif_netlink_port_query_by_number()
- dpif_netlink_port_query__()
   - dpif_netlink_vport_transact()

 825 dpif_netlink_vport_init(request);  
 826 request.cmd = OVS_VPORT_CMD_GET;
 827 request.dp_ifindex = dpif-dp_ifindex;  
 828 request.port_no = port_no;  
 829 request.name = port_name;   
 830 
 831 error = dpif_netlink_vport_transact(request, reply, buf);


 +};
 +PNL_ATTR vportAttrs[ARRAY_SIZE(ovsVportPolicy)];
 +
 +/* input buffer has been validated while validating write dev op. */
 +ASSERT(usrParamsCtx-inputBuffer != NULL);
 +
 +if (!NlAttrParse((PNL_MSG_HDR)msgIn,
 +NLMSG_HDRLEN + GENL_HDRLEN + OVS_HDRLEN,
 +ovsVportPolicy, vportAttrs, ARRAY_SIZE(vportAttrs))) {
 +return STATUS_INVALID_PARAMETER;
 +}
 +
 +if (msgOut == NULL || usrParamsCtx-outputLength  sizeof *msgOut) {
 +return STATUS_NDIS_INVALID_LENGTH;
 +}
 +
 +OvsAcquireCtrlLock();
 +if (!gOvsSwitchContext) {
 +OvsReleaseCtrlLock();
 +*replyLen = 0;
 +return STATUS_DATA_NOT_ACCEPTED

I am not sure if 'STATUS_DATA_NOT_ACCEPTED' is the correct error code to be 
returned here. The documentation says:
{Data Not Accepted} The TDI client could not handle the data received during 
an indication.

Returning a transactional error with ENODEV would be appropriate I think, or 
you could return STATUS_INVALID_PARAMETER.


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] rstp.at: Fix intermittent test failure.

2014-09-25 Thread Gurucharan Shetty
On Thu, Sep 25, 2014 at 8:51 AM, Alex Wang al...@nicira.com wrote:
 Sub-test RSTP - dummy interface checks the ovs-vswitchd.log
 output immediately after command execution.  The check may
 fail if the write of new log is delayed by the IO thread.

 This commit fixes the above issue by waiting for the
 ovs-vswitchd.log output.

 Signed-off-by: Alex Wang al...@nicira.com
This clearly fixes a couple of problems for Linux. So
Acked-by: Gurucharan Shetty gshe...@nicira.com

I still see failures on Windows without a time/warp. I will look into
it and send in a separate patch.

 ---
  tests/rstp.at |2 ++
  1 file changed, 2 insertions(+)

 diff --git a/tests/rstp.at b/tests/rstp.at
 index 3f56931..5caa84f 100644
 --- a/tests/rstp.at
 +++ b/tests/rstp.at
 @@ -213,6 +213,7 @@ AT_CHECK([ovs-ofctl add-flow br1 in_port=2 icmp 
 actions=8])

  AT_CHECK([ovs-vsctl set port p2 other_config:rstp-enable=true])

 +OVS_WAIT_UNTIL([test `cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY | wc -l` 
 -ge 2])
  AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY], [0], [dnl
  port p1: RSTP state changed from Disabled to Discarding
  port p2: RSTP state changed from Disabled to Discarding
 @@ -232,6 +233,7 @@ AT_CHECK([ovs-appctl netdev-dummy/set-admin-state p2 up], 
 [], [dnl
  OK
  ])

 +OVS_WAIT_UNTIL([test `cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY | wc -l` 
 -ge 4])
  AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY], [0], [dnl
  port p1: RSTP state changed from Disabled to Discarding
  port p2: RSTP state changed from Disabled to Discarding
 --
 1.7.9.5

 ___
 dev mailing list
 dev@openvswitch.org
 http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] netdev-dpdk: Fix crash when there is no pci numa info.

2014-09-25 Thread Alex Wang
When kernel cannot obtain the pci numa info, the numa_node file
in corresponding pci directory in sysfs will show -1.  Then the
rte_eth_dev_socket_id() function will return it to ovs.  On
current master, ovs assumes rte_eth_dev_socket_id() always
returns non-negative value.  So using this -1 in pmd thread
creation will cause ovs crash.

To fix the above issue, this commit makes ovs always check the
return value of rte_eth_dev_socket_id() and use numa node 0 if
the return value is negative.

Reported-by: Daniel Badea daniel.ba...@windriver.com
Signed-off-by: Alex Wang al...@nicira.com
---
 lib/netdev-dpdk.c |7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 50ea965..9c93768 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -485,13 +485,18 @@ netdev_dpdk_init(struct netdev *netdev_, unsigned int 
port_no)
 OVS_REQUIRES(dpdk_mutex)
 {
 struct netdev_dpdk *netdev = netdev_dpdk_cast(netdev_);
+int sid;
 int err = 0;
 
 ovs_mutex_init(netdev-mutex);
 
 ovs_mutex_lock(netdev-mutex);
 
-netdev-socket_id = rte_eth_dev_socket_id(port_no);
+/* If the 'sid' is negative, it means that the kernel fails
+ * to obtain the pci numa info.  In that situation, always
+ * use 'SOCKET0'. */
+sid = rte_eth_dev_socket_id(port_no);
+netdev-socket_id = sid  0 ? SOCKET0 : sid;
 netdev_dpdk_alloc_txq(netdev, NR_QUEUE);
 netdev-port_id = port_no;
 netdev-flags = 0;
-- 
1.7.9.5

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] rstp.at: Fix intermittent test failure.

2014-09-25 Thread Alex Wang
Thanks, applied to master,

On Thu, Sep 25, 2014 at 12:39 PM, Gurucharan Shetty shet...@nicira.com
wrote:

 On Thu, Sep 25, 2014 at 8:51 AM, Alex Wang al...@nicira.com wrote:
  Sub-test RSTP - dummy interface checks the ovs-vswitchd.log
  output immediately after command execution.  The check may
  fail if the write of new log is delayed by the IO thread.
 
  This commit fixes the above issue by waiting for the
  ovs-vswitchd.log output.
 
  Signed-off-by: Alex Wang al...@nicira.com
 This clearly fixes a couple of problems for Linux. So
 Acked-by: Gurucharan Shetty gshe...@nicira.com

 I still see failures on Windows without a time/warp. I will look into
 it and send in a separate patch.

  ---
   tests/rstp.at |2 ++
   1 file changed, 2 insertions(+)
 
  diff --git a/tests/rstp.at b/tests/rstp.at
  index 3f56931..5caa84f 100644
  --- a/tests/rstp.at
  +++ b/tests/rstp.at
  @@ -213,6 +213,7 @@ AT_CHECK([ovs-ofctl add-flow br1 in_port=2 icmp
 actions=8])
 
   AT_CHECK([ovs-vsctl set port p2 other_config:rstp-enable=true])
 
  +OVS_WAIT_UNTIL([test `cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY | wc
 -l` -ge 2])
   AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY], [0], [dnl
   port p1: RSTP state changed from Disabled to Discarding
   port p2: RSTP state changed from Disabled to Discarding
  @@ -232,6 +233,7 @@ AT_CHECK([ovs-appctl netdev-dummy/set-admin-state p2
 up], [], [dnl
   OK
   ])
 
  +OVS_WAIT_UNTIL([test `cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY | wc
 -l` -ge 4])
   AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY], [0], [dnl
   port p1: RSTP state changed from Disabled to Discarding
   port p2: RSTP state changed from Disabled to Discarding
  --
  1.7.9.5
 
  ___
  dev mailing list
  dev@openvswitch.org
  http://openvswitch.org/mailman/listinfo/dev

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 0/2] ofproto-dpif-rid: Some minor simplifications

2014-09-25 Thread Andy Zhou
Both patches applied. Thanks!

On Wed, Sep 24, 2014 at 8:57 PM, Simon Horman
simon.hor...@netronome.com wrote:
 Hi,

 this short series proposes to minor simplifications to ofproto-dpif-rid.
 They are not related to each other other than that there is a minor
 conflict in applying the second patch if the first one is not present.

 Simon Horman (2):
   ofproto-dpif-rid: remove struct rid_map
   ofproto-dpif-rid: remove unused return value of rid_pool_add()

  ofproto/ofproto-dpif-rid.c | 25 ++---
  1 file changed, 10 insertions(+), 15 deletions(-)

 --
 2.0.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] netdev: Fix a typo.

2014-09-25 Thread Daniele Di Proietto
Acked-by: Daniele Di Proietto ddiproie...@vmware.com

On 9/25/14, 11:45 AM, Alex Wang al...@nicira.com wrote:

Reported-by: Daniel Badea daniel.ba...@windriver.com
Signed-off-by: Alex Wang al...@nicira.com
---
 lib/netdev.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/netdev.c b/lib/netdev.c
index 1fd5121..a9ad7d1 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -682,7 +682,7 @@ netdev_set_multiq(struct netdev *netdev, unsigned int
n_txq,
 MAX(n_rxq, 1))
  : EOPNOTSUPP);
 
-if (error != EOPNOTSUPP) {
+if (error  error != EOPNOTSUPP) {
 VLOG_DBG_RL(rl, failed to set tx/rx queue for network device
%s:
 %s, netdev_get_name(netdev), ovs_strerror(error));
 }
-- 
1.7.9.5

___
dev mailing list
dev@openvswitch.org
https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/
listinfo/devk=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0Ar=MV9BdLjtFIdhBDBaw5z%2BU
6SSA2gAfY4L%2F1HCy3VjlKU%3D%0Am=x04ATQHoCiTD1f5JswJX0vXwQ%2F%2B3yw7COfQwE
y%2Fu4aw%3D%0As=c1b50f48e089f7096a31556d7f01ed3d794f3dd8462ba5749c078a0ce
30b7f32

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] netdev: Fix a typo.

2014-09-25 Thread Alex Wang
On Thu, Sep 25, 2014 at 2:00 PM, Daniele Di Proietto ddiproie...@vmware.com
 wrote:

 Acked-by: Daniele Di Proietto ddiproie...@vmware.com


Thanks, will apply it soon, also will change the name to 'fix error check'
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] netdev-dpdk: Fix crash when there is no pci numa info.

2014-09-25 Thread Daniele Di Proietto
Thanks for the fix!

Acked-by: Daniele Di Proietto ddiproie...@vmware.com

On 9/25/14, 1:28 PM, Alex Wang al...@nicira.com wrote:

When kernel cannot obtain the pci numa info, the numa_node file
in corresponding pci directory in sysfs will show -1.  Then the
rte_eth_dev_socket_id() function will return it to ovs.  On
current master, ovs assumes rte_eth_dev_socket_id() always
returns non-negative value.  So using this -1 in pmd thread
creation will cause ovs crash.

To fix the above issue, this commit makes ovs always check the
return value of rte_eth_dev_socket_id() and use numa node 0 if
the return value is negative.

Reported-by: Daniel Badea daniel.ba...@windriver.com
Signed-off-by: Alex Wang al...@nicira.com
---
 lib/netdev-dpdk.c |7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 50ea965..9c93768 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -485,13 +485,18 @@ netdev_dpdk_init(struct netdev *netdev_, unsigned
int port_no)
 OVS_REQUIRES(dpdk_mutex)
 {
 struct netdev_dpdk *netdev = netdev_dpdk_cast(netdev_);
+int sid;
 int err = 0;
 
 ovs_mutex_init(netdev-mutex);
 
 ovs_mutex_lock(netdev-mutex);
 
-netdev-socket_id = rte_eth_dev_socket_id(port_no);
+/* If the 'sid' is negative, it means that the kernel fails
+ * to obtain the pci numa info.  In that situation, always
+ * use 'SOCKET0'. */
+sid = rte_eth_dev_socket_id(port_no);
+netdev-socket_id = sid  0 ? SOCKET0 : sid;
 netdev_dpdk_alloc_txq(netdev, NR_QUEUE);
 netdev-port_id = port_no;
 netdev-flags = 0;
-- 
1.7.9.5

___
dev mailing list
dev@openvswitch.org
https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/
listinfo/devk=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0Ar=MV9BdLjtFIdhBDBaw5z%2BU
6SSA2gAfY4L%2F1HCy3VjlKU%3D%0Am=X596LDIOn3vB8hKejLfNln2Hv%2FE45SfcQeQbYSI
zMtM%3D%0As=0c07569815c01a727f199675214802aa8c941cbffb74b700ea79174397c7c
ab3

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] netdev-dpdk: Fix crash when there is no pci numa info.

2014-09-25 Thread Justin Pettit
Don't forget to add Daniel to AUTHORS.

--Justin


On September 25, 2014 at 1:27:30 PM, Alex Wang (al...@nicira.com) wrote:
 When kernel cannot obtain the pci numa info, the numa_node file
 in corresponding pci directory in sysfs will show -1. Then the
 rte_eth_dev_socket_id() function will return it to ovs. On
 current master, ovs assumes rte_eth_dev_socket_id() always
 returns non-negative value. So using this -1 in pmd thread
 creation will cause ovs crash.
 
 To fix the above issue, this commit makes ovs always check the
 return value of rte_eth_dev_socket_id() and use numa node 0 if
 the return value is negative.
 
 Reported-by: Daniel Badea 
 Signed-off-by: Alex Wang 
 ---
 lib/netdev-dpdk.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)
 
 diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
 index 50ea965..9c93768 100644
 --- a/lib/netdev-dpdk.c
 +++ b/lib/netdev-dpdk.c
 @@ -485,13 +485,18 @@ netdev_dpdk_init(struct netdev *netdev_, unsigned int 
 port_no) 
 OVS_REQUIRES(dpdk_mutex)
 {
 struct netdev_dpdk *netdev = netdev_dpdk_cast(netdev_);
 + int sid;
 int err = 0;
 
 ovs_mutex_init(netdev-mutex);
 
 ovs_mutex_lock(netdev-mutex);
 
 - netdev-socket_id = rte_eth_dev_socket_id(port_no);
 + /* If the 'sid' is negative, it means that the kernel fails
 + * to obtain the pci numa info. In that situation, always
 + * use 'SOCKET0'. */
 + sid = rte_eth_dev_socket_id(port_no);
 + netdev-socket_id = sid  0 ? SOCKET0 : sid;
 netdev_dpdk_alloc_txq(netdev, NR_QUEUE);
 netdev-port_id = port_no;
 netdev-flags = 0;
 --
 1.7.9.5
 
 ___
 dev mailing list
 dev@openvswitch.org
 http://openvswitch.org/mailman/listinfo/dev
 

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] netdev-dpdk: Fix crash when there is no pci numa info.

2014-09-25 Thread Alex Wang
Thx, Daniele also reminded me of that, will fold that in with the patch.

On Thu, Sep 25, 2014 at 2:07 PM, Justin Pettit jpet...@nicira.com wrote:

 Don't forget to add Daniel to AUTHORS.

 --Justin


 On September 25, 2014 at 1:27:30 PM, Alex Wang (al...@nicira.com) wrote:
  When kernel cannot obtain the pci numa info, the numa_node file
  in corresponding pci directory in sysfs will show -1. Then the
  rte_eth_dev_socket_id() function will return it to ovs. On
  current master, ovs assumes rte_eth_dev_socket_id() always
  returns non-negative value. So using this -1 in pmd thread
  creation will cause ovs crash.
 
  To fix the above issue, this commit makes ovs always check the
  return value of rte_eth_dev_socket_id() and use numa node 0 if
  the return value is negative.
 
  Reported-by: Daniel Badea
  Signed-off-by: Alex Wang
  ---
  lib/netdev-dpdk.c | 7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)
 
  diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
  index 50ea965..9c93768 100644
  --- a/lib/netdev-dpdk.c
  +++ b/lib/netdev-dpdk.c
  @@ -485,13 +485,18 @@ netdev_dpdk_init(struct netdev *netdev_, unsigned
 int port_no)
  OVS_REQUIRES(dpdk_mutex)
  {
  struct netdev_dpdk *netdev = netdev_dpdk_cast(netdev_);
  + int sid;
  int err = 0;
 
  ovs_mutex_init(netdev-mutex);
 
  ovs_mutex_lock(netdev-mutex);
 
  - netdev-socket_id = rte_eth_dev_socket_id(port_no);
  + /* If the 'sid' is negative, it means that the kernel fails
  + * to obtain the pci numa info. In that situation, always
  + * use 'SOCKET0'. */
  + sid = rte_eth_dev_socket_id(port_no);
  + netdev-socket_id = sid  0 ? SOCKET0 : sid;
  netdev_dpdk_alloc_txq(netdev, NR_QUEUE);
  netdev-port_id = port_no;
  netdev-flags = 0;
  --
  1.7.9.5
 
  ___
  dev mailing list
  dev@openvswitch.org
  http://openvswitch.org/mailman/listinfo/dev
 


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v3 1/6] README.ovs-vtep: Remotes can be connected through VTEP's manager table.

2014-09-25 Thread Justin Pettit
Acked-by: Justin Pettit jpet...@nicira.com


On Fri, Sep 19, 2014 at 7:29 AM, Gurucharan Shetty shet...@nicira.com
wrote:

 Reported-by: Ziyou Wang ziy...@vmware.com
 Signed-off-by: Gurucharan Shetty gshe...@nicira.com
 ---
  AUTHORS  |1 +
  vtep/README.ovs-vtep |1 +
  2 files changed, 2 insertions(+)

 diff --git a/AUTHORS b/AUTHORS
 index e3fe7ba..e2db8db 100644
 --- a/AUTHORS
 +++ b/AUTHORS
 @@ -297,6 +297,7 @@ Voravit T.  vora...@kth.se
  Yeming Zhao zhaoyem...@gmail.com
  Ying Chen   yingc...@vmware.com
  Yongqiang Liu   liuyq7...@gmail.com
 +Ziyou Wang  ziy...@vmware.com
  ankur dwivedi   ankurengg2...@gmail.com
  chen zhang  3zhangchen9...@gmail.com
  kk yap  yap...@stanford.edu
 diff --git a/vtep/README.ovs-vtep b/vtep/README.ovs-vtep
 index 60b39b7..5ce63c7 100644
 --- a/vtep/README.ovs-vtep
 +++ b/vtep/README.ovs-vtep
 @@ -95,6 +95,7 @@ step 2 of the Requirements section.

  ovsdb-server --pidfile --detach --log-file \
--remote punix:/var/run/openvswitch/db.sock \
 +  --remote=db:hardware_vtep,Global,managers \
/etc/openvswitch/ovs.db /etc/openvswitch/vtep.db

  3. Start OVS as normal:
 --
 1.7.9.5

 ___
 dev mailing list
 dev@openvswitch.org
 http://openvswitch.org/mailman/listinfo/dev

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v3 2/6] vtep-ctl: Add Tunnel table to vtep_ctl_table_class.

2014-09-25 Thread Justin Pettit
Acked-by: Justin Pettit jpet...@nicira.com


On Fri, Sep 19, 2014 at 7:29 AM, Gurucharan Shetty shet...@nicira.com
wrote:

 This is needed to create, get, set records in the Tunnel table.

 (We need to add the Tunnel table's 'local' and 'remote' columns
 that point to the Physical_Locator record to cache because vtep-ctl
 commands like 'add-ucast-local' will try to add an entry in
 Physical_Locator table based on the contents of the cache.)

 Signed-off-by: Gurucharan Shetty gshe...@nicira.com
 ---
  vtep/vtep-ctl.c |   18 ++
  1 file changed, 18 insertions(+)

 diff --git a/vtep/vtep-ctl.c b/vtep/vtep-ctl.c
 index 8a16450..b3ff671 100644
 --- a/vtep/vtep-ctl.c
 +++ b/vtep/vtep-ctl.c
 @@ -1076,6 +1076,7 @@ pre_get_info(struct vtep_ctl_context *ctx)

  ovsdb_idl_add_column(ctx-idl, vteprec_physical_switch_col_name);
  ovsdb_idl_add_column(ctx-idl, vteprec_physical_switch_col_ports);
 +ovsdb_idl_add_column(ctx-idl, vteprec_physical_switch_col_tunnels);

  ovsdb_idl_add_column(ctx-idl, vteprec_physical_port_col_name);
  ovsdb_idl_add_column(ctx-idl,
 vteprec_physical_port_col_vlan_bindings);
 @@ -,6 +1112,9 @@ pre_get_info(struct vtep_ctl_context *ctx)
   vteprec_physical_locator_col_dst_ip);
  ovsdb_idl_add_column(ctx-idl,

 vteprec_physical_locator_col_encapsulation_type);
 +
 +ovsdb_idl_add_column(ctx-idl, vteprec_tunnel_col_local);
 +ovsdb_idl_add_column(ctx-idl, vteprec_tunnel_col_remote);
  }

  static void
 @@ -1122,6 +1126,7 @@ vtep_ctl_context_populate_cache(struct
 vtep_ctl_context *ctx)
  const struct vteprec_ucast_macs_remote *ucast_remote_cfg;
  const struct vteprec_mcast_macs_local *mcast_local_cfg;
  const struct vteprec_mcast_macs_remote *mcast_remote_cfg;
 +const struct vteprec_tunnel *tunnel_cfg;
  struct sset pswitches, ports, lswitches;
  size_t i;

 @@ -1247,6 +1252,15 @@ vtep_ctl_context_populate_cache(struct
 vtep_ctl_context *ctx)
  mcast_mac-remote_cfg = mcast_remote_cfg;
  }

 +VTEPREC_TUNNEL_FOR_EACH (tunnel_cfg, ctx-idl) {
 +if (tunnel_cfg-local) {
 +add_ploc_to_cache(ctx, tunnel_cfg-local);
 +}
 +if (tunnel_cfg-remote) {
 +add_ploc_to_cache(ctx, tunnel_cfg-remote);
 +}
 +}
 +
  sset_init(pswitches);
  for (i = 0; i  vtep_global-n_switches; i++) {
  struct vteprec_physical_switch *ps_cfg = vtep_global-switches[i];
 @@ -2283,6 +2297,10 @@ static const struct vtep_ctl_table_class tables[] =
 {
   {{vteprec_table_physical_switch, vteprec_physical_switch_col_name,
 NULL},
{NULL, NULL, NULL}}},

 +{vteprec_table_tunnel,
 + {{NULL, NULL, NULL},
 +  {NULL, NULL, NULL}}},
 +
  {NULL, {{NULL, NULL, NULL}, {NULL, NULL, NULL}}}
  };

 --
 1.7.9.5

 ___
 dev mailing list
 dev@openvswitch.org
 http://openvswitch.org/mailman/listinfo/dev

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v3 3/6] ovs-vtep: Clear left-over local mac information.

2014-09-25 Thread Justin Pettit
Acked-by: Justin Pettit jpet...@nicira.com


On Fri, Sep 19, 2014 at 7:29 AM, Gurucharan Shetty shet...@nicira.com
wrote:

 Before destroying a logical switch, cleanup any left over local
 mac information in Ucast_Macs_Local or Mcast_Macs_Local table.
 We need to do this to atleast cleanup the 'unknown-dst' information
 added in the Mcast_Macs_Local table while creating the Logical_Switch
 class in setup_ls().

 Signed-off-by: Gurucharan Shetty gshe...@nicira.com
 ---
  vtep/ovs-vtep |1 +
  1 file changed, 1 insertion(+)

 diff --git a/vtep/ovs-vtep b/vtep/ovs-vtep
 index ea233e0..ea21794 100755
 --- a/vtep/ovs-vtep
 +++ b/vtep/ovs-vtep
 @@ -424,6 +424,7 @@ def handle_physical(ps_name):

  if not len(ls.ports):
  ovs_vsctl(del-br %s % Lswitches[ls_name].short_name)
 +vtep_ctl(clear-local-macs %s % Lswitches[ls_name].name)
  del Lswitches[ls_name]

  def setup(ps_name):
 --
 1.7.9.5

 ___
 dev mailing list
 dev@openvswitch.org
 http://openvswitch.org/mailman/listinfo/dev

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH v3 1/4] datapath-windows: fix OVS_VPORT_TYPE

2014-09-25 Thread Samuel Ghinet
The windows ovs kernel uses an OVS_VPORT_TYPE enum that is incompatible with
the userspace counterpart (enum ovs_vport_type from openvswitch.h). We must use
the same enum type for the netlink communication to work properly.

This patch makes the fix: typedef enum ovs_vport_type OVS_VPORT_TYPE and
changes the afferent kernel driver code:
o) vport types synthetic and emulated turn to: netdev
o) vport type internal turns to: internal
o) vport type external truns to: netdev (plus, we hold a field in vport,
isExternal

Signed-off-by: Samuel Ghinet sghi...@cloudbasesolutions.com
Acked-by: Nithin Raju nit...@vmware.com
---
 datapath-windows/include/OvsDpInterfaceExt.h |  1 +
 datapath-windows/include/OvsPub.h| 25 ++---
 datapath-windows/ovsext/Actions.c|  6 +--
 datapath-windows/ovsext/Tunnel.c |  4 +-
 datapath-windows/ovsext/Vport.c  | 81 ++--
 datapath-windows/ovsext/Vport.h  | 11 ++--
 datapath-windows/ovsext/Vxlan.c  |  4 +-
 datapath-windows/ovsext/precomp.h| 12 ++---
 8 files changed, 66 insertions(+), 78 deletions(-)

diff --git a/datapath-windows/include/OvsDpInterfaceExt.h 
b/datapath-windows/include/OvsDpInterfaceExt.h
index 64fd20e..72b2e8e 100644
--- a/datapath-windows/include/OvsDpInterfaceExt.h
+++ b/datapath-windows/include/OvsDpInterfaceExt.h
@@ -81,5 +81,6 @@ enum ovs_nl_mcast_attr {
 };
 
 typedef struct ovs_dp_stats OVS_DP_STATS;
+typedef enum ovs_vport_type OVS_VPORT_TYPE;
 
 #endif /* __OVS_DP_INTERFACE_EXT_H_ */
diff --git a/datapath-windows/include/OvsPub.h 
b/datapath-windows/include/OvsPub.h
index 36814c4..ff0e8ce 100644
--- a/datapath-windows/include/OvsPub.h
+++ b/datapath-windows/include/OvsPub.h
@@ -142,34 +142,19 @@ typedef struct _OVS_VPORT_GET {
 char name[OVS_MAX_PORT_NAME_LENGTH];
 } OVS_VPORT_GET, *POVS_VPORT_GET;
 
-
-typedef enum {
-OVSWIN_VPORT_TYPE_UNKNOWN,
-OVSWIN_VPORT_TYPE_RESERVED,
-OVSWIN_VPORT_TYPE_EXTERNAL,
-OVSWIN_VPORT_TYPE_INTERNAL,
-OVSWIN_VPORT_TYPE_SYNTHETIC,
-OVSWIN_VPORT_TYPE_EMULATED,
-OVSWIN_VPORT_TYPE_GRE,
-OVSWIN_VPORT_TYPE_GRE64,
-OVSWIN_VPORT_TYPE_VXLAN,
-OVSWIN_VPORT_TYPE_LOCAL,/* For bridge local port. */
-} OVS_VPORT_TYPE;
-
 static __inline const char *
 OvsVportTypeToStr(OVS_VPORT_TYPE t)
 {
 switch(t) {
-#define STR(t) case OVSWIN_VPORT_TYPE_##t : return VPORT_##t;
-STR(UNKNOWN)
-STR(EXTERNAL)
+#define STR(t) case OVS_VPORT_TYPE_##t : return VPORT_##t;
+STR(UNSPEC)
+STR(NETDEV)
 STR(INTERNAL)
-STR(SYNTHETIC)
-STR(EMULATED)
 STR(GRE)
 STR(GRE64)
 STR(VXLAN)
-STR(LOCAL)
+STR(GENEVE)
+STR(LISP)
 }
 #undef STR
 
diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index 35ebfdf..180b6b8 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -205,7 +205,7 @@ OvsDetectTunnelRxPkt(OvsForwardingContext *ovsFwdCtx,
 if (!flowKey-ipKey.nwFrag 
 flowKey-ipKey.nwProto == IPPROTO_UDP 
 flowKey-ipKey.l4.tpDst == VXLAN_UDP_PORT_NBO) {
-tunnelVport = OvsGetTunnelVport(OVSWIN_VPORT_TYPE_VXLAN);
+tunnelVport = OvsGetTunnelVport(OVS_VPORT_TYPE_VXLAN);
 ovsActionStats.rxVxlan++;
 }
 
@@ -616,7 +616,7 @@ OvsTunnelPortTx(OvsForwardingContext *ovsFwdCtx)
 
 /* Do the encap. Encap function does not consume the NBL. */
 switch(ovsFwdCtx-tunnelTxNic-ovsType) {
-case OVSWIN_VPORT_TYPE_VXLAN:
+case OVS_VPORT_TYPE_VXLAN:
 status = OvsEncapVxlan(ovsFwdCtx-curNbl, ovsFwdCtx-tunKey,
ovsFwdCtx-switchContext,
(VOID *)ovsFwdCtx-completionList,
@@ -678,7 +678,7 @@ OvsTunnelPortRx(OvsForwardingContext *ovsFwdCtx)
 }
 
 switch(tunnelRxVport-ovsType) {
-case OVSWIN_VPORT_TYPE_VXLAN:
+case OVS_VPORT_TYPE_VXLAN:
 /*
  * OvsDoDecapVxlan should return a new NBL if it was copied, and
  * this new NBL should be setup as the ovsFwdCtx-curNbl.
diff --git a/datapath-windows/ovsext/Tunnel.c b/datapath-windows/ovsext/Tunnel.c
index 2e7da10..304ab59 100644
--- a/datapath-windows/ovsext/Tunnel.c
+++ b/datapath-windows/ovsext/Tunnel.c
@@ -284,14 +284,14 @@ OvsInjectPacketThroughActions(PNET_BUFFER_LIST pNbl,
 
 SendFlags |= NDIS_SEND_FLAGS_DISPATCH_LEVEL;
 
-vport = OvsGetTunnelVport(OVSWIN_VPORT_TYPE_VXLAN);
+vport = OvsGetTunnelVport(OVS_VPORT_TYPE_VXLAN);
 
 if (vport == NULL){
 status = STATUS_UNSUCCESSFUL;
 goto unlockAndDrop;
 }
 
-ASSERT(vport-ovsType == OVSWIN_VPORT_TYPE_VXLAN);
+ASSERT(vport-ovsType == OVS_VPORT_TYPE_VXLAN);
 
 portNo = vport-portNo;
 
diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c
index 0d1e4ab..14b68d9 100644
--- a/datapath-windows/ovsext/Vport.c
+++ b/datapath-windows/ovsext/Vport.c
@@ -50,7 

Re: [ovs-dev] [PATCH v3 4/6] ovs-vtep: Store physical switch name globally.

2014-09-25 Thread Justin Pettit
Acked-by: Justin Pettit jpet...@nicira.com


On Fri, Sep 19, 2014 at 7:29 AM, Gurucharan Shetty shet...@nicira.com
wrote:

 ovs-vtep is an emulator and it works only on one
 physical switch. This switch name is stored in the variable
 'ps_name' and then passed around. An upcoming commit requires
 access to this variable at more places and it is easier if this
 variable is global.

 Signed-off-by: Gurucharan Shetty gshe...@nicira.com
 ---
  vtep/ovs-vtep |   20 +++-
  1 file changed, 11 insertions(+), 9 deletions(-)

 diff --git a/vtep/ovs-vtep b/vtep/ovs-vtep
 index ea21794..c70ed64 100755
 --- a/vtep/ovs-vtep
 +++ b/vtep/ovs-vtep
 @@ -38,6 +38,7 @@ __pychecker__ = 'no-reuseattr'  # Remove in pychecker =
 0.8.19.
  vlog = ovs.vlog.Vlog(ovs-vtep)
  exiting = False

 +ps_name = 
  Tunnel_Ip = 
  Lswitches = {}
  Bindings = {}
 @@ -307,7 +308,7 @@ class Logical_Switch(object):
  self.update_remote_macs()
  self.update_stats()

 -def add_binding(ps_name, binding, ls):
 +def add_binding(binding, ls):
  vlog.info(adding binding %s % binding)

  vlan, pp_name = binding.split(-, 1)
 @@ -348,7 +349,7 @@ def add_binding(ps_name, binding, ls):
  ls.add_lbinding(lbinding)
  Bindings[binding] = ls.name

 -def del_binding(ps_name, binding, ls):
 +def del_binding(binding, ls):
  vlog.info(removing binding %s % binding)

  vlan, pp_name = binding.split(-)
 @@ -377,7 +378,7 @@ def del_binding(ps_name, binding, ls):

  del Bindings[binding]

 -def handle_physical(ps_name):
 +def handle_physical():
  # Gather physical ports except the patch ports we created
  ovs_ports = ovs_vsctl(list-ports %s % ps_name).split()
  ovs_port_set = set([port for port in ovs_ports if port[-2:] != -p])
 @@ -410,9 +411,9 @@ def handle_physical(ps_name):
  if Bindings[binding] == ls_name:
  continue
  else:
 -del_binding(ps_name, binding,
 Lswitches[Bindings[binding]])
 +del_binding(binding, Lswitches[Bindings[binding]])

 -add_binding(ps_name, binding, ls)
 +add_binding(binding, ls)


  dead_bindings = set(Bindings.keys()).difference(new_bindings)
 @@ -420,14 +421,14 @@ def handle_physical(ps_name):
  ls_name = Bindings[binding]
  ls = Lswitches[ls_name]

 -del_binding(ps_name, binding, ls)
 +del_binding(binding, ls)

  if not len(ls.ports):
  ovs_vsctl(del-br %s % Lswitches[ls_name].short_name)
  vtep_ctl(clear-local-macs %s % Lswitches[ls_name].name)
  del Lswitches[ls_name]

 -def setup(ps_name):
 +def setup():
  br_list = ovs_vsctl(list-br).split()
  if (ps_name not in br_list):
  ovs.util.ovs_fatal(0, couldn't find OVS bridge %s % ps_name,
 vlog)
 @@ -485,6 +486,7 @@ def main():
  if args.root_prefix:
  root_prefix = args.root_prefix

 +global ps_name
  ps_name = args.ps_name

  ovs.daemon.daemonize()
 @@ -495,14 +497,14 @@ def main():
  if error:
  ovs.util.ovs_fatal(error, could not create unixctl server, vlog)

 -setup(ps_name)
 +setup()

  while True:
  unixctl.run()
  if exiting:
  break

 -handle_physical(ps_name)
 +handle_physical()

  for ls_name, ls in Lswitches.items():
  ls.run()
 --
 1.7.9.5

 ___
 dev mailing list
 dev@openvswitch.org
 http://openvswitch.org/mailman/listinfo/dev

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH v4 2/4] Netlink command: vport dump

2014-09-25 Thread Samuel Ghinet
Functionality for vport dump.
Later, when we will add more netlink dump commands, some common code will need
to be split to functions.

Notes:
a) the current implementation of vport assumes the datapath feature
multiple upcall pids is not used. A single upcall pid is used now.
c) the vxlan destination udp port is currently a constant. When it will become
configurable, the vport options netlink attribute will become relevant.

Signed-off-by: Samuel Ghinet sghi...@cloudbasesolutions.com
Acked-by: Alin Gabriel Serdean aserd...@cloudbasesolutions.com
Acked-by: Nithin Raju nit...@vmware.com
---
 datapath-windows/ovsext/Datapath.c | 273 -
 datapath-windows/ovsext/Vport.h|  11 +-
 2 files changed, 276 insertions(+), 8 deletions(-)

diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c
index 0dfdd57..25d6f87 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -99,7 +99,8 @@ static NetlinkCmdHandler OvsGetPidCmdHandler,
  OvsGetDpCmdHandler,
  OvsPendEventCmdHandler,
  OvsSubscribeEventCmdHandler,
- OvsSetDpCmdHandler;
+ OvsSetDpCmdHandler,
+ OvsGetVportCmdHandler;
 
 static NTSTATUS HandleGetDpTransaction(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
UINT32 *replyLen);
@@ -180,14 +181,22 @@ NETLINK_FAMILY nlPacketFamilyOps = {
 };
 
 /* Netlink vport family. */
-/* XXX: Add commands here. */
+NETLINK_CMD nlVportFamilyCmdOps[] = {
+{ .cmd = OVS_VPORT_CMD_GET,
+  .handler = OvsGetVportCmdHandler,
+  .supportedDevOp = OVS_WRITE_DEV_OP | OVS_READ_DEV_OP |
+OVS_TRANSACTION_DEV_OP,
+  .validateDpIndex = TRUE
+}
+};
+
 NETLINK_FAMILY nlVportFamilyOps = {
 .name = OVS_VPORT_FAMILY,
 .id   = OVS_WIN_NL_VPORT_FAMILY_ID,
 .version  = OVS_VPORT_VERSION,
 .maxAttr  = OVS_VPORT_ATTR_MAX,
-.cmds = NULL, /* XXX: placeholder. */
-.opsCount = 0
+.cmds = nlVportFamilyCmdOps,
+.opsCount = ARRAY_SIZE(nlVportFamilyCmdOps)
 };
 
 /* Netlink flow family. */
@@ -691,10 +700,11 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject,
 break;
 case OVS_WIN_NL_PACKET_FAMILY_ID:
 case OVS_WIN_NL_FLOW_FAMILY_ID:
-case OVS_WIN_NL_VPORT_FAMILY_ID:
 status = STATUS_NOT_IMPLEMENTED;
 goto done;
-
+case OVS_WIN_NL_VPORT_FAMILY_ID:
+nlFamilyOps = nlVportFamilyOps;
+break;
 default:
 status = STATUS_INVALID_PARAMETER;
 goto done;
@@ -1179,6 +1189,257 @@ OvsSetupDumpStart(POVS_USER_PARAMS_CONTEXT usrParamsCtx)
 return InitUserDumpState(instance, msgIn);
 }
 
+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 = nlDatapathFamilyOps.version;
+msgOut-genlMsg.reserved = 0;
+}
+
+static VOID
+BuildReplyMsgFromMsgIn(POVS_MESSAGE msgIn, POVS_MESSAGE msgOut, UINT16 flags)
+{
+BuildMsgOut(msgIn, msgOut, msgIn-nlMsg.nlmsgType, sizeof(OVS_MESSAGE),
+flags);
+}
+
+static NTSTATUS
+OvsCreateMsgFromVport(POVS_VPORT_ENTRY vport,
+  POVS_MESSAGE msgIn,
+  PVOID outBuffer,
+  UINT32 outBufLen,
+  int dpIfIndex)
+{
+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);
+if (!ok) {
+return STATUS_INSUFFICIENT_RESOURCES;
+}
+
+ok = NlMsgPutTailU32(nlBuffer, OVS_VPORT_ATTR_PORT_NO, vport-portNo);
+if (!ok) {
+return STATUS_INSUFFICIENT_RESOURCES;
+}
+
+ok = NlMsgPutTailU32(nlBuffer, OVS_VPORT_ATTR_TYPE, vport-ovsType);
+if (!ok) {
+return STATUS_INSUFFICIENT_RESOURCES;
+}
+
+ok = NlMsgPutTailString(nlBuffer, OVS_VPORT_ATTR_NAME, vport-ovsName);
+if (!ok) {
+return STATUS_INSUFFICIENT_RESOURCES;
+}
+
+/*
+ * XXX: when we implement OVS_DP_ATTR_USER_FEATURES in datapath,
+ * we'll need to check the OVS_DP_F_VPORT_PIDS flag: if it is set,
+ * it means we have an array of pids, instead of a single pid.
+ * ATM we assume we have one pid only.
+*/
+
+ok = NlMsgPutTailU32(nlBuffer, OVS_VPORT_ATTR_UPCALL_PID,
+ vport-upcallPid);
+if (!ok) {
+

Re: [ovs-dev] [PATCH v3 5/6] ovs-vtep: Use shlex module to split args.

2014-09-25 Thread Justin Pettit
Acked-by: Justin Pettit jpet...@nicira.com


On Fri, Sep 19, 2014 at 7:29 AM, Gurucharan Shetty shet...@nicira.com
wrote:

 string.split() function splits a quoted string if there is a whitespace
 inside the quote.
 ex: The following code snippet will output ['printing', 'No',
 'Diagnostic']
 args = 'printing No Diagnostic'
 print args.split()

 The above is a problem if we run the following command through vtep_ctl().
 vtep-ctl set tunnel $uuid bfd_status:diagnostic=No Diagnostic

 The workaround is to use the split() function from shlex module.

 Signed-off-by: Gurucharan Shetty gshe...@nicira.com
 ---
  vtep/ovs-vtep |7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)

 diff --git a/vtep/ovs-vtep b/vtep/ovs-vtep
 index c70ed64..0b855bb 100755
 --- a/vtep/ovs-vtep
 +++ b/vtep/ovs-vtep
 @@ -18,6 +18,7 @@

  import argparse
  import re
 +import shlex
  import subprocess
  import sys
  import time
 @@ -55,13 +56,13 @@ def call_prog(prog, args_list):
  return output

  def ovs_vsctl(args):
 -return call_prog(ovs-vsctl, args.split())
 +return call_prog(ovs-vsctl, shlex.split(args))

  def ovs_ofctl(args):
 -return call_prog(ovs-ofctl, args.split())
 +return call_prog(ovs-ofctl, shlex.split(args))

  def vtep_ctl(args):
 -return call_prog(vtep-ctl, args.split())
 +return call_prog(vtep-ctl, shlex.split(args))


  def unixctl_exit(conn, unused_argv, unused_aux):
 --
 1.7.9.5

 ___
 dev mailing list
 dev@openvswitch.org
 http://openvswitch.org/mailman/listinfo/dev

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH v2 4/4] Add Netlink vport command get

2014-09-25 Thread Samuel Ghinet
The transactional get vport command.
This command uses the netlink transactional errors.

Signed-off-by: Samuel Ghinet sghi...@cloudbasesolutions.com
Acked-by: Nithin Raju nit...@vmware.com
---
 datapath-windows/ovsext/Datapath.c | 83 +-
 1 file changed, 82 insertions(+), 1 deletion(-)

diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c
index b038026..2091777 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -1425,6 +1425,83 @@ OvsGetVportDumpNext(POVS_USER_PARAMS_CONTEXT 
usrParamsCtx,
 return STATUS_SUCCESS;
 }
 
+static NTSTATUS
+OvsGetVport(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
+UINT32 *replyLen)
+{
+NTSTATUS status = STATUS_SUCCESS;
+LOCK_STATE_EX lockState;
+
+POVS_MESSAGE msgIn = (POVS_MESSAGE)usrParamsCtx-inputBuffer;
+POVS_MESSAGE msgOut = (POVS_MESSAGE)usrParamsCtx-outputBuffer;
+POVS_VPORT_ENTRY vport = NULL;
+NL_ERROR nlError = NL_ERROR_SUCCESS;
+
+static const NL_POLICY ovsVportPolicy[] = {
+[OVS_VPORT_ATTR_PORT_NO] = { .type = NL_A_U32, .optional = TRUE },
+[OVS_VPORT_ATTR_NAME] = { .type = NL_A_STRING,
+  .maxLen = IFNAMSIZ,
+  .optional = TRUE},
+};
+PNL_ATTR vportAttrs[ARRAY_SIZE(ovsVportPolicy)];
+
+/* input buffer has been validated while validating write dev op. */
+ASSERT(usrParamsCtx-inputBuffer != NULL);
+
+if (!NlAttrParse((PNL_MSG_HDR)msgIn,
+NLMSG_HDRLEN + GENL_HDRLEN + OVS_HDRLEN,
+ovsVportPolicy, vportAttrs, ARRAY_SIZE(vportAttrs))) {
+return STATUS_INVALID_PARAMETER;
+}
+
+if (msgOut == NULL || usrParamsCtx-outputLength  sizeof *msgOut) {
+return STATUS_INVALID_BUFFER_SIZE;
+}
+
+OvsAcquireCtrlLock();
+if (!gOvsSwitchContext) {
+OvsReleaseCtrlLock();
+return STATUS_INVALID_PARAMETER;
+}
+OvsReleaseCtrlLock();
+
+if (vportAttrs[OVS_VPORT_ATTR_NAME] != NULL) {
+vport = OvsFindVportByOvsName(gOvsSwitchContext,
+NlAttrGet(vportAttrs[OVS_VPORT_ATTR_NAME]),
+NlAttrGetSize(vportAttrs[OVS_VPORT_ATTR_NAME]));
+} else if (vportAttrs[OVS_VPORT_ATTR_PORT_NO] != NULL) {
+vport = OvsFindVportByPortNo(gOvsSwitchContext,
+NlAttrGetU32(vportAttrs[OVS_VPORT_ATTR_PORT_NO]));
+} else {
+nlError = NL_ERROR_INVAL;
+goto Cleanup;
+}
+
+if (!vport) {
+nlError = NL_ERROR_NODEV;
+goto Cleanup;
+}
+
+NdisAcquireRWLockRead(gOvsSwitchContext-dispatchLock, lockState, 0);
+status = OvsCreateMsgFromVport(vport, msgIn, usrParamsCtx-outputBuffer,
+   usrParamsCtx-outputLength,
+   gOvsSwitchContext-dpNo);
+NdisReleaseRWLock(gOvsSwitchContext-dispatchLock, lockState);
+
+*replyLen = msgOut-nlMsg.nlmsgLen;
+
+Cleanup:
+if (nlError != NL_ERROR_SUCCESS) {
+POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)
+usrParamsCtx-outputBuffer;
+
+BuildErrorMsg(msgIn, msgError, nlError);
+*replyLen = msgError-nlMsg.nlmsgLen;
+}
+
+return STATUS_SUCCESS;
+}
+
 /*
  * --
  *  Handler for the get vport command. The function handles the initial call to
@@ -1435,15 +1512,19 @@ static NTSTATUS
 OvsGetVportCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
   UINT32 *replyLen)
 {
+*replyLen = 0;
+
 switch (usrParamsCtx-devOp)
 {
 case OVS_WRITE_DEV_OP:
-*replyLen = 0;
 return OvsSetupDumpStart(usrParamsCtx);
 
 case OVS_READ_DEV_OP:
 return OvsGetVportDumpNext(usrParamsCtx, replyLen);
 
+case OVS_TRANSACTION_DEV_OP:
+return OvsGetVport(usrParamsCtx, replyLen);
+
 default:
 return STATUS_INVALID_DEVICE_REQUEST;
 }
-- 
1.8.3.msysgit.0

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH v4 3/4] Add file: NetlinkError.h

2014-09-25 Thread Samuel Ghinet
Contains error codes for netlink transactional errors.
These errors are passed to the error field (INT) of the NL_MSG_ERR struct.
The userspace requires them to be negative values: the nl_msg_nlmsgerr userspace
function transforms them from negative to positive values.

These error codes correspond to the userspace error codes defined in:
C:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\include\errno.h

Signed-off-by: Samuel Ghinet sghi...@cloudbasesolutions.com
Acked-by: Eitan Eliahu elia...@vmware.com
Acked-by: Alin Gabriel Serdean aserd...@cloudbasesolutions.com
---
 datapath-windows/automake.mk   |   1 +
 datapath-windows/ovsext/Datapath.c |  10 ++
 datapath-windows/ovsext/Datapath.h |  10 +-
 datapath-windows/ovsext/Netlink/NetlinkError.h | 200 +
 datapath-windows/ovsext/Netlink/NetlinkProto.h |   2 +-
 datapath-windows/ovsext/ovsext.vcxproj |   3 +-
 datapath-windows/ovsext/precomp.h  |   1 +
 7 files changed, 224 insertions(+), 3 deletions(-)
 create mode 100644 datapath-windows/ovsext/Netlink/NetlinkError.h

diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
index 297a809..ac9f28f 100644
--- a/datapath-windows/automake.mk
+++ b/datapath-windows/automake.mk
@@ -34,6 +34,7 @@ EXTRA_DIST += \
datapath-windows/ovsext/Netlink/Netlink.h \
datapath-windows/ovsext/Netlink/NetlinkBuf.c \
datapath-windows/ovsext/Netlink/NetlinkBuf.h \
+datapath-windows/ovsext/Netlink/NetlinkError.h \
datapath-windows/ovsext/Netlink/NetlinkProto.h \
datapath-windows/ovsext/NetProto.h \
datapath-windows/ovsext/Oid.c \
diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c
index c4c3b71..05c06b6 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -1211,6 +1211,16 @@ BuildReplyMsgFromMsgIn(POVS_MESSAGE msgIn, POVS_MESSAGE 
msgOut, UINT16 flags)
 flags);
 }
 
+static VOID
+BuildErrorMsg(POVS_MESSAGE msgIn, POVS_MESSAGE_ERROR msgOut, UINT errorCode)
+{
+BuildMsgOut(msgIn, (POVS_MESSAGE)msgOut, NLMSG_ERROR,
+sizeof(OVS_MESSAGE_ERROR), 0);
+
+msgOut-errorMsg.error = errorCode;
+msgOut-errorMsg.nlMsg = msgIn-nlMsg;
+}
+
 static NTSTATUS
 OvsCreateMsgFromVport(POVS_VPORT_ENTRY vport,
   POVS_MESSAGE msgIn,
diff --git a/datapath-windows/ovsext/Datapath.h 
b/datapath-windows/ovsext/Datapath.h
index 4e6c9b1..73e6654 100644
--- a/datapath-windows/ovsext/Datapath.h
+++ b/datapath-windows/ovsext/Datapath.h
@@ -29,7 +29,7 @@
 #define __DATAPATH_H_ 1
 
 /*
- * Structure of any message passed between userspace and kernel.
+ * Structure of a general message passed between userspace and kernel.
  */
 typedef struct _OVS_MESSAGE {
 NL_MSG_HDR nlMsg;
@@ -38,6 +38,14 @@ typedef struct _OVS_MESSAGE {
 /* Variable length nl_attrs follow. */
 } OVS_MESSAGE, *POVS_MESSAGE;
 
+/*
+ * Structure of an error message sent as a reply from kernel.
+ */
+typedef struct _OVS_MESSAGE_ERROR {
+NL_MSG_HDR nlMsg;
+NL_MSG_ERR errorMsg;
+} OVS_MESSAGE_ERROR, *POVS_MESSAGE_ERROR;
+
 typedef struct _OVS_DEVICE_EXTENSION {
 INT numberOpenInstance;
 INT pidCount;
diff --git a/datapath-windows/ovsext/Netlink/NetlinkError.h 
b/datapath-windows/ovsext/Netlink/NetlinkError.h
new file mode 100644
index 000..46ebc5e
--- /dev/null
+++ b/datapath-windows/ovsext/Netlink/NetlinkError.h
@@ -0,0 +1,200 @@
+/*
+* Copyright 2014 Cloudbase Solutions Srl
+*
+* Licensed under the Apache License, Version 2.0 (the License);
+* you may not use this file except in compliance with the License.
+* You may obtain a copy of the License at
+*
+* http :/*www.apache.org/licenses/LICENSE-2.0
+*
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an AS IS BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/
+
+#pragma once
+
+#include precomp.h
+
+/*
+ * These are error codes to be used by netlink transactional operations.
+ * The error code is assigned to the error field (INT) of the NL_MSG_ERR
+ * struct.
+*/
+
+typedef enum _NL_ERROR_
+{
+NL_ERROR_SUCCESS = 0,
+/* The operation is not permitted */
+NL_ERROR_PERM = ((ULONG)-1),
+/* There is no such file or directory */
+NL_ERROR_NOENT = ((ULONG)-2),
+/* There is no such process */
+NL_ERROR_SRCH = ((ULONG)-3),
+/* An interrupted system call / interrupted function */
+NL_ERROR_INTR = ((ULONG)-4),
+/* An I/O error */
+NL_ERROR_IO = ((ULONG)-5),
+/* There is no such device or address */
+NL_ERROR_NXIO = ((ULONG)-6),
+/* The argument list is too long */
+NL_ERROR_2BIG = ((ULONG)-7),
+/* Executable file format error */
+NL_ERROR_NOEXEC = ((ULONG)-8),
+/* A 

Re: [ovs-dev] [PATCH 2/3] datapath-windows: Add Netlink vport command get

2014-09-25 Thread Samuel Ghinet
Hello Eitan,

 You want to return STATUS_INVALID_BUFFER_SIZE instead.
 XXX We need to go through all datapath.c and remove NDIS return code (perhaps 
 in a different change)
Done.

 Does the caller set *replyLen =0;
Perhaps regardless of the call, *replyLen should be initialized to 0.
I have moved that call a bit upwards, for now.

 +BuildErrorMsg(msgIn, msgError, nlError);
 +*replyLen = msgError-nlMsg.nlmsgLen;
 [EITAN]
 Can we NlBufSize ?
 [EITAN]
Not quite. the nlBuffer is a local variable in OvsCreateMsgFromVport.
Another option would have been for OvsCreateMsgFromVport to return the 
resulting buffer siz (or via an out parameter).
However, I am not sure whether in the future we should make use of the status 
returned from OvsCreateMsgFromVport , and I don't like having many parameters 
on a function.
So I left this the way it was.

Regards,
Sam

From: Eitan Eliahu [elia...@vmware.com]
Sent: Wednesday, September 24, 2014 8:00 PM
To: Samuel Ghinet; dev@openvswitch.org
Cc: Alin Serdean; Nithin Raju; Ankur Sharma; Saurabh Shah
Subject: RE: [PATCH 2/3] datapath-windows: Add Netlink vport command get

Looks good. Please find comments inline.
Thanks,
Eitan

-Original Message-
From: Samuel Ghinet [mailto:sghi...@cloudbasesolutions.com]
Sent: Wednesday, September 24, 2014 8:42 AM
To: dev@openvswitch.org
Cc: Alin Serdean; Eitan Eliahu; Nithin Raju; Ankur Sharma; Saurabh Shah
Subject: [PATCH 2/3] datapath-windows: Add Netlink vport command get

The transactional get vport command.
This command uses the netlink transactional errors.

Signed-off-by: Samuel Ghinet sghi...@cloudbasesolutions.com
---
 datapath-windows/ovsext/Datapath.c | 83 ++
 1 file changed, 83 insertions(+)

diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c
index 05c06b6..8a8c542 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -1425,6 +1425,86 @@ OvsGetVportDumpNext(POVS_USER_PARAMS_CONTEXT 
usrParamsCtx,
 return STATUS_SUCCESS;
 }

+static NTSTATUS
+OvsGetVport(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
+UINT32 *replyLen)
+{
+NDIS_STATUS status = STATUS_SUCCESS;
[EITAN]
NTSTATUS instead of NDIS_STATUS
[EITAN]
+LOCK_STATE_EX lockState;
+
+POVS_MESSAGE msgIn = (POVS_MESSAGE)usrParamsCtx-inputBuffer;
+POVS_MESSAGE msgOut = (POVS_MESSAGE)usrParamsCtx-outputBuffer;
+POVS_VPORT_ENTRY vport = NULL;
+NL_ERROR nlError = NL_ERROR_SUCCESS;
+
+static const NL_POLICY ovsVportPolicy[] = {
+[OVS_VPORT_ATTR_PORT_NO] = { .type = NL_A_U32 },
+[OVS_VPORT_ATTR_TYPE] = { .type = NL_A_U32 },
+[OVS_VPORT_ATTR_NAME] = { .type = NL_A_STRING, .maxLen = IFNAMSIZ },
+[OVS_VPORT_ATTR_UPCALL_PID] = { .type = NL_A_UNSPEC },
+[OVS_VPORT_ATTR_STATS] = { .type = NL_A_UNSPEC,
+   .minLen = sizeof(OVS_VPORT_FULL_STATS),
+   .maxLen = sizeof(OVS_VPORT_FULL_STATS)
+},
+[OVS_VPORT_ATTR_OPTIONS] = { .type = NL_A_NESTED, .optional = TRUE },
+};
+PNL_ATTR vportAttrs[ARRAY_SIZE(ovsVportPolicy)];
+
+/* input buffer has been validated while validating write dev op. */
+ASSERT(usrParamsCtx-inputBuffer != NULL);
+
+if (!NlAttrParse((PNL_MSG_HDR)msgIn,
+NLMSG_HDRLEN + GENL_HDRLEN + OVS_HDRLEN,
+ovsVportPolicy, vportAttrs, ARRAY_SIZE(vportAttrs))) {
+return STATUS_INVALID_PARAMETER;
+}
+
+if (msgOut == NULL || usrParamsCtx-outputLength  sizeof *msgOut) {
+return STATUS_NDIS_INVALID_LENGTH;
[EITAN]
You want to return STATUS_INVALID_BUFFER_SIZE instead.
XXX We need to go through all datapath.c and remove NDIS return code (perhaps 
in a different change)
[EITAN]
+}
+
+OvsAcquireCtrlLock();
+if (!gOvsSwitchContext) {
+OvsReleaseCtrlLock();
+*replyLen = 0;
[EITAN]
Does the caller set *replyLen =0;
[EITAN]
+return STATUS_DATA_NOT_ACCEPTED;
+}
+OvsReleaseCtrlLock();
+
+if (vportAttrs[OVS_VPORT_ATTR_NAME] != NULL) {
+vport = OvsFindVportByOvsName(gOvsSwitchContext,
+NlAttrGet(vportAttrs[OVS_VPORT_ATTR_NAME]),
+NlAttrGetSize(vportAttrs[OVS_VPORT_ATTR_NAME]));
+} else if (vportAttrs[OVS_VPORT_ATTR_PORT_NO] != NULL) {
+vport = OvsFindVportByPortNo(gOvsSwitchContext,
+NlAttrGetU32(vportAttrs[OVS_VPORT_ATTR_PORT_NO]));
+}
+
+if (!vport) {
+nlError = NL_ERROR_NODEV;
+goto Cleanup;
+}
+
+NdisAcquireRWLockRead(gOvsSwitchContext-dispatchLock, lockState, 0);
+status = OvsCreateMsgFromVport(vport, msgIn, usrParamsCtx-outputBuffer,
+   usrParamsCtx-outputLength,
+   gOvsSwitchContext-dpNo);
+NdisReleaseRWLock(gOvsSwitchContext-dispatchLock, lockState);
+
+*replyLen = msgOut-nlMsg.nlmsgLen;
+
+Cleanup:
+if 

Re: [ovs-dev] [PATCH 2/3] datapath-windows: Add Netlink vport command get

2014-09-25 Thread Samuel Ghinet
 All of the vport commands are implemented in Vport.c. Pls. move this to that 
 file. Perhaps even the Vport dump. You can move all of them at once in a 
 subsequent commit.
I know that. I was not sure of the approach I should take.

The linux ovs driver does all the netlink communication part in datapath.c. 
Except for the flows, which is in flow_netlink.c as I remember.
I had considered the same approach here. It may changed later the way you guys 
think it would look best.

 Are all the attributes mandatory? I'd think that one of 
 OVS_VPORT_ATTR_PORT_NO or OVS_VPORT_ATTR_NAME are mandatory. Rest are 
 optional.
Oops, my bad. I think I had tested this function upon not last commit, where 
the optional field did not exist yet. I fixed it.

 I am not sure if 'STATUS_DATA_NOT_ACCEPTED' is the correct error code to be 
 returned here. The documentation says:
 {Data Not Accepted} The TDI client could not handle the data received during 
 an indication.
I have STATUS_INVALID_PARAMETER for now. May not be the best value though.

 Returning a transactional error with ENODEV would be appropriate I think, or 
 you could return STATUS_INVALID_PARAMETER.
No. ENODEV will mean that the vport does not exist. Reason why I cannot use 
that error code here.
Later on we might need to consider a proper error value, either as 
transactional error or as error returned by DeviceIoControl, and apply this in 
one place for all netlink functions (all require the switch context).
Hmmm, in our implementation we replied with NL_ERROR_PERM for this case. 
Perhaps it might be a good fit.

Anyway, this approach we have:
[CODE]
OvsAcquireCtrlLock();
if (!gOvsSwitchContext) {
OvsReleaseCtrlLock();
return STATUS_INVALID_PARAMETER;
}
OvsReleaseCtrlLock();
[/CODE]
Is not proper either, because we cannot guarantee that after 
OvsReleaseCtrlLock, the extension will not detach or that gOvsSwitchContext 
will not become NULL or the memory it points to, deallocated.
We should normally handle the case where netlink commands are requested while 
the extension is detached, so that the detaching waits for all current netlink 
commands to finish, and that after the extension is detached, all 
OvsDeviceControl operations will fail.
But this will be another issue.

Regards,
Sam

From: Nithin Raju [nit...@vmware.com]
Sent: Thursday, September 25, 2014 9:51 PM
To: Samuel Ghinet
Cc: dev@openvswitch.org; Alin Serdean; Eitan Eliahu; Ankur Sharma; Saurabh Shah
Subject: Re: [PATCH 2/3] datapath-windows: Add Netlink vport command get

hi Samuel,
I had some minor comments. Looks good otherwise.

Acked-by: Nithin Raju nit...@vmware.com

On Sep 24, 2014, at 8:42 AM, Samuel Ghinet sghi...@cloudbasesolutions.com 
wrote:

 The transactional get vport command.
 This command uses the netlink transactional errors.

 Signed-off-by: Samuel Ghinet sghi...@cloudbasesolutions.com
 ---
 datapath-windows/ovsext/Datapath.c | 83 ++
 1 file changed, 83 insertions(+)

 diff --git a/datapath-windows/ovsext/Datapath.c 
 b/datapath-windows/ovsext/Datapath.c
 index 05c06b6..8a8c542 100644
 --- a/datapath-windows/ovsext/Datapath.c
 +++ b/datapath-windows/ovsext/Datapath.c
 @@ -1425,6 +1425,86 @@ OvsGetVportDumpNext(POVS_USER_PARAMS_CONTEXT 
 usrParamsCtx,
 return STATUS_SUCCESS;
 }

 +static NTSTATUS
 +OvsGetVport(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
 +UINT32 *replyLen)

All of the vport commands are implemented in Vport.c. Pls. move this to that 
file. Perhaps even the Vport dump. You can move all of them at once in a 
subsequent commit.

 +{
 +NDIS_STATUS status = STATUS_SUCCESS;

NDIS_STATUS = NTSTATUS.

 +LOCK_STATE_EX lockState;
 +
 +POVS_MESSAGE msgIn = (POVS_MESSAGE)usrParamsCtx-inputBuffer;
 +POVS_MESSAGE msgOut = (POVS_MESSAGE)usrParamsCtx-outputBuffer;
 +POVS_VPORT_ENTRY vport = NULL;
 +NL_ERROR nlError = NL_ERROR_SUCCESS;
 +
 +static const NL_POLICY ovsVportPolicy[] = {
 +[OVS_VPORT_ATTR_PORT_NO] = { .type = NL_A_U32 },
 +[OVS_VPORT_ATTR_TYPE] = { .type = NL_A_U32 },
 +[OVS_VPORT_ATTR_NAME] = { .type = NL_A_STRING, .maxLen = IFNAMSIZ },
 +[OVS_VPORT_ATTR_UPCALL_PID] = { .type = NL_A_UNSPEC },
 +[OVS_VPORT_ATTR_STATS] = { .type = NL_A_UNSPEC,
 +   .minLen = sizeof(OVS_VPORT_FULL_STATS),
 +   .maxLen = sizeof(OVS_VPORT_FULL_STATS)
 +},
 +[OVS_VPORT_ATTR_OPTIONS] = { .type = NL_A_NESTED, .optional = TRUE },

Are all the attributes mandatory? I'd think that one of OVS_VPORT_ATTR_PORT_NO 
or OVS_VPORT_ATTR_NAME are mandatory. Rest are optional.

You can see an example here:
dpif_netlink_port_query_by_number()
- dpif_netlink_port_query__()
   - dpif_netlink_vport_transact()

 825 dpif_netlink_vport_init(request);
 826 request.cmd = OVS_VPORT_CMD_GET;
 827 request.dp_ifindex = dpif-dp_ifindex;
 828 

Re: [ovs-dev] [PATCH v6 00/18] Rapid Spanning Tree Protocol

2014-09-25 Thread Jarno Rajahalme

On Sep 25, 2014, at 7:52 AM, Daniele Venturino daniele.ventur...@m3s.it wrote:

 After some testing, here’s my ack.
 
 Acked-by: Daniele Venturino daniele.ventur...@m3s.it
 

Pushed to master, thanks!

  Jarno

 
 diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
 index d3e527a..d3973e5 100644
 --- a/ofproto/ofproto-dpif.c
 +++ b/ofproto/ofproto-dpif.c
 @@ -2386,6 +2386,8 @@ set_rstp_port(struct ofport *ofport_,
  rstp_port_set(rp, s-port_num, s-priority, s-path_cost,
s-admin_edge_port, s-auto_edge, s-mcheck, ofport);
  update_rstp_port_state(ofport);
 +/* Synchronize operational status. */
 +rstp_port_set_mac_operational(rp, ofport-may_enable);
  }
  
  static void
 
 This helps, physical interfaces are no longer blocked in role disabled if 
 RSTP is enabled after they are added to a bridge.
 I’ll test this more thoroughly soon.
 
 
 I’ll merge this when I get your “Acked-by:”,
 
   Jarno
 

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 2/3] datapath-windows: Add Netlink vport command get

2014-09-25 Thread Nithin Raju
hi Sam,
Thanks for incorporating the comments in the latest patch.

On Sep 25, 2014, at 2:44 PM, Samuel Ghinet sghi...@cloudbasesolutions.com
 wrote:

 All of the vport commands are implemented in Vport.c. Pls. move this to that 
 file. Perhaps even the Vport dump. You can move all of them at once in a 
 subsequent commit.
 I know that. I was not sure of the approach I should take.
 
 The linux ovs driver does all the netlink communication part in datapath.c. 
 Except for the flows, which is in flow_netlink.c as I remember.
 I had considered the same approach here. It may changed later the way you 
 guys think it would look best.

It needs to be in one place - either Datapath.c or Vport.c. All the flow 
related commands are in Flow.c, so, Vport.c might be a good idea. We can do 
this later.


 Are all the attributes mandatory? I'd think that one of 
 OVS_VPORT_ATTR_PORT_NO or OVS_VPORT_ATTR_NAME are mandatory. Rest are 
 optional.
 Oops, my bad. I think I had tested this function upon not last commit, where 
 the optional field did not exist yet. I fixed it.
OK

 
 I am not sure if 'STATUS_DATA_NOT_ACCEPTED' is the correct error code to be 
 returned here. The documentation says:
 {Data Not Accepted} The TDI client could not handle the data received 
 during an indication.
 I have STATUS_INVALID_PARAMETER for now. May not be the best value though.

None of these error codes map to anything meaningful in userspace. We can 
revisit these when we update the userspace code. But, picking the closest match 
is a good idea for now.

 Returning a transactional error with ENODEV would be appropriate I think, or 
 you could return STATUS_INVALID_PARAMETER.
 No. ENODEV will mean that the vport does not exist. Reason why I cannot use 
 that error code here.
 Later on we might need to consider a proper error value, either as 
 transactional error or as error returned by DeviceIoControl, and apply this 
 in one place for all netlink functions (all require the switch context).
 Hmmm, in our implementation we replied with NL_ERROR_PERM for this case. 
 Perhaps it might be a good fit.
 
 Anyway, this approach we have:
 [CODE]
OvsAcquireCtrlLock();
if (!gOvsSwitchContext) {
OvsReleaseCtrlLock();
return STATUS_INVALID_PARAMETER;
}
OvsReleaseCtrlLock();
 [/CODE]
 Is not proper either, because we cannot guarantee that after 
 OvsReleaseCtrlLock, the extension will not detach or that gOvsSwitchContext 
 will not become NULL or the memory it points to, deallocated.
 We should normally handle the case where netlink commands are requested while 
 the extension is detached, so that the detaching waits for all current 
 netlink commands to finish, and that after the extension is detached, all 
 OvsDeviceControl operations will fail.
 But this will be another issue.

Yes, making the usage of the CtrlLock more meaningful is on my list of things 
to do.

-- Nithin
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] datapath-windows: NUL character should be left out during VPORT hash lookup

2014-09-25 Thread Nithin Raju
While calculating the hash on a VPORT name, we don't include the NUL character.
We should be doing the same while doing lookup as well.

Signed-off-by: Nithin Raju nit...@vmware.com
---
 datapath-windows/ovsext/Datapath.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c
index 495219e..65b4b81 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -1468,7 +1468,7 @@ OvsGetVport(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
 if (vportAttrs[OVS_VPORT_ATTR_NAME] != NULL) {
 vport = OvsFindVportByOvsName(gOvsSwitchContext,
 NlAttrGet(vportAttrs[OVS_VPORT_ATTR_NAME]),
-NlAttrGetSize(vportAttrs[OVS_VPORT_ATTR_NAME]));
+NlAttrGetSize(vportAttrs[OVS_VPORT_ATTR_NAME]) - 1);
 } else if (vportAttrs[OVS_VPORT_ATTR_PORT_NO] != NULL) {
 vport = OvsFindVportByPortNo(gOvsSwitchContext,
 NlAttrGetU32(vportAttrs[OVS_VPORT_ATTR_PORT_NO]));
-- 
1.7.4.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2] datapath-windows Event read handler

2014-09-25 Thread Eitan Eliahu
Thanks Sam for the review.
rc stands for Return Code and it holds the Boolean intermediate return code, 
returned from the NL functions. 
I removed the  blank line and lifted the else(s).

Regarding the sharing of code between READ and READ_EVENT, a common function 
would take too many parameters. Will consider it after adding the READ_PACKET 
ioctl..
Thank you,
Eitan 

-Original Message-
From: Samuel Ghinet [mailto:sghi...@cloudbasesolutions.com] 
Sent: Thursday, September 25, 2014 10:46 AM
To: dev@openvswitch.org
Cc: Eitan Eliahu
Subject: RE: [ovs-dev] [PATCH v2] datapath-windows Event read handler

I now see this new version.
The while has been removed I see, along with the issues I had pointed out with 
the values returned.

I will write my comments here, of the things that still remained :)

 }
 
 +
  /*
   * --
   *  Handler for the subscription for the event queue @@ -1214,4 
 +1240,131 @@ MapIrpOutputBuffer(PIRP irp
I believe we don't need that blank line :)

You've got a few else-s like this:
 }
 else {
 status = STATUS_NDIS_INVALID_LENGTH;
 goto done;
 }

I see that you now always returns status, and no longer rc.
Still, what does rc stand for?

And, 
 case OVS_IOCTL_READ_EVENT:
 /* This IOCTL is used to read events */
 if (outputBufferLen != 0) {
I see it shares some code with case OVS_IOCTL_READ:
An idea would be to extract a common function of the two. But this is very 
minor.

Regards,
Sam

Date: Thu, 25 Sep 2014 13:21:12 -0700
From: Eitan Eliahu elia...@vmware.com
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH v2] datapath-windows Event read handler
Message-ID: 1411676472-3644-1-git-send-email-elia...@vmware.com

The Read event handler is executed when user mode issues a socket receive on an 
MC socket associated with the event queue. A new IOCTL READ command is used to 
differentiate between transaction based and packet miss sockets.
An entry for the handler will be added once the Vport family implementation 
checked in.
User mode code for setting the socket type will follow

Signed-off-by: Eitan Eliahu elia...@vmware.com
---
 build-aux/extract-odp-netlink-windows-dp-h   |   4 +
 datapath-windows/include/OvsDpInterfaceExt.h |  10 +-
 datapath-windows/ovsext/Datapath.c   | 155 ++-
 datapath-windows/ovsext/Event.c  |  42 
 datapath-windows/ovsext/Event.h  |   2 +
 5 files changed, 210 insertions(+), 3 deletions(-)

diff --git a/build-aux/extract-odp-netlink-windows-dp-h 
b/build-aux/extract-odp-netlink-windows-dp-h
index 70514cb..82d95f1 100755
--- a/build-aux/extract-odp-netlink-windows-dp-h
+++ b/build-aux/extract-odp-netlink-windows-dp-h
@@ -23,3 +23,7 @@ s,#include linux/if_ether\.h,\n#ifndef ETH_ADDR_LEN \

 # Use OVS's own ETH_ADDR_LEN instead of Linux-specific ETH_ALEN.
 s/ETH_ALEN/ETH_ADDR_LEN/
+
+s/OVS_VPORT_CMD_GET,/OVS_VPORT_CMD_GET,\
+   OVS_VPORT_CMD_NOTIFY,/
+
diff --git a/datapath-windows/include/OvsDpInterfaceExt.h 
b/datapath-windows/include/OvsDpInterfaceExt.h
index 64fd20e..be1e49f 100644
--- a/datapath-windows/include/OvsDpInterfaceExt.h
+++ b/datapath-windows/include/OvsDpInterfaceExt.h
@@ -34,11 +34,17 @@
 #define OVS_IOCTL_READ \
 CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x0, METHOD_OUT_DIRECT,\
   FILE_READ_ACCESS)
+#define OVS_IOCTL_READ_PACKET \
+CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x1, METHOD_OUT_DIRECT, 
\
+  FILE_READ_ACCESS)
+#define OVS_IOCTL_READ_EVENT \
+CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x2, METHOD_OUT_DIRECT, 
\
+  FILE_READ_ACCESS)
 #define OVS_IOCTL_WRITE \
-CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x1, METHOD_IN_DIRECT,\
+CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x3, 
+ METHOD_IN_DIRECT,\
   FILE_READ_ACCESS)
 #define OVS_IOCTL_TRANSACT \
-CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x2, METHOD_OUT_DIRECT,\
+CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x4, 
+ METHOD_OUT_DIRECT,\
   FILE_WRITE_ACCESS)

 /*
diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c
index 0dfdd57..4954712 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -99,7 +99,8 @@ static NetlinkCmdHandler OvsGetPidCmdHandler,
  OvsGetDpCmdHandler,
  OvsPendEventCmdHandler,
  OvsSubscribeEventCmdHandler,
- OvsSetDpCmdHandler;
+ OvsSetDpCmdHandler,
+ OvsReadEventCmdHandler;

 static NTSTATUS HandleGetDpTransaction(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
UINT32 *replyLen); @@ -620,6 +621,30 @@ 
OvsDeviceControl(PDEVICE_OBJECT 

[ovs-dev] [PATCH v3] datapath-windows Event read handler

2014-09-25 Thread Eitan Eliahu
The Read event handler is executed when user mode issues a socket receive on
an MC socket associated with the event queue. A new IOCTL READ command is
used to differentiate between transaction based and packet miss sockets.
An entry for the handler will be added once the Vport family implementation
checked in.
User mode code for setting the socket type will follow

Signed-off-by: Eitan Eliahu elia...@vmware.com
---
 build-aux/extract-odp-netlink-windows-dp-h   |   4 +
 datapath-windows/include/OvsDpInterfaceExt.h |  10 +-
 datapath-windows/ovsext/Datapath.c   | 150 ++-
 datapath-windows/ovsext/Event.c  |  42 
 datapath-windows/ovsext/Event.h  |   2 +
 5 files changed, 205 insertions(+), 3 deletions(-)

diff --git a/build-aux/extract-odp-netlink-windows-dp-h 
b/build-aux/extract-odp-netlink-windows-dp-h
index 70514cb..82d95f1 100755
--- a/build-aux/extract-odp-netlink-windows-dp-h
+++ b/build-aux/extract-odp-netlink-windows-dp-h
@@ -23,3 +23,7 @@ s,#include linux/if_ether\.h,\n#ifndef ETH_ADDR_LEN \
 
 # Use OVS's own ETH_ADDR_LEN instead of Linux-specific ETH_ALEN.
 s/ETH_ALEN/ETH_ADDR_LEN/
+
+s/OVS_VPORT_CMD_GET,/OVS_VPORT_CMD_GET,\
+   OVS_VPORT_CMD_NOTIFY,/
+
diff --git a/datapath-windows/include/OvsDpInterfaceExt.h 
b/datapath-windows/include/OvsDpInterfaceExt.h
index 64fd20e..be1e49f 100644
--- a/datapath-windows/include/OvsDpInterfaceExt.h
+++ b/datapath-windows/include/OvsDpInterfaceExt.h
@@ -34,11 +34,17 @@
 #define OVS_IOCTL_READ \
 CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x0, METHOD_OUT_DIRECT,\
   FILE_READ_ACCESS)
+#define OVS_IOCTL_READ_PACKET \
+CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x1, METHOD_OUT_DIRECT, 
\
+  FILE_READ_ACCESS)
+#define OVS_IOCTL_READ_EVENT \
+CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x2, METHOD_OUT_DIRECT, 
\
+  FILE_READ_ACCESS)
 #define OVS_IOCTL_WRITE \
-CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x1, METHOD_IN_DIRECT,\
+CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x3, METHOD_IN_DIRECT,\
   FILE_READ_ACCESS)
 #define OVS_IOCTL_TRANSACT \
-CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x2, METHOD_OUT_DIRECT,\
+CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x4, METHOD_OUT_DIRECT,\
   FILE_WRITE_ACCESS)
 
 /*
diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c
index 0dfdd57..3102e66 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -99,7 +99,8 @@ static NetlinkCmdHandler OvsGetPidCmdHandler,
  OvsGetDpCmdHandler,
  OvsPendEventCmdHandler,
  OvsSubscribeEventCmdHandler,
- OvsSetDpCmdHandler;
+ OvsSetDpCmdHandler,
+ OvsReadEventCmdHandler;
 
 static NTSTATUS HandleGetDpTransaction(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
UINT32 *replyLen);
@@ -620,6 +621,29 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject,
 devOp = OVS_TRANSACTION_DEV_OP;
 break;
 
+case OVS_IOCTL_READ_EVENT:
+/* This IOCTL is used to read events */
+if (outputBufferLen != 0) {
+status = MapIrpOutputBuffer(irp, outputBufferLen,
+sizeof *ovsMsg, outputBuffer);
+if (status != STATUS_SUCCESS) {
+goto done;
+}
+ASSERT(outputBuffer);
+} else {
+status = STATUS_NDIS_INVALID_LENGTH;
+goto done;
+}
+inputBuffer = NULL;
+inputBufferLen = 0;
+
+ovsMsg = ovsMsgReadOp;
+ovsMsg-nlMsg.nlmsgType = OVS_WIN_NL_VPORT_FAMILY_ID;
+/* An artificial command so we can use NL family function table*/
+ovsMsg-genlMsg.cmd = OVS_VPORT_CMD_NOTIFY;
+devOp = OVS_READ_DEV_OP;
+break;
+
 case OVS_IOCTL_READ:
 /* Output buffer is mandatory. */
 if (outputBufferLen != 0) {
@@ -924,6 +948,7 @@ OvsPendEventCmdHandler(POVS_USER_PARAMS_CONTEXT 
usrParamsCtx,
 return status;
 }
 
+
 /*
  * --
  *  Handler for the subscription for the event queue
@@ -1214,4 +1239,127 @@ MapIrpOutputBuffer(PIRP irp,
 return STATUS_SUCCESS;
 }
 
+/*
+ * --
+ * Utility function to fill up information about the state of a port in a reply
+ * to* userspace.
+ * Assumes that 'gOvsCtrlLock' lock is acquired.
+ * --
+ */
+static NTSTATUS
+OvsPortFillInfo(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
+POVS_EVENT_ENTRY eventEntry,
+PNL_BUFFER nlBuf)
+{
+NTSTATUS status;
+BOOLEAN rc;
+OVS_MESSAGE