Re: [ovs-dev] [PATCH] windows-installer: Update DriverVersion to be streamlined to OVS version

[Ben Pfaff]

On Thu, Apr 13, 2017 at 04:51:40PM +, Alin Serdean wrote:
> Patch:
> https://github.com/openvswitch/ovs/commit/0c15b76511e78a1f84dec49138d7169c2f3eedf6
> introduced a version variable for the MSI itself but did not propagate it
> too the driver version (used by the windows certificate tests).
> 
> This patch updates the driver version.
> 
> Signed-off-by: Alin Gabriel Serdean 

I applied this to master.  Thank you!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] windows: Crash when the handle communication device cannot be found

[Ben Pfaff]

On Thu, Apr 13, 2017 at 07:25:45PM +, Alin Serdean wrote:
> From: Alin Serdean 
> 
> When trying to uninstall/disable the OVS extension the driver will
> fail to unload properly(require reboot)/hang until ovs-vswitchd is closed.
> 
> The root cause of this behavior is because the handles from ovs-vswitchd
> to the kernel communication devices are still opened although the
> actual device was removed from the kernel.
> 
> Trying to close the handles will also fail because they do not exist.
> 
> The remaining option is to cause a crash and rely on the service manager
> to restart ovs-vswitchd.
> 
> Reported-at: https://github.com/openvswitch/ovs-issues/issues/27
> Reported-by: Alin Gabriel Serdean 
> Signed-off-by: Alin Gabriel Serdean 
> ---
> v2: Change ovs_fatal to ovs_abort as suggested by Ben Pfaff 

Applied to master, thank you!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 6/6] doc: Remove latex output configuration

[Ben Pfaff]

From: Stephen Finucane 

We don't care about building LaTeX documentation, so there's no need to
keep this build cruft around.

Signed-off-by: Stephen Finucane 
Signed-off-by: Ben Pfaff 
---
 Documentation/automake.mk | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index cff33cc659cc..b0f88ed6c9ff 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -102,9 +102,7 @@ SPHINXSRCDIR = $(srcdir)/Documentation
 SPHINXBUILDDIR = $(builddir)/Documentation/_build
 
 # Internal variables.
-PAPEROPT_a4 = -D latex_paper_size=a4
-PAPEROPT_letter = -D latex_paper_size=letter
-ALLSPHINXOPTS = -W -n -d $(SPHINXBUILDDIR)/doctrees $(PAPEROPT_$(PAPER)) 
$(SPHINXOPTS) $(SPHINXSRCDIR)
+ALLSPHINXOPTS = -W -n -d $(SPHINXBUILDDIR)/doctrees $(SPHINXOPTS) 
$(SPHINXSRCDIR)
 
 sphinx_verbose = $(sphinx_verbose_@AM_V@)
 sphinx_verbose_ = $(sphinx_verbose_@AM_DEFAULT_V@)
-- 
2.10.2

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

[ovs-dev] [PATCH 5/6] doc: Convert ovs-test to rST

[Ben Pfaff]

From: Stephen Finucane 

Signed-off-by: Stephen Finucane 
Signed-off-by: Ben Pfaff 
---
 Documentation/automake.mk |   1 +
 Documentation/conf.py |   3 +
 Documentation/ref/index.rst   |   1 +
 Documentation/ref/ovs-test.8.rst  | 163 
 Documentation/ref/ovs-vlan-test.8.rst |  11 +-
 debian/openvswitch-test.manpages  |   1 -
 manpages.mk   | 337 --
 utilities/automake.mk |   3 -
 utilities/ovs-test.8.in   | 144 ---
 9 files changed, 172 insertions(+), 492 deletions(-)
 create mode 100644 Documentation/ref/ovs-test.8.rst
 delete mode 100644 utilities/ovs-test.8.in

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index 762255277102..cff33cc659cc 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -137,6 +137,7 @@ endif
 
 # rST formatted manpages under Documentation/ref.
 RST_MANPAGES = \
+   ovs-test.8.rst \
ovs-vlan-test.8.rst
 
 # The GNU standards say that these variables should control
diff --git a/Documentation/conf.py b/Documentation/conf.py
index 8671a097d17e..1737d771ad70 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -328,6 +328,9 @@ latex_documents = [
 # One entry per manual page. List of tuples
 # (source start file, name, description, authors, manual section).
 man_pages = [
+('ref/ovs-test.8', 'ovs-test',
+ u'Check Linux drivers for performance, vlan and L3 tunneling problems',
+ [author], 8),
 ('ref/ovs-vlan-test.8', 'ovs-vlan-test',
  u'Check Linux drivers for problems with vlan traffic',
  [author], 8)
diff --git a/Documentation/ref/index.rst b/Documentation/ref/index.rst
index 6b368809c11b..3e2f8d5d96e0 100644
--- a/Documentation/ref/index.rst
+++ b/Documentation/ref/index.rst
@@ -39,6 +39,7 @@ time:
 .. toctree::
:maxdepth: 3
 
+   ovs-test.8
ovs-vlan-test.8
 
 The remainder are still in roff format can be found below:
diff --git a/Documentation/ref/ovs-test.8.rst b/Documentation/ref/ovs-test.8.rst
new file mode 100644
index ..f902d9c3b05a
--- /dev/null
+++ b/Documentation/ref/ovs-test.8.rst
@@ -0,0 +1,163 @@
+
+ovs-test
+
+
+Synopsis
+
+
+**ovs-test** -s *port*
+
+**ovs-test** -c *server1* *server2* [**-b** *targetbandwidth*] [**-i** 
*testinterval*] [**-d**]
+  [**-l** *vlantag*] [**-t** *tunnelmodes*]
+
+Description
+===
+
+The :program:`ovs-test` program may be used to check for problems sending
+802.1Q or GRE traffic that Open vSwitch may uncover. These problems, for
+example, can occur when Open vSwitch is used to send 802.1Q traffic through
+physical interfaces running certain drivers of certain Linux kernel versions.
+To run a test, configure IP addresses on `server1` and `server2` for interfaces
+you intended to test. These interfaces could also be already configured OVS
+bridges that have a physical interface attached to them. Then, on one of the
+nodes, run :program:`ovs-test` in server mode and on the other node run it in
+client mode. The client will connect to :program:`ovs-test` server and schedule
+tests between both of them. The :program:`ovs-test` client will perform UDP and
+TCP tests.
+
+UDP tests can report packet loss and achieved bandwidth for various datagram
+sizes. By default target bandwidth for UDP tests is 1Mbit/s.
+
+TCP tests report only achieved bandwidth, because kernel TCP stack takes care
+of flow control and packet loss. TCP tests are essential to detect potential
+TSO related issues.
+
+To determine whether Open vSwitch is encountering any problems, the user must
+compare packet loss and achieved bandwidth in a setup where traffic is being
+directly sent and in one where it is not. If in the 802.1Q or L3 tunneled tests
+both :program:`ovs-test` processes are unable to communicate or the achieved
+bandwidth is much lower compared to direct setup, then, most likely, Open
+vSwitch has encountered a pre-existing kernel or driver bug.
+
+Some examples of the types of problems that may be encountered are:
+
+- When NICs use VLAN stripping on receive they must pass a pointer to a
+  `vlan_group` when reporting the stripped tag to the networking core. If no
+  `vlan_group` is in use then some drivers just drop the extracted tag.
+  Drivers are supposed to only enable stripping if a `vlan_group` is registered
+  but not all of them do that.
+
+- On receive, some drivers handle priority tagged packets specially and don't
+  pass the tag onto the network stack at all, so Open vSwitch never has a
+  chance to see it.
+
+- Some drivers size their receive buffers based on whether a `vlan_group` is
+  enabled, meaning that a maximum size packet with a VLAN tag will not fit if
+  no `vlan_group` is configured.
+
+- On transmit, some drivers expect that VLAN acceleration will be used if it is
+  available, which can 

[ovs-dev] [PATCH 4/6] doc: Convert ovs-vlan-test to rST

[Ben Pfaff]

From: Stephen Finucane 

Let's start with a simple one that lets us focus on setting up most of
the required "infrastructure" for building man pages using Sphinx.

This changes the 'check-htmldocs' target to 'check-docs' as its now
responsible for building man page docs too.

Other than that, hurrah for (mostly) legible syntaxes.

[1] http://www.tldp.org/HOWTO/Man-Page/q2.html

Signed-off-by: Stephen Finucane 
Signed-off-by: Ben Pfaff 
---
 Documentation/automake.mk  |  84 +--
 Documentation/conf.py  |   5 +-
 .../internals/contributing/documentation-style.rst |   3 +-
 Documentation/intro/install/documentation.rst  |   4 +-
 Documentation/ref/index.rst|  16 ++-
 Documentation/ref/ovs-vlan-test.8.rst  | 115 +
 debian/openvswitch-switch.manpages |   1 -
 manpages.mk|  10 --
 utilities/automake.mk  |   3 -
 utilities/ovs-vlan-test.8.in   |  96 -
 10 files changed, 211 insertions(+), 126 deletions(-)
 create mode 100644 Documentation/ref/ovs-vlan-test.8.rst
 delete mode 100644 utilities/ovs-vlan-test.8.in

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index 9911668c1ca9..762255277102 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -90,7 +90,8 @@ DOC_SOURCE = \
Documentation/internals/contributing/documentation-style.rst \
Documentation/internals/contributing/libopenvswitch-abi.rst \
Documentation/internals/contributing/submitting-patches.rst \
-   Documentation/requirements.txt
+   Documentation/requirements.txt \
+   $(addprefix Documentation/ref/,$(RST_MANPAGES))
 
 EXTRA_DIST += $(DOC_SOURCE)
 
@@ -110,20 +111,89 @@ sphinx_verbose_ = $(sphinx_verbose_@AM_DEFAULT_V@)
 sphinx_verbose_0 = -q
 
 if HAVE_SPHINX
-htmldocs-check: $(DOC_SOURCE)
+docs-check: $(DOC_SOURCE)
$(AM_V_GEN)$(SPHINXBUILD) $(sphinx_verbose) -b html $(ALLSPHINXOPTS) 
$(SPHINXBUILDDIR)/html && touch $@
-ALL_LOCAL += htmldocs-check
-CLEANFILES += htmldocs-check
+   $(AM_V_GEN)$(SPHINXBUILD) $(sphinx_verbose) -b man $(ALLSPHINXOPTS) 
$(SPHINXBUILDDIR)/man && touch $@
+ALL_LOCAL += docs-check
+CLEANFILES += docs-check
 
 check-docs:
$(SPHINXBUILD) -b linkcheck $(ALLSPHINXOPTS) $(SPHINXBUILDDIR)/linkcheck
 
 clean-docs:
-   rm -rf $(SPHINXBUILDDIR)/doctrees
-   rm -rf $(SPHINXBUILDDIR)/html
-   rm -rf $(SPHINXBUILDDIR)/linkcheck
+   rm -rf $(SPHINXBUILDDIR)
rm -f docs-check
 CLEAN_LOCAL += clean-docs
 endif
 .PHONY: check-docs
 .PHONY: clean-docs
+
+# Installing manpages based on rST.
+#
+# The docs-check target converts the rST files listed in RST_MANPAGES
+# into nroff manpages in Documentation/_build/man.  The easiest way to
+# get these installed by "make install" is to write our own helper
+# rules.
+
+# rST formatted manpages under Documentation/ref.
+RST_MANPAGES = \
+   ovs-vlan-test.8.rst
+
+# The GNU standards say that these variables should control
+# installation directories for manpages in each section.  Automake
+# will define them for us only if it sees that a manpage in the
+# appropriate section is to be installed through its built-in feature.
+# Since we're working independently, for best safety, we need to
+# define them ourselves.
+man1dir = $(mandir)/man1
+man2dir = $(mandir)/man2
+man3dir = $(mandir)/man3
+man4dir = $(mandir)/man4
+man5dir = $(mandir)/man5
+man6dir = $(mandir)/man6
+man7dir = $(mandir)/man7
+man8dir = $(mandir)/man8
+man9dir = $(mandir)/man9
+
+# Set a shell variable for each manpage directory.
+set_mandirs = \
+   man1dir='$(man1dir)' \
+   man2dir='$(man2dir)' \
+   man3dir='$(man3dir)' \
+   man4dir='$(man4dir)' \
+   man5dir='$(man5dir)' \
+   man6dir='$(man6dir)' \
+   man7dir='$(man7dir)' \
+   man8dir='$(man8dir)' \
+   man9dir='$(man9dir)'
+
+# Given an $rst of "ovs-vlan-test.8.rst", sets $stem to
+# "ovs-vlan-test", $section to "8", and $mandir to $man8dir.
+extract_stem_and_section = \
+   stem=`echo "$$rst" | sed -n 's/^\(.*\)\.\([0-9]\).rst$$/\1/p'`; \
+   section=`echo "$$rst" | sed -n 's/^\(.*\)\.\([0-9]\).rst$$/\2/p'`; \
+   test -n "$$section" || { echo "$$rst: cannot infer manpage section from 
filename" 2>&1; continue; }; \
+   eval "mandir=\$$man$${section}dir"; \
+   test -n "$$mandir" || { echo "unknown directory for manpage section 
$$section"; continue; }
+
+if HAVE_SPHINX
+INSTALL_DATA_LOCAL += install-man-rst
+install-man-rst: docs-check
+   @$(set_mandirs); \
+   for rst in $(RST_MANPAGES); do \
+   $(extract_stem_and_section); \
+   echo " $(MKDIR_P) '$(DESTDIR)'\"$$mandir\""; \
+   $(MKDIR_P) '$(DESTDIR)'"$$mandir"; \
+   echo " $(INSTALL_DATA) 

[ovs-dev] [PATCH 2/6] doc: Avoid need to generate conf.py.

[Ben Pfaff]

It's awkward to have to at the same time generate conf.py from conf.py.in
and to keep both versions in the repository.  This avoids the issue.

Signed-off-by: Ben Pfaff 
---
 Documentation/automake.mk |  10 --
 Documentation/conf.py |  23 ++-
 Documentation/conf.py.in  | 349 --
 3 files changed, 19 insertions(+), 363 deletions(-)
 delete mode 100644 Documentation/conf.py.in

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index ec60e0b1e831..9911668c1ca9 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -3,7 +3,6 @@ DOC_SOURCE = \
Documentation/_static/logo.png \
Documentation/_static/overview.png \
Documentation/conf.py \
-   Documentation/conf.py.in \
Documentation/index.rst \
Documentation/contents.rst \
Documentation/intro/index.rst \
@@ -125,15 +124,6 @@ clean-docs:
rm -rf $(SPHINXBUILDDIR)/linkcheck
rm -f docs-check
 CLEAN_LOCAL += clean-docs
-
-ALL_LOCAL += $(srcdir)/Documentation/conf.py
-$(srcdir)/Documentation/conf.py: $(srcdir)/Documentation/conf.py.in
-   $(AM_V_GEN)($(ro_shell) && sed -e 's,[@]VERSION[@],$(VERSION),g' \
-   -e 's,[@]OVS_MAJOR[@],$(shell echo $(VERSION) | cut -f1 -d.),g' 
\
-   -e 's,[@]OVS_MINOR[@],$(shell echo $(VERSION) | cut -f2 
-d.),g') \
-   < $(srcdir)/Documentation/$(@F).in > $(@F).tmp || exit 1; \
-   if cmp -s $(@F).tmp $@; then touch $@; rm $(@F).tmp; else mv $(@F).tmp 
$@; fi
-
 endif
 .PHONY: check-docs
 .PHONY: clean-docs
diff --git a/Documentation/conf.py b/Documentation/conf.py
index ae672cbe7b0d..bfd7f33d88ba 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -1,4 +1,3 @@
-# Generated automatically -- do not modify!-*- buffer-read-only: t -*-
 # -*- coding: utf-8 -*-
 #
 # Open vSwitch documentation build configuration file, created by
@@ -63,10 +62,26 @@ author = u'The Open vSwitch Development Community'
 # |version| and |release|, also used in various other places throughout the
 # built documents.
 #
+import string
+import sys
+def get_release():
+filename = "../configure.ac"
+with open(filename, 'rU') as f:
+for line in f:
+if 'AC_INIT' in line:
+# Parse "AC_INIT(openvswitch, 2.7.90, b...@openvswitch.org)":
+return line.split(',')[1].strip(string.whitespace + '[]')
+sys.stderr.write('%s: failed to determine Open vSwitch version\n'
+ % filename)
+sys.exit(1)
+release = get_release() # The full version, including alpha/beta/rc tags.
+
 # The short X.Y version.
-version = u'2.7'
-# The full version, including alpha/beta/rc tags.
-release = u'2.7.90'
+#
+# However, it's important to know the difference between, e.g., 2.7
+# and 2.7.90, which can be very different versions (2.7.90 may be much
+# closer to 2.8 than to 2.7), so check for that.
+version = release if '.90' in release else '.'.join(release.split('.')[0:2])
 
 # The language for content autogenerated by Sphinx. Refer to documentation
 # for a list of supported languages.
diff --git a/Documentation/conf.py.in b/Documentation/conf.py.in
deleted file mode 100644
index 5ed7006228f2..
--- a/Documentation/conf.py.in
+++ /dev/null
@@ -1,349 +0,0 @@
-# -*- coding: utf-8 -*-
-#
-# Open vSwitch documentation build configuration file, created by
-# sphinx-quickstart on Fri Sep 30 09:57:36 2016.
-#
-# This file is execfile()d with the current directory set to its
-# containing dir.
-#
-# Note that not all possible configuration values are present in this
-# autogenerated file.
-#
-# All configuration values have a default; values that are commented out
-# serve to show the default.
-
-# If extensions (or modules to document with autodoc) are in another directory,
-# add these directories to sys.path here. If the directory is relative to the
-# documentation root, use os.path.abspath to make it absolute, like shown here.
-#
-# import os
-# import sys
-# sys.path.insert(0, os.path.abspath('.'))
-try:
-import ovs_sphinx_theme
-use_ovs_theme = True
-except ImportError:
-print("Cannot find 'ovs_sphinx' package. Falling back to default theme.")
-use_ovs_theme = False
-
-# -- General configuration 
-
-# If your documentation needs a minimal Sphinx version, state it here.
-#
-needs_sphinx = '1.1'
-
-# Add any Sphinx extension module names here, as strings. They can be
-# extensions coming with Sphinx (named 'sphinx.ext.*') or your custom
-# ones.
-extensions = []
-
-# Add any paths that contain templates here, relative to this directory.
-templates_path = ['_templates']
-
-# The suffix(es) of source filenames.
-# You can specify multiple suffix as a list of string:
-#
-# source_suffix = ['.rst', '.md']
-source_suffix = '.rst'
-
-# The encoding of source files.
-#
-# source_encoding = 'utf-8-sig'
-
-# The master toctree 

[ovs-dev] [PATCH 1/6] doc: Also delete stamp file in clean-docs target.

[Ben Pfaff]

Otherwise "make docs-check" won't necessarily do anything since its
apparent target is up to date.

Signed-off-by: Ben Pfaff 
---
 Documentation/automake.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index ed12ad017107..ec60e0b1e831 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -123,6 +123,7 @@ clean-docs:
rm -rf $(SPHINXBUILDDIR)/doctrees
rm -rf $(SPHINXBUILDDIR)/html
rm -rf $(SPHINXBUILDDIR)/linkcheck
+   rm -f docs-check
 CLEAN_LOCAL += clean-docs
 
 ALL_LOCAL += $(srcdir)/Documentation/conf.py
-- 
2.10.2

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

[ovs-dev] [PATCH 0/6] Sphinx for Manpages

[Ben Pfaff]

This is Stephen Finucane's RFC series from
https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/330666.html
with my changes plus two extra patches at the beginning.  Please see my
comments on the former postings for more on the changes that I made.

Ben Pfaff (2):
  doc: Also delete stamp file in clean-docs target.
  doc: Avoid need to generate conf.py.

Stephen Finucane (4):
  doc: Add man page section to documentation guide
  doc: Convert ovs-vlan-test to rST
  doc: Convert ovs-test to rST
  doc: Remove latex output configuration

 Documentation/automake.mk  | 100 --
 Documentation/conf.py  |  31 +-
 Documentation/conf.py.in   | 349 -
 .../internals/contributing/documentation-style.rst |  85 -
 Documentation/intro/install/documentation.rst  |   4 +-
 Documentation/ref/index.rst|  17 +-
 Documentation/ref/ovs-test.8.rst   | 163 ++
 Documentation/ref/ovs-vlan-test.8.rst  | 112 +++
 debian/openvswitch-switch.manpages |   1 -
 debian/openvswitch-test.manpages   |   1 -
 manpages.mk| 347 
 utilities/automake.mk  |   6 -
 utilities/ovs-test.8.in| 144 -
 utilities/ovs-vlan-test.8.in   |  96 --
 14 files changed, 472 insertions(+), 984 deletions(-)
 delete mode 100644 Documentation/conf.py.in
 create mode 100644 Documentation/ref/ovs-test.8.rst
 create mode 100644 Documentation/ref/ovs-vlan-test.8.rst
 delete mode 100644 utilities/ovs-test.8.in
 delete mode 100644 utilities/ovs-vlan-test.8.in

-- 
2.10.2

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

Re: [ovs-dev] [RFC 3/4] doc: Convert ovs-test to rST

[Ben Pfaff]

On Mon, Apr 10, 2017 at 01:12:29PM +0100, Stephen Finucane wrote:
> Signed-off-by: Stephen Finucane 
> ---
> See comment on the previous change

Thanks again, I made similar changes to this one as I did to the
previous.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC 2/4] doc: Convert ovs-vlan-test to rST

[Ben Pfaff]

Thank you!  rST is much more readable than nroff.  I have some comments
below.

On Mon, Apr 10, 2017 at 01:12:28PM +0100, Stephen Finucane wrote:
> Let's start with a simple one that lets us focus on setting up most of
> the required "infrastructure" for building man pages using Sphinx.
> 
> There are a couple of things worth noting here:
> 
> - The 'check-htmldocs' target becomes 'check-docs' as its now
>   responsible for building man page docs too.
> 
> - The outputted file will always have a '.1' suffix. This is Sphinx's
>   decision, and likely stems from the man page guidelines [1] which
>   state that "the name of the man page's source file...is the name of
>   the command, function or file name, followed by a dot, followed by the
>   section character".

It looks to me like the last element of the tuples inside man_pages in
conf.py controls the section.  When I changed 1 to 8 there, it switched
the manpage to section 8.  So I made that change in conf.py and I
removed the above paragraph from the commit message.

> Other than that, hurrah for (mostly) legible syntaxes.
> 
> [1] http://www.tldp.org/HOWTO/Man-Page/q2.html
> 
> Signed-off-by: Stephen Finucane 
> ---
> I don't know if this is correctly integrated into the docs build system
> or not. I need someone to double check this for me. In particular, I
> think I need to integrate the 'dh_sphinxdoc' package [2] into the Debian
> build but I've no idea how to.
> 
> [2] http://manpages.ubuntu.com/manpages/zesty/man1/dh_sphinxdoc.1.html

I spent a couple of hours working on the build and install system here.

My first thought was to add rules to allow Automake to find and install
the generated manpages.  This turned out to be a huge mess that required
tons of nasty GNU Make specific crap in the makefiles, and I didn't like
it.

My second approach was better.  I gave up on integrating with the
builtin Automake rules for manpages.  All those really do anyway is
handle install and uninstall, so I wrote some Make rules to do that.
They're ugly because they're make+shell, but readable enough if you
squint.

The main question for install and uninstall is how to choose the right
section.  The easiest way seems to be to give the .rst files names that
embed the section, like "ovs-vlan-test.8.rst".  This is also handy to
distinguish manpages with the same name but in different sections, which
is sometimes a reasonable thing to have, so that's what I did.

The patch didn't delete the old manpage, so I did that too.

I noticed that the "Synposis" section of the new ovs-vlan-test.rst did
not use bold and italic in the canonical way for manpages, so I added *
and ** in the right places.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] OVS performance with Kernel Datapath of Linux upstream vs Linux OVS tree.

[Kapil Adhikesavalu]

Thanks Jarno!

On Fri, 14 Apr 2017, 2:53 AM Jarno Rajahalme,  wrote:

> On Apr 12, 2017, at 9:00 PM, Kapil Adhikesavalu 
> wrote:
>
> Hi Jarno,
>
> That's great! Thanks for the clarification. So, if anything it should only
> improve the performance when I move to OVS tree kernel module.
>
>
> Yes, assuming you will be using a recent OVS release rather than an old
> one.
>
>   Jarno
>
> On Thu, 13 Apr 2017, 1:05 AM Jarno Rajahalme,  wrote:
>
>>
>> > On Apr 12, 2017, at 12:49 AM, Kapil Adhikesavalu 
>> wrote:
>> >
>> > Hi,
>> >
>> > Is there any performance difference with using the OVS kernel Datapath
>> > available part of Linux upstream Vs the module built from Linux OVS
>> tree.
>> >
>>
>> OVS tree kernel module has an Exact Match Cache, which generally improves
>> performance. Upstream linux openvswitch module does not have it.
>>
>> > So far i have been using the DP part of the Linux upstream and as NAT
>> > feature requires Linux version 4.6, i plan to switch to DP module built
>> > from OVS tree.
>> >
>> > 1. In general is there any performance between using these two ? or
>> would
>> > it vary based on the features being in use.
>> > 2. I am currently using VXLAN and L2 forwarding + NAT(plan to use it),
>> > would like to know if there could be any performance difference expected
>> > when i switch from upstream DP to KLM.
>> >
>> > I didn't any specific mention about performance in FAQ -
>> > http://docs.openvswitch.org/en/latest/faq/releases/ expect for this
>> > statement 'Certain features require kernel support to function or to
>> have
>> > reasonable performance.’
>> >
>>
>> This note relates to using OVS with an older (upstream) kernel module.
>> While we maintain backwards compatibility, newer features perform better
>> with newer kernel module having explicit support for the feature.
>>
>>   Jarno
>>
>> > Regards
>> > Kapil.
>> > ___
>> > dev mailing list
>> > d...@openvswitch.org
>> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>> --
>
> Regards
> Kapil
>
>
> --

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

[ovs-dev] (no subject)

[Sgt John Adams]

I am Sgt Adam John , I have a Secured Monetary deal for you and
it'slegitimate,$25,000,000.00 USD


This message was sent using IMP, the Internet Messaging Program.

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

[ovs-dev] [PATCH] ofp-parse: Fix match parsing with [x..y]=z format.

[Jarno Rajahalme]

Commit 21b2fa617126 ("ofp-parse: Allow match field names in actions
and brackets in matches.") added support for matching a consecutive
set of bits with the [x..y]=z format, but the copying of the parsed
value ('z') to the match was done from a wrong offset, so that the
actual value matched would be incorrect.

Fix this and add a test case preventing regression in future.

Fixes: 21b2fa617126 ("ofp-parse: Allow match field names in actions and 
brackets in matches.")
Signed-off-by: Jarno Rajahalme 
---
 lib/ofp-parse.c| 6 +++---
 tests/ovs-ofctl.at | 2 ++
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 7826bc5..c8cac5b 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -290,11 +290,11 @@ parse_subfield(const char *name, const char *str_value, 
struct match *match,
 
 const struct mf_field *field = sf.field;
 union mf_value value, mask;
-unsigned int size = DIV_ROUND_UP(sf.n_bits, 8);
+unsigned int size = field->n_bytes;
 
 mf_get(field, match, , );
-bitwise_copy(, size, 0, , field->n_bytes, sf.ofs, sf.n_bits);
-bitwise_one (   ,  field->n_bytes, sf.ofs, sf.n_bits);
+bitwise_copy(, size, 0, , size, sf.ofs, sf.n_bits);
+bitwise_one (   ,  size, sf.ofs, sf.n_bits);
 *usable_protocols &= mf_set(field, , , match, );
 }
 return error;
diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index 737f609..18ab788 100644
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -285,6 +285,7 @@ AT_CLEANUP
 AT_SETUP([ovs-ofctl parse-flows (OpenFlow 1.2)])
 AT_DATA([flows.txt], [[
 # comment
+tcp,tp_src[5]=1,actions=flood
 tcp,tp_src=123,actions=flood
 in_port=LOCAL dl_vlan=9 dl_src=00:0A:E4:25:6B:B0 
actions=mod_vlan_vid:7,mod_vlan_pcp:2
 udp dl_vlan_pcp=7 idle_timeout=5 actions=strip_vlan output:0
@@ -309,6 +310,7 @@ AT_CHECK([ovs-ofctl --protocols OpenFlow12 parse-flows 
flows.txt
 AT_CHECK([[sed 's/ (xid=0x[0-9a-fA-F]*)//' stdout]], [0],
 [[usable protocols: NXM,OXM
 chosen protocol: OXM-OpenFlow12
+OFPT_FLOW_MOD (OF1.2): ADD tcp,tp_src=0x20/0x20 actions=FLOOD
 OFPT_FLOW_MOD (OF1.2): ADD tcp,tp_src=123 actions=FLOOD
 OFPT_FLOW_MOD (OF1.2): ADD in_port=LOCAL,dl_vlan=9,dl_src=00:0a:e4:25:6b:b0 
actions=set_field:4103->vlan_vid,set_field:2->vlan_pcp
 OFPT_FLOW_MOD (OF1.2): ADD udp,dl_vlan_pcp=7 idle:5 actions=pop_vlan,output:0
-- 
2.1.4

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

Re: [ovs-dev] [PATCH ovs V7 19/24] netdev-vport: Use common offloads interface

[Joe Stringer]

On 7 April 2017 at 06:13, Roi Dayan  wrote:
> From: Paul Blakey 
>
> netdev vports are backed by actualy netdev at the kernel
> level, so they can use the common netdev-tc offloads interface
> for flow offloading (if enabled).
>
> Signed-off-by: Paul Blakey 
> Signed-off-by: Simon Horman 
> Reviewed-by: Roi Dayan 
> ---



> @@ -779,11 +783,37 @@ get_stats(const struct netdev *netdev, struct 
> netdev_stats *stats)
>  return 0;
>  }
>
> -

Please don't drop the ^L, they're used to separate the file into
logically separate sections.

> +#ifdef __linux__
> +static int
> +netdev_vport_get_ifindex__(const struct netdev *netdev_)
> +{
> +char buf[NETDEV_VPORT_NAME_BUFSIZE];
> +const char *name = netdev_vport_get_dpif_port(netdev_, buf, sizeof(buf));
> +
> +return linux_get_ifindex(name);

Why link directly against linux implementation, ignoring that netdev
provides netdev_get_ifindex() ?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovs V7 15/24] netdev-tc-offloads: Implement netdev flow del using tc interface

[Joe Stringer]

On 7 April 2017 at 06:13, Roi Dayan  wrote:
> From: Paul Blakey 
>
> Signed-off-by: Paul Blakey 
> Reviewed-by: Roi Dayan 
> Reviewed-by: Simon Horman 
> ---
>  lib/netdev-tc-offloads.c | 33 ++---
>  1 file changed, 30 insertions(+), 3 deletions(-)
>
> diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
> index 4ef29d1..d5961e2 100644
> --- a/lib/netdev-tc-offloads.c
> +++ b/lib/netdev-tc-offloads.c
> @@ -874,10 +874,37 @@ netdev_tc_flow_get(struct netdev *netdev OVS_UNUSED,
>
>  int
>  netdev_tc_flow_del(struct netdev *netdev OVS_UNUSED,
> -   const ovs_u128 *ufid OVS_UNUSED,
> -   struct dpif_flow_stats *stats OVS_UNUSED)
> +   const ovs_u128 *ufid,
> +   struct dpif_flow_stats *stats)
>  {
> -return EOPNOTSUPP;
> +struct netdev *dev;
> +int prio = 0;
> +int ifindex;
> +int handle;
> +int error;
> +
> +handle = get_ufid_tc_mapping(ufid, , );
> +if (!handle) {
> +return ENOENT;
> +}
> +
> +ifindex = netdev_get_ifindex(dev);
> +if (ifindex < 0) {
> +VLOG_ERR_RL(_err, "failed to get ifindex for %s: %s",
> +netdev_get_name(dev), ovs_strerror(-ifindex));
> +netdev_close(dev);

Whose reference is this?

If the ifindex can't be gotten, was it ever inserted into the
ufid_tc_mapping in the first place?

If I follow, there's one reference on the netdev associated with the
flow, then two associated with the ufid_tc mapping.. is the caller
also likely to have a reference?

> +return -ifindex;
> +}
> +
> +error = tc_del_filter(ifindex, prio, handle);
> +del_ufid_tc_mapping(ufid);
> +
> +netdev_close(dev);
> +
> +if (stats) {
> +memset(stats, 0, sizeof(*stats));
> +}
> +return error;
>  }
>
>  int
> --
> 2.7.4
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovs V7 02/24] netdev: Adding a new netdev api to be used for offloading flows

[Joe Stringer]

On 7 April 2017 at 06:12, Roi Dayan  wrote:
> From: Paul Blakey 
>
> Signed-off-by: Paul Blakey 
> Reviewed-by: Roi Dayan 
> Reviewed-by: Simon Horman 
> ---



> diff --git a/lib/netdev.h b/lib/netdev.h
> index d6c07c1..6d2db7d 100644
> --- a/lib/netdev.h
> +++ b/lib/netdev.h
> @@ -156,6 +156,29 @@ int netdev_send(struct netdev *, int qid, struct 
> dp_packet_batch *,
>  bool may_steal, bool concurrent_txq);
>  void netdev_send_wait(struct netdev *, int qid);
>
> +/* Flow offloading. */
> +struct offload_info {
> +const void *port_hmap_obj; /* To query ports info from netdev port map */
> +ovs_be16 tp_dst_port; /* Destination port for tunnel in SET action */

Is this assuming there is only ever one tunnel destination port? What
about multiple output?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovs V7 11/24] dpif-netlink: Use netdev flow put api to insert a flow

[Joe Stringer]

On 7 April 2017 at 06:12, Roi Dayan  wrote:
> From: Paul Blakey 
>
> Using the new netdev flow api operate will now try and
> offload flows to the relevant netdev of the input port.
> Other operate methods flows will come in later patches.
>
> Signed-off-by: Paul Blakey 
> Reviewed-by: Roi Dayan 
> Reviewed-by: Simon Horman 
> ---



>  static void
> -dpif_netlink_operate(struct dpif *dpif_, struct dpif_op **ops, size_t n_ops)
> +dbg_print_flow(const struct nlattr *key, size_t key_len,
> +   const struct nlattr *mask, size_t mask_len,
> +   const struct nlattr *actions, size_t actions_len,
> +   const ovs_u128 *ufid,
> +   const char *op)

I wonder if we could refactor log_flow_message() into somewhere
neutral and share the output format for these flows from here and
there.

> +{
> +struct ds s;
> +
> +ds_init();
> +ds_put_cstr(, op);
> +ds_put_cstr(, " (");
> +odp_format_ufid(ufid, );
> +ds_put_cstr(, ")");
> +if (key_len) {
> +ds_put_cstr(, "\nflow (verbose): ");
> +odp_flow_format(key, key_len, mask, mask_len, NULL, , true);
> +ds_put_cstr(, "\nflow: ");
> +odp_flow_format(key, key_len, mask, mask_len, NULL, , false);
> +}
> +ds_put_cstr(, "\nactions: ");
> +format_odp_actions(, actions, actions_len);
> +VLOG_DBG("\n%s", ds_cstr());
> +ds_destroy();
> +}
> +
> +static int
> +try_send_to_netdev(struct dpif_netlink *dpif, struct dpif_op *op)
>  {
> -struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
> +switch (op->type) {
> +case DPIF_OP_FLOW_PUT: {
> +struct dpif_flow_put *put = >u.flow_put;
>
> +if (!put->ufid) {
> +break;
> +}
> +dbg_print_flow(put->key, put->key_len, put->mask, put->mask_len,
> +   put->actions, put->actions_len, put->ufid,
> +   (put->flags & DPIF_FP_MODIFY ? "PUT(MODIFY)" : 
> "PUT"));
> +return parse_flow_put(dpif, put);
> +}
> +case DPIF_OP_FLOW_DEL:
> +case DPIF_OP_FLOW_GET:
> +case DPIF_OP_EXECUTE:
> +default:
> +break;
> +}
> +return EOPNOTSUPP;
> +}
> +
> +static void
> +dpif_netlink_operate_chunks(struct dpif_netlink *dpif, struct dpif_op **ops,
> +size_t n_ops)
> +{
>  while (n_ops > 0) {
>  size_t chunk = dpif_netlink_operate__(dpif, ops, n_ops);
> +
>  ops += chunk;
>  n_ops -= chunk;
>  }
>  }
>
> +static void
> +dpif_netlink_operate(struct dpif *dpif_, struct dpif_op **ops, size_t n_ops)
> +{
> +struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
> +struct dpif_op *new_ops[OPERATE_MAX_OPS];
> +int count = 0;
> +int i = 0;
> +int err = 0;
> +
> +if (netdev_flow_api_enabled) {
> +while (n_ops > 0) {
> +count = 0;
> +
> +while (n_ops > 0 && count < OPERATE_MAX_OPS) {
> +struct dpif_op *op = ops[i++];
> +
> +err = try_send_to_netdev(dpif, op);
> +if (err && err != EEXIST) {
> +new_ops[count++] = op;
> +} else {
> +op->error = err;
> +}
> +
> +n_ops--;
> +}
> +
> +dpif_netlink_operate_chunks(dpif, new_ops, count);
> +}
> +
> +return;
> +}
> +
> +dpif_netlink_operate_chunks(dpif, ops, n_ops);
> +}
> +
>  #if _WIN32
>  static void
>  dpif_netlink_handler_uninit(struct dpif_handler *handler)
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index 8747778..349425e 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -41,6 +41,7 @@
>  #include "util.h"
>  #include "uuid.h"
>  #include "openvswitch/vlog.h"
> +#include "openvswitch/match.h"
>
>  VLOG_DEFINE_THIS_MODULE(odp_util);
>
> @@ -5497,6 +5498,58 @@ odp_flow_key_to_mask(const struct nlattr *mask_key, 
> size_t mask_key_len,
>  }
>  }
>
> +/* Converts the netlink formated key/mask to match.
> + * Fails if odp_flow_key_from_key/mask and odp_flow_key_key/mask
> + * disagree on the acceptable form of flow */
> +int
> +parse_key_and_mask_to_match(const struct nlattr *key, size_t key_len,
> +const struct nlattr *mask, size_t mask_len,
> +struct match *match)
> +{
> +enum odp_key_fitness fitness;
> +
> +fitness = odp_flow_key_to_flow(key, key_len, >flow);
> +if (fitness) {
> +/* This should not happen: it indicates that odp_flow_key_from_flow()
> + * and odp_flow_key_to_flow() disagree on the acceptable form of a
> + * flow.  Log the problem as an error, with enough details to enable
> + * debugging. */
> +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> +
> +if 

Re: [ovs-dev] [PATCH ovs V7 02/24] netdev: Adding a new netdev api to be used for offloading flows

[Joe Stringer]

On 7 April 2017 at 06:12, Roi Dayan  wrote:
> From: Paul Blakey 
>
> Signed-off-by: Paul Blakey 
> Reviewed-by: Roi Dayan 
> Reviewed-by: Simon Horman 
> ---



> @@ -769,6 +777,49 @@ struct netdev_class {
>
>  /* Discards all packets waiting to be received from 'rx'. */
>  int (*rxq_drain)(struct netdev_rxq *rx);
> +
> +/* ##  ## */
> +/* ## netdev flow offloading functions ## */
> +/* ##  ## */
> +
> +/* If a particular netdev class does not support offloading flows, all these
> + * function pointers must be NULL. */
> +
> +/* Deleting all offloaded flows from netdev */
> +int (*flow_flush)(struct netdev *);
> +/* Dumping interface:
> + * Usage is as with dpif_port_dump api (create, next, destory).
> + * Create sets dump on success or returns error status on failure. */

What is the error status? negative? positive? errno?

Common OVS descriptions say something like:

Returns ___, if successful. On failure, returns a negative errno value.

> +int (*flow_dump_create)(struct netdev *, struct netdev_flow_dump **dump);
> +int (*flow_dump_destroy)(struct netdev_flow_dump *);
> + /* rbuffer is for use of the implementation (e.g using nl_dump),
> + * and is usually shared for the given thread that runs flow_dump_next.
> + * wbuffer is the buffer that dumped actions will be stored in, and given
> + * pointers to. */
> +bool (*flow_dump_next)(struct netdev_flow_dump *, struct match *,
> +   struct nlattr **actions,
> +   struct dpif_flow_stats *stats, ovs_u128 *ufid,
> +   struct ofpbuf *rbuffer, struct ofpbuf *wbuffer);

How does this function expect 'rbuffer' and 'wbuffer' to be prepared?
Are they already allocated? Will this function reallocate them? Who
frees them?

What does this function return?

I think that the equivalent dpif interface describes that the
parameters to flow_dump_next() must be prepared by flow_dump_create().

> +
> +/* Offload the given flow (match, actions, stats, ufid) on netdev.
> + * If stats isn't null, sets the given stats for that flow.
> + * To modify the flow, use the same ufid.
> + * actions are in netlink format, as with struct dpif_flow_put.
> + * info is anything else that is need to offload the flow. */
> +int (*flow_put)(struct netdev *, struct match *, struct nlattr *actions,
> +size_t actions_len, struct dpif_flow_stats *,
> +const ovs_u128 *ufid, struct offload_info *info);

Really, it sets the stats? Does it ever retrieve stats (for example,
if you modify the flow)?

What does it return?

> +/* Queries the flow with specified ufid on netdev.
> + * Fills match, actions, stats as with flow_dump_next */
> +int (*flow_get)(struct netdev *, struct match *, struct nlattr **actions,
> +struct dpif_flow_stats *, const ovs_u128 *ufid,
> +struct ofpbuf *);
> +/* Deletes the given flow specified by ufid from netdev.
> + * If stats is not null, fills it with flow stats. */
> +int (*flow_del)(struct netdev *, const ovs_u128 *ufid,
> +struct dpif_flow_stats *);
> +/* Initializies the netdev flow api. */
> +int (*init_flow_api)(struct netdev *);
>  };

Hopefully you're getting a sense for the set of questions I'm asking
around how you describe this API, and can apply that feedback to the
rest of these as well.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovs V7 09/24] netdev-tc-offloads: Add ufid to tc/netdev map

[Joe Stringer]

On 7 April 2017 at 06:12, Roi Dayan  wrote:
> From: Paul Blakey 
>
> Flows offloaded to tc are identified by priority
> and handle pair while OVS flows are identified by ufid.
> Added a hash map to convert between the two for later
> retrieval and deleting of offloaded flows.
>
> Signed-off-by: Paul Blakey 
> Reviewed-by: Roi Dayan 
> Reviewed-by: Simon Horman 
> ---



> +/* Add ufid to ufid_tc hashmap and prio/handle/ifindex to tc_ufid hashmap.
> + * If those exists already they will be replaced. */
> +static void
> +add_ufid_tc_mapping(const ovs_u128 *ufid, int prio, int handle,
> +struct netdev *netdev, int ifindex)
> +{
> +size_t hash = hash_bytes(ufid, sizeof *ufid, 0);
> +size_t hash2 = hash_int(hash_int(prio, handle), ifindex);
> +struct ufid_to_tc_data *new_data = xzalloc(sizeof *new_data);
> +struct ufid_to_tc_data *new_data2 = xzalloc(sizeof *new_data2);

Is there a particular reason for duplicating this twice? Can we just
have two hmap nodes in 'struct ufid_to_tc_data'?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovs V7 08/24] dpif-netlink: Dump netdevs flows on flow dump

[Joe Stringer]

On 7 April 2017 at 06:12, Roi Dayan  wrote:
> From: Paul Blakey 
>
> While dumping flows, dump flows that were offloaded to
> netdev and parse them back to dpif flow.
>
> Signed-off-by: Paul Blakey 
> Reviewed-by: Roi Dayan 
> Reviewed-by: Simon Horman 
> ---
>  lib/dpif-netlink.c | 179 
> -
>  lib/netdev.c   |  32 ++
>  2 files changed, 210 insertions(+), 1 deletion(-)
>
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index 02dd6c2..4e3bc8c 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -38,6 +38,7 @@
>  #include "flow.h"
>  #include "fat-rwlock.h"
>  #include "netdev.h"
> +#include "netdev-provider.h"
>  #include "netdev-linux.h"
>  #include "netdev-vport.h"
>  #include "netlink-conntrack.h"
> @@ -55,6 +56,7 @@
>  #include "unaligned.h"
>  #include "util.h"
>  #include "openvswitch/vlog.h"
> +#include "openvswitch/match.h"
>
>  VLOG_DEFINE_THIS_MODULE(dpif_netlink);
>  #ifdef _WIN32
> @@ -72,6 +74,8 @@ enum { MAX_PORTS = USHRT_MAX };
>   * missing if we have old headers. */
>  #define ETH_FLAG_LRO  (1 << 15)/* LRO is enabled */
>
> +#define FLOW_DUMP_MAX_BATCH 50
> +
>  struct dpif_netlink_dp {
>  /* Generic Netlink header. */
>  uint8_t cmd;
> @@ -1373,6 +1377,10 @@ struct dpif_netlink_flow_dump {
>  struct dpif_flow_dump up;
>  struct nl_dump nl_dump;
>  atomic_int status;
> +struct netdev_flow_dump **netdev_dumps;
> +int netdev_dumps_num;/* Number of netdev_flow_dumps 
> */
> +struct ovs_mutex netdev_lock;/* Guards the following. */
> +int netdev_current_dump OVS_GUARDED; /* Shared current dump */
>  };
>
>  static struct dpif_netlink_flow_dump *
> @@ -1381,6 +1389,26 @@ dpif_netlink_flow_dump_cast(struct dpif_flow_dump 
> *dump)
>  return CONTAINER_OF(dump, struct dpif_netlink_flow_dump, up);
>  }
>
> +static void
> +start_netdev_dump(const struct dpif *dpif_,
> +  struct dpif_netlink_flow_dump *dump)
> +{
> +ovs_mutex_init(>netdev_lock);
> +
> +if (!netdev_flow_api_enabled) {
> +dump->netdev_dumps_num = 0;
> +dump->netdev_dumps = NULL;
> +return;
> +}
> +
> +ovs_mutex_lock(>netdev_lock);
> +dump->netdev_current_dump = 0;
> +dump->netdev_dumps
> += netdev_ports_flow_dumps_create(DPIF_HMAP_KEY(dpif_),
> + >netdev_dumps_num);
> +ovs_mutex_unlock(>netdev_lock);
> +}
> +
>  static struct dpif_flow_dump *
>  dpif_netlink_flow_dump_create(const struct dpif *dpif_, bool terse)
>  {
> @@ -1405,6 +1433,8 @@ dpif_netlink_flow_dump_create(const struct dpif *dpif_, 
> bool terse)
>  atomic_init(>status, 0);
>  dump->up.terse = terse;
>
> +start_netdev_dump(dpif_, dump);
> +
>  return >up;
>  }
>
> @@ -1415,6 +1445,16 @@ dpif_netlink_flow_dump_destroy(struct dpif_flow_dump 
> *dump_)
>  unsigned int nl_status = nl_dump_done(>nl_dump);
>  int dump_status;
>
> +for (int i = 0; i < dump->netdev_dumps_num; i++) {
> +int err = netdev_flow_dump_destroy(dump->netdev_dumps[i]);
> +if (err != 0 && err != EOPNOTSUPP) {
> +VLOG_ERR("failed dumping netdev: %s", ovs_strerror(err));
> +}
> +}
> +
> +free(dump->netdev_dumps);
> +ovs_mutex_destroy(>netdev_lock);
> +
>  /* No other thread has access to 'dump' at this point. */
>  atomic_read_relaxed(>status, _status);
>  free(dump);
> @@ -1428,6 +1468,13 @@ struct dpif_netlink_flow_dump_thread {
>  struct dpif_flow_stats stats;
>  struct ofpbuf nl_flows; /* Always used to store flows. */
>  struct ofpbuf *nl_actions;  /* Used if kernel does not supply actions. */
> +int netdev_dump_idx;/* This thread current netdev dump index */
> +bool netdev_done;   /* If we are finished dumping netdevs */
> +
> +/* (Key/Mask/Actions) Buffers for netdev dumping */
> +struct odputil_keybuf keybuf[FLOW_DUMP_MAX_BATCH];
> +struct odputil_keybuf maskbuf[FLOW_DUMP_MAX_BATCH];
> +struct odputil_keybuf actbuf[FLOW_DUMP_MAX_BATCH];
>  };
>
>  static struct dpif_netlink_flow_dump_thread *
> @@ -1447,6 +1494,8 @@ dpif_netlink_flow_dump_thread_create(struct 
> dpif_flow_dump *dump_)
>  thread->dump = dump;
>  ofpbuf_init(>nl_flows, NL_DUMP_BUFSIZE);
>  thread->nl_actions = NULL;
> +thread->netdev_dump_idx = 0;
> +thread->netdev_done = !(thread->netdev_dump_idx < 
> dump->netdev_dumps_num);
>
>  return >up;
>  }
> @@ -1484,6 +1533,96 @@ dpif_netlink_flow_to_dpif_flow(struct dpif *dpif, 
> struct dpif_flow *dpif_flow,
>  dpif_netlink_flow_get_stats(datapath_flow, _flow->stats);
>  }
>
> +/* The design is such that all threads are working together on the first dump
> + * to the last, in order (at first they all on dump 0).
> + * When the first 

Re: [ovs-dev] [RFC 1/4] doc: Add man page section to documentation guide

[Ben Pfaff]

On Mon, Apr 10, 2017 at 01:12:27PM +0100, Stephen Finucane wrote:
> We also replace 'reST' with the far more common 'rST'.
> 
> Signed-off-by: Stephen Finucane 

Thank you very much.  This is generally good, but I have a few comments.

> @@ -90,6 +90,12 @@ File Names
>  
>  - Use hyphens as space delimiters. For example: ``my-readme-document.rst``
>  
> +  .. note::
> +
> + An exception to this rule is any man pages, which take an trailing 
> number
> + corresponding to the number of arguments required. This number is 
> preceded
> + by an underscore.

I don't understand this.  What's an example?  Maybe you are talking
about the manpage section number, but that's not an argument count.

> +  Additional sections are allowed. Refer to `man-pages(8)` for information on
> +  the sections generally allowed.

On my (Debian) system, this is in man-pages(7); is this in section 8 on
other distros?

Thanks,

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

Re: [ovs-dev] [RFC 0/4] Introduce Sphinx for man pages

[Ben Pfaff]

On Mon, Apr 10, 2017 at 01:12:26PM +0100, Stephen Finucane wrote:
> This series introduces the use of Sphinx for building man pages. There are a
> couple of reasons for doing this:
> 
> - roff is ruff to write
> 
>   Sorry. roff is an old markup format that's mostly used for man pages today.
>   While certainly not impossible to parse, it isn't very pleasant to either
>   read or write (as evidenced by the XML-roff toolchain used for the OVN
>   sources). Sphinx/rST isn't perfect but it's certainly easier on the eyes.
> 
> - Sphinx/rST über alles
> 
>   We already use Sphinx for every other bit of documentation in the toolchain.
>   Why force people to learn a second syntax for man pages?
> 
> - Integration with other documentation
> 
>   By building with Sphinx, we can do basic things like output the man pages as
>   part of the documentation along with more advanced things like
>   cross-referencing applications from elesewhere in the docs.
> 
> This series begins work on converting the docs, starting with two small
> utilities: ovs-test and ovs-vlan-test. We can use these to tease out any 
> issues
> we might have with the idea before expanding this to more complex man pages
> (ovs-vsctl, I'm looking at you). The eventual goal would be to move all man
> pages to rST/Sphinx.
> ---
> I meant to send this months ago, but I completely forgot about it. It's
> unfinished, as evidenced by the commit footers. However, I think with a little
> help from the maintainers of the packaging in OVS, it will be easy push this
> over the line and move onto the bigger, more important man pages.

This is great.  I've spent a few hours getting it better integrated into
the build and install.  I'm going to send out a revised version of the
series in a bit, but I also have some questions and comments that I'll
send on individual patches.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 2/2] dpif: Log packet metadata on execute.

[Jarno Rajahalme]

Debug log output for execute operations is missing the packet
metadata, which can be instrumental in tracing what the datapath
should be executing.  No reason to have the metadata on the debug
output, so add it there.

Signed-off-by: Jarno Rajahalme 
---
 lib/dpif.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/lib/dpif.c b/lib/dpif.c
index 1760de8..4066f9c 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1760,9 +1760,13 @@ log_execute_message(struct dpif *dpif, const struct 
dpif_execute *execute,
 && !execute->probe) {
 struct ds ds = DS_EMPTY_INITIALIZER;
 char *packet;
+uint64_t stub[1024 / 8];
+struct ofpbuf md = OFPBUF_STUB_INITIALIZER(stub);
 
 packet = ofp_packet_to_string(dp_packet_data(execute->packet),
   dp_packet_size(execute->packet));
+odp_key_from_pkt_metadata(, >packet->md);
+
 ds_put_format(, "%s: %sexecute ",
   dpif_name(dpif),
   (subexecute ? "sub-"
@@ -1773,10 +1777,13 @@ log_execute_message(struct dpif *dpif, const struct 
dpif_execute *execute,
 ds_put_format(, " failed (%s)", ovs_strerror(error));
 }
 ds_put_format(, " on packet %s", packet);
+ds_put_format(, " with metadata ");
+odp_flow_format(md.data, md.size, NULL, 0, NULL, , true);
 ds_put_format(, " mtu %d", execute->mtu);
 vlog(_module, error ? VLL_WARN : VLL_DBG, "%s", ds_cstr());
 ds_destroy();
 free(packet);
+ofpbuf_uninit();
 }
 }
 
-- 
2.1.4

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

Re: [ovs-dev] [PATCH 1/5] netlink: extended ACK reporting

[Joe Stringer]

On 13/04/2017 12:24, "Johannes Berg"  wrote:

> On Thu, 2017-04-13 at 16:05 +0200, Nicolas Dichtel wrote:
>
> > Sure. It was just to mention that attribute 0 exists somewhere.
> > The other 0 attribute is OVS_TUNNEL_KEY_ATTR_ID.
>
> That looks like some really awkward hand-grown parsing - with all these
> "struct ovs_len_tbl" looking almost like a policy, but not using that
> code?
>
> Seems like something somebody should take a hard look at and see if it
> can't use more standard infrastructure.
>

I think that OVS was doing some more elaborate validation than most users,
so over time we picked up a bunch of extra parsing code that layers on top
of nla_parse(). I took a look at trying to broaden this and make it useful
to other users a while ago, but when I posted there wasn't much interest
from others on it so I just moved on. Maybe it's about time to pick that
back up.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] OVS performance with Kernel Datapath of Linux upstream vs Linux OVS tree.

[Jarno Rajahalme]

> On Apr 12, 2017, at 9:00 PM, Kapil Adhikesavalu  wrote:
> 
> Hi Jarno,
> 
> That's great! Thanks for the clarification. So, if anything it should only 
> improve the performance when I move to OVS tree kernel module.
> 
> 

Yes, assuming you will be using a recent OVS release rather than an old one.

  Jarno

> On Thu, 13 Apr 2017, 1:05 AM Jarno Rajahalme,  > wrote:
> 
> > On Apr 12, 2017, at 12:49 AM, Kapil Adhikesavalu  > > wrote:
> >
> > Hi,
> >
> > Is there any performance difference with using the OVS kernel Datapath
> > available part of Linux upstream Vs the module built from Linux OVS tree.
> >
> 
> OVS tree kernel module has an Exact Match Cache, which generally improves 
> performance. Upstream linux openvswitch module does not have it.
> 
> > So far i have been using the DP part of the Linux upstream and as NAT
> > feature requires Linux version 4.6, i plan to switch to DP module built
> > from OVS tree.
> >
> > 1. In general is there any performance between using these two ? or would
> > it vary based on the features being in use.
> > 2. I am currently using VXLAN and L2 forwarding + NAT(plan to use it),
> > would like to know if there could be any performance difference expected
> > when i switch from upstream DP to KLM.
> >
> > I didn't any specific mention about performance in FAQ -
> > http://docs.openvswitch.org/en/latest/faq/releases/ 
> >  expect for this
> > statement 'Certain features require kernel support to function or to have
> > reasonable performance.’
> >
> 
> This note relates to using OVS with an older (upstream) kernel module. While 
> we maintain backwards compatibility, newer features perform better with newer 
> kernel module having explicit support for the feature.
> 
>   Jarno
> 
> > Regards
> > Kapil.
> > ___
> > dev mailing list
> > d...@openvswitch.org 
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev 
> > 
> 
> -- 
> Regards
> Kapil
> 

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

Re: [ovs-dev] [PATCH branch-2.7 00/25] Backports for branch-2.7

[Joe Stringer]

On 13 April 2017 at 14:08, Joe Stringer  wrote:
> On 15 March 2017 at 16:31, Jarno Rajahalme  wrote:
>> These are (mostly) datapath backports from master that fix existing
>> features in branch-2.7 and/or make the datapath compilable with later
>> Linux kernel code.
>
> Hi Jarno,
>
> This series was bigger than I expected, based on the general policy of
> backporting fixes (in the least invasive way possible) and not
> introducing new features or unrelated changes to existing branches.
>
> I have a counter-proposal patch series existing in this branch here,
> which strips 15 of the patches out:
>
> https://github.com/joestringer/openvswitch/commits/dev/jarno_ct_27_bp_v1
>
> The nf_ct_delete() backport doesn't have a clear motivation described
> in the commit message so I don't know whether that's appropriate for
> backport.

Yes it does, I just missed it the first time. That patch should also
be dropped from the backport.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH branch-2.7 00/25] Backports for branch-2.7

[Joe Stringer]

On 15 March 2017 at 16:31, Jarno Rajahalme  wrote:
> These are (mostly) datapath backports from master that fix existing
> features in branch-2.7 and/or make the datapath compilable with later
> Linux kernel code.

Hi Jarno,

This series was bigger than I expected, based on the general policy of
backporting fixes (in the least invasive way possible) and not
introducing new features or unrelated changes to existing branches.

I have a counter-proposal patch series existing in this branch here,
which strips 15 of the patches out:

https://github.com/joestringer/openvswitch/commits/dev/jarno_ct_27_bp_v1

The nf_ct_delete() backport doesn't have a clear motivation described
in the commit message so I don't know whether that's appropriate for
backport.

If you consider that master labels inheritance misfunctioning is a
prominent bug for v2.7, here's the same branch with the labels
refactor and inheritance fix for that on top (extra 4 patches); I
didn't try to minimize the size of these changes, eg by dropping some
of the refactoring patches:

https://github.com/joestringer/openvswitch/tree/dev/jarno_ct_27_bp_v1%2Blabels

I also didn't go through and figure out which patches of this series
make sense to backport to v2.6 or earlier.

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

[ovs-dev] [PATCH v3 5/7] dpif-netlink-rtnl: add GRE creation support

[Eric Garver]

Creates GRE devices using rtnetlink and tunnel metadata.

Co-Authored-by: Thadeu Lima de Souza Cascardo 
Signed-off-by: Thadeu Lima de Souza Cascardo 
Signed-off-by: Eric Garver 
---
 lib/dpif-netlink-rtnl.c | 100 +++-
 1 file changed, 99 insertions(+), 1 deletion(-)

diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
index be40cfab393f..73022b5d86d0 100644
--- a/lib/dpif-netlink-rtnl.c
+++ b/lib/dpif-netlink-rtnl.c
@@ -40,6 +40,13 @@
 #define IFLA_VXLAN_COLLECT_METADATA 25
 #endif
 
+#ifndef IFLA_GRE_MAX
+#define IFLA_GRE_MAX 0
+#endif
+#if IFLA_GRE_MAX < 18
+#define IFLA_GRE_COLLECT_METADATA 18
+#endif
+
 static const struct nl_policy rtlink_policy[] = {
 [IFLA_LINKINFO] = { .type = NL_A_NESTED },
 };
@@ -198,6 +205,96 @@ dpif_netlink_rtnl_vxlan_create(struct netdev *netdev)
 return dpif_netlink_rtnl_vxlan_create_kind(netdev, "vxlan");
 }
 
+static int
+dpif_netlink_rtnl_gre_verify(struct netdev *netdev OVS_UNUSED,
+ const char *name, const char *kind)
+{
+struct ifinfomsg *ifmsg;
+struct ofpbuf *reply;
+int err;
+
+static const struct nl_policy gre_policy[] = {
+[IFLA_GRE_COLLECT_METADATA] = { .type = NL_A_FLAG },
+};
+
+err = dpif_netlink_rtnl_getlink(name, );
+
+if (!err) {
+struct nlattr *rtlink[ARRAY_SIZE(rtlink_policy)];
+struct nlattr *linkinfo[ARRAY_SIZE(linkinfo_policy)];
+struct nlattr *gre[ARRAY_SIZE(gre_policy)];
+
+ifmsg = ofpbuf_at(reply, NLMSG_HDRLEN, sizeof *ifmsg);
+if (!nl_policy_parse(reply, NLMSG_HDRLEN + sizeof *ifmsg,
+ rtlink_policy, rtlink, ARRAY_SIZE(rtlink_policy))
+|| !nl_parse_nested(rtlink[IFLA_LINKINFO], linkinfo_policy,
+linkinfo, ARRAY_SIZE(linkinfo_policy))
+|| strcmp(nl_attr_get_string(linkinfo[IFLA_INFO_KIND]), kind)
+|| !nl_parse_nested(linkinfo[IFLA_INFO_DATA], gre_policy, gre,
+ARRAY_SIZE(gre_policy))) {
+err = EINVAL;
+}
+if (!err) {
+if (!nl_attr_get_flag(gre[IFLA_GRE_COLLECT_METADATA])) {
+err = EINVAL;
+}
+}
+ofpbuf_delete(reply);
+}
+
+return err;
+}
+
+static int
+dpif_netlink_rtnl_gre_create_kind(struct netdev *netdev, const char *kind)
+{
+const struct netdev_tunnel_config *tnl_cfg;
+char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
+size_t linkinfo_off, infodata_off;
+struct ifinfomsg *ifinfo;
+struct ofpbuf request;
+const char *name;
+int err;
+
+tnl_cfg = netdev_get_tunnel_config(netdev);
+if (!tnl_cfg) {
+return EINVAL;
+}
+
+name = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf);
+
+ofpbuf_init(, 0);
+nl_msg_put_nlmsghdr(, 0, RTM_NEWLINK,
+NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE);
+ifinfo = ofpbuf_put_zeros(, sizeof(struct ifinfomsg));
+ifinfo->ifi_change = ifinfo->ifi_flags = IFF_UP;
+nl_msg_put_string(, IFLA_IFNAME, name);
+nl_msg_put_u32(, IFLA_MTU, UINT16_MAX);
+linkinfo_off = nl_msg_start_nested(, IFLA_LINKINFO);
+nl_msg_put_string(, IFLA_INFO_KIND, kind);
+infodata_off = nl_msg_start_nested(, IFLA_INFO_DATA);
+
+nl_msg_put_flag(, IFLA_GRE_COLLECT_METADATA);
+
+nl_msg_end_nested(, infodata_off);
+nl_msg_end_nested(, linkinfo_off);
+
+err = nl_transact(NETLINK_ROUTE, , NULL);
+ofpbuf_uninit();
+
+if (!err && (err = dpif_netlink_rtnl_gre_verify(netdev, name, kind))) {
+dpif_netlink_rtnl_destroy(name);
+}
+
+return err;
+}
+
+static int
+dpif_netlink_rtnl_gre_create(struct netdev *netdev)
+{
+return dpif_netlink_rtnl_gre_create_kind(netdev, "gretap");
+}
+
 int
 dpif_netlink_rtnl_port_create(struct netdev *netdev)
 {
@@ -205,6 +302,7 @@ dpif_netlink_rtnl_port_create(struct netdev *netdev)
 case OVS_VPORT_TYPE_VXLAN:
 return dpif_netlink_rtnl_vxlan_create(netdev);
 case OVS_VPORT_TYPE_GRE:
+return dpif_netlink_rtnl_gre_create(netdev);
 case OVS_VPORT_TYPE_GENEVE:
 case OVS_VPORT_TYPE_NETDEV:
 case OVS_VPORT_TYPE_INTERNAL:
@@ -223,8 +321,8 @@ dpif_netlink_rtnl_port_destroy(const char *name, const char 
*type)
 {
 switch (netdev_to_ovs_vport_type(type)) {
 case OVS_VPORT_TYPE_VXLAN:
-return dpif_netlink_rtnl_destroy(name);
 case OVS_VPORT_TYPE_GRE:
+return dpif_netlink_rtnl_destroy(name);
 case OVS_VPORT_TYPE_GENEVE:
 case OVS_VPORT_TYPE_NETDEV:
 case OVS_VPORT_TYPE_INTERNAL:
-- 
2.12.0

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

[ovs-dev] [PATCH v3 2/7] dpif-netlink: break up code that creates compat ports

[Eric Garver]

This breaks up creating compat ports so we can reuse some of the code to
create ports with rtnetlink.

Co-authored-by: Thadeu Lima de Souza Cascardo 
Signed-off-by: Thadeu Lima de Souza Cascardo 
Signed-off-by: Eric Garver 
Acked-by: Joe Stringer 
---
 lib/dpif-netlink.c | 138 ++---
 1 file changed, 79 insertions(+), 59 deletions(-)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index e27524754a05..0aa9722052cf 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -784,10 +784,8 @@ get_vport_type(const struct dpif_netlink_vport *vport)
 }
 
 static enum ovs_vport_type
-netdev_to_ovs_vport_type(const struct netdev *netdev)
+netdev_to_ovs_vport_type(const char *type)
 {
-const char *type = netdev_get_type(netdev);
-
 if (!strcmp(type, "tap") || !strcmp(type, "system")) {
 return OVS_VPORT_TYPE_NETDEV;
 } else if (!strcmp(type, "internal")) {
@@ -808,19 +806,14 @@ netdev_to_ovs_vport_type(const struct netdev *netdev)
 }
 
 static int
-dpif_netlink_port_add__(struct dpif_netlink *dpif, struct netdev *netdev,
+dpif_netlink_port_add__(struct dpif_netlink *dpif, const char *name,
+enum ovs_vport_type type,
+struct ofpbuf *options,
 odp_port_t *port_nop)
 OVS_REQ_WRLOCK(dpif->upcall_lock)
 {
-const struct netdev_tunnel_config *tnl_cfg;
-char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
-const char *name = netdev_vport_get_dpif_port(netdev,
-  namebuf, sizeof namebuf);
-const char *type = netdev_get_type(netdev);
 struct dpif_netlink_vport request, reply;
 struct ofpbuf *buf;
-uint64_t options_stub[64 / 8];
-struct ofpbuf options;
 struct nl_sock **socksp = NULL;
 uint32_t *upcall_pids;
 int error = 0;
@@ -835,17 +828,81 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, struct 
netdev *netdev,
 dpif_netlink_vport_init();
 request.cmd = OVS_VPORT_CMD_NEW;
 request.dp_ifindex = dpif->dp_ifindex;
-request.type = netdev_to_ovs_vport_type(netdev);
-if (request.type == OVS_VPORT_TYPE_UNSPEC) {
+request.type = type;
+request.name = name;
+
+request.port_no = *port_nop;
+upcall_pids = vport_socksp_to_pids(socksp, dpif->n_handlers);
+request.n_upcall_pids = socksp ? dpif->n_handlers : 1;
+request.upcall_pids = upcall_pids;
+
+if (options) {
+request.options = options->data;
+request.options_len = options->size;
+}
+
+error = dpif_netlink_vport_transact(, , );
+if (!error) {
+*port_nop = reply.port_no;
+} else {
+if (error == EBUSY && *port_nop != ODPP_NONE) {
+VLOG_INFO("%s: requested port %"PRIu32" is in use",
+  dpif_name(>dpif), *port_nop);
+}
+
+vport_del_socksp(dpif, socksp);
+goto exit;
+}
+
+if (socksp) {
+error = vport_add_channels(dpif, *port_nop, socksp);
+if (error) {
+VLOG_INFO("%s: could not add channel for port %s",
+  dpif_name(>dpif), name);
+
+/* Delete the port. */
+dpif_netlink_vport_init();
+request.cmd = OVS_VPORT_CMD_DEL;
+request.dp_ifindex = dpif->dp_ifindex;
+request.port_no = *port_nop;
+dpif_netlink_vport_transact(, NULL, NULL);
+vport_del_socksp(dpif, socksp);
+goto exit;
+}
+}
+free(socksp);
+
+exit:
+ofpbuf_delete(buf);
+free(upcall_pids);
+
+return error;
+}
+
+static int
+dpif_netlink_port_add_compat(struct dpif_netlink *dpif, struct netdev *netdev,
+ odp_port_t *port_nop)
+OVS_REQ_WRLOCK(dpif->upcall_lock)
+{
+const struct netdev_tunnel_config *tnl_cfg;
+char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
+const char *type = netdev_get_type(netdev);
+uint64_t options_stub[64 / 8];
+enum ovs_vport_type ovs_type;
+struct ofpbuf options;
+const char *name;
+
+name = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf);
+
+ovs_type = netdev_to_ovs_vport_type(netdev_get_type(netdev));
+if (ovs_type == OVS_VPORT_TYPE_UNSPEC) {
 VLOG_WARN_RL(_rl, "%s: cannot create port `%s' because it has "
  "unsupported type `%s'",
  dpif_name(>dpif), name, type);
-vport_del_socksp(dpif, socksp);
 return EINVAL;
 }
-request.name = name;
 
-if (request.type == OVS_VPORT_TYPE_NETDEV) {
+if (ovs_type == OVS_VPORT_TYPE_NETDEV) {
 #ifdef _WIN32
 /* XXX : Map appropiate Windows handle */
 #else
@@ -854,10 +911,9 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, struct 
netdev *netdev,
 }
 
 #ifdef _WIN32
-if (request.type == OVS_VPORT_TYPE_INTERNAL) {
+if (ovs_type == OVS_VPORT_TYPE_INTERNAL) {
 if 

[ovs-dev] [PATCH v3 7/7] dpif-netlink: Probe for out-of-tree tunnels, decides used interface

[Eric Garver]

On dpif init, probe for whether tunnels are created using in-tree
(upstream linux) or out-of-tree (OVS). This is done by probing for the
existence of "ovs_geneve" via rtnetlink. This is used to determine how
to create the tunnel devices.

For out-of-tree tunnels, only try genetlink/compat.
For in-tree kernel tunnels, try rtnetlink then fallback to genetlink.

Signed-off-by: Eric Garver 
---
 NEWS|  3 +++
 lib/dpif-netlink-rtnl.c | 38 +++
 lib/dpif-netlink-rtnl.h |  8 
 lib/dpif-netlink.c  | 53 ++---
 4 files changed, 86 insertions(+), 16 deletions(-)

diff --git a/NEWS b/NEWS
index 05af97a1f030..1b7df356f3de 100644
--- a/NEWS
+++ b/NEWS
@@ -3,6 +3,9 @@ Post-v2.7.0
- Tunnels:
  * Added support to set packet mark for tunnel endpoint using
`egress_pkt_mark` OVSDB option.
+ * When using Linux kernel datapath tunnels may be created using rtnetlink.
+   This will allow us to take advantage of new tunnel features without
+   having to make changes to the vport modules.
- EMC insertion probability is reduced to 1% and is configurable via
  the new 'other_config:emc-insert-inv-prob' option.
- DPDK:
diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
index 5a3326ea1f63..cc719bc209a3 100644
--- a/lib/dpif-netlink-rtnl.c
+++ b/lib/dpif-netlink-rtnl.c
@@ -448,3 +448,41 @@ dpif_netlink_rtnl_port_destroy(const char *name, const 
char *type)
 }
 return 0;
 }
+
+/**
+ * Probe for whether the modules are out-of-tree (openvswitch) or in-tree
+ * (upstream kernel).
+ *
+ * We probe for "ovs_geneve" via rtnetlink. As long as this returns something
+ * other than EOPNOTSUPP we know that the module in use is the out-of-tree one.
+ * This will be used to determine which netlink interface to use when creating
+ * ports; rtnetlink or compat/genetlink.
+ *
+ * See ovs_tunnels_out_of_tree
+ */
+bool
+dpif_netlink_rtnl_probe_oot_tunnels(void)
+{
+struct netdev *netdev = NULL;
+bool out_of_tree = false;
+int error;
+
+error = netdev_open("ovs-system-probe", "geneve", );
+if (!error) {
+error = dpif_netlink_rtnl_geneve_create_kind(netdev, "ovs_geneve");
+if (error != EOPNOTSUPP) {
+if (!error) {
+char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
+const char *dp_port;
+
+dp_port = netdev_vport_get_dpif_port(netdev, namebuf,
+ sizeof namebuf);
+dpif_netlink_rtnl_destroy(dp_port);
+}
+out_of_tree = true;
+}
+netdev_close(netdev);
+}
+
+return out_of_tree;
+}
diff --git a/lib/dpif-netlink-rtnl.h b/lib/dpif-netlink-rtnl.h
index 952c0d4187e5..5c790e0bc06f 100644
--- a/lib/dpif-netlink-rtnl.h
+++ b/lib/dpif-netlink-rtnl.h
@@ -25,6 +25,8 @@
 int dpif_netlink_rtnl_port_create(struct netdev *netdev);
 int dpif_netlink_rtnl_port_destroy(const char *name, const char *type);
 
+bool dpif_netlink_rtnl_probe_oot_tunnels(void);
+
 #ifndef __linux__
 /* Dummy implementations for non Linux builds. */
 
@@ -41,6 +43,12 @@ dpif_netlink_rtnl_port_destroy(const char *name OVS_UNUSED,
 return EOPNOTSUPP;
 }
 
+static inline bool
+dpif_netlink_rtnl_probe_oot_tunnels(void)
+{
+return true;
+}
+
 #endif
 
 #endif /* DPIF_NETLINK_RTNL_H */
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 2d34a68c57e4..5244d7e24179 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -61,9 +61,6 @@ VLOG_DEFINE_THIS_MODULE(dpif_netlink);
 #ifdef _WIN32
 #include "wmi.h"
 enum { WINDOWS = 1 };
-static int dpif_netlink_port_query__(const struct dpif_netlink *dpif,
- odp_port_t port_no, const char *port_name,
- struct dpif_port *dpif_port);
 #else
 enum { WINDOWS = 0 };
 #endif
@@ -214,6 +211,12 @@ static int ovs_packet_family;
  * Initialized by dpif_netlink_init(). */
 static unsigned int ovs_vport_mcgroup;
 
+/* If true, tunnel devices are created using OVS compat/genetlink.
+ * If false, tunnel devices are created with rtnetlink and using light weight
+ * tunnels. If we fail to create the tunnel the rtnetlink+LWT, then we fallback
+ * to using the compat interface. */
+static bool ovs_tunnels_out_of_tree = true;
+
 static int dpif_netlink_init(void);
 static int open_dpif(const struct dpif_netlink_dp *, struct dpif **);
 static uint32_t dpif_netlink_port_get_pid(const struct dpif *,
@@ -225,6 +228,9 @@ static void dpif_netlink_vport_to_ofpbuf(const struct 
dpif_netlink_vport *,
  struct ofpbuf *);
 static int dpif_netlink_vport_from_ofpbuf(struct dpif_netlink_vport *,
   const struct ofpbuf *);
+static int dpif_netlink_port_query__(const struct dpif_netlink *dpif,
+ odp_port_t port_no, const 

[ovs-dev] [PATCH v3 6/7] dpif-netlink-rtnl: add GENEVE creation support

[Eric Garver]

Creates GENEVE devices using rtnetlink and tunnel metadata.

Co-Authored-by: Thadeu Lima de Souza Cascardo 
Signed-off-by: Thadeu Lima de Souza Cascardo 
Signed-off-by: Eric Garver 
---
 lib/dpif-netlink-rtnl.c | 115 +++-
 1 file changed, 114 insertions(+), 1 deletion(-)

diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
index 73022b5d86d0..5a3326ea1f63 100644
--- a/lib/dpif-netlink-rtnl.c
+++ b/lib/dpif-netlink-rtnl.c
@@ -47,6 +47,15 @@
 #define IFLA_GRE_COLLECT_METADATA 18
 #endif
 
+#ifndef IFLA_GENEVE_MAX
+#define IFLA_GENEVE_MAX 0
+#endif
+#if IFLA_GENEVE_MAX < 10
+#define IFLA_GENEVE_PORT 5
+#define IFLA_GENEVE_COLLECT_METADATA 6
+#define IFLA_GENEVE_UDP_ZERO_CSUM6_RX 10
+#endif
+
 static const struct nl_policy rtlink_policy[] = {
 [IFLA_LINKINFO] = { .type = NL_A_NESTED },
 };
@@ -295,6 +304,109 @@ dpif_netlink_rtnl_gre_create(struct netdev *netdev)
 return dpif_netlink_rtnl_gre_create_kind(netdev, "gretap");
 }
 
+static int
+dpif_netlink_rtnl_geneve_verify(struct netdev *netdev, const char *name,
+ const char *kind)
+{
+const struct netdev_tunnel_config *tnl_cfg;
+struct ifinfomsg *ifmsg;
+struct ofpbuf *reply;
+int err;
+
+static const struct nl_policy geneve_policy[] = {
+[IFLA_GENEVE_COLLECT_METADATA] = { .type = NL_A_FLAG },
+[IFLA_GENEVE_UDP_ZERO_CSUM6_RX] = { .type = NL_A_U8 },
+[IFLA_GENEVE_PORT] = { .type = NL_A_U16 },
+};
+
+tnl_cfg = netdev_get_tunnel_config(netdev);
+if (!tnl_cfg) {
+return EINVAL;
+}
+
+err = dpif_netlink_rtnl_getlink(name, );
+
+if (!err) {
+struct nlattr *rtlink[ARRAY_SIZE(rtlink_policy)];
+struct nlattr *linkinfo[ARRAY_SIZE(linkinfo_policy)];
+struct nlattr *geneve[ARRAY_SIZE(geneve_policy)];
+
+ifmsg = ofpbuf_at(reply, NLMSG_HDRLEN, sizeof *ifmsg);
+if (!nl_policy_parse(reply, NLMSG_HDRLEN + sizeof *ifmsg,
+ rtlink_policy, rtlink, ARRAY_SIZE(rtlink_policy))
+|| !nl_parse_nested(rtlink[IFLA_LINKINFO], linkinfo_policy,
+linkinfo, ARRAY_SIZE(linkinfo_policy))
+|| strcmp(nl_attr_get_string(linkinfo[IFLA_INFO_KIND]), kind)
+|| !nl_parse_nested(linkinfo[IFLA_INFO_DATA], geneve_policy,
+geneve, ARRAY_SIZE(geneve_policy))) {
+err = EINVAL;
+}
+if (!err) {
+if (!nl_attr_get_flag(geneve[IFLA_GENEVE_COLLECT_METADATA])
+|| 1 != nl_attr_get_u8(geneve[IFLA_GENEVE_UDP_ZERO_CSUM6_RX])
+|| (tnl_cfg->dst_port !=
+nl_attr_get_be16(geneve[IFLA_GENEVE_PORT]))) {
+err = EINVAL;
+}
+}
+ofpbuf_delete(reply);
+}
+
+return err;
+}
+
+static int
+dpif_netlink_rtnl_geneve_create_kind(struct netdev *netdev, const char *kind)
+{
+const struct netdev_tunnel_config *tnl_cfg;
+char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
+size_t linkinfo_off, infodata_off;
+struct ifinfomsg *ifinfo;
+struct ofpbuf request;
+const char *name;
+int err;
+
+tnl_cfg = netdev_get_tunnel_config(netdev);
+if (!tnl_cfg) {
+return EINVAL;
+}
+
+name = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf);
+
+ofpbuf_init(, 0);
+nl_msg_put_nlmsghdr(, 0, RTM_NEWLINK,
+NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE);
+ifinfo = ofpbuf_put_zeros(, sizeof(struct ifinfomsg));
+ifinfo->ifi_change = ifinfo->ifi_flags = IFF_UP;
+nl_msg_put_string(, IFLA_IFNAME, name);
+nl_msg_put_u32(, IFLA_MTU, UINT16_MAX);
+linkinfo_off = nl_msg_start_nested(, IFLA_LINKINFO);
+nl_msg_put_string(, IFLA_INFO_KIND, kind);
+infodata_off = nl_msg_start_nested(, IFLA_INFO_DATA);
+
+nl_msg_put_flag(, IFLA_GENEVE_COLLECT_METADATA);
+nl_msg_put_u8(, IFLA_GENEVE_UDP_ZERO_CSUM6_RX, 1);
+nl_msg_put_be16(, IFLA_GENEVE_PORT, tnl_cfg->dst_port);
+
+nl_msg_end_nested(, infodata_off);
+nl_msg_end_nested(, linkinfo_off);
+
+err = nl_transact(NETLINK_ROUTE, , NULL);
+ofpbuf_uninit();
+
+if (!err && (err = dpif_netlink_rtnl_geneve_verify(netdev, name, kind))) {
+dpif_netlink_rtnl_destroy(name);
+}
+
+return err;
+}
+
+static int
+dpif_netlink_rtnl_geneve_create(struct netdev *netdev)
+{
+return dpif_netlink_rtnl_geneve_create_kind(netdev, "geneve");
+}
+
 int
 dpif_netlink_rtnl_port_create(struct netdev *netdev)
 {
@@ -304,6 +416,7 @@ dpif_netlink_rtnl_port_create(struct netdev *netdev)
 case OVS_VPORT_TYPE_GRE:
 return dpif_netlink_rtnl_gre_create(netdev);
 case OVS_VPORT_TYPE_GENEVE:
+return dpif_netlink_rtnl_geneve_create(netdev);
 case OVS_VPORT_TYPE_NETDEV:
 case OVS_VPORT_TYPE_INTERNAL:
 case OVS_VPORT_TYPE_LISP:
@@ -322,8 +435,8 @@ 

[ovs-dev] [PATCH v3 4/7] dpif-netlink-rtnl: add VXLAN creation support

[Eric Garver]

Creates VXLAN devices using rtnetlink and tunnel metadata.

Co-Authored-by: Thadeu Lima de Souza Cascardo 
Signed-off-by: Thadeu Lima de Souza Cascardo 
Signed-off-by: Eric Garver 
---
 lib/dpif-netlink-rtnl.c | 182 +++-
 lib/dpif-netlink-rtnl.h |   3 +-
 2 files changed, 182 insertions(+), 3 deletions(-)

diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
index 1f816feee569..be40cfab393f 100644
--- a/lib/dpif-netlink-rtnl.c
+++ b/lib/dpif-netlink-rtnl.c
@@ -17,14 +17,193 @@
 #include 
 
 #include "dpif-netlink-rtnl.h"
+
+#include 
+#include 
+#include 
+
 #include "dpif-netlink.h"
+#include "netdev-vport.h"
+#include "netlink-socket.h"
+
+/*
+ * On some older systems, these enums are not defined.
+ */
+#ifndef IFLA_VXLAN_MAX
+#define IFLA_VXLAN_MAX 0
+#endif
+#if IFLA_VXLAN_MAX < 25
+#define IFLA_VXLAN_LEARNING 7
+#define IFLA_VXLAN_PORT 15
+#define IFLA_VXLAN_UDP_ZERO_CSUM6_RX 20
+#define IFLA_VXLAN_GBP 23
+#define IFLA_VXLAN_COLLECT_METADATA 25
+#endif
+
+static const struct nl_policy rtlink_policy[] = {
+[IFLA_LINKINFO] = { .type = NL_A_NESTED },
+};
+static const struct nl_policy linkinfo_policy[] = {
+[IFLA_INFO_KIND] = { .type = NL_A_STRING },
+[IFLA_INFO_DATA] = { .type = NL_A_NESTED },
+};
+
+
+static int
+dpif_netlink_rtnl_destroy(const char *name)
+{
+struct ofpbuf request;
+int err;
+
+ofpbuf_init(, 0);
+nl_msg_put_nlmsghdr(, 0, RTM_DELLINK, NLM_F_REQUEST | NLM_F_ACK);
+ofpbuf_put_zeros(, sizeof(struct ifinfomsg));
+nl_msg_put_string(, IFLA_IFNAME, name);
+
+err = nl_transact(NETLINK_ROUTE, , NULL);
+ofpbuf_uninit();
+
+return err;
+}
+
+static int
+dpif_netlink_rtnl_getlink(const char *name, struct ofpbuf **reply)
+{
+struct ofpbuf request;
+int err;
+
+ofpbuf_init(, 0);
+nl_msg_put_nlmsghdr(, 0, RTM_GETLINK, NLM_F_REQUEST);
+ofpbuf_put_zeros(, sizeof(struct ifinfomsg));
+nl_msg_put_string(, IFLA_IFNAME, name);
+
+err = nl_transact(NETLINK_ROUTE, , reply);
+ofpbuf_uninit();
+
+return err;
+}
+
+static int
+dpif_netlink_rtnl_vxlan_verify(struct netdev *netdev, const char *name,
+   const char *kind)
+{
+const struct netdev_tunnel_config *tnl_cfg;
+struct ifinfomsg *ifmsg;
+struct ofpbuf *reply;
+int err;
+
+static const struct nl_policy vxlan_policy[] = {
+[IFLA_VXLAN_COLLECT_METADATA] = { .type = NL_A_U8 },
+[IFLA_VXLAN_LEARNING] = { .type = NL_A_U8 },
+[IFLA_VXLAN_UDP_ZERO_CSUM6_RX] = { .type = NL_A_U8 },
+[IFLA_VXLAN_PORT] = { .type = NL_A_U16 },
+};
+
+tnl_cfg = netdev_get_tunnel_config(netdev);
+if (!tnl_cfg) {
+return EINVAL;
+}
+
+err = dpif_netlink_rtnl_getlink(name, );
+
+if (!err) {
+struct nlattr *rtlink[ARRAY_SIZE(rtlink_policy)];
+struct nlattr *linkinfo[ARRAY_SIZE(linkinfo_policy)];
+struct nlattr *vxlan[ARRAY_SIZE(vxlan_policy)];
+
+ifmsg = ofpbuf_at(reply, NLMSG_HDRLEN, sizeof *ifmsg);
+if (!nl_policy_parse(reply, NLMSG_HDRLEN + sizeof *ifmsg,
+ rtlink_policy, rtlink, ARRAY_SIZE(rtlink_policy))
+|| !nl_parse_nested(rtlink[IFLA_LINKINFO], linkinfo_policy,
+linkinfo, ARRAY_SIZE(linkinfo_policy))
+|| strcmp(nl_attr_get_string(linkinfo[IFLA_INFO_KIND]), kind)
+|| !nl_parse_nested(linkinfo[IFLA_INFO_DATA], vxlan_policy, vxlan,
+ARRAY_SIZE(vxlan_policy))) {
+err = EINVAL;
+}
+if (!err) {
+if (0 != nl_attr_get_u8(vxlan[IFLA_VXLAN_LEARNING])
+|| 1 != nl_attr_get_u8(vxlan[IFLA_VXLAN_COLLECT_METADATA])
+|| 1 != nl_attr_get_u8(vxlan[IFLA_VXLAN_UDP_ZERO_CSUM6_RX])
+|| (tnl_cfg->dst_port !=
+nl_attr_get_be16(vxlan[IFLA_VXLAN_PORT]))) {
+err = EINVAL;
+}
+}
+if (!err) {
+if (tnl_cfg->exts & (1 << OVS_VXLAN_EXT_GBP)
+&& !nl_attr_get_flag(vxlan[IFLA_VXLAN_GBP])) {
+err = EINVAL;
+}
+}
+ofpbuf_delete(reply);
+}
+
+return err;
+}
+
+static int
+dpif_netlink_rtnl_vxlan_create_kind(struct netdev *netdev, const char *kind)
+{
+const struct netdev_tunnel_config *tnl_cfg;
+char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
+size_t linkinfo_off, infodata_off;
+struct ifinfomsg *ifinfo;
+struct ofpbuf request;
+const char *name;
+int err;
+
+tnl_cfg = netdev_get_tunnel_config(netdev);
+if (!tnl_cfg) {
+return EINVAL;
+}
+
+name = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf);
 
+ofpbuf_init(, 0);
+nl_msg_put_nlmsghdr(, 0, RTM_NEWLINK,
+NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE);
+ifinfo = ofpbuf_put_zeros(, 

[ovs-dev] [PATCH v3 1/7] netdev: get device type from vport prefix if it uses one

[Eric Garver]

From: Thadeu Lima de Souza Cascardo 

If the device name uses a vport prefix, then use that vport type.

Since these names are reserved, we can assume this is the right type.

This is important when we are querying the datapath right after vswitch has
started and using the right type will be even more important when we add support
to creating tunnel ports with rtnetlink.

Signed-off-by: Thadeu Lima de Souza Cascardo 
Acked-by: Joe Stringer 
---
 lib/netdev.c | 26 +++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/lib/netdev.c b/lib/netdev.c
index a8d8edad7243..26c413601550 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -288,6 +288,21 @@ netdev_enumerate_types(struct sset *types)
 }
 }
 
+static const char *
+netdev_vport_type_from_name(const char *name)
+{
+struct netdev_registered_class *rc;
+const char *type;
+CMAP_FOR_EACH (rc, cmap_node, _classes) {
+const char *dpif_port = netdev_vport_class_get_dpif_port(rc->class);
+if (dpif_port && !strncmp(name, dpif_port, strlen(dpif_port))) {
+type = rc->class->type;
+return type;
+}
+}
+return NULL;
+}
+
 /* Check that the network device name is not the same as any of the registered
  * vport providers' dpif_port name (dpif_port is NULL if the vport provider
  * does not define it) or the datapath internal port name (e.g. ovs-system).
@@ -1811,9 +1826,14 @@ netdev_get_vports(size_t *size)
 const char *
 netdev_get_type_from_name(const char *name)
 {
-struct netdev *dev = netdev_from_name(name);
-const char *type = dev ? netdev_get_type(dev) : NULL;
-netdev_close(dev);
+struct netdev *dev;
+const char *type;
+type = netdev_vport_type_from_name(name);
+if (type == NULL) {
+dev = netdev_from_name(name);
+type = dev ? netdev_get_type(dev) : NULL;
+netdev_close(dev);
+}
 return type;
 }
 
-- 
2.12.0

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

[ovs-dev] [PATCH v3 0/7] create tunnel devices using rtnetlink interface

[Eric Garver]

This series adds support for the creation of tunnels using the rtnetlink
interface. This will open the possibility for new features and flags on those
vports without the need to change vport compatibility code.

Support for STT and LISP have not been added because these are not upstream yet,
so we don't know how the interface will be like upstream. And there are no
features in the current drivers right now we could make use of.

Note: This work originally started by Thadeu Lima de Souza Cascardo.

Testing:
  - kernel 4.9.22, in-tree datapath
- rtnetlink successfully creates devices
  - kernel 4.2.8, in-tree datapath
- rtnetlink is tried, but fails due to no COLLECT_METADATA support
- genetlink successfully creates devices
  - kernel 4.2.8, out-of-tree datapath
- rtnetlink is not tried
- genetlink successfully creates devices

v3:
  - commonzie code to get port data to verify port
  - eliminate dpif_netlink_rtnl_vxlan_destroy() and alike
  - minor changes for coding style guidelines
  - add ACKs from previous reviews

Eric Garver (6):
  dpif-netlink: break up code that creates compat ports
  dpif-netlink: code to create/destroy tunnel ports via rtnetlink
  dpif-netlink-rtnl: add VXLAN creation support
  dpif-netlink-rtnl: add GRE creation support
  dpif-netlink-rtnl: add GENEVE creation support
  dpif-netlink: Probe for out-of-tree tunnels, decides used interface

Thadeu Lima de Souza Cascardo (1):
  netdev: get device type from vport prefix if it uses one

 NEWS|   3 +
 lib/automake.mk |   3 +
 lib/dpif-netlink-rtnl.c | 488 
 lib/dpif-netlink-rtnl.h |  54 ++
 lib/dpif-netlink.c  | 210 ++---
 lib/dpif-netlink.h  |   2 +
 lib/netdev.c|  26 ++-
 7 files changed, 712 insertions(+), 74 deletions(-)
 create mode 100644 lib/dpif-netlink-rtnl.c
 create mode 100644 lib/dpif-netlink-rtnl.h

-- 
2.12.0

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

Re: [ovs-dev] [PATCH] dpctl: add examples to the manpage

[Joe Stringer]

On 13 April 2017 at 13:30, Aaron Conole  wrote:
> Joe Stringer  writes:
>
>> On 21 March 2017 at 13:32, Aaron Conole  wrote:
>>> Signed-off-by: Aaron Conole 
>>> ---
>>
>> Hi Aaron, thanks for the patch. Feedback below, I can apply if you're
>> happy with it.
>
> LGTM, thanks!

Thanks, applied to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] doc: Fix broken formatting in releases FAQ

[Ben Pfaff]

Thank you!  I applied this to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] docs: Automatically extract version from NEWS

[Stephen Finucane]

On Thu, 2017-04-13 at 11:22 -0700, Ben Pfaff wrote:
> On Wed, Apr 12, 2017 at 12:32:24PM +0100, Stephen Finucane wrote:
> > Parse the version and release from the NEWS file. This looks a bit
> > hacky, but the NEWS file is generally well formatted and should be
> > reliable enough for our purposes.
> > 
> > Signed-off-by: Stephen Finucane 
> > Cc: Russell Bryant 
> > ---
> > I took a look through the 'git history' of NEWS and could spot no
> > other
> > formatting types for headers. Lemme know if I got this wrong
> > though.
> 
> The canonical place to get this is the AC_INIT line in configure.ac:
> AC_INIT(openvswitch, 2.7.90, b...@openvswitch.org)
> e.g. with:
> autom4te -l Autoconf -t 'AC_INIT:$2' configure.ac
> although that does assume that Autoconf is installed and so we can't
> use it unless we make that a (new) requirement.

Yeah, I think Russell's patch effectively covers this. You can ignore
this patch now.

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

Re: [ovs-dev] [PATCH v2] doc: Link to release FAQ from DPDK install guide

[Ben Pfaff]

On Mon, Apr 10, 2017 at 03:15:07PM +0100, Stephen Finucane wrote:
> I wanted to find the mappings of DPDK versions to OVS versions. This was
> a little more difficult than expected. Resolve the issue by linking to
> it from the DPDK install guide.
> 
> Signed-off-by: Stephen Finucane 
> ---
> v2:
> - Add a link to the releases FAQ from the installation section of the
>   index page

Thanks!  I applied this to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] dpctl: add examples to the manpage

[Aaron Conole]

Joe Stringer  writes:

> On 21 March 2017 at 13:32, Aaron Conole  wrote:
>> Signed-off-by: Aaron Conole 
>> ---
>
> Hi Aaron, thanks for the patch. Feedback below, I can apply if you're
> happy with it.

LGTM, thanks!

>>  lib/dpctl.man | 35 +++
>>  1 file changed, 35 insertions(+)
>>
>> diff --git a/lib/dpctl.man b/lib/dpctl.man
>> index 2fcbc94..c5fdaa7 100644
>> --- a/lib/dpctl.man
>> +++ b/lib/dpctl.man
>> @@ -136,7 +136,42 @@ With \fB\-\-clear\fR, \fBmod\-flow\fR zeros out the 
>> flow's
>>  statistics.  The statistics printed if \fB\-s\fR or
>>  \fB\-\-statistics\fR is also specified are those from just before
>>  clearing the statistics.
>> +.IP "NOTE:"
>> +\fIflow\fR and \fIactions\fR do not match the syntax used with the
>> +\fBovs\-ofctl\fR \fBadd\-flow\fR command.
>>  .
>> +.IP "Usage Examples:"
>> +.RS 4
>> +.PP
>> +\fBForward ARP between ports 3 and 4 on datapath myDP\fR
>> +.RS 4
>> +.nf
>> +ovs-dpctl add-flow myDP \\
>> +.
>> +  "in_port(3),eth(),eth_type(0x0806),arp()" 4
>> +.
>> +ovs-dpctl add-flow myDP \\
>> +.
>> +  "in_port(4),eth(),eth_type(0x0806),arp()" 3
>
> It seems that ports 3 and 4 are used here, but 1 and 2 are used in the
> later example.
>
> There's also a bunch of unusual formatting here, things like
> specifically indenting a certain distance using ".RS 4", and a few man
> directives I have no familiarity with.
>
> I tried to make this more consistent with the rest of the manpages in
> OVS, correct indentation, and so on. Here's the incremental:
>
> ---8<---
>
> diff --git a/lib/dpctl.man b/lib/dpctl.man
> index c5fdaa7e8bbd..f7ae311b90cc 100644
> --- a/lib/dpctl.man
> +++ b/lib/dpctl.man
> @@ -136,42 +136,43 @@ With \fB\-\-clear\fR, \fBmod\-flow\fR zeros out the 
> flow's
> statistics.  The statistics printed if \fB\-s\fR or
> \fB\-\-statistics\fR is also specified are those from just before
> clearing the statistics.
> -.IP "NOTE:"
> -\fIflow\fR and \fIactions\fR do not match the syntax used with the
> -\fBovs\-ofctl\fR \fBadd\-flow\fR command.
> +.IP
> +NOTE:
> +\fIflow\fR and \fIactions\fR do not match the syntax used with
> +\fBovs\-ofctl\fR(8)'s \fBadd\-flow\fR command.
> +.
> +.IP
> +\fBUsage Examples\fR
> .
> -.IP "Usage Examples:"
> -.RS 4
> +.RS
> .PP
> -\fBForward ARP between ports 3 and 4 on datapath myDP\fR
> -.RS 4
> -.nf
> +Forward ARP between ports 1 and 2 on datapath myDP:
> +.IP
> ovs-dpctl add-flow myDP \\
> .
> -  "in_port(3),eth(),eth_type(0x0806),arp()" 4
> +  "in_port(1),eth(),eth_type(0x0806),arp()" 2
> .
> +.IP
> ovs-dpctl add-flow myDP \\
> .
> -  "in_port(4),eth(),eth_type(0x0806),arp()" 3
> +  "in_port(2),eth(),eth_type(0x0806),arp()" 1
> .
> -.RE
> -.fi
> .PP
> -\fBForward all IPv4 traffic between two addresses on ports 1 and 2\fR
> -.RS 4
> -.nf
> +Forward all IPv4 traffic between two addresses on ports 1 and 2:
> +.
> +.IP
> ovs-dpctl add-flow myDP \\
> .
>   "in_port(1),eth(),eth_type(0x800),\\
> ipv4(src=172.31.110.4,dst=172.31.110.5)" 2
> .
> +.IP
> ovs-dpctl add-flow myDP \\
> .
>   "in_port(2),eth(),eth_type(0x800),\\
>ipv4(src=172.31.110.5,dst=172.31.110.4)" 1
> .
> .RE
> -.RE
> .TP
> .DO "[\fB\-s\fR | \fB\-\-statistics\fR]" "\*(DX\fBdel\-flow\fR"
> "[\fIdp\fR] \fIflow\fR"
> Deletes the flow from \fIdp\fR's flow table that matches \fIflow\fR.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] dpctl: add examples to the manpage

[Joe Stringer]

On 21 March 2017 at 13:32, Aaron Conole  wrote:
> Signed-off-by: Aaron Conole 
> ---

Hi Aaron, thanks for the patch. Feedback below, I can apply if you're
happy with it.

>  lib/dpctl.man | 35 +++
>  1 file changed, 35 insertions(+)
>
> diff --git a/lib/dpctl.man b/lib/dpctl.man
> index 2fcbc94..c5fdaa7 100644
> --- a/lib/dpctl.man
> +++ b/lib/dpctl.man
> @@ -136,7 +136,42 @@ With \fB\-\-clear\fR, \fBmod\-flow\fR zeros out the 
> flow's
>  statistics.  The statistics printed if \fB\-s\fR or
>  \fB\-\-statistics\fR is also specified are those from just before
>  clearing the statistics.
> +.IP "NOTE:"
> +\fIflow\fR and \fIactions\fR do not match the syntax used with the
> +\fBovs\-ofctl\fR \fBadd\-flow\fR command.
>  .
> +.IP "Usage Examples:"
> +.RS 4
> +.PP
> +\fBForward ARP between ports 3 and 4 on datapath myDP\fR
> +.RS 4
> +.nf
> +ovs-dpctl add-flow myDP \\
> +.
> +  "in_port(3),eth(),eth_type(0x0806),arp()" 4
> +.
> +ovs-dpctl add-flow myDP \\
> +.
> +  "in_port(4),eth(),eth_type(0x0806),arp()" 3

It seems that ports 3 and 4 are used here, but 1 and 2 are used in the
later example.

There's also a bunch of unusual formatting here, things like
specifically indenting a certain distance using ".RS 4", and a few man
directives I have no familiarity with.

I tried to make this more consistent with the rest of the manpages in
OVS, correct indentation, and so on. Here's the incremental:

---8<---

diff --git a/lib/dpctl.man b/lib/dpctl.man
index c5fdaa7e8bbd..f7ae311b90cc 100644
--- a/lib/dpctl.man
+++ b/lib/dpctl.man
@@ -136,42 +136,43 @@ With \fB\-\-clear\fR, \fBmod\-flow\fR zeros out the flow's
statistics.  The statistics printed if \fB\-s\fR or
\fB\-\-statistics\fR is also specified are those from just before
clearing the statistics.
-.IP "NOTE:"
-\fIflow\fR and \fIactions\fR do not match the syntax used with the
-\fBovs\-ofctl\fR \fBadd\-flow\fR command.
+.IP
+NOTE:
+\fIflow\fR and \fIactions\fR do not match the syntax used with
+\fBovs\-ofctl\fR(8)'s \fBadd\-flow\fR command.
+.
+.IP
+\fBUsage Examples\fR
.
-.IP "Usage Examples:"
-.RS 4
+.RS
.PP
-\fBForward ARP between ports 3 and 4 on datapath myDP\fR
-.RS 4
-.nf
+Forward ARP between ports 1 and 2 on datapath myDP:
+.IP
ovs-dpctl add-flow myDP \\
.
-  "in_port(3),eth(),eth_type(0x0806),arp()" 4
+  "in_port(1),eth(),eth_type(0x0806),arp()" 2
.
+.IP
ovs-dpctl add-flow myDP \\
.
-  "in_port(4),eth(),eth_type(0x0806),arp()" 3
+  "in_port(2),eth(),eth_type(0x0806),arp()" 1
.
-.RE
-.fi
.PP
-\fBForward all IPv4 traffic between two addresses on ports 1 and 2\fR
-.RS 4
-.nf
+Forward all IPv4 traffic between two addresses on ports 1 and 2:
+.
+.IP
ovs-dpctl add-flow myDP \\
.
  "in_port(1),eth(),eth_type(0x800),\\
ipv4(src=172.31.110.4,dst=172.31.110.5)" 2
.
+.IP
ovs-dpctl add-flow myDP \\
.
  "in_port(2),eth(),eth_type(0x800),\\
   ipv4(src=172.31.110.5,dst=172.31.110.4)" 1
.
.RE
-.RE
.TP
.DO "[\fB\-s\fR | \fB\-\-statistics\fR]" "\*(DX\fBdel\-flow\fR"
"[\fIdp\fR] \fIflow\fR"
Deletes the flow from \fIdp\fR's flow table that matches \fIflow\fR.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [datapath backport 10/10] datapath: Openvswitch: Refactor sample and recirc actions implementation

[Joe Stringer]

On 6 April 2017 at 17:18, Andy Zhou  wrote:
> Upstream commit:
> Openvswitch: Refactor sample and recirc actions implementation
>
> Added clone_execute() that both the sample and the recirc
> action implementation can use.
>
> Signed-off-by: Andy Zhou 
> Acked-by: Pravin B Shelar 
> Signed-off-by: David S. Miller 
>
> Upstream: bef7f7567a10 ("Openvswitch: Refactor sample and recirc actions 
> implementation")
> Signed-off-by: Andy Zhou 

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

Re: [ovs-dev] [datapath backport 09/10] datapath: openvswitch: Optimize sample action for the clone use cases

[Joe Stringer]

On 6 April 2017 at 17:18, Andy Zhou  wrote:
> Upstream commit:
> openvswitch: Optimize sample action for the clone use cases
>
> With the introduction of open flow 'clone' action, the OVS user space
> can now translate the 'clone' action into kernel datapath 'sample'
> action, with 100% probability, to ensure that the clone semantics,
> which is that the packet seen by the clone action is the same as the
> packet seen by the action after clone, is faithfully carried out
> in the datapath.
>
> While the sample action in the datpath has the matching semantics,
> its implementation is only optimized for its original use.
> Specifically, there are two limitation: First, there is a 3 level of
> nesting restriction, enforced at the flow downloading time. This
> limit turns out to be too restrictive for the 'clone' use case.
> Second, the implementation avoid recursive call only if the sample
> action list has a single userspace action.
>
> The main optimization implemented in this series removes the static
> nesting limit check, instead, implement the run time recursion limit
> check, and recursion avoidance similar to that of the 'recirc' action.
> This optimization solve both #1 and #2 issues above.
>
> One related optimization attempts to avoid copying flow key as
> long as the actions enclosed does not change the flow key. The
> detection is performed only once at the flow downloading time.
>
> Another related optimization is to rewrite the action list
> at flow downloading time in order to save the fast path from parsing
> the sample action list in its original form repeatedly.
>
> Signed-off-by: Andy Zhou 
> Acked-by: Pravin B Shelar 
> Signed-off-by: David S. Miller 
>
> Upstream: 798c166173ff ("openvswitch: Optimize sample action for the clone 
> use cases")
> Signed-off-by: Andy Zhou 
> ---
>  datapath/actions.c| 112 +
>  datapath/datapath.h   |   2 -
>  datapath/flow_netlink.c   | 141 
> +++---
>  datapath/linux/compat/include/linux/openvswitch.h |  15 +++
>  4 files changed, 172 insertions(+), 98 deletions(-)
>
> diff --git a/datapath/actions.c b/datapath/actions.c
> index f300307b422b..32c0c10e7c62 100644
> --- a/datapath/actions.c
> +++ b/datapath/actions.c
> @@ -43,6 +43,11 @@
>  #include "gso.h"
>  #include "vport.h"
>
> +/* U32_MAX was introduced in include/linux/kernel.h after version 3.14. */
> +#ifndef U32_MAX
> +#define U32_MAX((u32)~0U)
> +#endif
> +

I think this hunk should belong in the compat code. We already have
datapath/linux/compat/include/linux/kernel.h, so it can go there.

Other than that:

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

[ovs-dev] [PATCH v2] windows: Crash when the handle communication device cannot be found

[Alin Serdean]

From: Alin Serdean 

When trying to uninstall/disable the OVS extension the driver will
fail to unload properly(require reboot)/hang until ovs-vswitchd is closed.

The root cause of this behavior is because the handles from ovs-vswitchd
to the kernel communication devices are still opened although the
actual device was removed from the kernel.

Trying to close the handles will also fail because they do not exist.

The remaining option is to cause a crash and rely on the service manager
to restart ovs-vswitchd.

Reported-at: https://github.com/openvswitch/ovs-issues/issues/27
Reported-by: Alin Gabriel Serdean 
Signed-off-by: Alin Gabriel Serdean 
---
v2: Change ovs_fatal to ovs_abort as suggested by Ben Pfaff 
---
 lib/netlink-socket.c | 21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/lib/netlink-socket.c b/lib/netlink-socket.c
index 562f0c0..ccfd55e 100644
--- a/lib/netlink-socket.c
+++ b/lib/netlink-socket.c
@@ -62,6 +62,20 @@ static int get_sock_pid_from_kernel(struct nl_sock *sock);
 static int set_sock_property(struct nl_sock *sock);
 static int nl_sock_transact(struct nl_sock *sock, const struct ofpbuf *request,
 struct ofpbuf **replyp);
+
+/* In the case DeviceIoControl failed and GetLastError returns with
+ * ERROR_NOT_FOUND means we lost communication with the kernel device.
+ * CloseHandle will fail because the handle in 'theory' does not exist.
+ * The only remaining option is to crash and allow the service to be restarted
+ * via service manager.  This is the only way to close the handle from both
+ * userspace and kernel. */
+void
+lost_communication(DWORD last_err)
+{
+if (last_err == ERROR_NOT_FOUND) {
+ovs_abort(0, "lost communication with the kernel device");
+}
+}
 #endif
 
 /* Netlink sockets. */
@@ -280,6 +294,7 @@ get_sock_pid_from_kernel(struct nl_sock *sock)
 if (!DeviceIoControl(sock->handle, OVS_IOCTL_GET_PID,
  NULL, 0, , sizeof(pid),
  , NULL)) {
+lost_communication(GetLastError());
 retval = EINVAL;
 } else {
 if (bytes < sizeof(pid)) {
@@ -537,11 +552,12 @@ nl_sock_send__(struct nl_sock *sock, const struct ofpbuf 
*msg,
 if (!DeviceIoControl(sock->handle, OVS_IOCTL_WRITE,
  msg->data, msg->size, NULL, 0,
  , NULL)) {
+lost_communication(GetLastError());
 retval = -1;
 /* XXX: Map to a more appropriate error based on GetLastError(). */
 errno = EINVAL;
 VLOG_DBG_RL(, "fatal driver failure in write: %s",
-ovs_lasterror_to_string());
+ovs_lasterror_to_string());
 } else {
 retval = msg->size;
 }
@@ -631,6 +647,7 @@ nl_sock_recv__(struct nl_sock *sock, struct ofpbuf *buf, 
bool wait)
 DWORD bytes;
 if (!DeviceIoControl(sock->handle, sock->read_ioctl,
  NULL, 0, tail, sizeof tail, , NULL)) {
+lost_communication(GetLastError());
 VLOG_DBG_RL(, "fatal driver failure in transact: %s",
 ovs_lasterror_to_string());
 retval = -1;
@@ -881,6 +898,7 @@ nl_sock_transact_multiple__(struct nl_sock *sock,
 }
 } else if (!ret) {
 /* XXX: Map to a more appropriate error. */
+lost_communication(GetLastError());
 error = EINVAL;
 VLOG_DBG_RL(, "fatal driver failure: %s",
 ovs_lasterror_to_string());
@@ -1278,6 +1296,7 @@ pend_io_request(struct nl_sock *sock)
 error = GetLastError();
 /* Check if the I/O got pended */
 if (error != ERROR_IO_INCOMPLETE && error != ERROR_IO_PENDING) {
+lost_communication(error);
 VLOG_ERR("nl_sock_wait failed - %s\n", ovs_format_message(error));
 retval = EINVAL;
 }
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [datapath backport 01/10] dapatah: Fix missing CONFIG_NF_CONNTRACK_LABLES check

[Andy Zhou]

On Thu, Apr 13, 2017 at 11:10 AM, Joe Stringer  wrote:
> On 6 April 2017 at 17:18, Andy Zhou  wrote:
>> This config flag was not consistently checked. Fix it.
>>
>> Signed-off-by: Andy Zhou 
>
> nf_ct_labels_find() already changes its implementation based on
> CONFIG_NF_CONNTRACK_LABELS so the OVS code doesn't have to become more
> complicated by handling this case in the function. AFAICT this has no
> visible change in behaviour. Did you find a problem compiling this on
> a particular kernel or config?

ON a 3.10 kernel, It complained about undefined NF_CT_LABELS_MAX_SIZE
when CONFIG_NF_CONNTRACK_LABLES is not defined.

>
> Furthermore, there's no corresponding change upstream, and new
> development should go onto net-next first. This would make the OVS
> tree version diverge from upstream, not converge on it.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [datapath backport 03/10] compat: ipv6: orphan skbs in reassembly unit.

[Andy Zhou]

On Thu, Apr 13, 2017 at 11:31 AM, Joe Stringer  wrote:
> On 6 April 2017 at 17:18, Andy Zhou  wrote:
>> From: Eric Dumazet 
>>
>> Upstream commit:
>> ipv6: orphan skbs in reassembly unit
>>
>> Andrey reported a use-after-free in IPv6 stack.
>>
>> Issue here is that we free the socket while it still has skb
>> in TX path and in some queues.
>>
>> It happens here because IPv6 reassembly unit messes skb->truesize,
>> breaking skb_set_owner_w() badly.
>>
>> We fixed a similar issue for IPV4 in commit 8282f27449bf ("inet: frag:
>> Always orphan skbs inside ip_defrag()")
>> Acked-by: Joe Stringer 
>>
>> ==
>> BUG: KASAN: use-after-free in sock_wfree+0x118/0x120
>> Read of size 8 at addr 880062da0060 by task a.out/4140
>>
>> page:ea00018b6800 count:1 mapcount:0 mapping:  (null)
>> index:0x0 compound_mapcount: 0
>> flags: 0x1008100(slab|head)
>> raw: 01008100   000180130013
>> raw: dead0100 dead0200 88006741f140 
>> page dumped because: kasan: bad access detected
>>
>> CPU: 0 PID: 4140 Comm: a.out Not tainted 4.10.0-rc3+ #59
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 
>> 01/01/2011
>> Call Trace:
>>  __dump_stack lib/dump_stack.c:15
>>  dump_stack+0x292/0x398 lib/dump_stack.c:51
>>  describe_address mm/kasan/report.c:262
>>  kasan_report_error+0x121/0x560 mm/kasan/report.c:370
>>  kasan_report mm/kasan/report.c:392
>>  __asan_report_load8_noabort+0x3e/0x40 mm/kasan/report.c:413
>>  sock_flag ./arch/x86/include/asm/bitops.h:324
>>  sock_wfree+0x118/0x120 net/core/sock.c:1631
>>  skb_release_head_state+0xfc/0x250 net/core/skbuff.c:655
>>  skb_release_all+0x15/0x60 net/core/skbuff.c:668
>>  __kfree_skb+0x15/0x20 net/core/skbuff.c:684
>>  kfree_skb+0x16e/0x4e0 net/core/skbuff.c:705
>>  inet_frag_destroy+0x121/0x290 net/ipv4/inet_fragment.c:304
>>  inet_frag_put ./include/net/inet_frag.h:133
>>  nf_ct_frag6_gather+0x1125/0x38b0 
>> net/ipv6/netfilter/nf_conntrack_reasm.c:617
>>  ipv6_defrag+0x21b/0x350 net/ipv6/netfilter/nf_defrag_ipv6_hooks.c:68
>>  nf_hook_entry_hookfn ./include/linux/netfilter.h:102
>>  nf_hook_slow+0xc3/0x290 net/netfilter/core.c:310
>>  nf_hook ./include/linux/netfilter.h:212
>>  __ip6_local_out+0x52c/0xaf0 net/ipv6/output_core.c:160
>>  ip6_local_out+0x2d/0x170 net/ipv6/output_core.c:170
>>  ip6_send_skb+0xa1/0x340 net/ipv6/ip6_output.c:1722
>>  ip6_push_pending_frames+0xb3/0xe0 net/ipv6/ip6_output.c:1742
>>  rawv6_push_pending_frames net/ipv6/raw.c:613
>>  rawv6_sendmsg+0x2cff/0x4130 net/ipv6/raw.c:927
>>  inet_sendmsg+0x164/0x5b0 net/ipv4/af_inet.c:744
>>  sock_sendmsg_nosec net/socket.c:635
>>  sock_sendmsg+0xca/0x110 net/socket.c:645
>>  sock_write_iter+0x326/0x620 net/socket.c:848
>>  new_sync_write fs/read_write.c:499
>>  __vfs_write+0x483/0x760 fs/read_write.c:512
>>  vfs_write+0x187/0x530 fs/read_write.c:560
>>  SYSC_write fs/read_write.c:607
>>  SyS_write+0xfb/0x230 fs/read_write.c:599
>>  entry_SYSCALL_64_fastpath+0x1f/0xc2 arch/x86/entry/entry_64.S:203
>> RIP: 0033:0x7ff26e6f5b79
>> RSP: 002b:7ff268e0ed98 EFLAGS: 0206 ORIG_RAX: 0001
>> RAX: ffda RBX: 7ff268e0f9c0 RCX: 7ff26e6f5b79
>> RDX: 0010 RSI: 20f50fe1 RDI: 0003
>> RBP: 7ff26ebc1220 R08:  R09: 
>> R10:  R11: 0206 R12: 
>> R13: 7ff268e0f9c0 R14: 7ff26efec040 R15: 0003
>>
>> The buggy address belongs to the object at 880062da
>>  which belongs to the cache RAWv6 of size 1504
>> The buggy address 880062da0060 is located 96 bytes inside
>>  of 1504-byte region [880062da, 880062da05e0)
>>
>> Freed by task 4113:
>>  save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:57
>>  save_stack+0x43/0xd0 mm/kasan/kasan.c:502
>>  set_track mm/kasan/kasan.c:514
>>  kasan_slab_free+0x73/0xc0 mm/kasan/kasan.c:578
>>  slab_free_hook mm/slub.c:1352
>>  slab_free_freelist_hook mm/slub.c:1374
>>  slab_free mm/slub.c:2951
>>  kmem_cache_free+0xb2/0x2c0 mm/slub.c:2973
>>  sk_prot_free net/core/sock.c:1377
>>  __sk_destruct+0x49c/0x6e0 net/core/sock.c:1452
>>  sk_destruct+0x47/0x80 net/core/sock.c:1460
>>  __sk_free+0x57/0x230 net/core/sock.c:1468
>>  sk_free+0x23/0x30 net/core/sock.c:1479
>>  sock_put ./include/net/sock.h:1638
>>  sk_common_release+0x31e/0x4e0 net/core/sock.c:2782
>>  rawv6_close+0x54/0x80 net/ipv6/raw.c:1214
>>  inet_release+0xed/0x1c0 

Re: [ovs-dev] [PATCH 1/5] dpif-netdev: Skip EMC lookup when EMC is disabled.

[Kevin Traynor]

On 04/13/2017 07:30 PM, Bodireddy, Bhanuprakash wrote:
>> On 04/13/2017 07:11 PM, Kevin Traynor wrote:
>>> On 03/12/2017 05:33 PM, Bhanuprakash Bodireddy wrote:
 Conditional EMC insert patch gives the flexibility to configure the
 probability of flow insertion in to EMC. This also allows an option
 to entirely disable EMC by setting 'emc-insert-inv-prob=0' which can
 be useful at large number of parallel flows.

 This patch skips EMC lookup when EMC is disabled. This is useful to
 avoid wasting CPU cycles and also improve performance considerably.

>>>
>>> LGTM. How much does this improve performance?
> 
> I found  significant performance improvement when testing with few hundred 
> streams.  I remember the improvement was  ~800kpps  with smaller packets. 
> This is for the reason that emc_lookup() invokes expensive memcmp() to 
> compare the netdev_flow_key in EMC and it takes up significant cycles. Longer 
> the 'key', worse the performance.  
> 

I just tested and saw about the same improvement. The memcmp won't
happen if emc-insert-inv-prob=0 but you would still check the EMC for
the key, so it's nice to avoid that.

thanks,
Kevin.


>> Ack for the series,
>> Acked-by: Kevin Traynor 
> 
> Thanks kevin for the review and Acks.
> 
> Regards,
> Bhanuprakash. 
> 

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

Re: [ovs-dev] [datapath backport 08/10] datapath: openvswitch: Refactor recirc key allocation.

[Joe Stringer]

On 6 April 2017 at 17:18, Andy Zhou  wrote:
> Upstream commit:
> openvswitch: Refactor recirc key allocation.
>
> The logic of allocating and copy key for each 'exec_actions_level'
> was specific to execute_recirc(). However, future patches will reuse
> as well.  Refactor the logic into its own function clone_key().
>
> Signed-off-by: Andy Zhou 
> Acked-by: Pravin B Shelar 
> Signed-off-by: David S. Miller 
>
> Upstream: 4572ef52a00b ("openvswitch: Refactor recirc key allocation.")
> Signed-off-by: Andy Zhou 

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

Re: [ovs-dev] [datapath backport 07/10] datapath: openvswitch: Deferred fifo API change.

[Joe Stringer]

On 6 April 2017 at 17:18, Andy Zhou  wrote:
> Upstream commit:
> openvswitch: Deferred fifo API change.
>
> add_deferred_actions() API currently requires actions to be passed in
> as a fully encoded netlink message. So far both 'sample' and 'recirc'
> actions happens to carry actions as fully encoded netlink messages.
> However, this requirement is more restrictive than necessary, future
> patch will need to pass in action lists that are not fully encoded
> by themselves.
>
> Signed-off-by: Andy Zhou 
> Acked-by: Joe Stringer 
> Acked-by: Pravin B Shelar 
> Signed-off-by: David S. Miller 
>
> Upstream: 47c697aa2d07 ("openvswitch: Deferred fifo API change.")
> Signed-off-by: Andy Zhou 

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

Re: [ovs-dev] [datapath backport 06/10] datapath: openvswitch: Add missing case OVS_TUNNEL_KEY_ATTR_PAD

[Joe Stringer]

On 6 April 2017 at 17:18, Andy Zhou  wrote:
> From: Kris Murphy 
>
> openvswitch: Add missing case OVS_TUNNEL_KEY_ATTR_PAD
>
> Added a case for OVS_TUNNEL_KEY_ATTR_PAD to the switch statement
> in ip_tun_from_nlattr in order to prevent the default case
> returning an error.
>
> Fixes: b46f6ded906e ("libnl: nla_put_be64(): align on a 64-bit area")
> Signed-off-by: Kris Murphy 
> Acked-by: Joe Stringer 
> Signed-off-by: David S. Miller 
>
> Upstream: 8f3dbfd79ed9("openvswitch: Add missing case 
> OVS_TUNNEL_KEY_ATTR_PAD")
> Signed-off-by: Andy Zhou 

For OVS tree, it also:

Fixes: f34648187b03 ("datapath: backport: libnl: nla_put_be64(): align
on a 64-bit area")

Acked-by: Joe Stringer 

This affects v2.6+.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [datapath backport 03/10] compat: ipv6: orphan skbs in reassembly unit.

[Joe Stringer]

On 6 April 2017 at 17:18, Andy Zhou  wrote:
> From: Eric Dumazet 
>
> Upstream commit:
> ipv6: orphan skbs in reassembly unit
>
> Andrey reported a use-after-free in IPv6 stack.
>
> Issue here is that we free the socket while it still has skb
> in TX path and in some queues.
>
> It happens here because IPv6 reassembly unit messes skb->truesize,
> breaking skb_set_owner_w() badly.
>
> We fixed a similar issue for IPV4 in commit 8282f27449bf ("inet: frag:
> Always orphan skbs inside ip_defrag()")
> Acked-by: Joe Stringer 
>
> ==
> BUG: KASAN: use-after-free in sock_wfree+0x118/0x120
> Read of size 8 at addr 880062da0060 by task a.out/4140
>
> page:ea00018b6800 count:1 mapcount:0 mapping:  (null)
> index:0x0 compound_mapcount: 0
> flags: 0x1008100(slab|head)
> raw: 01008100   000180130013
> raw: dead0100 dead0200 88006741f140 
> page dumped because: kasan: bad access detected
>
> CPU: 0 PID: 4140 Comm: a.out Not tainted 4.10.0-rc3+ #59
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 
> 01/01/2011
> Call Trace:
>  __dump_stack lib/dump_stack.c:15
>  dump_stack+0x292/0x398 lib/dump_stack.c:51
>  describe_address mm/kasan/report.c:262
>  kasan_report_error+0x121/0x560 mm/kasan/report.c:370
>  kasan_report mm/kasan/report.c:392
>  __asan_report_load8_noabort+0x3e/0x40 mm/kasan/report.c:413
>  sock_flag ./arch/x86/include/asm/bitops.h:324
>  sock_wfree+0x118/0x120 net/core/sock.c:1631
>  skb_release_head_state+0xfc/0x250 net/core/skbuff.c:655
>  skb_release_all+0x15/0x60 net/core/skbuff.c:668
>  __kfree_skb+0x15/0x20 net/core/skbuff.c:684
>  kfree_skb+0x16e/0x4e0 net/core/skbuff.c:705
>  inet_frag_destroy+0x121/0x290 net/ipv4/inet_fragment.c:304
>  inet_frag_put ./include/net/inet_frag.h:133
>  nf_ct_frag6_gather+0x1125/0x38b0 
> net/ipv6/netfilter/nf_conntrack_reasm.c:617
>  ipv6_defrag+0x21b/0x350 net/ipv6/netfilter/nf_defrag_ipv6_hooks.c:68
>  nf_hook_entry_hookfn ./include/linux/netfilter.h:102
>  nf_hook_slow+0xc3/0x290 net/netfilter/core.c:310
>  nf_hook ./include/linux/netfilter.h:212
>  __ip6_local_out+0x52c/0xaf0 net/ipv6/output_core.c:160
>  ip6_local_out+0x2d/0x170 net/ipv6/output_core.c:170
>  ip6_send_skb+0xa1/0x340 net/ipv6/ip6_output.c:1722
>  ip6_push_pending_frames+0xb3/0xe0 net/ipv6/ip6_output.c:1742
>  rawv6_push_pending_frames net/ipv6/raw.c:613
>  rawv6_sendmsg+0x2cff/0x4130 net/ipv6/raw.c:927
>  inet_sendmsg+0x164/0x5b0 net/ipv4/af_inet.c:744
>  sock_sendmsg_nosec net/socket.c:635
>  sock_sendmsg+0xca/0x110 net/socket.c:645
>  sock_write_iter+0x326/0x620 net/socket.c:848
>  new_sync_write fs/read_write.c:499
>  __vfs_write+0x483/0x760 fs/read_write.c:512
>  vfs_write+0x187/0x530 fs/read_write.c:560
>  SYSC_write fs/read_write.c:607
>  SyS_write+0xfb/0x230 fs/read_write.c:599
>  entry_SYSCALL_64_fastpath+0x1f/0xc2 arch/x86/entry/entry_64.S:203
> RIP: 0033:0x7ff26e6f5b79
> RSP: 002b:7ff268e0ed98 EFLAGS: 0206 ORIG_RAX: 0001
> RAX: ffda RBX: 7ff268e0f9c0 RCX: 7ff26e6f5b79
> RDX: 0010 RSI: 20f50fe1 RDI: 0003
> RBP: 7ff26ebc1220 R08:  R09: 
> R10:  R11: 0206 R12: 
> R13: 7ff268e0f9c0 R14: 7ff26efec040 R15: 0003
>
> The buggy address belongs to the object at 880062da
>  which belongs to the cache RAWv6 of size 1504
> The buggy address 880062da0060 is located 96 bytes inside
>  of 1504-byte region [880062da, 880062da05e0)
>
> Freed by task 4113:
>  save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:57
>  save_stack+0x43/0xd0 mm/kasan/kasan.c:502
>  set_track mm/kasan/kasan.c:514
>  kasan_slab_free+0x73/0xc0 mm/kasan/kasan.c:578
>  slab_free_hook mm/slub.c:1352
>  slab_free_freelist_hook mm/slub.c:1374
>  slab_free mm/slub.c:2951
>  kmem_cache_free+0xb2/0x2c0 mm/slub.c:2973
>  sk_prot_free net/core/sock.c:1377
>  __sk_destruct+0x49c/0x6e0 net/core/sock.c:1452
>  sk_destruct+0x47/0x80 net/core/sock.c:1460
>  __sk_free+0x57/0x230 net/core/sock.c:1468
>  sk_free+0x23/0x30 net/core/sock.c:1479
>  sock_put ./include/net/sock.h:1638
>  sk_common_release+0x31e/0x4e0 net/core/sock.c:2782
>  rawv6_close+0x54/0x80 net/ipv6/raw.c:1214
>  inet_release+0xed/0x1c0 net/ipv4/af_inet.c:425
>  inet6_release+0x50/0x70 net/ipv6/af_inet6.c:431
>  sock_release+0x8d/0x1e0 net/socket.c:599
>  sock_close+0x16/0x20 net/socket.c:1063
>  

Re: [ovs-dev] [PATCH 1/5] dpif-netdev: Skip EMC lookup when EMC is disabled.

[Bodireddy, Bhanuprakash]

>On 04/13/2017 07:11 PM, Kevin Traynor wrote:
>> On 03/12/2017 05:33 PM, Bhanuprakash Bodireddy wrote:
>>> Conditional EMC insert patch gives the flexibility to configure the
>>> probability of flow insertion in to EMC. This also allows an option
>>> to entirely disable EMC by setting 'emc-insert-inv-prob=0' which can
>>> be useful at large number of parallel flows.
>>>
>>> This patch skips EMC lookup when EMC is disabled. This is useful to
>>> avoid wasting CPU cycles and also improve performance considerably.
>>>
>>
>> LGTM. How much does this improve performance?

I found  significant performance improvement when testing with few hundred 
streams.  I remember the improvement was  ~800kpps  with smaller packets. This 
is for the reason that emc_lookup() invokes expensive memcmp() to compare the 
netdev_flow_key in EMC and it takes up significant cycles. Longer the 'key', 
worse the performance.  

>Ack for the series,
>Acked-by: Kevin Traynor 

Thanks kevin for the review and Acks.

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

Re: [ovs-dev] [datapath backport 02/10] datapath: Pack struct sw_flow_key.

[Joe Stringer]

On 6 April 2017 at 17:18, Andy Zhou  wrote:
> From: Jarno Rajahalme 
>
> Upstream commit:
> openvswitch: Pack struct sw_flow_key.
>
> struct sw_flow_key has two 16-bit holes. Move the most matched
> conntrack match fields there.  In some typical cases this reduces the
> size of the key that needs to be hashed into half and into one cache
> line.
>
> Signed-off-by: Jarno Rajahalme 
> Acked-by: Joe Stringer 
> Acked-by: Pravin B Shelar 
> Signed-off-by: David S. Miller 
>
> Upstream: 316d4d78cf9b ("openvswitch: Pack struct sw_flow_key.")
> Signed-off-by: Joe Stringer 

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

Re: [ovs-dev] [PATCH] docs: Automatically extract version from NEWS

[Ben Pfaff]

On Wed, Apr 12, 2017 at 12:32:24PM +0100, Stephen Finucane wrote:
> Parse the version and release from the NEWS file. This looks a bit
> hacky, but the NEWS file is generally well formatted and should be
> reliable enough for our purposes.
> 
> Signed-off-by: Stephen Finucane 
> Cc: Russell Bryant 
> ---
> I took a look through the 'git history' of NEWS and could spot no other
> formatting types for headers. Lemme know if I got this wrong though.

The canonical place to get this is the AC_INIT line in configure.ac:
AC_INIT(openvswitch, 2.7.90, b...@openvswitch.org)
e.g. with:
autom4te -l Autoconf -t 'AC_INIT:$2' configure.ac
although that does assume that Autoconf is installed and so we can't
use it unless we make that a (new) requirement.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/5] dpif-netdev: Skip EMC lookup when EMC is disabled.

[Kevin Traynor]

On 04/13/2017 07:11 PM, Kevin Traynor wrote:
> On 03/12/2017 05:33 PM, Bhanuprakash Bodireddy wrote:
>> Conditional EMC insert patch gives the flexibility to configure the
>> probability of flow insertion in to EMC. This also allows an option to
>> entirely disable EMC by setting 'emc-insert-inv-prob=0' which can be
>> useful at large number of parallel flows.
>>
>> This patch skips EMC lookup when EMC is disabled. This is useful to
>> avoid wasting CPU cycles and also improve performance considerably.
>>
> 
> LGTM. How much does this improve performance?
> 

Ack for the series,
Acked-by: Kevin Traynor 

>> Signed-off-by: Bhanuprakash Bodireddy 
>> CC: Ciara Loftus 
>> CC: Georg Schmuecking 
>> ---
>>  lib/dpif-netdev.c | 6 +-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index 7d53a8d..faadedb 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -4517,8 +4517,11 @@ emc_processing(struct dp_netdev_pmd_thread *pmd,
>>  size_t n_missed = 0, n_dropped = 0;
>>  struct dp_packet *packet;
>>  const size_t size = dp_packet_batch_size(packets_);
>> +uint32_t cur_min;
>>  int i;
>>  
>> +atomic_read_relaxed(>dp->emc_insert_min, _min);
>> +
>>  DP_PACKET_BATCH_REFILL_FOR_EACH (i, size, packet, packets_) {
>>  struct dp_netdev_flow *flow;
>>  
>> @@ -4542,7 +4545,8 @@ emc_processing(struct dp_netdev_pmd_thread *pmd,
>>  key->len = 0; /* Not computed yet. */
>>  key->hash = dpif_netdev_packet_get_rss_hash(packet, >mf);
>>  
>> -flow = emc_lookup(flow_cache, key);
>> +/* If EMC is disabled skip emc_lookup */
>> +flow = (cur_min == 0) ? NULL: emc_lookup(flow_cache, key);
>>  if (OVS_LIKELY(flow)) {
>>  dp_netdev_queue_batches(packet, flow, >mf, batches,
>>  n_batches);
>>
> 
> ___
> 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 1/5] dpif-netdev: Skip EMC lookup when EMC is disabled.

[Kevin Traynor]

On 03/12/2017 05:33 PM, Bhanuprakash Bodireddy wrote:
> Conditional EMC insert patch gives the flexibility to configure the
> probability of flow insertion in to EMC. This also allows an option to
> entirely disable EMC by setting 'emc-insert-inv-prob=0' which can be
> useful at large number of parallel flows.
> 
> This patch skips EMC lookup when EMC is disabled. This is useful to
> avoid wasting CPU cycles and also improve performance considerably.
> 

LGTM. How much does this improve performance?

> Signed-off-by: Bhanuprakash Bodireddy 
> CC: Ciara Loftus 
> CC: Georg Schmuecking 
> ---
>  lib/dpif-netdev.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 7d53a8d..faadedb 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -4517,8 +4517,11 @@ emc_processing(struct dp_netdev_pmd_thread *pmd,
>  size_t n_missed = 0, n_dropped = 0;
>  struct dp_packet *packet;
>  const size_t size = dp_packet_batch_size(packets_);
> +uint32_t cur_min;
>  int i;
>  
> +atomic_read_relaxed(>dp->emc_insert_min, _min);
> +
>  DP_PACKET_BATCH_REFILL_FOR_EACH (i, size, packet, packets_) {
>  struct dp_netdev_flow *flow;
>  
> @@ -4542,7 +4545,8 @@ emc_processing(struct dp_netdev_pmd_thread *pmd,
>  key->len = 0; /* Not computed yet. */
>  key->hash = dpif_netdev_packet_get_rss_hash(packet, >mf);
>  
> -flow = emc_lookup(flow_cache, key);
> +/* If EMC is disabled skip emc_lookup */
> +flow = (cur_min == 0) ? NULL: emc_lookup(flow_cache, key);
>  if (OVS_LIKELY(flow)) {
>  dp_netdev_queue_batches(packet, flow, >mf, batches,
>  n_batches);
> 

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

Re: [ovs-dev] [datapath backport 01/10] dapatah: Fix missing CONFIG_NF_CONNTRACK_LABLES check

[Joe Stringer]

On 6 April 2017 at 17:18, Andy Zhou  wrote:
> This config flag was not consistently checked. Fix it.
>
> Signed-off-by: Andy Zhou 

nf_ct_labels_find() already changes its implementation based on
CONFIG_NF_CONNTRACK_LABELS so the OVS code doesn't have to become more
complicated by handling this case in the function. AFAICT this has no
visible change in behaviour. Did you find a problem compiling this on
a particular kernel or config?

Furthermore, there's no corresponding change upstream, and new
development should go onto net-next first. This would make the OVS
tree version diverge from upstream, not converge on it.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 2/2] lib/automake.mk: don't install runtime directories

[Aaron Conole]

The Open vSwitch run, log, and DB directories are installed as part of the
normal `make install` process.  However, this means they are created with
user and group ownership that may conflict with the desired user.  For
example, running `make install` as root will install those files as
root:root, whereas the runtime user desired may be openvswitch:openvswitch.

Since these directories are automatically created as part of the ovs-ctl
command, and with the correct user:group permissions, it makes sense to
delay creation until these directories are actually required.

Reviewed-by: Markos Chandras 
Signed-off-by: Aaron Conole 
---
 lib/automake.mk | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/lib/automake.mk b/lib/automake.mk
index 62b2f38..faace79 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -534,10 +534,7 @@ EXTRA_DIST += build-aux/extract-ofp-msgs
 
 INSTALL_DATA_LOCAL += lib-install-data-local
 lib-install-data-local:
-   $(MKDIR_P) $(DESTDIR)$(RUNDIR)
$(MKDIR_P) $(DESTDIR)$(PKIDIR)
-   $(MKDIR_P) $(DESTDIR)$(LOGDIR)
-   $(MKDIR_P) $(DESTDIR)$(DBDIR)
$(MKDIR_P) $(DESTDIR)$(sysconfdir)/openvswitch
 
 man_MANS += lib/ovs-fields.7
-- 
2.9.3

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

[ovs-dev] [PATCH 1/2] install-doc: suggest to use ovs-ctl for start/stop

[Aaron Conole]

The install documentation guided users to manually start/stop
daemons.  This is good information to have, but with the
existence of ovs-ctl, is probably not the best way to start
guiding new users of ovs.

Suggest that users start by running ovs-ctl start, and
document the ability to selectively start/stop the daemons.
The ovs-ctl script is already mentioned a bit in the install
doc, so this just reinforces its use.

Suggested-by: Ben Pfaff 
Signed-off-by: Aaron Conole 
---
 Documentation/intro/install/dpdk.rst|  2 +-
 Documentation/intro/install/general.rst | 36 +
 2 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/Documentation/intro/install/dpdk.rst 
b/Documentation/intro/install/dpdk.rst
index b947bd5..b18a6c1 100644
--- a/Documentation/intro/install/dpdk.rst
+++ b/Documentation/intro/install/dpdk.rst
@@ -196,7 +196,7 @@ the ``dpdk-init`` option must be set to ``true``. For 
example::
 
 $ export DB_SOCK=/usr/local/var/run/openvswitch/db.sock
 $ ovs-vsctl --no-wait set Open_vSwitch . other_config:dpdk-init=true
-$ ovs-vswitchd unix:$DB_SOCK --pidfile --detach
+$ ovs-ctl --no-ovsdb-server --db-sock="$DB_SOCK" start
 
 There are many other configuration options, the most important of which are
 listed below. Defaults will be provided for all values not explicitly set.
diff --git a/Documentation/intro/install/general.rst 
b/Documentation/intro/install/general.rst
index f2cd2b1..f61425a 100644
--- a/Documentation/intro/install/general.rst
+++ b/Documentation/intro/install/general.rst
@@ -422,10 +422,33 @@ Building
 Starting
 
 
-Before starting ovs-vswitchd itself, you need to start its configuration
-database, ovsdb-server. Each machine on which Open vSwitch is installed should
-run its own copy of ovsdb-server. Before ovsdb-server itself can be started,
-configure a database that it can use::
+On unix-alike systems, such as *BSD and Linux, starting the Open vSwitch
+suite of daemons is a simple process. Open vSwitch includes a shell script,
+and helpers, called ovs-ctl which automates much of the tasks for starting
+and stopping ovsdb-server, and ovs-vswitchd. After installation, the daemons
+can be started by using the ovs-ctl utility. This will take care to setup
+initial conditions, and start the daemons in the correct order. An example
+after install might be::
+
+$ ovs-ctl start
+
+Additionally, the ovs-ctl script allows starting / stopping the daemons
+individually using specific options.  To start just the ovsdb-server::
+
+$ ovs-ctl --no-ovs-vswitchd start
+
+Likewise, to start just the ovs-vswitchd::
+
+$ ovs-ctl --no-ovsdb-server start
+
+Refer to ovs-ctl(8) for more information on ovs-ctl.
+
+In addition to using the automated script to start Open vSwitch, you may
+wish to manually start the various daemons. Before starting ovs-vswitchd
+itself, you need to start its configuration database, ovsdb-server. Each
+machine on which Open vSwitch is installed should run its own copy of
+ovsdb-server. Before ovsdb-server itself can be started, configure a
+database that it can use::
 
$ mkdir -p /usr/local/etc/openvswitch
$ ovsdb-tool create /usr/local/etc/openvswitch/conf.db \
@@ -478,6 +501,11 @@ Upgrading
 When you upgrade Open vSwitch from one version to another you should also
 upgrade the database schema:
 
+.. note::
+   The following manual steps may also be accomplished by using ovs-ctl to
+   stop and start the daemons after upgrade.  The ovs-ctl script will
+   automatically upgrade the schema.
+
 1. Stop the Open vSwitch daemons, e.g.::
 
$ kill `cd /usr/local/var/run/openvswitch && cat ovsdb-server.pid 
ovs-vswitchd.pid`
-- 
2.9.3

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

[ovs-dev] [PATCH 0/2] installation: remove the runtime directories

[Aaron Conole]

This series refactors the documentation and then changes
the 'make install' behavior so that it does not automatically
create the runtime directories, because they will have ownership
of root:root, instead of a user-defined option that may be
passed to ovs-ctl.

RFC discussion here:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329600.html

Aaron Conole (2):
  install-doc: suggest to use ovs-ctl for start/stop
  lib/automake.mk: don't install runtime directories

 Documentation/intro/install/dpdk.rst|  2 +-
 Documentation/intro/install/general.rst | 36 +
 lib/automake.mk |  3 ---
 3 files changed, 33 insertions(+), 8 deletions(-)

-- 
2.9.3

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

[ovs-dev] [PATCH] debian, xenserver: Update logrotate config to match RHEL.

[Ben Pfaff]

Commit 618a5b45ae8b ("rhel: Avoid logrotate error if /var/run/openvswitch
does not exist") updated the RHEL logrotate configuration.  This commit
makes similar changes for Debian, by synchronizing with the RHEL version.

In particular:

- Indent to match logrotate.conf(5) examples.

- Use "sharedscripts" flag, because the postrotate script only needs to
  run once regardless of the number of rotations.

- Drop "delaycompress", because the postrotate script does make daemons
  reopen their log files.

- Ignore errors calling vlog/reopen.

Also make similar changes to the xenserver logrotate script.  I really
don't know if anyone uses the xenserver packaging anymore though.

CC: Timothy Redaelli 
Signed-off-by: Ben Pfaff 
---
 debian/openvswitch-switch.logrotate   | 14 +++---
 xenserver/etc_logrotate.d_openvswitch | 22 --
 2 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/debian/openvswitch-switch.logrotate 
b/debian/openvswitch-switch.logrotate
index a7a71bdd90ad..7752af90cfed 100644
--- a/debian/openvswitch-switch.logrotate
+++ b/debian/openvswitch-switch.logrotate
@@ -1,16 +1,16 @@
 /var/log/openvswitch/*.log {
 daily
 compress
+sharedscripts
 create 640 root adm
-delaycompress
 missingok
 rotate 30
 postrotate
-# Tell Open vSwitch daemons to reopen their log files
-if [ -d /var/run/openvswitch ]; then
-for pidfile in `cd /var/run/openvswitch && echo *.pid`; do
-ovs-appctl -t "${pidfile%%.pid}" vlog/reopen
-done
-fi
+   # Tell Open vSwitch daemons to reopen their log files
+   if [ -d /var/run/openvswitch ]; then
+   for pidfile in `cd /var/run/openvswitch && echo *.pid`; do
+   ovs-appctl -t "${pidfile%%.pid}" vlog/reopen 2>/dev/null || :
+   done
+   fi
 endscript
 }
diff --git a/xenserver/etc_logrotate.d_openvswitch 
b/xenserver/etc_logrotate.d_openvswitch
index 73751d4578b0..cd7b3a9d569d 100644
--- a/xenserver/etc_logrotate.d_openvswitch
+++ b/xenserver/etc_logrotate.d_openvswitch
@@ -1,4 +1,4 @@
-# Copyright (C) 2009, 2010, 2011, 2012 Nicira, Inc.
+# Copyright (C) 2009, 2010, 2011, 2012, 2017 Nicira, Inc.
 #
 # Copying and distribution of this file, with or without modification,
 # are permitted in any medium without royalty provided the copyright
@@ -6,14 +6,16 @@
 # without warranty of any kind.
 
 /var/log/openvswitch/*.log {
-   daily
-   compress
-   sharedscripts
-   missingok
-   postrotate
+daily
+compress
+sharedscripts
+missingok
+postrotate
# Tell Open vSwitch daemons to reopen their log files
-for pidfile in `cd /var/run/openvswitch && echo *.pid`; do
-ovs-appctl -t "${pidfile%%.pid}" vlog/reopen
-done
-   endscript
+if [ -d /var/run/openvswitch ]; then
+   for pidfile in `cd /var/run/openvswitch && echo *.pid`; do
+   ovs-appctl -t "${pidfile%%.pid}" vlog/reopen 2>/dev/null || :
+   done
+   fi
+endscript
 }
-- 
2.10.2

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

Re: [ovs-dev] Minutes: OvS Offload Discussion at Netdev 2.1

[Joe Stringer]

On 12 April 2017 at 15:57, Simon Horman  wrote:
> On Wed, Apr 12, 2017 at 03:01:29PM -0300, Flavio Leitner wrote:
>> On Sat, Apr 08, 2017 at 04:47:57PM -0400, Simon Horman wrote:
>> > At Netdev 2.1 a meeting was held to discuss OvS offload.  Minutes of the
>> > discussion follow. I apologise in advance for any errors or omissions;
>> > doubly for any errors in the attendee list.
>> >
>> > Topic: OVS Hardware Offload Using TC
>> > Date: 7th April 2017
>> > Location: Netdev 2.1, Montreal
>> > Attendees: Aaron Conole, Ben LaHaise, Eran Ben Elisha, Hannes Frederic 
>> > Sowa,
>> >   Jakub Kicinski, Jiri Pirko, Joe Stringer, John Fastabend, Nick Viljoen,
>> >   Rashid Khan, Rony Efraim, Simon Horman
>> >
>> > Joe raised 2 concerns:
>> >
>> > 1) How to enable users to understand whether offload is
>> >successful and if not, why not?
>> >
>> >   a) There is functionality in the v7[1] patchset to report which flows
>> >  are present in hardware.
>> >
>> >   b) New error reporting infrastructure from the kernel is forthcoming It
>> >  should allow TC to provide more error information if a flow can't be
>> >  added to hardware. This could be made available to users - e.g. logged
>> >  - to allow them better understand the reason for the failure.
>> >
>> > 2) Maintenance burden falling on existing maintainers
>> >
>> >   a) Simon offered to take some of the maintenance burden
>> >  immediately as he is already a committer.
>> >
>> >   b) The aim is to ensure that in future there are other committers
>> >  who are interested in this feature.
>> >
>> >   There was consensus that if the feature-set does not grow there should be
>> >   discussion of deprecating the HW offload support provided by [1].
>> >
>> > Joe raised issue of whether OVS should probe hardware capabilities at 
>> > runtime.
>> > John suggested this may be complex; potential combinatorial set is too 
>> > large.
>> >
>> > Rony then raised the increased complexity of using multiple NICs of
>> > different types with different offload capabilities, this was tabled to a
>> > later date.
>> >
>> > Joe has expressed a desire for more testing. There was a general agreement
>> > to contribute tests.
>> >
>> > [1] [PATCH ovs V7 00/24] Introducing HW offload support for openvswitch
>>
>> Thanks for the minutes.  I wasn't there so this helps to understand
>> what has been discussed.
>>
>> Have you discussed how we are going to tag/document this feature?  For
>> instance, are we going to say this is "experimental"?
>
> My recollection is that important detail was not discussed at the meeting.

Correct, this did not come up during the meeting but it did come up a
couple of times in other conversations at netdev. There seemed to be
high level acknowledgement that it would be reasonable to document
this feature in some way to indicate it's not ready for widespread
consumption, so I figured that we can discuss the particulars of this
further on the list.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC 0/5] role-based access controls for ovsdb-server, ovn-sb

[Lance Richardson]

> From: "Ben Pfaff" 
> To: "Lance Richardson" 
> Cc: d...@openvswitch.org, "mickeys dev" , "Russell 
> Bryant" 
> Sent: Thursday, 13 April, 2017 12:03:28 PM
> Subject: Re: [RFC 0/5] role-based access controls for ovsdb-server, ovn-sb
> 
> On Wed, Apr 12, 2017 at 09:30:42AM -0400, Lance Richardson wrote:
> > > From: "Lance Richardson" 
> > > To: "Ben Pfaff" 
> > > Cc: d...@openvswitch.org, "mickeys dev" , "Russell
> > > Bryant" 
> > > Sent: Thursday, 6 April, 2017 12:19:46 PM
> > > Subject: Re: [RFC 0/5] role-based access controls for ovsdb-server,
> > > ovn-sb
> > > 
> > > > From: "Ben Pfaff" 
> > > > To: "Lance Richardson" 
> > > > Cc: d...@openvswitch.org, russe...@ovn.org, "mickeys dev"
> > > > 
> > > > Sent: Thursday, 6 April, 2017 12:03:44 PM
> > > > Subject: Re: [RFC 0/5] role-based access controls for ovsdb-server,
> > > > ovn-sb
> > > > 
> > > > On Mon, Mar 27, 2017 at 02:56:08PM -0400, Lance Richardson wrote:
> > > > > This series implements role-based access control infrastructure for
> > > > > ovsdb-server, and uses that infrastructure to apply role-based access
> > > > > controls to the OVN_Southbound database. This implementation follows
> > > > > the outline discussed at:
> > > > > 
> > > > >  
> > > > > https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329801.html
> > > > > 
> > > > > With this series applied, enabling role-based ACLs is a matter of:
> > > > > 
> > > > > - Configuring southbound ovsdb-server and ovn-controller to use
> > > > > SSL,
> > > > >   configuring an ovn-controller "role" for SSL connections via
> > > > >   e.g.:
> > > > >  ovn-sbctl set-connection role=ovn-controller pssl:6642
> > > > > - Using unique certificates for each ovn-controller with a unique
> > > > >   CN for each chassis, generated e.g. via:
> > > > >  ovs-pki -B 1024 req+sign chassis1 switch
> > > > >  ovs-pki -B 1024 req+sign chassis2 switch
> > > > >  ovs-pki -B 1024 req+sign chassis3 switch
> > > > > - Starting the southbound ovsdb-server with the "--rbac"
> > > > > command-line
> > > > >   option:
> > > > >  --rbac=db:OVN_Southbound,RBAC_Role
> > > > 
> > > > This series is promising.
> > > > 
> > > > I'm a little concerned about additional per-DB command-line options
> > > > because it makes it hard to add and remove databases at runtime.
> > > > 
> > 
> > Hi Ben,
> > 
> > Could we extend the database schema format to add something like:
> > 
> > "_rbac_role": 
> > 
> > If so, I think we could eliminate the need to do anything extra for RBAC
> > support as databases are added/removed at runtime, and the --rbac= command-
> > line option would no longer be necessary.
> 
> It's difficult to gracefully extend the database schema format because
> of downgrades: if you start from an upgraded schema and then want to
> downgrade to an older version of the schema, you can only do it with a
> newer ovsdb-tool that understands the new feature, because the older
> version of ovsdb-tool will reject it.  (The schema language could have
> been designed better to avoid this problem in common cases, but I was
> not insightful enough at the time.)  So, except for very important
> features, I prefer to avoid extending the schema format.
> 
> In this case, I'm having trouble seeing the harm if ovsdb-server simply
> treats any table named "RBAC_Role" as the RBAC role table.  I guess that
> there is a risk that someone could start an older ovsdb-server with such
> a schema, expecting that the RBACs are in effect, but actually leaving
> the server fully open to any client.  I don't know whether that is
> something to worry about.
> 

Makes sense, I will make that change in the next revision.

Thanks,

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

Re: [ovs-dev] [PATCH 2/2] rhel: Avoid logrotate error if /var/run/openvswitch does not exist

[Ben Pfaff]

On Thu, Apr 13, 2017 at 11:48:20AM +0200, Timothy Redaelli wrote:
> Avoid also errors if an ovs server didn't start correctly or it crashed 
> without
> deleting the pid file.
> 
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1441524
> Signed-off-by: Timothy Redaelli 

Thanks!  I applied this to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/2] rhel/etc_logrotate.d_openvswitch: Fix coding style

[Ben Pfaff]

On Thu, Apr 13, 2017 at 11:48:19AM +0200, Timothy Redaelli wrote:
> Replace tabs by 4 spaces and indent the postrotate script like the
> examples in 'man logrotate.conf'
> 
> Signed-off-by: Timothy Redaelli 

Thanks!  I applied this to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] Add NAT information to the logical routers in nbctl show output

[Ben Pfaff]

On Thu, Apr 13, 2017 at 10:11:58AM +0100, Lucas Alvares Gomes wrote:
> This patch is changing the print_lr() function in ovn-nbctl.c to include
> logical router NAT information as part of the output (external ip,
> logical ip and type).
> 
> Signed-off-by: Lucas Alvares Gomes 

Thank you for your contribution.  I applied this to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] windows-installer: Update DriverVersion to be streamlined to OVS version

[Alin Serdean]

Patch:
https://github.com/openvswitch/ovs/commit/0c15b76511e78a1f84dec49138d7169c2f3eedf6
introduced a version variable for the MSI itself but did not propagate it
too the driver version (used by the windows certificate tests).

This patch updates the driver version.

Signed-off-by: Alin Gabriel Serdean 
---
 windows/ovs-windows-installer/Product.wxs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/windows/ovs-windows-installer/Product.wxs 
b/windows/ovs-windows-installer/Product.wxs
index 23bc880..599fd43 100644
--- a/windows/ovs-windows-installer/Product.wxs
+++ b/windows/ovs-windows-installer/Product.wxs
@@ -24,7 +24,7 @@
 
 
 
-
+
 
 
 
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] docs: Update version numbers in doc config.

[Ben Pfaff]

On Wed, Apr 12, 2017 at 11:05:44AM -0400, Russell Bryant wrote:
> On Wed, Apr 12, 2017 at 9:12 AM, Stephen Finucane  wrote:
> > I realized that about 10 minutes ago after diving further into the git
> > history of NEWS :) Why '.90' , out of curiosity?
> 
> It predates my involvement, but I think it's roughly:
> 
> 2.7.90 === much more than 2.7.0, but not quite 2.8.0

Yes, that's right.  Other projects do it differently, but this seems to
work OK for OVS, so far.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3] python: Allow tuning the session probe_interval from IDL

[Russell Bryant]

On Tue, Apr 11, 2017 at 11:00 AM, Lucas Alvares Gomes
 wrote:
> This patch is adding a new parameter called "probe_interval" to the
> constructor of the Idl class. This new parameter will be used to tune
> the database connection probing for that IDL session, some users might
> want to tune it to be less agressive than the current 5s default in OVS
> or even disable it.
>
> Reported-at: https://bugs.launchpad.net/networking-ovn/+bug/1680146
> Signed-off-by: Lucas Alvares Gomes 
> ---
>  python/ovs/db/idl.py  | 12 +---
>  python/ovs/jsonrpc.py | 11 +--
>  2 files changed, 18 insertions(+), 5 deletions(-)


Thanks!  I updated this patch to add your name to the AUTHORS file and
applied this to master.

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

Re: [ovs-dev] [RFC 0/5] role-based access controls for ovsdb-server, ovn-sb

[Ben Pfaff]

On Wed, Apr 12, 2017 at 09:30:42AM -0400, Lance Richardson wrote:
> > From: "Lance Richardson" 
> > To: "Ben Pfaff" 
> > Cc: d...@openvswitch.org, "mickeys dev" , "Russell 
> > Bryant" 
> > Sent: Thursday, 6 April, 2017 12:19:46 PM
> > Subject: Re: [RFC 0/5] role-based access controls for ovsdb-server, ovn-sb
> > 
> > > From: "Ben Pfaff" 
> > > To: "Lance Richardson" 
> > > Cc: d...@openvswitch.org, russe...@ovn.org, "mickeys dev"
> > > 
> > > Sent: Thursday, 6 April, 2017 12:03:44 PM
> > > Subject: Re: [RFC 0/5] role-based access controls for ovsdb-server, ovn-sb
> > > 
> > > On Mon, Mar 27, 2017 at 02:56:08PM -0400, Lance Richardson wrote:
> > > > This series implements role-based access control infrastructure for
> > > > ovsdb-server, and uses that infrastructure to apply role-based access
> > > > controls to the OVN_Southbound database. This implementation follows
> > > > the outline discussed at:
> > > > 
> > > >  
> > > > https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329801.html
> > > > 
> > > > With this series applied, enabling role-based ACLs is a matter of:
> > > > 
> > > > - Configuring southbound ovsdb-server and ovn-controller to use SSL,
> > > >   configuring an ovn-controller "role" for SSL connections via e.g.:
> > > >  ovn-sbctl set-connection role=ovn-controller pssl:6642
> > > > - Using unique certificates for each ovn-controller with a unique
> > > >   CN for each chassis, generated e.g. via:
> > > >  ovs-pki -B 1024 req+sign chassis1 switch
> > > >  ovs-pki -B 1024 req+sign chassis2 switch
> > > >  ovs-pki -B 1024 req+sign chassis3 switch
> > > > - Starting the southbound ovsdb-server with the "--rbac" 
> > > > command-line
> > > >   option:
> > > >  --rbac=db:OVN_Southbound,RBAC_Role
> > > 
> > > This series is promising.
> > > 
> > > I'm a little concerned about additional per-DB command-line options
> > > because it makes it hard to add and remove databases at runtime.
> > > 
> 
> Hi Ben,
> 
> Could we extend the database schema format to add something like:
> 
> "_rbac_role": 
> 
> If so, I think we could eliminate the need to do anything extra for RBAC
> support as databases are added/removed at runtime, and the --rbac= command-
> line option would no longer be necessary.

It's difficult to gracefully extend the database schema format because
of downgrades: if you start from an upgraded schema and then want to
downgrade to an older version of the schema, you can only do it with a
newer ovsdb-tool that understands the new feature, because the older
version of ovsdb-tool will reject it.  (The schema language could have
been designed better to avoid this problem in common cases, but I was
not insightful enough at the time.)  So, except for very important
features, I prefer to avoid extending the schema format.

In this case, I'm having trouble seeing the harm if ovsdb-server simply
treats any table named "RBAC_Role" as the RBAC role table.  I guess that
there is a risk that someone could start an older ovsdb-server with such
a schema, expecting that the RBACs are in effect, but actually leaving
the server fully open to any client.  I don't know whether that is
something to worry about.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] OVN meeting report

[Ben Pfaff]

On Wed, Apr 12, 2017 at 06:09:28PM +0500, Valentine Sinitsyn wrote:
> Hi,
> 
> On 04.04.2017 15:29, Valentine Sinitsyn wrote:
> >On 03.04.2017 20:29, Valentine Sinitsyn wrote:
> >>Hi Ben,
> >>
> >>On 23.03.2017 08:11, Ben Pfaff wrote:
> >>>Hello everyone.  I am not sure whether I am going to be able to attend
> >>>the OVN meeting tomorrow, because I will be in another possibly
> >>>distracting meeting, so I'm going to give my report here.
> >>>
> >>>Toward the end of last week I did a full pass of reviews through
> >>>patchwork.  The most notable result, I think, is that I applied patches
> >>>that add 802.1ad support.  For OVN, this makes it more reasonable to
> >>>consider adding support for tagged logical ports--currently, OVN drops
> >>>all tagged logical packets--which I've heard requested once or twice,
> >>>because it means that they can now be gatewayed to physical ports within
> >>>an outer VLAN.  I don't have any plans to work on that, but I think that
> >>>it is worth pointing out.
> >>>
> >>>The OVS "Open Source Day" talks have been scheduled at OpenStack
> >>>Boston.  They are all on Wednesday:
> >>>https://www.openstack.org/summit/boston-2017/summit-schedule/#track=135
> >>>
> >>>I've been spending what dev time I have on database clustering.  Today,
> >>>I managed to get it working, with many caveats.  It will take weeks or
> >>>months longer to get it finished, tested, and ready for posting.  (If
> >>>you want what I have, check out the raft3 branch in my ovs-reviews repo
> >>>at github.)
> >>I've checked out your raft3 branch, and even learned how to create an
> >>OVSDB cluster. Thanks for the docs!
> >>
> >>What I don't get though is how do I instruct IDL to connect to the
> >>cluster now? Do I just connect to a random server, or there should be
> >>some dispatcher, or whatever?
> >OK I see this is an ongoing work in your branch.
> 
> I had some time to play with raft3 branch last week.
> 
> I added very basic and hacky replica set support to IDL and brought up an
> OVN setup with clustered southbound database. It works to some extent, yet
> if I try to throw several hundreds of logical ports into the mix, the
> database becomes inconsistent. The reason is probably the race window
> between when the raft leader appends a log entry to other nodes (so a client
> such as ovn-northd already sees it) and the entry really appears in the
> leader's log itself. Not sure if it is my bug or not. The original code had
> some minor issues as well (which is absolutely normal for WIP) - I can send
> my (rather trivial) patches if there is any interest.

I'm not surprised that there are inconsistency bugs.  The testing I've
done so far is really sketchy.  Let me assure you that I will implement
much more thorough testing before I will propose anything to be merged.

> Is there some design outline for the missing implementation bits?
> Specifically, it would be good to know the following:
> 
> 1. With clustered OVSDB, a client such as IDL needs two JSON RPC
> connections: to the leader (to commit transactions), and a read-only one to
> an arbitrary replica set (scaling reads). Will it be implemented on
> ovsdb_idl level or encapsulated inside jsonrpc_session? The former seems
> natural yet multiple remotes support went to jsonrpc_session already.

There are multiple possible approaches here.  The one that I am planning
to try out first is to have a client connect to only one randomly
selected server, and then have that server be responsible for relaying
write transactions to the leader.

> 2. How does the client know which replica set member is currently a leader?
> I just loop over remotes until one accepts the transaction (which is an
> awful idea). It would be nice to send some sort of cluster metadata snapshot
> to JSON RPC client during initial handshake. Alternatively, one can extend
> the "not leader" error object with a leader URL.

If we do adopt the idea that followers relay write transactions to the
leader, then the client doesn't need to know the leader.  But if that
isn't practical, then the Raft thesis, section 6.2, suggests the same
idea as you did, of having the follower point to the leader if it knows
it.

> 3. For eventual consistency reasons, if an IDL reads from one member (A) but
> writes to another one (B), it can try to delete a row not yet in A's
> database. This would make all further requests fail with "inconsistent data"
> error and basically is what I observe in my tests. How do you plan to
> overcome this?

This sounds like a bug in the existing code (not too surprising).  What
is supposed to happen is that the client waits until it receives updated
data from the server, which it knows will eventually arrive because it
knows that its write was against an inconsistent copy.  Then, it
recomposes its change against the updated database and sends a new
transaction.  This is similar to what the clients already do when their
transactions fail because another client has 

Re: [ovs-dev] [RFC][PATCH] netdev-dpdk: add support for TSO

[Kavanagh, Mark B]

>
>"Kavanagh, Mark B"  writes:
>
>>>Hi Mark,
>>>
>>>Mark Kavanagh  writes:
>>>
 TCP Segmentation Offload (TSO) is a feature which enables
 the TCP/IP network stack to delegate segmentation of a TCP
 segment to the NIC, thus saving compute resources.

 This commit adds support for TSO in the DPDK vHost-User backend,
 to OvS v2.6.1; this enables a guest to offload segmentation of
 TCP segments that it sends to OvS.

 This patch is not intended for upstreaming, but rather was produced
 in response to requests for an updated version of the initial TSO RFC
 patch posted here:
 https://mail.openvswitch.org/pipermail/ovs-dev/2016-June/316414.html

 Signed-off-by: Mark Kavanagh 
 ---
>>>
>>>...
>>>
 diff --git a/lib/packets.c b/lib/packets.c
 index e4c29d5..2417ba2 100644
 --- a/lib/packets.c
 +++ b/lib/packets.c
 @@ -33,6 +33,10 @@
  #include "dp-packet.h"
  #include "unaligned.h"

 +#ifdef DPDK_NETDEV
 +#include "rte_ether.h"
 +#endif
 +
  const struct in6_addr in6addr_exact = IN6ADDR_EXACT_INIT;
  const struct in6_addr in6addr_all_hosts = IN6ADDR_ALL_HOSTS_INIT;

 @@ -204,6 +208,11 @@ eth_push_vlan(struct dp_packet *packet, ovs_be16 
 tpid, ovs_be16 tci)
  memmove(veh, (char *)veh + VLAN_HEADER_LEN, 2 * ETH_ADDR_LEN);
  veh->veth_type = tpid;
  veh->veth_tci = tci & htons(~VLAN_CFI);
 +
 +#ifdef DPDK_NETDEV
 +struct rte_mbuf *pkt = &(packet->mbuf);
 +pkt->l2_len += sizeof(struct vlan_hdr);
 +#endif
  }

  /* Removes outermost VLAN header (if any is present) from 'packet'.
 @@ -221,6 +230,11 @@ eth_pop_vlan(struct dp_packet *packet)
  memmove((char *)veh + VLAN_HEADER_LEN, veh, 2 * ETH_ADDR_LEN);
  dp_packet_resize_l2(packet, -VLAN_HEADER_LEN);
  }
 +
 +#ifdef DPDK_NETDEV
 +struct rte_mbuf *pkt = &(packet->mbuf);
 +pkt->l2_len -= sizeof(struct vlan_hdr);
 +#endif
  }

  /* Set ethertype of the packet. */
>>>
>>>Would it be better to change the dp_packet_resize_l2 call?  Are you
>>
>> Hey Aaron,
>>
>> Good call - that would definitely be a more suitable location.
>>
>>>worried about the mpls case?
>>
>> I haven't considered mpls here at all, as it wasn't part of the use
>> case for which this new version of the patch was produced I'm afraid.
>> Out of curiosity, what is your concern here?
>
>The two users of dp_packet_resize_l2 are the vlan and the mpls code.  I
>had assumed you skipped for that reason, but maybe that isn't the case.

Yeah, I hadn't considered mpls (intentionally); if dp_packet_resize_l2 is 
invoked as part of mpls, then the code is better as-is.

Thanks again,
Mark
>
>> Thanks in advance,
>> Mark
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC v2 0/5] role-based access controls for ovsdb-server, ovn-sb

[Lance Richardson]

> From: "Lance Richardson" 
> To: d...@openvswitch.org, b...@ovn.org, russ...@ovn.org, "mickeys dev" 
> 
> Sent: Thursday, 13 April, 2017 11:00:21 AM
> Subject: [ovs-dev] [RFC v2 0/5] role-based access controls for ovsdb-server,  
> ovn-sb
> 
> This series implements role-based access control infrastructure for
> ovsdb-server, and uses that infrastructure to apply role-based access
> controls to the OVN_Southbound database. This implementation follows
> the outline discussed at:
> 
>  https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329801.html
> 
> With this series applied, enabling role-based ACLs is a matter of:
> 
> - Configuring southbound ovsdb-server and ovn-controller to use SSL,
>   configuring an ovn-controller "role" for SSL connections via e.g.:
>  ovn-sbctl set-connection role=ovn-controller pssl:6642
> - Using unique certificates for each ovn-controller with a unique
>   CN for each chassis, generated e.g. via:
>  ovs-pki -B 1024 req+sign chassis1 switch
>  ovs-pki -B 1024 req+sign chassis2 switch
>  ovs-pki -B 1024 req+sign chassis3 switch
> - Starting the southbound ovsdb-server with the "--rbac" command-line
>   option:
>  --rbac=db:OVN_Southbound,RBAC_Role
> 

Here are the local modifications I've been using to exercise ovn with
rbac enabled in the sandbox environment:

diff --git a/tutorial/ovs-sandbox b/tutorial/ovs-sandbox
  
index 3da1c48..04f46c6 100755
--- a/tutorial/ovs-sandbox
+++ b/tutorial/ovs-sandbox
@@ -337,7 +337,7 @@ if $ovn; then
 $OVS_PKI -B 1024 init
 $OVS_PKI -B 1024 req+sign ovnsb switch
 $OVS_PKI -B 1024 req+sign ovnnb switch
-$OVS_PKI -B 1024 req+sign ovn-controller switch
+$OVS_PKI -B 1024 req+sign chassis-1 switch
 fi
 fi
 rungdb $gdb_ovsdb $gdb_ovsdb_ex ovsdb-server --detach --no-chdir --pidfile 
-vconsole:off --log-file \
@@ -351,7 +351,7 @@ if $ovn; then
 --certificate=db:OVN_Northbound,SSL,certificate \
 --ca-cert=db:OVN_Northbound,SSL,ca_cert \
 --remote=punix:"$sandbox"/ovnnb_db.sock $ovsdb_nb_server_args
-rungdb $gdb_ovsdb $gdb_ovsdb_ex ovsdb-server --detach --no-chdir \
+rungdb $gdb_ovsdb $gdb_ovsdb_ex ovsdb-server --detach --no-chdir 
--rbac=db:OVN_Southbound,RBAC_Role \
 --pidfile="$sandbox"/ovnsb_db.pid -vconsole:off \
 --log-file="$sandbox"/ovnsb_db.log \
 --remote=db:OVN_Southbound,SB_Global,connections \
@@ -395,7 +395,7 @@ if $ovn; then
 ovn-nbctl init
 ovn-sbctl init
 
-ovs-vsctl set open . 
external-ids:system-id=56b18105-5706-46ef-80c4-ff20979ab068
+ovs-vsctl set open . external-ids:system-id=chassis-1
 ovs-vsctl set open . external-ids:hostname=sandbox
 ovs-vsctl set open . external-ids:ovn-encap-type=geneve
 ovs-vsctl set open . external-ids:ovn-encap-ip=127.0.0.1
@@ -404,9 +404,9 @@ if $ovn; then
 ovn-nbctl set-ssl $sandbox/ovnnb-privkey.pem  $sandbox/ovnnb-cert.pem 
$sandbox/pki/switchca/cacert.pem
 ovn-nbctl set-connection pssl:6641
 ovn-sbctl set-ssl $sandbox/ovnsb-privkey.pem  $sandbox/ovnsb-cert.pem 
$sandbox/pki/switchca/cacert.pem
-ovn-sbctl set-connection pssl:6642
+ovn-sbctl set-connection role=ovn-controller pssl:6642
 ovs-vsctl set open . external-ids:ovn-remote=ssl:127.0.0.1:6642
-OVN_CTRLR_PKI="-p $sandbox/ovn-controller-privkey.pem -c 
$sandbox/ovn-controller-cert.pem -C $sandbox/pki/switchca/cacert.pem"
+OVN_CTRLR_PKI="-p $sandbox/chassis-1-privkey.pem -c 
$sandbox/chassis-1-cert.pem -C $sandbox/pki/switchca/cacert.pem"
 else
 ovs-vsctl set open . 
external-ids:ovn-remote=unix:"$sandbox"/ovnsb_db.sock
 OVN_CTRLR_PKI=""
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [RFC v2 3/5] ovsdb: add support for role-based access controls

[Lance Richardson]

Add suport for ovsdb RBAC (role-based access control). This includes:

   - Support for new "--rbac " command-line option to
 ovsdb-server, to specify the RBAC roles table to be used.

 This table has one row per role, with each row having a
 "name" column (role name) and a "permissions" column (map of
 table name to UUID of row in separate permission table.) The
 permission table has one row per access control configuration,
 with columns:
  "name" - name of table to which this row applies
  "authorization" - set of column names and column:key pairs
to be compared against client ID to
determine authorization status
  "insert_delete" - boolean, true if insertions and
authorized deletions are allowed.
  "update"- Set of columns and column:key pairs for
which authorized updates are allowed.
   - Support for a new "role" column in the remote configuration
 table.
   - Logic for applying the RBAC role and permission tables, in
 combination with session role and client id, to determine
 whether operations modifying database contents should be
 permitted.

Signed-off-by: Lance Richardson 
---
v2:
 - Added ovsdb_perm_error() to format permission error strings.
 - Re-implemented RBAC enforcing code to apply RBAC rules
   to entire delete/update/mutate operation before applying
   any changes to avoid partial updates.
 - Implemented basic unit tests for RBAC enforcement.

 lib/jsonrpc.c  |  10 ++
 lib/jsonrpc.h  |   2 +
 lib/ovsdb-error.c  |  13 ++
 lib/ovsdb-error.h  |   4 +
 ovsdb/automake.mk  |   2 +
 ovsdb/execution.c  |  41 -
 ovsdb/jsonrpc-server.c |   6 +-
 ovsdb/jsonrpc-server.h |   1 +
 ovsdb/mutation.c   |   2 +
 ovsdb/mutation.h   |   5 +-
 ovsdb/ovsdb-server.c   |  70 +++-
 ovsdb/ovsdb-tool.c |   2 +-
 ovsdb/ovsdb-util.c |  39 +
 ovsdb/ovsdb-util.h |   3 +
 ovsdb/ovsdb.h  |   1 +
 ovsdb/rbac.c   | 449 +
 ovsdb/rbac.h   |  36 
 ovsdb/trigger.c|   8 +-
 ovsdb/trigger.h|   5 +-
 tests/automake.mk  |   1 +
 tests/ovsdb-rbac.at| 253 
 tests/ovsdb.at |   1 +
 tests/test-ovsdb.c |   5 +-
 23 files changed, 945 insertions(+), 14 deletions(-)
 create mode 100644 ovsdb/rbac.c
 create mode 100644 ovsdb/rbac.h
 create mode 100644 tests/ovsdb-rbac.at

diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c
index a0ade9c..2fae057 100644
--- a/lib/jsonrpc.c
+++ b/lib/jsonrpc.c
@@ -1005,6 +1005,16 @@ jsonrpc_session_get_name(const struct jsonrpc_session *s)
 return reconnect_get_name(s->reconnect);
 }
 
+const char *
+jsonrpc_session_get_id(const struct jsonrpc_session *s)
+{
+if (s->rpc && s->rpc->stream) {
+return stream_get_peer_id(s->rpc->stream);
+} else {
+return NULL;
+}
+}
+
 /* Always takes ownership of 'msg', regardless of success. */
 int
 jsonrpc_session_send(struct jsonrpc_session *s, struct jsonrpc_msg *msg)
diff --git a/lib/jsonrpc.h b/lib/jsonrpc.h
index 982017a..6a82954 100644
--- a/lib/jsonrpc.h
+++ b/lib/jsonrpc.h
@@ -130,5 +130,7 @@ void jsonrpc_session_set_probe_interval(struct 
jsonrpc_session *,
 int probe_interval);
 void jsonrpc_session_set_dscp(struct jsonrpc_session *,
   uint8_t dscp);
+const char *jsonrpc_session_get_id(const struct jsonrpc_session *);
+
 
 #endif /* jsonrpc.h */
diff --git a/lib/ovsdb-error.c b/lib/ovsdb-error.c
index dfa4249..6331668 100644
--- a/lib/ovsdb-error.c
+++ b/lib/ovsdb-error.c
@@ -167,6 +167,19 @@ ovsdb_internal_error(struct ovsdb_error *inner_error,
 return error;
 }
 
+struct ovsdb_error *ovsdb_perm_error(const char *details, ...)
+{
+struct ovsdb_error *error;
+va_list args;
+
+va_start(args, details);
+error = ovsdb_error_valist("permission error", details, args);
+va_end(args);
+
+return error;
+}
+
+
 void
 ovsdb_error_destroy(struct ovsdb_error *error)
 {
diff --git a/lib/ovsdb-error.h b/lib/ovsdb-error.h
index 2bc259a..da91b74 100644
--- a/lib/ovsdb-error.h
+++ b/lib/ovsdb-error.h
@@ -41,6 +41,10 @@ struct ovsdb_error *ovsdb_internal_error(struct ovsdb_error 
*error,
 OVS_PRINTF_FORMAT(4, 5)
 OVS_WARN_UNUSED_RESULT;
 
+struct ovsdb_error *ovsdb_perm_error(const char *details, ...)
+OVS_PRINTF_FORMAT(1, 2)
+OVS_WARN_UNUSED_RESULT;
+
 /* Returns a pointer to an ovsdb_error that represents an internal error for
  * the current file name and line number with MSG as the associated message.
  * The caller is responsible for freeing the internal error. */
diff --git a/ovsdb/automake.mk b/ovsdb/automake.mk
index c218bf5..ac0f741 100644
--- a/ovsdb/automake.mk
+++ b/ovsdb/automake.mk
@@ -24,6 +24,8 @@ 

[ovs-dev] [RFC v2 5/5] ovn-sbctl: support setting rbac role for remote connections

[Lance Richardson]

Add support for specifying rbac "role" when setting remote
connection configuration in southbound database.

Signed-off-by: Lance Richardson 
---
v2: no changes

 ovn/utilities/ovn-sbctl.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c
index ffa931a..bf09ef7 100644
--- a/ovn/utilities/ovn-sbctl.c
+++ b/ovn/utilities/ovn-sbctl.c
@@ -862,6 +862,7 @@ pre_connection(struct ctl_context *ctx)
 ovsdb_idl_add_column(ctx->idl, _sb_global_col_connections);
 ovsdb_idl_add_column(ctx->idl, _connection_col_target);
 ovsdb_idl_add_column(ctx->idl, _connection_col_read_only);
+ovsdb_idl_add_column(ctx->idl, _connection_col_role);
 }
 
 static void
@@ -879,8 +880,10 @@ cmd_get_connection(struct ctl_context *ctx)
 SBREC_CONNECTION_FOR_EACH(conn, ctx->idl) {
 char *s;
 
-s = xasprintf("%s %s", conn->read_only ? "read-only" : "read-write",
-   conn->target);
+s = xasprintf("%s role=\"%s\" %s",
+  conn->read_only ? "read-only" : "read-write",
+  conn->role,
+  conn->target);
 svec_add(, s);
 free(s);
 }
@@ -921,6 +924,7 @@ insert_connections(struct ctl_context *ctx, char 
*targets[], size_t n)
 struct sbrec_connection **connections;
 size_t i, conns=0;
 bool read_only = false;
+char *role = "";
 
 /* Insert each connection in a new row in Connection table. */
 connections = xmalloc(n * sizeof *connections);
@@ -931,6 +935,9 @@ insert_connections(struct ctl_context *ctx, char 
*targets[], size_t n)
 } else if (!strcmp(targets[i], "read-write")) {
 read_only = false;
 continue;
+} else if (!strncmp(targets[i], "role=", 5)) {
+role = targets[i] + 5;
+continue;
 } else if (stream_verify_name(targets[i]) &&
pstream_verify_name(targets[i])) {
 VLOG_WARN("target type \"%s\" is possibly erroneous", targets[i]);
@@ -939,6 +946,7 @@ insert_connections(struct ctl_context *ctx, char 
*targets[], size_t n)
 connections[conns] = sbrec_connection_insert(ctx->txn);
 sbrec_connection_set_target(connections[conns], targets[i]);
 sbrec_connection_set_read_only(connections[conns], read_only);
+sbrec_connection_set_role(connections[conns], role);
 conns++;
 }
 
-- 
2.7.4

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

[ovs-dev] [RFC v2 4/5] ovn: add rbac tables to ovn southbound schema

[Lance Richardson]

Add rbac "roles" and "permissions" tables to ovn southbound
database schema, add support to ovn-northd for managing these
tables.

Signed-off-by: Lance Richardson 
---
v2:
  - Corrected authorization setup for Chassis and Encap tables.

 ovn/northd/ovn-northd.c | 190 
 ovn/ovn-sb.ovsschema|  28 ++-
 ovn/ovn-sb.xml  |  39 ++
 3 files changed, 254 insertions(+), 3 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 5a2e5ab..6bde5f8 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -5502,6 +5502,182 @@ check_and_add_supported_dhcpv6_opts_to_sb_db(struct 
northd_context *ctx)
 hmap_destroy(_opts_to_add);
 }
 
+static const char *rbac_chassis_auth[] =
+{"name"};
+static const char *rbac_chassis_update[] =
+{"nb_cfg", "external_ids", "encaps", "vtep_logical_switches"};
+
+static const char *rbac_encap_auth[] =
+{""};
+static const char *rbac_encap_update[] =
+{"type", "options", "ip"};
+
+static const char *rbac_port_binding_auth[] =
+{""};
+static const char *rbac_port_binding_update[] =
+{"chassis"};
+
+static const char *rbac_mac_binding_auth[] =
+{""};
+static const char *rbac_mac_binding_update[] =
+{"logical_port", "ip", "mac", "datapath"};
+
+static struct rbac_perm_cfg {
+const char *table;
+const char **auth;
+int n_auth;
+bool insdel;
+const char **update;
+int n_update;
+const struct sbrec_rbac_permission *row;
+} rbac_perm_cfg[] = {
+{
+"Chassis",
+rbac_chassis_auth,
+ARRAY_SIZE(rbac_chassis_auth),
+true,
+rbac_chassis_update,
+ARRAY_SIZE(rbac_chassis_update),
+NULL
+},{
+"Encap",
+rbac_encap_auth,
+ARRAY_SIZE(rbac_encap_auth),
+true,
+rbac_encap_update,
+ARRAY_SIZE(rbac_encap_update),
+NULL
+},{
+"Port_Binding",
+rbac_port_binding_auth,
+ARRAY_SIZE(rbac_port_binding_auth),
+false,
+rbac_port_binding_update,
+ARRAY_SIZE(rbac_port_binding_update),
+NULL
+},{
+"MAC_Binding",
+rbac_mac_binding_auth,
+ARRAY_SIZE(rbac_mac_binding_auth),
+true,
+rbac_mac_binding_update,
+ARRAY_SIZE(rbac_mac_binding_update),
+NULL
+},
+{}
+};
+
+static bool
+ovn_rbac_validate_perm(const struct sbrec_rbac_permission *perm)
+{
+struct rbac_perm_cfg *pcfg;
+int i, j, n_found;
+
+for (pcfg = rbac_perm_cfg; pcfg->table; pcfg++) {
+if (!strcmp(perm->table, pcfg->table)) {
+break;
+}
+}
+if (!pcfg->table) {
+return false;
+}
+if (perm->n_authorization != pcfg->n_auth ||
+perm->n_update != pcfg->n_update) {
+return false;
+}
+if (perm->insert_delete != pcfg->insdel) {
+return false;
+}
+/* verify perm->authorization vs. pcfg->auth */
+n_found = 0;
+for (i = 0; i < pcfg->n_auth; i++) {
+for (j = 0; j < perm->n_authorization; j++) {
+if (!strcmp(pcfg->auth[i], perm->authorization[j])) {
+n_found++;
+break;
+}
+}
+}
+if (n_found != pcfg->n_auth) {
+return false;
+}
+
+/* verify perm->update vs. pcfg->update */
+n_found = 0;
+for (i = 0; i < pcfg->n_update; i++) {
+for (j = 0; j < perm->n_update; j++) {
+if (!strcmp(pcfg->update[i], perm->update[j])) {
+n_found++;
+break;
+}
+}
+}
+if (n_found != pcfg->n_update) {
+return false;
+}
+
+/* Success, db state matches expected state */
+pcfg->row = perm;
+return true;
+}
+
+static void
+ovn_rbac_create_perm(struct rbac_perm_cfg *pcfg,
+ struct northd_context *ctx,
+ const struct sbrec_rbac_role *rbac_role)
+{
+struct sbrec_rbac_permission *rbac_perm;
+
+rbac_perm = sbrec_rbac_permission_insert(ctx->ovnsb_txn);
+sbrec_rbac_permission_set_table(rbac_perm, pcfg->table);
+sbrec_rbac_permission_set_authorization(rbac_perm,
+pcfg->auth,
+pcfg->n_auth);
+sbrec_rbac_permission_set_insert_delete(rbac_perm, pcfg->insdel);
+sbrec_rbac_permission_set_update(rbac_perm,
+ pcfg->update,
+ pcfg->n_update);
+sbrec_rbac_role_update_permissions_setkey(rbac_role, pcfg->table,
+  rbac_perm);
+}
+
+static void
+check_and_update_rbac(struct northd_context *ctx)
+{
+const struct sbrec_rbac_role *rbac_role = NULL;
+const struct sbrec_rbac_permission *perm_row, *perm_next;
+const struct sbrec_rbac_role *role_row, *role_row_next;
+struct rbac_perm_cfg *pcfg;
+
+for 

[ovs-dev] [RFC v2 2/5] ovsdb: refactor utility functions into separate file

[Lance Richardson]

Move local db access functions to a new file and make give them
global scope so they can be included in the ovsdb library and used
by other ovsdb library functions.

Signed-off-by: Lance Richardson 
---
v2:
  - Renamed functions in ovsdb-util.c to have "ovsdb_util_" prefix.
  - Included  in ovsdb-util.c.

 ovsdb/automake.mk|   4 +-
 ovsdb/ovsdb-server.c | 187 +++-
 ovsdb/ovsdb-util.c   | 196 +++
 ovsdb/ovsdb-util.h   |  48 +
 4 files changed, 258 insertions(+), 177 deletions(-)
 create mode 100644 ovsdb/ovsdb-util.c
 create mode 100644 ovsdb/ovsdb-util.h

diff --git a/ovsdb/automake.mk b/ovsdb/automake.mk
index 33d04f8..c218bf5 100644
--- a/ovsdb/automake.mk
+++ b/ovsdb/automake.mk
@@ -35,7 +35,9 @@ ovsdb_libovsdb_la_SOURCES = \
ovsdb/trigger.c \
ovsdb/trigger.h \
ovsdb/transaction.c \
-   ovsdb/transaction.h
+   ovsdb/transaction.h \
+   ovsdb/ovsdb-util.c \
+   ovsdb/ovsdb-util.h
 ovsdb_libovsdb_la_CFLAGS = $(AM_CFLAGS)
 ovsdb_libovsdb_la_CPPFLAGS = $(AM_CPPFLAGS)
 
diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index 8e3bdaf..50c3555 100644
--- a/ovsdb/ovsdb-server.c
+++ b/ovsdb/ovsdb-server.c
@@ -56,6 +56,7 @@
 #include "util.h"
 #include "unixctl.h"
 #include "perf-counter.h"
+#include "ovsdb-util.h"
 #include "openvswitch/vlog.h"
 
 VLOG_DEFINE_THIS_MODULE(ovsdb_server);
@@ -673,174 +674,6 @@ add_remote(struct shash *remotes, const char *target)
 return options;
 }
 
-static struct ovsdb_datum *
-get_datum(struct ovsdb_row *row, const char *column_name,
-  const enum ovsdb_atomic_type key_type,
-  const enum ovsdb_atomic_type value_type,
-  const size_t n_max)
-{
-static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
-const struct ovsdb_table_schema *schema = row->table->schema;
-const struct ovsdb_column *column;
-
-column = ovsdb_table_schema_get_column(schema, column_name);
-if (!column) {
-VLOG_DBG_RL(, "Table `%s' has no `%s' column",
-schema->name, column_name);
-return NULL;
-}
-
-if (column->type.key.type != key_type
-|| column->type.value.type != value_type
-|| column->type.n_max != n_max) {
-if (!VLOG_DROP_DBG()) {
-char *type_name = ovsdb_type_to_english(>type);
-VLOG_DBG("Table `%s' column `%s' has type %s, not expected "
- "key type %s, value type %s, max elements %"PRIuSIZE".",
- schema->name, column_name, type_name,
- ovsdb_atomic_type_to_string(key_type),
- ovsdb_atomic_type_to_string(value_type),
- n_max);
-free(type_name);
-}
-return NULL;
-}
-
-return >fields[column->index];
-}
-
-/* Read string-string key-values from a map.  Returns the value associated with
- * 'key', if found, or NULL */
-static const char *
-read_map_string_column(const struct ovsdb_row *row, const char *column_name,
-   const char *key)
-{
-const struct ovsdb_datum *datum;
-union ovsdb_atom *atom_key = NULL, *atom_value = NULL;
-size_t i;
-
-datum = get_datum(CONST_CAST(struct ovsdb_row *, row), column_name,
-  OVSDB_TYPE_STRING, OVSDB_TYPE_STRING, UINT_MAX);
-
-if (!datum) {
-return NULL;
-}
-
-for (i = 0; i < datum->n; i++) {
-atom_key = >keys[i];
-if (!strcmp(atom_key->string, key)){
-atom_value = >values[i];
-break;
-}
-}
-
-return atom_value ? atom_value->string : NULL;
-}
-
-static const union ovsdb_atom *
-read_column(const struct ovsdb_row *row, const char *column_name,
-enum ovsdb_atomic_type type)
-{
-const struct ovsdb_datum *datum;
-
-datum = get_datum(CONST_CAST(struct ovsdb_row *, row), column_name, type,
-  OVSDB_TYPE_VOID, 1);
-return datum && datum->n ? datum->keys : NULL;
-}
-
-static bool
-read_integer_column(const struct ovsdb_row *row, const char *column_name,
-long long int *integerp)
-{
-const union ovsdb_atom *atom;
-
-atom = read_column(row, column_name, OVSDB_TYPE_INTEGER);
-*integerp = atom ? atom->integer : 0;
-return atom != NULL;
-}
-
-static bool
-read_string_column(const struct ovsdb_row *row, const char *column_name,
-   const char **stringp)
-{
-const union ovsdb_atom *atom;
-
-atom = read_column(row, column_name, OVSDB_TYPE_STRING);
-*stringp = atom ? atom->string : NULL;
-return atom != NULL;
-}
-
-static bool
-read_bool_column(const struct ovsdb_row *row, const char *column_name,
-   bool *boolp)
-{
-const union ovsdb_atom *atom;
-
-atom = read_column(row, column_name, OVSDB_TYPE_BOOLEAN);
-*boolp = atom ? atom->boolean : false;
-return atom != 

[ovs-dev] [RFC v2 0/5] role-based access controls for ovsdb-server, ovn-sb

[Lance Richardson]

This series implements role-based access control infrastructure for
ovsdb-server, and uses that infrastructure to apply role-based access
controls to the OVN_Southbound database. This implementation follows
the outline discussed at:

 https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329801.html

With this series applied, enabling role-based ACLs is a matter of:

- Configuring southbound ovsdb-server and ovn-controller to use SSL,
  configuring an ovn-controller "role" for SSL connections via e.g.:
 ovn-sbctl set-connection role=ovn-controller pssl:6642
- Using unique certificates for each ovn-controller with a unique
  CN for each chassis, generated e.g. via:
 ovs-pki -B 1024 req+sign chassis1 switch
 ovs-pki -B 1024 req+sign chassis2 switch
 ovs-pki -B 1024 req+sign chassis3 switch
- Starting the southbound ovsdb-server with the "--rbac" command-line
  option:
 --rbac=db:OVN_Southbound,RBAC_Role

This series is posted as RFC mainly to solicit high-level feedback about
the approach, although feedback about implementation details would also
be welcome. Outstanding work items:

- Unit test for authorization from map.
- Update man page for ovsdb-server --rbac option.
- Add security section to ovn-architecture document to describe how
  SSL and RBAC can be applied.
- Add section to ovn-northd man page describing how northd configures
  RBAC for the southbound db.
- Evaluate other methods for enabling RBAC enforcement in ovsdb-server,
  see https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/330718.html
- Add "chassis" column to OVN southbound Encap table to allow more
  effective RBAC.


Lance Richardson (5):
  stream: store stream peer id with stream state
  ovsdb: refactor utility functions into separate file
  ovsdb: add support for role-based access controls
  ovn: add rbac tables to ovn southbound schema
  ovn-sbctl: support setting rbac role for remote connections

 lib/jsonrpc.c |  10 ++
 lib/jsonrpc.h |   2 +
 lib/ovsdb-error.c |  13 ++
 lib/ovsdb-error.h |   4 +
 lib/stream-provider.h |   1 +
 lib/stream-ssl.c  |  51 ++
 lib/stream.c  |  16 ++
 lib/stream.h  |   3 +
 ovn/northd/ovn-northd.c   | 190 
 ovn/ovn-sb.ovsschema  |  28 ++-
 ovn/ovn-sb.xml|  39 
 ovn/utilities/ovn-sbctl.c |  12 +-
 ovsdb/automake.mk |   6 +-
 ovsdb/execution.c |  41 -
 ovsdb/jsonrpc-server.c|   6 +-
 ovsdb/jsonrpc-server.h|   1 +
 ovsdb/mutation.c  |   2 +
 ovsdb/mutation.h  |   5 +-
 ovsdb/ovsdb-server.c  | 247 -
 ovsdb/ovsdb-tool.c|   2 +-
 ovsdb/ovsdb-util.c| 235 
 ovsdb/ovsdb-util.h|  51 ++
 ovsdb/ovsdb.h |   1 +
 ovsdb/rbac.c  | 449 ++
 ovsdb/rbac.h  |  36 
 ovsdb/trigger.c   |   8 +-
 ovsdb/trigger.h   |   5 +-
 tests/automake.mk |   1 +
 tests/ovsdb-rbac.at   | 253 ++
 tests/ovsdb.at|   1 +
 tests/test-ovsdb.c|   5 +-
 31 files changed, 1533 insertions(+), 191 deletions(-)
 create mode 100644 ovsdb/ovsdb-util.c
 create mode 100644 ovsdb/ovsdb-util.h
 create mode 100644 ovsdb/rbac.c
 create mode 100644 ovsdb/rbac.h
 create mode 100644 tests/ovsdb-rbac.at

-- 
2.7.4

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

[ovs-dev] [RFC v2 1/5] stream: store stream peer id with stream state

[Lance Richardson]

Keep track of authenticated ID for stream peer. For SSL connections,
the authenticated ID is the CN (Common Name) field from the peer's
SSL certificate.

Signed-off-by: Lance Richardson 
---
v2:
  - Accomodate OpenSSL 1.1 deprecation of ASN1_STRING_data().
  - Added comment explaining source of "id:" in CN field.

 lib/stream-provider.h |  1 +
 lib/stream-ssl.c  | 51 +++
 lib/stream.c  | 16 
 lib/stream.h  |  3 +++
 4 files changed, 71 insertions(+)

diff --git a/lib/stream-provider.h b/lib/stream-provider.h
index 2226a80..ce88785 100644
--- a/lib/stream-provider.h
+++ b/lib/stream-provider.h
@@ -30,6 +30,7 @@ struct stream {
 int state;
 int error;
 char *name;
+char *peer_id;
 };
 
 void stream_init(struct stream *, const struct stream_class *,
diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
index 5d88b52..4816a11 100644
--- a/lib/stream-ssl.c
+++ b/lib/stream-ssl.c
@@ -420,6 +420,44 @@ do_ca_cert_bootstrap(struct stream *stream)
 return EPROTO;
 }
 
+static char *
+get_peer_common_name(const struct ssl_stream *sslv)
+{
+X509_NAME_ENTRY *cn_entry;
+ASN1_STRING *cn_data;
+X509 *peer_cert;
+int cn_index;
+const char *cn;
+
+peer_cert = SSL_get_peer_certificate(sslv->ssl);
+if (!peer_cert) {
+return NULL;
+}
+cn_index = X509_NAME_get_index_by_NID(X509_get_subject_name(peer_cert),
+  NID_commonName, -1);
+if (cn_index < 0) {
+return NULL;
+}
+
+cn_entry = X509_NAME_get_entry(X509_get_subject_name(peer_cert), cn_index);
+if (!cn_entry) {
+return NULL;
+}
+
+cn_data = X509_NAME_ENTRY_get_data(cn_entry);
+if (!cn_data) {
+return NULL;
+}
+
+#if OPENSSL_VERSION_NUMBER < 0x1010L
+/* ASN1_STRING_data() is deprecated as of OpenSSL version 1.1 */
+cn = (const char *)ASN1_STRING_data(cn_data);
+#else
+cn = (const char *)ASN1_STRING_get0_data(cn_data);
+ #endif
+return xstrdup(cn);
+}
+
 static int
 ssl_connect(struct stream *stream)
 {
@@ -477,6 +515,19 @@ ssl_connect(struct stream *stream)
 VLOG_INFO("rejecting SSL connection during bootstrap race window");
 return EPROTO;
 } else {
+char *cn = get_peer_common_name(sslv);
+
+if (cn) {
+/* ovs-pki appends " id:" to the user-specified name,
+ * remove it if present */
+char *ptr = strstr(cn, " id:");
+
+if (ptr) {
+*ptr = '\0';
+}
+stream_set_peer_id(stream, cn);
+free(cn);
+}
 return 0;
 }
 }
diff --git a/lib/stream.c b/lib/stream.c
index f6ea849..f183a4d 100644
--- a/lib/stream.c
+++ b/lib/stream.c
@@ -278,8 +278,10 @@ stream_close(struct stream *stream)
 {
 if (stream != NULL) {
 char *name = stream->name;
+char *peer_id = stream->peer_id;
 (stream->class->close)(stream);
 free(name);
+free(peer_id);
 }
 }
 
@@ -430,6 +432,20 @@ stream_send_wait(struct stream *stream)
 stream_wait(stream, STREAM_SEND);
 }
 
+void
+stream_set_peer_id(struct stream *stream, const char *peer_id)
+{
+free(stream->peer_id);
+stream->peer_id = xstrdup(peer_id);
+}
+
+const char *
+stream_get_peer_id(const struct stream *stream)
+{
+return stream->peer_id;
+}
+
+
 /* Given 'name', a pstream name in the form "TYPE:ARGS", stores the class
  * named "TYPE" into '*classp' and returns 0.  Returns EAFNOSUPPORT and stores
  * a null pointer into '*classp' if 'name' is in the wrong form or if no such
diff --git a/lib/stream.h b/lib/stream.h
index f8e1891..f9bfdc8 100644
--- a/lib/stream.h
+++ b/lib/stream.h
@@ -53,6 +53,9 @@ void stream_wait(struct stream *, enum stream_wait_type);
 void stream_connect_wait(struct stream *);
 void stream_recv_wait(struct stream *);
 void stream_send_wait(struct stream *);
+void stream_set_peer_id(struct stream *, const char *);
+const char *stream_get_peer_id(const struct stream *);
+
 
 /* Passive streams: listeners for incoming stream connections. */
 int pstream_verify_name(const char *name);
-- 
2.7.4

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

[ovs-dev] SugarCRM Updated Directory

[Модест Кабанов]

Hi,

Would you be interested in acquiring *SugarCRM Users* contact information
in excel sheet for unlimited marketing usage?

We also provide IT Decision Makers, Sales and Marketing Decision Makers,
C-level Titles and Decision Makers from all Departments.

We also have companies using: *Salesforce, DynamicsCRM, Dynamics GP,
Microsoft Dynamics, OpenERP, Sharepoint, SAP CRM, InfusionSoft, Zurmo,
CiviCRM And many more...*

You can also let me know if you have a different target audience.

Kindly let me know your thoughts on it or you could pass this email to the
right person in your company.

Await your response!

Thanks,

Modest Kabanov

Information Specialist

 To remove from this mailing: reply with
subject line as "leave out”
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC][PATCH] netdev-dpdk: add support for TSO

[Aaron Conole]

"Kavanagh, Mark B"  writes:

>>Hi Mark,
>>
>>Mark Kavanagh  writes:
>>
>>> TCP Segmentation Offload (TSO) is a feature which enables
>>> the TCP/IP network stack to delegate segmentation of a TCP
>>> segment to the NIC, thus saving compute resources.
>>>
>>> This commit adds support for TSO in the DPDK vHost-User backend,
>>> to OvS v2.6.1; this enables a guest to offload segmentation of
>>> TCP segments that it sends to OvS.
>>>
>>> This patch is not intended for upstreaming, but rather was produced
>>> in response to requests for an updated version of the initial TSO RFC
>>> patch posted here:
>>> https://mail.openvswitch.org/pipermail/ovs-dev/2016-June/316414.html
>>>
>>> Signed-off-by: Mark Kavanagh 
>>> ---
>>
>>...
>>
>>> diff --git a/lib/packets.c b/lib/packets.c
>>> index e4c29d5..2417ba2 100644
>>> --- a/lib/packets.c
>>> +++ b/lib/packets.c
>>> @@ -33,6 +33,10 @@
>>>  #include "dp-packet.h"
>>>  #include "unaligned.h"
>>>
>>> +#ifdef DPDK_NETDEV
>>> +#include "rte_ether.h"
>>> +#endif
>>> +
>>>  const struct in6_addr in6addr_exact = IN6ADDR_EXACT_INIT;
>>>  const struct in6_addr in6addr_all_hosts = IN6ADDR_ALL_HOSTS_INIT;
>>>
>>> @@ -204,6 +208,11 @@ eth_push_vlan(struct dp_packet *packet, ovs_be16 tpid, 
>>> ovs_be16 tci)
>>>  memmove(veh, (char *)veh + VLAN_HEADER_LEN, 2 * ETH_ADDR_LEN);
>>>  veh->veth_type = tpid;
>>>  veh->veth_tci = tci & htons(~VLAN_CFI);
>>> +
>>> +#ifdef DPDK_NETDEV
>>> +struct rte_mbuf *pkt = &(packet->mbuf);
>>> +pkt->l2_len += sizeof(struct vlan_hdr);
>>> +#endif
>>>  }
>>>
>>>  /* Removes outermost VLAN header (if any is present) from 'packet'.
>>> @@ -221,6 +230,11 @@ eth_pop_vlan(struct dp_packet *packet)
>>>  memmove((char *)veh + VLAN_HEADER_LEN, veh, 2 * ETH_ADDR_LEN);
>>>  dp_packet_resize_l2(packet, -VLAN_HEADER_LEN);
>>>  }
>>> +
>>> +#ifdef DPDK_NETDEV
>>> +struct rte_mbuf *pkt = &(packet->mbuf);
>>> +pkt->l2_len -= sizeof(struct vlan_hdr);
>>> +#endif
>>>  }
>>>
>>>  /* Set ethertype of the packet. */
>>
>>Would it be better to change the dp_packet_resize_l2 call?  Are you
>
> Hey Aaron,
>
> Good call - that would definitely be a more suitable location.
>
>>worried about the mpls case?
>
> I haven't considered mpls here at all, as it wasn't part of the use
> case for which this new version of the patch was produced I'm afraid.
> Out of curiosity, what is your concern here?

The two users of dp_packet_resize_l2 are the vlan and the mpls code.  I
had assumed you skipped for that reason, but maybe that isn't the case.

> Thanks in advance,
> Mark
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 2/2] rhel: Avoid logrotate error if /var/run/openvswitch does not exist

[Timothy Redaelli]

Avoid also errors if an ovs server didn't start correctly or it crashed without
deleting the pid file.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1441524
Signed-off-by: Timothy Redaelli 
---
 rhel/etc_logrotate.d_openvswitch | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/rhel/etc_logrotate.d_openvswitch b/rhel/etc_logrotate.d_openvswitch
index 914e67c..d93a56e 100644
--- a/rhel/etc_logrotate.d_openvswitch
+++ b/rhel/etc_logrotate.d_openvswitch
@@ -12,8 +12,10 @@
 missingok
 postrotate
 # Tell Open vSwitch daemons to reopen their log files
-for pidfile in `cd /var/run/openvswitch && echo *.pid`; do
-ovs-appctl -t "${pidfile%%.pid}" vlog/reopen
-done
+if [ -d /var/run/openvswitch ]; then
+for pidfile in `cd /var/run/openvswitch && echo *.pid`; do
+ovs-appctl -t "${pidfile%%.pid}" vlog/reopen 2>/dev/null || :
+done
+fi
 endscript
 }
-- 
2.9.3

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

[ovs-dev] [PATCH 1/2] rhel/etc_logrotate.d_openvswitch: Fix coding style

[Timothy Redaelli]

Replace tabs by 4 spaces and indent the postrotate script like the
examples in 'man logrotate.conf'

Signed-off-by: Timothy Redaelli 
---
 rhel/etc_logrotate.d_openvswitch | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/rhel/etc_logrotate.d_openvswitch b/rhel/etc_logrotate.d_openvswitch
index 73751d4..914e67c 100644
--- a/rhel/etc_logrotate.d_openvswitch
+++ b/rhel/etc_logrotate.d_openvswitch
@@ -6,14 +6,14 @@
 # without warranty of any kind.
 
 /var/log/openvswitch/*.log {
-   daily
-   compress
-   sharedscripts
-   missingok
-   postrotate
-   # Tell Open vSwitch daemons to reopen their log files
+daily
+compress
+sharedscripts
+missingok
+postrotate
+# Tell Open vSwitch daemons to reopen their log files
 for pidfile in `cd /var/run/openvswitch && echo *.pid`; do
 ovs-appctl -t "${pidfile%%.pid}" vlog/reopen
 done
-   endscript
+endscript
 }
-- 
2.9.3

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

[ovs-dev] [PATCH] Add NAT information to the logical routers in nbctl show output

[Lucas Alvares Gomes]

This patch is changing the print_lr() function in ovn-nbctl.c to include
logical router NAT information as part of the output (external ip,
logical ip and type).

Signed-off-by: Lucas Alvares Gomes 
---
 ovn/utilities/ovn-nbctl.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 598f502af..e9dcde701 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -589,6 +589,18 @@ print_lr(const struct nbrec_logical_router *lr, struct ds 
*s)
 ds_put_cstr(s, "]\n");
 }
 }
+
+for (size_t i = 0; i < lr->n_nat; i++) {
+const struct nbrec_nat *nat = lr->nat[i];
+ds_put_format(s, "nat "UUID_FMT"\n",
+  UUID_ARGS(>header_.uuid));
+ds_put_cstr(s, "external ip: ");
+ds_put_format(s, "\"%s\"\n", nat->external_ip);
+ds_put_cstr(s, "logical ip: ");
+ds_put_format(s, "\"%s\"\n", nat->logical_ip);
+ds_put_cstr(s, "type: ");
+ds_put_format(s, "\"%s\"\n", nat->type);
+}
 }
 
 static void
-- 
2.12.2

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

Re: [ovs-dev] [RFC][PATCH] netdev-dpdk: add support for TSO

[Kavanagh, Mark B]

>Hi Mark,
>
>Mark Kavanagh  writes:
>
>> TCP Segmentation Offload (TSO) is a feature which enables
>> the TCP/IP network stack to delegate segmentation of a TCP
>> segment to the NIC, thus saving compute resources.
>>
>> This commit adds support for TSO in the DPDK vHost-User backend,
>> to OvS v2.6.1; this enables a guest to offload segmentation of
>> TCP segments that it sends to OvS.
>>
>> This patch is not intended for upstreaming, but rather was produced
>> in response to requests for an updated version of the initial TSO RFC
>> patch posted here:
>> https://mail.openvswitch.org/pipermail/ovs-dev/2016-June/316414.html
>>
>> Signed-off-by: Mark Kavanagh 
>> ---
>
>...
>
>> diff --git a/lib/packets.c b/lib/packets.c
>> index e4c29d5..2417ba2 100644
>> --- a/lib/packets.c
>> +++ b/lib/packets.c
>> @@ -33,6 +33,10 @@
>>  #include "dp-packet.h"
>>  #include "unaligned.h"
>>
>> +#ifdef DPDK_NETDEV
>> +#include "rte_ether.h"
>> +#endif
>> +
>>  const struct in6_addr in6addr_exact = IN6ADDR_EXACT_INIT;
>>  const struct in6_addr in6addr_all_hosts = IN6ADDR_ALL_HOSTS_INIT;
>>
>> @@ -204,6 +208,11 @@ eth_push_vlan(struct dp_packet *packet, ovs_be16 tpid, 
>> ovs_be16 tci)
>>  memmove(veh, (char *)veh + VLAN_HEADER_LEN, 2 * ETH_ADDR_LEN);
>>  veh->veth_type = tpid;
>>  veh->veth_tci = tci & htons(~VLAN_CFI);
>> +
>> +#ifdef DPDK_NETDEV
>> +struct rte_mbuf *pkt = &(packet->mbuf);
>> +pkt->l2_len += sizeof(struct vlan_hdr);
>> +#endif
>>  }
>>
>>  /* Removes outermost VLAN header (if any is present) from 'packet'.
>> @@ -221,6 +230,11 @@ eth_pop_vlan(struct dp_packet *packet)
>>  memmove((char *)veh + VLAN_HEADER_LEN, veh, 2 * ETH_ADDR_LEN);
>>  dp_packet_resize_l2(packet, -VLAN_HEADER_LEN);
>>  }
>> +
>> +#ifdef DPDK_NETDEV
>> +struct rte_mbuf *pkt = &(packet->mbuf);
>> +pkt->l2_len -= sizeof(struct vlan_hdr);
>> +#endif
>>  }
>>
>>  /* Set ethertype of the packet. */
>
>Would it be better to change the dp_packet_resize_l2 call?  Are you

Hey Aaron,

Good call - that would definitely be a more suitable location.

>worried about the mpls case?

I haven't considered mpls here at all, as it wasn't part of the use case for 
which this new version of the patch was produced I'm afraid.
Out of curiosity, what is your concern here?

Thanks in advance,
Mark
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v5] tunneling: Avoid recirculation on datapath by computing the recirculate actions at translate time.

[Chandran, Sugesh]

Can anyone have a look on this patch??

Regards
_Sugesh


> -Original Message-
> From: Chandran, Sugesh
> Sent: Tuesday, April 11, 2017 11:14 AM
> To: d...@openvswitch.org; b...@ovn.org
> Cc: Chandran, Sugesh ; Zoltán Balogh
> 
> Subject: [PATCH v5] tunneling: Avoid recirculation on datapath by computing
> the recirculate actions at translate time.
> 
> Openvswitch datapath recirculates packets for tunneling, i.e.
> the incoming packets are encapsulated at first pass. Further actions are
> applied on encapsulated packets on the second pass after recirculating.
> The proposed patch compute and append the post tunnel actions at the time
> of translation itself instead of recirculating at datapath. These actions are
> solely depends on tunnel attributes so there is no need of datapath
> recirculation.
> By avoiding the recirculation at datapath, the patch offers upto 30%
> performance improvement for VxLAN tunneling in our testing.
> The action execution logic is using the new CLONE action to define the packet
> cloning when the actions are combined. The lenght in the CLONE action
> specifies the size of nested action set.
> 
> It also fixing the test suites failures that are introduced by nested CLONE
> action in tunneling.
> 
> v5
> - Fix the OVN test case failure by commenting the test validation as its not
>   relevant with the new tunnel CLONE action.
> - Code changes for applying CLONE action on a batch than individual packets
>   are already pushed to the master. V5 patch is now only doing CLONE at
> tunnel
>   push.
> v4
> - Rename the function to compute post tunnel nested function.
> - Use the clone action syntax itself for the flow display.
> - Use nl_msg functions for handling the nested attribute.
> - Modify the CLONE action to process packets in batch than individually.
> v3
> - Rebase with newely clone action and use it for tunneling.
> v2
> - Use only single CLONE action with length to mark the tunnel combine action
> set.
> - Update the datapath trace display functions to handle CLONE.
> - Fixed test cases to work with CLONE action.
> 
> Signed-off-by: Sugesh Chandran 
> Signed-off-by: Zoltán Balogh 
> Co-authored-by: Zoltán Balogh 
> ---
>  lib/dpif-netdev.c |  18 +--
>  ofproto/ofproto-dpif-xlate.c  | 280 ++---
> -
>  tests/ofproto-dpif.at |  11 +-
>  tests/ovn.at  |   6 +-
>  tests/tunnel-push-pop-ipv6.at |  10 +-
>  tests/tunnel-push-pop.at  |  12 +-
>  6 files changed, 164 insertions(+), 173 deletions(-)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index a14a2eb..41d0836
> 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -4995,24 +4995,8 @@ dp_execute_cb(void *aux_, struct
> dp_packet_batch *packets_,
> 
>  case OVS_ACTION_ATTR_TUNNEL_PUSH:
>  if (*depth < MAX_RECIRC_DEPTH) {
> -struct dp_packet_batch tnl_pkt;
> -struct dp_packet_batch *orig_packets_ = packets_;
> -int err;
> -
> -if (!may_steal) {
> -dp_packet_batch_clone(_pkt, packets_);
> -packets_ = _pkt;
> -dp_packet_batch_reset_cutlen(orig_packets_);
> -}
> -
>  dp_packet_batch_apply_cutlen(packets_);
> -
> -err = push_tnl_action(pmd, a, packets_);
> -if (!err) {
> -(*depth)++;
> -dp_netdev_recirculate(pmd, packets_);
> -(*depth)--;
> -}
> +push_tnl_action(pmd, a, packets_);
>  return;
>  }
>  break;
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index
> a24aef9..b416f46 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -423,6 +423,10 @@ static void xlate_action_set(struct xlate_ctx *ctx);
> static void xlate_commit_actions(struct xlate_ctx *ctx);
> 
>  static void
> +apply_nested_clone_actions(struct xlate_ctx *ctx, const struct xport
> *in_dev,
> +  struct xport *out_dev);
> +
> +static void
>  ctx_trigger_freeze(struct xlate_ctx *ctx)  {
>  ctx->exit = true;
> @@ -3204,7 +3208,17 @@ build_tunnel_send(struct xlate_ctx *ctx, const
> struct xport *xport,
>  }
>  tnl_push_data.tnl_port = odp_to_u32(tunnel_odp_port);
>  tnl_push_data.out_port = odp_to_u32(out_dev->odp_port);
> +
> +size_t push_action_size = 0;
> +size_t clone_ofs = nl_msg_start_nested(ctx->odp_actions,
> +   OVS_ACTION_ATTR_CLONE);
>  odp_put_tnl_push_action(ctx->odp_actions, _push_data);
> +push_action_size = ctx->odp_actions->size;
> +apply_nested_clone_actions(ctx, xport, out_dev);
> +if (ctx->odp_actions->size > push_action_size) {
> +/* Update the CLONE action only when combined */
> +