Re: [ovs-dev] [PATCH v6] netdev-dpdk: Enable Rx checksum offloading feature on DPDK physical ports.

[Chandran, Sugesh]

Regards
_Sugesh

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Tuesday, December 13, 2016 4:29 PM
> To: Chandran, Sugesh 
> Cc: d...@openvswitch.org; je...@kernel.org
> Subject: Re: [ovs-dev] [PATCH v6] netdev-dpdk: Enable Rx checksum
> offloading feature on DPDK physical ports.
> 
> On Tue, Dec 13, 2016 at 10:19:53AM +, Chandran, Sugesh wrote:
> > Hi Ben,
> >
> > Regards
> > _Sugesh
> >
> > > -Original Message-
> > > From: Ben Pfaff [mailto:b...@ovn.org]
> > > Sent: Monday, December 12, 2016 10:24 PM
> > > To: Chandran, Sugesh 
> > > Cc: d...@openvswitch.org; je...@kernel.org
> > > Subject: Re: [ovs-dev] [PATCH v6] netdev-dpdk: Enable Rx checksum
> > > offloading feature on DPDK physical ports.
> > >
> > > On Mon, Nov 28, 2016 at 03:24:24PM +, Sugesh Chandran wrote:
> > > > Add Rx checksum offloading feature support on DPDK physical ports.
> > > > By default, the Rx checksum offloading is enabled if NIC supports.
> > > > However, the checksum offloading can be turned OFF either while
> > > > adding a new DPDK physical port to OVS or at runtime.
> > > >
> > > > The rx checksum offloading can be turned off by setting the
> > > > parameter to 'false'. For eg: To disable the rx checksum
> > > > offloading when adding a port,
> > > >
> > > >  'ovs-vsctl add-port br0 dpdk0 -- \
> > > >   set Interface dpdk0 type=dpdk options:rx-checksum-offload=false'
> > > >
> > > > OR (to disable at run time after port is being added to OVS)
> > > >
> > > > 'ovs-vsctl set Interface dpdk0 options:rx-checksum-offload=false'
> > > >
> > > > Similarly to turn ON rx checksum offloading at run time,
> > > >
> > > > 'ovs-vsctl set Interface dpdk0 options:rx-checksum-offload=true'
> > > >
> > > > The Tx checksum offloading support is not implemented due to the
> > > > following reasons.
> > > >
> > > > 1) Checksum offloading and vectorization are mutually exclusive in
> > > > DPDK poll mode driver. Vector packet processing is turned OFF when
> > > > checksum offloading is enabled which causes significant
> > > > performance drop
> > > at Tx side.
> > > >
> > > > 2) Normally, OVS generates checksum for tunnel packets in software
> > > > at the 'tunnel push' operation, where the tunnel headers are created.
> > > > However enabling Tx checksum offloading involves,
> > > >
> > > >   *) Mark every packets for tx checksum offloading at 'tunnel_push' and
> > > >   recirculate.
> > > >   *) At the time of xmit, validate the same flag and instruct the
> > > > NIC to do
> > > the
> > > >   checksum calculation.  In case NIC doesnt support Tx checksum
> offloading,
> > > >   the checksum calculation has to be done in software before
> > > > sending out
> > > the
> > > >   packets.
> > > >
> > > > No significant performance improvement noticed with Tx checksum
> > > > offloading due to the e overhead of additional validations + non
> > > > vector
> > > packet processing.
> > > > In some test scenarios, it introduces performance drop too.
> > > >
> > > > Rx checksum offloading still offers 8-9% of improvement on VxLAN
> > > > tunneling decapsulation even though the SSE vector Rx function is
> > > > disabled in DPDK poll mode driver.
> > > >
> > > > Signed-off-by: Sugesh Chandran 
> > > > Acked-by: Jesse Gross 
> > >
> > > Why would a user want to turn off checksum offloading?  The patch
> > > and the documentation does not explain why, and it seems strange to
> > > offer an optimization that a user would not want to use.
> > [Sugesh] In DPDK, The vectorization get turned off when checksum
> > offloading is enabled, which impacts the performance of smaller(64
> > bytes) packet traffic. The performance gain offered by checksum offload
> alleviates the vectorization impact in tunneling decapsulation. However it is
> not the case for non tunnel traffic. We also noticed the impact of
> vectorization is not very significant for larger packets >256 bytes for any 
> type
> of traffic.
> > Considering all these aspects and according to the previous
> communications on this patch, we decided to give flexibility to user to turn
> ON/OFF checksum. So that a user who have only non tunnel traffic(with
> smaller packets) can turn off the checksum offload to get the same old good
> performance. By default the checksum offload is in ON state.
> >
> > Do you think this information must be mentioned in the documentation?
> There are lot of parameters involved in there for a normal user to decide
> upon the checksum offload ON/OFF. Considering the style of documentation
> which talks only about the available configuration options and its syntax, do
> we have to put all these information which may cause confusion?
> > As you mentioned, I can provide these details in the commit message. Do
> you think it is suffice?
> 
> I think that the documentation should try to explain.  Users will not read
> commit messages.
[Sugesh] 

[ovs-dev] [PATCH v2 1/4] doc: Split dpdk, dpdk-advanced into multiple docs

[Stephen Finucane]

Combined, the dpdk and dpdk-advanced installation documents provide a
lot of useful information, but most of this information is unrelated to
installation. Rework these documents, completely breaking up the
dpdk-advanced document into multiple smaller documents in other sections
and moving non-install aspects of the dpdk document into these sections.
This aims to tie the DPDK docs into the documentation structure.

Signed-off-by: Stephen Finucane 
---
v2:
- Resolve merge conflicts
---
 Documentation/automake.mk  |   6 +-
 Documentation/howto/dpdk.rst   | 603 +
 Documentation/howto/index.rst  |   1 +
 Documentation/index.rst|  13 +
 Documentation/intro/install/dpdk-advanced.rst  | 938 -
 Documentation/intro/install/dpdk.rst   | 584 ++---
 Documentation/intro/install/index.rst  |   5 -
 Documentation/topics/dpdk/index.rst|  32 +
 .../topics/{dpdk.rst => dpdk/ivshmem.rst}  |   6 +-
 Documentation/topics/dpdk/vhost-user.rst   | 396 +
 Documentation/topics/index.rst |   3 +-
 Documentation/topics/testing.rst   |  38 +
 12 files changed, 1369 insertions(+), 1256 deletions(-)
 create mode 100644 Documentation/howto/dpdk.rst
 delete mode 100644 Documentation/intro/install/dpdk-advanced.rst
 create mode 100644 Documentation/topics/dpdk/index.rst
 rename Documentation/topics/{dpdk.rst => dpdk/ivshmem.rst} (93%)
 create mode 100644 Documentation/topics/dpdk/vhost-user.rst
 create mode 100644 Documentation/topics/testing.rst

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index b02d63e..ffb8ae3 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -9,7 +9,6 @@ EXTRA_DIST += \
Documentation/intro/install/index.rst \
Documentation/intro/install/bash-completion.rst \
Documentation/intro/install/debian.rst \
-   Documentation/intro/install/dpdk-advanced.rst \
Documentation/intro/install/dpdk.rst \
Documentation/intro/install/fedora.rst \
Documentation/intro/install/general.rst \
@@ -25,7 +24,10 @@ EXTRA_DIST += \
Documentation/topics/bonding.rst \
Documentation/topics/datapath.rst \
Documentation/topics/design.rst \
-   Documentation/topics/dpdk.rst \
+   Documentation/topics/dpdk/index.rst \
+   Documentation/topics/dpdk/vhost-user.rst \
+   Documentation/topics/dpdk/ivshmem.rst \
+   Documentation/topics/testing.rst \
Documentation/topics/high-availability.rst \
Documentation/topics/integration.rst \
Documentation/topics/openflow.rst \
diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
new file mode 100644
index 000..f55ae3b
--- /dev/null
+++ b/Documentation/howto/dpdk.rst
@@ -0,0 +1,603 @@
+..
+  Licensed under the Apache License, Version 2.0 (the "License"); you may
+  not use this file except in compliance with the License. You may obtain
+  a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+  License for the specific language governing permissions and limitations
+  under the License.
+
+  Convention for heading levels in Open vSwitch documentation:
+
+  ===  Heading 0 (reserved for the title in a document)
+  ---  Heading 1
+  ~~~  Heading 2
+  +++  Heading 3
+  '''  Heading 4
+
+  Avoid deeper levels because they do not render well.
+
+
+Using Open vSwitch with DPDK
+
+
+This document describes how to use Open vSwitch with DPDK datapath.
+
+.. important::
+
+   Using the DPDK datapath requires building OVS with DPDK support. Refer to
+   :doc:`/intro/install/dpdk` for more information.
+
+Ports and Bridges
+-
+
+ovs-vsctl can be used to set up bridges and other Open vSwitch features.
+Bridges should be created with a ``datapath_type=netdev``::
+
+$ ovs-vsctl add-br br0 -- set bridge br0 datapath_type=netdev
+
+ovs-vsctl can also be used to add DPDK devices. OVS expects DPDK device names
+to start with ``dpdk`` and end with a portid. ovs-vswitchd should print the
+number of dpdk devices found in the log file::
+
+$ ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk
+$ ovs-vsctl add-port br0 dpdk1 -- set Interface dpdk1 type=dpdk
+
+After the DPDK ports get added to switch, a polling thread continuously polls
+DPDK devices and consumes 100% of the core, as can be checked from ``top`` and
+``ps`` commands::
+
+$ top -H
+$ ps -eLo pid,psr,comm | grep 

[ovs-dev] [PATCH v2 3/4] doc: Document Patchwork instance

[Stephen Finucane]

I know more than a little bit about this :)

Signed-off-by: Stephen Finucane 
---
 Documentation/automake.mk |  1 +
 Documentation/index.rst   |  1 +
 Documentation/internals/index.rst |  1 +
 Documentation/internals/patchwork.rst | 61 +++
 4 files changed, 64 insertions(+)
 create mode 100644 Documentation/internals/patchwork.rst

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index ffb8ae3..8bf1e07 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -64,6 +64,7 @@ EXTRA_DIST += \
Documentation/internals/committer-responsibilities.rst \
Documentation/internals/mailing-lists.rst \
Documentation/internals/maintainers.rst \
+   Documentation/internals/patchwork.rst \
Documentation/internals/release-process.rst \
Documentation/internals/security.rst \
Documentation/internals/contributing/index.rst \
diff --git a/Documentation/index.rst b/Documentation/index.rst
index 2eecf95..f410487 100644
--- a/Documentation/index.rst
+++ b/Documentation/index.rst
@@ -84,6 +84,7 @@ Learn more about the Open vSwitch project and about how you 
can contribute:
 - **Community:** :doc:`internals/release-process` |
   :doc:`internals/authors` |
   :doc:`internals/mailing-lists` |
+  :doc:`internals/patchwork` |
   :doc:`internals/bugs` |
   :doc:`internals/security`
 
diff --git a/Documentation/internals/index.rst 
b/Documentation/internals/index.rst
index 9588861..b0ae9b4 100644
--- a/Documentation/internals/index.rst
+++ b/Documentation/internals/index.rst
@@ -35,6 +35,7 @@ itself and how they might involved.
 
contributing/index
mailing-lists
+   patchwork
release-process
bugs
security
diff --git a/Documentation/internals/patchwork.rst 
b/Documentation/internals/patchwork.rst
new file mode 100644
index 000..3ae0d95
--- /dev/null
+++ b/Documentation/internals/patchwork.rst
@@ -0,0 +1,61 @@
+..
+  Copyright (C) 2016, Stephen Finucane 
+
+  Licensed under the Apache License, Version 2.0 (the "License"); you may
+  not use this file except in compliance with the License. You may obtain
+  a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+  License for the specific language governing permissions and limitations
+  under the License.
+
+  Convention for heading levels in Open vSwitch documentation:
+
+  ===  Heading 0 (reserved for the title in a document)
+  ---  Heading 1
+  ~~~  Heading 2
+  +++  Heading 3
+  '''  Heading 4
+
+  Avoid deeper levels because they do not render well.
+
+=
+Patchwork
+=
+
+Open vSwitch uses `Patchwork`__ to track the status of patches sent to the
+:doc:`ovs-dev mailing list `. The Open vSwitch Patchwork
+instance can be found on `ozlabs.org`__. The ``pwclientrc`` file, required for
+*pwclient*, can be found on the `project page`__
+
+Patchwork provides a number of useful features for developers working on Open
+vSwitch:
+
+- Tracking the lifecycle of patches (accepted, rejected, under-review, ...)
+- Assigning reviewers (delegates) to patches
+- Downloading/applying patches via the web UI or the XML-RPC API (see
+  :ref:`pwclient`)
+- A usable UI for viewing patch discussions
+
+__ https://github.com/getpatchwork/patchwork
+__ https://patchwork.ozlabs.org/project/openvswitch/list/
+__ https://patchwork.ozlabs.org/project/openvswitch/
+
+.. _pwclient:
+
+pwclient
+
+
+The *pwclient* tool provides an way to download and apply patches, change the
+state of patches in Patchwork, and more. You can download *pwclient* from
+`here`__. Once downloaded, run::
+
+$ pwclient help
+
+to get more information about the functionality pwclient provides.
+
+__ https://patchwork.ozlabs.org/pwclient/
-- 
2.9.3

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

[ovs-dev] [PATCH v2 0/4] doc: Rework DPDK guide

[Stephen Finucane]

As promised, this series reworks the largest of the installation guides
to suit the new structure of the documentation. A couple of related
patches are included to build upon this.

Changes since v2:
- Rebase onto master

Stephen Finucane (4):
  doc: Split dpdk, dpdk-advanced into multiple docs
  doc: Move testing to testing section
  doc: Document Patchwork instance
  gitignore: Ignore venv

 .gitignore |   1 +
 Documentation/automake.mk  |   7 +-
 Documentation/howto/dpdk.rst   | 603 +
 Documentation/howto/index.rst  |   1 +
 Documentation/index.rst|  14 +
 Documentation/internals/index.rst  |   1 +
 Documentation/internals/patchwork.rst  |  61 ++
 Documentation/intro/install/dpdk-advanced.rst  | 938 -
 Documentation/intro/install/dpdk.rst   | 584 ++---
 Documentation/intro/install/general.rst| 348 +---
 Documentation/intro/install/index.rst  |   5 -
 Documentation/topics/dpdk/index.rst|  32 +
 .../topics/{dpdk.rst => dpdk/ivshmem.rst}  |   6 +-
 Documentation/topics/dpdk/vhost-user.rst   | 396 +
 Documentation/topics/index.rst |   3 +-
 Documentation/topics/testing.rst   | 393 +
 16 files changed, 1791 insertions(+), 1602 deletions(-)
 create mode 100644 Documentation/howto/dpdk.rst
 create mode 100644 Documentation/internals/patchwork.rst
 delete mode 100644 Documentation/intro/install/dpdk-advanced.rst
 create mode 100644 Documentation/topics/dpdk/index.rst
 rename Documentation/topics/{dpdk.rst => dpdk/ivshmem.rst} (93%)
 create mode 100644 Documentation/topics/dpdk/vhost-user.rst
 create mode 100644 Documentation/topics/testing.rst

-- 
2.9.3

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

Re: [ovs-dev] [PATCH 2/4] trivial: Consistently indent HTML with 2 spaces

[Stephen Finucane]

On Tue, 2016-12-13 at 11:41 -0800, Ben Pfaff wrote:
> On Tue, Dec 13, 2016 at 05:57:22PM +, Stephen Finucane wrote:
> > This will make upcoming changes easier to review.
> > 
> > Signed-off-by: Stephen Finucane 
> 
> "git am" tells me:
> 
> fatal: corrupt patch at line 397
> Patch failed at 0001 trivial: Consistently indent HTML with 2
> spaces
> 
> Maybe a Github PR would work better, dunno.

I guess that warning I got about a line being greater than 998
characters /wasn't/ safe to ignore :)

You can find the PR at the link below - it still applies cleanly
afaict:

  https://github.com/openvswitch/openvswitch.github.io/pull/28

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

Re: [ovs-dev] [PATCH] docs: Add link to man pages.

[Stephen Finucane]

On Tue, 2016-12-13 at 15:09 -0500, Russell Bryant wrote:
> 
> 
> On Tue, Dec 13, 2016 at 1:30 PM, Stephen Finucane 
> wrote:
> > On Tue, 2016-12-13 at 18:17 +, Stephen Finucane wrote:
> > > On Tue, 2016-12-13 at 10:57 -0500, Russell Bryant wrote:
> > > > Our new docs site does not yet include all of the man
> > pages.  Add a
> > > > link
> > > > to where they are currently hosted on openvswitch.org.
> > >
> > > Working on it :)
> > >
> > > > Signed-off-by: Russell Bryant 
> > > > ---
> > > >  Documentation/index.rst | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/Documentation/index.rst b/Documentation/index.rst
> > > > index f15993f..b2b1be9 100644
> > > > --- a/Documentation/index.rst
> > > > +++ b/Documentation/index.rst
> > > > @@ -61,7 +61,8 @@ vSwitch? Start here.
> > > >  Deeper Dive
> > > >  ---
> > > >  
> > > > -**TODO**
> > > > +Detailed man pages can be found on openvswitch.org
> > > > +(http://openvswitch.org/support/dist-docs/).
> > >
> > > Perhaps:
> > >
> > >   `openvswitch.org `__
> > >
> > > might be a little cleaner? That's a nit though, so...
> > 
> > Actually - I already have this done in
> > 'Documentation/ref/index.rst'
> > but forgot to include it anywhere. I'll submit a patch shortly to
> > resolve this.
> 
> OK, I'll just wait for your patch.

Ben merged this yesterday

  mail.openvswitch.org/pipermail/ovs-dev/2016-December/326179.html

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

[ovs-dev] [PATCH v2 2/4] doc: Move testing to testing section

[Stephen Finucane]

This makes more sense here, seeing as it's not exactly installation
related.

Signed-off-by: Stephen Finucane 
---
 Documentation/intro/install/general.rst | 348 +--
 Documentation/topics/testing.rst| 359 +++-
 2 files changed, 359 insertions(+), 348 deletions(-)

diff --git a/Documentation/intro/install/general.rst 
b/Documentation/intro/install/general.rst
index 89b4e74..371643a 100644
--- a/Documentation/intro/install/general.rst
+++ b/Documentation/intro/install/general.rst
@@ -436,7 +436,8 @@ and ``vif1.0`` to it::
 $ ovs-vsctl add-port br0 eth0
 $ ovs-vsctl add-port br0 vif1.0
 
-Refer to ovs-vsctl(8) for more details.
+Refer to ovs-vsctl(8) for more details. You may also wish to refer to
+:doc:`/topics/testing` for information on more generic testing of OVS.
 
 Upgrading
 -
@@ -511,351 +512,6 @@ above, but also replaces the old kernel module with the 
new one. Open vSwitch
 startup scripts for Debian, XenServer and RHEL use ovs-ctl's functions and it
 is recommended that these functions be used for other software platforms too.
 
-.. _general-testing:
-
-Testing

-
-This section describe Open vSwitch's built-in support for various test
-suites. You must bootstrap, configure and build Open vSwitch (steps are
-in "Building and Installing Open vSwitch for Linux, FreeBSD or NetBSD"
-above) before you run the tests described here. You do not need to
-install Open vSwitch or to build or load the kernel module to run these
-test suites. You do not need supervisor privilege to run these test
-suites.
-
-Unit Tests
-~~
-
-Open vSwitch includes a suite of self-tests. Before you submit patches
-upstream, we advise that you run the tests and ensure that they pass. If you
-add new features to Open vSwitch, then adding tests for those features will
-ensure your features don't break as developers modify other areas of Open
-vSwitch.
-
-To run all the unit tests in Open vSwitch, one at a time, run::
-
-$ make check
-
-This takes under 5 minutes on a modern desktop system.
-
-To run all the unit tests in Open vSwitch in parallel, run::
-
-$ make check TESTSUITEFLAGS=-j8
-
-You can run up to eight threads. This takes under a minute on a modern 4-core
-desktop system.
-
-To see a list of all the available tests, run:
-
-$ make check TESTSUITEFLAGS=--list
-
-To run only a subset of tests, e.g. test 123 and tests 477 through 484, run::
-
-$ make check TESTSUITEFLAGS='123 477-484'
-
-Tests do not have inter-dependencies, so you may run any subset.
-
-To run tests matching a keyword, e.g. ``ovsdb``, run::
-
-$ make check TESTSUITEFLAGS='-k ovsdb'
-
-To see a complete list of test options, run::
-
-$ make check TESTSUITEFLAGS=--help
-
-The results of a testing run are reported in ``tests/testsuite.log``. Report
-report test failures as bugs and include the ``testsuite.log`` in your report.
-
-.. note::
-  Sometimes a few tests may fail on some runs but not others. This is usually a
-  bug in the testsuite, not a bug in Open vSwitch itself. If you find that a
-  test fails intermittently, please report it, since the developers may not
-  have noticed. You can make the testsuite automatically rerun tests that fail,
-  by adding ``RECHECK=yes`` to the ``make`` command line, e.g.::
-
-  $ make check TESTSUITEFLAGS=-j8 RECHECK=yes
-
-Coverage
-
-
-If the build was configured with ``--enable-coverage`` and the ``lcov`` utility
-is installed, you can run the testsuite and generate a code coverage report by
-using the ``check-lcoc`` target::
-
-$ make check-lcov
-
-All the same options are avaiable via TESTSUITEFLAGS. For example::
-
-$ make check-lcov TESTSUITEFLAGS=-j8 -k ovn
-
-Valgrind
-
-
-If you have ``valgrind`` installed, you can run the testsuite under
-valgrind by using the ``check-valgrind`` target::
-
-$ make check-valgrind
-
-When you do this, the "valgrind" results for test  are reported in files
-named ``tests/testsuite.dir//valgrind.*``.
-
-All the same options are available via TESTSUITEFLAGS.
-
-.. hint::
-  You may find that the valgrind results are easier to interpret if you put
-  ``-q`` in ``~/.valgrindrc``, since that reduces the amount of output.
-
-.. _general-oftest:
-
-OFTest
-~~
-
-OFTest is an OpenFlow protocol testing suite. Open vSwitch includes a Makefile
-target to run OFTest with Open vSwitch in "dummy mode". In this mode of
-testing, no packets travel across physical or virtual networks.  Instead, Unix
-domain sockets stand in as simulated networks. This simulation is imperfect,
-but it is much easier to set up, does not require extra physical or virtual
-hardware, and does not require supervisor privileges.
-
-To run OFTest with Open vSwitch, first read and follow the instructions under
-**Testing** above. Second, obtain a copy of OFTest and install its
-prerequisites. You need a copy of OFTest that includes commit 

[ovs-dev] [PATCH v2 4/4] gitignore: Ignore venv

[Stephen Finucane]

This is the traditional name for Python virtualenv directories. I use
this when building docs to avoid installating system libraries.

Signed-off-by: Stephen Finucane 
---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index f61f772..4f50440 100644
--- a/.gitignore
+++ b/.gitignore
@@ -73,3 +73,4 @@ testsuite.tmp.orig
 /openvswitch*.tar.gz
 /tests/lcov/
 /Documentation/_build
+/.venv
-- 
2.9.3

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

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

[Michał Mirosław]

On Tue, Dec 13, 2016 at 05:21:18PM -0800, Stephen Hemminger wrote:
> On Sat,  3 Dec 2016 10:22:28 +0100 (CET)
> Michał Mirosław  wrote:
> 
> > This All-in-one patch removes abuse of VLAN CFI bit, so it can be passed
> > intact through linux networking stack.
> > 
> > Signed-off-by: Michał Mirosław 
> > ---
> > 
> > Dear NetDevs
> > 
> > I guess this needs to be split to the prep..convert[]..finish sequence,
> > but if you like it as is, then it's ready.
> > 
> > The biggest question is if the modified interface and vlan_present
> > is the way to go. This can be changed to use vlan_proto != 0 instead
> > of an extra flag bit.
> > 
> > As I can't test most of the driver changes, please look at them carefully.
> > OVS and bridge eyes are especially welcome.
> > 
> > Best Regards,
> > Michał Mirosław
> 
> Is the motivation to support 802.1ad Drop Eligability Indicator (DEI)?
> 
> If so then you need to be more verbose in the commit log, and lots more
> work is needed. You need to rename fields and validate every place a
> driver is using DEI bit to make sure it really does the right thing
> on that hardware. It is not just a mechanical change.

There are not many mentions of CFI bit in the Linux tree. Places that
used it as VLAN_TAG_PRESENT are fixed with this patchset. Other uses are:

 - VLAN code: ignored
 - ebt_vlan: ignored
 - OVS: cleared because of netlink API assumptions
 - DSA: transferred to/from (E)DSA tag
 - drivers: gianfar: uses properly in filtering rules
 - drivers: cnic: false-positive (uses only VLAN ID, CFI bit marks the field 
'valid')
 - drivers: qedr: false-positive (like cnic)

So unless there is something hidden in the hardware, no driver does anything
special with the CFI bit.

After this patchset only OVS will need further modifications to be able to
support handling of DEI bit.

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

Re: [ovs-dev] [PATCH 3/3] tests: Ignore options order in dhcpv4 ovn test.

[Balazs Nemeth]

>On 12/12/2016 13:14, "Ben Pfaff" > wrote:
>
>>On Thu, Dec 08, 2016 at 06:50:32PM -0800, Daniele Di Proietto wrote:
>>> The order of the options in the packet generated by ovs-controller
>>> depends on the hash function.  I believe that murmur hash (our default)
>>> produces different outputs depending on the endianness of the system.
>>>
>>> This commit fixes the test by reordering the options in the packet
>>> before checking them.
>>>
>>> This was reported before as a failure of the test on x86 with sse42
>>> enabled, I'm fixing it now because it affects other targets build by
>>> distros by default.
>>>
>>> Reported-at: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=840770
>>> Signed-off-by: Daniele Di Proietto 
>>> >
>>
>>I agree that this would solve the problem.
>>
>>I think that it might be nicer to instead sort the options in
>>build_dhcpv4_action() and build_dhcpv6_action() in ovn-northd.c.  Then
>>the generated logical actions won't differ from one architecture to
>>another, so that if we one day push a test to a higher level we won't
>>have an underlying source of differences.  There's even a helper
>>smap_sort() that could do most of the work.
>>
>>Are you willing to make that change?
>
>Sure, it's better than parsing TLVs with awk.  I'll send a v2.
>
>Thanks,
>
>Daniele
>
>>
>>Thanks,
>>
>>Ben.

Hi Daniele,

I applied this patch on top of branch-2.6 (which is including 1/3 and 2/3 
patch). I built OVS with SSE4.2 flag. UT 2246 is still failing due to only 
571st char of 'packets' and 'expected' files are compared and those are 
different in case of CRC32 hash. The following patch will fix the failing UT in 
case of build without any extra flag or with SSE4.2 as well:

diff --git a/tests/ovn.at b/tests/ovn.at
index 8b1228f..3f4c6cf 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -3724,8 +3724,8 @@ AT_CHECK([cat 1.packets | cut -c 53-570], [0], [expout])
# The ordering of the options in the packet depends on the hash function used
# and possibly on the endianness of the system.  We parse and sort both the
# expected and the generated packet.
-cat 1.expected | cut -c 571 | sort_dhcpv4opts_tlv > expout
-AT_CHECK([cat 1.packets | cut -c 571 | sort_dhcpv4opts_tlv], [0], [expout])
+cat 1.expected | cut -c 571- | sort_dhcpv4opts_tlv > expout
+AT_CHECK([cat 1.packets | cut -c 571- | sort_dhcpv4opts_tlv], [0], [expout])

# ovs-ofctl also resumes the packets and this causes other ports to receive
# the DHCP request packet. So reset the pcap files so that its easier to test.
@@ -3752,8 +3752,8 @@ AT_CHECK([cat 2.packets | cut -c 53-570], [0], [expout])
# The ordering of the options in the packet depends on the hash function used
# and possibly on the endianness of the system.  We parse and sort both the
# expected and the generated packet.
-cat 2.expected | cut -c 571 | sort_dhcpv4opts_tlv > expout
-AT_CHECK([cat 2.packets | cut -c 571 | sort_dhcpv4opts_tlv], [0], [expout])
+cat 2.expected | cut -c 571- | sort_dhcpv4opts_tlv > expout
+AT_CHECK([cat 2.packets | cut -c 571- | sort_dhcpv4opts_tlv], [0], [expout])

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

[ovs-dev] [PATCH v2 1/3] netdev-dpdk: add hotplug support

[Ciara Loftus]

In order to use dpdk ports in ovs they have to be bound to a DPDK
compatible driver before ovs is started.

This patch adds the possibility to hotplug (or hot-unplug) a device
after ovs has been started. The implementation adds two appctl commands:
netdev-dpdk/attach and netdev-dpdk/detach

After the user attaches a new device, it has to be added to a bridge
using the add-port command, similarly, before detaching a device,
it has to be removed using the del-port command.

Signed-off-by: Mauricio Vasquez B 
Signed-off-by: Ciara Loftus 
Co-authored-by: Ciara Loftus 
---
Changelog:
* Use xasprintf instead of snprintf when reporting attach/detach errors.
* Updated format of link in docs to comply with new format.
* Remove 'port-' from appctl netdev-dpdk/(port-)attach and detach commands
* Free 'response' in attach and detach functions in netdev_dpdk
* Add my Sign-off and Co-author tags

 Documentation/intro/install/dpdk-advanced.rst | 25 +++
 NEWS  |  1 +
 lib/netdev-dpdk.c | 95 +--
 3 files changed, 115 insertions(+), 6 deletions(-)

diff --git a/Documentation/intro/install/dpdk-advanced.rst 
b/Documentation/intro/install/dpdk-advanced.rst
index 44d1cd7..43acfc6 100644
--- a/Documentation/intro/install/dpdk-advanced.rst
+++ b/Documentation/intro/install/dpdk-advanced.rst
@@ -932,6 +932,31 @@ validate the suitability of different vSwitch 
implementations in a telco
 deployment environment. More information can be found on the `OPNFV wiki
 `__.
 
+Port Hotplug
+
+
+OvS supports port hotplugging, it allows to use ports that were not bound
+to DPDK when vswitchd was started.
+In order to attach a port, it has to be bound to DPDK using the
+dpdk_nic_bind.py script:
+
+   $ $DPDK_DIR/tools/dpdk_nic_bind.py --bind=igb_uio :01:00.0
+
+Then it can be attached to OVS:
+
+   $ ovs-appctl netdev-dpdk/attach :01:00.0
+
+At this point, the user can create a ovs port using the add-port command.
+
+It is also possible to detach a port from ovs, the user has to remove the
+port using the del-port command, then it can be detached using:
+
+   $ ovs-appctl netdev-dpdk/detach :01:00.0
+
+This feature is not supported with VFIO and could not work with some NICs.
+For more information please refer to the `DPDK Port Hotplug Framework
+`__.
+
 Bug Reporting
 -
 
diff --git a/NEWS b/NEWS
index 3a08dbc..b596cf3 100644
--- a/NEWS
+++ b/NEWS
@@ -41,6 +41,7 @@ Post-v2.6.0
  * New option 'n_rxq_desc' and 'n_txq_desc' fields for DPDK interfaces
which set the number of rx and tx descriptors to use for the given port.
  * Support for DPDK v16.11.
+ * Port Hotplug is now supported.
- Fedora packaging:
  * A package upgrade does not automatically restart OVS service.
- ovs-vswitchd/ovs-vsctl:
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 625f425..0d57227 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2307,6 +2307,82 @@ netdev_dpdk_set_admin_state(struct unixctl_conn *conn, 
int argc,
 unixctl_command_reply(conn, "OK");
 }
 
+static void
+netdev_dpdk_attach(struct unixctl_conn *conn, int argc OVS_UNUSED,
+   const char *argv[], void *aux OVS_UNUSED)
+{
+int ret;
+char *response;
+uint8_t port_id;
+
+ovs_mutex_lock(_mutex);
+
+ret = rte_eth_dev_attach(argv[1], _id);
+if (ret < 0) {
+response = xasprintf("Error attaching device '%s'", argv[1]);
+ovs_mutex_unlock(_mutex);
+unixctl_command_reply_error(conn, response);
+free(response);
+return;
+}
+
+response = xasprintf("Device '%s' has been attached as 'dpdk%d'",
+ argv[1], port_id);
+
+ovs_mutex_unlock(_mutex);
+unixctl_command_reply(conn, response);
+free(response);
+}
+
+static void
+netdev_dpdk_detach(struct unixctl_conn *conn, int argc OVS_UNUSED,
+   const char *argv[], void *aux OVS_UNUSED)
+{
+int ret;
+char *response;
+unsigned int parsed_port;
+uint8_t port_id;
+char devname[RTE_ETH_NAME_MAX_LEN];
+
+ovs_mutex_lock(_mutex);
+
+ret = dpdk_dev_parse_name(argv[1], "dpdk", _port);
+if (ret) {
+response = xasprintf("'%s' is not a valid port", argv[1]);
+goto error;
+}
+
+port_id = parsed_port;
+
+struct netdev *netdev = netdev_from_name(argv[1]);
+if (netdev) {
+netdev_close(netdev);
+response = xasprintf("Port '%s' is being used. Remove it before"
+ "detaching", argv[1]);
+goto error;
+}
+
+rte_eth_dev_close(port_id);
+
+ret = rte_eth_dev_detach(port_id, devname);
+if (ret < 0) {
+response = 

[ovs-dev] [PATCH v2 3/3] netdev-dpdk: Add support for virtual DPDK PMDs (vdevs)

[Ciara Loftus]

Prior to this commit, the 'dpdk' port type could only be used for
physical DPDK devices. Now, virtual devices (or 'vdevs') are supported.
'vdev' devices are those which use virtual DPDK Poll Mode Drivers eg.
null, pcap. To add a DPDK vdev, a valid 'dpdk-devargs' must be set for
the given dpdk port. The format expected is 'eth_' where
'x' is a number between 0 and RTE_MAX_ETHPORTS -1.

For example to add a port that uses the 'null' DPDK PMD driver:

ovs-vsctl set Interface null0 options:dpdk-devargs=eth_null0

Not all DPDK vdevs have been verified to work at this point in time.

Signed-off-by: Ciara Loftus 
---
Changelog:
* Updated process_vdevargs to work with Daniele's incremental in the
  previous patch.
* Allow vdev detach
* Update docs to show af_packet example

 Documentation/intro/install/dpdk-advanced.rst | 27 ++
 NEWS  |  1 +
 lib/netdev-dpdk.c | 28 ---
 vswitchd/vswitch.xml  |  9 +++--
 4 files changed, 43 insertions(+), 22 deletions(-)

diff --git a/Documentation/intro/install/dpdk-advanced.rst 
b/Documentation/intro/install/dpdk-advanced.rst
index 260ed59..7b8864f 100644
--- a/Documentation/intro/install/dpdk-advanced.rst
+++ b/Documentation/intro/install/dpdk-advanced.rst
@@ -956,6 +956,33 @@ This feature is not supported with VFIO and could not work 
with some NICs.
 For more information please refer to the `DPDK Port Hotplug Framework
 `__.
 
+Vdev Support
+
+
+DPDK provides drivers for both physical and virtual devices. Physical DPDK
+devices are added to OVS by specifying a valid PCI address in 'dpdk-devargs'.
+Virtual DPDK devices which do not have PCI addresses can be added using a
+different format for 'dpdk-devargs'.
+
+Typically, the format expected is 'eth_' where 'x' is a
+number between 0 and RTE_MAX_ETHPORTS -1 (31).
+
+For example to add a dpdk port that uses the 'null' DPDK PMD driver:
+
+   $ ovs-vsctl add-port br0 null0 -- set Interface null0 type=dpdk \
+   options:dpdk-devargs=eth_null0
+
+Similarly, to add a dpdk port that uses the 'af_packet' DPDK PMD driver:
+
+   $ ovs-vsctl add-port br0 af0 -- set Interface af0 type=dpdk \
+   options:dpdk-devargs=eth_af_packet0
+
+More information on the different types of virtual DPDK PMDs can be found in
+the `DPDK documentation
+`__.
+
+Note: Not all DPDK virtual PMD drivers have been tested and verified to work.
+
 Bug Reporting
 -
 
diff --git a/NEWS b/NEWS
index 8c7f38e..cb1b96c 100644
--- a/NEWS
+++ b/NEWS
@@ -47,6 +47,7 @@ Post-v2.6.0
with the old dpdk naming scheme is broken, and as such a
device will not be available for use until a valid dpdk-devargs is
specified.
+ * Virtual DPDK Poll Mode Driver (vdev PMD) support.
- Fedora packaging:
  * A package upgrade does not automatically restart OVS service.
- ovs-vswitchd/ovs-vsctl:
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 7d74d2f..167953f 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1096,21 +1096,16 @@ static int
 netdev_dpdk_process_devargs(const char *devargs)
 OVS_REQUIRES(dev->mutex)
 {
-struct rte_pci_addr addr;
 uint8_t new_port_id = UINT8_MAX;
 
-if (!eal_parse_pci_DomBDF(devargs, )) {
-/* Valid PCI address format detected - configure physical device */
-if (rte_eth_dev_get_port_by_name(devargs, _port_id)) {
-/* PCI device not found in DPDK, attempt to attach it */
-if (!rte_eth_dev_attach(devargs, _port_id)) {
-/* Attach successful */
-VLOG_INFO("Device "PCI_PRI_FMT" has been attached to DPDK",
-  addr.domain, addr.bus, addr.devid, addr.function);
-} else {
-/* Attach unsuccessful */
-return -1;
-}
+if (rte_eth_dev_get_port_by_name(devargs, _port_id)) {
+/* Virtual device not found in DPDK, attempt to attach it */
+if (!rte_eth_dev_attach(devargs, _port_id)) {
+/* Attach successful */
+VLOG_INFO("Device '%s' attached to DPDK", devargs);
+} else {
+/* Attach unsuccessful */
+return -1;
 }
 }
 
@@ -2398,17 +2393,10 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc 
OVS_UNUSED,
 char *response;
 uint8_t port_id;
 char devname[RTE_ETH_NAME_MAX_LEN];
-struct rte_pci_addr addr;
 struct netdev_dpdk *dev;
 
 ovs_mutex_lock(_mutex);
 
-if (eal_parse_pci_DomBDF(argv[1], )) {
-response = xasprintf("Invalid PCI address '%s'. Cannot detach.",
- argv[1]);
-goto error;
-}
-
 if (rte_eth_dev_get_port_by_name(argv[1], _id)) {
 response = 

[ovs-dev] Cómo Defendese del Fisco - En Línea

[Principales medios de defensa]

 

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

CÓMO DEFENDERSE DEL FISCO
28 de diciembre - Online en Vivo - 10:00 a 13:00 Hrs   
 
En un juicio ante el SAT, las probabilidades de que la autoridad gane son 
mayores. Pero hay mecanismos que le ayudan al contribuyente a cambiar la 
estadística. La sola posibilidad de enfrentar al fisco suena como una 
pesadilla, los dos principales temas de disputa con el SAT son la determinación 
de créditos fiscales, es decir, las multas y negativas a una devolución de 
impuestos, como IVA y el ISR. 
"Pregunte por nuestra Promoción Navideña"


Temario: 

1. Las Auditorías Fiscales y sus etapas.

2. Los principales medios de defensa.

3. Recurso de Revocación.

4. Juicio de lo Contencioso.

5. Juicio de Amparo.



...¡Y mucho más!


 
¿Requiere la información a la Brevedad?
responda este email con la palabra: 
Info - Fisco.
centro telefónico: 018002129393
 

Lic. Arturo López
Coordinador de Evento


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

 

 

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

[ovs-dev] [PATCH] ovn-northd: fix monitor process naming

[Lance Richardson]

Currently the ovn-northd monitor process and the ovn-northd process
have the same name, e.g. ps -ef | grep northd shows (edited for space):

... ovn-northd --detach --monitor --log-file=ovn-northd.log --pidfile
... ovn-northd --detach --monitor --log-file=ovn-northd.log --pidfile

With the call to ovs_cmdl_proctitle_init() added, we have:

... ovn-northd: monitoring pid 15662 (healthy)
... ovn-northd --detach --monitor --log-file=ovn-northd.log --pidfile

Signed-off-by: Lance Richardson 
---
 ovn/northd/ovn-northd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index c56ac79..a28327b 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -4913,6 +4913,7 @@ main(int argc, char *argv[])
 bool exiting;
 
 fatal_ignore_sigpipe();
+ovs_cmdl_proctitle_init(argc, argv);
 set_program_name(argv[0]);
 service_start(, );
 parse_options(argc, argv);
-- 
2.5.5

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

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

[Łukasz Rząsik]

Hi Ben,

Thank you for looking at the patch.

I was not worrying about this problem because I assumed it was already
there. A user could provide a very long list of integers and this would
affect memory in similar way. There is a limit for how long the list could
get, kernel's limit on size of arguments, so in reality the list would not
get that long but this is outside of OVS control.

Of course with the change it's much easier for a user to specify something
that will explode OVS memory.

I would propose three possible solutions, starting from the easiest and
simplest:
1. Specify an arbitrary limit for size of a range. If a user specifies a
range bigger than the limit, OVS will abort the command and inform the user
about the limit. Actually most of the columns in the database already have
a limit. Quickly looking through the schema I found only one without the
limit: cfm_remote_mpids in Interface table.
2. Internally split very big ranges into smaller ones and commit each range
separately.
3. Create a new struct, e.g. ovsdb_atom_range. Instead of adding every atom
separately to datum, add the struct, send it to the database and handle it
on the database side.

What do you think about it? Maybe I'm missing something? Do you have a
solution in mind?

Thanks,

Lucas

2016-12-13 1:17 GMT+01:00 Ben Pfaff :

> On Mon, Dec 05, 2016 at 09:47:47PM +0100, Łukasz Rząsik wrote:
> > Adding / removing a range of integers to a column accepting a set of
> > integers requires enumarating all of the integers. This patch simplifies
> > it by introducing 'range' concept to the database commands. Two integers
> > separated by a hyphen represent an inclusive range.
> >
> > The patch adds positive and negative tests for the new syntax.
> > The patch was tested by 'make check'. Covarage was tested by
> > 'make check-lcov'.
> >
> > Signed-off-by: Lukasz Rzasik 
> > Suggested-by: 
> > Suggested-by: Ben Pfaff 
>
> Thanks for contributing to Open vSwitch!
>
> This makes it easy for a user to explode OVS memory requirements.  For
> example, if I just write "1-10", it will use about 16 GB of
> memory in ovs-vsctl (maybe double or triple that!), transfer I guess
> about the same amount of data over the wire, and then write the same
> amount to disk at the database server.  Do you have an idea of how to
> avoid that kind of problem?
>
> Please use 4-space indents consistently.  I see some 2-space indents.
>
> Please use /* */ comments consistently.  I see some // comments.
>
> Thanks,
>
> Ben.
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC PATCHv2] ofp-actions: Add clone action.

[Ben Pfaff]

On Wed, Nov 30, 2016 at 01:35:49PM -0800, William Tu wrote:
> This patch adds OpenFlow clone action with syntax as below:
> "clone([action][,action...])".  The clone() action makes a copy of the
> current packet and executes the list of actions against the packet,
> without affecting the packet after the "clone(...)" action.  In other
> word, the packet before the clone() and after the clone() is the same,
> no matter what actions executed inside the clone().
> 
> The patch reuses the dapatah SAMPLE action, with probability of 100%.
> The kernel datapath 'sample()' clones the skb and its flow key under this
> circumstance, and actions specified in the clone() are executed against
> the cloned skb.  Note that there is no performance overhead of clone, since
> if eventually the packet is output, it will also clone the packet.  Here we
> simply move the copy at the beginning.
> 
> The complexity of adding clone might outweight its use cases.  I'm looking
> for comments as well as listing some cases below:
> Use case 1:
> Set different fields and output to different ports without unset
> actions=
>   clone(mod_dl_src:, output:1), clone(mod_dl_dst:, output:2), 
> output:3
> Since each clone() has independent packet, output:1 has only dl_src modified,
> output:2 has only dl_dst modified, output:3 has original packet.
> 
> Similar to case1
> actions=
>   push_vlan(...), output:2, pop_vlan, push_vlan(...), output:3
> can be changed to
> actions=
>   clone(push_vlan(...), output:2),clone(push_vlan(...), output:3)
> without having to add pop_vlan.
> 
> case 2: resubmit to another table without worrying packet being modified
>   actions=clone(resubmit(1,2)), ...

Using "sample" for the above use cases is unneeded, and it's undesirable
because it makes the translations bigger and slower.

> case 3: truncate in the clone action
> Currently OVS can truncate packet by 'output(port=1,max_len=100)', which
> ties truncate and output together.  However, sometimes the layer decides
> to truncate is separate from the layer to output.  One proposal is to
> introduce actions s.t like truncate_set() and truncate_unset(), where
> only the action in between sees truncated packet.  Another approach is
> to use clone() as below:
> actions=
>   clone(truncate(100), push_vlan, resubmit, ...)
> where we don't need to worry about missing the truncate_unset() because
> truncated packet is not visible outside the clone().

I see how "clone" helps with this conceptually, but I'm not sure why the
"sample" is necessary.  I think that the proposed value here is that
"sample" allows the truncate to be canceled if no output occurs after
"truncate" and before the end of the "sample" action.  But it's also
possible for the translation code to see whether there's an output
action within the clone and, if there is none, then to refrain from
emitting the datapath "truncate" action entirely.  That seems like a
more efficient way to implement it.  Will that work?

> We definitely should put some limit on the action types available inside
> clone(). For this patch, there is no restriction.

Why should we limit the actions available inside clone?

> Signed-off-by: William Tu 

This incremental is needed to avoid putting anything emitted by
xlate_commit_actions() into the middle of the sample action, otherwise
the OVN test cases segfault (after my patches are applied):

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 9bcefcd..c30f93b 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4296,8 +4296,6 @@ compose_clone_action(struct xlate_ctx *ctx,
 {
 struct flow old_flow, old_base_flow;
 size_t actions_offset;
-size_t sample_offset = nl_msg_start_nested(ctx->odp_actions,
-   OVS_ACTION_ATTR_SAMPLE);
 
 /* Ensure that any prior actions are applied. */
 xlate_commit_actions(ctx);
@@ -4307,6 +4305,8 @@ compose_clone_action(struct xlate_ctx *ctx,
 old_base_flow = ctx->base_flow;
 
 /* Sample with 100% Probability */
+size_t sample_offset = nl_msg_start_nested(ctx->odp_actions,
+   OVS_ACTION_ATTR_SAMPLE);
 nl_msg_put_u32(ctx->odp_actions, OVS_SAMPLE_ATTR_PROBABILITY, UINT32_MAX);
 actions_offset = nl_msg_start_nested(ctx->odp_actions,
  OVS_SAMPLE_ATTR_ACTIONS);

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

[ovs-dev] [PATCH] Documentation: fix some typos

[Lance Richardson]

s/deamon/daemon/
s/dependant/dependent/

Signed-off-by: Lance Richardson 
---
 Documentation/intro/install/dpdk-advanced.rst | 2 +-
 Documentation/topics/openflow.rst | 4 ++--
 lib/daemon.xml| 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/intro/install/dpdk-advanced.rst 
b/Documentation/intro/install/dpdk-advanced.rst
index 44d1cd7..f1481db 100644
--- a/Documentation/intro/install/dpdk-advanced.rst
+++ b/Documentation/intro/install/dpdk-advanced.rst
@@ -306,7 +306,7 @@ Generally, smaller queue sizes can have a positive impact 
for latency at the
 expense of throughput. The opposite is often true for larger queue sizes.
 Note: increasing the number of rx descriptors eg. to 4096  may have a negative
 impact on performance due to the fact that non-vectorised DPDK rx functions may
-be used. This is dependant on the driver in use, but is true for the commonly
+be used. This is dependent on the driver in use, but is true for the commonly
 used i40e and ixgbe DPDK drivers.
 
 Exact Match Cache
diff --git a/Documentation/topics/openflow.rst 
b/Documentation/topics/openflow.rst
index a2c22a8..7712467 100644
--- a/Documentation/topics/openflow.rst
+++ b/Documentation/topics/openflow.rst
@@ -164,7 +164,7 @@ mechanism with the ONF Experimenter ID.
 When defined integrated in 1.4, the feature use the standard OpenFlow
 structures (for example defined in openflow-1.4.h).
 
-The two definitions for each feature are independant and can exist in parallel
+The two definitions for each feature are independent and can exist in parallel
 in OVS.
 
 
@@ -303,7 +303,7 @@ mechanism with the ONF Experimenter ID.  When defined 
integrated in 1.5, the
 feature use the standard OpenFlow structures (for example defined in
 openflow-1.5.h).
 
-The two definitions for each feature are independant and can exist in parallel
+The two definitions for each feature are independent and can exist in parallel
 in OVS.
 
 * Time scheduled bundles
diff --git a/lib/daemon.xml b/lib/daemon.xml
index 737ae55..5cb447c 100644
--- a/lib/daemon.xml
+++ b/lib/daemon.xml
@@ -59,7 +59,7 @@
   --no-chdir
   
 
-  By default, when --detach is specified, the deamon changes
+  By default, when --detach is specified, the daemon changes
   its current working directory to the root directory after it detaches.
   Otherwise, invoking the daemon from a carelessly chosen directory would
   prevent the administrator from unmounting the file system that holds that
-- 
2.5.5

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

Re: [ovs-dev] [PATCH 2/4] trivial: Consistently indent HTML with 2 spaces

[Ben Pfaff]

On Wed, Dec 14, 2016 at 07:35:56PM +, Stephen Finucane wrote:
> On Wed, 2016-12-14 at 11:25 -0800, Ben Pfaff wrote:
> > Now, the background for the page is gray in Firefox and white in
> > Chromium/Chrome.  Maybe that comes from the default stylesheet,
> > because
> > at a glance I don't see anything in the CSS that sets the page
> > background.
> 
> They look identical in Chrome 55.0.2883.75 (x64) and Firefox 50.0.2
> (both evergreen) on Fedora. Perhaps it is a default. Mind sending on a
> screenshot and I'll submit another PR to force a default.

http://benpfaff.org/~blp/private/ab8st5Mp/screenshot.png

This is Firefox 45.5.0.

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

[ovs-dev] Courier was not able to deliver your parcel (ID1897831, USPS)

[USPS Delivery]

Dear Dev,

Your parcel was successfully delivered December 12 to USPS Station, but our 
courier cound not contact you.

Review the document that is attached to this e-mail!

Sincerely yours,
Manuel Tracy,
USPS Senior Station Manager.

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

Re: [ovs-dev] [PATCH v7] netdev-dpdk: Enable Rx checksum offloading feature on DPDK physical ports.

[Pravin Shelar]

Thanks for the patch. I have couple of questions.

On Wed, Dec 14, 2016 at 7:30 AM, Sugesh Chandran
 wrote:
> Add Rx checksum offloading feature support on DPDK physical ports. By default,
> the Rx checksum offloading is enabled if NIC supports. However,
> the checksum offloading can be turned OFF either while adding a new DPDK
> physical port to OVS or at runtime.
>
> The rx checksum offloading can be turned off by setting the parameter to
> 'false'. For eg: To disable the rx checksum offloading when adding a port,
>
>  'ovs-vsctl add-port br0 dpdk0 -- \
>   set Interface dpdk0 type=dpdk options:rx-checksum-offload=false'
>
> OR (to disable at run time after port is being added to OVS)
>
> 'ovs-vsctl set Interface dpdk0 options:rx-checksum-offload=false'
>
> Similarly to turn ON rx checksum offloading at run time,
> 'ovs-vsctl set Interface dpdk0 options:rx-checksum-offload=true'
>
> The Tx checksum offloading support is not implemented due to the following
> reasons.
>
> 1) Checksum offloading and vectorization are mutually exclusive in DPDK poll
> mode driver. Vector packet processing is turned OFF when checksum offloading
> is enabled which causes significant performance drop at Tx side.
>
> 2) Normally, OVS generates checksum for tunnel packets in software at the
> 'tunnel push' operation, where the tunnel headers are created. However
> enabling Tx checksum offloading involves,
>
> *) Mark every packets for tx checksum offloading at 'tunnel_push' and
> recirculate.
> *) At the time of xmit, validate the same flag and instruct the NIC to do the
> checksum calculation.  In case NIC doesnt support Tx checksum offloading,
> the checksum calculation has to be done in software before sending out the
> packets.
>
> No significant performance improvement noticed with Tx checksum offloading
> due to the e overhead of additional validations + non vector packet 
> processing.
> In some test scenarios, it introduces performance drop too.
>
> Rx checksum offloading still offers 8-9% of improvement on VxLAN tunneling
> decapsulation even though the SSE vector Rx function is disabled in DPDK poll
> mode driver.
>
> Signed-off-by: Sugesh Chandran 
> Acked-by: Jesse Gross 
>
...
...
>  static void
> +dpdk_eth_checksum_offload_configure(struct netdev_dpdk *dev)
> +OVS_REQUIRES(dev->mutex)
> +{
> +struct rte_eth_dev_info info;
> +bool rx_csum_ol_flag = false;
> +uint32_t rx_chksm_offload_capa = DEV_RX_OFFLOAD_UDP_CKSUM |
> + DEV_RX_OFFLOAD_TCP_CKSUM |
> + DEV_RX_OFFLOAD_IPV4_CKSUM;
> +rte_eth_dev_info_get(dev->port_id, );
> +rx_csum_ol_flag = (dev->hw_ol_features & NETDEV_RX_CHECKSUM_OFFLOAD) != 
> 0;
> +
> +if (rx_csum_ol_flag &&
> +(info.rx_offload_capa & rx_chksm_offload_capa) !=
> + rx_chksm_offload_capa) {
> +VLOG_WARN_ONCE("Failed to enable Rx checksum offload on device %d",
> +   dev->port_id);
> +dev->hw_ol_features &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
> +return;
> +}
> +netdev_request_reconfigure(>up);
> +}
> +
I am not sure about need for netdev reconfigure here.

> +static void
>  dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev) OVS_REQUIRES(dev->mutex)
>  {
>  if (rte_eth_dev_flow_ctrl_set(dev->port_id, >fc_conf)) {
> @@ -851,6 +884,9 @@ netdev_dpdk_init(struct netdev *netdev, unsigned int 
> port_no,
>
>  /* Initialize the flow control to NULL */
>  memset(>fc_conf, 0, sizeof dev->fc_conf);
> +
> +/* Initilize the hardware offload flags to 0 */
> +dev->hw_ol_features = 0;
>  if (type == DPDK_DEV_ETH) {
>  err = dpdk_eth_dev_init(dev);
>  if (err) {
> @@ -1118,6 +1154,8 @@ netdev_dpdk_set_config(struct netdev *netdev, const 
> struct smap *args)
>  {RTE_FC_NONE, RTE_FC_TX_PAUSE},
>  {RTE_FC_RX_PAUSE, RTE_FC_FULL}
>  };
> +bool rx_chksm_ofld;
> +bool temp_flag;
>
>  ovs_mutex_lock(>mutex);
>
> @@ -1141,6 +1179,15 @@ netdev_dpdk_set_config(struct netdev *netdev, const 
> struct smap *args)
>  dpdk_eth_flow_ctrl_setup(dev);
>  }
>
> +/* Rx checksum offload configuration */
> +/* By default the Rx checksum offload is ON */
> +rx_chksm_ofld = smap_get_bool(args, "rx-checksum-offload", true);
> +temp_flag = (dev->hw_ol_features & NETDEV_RX_CHECKSUM_OFFLOAD)
> +!= 0;
> +if (temp_flag != rx_chksm_ofld) {
> +dev->hw_ol_features ^= NETDEV_RX_CHECKSUM_OFFLOAD;
> +dpdk_eth_checksum_offload_configure(dev);
> +}
>  ovs_mutex_unlock(>mutex);
>
Can you also reflect this feature back to user via get-status.

>  return 0;
> diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
> index ce2582f..c730e72 100644
> --- a/lib/netdev-native-tnl.c
> +++ b/lib/netdev-native-tnl.c
> @@ -85,9 +85,11 @@ 

Re: [ovs-dev] [PATCH 2/4] trivial: Consistently indent HTML with 2 spaces

[Stephen Finucane]

On Wed, 2016-12-14 at 09:46 -0800, Ben Pfaff wrote:
> On Wed, Dec 14, 2016 at 09:27:41AM -0800, Ben Pfaff wrote:
> > On Wed, Dec 14, 2016 at 10:10:08AM +, Stephen Finucane wrote:
> > > On Tue, 2016-12-13 at 11:41 -0800, Ben Pfaff wrote:
> > > > On Tue, Dec 13, 2016 at 05:57:22PM +, Stephen Finucane
> > > > wrote:
> > > > > This will make upcoming changes easier to review.
> > > > > 
> > > > > Signed-off-by: Stephen Finucane 
> > > > 
> > > > "git am" tells me:
> > > > 
> > > > fatal: corrupt patch at line 397
> > > > Patch failed at 0001 trivial: Consistently indent HTML with
> > > > 2
> > > > spaces
> > > > 
> > > > Maybe a Github PR would work better, dunno.
> > > 
> > > I guess that warning I got about a line being greater than 998
> > > characters /wasn't/ safe to ignore :)
> > 
> > It is somewhat amusing that "git send-email" cleanly handles binary
> > files, but not text files with long lines.
> > 
> > > You can find the PR at the link below - it still applies cleanly
> > > afaict:
> > > 
> > >   https://github.com/openvswitch/openvswitch.github.io/pull/28
> > 
> > I applied it.  Thanks!
> 
> This totally screwed up the design:
> http://benpfaff.org/~blp/private/SS2lphsn/Screenshot%20-%2012
> 142016%20-%2009:29:08%20AM.png
> so I un-applied it, but it didn't help.  I have no idea why the
> webpage is still broken.

I'm guessing that GitHub Pages' Jekyll system might not be able to
handle force pushes. I've just pushed another version with the fixe (a
typo :() included. If you want to merge that, it should force the
rebuild.

Stephen
('sfinucan' on freenode)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] ofproto-dpif-xlate: Adding IGMP/MLD checksum verification

[Eelco Chaudron]

When IGMP or MLD packets arrive their content is used without the checksum
being verified. With this change the checksum is verified, and the packet
is not used for multicast snooping on failure.

Signed-off-by: Eelco Chaudron 
---
 lib/packets.c| 28 
 lib/packets.h|  2 ++
 ofproto/ofproto-dpif-xlate.c | 27 +++
 tests/mcast-snooping.at  | 37 +
 4 files changed, 94 insertions(+)

diff --git a/lib/packets.c b/lib/packets.c
index ae81f46..8075dda 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -1474,6 +1474,34 @@ packet_csum_pseudoheader6(const struct 
ovs_16aligned_ip6_hdr *ip6)
 
 return partial;
 }
+
+/* Calculate the IPv6 upper layer checksum according to RFC2460. We pass the
+   ip6_nxt and ip6_plen values, so it will also work if extension headers
+   are present. */
+uint16_t
+packet_csum_upperlayer6(const struct ovs_16aligned_ip6_hdr *ip6,
+const void *data, uint8_t l4_protocol,
+uint16_t l4_size)
+{
+uint32_t partial = 0;
+
+partial = csum_add32(partial, get_16aligned_be32(&(ip6->ip6_src.be32[0])));
+partial = csum_add32(partial, get_16aligned_be32(&(ip6->ip6_src.be32[1])));
+partial = csum_add32(partial, get_16aligned_be32(&(ip6->ip6_src.be32[2])));
+partial = csum_add32(partial, get_16aligned_be32(&(ip6->ip6_src.be32[3])));
+
+partial = csum_add32(partial, get_16aligned_be32(&(ip6->ip6_dst.be32[0])));
+partial = csum_add32(partial, get_16aligned_be32(&(ip6->ip6_dst.be32[1])));
+partial = csum_add32(partial, get_16aligned_be32(&(ip6->ip6_dst.be32[2])));
+partial = csum_add32(partial, get_16aligned_be32(&(ip6->ip6_dst.be32[3])));
+
+partial = csum_add16(partial, htons(l4_protocol));
+partial = csum_add16(partial, htons(l4_size));
+
+partial = csum_continue(partial, data, l4_size);
+
+return csum_finish(partial);
+}
 #endif
 
 void
diff --git a/lib/packets.h b/lib/packets.h
index 21bd35c..0bb6284 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -840,6 +840,8 @@ struct icmp6_header {
 BUILD_ASSERT_DECL(ICMP6_HEADER_LEN == sizeof(struct icmp6_header));
 
 uint32_t packet_csum_pseudoheader6(const struct ovs_16aligned_ip6_hdr *);
+uint16_t packet_csum_upperlayer6(const struct ovs_16aligned_ip6_hdr *,
+ const void *, uint8_t, uint16_t);
 
 /* Neighbor Discovery option field.
  * ND options are always a multiple of 8 bytes in size. */
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index d0f9a33..65921f0 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -30,6 +30,7 @@
 #include "cfm.h"
 #include "connmgr.h"
 #include "coverage.h"
+#include "csum.h"
 #include "dp-packet.h"
 #include "dpif.h"
 #include "in-band.h"
@@ -2021,9 +2022,20 @@ update_mcast_snooping_table4__(const struct xbridge 
*xbridge,
 OVS_REQ_WRLOCK(ms->rwlock)
 {
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(60, 30);
+const struct igmp_header *igmp;
 int count;
+size_t offset;
 ovs_be32 ip4 = flow->igmp_group_ip4;
 
+offset = (char *) dp_packet_l4(packet) - (char *) dp_packet_data(packet);
+igmp = dp_packet_at(packet, offset, IGMP_HEADER_LEN);
+if (!igmp || csum(igmp, dp_packet_l4_size(packet)) != 0) {
+VLOG_DBG_RL(, "bridge %s: multicast snooping received bad IGMP "
+"checksum on port %s in VLAN %d",
+xbridge->name, in_xbundle->name, vlan);
+return;
+}
+
 switch (ntohs(flow->tp_src)) {
 case IGMP_HOST_MEMBERSHIP_REPORT:
 case IGMPV2_HOST_MEMBERSHIP_REPORT:
@@ -2069,7 +2081,22 @@ update_mcast_snooping_table6__(const struct xbridge 
*xbridge,
 OVS_REQ_WRLOCK(ms->rwlock)
 {
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(60, 30);
+const struct mld_header *mld;
 int count;
+size_t offset;
+
+offset = (char *) dp_packet_l4(packet) - (char *) dp_packet_data(packet);
+mld = dp_packet_at(packet, offset, MLD_HEADER_LEN);
+
+if (!mld ||
+packet_csum_upperlayer6(dp_packet_l3(packet),
+mld, IPPROTO_ICMPV6,
+dp_packet_l4_size(packet)) != 0) {
+VLOG_DBG_RL(, "bridge %s: multicast snooping received bad MLD "
+"checksum on port %s in VLAN %d",
+xbridge->name, in_xbundle->name, vlan);
+return;
+}
 
 switch (ntohs(flow->tp_src)) {
 case MLD_QUERY:
diff --git a/tests/mcast-snooping.at b/tests/mcast-snooping.at
index 9cceace..c03aba3 100644
--- a/tests/mcast-snooping.at
+++ b/tests/mcast-snooping.at
@@ -63,5 +63,42 @@ AT_CHECK([cat p2.pcap.txt], [0], [dnl
 
01005e5e0101aa55aa550001810006bd0800451c401180710a01ef5e01011f48e63d
 ])
 
+# Clear the mdb, send a IGMP packet with invalid checksum and make sure it
+# 

Re: [ovs-dev] [PATCH 3/3] tests: Ignore options order in dhcpv4 ovn test.

[Daniele Di Proietto]

On 14/12/2016 06:38, "Balazs Nemeth"  wrote:

>>On 12/12/2016 13:14, "Ben Pfaff"  wrote:
>>
>>>On Thu, Dec 08, 2016 at 06:50:32PM -0800, Daniele Di Proietto wrote:
 The order of the options in the packet generated by ovs-controller
 depends on the hash function.  I believe that murmur hash (our default)
 produces different outputs depending on the endianness of the system.

 This commit fixes the test by reordering the options in the packet
 before checking them.

 This was reported before as a failure of the test on x86 with sse42
 enabled, I'm fixing it now because it affects other targets build by
 distros by default.

 Reported-at: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=840770
 Signed-off-by: Daniele Di Proietto 
>>>
>>>I agree that this would solve the problem.
>>>
>>>I think that it might be nicer to instead sort the options in
>>>build_dhcpv4_action() and build_dhcpv6_action() in ovn-northd.c.  Then
>>>the generated logical actions won't differ from one architecture to
>>>another, so that if we one day push a test to a higher level we won't
>>>have an underlying source of differences.  There's even a helper
>>>smap_sort() that could do most of the work.
>>>
>>>Are you willing to make that change?
>>
>>Sure, it's better than parsing TLVs with awk.  I'll send a v2.
>>
>>Thanks,
>>
>>Daniele
>>
>>>
>>>Thanks,
>>>
>>>Ben.
>
>
>Hi Daniele,
> 
>I applied this patch on top of branch-2.6 (which is including 1/3 and 2/3 
>patch). I built OVS with SSE4.2 flag. UT 2246 is still failing
> due to only 571st char of ‘packets’ and ‘expected’ files are compared and 
> those are different in case of CRC32 hash. The following patch will fix the 
> failing UT in case of build without any extra flag or with SSE4.2 as well:
> 
>diff --git a/tests/ovn.at b/tests/ovn.at
>index 8b1228f..3f4c6cf 100644
>--- a/tests/ovn.at
>+++ b/tests/ovn.at
>@@ -3724,8 +3724,8 @@ AT_CHECK([cat 1.packets | cut -c 53-570], [0], [expout])
># The ordering of the options in the packet depends on the hash function used
># and possibly on the endianness of the system.  We parse and sort both the
># expected and the generated packet.
>-cat 1.expected | cut -c 571 | sort_dhcpv4opts_tlv > expout
>-AT_CHECK([cat 1.packets | cut -c 571 | sort_dhcpv4opts_tlv], [0], [expout])
>+cat 1.expected | cut -c 571- | sort_dhcpv4opts_tlv > expout
>+AT_CHECK([cat 1.packets | cut -c 571- | sort_dhcpv4opts_tlv], [0], [expout])
> 
># ovs-ofctl also resumes the packets and this causes other ports to receive
># the DHCP request packet. So reset the pcap files so that its easier to test.
>@@ -3752,8 +3752,8 @@ AT_CHECK([cat 2.packets | cut -c 53-570], [0], [expout])
># The ordering of the options in the packet depends on the hash function used
># and possibly on the endianness of the system.  We parse and sort both the
># expected and the generated packet.
>-cat 2.expected | cut -c 571 | sort_dhcpv4opts_tlv > expout
>-AT_CHECK([cat 2.packets | cut -c 571 | sort_dhcpv4opts_tlv], [0], [expout])
>+cat 2.expected | cut -c 571- | sort_dhcpv4opts_tlv > expout
>+AT_CHECK([cat 2.packets | cut -c 571- | sort_dhcpv4opts_tlv], [0], [expout])
> 
>reset_pcap_file hv1-vif1 hv1/vif1
>reset_pcap_file hv1-vif2 hv1/vif2

Hi Balazs,

I think you're right, I was only cutting the 571st character, thanks for 
spotting the error.

Anyway, we decided to drop this patch, go with another approach and sort the 
options in the program, not in the test:

https://mail.openvswitch.org/pipermail/ovs-dev/2016-December/326133.html

I already merged this to master:

7c76bf4e0e27 ("ovn-northd: Sort options in put_dhcp(v6)_opts.")

and branch-2.6:

cbd39073245e ("ovn-northd: Sort options in put_dhcp(v6)_opts.")

Can you try latest master and branch-2.6 without any other patch applied?

It seems to pass for me on x86_64 x86_64+sse4.2 and mips.

Thanks,

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

Re: [ovs-dev] [PATCH 2/4] trivial: Consistently indent HTML with 2 spaces

[Stephen Finucane]

On Wed, 2016-12-14 at 11:25 -0800, Ben Pfaff wrote:
> On Wed, Dec 14, 2016 at 06:27:13PM +, Stephen Finucane wrote:
> > On Wed, 2016-12-14 at 10:06 -0800, Ben Pfaff wrote:
> > > On Wed, Dec 14, 2016 at 05:59:44PM +, Stephen Finucane wrote:
> > > > On Wed, 2016-12-14 at 09:46 -0800, Ben Pfaff wrote:
> > > > > On Wed, Dec 14, 2016 at 09:27:41AM -0800, Ben Pfaff wrote:
> > > > > > On Wed, Dec 14, 2016 at 10:10:08AM +, Stephen Finucane
> > > > > > wrote:
> > > > > > > On Tue, 2016-12-13 at 11:41 -0800, Ben Pfaff wrote:
> > > > > > > > On Tue, Dec 13, 2016 at 05:57:22PM +, Stephen
> > > > > > > > Finucane
> > > > > > > > wrote:
> > > > > > > > > This will make upcoming changes easier to review.
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Stephen Finucane 
> > > > > > > > 
> > > > > > > > "git am" tells me:
> > > > > > > > 
> > > > > > > > fatal: corrupt patch at line 397
> > > > > > > > Patch failed at 0001 trivial: Consistently indent
> > > > > > > > HTML
> > > > > > > > with
> > > > > > > > 2
> > > > > > > > spaces
> > > > > > > > 
> > > > > > > > Maybe a Github PR would work better, dunno.
> > > > > > > 
> > > > > > > I guess that warning I got about a line being greater
> > > > > > > than
> > > > > > > 998
> > > > > > > characters /wasn't/ safe to ignore :)
> > > > > > 
> > > > > > It is somewhat amusing that "git send-email" cleanly
> > > > > > handles
> > > > > > binary
> > > > > > files, but not text files with long lines.
> > > > > > 
> > > > > > > You can find the PR at the link below - it still applies
> > > > > > > cleanly
> > > > > > > afaict:
> > > > > > > 
> > > > > > >   https://github.com/openvswitch/openvswitch.github.io/pu
> > > > > > > ll/2
> > > > > > > 8
> > > > > > 
> > > > > > I applied it.  Thanks!
> > > > > 
> > > > > This totally screwed up the design:
> > > > > http://benpfaff.org/~blp/private/SS2lphsn/Screenshot%
> > > > > 20-%
> > > > > 2012
> > > > > 142016%20-%2009:29:08%20AM.png
> > > > > so I un-applied it, but it didn't help.  I have no idea why
> > > > > the
> > > > > webpage is still broken.
> > > > 
> > > > I'm guessing that GitHub Pages' Jekyll system might not be able
> > > > to
> > > > handle force pushes. I've just pushed another version with the
> > > > fixe
> > > > (a
> > > > typo :() included. If you want to merge that, it should force
> > > > the
> > > > rebuild.
> > > > 
> > > > Stephen
> > > > ('sfinucan' on freenode)
> > > 
> > > OK, I pushed it.
> > 
> > Still broken on my end, so I've submitted another PR:
> > 
> >   https://github.com/openvswitch/openvswitch.github.io/pull/32
> > 
> > I've no idea why it's happening, but GitHub appears to be
> > substituting
> > our 'style.css' with one based on normalize.css. This change should
> > workaround this peculiarity until I can figure out what on earth is
> > happening.
> 
> OK, that fixed the problem.

Yeah, I think that's actually a problem with GitHub pages. All the
tutorials I've looked at place css in '/css' and don't bother with the
'{{site}}'. I'll wait a while before potentially breaking anything
again.

> Now, the background for the page is gray in Firefox and white in
> Chromium/Chrome.  Maybe that comes from the default stylesheet,
> because
> at a glance I don't see anything in the CSS that sets the page
> background.

They look identical in Chrome 55.0.2883.75 (x64) and Firefox 50.0.2
(both evergreen) on Fedora. Perhaps it is a default. Mind sending on a
screenshot and I'll submit another PR to force a default.

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

Re: [ovs-dev] [PATCH v2] ovn-controller: Fix duplicated flows in table 32.

[Han Zhou]

On Wed, Dec 14, 2016 at 9:36 AM, Darrell Ball  wrote:
>
>
>
> On 12/13/16, 11:15 PM, "ovs-dev-boun...@openvswitch.org on behalf of Han
Zhou" 
wrote:
>
> In commit 475f0a2c it introduced a priority 150 flow for filtering
> the sending of traffic received from vxlan tunnels back out tunnels.
> However, it added the flow for every remote port processing, which
> results in continuous logs about duplicated flows. We only need to
> install this flow once per physical_run() loop iteration.
>
> All flows are attempted to be installed each iteration of physical_run()
> We filter those by keeping context (installed_flows) across physical_run()
> iterations.
> In a given iteration of physical_run(), we also check for duplicated flow
> attempts (desired_flows) and treat those as info logs.
>
> Fix duplicated flows in table32.
> should be
> Fix duplicated flow add attempts in table 32.
>
> However, it added the flow for every remote port processing, which
> results in continuous logs about duplicated flows. We only need to
> install this flow once per physical_run() loop iteration.
>
> should be
>
> However, it attempted to add the flow for every remote port processing,
> which results in continuous logs about duplicated flow add attempts. We
> only need to attempt to install this flow once per physical_run() loop
iteration.
>
Agree. This is more accurate.

> I remember at some point we had lots of duplicated flow logs and hence
> maybe ignored them. At least for some tests we don’t see those anymore.
>
> Perhaps we should update one test to check for duplicated flow
ovn-controller logs ?
> The vtep test could be one such test since those logs seem clean now ?
> What do you think ?
>
Thanks for your suggestion. I agree it is better to have it covered in
automation. I just wonder how can we tell whether the duplication is
unexpected or just some normal case, if not all duplicates are unexpected.
If we don't expect any duplicated flows at all, we should change the log
level to ERROR, and then in test cases we just need to make sure there is
no error log.

>
> Signed-off-by: Han Zhou 
> Acked-by: Darrell Ball 
> ---
>
> Notes:
> v1 -> v2: update commit message according to Darrell's comments.
>
>  ovn/controller/physical.c | 47
---
>  1 file changed, 24 insertions(+), 23 deletions(-)
>
> diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
> index 48adb78..3b653dd 100644
> --- a/ovn/controller/physical.c
> +++ b/ovn/controller/physical.c
> @@ -465,33 +465,16 @@ consider_port_binding(enum mf_field_id
mff_ovn_geneve,
>  } else {
>  /* Remote port connected by tunnel */
>
> -/* Table 32, priority 150 and 100.
> +/* Table 32, priority 100.
>   * ===
>   *
> - * Priority 150 is for packets received from a VXLAN tunnel
> - * which get resubmitted to OFTABLE_LOG_INGRESS_PIPELINE due
to
> - * lack of needed metadata in VXLAN, explicitly skip sending
> - * back out any tunnels and resubmit to table 33 for local
> - * delivery.
> - *
> - * Priority 100 is for all other traffic which need to be
sent
> - * to a remote hypervisor.  Each flow matches an output port
> - * that includes a logical port on a remote hypervisor, and
> - * tunnels the packet to that hypervisor.
> + * Priority 100 is for traffic that needs to be sent to a
remote
> + * hypervisor.  Each flow matches an output port that
includes a
> + * logical port on a remote hypervisor, and tunnels the
packet to
> + * that hypervisor.
>   */
>  match_init_catchall();
>  ofpbuf_clear(ofpacts_p);
> -match_set_reg_masked(, MFF_LOG_FLAGS - MFF_REG0,
> - MLF_RCV_FROM_VXLAN, MLF_RCV_FROM_VXLAN);
> -
> -/* Resubmit to table 33. */
> -put_resubmit(OFTABLE_LOCAL_OUTPUT, ofpacts_p);
> -ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 150,
,
> -ofpacts_p);
> -
> -
> -match_init_catchall();
> -ofpbuf_clear(ofpacts_p);
>
>  /* Match MFF_LOG_DATAPATH, MFF_LOG_OUTPORT. */
>  match_set_metadata(, htonll(dp_key));
> @@ -870,12 +853,30 @@ physical_run(struct controller_ctx *ctx, enum
mf_field_id mff_ovn_geneve,
>  }
>  }
>
> +/* Table 32, priority 150.
> + * ===
> + *
> + * Priority 150 is for packets received from a VXLAN tunnel
> + * which get resubmitted to OFTABLE_LOG_INGRESS_PIPELINE due to

Re: [ovs-dev] [PATCH v2] ovn-controller: Fix duplicated flows in table 32.

[Darrell Ball]

From: Han Zhou 
Date: Wednesday, December 14, 2016 at 11:53 AM
To: Darrell Ball 
Cc: "d...@openvswitch.org" 
Subject: Re: [ovs-dev] [PATCH v2] ovn-controller: Fix duplicated flows in table 
32.



On Wed, Dec 14, 2016 at 9:36 AM, Darrell Ball 
> wrote:
>
>
>
> On 12/13/16, 11:15 PM, 
> "ovs-dev-boun...@openvswitch.org on 
> behalf of Han Zhou" 
>  on 
> behalf of zhou...@gmail.com> wrote:
>
> In commit 475f0a2c it introduced a priority 150 flow for filtering
> the sending of traffic received from vxlan tunnels back out tunnels.
> However, it added the flow for every remote port processing, which
> results in continuous logs about duplicated flows. We only need to
> install this flow once per physical_run() loop iteration.
>
> All flows are attempted to be installed each iteration of physical_run()
> We filter those by keeping context (installed_flows) across physical_run()
> iterations.
> In a given iteration of physical_run(), we also check for duplicated flow
> attempts (desired_flows) and treat those as info logs.
>
> Fix duplicated flows in table32.
> should be
> Fix duplicated flow add attempts in table 32.
>
> However, it added the flow for every remote port processing, which
> results in continuous logs about duplicated flows. We only need to
> install this flow once per physical_run() loop iteration.
>
> should be
>
> However, it attempted to add the flow for every remote port processing,
> which results in continuous logs about duplicated flow add attempts. We
> only need to attempt to install this flow once per physical_run() loop 
> iteration.
>
Agree. This is more accurate.

> I remember at some point we had lots of duplicated flow logs and hence
> maybe ignored them. At least for some tests we don’t see those anymore.
>
> Perhaps we should update one test to check for duplicated flow ovn-controller 
> logs ?
> The vtep test could be one such test since those logs seem clean now ?
> What do you think ?
>
Thanks for your suggestion. I agree it is better to have it covered in 
automation. I just wonder how can we tell whether the duplication is unexpected 
or just some normal case, if not all duplicates are unexpected. If we don't 
expect any duplicated flows at all, we should change the log level to ERROR, 
and then in test cases we just need to make sure there is no error log.

ERROR log level is not appropriate since we always filter adding the extra 
flows anyways
and it amount to extra processing and filtering.
We anyways attempt to re-add every flow for every iteration of the 
ovn-controller main loop,
not just physical flows; we filter these attempts with “installed_flows”, so we 
are doing lots
of extra processing normally, at this time.

With the vtep test, the non-physical flows are not as many and diverse. So 
checking
for attempting to add the same flow in the same iteration of ovn-controller 
main loop
where the desired_flows log check is made (which is before the installed_flows 
check)
should help to catch the physical flow duplicate attempts in a given iteration 
of the main loop.

If we added the check there, we would be trying to catch some portion of the 
unnecessary extra
processing and filtering which seems valid and avoid noisy logs, which could 
obscure other logs ?

>
> Signed-off-by: Han Zhou >
> Acked-by: Darrell Ball >
> ---
>
> Notes:
> v1 -> v2: update commit message according to Darrell's comments.
>
>  ovn/controller/physical.c | 47 
> ---
>  1 file changed, 24 insertions(+), 23 deletions(-)
>
> diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
> index 48adb78..3b653dd 100644
> --- a/ovn/controller/physical.c
> +++ b/ovn/controller/physical.c
> @@ -465,33 +465,16 @@ consider_port_binding(enum mf_field_id 
> mff_ovn_geneve,
>  } else {
>  /* Remote port connected by tunnel */
>
> -/* Table 32, priority 150 and 100.
> +/* Table 32, priority 100.
>   * ===
>   *
> - * Priority 150 is for packets received from a VXLAN tunnel
> - * which get resubmitted to OFTABLE_LOG_INGRESS_PIPELINE due to
> - * lack of needed metadata in VXLAN, explicitly skip sending
> - * back out any tunnels and resubmit to table 33 for local
> - * delivery.
> - *
> - * Priority 100 is for all other traffic which need to be sent
> - * to a remote hypervisor.  Each flow matches an output port
> - * that includes a logical port on a remote 

Re: [ovs-dev] [PATCH 2/4] trivial: Consistently indent HTML with 2 spaces

[Ben Pfaff]

On Wed, Dec 14, 2016 at 06:27:13PM +, Stephen Finucane wrote:
> On Wed, 2016-12-14 at 10:06 -0800, Ben Pfaff wrote:
> > On Wed, Dec 14, 2016 at 05:59:44PM +, Stephen Finucane wrote:
> > > On Wed, 2016-12-14 at 09:46 -0800, Ben Pfaff wrote:
> > > > On Wed, Dec 14, 2016 at 09:27:41AM -0800, Ben Pfaff wrote:
> > > > > On Wed, Dec 14, 2016 at 10:10:08AM +, Stephen Finucane
> > > > > wrote:
> > > > > > On Tue, 2016-12-13 at 11:41 -0800, Ben Pfaff wrote:
> > > > > > > On Tue, Dec 13, 2016 at 05:57:22PM +, Stephen Finucane
> > > > > > > wrote:
> > > > > > > > This will make upcoming changes easier to review.
> > > > > > > > 
> > > > > > > > Signed-off-by: Stephen Finucane 
> > > > > > > 
> > > > > > > "git am" tells me:
> > > > > > > 
> > > > > > > fatal: corrupt patch at line 397
> > > > > > > Patch failed at 0001 trivial: Consistently indent HTML
> > > > > > > with
> > > > > > > 2
> > > > > > > spaces
> > > > > > > 
> > > > > > > Maybe a Github PR would work better, dunno.
> > > > > > 
> > > > > > I guess that warning I got about a line being greater than
> > > > > > 998
> > > > > > characters /wasn't/ safe to ignore :)
> > > > > 
> > > > > It is somewhat amusing that "git send-email" cleanly handles
> > > > > binary
> > > > > files, but not text files with long lines.
> > > > > 
> > > > > > You can find the PR at the link below - it still applies
> > > > > > cleanly
> > > > > > afaict:
> > > > > > 
> > > > > >   https://github.com/openvswitch/openvswitch.github.io/pull/2
> > > > > > 8
> > > > > 
> > > > > I applied it.  Thanks!
> > > > 
> > > > This totally screwed up the design:
> > > > http://benpfaff.org/~blp/private/SS2lphsn/Screenshot%20-%
> > > > 2012
> > > > 142016%20-%2009:29:08%20AM.png
> > > > so I un-applied it, but it didn't help.  I have no idea why the
> > > > webpage is still broken.
> > > 
> > > I'm guessing that GitHub Pages' Jekyll system might not be able to
> > > handle force pushes. I've just pushed another version with the fixe
> > > (a
> > > typo :() included. If you want to merge that, it should force the
> > > rebuild.
> > > 
> > > Stephen
> > > ('sfinucan' on freenode)
> > 
> > OK, I pushed it.
> 
> Still broken on my end, so I've submitted another PR:
> 
>   https://github.com/openvswitch/openvswitch.github.io/pull/32
> 
> I've no idea why it's happening, but GitHub appears to be substituting
> our 'style.css' with one based on normalize.css. This change should
> workaround this peculiarity until I can figure out what on earth is
> happening.

OK, that fixed the problem.

Now, the background for the page is gray in Firefox and white in
Chromium/Chrome.  Maybe that comes from the default stylesheet, because
at a glance I don't see anything in the CSS that sets the page
background.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] travis: Update build matrix for latest kernels.

[Joe Stringer]

On 13 December 2016 at 20:32, Pravin Shelar  wrote:
> On Tue, Dec 13, 2016 at 4:00 PM, Joe Stringer  wrote:
>> Signed-off-by: Joe Stringer 
>
> LGTM.
>
> Acked-by: Pravin B Shelar 

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

Re: [ovs-dev] [PATCH 2/4] trivial: Consistently indent HTML with 2 spaces

[Ben Pfaff]

On Wed, Dec 14, 2016 at 05:59:44PM +, Stephen Finucane wrote:
> On Wed, 2016-12-14 at 09:46 -0800, Ben Pfaff wrote:
> > On Wed, Dec 14, 2016 at 09:27:41AM -0800, Ben Pfaff wrote:
> > > On Wed, Dec 14, 2016 at 10:10:08AM +, Stephen Finucane wrote:
> > > > On Tue, 2016-12-13 at 11:41 -0800, Ben Pfaff wrote:
> > > > > On Tue, Dec 13, 2016 at 05:57:22PM +, Stephen Finucane
> > > > > wrote:
> > > > > > This will make upcoming changes easier to review.
> > > > > > 
> > > > > > Signed-off-by: Stephen Finucane 
> > > > > 
> > > > > "git am" tells me:
> > > > > 
> > > > > fatal: corrupt patch at line 397
> > > > > Patch failed at 0001 trivial: Consistently indent HTML with
> > > > > 2
> > > > > spaces
> > > > > 
> > > > > Maybe a Github PR would work better, dunno.
> > > > 
> > > > I guess that warning I got about a line being greater than 998
> > > > characters /wasn't/ safe to ignore :)
> > > 
> > > It is somewhat amusing that "git send-email" cleanly handles binary
> > > files, but not text files with long lines.
> > > 
> > > > You can find the PR at the link below - it still applies cleanly
> > > > afaict:
> > > > 
> > > >   https://github.com/openvswitch/openvswitch.github.io/pull/28
> > > 
> > > I applied it.  Thanks!
> > 
> > This totally screwed up the design:
> > http://benpfaff.org/~blp/private/SS2lphsn/Screenshot%20-%2012
> > 142016%20-%2009:29:08%20AM.png
> > so I un-applied it, but it didn't help.  I have no idea why the
> > webpage is still broken.
> 
> I'm guessing that GitHub Pages' Jekyll system might not be able to
> handle force pushes. I've just pushed another version with the fixe (a
> typo :() included. If you want to merge that, it should force the
> rebuild.
> 
> Stephen
> ('sfinucan' on freenode)

OK, I pushed it.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/4] trivial: Consistently indent HTML with 2 spaces

[Stephen Finucane]

On Wed, 2016-12-14 at 10:06 -0800, Ben Pfaff wrote:
> On Wed, Dec 14, 2016 at 05:59:44PM +, Stephen Finucane wrote:
> > On Wed, 2016-12-14 at 09:46 -0800, Ben Pfaff wrote:
> > > On Wed, Dec 14, 2016 at 09:27:41AM -0800, Ben Pfaff wrote:
> > > > On Wed, Dec 14, 2016 at 10:10:08AM +, Stephen Finucane
> > > > wrote:
> > > > > On Tue, 2016-12-13 at 11:41 -0800, Ben Pfaff wrote:
> > > > > > On Tue, Dec 13, 2016 at 05:57:22PM +, Stephen Finucane
> > > > > > wrote:
> > > > > > > This will make upcoming changes easier to review.
> > > > > > > 
> > > > > > > Signed-off-by: Stephen Finucane 
> > > > > > 
> > > > > > "git am" tells me:
> > > > > > 
> > > > > > fatal: corrupt patch at line 397
> > > > > > Patch failed at 0001 trivial: Consistently indent HTML
> > > > > > with
> > > > > > 2
> > > > > > spaces
> > > > > > 
> > > > > > Maybe a Github PR would work better, dunno.
> > > > > 
> > > > > I guess that warning I got about a line being greater than
> > > > > 998
> > > > > characters /wasn't/ safe to ignore :)
> > > > 
> > > > It is somewhat amusing that "git send-email" cleanly handles
> > > > binary
> > > > files, but not text files with long lines.
> > > > 
> > > > > You can find the PR at the link below - it still applies
> > > > > cleanly
> > > > > afaict:
> > > > > 
> > > > >   https://github.com/openvswitch/openvswitch.github.io/pull/2
> > > > > 8
> > > > 
> > > > I applied it.  Thanks!
> > > 
> > > This totally screwed up the design:
> > > http://benpfaff.org/~blp/private/SS2lphsn/Screenshot%20-%
> > > 2012
> > > 142016%20-%2009:29:08%20AM.png
> > > so I un-applied it, but it didn't help.  I have no idea why the
> > > webpage is still broken.
> > 
> > I'm guessing that GitHub Pages' Jekyll system might not be able to
> > handle force pushes. I've just pushed another version with the fixe
> > (a
> > typo :() included. If you want to merge that, it should force the
> > rebuild.
> > 
> > Stephen
> > ('sfinucan' on freenode)
> 
> OK, I pushed it.

Still broken on my end, so I've submitted another PR:

  https://github.com/openvswitch/openvswitch.github.io/pull/32

I've no idea why it's happening, but GitHub appears to be substituting
our 'style.css' with one based on normalize.css. This change should
workaround this peculiarity until I can figure out what on earth is
happening.

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

Re: [ovs-dev] [PATCH 3/3] system-traffic: Skip test cases if firewalld is on.

[Joe Stringer]

On 8 December 2016 at 18:34, Yi-Hung Wei  wrote:
> On RHEL 7.3, test cases that use vxlan, gre, and geneve tunnels fail because
> traffic is blocked by default firewall configuration. This commit detects the
> status of firewalld, and skips the tests if firewalld is on.
>
> Signed-off-by: Yi-Hung Wei 

This macro could be made a bit smarter - perhaps by standing up a
couple of namespaces and running traffic through to find out if a
firewall is interfering - but that could be introduced in a followup
patch.

I applied this to master anyway, since it's better than what we had before.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/3] datapath: compat: Fix build on RHEL 7.3

[Joe Stringer]

On 8 December 2016 at 18:34, Yi-Hung Wei  wrote:
> RHEL 7.3 provides upstream tunnel but it does not support name_assign_type
> attribute in net-device. This patch fixes the build problem by backporting
> functions with name_assign_type, and using proper flags in acinclude.m4 to
> invoke backport functions.
>
> Tested on RHEL 7.3 with kernel 3.10.0-514.el7.x86_64
>
> Signed-off-by: Yi-Hung Wei 

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

Re: [ovs-dev] [RFC PATCHv2] ofp-actions: Add clone action.

[William Tu]

Thanks for the feedback.

>> actions=
>>   clone(truncate(100), push_vlan, resubmit, ...)
>> where we don't need to worry about missing the truncate_unset() because
>> truncated packet is not visible outside the clone().
>
> I see how "clone" helps with this conceptually, but I'm not sure why the
> "sample" is necessary.  I think that the proposed value here is that
> "sample" allows the truncate to be canceled if no output occurs after
> "truncate" and before the end of the "sample" action.  But it's also
> possible for the translation code to see whether there's an output
> action within the clone and, if there is none, then to refrain from
> emitting the datapath "truncate" action entirely.  That seems like a
> more efficient way to implement it.  Will that work?
>

Yes that will work, but we have to add a new datapath clone action.
The reason I use "sample" is try not to add another new datapath
action to kernel.  Current "sample" action happens to provide a list
of actions to execute, so I repurpose it to clone.

>> We definitely should put some limit on the action types available inside
>> clone(). For this patch, there is no restriction.
>
> Why should we limit the actions available inside clone?
>
Currently there is a limit of max 10 actions to execute inside sample.
"#define DEFERRED_ACTION_FIFO_SIZE 10".  So I assume we need to limit
the number or bump up this value.  Other than this, I'm not sure
whether any action can put inside clone, for example some nested case:
clone(clone(sample(...))).

>> Signed-off-by: William Tu 
>
> This incremental is needed to avoid putting anything emitted by
> xlate_commit_actions() into the middle of the sample action, otherwise
> the OVN test cases segfault (after my patches are applied):
>
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 9bcefcd..c30f93b 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -4296,8 +4296,6 @@ compose_clone_action(struct xlate_ctx *ctx,
>  {
>  struct flow old_flow, old_base_flow;
>  size_t actions_offset;
> -size_t sample_offset = nl_msg_start_nested(ctx->odp_actions,
> -   OVS_ACTION_ATTR_SAMPLE);
>
>  /* Ensure that any prior actions are applied. */
>  xlate_commit_actions(ctx);
> @@ -4307,6 +4305,8 @@ compose_clone_action(struct xlate_ctx *ctx,
>  old_base_flow = ctx->base_flow;
>
>  /* Sample with 100% Probability */
> +size_t sample_offset = nl_msg_start_nested(ctx->odp_actions,
> +   OVS_ACTION_ATTR_SAMPLE);
>  nl_msg_put_u32(ctx->odp_actions, OVS_SAMPLE_ATTR_PROBABILITY, 
> UINT32_MAX);
>  actions_offset = nl_msg_start_nested(ctx->odp_actions,
>   OVS_SAMPLE_ATTR_ACTIONS);
>
Thanks for the fix.
William
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC PATCHv2] ofp-actions: Add clone action.

[Ben Pfaff]

On Wed, Dec 14, 2016 at 06:41:46PM -0800, William Tu wrote:
> >> actions=
> >>   clone(truncate(100), push_vlan, resubmit, ...)
> >> where we don't need to worry about missing the truncate_unset() because
> >> truncated packet is not visible outside the clone().
> >
> > I see how "clone" helps with this conceptually, but I'm not sure why the
> > "sample" is necessary.  I think that the proposed value here is that
> > "sample" allows the truncate to be canceled if no output occurs after
> > "truncate" and before the end of the "sample" action.  But it's also
> > possible for the translation code to see whether there's an output
> > action within the clone and, if there is none, then to refrain from
> > emitting the datapath "truncate" action entirely.  That seems like a
> > more efficient way to implement it.  Will that work?
> 
> Yes that will work, but we have to add a new datapath clone action.
> The reason I use "sample" is try not to add another new datapath
> action to kernel.  Current "sample" action happens to provide a list
> of actions to execute, so I repurpose it to clone.

I still don't understand.

Here are some examples of translations from OpenFlow to datapath
actions, the way I would expect them to happen:

OF: 1, clone(truncate(100), push_vlan, 2), 3
dp: 1, truncate(100), push_vlan, 2, pop_vlan, 3

OF: 1, clone(truncate(100), push_vlan), 2
dp: 1, 2

Where does the sample action help?

Thanks,

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

Re: [ovs-dev] [PATCH 2/4] trivial: Consistently indent HTML with 2 spaces

[Ben Pfaff]

On Wed, Dec 14, 2016 at 10:10:08AM +, Stephen Finucane wrote:
> On Tue, 2016-12-13 at 11:41 -0800, Ben Pfaff wrote:
> > On Tue, Dec 13, 2016 at 05:57:22PM +, Stephen Finucane wrote:
> > > This will make upcoming changes easier to review.
> > > 
> > > Signed-off-by: Stephen Finucane 
> > 
> > "git am" tells me:
> > 
> > fatal: corrupt patch at line 397
> > Patch failed at 0001 trivial: Consistently indent HTML with 2
> > spaces
> > 
> > Maybe a Github PR would work better, dunno.
> 
> I guess that warning I got about a line being greater than 998
> characters /wasn't/ safe to ignore :)

It is somewhat amusing that "git send-email" cleanly handles binary
files, but not text files with long lines.

> You can find the PR at the link below - it still applies cleanly
> afaict:
> 
>   https://github.com/openvswitch/openvswitch.github.io/pull/28

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

Re: [ovs-dev] [PATCH 2/4] trivial: Consistently indent HTML with 2 spaces

[Ben Pfaff]

On Wed, Dec 14, 2016 at 09:27:41AM -0800, Ben Pfaff wrote:
> On Wed, Dec 14, 2016 at 10:10:08AM +, Stephen Finucane wrote:
> > On Tue, 2016-12-13 at 11:41 -0800, Ben Pfaff wrote:
> > > On Tue, Dec 13, 2016 at 05:57:22PM +, Stephen Finucane wrote:
> > > > This will make upcoming changes easier to review.
> > > > 
> > > > Signed-off-by: Stephen Finucane 
> > > 
> > > "git am" tells me:
> > > 
> > > fatal: corrupt patch at line 397
> > > Patch failed at 0001 trivial: Consistently indent HTML with 2
> > > spaces
> > > 
> > > Maybe a Github PR would work better, dunno.
> > 
> > I guess that warning I got about a line being greater than 998
> > characters /wasn't/ safe to ignore :)
> 
> It is somewhat amusing that "git send-email" cleanly handles binary
> files, but not text files with long lines.
> 
> > You can find the PR at the link below - it still applies cleanly
> > afaict:
> > 
> >   https://github.com/openvswitch/openvswitch.github.io/pull/28
> 
> I applied it.  Thanks!

This totally screwed up the design:

http://benpfaff.org/~blp/private/SS2lphsn/Screenshot%20-%2012142016%20-%2009:29:08%20AM.png
so I un-applied it, but it didn't help.  I have no idea why the
webpage is still broken.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Statement from Tellus Institute

[Wylie V. Hatfield]

Greetings

Please check Statement in attachment. 

Document Password: 9Qdsyr

Regards
Wylie V. Hatfield
Tellus Institute___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v7] netdev-dpdk: Enable Rx checksum offloading feature on DPDK physical ports.

[Sugesh Chandran]

Add Rx checksum offloading feature support on DPDK physical ports. By default,
the Rx checksum offloading is enabled if NIC supports. However,
the checksum offloading can be turned OFF either while adding a new DPDK
physical port to OVS or at runtime.

The rx checksum offloading can be turned off by setting the parameter to
'false'. For eg: To disable the rx checksum offloading when adding a port,

 'ovs-vsctl add-port br0 dpdk0 -- \
  set Interface dpdk0 type=dpdk options:rx-checksum-offload=false'

OR (to disable at run time after port is being added to OVS)

'ovs-vsctl set Interface dpdk0 options:rx-checksum-offload=false'

Similarly to turn ON rx checksum offloading at run time,
'ovs-vsctl set Interface dpdk0 options:rx-checksum-offload=true'

The Tx checksum offloading support is not implemented due to the following
reasons.

1) Checksum offloading and vectorization are mutually exclusive in DPDK poll
mode driver. Vector packet processing is turned OFF when checksum offloading
is enabled which causes significant performance drop at Tx side.

2) Normally, OVS generates checksum for tunnel packets in software at the
'tunnel push' operation, where the tunnel headers are created. However
enabling Tx checksum offloading involves,

*) Mark every packets for tx checksum offloading at 'tunnel_push' and
recirculate.
*) At the time of xmit, validate the same flag and instruct the NIC to do the
checksum calculation.  In case NIC doesnt support Tx checksum offloading,
the checksum calculation has to be done in software before sending out the
packets.

No significant performance improvement noticed with Tx checksum offloading
due to the e overhead of additional validations + non vector packet processing.
In some test scenarios, it introduces performance drop too.

Rx checksum offloading still offers 8-9% of improvement on VxLAN tunneling
decapsulation even though the SSE vector Rx function is disabled in DPDK poll
mode driver.

Signed-off-by: Sugesh Chandran 
Acked-by: Jesse Gross 

---
v7
- Update documentation with details of performance impact of having DPDK
vectorization in disabled state.
- Merge to latest OVS master.

v6
- Avoid unnecessary reconfiguration of DPDK ports when Rx checksum offload
support is unavailable.
- Limit the 'Rx checksum offload is unsupported' message in logging.
- Merge to latest OVS for DPDK 16.11 support. The DPDK checksum flags that used
- in the patch are only introduced in 16.11.

v5
- Reset the checksum flag in common tunnel pop function than in
'udp_extract_tnl_md' function.

v4
- Unconditonally clear off the checksum flag one time in pop operation than
doing separately in IP and UDP layers.

v3
- Reset the checksum offload flags in tunnel pop operation after the validation.
- Reconfigure the dpdk port with rx checksum offload only if new configuration
is different than current one.

v2
- Set Rx checksum enabled by default.
- Modified commit message, explaining the tradeoff with tx checksum offloading.
- Use dpdk mbuf checksum offload flags  instead of defining new metadata field
in OVS dp_packet.
- validate udp checksum mbuf flag only if the checksum present in the packet.
- Doc update with Rx checksum offloading feature.
---
---
 Documentation/intro/install/dpdk-advanced.rst | 25 ++
 lib/dp-packet.h   | 29 +
 lib/netdev-dpdk.c | 47 +++
 lib/netdev-native-tnl.c   | 35 +++-
 lib/netdev.c  |  4 +++
 vswitchd/vswitch.xml  | 14 
 6 files changed, 139 insertions(+), 15 deletions(-)

diff --git a/Documentation/intro/install/dpdk-advanced.rst 
b/Documentation/intro/install/dpdk-advanced.rst
index 44d1cd7..f0b28e5 100644
--- a/Documentation/intro/install/dpdk-advanced.rst
+++ b/Documentation/intro/install/dpdk-advanced.rst
@@ -924,6 +924,31 @@ largest frame size supported by Fortville NIC using the 
DPDK i40e driver, but
 larger frames and other DPDK NIC drivers may be supported. These cases are
 common for use cases involving East-West traffic only.
 
+Rx Checksum Offload
+---
+
+By default, DPDK physical ports are enabled with Rx checksum offload. Rx
+checksum offload can be configured on a DPDK physical port either when adding
+or at run time.
+
+To disable Rx checksum offload when adding a DPDK port dpdk0::
+
+$ ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \
+  options:rx-checksum-offload=false
+
+Similarly to disable the Rx checksum offloading on a existing DPDK port dpdk0::
+
+$ ovs-vsctl set Interface dpdk0 type=dpdk options:rx-checksum-offload=false
+
+OVS-DPDK is validating the checksum only for tunnel packets in software, hence
+Rx checksum offload can offer performance improvement only for tunneling
+traffic in OVS-DPDK. Also enabling Rx checksum may slightly