Re: [ovs-dev] [PATCH v9 2/2] OVN: Enable N-S Traffic, Vlan backed DVR

[Ankur Sharma]

Hi Numan,

Thanks for going through the commit plan.
I submitted a V10 now, which has just the E-W changes.
It has all the review comments handled.

You wanted to have periodic  GARP advertisement patch together as well, however 
as per our
discussion on another email thread, since we want your garp advertisement patch 
to go in first,
hence I could not accommodate those changes in this patch.

Please take a look and looking forward toy our feedback.

Thanks

Regards,
Ankur

From: Numan Siddique 
Sent: Monday, June 10, 2019 10:18 AM
To: Ankur Sharma 
Cc: ovs-dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v9 2/2] OVN: Enable N-S Traffic, Vlan backed DVR



On Fri, Jun 7, 2019 at 5:15 AM Ankur Sharma 
mailto:ankur.sha...@nutanix.com>> wrote:
Hi Numan,

Thanks for trying out the patch and providing feedback.
I am planning to change this series to reflect only the E-W and would send out 
separate patches for N-S improvements.

Following is the reasoning:
==
a. I agree that the network_type construct is adding to the confusion and may 
be we should rely on optional/external-id based key-value config.
b. N-S patch has 3- changes (on a high level) which are distinct from each 
other, having them in a single patch is causing the confusion and is holding 
rest of the reviewed changes.
c. Last but not the least, there were some gaps in the patch as well.

Here is what I am planning to do:
==
a. Keep this series for E-W only and remove all the network_type related 
changes from here (including showing type as bridged/vlan).


Hi Ankur.

I agree with the approach you are planning to take.



b. For N-S Changes, this series has following changes. I will send them out in 
separate patches, especially the ones which are more of a bug fix.
i. Do not allow ARP resolution from physical network unless gateway chassis 
is configured ==> More of a bug fix, will be sent as a separate standalone 
patch.
   ii. GARP advertisement during failover  in the absence of NAT configuration 
==> More of a bug fix, will be sent as a separate standalone patch.
  iii. Periodic GARP advertisement with/without NAT configuration ==> New 
feature, will be added along with SNAT changes.

This periodic GARP adv will be required irrespective of network_type of the 
logical switches connected to a router. So I would request you to handle this 
as well
when you submit the patch.

  iv. Avoid redirection ==> New feature, will come as a separate patchset, we 
will make it as optional feature, i.e by default even non NATed traffic will go 
via gateway chassis, but config knob can override it.

Agree. This makes sense and easier.

   v. No chassis mac replace on gateway chassis ==> More of a addendum to E-W, 
I am thinking about clubbing it some of N-S changes as this is where it will be 
relevant.

c. Will send out separate patch for showing network type as overlay or bridged 
(based on localnet port’s presence), I believe it is good to have .
i.e we will not have any new column in logical switch table, but the output 
of relevant ovn-nbctl show command will show type as “overlay” or “bridged”.

Above will allow us to make progress on the changes we are in agreement on, 
while having thorough discussion on the remaining.
Let me know, if you are fine with the plan, I should be able to send E-W only 
changes in a couple of days and should be able to individual bug fixes soon 
after as well.

For rest of the comments, please find my replies inline.

Appreciate your feedback.

Regards,
Ankur


From: Numan Siddique mailto:nusid...@redhat.com>>
Sent: Monday, June 3, 2019 3:06 AM
To: Ankur Sharma mailto:ankur.sha...@nutanix.com>>
Cc: ovs-dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v9 2/2] OVN: Enable N-S Traffic, Vlan backed DVR


Hi Ankur,

Please see some comments inline. Please note that I haven't got the chance to 
look into the code
in detail. I am first trying to test out the patches. (I am in PTO. Expect some 
delay in my replies).



On Thu, May 30, 2019 at 5:58 AM Ankur Sharma 
mailto:ankur.sha...@nutanix.com>> wrote:
Background:
[1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-October/353066.html 
[mail.openvswitch.org]
[2] 
https://docs.google.com/document/d/1uoQH478wM1OZ16HrxzbOUvk5LvFnfNEWbkPT6Zmm9OU/edit?usp=sharing
 
[docs.google.com]

This Series:
Layer 2, 

[ovs-dev] [PATCH v10] OVN: Enable E-W Traffic, Vlan backed DVR

[Ankur Sharma]

Background:
[1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-October/353066.html
[2] 
https://docs.google.com/document/d/1uoQH478wM1OZ16HrxzbOUvk5LvFnfNEWbkPT6Zmm9OU/edit?usp=sharing

Key difference between an overlay logical switch and
vlan backed logical switch is that for vlan logical switches
packets are not encapsulated.

Hence, if a distributed router port is connected to vlan backed
logical switch, then router port mac as source mac could be
seen from multiple hypervisors. Same  pairs coming
from multiple ports from a top of the rack switch (TOR) perspective
could be seen as a security threat and it could send alarms, drop
the packets or block the ports etc.

This patch addresses the same by introducing the concept of chassis mac.
A chassis mac is CMS provisioned unique mac per chassis. For any routed packet
(i.e source mac is router port mac) going on the wire on a vlan type
logical switch, we will replace its source mac with chassis mac.

This replacing of source mac with chassis mac will happen in table=65
of the logical switch datapath. A flow is added at priority 150, which
matches the source mac and replaces it with chassis mac if the value
is a router port mac.

Example flow:
cookie=0x0, duration=67765.830s, table=65, n_packets=0, n_bytes=0,
idle_age=65534, hard_age=65534, priority=150,reg15=0x1,metadata=0x4,
dl_src=00:00:01:01:02:03 actions=mod_dl_src:aa:bb:cc:dd:ee:ff,
mod_vlan_vid:1000,output:16

Here, 00:00:01:01:02:03 is router port mac and aa:bb:cc:dd:ee:ff
is chassis mac.

Signed-off-by: Ankur Sharma 
---
 ovn/controller/binding.c|  12 +--
 ovn/controller/chassis.c|  64 +++-
 ovn/controller/chassis.h|   4 +
 ovn/controller/ovn-controller.8.xml |  10 ++
 ovn/controller/ovn-controller.c |   4 +-
 ovn/controller/ovn-controller.h |   5 +-
 ovn/controller/physical.c   |  95 +
 ovn/ovn-architecture.7.xml  |  24 +
 ovn/ovn-sb.xml  |   8 ++
 tests/ovn.at| 197 
 10 files changed, 411 insertions(+), 12 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index b62b3da..c73d1aa 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -159,13 +159,11 @@ add_local_datapath__(struct ovsdb_idl_index 
*sbrec_datapath_binding_by_key,
  sbrec_port_binding_by_name,
  peer->datapath, false,
  depth + 1, local_datapaths);
-ld->n_peer_dps++;
-ld->peer_dps = xrealloc(
-ld->peer_dps,
-ld->n_peer_dps * sizeof *ld->peer_dps);
-ld->peer_dps[ld->n_peer_dps - 1] = datapath_lookup_by_key(
-sbrec_datapath_binding_by_key,
-peer->datapath->tunnel_key);
+ld->n_peer_ports++;
+ld->peer_ports = xrealloc(ld->peer_ports,
+  ld->n_peer_ports *
+  sizeof *ld->peer_ports);
+ld->peer_ports[ld->n_peer_ports - 1] = peer;
 }
 }
 }
diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c
index 0f537f1..8403212 100644
--- a/ovn/controller/chassis.c
+++ b/ovn/controller/chassis.c
@@ -23,6 +23,7 @@
 #include "lib/vswitch-idl.h"
 #include "openvswitch/dynamic-string.h"
 #include "openvswitch/vlog.h"
+#include "openvswitch/ofp-parse.h"
 #include "ovn/lib/chassis-index.h"
 #include "ovn/lib/ovn-sb-idl.h"
 #include "ovn-controller.h"
@@ -69,6 +70,12 @@ get_bridge_mappings(const struct smap *ext_ids)
 }
 
 static const char *
+get_chassis_mac_mappings(const struct smap *ext_ids)
+{
+return smap_get_def(ext_ids, "ovn-chassis-mac-mappings", "");
+}
+
+static const char *
 get_cms_options(const struct smap *ext_ids)
 {
 return smap_get_def(ext_ids, "ovn-cms-options", "");
@@ -162,6 +169,7 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
 const char *datapath_type =
 br_int && br_int->datapath_type ? br_int->datapath_type : "";
 const char *cms_options = get_cms_options(>external_ids);
+const char *chassis_macs = get_chassis_mac_mappings(>external_ids);
 
 struct ds iface_types = DS_EMPTY_INITIALIZER;
 ds_put_cstr(_types, "");
@@ -190,18 +198,22 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
 = smap_get_def(_rec->external_ids, "iface-types", "");
 const char *chassis_cms_options
 = get_cms_options(_rec->external_ids);
+const char *chassis_mac_mappings
+= get_chassis_mac_mappings(_rec->external_ids);
 
 /* If any of the external-ids should change, update them. */
 if (strcmp(bridge_mappings, chassis_bridge_mappings) ||
 strcmp(datapath_type, 

Re: [ovs-dev] [PATCH v2] ovn-controller: Cleanup memory in binding_evaluate_port_binding_changes

[Ben Pfaff]

On Tue, Jun 11, 2019 at 04:55:34PM +0200, Dumitru Ceara wrote:
> The 'lport_to_iface' and 'egress_ifaces' hashtables were not cleaned up
> when checking if port bindings require a recompute.
> 
> Reported-at: 
> https://mail.openvswitch.org/pipermail/ovs-discuss/2019-June/048822.html
> Reported-by: Daniel Alvarez Sanchez 
> Fixes: 9d0b504abdee ("ovn-controller: runtime_data change handler for SB 
> port-binding")
> Acked-by: Han Zhou 
> Signed-off-by: Dumitru Ceara 
> 
> ---
> v2: Amend commit log message.

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

Re: [ovs-dev] OVS command line style/syntax modification

[Ben Pfaff]

This isn't a modification to ovs-vsctl, this would be a new utility that
accepts the syntax you want.  I guess you can write such a utility that
invokes ovs-vsctl.

On Tue, Jun 11, 2019 at 10:37:43PM +0430, Saeed sami wrote:
> Hi.
> Thank you for your response.
> 
> We are looking for a way to modify the command line inputs. for example
> instead fo using a command such as "
> 
> ovs-vsctl -- add-br vlan-br \
>   -- add-port vlan-br eth1 \
>   -- add-port vlan-br vlan-br-tag tag=10 \
>   -- set Interface vlan-br-tag type=internalip addr add 1.2.3.4/8 dev
> vlan-br-tag
> 
> "
> Simply write :
> Switch add vlan interface
>switch set interface eth1
>  switch set vlan 10
>  ip address 1.2.3.4/8
> 
> 
> to be more clear make some modifications to commands for easier
> configurations. most of our team are used to Cisco like commands and it is
> hard to train them. But if commands can be changed a little it will make
> their future work easier.
> 
> Vtysh is the CLI used in NOSs such as Quagga or FRRouting. Clish/Klish are
> CLI interpreters that use XML fro command definitions.
> 
> Thank you for teh help.
> 
> Sincerely
> Sami
> 
> 
> On Tue, Jun 11, 2019 at 7:30 PM Ben Pfaff  wrote:
> 
> > On Tue, Jun 11, 2019 at 11:18:22AM +0430, s...@qbicomm.com wrote:
> > > IS it possible to modify command line style? i mean is it possible to
> > modify
> > > command syntax and how commands and sub commands are entered to make
> > working
> > > with OVS easier?
> >
> > What changes are you looking for?
> >
> > > or use tools such as vtysh/Clish/Klish with OVS ?
> >
> > I have not heard of those tools, so I do not know.  Maybe someone else
> > will speak up.
> >
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH net-next] net: openvswitch: remove unnecessary ASSERT_OVSL in ovs_vport_del()

[David Miller]

From: Taehee Yoo 
Date: Mon, 10 Jun 2019 02:19:06 +0900

> ASSERT_OVSL() in ovs_vport_del() is unnecessary because
> ovs_vport_del() is only called by ovs_dp_detach_port() and
> ovs_dp_detach_port() calls ASSERT_OVSL() too.
> 
> Signed-off-by: Taehee Yoo 

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

Re: [ovs-dev] [PATCH net] net: openvswitch: do not free vport if register_netdevice() is failed.

[David Miller]

From: Taehee Yoo 
Date: Sun,  9 Jun 2019 23:26:21 +0900

> In order to create an internal vport, internal_dev_create() is used and
> that calls register_netdevice() internally.
> If register_netdevice() fails, it calls dev->priv_destructor() to free
> private data of netdev. actually, a private data of this is a vport.
> 
> Hence internal_dev_create() should not free and use a vport after failure
> of register_netdevice().
> 
> Test command
> ovs-dpctl add-dp bonding_masters
 ...
> Fixes: cf124db566e6 ("net: Fix inconsistent teardown and release of private 
> netdev state.")
> Signed-off-by: Taehee Yoo 

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

Re: [ovs-dev] OVS command line style/syntax modification

[Saeed sami]

Hi.
Thank you for your response.

We are looking for a way to modify the command line inputs. for example
instead fo using a command such as "

ovs-vsctl -- add-br vlan-br \
  -- add-port vlan-br eth1 \
  -- add-port vlan-br vlan-br-tag tag=10 \
  -- set Interface vlan-br-tag type=internalip addr add 1.2.3.4/8 dev
vlan-br-tag

"
Simply write :
Switch add vlan interface
   switch set interface eth1
 switch set vlan 10
 ip address 1.2.3.4/8


to be more clear make some modifications to commands for easier
configurations. most of our team are used to Cisco like commands and it is
hard to train them. But if commands can be changed a little it will make
their future work easier.

Vtysh is the CLI used in NOSs such as Quagga or FRRouting. Clish/Klish are
CLI interpreters that use XML fro command definitions.

Thank you for teh help.

Sincerely
Sami


On Tue, Jun 11, 2019 at 7:30 PM Ben Pfaff  wrote:

> On Tue, Jun 11, 2019 at 11:18:22AM +0430, s...@qbicomm.com wrote:
> > IS it possible to modify command line style? i mean is it possible to
> modify
> > command syntax and how commands and sub commands are entered to make
> working
> > with OVS easier?
>
> What changes are you looking for?
>
> > or use tools such as vtysh/Clish/Klish with OVS ?
>
> I have not heard of those tools, so I do not know.  Maybe someone else
> will speak up.
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 3/3] rconn: Increase precision of timers.

[0-day Robot]

Bleep bloop.  Greetings Ben Pfaff, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 82 characters long (recommended limit is 79)
#456 FILE: ofproto/connmgr.c:2:
 * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2019 Nicira, 
Inc.

Lines checked: 494, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@bytheb.org

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

Re: [ovs-dev] [PATCH v2 1/3] sat-math: Add functions for saturating arithmetic on "long long int".

[0-day Robot]

Bleep bloop.  Greetings Ben Pfaff, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Use ovs_abort() in place of abort()
#167 FILE: tests/test-util.c:1149:
abort();

Lines checked: 211, Warnings: 0, Errors: 1


Please check this out.  If you feel there has been an error, please email 
acon...@bytheb.org

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

[ovs-dev] Enabling compilation of C JSON parser (C python extension)

[nitish nagesh]

Hello OVS developers,



   I am working on an ARM9 based embedded product which runs software that
has OpenVSwitch integrated into it. The software has a few daemons written
in Python that perform JSON parsing to build the OVSDB IDL cache. These
operations are resulting in high CPU usage for prolonged intervals. This is
found to be due to the Python JSON parser that is being invoked. I learnt
that the OpenVSwitch already provides for a C based JSON parser as C-Python
extension which enables speed-ups. I checked that ovs._json module is not
present on the target which is resulting in Python parser to be
loaded/executed leading to the issue. Eventually I found it’s not even
being compiled. I have tried to enable it for compilation but am facing
issues which is where I need help.



   Our product uses yocto to build/compile the codebase. We have a recipe
for openVSwitch to which I have added a compile_append() routine with the
following content to enable the _json.c compilation:



*do_compile_append() {*

*   cd
${S}/python
*

*   export BUILD_SYS=${BUILD_SYS}*

*   export
HOST_SYS="${HOST_SYS}"
*

*   export STAGING_LIBDIR="${STAGING_LIBDIR}"*

*   export
STAGING_INCDIR="${STAGING_INCDIR}"
*

*   ${STAGING_BINDIR_NATIVE}/python-native/python2.7 setup.py build
${DISTUTILS_BUILD_ARGS}

 *

*}*



  I have attached the “automake.mk” & “setup.py” scripts (python/
directory)too for reference here (as they may not be in sync with the
latest upstream OVS). With this, I see the build system tries to build the
extension however it fails with the following error:



*| running build*

*| running build_py*

*| copying ovs/json.py -> build/lib.linux-x86_64-2.7/ovs*

*| copying ovs/version.py -> build/lib.linux-x86_64-2.7/ovs*

*| running build_ext*

*| building 'ovs._json' extension*

| arm-cnos-linux-gnueabi-gcc -march=armv7-a
--sysroot=/build/tmp/sysroots/switch -DNDEBUG -g -O3 -Wall
-Wstrict-prototypes -O2 -pipe -ggdb3 -feliminate-unused-debug-types
-fdebug-prefix-map=/build/tmp/work/armv7a-cnos-linux-gnueabi/ops-openvswitch/1.0-r0_switch=/usr/src/debug/ops-openvswitch/1.0-r0_switch
-fdebug-prefix-map=/build/tmp/sysroots/x86_64-linux=
-fdebug-prefix-map=/build/tmp/sysroots/switch= -DOPS -fPIC -INone -I
/build/tmp/sysroots/dover/usr/include/python2.7 -c ovs/_json.c -o
build/temp.linux-x86_64-2.7/ovs/_json.o

| ovs/_json.c:2:30: fatal error: openvswitch/json.h: No such file or
directory



I tried several option, to the best of my knowledge, to pass he include
path to the build system (using include_dirs in setup.py, through
automake.mk etc). But I can’t seem to get my head around this. I am a
novice to yocto & autotools. I am starting to doubt if the approach I am
following is even correct?



 Any guidance on how I can resolve this is really appreciated. Also let
me know if the details provided are insufficient.



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

Re: [ovs-dev] [PATCH v2] travis: Don't install kernel for DPDK checks.

[David Marchand]

On Tue, Jun 11, 2019 at 5:32 PM Ilya Maximets 
wrote:

> We don't need to build DPDK kernel modules to test build with OVS.
> And we don't need to build OVS datapath modules for checking
> userspace with DPDK.
>
> Removed 'max-inline-insns-single' changes that only was needed for
> DPDK kernel modules. Config modifications changed to update
> generated build/.config instead of changing sources.
>
> Signed-off-by: Ilya Maximets 
> ---
>
> Version 2:
>
>   * Removed -Wno-error=inline.
>   * config/common_base --> build/.config
>
>  .travis.yml|  8 
>  .travis/linux-build.sh | 25 +
>  2 files changed, 17 insertions(+), 16 deletions(-)
>
> diff --git a/.travis.yml b/.travis.yml
> index 6621fb535..7addcb231 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -31,10 +31,10 @@ env:
>- TESTSUITE=1 KERNEL=3.16
>- TESTSUITE=1 OPTS="--enable-shared"
>- BUILD_ENV="-m32" OPTS="--disable-ssl"
> -  - KERNEL=3.16 DPDK=1 OPTS="--enable-shared"
> -  - KERNEL=3.16 TESTSUITE=1 DPDK=1
> -  - KERNEL=3.16 DPDK_SHARED=1
> -  - KERNEL=3.16 DPDK_SHARED=1 OPTS="--enable-shared"
> +  - DPDK=1 OPTS="--enable-shared"
> +  - TESTSUITE=1 DPDK=1
> +  - DPDK_SHARED=1
> +  - DPDK_SHARED=1 OPTS="--enable-shared"
>- KERNEL=4.20
>- KERNEL=4.19
>- KERNEL=4.18
> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
> index 123cde575..b88bfb53e 100755
> --- a/.travis/linux-build.sh
> +++ b/.travis/linux-build.sh
> @@ -3,7 +3,6 @@
>  set -o errexit
>  set -x
>
> -KERNELSRC=""
>  CFLAGS=""
>  SPARSE_FLAGS=""
>  EXTRA_OPTS="--enable-Werror"
> @@ -57,10 +56,7 @@ function install_kernel()
>  make net/bridge/
>  fi
>
> -KERNELSRC=$(pwd)
> -if [ ! "$DPDK" ] && [ ! "$DPDK_SHARED" ]; then
> -EXTRA_OPTS="${EXTRA_OPTS} --with-linux=$(pwd)"
> -fi
> +EXTRA_OPTS="${EXTRA_OPTS} --with-linux=$(pwd)"
>  echo "Installed kernel source in $(pwd)"
>  cd ..
>  }
> @@ -78,16 +74,21 @@ function install_dpdk()
>  if [ $DIR_NAME != "dpdk-$1"  ]; then mv $DIR_NAME dpdk-$1; fi
>  cd dpdk-$1
>  fi
> -find ./ -type f | xargs sed -i
> 's/max-inline-insns-single=100/max-inline-insns-single=400/'
> -find ./ -type f | xargs sed -i 's/-Werror/-Werror -Wno-error=inline/'
>  echo 'CONFIG_RTE_BUILD_FPIC=y' >>config/common_linuxapp
>  sed -ri '/EXECENV_CFLAGS  = -pthread -fPIC/{s/$/\nelse ifeq
> ($(CONFIG_RTE_BUILD_FPIC),y)/;s/$/\nEXECENV_CFLAGS  = -pthread -fPIC/}'
> mk/exec-env/linuxapp/rte.vars.mk
> +
> +make config CC=gcc T=$TARGET
> +
>  if [ "$DPDK_SHARED" ]; then
> -sed -i '/CONFIG_RTE_BUILD_SHARED_LIB=n/s/=n/=y/'
> config/common_base
> +sed -i '/CONFIG_RTE_BUILD_SHARED_LIB=n/s/=n/=y/' build/.config
>  export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$(pwd)/$TARGET/lib
>  fi
> -make config CC=gcc T=$TARGET
> -make -j4 CC=gcc RTE_KERNELDIR=$KERNELSRC
> +
> +# Disable building DPDK kernel modules. Not needed for OVS build or
> tests.
> +sed -i '/CONFIG_RTE_EAL_IGB_UIO=y/s/=y/=n/' build/.config
> +sed -i '/CONFIG_RTE_KNI_KMOD=y/s/=y/=n/' build/.config
>

We could also disable the librte_kni but we are fine with just disabling
the kmods.


+
> +make -j4 CC=gcc
>  echo "Installed DPDK source in $(pwd)"
>  cd ..
>  }
> @@ -97,7 +98,7 @@ function configure_ovs()
>  ./boot.sh && ./configure $* || { cat config.log; exit 1; }
>  }
>
> -if [ "$KERNEL" ] || [ "$DPDK" ] || [ "$DPDK_SHARED" ]; then
> +if [ "$KERNEL" ]; then
>  install_kernel $KERNEL
>  fi
>
> @@ -141,7 +142,7 @@ else
>  make selinux-policy
>
>  # Only build datapath if we are testing kernel w/o running testsuite
> -if [ "$KERNEL" ] && [ ! "$DPDK" ] && [ ! "$DPDK_SHARED" ]; then
> +if [ "$KERNEL" ]; then
>  cd datapath
>  fi
>  make -j4
> --
> 2.17.1
>
>
Reviewed-by: David Marchand 
Tested-by: David Marchand 


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

[ovs-dev] [PATCH v2 2/3] rconn: Remove write-only struct members.

[Ben Pfaff]

Signed-off-by: Ben Pfaff 
---
 lib/rconn.c | 16 
 1 file changed, 16 deletions(-)

diff --git a/lib/rconn.c b/lib/rconn.c
index a5b31d099efd..ba95bb3a1a61 100644
--- a/lib/rconn.c
+++ b/lib/rconn.c
@@ -118,12 +118,6 @@ struct rconn {
 bool probably_admitted;
 time_t last_admitted;
 
-/* These values are simply for statistics reporting, not used directly by
- * anything internal to the rconn (or ofproto for that matter). */
-unsigned int n_attempted_connections, n_successful_connections;
-time_t creation_time;
-unsigned long int total_time_connected;
-
 /* Throughout this file, "probe" is shorthand for "inactivity probe".  When
  * no activity has been observed from the peer for a while, we send out an
  * echo request as an inactivity probe packet.  We should receive back a
@@ -272,11 +266,6 @@ rconn_create(int probe_interval, int max_backoff, uint8_t 
dscp,
 rc->probably_admitted = false;
 rc->last_admitted = time_now();
 
-rc->n_attempted_connections = 0;
-rc->n_successful_connections = 0;
-rc->creation_time = time_now();
-rc->total_time_connected = 0;
-
 rc->last_activity = time_now();
 
 rconn_set_probe_interval(rc, probe_interval);
@@ -468,7 +457,6 @@ reconnect(struct rconn *rc)
 if (rconn_logging_connection_attempts__(rc)) {
 VLOG_INFO("%s: connecting...", rc->name);
 }
-rc->n_attempted_connections++;
 retval = vconn_open(rc->target, rc->allowed_versions, rc->dscp,
 >vconn);
 if (!retval) {
@@ -512,7 +500,6 @@ run_CONNECTING(struct rconn *rc)
 int retval = vconn_connect(rc->vconn);
 if (!retval) {
 VLOG(rc->reliable ? VLL_INFO : VLL_DBG, "%s: connected", rc->name);
-rc->n_successful_connections++;
 state_transition(rc, S_ACTIVE);
 rc->version = vconn_get_version(rc->vconn);
 rc->last_connected = rc->state_entered;
@@ -1286,9 +1273,6 @@ state_transition(struct rconn *rc, enum state state)
 if (is_connected_state(state) && !is_connected_state(rc->state)) {
 rc->probably_admitted = false;
 }
-if (rconn_is_connected(rc)) {
-rc->total_time_connected += elapsed_in_this_state(rc);
-}
 VLOG_DBG("%s: entering %s", rc->name, state_name(state));
 rc->state = state;
 rc->state_entered = time_now();
-- 
2.20.1

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

[ovs-dev] [PATCH v2 3/3] rconn: Increase precision of timers.

[Ben Pfaff]

Until now, the rconn timers have been precise only to the nearest second.
This increases them to millisecond precision, which seems cleaner these
days.

Signed-off-by: Ben Pfaff 
---
 include/openvswitch/rconn.h |   7 +-
 lib/rconn.c | 138 ++--
 ofproto/connmgr.c   |  16 ++---
 3 files changed, 80 insertions(+), 81 deletions(-)

diff --git a/include/openvswitch/rconn.h b/include/openvswitch/rconn.h
index fd60a6ce1dea..25c18f97e405 100644
--- a/include/openvswitch/rconn.h
+++ b/include/openvswitch/rconn.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2015 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2015, 2019 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -19,7 +19,6 @@
 
 #include 
 #include 
-#include 
 #include "openvswitch/types.h"
 
 /* A wrapper around vconn that provides queuing and optionally reliability.
@@ -88,8 +87,8 @@ int rconn_failure_duration(const struct rconn *);
 int rconn_get_version(const struct rconn *);
 
 const char *rconn_get_state(const struct rconn *);
-time_t rconn_get_last_connection(const struct rconn *);
-time_t rconn_get_last_disconnect(const struct rconn *);
+long long int rconn_get_last_connection(const struct rconn *);
+long long int rconn_get_last_disconnect(const struct rconn *);
 unsigned int rconn_get_connection_seqno(const struct rconn *);
 int rconn_get_last_error(const struct rconn *);
 unsigned int rconn_count_txqlen(const struct rconn *);
diff --git a/lib/rconn.c b/lib/rconn.c
index ba95bb3a1a61..9fab33e2e59a 100644
--- a/lib/rconn.c
+++ b/lib/rconn.c
@@ -84,13 +84,16 @@ state_name(enum state state)
 }
 
 /* A reliable connection to an OpenFlow switch or controller.
+ *
+ * Members of type 'long long int' are times in milliseconds on the monotonic
+ * clock, as returned by time_msec().  Other times are durations in seconds.
  *
  * See the large comment in rconn.h for more information. */
 struct rconn {
 struct ovs_mutex mutex;
 
 enum state state;
-time_t state_entered;
+long long int state_entered;
 
 struct vconn *vconn;
 char *name; /* Human-readable descriptive name. */
@@ -99,11 +102,11 @@ struct rconn {
 
 struct ovs_list txq;/* Contains "struct ofpbuf"s. */
 
-int backoff;
-int max_backoff;
-time_t backoff_deadline;
-time_t last_connected;
-time_t last_disconnected;
+int backoff; /* Current backoff, in seconds. */
+int max_backoff; /* Limit for backoff, In seconds. */
+long long int backoff_deadline;
+long long int last_connected;
+long long int last_disconnected;
 unsigned int seqno;
 int last_error;
 
@@ -116,7 +119,7 @@ struct rconn {
  * last_admitted reports the last time we believe such a positive admission
  * control decision was made. */
 bool probably_admitted;
-time_t last_admitted;
+long long int last_admitted; /* Milliseconds on monotonic clock. */
 
 /* Throughout this file, "probe" is shorthand for "inactivity probe".  When
  * no activity has been observed from the peer for a while, we send out an
@@ -126,7 +129,7 @@ struct rconn {
  * "Activity" is defined as either receiving an OpenFlow message from the
  * peer or successfully sending a message that had been in 'txq'. */
 int probe_interval; /* Secs of inactivity before sending probe. */
-time_t last_activity;   /* Last time we saw some activity. */
+long long int last_activity;   /* Last time we saw some activity. */
 
 uint8_t dscp;
 
@@ -152,9 +155,9 @@ uint32_t rconn_get_allowed_versions(const struct rconn 
*rconn)
 return rconn->allowed_versions;
 }
 
-static unsigned int elapsed_in_this_state(const struct rconn *rc)
+static long long int elapsed_in_this_state(const struct rconn *rc)
 OVS_REQUIRES(rc->mutex);
-static unsigned int timeout(const struct rconn *rc) OVS_REQUIRES(rc->mutex);
+static long long int timeout(const struct rconn *rc) OVS_REQUIRES(rc->mutex);
 static bool timed_out(const struct rconn *rc) OVS_REQUIRES(rc->mutex);
 static void state_transition(struct rconn *rc, enum state)
 OVS_REQUIRES(rc->mutex);
@@ -247,7 +250,7 @@ rconn_create(int probe_interval, int max_backoff, uint8_t 
dscp,
 ovs_mutex_init(>mutex);
 
 rc->state = S_VOID;
-rc->state_entered = time_now();
+rc->state_entered = time_msec();
 
 rc->vconn = NULL;
 rc->name = xstrdup("void");
@@ -258,15 +261,15 @@ rconn_create(int probe_interval, int max_backoff, uint8_t 
dscp,
 
 rc->backoff = 0;
 rc->max_backoff = max_backoff ? max_backoff : 8;
-rc->backoff_deadline = TIME_MIN;
-rc->last_connected = TIME_MIN;
-rc->last_disconnected = TIME_MIN;
+rc->backoff_deadline = LLONG_MIN;
+rc->last_connected = LLONG_MIN;
+

[ovs-dev] [PATCH v2 1/3] sat-math: Add functions for saturating arithmetic on "long long int".

[Ben Pfaff]

The first users will be added in an upcoming commit.

Also add tests.

Signed-off-by: Ben Pfaff 
---
 lib/sat-math.h| 72 +--
 tests/library.at  |  5 
 tests/test-util.c | 42 ++-
 3 files changed, 115 insertions(+), 4 deletions(-)

diff --git a/lib/sat-math.h b/lib/sat-math.h
index beeff8b2b429..8dda1515fdd0 100644
--- a/lib/sat-math.h
+++ b/lib/sat-math.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2012 Nicira, Inc.
+ * Copyright (c) 2008, 2012, 2019 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -20,24 +20,90 @@
 #include 
 #include "openvswitch/util.h"
 
-/* Saturating addition: overflow yields UINT_MAX. */
+#ifndef __has_builtin
+#define __has_builtin(x) 0
+#endif
+
+/* Returns x + y, clamping out-of-range results into the range of the return
+ * type. */
 static inline unsigned int
 sat_add(unsigned int x, unsigned int y)
 {
 return x + y >= x ? x + y : UINT_MAX;
 }
+static inline long long int
+llsat_add__(long long int x, long long int y)
+{
+return (x >= 0 && y >= 0 && x > LLONG_MAX - y ? LLONG_MAX
+: x < 0 && y < 0 && x < LLONG_MIN - y ? LLONG_MIN
+: x + y);
+}
+static inline long long int
+llsat_add(long long int x, long long int y)
+{
+#if __GNUC__ >= 5 || __has_builtin(__builtin_saddll_overflow)
+long long int sum;
+return (!__builtin_saddll_overflow(x, y, ) ? sum
+: x > 0 ? LLONG_MAX : LLONG_MIN);
+#else
+return llsat_add__(x, y);
+#endif
+}
 
-/* Saturating subtraction: underflow yields 0. */
+/* Returns x - y, clamping out-of-range results into the range of the return
+ * type. */
 static inline unsigned int
 sat_sub(unsigned int x, unsigned int y)
 {
 return x >= y ? x - y : 0;
 }
+static inline long long int
+llsat_sub__(long long int x, long long int y)
+{
+return (x >= 0 && y < 0 && x > LLONG_MAX + y ? LLONG_MAX
+: x < 0 && y >= 0 && x < LLONG_MIN + y ? LLONG_MIN
+: x - y);
+}
+static inline long long int
+llsat_sub(long long int x, long long int y)
+{
+#if __GNUC__ >= 5 || __has_builtin(__builtin_ssubll_overflow)
+long long int difference;
+return (!__builtin_ssubll_overflow(x, y, ) ? difference
+: x >= 0 ? LLONG_MAX : LLONG_MIN);
+#else
+return llsat_sub__(x, y);
+#endif
+}
 
+/* Returns x * y, clamping out-of-range results into the range of the return
+ * type. */
 static inline unsigned int
 sat_mul(unsigned int x, unsigned int y)
 {
 return OVS_SAT_MUL(x, y);
 }
+static inline long long int
+llsat_mul__(long long int x, long long int y)
+{
+return (  x > 0 && y > 0 && x > LLONG_MAX / y ? LLONG_MAX
+: x < 0 && y > 0 && x < LLONG_MIN / y ? LLONG_MIN
+: x > 0 && y < 0 && y < LLONG_MIN / x ? LLONG_MIN
+/* Special case because -LLONG_MIN / -1 overflows: */
+: x == LLONG_MIN && y == -1 ? LLONG_MAX
+: x < 0 && y < 0 && x < LLONG_MAX / y ? LLONG_MAX
+: x * y);
+}
+static inline long long int
+llsat_mul(long long int x, long long int y)
+{
+#if __GNUC__ >= 5 || __has_builtin(__builtin_smulll_overflow)
+long long int product;
+return (!__builtin_smulll_overflow(x, y, ) ? product
+: (x > 0) == (y > 0) ? LLONG_MAX : LLONG_MIN);
+#else
+return llsat_mul__(x, y);
+#endif
+}
 
 #endif /* sat-math.h */
diff --git a/tests/library.at b/tests/library.at
index a30d362e34bf..ecb9268d40df 100644
--- a/tests/library.at
+++ b/tests/library.at
@@ -231,6 +231,11 @@ AT_CHECK([sed 's/.*: //
 
 AT_CLEANUP
 
+AT_SETUP([saturating arithmetic])
+AT_KEYWORDS([sat math sat_math])
+AT_CHECK([ovstest test-util sat_math])
+AT_CLEANUP
+
 AT_SETUP([snprintf])
 AT_CHECK([ovstest test-util snprintf])
 AT_CLEANUP
diff --git a/tests/test-util.c b/tests/test-util.c
index 9222355ec1b0..f0fd0421086b 100644
--- a/tests/test-util.c
+++ b/tests/test-util.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
+ * Copyright (c) 2011, 2012, 2013, 2014, 2015, 2016, 2019 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -27,6 +27,7 @@
 #include "command-line.h"
 #include "ovstest.h"
 #include "random.h"
+#include "sat-math.h"
 #include "openvswitch/vlog.h"
 
 static void
@@ -1138,6 +1139,44 @@ test_snprintf(struct ovs_cmdl_context *ctx OVS_UNUSED)
 ovs_assert(snprintf(NULL, 0, "abcde") == 5);
 }
 
+static void
+check_sat(long long int x, long long int y, const char *op,
+  long long int r_a, long long int r_b)
+{
+if (r_a != r_b) {
+fprintf(stderr, "%lld %s %lld saturates differently: %lld vs %lld\n",
+x, op, y, r_a, r_b);
+abort();
+}
+}
+
+static void
+test_sat_math(struct ovs_cmdl_context *ctx OVS_UNUSED)
+{
+long long int values[] = {
+

[ovs-dev] [PATCH v2 0/3] Improve rconn timing precision

[Ben Pfaff]

v1->v2: Fix arithmetic and add tests in patch 1.

Ben Pfaff (3):
  sat-math: Add functions for saturating arithmetic on "long long int".
  rconn: Remove write-only struct members.
  rconn: Increase precision of timers.

 include/openvswitch/rconn.h |   7 +-
 lib/rconn.c | 154 
 lib/sat-math.h  |  72 -
 ofproto/connmgr.c   |  16 ++--
 tests/library.at|   5 ++
 tests/test-util.c   |  42 +-
 6 files changed, 195 insertions(+), 101 deletions(-)

-- 
2.20.1

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

[ovs-dev] Obligaciones patronales

[SUA y Obligaciones patronales]

Me da mucho gusto saludarte.

Es para mí un placer poder invitarte a nuestro webinar "Nómina NOI, SUA y 
obligaciones patronales ", que se estará llevando
a cabo el día jueves 04 de julio con un horario de 10:00 a 17:00 hrs. (hora del 
centro de México)


Con este curso en línea lograremos:

• Prevenir demandas laborales, revisar y elaborar contratos, planear y cuidar 
la operación con base a las aplicaciones
 fiscales vigentes.
• Elaborar y supervisar de manera más eficiente todo el proceso de la nómina y 
aplicará beneficios inmediatos en el
pago de la Nómina.
• Tomar importantes decisiones corporativas de operación así como el correcto 
manejo de la nómina, minimizando
riesgos laborales y optimizando recursos.

La experta que nos acompañará será Socorro Ávila, quién es contadora, cuenta 
con diversos estudios en impuestos y una 
maestría en derecho. También tiene amplia experiencia como consultora en temas 
contables y fiscales, como docente 
universitaria, asesora de pymes y nuevos proyectos. Su especialidad es la 
contabilidad fiscal lo que la ha llevado a impartir
conferencias para múltiples organismos entre los que se encuentran el Colegio 
de Contadores y el Senado de la República.

Te invito a que participes con nosotros y conozcas los beneficios de la 
capacitación en línea.

Tel: (045) 55 15 54 66 30 

Solicita información respondiendo con la palabra Obligaciones junto con los 
siguientes datos:

Nombre:
Correo electrónico:
Número telefónico:


¡Qué tengas un excelente día!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/3] sat-math: Add functions for saturating arithmetic on "long long int".

[Ben Pfaff]

On Mon, Jun 10, 2019 at 05:35:43PM +0300, Ilya Maximets wrote:
> On 02.03.2019 2:54, Ben Pfaff wrote:
> > The first users will be added in an upcoming commit.
> > 
> > Signed-off-by: Ben Pfaff 
> > ---
> >  lib/automake.mk |  3 ++-
> >  lib/sat-math.c  | 31 +++
> >  lib/sat-math.h  | 25 ++---
> >  3 files changed, 55 insertions(+), 4 deletions(-)
> >  create mode 100644 lib/sat-math.c
> > 
> > diff --git a/lib/automake.mk b/lib/automake.mk
> > index bae032bd835e..6df8037a3fd8 100644
> > --- a/lib/automake.mk
> > +++ b/lib/automake.mk
> > @@ -1,4 +1,4 @@
> > -# Copyright (C) 2009-2018 Nicira, Inc.
> > +# Copyright (C) 2009-2019 Nicira, Inc.
> >  #
> >  # Copying and distribution of this file, with or without modification,
> >  # are permitted in any medium without royalty provided the copyright
> > @@ -248,6 +248,7 @@ lib_libopenvswitch_la_SOURCES = \
> > lib/rstp-common.h \
> > lib/rstp-state-machines.c \
> > lib/rstp-state-machines.h \
> > +   lib/sat-math.c \
> > lib/sat-math.h \
> > lib/seq.c \
> > lib/seq.h \
> > diff --git a/lib/sat-math.c b/lib/sat-math.c
> > new file mode 100644
> > index ..24b73af12eb4
> > --- /dev/null
> > +++ b/lib/sat-math.c
> > @@ -0,0 +1,31 @@
> > +/*
> > + * Copyright (c) 2019 Nicira, Inc.
> > + *
> > + * 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.
> > + */
> > +#include 
> > +#include "sat-math.h"
> > +
> > +/* Returns x * y, clamping out-of-range results into the range of the 
> > return
> > + * type. */
> > +long long int
> > +llsat_mul(long long int x, long long int y)
> > +{
> > +return (  x > 0 && y > 0 && x >  LLONG_MAX / y ? LLONG_MAX
> > +: x < 0 && y > 0 && x <= LLONG_MIN / y ? LLONG_MIN
> > +: x > 0 && y < 0 && y <= LLONG_MIN / x ? LLONG_MIN
> > +/* Special case because -LLONG_MIN / -1 overflows: */
> > +: x == LLONG_MIN && y == -1 ? LLONG_MAX
> > +: x < 0 && y < 0 && x < LLONG_MIN / y ? LLONG_MAX
> 
> Shouldn't there be x < LLONG_MAX / y ?
> 
> if (y < 0) --> (LLONG_MIN / y) > 0
> --> x always less than (LLONG_MIN / y), because x < 0.
> 
> 
> Code become complicated here. Maybe it's time to write some unit tests for
> this library?

Maybe that was your polite way of saying "all your math is wrong" ;-)
I wrote some tests and I'll post v2 in a minute.

> Have you considered using compiler builtins like __builtin_mul_overflow()
> for these functions? Modern clang and gcc supports them.

Yes, those functions are awesome but OVS also supports MSVC so we can't
rely solely on them.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [dpdk-latest PATCH] netdev-dpdk: Prefix network structures with rte_.

[David Marchand]

On Tue, Jun 11, 2019 at 6:30 PM Ilya Maximets 
wrote:

> On 11.06.2019 19:10, David Marchand wrote:
> >
> >
> > On Tue, Jun 11, 2019 at 5:36 PM Ilya Maximets  > wrote:
> >
> > On 11.06.2019 18:21, Ian Stokes wrote:
> > > On 6/11/2019 3:40 PM, Aaron Conole wrote:
> > >> Ian Stokes mailto:ian.sto...@intel.com>>
> writes:
> > >>
> > >>> On 6/10/2019 3:57 PM, Ian Stokes wrote:
> >  On 6/6/2019 12:36 PM, David Marchand wrote:
> > >
> > >
> > > On Thu, Jun 6, 2019 at 1:25 PM Ian Stokes <
> ian.sto...@intel.com 
> > > >>
> wrote:
> > >
> > >  On 6/4/2019 12:14 PM, David Marchand wrote:
> > >   > On Tue, Jun 4, 2019 at 11:29 AM David Marchand
> > >   >  david.march...@redhat.com> >
> > >  
> > >   > >   >
> > >   > Following a rework of dpdk network structures
> names [1],
> > >  update the
> > >   > concerned parts.
> > >   >
> > >   > Ran Olivier script:
> > >   > sh prefix-net-rte.sh $(find -name "*dpdk*.c")
> > >   > sh prefix-net-rte.sh $(find -name "*dpdk*.h")
> > >   > sh prefix-net-rte.sh $(find -name "*rte*.c")
> > >   > sh prefix-net-rte.sh $(find -name "*rte*.h")
> > >   >
> > >   > Plus an extra pass following further changes [2]:
> > >   > old=RTE_IPv4
> > >   > new=RTE_IPV4
> > >   > git grep -lw $old | xargs sed -i -e
> "s/\<$old\>/$new/g"
> > >   >
> > >   > old=RTE_ETHER_TYPE_IPv4
> > >   > new=RTE_ETHER_TYPE_IPV4
> > >   > git grep -lw $old | xargs sed -i -e
> "s/\<$old\>/$new/g"
> > >   >
> > >   > old=RTE_ETHER_TYPE_IPv6
> > >   > new=RTE_ETHER_TYPE_IPV6
> > >   > git grep -lw $old | xargs sed -i -e
> "s/\<$old\>/$new/g"
> > >   >
> > >   > 1:
> http://mails.dpdk.org/archives/dev/2019-May/132612.html
> > >   > 2:
> https://git.dpdk.org/dpdk/commit/?id=0c9da7555da8
> > >   >
> > >   >
> > >   > Olivier noticed that I had used an early version of
> his patch.
> > >   > The published one handles the update on RTE_IPv4.
> > >   > I tried the last version which gives the same result
> anyway.
> > >   > So the extra pass is unnecessary.
> > >   >
> > >   > I can send a v2 to update the commitlog accordingly.
> > >   >
> > >
> > >  Hi David,
> > >
> > >  thanks for this, upon inspection the patch looks fine and
> I can
> > > confirm
> > >  that dpdk-latest is now building with Master of DPDK
> again.
> > >
> > >  I'm just in the process of running a few smoke tests to
> make sure
> > >  there's no issues functionally (I don't expect to see any
> as the
> > >  changes
> > >  seem straight forward).
> > >
> > >  WRT the v2, what exactly do you want to change in the
> commit? If it's
> > >  trivial I can amend it before committing.
> > >
> > >
> > >
> > > I just stripped the useless part in the commitlog and put a
> link to
> > > Olivier mail which contained his script.
> > > You can see the commitlog here:
> > >
> https://github.com/david-marchand/ovs/commit/9d367de7d323c28f7c89d590ff60373c47ffa073
> > >
> > >
> > >
> > >
> > >  I'll be applying this to dpdk-latest and dpdk-hwol
> branches but
> > > not ovs
> > >  master (master is still using DPDK 18.11.1 currently so
> no need for
> > >  these changes until it moves to 19.11).
> > >
> > >
> > >
> > > Yes, makes sense.
> > > Thanks Ian.
> > 
> >  Thanks David, validated and pushed to dpdk-latest and dpdk-wol.
> > >>>
> > >>> Good catch actually. In the past we had previously tracked the
> latest
> > >>> DPDK release to compile against, but now that dpdk-latest is
> used with
> > >>> the UNH DPDK CI it probably makes more sense to track DPDK
> master at
> > >>> that's what dpdk-latest looks to enable compilation of.
> > >>>
> > >>> I'm not against this, would be interested in what peoples
> thoughts are
> > >>> and if so we can modify travis for the dpdk-latest branch.
> > 

Re: [ovs-dev] [dpdk-latest PATCH] netdev-dpdk: Prefix network structures with rte_.

[Ilya Maximets]

On 11.06.2019 19:10, David Marchand wrote:
> 
> 
> On Tue, Jun 11, 2019 at 5:36 PM Ilya Maximets  > wrote:
> 
> On 11.06.2019 18:21, Ian Stokes wrote:
> > On 6/11/2019 3:40 PM, Aaron Conole wrote:
> >> Ian Stokes mailto:ian.sto...@intel.com>> writes:
> >>
> >>> On 6/10/2019 3:57 PM, Ian Stokes wrote:
>  On 6/6/2019 12:36 PM, David Marchand wrote:
> >
> >
> > On Thu, Jun 6, 2019 at 1:25 PM Ian Stokes  
> > >> wrote:
> >
> >  On 6/4/2019 12:14 PM, David Marchand wrote:
> >   > On Tue, Jun 4, 2019 at 11:29 AM David Marchand
> >   >    >
> >   
> >    >   >
> >   >     Following a rework of dpdk network structures names [1],
> >  update the
> >   >     concerned parts.
> >   >
> >   >     Ran Olivier script:
> >   >     sh prefix-net-rte.sh $(find -name "*dpdk*.c")
> >   >     sh prefix-net-rte.sh $(find -name "*dpdk*.h")
> >   >     sh prefix-net-rte.sh $(find -name "*rte*.c")
> >   >     sh prefix-net-rte.sh $(find -name "*rte*.h")
> >   >
> >   >     Plus an extra pass following further changes [2]:
> >   >     old=RTE_IPv4
> >   >     new=RTE_IPV4
> >   >     git grep -lw $old | xargs sed -i -e "s/\<$old\>/$new/g"
> >   >
> >   >     old=RTE_ETHER_TYPE_IPv4
> >   >     new=RTE_ETHER_TYPE_IPV4
> >   >     git grep -lw $old | xargs sed -i -e "s/\<$old\>/$new/g"
> >   >
> >   >     old=RTE_ETHER_TYPE_IPv6
> >   >     new=RTE_ETHER_TYPE_IPV6
> >   >     git grep -lw $old | xargs sed -i -e "s/\<$old\>/$new/g"
> >   >
> >   >     1: 
> http://mails.dpdk.org/archives/dev/2019-May/132612.html
> >   >     2: https://git.dpdk.org/dpdk/commit/?id=0c9da7555da8
> >   >
> >   >
> >   > Olivier noticed that I had used an early version of his 
> patch.
> >   > The published one handles the update on RTE_IPv4.
> >   > I tried the last version which gives the same result anyway.
> >   > So the extra pass is unnecessary.
> >   >
> >   > I can send a v2 to update the commitlog accordingly.
> >   >
> >
> >  Hi David,
> >
> >  thanks for this, upon inspection the patch looks fine and I can
> > confirm
> >  that dpdk-latest is now building with Master of DPDK again.
> >
> >  I'm just in the process of running a few smoke tests to make 
> sure
> >  there's no issues functionally (I don't expect to see any as 
> the
> >  changes
> >  seem straight forward).
> >
> >  WRT the v2, what exactly do you want to change in the commit? 
> If it's
> >  trivial I can amend it before committing.
> >
> >
> >
> > I just stripped the useless part in the commitlog and put a link to
> > Olivier mail which contained his script.
> > You can see the commitlog here:
> > 
> https://github.com/david-marchand/ovs/commit/9d367de7d323c28f7c89d590ff60373c47ffa073
> >
> >
> >
> >
> >  I'll be applying this to dpdk-latest and dpdk-hwol branches but
> > not ovs
> >  master (master is still using DPDK 18.11.1 currently so no 
> need for
> >  these changes until it moves to 19.11).
> >
> >
> >
> > Yes, makes sense.
> > Thanks Ian.
> 
>  Thanks David, validated and pushed to dpdk-latest and dpdk-wol.
> >>>
> >>> Good catch actually. In the past we had previously tracked the latest
> >>> DPDK release to compile against, but now that dpdk-latest is used with
> >>> the UNH DPDK CI it probably makes more sense to track DPDK master at
> >>> that's what dpdk-latest looks to enable compilation of.
> >>>
> >>> I'm not against this, would be interested in what peoples thoughts are
> >>> and if so we can modify travis for the dpdk-latest branch.
> >>
> >> At least for this part (per-branch), the travis yml is read on a
> >> per-branch basis.  If it changes on one branch, only that branch will
> >> be affected.  Is this what you mean?
> >
> > Yes, travis would be changed to track 

Re: [ovs-dev] [dpdk-latest PATCH] netdev-dpdk: Prefix network structures with rte_.

[David Marchand]

On Tue, Jun 11, 2019 at 5:36 PM Ilya Maximets 
wrote:

> On 11.06.2019 18:21, Ian Stokes wrote:
> > On 6/11/2019 3:40 PM, Aaron Conole wrote:
> >> Ian Stokes  writes:
> >>
> >>> On 6/10/2019 3:57 PM, Ian Stokes wrote:
>  On 6/6/2019 12:36 PM, David Marchand wrote:
> >
> >
> > On Thu, Jun 6, 2019 at 1:25 PM Ian Stokes  > > wrote:
> >
> >  On 6/4/2019 12:14 PM, David Marchand wrote:
> >   > On Tue, Jun 4, 2019 at 11:29 AM David Marchand
> >   > mailto:david.march...@redhat.com
> >
> >   >  >> wrote:
> >   >
> >   > Following a rework of dpdk network structures names [1],
> >  update the
> >   > concerned parts.
> >   >
> >   > Ran Olivier script:
> >   > sh prefix-net-rte.sh $(find -name "*dpdk*.c")
> >   > sh prefix-net-rte.sh $(find -name "*dpdk*.h")
> >   > sh prefix-net-rte.sh $(find -name "*rte*.c")
> >   > sh prefix-net-rte.sh $(find -name "*rte*.h")
> >   >
> >   > Plus an extra pass following further changes [2]:
> >   > old=RTE_IPv4
> >   > new=RTE_IPV4
> >   > git grep -lw $old | xargs sed -i -e "s/\<$old\>/$new/g"
> >   >
> >   > old=RTE_ETHER_TYPE_IPv4
> >   > new=RTE_ETHER_TYPE_IPV4
> >   > git grep -lw $old | xargs sed -i -e "s/\<$old\>/$new/g"
> >   >
> >   > old=RTE_ETHER_TYPE_IPv6
> >   > new=RTE_ETHER_TYPE_IPV6
> >   > git grep -lw $old | xargs sed -i -e "s/\<$old\>/$new/g"
> >   >
> >   > 1:
> http://mails.dpdk.org/archives/dev/2019-May/132612.html
> >   > 2: https://git.dpdk.org/dpdk/commit/?id=0c9da7555da8
> >   >
> >   >
> >   > Olivier noticed that I had used an early version of his
> patch.
> >   > The published one handles the update on RTE_IPv4.
> >   > I tried the last version which gives the same result anyway.
> >   > So the extra pass is unnecessary.
> >   >
> >   > I can send a v2 to update the commitlog accordingly.
> >   >
> >
> >  Hi David,
> >
> >  thanks for this, upon inspection the patch looks fine and I can
> > confirm
> >  that dpdk-latest is now building with Master of DPDK again.
> >
> >  I'm just in the process of running a few smoke tests to make
> sure
> >  there's no issues functionally (I don't expect to see any as the
> >  changes
> >  seem straight forward).
> >
> >  WRT the v2, what exactly do you want to change in the commit?
> If it's
> >  trivial I can amend it before committing.
> >
> >
> >
> > I just stripped the useless part in the commitlog and put a link to
> > Olivier mail which contained his script.
> > You can see the commitlog here:
> >
> https://github.com/david-marchand/ovs/commit/9d367de7d323c28f7c89d590ff60373c47ffa073
> >
> >
> >
> >
> >  I'll be applying this to dpdk-latest and dpdk-hwol branches but
> > not ovs
> >  master (master is still using DPDK 18.11.1 currently so no need
> for
> >  these changes until it moves to 19.11).
> >
> >
> >
> > Yes, makes sense.
> > Thanks Ian.
> 
>  Thanks David, validated and pushed to dpdk-latest and dpdk-wol.
> >>>
> >>> Good catch actually. In the past we had previously tracked the latest
> >>> DPDK release to compile against, but now that dpdk-latest is used with
> >>> the UNH DPDK CI it probably makes more sense to track DPDK master at
> >>> that's what dpdk-latest looks to enable compilation of.
> >>>
> >>> I'm not against this, would be interested in what peoples thoughts are
> >>> and if so we can modify travis for the dpdk-latest branch.
> >>
> >> At least for this part (per-branch), the travis yml is read on a
> >> per-branch basis.  If it changes on one branch, only that branch will
> >> be affected.  Is this what you mean?
> >
> > Yes, travis would be changed to track master just for dpdk-latest is my
> understanding. OVS master and OVS release branches should still only track
> the DPDK LTS release they are currently validated against in their travis
> yml.
>
> Makes sense. Broken travis builds on dpdk-latest branch are annoying.
>

I have changes that apply to both master and dpdk-latest branch, then a
change for the switch to dpdk master branch in dpdk-latest.

I can post the changes for master on top of your patch that disables
kni/igb_uio.
We merge yours and mine in master, then pull master into dpdk-latest.

Then I would only send the switch to dpdk master branch for the dpdk-latest
ovs branch.

Is this ok this way ?


-- 
David Marchand
___
dev 

Re: [ovs-dev] [PATCH] ovn: Send GARP for the router ports connected to localnet switches

[Han Zhou]

On Mon, Jun 10, 2019 at 10:27 AM Numan Siddique  wrote:

>
>
> On Fri, Jun 7, 2019 at 5:18 AM Han Zhou  wrote:
>
>>
>>
>> On Wed, May 1, 2019 at 9:04 AM  wrote:
>> >
>> > From: Numan Siddique 
>> >
>> > With the commit [1], the routing for the provider logical switches
>> > connected to a router is centralized on the master gateway chassis
>> > (if the option - reside-on-redirect-chassis) is set. When the
>> > failover happens and a standby gateway chassis becomes master,
>> > it should send GARPs for the router port macs. Without this, the
>> > physical switch doesn't learn the new location of the router port macs
>> > immediately and this could result in traffic disruption.
>> >
>> > This patch addresses this issue so that the ovn-controller which claims
>> the
>> > distributed gatweway router port sends out the GARPs.
>> >
>> > [1] - 85706c34d53d ("ovn: Avoid tunneling for VLAN packets redirected
>> to a gateway chassis")
>> >
>> > Signed-off-by: Numan Siddique 
>> > ---
>> >  ovn/northd/ovn-northd.c | 21 +++
>> >  tests/ovn.at| 58 +++--
>> >  2 files changed, 77 insertions(+), 2 deletions(-)
>> >
>> Hi Numan,
>>
>> Thanks for the fix. I have 2 comments:
>>
>> 1. The title is general which seems to address the problem for all
>> "router ports connected to localnet switches". However, the commit message
>> body and the code seems only handling the "reside-on-redirect-chassis"
>> scenario, without taking care of the more common scenario - the gateway
>> port GARP.
>>
>
> Agree. I think Ankur (CC'ed) is planning to handle this scenario - i.e
> sending the GAR for the gateway ports.
>
>
>
>>
>> 2. The fix seems all related to "nat_addresses" handling, but this
>> problem is not directly related to "NAT". After reading more context of the
>> code, I realized that it is the right place to fix, but it is really not
>> straightforward to understand. Of course this is not a problem introduced
>> by current patch. It would be better if we had named the option
>> "garp_addresses" instead of "nat_addresses", but I think it may be hard to
>> rename at this stage because it will introduce compatibility problem. So
>> probably we can add some more comments just to make the context more clear
>> for readers.
>>
>
> How about adding a new column "garp_addresses" ? This column can be used
> both for the gw port GARP and for the "reside-on-redirect-chassis" ?
>

Hi Numan, I am not sure if adding a new column is better than clarifying
with some documentation. If we add a new column, we should deprecate the
old "nat_addresses" column (and kept there only for ovsdb upgrading). I
don't think there is a need to distinguish in the schema if we are sending
GARP for NAT or for GW port.

>
> If this makes sense, I will work on it.
> @Ankur - I will submit a patch with a new column (which will send GARPs
> for both the gw router ports and other router ports with the option -
> reside-on-redirect-chassis ) and and then you can enhance it to send the
> GARPs in periodic interval instead of initial bursts  (which the present
> code does for NAT addresses ) ? Does this sound good ?
>
> Thanks
> Numan
>
>
>
>> Thanks,
>> Han
>>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [dpdk-latest PATCH] netdev-dpdk: Prefix network structures with rte_.

[Ilya Maximets]

On 11.06.2019 18:21, Ian Stokes wrote:
> On 6/11/2019 3:40 PM, Aaron Conole wrote:
>> Ian Stokes  writes:
>>
>>> On 6/10/2019 3:57 PM, Ian Stokes wrote:
 On 6/6/2019 12:36 PM, David Marchand wrote:
>
>
> On Thu, Jun 6, 2019 at 1:25 PM Ian Stokes  > wrote:
>
>  On 6/4/2019 12:14 PM, David Marchand wrote:
>   > On Tue, Jun 4, 2019 at 11:29 AM David Marchand
>   > mailto:david.march...@redhat.com>
>    >> wrote:
>   >
>   >     Following a rework of dpdk network structures names [1],
>  update the
>   >     concerned parts.
>   >
>   >     Ran Olivier script:
>   >     sh prefix-net-rte.sh $(find -name "*dpdk*.c")
>   >     sh prefix-net-rte.sh $(find -name "*dpdk*.h")
>   >     sh prefix-net-rte.sh $(find -name "*rte*.c")
>   >     sh prefix-net-rte.sh $(find -name "*rte*.h")
>   >
>   >     Plus an extra pass following further changes [2]:
>   >     old=RTE_IPv4
>   >     new=RTE_IPV4
>   >     git grep -lw $old | xargs sed -i -e "s/\<$old\>/$new/g"
>   >
>   >     old=RTE_ETHER_TYPE_IPv4
>   >     new=RTE_ETHER_TYPE_IPV4
>   >     git grep -lw $old | xargs sed -i -e "s/\<$old\>/$new/g"
>   >
>   >     old=RTE_ETHER_TYPE_IPv6
>   >     new=RTE_ETHER_TYPE_IPV6
>   >     git grep -lw $old | xargs sed -i -e "s/\<$old\>/$new/g"
>   >
>   >     1: http://mails.dpdk.org/archives/dev/2019-May/132612.html
>   >     2: https://git.dpdk.org/dpdk/commit/?id=0c9da7555da8
>   >
>   >
>   > Olivier noticed that I had used an early version of his patch.
>   > The published one handles the update on RTE_IPv4.
>   > I tried the last version which gives the same result anyway.
>   > So the extra pass is unnecessary.
>   >
>   > I can send a v2 to update the commitlog accordingly.
>   >
>
>  Hi David,
>
>  thanks for this, upon inspection the patch looks fine and I can
> confirm
>  that dpdk-latest is now building with Master of DPDK again.
>
>  I'm just in the process of running a few smoke tests to make sure
>  there's no issues functionally (I don't expect to see any as the
>  changes
>  seem straight forward).
>
>  WRT the v2, what exactly do you want to change in the commit? If it's
>  trivial I can amend it before committing.
>
>
>
> I just stripped the useless part in the commitlog and put a link to
> Olivier mail which contained his script.
> You can see the commitlog here:
> https://github.com/david-marchand/ovs/commit/9d367de7d323c28f7c89d590ff60373c47ffa073
>
>
>
>
>  I'll be applying this to dpdk-latest and dpdk-hwol branches but
> not ovs
>  master (master is still using DPDK 18.11.1 currently so no need for
>  these changes until it moves to 19.11).
>
>
>
> Yes, makes sense.
> Thanks Ian.

 Thanks David, validated and pushed to dpdk-latest and dpdk-wol.
>>>
>>> Good catch actually. In the past we had previously tracked the latest
>>> DPDK release to compile against, but now that dpdk-latest is used with
>>> the UNH DPDK CI it probably makes more sense to track DPDK master at
>>> that's what dpdk-latest looks to enable compilation of.
>>>
>>> I'm not against this, would be interested in what peoples thoughts are
>>> and if so we can modify travis for the dpdk-latest branch.
>>
>> At least for this part (per-branch), the travis yml is read on a
>> per-branch basis.  If it changes on one branch, only that branch will
>> be affected.  Is this what you mean?
> 
> Yes, travis would be changed to track master just for dpdk-latest is my 
> understanding. OVS master and OVS release branches should still only track 
> the DPDK LTS release they are currently validated against in their travis yml.

Makes sense. Broken travis builds on dpdk-latest branch are annoying.

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

[ovs-dev] [PATCH v2] travis: Don't install kernel for DPDK checks.

[Ilya Maximets]

We don't need to build DPDK kernel modules to test build with OVS.
And we don't need to build OVS datapath modules for checking
userspace with DPDK.

Removed 'max-inline-insns-single' changes that only was needed for
DPDK kernel modules. Config modifications changed to update
generated build/.config instead of changing sources.

Signed-off-by: Ilya Maximets 
---

Version 2:

  * Removed -Wno-error=inline.
  * config/common_base --> build/.config

 .travis.yml|  8 
 .travis/linux-build.sh | 25 +
 2 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 6621fb535..7addcb231 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -31,10 +31,10 @@ env:
   - TESTSUITE=1 KERNEL=3.16
   - TESTSUITE=1 OPTS="--enable-shared"
   - BUILD_ENV="-m32" OPTS="--disable-ssl"
-  - KERNEL=3.16 DPDK=1 OPTS="--enable-shared"
-  - KERNEL=3.16 TESTSUITE=1 DPDK=1
-  - KERNEL=3.16 DPDK_SHARED=1
-  - KERNEL=3.16 DPDK_SHARED=1 OPTS="--enable-shared"
+  - DPDK=1 OPTS="--enable-shared"
+  - TESTSUITE=1 DPDK=1
+  - DPDK_SHARED=1
+  - DPDK_SHARED=1 OPTS="--enable-shared"
   - KERNEL=4.20
   - KERNEL=4.19
   - KERNEL=4.18
diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
index 123cde575..b88bfb53e 100755
--- a/.travis/linux-build.sh
+++ b/.travis/linux-build.sh
@@ -3,7 +3,6 @@
 set -o errexit
 set -x
 
-KERNELSRC=""
 CFLAGS=""
 SPARSE_FLAGS=""
 EXTRA_OPTS="--enable-Werror"
@@ -57,10 +56,7 @@ function install_kernel()
 make net/bridge/
 fi
 
-KERNELSRC=$(pwd)
-if [ ! "$DPDK" ] && [ ! "$DPDK_SHARED" ]; then
-EXTRA_OPTS="${EXTRA_OPTS} --with-linux=$(pwd)"
-fi
+EXTRA_OPTS="${EXTRA_OPTS} --with-linux=$(pwd)"
 echo "Installed kernel source in $(pwd)"
 cd ..
 }
@@ -78,16 +74,21 @@ function install_dpdk()
 if [ $DIR_NAME != "dpdk-$1"  ]; then mv $DIR_NAME dpdk-$1; fi
 cd dpdk-$1
 fi
-find ./ -type f | xargs sed -i 
's/max-inline-insns-single=100/max-inline-insns-single=400/'
-find ./ -type f | xargs sed -i 's/-Werror/-Werror -Wno-error=inline/'
 echo 'CONFIG_RTE_BUILD_FPIC=y' >>config/common_linuxapp
 sed -ri '/EXECENV_CFLAGS  = -pthread -fPIC/{s/$/\nelse ifeq 
($(CONFIG_RTE_BUILD_FPIC),y)/;s/$/\nEXECENV_CFLAGS  = -pthread -fPIC/}' 
mk/exec-env/linuxapp/rte.vars.mk
+
+make config CC=gcc T=$TARGET
+
 if [ "$DPDK_SHARED" ]; then
-sed -i '/CONFIG_RTE_BUILD_SHARED_LIB=n/s/=n/=y/' config/common_base
+sed -i '/CONFIG_RTE_BUILD_SHARED_LIB=n/s/=n/=y/' build/.config
 export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$(pwd)/$TARGET/lib
 fi
-make config CC=gcc T=$TARGET
-make -j4 CC=gcc RTE_KERNELDIR=$KERNELSRC
+
+# Disable building DPDK kernel modules. Not needed for OVS build or tests.
+sed -i '/CONFIG_RTE_EAL_IGB_UIO=y/s/=y/=n/' build/.config
+sed -i '/CONFIG_RTE_KNI_KMOD=y/s/=y/=n/' build/.config
+
+make -j4 CC=gcc
 echo "Installed DPDK source in $(pwd)"
 cd ..
 }
@@ -97,7 +98,7 @@ function configure_ovs()
 ./boot.sh && ./configure $* || { cat config.log; exit 1; }
 }
 
-if [ "$KERNEL" ] || [ "$DPDK" ] || [ "$DPDK_SHARED" ]; then
+if [ "$KERNEL" ]; then
 install_kernel $KERNEL
 fi
 
@@ -141,7 +142,7 @@ else
 make selinux-policy
 
 # Only build datapath if we are testing kernel w/o running testsuite
-if [ "$KERNEL" ] && [ ! "$DPDK" ] && [ ! "$DPDK_SHARED" ]; then
+if [ "$KERNEL" ]; then
 cd datapath
 fi
 make -j4
-- 
2.17.1

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

Re: [ovs-dev] [PATCH] travis: Don't install kernel for DPDK checks.

[Ilya Maximets]

On 11.06.2019 16:49, David Marchand wrote:
> 
> 
> On Tue, Jun 11, 2019 at 3:31 PM Ilya Maximets  > wrote:
> 
> We don't need to build DPDK kernel modules to test build with OVS.
> And we don't need to build OVS datapath modules for checking
> userspace with DPDK.
> 
> Removed 'max-inline-insns-single' changes that only was needed for
> DPDK kernel modules.
> 
> Signed-off-by: Ilya Maximets  >
> ---
>  .travis.yml            |  8 
>  .travis/linux-build.sh | 18 +-
>  2 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/.travis.yml b/.travis.yml
> index 6621fb535..7addcb231 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -31,10 +31,10 @@ env:
>    - TESTSUITE=1 KERNEL=3.16
>    - TESTSUITE=1 OPTS="--enable-shared"
>    - BUILD_ENV="-m32" OPTS="--disable-ssl"
> -  - KERNEL=3.16 DPDK=1 OPTS="--enable-shared"
> -  - KERNEL=3.16 TESTSUITE=1 DPDK=1
> -  - KERNEL=3.16 DPDK_SHARED=1
> -  - KERNEL=3.16 DPDK_SHARED=1 OPTS="--enable-shared"
> +  - DPDK=1 OPTS="--enable-shared"
> +  - TESTSUITE=1 DPDK=1
> +  - DPDK_SHARED=1
> +  - DPDK_SHARED=1 OPTS="--enable-shared"
>    - KERNEL=4.20
>    - KERNEL=4.19
>    - KERNEL=4.18
> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
> index 123cde575..8c0291dd2 100755
> --- a/.travis/linux-build.sh
> +++ b/.travis/linux-build.sh
> @@ -3,7 +3,6 @@
>  set -o errexit
>  set -x
> 
> -KERNELSRC=""
>  CFLAGS=""
>  SPARSE_FLAGS=""
>  EXTRA_OPTS="--enable-Werror"
> @@ -57,10 +56,7 @@ function install_kernel()
>          make net/bridge/
>      fi
> 
> -    KERNELSRC=$(pwd)
> -    if [ ! "$DPDK" ] && [ ! "$DPDK_SHARED" ]; then
> -        EXTRA_OPTS="${EXTRA_OPTS} --with-linux=$(pwd)"
> -    fi
> 
> +    EXTRA_OPTS="${EXTRA_OPTS} --with-linux=$(pwd)"
>      echo "Installed kernel source in $(pwd)"
>      cd ..
>  }
> @@ -78,7 +74,6 @@ function install_dpdk()
>          if [ $DIR_NAME != "dpdk-$1"  ]; then mv $DIR_NAME dpdk-$1; fi
>          cd dpdk-$1
>      fi
> -    find ./ -type f | xargs sed -i 
> 's/max-inline-insns-single=100/max-inline-insns-single=400/'
>      find ./ -type f | xargs sed -i 's/-Werror/-Werror -Wno-error=inline/'
> 
> 
> Both lines go together (else, that would mean that dpdk does not build fine 
> by itself).
> So you should remove both of them.

Indeed. Thanks.

> 
> 
> 
>      echo 'CONFIG_RTE_BUILD_FPIC=y' >>config/common_linuxapp
>      sed -ri '/EXECENV_CFLAGS  = -pthread -fPIC/{s/$/\nelse ifeq 
> ($(CONFIG_RTE_BUILD_FPIC),y)/;s/$/\nEXECENV_CFLAGS  = -pthread -fPIC/}' 
> mk/exec-env/linuxapp/rte.vars.mk 
> 
> @@ -86,8 +81,13 @@ function install_dpdk()
>          sed -i '/CONFIG_RTE_BUILD_SHARED_LIB=n/s/=n/=y/' 
> config/common_base
>          export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$(pwd)/$TARGET/lib
>      fi
> +
> +    # Disable building DPDK kernel modules. Not needed for OVS build or 
> tests.
> +    sed -i '/CONFIG_RTE_EAL_IGB_UIO=y/s/=y/=n/' config/common_linuxapp
> +    sed -i '/CONFIG_RTE_KNI_KMOD=y/s/=y/=n/' config/common_linuxapp
> +
> 
> 
> Patching the dpdk sources feels quite dangerous to me.
> And it is broken on dpdk master since this file has been renamed.
> 
> How about editing build/.config after calling 'make config' instead ?
> 
> I did like this, to compile against dpdk master:
> https://github.com/david-marchand/ovs/commit/e3c7877aa76247cb06f8c2832f99230a6d8e8675

Sure. This will be cleaner.

BTW, you may send fPIC related part of your change as a separate patch
for current master. It looks much better to pass it via EXTRA_CFLAGS.

> 
> 
>      make config CC=gcc T=$TARGET
> -    make -j4 CC=gcc RTE_KERNELDIR=$KERNELSRC
> +    make -j4 CC=gcc
>      echo "Installed DPDK source in $(pwd)"
>      cd ..
>  }
> @@ -97,7 +97,7 @@ function configure_ovs()
>      ./boot.sh && ./configure $* || { cat config.log; exit 1; }
>  }
> 
> -if [ "$KERNEL" ] || [ "$DPDK" ] || [ "$DPDK_SHARED" ]; then
> +if [ "$KERNEL" ]; then
>      install_kernel $KERNEL
>  fi
> 
> @@ -141,7 +141,7 @@ else
>      make selinux-policy
> 
>      # Only build datapath if we are testing kernel w/o running testsuite
> -    if [ "$KERNEL" ] && [ ! "$DPDK" ] && [ ! "$DPDK_SHARED" ]; then
> +    if [ "$KERNEL" ]; then
>          cd datapath
>      fi
>      make -j4
> -- 
> 2.17.1
> 
> 
> 
> -- 
> David Marchand
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [dpdk-latest PATCH] netdev-dpdk: Prefix network structures with rte_.

[Ian Stokes]

On 6/11/2019 3:40 PM, Aaron Conole wrote:

Ian Stokes  writes:


On 6/10/2019 3:57 PM, Ian Stokes wrote:

On 6/6/2019 12:36 PM, David Marchand wrote:



On Thu, Jun 6, 2019 at 1:25 PM Ian Stokes mailto:ian.sto...@intel.com>> wrote:

     On 6/4/2019 12:14 PM, David Marchand wrote:
  > On Tue, Jun 4, 2019 at 11:29 AM David Marchand
  > mailto:david.march...@redhat.com>
     >> wrote:
  >
  >     Following a rework of dpdk network structures names [1],
     update the
  >     concerned parts.
  >
  >     Ran Olivier script:
  >     sh prefix-net-rte.sh $(find -name "*dpdk*.c")
  >     sh prefix-net-rte.sh $(find -name "*dpdk*.h")
  >     sh prefix-net-rte.sh $(find -name "*rte*.c")
  >     sh prefix-net-rte.sh $(find -name "*rte*.h")
  >
  >     Plus an extra pass following further changes [2]:
  >     old=RTE_IPv4
  >     new=RTE_IPV4
  >     git grep -lw $old | xargs sed -i -e "s/\<$old\>/$new/g"
  >
  >     old=RTE_ETHER_TYPE_IPv4
  >     new=RTE_ETHER_TYPE_IPV4
  >     git grep -lw $old | xargs sed -i -e "s/\<$old\>/$new/g"
  >
  >     old=RTE_ETHER_TYPE_IPv6
  >     new=RTE_ETHER_TYPE_IPV6
  >     git grep -lw $old | xargs sed -i -e "s/\<$old\>/$new/g"
  >
  >     1: http://mails.dpdk.org/archives/dev/2019-May/132612.html
  >     2: https://git.dpdk.org/dpdk/commit/?id=0c9da7555da8
  >
  >
  > Olivier noticed that I had used an early version of his patch.
  > The published one handles the update on RTE_IPv4.
  > I tried the last version which gives the same result anyway.
  > So the extra pass is unnecessary.
  >
  > I can send a v2 to update the commitlog accordingly.
  >

     Hi David,

     thanks for this, upon inspection the patch looks fine and I can
confirm
     that dpdk-latest is now building with Master of DPDK again.

     I'm just in the process of running a few smoke tests to make sure
     there's no issues functionally (I don't expect to see any as the
     changes
     seem straight forward).

     WRT the v2, what exactly do you want to change in the commit? If it's
     trivial I can amend it before committing.



I just stripped the useless part in the commitlog and put a link to
Olivier mail which contained his script.
You can see the commitlog here:
https://github.com/david-marchand/ovs/commit/9d367de7d323c28f7c89d590ff60373c47ffa073




     I'll be applying this to dpdk-latest and dpdk-hwol branches but
not ovs
     master (master is still using DPDK 18.11.1 currently so no need for
     these changes until it moves to 19.11).



Yes, makes sense.
Thanks Ian.


Thanks David, validated and pushed to dpdk-latest and dpdk-wol.


Good catch actually. In the past we had previously tracked the latest
DPDK release to compile against, but now that dpdk-latest is used with
the UNH DPDK CI it probably makes more sense to track DPDK master at
that's what dpdk-latest looks to enable compilation of.

I'm not against this, would be interested in what peoples thoughts are
and if so we can modify travis for the dpdk-latest branch.


At least for this part (per-branch), the travis yml is read on a
per-branch basis.  If it changes on one branch, only that branch will
be affected.  Is this what you mean?


Yes, travis would be changed to track master just for dpdk-latest is my 
understanding. OVS master and OVS release branches should still only 
track the DPDK LTS release they are currently validated against in their 
travis yml.


Ian



Ian


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


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

Re: [ovs-dev] [RFC 1/3] OVN: introduce Controller_Event table

[Lorenzo Bianconi]

> On Tue, Jun 11, 2019 at 11:54:31AM +0200, Lorenzo Bianconi wrote:
> > > On Wed, May 29, 2019 at 04:05:07PM +0200, Lorenzo Bianconi wrote:
> > > > > On Thu, May 16, 2019 at 06:05:24PM +0200, Lorenzo Bianconi wrote:
> > > > > > Add Controller_Event table to OVN SBDB in order to
> > > > > > report CMS related event.
> > > > > > Introduce event_table hashmap array and controller_event related
> > > > > > structures to ovn-controller in order to track pending events
> > > > > > forwarded by ovs-vswitchd. Moreover integrate event_table hashmap
> > > > > > array with event_table ovn-sbdb table
> > > > > > 
> > > > > > Signed-off-by: Mark Michelson 
> > > > > > Co-authored-by: Mark Michelson 
> > > > > > Signed-off-by: Lorenzo Bianconi 
> > > 
> > > ...
> > > 
> > > > > 4. What is the tolerance for events that are never delivered or that 
> > > > > are
> > > > >delivered more than once?  What can actually be guaranteed, given
> > > > >that the database can die and that ovn-controller can die?  (Also,
> > > > >OVSDB transactions cannot guarantee exactly-once semantics in 
> > > > > corner
> > > > >cases unless the transactions are idempotent.)
> > > > 
> > > > If the ovn-controller dies I think there is no too much we can do, 
> > > > events will
> > > > be lost until the controller restarts properly.
> > > > If ovn-northd or the connection to the db dies, controller_event_run() 
> > > > will not
> > > > manage the Controller_Event table and pinctrl_handle_event() will queue 
> > > > the
> > > > pending events in the event_table hash until the upper limit is reached.
> > > > We can probably add a garbage collector for the pending events in the 
> > > > table.
> > > > What do you think?
> > > 
> > > What's the consequence if an event is missed?  What's the consequence if
> > > an event is pushed two or more times?  It's easiest to design a
> > > distributed system so that it's OK if an event is delivered zero times
> > > or multiple times.  It's a little harder to design so that an event is
> > > delivered one or more times.  It's hardest to design so that an event is
> > > delivered exactly one time.
> > 
> > Hi Ben,
> > 
> > thx a lot for your comments,
> > 
> > > 
> > > There are the following obvious points of failure from these points of
> > > view:
> > > 
> > > 1. ovn-controller.  If it dies, it might not push an event that it
> > >should.  When it comes back up, will it know to push the event that
> > >it missed?  What about if it dies while it is pushing an event; is it
> > >possible that it will push it again when it comes up?
> > 
> > This is probably not an issue since if the event is lost because the 
> > controller
> > is dead we will receive a new one when the controller comes back.
> > If the controller dies after sending the event to the db it will not receive
> > new events when it comes back
> > 
> > > 
> > > 2. The OVSDB protocol.  If the OVSDB connection dies after
> > >ovn-controller's transaction is committed but before ovn-controller
> > >receives the acknowledgment, then when it reconnects ovn-controller
> > >might retry it, which could lead to an event being pushed two or more
> > >times.
> > > 
> > > 3. ovsdb-server.  Clients don't typically use the OVSDB protocol feature
> > >that ensures that a transaction is committed to stable storage before
> > >it is acknowledged, so an event could get lost if ovsdb-server dies
> > >after acknowledging a transaction but before it gets written to disk.
> > >(Clustered OVSDB always does sync to stable storage though.)
> > > 
> > > 4. ovn-northd.  There is a race between ovn-northd acting on an event
> > >and marking it handled (or deleting it).  There are also the same
> > >OVSDB protocol and ovsdb-server races in the reverse direction.
> > 
> > I agree with you, even if these kind of events (duplicated events or 
> > duplicated
> > rows in the db) are quite unlikely since controller_event processing is 
> > done holding
> > pinctrl_mutex, they can happen. However I think these kind of events can be 
> > managed
> > by the CMS since the controller does not have the 'history' of already 
> > handled events.
> 
> OK.
> 
> Will you please document what is (not) guaranteed in the documentation
> somewhere?  It's important to write these things down or people are
> likely to make bad assumptions later.

ack, will do posting a formal series.

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

Re: [ovs-dev] OVS command line style/syntax modification

[Ben Pfaff]

On Tue, Jun 11, 2019 at 11:18:22AM +0430, s...@qbicomm.com wrote:
> IS it possible to modify command line style? i mean is it possible to modify
> command syntax and how commands and sub commands are entered to make working
> with OVS easier?

What changes are you looking for?

> or use tools such as vtysh/Clish/Klish with OVS ?

I have not heard of those tools, so I do not know.  Maybe someone else
will speak up.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovn-controller: Cleanup memory in binding_evaluate_port_binding_changes

[Dumitru Ceara]

On Tue, Jun 11, 2019 at 4:48 PM Han Zhou  wrote:
>
>
>
> On Tue, Jun 11, 2019 at 5:09 AM Dumitru Ceara  wrote:
> >
> > The 'lport_to_iface' and 'egress_ifaces' hashtables were not cleaned up
> > in case a port binding was found that would require a recompute.
> >
> > Reported-at: 
> > https://mail.openvswitch.org/pipermail/ovs-discuss/2019-June/048822.html
> > Reported-by: Daniel Alvarez Sanchez 
> > Fixes: 9d0b504abdee ("ovn-controller: runtime_data change handler for SB 
> > port-binding")
> > CC: Han Zhou 
> > Signed-off-by: Dumitru Ceara 
> > ---
> >  ovn/controller/binding.c | 10 --
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
> > index b62b3da..87d0b6d 100644
> > --- a/ovn/controller/binding.c
> > +++ b/ovn/controller/binding.c
> > @@ -695,6 +695,8 @@ binding_evaluate_port_binding_changes(
> >  return true;
> >  }
> >
> > +bool changed = false;
> > +
> >  const struct sbrec_port_binding *binding_rec;
> >  struct shash lport_to_iface = SHASH_INITIALIZER(_to_iface);
> >  struct sset egress_ifaces = SSET_INITIALIZER(_ifaces);
> > @@ -718,10 +720,14 @@ binding_evaluate_port_binding_changes(
> >  || is_our_chassis(chassis_rec, binding_rec,
> >active_tunnels, _to_iface, 
> > local_lports)
> >  || strcmp(binding_rec->type, "")) {
> > -return true;
> > +changed = true;
> > +break;
> >  }
> >  }
> > -return false;
> > +
> > +shash_destroy(_to_iface);
> > +sset_destroy(_ifaces);
> > +return changed;
> >  }
> >
> >  /* Returns true if the database is all cleaned up, false if more work is
> > --
> > 1.8.3.1
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
> Thanks for the fix! This was my bad.
> The leak should be there regardless of recompute or not, so the commit 
> message may be updated.
>
> Acked-by: Han Zhou 
>

Thanks for the review. You're right, the commit log was wrong, i sent
an amended version:
https://patchwork.ozlabs.org/patch/1113901/

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

Re: [ovs-dev] [RFC 1/3] OVN: introduce Controller_Event table

[Ben Pfaff]

On Tue, Jun 11, 2019 at 11:54:31AM +0200, Lorenzo Bianconi wrote:
> > On Wed, May 29, 2019 at 04:05:07PM +0200, Lorenzo Bianconi wrote:
> > > > On Thu, May 16, 2019 at 06:05:24PM +0200, Lorenzo Bianconi wrote:
> > > > > Add Controller_Event table to OVN SBDB in order to
> > > > > report CMS related event.
> > > > > Introduce event_table hashmap array and controller_event related
> > > > > structures to ovn-controller in order to track pending events
> > > > > forwarded by ovs-vswitchd. Moreover integrate event_table hashmap
> > > > > array with event_table ovn-sbdb table
> > > > > 
> > > > > Signed-off-by: Mark Michelson 
> > > > > Co-authored-by: Mark Michelson 
> > > > > Signed-off-by: Lorenzo Bianconi 
> > 
> > ...
> > 
> > > > 4. What is the tolerance for events that are never delivered or that are
> > > >delivered more than once?  What can actually be guaranteed, given
> > > >that the database can die and that ovn-controller can die?  (Also,
> > > >OVSDB transactions cannot guarantee exactly-once semantics in corner
> > > >cases unless the transactions are idempotent.)
> > > 
> > > If the ovn-controller dies I think there is no too much we can do, events 
> > > will
> > > be lost until the controller restarts properly.
> > > If ovn-northd or the connection to the db dies, controller_event_run() 
> > > will not
> > > manage the Controller_Event table and pinctrl_handle_event() will queue 
> > > the
> > > pending events in the event_table hash until the upper limit is reached.
> > > We can probably add a garbage collector for the pending events in the 
> > > table.
> > > What do you think?
> > 
> > What's the consequence if an event is missed?  What's the consequence if
> > an event is pushed two or more times?  It's easiest to design a
> > distributed system so that it's OK if an event is delivered zero times
> > or multiple times.  It's a little harder to design so that an event is
> > delivered one or more times.  It's hardest to design so that an event is
> > delivered exactly one time.
> 
> Hi Ben,
> 
> thx a lot for your comments,
> 
> > 
> > There are the following obvious points of failure from these points of
> > view:
> > 
> > 1. ovn-controller.  If it dies, it might not push an event that it
> >should.  When it comes back up, will it know to push the event that
> >it missed?  What about if it dies while it is pushing an event; is it
> >possible that it will push it again when it comes up?
> 
> This is probably not an issue since if the event is lost because the 
> controller
> is dead we will receive a new one when the controller comes back.
> If the controller dies after sending the event to the db it will not receive
> new events when it comes back
> 
> > 
> > 2. The OVSDB protocol.  If the OVSDB connection dies after
> >ovn-controller's transaction is committed but before ovn-controller
> >receives the acknowledgment, then when it reconnects ovn-controller
> >might retry it, which could lead to an event being pushed two or more
> >times.
> > 
> > 3. ovsdb-server.  Clients don't typically use the OVSDB protocol feature
> >that ensures that a transaction is committed to stable storage before
> >it is acknowledged, so an event could get lost if ovsdb-server dies
> >after acknowledging a transaction but before it gets written to disk.
> >(Clustered OVSDB always does sync to stable storage though.)
> > 
> > 4. ovn-northd.  There is a race between ovn-northd acting on an event
> >and marking it handled (or deleting it).  There are also the same
> >OVSDB protocol and ovsdb-server races in the reverse direction.
> 
> I agree with you, even if these kind of events (duplicated events or 
> duplicated
> rows in the db) are quite unlikely since controller_event processing is done 
> holding
> pinctrl_mutex, they can happen. However I think these kind of events can be 
> managed
> by the CMS since the controller does not have the 'history' of already 
> handled events.

OK.

Will you please document what is (not) guaranteed in the documentation
somewhere?  It's important to write these things down or people are
likely to make bad assumptions later.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] ovn-controller: Cleanup memory in binding_evaluate_port_binding_changes

[Dumitru Ceara]

The 'lport_to_iface' and 'egress_ifaces' hashtables were not cleaned up
when checking if port bindings require a recompute.

Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-discuss/2019-June/048822.html
Reported-by: Daniel Alvarez Sanchez 
Fixes: 9d0b504abdee ("ovn-controller: runtime_data change handler for SB 
port-binding")
Acked-by: Han Zhou 
Signed-off-by: Dumitru Ceara 

---
v2: Amend commit log message.
---
 ovn/controller/binding.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index b62b3da..87d0b6d 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -695,6 +695,8 @@ binding_evaluate_port_binding_changes(
 return true;
 }
 
+bool changed = false;
+
 const struct sbrec_port_binding *binding_rec;
 struct shash lport_to_iface = SHASH_INITIALIZER(_to_iface);
 struct sset egress_ifaces = SSET_INITIALIZER(_ifaces);
@@ -718,10 +720,14 @@ binding_evaluate_port_binding_changes(
 || is_our_chassis(chassis_rec, binding_rec,
   active_tunnels, _to_iface, local_lports)
 || strcmp(binding_rec->type, "")) {
-return true;
+changed = true;
+break;
 }
 }
-return false;
+
+shash_destroy(_to_iface);
+sset_destroy(_ifaces);
+return changed;
 }
 
 /* Returns true if the database is all cleaned up, false if more work is
-- 
1.8.3.1

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

Re: [ovs-dev] [PATCHv11] netdev-afxdp: add new netdev type for AF_XDP.

[Ben Pfaff]

On Tue, Jun 11, 2019 at 08:47:19AM +0200, Eelco Chaudron wrote:
> 
> 
> On 8 Jun 2019, at 6:48, William Tu wrote:
> 
> > > > > +  ethtool -L enp2s0 combined 1
> > > > > +  ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x10
> > > > > +  ovs-vsctl add-port br0 enp2s0 -- set interface enp2s0
> > > > > type="afxdp"
> > > > > \
> > > > > +options:n_rxq=1 options:xdpmode=drv \
> > > > > +other_config:pmd-rxq-affinity="0:4"
> > 
> > another feature I'm thinking about to add is a new options
> > for loading custom XDP program
> > 
> > For example:
> > ovs-vsctl add-port br0 enp2s0 -- set interface enp2s0 type="afxdp"
> > options:n_rxq=1 options:xdpmode=drv
> > options:xdp_prog=/path/to/xdp.o
> > 
> > If users do not specify the path, then it is using the libbpf's default
> > program
> > (which forwards all packets to userspace)
> > 
> > If users want to use their own xdp object, then this option can load the
> > xdp object file from the path.
> 
> This might be useful, specially if you would like to do some experiments.

There could be a security risk here depending on how we sanitize the
path.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovn-controller: Cleanup memory in binding_evaluate_port_binding_changes

[Han Zhou]

On Tue, Jun 11, 2019 at 5:09 AM Dumitru Ceara  wrote:
>
> The 'lport_to_iface' and 'egress_ifaces' hashtables were not cleaned up
> in case a port binding was found that would require a recompute.
>
> Reported-at:
https://mail.openvswitch.org/pipermail/ovs-discuss/2019-June/048822.html
> Reported-by: Daniel Alvarez Sanchez 
> Fixes: 9d0b504abdee ("ovn-controller: runtime_data change handler for SB
port-binding")
> CC: Han Zhou 
> Signed-off-by: Dumitru Ceara 
> ---
>  ovn/controller/binding.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
> index b62b3da..87d0b6d 100644
> --- a/ovn/controller/binding.c
> +++ b/ovn/controller/binding.c
> @@ -695,6 +695,8 @@ binding_evaluate_port_binding_changes(
>  return true;
>  }
>
> +bool changed = false;
> +
>  const struct sbrec_port_binding *binding_rec;
>  struct shash lport_to_iface = SHASH_INITIALIZER(_to_iface);
>  struct sset egress_ifaces = SSET_INITIALIZER(_ifaces);
> @@ -718,10 +720,14 @@ binding_evaluate_port_binding_changes(
>  || is_our_chassis(chassis_rec, binding_rec,
>active_tunnels, _to_iface,
local_lports)
>  || strcmp(binding_rec->type, "")) {
> -return true;
> +changed = true;
> +break;
>  }
>  }
> -return false;
> +
> +shash_destroy(_to_iface);
> +sset_destroy(_ifaces);
> +return changed;
>  }
>
>  /* Returns true if the database is all cleaned up, false if more work is
> --
> 1.8.3.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Thanks for the fix! This was my bad.
The leak should be there regardless of recompute or not, so the commit
message may be updated.

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

[ovs-dev] Génesis y desarrollo del México actual

[Cultura para ejecutivos extranjeros]

¡Hola!

Me da mucho gusto saludarte.

Es para mi un placer poder invitarte a nuestro webinar "Historia y cultura de 
México para ejecutivos extranjeros" que se estará
llevando a cabo el día Viernes 05 de Julio con un horario de 10:00 a 14:00 hrs. 
(hora de la ciudad de México)


Con este curso en línea lograremos:

Las raíces culturales indígenas aportadas por el México prehispánico. 
La influencia española y la formación de una cultura criolla y mestiza 
mexicana. 
La génesis y desarrollo del México actual, en el inicio de vida independiente.
El experto que nos acompañará Alejandro Ibarra tiene una formación dual idónea 
para el curso: posee una trayectoria de más de 30 años como alto
ejecutivo de la función de compras en empresas nacionales, globales, públicas y 
privadas, y se desempeñaactualmente como consultor y capacitador. 
Tiene asimismo estudios de maestría en historia de México (candidato al grado), 
especializado en historia política del siglo XIX. Para el expositor el
conocimiento histórico es esencial para comprender la cultura de un país, y ha 
podido constatar las limitaciones que la falta de este conocimiento provoca 
a los ejecutivos extranjeros y sus familias en su integración profesional y 
social.

Te invito a que participes con nosotros y conozcas los beneficios de la 
capacitación en línea.

Tel: (045) 55 15 54 66 30 

Solicita información respondiendo a este correo con la palabra México, junto 
con los siguientes datos:

Nombre:
Correo electrónico:
Número telefónico:

¡Qué tengas un excelente día!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [dpdk-latest PATCH] netdev-dpdk: Prefix network structures with rte_.

[Aaron Conole]

Ian Stokes  writes:

> On 6/10/2019 3:57 PM, Ian Stokes wrote:
>> On 6/6/2019 12:36 PM, David Marchand wrote:
>>>
>>>
>>> On Thu, Jun 6, 2019 at 1:25 PM Ian Stokes >> > wrote:
>>>
>>>     On 6/4/2019 12:14 PM, David Marchand wrote:
>>>  > On Tue, Jun 4, 2019 at 11:29 AM David Marchand
>>>  > mailto:david.march...@redhat.com>
>>>     >>     >> wrote:
>>>  >
>>>  >     Following a rework of dpdk network structures names [1],
>>>     update the
>>>  >     concerned parts.
>>>  >
>>>  >     Ran Olivier script:
>>>  >     sh prefix-net-rte.sh $(find -name "*dpdk*.c")
>>>  >     sh prefix-net-rte.sh $(find -name "*dpdk*.h")
>>>  >     sh prefix-net-rte.sh $(find -name "*rte*.c")
>>>  >     sh prefix-net-rte.sh $(find -name "*rte*.h")
>>>  >
>>>  >     Plus an extra pass following further changes [2]:
>>>  >     old=RTE_IPv4
>>>  >     new=RTE_IPV4
>>>  >     git grep -lw $old | xargs sed -i -e "s/\<$old\>/$new/g"
>>>  >
>>>  >     old=RTE_ETHER_TYPE_IPv4
>>>  >     new=RTE_ETHER_TYPE_IPV4
>>>  >     git grep -lw $old | xargs sed -i -e "s/\<$old\>/$new/g"
>>>  >
>>>  >     old=RTE_ETHER_TYPE_IPv6
>>>  >     new=RTE_ETHER_TYPE_IPV6
>>>  >     git grep -lw $old | xargs sed -i -e "s/\<$old\>/$new/g"
>>>  >
>>>  >     1: http://mails.dpdk.org/archives/dev/2019-May/132612.html
>>>  >     2: https://git.dpdk.org/dpdk/commit/?id=0c9da7555da8
>>>  >
>>>  >
>>>  > Olivier noticed that I had used an early version of his patch.
>>>  > The published one handles the update on RTE_IPv4.
>>>  > I tried the last version which gives the same result anyway.
>>>  > So the extra pass is unnecessary.
>>>  >
>>>  > I can send a v2 to update the commitlog accordingly.
>>>  >
>>>
>>>     Hi David,
>>>
>>>     thanks for this, upon inspection the patch looks fine and I can
>>> confirm
>>>     that dpdk-latest is now building with Master of DPDK again.
>>>
>>>     I'm just in the process of running a few smoke tests to make sure
>>>     there's no issues functionally (I don't expect to see any as the
>>>     changes
>>>     seem straight forward).
>>>
>>>     WRT the v2, what exactly do you want to change in the commit? If it's
>>>     trivial I can amend it before committing.
>>>
>>>
>>>
>>> I just stripped the useless part in the commitlog and put a link to
>>> Olivier mail which contained his script.
>>> You can see the commitlog here:
>>> https://github.com/david-marchand/ovs/commit/9d367de7d323c28f7c89d590ff60373c47ffa073
>>>  
>>>
>>>
>>>
>>>
>>>     I'll be applying this to dpdk-latest and dpdk-hwol branches but
>>> not ovs
>>>     master (master is still using DPDK 18.11.1 currently so no need for
>>>     these changes until it moves to 19.11).
>>>
>>>
>>>
>>> Yes, makes sense.
>>> Thanks Ian.
>>
>> Thanks David, validated and pushed to dpdk-latest and dpdk-wol.
>
> Good catch actually. In the past we had previously tracked the latest
> DPDK release to compile against, but now that dpdk-latest is used with
> the UNH DPDK CI it probably makes more sense to track DPDK master at
> that's what dpdk-latest looks to enable compilation of.
>
> I'm not against this, would be interested in what peoples thoughts are
> and if so we can modify travis for the dpdk-latest branch.

At least for this part (per-branch), the travis yml is read on a
per-branch basis.  If it changes on one branch, only that branch will
be affected.  Is this what you mean?

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

Re: [ovs-dev] [dpdk-dev] Forcing inlining for igb_uio and kni

[Bruce Richardson]

On Tue, Jun 11, 2019 at 07:25:09AM -0700, Stephen Hemminger wrote:
> On Tue, 11 Jun 2019 11:18:22 +0100
> Bruce Richardson  wrote:
> 
> > On Tue, Jun 11, 2019 at 11:44:01AM +0200, David Marchand wrote:
> > >On Tue, Jun 11, 2019 at 11:31 AM Ilya Maximets
> > ><[1]i.maxim...@samsung.com> wrote:
> > > 
> > >  On 11.06.2019 11:45, David Marchand wrote:  
> > >  > I noticed that OVS CI [1] patches the dpdk sources to force some  
> > >  inlining parameters and get kni and igb_uio to build fine.  
> > >  >
> > >  > Looking at it in dpdk, meson support dropped this.
> > >  > In the makefiles, I can't find a reason in the git history (we go  
> > >  back to 1.3.0rX version).  
> > >  >
> > >  > [dmarchan@dmarchan dpdk]$ git grep max-inline-insns-single
> > >  > kernel/linux/igb_uio/Makefile:MODULE_CFLAGS += -I$(SRCDIR) --param 
> > >  
> > >  max-inline-insns-single=100  
> > >  > kernel/linux/kni/Makefile:MODULE_CFLAGS += -I$(SRCDIR) --param  
> > >  max-inline-insns-single=50  
> > >  > [dmarchan@dmarchan dpdk]$ git blame origin/master --  
> > >  kernel/linux/igb_uio/Makefile |grep max-inline-insns-single  
> > >  > 13dc56a6 lib/librte_eal/linuxapp/igb_uio/Makefile (Intel  
> > >   2012-12-20 00:00:00 +0100 15) MODULE_CFLAGS += -I$(SRCDIR) --param
> > >  max-inline-insns-single=100  
> > >  > [dmarchan@dmarchan dpdk]$ git blame origin/master --  
> > >  kernel/linux/kni/Makefile |grep max-inline-insns-single  
> > >  > 3fc5ca2f lib/librte_eal/linuxapp/kni/Makefile (Intel  
> > >  2012-12-20 00:00:00 +0100 14) MODULE_CFLAGS += -I$(SRCDIR) --param
> > >  max-inline-insns-single=50  
> > >  >
> > >  > Is there a valid reason to keep this?
> > >  > 1:  
> > >  [2]https://github.com/openvswitch/ovs/blob/master/.travis/linux-buil
> > >  d.sh#L81
> > >  <[3]https://protect2.fireeye.com/url?k=b7de159c45b1fa79.b7df9ed3-c48
> > >  06461f28ecaf5=https://github.com/openvswitch/ovs/blob/master/.trav
> > >  is/linux-build.sh#L81>
> > >  Hi, David.
> > >  I don't know the reason for these in OVS travis config, But we don't
> > >  need to know them, actually. I have a patch to drop all the kernel
> > >  related stuff from the DPDK build in OVS Travis checks, just didn't
> > >  send it yet. Will send soon, probably.
> > > 
> > >I had this in mind since we don't need those kmods in the CI.
> > >Thanks Ilya.
> > >The question on dpdk side remains open :-).
> > >--
> > >David Marchand  
> > 
> > I know that previously we did have issues with the modules not compiling
> > due to errors about maximum levels of inlines. Whether that was because of
> > the compiler, or the kernel makefiles at the time, I'm not sure. The kmods
> > seem to build fine for me now without the parameters, and the fact that
> > there has never been a problem reported with building using meson either,
> > probably indicates that the extra compiler flags can be dropped.
> > 
> > /Bruce
> 
> I really doubt forcing inlining makes any difference to the performance.

I agree, especially for uio module. This was not a "perf" issue, but a
"just own't compile" issue, IIRC. :-)

That being said, given that this is just a line in two makefiles, the rule
about "if it ain't broke" may apply
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [dpdk-dev] Forcing inlining for igb_uio and kni

[Stephen Hemminger]

On Tue, 11 Jun 2019 11:18:22 +0100
Bruce Richardson  wrote:

> On Tue, Jun 11, 2019 at 11:44:01AM +0200, David Marchand wrote:
> >On Tue, Jun 11, 2019 at 11:31 AM Ilya Maximets
> ><[1]i.maxim...@samsung.com> wrote:
> > 
> >  On 11.06.2019 11:45, David Marchand wrote:  
> >  > I noticed that OVS CI [1] patches the dpdk sources to force some  
> >  inlining parameters and get kni and igb_uio to build fine.  
> >  >
> >  > Looking at it in dpdk, meson support dropped this.
> >  > In the makefiles, I can't find a reason in the git history (we go  
> >  back to 1.3.0rX version).  
> >  >
> >  > [dmarchan@dmarchan dpdk]$ git grep max-inline-insns-single
> >  > kernel/linux/igb_uio/Makefile:MODULE_CFLAGS += -I$(SRCDIR) --param  
> >  max-inline-insns-single=100  
> >  > kernel/linux/kni/Makefile:MODULE_CFLAGS += -I$(SRCDIR) --param  
> >  max-inline-insns-single=50  
> >  > [dmarchan@dmarchan dpdk]$ git blame origin/master --  
> >  kernel/linux/igb_uio/Makefile |grep max-inline-insns-single  
> >  > 13dc56a6 lib/librte_eal/linuxapp/igb_uio/Makefile (Intel  
> >   2012-12-20 00:00:00 +0100 15) MODULE_CFLAGS += -I$(SRCDIR) --param
> >  max-inline-insns-single=100  
> >  > [dmarchan@dmarchan dpdk]$ git blame origin/master --  
> >  kernel/linux/kni/Makefile |grep max-inline-insns-single  
> >  > 3fc5ca2f lib/librte_eal/linuxapp/kni/Makefile (Intel  
> >  2012-12-20 00:00:00 +0100 14) MODULE_CFLAGS += -I$(SRCDIR) --param
> >  max-inline-insns-single=50  
> >  >
> >  > Is there a valid reason to keep this?
> >  > 1:  
> >  [2]https://github.com/openvswitch/ovs/blob/master/.travis/linux-buil
> >  d.sh#L81
> >  <[3]https://protect2.fireeye.com/url?k=b7de159c45b1fa79.b7df9ed3-c48
> >  06461f28ecaf5=https://github.com/openvswitch/ovs/blob/master/.trav
> >  is/linux-build.sh#L81>
> >  Hi, David.
> >  I don't know the reason for these in OVS travis config, But we don't
> >  need to know them, actually. I have a patch to drop all the kernel
> >  related stuff from the DPDK build in OVS Travis checks, just didn't
> >  send it yet. Will send soon, probably.
> > 
> >I had this in mind since we don't need those kmods in the CI.
> >Thanks Ilya.
> >The question on dpdk side remains open :-).
> >--
> >David Marchand  
> 
> I know that previously we did have issues with the modules not compiling
> due to errors about maximum levels of inlines. Whether that was because of
> the compiler, or the kernel makefiles at the time, I'm not sure. The kmods
> seem to build fine for me now without the parameters, and the fact that
> there has never been a problem reported with building using meson either,
> probably indicates that the extra compiler flags can be dropped.
> 
> /Bruce

I really doubt forcing inlining makes any difference to the performance.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] travis: Don't install kernel for DPDK checks.

[David Marchand]

On Tue, Jun 11, 2019 at 3:31 PM Ilya Maximets 
wrote:

> We don't need to build DPDK kernel modules to test build with OVS.
> And we don't need to build OVS datapath modules for checking
> userspace with DPDK.
>
> Removed 'max-inline-insns-single' changes that only was needed for
> DPDK kernel modules.
>
> Signed-off-by: Ilya Maximets 
> ---
>  .travis.yml|  8 
>  .travis/linux-build.sh | 18 +-
>  2 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/.travis.yml b/.travis.yml
> index 6621fb535..7addcb231 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -31,10 +31,10 @@ env:
>- TESTSUITE=1 KERNEL=3.16
>- TESTSUITE=1 OPTS="--enable-shared"
>- BUILD_ENV="-m32" OPTS="--disable-ssl"
> -  - KERNEL=3.16 DPDK=1 OPTS="--enable-shared"
> -  - KERNEL=3.16 TESTSUITE=1 DPDK=1
> -  - KERNEL=3.16 DPDK_SHARED=1
> -  - KERNEL=3.16 DPDK_SHARED=1 OPTS="--enable-shared"
> +  - DPDK=1 OPTS="--enable-shared"
> +  - TESTSUITE=1 DPDK=1
> +  - DPDK_SHARED=1
> +  - DPDK_SHARED=1 OPTS="--enable-shared"
>- KERNEL=4.20
>- KERNEL=4.19
>- KERNEL=4.18
> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
> index 123cde575..8c0291dd2 100755
> --- a/.travis/linux-build.sh
> +++ b/.travis/linux-build.sh
> @@ -3,7 +3,6 @@
>  set -o errexit
>  set -x
>
> -KERNELSRC=""
>  CFLAGS=""
>  SPARSE_FLAGS=""
>  EXTRA_OPTS="--enable-Werror"
> @@ -57,10 +56,7 @@ function install_kernel()
>  make net/bridge/
>  fi
>
> -KERNELSRC=$(pwd)
> -if [ ! "$DPDK" ] && [ ! "$DPDK_SHARED" ]; then
> -EXTRA_OPTS="${EXTRA_OPTS} --with-linux=$(pwd)"
> -fi
>
+EXTRA_OPTS="${EXTRA_OPTS} --with-linux=$(pwd)"
>  echo "Installed kernel source in $(pwd)"
>  cd ..
>  }
> @@ -78,7 +74,6 @@ function install_dpdk()
>  if [ $DIR_NAME != "dpdk-$1"  ]; then mv $DIR_NAME dpdk-$1; fi
>  cd dpdk-$1
>  fi
> -find ./ -type f | xargs sed -i
> 's/max-inline-insns-single=100/max-inline-insns-single=400/'
>  find ./ -type f | xargs sed -i 's/-Werror/-Werror -Wno-error=inline/'
>

Both lines go together (else, that would mean that dpdk does not build fine
by itself).
So you should remove both of them.



 echo 'CONFIG_RTE_BUILD_FPIC=y' >>config/common_linuxapp
>  sed -ri '/EXECENV_CFLAGS  = -pthread -fPIC/{s/$/\nelse ifeq
> ($(CONFIG_RTE_BUILD_FPIC),y)/;s/$/\nEXECENV_CFLAGS  = -pthread -fPIC/}'
> mk/exec-env/linuxapp/rte.vars.mk
> @@ -86,8 +81,13 @@ function install_dpdk()
>  sed -i '/CONFIG_RTE_BUILD_SHARED_LIB=n/s/=n/=y/'
> config/common_base
>  export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$(pwd)/$TARGET/lib
>  fi
> +
> +# Disable building DPDK kernel modules. Not needed for OVS build or
> tests.
> +sed -i '/CONFIG_RTE_EAL_IGB_UIO=y/s/=y/=n/' config/common_linuxapp
> +sed -i '/CONFIG_RTE_KNI_KMOD=y/s/=y/=n/' config/common_linuxapp
> +
>

Patching the dpdk sources feels quite dangerous to me.
And it is broken on dpdk master since this file has been renamed.

How about editing build/.config after calling 'make config' instead ?

I did like this, to compile against dpdk master:
https://github.com/david-marchand/ovs/commit/e3c7877aa76247cb06f8c2832f99230a6d8e8675


 make config CC=gcc T=$TARGET
> -make -j4 CC=gcc RTE_KERNELDIR=$KERNELSRC
> +make -j4 CC=gcc
>  echo "Installed DPDK source in $(pwd)"
>  cd ..
>  }
> @@ -97,7 +97,7 @@ function configure_ovs()
>  ./boot.sh && ./configure $* || { cat config.log; exit 1; }
>  }
>
> -if [ "$KERNEL" ] || [ "$DPDK" ] || [ "$DPDK_SHARED" ]; then
> +if [ "$KERNEL" ]; then
>  install_kernel $KERNEL
>  fi
>
> @@ -141,7 +141,7 @@ else
>  make selinux-policy
>
>  # Only build datapath if we are testing kernel w/o running testsuite
> -if [ "$KERNEL" ] && [ ! "$DPDK" ] && [ ! "$DPDK_SHARED" ]; then
> +if [ "$KERNEL" ]; then
>  cd datapath
>  fi
>  make -j4
> --
> 2.17.1
>
>

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

[ovs-dev] [PATCH] travis: Don't install kernel for DPDK checks.

[Ilya Maximets]

We don't need to build DPDK kernel modules to test build with OVS.
And we don't need to build OVS datapath modules for checking
userspace with DPDK.

Removed 'max-inline-insns-single' changes that only was needed for
DPDK kernel modules.

Signed-off-by: Ilya Maximets 
---
 .travis.yml|  8 
 .travis/linux-build.sh | 18 +-
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 6621fb535..7addcb231 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -31,10 +31,10 @@ env:
   - TESTSUITE=1 KERNEL=3.16
   - TESTSUITE=1 OPTS="--enable-shared"
   - BUILD_ENV="-m32" OPTS="--disable-ssl"
-  - KERNEL=3.16 DPDK=1 OPTS="--enable-shared"
-  - KERNEL=3.16 TESTSUITE=1 DPDK=1
-  - KERNEL=3.16 DPDK_SHARED=1
-  - KERNEL=3.16 DPDK_SHARED=1 OPTS="--enable-shared"
+  - DPDK=1 OPTS="--enable-shared"
+  - TESTSUITE=1 DPDK=1
+  - DPDK_SHARED=1
+  - DPDK_SHARED=1 OPTS="--enable-shared"
   - KERNEL=4.20
   - KERNEL=4.19
   - KERNEL=4.18
diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
index 123cde575..8c0291dd2 100755
--- a/.travis/linux-build.sh
+++ b/.travis/linux-build.sh
@@ -3,7 +3,6 @@
 set -o errexit
 set -x
 
-KERNELSRC=""
 CFLAGS=""
 SPARSE_FLAGS=""
 EXTRA_OPTS="--enable-Werror"
@@ -57,10 +56,7 @@ function install_kernel()
 make net/bridge/
 fi
 
-KERNELSRC=$(pwd)
-if [ ! "$DPDK" ] && [ ! "$DPDK_SHARED" ]; then
-EXTRA_OPTS="${EXTRA_OPTS} --with-linux=$(pwd)"
-fi
+EXTRA_OPTS="${EXTRA_OPTS} --with-linux=$(pwd)"
 echo "Installed kernel source in $(pwd)"
 cd ..
 }
@@ -78,7 +74,6 @@ function install_dpdk()
 if [ $DIR_NAME != "dpdk-$1"  ]; then mv $DIR_NAME dpdk-$1; fi
 cd dpdk-$1
 fi
-find ./ -type f | xargs sed -i 
's/max-inline-insns-single=100/max-inline-insns-single=400/'
 find ./ -type f | xargs sed -i 's/-Werror/-Werror -Wno-error=inline/'
 echo 'CONFIG_RTE_BUILD_FPIC=y' >>config/common_linuxapp
 sed -ri '/EXECENV_CFLAGS  = -pthread -fPIC/{s/$/\nelse ifeq 
($(CONFIG_RTE_BUILD_FPIC),y)/;s/$/\nEXECENV_CFLAGS  = -pthread -fPIC/}' 
mk/exec-env/linuxapp/rte.vars.mk
@@ -86,8 +81,13 @@ function install_dpdk()
 sed -i '/CONFIG_RTE_BUILD_SHARED_LIB=n/s/=n/=y/' config/common_base
 export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$(pwd)/$TARGET/lib
 fi
+
+# Disable building DPDK kernel modules. Not needed for OVS build or tests.
+sed -i '/CONFIG_RTE_EAL_IGB_UIO=y/s/=y/=n/' config/common_linuxapp
+sed -i '/CONFIG_RTE_KNI_KMOD=y/s/=y/=n/' config/common_linuxapp
+
 make config CC=gcc T=$TARGET
-make -j4 CC=gcc RTE_KERNELDIR=$KERNELSRC
+make -j4 CC=gcc
 echo "Installed DPDK source in $(pwd)"
 cd ..
 }
@@ -97,7 +97,7 @@ function configure_ovs()
 ./boot.sh && ./configure $* || { cat config.log; exit 1; }
 }
 
-if [ "$KERNEL" ] || [ "$DPDK" ] || [ "$DPDK_SHARED" ]; then
+if [ "$KERNEL" ]; then
 install_kernel $KERNEL
 fi
 
@@ -141,7 +141,7 @@ else
 make selinux-policy
 
 # Only build datapath if we are testing kernel w/o running testsuite
-if [ "$KERNEL" ] && [ ! "$DPDK" ] && [ ! "$DPDK_SHARED" ]; then
+if [ "$KERNEL" ]; then
 cd datapath
 fi
 make -j4
-- 
2.17.1

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

Re: [ovs-dev] [PATCH v10] Improved Packet Drop Statistics in OVS

[Eelco Chaudron]

On 7 Jun 2019, at 7:27, Anju Thomas wrote:

Did a quick review, and I noticed some styling issues (mainly 
indentation), the rest seems ok. One small issue with applying the 
patchset to master, but nothing a manual inspection could quickly fix.


Did some basic testing by forcing some of the drops, and seems to work 
fine.


Will do a more thorough review on next patch, including tests. See also 
Gregory Rose’s comments.


//Eelco


Currently OVS maintains explicit packet drop/error counters only on 
port level.
Packets that are dropped as part of normal OpenFlow processing are 
counted in flow stats of “drop” flows or as table misses in table 
stats.
These can only be interpreted by controllers that know the semantics 
of the configured OpenFlow pipeline. Without that knowledge, it is 
impossible for an OVS user to obtain e.g. the total number of packets 
dropped due to OpenFlow rules.


Furthermore, there are numerous other reasons for which packets can be 
dropped by OVS slow path that are not related to the OpenFlow 
pipeline.
The generated datapath flow entries include a drop action to avoid 
further expensive upcalls to the slow path, but subsequent packets 
dropped by the datapath are not accounted anywhere.


Finally, the datapath itself drops packets in certain error 
situations.

Also, these drops are today not accounted for.

This makes it difficult for OVS users to monitor packet drop in an OVS 
instance and to alert a management system in case of a unexpected 
increase of such drops. Also OVS trouble-shooters face difficulties in 
analysing packet drops.


With this patch we implement following changes to address the issues 
mentioned above.


1. Identify and account all the silent packet drop scenarios

2. Display these drops in ovs-appctl coverage/show


Please add some details on the explicit drop actions added



A detailed presentation on this was presented at OvS conference 2017 
and link for the corresponding presentation is available at:


https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-data-plane-in-ovs-82280329

Co-authored-by: Rohith Basavaraja 
Co-authored-by: Keshav Gupta 
Signed-off-by: Anju Thomas 
Signed-off-by: Rohith Basavaraja 
Signed-off-by: Keshav Gupta 
---
 datapath/linux/compat/include/linux/openvswitch.h |   1 +
 lib/dpif-netdev.c |  46 -
 lib/dpif.c|   7 +
 lib/dpif.h|   4 +
 lib/odp-execute.c |  78 
 lib/odp-util.c|   9 +
 ofproto/ofproto-dpif-ipfix.c  |   1 +
 ofproto/ofproto-dpif-sflow.c  |   1 +
 ofproto/ofproto-dpif-xlate.c  |  34 +++-
 ofproto/ofproto-dpif-xlate.h  |   3 +
 ofproto/ofproto-dpif.c|   7 +
 ofproto/ofproto-dpif.h|   8 +-
 tests/automake.mk |   3 +-
 tests/dpif-netdev.at  |   8 +
 tests/drop-stats.at   | 211 
++

 tests/ofproto-dpif.at |   2 +-
 tests/tunnel-push-pop.at  |  29 ++-
 tests/tunnel.at   |  14 +-
 18 files changed, 455 insertions(+), 11 deletions(-)
 create mode 100644 tests/drop-stats.at

diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h

index 65a003a..2de702e 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -985,6 +985,7 @@ enum ovs_action_attr {
OVS_ACTION_ATTR_METER,/* u32 meter number. */
OVS_ACTION_ATTR_CLONE,/* Nested OVS_CLONE_ATTR_*.  */
 	OVS_ACTION_ATTR_CHECK_PKT_LEN, /* Nested OVS_CHECK_PKT_LEN_ATTR_*. 
*/

+OVS_ACTION_ATTR_DROP,


Indentation


 #ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH,   /* struct ovs_action_push_tnl*/
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 5a6f2ab..ec72403 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -101,6 +101,17 @@ enum { MAX_METERS = 65536 };/* Maximum number 
of meters. */
 enum { MAX_BANDS = 8 }; /* Maximum number of bands / meter. 
*/

 enum { N_METER_LOCKS = 64 };/* Maximum number of meters. */

+COVERAGE_DEFINE(datapath_drop_meter);
+COVERAGE_DEFINE(datapath_drop_upcall_error);
+COVERAGE_DEFINE(datapath_drop_lock_error);
+COVERAGE_DEFINE(datapath_drop_userspace_action_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_push_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_pop_error);
+COVERAGE_DEFINE(datapath_drop_recirc_error);
+COVERAGE_DEFINE(datapath_drop_invalid_port);
+COVERAGE_DEFINE(datapath_drop_invalid_tnl_port);
+COVERAGE_DEFINE(datapath_drop_rx_invalid_packet);
+
 /* Protects against changes to 'dp_netdevs'. */
 static struct 

[ovs-dev] [RFC v2 3/3] OVN: use send_event action to report 'empty_lb_rule' events

[Lorenzo Bianconi]

Add northd logical flows in order to reports that the controller
received an IP packet for LB rule witn no backends.
This configuration is used by OpenShift to spin up a idle POD

Signed-off-by: Mark Michelson 
Co-authored-by: Mark Michelson 
Signed-off-by: Lorenzo Bianconi 
---
 ovn/northd/ovn-northd.c | 33 +
 ovn/ovn-nb.xml  |  5 
 tests/ovn.at| 64 +
 3 files changed, 102 insertions(+)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index de0c06d4b..804dca03e 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -70,6 +70,8 @@ static const char *unixctl_path;
 static struct hmap macam = HMAP_INITIALIZER();
 static struct eth_addr mac_prefix;
 
+static bool controller_event_en;
+
 #define MAX_OVN_TAGS 4096
 
 /* Pipeline stages. */
@@ -3571,6 +3573,34 @@ build_pre_lb(struct ovn_datapath *od, struct hmap 
*lflows)
 sset_add(_ips, ip_address);
 }
 
+if (controller_event_en && !strlen(node->value)) {
+struct ds match = DS_EMPTY_INITIALIZER;
+char *action;
+
+if (addr_family == AF_INET) {
+ds_put_format(, "ip && ip4.dst == %s && %s",
+  ip_address, lb->protocol);
+} else {
+ds_put_format(, "ip && ip6.dst == %s && %s",
+  ip_address, lb->protocol);
+}
+if (port) {
+ds_put_format(, " && %s.dst == %u", lb->protocol,
+  port);
+}
+action = xasprintf("trigger_event(event = \"%s\", "
+   "vip = \"%s\", protocol = \"%s\", "
+   "load_balancer = \"" UUID_FMT "\");",
+   event_to_string(OVN_EVENT_EMPTY_LB_BACKENDS),
+   node->key, lb->protocol,
+   UUID_ARGS(>header_.uuid));
+ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 120,
+  ds_cstr(), action);
+ds_destroy();
+free(action);
+continue;
+}
+
 free(ip_address);
 
 /* Ignore L4 port information in the key because fragmented packets
@@ -8044,6 +8074,9 @@ ovnnb_db_run(struct northd_context *ctx,
 smap_destroy();
 }
 
+controller_event_en = smap_get_bool(>options,
+"controller_event", false);
+
 cleanup_macam();
 }
 
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index cbaa9495f..c9d4acbe4 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -107,6 +107,11 @@
 Configure a given OUI to be used as prefix when L2 address is
 dynamically assigned, e.g. 00:11:22
   
+
+  
+Value used to enable/disable ovn-controller event reporting to the CMS.
+Please see the  table in SBDB.
+  
 
 
 
diff --git a/tests/ovn.at b/tests/ovn.at
index a09f545ab..7cba18734 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -14027,3 +14027,67 @@ ovn-hv4-0
 
 OVN_CLEANUP([hv1], [hv2], [hv3])
 AT_CLEANUP
+
+AT_SETUP([ovn -- controller event])
+ovn_start
+
+# Create hypervisors hv[12].
+# Add vif1[12] to hv1, vif2[12] to hv2
+# Add all of the vifs to a single logical switch sw0.
+
+net_add n1
+ovn-nbctl ls-add sw0
+for i in 1 2; do
+sim_add hv$i
+as hv$i
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.$i
+
+for j in 1 2; do
+ovn-nbctl lsp-add sw0 sw0-p$i$j -- \
+lsp-set-addresses sw0-p$i$j "00:00:00:00:00:$i$j 
192.168.1.$i$j"
+
+ovs-vsctl -- add-port br-int vif$i$j -- \
+set interface vif$i$j \
+external-ids:iface-id=sw0-p$i$j \
+options:tx_pcap=hv$i/vif$i$j-tx.pcap \
+options:rxq_pcap=hv$i/vif$i$j-rx.pcap \
+ofport-request=$i$j
+done
+done
+
+ovn-nbctl --wait=hv set NB_Global . options:controller_event=true
+ovn-nbctl lb-add lb0 192.168.1.100:80 ""
+ovn-nbctl ls-lb-add sw0 lb0
+uuid_lb=$(ovn-nbctl --bare --columns=_uuid find load_balancer name=lb0)
+
+OVN_POPULATE_ARP
+ovn-nbctl --timeout=3 --wait=hv sync
+ovn-sbctl lflow-list
+as hv1 ovs-ofctl dump-flows br-int
+
+packet="inport==\"sw0-p11\" && eth.src==00:00:00:00:00:11 && 
eth.dst==00:00:00:00:00:21 &&
+   ip4 && ip.ttl==64 && ip4.src==192.168.1.11 && ip4.dst==192.168.1.100 &&
+   tcp && tcp.src==1 && tcp.dst==80"
+as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet"
+
+ovn-sbctl list controller_event
+uuid=$(ovn-sbctl list controller_event | awk '/_uuid/{print $3}')
+AT_CHECK([ovn-sbctl get controller_event $uuid event_type], [0], [dnl
+empty_lb_backends
+])
+AT_CHECK([ovn-sbctl get controller_event $uuid event_info:vip], [0], [dnl
+"192.168.1.100:80"
+])
+AT_CHECK([ovn-sbctl get 

[ovs-dev] [RFC v2 1/3] OVN: introduce Controller_Event table

[Lorenzo Bianconi]

Add Controller_Event table to OVN SBDB in order to
report CMS related event.
Introduce event_table hashmap array and controller_event related
structures to ovn-controller in order to track pending events
forwarded by ovs-vswitchd. Moreover integrate event_table hashmap
array with event_table ovn-sbdb table

Signed-off-by: Mark Michelson 
Co-authored-by: Mark Michelson 
Signed-off-by: Lorenzo Bianconi 
---
 include/ovn/logical-fields.h|  26 ++
 ovn/controller/ovn-controller.c |  10 ++
 ovn/controller/pinctrl.c| 156 
 ovn/controller/pinctrl.h|   2 +
 ovn/ovn-sb.ovsschema|  20 +++-
 ovn/ovn-sb.xml  |  37 
 6 files changed, 248 insertions(+), 3 deletions(-)

diff --git a/include/ovn/logical-fields.h b/include/ovn/logical-fields.h
index 164b338b5..edfff2456 100644
--- a/include/ovn/logical-fields.h
+++ b/include/ovn/logical-fields.h
@@ -20,6 +20,32 @@
 
 struct shash;
 
+enum ovn_controller_event {
+OVN_EVENT_EMPTY_LB_BACKENDS = 0,
+OVN_EVENT_MAX,
+};
+
+static inline char *
+event_to_string(enum ovn_controller_event event)
+{
+switch (event) {
+case OVN_EVENT_EMPTY_LB_BACKENDS:
+return "empty_lb_backends";
+case OVN_EVENT_MAX:
+default:
+return "";
+}
+}
+
+static inline int
+string_to_event(const char *s)
+{
+if (!strcmp(s, "empty_lb_backends")) {
+return OVN_EVENT_EMPTY_LB_BACKENDS;
+}
+return -1;
+}
+
 /* Logical fields.
  *
  * These values are documented in ovn-architecture(7), please update the
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 60190161f..c2d96df0c 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -132,6 +132,8 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
  * Monitor Logical_Flow, MAC_Binding, Multicast_Group, and DNS tables for
  * local datapaths.
  *
+ * Monitor Controller_Event rows for local chassis.
+ *
  * We always monitor patch ports because they allow us to see the linkages
  * between related logical datapaths.  That way, when we know that we have
  * a VIF on a particular logical switch, we immediately know to monitor all
@@ -141,6 +143,7 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
 struct ovsdb_idl_condition mb = OVSDB_IDL_CONDITION_INIT();
 struct ovsdb_idl_condition mg = OVSDB_IDL_CONDITION_INIT();
 struct ovsdb_idl_condition dns = OVSDB_IDL_CONDITION_INIT();
+struct ovsdb_idl_condition ce =  OVSDB_IDL_CONDITION_INIT();
 sbrec_port_binding_add_clause_type(, OVSDB_F_EQ, "patch");
 /* XXX: We can optimize this, if we find a way to only monitor
  * ports that have a Gateway_Chassis that point's to our own
@@ -164,6 +167,9 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
 sbrec_port_binding_add_clause_options(, OVSDB_F_INCLUDES, );
 const struct smap l3 = SMAP_CONST1(, "l3gateway-chassis", id);
 sbrec_port_binding_add_clause_options(, OVSDB_F_INCLUDES, );
+
+sbrec_controller_event_add_clause_chassis(, OVSDB_F_EQ,
+  >header_.uuid);
 }
 if (local_ifaces) {
 const char *name;
@@ -190,11 +196,13 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
 sbrec_mac_binding_set_condition(ovnsb_idl, );
 sbrec_multicast_group_set_condition(ovnsb_idl, );
 sbrec_dns_set_condition(ovnsb_idl, );
+sbrec_controller_event_set_condition(ovnsb_idl, );
 ovsdb_idl_condition_destroy();
 ovsdb_idl_condition_destroy();
 ovsdb_idl_condition_destroy();
 ovsdb_idl_condition_destroy();
 ovsdb_idl_condition_destroy();
+ovsdb_idl_condition_destroy();
 }
 
 static const char *
@@ -1943,6 +1951,8 @@ main(int argc, char *argv[])
 sbrec_port_binding_by_name,
 sbrec_mac_binding_by_lport_ip,
 sbrec_dns_table_get(ovnsb_idl_loop.idl),
+sbrec_controller_event_table_get(
+ovnsb_idl_loop.idl),
 br_int, chassis,
 _runtime_data.local_datapaths,
 _runtime_data.active_tunnels);
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index b7bb4c990..bd524c081 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -225,6 +225,158 @@ static bool may_inject_pkts(void);
 
 COVERAGE_DEFINE(pinctrl_drop_put_mac_binding);
 COVERAGE_DEFINE(pinctrl_drop_buffered_packets_map);
+COVERAGE_DEFINE(pinctrl_drop_controller_event);
+
+struct empty_lb_backends_event {
+struct hmap_node hmap_node;
+long long int timestamp;
+
+char *vip;
+char *protocol;
+char *load_balancer;
+};
+
+static struct hmap event_table[OVN_EVENT_MAX];
+
+static void init_event_table(void)
+{
+for (size_t i = 0; i < OVN_EVENT_MAX; i++) 

[ovs-dev] [RFC v2 2/3] OVN: introduce send_event() action

[Lorenzo Bianconi]

Add send_event() ovn action in order to allow ovs-vswitchd to report
CMS related events.
This commit introduces a new event, empty_lb_backends. This event is
raised if a received packet is destined for a load balancer VIP that has
no configured backend destinations. For this event, the event info
includes the load balancer VIP, the load balancer UUID, and the
transport protocol.
The use case for this particular event is for the CMS to supply backend
resources to handle this traffic. For example, in Openshift, this event
can be used to spin up new containers to handle the incoming traffic.

Signed-off-by: Mark Michelson 
Co-authored-by: Mark Michelson 
Signed-off-by: Lorenzo Bianconi 
---
 include/ovn/actions.h |  18 +++-
 ovn/controller/lflow.c|  26 +-
 ovn/controller/pinctrl.c  | 110 
 ovn/lib/actions.c | 176 ++
 ovn/lib/ovn-l7.h  |  46 ++
 ovn/utilities/ovn-trace.c |   3 +
 tests/ovn.at  |  10 +++
 tests/test-ovn.c  |  11 ++-
 8 files changed, 394 insertions(+), 6 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index f42bbc277..5ed70e798 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -83,7 +83,8 @@ struct ovn_extend_table;
 OVNACT(ND_NS, ovnact_nest)\
 OVNACT(SET_METER, ovnact_set_meter)   \
 OVNACT(OVNFIELD_LOAD, ovnact_load)\
-OVNACT(CHECK_PKT_LARGER,  ovnact_check_pkt_larger)
+OVNACT(CHECK_PKT_LARGER,  ovnact_check_pkt_larger) \
+OVNACT(TRIGGER_EVENT, ovnact_controller_event)
 
 /* enum ovnact_type, with a member OVNACT_ for each action. */
 enum OVS_PACKED_ENUM ovnact_type {
@@ -318,6 +319,14 @@ struct ovnact_check_pkt_larger {
 struct expr_field dst;  /* 1-bit destination field. */
 };
 
+/* OVNACT_EVENT. */
+struct ovnact_controller_event {
+struct ovnact ovnact;
+int event_type;   /* controller event type */
+struct ovnact_gen_option *options;
+size_t n_options;
+};
+
 /* Internal use by the helpers below. */
 void ovnact_init(struct ovnact *, enum ovnact_type, size_t len);
 void *ovnact_put(struct ofpbuf *, enum ovnact_type, size_t len);
@@ -486,6 +495,9 @@ enum action_opcode {
  * The actions, in OpenFlow 1.3 format, follow the action_header.
  */
 ACTION_OPCODE_ICMP4_ERROR,
+
+/* "trigger_event (event_type)" */
+ACTION_OPCODE_EVENT,
 };
 
 /* Header. */
@@ -515,6 +527,10 @@ struct ovnact_parse_params {
 /* hmap of 'struct gen_opts_map' to support 'put_nd_ra_opts' action */
 const struct hmap *nd_ra_opts;
 
+/* Array of hmap of 'struct gen_opts_map' to support 'trigger_event'
+ * action */
+const struct controller_event_options *controller_event_opts;
+
 /* Each OVN flow exists in a logical table within a logical pipeline.
  * These parameters express this context for a set of OVN actions being
  * parsed:
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index feb8f8ff7..1aafafb33 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -70,6 +70,7 @@ static bool consider_logical_flow(
 struct hmap *dhcp_opts,
 struct hmap *dhcpv6_opts,
 struct hmap *nd_ra_opts,
+struct controller_event_options *controller_event_opts,
 const struct shash *addr_sets,
 const struct shash *port_groups,
 const struct sset *active_tunnels,
@@ -297,12 +298,16 @@ add_logical_flows(
 struct hmap nd_ra_opts = HMAP_INITIALIZER(_ra_opts);
 nd_ra_opts_init(_ra_opts);
 
+struct controller_event_options controller_event_opts;
+controller_event_opts_init(_event_opts);
+
 SBREC_LOGICAL_FLOW_TABLE_FOR_EACH (lflow, logical_flow_table) {
 if (!consider_logical_flow(sbrec_multicast_group_by_name_datapath,
sbrec_port_binding_by_name,
lflow, local_datapaths,
chassis, _opts, _opts,
-   _ra_opts, addr_sets, port_groups,
+   _ra_opts, _event_opts,
+   addr_sets, port_groups,
active_tunnels, local_lport_ids,
flow_table, group_table, meter_table,
lfrr, conj_id_ofs)) {
@@ -315,6 +320,7 @@ add_logical_flows(
 dhcp_opts_destroy(_opts);
 dhcp_opts_destroy(_opts);
 nd_ra_opts_destroy(_ra_opts);
+controller_event_opts_destroy(_event_opts);
 }
 
 bool
@@ -371,6 +377,10 @@ lflow_handle_changed_flows(
 lflow_resource_destroy_lflow(lfrr, >header_.uuid);
 }
 }
+
+struct controller_event_options controller_event_opts;
+controller_event_opts_init(_event_opts);
+
 SBREC_LOGICAL_FLOW_TABLE_FOR_EACH_TRACKED (lflow, logical_flow_table) {
 if (!sbrec_logical_flow_is_deleted(lflow)) {
 /* 

[ovs-dev] [RFC v2 0/3] OVN: add Controller Events

[Lorenzo Bianconi]

There are situations where arrival of certain types of traffic into OVS
does not warrant a "typical" action, such as output to a specific port
or dropping. Rather, the decision about what to do needs to be left to a
CMS.

The series here introduces a new table, Controller_Event, for this
purpose. Traffic into OVS can raise a controller() event that results in
a Controller_Event being written to the southbound database. The
intention is for a CMS to see the events and take some sort of action.
The "handled" column of the event is initially set to "false" by OVN.
When the CMS has seen the event and taken appropriate action, then the
CMS sets this field to "true'. When ovn-controller sees that the event
has been handled, it deletes the event from the database.

Controller events are only added to the southbound database if they have
been enabled in the nb_global table via the options:controller_event
setting.

This series introduces a new event, empty_lb_backends. This event is
raised if a received packet is destined for a load balancer VIP that has
no configured backend destinations. For this event, the event info
includes the load balancer VIP, the load balancer UUID, and the
transport protocol.

The use case for this particular event is for the CMS to supply backend
resources to handle this traffic. For example, in Openshift, this event
can be used to spin up new containers to handle the incoming traffic.

Changes since RFCv1:
- added garbage collector for event hash table
- rename send_event in trigger_event
- modify event_type from int to string in trigger_event action
- added chassis column in Controller_Event as weak reference to
  Chassis table
- added monitoring to 'local' rows in Controller_Event table
- fix typos

Lorenzo Bianconi (3):
  OVN: introduce Controller_Event table
  OVN: introduce send_event() action
  OVN: use send_event action to report 'empty_lb_rule' events

 include/ovn/actions.h   |  18 ++-
 include/ovn/logical-fields.h|  26 
 ovn/controller/lflow.c  |  26 +++-
 ovn/controller/ovn-controller.c |  10 ++
 ovn/controller/pinctrl.c| 266 
 ovn/controller/pinctrl.h|   2 +
 ovn/lib/actions.c   | 176 +
 ovn/lib/ovn-l7.h|  46 ++
 ovn/northd/ovn-northd.c |  33 
 ovn/ovn-nb.xml  |   5 +
 ovn/ovn-sb.ovsschema|  20 ++-
 ovn/ovn-sb.xml  |  37 +
 ovn/utilities/ovn-trace.c   |   3 +
 tests/ovn.at|  74 +
 tests/test-ovn.c|  11 +-
 15 files changed, 744 insertions(+), 9 deletions(-)

-- 
2.21.0

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

[ovs-dev] [PATCH] ovn-controller: Cleanup memory in binding_evaluate_port_binding_changes

[Dumitru Ceara]

The 'lport_to_iface' and 'egress_ifaces' hashtables were not cleaned up
in case a port binding was found that would require a recompute.

Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-discuss/2019-June/048822.html
Reported-by: Daniel Alvarez Sanchez 
Fixes: 9d0b504abdee ("ovn-controller: runtime_data change handler for SB 
port-binding")
CC: Han Zhou 
Signed-off-by: Dumitru Ceara 
---
 ovn/controller/binding.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index b62b3da..87d0b6d 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -695,6 +695,8 @@ binding_evaluate_port_binding_changes(
 return true;
 }
 
+bool changed = false;
+
 const struct sbrec_port_binding *binding_rec;
 struct shash lport_to_iface = SHASH_INITIALIZER(_to_iface);
 struct sset egress_ifaces = SSET_INITIALIZER(_ifaces);
@@ -718,10 +720,14 @@ binding_evaluate_port_binding_changes(
 || is_our_chassis(chassis_rec, binding_rec,
   active_tunnels, _to_iface, local_lports)
 || strcmp(binding_rec->type, "")) {
-return true;
+changed = true;
+break;
 }
 }
-return false;
+
+shash_destroy(_to_iface);
+sset_destroy(_ifaces);
+return changed;
 }
 
 /* Returns true if the database is all cleaned up, false if more work is
-- 
1.8.3.1

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

Re: [ovs-dev] [PATCH 1/1] netdev-tc-offloads: Support match on priority tags

[Ilya Maximets]

> On Mon, Jun 3, 2019 at 4:42 PM Simon Horman 
> wrote:
> 
>> On Fri, May 31, 2019 at 02:45:25PM +0200, Simon Horman wrote:
>> > Sorry Eli,
>> >
>> > I had missed this but I have it now.
>> >
>> > On Fri, 31 May 2019 at 09:35, Eli Britstein  wrote:
>> >
>> > > Ping
>> > > --
>> > > *From:* Eli Britstein 
>> > > *Sent:* Tuesday, May 21, 2019 3:11:51 PM
>> > > *To:* dev at openvswitch.org; Simon Horman
>> > > *Cc:* Roi Dayan; Paul Blakey; Eli Cohen; Eli Britstein
>> > > *Subject:* [PATCH 1/1] netdev-tc-offloads: Support match on priority
>> tags
>> > >
>> > > The logic by which a TC rule has a VLAN match is by the VLAN TCI field,
>> > > either the VID, PCP or CFI are non-zero. For priority-tag packets
>> > > there is a VLAN tag header with a zero VLAN TCI. Match on existence of
>> > > VLAN header (TPID) regardless of TCI matching.
>> > >
>> > > Signed-off-by: Eli Britstein 
>> > > Reviewed-by: Roi Dayan 
>>
>> Thanks this seems to be a nice enhancement, applied to master.
>>
> 
> Hey
> 
> During some internal testing we found that this patch broke some
> set-L4 cases when using tc-offloads. Setting up a simple bridge and
> flow rule looking something like this:
> 
>   ovs-vsctl show:
> Bridge "br0"
> Port "br0"
> Interface "br0"
> type: internal
> Port "veth0r"
> Interface "veth0r"
> Port "veth1r"
> Interface "veth1r"
> 
>   ovs-ofctl add-flow br0 cookie=1,table=0,in_port=veth0r,tcp,tcp_dst=4000, \
> actions=set_field:4002-\>tcp_dst,output:veth1r
> 
> and then sending VLAN tagged traffic leads to packets not egressing the
> port.
> 
> The TC rule that gets installed looks like this:
>   vlan_ethtype ip

There must be no vlan_ethertype match just because there was no such field match
in the original flow.

Looking at the code I see that it checks for key.tpid which makes no sense,
because it must check for the mask first. At least there must be something like
this:

diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 2af0f10d9..7e7160426 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -1146,9 +1146,11 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
*match,
 }
 mask->mpls_lse[0] = 0;
 
-if (eth_type_vlan(key->vlans[0].tpid)) {
+if (mask->vlans[0].tpid && eth_type_vlan(key->vlans[0].tpid)) {
 flower.key.encap_eth_type[0] = flower.key.eth_type;
+flower.mask.encap_eth_type[0] = flower.mask.eth_type;
 flower.key.eth_type = key->vlans[0].tpid;
+flower.mask.eth_type = mask->vlans[0].tpid;
 }
 if (mask->vlans[0].tci) {
 ovs_be16 vid_mask = mask->vlans[0].tci & htons(VLAN_VID_MASK);
@@ -1179,9 +1181,11 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
*match,
 }
 }
 
-if (eth_type_vlan(key->vlans[1].tpid)) {
+if (mask->vlans[1].tpid && eth_type_vlan(key->vlans[1].tpid)) {
 flower.key.encap_eth_type[1] = flower.key.encap_eth_type[0];
+flower.mask.encap_eth_type[1] = flower.mask.encap_eth_type[0];
 flower.key.encap_eth_type[0] = key->vlans[1].tpid;
+flower.mask.encap_eth_type[0] = mask->vlans[1].tpid;
 }
 if (mask->vlans[1].tci) {
 ovs_be16 vid_mask = mask->vlans[1].tci & htons(VLAN_VID_MASK);
---

I'm not an expert here, so it needs checking and review form someone more
familiar with flower.


>   eth_type ipv4
>   ip_proto tcp
>   dst_port 4000
>   ip_flags nofrag
>   not_in_hw
> action order 1:  pedit action pipe keys 1
>  index 1 ref 1 bind 1 installed 3 sec used 3 sec
>  key #0  at tcp+0: val 0fa2 mask 
> Action statistics:
> Sent 368 bytes 8 pkt (dropped 0, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
> 
> action order 2: csum (tcp) action pipe
> index 1 ref 1 bind 1 installed 3 sec used 3 sec
> Action statistics:
> Sent 368 bytes 8 pkt (dropped 8, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
> 
> action order 3: mirred (Egress Redirect to device ens260np1) stolen
> index 1 ref 1 bind 1 installed 3 sec used 3 sec
> Action statistics:
> Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
> cookie 8eb331199f4d41dc739e769a2f79f1ba
> 
> Note the drop counters in the csum(tcp) section. Before the patch the rule
> looks like this:
>   eth_type ipv4
>   ip_proto tcp
>   dst_port 4000
>   ip_flags nofrag
>   not_in_hw
> action order 1:  pedit action pipe keys 1
>  index 1 ref 1 bind 1 installed 3 sec used 3 sec
>  key #0  at tcp+0: val 0fa2 mask 
> Action statistics:
> Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
> 
> action order 2: csum (tcp) action pipe
> index 1 ref 1 bind 1 installed 3 sec used 3 sec
> Action 

Re: [ovs-dev] Forcing inlining for igb_uio and kni

[Bruce Richardson]

On Tue, Jun 11, 2019 at 11:44:01AM +0200, David Marchand wrote:
>On Tue, Jun 11, 2019 at 11:31 AM Ilya Maximets
><[1]i.maxim...@samsung.com> wrote:
> 
>  On 11.06.2019 11:45, David Marchand wrote:
>  > I noticed that OVS CI [1] patches the dpdk sources to force some
>  inlining parameters and get kni and igb_uio to build fine.
>  >
>  > Looking at it in dpdk, meson support dropped this.
>  > In the makefiles, I can't find a reason in the git history (we go
>  back to 1.3.0rX version).
>  >
>  > [dmarchan@dmarchan dpdk]$ git grep max-inline-insns-single
>  > kernel/linux/igb_uio/Makefile:MODULE_CFLAGS += -I$(SRCDIR) --param
>  max-inline-insns-single=100
>  > kernel/linux/kni/Makefile:MODULE_CFLAGS += -I$(SRCDIR) --param
>  max-inline-insns-single=50
>  > [dmarchan@dmarchan dpdk]$ git blame origin/master --
>  kernel/linux/igb_uio/Makefile |grep max-inline-insns-single
>  > 13dc56a6 lib/librte_eal/linuxapp/igb_uio/Makefile (Intel
>   2012-12-20 00:00:00 +0100 15) MODULE_CFLAGS += -I$(SRCDIR) --param
>  max-inline-insns-single=100
>  > [dmarchan@dmarchan dpdk]$ git blame origin/master --
>  kernel/linux/kni/Makefile |grep max-inline-insns-single
>  > 3fc5ca2f lib/librte_eal/linuxapp/kni/Makefile (Intel
>  2012-12-20 00:00:00 +0100 14) MODULE_CFLAGS += -I$(SRCDIR) --param
>  max-inline-insns-single=50
>  >
>  > Is there a valid reason to keep this?
>  > 1:
>  [2]https://github.com/openvswitch/ovs/blob/master/.travis/linux-buil
>  d.sh#L81
>  <[3]https://protect2.fireeye.com/url?k=b7de159c45b1fa79.b7df9ed3-c48
>  06461f28ecaf5=https://github.com/openvswitch/ovs/blob/master/.trav
>  is/linux-build.sh#L81>
>  Hi, David.
>  I don't know the reason for these in OVS travis config, But we don't
>  need to know them, actually. I have a patch to drop all the kernel
>  related stuff from the DPDK build in OVS Travis checks, just didn't
>  send it yet. Will send soon, probably.
> 
>I had this in mind since we don't need those kmods in the CI.
>Thanks Ilya.
>The question on dpdk side remains open :-).
>--
>David Marchand

I know that previously we did have issues with the modules not compiling
due to errors about maximum levels of inlines. Whether that was because of
the compiler, or the kernel makefiles at the time, I'm not sure. The kmods
seem to build fine for me now without the parameters, and the fact that
there has never been a problem reported with building using meson either,
probably indicates that the extra compiler flags can be dropped.

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

Re: [ovs-dev] [PATCH v2] ovn-controller: Fix chassis ovn-sbdb record init

[Dumitru Ceara]

On Fri, Jun 7, 2019 at 10:42 PM Ben Pfaff  wrote:
>
> On Wed, May 22, 2019 at 01:26:03PM +0200, Dumitru Ceara wrote:
> > The chassis_run code didn't take into account the scenario when the
> > system-id was changed in the Open_vSwitch table. Due to this the code
> > was trying to insert a new Chassis record in the OVN_Southbound DB with
> > the same Encaps as the previous Chassis record. The transaction used
> > to insert the new records was aborting due to the ["type", "ip"]
> > index constraint violation as we were creating new Encap entries with
> > the same "type" and "ip" as the old ones.
> >
> > In order to fix this issue the flow is now:
> > 1. the first time ovn-controller initializes the Chassis entry (shortly
> > after start up) we first check if there is a stale Chassis record in the
> > OVN_Southbound DB by checking if any of the old Encap entries associated
> > to the Chassis record match the new tunnel configuration. If found it
> > means that ovn-controller didn't shutdown gracefully last time it was
> > run so it didn't cleanup the Chassis table. Potentially in the meantime
> > the OVS system-id was also changed. We then update the stale entry with
> > the new configuration and store the last configured chassis-id in memory
> > to avoid walking the Chassis table every time.
> > 2. for subsequent chassis_run calls we use the last configured
> > chassis-id stored at the previous step to lookup the old Chassis record.
> > 3. when ovn-controller shuts down gracefully we lookup the Chassis
> > record based on the chassis-id stored in memory at steps 1 and 2 above.
> > This is to avoid failing to cleanup the Chassis record in OVN_Southbound
> > DB if the OVS system-id changes between the last call to chassis_run and
> > chassis_cleanup.
> >
> > With this commit we also:
> > - refactor chassis.c to abstract the string processing and use
> >   library data structures (e.g., sset)
> > - rename the get_chassis_id function in ovn-controller.c to
> >   get_ovs_chassis_id to avoid confusion with the newly added
> >   chassis_get_id function from chassis.c which returns the last
> >   successfully configured chassis-id.
> > - add a test case in ovn-controller.at to check that OVS system-id
> >   changes are properly propagated to OVN_Southbound DB
>
> Thanks for working on this.
>
> This is a large patch that incorporates both a bug fix and refactoring.
> I would prefer to see the refactoring broken out into a separate patch.
> Preferably, it would go after the bug fix so that the bug fix could be
> backported by itself (if necessary), but if the refactoring is essential
> to the bug fix then the refactoring could go first.
>
> Would you mind breaking the patch apart?

I'll try to split this in two patches.

Thanks,
Dumitru

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

Re: [ovs-dev] [PATCH v3] ovn-controller: Fix parsing of OVN tunnel IDs

[Dumitru Ceara]

On Fri, Jun 7, 2019 at 6:42 PM Ben Pfaff  wrote:
>
> On Mon, May 27, 2019 at 10:55:52AM +0200, Dumitru Ceara wrote:
> > Encap tunnel-ids are of the form:
> > .
> > In physical_run we were checking if a tunnel-id corresponds
> > to the local chassis-id by searching if the chassis-id string
> > is included in the tunnel-id (strstr). This can break quite
> > easily, for example, if the local chassis-id is a substring
> > of a remote chassis-id. In that case we were wrongfully
> > skipping the tunnel creation.
> >
> > To fix that new tunnel-id creation and parsing functions are added in
> > encaps.[ch]. These functions are now used everywhere where applicable.
> >
> > Acked-by: Venu Iyer 
> > Reported-at: https://bugzilla.redhat.com/1708131
> > Reported-by: Haidong Li 
> > Fixes: b520ca7 ("Support for multiple VTEP in OVN")
> > Signed-off-by: Dumitru Ceara 
> > ---
> >
> > v3: Rebase
> > v2: Update commit message
>
> This seems about right.
>
> Here's an incremental with some suggestions for encaps_tunnel_id_parse()
> and encaps_tunnel_id_match().  I'm pretty happy with the
> encaps_tunnel_id_parse() change; the encaps_tunnel_id_match() one might
> be going over the top.
>
> Let me know what you think.

Thanks for the suggestions.
I like the new encaps_tunnel_id_parse() as it's more succinct and even
though encaps_tunnel_id_match() might be going a bit over the top it's
the most efficient way to do it indeed.

Thanks,
Dumitru

>
> diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
> index 61b3eab3971f..3b3921a736e2 100644
> --- a/ovn/controller/encaps.c
> +++ b/ovn/controller/encaps.c
> @@ -102,64 +102,51 @@ encaps_tunnel_id_create(const char *chassis_id, const 
> char *encap_ip)
>   * If the 'encap_ip' argument is not NULL the function will allocate memory
>   * and store the encapsulation IP part of the tunnel-id at '*encap_ip'.
>   */
> -bool encaps_tunnel_id_parse(const char *tunnel_id, char **chassis_id,
> -char **encap_ip)
> +bool
> +encaps_tunnel_id_parse(const char *tunnel_id, char **chassis_id,
> +   char **encap_ip)
>  {
> -const char *match = strchr(tunnel_id, OVN_MVTEP_CHASSISID_DELIM);
> -
> -if (!match) {
> +/* Find the delimiter.  Fail if there is no delimiter or if 
> 
> + * or  is the empty string.*/
> +const char *d = strchr(tunnel_id, OVN_MVTEP_CHASSISID_DELIM);
> +if (d == tunnel_id || !d || !d[1]) {
>  return false;
>  }
>
>  if (chassis_id) {
> -size_t chassis_id_len = (match - tunnel_id);
> -
> -*chassis_id = xmemdup0(tunnel_id, chassis_id_len);
> +*chassis_id = xmemdup0(tunnel_id, d - tunnel_id);
>  }
> -
> -/* Consume the tunnel-id delimiter. */
> -match++;
> -
>  if (encap_ip) {
> -/*
> - * If the value has morphed into something other than
> - * , fail and free already allocated
> - * memory (i.e., chassis_id).
> - */
> -if (*match == 0) {
> -if (chassis_id) {
> -free(*chassis_id);
> -}
> -return false;
> -}
> -*encap_ip = xstrdup(match);
> +*encap_ip = xstrdup(d + 1);
>  }
> -
>  return true;
>  }
>
>  /*
> - * Returns true if a given tunnel_id contains 'chassis_id' and, if specified,
> - * the given 'encap_ip'. Returns false otherwise.
> + * Returns true if 'tunnel_id' contains 'chassis_id' and, if specified, the
> + * given 'encap_ip'. Returns false otherwise.
>   */
> -bool encaps_tunnel_id_match(const char *tunnel_id, const char *chassis_id,
> -const char *encap_ip)
> +bool
> +encaps_tunnel_id_match(const char *tunnel_id, const char *chassis_id,
> +   const char *encap_ip)
>  {
> -if (strstr(tunnel_id, chassis_id) != tunnel_id) {
> -return false;
> -}
> -
> -size_t chassis_id_len = strlen(chassis_id);
> -
> -if (tunnel_id[chassis_id_len] != OVN_MVTEP_CHASSISID_DELIM) {
> -return false;
> -}
> -
> -if (encap_ip && strcmp(_id[chassis_id_len + 1], encap_ip)) {
> -return false;
> +while (*tunnel_id == *chassis_id) {
> +if (!*tunnel_id) {
> +/* 'tunnel_id' and 'chassis_id' are equal strings.  This is a
> + * mismatch because 'tunnel_id' is missing the delimiter and IP. 
> */
> +return false;
> +}
> +tunnel_id++;
> +chassis_id++;
>  }
>
> -return true;
> +/* We found the first byte that disagrees between 'tunnel_id' and
> + * 'chassis_id'.  If we consumed all of 'chassis_id' and arrived at the
> + * delimiter in 'tunnel_id' (and if 'encap_ip' is correct, if it was
> + * supplied), it's a match. */
> +return (*tunnel_id == OVN_MVTEP_CHASSISID_DELIM
> +&& *chassis_id == '\0'
> +&& (!encap_ip || !strcmp(tunnel_id + 1, encap_ip)));
>  }
>
>  static void
___
dev 

Re: [ovs-dev] [RFC 1/3] OVN: introduce Controller_Event table

[Lorenzo Bianconi]

> On Wed, May 29, 2019 at 04:05:07PM +0200, Lorenzo Bianconi wrote:
> > > On Thu, May 16, 2019 at 06:05:24PM +0200, Lorenzo Bianconi wrote:
> > > > Add Controller_Event table to OVN SBDB in order to
> > > > report CMS related event.
> > > > Introduce event_table hashmap array and controller_event related
> > > > structures to ovn-controller in order to track pending events
> > > > forwarded by ovs-vswitchd. Moreover integrate event_table hashmap
> > > > array with event_table ovn-sbdb table
> > > > 
> > > > Signed-off-by: Mark Michelson 
> > > > Co-authored-by: Mark Michelson 
> > > > Signed-off-by: Lorenzo Bianconi 
> 
> ...
> 
> > > 4. What is the tolerance for events that are never delivered or that are
> > >delivered more than once?  What can actually be guaranteed, given
> > >that the database can die and that ovn-controller can die?  (Also,
> > >OVSDB transactions cannot guarantee exactly-once semantics in corner
> > >cases unless the transactions are idempotent.)
> > 
> > If the ovn-controller dies I think there is no too much we can do, events 
> > will
> > be lost until the controller restarts properly.
> > If ovn-northd or the connection to the db dies, controller_event_run() will 
> > not
> > manage the Controller_Event table and pinctrl_handle_event() will queue the
> > pending events in the event_table hash until the upper limit is reached.
> > We can probably add a garbage collector for the pending events in the table.
> > What do you think?
> 
> What's the consequence if an event is missed?  What's the consequence if
> an event is pushed two or more times?  It's easiest to design a
> distributed system so that it's OK if an event is delivered zero times
> or multiple times.  It's a little harder to design so that an event is
> delivered one or more times.  It's hardest to design so that an event is
> delivered exactly one time.

Hi Ben,

thx a lot for your comments,

> 
> There are the following obvious points of failure from these points of
> view:
> 
> 1. ovn-controller.  If it dies, it might not push an event that it
>should.  When it comes back up, will it know to push the event that
>it missed?  What about if it dies while it is pushing an event; is it
>possible that it will push it again when it comes up?

This is probably not an issue since if the event is lost because the controller
is dead we will receive a new one when the controller comes back.
If the controller dies after sending the event to the db it will not receive
new events when it comes back

> 
> 2. The OVSDB protocol.  If the OVSDB connection dies after
>ovn-controller's transaction is committed but before ovn-controller
>receives the acknowledgment, then when it reconnects ovn-controller
>might retry it, which could lead to an event being pushed two or more
>times.
> 
> 3. ovsdb-server.  Clients don't typically use the OVSDB protocol feature
>that ensures that a transaction is committed to stable storage before
>it is acknowledged, so an event could get lost if ovsdb-server dies
>after acknowledging a transaction but before it gets written to disk.
>(Clustered OVSDB always does sync to stable storage though.)
> 
> 4. ovn-northd.  There is a race between ovn-northd acting on an event
>and marking it handled (or deleting it).  There are also the same
>OVSDB protocol and ovsdb-server races in the reverse direction.

I agree with you, even if these kind of events (duplicated events or duplicated
rows in the db) are quite unlikely since controller_event processing is done 
holding
pinctrl_mutex, they can happen. However I think these kind of events can be 
managed
by the CMS since the controller does not have the 'history' of already handled 
events.

Regards,
Lorenzo

> 
> We may be able to work around some or all of these issues if necessary.
> Have you considered them?  How important are they?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] Forcing inlining for igb_uio and kni

[David Marchand]

On Tue, Jun 11, 2019 at 11:31 AM Ilya Maximets 
wrote:

> On 11.06.2019 11:45, David Marchand wrote:
> > I noticed that OVS CI [1] patches the dpdk sources to force some
> inlining parameters and get kni and igb_uio to build fine.
> >
> > Looking at it in dpdk, meson support dropped this.
> > In the makefiles, I can't find a reason in the git history (we go back
> to 1.3.0rX version).
> >
> > [dmarchan@dmarchan dpdk]$ git grep max-inline-insns-single
> > kernel/linux/igb_uio/Makefile:MODULE_CFLAGS += -I$(SRCDIR) --param
> max-inline-insns-single=100
> > kernel/linux/kni/Makefile:MODULE_CFLAGS += -I$(SRCDIR) --param
> max-inline-insns-single=50
> > [dmarchan@dmarchan dpdk]$ git blame origin/master --
> kernel/linux/igb_uio/Makefile |grep max-inline-insns-single
> > 13dc56a6 lib/librte_eal/linuxapp/igb_uio/Makefile (Intel
>  2012-12-20 00:00:00 +0100 15) MODULE_CFLAGS += -I$(SRCDIR) --param
> max-inline-insns-single=100
> > [dmarchan@dmarchan dpdk]$ git blame origin/master --
> kernel/linux/kni/Makefile |grep max-inline-insns-single
> > 3fc5ca2f lib/librte_eal/linuxapp/kni/Makefile (Intel
>  2012-12-20 00:00:00 +0100 14) MODULE_CFLAGS += -I$(SRCDIR) --param
> max-inline-insns-single=50
> >
> > Is there a valid reason to keep this?
> > 1:
> https://github.com/openvswitch/ovs/blob/master/.travis/linux-build.sh#L81
> <
> https://protect2.fireeye.com/url?k=b7de159c45b1fa79.b7df9ed3-c4806461f28ecaf5=https://github.com/openvswitch/ovs/blob/master/.travis/linux-build.sh#L81
> >
>
> Hi, David.
> I don't know the reason for these in OVS travis config, But we don't
> need to know them, actually. I have a patch to drop all the kernel
> related stuff from the DPDK build in OVS Travis checks, just didn't
> send it yet. Will send soon, probably.
>

I had this in mind since we don't need those kmods in the CI.
Thanks Ilya.


The question on dpdk side remains open :-).

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

Re: [ovs-dev] Forcing inlining for igb_uio and kni

[Ilya Maximets]

On 11.06.2019 11:45, David Marchand wrote:
> I noticed that OVS CI [1] patches the dpdk sources to force some inlining 
> parameters and get kni and igb_uio to build fine.
> 
> Looking at it in dpdk, meson support dropped this.
> In the makefiles, I can't find a reason in the git history (we go back to 
> 1.3.0rX version).
> 
> [dmarchan@dmarchan dpdk]$ git grep max-inline-insns-single
> kernel/linux/igb_uio/Makefile:MODULE_CFLAGS += -I$(SRCDIR) --param 
> max-inline-insns-single=100
> kernel/linux/kni/Makefile:MODULE_CFLAGS += -I$(SRCDIR) --param 
> max-inline-insns-single=50
> [dmarchan@dmarchan dpdk]$ git blame origin/master -- 
> kernel/linux/igb_uio/Makefile |grep max-inline-insns-single
> 13dc56a6 lib/librte_eal/linuxapp/igb_uio/Makefile (Intel            
> 2012-12-20 00:00:00 +0100 15) MODULE_CFLAGS += -I$(SRCDIR) --param 
> max-inline-insns-single=100
> [dmarchan@dmarchan dpdk]$ git blame origin/master -- 
> kernel/linux/kni/Makefile |grep max-inline-insns-single
> 3fc5ca2f lib/librte_eal/linuxapp/kni/Makefile (Intel            2012-12-20 
> 00:00:00 +0100 14) MODULE_CFLAGS += -I$(SRCDIR) --param 
> max-inline-insns-single=50
> 
> Is there a valid reason to keep this?
> 1: https://github.com/openvswitch/ovs/blob/master/.travis/linux-build.sh#L81 
> 

Hi, David.
I don't know the reason for these in OVS travis config, But we don't
need to know them, actually. I have a patch to drop all the kernel
related stuff from the DPDK build in OVS Travis checks, just didn't
send it yet. Will send soon, probably.
If (or when) OVS Travis will migrate to meson, I'd suggest to just
use '-Denable_kmods=false'.

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

Re: [ovs-dev] [dpdk-latest PATCH] netdev-dpdk: Prefix network structures with rte_.

[Ian Stokes]

On 6/10/2019 3:57 PM, Ian Stokes wrote:

On 6/6/2019 12:36 PM, David Marchand wrote:



On Thu, Jun 6, 2019 at 1:25 PM Ian Stokes > wrote:


    On 6/4/2019 12:14 PM, David Marchand wrote:
 > On Tue, Jun 4, 2019 at 11:29 AM David Marchand
 > mailto:david.march...@redhat.com>
    >> wrote:
 >
 >     Following a rework of dpdk network structures names [1],
    update the
 >     concerned parts.
 >
 >     Ran Olivier script:
 >     sh prefix-net-rte.sh $(find -name "*dpdk*.c")
 >     sh prefix-net-rte.sh $(find -name "*dpdk*.h")
 >     sh prefix-net-rte.sh $(find -name "*rte*.c")
 >     sh prefix-net-rte.sh $(find -name "*rte*.h")
 >
 >     Plus an extra pass following further changes [2]:
 >     old=RTE_IPv4
 >     new=RTE_IPV4
 >     git grep -lw $old | xargs sed -i -e "s/\<$old\>/$new/g"
 >
 >     old=RTE_ETHER_TYPE_IPv4
 >     new=RTE_ETHER_TYPE_IPV4
 >     git grep -lw $old | xargs sed -i -e "s/\<$old\>/$new/g"
 >
 >     old=RTE_ETHER_TYPE_IPv6
 >     new=RTE_ETHER_TYPE_IPV6
 >     git grep -lw $old | xargs sed -i -e "s/\<$old\>/$new/g"
 >
 >     1: http://mails.dpdk.org/archives/dev/2019-May/132612.html
 >     2: https://git.dpdk.org/dpdk/commit/?id=0c9da7555da8
 >
 >
 > Olivier noticed that I had used an early version of his patch.
 > The published one handles the update on RTE_IPv4.
 > I tried the last version which gives the same result anyway.
 > So the extra pass is unnecessary.
 >
 > I can send a v2 to update the commitlog accordingly.
 >

    Hi David,

    thanks for this, upon inspection the patch looks fine and I can 
confirm

    that dpdk-latest is now building with Master of DPDK again.

    I'm just in the process of running a few smoke tests to make sure
    there's no issues functionally (I don't expect to see any as the
    changes
    seem straight forward).

    WRT the v2, what exactly do you want to change in the commit? If it's
    trivial I can amend it before committing.



I just stripped the useless part in the commitlog and put a link to 
Olivier mail which contained his script.

You can see the commitlog here:
https://github.com/david-marchand/ovs/commit/9d367de7d323c28f7c89d590ff60373c47ffa073 





    I'll be applying this to dpdk-latest and dpdk-hwol branches but 
not ovs

    master (master is still using DPDK 18.11.1 currently so no need for
    these changes until it moves to 19.11).



Yes, makes sense.
Thanks Ian.


Thanks David, validated and pushed to dpdk-latest and dpdk-wol.


Good catch actually. In the past we had previously tracked the latest 
DPDK release to compile against, but now that dpdk-latest is used with 
the UNH DPDK CI it probably makes more sense to track DPDK master at 
that's what dpdk-latest looks to enable compilation of.


I'm not against this, would be interested in what peoples thoughts are 
and if so we can modify travis for the dpdk-latest branch.


Ian


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


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

Re: [ovs-dev] [PATCH v4 0/4] netdev: Dynamic per-port Flow API + Offload Split Up.

[Ilya Maximets]

On 15.05.2019 18:35, Ilya Maximets wrote:
> This is the combined patch-set for dynamic flow API along with
> the further work about splitting up netdev offloading from the
> generic netdev related code and making different netdev flow
> offloading implementations more or less consistent.
> 
> Version 4:
>   * Added dpctl related patch.
>   * No other changes.
> 
> Version 3:
>   * Fixed uninitialized 'error' in provider register function.
>   * Fixed opposite bahaviour of .init_flow_api for dpdk due
> to wrong bool to int conversion.
>   * Sanity checked with all 3 available offload providers:
> dummy, tc and dpdk.
> 
> Version 2:
>   * No functional changes.
>   * Patches combined in a single patch-set.
>   * Fixed non-alphabetical order of files in mk file.
>   * Added Acks from Ben.
> 
> For patch #1 since RFC:
>   * Fixed sparce build.
>   * Some logs turned from INFO to DBG.
>   * Enabled HW offloading on non-Linux systems
> (For testing with dummy provider).
> 
> 
> Ilya Maximets (4):
>   netdev: Dynamic per-port Flow API.
>   dpctl: Update docs about dump-flows and HW offloading.
>   netdev: Split up netdev offloading to separate module.
>   netdev-offload: Rename offload providers.
> 
>  Documentation/faq/design.rst  |   5 +-
>  lib/automake.mk   |   9 +-
>  lib/dpctl.c   |   7 +
>  lib/dpctl.man |   2 +
>  lib/dpdk.c|   2 +
>  lib/dpif-netdev.c |   1 +
>  lib/dpif-netlink.c|   1 +
>  lib/netdev-dpdk.c |  25 +-
>  lib/netdev-dpdk.h |   3 +
>  lib/netdev-dummy.c|  24 +-
>  lib/netdev-linux.c|   3 -
>  lib/netdev-linux.h|  10 -
>  ...v-rte-offloads.c => netdev-offload-dpdk.c} |  56 +-
>  lib/netdev-offload-provider.h | 105 +++
>  ...tdev-tc-offloads.c => netdev-offload-tc.c} |  42 +-
>  lib/netdev-offload.c  | 658 ++
>  lib/netdev-offload.h  | 126 
>  lib/netdev-provider.h |  88 +--
>  lib/netdev-rte-offloads.h |  57 --
>  lib/netdev-tc-offloads.h  |  44 --
>  lib/netdev-vport.c|   6 +-
>  lib/netdev.c  | 465 +
>  lib/netdev.h  |  57 --
>  tests/dpif-netdev.at  |   4 +-
>  tests/ofproto-macros.at   |   1 -
>  vswitchd/bridge.c |   1 +
>  vswitchd/vswitch.xml  |   5 +
>  27 files changed, 1044 insertions(+), 763 deletions(-)
>  rename lib/{netdev-rte-offloads.c => netdev-offload-dpdk.c} (93%)
>  create mode 100644 lib/netdev-offload-provider.h
>  rename lib/{netdev-tc-offloads.c => netdev-offload-tc.c} (98%)
>  create mode 100644 lib/netdev-offload.c
>  create mode 100644 lib/netdev-offload.h
>  delete mode 100644 lib/netdev-rte-offloads.h
>  delete mode 100644 lib/netdev-tc-offloads.h
> 

The series was reviewed and validated for some time now, so I'm applying
it to master to not block further development.

Thanks, Roni, Ophir, Ben and Roi.

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

Re: [ovs-dev] [PATCH 1/1] netdev-tc-offloads: Support match on priority tags

[Louis Peens]

On Mon, Jun 3, 2019 at 4:42 PM Simon Horman 
wrote:

> On Fri, May 31, 2019 at 02:45:25PM +0200, Simon Horman wrote:
> > Sorry Eli,
> >
> > I had missed this but I have it now.
> >
> > On Fri, 31 May 2019 at 09:35, Eli Britstein  wrote:
> >
> > > Ping
> > > --
> > > *From:* Eli Britstein 
> > > *Sent:* Tuesday, May 21, 2019 3:11:51 PM
> > > *To:* d...@openvswitch.org; Simon Horman
> > > *Cc:* Roi Dayan; Paul Blakey; Eli Cohen; Eli Britstein
> > > *Subject:* [PATCH 1/1] netdev-tc-offloads: Support match on priority
> tags
> > >
> > > The logic by which a TC rule has a VLAN match is by the VLAN TCI field,
> > > either the VID, PCP or CFI are non-zero. For priority-tag packets
> > > there is a VLAN tag header with a zero VLAN TCI. Match on existence of
> > > VLAN header (TPID) regardless of TCI matching.
> > >
> > > Signed-off-by: Eli Britstein 
> > > Reviewed-by: Roi Dayan 
>
> Thanks this seems to be a nice enhancement, applied to master.
>

Hey

During some internal testing we found that this patch broke some
set-L4 cases when using tc-offloads. Setting up a simple bridge and
flow rule looking something like this:

  ovs-vsctl show:
Bridge "br0"
Port "br0"
Interface "br0"
type: internal
Port "veth0r"
Interface "veth0r"
Port "veth1r"
Interface "veth1r"

  ovs-ofctl add-flow br0 cookie=1,table=0,in_port=veth0r,tcp,tcp_dst=4000, \
actions=set_field:4002-\>tcp_dst,output:veth1r

and then sending VLAN tagged traffic leads to packets not egressing the
port.

The TC rule that gets installed looks like this:
  vlan_ethtype ip
  eth_type ipv4
  ip_proto tcp
  dst_port 4000
  ip_flags nofrag
  not_in_hw
action order 1:  pedit action pipe keys 1
 index 1 ref 1 bind 1 installed 3 sec used 3 sec
 key #0  at tcp+0: val 0fa2 mask 
Action statistics:
Sent 368 bytes 8 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0

action order 2: csum (tcp) action pipe
index 1 ref 1 bind 1 installed 3 sec used 3 sec
Action statistics:
Sent 368 bytes 8 pkt (dropped 8, overlimits 0 requeues 0)
backlog 0b 0p requeues 0

action order 3: mirred (Egress Redirect to device ens260np1) stolen
index 1 ref 1 bind 1 installed 3 sec used 3 sec
Action statistics:
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
cookie 8eb331199f4d41dc739e769a2f79f1ba

Note the drop counters in the csum(tcp) section. Before the patch the rule
looks like this:
  eth_type ipv4
  ip_proto tcp
  dst_port 4000
  ip_flags nofrag
  not_in_hw
action order 1:  pedit action pipe keys 1
 index 1 ref 1 bind 1 installed 3 sec used 3 sec
 key #0  at tcp+0: val 0fa2 mask 
Action statistics:
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0

action order 2: csum (tcp) action pipe
index 1 ref 1 bind 1 installed 3 sec used 3 sec
Action statistics:
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0

action order 3: mirred (Egress Redirect to device ens260np1) stolen
index 1 ref 1 bind 1 installed 3 sec used 3 sec
Action statistics:
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
cookie 83b3464d0c47e3747956bca7e8857a53

Here I suspect there may be a different issue as none of the counters
increase
so the TC rule is probably never hit (confirmed by the datapath rule timing
out even with continuous traffic), but at least packets are correctly
egressing via the userspace fallback path. It looks like this patch is
installing a rule in TC that does get hit, but is now mysteriously
dropping packets in the middle of the action processing pipeline.

Another strange observation is that this only seems to happen when setting
one of the layer4 fields, setting layer3 seems to work as expected.

Regards
Louis Peens


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

[ovs-dev] OVS command line style/syntax modification

[sami]

Hello.

We are trying to make some changes in our infra and implement some 
instances of OVS for tests over X86 platforms.


IS it possible to modify command line style? i mean is it possible to 
modify command syntax and how commands and sub commands are entered to 
make working with OVS easier? or use tools such as vtysh/Clish/Klish 
with OVS ?


if possible it will be appreciated if any instruction or guide is 
provided.


Thank you for the help.

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

Re: [ovs-dev] [PATCHv11] netdev-afxdp: add new netdev type for AF_XDP.

[Eelco Chaudron]

On 8 Jun 2019, at 6:48, William Tu wrote:


+  ethtool -L enp2s0 combined 1
+  ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x10
+  ovs-vsctl add-port br0 enp2s0 -- set interface enp2s0 
type="afxdp"

\
+options:n_rxq=1 options:xdpmode=drv \
+other_config:pmd-rxq-affinity="0:4"


another feature I'm thinking about to add is a new options
for loading custom XDP program

For example:
ovs-vsctl add-port br0 enp2s0 -- set interface enp2s0 type="afxdp"
options:n_rxq=1 options:xdpmode=drv
options:xdp_prog=/path/to/xdp.o

If users do not specify the path, then it is using the libbpf's 
default program

(which forwards all packets to userspace)

If users want to use their own xdp object, then this option can load 
the

xdp object file from the path.


This might be useful, specially if you would like to do some 
experiments.

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