Re: [ovs-dev] [RFC PATCH] datapath: allow tunnels to be created with rtnetlink

[Yang, Yi Y]

Hi, guys

This patch isn't updated from June on, Cascardo said he/Eric is still working 
on this, but six months passed, we don't see any following patch for this, now 
I have time to revisit it, for case 3 Pravin mentioned, I can make it work by 
applying the below patch [1] against 
https://mail.openvswitch.org/pipermail/ovs-dev/2016-June/316881.html, but it 
only can create vxlan port, it will also create vxlan port even if I create 
vxlan-gpe port by cmd "sudo ovs-vsctl add-port br-int vxlan_gpe1 -- set 
interface vxlan_gpe1 type=vxlan options:remote_ip=flow options:key=flow 
options:dst_port=4790 options:exts=gpe", the reason is very simple, 
vxlan_configure_exts in datapath/vport-vxlan.c will be called to set vxlan 
configuration, but it can't recognize vxlangpe extension and flags, you guys 
told me datapath/vport-vxlan.c and 
datapath/linux/compat/include/linux/openvswitch.h are compatibility code, we 
can't change them, but for case 3, we have to change them like patch [2], I 
know we shoul
 d change them on Linux net-next kernel first, here I just show you we have to 
do so for case 3 Pravin mentioned, I'm happy to hear you have better way for 
this.

So my advice about this is we can push patch [2] to Linux net-next first, then 
apply patch series 
https://mail.openvswitch.org/pipermail/ovs-dev/2016-June/316879.html from 
Cascardo and apply [1], that can cover all the cases Pravin mentioned.

This patch has blocked us too long, we look forward to seeing progress on this. 


[1]:
>From f40b6fec09e1f9ad14e50ba224f46b1b9657399c Mon Sep 17 00:00:00 2001
From: Yi Yang 
Date: Tue, 6 Dec 2016 12:39:41 +0800
Subject: [PATCH] Use ovs compat modules if USE_UPSTREAM_TUNNEL is defined

Signed-off-by: Yi Yang 
---
 lib/dpif-netlink.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 0d03334..2286c3e 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -1062,6 +1062,9 @@ dpif_netlink_port_query__(const struct dpif_netlink 
*dpif, odp_port_t port_no,
 static int
 dpif_netlink_port_create(struct netdev *netdev)
 {
+#ifndef USE_UPSTREAM_TUNNEL
+return EOPNOTSUPP;
+#else
 switch (netdev_to_ovs_vport_type(netdev_get_type(netdev))) {
 case OVS_VPORT_TYPE_VXLAN:
 return netdev_vxlan_create(netdev);
@@ -1077,6 +1080,7 @@ dpif_netlink_port_create(struct netdev *netdev)
 return EOPNOTSUPP;
 }
 return 0;
+#endif
 }

 static int
--
2.1.0


[2]:

diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h
index 12260d8..17e21cb 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -291,6 +291,7 @@ enum ovs_vport_attr {
 enum {
OVS_VXLAN_EXT_UNSPEC,
OVS_VXLAN_EXT_GBP,  /* Flag or __u32 */
+   OVS_VXLAN_EXT_GPE,  /* Flag or __u32 */
__OVS_VXLAN_EXT_MAX,
 };

diff --git a/datapath/vport-vxlan.c b/datapath/vport-vxlan.c
index 11965c0..a5882ed 100644
--- a/datapath/vport-vxlan.c
+++ b/datapath/vport-vxlan.c
@@ -54,11 +54,26 @@ static int vxlan_get_options(const struct vport *vport, 
struct sk_buff *skb)
nla_nest_end(skb, exts);
}

+   if (vxlan->flags & VXLAN_F_GPE) {
+   struct nlattr *exts;
+
+   exts = nla_nest_start(skb, OVS_TUNNEL_ATTR_EXTENSION);
+   if (!exts)
+   return -EMSGSIZE;
+
+   if (vxlan->flags & VXLAN_F_GPE &&
+   nla_put_flag(skb, OVS_VXLAN_EXT_GPE))
+   return -EMSGSIZE;
+
+   nla_nest_end(skb, exts);
+   }
+
return 0;
 }

 static const struct nla_policy exts_policy[OVS_VXLAN_EXT_MAX + 1] = {
[OVS_VXLAN_EXT_GBP] = { .type = NLA_FLAG, },
+   [OVS_VXLAN_EXT_GPE] = { .type = NLA_FLAG, },
 };

 static int vxlan_configure_exts(struct vport *vport, struct nlattr *attr,
@@ -76,6 +91,8 @@ static int vxlan_configure_exts(struct vport *vport, struct 
nlattr *attr,

if (exts[OVS_VXLAN_EXT_GBP])
conf->flags |= VXLAN_F_GBP;
+   if (exts[OVS_VXLAN_EXT_GPE])
+   conf->flags |= VXLAN_F_GPE;

return 0;
 }

-Original Message-
From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Pravin Shelar
Sent: Friday, October 21, 2016 1:31 AM
To: Thadeu Lima de Souza Cascardo 
Cc: ovs dev ; Eric Garver 
Subject: Re: [ovs-dev] [RFC PATCH] datapath: allow tunnels to be created with 
rtnetlink

On Tue, Sep 20, 2016 at 7:01 AM, Thadeu Lima de Souza Cascardo 
 wrote:
> In order to use rtnetlink to create new tunnel vports, the backported 
> tunnels require some code that was removed from their upstream 
> version, mainly the necessary code for newlink and for start_xmit.
>
> This patch adds back the necessary code for VXLAN, GRE and Geneve 

[ovs-dev] [PATCH] netdev-dpdk: Don't use dev->vhost_id without mutex.

[Ilya Maximets]

> How about "if (!vhost_id[0])", to avoid a useless strlen call?

LGTM, I've posted v2 with that fix:
https://mail.openvswitch.org/pipermail/ovs-dev/2016-December/325807.html

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] netdev-dpdk: Don't use dev->vhost_id without mutex.

[Ilya Maximets]

The copy should be used here.
Additionally, 'strlen' changed to the faster check.

Fixes: 821b86649a90 ("netdev-dpdk: Don't try to unregister empty vhost_id.")
Signed-off-by: Ilya Maximets 
---
Version 2:
* 'strlen' --> '[0]' (Suggested by Ben Pfaff)

 lib/netdev-dpdk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 6e5cd43..61d7aa3 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1027,7 +1027,7 @@ netdev_dpdk_vhost_destruct(struct netdev *netdev)
 ovs_mutex_unlock(>mutex);
 ovs_mutex_unlock(_mutex);
 
-if (!strlen(dev->vhost_id)) {
+if (!vhost_id[0]) {
 goto out;
 }
 
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 4/4] build-windows: Propagate PACKAGE_VERSION to the MSI

[Alin Serdean]

This patch propagates the automake variable PACKAGE_VERSION when building
the MSI via msys.

Signed-off-by: Alin Gabriel Serdean 
---
 windows/automake.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/windows/automake.mk b/windows/automake.mk
index fa610ec..db2b616 100644
--- a/windows/automake.mk
+++ b/windows/automake.mk
@@ -35,7 +35,7 @@ windows_installer: all
cp -f 
$(top_srcdir)/datapath-windows/x64/Win8.1$(VSTUDIO_CONFIG)/package/ovsext.cat 
windows/ovs-windows-installer/Driver/Win8.1/ovsext.cat
cp -f 
$(top_srcdir)/datapath-windows/x64/Win8.1$(VSTUDIO_CONFIG)/package/ovsext.inf 
windows/ovs-windows-installer/Driver/Win8.1/ovsext.inf
cp -f 
$(top_srcdir)/datapath-windows/x64/Win8.1$(VSTUDIO_CONFIG)/package/ovsext.sys 
windows/ovs-windows-installer/Driver/Win8.1/ovsext.sys
-   MSBuild.exe windows/ovs-windows-installer.sln /target:Build 
/property:Configuration="Release"
+   MSBuild.exe windows/ovs-windows-installer.sln /target:Build 
/property:Configuration="Release" /property:Version="$(PACKAGE_VERSION)"
 
 EXTRA_DIST += \
windows/.gitignore \
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 2/4] build-windows: Propagate PACKAGE_VERSION to the driver files

[Alin Serdean]

This patch propagates the automake value 'PACKAGE_VERSION' to the driver
specific information files, overwriting the Visual Studio default value of
Version, when building the driver via msys.

Signed-off-by: Alin Gabriel Serdean 
---
 Makefile.am | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 974cb9a..6053301 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -394,13 +394,13 @@ CLEANFILES += manpage-dep-check
 if VSTUDIO_DDK
 ALL_LOCAL += ovsext
 ovsext: datapath-windows/ovsext.sln 
$(srcdir)/datapath-windows/include/OvsDpInterface.h
-   MSBuild.exe //maxcpucount datapath-windows/ovsext.sln /target:Build 
/property:Configuration="Win8$(VSTUDIO_CONFIG)"
-   MSBuild.exe //maxcpucount datapath-windows/ovsext.sln /target:Build 
/property:Configuration="Win8.1$(VSTUDIO_CONFIG)"
+   MSBuild.exe //maxcpucount datapath-windows/ovsext.sln /target:Build 
/property:Configuration="Win8$(VSTUDIO_CONFIG)" 
/property:Version="$(PACKAGE_VERSION)"
+   MSBuild.exe //maxcpucount datapath-windows/ovsext.sln /target:Build 
/property:Configuration="Win8.1$(VSTUDIO_CONFIG)" 
/property:Version="$(PACKAGE_VERSION)"
 
 CLEAN_LOCAL += ovsext_clean
 ovsext_clean: datapath-windows/ovsext.sln
-   MSBuild.exe //maxcpucount datapath-windows/ovsext.sln /target:Clean 
/property:Configuration="Win8$(VSTUDIO_CONFIG)"
-   MSBuild.exe //maxcpucount datapath-windows/ovsext.sln /target:Clean 
/property:Configuration="Win8.1$(VSTUDIO_CONFIG)"
+   MSBuild.exe //maxcpucount datapath-windows/ovsext.sln /target:Clean 
/property:Configuration="Win8$(VSTUDIO_CONFIG)" 
/property:Version="$(PACKAGE_VERSION)"
+   MSBuild.exe //maxcpucount datapath-windows/ovsext.sln /target:Clean 
/property:Configuration="Win8.1$(VSTUDIO_CONFIG)" 
/property:Version="$(PACKAGE_VERSION)"
 endif
 .PHONY: ovsext
 
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 0/4] Unify the version used by the driver and MSI

[Alin Serdean]

The following series will allow to propagate the automake value
'PACKAGE_VERSION' to the driver information values and MSI properties.

The current approach focuses only when building the driver/MSI from
the command line. If projects are built seperatly via UI, the default
value of '1.0.0.0' will be used as the version number.

Alin Serdean (4):
  datapath-windows: Force driver version to depend on a variable
  build-windows: Propagate PACKAGE_VERSION to the driver files
  msi-windows: Add version variable
  build-windows: Propagate PACKAGE_VERSION to the MSI

 Makefile.am   |  8 
 datapath-windows/ovsext/ovsext.rc | 11 ++-
 datapath-windows/ovsext/ovsext.vcxproj| 19 ++-
 windows/automake.mk   |  2 +-
 windows/ovs-windows-installer/Product.wxs |  2 +-
 .../ovs-windows-installer.wixproj | 15 ---
 6 files changed, 38 insertions(+), 19 deletions(-)

-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 1/4] datapath-windows: Force driver version to depend on a variable

[Alin Serdean]

The following components use Windows driver information:
-   System (inf file); used during device installation
-   Resource file (rc file); used by applications when looking over the 
driver
file(sys)

Currently we have the following for the driver version number:
-   (inf file) generated value from the build timestamp
-   (rc file) predefined value

This patch forces both files to depend on a variable: '$(Version)'.
This is a predefined variable from Visual Studio.

To achieve the above we change the current project settings used by the
'stampinf' utility and we define a new preprocessor value named
'VersionWithCommas' (which is obtained by replacing all
'.' with ',' from $(Version) ).
Certain values from the resource file are expected to use ',' instead of '.' .

The resource file has been updated to use the new values when generating
information about the driver (sys).

The variable '$(Version' can be changed from the command line via the
'msbuild' utility.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/ovsext.rc  | 11 ++-
 datapath-windows/ovsext/ovsext.vcxproj | 19 ++-
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/datapath-windows/ovsext/ovsext.rc 
b/datapath-windows/ovsext/ovsext.rc
index 0b92e2e..578367d 100644
--- a/datapath-windows/ovsext/ovsext.rc
+++ b/datapath-windows/ovsext/ovsext.rc
@@ -8,14 +8,15 @@
 LANGUAGE LANG_ENGLISH, SUBLANG_ENGLISH_US
 #pragma code_page(1252)
 
+#define STR(x)  #x
 /
 //
 // Version
 //
 
 VS_VERSION_INFO VERSIONINFO
- FILEVERSION 6,3,9600,17298
- PRODUCTVERSION 6,3,9600,17298
+ FILEVERSION VersionWithCommas
+ PRODUCTVERSION VersionWithCommas
  FILEFLAGSMASK 0x3fL
 #ifdef _DEBUG
  FILEFLAGS 0x9L
@@ -32,12 +33,12 @@ BEGIN
 BEGIN
 VALUE "CompanyName", "Open vSwitch"
 VALUE "FileDescription", "Open vSwitch Extension"
-VALUE "FileVersion", "6.3.9600.17298"
+VALUE "FileVersion", STR(Version)
 VALUE "InternalName", "OVSExt.SYS"
 VALUE "LegalCopyright", "Licensed under the Apache License, 
Version 2.0 (the ""License"")"
 VALUE "OriginalFilename", "OVSExt.SYS"
-VALUE "ProductName", "Open vSwitch 8/8.1 DDK driver"
-VALUE "ProductVersion", "6.3.9600.17298"
+VALUE "ProductName", "Open vSwitch"
+VALUE "ProductVersion", STR(Version)
 END
 END
 BLOCK "VarFileInfo"
diff --git a/datapath-windows/ovsext/ovsext.vcxproj 
b/datapath-windows/ovsext/ovsext.vcxproj
index 77530fd..dc0d2db 100644
--- a/datapath-windows/ovsext/ovsext.vcxproj
+++ b/datapath-windows/ovsext/ovsext.vcxproj
@@ -174,6 +174,18 @@
   true
   true
 
+
+  $(Version)
+
+
+  $(Version)
+
+
+  $(Version)
+
+
+  $(Version)
+
   
   
 
@@ -212,7 +224,12 @@
 
 
 
-
+
+  %(PreprocessorDefinitions);NDIS_WDM=1;NDIS630=1;VersionWithCommas=$(Version.Replace('.',','))
+  %(PreprocessorDefinitions);NDIS_WDM=1;NDIS630=1;VersionWithCommas=$(Version.Replace('.',','))
+  %(PreprocessorDefinitions);NDIS_WDM=1;NDIS640=1;VersionWithCommas=$(Version.Replace('.',','))
+  %(PreprocessorDefinitions);NDIS_WDM=1;NDIS640=1;VersionWithCommas=$(Version.Replace('.',','))
+
   
   
 
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 5/9] lport: Add index for logical datapaths.

[Mickey Spiegel]

On Sun, Dec 4, 2016 at 11:17 PM, Ben Pfaff  wrote:

> This will have its first real user in an upcoming commit.
>

I am not sure this is really necessary. See my comments on patch 6.

At the end there is deleted code for destroying patched_datapaths.
That deletion should not be in this patch, it should be in patch 6.

Acked-by: Mickey Spiegel 


> Signed-off-by: Ben Pfaff 
> ---
>  ovn/controller/lport.c  | 62 ++
> +++
>  ovn/controller/lport.h  | 33 --
>  ovn/controller/ovn-controller.c | 12 +++-
>  3 files changed, 97 insertions(+), 10 deletions(-)
>
> diff --git a/ovn/controller/lport.c b/ovn/controller/lport.c
> index 38aca22..906fda2 100644
> --- a/ovn/controller/lport.c
> +++ b/ovn/controller/lport.c
> @@ -22,6 +22,68 @@
>
>  VLOG_DEFINE_THIS_MODULE(lport);
>
> +static struct ldatapath *ldatapath_lookup_by_key__(
> +const struct ldatapath_index *, uint32_t dp_key);
> +
> +void
> +ldatapath_index_init(struct ldatapath_index *ldatapaths,
> + struct ovsdb_idl *ovnsb_idl)
> +{
> +hmap_init(>by_key);
> +
> +const struct sbrec_port_binding *pb;
> +SBREC_PORT_BINDING_FOR_EACH (pb, ovnsb_idl) {
> +if (!pb->datapath) {
> +continue;
> +}
> +uint32_t dp_key = pb->datapath->tunnel_key;
> +struct ldatapath *ld = ldatapath_lookup_by_key__(ldatapaths,
> dp_key);
> +if (!ld) {
> +ld = xzalloc(sizeof *ld);
> +hmap_insert(>by_key, >by_key_node, dp_key);
> +ld->db = pb->datapath;
> +}
> +
> +if (ld->n_lports >= ld->allocated_lports) {
> +ld->lports = x2nrealloc(ld->lports, >allocated_lports,
> +sizeof *ld->lports);
> +}
> +ld->lports[ld->n_lports++] = pb;
> +}
> +}
> +
> +void
> +ldatapath_index_destroy(struct ldatapath_index *ldatapaths)
> +{
> +if (!ldatapaths) {
> +return;
> +}
> +
> +struct ldatapath *ld, *ld_next;
> +HMAP_FOR_EACH_SAFE (ld, ld_next, by_key_node, >by_key) {
> +hmap_remove(>by_key, >by_key_node);
> +free(ld->lports);
> +free(ld);
> +}
> +hmap_destroy(>by_key);
> +}
> +
> +static struct ldatapath *ldatapath_lookup_by_key__(
> +const struct ldatapath_index *ldatapaths, uint32_t dp_key)
> +{
> +struct ldatapath *ld;
> +HMAP_FOR_EACH_WITH_HASH (ld, by_key_node, dp_key,
> >by_key) {
> +return ld;
> +}
> +return NULL;
> +}
> +
> +const struct ldatapath *ldatapath_lookup_by_key(
> +const struct ldatapath_index *ldatapaths, uint32_t dp_key)
> +{
> +return ldatapath_lookup_by_key__(ldatapaths, dp_key);
> +}
> +
>  /* A logical port. */
>  struct lport {
>  struct hmap_node name_node; /* Index by name. */
> diff --git a/ovn/controller/lport.h b/ovn/controller/lport.h
> index 0cad74a..fe0e430 100644
> --- a/ovn/controller/lport.h
> +++ b/ovn/controller/lport.h
> @@ -22,8 +22,37 @@
>  struct ovsdb_idl;
>  struct sbrec_datapath_binding;
>
> -/* Logical port and multicast group indexes
> - * 
> +/* Database indexes.
> + * =
> + *
> + * If the database IDL were a little smarter, it would allow us to
> directly
> + * look up data based on values of its fields.  It's not that smart
> (yet), so
> + * instead we define our own indexes.
> + */
> +
> +/* Logical datapath index
> + * ==
> + */
> +
> +struct ldatapath {
> +struct hmap_node by_key_node; /* Index by tunnel key. */
> +const struct sbrec_datapath_binding *db;
> +const struct sbrec_port_binding **lports;
> +size_t n_lports, allocated_lports;
> +};
> +
> +struct ldatapath_index {
> +struct hmap by_key;
> +};
> +
> +void ldatapath_index_init(struct ldatapath_index *, struct ovsdb_idl *);
> +void ldatapath_index_destroy(struct ldatapath_index *);
> +
> +const struct ldatapath *ldatapath_lookup_by_key(
> +const struct ldatapath_index *, uint32_t dp_key);
> +
> +/* Logical port index
> + * ==
>   *
>   * This data structure holds multiple indexes over logical ports, to
> allow for
>   * efficient searching for logical ports by name or number.
> diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-
> controller.c
> index 894af6f..5b95a55 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -512,8 +512,10 @@ main(int argc, char *argv[])
>  const struct ovsrec_bridge *br_int = get_br_int();
>  const char *chassis_id = get_chassis_id(ctx.ovs_idl);
>
> +struct ldatapath_index ldatapaths;
>  struct lport_index lports;
>  struct mcgroup_index mcgroups;
> +ldatapath_index_init(, ctx.ovnsb_idl);
>  lport_index_init(, ctx.ovnsb_idl);
>  mcgroup_index_init(, ctx.ovnsb_idl);
>
> @@ -561,6 +563,8 @@ main(int argc, char 

Re: [ovs-dev] [PATCH 4/9] lport: Tolerate null pointers in destroy functions.

[Mickey Spiegel]

On Sun, Dec 4, 2016 at 11:17 PM, Ben Pfaff  wrote:

> The coding style says to do this.
>
> The actual caller doesn't pass a null pointer.
>

Acked-by: Mickey Spiegel 


> Signed-off-by: Ben Pfaff 
> ---
>  ovn/controller/lport.c | 8 
>  1 file changed, 8 insertions(+)
>
> diff --git a/ovn/controller/lport.c b/ovn/controller/lport.c
> index 3484c2c..38aca22 100644
> --- a/ovn/controller/lport.c
> +++ b/ovn/controller/lport.c
> @@ -68,6 +68,10 @@ lport_index_init(struct lport_index *lports, struct
> ovsdb_idl *ovnsb_idl)
>  void
>  lport_index_destroy(struct lport_index *lports)
>  {
> +if (!lports) {
> +return;
> +}
> +
>  /* Destroy all of the "struct lport"s.
>   *
>   * We don't have to remove the node from both indexes. */
> @@ -141,6 +145,10 @@ mcgroup_index_init(struct mcgroup_index *mcgroups,
> struct ovsdb_idl *ovnsb_idl)
>  void
>  mcgroup_index_destroy(struct mcgroup_index *mcgroups)
>  {
> +if (!mcgroups) {
> +return;
> +}
> +
>  struct mcgroup *mcgroup, *next;
>  HMAP_FOR_EACH_SAFE (mcgroup, next, dp_name_node,
> >by_dp_name) {
>  hmap_remove(>by_dp_name, >dp_name_node);
> --
> 2.10.2
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 3/9] lport: Be a little more careful building lport index.

[Mickey Spiegel]

On Sun, Dec 4, 2016 at 11:17 PM, Ben Pfaff  wrote:

> It seems like a good idea to check for and warn about all kinds of
> duplicates, and to avoid segfaulting if a datapath column is empty.
> (However, the database schema should prevent both issues.)
>

Acked-by: Mickey Spiegel 


>
> Signed-off-by: Ben Pfaff 
> ---
>  ovn/controller/lport.c | 12 
>  1 file changed, 12 insertions(+)
>
> diff --git a/ovn/controller/lport.c b/ovn/controller/lport.c
> index e1ecf21..3484c2c 100644
> --- a/ovn/controller/lport.c
> +++ b/ovn/controller/lport.c
> @@ -37,12 +37,24 @@ lport_index_init(struct lport_index *lports, struct
> ovsdb_idl *ovnsb_idl)
>
>  const struct sbrec_port_binding *pb;
>  SBREC_PORT_BINDING_FOR_EACH (pb, ovnsb_idl) {
> +if (!pb->datapath) {
> +continue;
> +}
> +
>  if (lport_lookup_by_name(lports, pb->logical_port)) {
>  static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
>  VLOG_WARN_RL(, "duplicate logical port name '%s'",
>   pb->logical_port);
>  continue;
>  }
> +if (lport_lookup_by_key(lports, pb->datapath->tunnel_key,
> +pb->tunnel_key)) {
> +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> +VLOG_WARN_RL(, "duplicate logical port %"PRId64" in
> logical "
> + "datapath %"PRId64,
> + pb->tunnel_key, pb->datapath->tunnel_key);
> +continue;
> +}
>
>  struct lport *p = xmalloc(sizeof *p);
>  hmap_insert(>by_name, >name_node,
> --
> 2.10.2
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovsdb-server: Add ovsdb-server/is-backup-server command in unixctl

[Numan Siddique]

On Dec 6, 2016 3:18 AM, "Ben Pfaff"  wrote:

Hi Numan, is this still relevant?  It seems to have been overlooked,
since I can't find any followups on the mailing lists.  If it's still
worth merging, would you mind posting a rebased version?

Thanks, and sorry to have missed this!


Hi Ben,
No its not relevant as "ovsdb-server/sync-status" provides the same
information.

Thanks
Numan


Ben

On Fri, Aug 19, 2016 at 08:17:43PM +0530, Numan Siddique wrote:
> This command will be useful to query if the ovsdb-server
> instance is active or backup.
>
> Signed-off-by: Numan Siddique 
> ---
>  ovsdb/ovsdb-server.c | 17 +
>  1 file changed, 17 insertions(+)
>
> diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
> index e08c341..283ec1b 100644
> --- a/ovsdb/ovsdb-server.c
> +++ b/ovsdb/ovsdb-server.c
> @@ -81,6 +81,7 @@ static unixctl_cb_func ovsdb_server_set_active_ovsdb_
server;
>  static unixctl_cb_func ovsdb_server_get_active_ovsdb_server;
>  static unixctl_cb_func ovsdb_server_connect_active_ovsdb_server;
>  static unixctl_cb_func ovsdb_server_disconnect_active_ovsdb_server;
> +static unixctl_cb_func ovsdb_server_is_backup_server;
>  static unixctl_cb_func ovsdb_server_set_sync_excluded_tables;
>  static unixctl_cb_func ovsdb_server_get_sync_excluded_tables;
>
> @@ -361,6 +362,9 @@ main(int argc, char *argv[])
>  unixctl_command_register("ovsdb-server/disconnect-active-ovsdb-server",
"",
>   0, 0, ovsdb_server_disconnect_
active_ovsdb_server,
>   NULL);
> +unixctl_command_register("ovsdb-server/is-backup-server", "",
> + 0, 0, ovsdb_server_is_backup_server,
> + NULL);
>  unixctl_command_register("ovsdb-server/set-sync-excluded-tables", "",
>   0, 1, ovsdb_server_set_sync_excluded_tables,
>   NULL);
> @@ -1104,6 +1108,19 @@ ovsdb_server_disconnect_active_ovsdb_server(struct
unixctl_conn *conn,
>  }
>
>  static void
> +ovsdb_server_is_backup_server(struct unixctl_conn *conn,
> + int argc OVS_UNUSED,
> + const char *argv[] OVS_UNUSED,
> + void *arg_ OVS_UNUSED)
> +{
> +struct ds s;
> +ds_init();
> +ds_put_format(, "%s\n", is_backup_server ? "true": "false");
> +unixctl_command_reply(conn, ds_cstr());
> +ds_destroy();
> +}
> +
> +static void
>  ovsdb_server_set_sync_excluded_tables(struct unixctl_conn *conn,
>int argc OVS_UNUSED,
>const char *argv[],
> --
> 2.7.4
>
> ___
> dev mailing list
> d...@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 6/9] ovn-controller: Handle only relevant ports and flows.

[Mickey Spiegel]

On Sun, Dec 4, 2016 at 11:17 PM, Ben Pfaff  wrote:

> On a particular hypervisor, ovn-controller only needs to handle ports
> and datapaths that have some relationship with it, that is, the
> ports that actually reside on the hypervisor, plus all the other ports on
> those ports' datapaths, plus all of the ports and datapaths that are
> reachable from those via logical patch ports.  Until now, ovn-controller
> has done a poor job of limiting what it deals with to this set.  This
> commit improves the situation.
>
> This commit gets rid of the concept of a "patched_datapath" which until now
> was used to represent any datapath that contained a logical patch port.
> Previously, the concept of a "local_datapath" meant a datapath with a VIF
> that resides on the local hypervisor.  This commit extends that concept to
> include any other datapath that can be reached from a VIF on the local
> hypervisor, which is a simplification that makes the code easier to
> understand in a few places.
>

I like the change to remove "patched_datapath" and consolidate into
the concept of "local_datapath". Among other issues, it solves my
localnet problem for distributed NAT.

The big question is the relationship with datapaths of interest. Both
this and datapaths of interest trigger a computation from
add_local_datapath, to find other datapaths that can be reached on
the local hypervisor. At a high level, the difference is that this
computation is based on patch ports from a datapath, while the
datapaths of interest computation is based on a list of neighbor
datapaths that is populated by northd into each datapath_binding.
Note that northd populates related_datapaths based on patch
ports, so in the end it is the same source of information, but
different parts of the computation are done in different places at
different times.

If it were only desired to do conditional monitoring of logical_flow,
then related_datapaths would not be necessary at all.
Local_datapath, as computed in this patch, could be used to
trigger the necessary logical flow clauses.

With the desire to do conditional monitoring of port_binding,
things get more complicated. The computation in this patch
depends on having the port_bindings for other datapaths,
leading to a chicken and egg problem. It seems like northd
population of related_datapaths is necessary for conditional
monitoring of port_bindings. This leads to the question
whether local_datapaths should be computed as in this
patch? or whether there should be a single computation
over related_datapaths to determine local_datapaths, which
would then be used to trigger conditional monitoring clauses
for datapaths? Is it safe for this computation to depend on
partial computation from northd? Or are there any timing
windows that might lead to corner cases?

Depending on the answer to this question, patches 2 and 5 in
this patch set may or may not be necessary.

One question and one comment inline.

>
> CC: Gurucharan Shetty 
> Signed-off-by: Ben Pfaff 
> ---
>  ovn/controller/binding.c| 75 ++-
>  ovn/controller/binding.h|  7 ++-
>  ovn/controller/lflow.c  | 46 ++-
>  ovn/controller/lflow.h  |  1 -
>  ovn/controller/ovn-controller.c | 41 ++---
>  ovn/controller/ovn-controller.h | 33 +++---
>  ovn/controller/patch.c  | 98 --
> ---
>  ovn/controller/patch.h  |  5 +--
>  ovn/controller/physical.c   | 30 ++---
>  ovn/controller/physical.h   |  4 +-
>  tests/ovn-controller.at | 19 +++-
>  11 files changed, 165 insertions(+), 194 deletions(-)
>
> diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
> index fb76032..b53af98 100644
> --- a/ovn/controller/binding.c
> +++ b/ovn/controller/binding.c
>




> @@ -342,7 +390,8 @@ consider_local_datapath(struct controller_ctx *ctx,
>  const char *chassis = smap_get(_rec->options,
> "l3gateway-chassis");
>  if (!strcmp(chassis, chassis_rec->name) && ctx->ovnsb_idl_txn) {
> -add_local_datapath(local_datapaths, binding_rec);
> +add_local_datapath(ldatapaths, lports, binding_rec->datapath,
> +   false, local_datapaths);
>

Why did you set has_local_l3gateway to false here, then change
it to true in patch 9?
I don't see the harm in setting it to true here.


>  }
>  } else if (chassis_rec && binding_rec->chassis == chassis_rec) {
>  if (ctx->ovnsb_idl_txn) {
>




> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
>

There is a fragment of code to destroy patched_datapaths that
was deleted at the end of patch 5. It should not have been deleted
in patch 5, but it should be deleted here.

Mickey
___
dev mailing list
d...@openvswitch.org

Re: [ovs-dev] [PATCH 2/2] tests: Add tracing to "ovn -- 2 HVs, 4 lports/HV, localnet ports" test.

[Russell Bryant]

On Sun, Oct 23, 2016 at 1:50 PM, Ben Pfaff  wrote:

> Adding ovn-trace calls makes failures easier to understand and diagnose.
>
> Signed-off-by: Ben Pfaff 
> ---
>  tests/ovn.at | 96 ++
> +++---
>  1 file changed, 60 insertions(+), 36 deletions(-)
>

Acked-by: Russell Bryant 


-- 
Russell Bryant
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/2] tests: Fix order confusion in "ovn -- 2 HVs, 4 lports/HV, localnet ports".

[Russell Bryant]

On Sun, Oct 23, 2016 at 1:50 PM, Ben Pfaff  wrote:

> The order of src and dst was swapped both in assignment and reference,
> which meant that the result worked OK but was really confusing to try to
> extend or modify.
>
> Signed-off-by: Ben Pfaff 
>

Acked-by: Russell Bryant 


> ---
>  tests/ovn.at | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index da0291f..1d782a3 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -1592,8 +1592,8 @@ for i in 1 2; do
>  done
>  done
>  test_packet() {
> -local inport=$1 src=$2 dst=$3 eth=$4; shift; shift; shift; shift
> -local packet=${src}${dst}${eth}
> +local inport=$1 dst=$2 src=$3 eth=$4; shift; shift; shift; shift
> +local packet=${dst}${src}${eth}
>  hv=`vif_to_hv $inport`
>  vif=vif$inport
>  as $hv ovs-appctl netdev-dummy/receive $vif $packet
> --
> 2.1.3
>
> ___
> dev mailing list
> d...@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>



-- 
Russell Bryant
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovn-trace: Add output format options to usage message.

[Russell Bryant]

On Mon, Dec 5, 2016 at 8:01 PM, Ben Pfaff  wrote:

> On Sun, Oct 23, 2016 at 11:25:02AM -0700, Ben Pfaff wrote:
> > Also adjust the indentation of the option explanations so that they line
> up
> > better.
> >
> > Signed-off-by: Ben Pfaff 
>
> This still needs a review, which should be trivial.


Acked-by: Russell Bryant 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/2] tests: Fix order confusion in "ovn -- 2 HVs, 4 lports/HV, localnet ports".

[Ben Pfaff]

On Sun, Oct 23, 2016 at 10:50:35AM -0700, Ben Pfaff wrote:
> The order of src and dst was swapped both in assignment and reference,
> which meant that the result worked OK but was really confusing to try to
> extend or modify.
> 
> Signed-off-by: Ben Pfaff 

This series still needs a review.

Thanks,

Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovn-trace: Add output format options to usage message.

[Ben Pfaff]

On Sun, Oct 23, 2016 at 11:25:02AM -0700, Ben Pfaff wrote:
> Also adjust the indentation of the option explanations so that they line up
> better.
> 
> Signed-off-by: Ben Pfaff 

This still needs a review, which should be trivial.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH net-next] net: remove abuse of VLAN DEI/CFI bit

[Ben Pfaff]

On Mon, Dec 05, 2016 at 11:52:47PM +0100, Michał Mirosław wrote:
> On Mon, Dec 05, 2016 at 10:55:45AM -0800, Ben Pfaff wrote:
> > On Mon, Dec 05, 2016 at 06:24:36PM +0100, Michał Mirosław wrote:
> > > On Sat, Dec 03, 2016 at 03:27:30PM -0800, Ben Pfaff wrote:
> > > > On Sat, Dec 03, 2016 at 10:22:28AM +0100, Michał Mirosław wrote:
> > > > > This All-in-one patch removes abuse of VLAN CFI bit, so it can be 
> > > > > passed
> > > > > intact through linux networking stack.
> > > > This appears to change the established Open vSwitch userspace API.  You
> > > > can see that simply from the way that it changes the documentation for
> > > > the userspace API.  If I'm right about that, then this change will break
> > > > all userspace programs that use the Open vSwitch kernel module,
> > > > including Open vSwitch itself.
> > > 
> > > If I understood the code correctly, it does change expected meaning for
> > > the (unlikely?) case of header truncated just before the VLAN TCI - it 
> > > will
> > > be impossible to differentiate this case from the VLAN TCI == 0.
> > > 
> > > I guess this is a problem with OVS API, because it doesn't directly show
> > > the "missing" state of elements, but relies on an "invalid" value.
> > 
> > That particular corner case should not be a huge problem in any case.
> > 
> > The real problem is that this appears to break the common case use of
> > VLANs in Open vSwitch.  After this patch, parse_vlan() in
> > net/openvswitch/flow.c copies the tpid and tci from sk_buff (either the
> > accelerated version of them or the version in the skb data) into
> > sw_flow_key members.  OK, that's fine on it's own.  However, I don't see
> > any corresponding change to the code in flow_netlink.c to compensate for
> > the fact that, until now, the VLAN CFI bit (formerly VLAN_TAG_PRESENT)
> > was always required to be set to 1 in flow matches inside Netlink
> > messages sent from userspace, and the kernel always set it to 1 in
> > corresponding messages sent to userspace.
> > 
> > In other words, if I'm reading this change correctly:
> > 
> > * With a kernel before this change, userspace always had to set
> >   VLAN_TAG_PRESENT to 1 to match on a VLAN, or the kernel would
> >   reject the flow match.
> > 
> > * With a kernel after this change, userspace must not set
> >   VLAN_TAG_PRESENT to 1, otherwise the kernel will accept the flow
> >   match but nothing will ever match because packets do not actually
> >   have the CFI bit set.
> > 
> > Take a look at this code that the patch deletes from
> > validate_vlan_from_nlattrs(), for example, and see how it insisted that
> > VLAN_TAG_PRESENT was set:
> > 
> > if (!(tci & htons(VLAN_TAG_PRESENT))) {
> > if (tci) {
> > OVS_NLERR(log, "%s TCI does not have VLAN_TAG_PRESENT 
> > bit set.",
> >   (inner) ? "C-VLAN" : "VLAN");
> > return -EINVAL;
> > } else if (nla_len(a[OVS_KEY_ATTR_ENCAP])) {
> > /* Corner case for truncated VLAN header. */
> > OVS_NLERR(log, "Truncated %s header has non-zero encap 
> > attribute.",
> >   (inner) ? "C-VLAN" : "VLAN");
> > return -EINVAL;
> > }
> > }
> > 
> > Please let me know if I'm overlooking something.
> 
> Hmm. So the easiest change without disrupting current userspace, would be
> to flip the CFI bit on the way to/from OVS userspace. Does this seem
> correct?

That sounds correct.  (The bit should not be flipped in the mask.)

Thanks,

Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Trabajadores conflictivos e improductivos

[Experto en Rescisión Laboral]

En línea y en Vivo / Para todo su Equipo con una sola Conexión 

Promoción Navideña – Pregunte por Ella



Seminario Experto en Rescisión Laboral
Cómo despedir trabajadores flojos, conflictivos e improductivos


 Devolución de Impuestos Y Las Reacciones de las Autoridades fiscales 
Cómo proteger sus derechos en la PRODECON
 
Temario: 

1. Conocimiento de las causales de rescisión.
2. Por qué se hace la rescisión de contrato y a quiénes se aplica.
3. Cómo tomar la decisión de despedir a un trabajador.
4. Jurisprudencias relativas, comentarios, ejemplos, ejercicios.
5. Cómo una manzana podrida puede dañar a los demás.


 Temario: 

1. Cómo contestar los requerimientos excesivos.

2. Cómo documentar operaciones.

3. Implicaciones y solicitudes de devolución, compensación y auditorías.

4. Las prácticas de autoridades fiscales para dilatar, negar o tener por 
desistidas las solicitudes.
 
¿Requiere la información a la Brevedad? responda este email con la palabra: 
Info – Laboral.
Junto con los Siguientes Datos: Nombre, Teléfono, Empresa.
 ¿Requiere la información a la Brevedad?
responda este email con la palabra: 
Info - Impuestos.
Junto con los Siguientes Datos: Nombre, Teléfono, Empresa.


 
centro telefónico:018002129393
 
¿Demasiados mensajes en su cuenta? Responda este mensaje indicando que solo 
desea recibir CALENDARIO y sólo recibirá un correo al mes. Si desea cancelar la 
suscripción, solicite su BAJA. 
 
 


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Implementación de Lean Manufacturing

[Reducción de Desperdicios]

 

Una herramienta financiera para mejorar resultados

Implementación de Lean Manufacturing
16 de diciembre - Zona Reforma (Ciudad de México) - 9:00 a 18:00
Imparte: MCE. Abdón Guzmán S.  
 
Lean Manufacturing es un sistema de gestión de cómo operar de forma adecuada un 
negocio, enfocado en una serie de herramientas que permiten la eliminación de 
todo tipo de desperdicios, procesos y actividades que utilizan más recursos de 
los necesarios. Con la implementación de Lean, se pretende que exista una 
reducción considerable en los costos de producción. Lean Manufacturing, más 
allá de las técnicas concretas que se apliquen, es un sistema que incorpora una 
reorganización cultural, que requiere un alto grado de compromiso de toda la 
empresa. 
 
TEMARIO: 


1.Porque Lean Manufacturing es exitoso 

2. Cinco elementos primarios de Lean Manufacturing. 

3. Como identificar las áreas que generan desperdicios o pérdidas 



...¡Y mucho más!


 
¿Requiere la información a la Brevedad?
responda este email con la palabra: 
Info Lean
centro telefónico:018002120744
 


Será un placer Atenderle 


 
Si desea cancelar la suscripción, solicite su BAJA y se realizará en las 
próximas 24 Hrs. 
 

 

 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] datapath-windows: null comparison for icmp and tcp header

[Joe Stringer]

On 5 December 2016 at 12:16, Alin Serdean
 wrote:
> Thanks for the tips Joe!
>
> It causes a kernel crash under low resources.

OK, thanks. Please include this information in future git commit messages.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] ovn-controller(8): ovn-remote must be an ovsdb remote.

[Russell Bryant]

Document that the value of the ovn-remote configuration
option must be in the form of an ovsdb remote as previously documented
in this man page. This came up on IRC where someone trying OVN
put a hostname here and observed that it did not work.

Signed-off-by: Russell Bryant 
---
 ovn/controller/ovn-controller.8.xml | 9 ++---
 ovn/controller/ovn-controller.c | 5 +
 2 files changed, 3 insertions(+), 11 deletions(-)

v1->v2:
 - Drop old docs and a code comment that said you couldn't change ovn-remote at 
runtime
 - Update patch to specify that the value is an ovsdb remote (not an IP address 
as
   specified in v1)

diff --git a/ovn/controller/ovn-controller.8.xml 
b/ovn/controller/ovn-controller.8.xml
index 5f51cb1..9f4dad1 100644
--- a/ovn/controller/ovn-controller.8.xml
+++ b/ovn/controller/ovn-controller.8.xml
@@ -93,13 +93,8 @@
   
 
   The OVN database that this system should connect to for its
-  configuration.
-
-
-
-  Currently, ovn-controller does not support changing this
-  setting mid-run.  If the value needs to change, the daemon must be
-  restarted.  (This behavior should be improved.)
+  configuration, in one of the same forms documented above for the
+  ovs-database.
 
   
 
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index fea7841..4fbf455 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -204,10 +204,7 @@ get_chassis_id(const struct ovsdb_idl *ovs_idl)
 }
 
 /* Retrieves the OVN Southbound remote location from the
- * "external-ids:ovn-remote" key in 'ovs_idl' and returns a copy of it.
- *
- * XXX ovn-controller does not support this changing mid-run, but that should
- * be addressed later. */
+ * "external-ids:ovn-remote" key in 'ovs_idl' and returns a copy of it. */
 static char *
 get_ovnsb_remote(struct ovsdb_idl *ovs_idl)
 {
-- 
2.9.3

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC] Introducing experimental OVN integration for Mesos

[Ben Pfaff]

Guru, did you ever have a look at this?  It's always nice to integrate
with more systems.

Nimay, I feel bad that apparently no one ever responded to this.

On Fri, Aug 05, 2016 at 03:54:47PM -0700, Nimay Desai wrote:
> This commit introduces experimental support for OVN integration with Apache
> Mesos.  It is experimental because the network plugability infrastructure for
> Mesos is being continuously developed in the Mesos master branch.  Mesos does
> not yet have all the components necessary to allow usage of OVN as a complete
> container networking solution.  Mainly, it lacks the port mapping
> infrastructure to support North to South connectivity.
> 
> With this commit, you can have clean East-West and South-North connectivity.
> INSTALL.Mesos.md includes instructions on how to use the setup scripts to
> create an OVN overlay network and attach Mesos nodes to the network.  It also
> includes instructions on how to set up the plugin and start Mesos so that
> containers automatically connect to the OVN network upon creation.
> 
> Signed-off-by: Nimay Desai 
> ---
>  INSTALL.Mesos.md   | 176 +++
>  Makefile.am|   1 +
>  ovn/utilities/automake.mk  |   8 +-
>  ovn/utilities/ovn-mesos-overlay-driver | 182 
> +
>  ovn/utilities/ovn-mesos-plugin | 105 +++
>  python/automake.mk |  15 ++-
>  python/ovn/__init__.py |   1 +
>  python/ovn/mesos/__init__.py   |   1 +
>  python/ovn/mesos/ovnutil.py|  71 +
>  9 files changed, 554 insertions(+), 6 deletions(-)
>  create mode 100644 INSTALL.Mesos.md
>  create mode 100755 ovn/utilities/ovn-mesos-overlay-driver
>  create mode 100755 ovn/utilities/ovn-mesos-plugin
>  create mode 100644 python/ovn/__init__.py
>  create mode 100644 python/ovn/mesos/__init__.py
>  create mode 100644 python/ovn/mesos/ovnutil.py
> 
> diff --git a/INSTALL.Mesos.md b/INSTALL.Mesos.md
> new file mode 100644
> index 000..dbeee81
> --- /dev/null
> +++ b/INSTALL.Mesos.md
> @@ -0,0 +1,176 @@
> +How to Use Open vSwitch with Apache Mesos
> +=
> +
> +This document describes how to use Open Virtual Networking with Apache Mesos 
> .
> +This document assumes that you installed Open vSwitch by following 
> [INSTALL.md]
> +or by using the distribution packages such as .deb or .rpm.  Consult
> +www.mesos.apache.org for instructions on how to install Mesos.
> +
> +
> +Setup
> +=
> +
> +* Start the central components.
> +
> +OVN architecture has a central component which stores your networking intent
> +in a database.  On one of your machines, with an IP Address of $CENTRAL_IP,
> +where you have installed and started Open vSwitch, you will need to start 
> some
> +central components.
> +
> +Start ovn-northd daemon.  This daemon translates networking intent from Mesos
> +stored in the OVN_Northbound database to logical flows in OVN_Southbound
> +database.  It is also responsible for managing and dynamically allocating
> +IP/MAC addresses for Mesos containers.
> +
> +```
> +/usr/share/openvswitch/scripts/ovn-ctl start_northd
> +```
> +
> +* One time setup.
> +
> +On each host, where you plan to spawn your containers, you will need to
> +run the following command once.  (You need to run it again if your OVS 
> database
> +gets cleared.  It is harmless to run it again in any case.)
> +
> +$LOCAL_IP in the below command is the IP address via which other hosts
> +can reach this host.  This acts as your local tunnel endpoint.
> +
> +$ENCAP_TYPE is the type of tunnel that you would like to use for overlay
> +networking.  The options are "geneve" or "stt".  (Please note that your
> +kernel should have support for your chosen $ENCAP_TYPE.  Both geneve
> +and stt are part of the Open vSwitch kernel module that is compiled from this
> +repo.  If you use the Open vSwitch kernel module from upstream Linux,
> +you will need a minumum kernel version of 3.18 for geneve.  There is no stt
> +support in upstream Linux.  You can verify whether you have the support in 
> your
> +kernel by doing a "lsmod | grep $ENCAP_TYPE".)
> +
> +```
> +ovs-vsctl set Open_vSwitch . external_ids:ovn-remote="tcp:$CENTRAL_IP:6641" \
> +  external_ids:ovn-nb="tcp:$CENTRAL_IP:6641" 
> external_ids:ovn-encap-ip=$LOCAL_IP external_ids:ovn-encap-type="$ENCAP_TYPE"
> +```
> +
> +And finally, start the ovn-controller.  (You need to run the below command
> +on every boot)
> +
> +```
> +/usr/share/openvswitch/scripts/ovn-ctl start_controller
> +```
> +
> +* Initialize the OVN network using the OVN network driver.
> +
> +Run the OVN network driver with the "plugin-init" subcommand once on any 
> host.
> +Running "ovn-nbctl show" should now display a single logical router called
> +"mesos-router."
> +
> +```
> +PYTHONPATH=$OVS_PYTHON_LIBS_PATH ovn-mesos-overlay-driver plugin-init
> 

Re: [ovs-dev] [PATCH] ovn-controller(8): ovn-remote must be an IP adddress.

[Russell Bryant]

On Mon, Dec 5, 2016 at 1:13 PM, Ben Pfaff  wrote:

> On Mon, Dec 05, 2016 at 10:10:43AM -0800, Ben Pfaff wrote:
> > On Mon, Dec 05, 2016 at 12:05:33PM -0500, Russell Bryant wrote:
> > > Document that the value of the ovn-remote configuration
> > > option must be an IP address.  This came up on IRC where
> > > someone trying OVN put a hostname here and observed that
> > > it did not work.
> > >
> > > Signed-off-by: Russell Bryant 
> > > ---
> > >  ovn/controller/ovn-controller.8.xml | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/ovn/controller/ovn-controller.8.xml b/ovn/controller/ovn-
> controller.8.xml
> > > index 5f51cb1..825a538 100644
> > > --- a/ovn/controller/ovn-controller.8.xml
> > > +++ b/ovn/controller/ovn-controller.8.xml
> > > @@ -93,7 +93,7 @@
> > >
> > >  
> > >The OVN database that this system should connect to for its
> > > -  configuration.
> > > +  configuration.  The value must be an IP address.
> > >  
> >
> > Not an IP address but an OVSDB remote in the form tcp:ip[:port] or
> > ssl:ip[:port].
>

Ah yes.  Thanks.


>
> Actually the documentation just below that paragraph is wrong too.
>
> Maybe something like this:
>
> diff --git a/ovn/controller/ovn-controller.8.xml b/ovn/controller/ovn-
> controller.8.xml
> index 5f51cb1..9f4dad1 100644
> --- a/ovn/controller/ovn-controller.8.xml
> +++ b/ovn/controller/ovn-controller.8.xml
> @@ -93,13 +93,8 @@
>
>  
>The OVN database that this system should connect to for its
> -  configuration.
> -
> -
> -
> -  Currently, ovn-controller does not support
> changing this
> -  setting mid-run.  If the value needs to change, the daemon must
> be
> -  restarted.  (This behavior should be improved.)
> +  configuration, in one of the same forms documented above for the
> +  ovs-database.
>  
>
>
>
I thought this option didn't take effect mid-run, but it's because there's
a stale comment left in the code.

I'll fix this up in v2.

diff --git a/ovn/controller/ovn-controller.c
b/ovn/controller/ovn-controller.c
index fea7841..4fbf455 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -204,10 +204,7 @@ get_chassis_id(const struct ovsdb_idl *ovs_idl)
 }

 /* Retrieves the OVN Southbound remote location from the
- * "external-ids:ovn-remote" key in 'ovs_idl' and returns a copy of it.
- *
- * XXX ovn-controller does not support this changing mid-run, but that
should
- * be addressed later. */
+ * "external-ids:ovn-remote" key in 'ovs_idl' and returns a copy of it. */
 static char *
 get_ovnsb_remote(struct ovsdb_idl *ovs_idl)
 {
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovsdb-server: Add ovsdb-server/is-backup-server command in unixctl

[Ben Pfaff]

Hi Numan, is this still relevant?  It seems to have been overlooked,
since I can't find any followups on the mailing lists.  If it's still
worth merging, would you mind posting a rebased version?

Thanks, and sorry to have missed this!

Ben

On Fri, Aug 19, 2016 at 08:17:43PM +0530, Numan Siddique wrote:
> This command will be useful to query if the ovsdb-server
> instance is active or backup.
> 
> Signed-off-by: Numan Siddique 
> ---
>  ovsdb/ovsdb-server.c | 17 +
>  1 file changed, 17 insertions(+)
> 
> diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
> index e08c341..283ec1b 100644
> --- a/ovsdb/ovsdb-server.c
> +++ b/ovsdb/ovsdb-server.c
> @@ -81,6 +81,7 @@ static unixctl_cb_func ovsdb_server_set_active_ovsdb_server;
>  static unixctl_cb_func ovsdb_server_get_active_ovsdb_server;
>  static unixctl_cb_func ovsdb_server_connect_active_ovsdb_server;
>  static unixctl_cb_func ovsdb_server_disconnect_active_ovsdb_server;
> +static unixctl_cb_func ovsdb_server_is_backup_server;
>  static unixctl_cb_func ovsdb_server_set_sync_excluded_tables;
>  static unixctl_cb_func ovsdb_server_get_sync_excluded_tables;
>  
> @@ -361,6 +362,9 @@ main(int argc, char *argv[])
>  unixctl_command_register("ovsdb-server/disconnect-active-ovsdb-server", 
> "",
>   0, 0, 
> ovsdb_server_disconnect_active_ovsdb_server,
>   NULL);
> +unixctl_command_register("ovsdb-server/is-backup-server", "",
> + 0, 0, ovsdb_server_is_backup_server,
> + NULL);
>  unixctl_command_register("ovsdb-server/set-sync-excluded-tables", "",
>   0, 1, ovsdb_server_set_sync_excluded_tables,
>   NULL);
> @@ -1104,6 +1108,19 @@ ovsdb_server_disconnect_active_ovsdb_server(struct 
> unixctl_conn *conn,
>  }
>  
>  static void
> +ovsdb_server_is_backup_server(struct unixctl_conn *conn,
> + int argc OVS_UNUSED,
> + const char *argv[] OVS_UNUSED,
> + void *arg_ OVS_UNUSED)
> +{
> +struct ds s;
> +ds_init();
> +ds_put_format(, "%s\n", is_backup_server ? "true": "false");
> +unixctl_command_reply(conn, ds_cstr());
> +ds_destroy();
> +}
> +
> +static void
>  ovsdb_server_set_sync_excluded_tables(struct unixctl_conn *conn,
>int argc OVS_UNUSED,
>const char *argv[],
> -- 
> 2.7.4
> 
> ___
> dev mailing list
> d...@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH net-next v2] ofproto: Honor OFPFF_RESET_COUNTS flag in flow modify message.

[Jarno Rajahalme]

> On Dec 1, 2016, at 3:44 PM, Ben Pfaff  wrote:
> 
> On Thu, Dec 01, 2016 at 01:09:12PM -0800, Jarno Rajahalme wrote:
>>> If I may be permitted to nit-pick, the name "modify_forward_counts" took
>>> me a bit of thinking to properly understand.  Maybe "modify_keep_stats"
>>> would be easier for me to understand at first glance.
>> 
>> “stats” include the last used timestamp, which is treated
>> independently of the byte and packet counts. How about
>> “modify_keep_counts”?
> 
> Sure.


Pushed to master and branch-2.6.

Backported the earlier used-timestamp fix to branch-2.5, and squashed in a fix 
for that patch, since it was the one introducing the reset_counts regression, 
and also squashed in the new test case to verify behavior on branch-2.5. 
Branch-2.5 is OK apart from a difference in reported packet size (54 vs. 60 
bytes). Since that difference is unrelated, I modified the test case and pushed 
the result to branch-2.5.

  Jarno

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC] ovn: Securing from compromised host - removal of MAC_Binding

[Ben Pfaff]

On Fri, Nov 18, 2016 at 10:13:23AM +0530, Babu Shanmugam wrote:
> 2) When the destination for an ARP reply is an OVN distributed router,
> broadcast the response to all instances of the logical router so that the
> result is available in the local cache of each router instance.

This is a reasonable approach, if we can come up with a good way to
implement the broadcast.

3) The approach used in NSX is to assign each HV that hosts a
   distributed router an individual MAC address and use that to forward
   the reply back to the particular HV that needed it.

The MAC_Binding implementation isn't industrial strength, so it needs
work in any case and I was leaning toward either 2) or 3).
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 9/9] ovn-controller: Drop most uses of OVS patch ports.

[Mickey Spiegel]

On Sun, Dec 4, 2016 at 11:17 PM, Ben Pfaff  wrote:

> Until now, ovn-controller has implemented OVN logical patch ports and
> l3gateway ports in terms of OVS patch ports.  It is a hassle to create and
> destroy ports, and it is also wasteful compared to what the patch ports
> actually buy us: the ability to "save and restore" a packet around a
> recursive trip through the flow table.  The "clone" action can do that too,
> without the need to create a port.  This commit takes advantage of the
> clone action for that purpose, getting rid of most of the patch ports
> previously created by ovn-controller.
>

I like it. It took me a bit to change my mindset from most physical.c flows
based on local ports with ofport values, splitting into some local ports
with ofport values and other local ports accessing the same refactored
code directly from a walk of sbrec_port_binding, based on port type.

This will allow me to cut a bunch of code from the first chassisredirect
patch in my distributed NAT patch set, similar to the way you cut
add_logical_path_ports in patch.c. More importantly for the distributed
NAT patch set, this solves most of my egress loopback open issue.
The local_datapath changes earlier in the patch set probably solve
my localnet issue. I need to look at that a little more carefully.

A few comments below, mostly about code organization. One real
technical issue.

After this, I will go back to the first patch and work my way through
patch by patch.


> Signed-off-by: Ben Pfaff 
> ---
>  ovn/controller/binding.c|   2 +-
>  ovn/controller/ovn-controller.c |   8 ++--
>  ovn/controller/patch.c  | 101 +++---
> --
>  ovn/controller/patch.h  |   3 +-
>  ovn/controller/physical.c   |  96 +-
> 
>  ovn/controller/physical.h   |   7 +--
>  ovn/controller/pinctrl.c|  36 --
>  ovn/controller/pinctrl.h|   4 +-
>  tests/ovn-controller.at |  57 ++-
>  9 files changed, 141 insertions(+), 173 deletions(-)
>
> diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
> index b53af98..2759e23 100644
> --- a/ovn/controller/binding.c
> +++ b/ovn/controller/binding.c
> @@ -391,7 +391,7 @@ consider_local_datapath(struct controller_ctx *ctx,
> "l3gateway-chassis");
>  if (!strcmp(chassis, chassis_rec->name) && ctx->ovnsb_idl_txn) {
>  add_local_datapath(ldatapaths, lports, binding_rec->datapath,
> -   false, local_datapaths);
> +   true, local_datapaths);
>  }
>  } else if (chassis_rec && binding_rec->chassis == chassis_rec) {
>  if (ctx->ovnsb_idl_txn) {
> diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-
> controller.c
> index ff48a94..7d372b4 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -515,12 +515,12 @@ main(int argc, char *argv[])
>  }
>
>  if (br_int && chassis) {
> -patch_run(, br_int, chassis_id, _datapaths);
> +patch_run(, br_int, chassis, _datapaths);
>
>  enum mf_field_id mff_ovn_geneve = ofctrl_run(br_int,
>
> _ct_zones);
>
> -pinctrl_run(, , br_int, chassis_id,
> _datapaths);
> +pinctrl_run(, , br_int, chassis, _datapaths);
>  update_ct_zones(_lports, _datapaths, _zones,
>  ct_zone_bitmap, _ct_zones);
>  if (ctx.ovs_idl_txn) {
> @@ -531,8 +531,8 @@ main(int argc, char *argv[])
>_table, _zones, _table);
>
>  physical_run(, mff_ovn_geneve,
> - br_int, chassis_id, _zones, _table,
> - _datapaths);
> + br_int, chassis, _zones, ,
> + _table, _datapaths);
>
>  ofctrl_put(_table, _ct_zones,
> get_nb_cfg(ctx.ovnsb_idl));
> diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c
> index ce82b89..9893c79 100644
> --- a/ovn/controller/patch.c
> +++ b/ovn/controller/patch.c
> @@ -137,7 +137,7 @@ add_bridge_mappings(struct controller_ctx *ctx,
>  const struct ovsrec_bridge *br_int,
>  struct shash *existing_ports,
>  struct hmap *local_datapaths,
> -const char *chassis_id)
> +const struct sbrec_chassis *chassis)
>  {
>  /* Get ovn-bridge-mappings. */
>  const char *mappings_cfg = "";
> @@ -207,7 +207,7 @@ add_bridge_mappings(struct controller_ctx *ctx,
>  patch_port_id = "ovn-localnet-port";
>  } else if (!strcmp(binding->type, "l2gateway")) {
>  if (!binding->chassis
> -|| strcmp(chassis_id, binding->chassis->name)) {
> +|| 

[ovs-dev] [PATCH] ovsdb-data: Add support for integer ranges in database commands

[Łukasz Rząsik]

Adding / removing a range of integers to a column accepting a set of
integers requires enumarating all of the integers. This patch simplifies
it by introducing 'range' concept to the database commands. Two integers
separated by a hyphen represent an inclusive range.

The patch adds positive and negative tests for the new syntax.
The patch was tested by 'make check'. Covarage was tested by
'make check-lcov'.

Signed-off-by: Lukasz Rzasik 
Suggested-by: 
Suggested-by: Ben Pfaff 
---
 lib/db-ctl-base.c   |  10 +--
 lib/db-ctl-base.man |   4 +-
 lib/ovsdb-data.c| 184
++--
 lib/ovsdb-data.h|  11 +++-
 lib/util.c  |  34 +-
 lib/util.h  |   2 +
 tests/ovs-vsctl.at  |   2 +-
 tests/ovsdb-data.at |  43 +++-
 tests/test-ovsdb.c  |  16 -
 9 files changed, 240 insertions(+), 66 deletions(-)

diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index 02eb328..e32e945 100644
--- a/lib/db-ctl-base.c
+++ b/lib/db-ctl-base.c
@@ -677,7 +677,7 @@ is_condition_satisfied(const struct ctl_table_class
*table,
   column->name);
 }

-die_if_error(ovsdb_atom_from_string(_key, >type.key,
+die_if_error(ovsdb_atom_from_string(_key, NULL,
>type.key,
 key_string, symtab));

 type.key = type.value;
@@ -823,7 +823,7 @@ cmd_get(struct ctl_context *ctx)
   column->name);
 }

-die_if_error(ovsdb_atom_from_string(,
+die_if_error(ovsdb_atom_from_string(, NULL,
 >type.key,
 key_string, ctx->symtab));

@@ -1118,13 +1118,13 @@ set_column(const struct ctl_table_class *table,
   column->name);
 }

-die_if_error(ovsdb_atom_from_string(, >type.key,
+die_if_error(ovsdb_atom_from_string(, NULL, >type.key,
 key_string, symtab));
-die_if_error(ovsdb_atom_from_string(, >type.value,
+die_if_error(ovsdb_atom_from_string(, NULL,
>type.value,
 value_string, symtab));

 ovsdb_datum_init_empty();
-ovsdb_datum_add_unsafe(, , , >type);
+ovsdb_datum_add_unsafe(, , , >type, NULL);

 ovsdb_atom_destroy(, column->type.key.type);
 ovsdb_atom_destroy(, column->type.value.type);
diff --git a/lib/db-ctl-base.man b/lib/db-ctl-base.man
index 7b30501..74d406c 100644
--- a/lib/db-ctl-base.man
+++ b/lib/db-ctl-base.man
@@ -29,7 +29,9 @@ single comma.  When multiple values are present,
duplicates are not
 allowed, and order is not important.  Conversely, some database
 columns can have an empty set of values, represented as \fB[]\fR, and
 square brackets may optionally enclose other non-empty sets or single
-values as well.
+values as well. For a column accepting a set of integers, database commands
+accept a range. A range is represented by two integers separated by
+\fB-\fR. A range is inclusive.
 .PP
 A few database columns are ``maps'' of key-value pairs, where the key
 and the value are each some fixed database type.  These are specified
diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c
index 0dda73a..f89f9ce 100644
--- a/lib/ovsdb-data.c
+++ b/lib/ovsdb-data.c
@@ -305,6 +305,25 @@ ovsdb_symbol_referenced(struct ovsdb_symbol *symbol,
 }
 }

+static union ovsdb_atom *
+alloc_default_atoms(enum ovsdb_atomic_type type, size_t n)
+{
+if (type != OVSDB_TYPE_VOID && n) {
+union ovsdb_atom *atoms;
+unsigned int i;
+
+atoms = xmalloc(n * sizeof *atoms);
+for (i = 0; i < n; i++) {
+ovsdb_atom_init_default([i], type);
+}
+return atoms;
+} else {
+/* Avoid wasting memory in the n == 0 case, because xmalloc(0) is
+ * treated as xmalloc(1). */
+return NULL;
+}
+}
+
 static struct ovsdb_error * OVS_WARN_UNUSED_RESULT
 ovsdb_atom_parse_uuid(struct uuid *uuid, const struct json *json,
   struct ovsdb_symbol_table *symtab,
@@ -468,6 +487,7 @@ ovsdb_atom_to_json(const union ovsdb_atom *atom, enum
ovsdb_atomic_type type)

 static char *
 ovsdb_atom_from_string__(union ovsdb_atom *atom,
+ union ovsdb_atom **range_end_atom,
  const struct ovsdb_base_type *base, const char *s,
  struct ovsdb_symbol_table *symtab)
 {
@@ -478,9 +498,20 @@ ovsdb_atom_from_string__(union ovsdb_atom *atom,
 OVS_NOT_REACHED();

 case OVSDB_TYPE_INTEGER: {
-long long int integer;
-if (!str_to_llong(s, 10, )) {
-return xasprintf("\"%s\" is not a valid integer", s);
+long long int integer, end;
+if (range_end_atom
+&& str_to_llong_range(s, 10, , )) {
+if 

Re: [ovs-dev] [PATCH] mpls: Fix MPLS restoration after patch port and group bucket.

[Jarno Rajahalme]

Thanks for the review, pushed to master, branch-2.6, and branch-2.5.

  Jarno

> On Dec 3, 2016, at 5:54 AM, Takashi YAMAMOTO  wrote:
> 
> 
> 
> On Sat, Dec 3, 2016 at 12:38 PM, Jarno Rajahalme  > wrote:
> This patch fixes problems with MPLS handling related to patch ports
> and group buckets.
> 
> If a group bucket or a peer bridge across a patch port pushes MPLS
> headers to a non-MPLS packet and outputs, the flow translation after
> returning from the group bucket or patch port would undo the packet
> transformations so that the processing could continue with the packet
> as it was before entering the patch port.  There were two problems
> with this:
> 
> 1. As part of the first MPLS push on a non-MPLS packet, the flow
> translation would first clear the L3/4 headers of the 'flow' to mark
> those fields invalid.  Later, when committing 'flow' changes to
> datapath actions before output, the necessary datapath MPLS actions
> are created and the corresponding changes updated to the 'base flow'.
> This was done using the same flow_push_mpls() function that clears
> the L2/3 headers, so also the 'base flow' L2/3 headers were cleared.
> 
> Then, when translation returns from a patch port or group bucket, the
> original 'flow' is restored, now showing no sign of the MPLS labels.
> Since the 'base flow' now has the MPLS labels, following translations
> know to issue MPLS POP actions before any output actions.  However, as
> part of checking for changes to IP headers we test that the IP
> protocol type was not changed.  But now the 'base flow's 'nw_proto'
> field is zero and an assert fail crashes OVS.
> 
> This is solved by not clearing the L3/4 fields of the 'base
> flow'. This allows the processing after the patch port to continue
> with L3/4 fields as if no MPLS was done, after first issuing the
> necessary MPLS POP actions.
> 
> 2. IP header updates were done before the MPLS POP actions were
> issued. This caused incorrect packet output after, e.g., group action
> or patch port.  For example, with actions:
> 
> group 1234: all bucket=push_mpls,output:LOCAL
> 
> ip actions=group:1234,dec_ttl,output:LOCAL,output:LOCAL
> 
> the dec_ttl would only be executed before the last output to LOCAL,
> since at the time of committing IP changes after the group action the
> packet was still an MPLS packet.
> 
> This is solved by checking the dl_type of both 'flow' and 'base flow'
> and issuing MPLS actions if they can transform the packet from an MPLS
> packet to a non-MPLS packet.  For an IP packet the change in ttl can
> then be correctly committed before the last two output actions.
> 
> Two test cases are added to prevent future regressions.
> 
> Reported-by: Thomas Morin  >
> Suggested-by: Takashi YAMAMOTO >
> Fixes: 8bfd0fdac ("Enhance userspace support for MPLS, for up to 3 labels.")
> Fixes: 1b035ef20 ("mpls: Allow l3 and l4 actions to prior to a push_mpls 
> action")
> Signed-off-by: Jarno Rajahalme >
> 
> Acked-by: YAMAMOTO Takashi >
> 
> ---
>  lib/flow.c   | 14 +-
>  lib/flow.h   |  2 +-
>  lib/odp-util.c   | 16 +--
>  ofproto/ofproto-dpif-xlate.c |  3 ++-
>  tests/mpls-xlate.at   | 64 
> 
>  5 files changed, 89 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/flow.c b/lib/flow.c
> index f4ac8b3..b6d0d15 100644
> --- a/lib/flow.c
> +++ b/lib/flow.c
> @@ -2096,7 +2096,7 @@ flow_count_common_mpls_labels(const struct flow *a, int 
> an,
>   */
>  void
>  flow_push_mpls(struct flow *flow, int n, ovs_be16 mpls_eth_type,
> -   struct flow_wildcards *wc)
> +   struct flow_wildcards *wc, bool clear_flow_L3)
>  {
>  ovs_assert(eth_type_mpls(mpls_eth_type));
>  ovs_assert(n < FLOW_MAX_MPLS_LABELS);
> @@ -2134,11 +2134,13 @@ flow_push_mpls(struct flow *flow, int n, ovs_be16 
> mpls_eth_type,
> 
>  flow->mpls_lse[0] = set_mpls_lse_values(ttl, tc, 1, htonl(label));
> 
> -/* Clear all L3 and L4 fields and dp_hash. */
> -BUILD_ASSERT(FLOW_WC_SEQ == 36);
> -memset((char *) flow + FLOW_SEGMENT_2_ENDS_AT, 0,
> -   sizeof(struct flow) - FLOW_SEGMENT_2_ENDS_AT);
> -flow->dp_hash = 0;
> +if (clear_flow_L3) {
> +/* Clear all L3 and L4 fields and dp_hash. */
> +BUILD_ASSERT(FLOW_WC_SEQ == 36);
> +memset((char *) flow + FLOW_SEGMENT_2_ENDS_AT, 0,
> +   sizeof(struct flow) - FLOW_SEGMENT_2_ENDS_AT);
> +flow->dp_hash = 0;
> +}
>  }
>  flow->dl_type = mpls_eth_type;
>  }
> diff --git a/lib/flow.h b/lib/flow.h
> index 35724b1..62315bc 100644
> --- a/lib/flow.h
> +++ b/lib/flow.h
> @@ -95,7 +95,7 

Re: [ovs-dev] [PATCH] datapath-windows: null comparison for icmp and tcp header

[Alin Serdean]

The problem is here:
https://github.com/openvswitch/ovs/blob/master/datapath-windows/ovsext/Conntrack-tcp.c#L460

Looking back, it makes more sense to add them to 
OvsConntrackValidateIcmpPacket/OvsConntrackValidateTcpPacket.

Should I revert the original patch and send one that chages the functions 
mentioned above?

Thanks,
Alin.

> -Original Message-
> From: Sairam Venugopal [mailto:vsai...@vmware.com]
> Sent: Wednesday, November 30, 2016 8:15 PM
> To: Yin Lin ; Alin Serdean
> 
> Cc: d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH] datapath-windows: null comparison for icmp
> and tcp header
> 
> OvsConntrackValidateTcpPacket currently checks for NULL. I will update
> OvsConntrackValidateIcmpPacket to check for NULL as well.
> 
> 
> I acked this change to keep the checks consistent across different protocol.
> 
> Thanks,
> Sairam
> 
> 
> On 11/29/16, 2:30 PM, "Yin Lin"  wrote:
> 
> >Can we decide if tcp and icmp is null in OvsConntrackValidateTcpPacket?
> >It makes the function more complete and safer by itself.
> >
> >On Mon, Nov 28, 2016 at 6:11 AM, Alin Serdean <
> >aserd...@cloudbasesolutions.com> wrote:
> >
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] datapath-windows: null comparison for icmp and tcp header

[Alin Serdean]

Thanks for the tips Joe!

It causes a kernel crash under low resources.

Thanks,
Alin.

> -Original Message-
> From: Joe Stringer [mailto:j...@ovn.org]
> Sent: Tuesday, November 29, 2016 9:35 PM
> To: Alin Serdean 
> Cc: d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH] datapath-windows: null comparison for icmp
> and tcp header
> 
> On 28 November 2016 at 06:11, Alin Serdean
>  wrote:
> > This patch checks if the TCP or ICMP header exists before trying to
> > use them.
> >
> > The issue was found using the driver under low resources.
> >
> > Signed-off-by: Alin Gabriel Serdean 
> 
> (This is not a review, just a friendly reminder about patch ettiquette)
> 
> Does this cause a kernel crash?
> 
> Please use the imperative in git commit titles - It is easier to understand 
> the
> evolution of the code in git logs when commits have clear titles like "Fix 
> null
> dereference in conntrack". It is also helpful to include the potential impact 
> of
> the change in the commit message (ie, kernel crash? memory leak? etc.). The
> "why" is more important in the commit message than the "what" - the
> "what" is already stated in the code.
> 
> Linus wrote some good patch commit message guidelines here:
> https://github.com/torvalds/subsurface/blob/a48494d2fbed58c751e9b7e8fb
> ff88582f9b2d02/README#L91
> 
> Thanks,
> Joe
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] OVS-Hyper-V-Discovery-Agent Design Document

[Alin Serdean]

Sorry for the late reply. We had a few days of bank holiday last week.

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Tuesday, November 29, 2016 2:28 AM
> To: Nithin Raju 
> Cc: Yin Lin ; d...@openvswitch.org; Alin Serdean
> ; Justin Pettit 
> Subject: Re: [ovs-dev] OVS-Hyper-V-Discovery-Agent Design Document
> 
> OK, I understand now.
> 
> Having ovs-vswitchd itself add ports would be unprecedented.  Normally, we
> depend on some part of the system integration to do that: libvirt does it in
> modern KVM environments (as you say), a hook script does it on XenServer,
> and so on.
[Alin Serdean] I think we need to look at a bigger picture on what we are 
trying to achieve.
AFAIK, in the case of libvirt/Xen, someone asks them to create an adapter and 
after it is created the result is added to OVSDB.
On Windows, someone asks vmms 
(https://technet.microsoft.com/en-us/library/dd582295(v=ws.10).aspx) to create 
a port on a switch, rename the port which was created, and after add it to 
OVSDB afterwards.
In the case of Windows+OpenStack (with or without OVN) things are already 
handled since the code for port renaming is already there. If someone wants to 
integrate it in his solution, he could use/reuse/implement the code in the 
powershell script which is in our repository (1).
This daemon is targeted for unexperienced users which do not want/do not know 
how to do the extra step of renaming the port name. This will give us better 
user experience.
The reasons I would like to add the functionality in vswitchd are simplicity 
and speed (we see the port creation in the windows datapath, after it was 
created by vmms, and during an upcall we could add the port).
> 
> My preference would be to keep these details of the system integration
> separate from ovs-vswitchd, since it matches the implementation
> elsewhere.  I'd expect this to be a pretty simple daemon, which probably
> wouldn't use much CPU or memory.
> 
[Alin Serdean] My main problem with the current implementation is that WMI 
calls are slow. Using vswitchd or another monitor that reuses the netlink 
implementation would be better IMO.

(1) 
https://github.com/openvswitch/ovs/blob/master/datapath-windows/misc/OVS.psm1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 18/19] dpif-netdev: Centralized threads and queues handling code.

[Daniele Di Proietto]

Thanks again for looking at this, please let me know if you have other comments.


On 05/12/2016 05:39, "Ilya Maximets"  wrote:

>Thanks for v2.
>
>Global comment:
>Could you please add a space between '{C,H}MAP_FOR_EACH[_WITH_HASH]' and
>the expressions that follow them. (Not only in that patch)

Good point, I fixed that in every commit of the series.

>
>Few more comments inline.
>
>Best regards, Ilya Maximets.
>
>On 03.12.2016 05:14, Daniele Di Proietto wrote:
>> Currently we have three different code paths that deal with pmd threads
>> and queues, in response to different input
>> 
>> 1. When a port is added
>> 2. When a port is deleted
>> 3. When the cpumask changes or a port must be reconfigured.
>> 
>> 1. and 2. are carefully written to minimize disruption to the running
>> datapath, while 3. brings down all the threads reconfigure all the ports
>> and restarts everything.
>> 
>> This commit removes the three separate code paths by introducing the
>> reconfigure_datapath() function, that takes care of adapting the pmd
>> threads and queues to the current datapath configuration, no matter how
>> we got there.
>> 
>> This aims at simplifying mantenaince and introduces a long overdue
>
>s/mantenaince/maintenance

changed, thanks

>
>> improvement: port reconfiguration (can happen quite frequently for
>> dpdkvhost ports) is now done without shutting down the whole datapath,
>> but just by temporarily removing the port that needs to be reconfigured
>> (while the rest of the datapath is running).
>> 
>> We now also recompute the rxq scheduling from scratch every time a port
>> is added of deleted.  This means that the queues will be more balanced,
>> especially when dealing with explicit rxq-affinity from the user
>> (without shutting down the threads and restarting them), but it also
>> means that adding or deleting a port might cause existing queues to be
>> moved between pmd threads.  This negative effect can be avoided by
>> taking into account the existing distribution when computing the new
>> scheduling, but I considered code clarity and fast reconfiguration more
>> important than optimizing port addition or removal (a port is added and
>> removed only once, but can be reconfigured many times)
>> 
>> Lastly, this commit moves the pmd threads state away from ovs-numa.  Now
>> the pmd threads state is kept only in dpif-netdev.
>> 
>> Signed-off-by: Daniele Di Proietto 
>> Co-authored-by: Ilya Maximets 
>> Signed-off-by: Ilya Maximets 
>> ---
>>  lib/dpif-netdev.c | 910 
>> +++---
>>  tests/pmd.at  |   3 +-
>>  2 files changed, 464 insertions(+), 449 deletions(-)
>> 
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index 37479b8..3509493 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -289,6 +289,7 @@ struct dp_netdev_rxq {
>>pinned. RXQ_CORE_UNPINNED if the
>>queue doesn't need to be pinned 
>> to a
>>particular core. */
>> +struct dp_netdev_pmd_thread *pmd;  /* pmd thread that will poll this 
>> queue. */
>>  };
>>  
>>  /* A port in a netdev-based datapath. */
>> @@ -304,6 +305,7 @@ struct dp_netdev_port {
>>  struct ovs_mutex txq_used_mutex;
>>  char *type; /* Port type as requested by user. */
>>  char *rxq_affinity_list;/* Requested affinity of rx queues. */
>> +bool need_reconfigure;  /* True if we should reconfigure netdev. */
>>  };
>>  
>>  /* Contained by struct dp_netdev_flow's 'stats' member.  */
>> @@ -506,7 +508,7 @@ struct dp_netdev_pmd_thread {
>>  
>>  /* Queue id used by this pmd thread to send packets on all netdevs if
>>   * XPS disabled for this netdev. All static_tx_qid's are unique and less
>> - * than 'ovs_numa_get_n_cores() + 1'. */
>> + * than 'cmap_count(dp->poll_threads)'. */
>>  const int static_tx_qid;
>>  
>>  struct ovs_mutex port_mutex;/* Mutex for 'poll_list' and 
>> 'tx_ports'. */
>> @@ -535,6 +537,9 @@ struct dp_netdev_pmd_thread {
>>   * reporting to the user */
>>  unsigned long long stats_zero[DP_N_STATS];
>>  uint64_t cycles_zero[PMD_N_CYCLES];
>> +
>> +/* Set to true if the pmd thread needs to be reloaded. */
>> +bool need_reload;
>>  };
>>  
>>  /* Interface to netdev-based datapath. */
>> @@ -579,29 +584,26 @@ static void dp_netdev_destroy_pmd(struct 
>> dp_netdev_pmd_thread *pmd);
>>  static void dp_netdev_set_nonpmd(struct dp_netdev *dp)
>>  OVS_REQUIRES(dp->port_mutex);
>>  
>> +static void *pmd_thread_main(void *);
>>  static struct dp_netdev_pmd_thread *dp_netdev_get_pmd(struct dp_netdev *dp,
>>unsigned core_id);
>>  static struct dp_netdev_pmd_thread *
>>  dp_netdev_pmd_get_next(struct dp_netdev *dp, 

Re: [ovs-dev] [RFC PATCH] Pipeline packet processing in OVS using FVL flow director.

[Ben Pfaff]

On Thu, Dec 01, 2016 at 06:23:55PM +, Sugesh Chandran wrote:
> The RFC patch that uses the metadata information from the NIC to avoid/reduce 
> the
> packet processing overhead in OVS.
>   
> It uses the flow director feature in XL710 NICs to identify and report the
>   
> VxLAN tunneled packets. OVS-DPDK uses the reported information to bypass the
> VxLAN tunnel processing in software.  
>

This is interesting and I'd like to apply it locally to look at it more
closely.  What commit is it based on?  It doesn't seem to apply cleanly
to current master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] tunnel: set udp dst-port in tunnel metadata

[Jarno Rajahalme]

Acked-by: Jarno Rajahalme 

Is it OK to leave the tunnel.tp_src unset, or left set to the value from the 
incoming tunnel?

  Jarno

> On Dec 4, 2016, at 11:52 PM, Pravin B Shelar  wrote:
> 
> VxLan device expect valid tp-dst in tunnel metadata.
> Following patch sets consistent tp-dst with respect to
> the egress tunnel port.
> 
> Reported-by: Gerhard Stenzel 
> Tested-by: Gerhard Stenzel 
> Signed-off-by: Pravin B Shelar 
> ---
> ofproto/tunnel.c  | 1 +
> tests/ofproto-dpif.at | 2 +-
> tests/tunnel.at   | 8 
> 3 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
> index 97de59e..ce727f4 100644
> --- a/ofproto/tunnel.c
> +++ b/ofproto/tunnel.c
> @@ -427,6 +427,7 @@ tnl_port_send(const struct ofport_dpif *ofport, struct 
> flow *flow,
> flow->tunnel.ipv6_dst = in6addr_any;
> }
> }
> +flow->tunnel.tp_dst = cfg->dst_port;
> if (!cfg->out_key_flow) {
> flow->tunnel.tun_id = cfg->out_key;
> }
> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> index ec7bd60..6c7ac36 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -6378,7 +6378,7 @@ AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
> 'in_port(3),eth(src=50:54:00:00:00:
> dnl Make sure flow sample action in datapath is behind set tunnel
> dnl action at egress point of tunnel port.
> AT_CHECK([tail -1 stdout], [0], [dnl
> -Datapath actions: 
> set(tunnel(tun_id=0x5,src=2.2.2.2,dst=1.1.1.1,tos=0x1,ttl=64,flags(df|key))),sample(sample=100.0%,actions(userspace(pid=0,flow_sample(probability=65535,collector_set_id=1,obs_domain_id=0,obs_point_id=0,output_port=1),tunnel_out_port=1))),1,set(tunnel(tun_id=0x6,src=2.2.2.3,dst=1.1.1.2,tos=0x1,ttl=64,flags(df|key))),sample(sample=100.0%,actions(userspace(pid=0,flow_sample(probability=65535,collector_set_id=1,obs_domain_id=0,obs_point_id=0,output_port=7471),tunnel_out_port=7471))),7471
> +Datapath actions: 
> set(tunnel(tun_id=0x5,src=2.2.2.2,dst=1.1.1.1,tos=0x1,ttl=64,flags(df|key))),sample(sample=100.0%,actions(userspace(pid=0,flow_sample(probability=65535,collector_set_id=1,obs_domain_id=0,obs_point_id=0,output_port=1),tunnel_out_port=1))),1,set(tunnel(tun_id=0x6,src=2.2.2.3,dst=1.1.1.2,tos=0x1,ttl=64,tp_dst=7471,flags(df|key))),sample(sample=100.0%,actions(userspace(pid=0,flow_sample(probability=65535,collector_set_id=1,obs_domain_id=0,obs_point_id=0,output_port=7471),tunnel_out_port=7471))),7471
> ])
> 
> dnl Remove the flow which contains sample action.
> diff --git a/tests/tunnel.at b/tests/tunnel.at
> index 647a466..1ba209d 100644
> --- a/tests/tunnel.at
> +++ b/tests/tunnel.at
> @@ -484,7 +484,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
> dnl Option generation
> AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
> 'in_port(2),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no),icmp(type=8,code=0)'],
>  [0], [stdout])
> AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: 
> set(tunnel(dst=1.1.1.1,ttl=64,geneve({class=0x,type=0,len=4,0xa}{class=0x,type=0x1,len=8,0x1234567890abcdef}),flags(df))),6081
> +  [Datapath actions: 
> set(tunnel(dst=1.1.1.1,ttl=64,tp_dst=6081,geneve({class=0x,type=0,len=4,0xa}{class=0x,type=0x1,len=8,0x1234567890abcdef}),flags(df))),6081
> ])
> 
> dnl Option match
> @@ -573,7 +573,7 @@ Datapath actions: 2
> AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
> 'recirc_id(0),tunnel(tun_id=0x0,src=1.1.1.1,dst=1.1.1.2,ttl=64,geneve({class=0x,type=1,len=0}),flags(df|key)),in_port(6081),skb_mark(0),eth_type(0x0800),ipv4(frag=no)'],
>  [0], [stdout])
> AT_CHECK([tail -2 stdout], [0],
>   [Megaflow: 
> recirc_id=0,ip,tun_id=0,tun_src=1.1.1.1,tun_dst=1.1.1.2,tun_tos=0,tun_flags=+df-csum+key,tun_metadata1,tun_metadata2=NP,in_port=1,nw_ecn=0,nw_frag=no
> -Datapath actions: 
> set(tunnel(tun_id=0x0,dst=1.1.1.1,ttl=64,geneve({class=0x,type=0x1,len=0}),flags(df|key))),6081
> +Datapath actions: 
> set(tunnel(tun_id=0x0,dst=1.1.1.1,ttl=64,tp_dst=6081,geneve({class=0x,type=0x1,len=0}),flags(df|key))),6081
> ])
> 
> OVS_VSWITCHD_STOP
> @@ -594,12 +594,12 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
> 
> AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
> 'tunnel(tun_id=0,src=1.1.1.1,dst=1.1.1.2,ttl=64),in_port(4789)'], [0], 
> [stdout])
> AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: 
> set(tunnel(tun_id=0x0,ipv6_dst=2001:cafe::1,ttl=64,flags(df|key))),4789
> +  [Datapath actions: 
> set(tunnel(tun_id=0x0,ipv6_dst=2001:cafe::1,ttl=64,tp_dst=4789,flags(df|key))),4789
> ])
> 
> AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
> 'tunnel(tun_id=0x0,ipv6_src=2001:cafe::1,ipv6_dst=2001:cafe::2,ttl=64),in_port(4789)'],
>  [0], [stdout])
> AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: 
> set(tunnel(tun_id=0x0,dst=1.1.1.1,ttl=64,flags(df|key))),4789
> 

Re: [ovs-dev] [PATCH net-next] net: remove abuse of VLAN DEI/CFI bit

[Ben Pfaff]

On Mon, Dec 05, 2016 at 06:24:36PM +0100, Michał Mirosław wrote:
> On Sat, Dec 03, 2016 at 03:27:30PM -0800, Ben Pfaff wrote:
> > On Sat, Dec 03, 2016 at 10:22:28AM +0100, Michał Mirosław wrote:
> > > This All-in-one patch removes abuse of VLAN CFI bit, so it can be passed
> > > intact through linux networking stack.
> > This appears to change the established Open vSwitch userspace API.  You
> > can see that simply from the way that it changes the documentation for
> > the userspace API.  If I'm right about that, then this change will break
> > all userspace programs that use the Open vSwitch kernel module,
> > including Open vSwitch itself.
> 
> If I understood the code correctly, it does change expected meaning for
> the (unlikely?) case of header truncated just before the VLAN TCI - it will
> be impossible to differentiate this case from the VLAN TCI == 0.
> 
> I guess this is a problem with OVS API, because it doesn't directly show
> the "missing" state of elements, but relies on an "invalid" value.

That particular corner case should not be a huge problem in any case.

The real problem is that this appears to break the common case use of
VLANs in Open vSwitch.  After this patch, parse_vlan() in
net/openvswitch/flow.c copies the tpid and tci from sk_buff (either the
accelerated version of them or the version in the skb data) into
sw_flow_key members.  OK, that's fine on it's own.  However, I don't see
any corresponding change to the code in flow_netlink.c to compensate for
the fact that, until now, the VLAN CFI bit (formerly VLAN_TAG_PRESENT)
was always required to be set to 1 in flow matches inside Netlink
messages sent from userspace, and the kernel always set it to 1 in
corresponding messages sent to userspace.

In other words, if I'm reading this change correctly:

* With a kernel before this change, userspace always had to set
  VLAN_TAG_PRESENT to 1 to match on a VLAN, or the kernel would
  reject the flow match.

* With a kernel after this change, userspace must not set
  VLAN_TAG_PRESENT to 1, otherwise the kernel will accept the flow
  match but nothing will ever match because packets do not actually
  have the CFI bit set.

Take a look at this code that the patch deletes from
validate_vlan_from_nlattrs(), for example, and see how it insisted that
VLAN_TAG_PRESENT was set:

if (!(tci & htons(VLAN_TAG_PRESENT))) {
if (tci) {
OVS_NLERR(log, "%s TCI does not have VLAN_TAG_PRESENT 
bit set.",
  (inner) ? "C-VLAN" : "VLAN");
return -EINVAL;
} else if (nla_len(a[OVS_KEY_ATTR_ENCAP])) {
/* Corner case for truncated VLAN header. */
OVS_NLERR(log, "Truncated %s header has non-zero encap 
attribute.",
  (inner) ? "C-VLAN" : "VLAN");
return -EINVAL;
}
}

Please let me know if I'm overlooking something.

Thanks,

Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 0/4] datapath-windows: Enable support for tracking FTP connections

[Alin Serdean]

Thanks a lot for the series.

Overall it looks like a good start for the support!

Thanks,
Alin.

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Sairam Venugopal
> Sent: Thursday, December 1, 2016 11:19 PM
> To: d...@openvswitch.org
> Subject: [ovs-dev] [PATCH 0/4] datapath-windows: Enable support for
> tracking FTP connections
> 
> Add support for maintaining and tracking related connections. This patch
> introduces the concept of related-connections table. There is an FTP parser
> in place to parse FTP PASV and PORT commands. Support for traking
> extended FTP commands will be added in subsequently.
> 
> 
> Sairam Venugopal (4):
>   datapath-windows: Conntrack - Fix OvsGetTcpPayloadLength()
>   datapath-windows: Cleanup Conntrack definitions and introduce related
> entries
>   datapath-windows: Conntrack - Introduce support for tracking related
> connections
>   datapath-windows: Conntrack - Enable FTP support
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 4/4] datapath-windows: Conntrack - Enable FTP support

[Alin Serdean]

Thanks for the patch.

I think OvsInitCtRelated/ OvsCleanupCtRelated would make more sense to be 
inside OvsInitConntrack/OvsCleanupConntrack since the functionality are tied 
together.
One small nit.

Thanks,
Alin.
> +ctAttr = NlAttrFindNested(a, OVS_CT_ATTR_HELPER);
> +if (ctAttr) {
> +helper = NlAttrGet(ctAttr);
> +if (!memchr(helper, '\0', 16)) {
[Alin Serdean] We must be careful here, because the size may differ(i.e. a 
message could be forged). I think we should add 
https://github.com/openvswitch/ovs/blob/master/lib/netlink.c#L649 to the 
windows datapath and use it.
> +OVS_LOG_ERROR("Invalid CT_ATTR_HELPER:%s", helper);
> +return NDIS_STATUS_INVALID_PARAMETER;
> +}
> +if (strcmp("ftp", helper) != 0) {
> +/* Only support FTP */
> +return NDIS_STATUS_NOT_SUPPORTED;
> +}
> +}
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 3/4] datapath-windows: Conntrack - Introduce support for tracking related connections

[Alin Serdean]

Thanks for the patch!

It looks good as an initial point that we can add more complexity in the future 
:).

One personal preference would be to use the string Rtl* functions:
https://msdn.microsoft.com/en-us/library/windows/hardware/ff563642(v=vs.85).aspx
https://msdn.microsoft.com/en-us/library/windows/hardware/ff563644(v=vs.85).aspx

Some white space issues:
../ovs-dev-3-4-datapath-windows-Conntrack---Introduce-support-for-tracking-related-connections.patch:591:
 new blank line at EOF.
+
warning: 1 line adds whitespace errors.

Other small comments inlined.

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Sairam Venugopal
> Sent: Thursday, December 1, 2016 11:19 PM
> To: d...@openvswitch.org
> Subject: [ovs-dev] [PATCH 3/4] datapath-windows: Conntrack - Introduce
> support for tracking related connections
> 
> Introduce a new table to track related connections. This table will be used to
> track FTP data connections based on the control connection. There is a new
> Conntrack-ftp.c to parse incoming FTP messages to determine the related
> data ports. It creates a new entry in the related connections tracker table. 
> If
> there is a matching FTP data connection, then the state for that connection is
> marked as RELATED.
> 
> Signed-off-by: Sairam Venugopal 
> ---
>cut<--
> +/* End of FTP response is either ) or \r\n */
[Alin Serdean] Check for \n as well.
> +if (*buf == ')' || *buf == '\r') {
>cut<--
[Alin Serdean] Maybe add define for 227 (Passive mode) and the comment above in 
the define.
> +if ((len >= 4) && (OvsStrncmp("227", ftpMsg, 3) == 0)) {
> +ftpType = FTP_TYPE_PASV;
> +/* There are various formats for PASV command. We try to support
> + * some of them. This has been addressed by RFC 2428 - EPSV.
> + * Eg:
> + *227 Entering Passive Mode (h1,h2,h3,h4,p1,p2).[Alin 
> Serdean]   
> + *227 Entering Passive Mode (h1,h2,h3,h4,p1,p2
> + *227 Entering Passive Mode. h1,h2,h3,h4,p1,p2
> + *227 =h1,h2,h3,h4,p1,p2
> + */
>cut<--
> +VOID
> +OvsCleanupCtRelated (VOID)
[Alin Serdean] Extra " " character. 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovn-controller(8): ovn-remote must be an IP adddress.

[Ben Pfaff]

On Mon, Dec 05, 2016 at 10:10:43AM -0800, Ben Pfaff wrote:
> On Mon, Dec 05, 2016 at 12:05:33PM -0500, Russell Bryant wrote:
> > Document that the value of the ovn-remote configuration
> > option must be an IP address.  This came up on IRC where
> > someone trying OVN put a hostname here and observed that
> > it did not work.
> > 
> > Signed-off-by: Russell Bryant 
> > ---
> >  ovn/controller/ovn-controller.8.xml | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/ovn/controller/ovn-controller.8.xml 
> > b/ovn/controller/ovn-controller.8.xml
> > index 5f51cb1..825a538 100644
> > --- a/ovn/controller/ovn-controller.8.xml
> > +++ b/ovn/controller/ovn-controller.8.xml
> > @@ -93,7 +93,7 @@
> >
> >  
> >The OVN database that this system should connect to for its
> > -  configuration.
> > +  configuration.  The value must be an IP address.
> >  
> 
> Not an IP address but an OVSDB remote in the form tcp:ip[:port] or
> ssl:ip[:port].

Actually the documentation just below that paragraph is wrong too.

Maybe something like this:

diff --git a/ovn/controller/ovn-controller.8.xml 
b/ovn/controller/ovn-controller.8.xml
index 5f51cb1..9f4dad1 100644
--- a/ovn/controller/ovn-controller.8.xml
+++ b/ovn/controller/ovn-controller.8.xml
@@ -93,13 +93,8 @@
   
 
   The OVN database that this system should connect to for its
-  configuration.
-
-
-
-  Currently, ovn-controller does not support changing this
-  setting mid-run.  If the value needs to change, the daemon must be
-  restarted.  (This behavior should be improved.)
+  configuration, in one of the same forms documented above for the
+  ovs-database.
 
   
 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovn-controller(8): ovn-remote must be an IP adddress.

[Ben Pfaff]

On Mon, Dec 05, 2016 at 12:05:33PM -0500, Russell Bryant wrote:
> Document that the value of the ovn-remote configuration
> option must be an IP address.  This came up on IRC where
> someone trying OVN put a hostname here and observed that
> it did not work.
> 
> Signed-off-by: Russell Bryant 
> ---
>  ovn/controller/ovn-controller.8.xml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ovn/controller/ovn-controller.8.xml 
> b/ovn/controller/ovn-controller.8.xml
> index 5f51cb1..825a538 100644
> --- a/ovn/controller/ovn-controller.8.xml
> +++ b/ovn/controller/ovn-controller.8.xml
> @@ -93,7 +93,7 @@
>
>  
>The OVN database that this system should connect to for its
> -  configuration.
> +  configuration.  The value must be an IP address.
>  

Not an IP address but an OVSDB remote in the form tcp:ip[:port] or
ssl:ip[:port].
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH net-next] net: remove abuse of VLAN DEI/CFI bit

[Michał Mirosław]

On Sat, Dec 03, 2016 at 03:27:30PM -0800, Ben Pfaff wrote:
> On Sat, Dec 03, 2016 at 10:22:28AM +0100, Michał Mirosław wrote:
> > This All-in-one patch removes abuse of VLAN CFI bit, so it can be passed
> > intact through linux networking stack.
> > 
> > Signed-off-by: Michał Mirosław 
> > ---
> > 
> > Dear NetDevs
> > 
> > I guess this needs to be split to the prep..convert[]..finish sequence,
> > but if you like it as is, then it's ready.
> > 
> > The biggest question is if the modified interface and vlan_present
> > is the way to go. This can be changed to use vlan_proto != 0 instead
> > of an extra flag bit.
> > 
> > As I can't test most of the driver changes, please look at them carefully.
> > OVS and bridge eyes are especially welcome.
> 
> This appears to change the established Open vSwitch userspace API.  You
> can see that simply from the way that it changes the documentation for
> the userspace API.  If I'm right about that, then this change will break
> all userspace programs that use the Open vSwitch kernel module,
> including Open vSwitch itself.

If I understood the code correctly, it does change expected meaning for
the (unlikely?) case of header truncated just before the VLAN TCI - it will
be impossible to differentiate this case from the VLAN TCI == 0.

I guess this is a problem with OVS API, because it doesn't directly show
the "missing" state of elements, but relies on an "invalid" value.

I can probably change the code to mask this change in the OVS code (by keeping
the CFI/DEI bit unusable). This somehow feels wrong, though.

Best Regards,
Michał Mirosław
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] ovn-controller(8): ovn-remote must be an IP adddress.

[Russell Bryant]

Document that the value of the ovn-remote configuration
option must be an IP address.  This came up on IRC where
someone trying OVN put a hostname here and observed that
it did not work.

Signed-off-by: Russell Bryant 
---
 ovn/controller/ovn-controller.8.xml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ovn/controller/ovn-controller.8.xml 
b/ovn/controller/ovn-controller.8.xml
index 5f51cb1..825a538 100644
--- a/ovn/controller/ovn-controller.8.xml
+++ b/ovn/controller/ovn-controller.8.xml
@@ -93,7 +93,7 @@
   
 
   The OVN database that this system should connect to for its
-  configuration.
+  configuration.  The value must be an IP address.
 
 
 
-- 
2.9.3

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] assertion failing in commit_set_ipv4_action(), flow->nw_proto != base_flow->nw_proto

[Thomas Morin]

2016-12-03, Jarno Rajahalme:
I sent a patch to fix this on OVS master. After it is reviewed I’ll 
backport it to branch-2.5 (it only needs trivial fixes to apply, so 
you may want to try that as well). 


Thank you very much.
I'll try it this week.

Best,

-Thomas



On Dec 1, 2016, at 5:57 PM, Jarno Rajahalme > wrote:



On Nov 30, 2016, at 8:50 PM, Ben Pfaff > wrote:


On Wed, Nov 30, 2016 at 06:58:57PM -0800, Jarno Rajahalme wrote:


On Nov 30, 2016, at 8:41 AM, Ben Pfaff > wrote:


On Wed, Nov 30, 2016 at 12:05:46PM +0100, Thomas Morin wrote:

Hi Ben,

2016-11-30, Ben Pfaff:

Do you have any idea what in your OpenFlow pipeline might do that,
i.e. is there anything especially tricky in the OpenFlow flows?

Are you willing to show us your OpenFlow flow table?


The setup involves three OVS bridges connected with patch-ports: 
br-int --
br-tun -- br-mpls, with the traffic that triggers the assert 
being processed

by br-int with a NORMAL action (ie. MAC learning).

The flows in this setup aren't particularly tricky, I think, 
although I'm

not sure what qualifies as tricky or non-tricky :)

Anyway, since yesterday I managed to identify the event that 
trigger the
assert, by adding more logging before the assert and displaying 
the actions

taken:

2016-11-29T14:44:40.126Z|1|odp_util(revalidator45)|WARN|commit_set_ipv4_action
assert would fail
2016-11-29T14:44:40.126Z|2|odp_util(revalidator45)|WARN| 
 base_flow: 
ip,in_port=5,dl_vlan=3,dl_vlan_pcp=0,dl_src=fa:16:3e:33:f7:fe,dl_dst=00:00:5e:00:43:64,nw_src=0.0.0.0,nw_dst=0.0.0.0,nw_proto=0,nw_tos=0,nw_ecn=0,nw_ttl=0
2016-11-29T14:44:40.126Z|3|odp_util(revalidator45)|WARN| 
 flow: 
tcp,in_port=5,dl_vlan=3,dl_vlan_pcp=0,dl_src=fa:16:3e:33:f7:fe,dl_dst=00:00:5e:00:43:64,nw_src=10.0.1.22,nw_dst=10.0.0.3,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=53295,tp_dst=8080,tcp_flags=psh|ack
2016-11-29T14:44:40.126Z|4|odp_util(revalidator45)|WARN| 
 masks: 
recirc_id=0x,reg0=0x,in_port=4294967295,dl_vlan=4095,dl_vlan_pcp=7,dl_src=ff:ff:ff:ff:ff:ff,dl_dst=ff:ff:ff:ff:ff:ff,dl_type=0x
2016-11-29T14:44:40.126Z|5|odp_util(revalidator45)|WARN| 
 actions: 
set(ipv4(src=10.0.1.22,dst=10.0.0.3,ttl=63)),set(eth(src=b8:2a:72:de:1b:e3,dst=00:17:cb:79:2c:01)),push_mpls(label=410384,tc=0,ttl=63,bos=1,eth_type=0x8847),9,set(eth(src=fa:16:3e:33:f7:fe,dst=00:00:5e:00:43:64)),pop_mpls(eth_type=0x800),push_vlan(vid=3,pcp=0),1


push_mpls clears L3/L4, while pop_mpls re-populates them, and then 
processing the output to port 1 hits the assert?


That's what I'm thinking too.

Jarno, is this something you have time to look into?  It'd be great, if
you do.  I'm way behind.


I’m looking at this.

Based on the trace given it seems that:
1. Packet is received on br-int port 32, which outputs it via NORMAL 
action over a patch port to another bridge. The only patch-port on 
br-int is 2 (patch-tun). The NORMAL action adds dl_vlan=1.
2. br-tun receives the packet on in_port 1 (patch-int), and outputs 
it on it’s port 2 (patch-to-mpls)
3. br-mpls receives the packet on it’s in_port 2 (patch-to-tun), does 
pop_vlan, and outputs to it’s port 21 (ipvpn-pp-out), which is also 
an patch port.
4. br-mpls (?) receives the packet on it’s in_port 20 (ipvpn-pp-in), 
does 
dec_ttl,push_mpls:0x8847,load:0x644c0->OXM_OF_MPLS_LABEL[],set_field:b8:2a:72:de:1b:e3->eth_src,set_field:00:17:cb:79:2c:01->eth_dst,output:1


All this generates a megaflow: 
set(ipv4(src=10.0.1.23,dst=10.0.0.3,ttl=63)),set(eth(src=b8:2a:72:de:1b:e3,dst=00:17:cb:79:2c:01)),push_mpls(label=410816,tc=0,ttl=63,bos=1,eth_type=0x8847),9


This is only the beginning part of the trouble-some megaflow, in 
which br-int sends the packet also to another port (vlan 3), and as 
part of that pops the MPLS and restores the original ethernet 
addresses. Maybe this would happen with the trace too, if you flushed 
MACs before the trace?


The patch ports 21 and 20 appear to be in the same bridge and patched 
to each other. Is this the case?


The crashing megaflow has in_port=5,dl_vlan=3. Is this also on br-int?

Also, OVS 2.6 is a little bit less aggressive about avoiding 
recirculation after mpls operations, and I’d be interested to know if 
your case fails the same way with OVS 2.6?


Thanks,

  Jarno





___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] netdev-dpdk: Don't use dev->vhost_id without mutex.

[Ben Pfaff]

How about "if (!vhost_id[0])", to avoid a useless strlen call?

On Mon, Dec 05, 2016 at 09:14:27AM +0300, Ilya Maximets wrote:
> The copy should be used here.
> 
> Fixes: 821b86649a90 ("netdev-dpdk: Don't try to unregister empty vhost_id.")
> Signed-off-by: Ilya Maximets 
> ---
>  lib/netdev-dpdk.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 6e5cd43..e06aa28 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1027,7 +1027,7 @@ netdev_dpdk_vhost_destruct(struct netdev *netdev)
>  ovs_mutex_unlock(>mutex);
>  ovs_mutex_unlock(_mutex);
>  
> -if (!strlen(dev->vhost_id)) {
> +if (!strlen(vhost_id)) {
>  goto out;
>  }
>  
> -- 
> 2.7.4
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/4] datapath-windows: Cleanup Conntrack definitions and introduce related entries

[Alin Serdean]

Looks good, but I have a question regarding the includes. Do you need 
"NetProto.h", ?

Thanks,
Alin.

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Sairam Venugopal
> Sent: Thursday, December 1, 2016 11:19 PM
> To: d...@openvswitch.org
> Subject: [ovs-dev] [PATCH 2/4] datapath-windows: Cleanup Conntrack
> definitions and introduce related entries
> 
> Consolidate the reusable structs and includes. Introduce the new
> OVS_CT_REL_ENTRY to track related connections.
> 
> Signed-off-by: Sairam Venugopal 
> ---
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/4] datapath-windows: Conntrack - Fix OvsGetTcpPayloadLength()

[Alin Serdean]

Thanks for the patch.

Looks good.

I was wondering if we could adapt and reuse OvsParseTcp 
(https://github.com/openvswitch/ovs/blob/master/datapath-windows/ovsext/PacketParser.c#L178)
 instead?

Thanks,
Alin.

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Sairam Venugopal
> Sent: Thursday, December 1, 2016 11:19 PM
> To: d...@openvswitch.org
> Subject: [ovs-dev] [PATCH 1/4] datapath-windows: Conntrack - Fix
> OvsGetTcpPayloadLength()
> 
> Move the OvsGetTcpPayloadLength() to common header. Update the code
> to check for null references and the correct size of the TCP header.
> 
> Signed-off-by: Sairam Venugopal 
> ---
>  datapath-windows/ovsext/Conntrack-tcp.c | 15 ---
>  datapath-windows/ovsext/Conntrack.h | 22 ++
>  2 files changed, 22 insertions(+), 15 deletions(-)
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 5/5] datapath-windows: Conntrack disable type truncation warning

[Alin Serdean]

Compiling with the WDK 10 gave the following warning:
Warning C4311   'type cast': pointer truncation from 'POVS_CT_ENTRY' to 'UINT32'
ovsext (OVSExt\ovsext)  Conntrack.c 1139

This patch disables the warning on the file Conntrack.c.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Conntrack.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index 84c4091..bdcf42c 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -25,6 +25,9 @@
 #include "Debug.h"
 #include "Event.h"
 
+#pragma warning( push )
+#pragma warning( disable:4311 )
+
 #define WINDOWS_TICK 1000
 #define SEC_TO_UNIX_EPOCH 11644473600LL
 #define SEC_TO_NANOSEC 10LL
@@ -1266,3 +1269,5 @@ OvsCtDumpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
 
 return STATUS_SUCCESS;
 }
+
+#pragma warning( pop )
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 4/5] datapath-windows: Fix function prototypes

[Alin Serdean]

There is a mismatch between OvsInitCompletionList and OvsAddPktCompletionList
prototypes.

Eg:
https://github.com/openvswitch/ovs/blob/master/datapath-windows/ovsext/PacketIO.h#L33
https://github.com/openvswitch/ovs/blob/master/datapath-windows/ovsext/PacketIO.c#L54

Found while compiling with Windows 10 kernel tool chain.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/PacketIO.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/datapath-windows/ovsext/PacketIO.c 
b/datapath-windows/ovsext/PacketIO.c
index a0ddc3d..dacd6f9 100644
--- a/datapath-windows/ovsext/PacketIO.c
+++ b/datapath-windows/ovsext/PacketIO.c
@@ -51,7 +51,7 @@ static NTSTATUS OvsCreateNewNBLsFromMultipleNBs(
 PNET_BUFFER_LIST *curNbl,
 PNET_BUFFER_LIST *nextNbl);
 
-__inline VOID
+VOID
 OvsInitCompletionList(OvsCompletionList *completionList,
   POVS_SWITCH_CONTEXT switchContext,
   ULONG sendCompleteFlags)
@@ -64,7 +64,7 @@ OvsInitCompletionList(OvsCompletionList *completionList,
 }
 
 /* Utility function used to complete an NBL. */
-__inline VOID
+VOID
 OvsAddPktCompletionList(OvsCompletionList *completionList,
 BOOLEAN incoming,
 NDIS_SWITCH_PORT_ID sourcePort,
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 3/5] datapath-windows: Remove unused section from driver inf

[Alin Serdean]

The new tool chain for Windows 10 driver contains a inf file checker.
While compiling it found the following issue:
 - Common.Params.reg is missing.

This patch removes the entry since it is not used, and at the moment
we do not add any specific keys to the Windows registry.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/ovsext.inf | 1 -
 1 file changed, 1 deletion(-)

diff --git a/datapath-windows/ovsext/ovsext.inf 
b/datapath-windows/ovsext/ovsext.inf
index c067e72..ad98d0d 100644
--- a/datapath-windows/ovsext/ovsext.inf
+++ b/datapath-windows/ovsext/ovsext.inf
@@ -74,7 +74,6 @@ ErrorControl= 1 ;SERVICE_ERROR_NORMAL
 ServiceBinary   = %12%\OVSExt.sys
 LoadOrderGroup  = NDIS
 Description = %OVSExt_Desc%
-AddReg  = Common.Params.reg
 
 [Install.Remove.Services]
 DelService=OVSExt,0x200
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev