Re: [ovs-dev] [PATCH] memory: kill ovs-vswitchd under super

[王志克]

Hi,

I also found that if once theres are lots of flows, the memory (RSS) usage of 
OVS process would be quite high, 2~3GB. Even then the flows disappear later, 
the memory still keeps.

I am not sure how many people notices this, but if indeed OVS has such defect, 
I guess this should be critical blocker.

Would like to get more info whether others meet such issue, and any clue if 
possible.

Br,
Wang Zhike

--

Message: 1
Date: Sat, 18 Nov 2017 10:07:34 -0800
From: Ben Pfaff 
To: ovs-dev@openvswitch.org,William Tu 
Subject: Re: [ovs-dev] [PATCH] memory: kill ovs-vswitchd under super
highmemory  usage.
Message-ID: 
Content-Type: text/plain; charset=utf-8

On November 16, 2017 10:49:16 PM PST, William Tu  wrote:
>When deploying OVS on a large scale testbed, we occationally see OVS
>gets killed by the oom (out-of-memory) killer, after installing 100k
>rules and seeing ovs-vswitchd consumes more than 4GB of memory.
>Unfortunately, there is no better way to debug and root cause the
>memory
>leak.  The patch tries to add heuristic about the memory consumption
>of numbers of rules and the memory usage (typically 1-2 kB per rule)
>and set an upper bound for the memory usage of ovs-vswitchd.  If the
>memory usage, rss (resident set size), is larger than x16 num_rules,
>we kill the ovs-vswitchd with SIGSEGV, hoping to generate coredump
>file to help debugging.
>
>Signed-off-by: William Tu 
>---
> lib/memory.c | 26 ++
> 1 file changed, 26 insertions(+)
>
>diff --git a/lib/memory.c b/lib/memory.c
>index da97476c6a45..75cce6e5dcc3 100644
>--- a/lib/memory.c
>+++ b/lib/memory.c
>@@ -25,6 +25,7 @@
> #include "timeval.h"
> #include "unixctl.h"
> #include "openvswitch/vlog.h"
>+#include 
> 
> VLOG_DEFINE_THIS_MODULE(memory);
> 
>@@ -110,6 +111,27 @@ memory_should_report(void)
> }
> 
> static void
>+check_memory_usage(unsigned int num_rules)
>+{
>+struct rusage usage;
>+unsigned long int rss;
>+
>+getrusage(RUSAGE_SELF, );
>+rss = (unsigned long int) usage.ru_maxrss; /* in kilobytes */
>+
>+/* Typically a rule takes about 1-2 kilobytes of memory.  If the
>rss
>+ * (resident set size) is larger than 1GB and x16 of num_rules, we
>+ * might have a memory leak.  Thus, kill it with SIGSEGV to
>generate a
>+ * coredump.
>+ */
>+if (rss > 1024 * 1024 && rss > num_rules * 16) {
>+VLOG_ERR("Unexpected high memory usage of %lu kB,"
>+ " rules %u killed with SIGSEGV", rss, num_rules);
>+raise(SIGSEGV);
>+}
>+}
>+
>+static void
> compose_report(const struct simap *usage, struct ds *s)
> {
> const struct simap_node **nodes = simap_sort(usage);
>@@ -120,6 +142,10 @@ compose_report(const struct simap *usage, struct
>ds *s)
> const struct simap_node *node = nodes[i];
> 
> ds_put_format(s, "%s:%u ", node->name, node->data);
>+
>+if (!strcmp(node->name, "rules")) {
>+check_memory_usage(node->data);
>+  }
> }
> ds_chomp(s, ' ');
> free(nodes);
>-- 
>2.7.4
>
>___
>dev mailing list
>d...@openvswitch.org
>https://mail.openvswitch.org/mailman/listinfo/ovs-dev

I know I suggested this but I didn't mean it as something that we'd carry in 
the tree but only as a temporary patch while we're trying to track down the 
leak.

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

Re: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support

[Satyavalli Rama]

Hi Ben,

We didn't observed any test case breaks and we've updated the same with logs in 
our previous conversations.
Could you please provide your inputs regarding the Jan's comments about command 
syntax modifications.

Thanks & Regards
Satya Valli
Tata Consultancy Services
Mailto: satyavalli.r...@tcs.com
Website: http://www.tcs.com

Experience certainty.   IT Services
Business Solutions
Consulting



-Ben Pfaff  wrote: -
To: Satyavalli Rama 
From: Ben Pfaff 
Date: 02/02/2018 03:43AM
Cc: SatyaValli , "d...@openvswitch.org" 
, manasa.cherukupa...@tcs.com, p.pava...@tcs.com, 
Harivelam Lavanya , muttamsetty.su...@tcs.com, Jan 
Scheurich 
Subject: Re: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry 
Statistics Support

At the very minimum I can't review a patch that breaks tests.

On Wed, Jan 31, 2018 at 05:29:12PM +0530, Satyavalli Rama wrote:
> Hi Ben,
> 
> Are you also agreeing with the Jan's comments.
> 
> Thanks & Regards
> Satya Valli
> Tata Consultancy Services
> Mailto: satyavalli.r...@tcs.com
> Website: http://www.tcs.com
> 
> Experience certainty. IT Services
> Business Solutions
> Consulting
> 
> 
> 
> -Jan Scheurich  wrote: -
> To: Satyavalli Rama , Ben Pfaff 
> From: Jan Scheurich 
> Date: 01/08/2018 06:31PM
> Cc: SatyaValli , "d...@openvswitch.org" 
> , "manasa.cherukupa...@tcs.com" 
> , "p.pava...@tcs.com" , 
> Harivelam Lavanya , "muttamsetty.su...@tcs.com" 
> 
> Subject: RE: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry 
> Statistics Support
> 
> Hi Satyavalli,
>  
> Please find my responses below.
>  
> Regards, Jan
>  
> From: Satyavalli Rama [mailto:satyavalli.r...@tcs.com] 
> Sent: Friday, 05 January, 2018 12:25
> To: Ben Pfaff ; Jan Scheurich 
> Cc: SatyaValli ; d...@openvswitch.org; 
> manasa.cherukupa...@tcs.com; p.pava...@tcs.com; Harivelam Lavanya 
> ; muttamsetty.su...@tcs.com
> Subject: Re: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry 
> Statistics Support
>  
> Hi Jan and Ben,
>  
> Please find the inline responses.
>  
> 
> -Ben Pfaff  wrote: -
> To: Jan Scheurich 
> From: Ben Pfaff 
> Date: 01/05/2018 02:35AM
> Cc: SatyaValli , "d...@openvswitch.org" 
> , Manasa Cherukupally , 
> Pavani Panthagada , Lavanya Harivelam 
> , Surya Muttamsetty , 
> SatyaValli 
> Subject: Re: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry 
> Statistics Support
> 
> On Wed, Jan 03, 2018 at 04:24:06PM +, Jan Scheurich wrote:
> > > > >
> > > > > This Patch provides implementation Existing flow entry statistics are
> > > > > redefined as standard OXS(OpenFlow Extensible Statistics) fields for
> > > > > displaying the arbitrary flow stats.The existing Flow Stats were 
> > > > > renamed
> > > > > as Flow Description.
> > > > >
> > > > > To support this implementation below messages are newly added
> > > > >
> > > > > OFPRAW_OFPT15_FLOW_REMOVED,
> > > > > OFPRAW_OFPST15_FLOW_REQUEST,
> > > > > OFPRAW_OFPST15_FLOW_DESC_REQUEST,
> > > > > OFPRAW_OFPST15_AGGREGATE_REQUEST,
> > > > > OFPRAW_OFPST15_FLOW_REPLY,
> > > > > OFPRAW_OFPST15_FLOW_DESC_REPLY,
> > > > > OFPRAW_OFPST15_AGGREGATE_REPLY,
> > > > >
> > > > > The current commit adds support for the new feature in flow statistics
> > > > > multipart messages,aggregate multipart messages and OXS support for 
> > > > > flow
> > > > > removal message, individual flow description messages.
> > > > >
> > > > > "ovs-ofctl dump-flows" needs to be provided with the arbitrary OXS 
> > > > > fields
> > > > > for displaying the desired flow stats.
> > > > >
> > > > > Below are Commands to display OXS stats field wise
> > > > >
> > > > > Flow Statistics Multipart
> > > > > ovs-ofctl dump-flows -O OpenFlow15  idle_time
> > > > > ovs-ofctl dump-flows -O OpenFlow15  packet_count
> > > > > ovs-ofctl dump-flows -O OpenFlow15  byte_count
> > > >
> > > > This would break backward compatibility for one of the most frequently 
> > > > used OVS CLI commands. Why don't you introduce a new
> > > command such as "ovs-ofctl dump-flow-stats" for the new OXS stats?
> > > 
> > > I think you might be 

[ovs-dev] [ovs-dev, v4] netdev-dpdk: Configurable Link State Change (LSC) detection mode

[Róbert Mulik]

It is possible to change LSC detection mode to polling or interrupt mode
for DPDK interfaces. The default is polling mode. To set interrupt mode,
option dpdk-lsc-interrupt has to be set to true.

In polling mode more processor time is needed, since the OVS repeatedly reads
the link state with a short period. It can lead to packet loss for certain
systems.

In interrupt mode the hardware itself triggers an interrupt when link state
change happens, so less processing time needs for the OVS. It is not possible
to enable the interrupt mode on all hardware.

For detailed description and usage see the dpdk install documentation.

Signed-off-by: Robert Mulik 
Reviewed-by: Ilya Maximets 
Reviewed-by: Ian Stokes 
Reviewed-by: Eelco Chaudron 
---
 Documentation/intro/install/dpdk.rst | 48 
 lib/netdev-dpdk.c| 42 ---
 lib/netdev-dpdk.h|  2 ++
 vswitchd/bridge.c|  8 ++
 vswitchd/vswitch.xml | 45 +
 5 files changed, 142 insertions(+), 3 deletions(-)

diff --git a/Documentation/intro/install/dpdk.rst 
b/Documentation/intro/install/dpdk.rst
index ed358d5..14a6684 100644
--- a/Documentation/intro/install/dpdk.rst
+++ b/Documentation/intro/install/dpdk.rst
@@ -628,6 +628,54 @@ The average number of packets per output batch can be 
checked in PMD stats::

 $ ovs-appctl dpif-netdev/pmd-stats-show

+Link State Change (LSC) detection configuration
+~~~
+
+There are two methods to get the information when Link State Change (LSC)
+happens on a network interface: by polling or interrupt.
+
+With polling method, a process is running in the background and repeatedly
+reads the link state with a short period. It continuously needs processor time
+and between 2 reading periods it can`t see the link state change, therefore
+the reaction time depends on the polling period. With higher rate, more
+processor time is needed. Another problem with the poll mode is that on some
+hardware a polling cycle takes too much time, which (in the end) leads to
+packet loss for certain systems.
+
+If interrupts are used to get LSC information, the hardware itself triggers
+an interrupt when link state change happens, the thread wakes up from sleep,
+updates the information, and goes back to sleep mode. When no link state
+change happens (most of the time), the thread remains in sleep mode and
+doesn`t use processor time at all. The disadvantage of this method is that
+when interrupt happens, the processor has to handle it immediately, so it
+puts the currently running process to background, handles the interrupt, and
+takes the background process back. Another disadvantage is that some hardware
+can`t be configured to generate LSC interrupts.
+
+The default configuration is polling mode. To set interrupt mode, option
+dpdk-lsc-interrupt has to be set to true.
+
+Global settings
+
+Command to set interrupt mode for all interfaces:
+ovs-vsctl set Open_vSwitch . other_config:dpdk-lsc-interrupt=true
+
+Command to set polling mode for all interfaces:
+ovs-vsctl set Open_vSwitch . other_config:dpdk-lsc-interrupt=false
+or:
+ovs-vsctl remove Open_vSwitch . other_config dpdk-lsc-interrupt
+
+Interface specific settings (override global settings)
+
+Command to set interrupt mode for a specific interface:
+ovs-vsctl set interface  options:dpdk-lsc-interrupt=true
+
+Command to set polling mode for a specific interface:
+ovs-vsctl set interface  options:dpdk-lsc-interrupt=false
+
+Command to reset to globally defined mode for a specific interface:
+ovs-vsctl remove interface  options dpdk-lsc-interrupt
+
 Limitations
 

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 94fb163..73d0d4b 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -148,7 +148,7 @@ typedef uint16_t dpdk_port_t;
 #define VHOST_ENQ_RETRY_NUM 8
 #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ)

-static const struct rte_eth_conf port_conf = {
+static struct rte_eth_conf port_conf = {
 .rxmode = {
 .mq_mode = ETH_MQ_RX_RSS,
 .split_hdr_size = 0,
@@ -167,6 +167,10 @@ static const struct rte_eth_conf port_conf = {
 .txmode = {
 .mq_mode = ETH_MQ_TX_NONE,
 },
+.intr_conf = {
+/* LSC interrupt mode disabled, polling mode used. */
+.lsc = 0,
+},
 };

 /*
@@ -433,6 +437,12 @@ struct netdev_dpdk {
 /* DPDK-ETH hardware offload features,
  * from the enum set 'dpdk_hw_ol_features' */
 uint32_t hw_ol_features;
+
+/* Properties for link state change detection mode.
+ * If lsc_interrupt_mode is set to false, poll mode is used,
+ * otherwise interrupt mode is used. */
+bool requested_lsc_interrupt_mode;
+bool lsc_interrupt_mode;
 

Re: [ovs-dev] [PATCH branch-2.7] docs: Use DPDK 16.11.4 stable release.

[Kavanagh, Mark B]

>From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
>boun...@openvswitch.org] On Behalf Of Ian Stokes
>Sent: Tuesday, January 30, 2018 10:07 AM
>To: d...@openvswitch.org
>Subject: [ovs-dev] [PATCH branch-2.7] docs: Use DPDK 16.11.4 stable release.
>
>Modify docs and travis linux build script to use DPDK 16.11.4 stable
>branch to benefit from most recent bug fixes.

Hey Ian,

LGTM on the whole.

If I had to be super-critical, I'd probably change the patch subject line from 
'docs:' to 'dpdk:' or something along those lines, since the included changes 
impact travis scripts, as well as documentation. Also, one minor comment below. 
There's no need to spin a new version; once these changes are addressed, feel 
free to add my Ack when pushing the patch.

Cheers,
Mark

>
>There are no new features introduced in the DPDK release, only back
>ported bug fixes. For completeness these bug fixes have been
>documented under the 16.11.4 section in the link below.
>
>http://dpdk.org/doc/guides-16.11/rel_notes/release_16_11.html

Direct link to the 16.11.4 section: 
http://dpdk.org/doc/guides-16.11/rel_notes/release_16_11.html#id4 

>
>Signed-off-by: Ian Stokes 
>---
> .travis/linux-build.sh   | 2 +-
> Documentation/faq/releases.rst   | 2 +-
> Documentation/intro/install/dpdk.rst | 6 +++---
> Documentation/topics/dpdk/vhost-user.rst | 8 
> 4 files changed, 9 insertions(+), 9 deletions(-)
>
>diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
>index 67f3ec2..088f20f 100755
>--- a/.travis/linux-build.sh
>+++ b/.travis/linux-build.sh
>@@ -80,7 +80,7 @@ fi
>
> if [ "$DPDK" ]; then
> if [ -z "$DPDK_VER" ]; then
>-DPDK_VER="16.11.3"
>+DPDK_VER="16.11.4"
> fi
> install_dpdk $DPDK_VER
> if [ "$CC" = "clang" ]; then
>diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
>index fa983cb..c60d0ad 100644
>--- a/Documentation/faq/releases.rst
>+++ b/Documentation/faq/releases.rst
>@@ -160,7 +160,7 @@ Q: What DPDK version does each Open vSwitch release work
>with?
> 2.4.x2.0
> 2.5.x2.2
> 2.6.x16.07.2
>-2.7.x16.11.3
>+2.7.x16.11.4
>  ===
>
> Q: I get an error like this when I configure Open vSwitch::
>diff --git a/Documentation/intro/install/dpdk.rst
>b/Documentation/intro/install/dpdk.rst
>index 55d8bd1..4ee93f2 100644
>--- a/Documentation/intro/install/dpdk.rst
>+++ b/Documentation/intro/install/dpdk.rst
>@@ -64,9 +64,9 @@ Install DPDK
> #. Download the `DPDK sources`_, extract the file and set ``DPDK_DIR``::
>
>$ cd /usr/src/
>-   $ wget http://fast.dpdk.org/rel/dpdk-16.11.3.tar.xz
>-   $ tar xf dpdk-16.11.3.tar.xz
>-   $ export DPDK_DIR=/usr/src/dpdk-stable-16.11.3
>+   $ wget http://fast.dpdk.org/rel/dpdk-16.11.4.tar.xz
>+   $ tar xf dpdk-16.11.4.tar.xz
>+   $ export DPDK_DIR=/usr/src/dpdk-stable-16.11.4
>$ cd $DPDK_DIR
>
> #. (Optional) Configure DPDK as a shared library
>diff --git a/Documentation/topics/dpdk/vhost-user.rst
>b/Documentation/topics/dpdk/vhost-user.rst
>index 2150809..b75c9dc 100644
>--- a/Documentation/topics/dpdk/vhost-user.rst
>+++ b/Documentation/topics/dpdk/vhost-user.rst
>@@ -278,9 +278,9 @@ To begin, instantiate a guest as described in
>:ref:`dpdk-vhost-user` or
> DPDK sources to VM and build DPDK::
>
> $ cd /root/dpdk/
>-$ wget http://fast.dpdk.org/rel/dpdk-16.11.3.tar.xz
>-$ tar xf dpdk-16.11.3.tar.xz
>-$ export DPDK_DIR=/root/dpdk/dpdk-stable-16.11.3
>+$ wget http://fast.dpdk.org/rel/dpdk-16.11.4.tar.xz
>+$ tar xf dpdk-16.11.4.tar.xz
>+$ export DPDK_DIR=/root/dpdk/dpdk-stable-16.11.4
> $ export DPDK_TARGET=x86_64-native-linuxapp-gcc
> $ export DPDK_BUILD=$DPDK_DIR/$DPDK_TARGET
> $ cd $DPDK_DIR
>@@ -364,7 +364,7 @@ Sample XML
> 
> 
>   
>-  
>+  
>   
>   
> 
>--
>2.7.5
>
>___
>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] docs: Update supported DPDK versions.

[Stokes, Ian]

> -Original Message-
> From: Kavanagh, Mark B
> Sent: Friday, February 2, 2018 11:43 AM
> To: Stokes, Ian ; d...@openvswitch.org
> Subject: RE: [ovs-dev] [PATCH] docs: Update supported DPDK versions.
> 
> >From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> >boun...@openvswitch.org] On Behalf Of Ian Stokes
> >Sent: Tuesday, January 30, 2018 10:08 AM
> >To: d...@openvswitch.org
> >Subject: [ovs-dev] [PATCH] docs: Update supported DPDK versions.
> >
> >Update the OVS to DPDK release table to use the latest stable DPDK
> >16.11.4 for OVS 2.7.
> >
> >Signed-off-by: Ian Stokes 
> >---
> > Documentation/faq/releases.rst | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/Documentation/faq/releases.rst
> >b/Documentation/faq/releases.rst index 62a1957..86eefeb 100644
> >--- a/Documentation/faq/releases.rst
> >+++ b/Documentation/faq/releases.rst
> >@@ -162,7 +162,7 @@ Q: What DPDK version does each Open vSwitch release
> >work with?
> > 2.4.x2.0
> > 2.5.x2.2
> > 2.6.x16.07.2
> >-2.7.x16.11.3
> >+2.7.x16.11.4
> > 2.8.x17.05.2
> > 2.9.x17.11
> >  ===
> 
> Hey Ian,
> 
> I built OvS versions 2.7.1-.3 against DPDK v16.11.4 without issue.
> 
> However, with OvS v2.7.0, I see the following:
> 
>   Warning, treated as error:
>   /ovs/Documentation/topics/windows.rst:506:Footnote [5] is
> not referenced.
>   make[2]: *** [htmldocs] Error 1
>   make[2]: *** Waiting for unfinished jobs
>   make[2]: Leaving directory `//dev/ovs'
>   make[1]: *** [all-recursive] Error 1
>   make[1]: Leaving directory `//dev/ovs'
>   make: *** [all] Error 2
> 
> It's worth noting though, that this issue is also present when building
> OvS v2.7.0 with DPDK v16.11.3. As such, I don't see this as an issues
> specific to this patch, but which should be resolved separately.
> 

Thanks Mark,

I would guess this is an issue in the Windows documentation that was 
subsequently fixed for the 2.7.1 release. I guess users should be pushed to the 
latest release here.

Thanks
Ian

> Acked-by: Mark Kavanagh 
> 
> Cheers,
> Mark
> 
> 
> >--
> >2.7.5
> >
> >___
> >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 v3 3/3] xlate: call tnl_neigh_snoop() from terminate_native_tunnel()

[Zoltán Balogh]

Hi Justin,

I rebased the patches to recent master. Please find them attached.

Best regards,
Zoltan

> -Original Message-
> From: Justin Pettit [mailto:jpet...@ovn.org]
> Sent: Friday, February 02, 2018 12:00 AM
> To: Ben Pfaff 
> Cc: Zoltán Balogh ; g...@ovn.org; 
> d...@openvswitch.org; Jan Scheurich
> 
> Subject: Re: [ovs-dev] [PATCH v3 3/3] xlate: call tnl_neigh_snoop() from 
> terminate_native_tunnel()
> 
> I wasn't able to get this patch to apply to the tip of master.  Zoltan, can 
> you rebase this patch and repost?
> 
> The main thing my patch series does is make it so that packets that have a 
> controller action aren't processed
> entirely in userspace.  If, for example, the patches expect packets to be in 
> userspace without an explicit slow-path
> request when generating the datapath flow, then that would be a problem.
> 
> --Justin
> 
> 
> > On Feb 1, 2018, at 2:17 PM, Ben Pfaff  wrote:
> >
> > Justin, I think this is mainly a question about your patches, can you
> > take a look?
> >
> > On Fri, Jan 26, 2018 at 01:08:35PM +, Zoltán Balogh wrote:
> >> Hi,
> >>
> >> I've been investigating the failing unit test. I can confirm, it does fail
> >> with my series on master. However, when I created the series and sent it to
> >> the mailing list it did not.
> >>
> >> I've rebased my series to this commit before I sent it to the mailing list.
> >>  commit f59cb331c481d08f9a851c07cf31e9d826650485
> >>  Author: Yi Yang 
> >>  Date:   Sat Jan 6 13:47:51 2018 +0800
> >>
> >> If I apply the series to the commit below, the test does pass.
> >>
> >> So, I started to search for any commits between the one from Yi Yang and 
> >> the
> >> Last one on master which could make my series fail on the OVN unit test
> >> "/32 router IP address".
> >>
> >> I found, that reverting the series of two commits from Justin Pettit below,
> >> makes my series pass the unit test again. Even rebased to master.
> >>
> >>  commit 74c4530dca939109f3fb79776b60b8722e149738
> >>  Author: Justin Pettit 
> >>  Date:   Wed Oct 18 23:16:22 2017 -0700
> >>
> >>  ofproto-dpif: Don't slow-path controller actions with pause.
> >>
> >>  A previous patch removed slow-pathing for controller actions with the
> >>  exception of ones that specified "pause".  This commit removes that
> >>  restriction so that no controller actions are slow-pathed.
> >>
> >>  Signed-off-by: Justin Pettit 
> >>  Acked-by: Ben Pfaff 
> >>
> >>  commit d39ec23de38464ee35b3098b9f6c5f06d5191015
> >>  Author: Justin Pettit 
> >>  Date:   Wed Jul 5 15:17:52 2017 -0700
> >>
> >>  ofproto-dpif: Don't slow-path controller actions.
> >>
> >>  Controller actions have become more commonly used for purposes other
> >>  than just making forwarding decisions (e.g., packet logging).  A 
> >> packet
> >>  that needs to be copied to the controller and forwarded would always 
> >> be
> >>  sent to ovs-vswitchd to be handled, which could negatively affect
> >>  performance and cause heavier CPU utilization in ovs-vswitchd.
> >>
> >>  This commit changes the behavior so that OpenFlow controller actions
> >>  become userspace datapath actions while continuing to let packet
> >>  forwarding and manipulation continue to be handled by the datapath
> >>  directly.
> >>
> >>  This patch still slow-paths controller actions with the "pause" flag
> >>  set.  A future patch will stop slow-pathing these pause actions as
> >>  well.
> >>
> >>  Signed-off-by: Justin Pettit 
> >>  Acked-by: Ben Pfaff 
> >>
> >> The main difference, my series does compared to master, is not putting the
> >> ARP entry of {10.0.0.2, f0:00:00:01:02:04, br-int} into tunnel neighbor
> >> cache. All the OF rules are almost the same, except loading different
> >> numbers into NXM_NX_REGx and using different metadata.
> >>
> >> How should this be related to Justin's series?
> >>
> >> If I modify the unit test and populate the missing ARP entry then the test
> >> does pass.
> >>
> >> Should this entry be present in the ARP neighbor cache in order to make the
> >> test pass? If yes, then why?
> >>
> >> Best regards,
> >> Zoltan
> >>
> >>> -Original Message-
> >>> From: Ben Pfaff [mailto:b...@ovn.org]
> >>> Sent: Tuesday, January 23, 2018 8:02 PM
> >>> To: Zoltán Balogh 
> >>> Cc: d...@openvswitch.org; Jan Scheurich 
> >>> Subject: Re: [ovs-dev] [PATCH v3 3/3] xlate: call tnl_neigh_snoop() from 
> >>> terminate_native_tunnel()
> >>>
> >>> On Tue, Jan 09, 2018 at 07:54:33PM +0100, Zoltan Balogh wrote:
>  Currenlty, OVS snoops any ARP or ND packets in any bridge and populates
>  the tunnel neighbor cache with the retreived data. For instance, when
>  ARP reply 

[ovs-dev] [RFC PATCH 1/1]: Enhance conjunctive match support in OVN

[nusiddiq]

From: Numan Siddique 

Presently, if a logical flow has possible conjunctive matches, OVN expression
parser has support for that. But certain fields like ip4.src, ip4.dst are not
considered as dimensions in the conjunctive matches.

In order to support all the possible fields as dimensions, this patch has added
a new expression type 'EXPR_T_CONJ'. After a expression is simplified by
expr_simplify(), before calling expr_normalize(), a new function
expr_eval_conj() is called, which evaluates for possible dimensions for
conjunctive matches.

For example if the match expression is
"ip4 && ip4.src == {10.0.0.4, 10.0.0.5, 10.0.0.6} &&
 ip4.dst == {20.0.0.4, 20.0.0.5, 20.0.0.6}"

expr_simplify() would have generated the expression as -

AND(CMP(IP4),
OR((CMP(ip4.src == 10.0.0.4), CMP(ip4.src == 10.0.0.5),
CMP(ip4.src == 10.0.0.6)),
OR((CMP(ip4.dst == 20.0.0.4), CMP(ip4.src == 20.0.0.5),
CMP(ip4.src == 20.0.0.6))).

expr_eval_conj() would return a new expression something like

CONJ(AND(CMP(IP4),
 OR((CMP(ip4.src == 10.0.0.4), CMP(ip4.src == 10.0.0.5),
 CMP(ip4.src == 10.0.0.6))),
 AND(CMP(IP4),
 OR((CMP(ip4.dst == 20.0.0.4), CMP(ip4.dst == 20.0.0.5),
 CMP(ip4.dst == 20.0.0.6

expr_normalize() would normalize each individual 'AND' clause in the CONJ and
expr_to_matches() would add the necessary conjunctive matches.

TODO: If the proposed approach is reasonable, then test cases and necessary
code comments needs to be added.

Signed-off-by: Numan Siddique 
---
 include/ovn/expr.h |   7 ++-
 ovn/controller/lflow.c |   2 +
 ovn/lib/expr.c | 166 +
 tests/test-ovn.c   |   2 +-
 4 files changed, 175 insertions(+), 2 deletions(-)

diff --git a/include/ovn/expr.h b/include/ovn/expr.h
index 711713e08..4259e5938 100644
--- a/include/ovn/expr.h
+++ b/include/ovn/expr.h
@@ -295,6 +295,7 @@ enum expr_type {
 EXPR_T_CONDITION,   /* Conditional to be evaluated in the
  * controller during expr_simplify(),
  * prior to constructing OpenFlow matches. */
+EXPR_T_CONJ,
 };
 
 /* Expression condition type. */
@@ -366,12 +367,16 @@ struct expr {
 /* XXX Should arguments for conditions be generic? */
 char *string;
 } cond;
+
+/* EXPR_T_CONJ. */
+struct ovs_list conj;
 };
 };
 
 struct expr *expr_create_boolean(bool b);
 struct expr *expr_create_andor(enum expr_type);
 struct expr *expr_combine(enum expr_type, struct expr *a, struct expr *b);
+struct expr *expr_create_conj(enum expr_type);
 
 static inline struct expr *
 expr_from_node(const struct ovs_list *node)
@@ -500,5 +505,5 @@ void expr_addr_sets_add(struct shash *addr_sets, const char 
*name,
 const char * const *values, size_t n_values);
 void expr_addr_sets_remove(struct shash *addr_sets, const char *name);
 void expr_addr_sets_destroy(struct shash *addr_sets);
-
+struct expr *expr_eval_conj(struct expr *expr);
 #endif /* ovn/expr.h */
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index 1e79a5355..13413c77d 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -289,6 +289,7 @@ consider_logical_flow(struct controller_ctx *ctx,
 expr = expr_combine(EXPR_T_AND, expr, prereqs);
 prereqs = NULL;
 }
+
 expr = expr_annotate(expr, , );
 }
 if (error) {
@@ -304,6 +305,7 @@ consider_logical_flow(struct controller_ctx *ctx,
 struct condition_aux cond_aux = { ctx->ovnsb_idl, chassis, active_tunnels,
   chassis_index};
 expr = expr_simplify(expr, is_chassis_resident_cb, _aux);
+expr = expr_eval_conj(expr);
 expr = expr_normalize(expr);
 uint32_t n_conjs = expr_to_matches(expr, lookup_port_cb, ,
);
diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
index 79ff45762..3d4c68dd8 100644
--- a/ovn/lib/expr.c
+++ b/ovn/lib/expr.c
@@ -152,6 +152,14 @@ expr_create_andor(enum expr_type type)
 return e;
 }
 
+struct expr *
+expr_create_conj(enum expr_type type)
+{
+struct expr *e = xmalloc(sizeof *e);
+e->type = type;
+ovs_list_init(>conj);
+return e;
+}
 /* Returns a logical AND or OR expression (according to 'type', which must be
  * EXPR_T_AND or EXPR_T_OR) whose sub-expressions are 'a' and 'b', with some
  * flexibility:
@@ -238,6 +246,7 @@ expr_not(struct expr *expr)
 expr->cond.not = !expr->cond.not;
 break;
 
+case EXPR_T_CONJ:
 default:
 OVS_NOT_REACHED();
 }
@@ -298,6 +307,7 @@ expr_fix(struct expr *expr)
 case EXPR_T_CONDITION:
 return expr;
 
+case EXPR_T_CONJ:
 default:
 OVS_NOT_REACHED();
 }
@@ -442,6 +452,9 @@ expr_format(const struct expr *e, struct ds *s)
 case EXPR_T_CONDITION:
 

Re: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support

[Ben Pfaff]

I'm not going to change the established semantics of dump-flows, but I
want a patch that applies and doesn't break tests before I provide any
more detailed feedback.

On Fri, Feb 02, 2018 at 04:00:44PM +, Jan Scheurich wrote:
> Hi Ben,
> 
> I would appreciate if you could comment on my general concerns with this 
> patch.
> 
> I think it is unwise to sacrifice the semantics of the established ovs-ofctl 
> dump-flow command just in order to align the CLI commands with the particular 
> re-arrangement of  multipart requests messages in OpenFlow 1.5.
> 
> There is no need for one-to-one correspondence between ovs-ofctl commands and 
> underlying OF message types. Especially not if these change between OF 
> versions. The CLI commands should have a well-defined stable semantics and 
> use whatever message is appropriate for the OF protocol version in question 
> to implement that.
> 
> Thanks, Jan
> 
> > -Original Message-
> > From: Ben Pfaff [mailto:b...@ovn.org]
> > Sent: Friday, 02 February, 2018 16:52
> > To: Satyavalli Rama 
> > Cc: SatyaValli ; d...@openvswitch.org; 
> > manasa.cherukupa...@tcs.com; p.pava...@tcs.com; Harivelam
> > Lavanya ; muttamsetty.su...@tcs.com; Jan 
> > Scheurich 
> > Subject: Re: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry 
> > Statistics Support
> > 
> > Please start by rebasing and reposting.
> > 
> > On Fri, Feb 02, 2018 at 06:04:49PM +0530, Satyavalli Rama wrote:
> > > Hi Ben,
> > >
> > > We didn't observed any test case breaks and we've updated the same with 
> > > logs in our previous conversations.
> > > Could you please provide your inputs regarding the Jan's comments about 
> > > command syntax modifications.
> > >
> > > Thanks & Regards
> > > Satya Valli
> > > Tata Consultancy Services
> > > Mailto: satyavalli.r...@tcs.com
> > > Website: http://www.tcs.com
> > > 
> > > Experience certainty. IT Services
> > > Business Solutions
> > > Consulting
> > > 
> > >
> > >
> > > -Ben Pfaff  wrote: -
> > > To: Satyavalli Rama 
> > > From: Ben Pfaff 
> > > Date: 02/02/2018 03:43AM
> > > Cc: SatyaValli , "d...@openvswitch.org" 
> > > , manasa.cherukupa...@tcs.com,
> > p.pava...@tcs.com, Harivelam Lavanya , 
> > muttamsetty.su...@tcs.com, Jan Scheurich
> > 
> > > Subject: Re: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry 
> > > Statistics Support
> > >
> > > At the very minimum I can't review a patch that breaks tests.
> > >
> > > On Wed, Jan 31, 2018 at 05:29:12PM +0530, Satyavalli Rama wrote:
> > > > Hi Ben,
> > > >
> > > > Are you also agreeing with the Jan's comments.
> > > >
> > > > Thanks & Regards
> > > > Satya Valli
> > > > Tata Consultancy Services
> > > > Mailto: satyavalli.r...@tcs.com
> > > > Website: http://www.tcs.com
> > > > 
> > > > Experience certainty.   IT Services
> > > > Business Solutions
> > > > Consulting
> > > > 
> > > >
> > > >
> > > > -Jan Scheurich  wrote: -
> > > > To: Satyavalli Rama , Ben Pfaff 
> > > > From: Jan Scheurich 
> > > > Date: 01/08/2018 06:31PM
> > > > Cc: SatyaValli , "d...@openvswitch.org" 
> > > > , "manasa.cherukupa...@tcs.com"
> > , "p.pava...@tcs.com" , 
> > Harivelam Lavanya ,
> > "muttamsetty.su...@tcs.com" 
> > > > Subject: RE: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow 
> > > > Entry Statistics Support
> > > >
> > > > Hi Satyavalli,
> > > >
> > > > Please find my responses below.
> > > >
> > > > Regards, Jan
> > > >
> > > > From: Satyavalli Rama [mailto:satyavalli.r...@tcs.com]
> > > > Sent: Friday, 05 January, 2018 12:25
> > > > To: Ben Pfaff ; Jan Scheurich 
> > > > Cc: SatyaValli ; d...@openvswitch.org; 
> > > > manasa.cherukupa...@tcs.com; p.pava...@tcs.com; Harivelam
> > Lavanya ; muttamsetty.su...@tcs.com
> > > > Subject: Re: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow 
> > > > Entry Statistics Support
> > > >
> > > > Hi Jan and Ben,
> > > >
> > > > Please find the inline responses.
> > > >
> > > >
> > > > -Ben Pfaff  wrote: -
> > > > To: Jan Scheurich 
> > > > From: Ben Pfaff 
> > > > Date: 01/05/2018 02:35AM
> > > > Cc: SatyaValli , "d...@openvswitch.org" 
> > > > 

Re: [ovs-dev] [PATCH 12/20] datapath: Add meter infrastructure

[Pravin Shelar]

On Tue, Jan 30, 2018 at 3:40 PM, Greg Rose  wrote:
> From: Andy Zhou 
>
> Upstream commit:
> commit 96fbc13d7e770b542d2d1fcf700d0baadc6e8063
> Author: Andy Zhou 
> Date:   Fri Nov 10 12:09:42 2017 -0800
>
> openvswitch: Add meter infrastructure
>
> OVS kernel datapath so far does not support Openflow meter action.
> This is the first stab at adding kernel datapath meter support.
> This implementation supports only drop band type.
>
> Signed-off-by: Andy Zhou 
> Signed-off-by: David S. Miller 
>
> Added a compat layer fixup for nla_parse.
> Added another compat fixup for ktime_get_ns.
>
> Cc: Andy Zhou 
> Signed-off-by: Greg Rose 

Recently another patch is merged related to metering, can you include
that to have all fixes in kernel module:
---8<---
commit 5b7789e8fa8f353ad8f2c44de2385cb161b22d32
Author: Gustavo A. R. Silva 
Date:   Tue Jan 30 22:55:33 2018 -0600

openvswitch: meter: Use 64-bit arithmetic instead of 32-bit

> ---
>  acinclude.m4|   3 +
>  datapath/Modules.mk |   6 +-
>  datapath/datapath.c |  14 +-
>  datapath/datapath.h |   3 +
>  datapath/linux/compat/include/net/netlink.h |   9 +
>  datapath/meter.c| 614 
> 
>  datapath/meter.h|  54 +++
>  7 files changed, 699 insertions(+), 4 deletions(-)
>  create mode 100644 datapath/meter.c
>  create mode 100644 datapath/meter.h
>
> diff --git a/acinclude.m4 b/acinclude.m4
> index 1f0b592..bc1ec72 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -804,6 +804,9 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
>OVS_GREP_IFELSE([$KSRC/include/net/net_namespace.h],
>[EXPORT_SYMBOL_GPL(peernet2id_alloc)],
>[OVS_DEFINE([HAVE_PEERNET2ID_ALLOC])])
> +  OVS_GREP_IFELSE([$KSRC/include/linux/timekeeping.h],
> +  [ktime_get_ns],
> +  [OVS_DEFINE([HAVE_KTIME_GET_NS])])
>
>if cmp -s datapath/linux/kcompat.h.new \
>  datapath/linux/kcompat.h >/dev/null 2>&1; then
> diff --git a/datapath/Modules.mk b/datapath/Modules.mk
> index 21f04a0..a9e2880 100644
> --- a/datapath/Modules.mk
> +++ b/datapath/Modules.mk
> @@ -26,7 +26,8 @@ openvswitch_sources = \
> flow_table.c \
> vport.c \
> vport-internal_dev.c \
> -   vport-netdev.c
> +   vport-netdev.c \
> +   meter.c
>
>  vport_geneve_sources = vport-geneve.c
>  vport_vxlan_sources = vport-vxlan.c
> @@ -43,7 +44,8 @@ openvswitch_headers = \
> flow_table.h \
> vport.h \
> vport-internal_dev.h \
> -   vport-netdev.h
> +   vport-netdev.h \
> +   meter.h
>
>  dist_sources = $(foreach module,$(dist_modules),$($(module)_sources))
>  dist_headers = $(foreach module,$(dist_modules),$($(module)_headers))
> diff --git a/datapath/datapath.c b/datapath/datapath.c
> index 13b055a..07b6c71 100644
> --- a/datapath/datapath.c
> +++ b/datapath/datapath.c
> @@ -57,6 +57,7 @@
>  #include "flow.h"
>  #include "flow_table.h"
>  #include "flow_netlink.h"
> +#include "meter.h"
>  #include "gso.h"
>  #include "vport-internal_dev.h"
>  #include "vport-netdev.h"
> @@ -177,6 +178,7 @@ static void destroy_dp_rcu(struct rcu_head *rcu)
> ovs_flow_tbl_destroy(>table);
> free_percpu(dp->stats_percpu);
> kfree(dp->ports);
> +   ovs_meters_exit(dp);
> kfree(dp);
>  }
>
> @@ -1601,6 +1603,10 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct 
> genl_info *info)
> for (i = 0; i < DP_VPORT_HASH_BUCKETS; i++)
> INIT_HLIST_HEAD(>ports[i]);
>
> +   err = ovs_meters_init(dp);
> +   if (err)
> +   goto err_destroy_ports_array;
> +
> /* Set up our datapath device. */
> parms.name = nla_data(a[OVS_DP_ATTR_NAME]);
> parms.type = OVS_VPORT_TYPE_INTERNAL;
> @@ -1629,7 +1635,7 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct 
> genl_info *info)
> ovs_dp_reset_user_features(skb, info);
> }
>
> -   goto err_destroy_ports_array;
> +   goto err_destroy_meters;
> }
>
> err = ovs_dp_cmd_fill_info(dp, reply, info->snd_portid,
> @@ -1644,8 +1650,10 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct 
> genl_info *info)
> ovs_notify(_datapath_genl_family, 
> _dp_datapath_multicast_group, reply, info);
> return 0;
>
> -err_destroy_ports_array:
> +err_destroy_meters:
> ovs_unlock();
> +   ovs_meters_exit(dp);
> +err_destroy_ports_array:
> kfree(dp->ports);
>  err_destroy_percpu:
> free_percpu(dp->stats_percpu);
> @@ -2295,6 +2303,7 @@ static struct genl_family *dp_genl_families[] = {
> 

Re: [ovs-dev] [PATCH 18/20] datapath: use ktime_get_ts64() instead of ktime_get_ts()

[Pravin Shelar]

On Tue, Jan 30, 2018 at 4:40 PM, Gregory Rose  wrote:
> On 1/30/2018 3:40 PM, Greg Rose wrote:
>>
>> From: Arnd Bergmann 
>>
>> Upstream commit:
>>  commit 311af51dcb5629f04976a8e451673f77e3301041
>>  Author: Arnd Bergmann 
>>  Date:   Mon Nov 27 12:41:38 2017 +0100
>>
>>  openvswitch: use ktime_get_ts64() instead of ktime_get_ts()
>>
>>  timespec is deprecated because of the y2038 overflow, so let's
>> convert
>>  this one to ktime_get_ts64(). The code is already safe even on 32-bit
>>  architectures, since it uses monotonic times. On 64-bit
>> architectures,
>>  nothing changes, while on 32-bit architectures this avoids one
>>  type conversion.
>>
>>  Signed-off-by: Arnd Bergmann 
>>  Signed-off-by: David S. Miller 
>>
>> Additional compatability check for ktime_get_ts64() exists or not.
>> If not, then just continue using ktime_get_ts().
>>
>> Cc: Arnd Bergmann 
>> Signed-off-by: Greg Rose 
>
>
> Oops, I screwed this up.  ktime_get_ts64 isn't a macro.  We'll need this
> incremental...
>
> diff --git a/acinclude.m4 b/acinclude.m4
> index bc1ec72..5c63222 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -807,6 +807,9 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
>OVS_GREP_IFELSE([$KSRC/include/linux/timekeeping.h],
>[ktime_get_ns],
>[OVS_DEFINE([HAVE_KTIME_GET_NS])])
> +  OVS_GREP_IFELSE([$KSRC/include/linux/timekeeping.h],
> +  [ktime_get_ts64],
> +  [OVS_DEFINE([HAVE_KTIME_GET_TS64])])
>
>if cmp -s datapath/linux/kcompat.h.new \
>  datapath/linux/kcompat.h >/dev/null 2>&1; then
> diff --git a/datapath/flow.c b/datapath/flow.c
> index 385e481..cd8d422 100644
> --- a/datapath/flow.c
> +++ b/datapath/flow.c
> @@ -52,7 +52,7 @@
>  #include "flow_netlink.h"
>  #include "vport.h"
>
> -#ifndef ktime_get_ts64
> +#ifndef HAVE_KTIME_GET_TS64
>  #define ktime_get_ts64 ktime_get_ts
>  #define timespec64 timespec
>  #endif
>
>
>
>> ---
>>   datapath/flow.c | 11 ---
>>   1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/datapath/flow.c b/datapath/flow.c
>> index 5da7e3e..385e481 100644
>> --- a/datapath/flow.c
>> +++ b/datapath/flow.c
>> @@ -52,14 +52,19 @@
>>   #include "flow_netlink.h"
>>   #include "vport.h"
>>   +#ifndef ktime_get_ts64
>> +#define ktime_get_ts64 ktime_get_ts
>> +#define timespec64 timespec
>> +#endif
>> +

This is done in compat code, can you move it to respective header file?

>>   u64 ovs_flow_used_time(unsigned long flow_jiffies)
>>   {
>> -   struct timespec cur_ts;
>> +   struct timespec64 cur_ts;
>> u64 cur_ms, idle_ms;
>>   - ktime_get_ts(_ts);
>> +   ktime_get_ts64(_ts);
>> idle_ms = jiffies_to_msecs(jiffies - flow_jiffies);
>> -   cur_ms = (u64)cur_ts.tv_sec * MSEC_PER_SEC +
>> +   cur_ms = (u64)(u32)cur_ts.tv_sec * MSEC_PER_SEC +
>>  cur_ts.tv_nsec / NSEC_PER_MSEC;
>> return cur_ms - idle_ms;
>
>
> ___
> 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 09/20] datapath: reliable interface indentification in port dumps

[Pravin Shelar]

On Tue, Jan 30, 2018 at 3:40 PM, Greg Rose  wrote:
> From: Jiri Benc 
>
> Upstream commit:
> commit 9354d452034273a50a4fd703bea31e5d6b1fc20b
> Author: Jiri Benc 
> Date:   Thu Nov 2 17:04:37 2017 -0200
>
> openvswitch: reliable interface indentification in port dumps
>
> This patch allows reliable identification of netdevice interfaces 
> connected
> to openvswitch bridges. In particular, user space queries the netdev
> interfaces belonging to the ports for statistics, up/down state, etc.
> Datapath dump needs to provide enough information for the user space to be
> able to do that.
>
> Currently, only interface names are returned. This is not sufficient, as
> openvswitch allows its ports to be in different name spaces and the
> interface name is valid only in its name space. What is needed and 
> generally
> used in other netlink APIs, is the pair ifindex+netnsid.
>
> The solution is addition of the ifindex+netnsid pair (or only ifindex if 
> in
> the same name space) to vport get/dump operation.
>
> On request side, ideally the ifindex+netnsid pair could be used to
> get/set/del the corresponding vport. This is not implemented by this patch
> and can be added later if needed.
>
> Signed-off-by: Jiri Benc 
> Signed-off-by: David S. Miller 
>
> Added compat fixup for peernet2id.
>
> Cc: Jiri Benc 
> Signed-off-by: Greg Rose 

This patch looks good, But I think we can target it for master only,
since the userspace component is not in yet.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 11/20] datapath: Add meter netlink definitions

[Gregory Rose]

On 2/2/2018 10:18 AM, Pravin Shelar wrote:

On Tue, Jan 30, 2018 at 3:40 PM, Greg Rose  wrote:

From: Andy Zhou 

Upstream commit:
 commit 5794040647de4011598a6d005fdad95d24fd385b
 Author: Andy Zhou 
 Date:   Fri Nov 10 12:09:40 2017 -0800

 openvswitch: Add meter netlink definitions

 Meter has its own netlink family. Define netlink messages and attributes
 for communicating with the user space programs.

 Signed-off-by: Andy Zhou 
 Signed-off-by: David S. Miller 

Cc: Andy Zhou 
Signed-off-by: Greg Rose 

I am not sure if meter userspace is targeted for 2.9, if it is then we
can backport metering related patches to 2.9


Andy sent me a patch for the  meter side userspace code that will 
interface to the kernel datapath metering.

I just need to find a bit of time to appy, test and post for review.

Thanks,

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

[ovs-dev] [RFC PATCH 0/1] Enhance conjunctive match support in OVN

[nusiddiq]

From: Numan Siddique 

Presently, if a logical flow has possible conjunctive matches, OVN expression
parser has support for that. But certain fields like ip4.src, ip4.dst are not
considered as dimensions in the conjunctive matches.

In order to support all the possible fields as dimensions, this patch has added
a new expression type 'EXPR_T_CONJ'. After a expression is simplified by
expr_simplify(), before calling expr_normalize(), a new function
expr_eval_conj() is called, which evaluates for possible dimensions for
conjunctive matches.

For example if the match expression is
"ip4 && ip4.src == {10.0.0.4, 10.0.0.5, 10.0.0.6} &&
 ip4.dst == {20.0.0.4, 20.0.0.5, 20.0.0.6}"

expr_simplify() would have generated the expression as -

AND(CMP(IP4),
OR((CMP(ip4.src == 10.0.0.4), CMP(ip4.src == 10.0.0.5),
CMP(ip4.src == 10.0.0.6)),
OR((CMP(ip4.dst == 20.0.0.4), CMP(ip4.src == 20.0.0.5),
CMP(ip4.src == 20.0.0.6))).

expr_eval_conj() would return a new expression something like

CONJ(AND(CMP(IP4),
 OR((CMP(ip4.src == 10.0.0.4), CMP(ip4.src == 10.0.0.5),
 CMP(ip4.src == 10.0.0.6))),
 AND(CMP(IP4),
 OR((CMP(ip4.dst == 20.0.0.4), CMP(ip4.dst == 20.0.0.5),
 CMP(ip4.dst == 20.0.0.6

expr_normalize() would normalize each individual 'AND' clause in the CONJ and
expr_to_matches() would add the necessary conjunctive matches.


Below are few examples which compares the matches generated by proposed 
approach with the
existing one.

Eg. 1
match is "ip4 && ip4.src == {10.0.0.4, 10.0.0.5, 10.0.0.6} &&
  ip4.dst == {20.0.0.4, 20.0.0.5, 20.0.0.6}"


Proposed approach

conj_id=1
ip,nw_dst=20.0.0.4: conjunction(1, 1/2)
ip,nw_dst=20.0.0.5: conjunction(1, 1/2)
ip,nw_dst=20.0.0.6: conjunction(1, 1/2)
ip,nw_src=10.0.0.4: conjunction(1, 0/2)
ip,nw_src=10.0.0.5: conjunction(1, 0/2)
ip,nw_src=10.0.0.6: conjunction(1, 0/2)

total matches - 7

Existing

ip4.dst == {20.0.0.4, 20.0.0.5, 20.0.0.6}'
ip,nw_src=10.0.0.4,nw_dst=20.0.0.4
ip,nw_src=10.0.0.4,nw_dst=20.0.0.5
ip,nw_src=10.0.0.4,nw_dst=20.0.0.6
ip,nw_src=10.0.0.5,nw_dst=20.0.0.4
ip,nw_src=10.0.0.5,nw_dst=20.0.0.5
ip,nw_src=10.0.0.5,nw_dst=20.0.0.6
ip,nw_src=10.0.0.6,nw_dst=20.0.0.4
ip,nw_src=10.0.0.6,nw_dst=20.0.0.5
ip,nw_src=10.0.0.6,nw_dst=20.0.0.6


total matches - 9

Eg. 2
match is "ip4 && ip4.src == {10.0.0.4, 10.0.0.5, 10.0.0.6}
  && ip4.dst == {20.0.0.4, 20.0.0.5, 20.0.0.6}
  && tcp.src >= 1000 && tcp.src <= 2000
  && tcp.dst >= 3000 && tcp.dst <= 4000"

Proposed approach
-
conj_id=1
tcp,nw_dst=20.0.0.4: conjunction(1, 3/4)
tcp,nw_dst=20.0.0.5: conjunction(1, 3/4)
tcp,nw_dst=20.0.0.6: conjunction(1, 3/4)
tcp,nw_src=10.0.0.4: conjunction(1, 2/4)
tcp,nw_src=10.0.0.5: conjunction(1, 2/4)
tcp,nw_src=10.0.0.6: conjunction(1, 2/4)
tcp,tp_dst=0xbba/0xfffe: conjunction(1, 0/4)
tcp,tp_dst=0xbbc/0xfffc: conjunction(1, 0/4)
tcp,tp_dst=0xbc0/0xffc0: conjunction(1, 0/4)
tcp,tp_dst=0xc00/0xfe00: conjunction(1, 0/4)
tcp,tp_dst=0xe00/0xff00: conjunction(1, 0/4)
tcp,tp_dst=0xf00/0xff80: conjunction(1, 0/4)
tcp,tp_dst=0xf80/0xffe0: conjunction(1, 0/4)
tcp,tp_dst=3000: conjunction(1, 0/4)
tcp,tp_dst=3001: conjunction(1, 0/4)
tcp,tp_dst=4000: conjunction(1, 0/4)
tcp,tp_src=0x3ea/0xfffe: conjunction(1, 1/4)
tcp,tp_src=0x3ec/0xfffc: conjunction(1, 1/4)
tcp,tp_src=0x3f0/0xfff0: conjunction(1, 1/4)
tcp,tp_src=0x400/0xfe00: conjunction(1, 1/4)
tcp,tp_src=0x600/0xff00: conjunction(1, 1/4)
tcp,tp_src=0x700/0xff80: conjunction(1, 1/4)
tcp,tp_src=0x780/0xffc0: conjunction(1, 1/4)
tcp,tp_src=0x7c0/0xfff0: conjunction(1, 1/4)
tcp,tp_src=1000: conjunction(1, 1/4)
tcp,tp_src=1001: conjunction(1, 1/4)
tcp,tp_src=2000: conjunction(1, 1/4)

total matches - 28


Existing

conj_id=1,tcp,nw_src=10.0.0.4,nw_dst=20.0.0.4
conj_id=2,tcp,nw_src=10.0.0.4,nw_dst=20.0.0.5
conj_id=3,tcp,nw_src=10.0.0.4,nw_dst=20.0.0.6
conj_id=4,tcp,nw_src=10.0.0.5,nw_dst=20.0.0.4
conj_id=5,tcp,nw_src=10.0.0.5,nw_dst=20.0.0.5
conj_id=6,tcp,nw_src=10.0.0.5,nw_dst=20.0.0.6
conj_id=7,tcp,nw_src=10.0.0.6,nw_dst=20.0.0.4
conj_id=8,tcp,nw_src=10.0.0.6,nw_dst=20.0.0.5
conj_id=9,tcp,nw_src=10.0.0.6,nw_dst=20.0.0.6
tcp,nw_src=10.0.0.4,nw_dst=20.0.0.4,tp_dst=0xbba/0xfffe: conjunction(1, 0/2)
tcp,nw_src=10.0.0.4,nw_dst=20.0.0.4,tp_dst=0xbbc/0xfffc: conjunction(1, 0/2)
tcp,nw_src=10.0.0.4,nw_dst=20.0.0.4,tp_dst=0xbc0/0xffc0: conjunction(1, 0/2)
tcp,nw_src=10.0.0.4,nw_dst=20.0.0.4,tp_dst=0xc00/0xfe00: conjunction(1, 0/2)
tcp,nw_src=10.0.0.4,nw_dst=20.0.0.4,tp_dst=0xe00/0xff00: conjunction(1, 0/2)
tcp,nw_src=10.0.0.4,nw_dst=20.0.0.4,tp_dst=0xf00/0xff80: conjunction(1, 0/2)
tcp,nw_src=10.0.0.4,nw_dst=20.0.0.4,tp_dst=0xf80/0xffe0: conjunction(1, 0/2)
tcp,nw_src=10.0.0.4,nw_dst=20.0.0.4,tp_dst=3000: conjunction(1, 0/2)
tcp,nw_src=10.0.0.4,nw_dst=20.0.0.4,tp_dst=3001: conjunction(1, 0/2)
tcp,nw_src=10.0.0.4,nw_dst=20.0.0.4,tp_dst=4000: conjunction(1, 0/2)

Re: [ovs-dev] [PATCH] memory: kill ovs-vswitchd under super

[Ben Pfaff]

On Fri, Feb 02, 2018 at 12:37:58PM +, 王志克 wrote:
> I also found that if once theres are lots of flows, the memory (RSS) usage of 
> OVS process would be quite high, 2~3GB. Even then the flows disappear later, 
> the memory still keeps.
> 
> I am not sure how many people notices this, but if indeed OVS has such 
> defect, I guess this should be critical blocker.

This is normal behavior of the C library malloc implementation.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [ovs-dev, ovs-dev, v4] netdev-dpdk: Configurable Link State Change (LSC) detection mode

[Ilya Maximets]

On 02.02.2018 17:05, Róbert Mulik wrote:
> It is possible to change LSC detection mode to polling or interrupt mode
> for DPDK interfaces. The default is polling mode. To set interrupt mode,
> option dpdk-lsc-interrupt has to be set to true.
> 
> In polling mode more processor time is needed, since the OVS repeatedly reads
> the link state with a short period. It can lead to packet loss for certain
> systems.
> 
> In interrupt mode the hardware itself triggers an interrupt when link state
> change happens, so less processing time needs for the OVS. It is not possible
> to enable the interrupt mode on all hardware.
> 
> For detailed description and usage see the dpdk install documentation.
> 
> Signed-off-by: Robert Mulik 
> Reviewed-by: Ilya Maximets 

I'm sorry, but this is not true. I even stated that in my reply to v3:
"Not a full review.".

Also, we didn't decide what to do with some of my concerns.

As OVS doesn't usually use 'Reviewed-by' tag, I'll point you to the kernel docs:

https://www.kernel.org/doc/html/v4.12/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes

> Reviewed-by: Ian Stokes 
> Reviewed-by: Eelco Chaudron 
> ---

It's a good common practice to put the changes between patch versions here.
It's really hard to track all the changes.

>  Documentation/intro/install/dpdk.rst | 48 
> 
>  lib/netdev-dpdk.c| 42 ---
>  lib/netdev-dpdk.h|  2 ++
>  vswitchd/bridge.c|  8 ++
>  vswitchd/vswitch.xml | 45 +
>  5 files changed, 142 insertions(+), 3 deletions(-)
> 
> --
> 1.9.1
> 
> diff --git a/Documentation/intro/install/dpdk.rst 
> b/Documentation/intro/install/dpdk.rst
> index ed358d5..14a6684 100644
> --- a/Documentation/intro/install/dpdk.rst
> +++ b/Documentation/intro/install/dpdk.rst
> @@ -628,6 +628,54 @@ The average number of packets per output batch can be 
> checked in PMD stats::
> 
>  $ ovs-appctl dpif-netdev/pmd-stats-show
> 

It's still not a full review.
But I wanted to say that below documentation is just text and definitely
not a reStructuredText. Please re-format it using rST markup features.
Look at the docs around for examples.

> +Link State Change (LSC) detection configuration
> +~~~
> +
> +There are two methods to get the information when Link State Change (LSC)
> +happens on a network interface: by polling or interrupt.
> +
> +With polling method, a process is running in the background and repeatedly
> +reads the link state with a short period. It continuously needs processor 
> time
> +and between 2 reading periods it can`t see the link state change, therefore
> +the reaction time depends on the polling period. With higher rate, more
> +processor time is needed. Another problem with the poll mode is that on some
> +hardware a polling cycle takes too much time, which (in the end) leads to
> +packet loss for certain systems.
> +
> +If interrupts are used to get LSC information, the hardware itself triggers
> +an interrupt when link state change happens, the thread wakes up from sleep,
> +updates the information, and goes back to sleep mode. When no link state
> +change happens (most of the time), the thread remains in sleep mode and
> +doesn`t use processor time at all. The disadvantage of this method is that
> +when interrupt happens, the processor has to handle it immediately, so it
> +puts the currently running process to background, handles the interrupt, and
> +takes the background process back. Another disadvantage is that some hardware
> +can`t be configured to generate LSC interrupts.
> +
> +The default configuration is polling mode. To set interrupt mode, option
> +dpdk-lsc-interrupt has to be set to true.
> +
> +Global settings
> +
> +Command to set interrupt mode for all interfaces:
> +ovs-vsctl set Open_vSwitch . other_config:dpdk-lsc-interrupt=true
> +
> +Command to set polling mode for all interfaces:
> +ovs-vsctl set Open_vSwitch . other_config:dpdk-lsc-interrupt=false
> +or:
> +ovs-vsctl remove Open_vSwitch . other_config dpdk-lsc-interrupt
> +
> +Interface specific settings (override global settings)
> +
> +Command to set interrupt mode for a specific interface:
> +ovs-vsctl set interface  options:dpdk-lsc-interrupt=true
> +
> +Command to set polling mode for a specific interface:
> +ovs-vsctl set interface  options:dpdk-lsc-interrupt=false
> +
> +Command to reset to globally defined mode for a specific interface:
> +ovs-vsctl remove interface  options dpdk-lsc-interrupt
> +
>  Limitations
>  
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 94fb163..73d0d4b 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -148,7 +148,7 @@ typedef uint16_t dpdk_port_t;
>  

Re: [ovs-dev] [PATCH 01/20] datapath: Fix netdev_master_upper_dev_link for 4.14

[Pravin Shelar]

On Tue, Jan 30, 2018 at 3:40 PM, Greg Rose  wrote:
> An extended netlink ack has been added for 4.14 - add compat layer
> changes so that it compiles for all kernels up to and including
> 4.14.
>
> Signed-off-by: Greg Rose 
> ---
>  acinclude.m4|  3 +++
>  datapath/linux/compat/include/linux/netdevice.h | 15 ++-
>  datapath/vport-netdev.c |  9 -
>  3 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/acinclude.m4 b/acinclude.m4
> index c04c2c6..768c20c 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -795,6 +795,9 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
>  [OVS_DEFINE([HAVE_LIST_IN_NF_HOOK_OPS])])
>OVS_GREP_IFELSE([$KSRC/include/uapi/linux/netfilter/nf_conntrack_common.h],
>[IP_CT_UNTRACKED])
> +  OVS_FIND_PARAM_IFELSE([$KSRC/include/linux/netdevice.h],
> +[netdev_master_upper_dev_link], [extack],
> +[OVS_DEFINE([HAVE_UPPER_DEV_LINK_EXTACK])])
>
>if cmp -s datapath/linux/kcompat.h.new \
>  datapath/linux/kcompat.h >/dev/null 2>&1; then
> diff --git a/datapath/linux/compat/include/linux/netdevice.h 
> b/datapath/linux/compat/include/linux/netdevice.h
> index 3c3cf42..c460332 100644
> --- a/datapath/linux/compat/include/linux/netdevice.h
> +++ b/datapath/linux/compat/include/linux/netdevice.h
> @@ -101,13 +101,26 @@ static inline bool netif_needs_gso(struct sk_buff *skb,
>  #ifndef HAVE_NETDEV_MASTER_UPPER_DEV_LINK_RH
>  static inline int rpl_netdev_master_upper_dev_link(struct net_device *dev,
>struct net_device *upper_dev,
> -  void *upper_priv, void 
> *upper_info)
> +  void *upper_priv,
> +  void *upper_info, void *extack)
>  {
> return netdev_master_upper_dev_link(dev, upper_dev);
>  }
>  #define netdev_master_upper_dev_link rpl_netdev_master_upper_dev_link
>
>  #endif
> +#else
> +#ifndef HAVE_UPPER_DEV_LINK_EXTACK
> +static inline int rpl_netdev_master_upper_dev_link(struct net_device *dev,
> +  struct net_device *upper_dev,
> +  void *upper_priv,
> +  void *upper_info, void *extack)
> +{
> +   return netdev_master_upper_dev_link(dev, upper_dev, upper_priv,
> +   upper_info);
> +}
> +#define netdev_master_upper_dev_link rpl_netdev_master_upper_dev_link
> +#endif
>  #endif
>
>  #if LINUX_VERSION_CODE < KERNEL_VERSION(3,16,0)
> diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c
> index 697c442..e2d8eaf 100644
> --- a/datapath/vport-netdev.c
> +++ b/datapath/vport-netdev.c
> @@ -112,8 +112,15 @@ struct vport *ovs_netdev_link(struct vport *vport, const 
> char *name)
> }
>
> rtnl_lock();
> +#ifdef HAVE_NETDEV_MASTER_UPPER_DEV_LINK_RH
> err = netdev_master_upper_dev_link(vport->dev,
> -  get_dpdev(vport->dp), NULL, NULL);
> +  get_dpdev(vport->dp),
> +  NULL, NULL);
> +#else
> +   err = netdev_master_upper_dev_link(vport->dev,
> +  get_dpdev(vport->dp),
> +  NULL, NULL, NULL);
> +#endif
Since the parameters are pretty much the same, this (#ifdef) can be
moved to compat code.

> if (err)
> goto error_unlock;
>
> --
> 1.8.3.1
>
> ___
> 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 01/20] datapath: Fix netdev_master_upper_dev_link for 4.14

[Gregory Rose]

On 2/2/2018 10:17 AM, Pravin Shelar wrote:

On Tue, Jan 30, 2018 at 3:40 PM, Greg Rose  wrote:

An extended netlink ack has been added for 4.14 - add compat layer
changes so that it compiles for all kernels up to and including
4.14.

Signed-off-by: Greg Rose 
---
  acinclude.m4|  3 +++
  datapath/linux/compat/include/linux/netdevice.h | 15 ++-
  datapath/vport-netdev.c |  9 -
  3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index c04c2c6..768c20c 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -795,6 +795,9 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
  [OVS_DEFINE([HAVE_LIST_IN_NF_HOOK_OPS])])
OVS_GREP_IFELSE([$KSRC/include/uapi/linux/netfilter/nf_conntrack_common.h],
[IP_CT_UNTRACKED])
+  OVS_FIND_PARAM_IFELSE([$KSRC/include/linux/netdevice.h],
+[netdev_master_upper_dev_link], [extack],
+[OVS_DEFINE([HAVE_UPPER_DEV_LINK_EXTACK])])

if cmp -s datapath/linux/kcompat.h.new \
  datapath/linux/kcompat.h >/dev/null 2>&1; then
diff --git a/datapath/linux/compat/include/linux/netdevice.h 
b/datapath/linux/compat/include/linux/netdevice.h
index 3c3cf42..c460332 100644
--- a/datapath/linux/compat/include/linux/netdevice.h
+++ b/datapath/linux/compat/include/linux/netdevice.h
@@ -101,13 +101,26 @@ static inline bool netif_needs_gso(struct sk_buff *skb,
  #ifndef HAVE_NETDEV_MASTER_UPPER_DEV_LINK_RH
  static inline int rpl_netdev_master_upper_dev_link(struct net_device *dev,
struct net_device *upper_dev,
-  void *upper_priv, void 
*upper_info)
+  void *upper_priv,
+  void *upper_info, void *extack)
  {
 return netdev_master_upper_dev_link(dev, upper_dev);
  }
  #define netdev_master_upper_dev_link rpl_netdev_master_upper_dev_link

  #endif
+#else
+#ifndef HAVE_UPPER_DEV_LINK_EXTACK
+static inline int rpl_netdev_master_upper_dev_link(struct net_device *dev,
+  struct net_device *upper_dev,
+  void *upper_priv,
+  void *upper_info, void *extack)
+{
+   return netdev_master_upper_dev_link(dev, upper_dev, upper_priv,
+   upper_info);
+}
+#define netdev_master_upper_dev_link rpl_netdev_master_upper_dev_link
+#endif
  #endif

  #if LINUX_VERSION_CODE < KERNEL_VERSION(3,16,0)
diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c
index 697c442..e2d8eaf 100644
--- a/datapath/vport-netdev.c
+++ b/datapath/vport-netdev.c
@@ -112,8 +112,15 @@ struct vport *ovs_netdev_link(struct vport *vport, const 
char *name)
 }

 rtnl_lock();
+#ifdef HAVE_NETDEV_MASTER_UPPER_DEV_LINK_RH
 err = netdev_master_upper_dev_link(vport->dev,
-  get_dpdev(vport->dp), NULL, NULL);
+  get_dpdev(vport->dp),
+  NULL, NULL);
+#else
+   err = netdev_master_upper_dev_link(vport->dev,
+  get_dpdev(vport->dp),
+  NULL, NULL, NULL);
+#endif

Since the parameters are pretty much the same, this (#ifdef) can be
moved to compat code.


OK, noted.

Thanks,

- Greg





 if (err)
 goto error_unlock;

--
1.8.3.1

___
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 18/20] datapath: use ktime_get_ts64() instead of ktime_get_ts()

[Gregory Rose]

On 2/2/2018 10:18 AM, Pravin Shelar wrote:

On Tue, Jan 30, 2018 at 4:40 PM, Gregory Rose  wrote:

On 1/30/2018 3:40 PM, Greg Rose wrote:

From: Arnd Bergmann 

Upstream commit:
  commit 311af51dcb5629f04976a8e451673f77e3301041
  Author: Arnd Bergmann 
  Date:   Mon Nov 27 12:41:38 2017 +0100

  openvswitch: use ktime_get_ts64() instead of ktime_get_ts()

  timespec is deprecated because of the y2038 overflow, so let's
convert
  this one to ktime_get_ts64(). The code is already safe even on 32-bit
  architectures, since it uses monotonic times. On 64-bit
architectures,
  nothing changes, while on 32-bit architectures this avoids one
  type conversion.

  Signed-off-by: Arnd Bergmann 
  Signed-off-by: David S. Miller 

Additional compatability check for ktime_get_ts64() exists or not.
If not, then just continue using ktime_get_ts().

Cc: Arnd Bergmann 
Signed-off-by: Greg Rose 


Oops, I screwed this up.  ktime_get_ts64 isn't a macro.  We'll need this
incremental...

diff --git a/acinclude.m4 b/acinclude.m4
index bc1ec72..5c63222 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -807,6 +807,9 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
OVS_GREP_IFELSE([$KSRC/include/linux/timekeeping.h],
[ktime_get_ns],
[OVS_DEFINE([HAVE_KTIME_GET_NS])])
+  OVS_GREP_IFELSE([$KSRC/include/linux/timekeeping.h],
+  [ktime_get_ts64],
+  [OVS_DEFINE([HAVE_KTIME_GET_TS64])])

if cmp -s datapath/linux/kcompat.h.new \
  datapath/linux/kcompat.h >/dev/null 2>&1; then
diff --git a/datapath/flow.c b/datapath/flow.c
index 385e481..cd8d422 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -52,7 +52,7 @@
  #include "flow_netlink.h"
  #include "vport.h"

-#ifndef ktime_get_ts64
+#ifndef HAVE_KTIME_GET_TS64
  #define ktime_get_ts64 ktime_get_ts
  #define timespec64 timespec
  #endif




---
   datapath/flow.c | 11 ---
   1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/datapath/flow.c b/datapath/flow.c
index 5da7e3e..385e481 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -52,14 +52,19 @@
   #include "flow_netlink.h"
   #include "vport.h"
   +#ifndef ktime_get_ts64
+#define ktime_get_ts64 ktime_get_ts
+#define timespec64 timespec
+#endif
+

This is done in compat code, can you move it to respective header file?


Yes - my own preference is to keep these sorts of things close to where 
they're used but
I suppose there is a good chance we'll use ktime_get_ts64 elsewhere in 
the future.  So

that's fine by me.

Thanks,

- Greg


   u64 ovs_flow_used_time(unsigned long flow_jiffies)
   {
-   struct timespec cur_ts;
+   struct timespec64 cur_ts;
 u64 cur_ms, idle_ms;
   - ktime_get_ts(_ts);
+   ktime_get_ts64(_ts);
 idle_ms = jiffies_to_msecs(jiffies - flow_jiffies);
-   cur_ms = (u64)cur_ts.tv_sec * MSEC_PER_SEC +
+   cur_ms = (u64)(u32)cur_ts.tv_sec * MSEC_PER_SEC +
  cur_ts.tv_nsec / NSEC_PER_MSEC;
 return cur_ms - idle_ms;


___
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 V2] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support

[Ben Pfaff]

Please start by rebasing and reposting.

On Fri, Feb 02, 2018 at 06:04:49PM +0530, Satyavalli Rama wrote:
> Hi Ben,
> 
> We didn't observed any test case breaks and we've updated the same with logs 
> in our previous conversations.
> Could you please provide your inputs regarding the Jan's comments about 
> command syntax modifications.
> 
> Thanks & Regards
> Satya Valli
> Tata Consultancy Services
> Mailto: satyavalli.r...@tcs.com
> Website: http://www.tcs.com
> 
> Experience certainty. IT Services
> Business Solutions
> Consulting
> 
> 
> 
> -Ben Pfaff  wrote: -
> To: Satyavalli Rama 
> From: Ben Pfaff 
> Date: 02/02/2018 03:43AM
> Cc: SatyaValli , "d...@openvswitch.org" 
> , manasa.cherukupa...@tcs.com, p.pava...@tcs.com, 
> Harivelam Lavanya , muttamsetty.su...@tcs.com, Jan 
> Scheurich 
> Subject: Re: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry 
> Statistics Support
> 
> At the very minimum I can't review a patch that breaks tests.
> 
> On Wed, Jan 31, 2018 at 05:29:12PM +0530, Satyavalli Rama wrote:
> > Hi Ben,
> > 
> > Are you also agreeing with the Jan's comments.
> > 
> > Thanks & Regards
> > Satya Valli
> > Tata Consultancy Services
> > Mailto: satyavalli.r...@tcs.com
> > Website: http://www.tcs.com
> > 
> > Experience certainty.   IT Services
> > Business Solutions
> > Consulting
> > 
> > 
> > 
> > -Jan Scheurich  wrote: -
> > To: Satyavalli Rama , Ben Pfaff 
> > From: Jan Scheurich 
> > Date: 01/08/2018 06:31PM
> > Cc: SatyaValli , "d...@openvswitch.org" 
> > , "manasa.cherukupa...@tcs.com" 
> > , "p.pava...@tcs.com" , 
> > Harivelam Lavanya , "muttamsetty.su...@tcs.com" 
> > 
> > Subject: RE: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry 
> > Statistics Support
> > 
> > Hi Satyavalli,
> >  
> > Please find my responses below.
> >  
> > Regards, Jan
> >  
> > From: Satyavalli Rama [mailto:satyavalli.r...@tcs.com] 
> > Sent: Friday, 05 January, 2018 12:25
> > To: Ben Pfaff ; Jan Scheurich 
> > Cc: SatyaValli ; d...@openvswitch.org; 
> > manasa.cherukupa...@tcs.com; p.pava...@tcs.com; Harivelam Lavanya 
> > ; muttamsetty.su...@tcs.com
> > Subject: Re: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry 
> > Statistics Support
> >  
> > Hi Jan and Ben,
> >  
> > Please find the inline responses.
> >  
> > 
> > -Ben Pfaff  wrote: -
> > To: Jan Scheurich 
> > From: Ben Pfaff 
> > Date: 01/05/2018 02:35AM
> > Cc: SatyaValli , "d...@openvswitch.org" 
> > , Manasa Cherukupally , 
> > Pavani Panthagada , Lavanya Harivelam 
> > , Surya Muttamsetty , 
> > SatyaValli 
> > Subject: Re: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry 
> > Statistics Support
> > 
> > On Wed, Jan 03, 2018 at 04:24:06PM +, Jan Scheurich wrote:
> > > > > >
> > > > > > This Patch provides implementation Existing flow entry statistics 
> > > > > > are
> > > > > > redefined as standard OXS(OpenFlow Extensible Statistics) fields for
> > > > > > displaying the arbitrary flow stats.The existing Flow Stats were 
> > > > > > renamed
> > > > > > as Flow Description.
> > > > > >
> > > > > > To support this implementation below messages are newly added
> > > > > >
> > > > > > OFPRAW_OFPT15_FLOW_REMOVED,
> > > > > > OFPRAW_OFPST15_FLOW_REQUEST,
> > > > > > OFPRAW_OFPST15_FLOW_DESC_REQUEST,
> > > > > > OFPRAW_OFPST15_AGGREGATE_REQUEST,
> > > > > > OFPRAW_OFPST15_FLOW_REPLY,
> > > > > > OFPRAW_OFPST15_FLOW_DESC_REPLY,
> > > > > > OFPRAW_OFPST15_AGGREGATE_REPLY,
> > > > > >
> > > > > > The current commit adds support for the new feature in flow 
> > > > > > statistics
> > > > > > multipart messages,aggregate multipart messages and OXS support for 
> > > > > > flow
> > > > > > removal message, individual flow description messages.
> > > > > >
> > > > > > "ovs-ofctl dump-flows" needs to be provided with the arbitrary OXS 
> > > > > > fields
> > > > > > for displaying the desired flow stats.
> > > > > >
> > > > > > Below are Commands to display OXS stats field wise
> > > > > >
> > > > > > Flow Statistics Multipart
> > > > > > ovs-ofctl dump-flows -O OpenFlow15  idle_time

Re: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support

[Jan Scheurich]

Hi Ben,

I would appreciate if you could comment on my general concerns with this patch.

I think it is unwise to sacrifice the semantics of the established ovs-ofctl 
dump-flow command just in order to align the CLI commands with the particular 
re-arrangement of  multipart requests messages in OpenFlow 1.5.

There is no need for one-to-one correspondence between ovs-ofctl commands and 
underlying OF message types. Especially not if these change between OF 
versions. The CLI commands should have a well-defined stable semantics and use 
whatever message is appropriate for the OF protocol version in question to 
implement that.

Thanks, Jan

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Friday, 02 February, 2018 16:52
> To: Satyavalli Rama 
> Cc: SatyaValli ; d...@openvswitch.org; 
> manasa.cherukupa...@tcs.com; p.pava...@tcs.com; Harivelam
> Lavanya ; muttamsetty.su...@tcs.com; Jan Scheurich 
> 
> Subject: Re: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry 
> Statistics Support
> 
> Please start by rebasing and reposting.
> 
> On Fri, Feb 02, 2018 at 06:04:49PM +0530, Satyavalli Rama wrote:
> > Hi Ben,
> >
> > We didn't observed any test case breaks and we've updated the same with 
> > logs in our previous conversations.
> > Could you please provide your inputs regarding the Jan's comments about 
> > command syntax modifications.
> >
> > Thanks & Regards
> > Satya Valli
> > Tata Consultancy Services
> > Mailto: satyavalli.r...@tcs.com
> > Website: http://www.tcs.com
> > 
> > Experience certainty.   IT Services
> > Business Solutions
> > Consulting
> > 
> >
> >
> > -Ben Pfaff  wrote: -
> > To: Satyavalli Rama 
> > From: Ben Pfaff 
> > Date: 02/02/2018 03:43AM
> > Cc: SatyaValli , "d...@openvswitch.org" 
> > , manasa.cherukupa...@tcs.com,
> p.pava...@tcs.com, Harivelam Lavanya , 
> muttamsetty.su...@tcs.com, Jan Scheurich
> 
> > Subject: Re: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry 
> > Statistics Support
> >
> > At the very minimum I can't review a patch that breaks tests.
> >
> > On Wed, Jan 31, 2018 at 05:29:12PM +0530, Satyavalli Rama wrote:
> > > Hi Ben,
> > >
> > > Are you also agreeing with the Jan's comments.
> > >
> > > Thanks & Regards
> > > Satya Valli
> > > Tata Consultancy Services
> > > Mailto: satyavalli.r...@tcs.com
> > > Website: http://www.tcs.com
> > > 
> > > Experience certainty. IT Services
> > > Business Solutions
> > > Consulting
> > > 
> > >
> > >
> > > -Jan Scheurich  wrote: -
> > > To: Satyavalli Rama , Ben Pfaff 
> > > From: Jan Scheurich 
> > > Date: 01/08/2018 06:31PM
> > > Cc: SatyaValli , "d...@openvswitch.org" 
> > > , "manasa.cherukupa...@tcs.com"
> , "p.pava...@tcs.com" , 
> Harivelam Lavanya ,
> "muttamsetty.su...@tcs.com" 
> > > Subject: RE: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry 
> > > Statistics Support
> > >
> > > Hi Satyavalli,
> > >
> > > Please find my responses below.
> > >
> > > Regards, Jan
> > >
> > > From: Satyavalli Rama [mailto:satyavalli.r...@tcs.com]
> > > Sent: Friday, 05 January, 2018 12:25
> > > To: Ben Pfaff ; Jan Scheurich 
> > > Cc: SatyaValli ; d...@openvswitch.org; 
> > > manasa.cherukupa...@tcs.com; p.pava...@tcs.com; Harivelam
> Lavanya ; muttamsetty.su...@tcs.com
> > > Subject: Re: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry 
> > > Statistics Support
> > >
> > > Hi Jan and Ben,
> > >
> > > Please find the inline responses.
> > >
> > >
> > > -Ben Pfaff  wrote: -
> > > To: Jan Scheurich 
> > > From: Ben Pfaff 
> > > Date: 01/05/2018 02:35AM
> > > Cc: SatyaValli , "d...@openvswitch.org" 
> > > , Manasa Cherukupally
> , Pavani Panthagada , Lavanya 
> Harivelam , Surya
> Muttamsetty , SatyaValli 
> > > Subject: Re: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry 
> > > Statistics Support
> > >
> > > On Wed, Jan 03, 2018 at 04:24:06PM +, Jan Scheurich wrote:
> > > > > > >
> > > > > > > This Patch provides 

Re: [ovs-dev] [PATCH] memory: kill ovs-vswitchd under super

[William Tu]

Hi Zhike,

On Fri, Feb 2, 2018 at 7:48 AM, Ben Pfaff  wrote:
> On Fri, Feb 02, 2018 at 12:37:58PM +, 王志克 wrote:
>> I also found that if once theres are lots of flows, the memory (RSS) usage 
>> of OVS process would be quite high, 2~3GB. Even then the flows disappear 
>> later, the memory still keeps.

Are you able to reproduce the issue?
There might be a memory leak around cls_rule, miniflow/minimatch allocation.
Can you share more information about your setup?

Thanks!
William

>>
>> I am not sure how many people notices this, but if indeed OVS has such 
>> defect, I guess this should be critical blocker.
>
> This is normal behavior of the C library malloc implementation.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 00/20] Update and backport of upstream Linux

[Pravin Shelar]

On Tue, Jan 30, 2018 at 3:40 PM, Greg Rose  wrote:
> Allow OVS to compile and build on Linux 4.14.x kernels.  Added
> necessary compatability layer changes to the respective patches
> as required for our OOT build environment.
>
> Note that NSH and ERSPAN patches are not in this series.  We
> are working with the authors of those patches to get them
> backported.
>
> This series of patches was originally sent as two separate sets
> however the dependencies and compatability layer requirements
> made it more convenient to combine the two sets.
>
> Andy Zhou (3):
>   datapath: export get_dp() API
>   datapath: Add meter netlink definitions
>   datapath: Add meter infrastructure
>
> Arnd Bergmann (1):
>   datapath: use ktime_get_ts64() instead of ktime_get_ts()
>
> Christophe JAILLET (1):
>   datapath:  Fix an error handling path in
> 'ovs_nla_init_match_and_action()
>
> Florian Westphal (1):
>   datapath: conntrack: make protocol tracker pointers const
>
> Greg Rose (8):
>   datapath: Fix netdev_master_upper_dev_link for 4.14
>   compat: Do not include headers when not compiling
>   datapath: Fix SKB_GSO_UDP usage
>   acinclude.m4: Enable Linux 4.14
>   travis: Update kernel test list from kernel.org
>   compat: Fix compiler headers
>   compat:inet_frag.h: Check for frag_percpu_counter_batch
>   Documentation: Update NEWS and faq
>
> Gustavo A. R. Silva (2):
>   datapath: meter: fix NULL pointer dereference in
> ovs_meter_cmd_reply_start
>   datapath: fix data type in queue_gso_packets
>
> Jiri Benc (1):
>   datapath: reliable interface indentification in port dumps
>
> Wei Yongjun (2):
>   datapath: Fix return value check in ovs_meter_cmd_features()
>   datapath: Using kfree_rcu() to simplify the code
>
> zhangliping (1):
>   datapath: fix the incorrect flow action alloc size
>
The patch series looks good. I have few comment on couple of patches.
About metering and namespace related userspace patches are not in 2.9,
so can you create two separate series. one with fixes for master ( and
can be backported  2.9) and second with the features which can be
targeted for master branch only.

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

Re: [ovs-dev] [PATCH 11/20] datapath: Add meter netlink definitions

[Pravin Shelar]

On Tue, Jan 30, 2018 at 3:40 PM, Greg Rose  wrote:
> From: Andy Zhou 
>
> Upstream commit:
> commit 5794040647de4011598a6d005fdad95d24fd385b
> Author: Andy Zhou 
> Date:   Fri Nov 10 12:09:40 2017 -0800
>
> openvswitch: Add meter netlink definitions
>
> Meter has its own netlink family. Define netlink messages and attributes
> for communicating with the user space programs.
>
> Signed-off-by: Andy Zhou 
> Signed-off-by: David S. Miller 
>
> Cc: Andy Zhou 
> Signed-off-by: Greg Rose 

I am not sure if meter userspace is targeted for 2.9, if it is then we
can backport metering related patches to 2.9
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] ovn-controller: Document southbound database use and graceful termination.

[Ben Pfaff]

A lot of people seem to think that "kill" gracefully terminates
ovn-controller, but it doesn't, so this documentation at least provides
something to point to.

Signed-off-by: Ben Pfaff 
---
 ovn/controller/ovn-controller.8.xml | 47 +
 1 file changed, 47 insertions(+)

diff --git a/ovn/controller/ovn-controller.8.xml 
b/ovn/controller/ovn-controller.8.xml
index 0df59acd3c8b..96a58ddf358d 100644
--- a/ovn/controller/ovn-controller.8.xml
+++ b/ovn/controller/ovn-controller.8.xml
@@ -304,6 +304,53 @@
   
 
 
+OVN Southbound Database Usage
+
+
+  ovn-controller reads from much of the
+  OVN_Southbound database to guide its operation.
+  ovn-controller also writes to the following tables:
+
+
+
+  Chassis
+  
+Upon startup, ovn-controller creates a row in this table
+to represent its own chassis.  Upon graceful termination, e.g. with
+ovs-appctl -t ovn-controller exit (but not
+SIGTERM), ovn-controller removes its row.
+  
+
+  Encap
+  
+Upon startup, ovn-controller creates a row or rows in this
+table that represent the tunnel encapsulations by which its chassis can
+be reached, and points its Chassis row to them.  Upon
+graceful termination, ovn-controller removes these rows.
+  
+
+  Port_Binding
+  
+At runtime, ovn-controller sets the chassis
+columns of ports that are resident on its chassis to point to its
+Chassis row, and, conversely, clears the
+chassis column of ports that point to its
+Chassis row but are no longer resident on its chassis.
+The chassis column has a weak reference type, so when
+ovn-controller gracefully exits and removes its
+Chassis row, the database server automatically clears any
+remaining references to that row.
+  
+
+  MAC_Binding
+  
+At runtime, ovn-controller updates the
+MAC_Binding table as instructed by put_arp
+and put_nd logical actions.  These changes persist beyond
+the lifetime of ovn-controller.
+  
+
+
 Runtime Management Commands
 
   ovs-appctl can send commands to a running
-- 
2.15.1

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

Re: [ovs-dev] [PATCH 00/20] Update and backport of upstream Linux

[Gregory Rose]

On 2/2/2018 10:20 AM, Pravin Shelar wrote:

On Tue, Jan 30, 2018 at 3:40 PM, Greg Rose  wrote:

Allow OVS to compile and build on Linux 4.14.x kernels.  Added
necessary compatability layer changes to the respective patches
as required for our OOT build environment.

Note that NSH and ERSPAN patches are not in this series.  We
are working with the authors of those patches to get them
backported.

This series of patches was originally sent as two separate sets
however the dependencies and compatability layer requirements
made it more convenient to combine the two sets.

Andy Zhou (3):
   datapath: export get_dp() API
   datapath: Add meter netlink definitions
   datapath: Add meter infrastructure

Arnd Bergmann (1):
   datapath: use ktime_get_ts64() instead of ktime_get_ts()

Christophe JAILLET (1):
   datapath:  Fix an error handling path in
 'ovs_nla_init_match_and_action()

Florian Westphal (1):
   datapath: conntrack: make protocol tracker pointers const

Greg Rose (8):
   datapath: Fix netdev_master_upper_dev_link for 4.14
   compat: Do not include headers when not compiling
   datapath: Fix SKB_GSO_UDP usage
   acinclude.m4: Enable Linux 4.14
   travis: Update kernel test list from kernel.org
   compat: Fix compiler headers
   compat:inet_frag.h: Check for frag_percpu_counter_batch
   Documentation: Update NEWS and faq

Gustavo A. R. Silva (2):
   datapath: meter: fix NULL pointer dereference in
 ovs_meter_cmd_reply_start
   datapath: fix data type in queue_gso_packets

Jiri Benc (1):
   datapath: reliable interface indentification in port dumps

Wei Yongjun (2):
   datapath: Fix return value check in ovs_meter_cmd_features()
   datapath: Using kfree_rcu() to simplify the code

zhangliping (1):
   datapath: fix the incorrect flow action alloc size


The patch series looks good. I have few comment on couple of patches.
About metering and namespace related userspace patches are not in 2.9,
so can you create two separate series. one with fixes for master ( and
can be backported  2.9) and second with the features which can be
targeted for master branch only.

Thanks.


I will do that.  Thanks for the reviews Pravin!!

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

Re: [ovs-dev] [PATCH v2 0/5] datapath: enable NSH support in kernel compat mode

[Gregory Rose]

On 1/31/2018 5:53 AM, Yi Yang wrote:

v1->v2
  - Fix compilation error in linux-3.10.107

This patch series is to backport NSH support patches in Linux net-next tree
to OVS in order that it can support NSH in kernel compat mode.

Yi Yang (5):
   datapath: ether: add NSH ethertype
   datapath: vxlan: factor out VXLAN-GPE next protocol
   datapath: net: add NSH header structures and helpers
   datapath: nsh: add GSO support
   datapath: enable NSH support

  NEWS  |   1 +
  acinclude.m4  |   1 +
  datapath/Modules.mk   |   4 +-
  datapath/actions.c| 116 
  datapath/datapath.c   |   4 +
  datapath/flow.c   |  51 
  datapath/flow.h   |   7 +
  datapath/flow_netlink.c   | 343 +-
  datapath/flow_netlink.h   |   5 +
  datapath/linux/Modules.mk |   2 +
  datapath/linux/compat/include/linux/if_ether.h|   4 +
  datapath/linux/compat/include/linux/netdevice.h   |  14 +
  datapath/linux/compat/include/linux/openvswitch.h |   6 +-
  datapath/linux/compat/include/net/nsh.h   | 313 
  datapath/linux/compat/include/net/tun_proto.h |  49 
  datapath/linux/compat/include/net/vxlan.h |   6 -
  datapath/linux/compat/vxlan.c |  32 +-
  datapath/nsh.c| 142 +
  18 files changed, 1063 insertions(+), 37 deletions(-)
  create mode 100644 datapath/linux/compat/include/net/nsh.h
  create mode 100644 datapath/linux/compat/include/net/tun_proto.h
  create mode 100644 datapath/nsh.c



Yi,

I have finished review of the patches and they look fine to me.  As per 
our offline discussions it
doesn't seem as if there is a test framework available for me right now 
to be able to test the patches
but I did make sure there are no regressions in our current test and 
compile.  The patches passed

the travis check here:

https://travis-ci.org/gvrose8192/ovs-experimental/builds/336662468

I am collaborating with a co-worker here at VMware to document and 
create an NSH test bed but that won't

be done for a while.

I'll go ahead and provide my review sign off but for obvious reasons I 
can't apply a tested by sign off.
I think the patches are pretty well vetted both upstream and here and I 
think they're fine to apply so
that we can get started testing.  I don't think there will be any 
negative effect on current features or
code stability - no "make check" and "make check-kmod" tests show any 
regressions  It will be up to

the maintainers whether this is sufficient to apply your patches.

For the series...

Reviewed-by: Greg Rose 

Thanks,

- Greg

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

[ovs-dev] [PATCH 1/4] ovs-vsctl: Remove superfluous OVS_VSCTL_CLEANUP from tests.

[Ben Pfaff]

Since on_exit was introduced a long, long time ago, it has no longer been
necessary to have individual calls to OVS_VSCTL_CLEANUP sprinkled
everywhere in the test code.  This change makes the tests easier to read.

Signed-off-by: Ben Pfaff 
---
 tests/ovs-vsctl.at | 321 +++--
 1 file changed, 138 insertions(+), 183 deletions(-)

diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index d54188976ad9..2c4c45d70947 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -36,49 +36,43 @@ dnl which must be in alphabetical order.  Also checks that 
each BRIDGE has the
 dnl specified PARENT and is on the given VLAN.
 m4_define([_CHECK_BRIDGE],
   [AT_CHECK([RUN_OVS_VSCTL([br-to-parent $1])], [0], [$2
-], [], [OVS_VSCTL_CLEANUP])
+])
 
# Check br-to-vlan, without --oneline.
AT_CHECK([RUN_OVS_VSCTL([br-to-vlan $1])], [0], [$3
-], [], [OVS_VSCTL_CLEANUP])
+])
# Check br-to-vlan, with --oneline.
# (This particular test is interesting with --oneline because it returns
# an integer instead of a string and that can cause type mismatches inside
# python if not done carefully.)
AT_CHECK([RUN_OVS_VSCTL_ONELINE([br-to-vlan $1])], [0], [$3
-], [], [OVS_VSCTL_CLEANUP])
+])
 
# Check multiple queries in a single run.
AT_CHECK([RUN_OVS_VSCTL_TOGETHER([br-to-parent $1], [br-to-vlan $1])], [0],
 [$2
 $3
-], [], [OVS_VSCTL_CLEANUP])])
+])])
 m4_define([CHECK_BRIDGES],
   [dnl Check that the bridges appear on list-br, without --oneline.
AT_CHECK(
  [RUN_OVS_VSCTL([list-br])],
  [0],
  [m4_foreach([brinfo], [$@], [m4_car(brinfo)
-])],
- [],
- [OVS_VSCTL_CLEANUP])
+])])
 
dnl Check that the bridges appear on list-br, with --oneline.
AT_CHECK(
  [RUN_OVS_VSCTL_ONELINE([list-br])],
  [0],
  [m4_join([\n], m4_foreach([brinfo], [$@], [m4_car(brinfo),]))
-],
- [],
- [OVS_VSCTL_CLEANUP])
+])
 
dnl Check that each bridge exists according to br-exists and that
dnl a bridge that should not exist does not.
m4_foreach([brinfo], [$@],
-  [AT_CHECK([RUN_OVS_VSCTL([br-exists m4_car(brinfo)])], [0], [],
-[], [OVS_VSCTL_CLEANUP])])
-   AT_CHECK([RUN_OVS_VSCTL([br-exists nonexistent])], [2], [], [],
-[OVS_VSCTL_CLEANUP])
+  [AT_CHECK([RUN_OVS_VSCTL([br-exists m4_car(brinfo)])])])
+   AT_CHECK([RUN_OVS_VSCTL([br-exists nonexistent])], [2])
 
dnl Check that each bridge has the expected parent and VLAN.
m4_map([_CHECK_BRIDGE], [$@])])
@@ -95,26 +89,21 @@ m4_define([CHECK_PORTS],
  [RUN_OVS_VSCTL([list-ports $1])],
  [0],
  [m4_foreach([port], m4_cdr($@), [port
-])],
- [],
- [OVS_VSCTL_CLEANUP])
+])])
 
dnl Check ports with --oneline.
AT_CHECK(
  [RUN_OVS_VSCTL_ONELINE([list-ports $1])],
  [0],
  [m4_join([\n], m4_shift($@))
-],
- [],
- [OVS_VSCTL_CLEANUP])
+])
AT_CHECK([RUN_OVS_VSCTL([port-to-br $1])], [1], [],
 [ovs-vsctl: no port named $1
-],
-[OVS_VSCTL_CLEANUP])
+])
m4_foreach(
  [port], m4_cdr($@),
  [AT_CHECK([RUN_OVS_VSCTL([[port-to-br] port])], [0], [$1
-], [], [OVS_VSCTL_CLEANUP])])])
+])])])
 
 dnl CHECK_IFACES(BRIDGE, IFACE[, IFACE...])
 dnl
@@ -127,18 +116,14 @@ m4_define([CHECK_IFACES],
  [RUN_OVS_VSCTL([list-ifaces $1])],
  [0],
  [m4_foreach([iface], m4_cdr($@), [iface
-])],
- [],
- [OVS_VSCTL_CLEANUP])
+])])
AT_CHECK([RUN_OVS_VSCTL([iface-to-br $1])], [1], [],
 [ovs-vsctl: no interface named $1
-],
-[OVS_VSCTL_CLEANUP])
+])
m4_foreach(
  [iface], m4_cdr($@),
  [AT_CHECK([RUN_OVS_VSCTL([[iface-to-br] iface])], [0], [$1
-],
-   [], [OVS_VSCTL_CLEANUP])])])
+])])])
 
 dnl --
 AT_BANNER([ovs-vsctl unit tests])
@@ -190,7 +175,7 @@ AT_BANNER([ovs-vsctl unit tests -- real bridges])
 AT_SETUP([add-br a])
 AT_KEYWORDS([ovs-vsctl])
 OVS_VSCTL_SETUP
-AT_CHECK([RUN_OVS_VSCTL([add-br a])], [0], [], [], [OVS_VSCTL_CLEANUP])
+AT_CHECK([RUN_OVS_VSCTL([add-br a])])
 CHECK_BRIDGES([a, a, 0])
 CHECK_PORTS([a])
 CHECK_IFACES([a])
@@ -200,25 +185,23 @@ AT_CLEANUP
 AT_SETUP([add-br a, add-br a])
 AT_KEYWORDS([ovs-vsctl])
 OVS_VSCTL_SETUP
-AT_CHECK([RUN_OVS_VSCTL([add-br a])], [0], [], [], [OVS_VSCTL_CLEANUP])
+AT_CHECK([RUN_OVS_VSCTL([add-br a])], [0])
 AT_CHECK([RUN_OVS_VSCTL([add-br a])], [1], [],
   [ovs-vsctl: cannot create a bridge named a because a bridge named a already 
exists
-], [OVS_VSCTL_CLEANUP])
+])
 AT_CHECK([RUN_OVS_VSCTL([add-br ''])], [1], [],
   [ovs-vsctl: bridge name must not be empty string
-], [OVS_VSCTL_CLEANUP])
+])
 OVS_VSCTL_CLEANUP
 AT_CLEANUP
 
 AT_SETUP([add-br a, add-br b])
 AT_KEYWORDS([ovs-vsctl])
 OVS_VSCTL_SETUP
-AT_CHECK([RUN_OVS_VSCTL([add-br a], [add-br b])], [0], [], [],
- [OVS_VSCTL_CLEANUP])
+AT_CHECK([RUN_OVS_VSCTL([add-br a], [add-br b])])
 

Re: [ovs-dev] [PATCH] ovn-controller: Document southbound database use and graceful termination.

[Ben Pfaff]

On Fri, Feb 02, 2018 at 12:07:26PM -0800, Han Zhou wrote:
> On Fri, Feb 2, 2018 at 9:44 AM, Ben Pfaff  wrote:
> >
> > A lot of people seem to think that "kill" gracefully terminates
> > ovn-controller, but it doesn't, so this documentation at least provides
> > something to point to.
> >
> > Signed-off-by: Ben Pfaff 
> > ---
> >  ovn/controller/ovn-controller.8.xml | 47
> +
> >  1 file changed, 47 insertions(+)
> >
> > diff --git a/ovn/controller/ovn-controller.8.xml
> b/ovn/controller/ovn-controller.8.xml
> > index 0df59acd3c8b..96a58ddf358d 100644
> > --- a/ovn/controller/ovn-controller.8.xml
> > +++ b/ovn/controller/ovn-controller.8.xml
> > @@ -304,6 +304,53 @@
> >
> >  
> >
> > +OVN Southbound Database Usage
> > +
> > +
> > +  ovn-controller reads from much of the
> > +  OVN_Southbound database to guide its operation.
> > +  ovn-controller also writes to the following tables:
> > +
> > +
> > +
> > +  Chassis
> > +  
> > +Upon startup, ovn-controller creates a row in this
> table
> > +to represent its own chassis.  Upon graceful termination, e.g.
> with
> > +ovs-appctl -t ovn-controller exit (but not
> > +SIGTERM), ovn-controller removes its
> row.
> > +  
> > +
> > +  Encap
> > +  
> > +Upon startup, ovn-controller creates a row or rows
> in this
> > +table that represent the tunnel encapsulations by which its
> chassis can
> > +be reached, and points its Chassis row to them.
> Upon
> > +graceful termination, ovn-controller removes these
> rows.
> > +  
> > +
> > +  Port_Binding
> > +  
> > +At runtime, ovn-controller sets the
> chassis
> > +columns of ports that are resident on its chassis to point to its
> > +Chassis row, and, conversely, clears the
> > +chassis column of ports that point to its
> > +Chassis row but are no longer resident on its
> chassis.
> > +The chassis column has a weak reference type, so
> when
> > +ovn-controller gracefully exits and removes its
> > +Chassis row, the database server automatically
> clears any
> > +remaining references to that row.
> > +  
> > +
> > +  MAC_Binding
> > +  
> > +At runtime, ovn-controller updates the
> > +MAC_Binding table as instructed by
> put_arp
> > +and put_nd logical actions.  These changes persist
> beyond
> > +the lifetime of ovn-controller.
> > +  
> > +
> > +
> >  Runtime Management Commands
> >  
> >ovs-appctl can send commands to a running
> > --
> > 2.15.1
> >
> 
> Thanks Ben!
> 
> Acked-by: Han Zhou 

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

Re: [ovs-dev] [PATCH] datapath-windows: Add trace level logs in conntrack for invalid ct state.

[Alin Serdean]

Looks good just a small nit:
s/syn/SYN/g
s/ack/ACK/g

Also I would prefer if you drop the text `Invalid!`. Either just remove it or 
expand it, i.e.:
"Invalid! ICMPhdr cannot be NULL" => "Invalid ICMP packet detected the header 
cannot be NULL"

Thanks,
Alin.

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-boun...@openvswitch.org] 
În numele Anand Kumar
Trimis: Friday, February 2, 2018 11:19 PM
Către: d...@openvswitch.org
Subiect: [ovs-dev] [PATCH] datapath-windows: Add trace level logs in conntrack 
for invalid ct state.

Signed-off-by: Anand Kumar 
---
 datapath-windows/ovsext/Conntrack-icmp.c | 1 +  
datapath-windows/ovsext/Conntrack-tcp.c  | 4 
 datapath-windows/ovsext/Conntrack.c  | 4 
 3 files changed, 9 insertions(+)

diff --git a/datapath-windows/ovsext/Conntrack-icmp.c 
b/datapath-windows/ovsext/Conntrack-icmp.c
index 4da0665..d86feed 100644
--- a/datapath-windows/ovsext/Conntrack-icmp.c
+++ b/datapath-windows/ovsext/Conntrack-icmp.c
@@ -61,6 +61,7 @@ BOOLEAN
 OvsConntrackValidateIcmpPacket(const ICMPHdr *icmp)  {
 if (!icmp) {
+OVS_LOG_TRACE("Invalid! ICMPhdr cannot be NULL");
 return FALSE;
 }
 
diff --git a/datapath-windows/ovsext/Conntrack-tcp.c 
b/datapath-windows/ovsext/Conntrack-tcp.c
index f8e85a2..65eaac5 100644
--- a/datapath-windows/ovsext/Conntrack-tcp.c
+++ b/datapath-windows/ovsext/Conntrack-tcp.c
@@ -444,12 +444,14 @@ BOOLEAN
 OvsConntrackValidateTcpPacket(const TCPHdr *tcp)  {
 if (!tcp) {
+OVS_LOG_TRACE("Invalid! TCPHdr cannot be NULL");
 return FALSE;
 }
 
 UINT16 tcp_flags = ntohs(tcp->flags);
 
 if (OvsCtInvalidTcpFlags(tcp_flags)) {
+OVS_LOG_TRACE("Invalid! tcp_flags %hu", tcp_flags);
 return FALSE;
 }
 
@@ -457,6 +459,8 @@ OvsConntrackValidateTcpPacket(const TCPHdr *tcp)
  * totally new connections (syn) or already established, not partially
  * open (syn+ack). */
 if ((tcp_flags & TCP_SYN) && (tcp_flags & TCP_ACK)) {
+OVS_LOG_TRACE("Invalid! syn+ack flags not allowed, tcp_flags %hu",
+  tcp_flags);
 return FALSE;
 }
 
diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index 43c9dd3..7e413c6 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -317,6 +317,9 @@ OvsCtEntryCreate(OvsForwardingContext *fwdCtx,
 const ICMPHdr *icmp;
 icmp = OvsGetIcmp(curNbl, l4Offset, );
 if (!OvsConntrackValidateIcmpPacket(icmp)) {
+if(icmp) {
+OVS_LOG_TRACE("Invalid! icmp->type %u", icmp->type);
+}
 state = OVS_CS_F_INVALID;
 break;
 }
@@ -334,6 +337,7 @@ OvsCtEntryCreate(OvsForwardingContext *fwdCtx,
 break;
 }
 default:
+OVS_LOG_TRACE("Invalid! Not supported protocol, ipProto %u", 
+ ipProto);
 state = OVS_CS_F_INVALID;
 break;
 }
--
2.9.3.windows.1

___
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] [PATCH] datapath-windows: Add trace level logs in conntrack for invalid ct state.

[Anand Kumar]

Signed-off-by: Anand Kumar 
---
 datapath-windows/ovsext/Conntrack-icmp.c | 1 +
 datapath-windows/ovsext/Conntrack-tcp.c  | 4 
 datapath-windows/ovsext/Conntrack.c  | 4 
 3 files changed, 9 insertions(+)

diff --git a/datapath-windows/ovsext/Conntrack-icmp.c 
b/datapath-windows/ovsext/Conntrack-icmp.c
index 4da0665..d86feed 100644
--- a/datapath-windows/ovsext/Conntrack-icmp.c
+++ b/datapath-windows/ovsext/Conntrack-icmp.c
@@ -61,6 +61,7 @@ BOOLEAN
 OvsConntrackValidateIcmpPacket(const ICMPHdr *icmp)
 {
 if (!icmp) {
+OVS_LOG_TRACE("Invalid! ICMPhdr cannot be NULL");
 return FALSE;
 }
 
diff --git a/datapath-windows/ovsext/Conntrack-tcp.c 
b/datapath-windows/ovsext/Conntrack-tcp.c
index f8e85a2..65eaac5 100644
--- a/datapath-windows/ovsext/Conntrack-tcp.c
+++ b/datapath-windows/ovsext/Conntrack-tcp.c
@@ -444,12 +444,14 @@ BOOLEAN
 OvsConntrackValidateTcpPacket(const TCPHdr *tcp)
 {
 if (!tcp) {
+OVS_LOG_TRACE("Invalid! TCPHdr cannot be NULL");
 return FALSE;
 }
 
 UINT16 tcp_flags = ntohs(tcp->flags);
 
 if (OvsCtInvalidTcpFlags(tcp_flags)) {
+OVS_LOG_TRACE("Invalid! tcp_flags %hu", tcp_flags);
 return FALSE;
 }
 
@@ -457,6 +459,8 @@ OvsConntrackValidateTcpPacket(const TCPHdr *tcp)
  * totally new connections (syn) or already established, not partially
  * open (syn+ack). */
 if ((tcp_flags & TCP_SYN) && (tcp_flags & TCP_ACK)) {
+OVS_LOG_TRACE("Invalid! syn+ack flags not allowed, tcp_flags %hu",
+  tcp_flags);
 return FALSE;
 }
 
diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index 43c9dd3..7e413c6 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -317,6 +317,9 @@ OvsCtEntryCreate(OvsForwardingContext *fwdCtx,
 const ICMPHdr *icmp;
 icmp = OvsGetIcmp(curNbl, l4Offset, );
 if (!OvsConntrackValidateIcmpPacket(icmp)) {
+if(icmp) {
+OVS_LOG_TRACE("Invalid! icmp->type %u", icmp->type);
+}
 state = OVS_CS_F_INVALID;
 break;
 }
@@ -334,6 +337,7 @@ OvsCtEntryCreate(OvsForwardingContext *fwdCtx,
 break;
 }
 default:
+OVS_LOG_TRACE("Invalid! Not supported protocol, ipProto %u", ipProto);
 state = OVS_CS_F_INVALID;
 break;
 }
-- 
2.9.3.windows.1

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

Re: [ovs-dev] [PATCH 18/20] datapath: use ktime_get_ts64() instead of ktime_get_ts()

[Arnd Bergmann]

On Fri, Feb 2, 2018 at 7:31 PM, Gregory Rose  wrote:
> On 2/2/2018 10:18 AM, Pravin Shelar wrote:
>>
>> On Tue, Jan 30, 2018 at 4:40 PM, Gregory Rose 
>>
>> This is done in compat code, can you move it to respective header file?
>
>
> Yes - my own preference is to keep these sorts of things close to where
> they're used but
> I suppose there is a good chance we'll use ktime_get_ts64 elsewhere in the
> future.  So
> that's fine by me.

You could decide to use ktime_get_ns() divided by NSEC_PER_MSEC
instead of getting that number from ktime_get_ts64(), that would be
more portable, though a little bit slower on 32-bit architectures.

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

Re: [ovs-dev] [PATCH v4 3/3] datapath-windows: Optimize conntrack lock implementation.

[Alin Serdean]

Thanks for the series Anand.

I folded the following to suppress the warning:
diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index c90c00074..43c9dd319 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -151,6 +151,9 @@ OvsCleanupConntrack(VOID)
 }

 for (UINT32 i = 0; i < CT_HASH_TABLE_SIZE; i++) {
+/* Disabling the uninitialized memory warning because it should
+ * always be initialized during OvsInitConntrack */
+#pragma warning(suppress: 6001)
 if (ovsCtBucketLock[i] != NULL) {
 NdisFreeRWLock(ovsCtBucketLock[i]);
 }
and applied it on master.

Alin.

-Mesaj original-
De la: Alin Serdean 
Trimis: Friday, February 2, 2018 10:52 PM
Către: Anand Kumar ; d...@openvswitch.org
Subiect: RE: [ovs-dev] [PATCH v4 3/3] datapath-windows: Optimize conntrack lock 
implementation.

Acked-by: Alin Gabriel Serdean 

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-boun...@openvswitch.org] 
În numele Anand Kumar
Trimis: Monday, January 29, 2018 8:28 PM
Către: d...@openvswitch.org
Subiect: [ovs-dev] [PATCH v4 3/3] datapath-windows: Optimize conntrack lock 
implementation.

Currently, there is one global lock for conntrack module, which protects 
conntrack entries and conntrack table. All the NAT operations are performed 
holding this lock.

This becomes inefficient, as the number of conntrack entries grow.
With new implementation, we will have two PNDIS_RW_LOCK_EX locks in conntrack.

1. ovsCtBucketLock - one rw lock per bucket of the conntrack table, which is 
shared by all the ct entries that belong to the same bucket.
2. lock -  a rw lock in OVS_CT_ENTRY structure that protects the members of 
conntrack entry.

Also, OVS_CT_ENTRY structure will have a lock reference(bucketLockRef) to the 
corresponding OvsCtBucketLock of conntrack table.
We need this reference to retrieve ovsCtBucketLock from ct entry for delete 
operation.

Signed-off-by: Anand Kumar 
---
v1->v2: Address potential memory leak in conntrack initialization.
v2->v3: Fix invalid memory access after deleting ct entry.
v3->v4: Address warning "uninitialized memory"
---
 datapath-windows/ovsext/Conntrack-nat.c |   6 +
 datapath-windows/ovsext/Conntrack.c | 233 
 datapath-windows/ovsext/Conntrack.h |   3 +
 3 files changed, 157 insertions(+), 85 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack-nat.c 
b/datapath-windows/ovsext/Conntrack-nat.c
index 7975770..316c946 100644
--- a/datapath-windows/ovsext/Conntrack-nat.c
+++ b/datapath-windows/ovsext/Conntrack-nat.c
@@ -167,12 +167,16 @@ OvsNatPacket(OvsForwardingContext *ovsFwdCtx,  {
 UINT32 natFlag;
 const struct ct_endpoint* endpoint;
+LOCK_STATE_EX lockState;
+/* XXX: Move conntrack locks out of NAT after implementing lock in NAT. */
+NdisAcquireRWLockRead(entry->lock, , 0);
 /* When it is NAT, only entry->rev_key contains NATTED address;
When it is unNAT, only entry->key contains the UNNATTED address;*/
 const OVS_CT_KEY *ctKey = reverse ? >key : >rev_key;
 BOOLEAN isSrcNat;
 
 if (!(natAction & (NAT_ACTION_SRC | NAT_ACTION_DST))) {
+NdisReleaseRWLock(entry->lock, );
 return;
 }
 isSrcNat = (((natAction & NAT_ACTION_SRC) && !reverse) || @@ -202,6 +206,7 
@@ OvsNatPacket(OvsForwardingContext *ovsFwdCtx,
 }
 } else if (ctKey->dl_type == htons(ETH_TYPE_IPV6)){
 // XXX: IPv6 packet not supported yet.
+NdisReleaseRWLock(entry->lock, );
 return;
 }
 if (natAction & (NAT_ACTION_SRC_PORT | NAT_ACTION_DST_PORT)) { @@ -215,6 
+220,7 @@ OvsNatPacket(OvsForwardingContext *ovsFwdCtx,
 }
 }
 }
+NdisReleaseRWLock(entry->lock, );
 }
 
 
diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index 7d56a50..c90c000 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -31,7 +31,7 @@
 KSTART_ROUTINE OvsConntrackEntryCleaner;  static PLIST_ENTRY 
ovsConntrackTable;  static OVS_CT_THREAD_CTX ctThreadCtx; -static 
PNDIS_RW_LOCK_EX ovsConntrackLockObj;
+static PNDIS_RW_LOCK_EX *ovsCtBucketLock = NULL;
 static PNDIS_RW_LOCK_EX ovsCtNatLockObj;  extern POVS_SWITCH_CONTEXT 
gOvsSwitchContext;  static LONG ctTotalEntries; @@ -49,20 +49,14 @@ 
MapNlToCtTuple(POVS_MESSAGE msgIn, PNL_ATTR attr,  NTSTATUS  
OvsInitConntrack(POVS_SWITCH_CONTEXT context)  {
-NTSTATUS status;
+NTSTATUS status = STATUS_SUCCESS;
 HANDLE threadHandle = NULL;
 ctTotalEntries = 0;
+UINT32 numBucketLocks = CT_HASH_TABLE_SIZE;
 
 /* Init the sync-lock */
-ovsConntrackLockObj = NdisAllocateRWLock(context->NdisFilterHandle);
-if (ovsConntrackLockObj == NULL) {
-return 

[ovs-dev] [PATCH 4/4] ovs-vsctl: Add commands "add-bond-iface" and "del-bond-iface".

[Ben Pfaff]

It was not too hard to build these commands using the database commands,
but a few people have asked for them over the years, so here they are.

Signed-off-by: Ben Pfaff 
---
 NEWS |  1 +
 tests/ovs-vsctl.at   | 60 +
 utilities/ovs-vsctl.8.in | 70 ++-
 utilities/ovs-vsctl.c| 77 ++--
 4 files changed, 185 insertions(+), 23 deletions(-)

diff --git a/NEWS b/NEWS
index 3b6ff74ad368..cc6dd95026de 100644
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,7 @@ Post-v2.9.0
  * ovs-ofctl now accepts and display table names in place of numbers.  By
default it always accepts names and in interactive use it displays them;
use --names or --no-names to override.  See ovs-ofctl(8) for details.
+   - ovs-vsctl: New commands "add-bond-iface" and "del-bond-iface".
 
 
 v2.9.0 - xx xxx 
diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index 415e14833249..e6c3f4596793 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -313,6 +313,66 @@ CHECK_IFACES([a], [a1], [a2], [a3])
 OVS_VSCTL_CLEANUP
 AT_CLEANUP
 
+AT_SETUP([add-bond-iface and del-bond-iface])
+AT_KEYWORDS([ovs-vsctl])
+OVS_VSCTL_SETUP
+
+# Create 2-interface bond.
+AT_CHECK([RUN_OVS_VSCTL(
+   [add-br a],
+   [add-bond a bond0 a1 a2])])
+CHECK_BRIDGES([a, a, 0])
+CHECK_PORTS([a], [bond0])
+CHECK_IFACES([a], [a1], [a2])
+
+# Add interface a3 to bond.
+AT_CHECK([RUN_OVS_VSCTL([add-bond-iface bond0 a3])])
+CHECK_BRIDGES([a, a, 0])
+CHECK_PORTS([a], [bond0])
+CHECK_IFACES([a], [a1], [a2], [a3])
+
+# Delete interface a2 from bond.
+AT_CHECK([RUN_OVS_VSCTL([del-bond-iface bond0 a2])])
+CHECK_BRIDGES([a, a, 0])
+CHECK_PORTS([a], [bond0])
+CHECK_IFACES([a], [a1], [a3])
+
+# Add interface a2 to bond.
+AT_CHECK([RUN_OVS_VSCTL([add-bond-iface bond0 a2])])
+CHECK_BRIDGES([a, a, 0])
+CHECK_PORTS([a], [bond0])
+CHECK_IFACES([a], [a1], [a2], [a3])
+
+# Delete interface a2 from bond.
+AT_CHECK([RUN_OVS_VSCTL([del-bond-iface a2])])
+CHECK_BRIDGES([a, a, 0])
+CHECK_PORTS([a], [bond0])
+CHECK_IFACES([a], [a1], [a3])
+
+AT_CHECK([RUN_OVS_VSCTL([--if-exists del-bond-iface bond0 a4])])
+AT_CHECK([RUN_OVS_VSCTL([del-bond-iface bond0 a4])], [1], [],
+  [ovs-vsctl: no interface named a4
+])
+AT_CHECK([RUN_OVS_VSCTL([del-bond-iface a4])], [1], [],
+  [ovs-vsctl: no interface named a4
+])
+AT_CHECK([RUN_OVS_VSCTL_TOGETHER([add-port a a4], [del-bond-iface bond0 a4])], 
[1], [],
+  [ovs-vsctl: port bond0 does not have an interface a4
+])
+AT_CHECK([RUN_OVS_VSCTL([--may-exist add-bond-iface bond0 a3])])
+AT_CHECK([RUN_OVS_VSCTL_TOGETHER([add-bond a bond1 b1 b2 b3], [--may-exist 
add-bond-iface bond1 a3])], [1], [],
+  [ovs-vsctl: "--may-exist add-bond-iface bond1 a3" but a3 is actually 
attached to port bond0
+])
+AT_CHECK([RUN_OVS_VSCTL([add-bond-iface bond0 a3])], [1], [],
+  [ovs-vsctl: cannot create an interface named a3 because an interface named 
a3 already exists on bridge a
+])
+AT_CHECK([RUN_OVS_VSCTL_TOGETHER([del-bond-iface a1], [del-bond-iface a3])], 
[1], [],
+  [ovs-vsctl: cannot delete last interface from port bond0
+])
+
+OVS_VSCTL_CLEANUP
+AT_CLEANUP
+
 AT_SETUP([add-br a b, add-port a a1, add-port b b1, del-port a a1])
 AT_KEYWORDS([ovs-vsctl])
 OVS_VSCTL_SETUP
diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
index f539af5486e4..34f41e4e88e6 100644
--- a/utilities/ovs-vsctl.8.in
+++ b/utilities/ovs-vsctl.8.in
@@ -274,26 +274,6 @@ Without \fB\-\-may\-exist\fR, attempting to create a port 
that exists
 is an error.  With \fB\-\-may\-exist\fR, this command does nothing if
 \fIport\fR already exists on \fIbridge\fR and is not a bonded port.
 .
-.IP "[\fB\-\-fake\-iface\fR] \fBadd\-bond \fIbridge port iface\fR\&... 
[\fIcolumn\fR[\fB:\fIkey\fR]\fR=\fIvalue\fR]\&...\fR"
-Creates on \fIbridge\fR a new port named \fIport\fR that bonds
-together the network devices given as each \fIiface\fR.  At least two
-interfaces must be named.  If the interfaces are DPDK enabled then
-the transaction will need to include operations to explicitly set the
-interface type to 'dpdk'.
-.IP
-Optional arguments set values of column in the Port record created by
-the command.  The syntax is the same as that for the \fBset\fR command
-(see \fBDatabase Commands\fR below).
-.IP
-With \fB\-\-fake\-iface\fR, a fake interface with the name \fIport\fR is
-created.  This should only be used for compatibility with legacy
-software that requires it.
-.IP
-Without \fB\-\-may\-exist\fR, attempting to create a port that exists
-is an error.  With \fB\-\-may\-exist\fR, this command does nothing if
-\fIport\fR already exists on \fIbridge\fR and bonds together exactly
-the specified interfaces.
-.
 .IP "[\fB\-\-if\-exists\fR] \fBdel\-port \fR[\fIbridge\fR] \fIport\fR"
 Deletes \fIport\fR.  If \fIbridge\fR is omitted, \fIport\fR is removed
 from whatever bridge contains it; if \fIbridge\fR is specified, it
@@ -318,6 +298,56 @@ no 

[ovs-dev] [PATCH 3/4] NEWS: Consolidate ovs-vswitchd sections and fix indentation.

[Ben Pfaff]

Signed-off-by: Ben Pfaff 
---
 NEWS | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/NEWS b/NEWS
index 8c360ba6cb9d..3b6ff74ad368 100644
--- a/NEWS
+++ b/NEWS
@@ -1,15 +1,14 @@
 Post-v2.9.0
 
-- ovs-vswitchd:
-  * New options --l7 and --l7-len to "ofproto/trace" command.
-   - ovs-ofctl:
- * ovs-ofctl now accepts and display table names in place of numbers.  By
-   default it always accepts names and in interactive use it displays them;
-   use --names or --no-names to override.  See ovs-ofctl(8) for details.
- ovs-vswitchd:
+ * New options --l7 and --l7-len to "ofproto/trace" command.
  * Previous versions gave OpenFlow tables default names of the form
"table#".  These are not helpful names for the purpose of accepting
and displaying table names, so now tables by default have no names.
+   - ovs-ofctl:
+ * ovs-ofctl now accepts and display table names in place of numbers.  By
+   default it always accepts names and in interactive use it displays them;
+   use --names or --no-names to override.  See ovs-ofctl(8) for details.
 
 
 v2.9.0 - xx xxx 
-- 
2.15.1

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

[ovs-dev] [PATCH 2/4] ovs-vsctl: Use default socket name in tests.

[Ben Pfaff]

By using the default socket name "db.sock", instead of "socket", we can
avoid passing --db=unix:socket to all the ovs-vsctl invocations, which is
kind of nice.

Signed-off-by: Ben Pfaff 
---
 tests/ovs-vsctl.at | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index 2c4c45d70947..415e14833249 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -4,7 +4,7 @@ dnl Creates an empty database in the current directory and then 
starts
 dnl an ovsdb-server on it for ovs-vsctl to connect to.
 m4_define([OVS_VSCTL_SETUP],
   [OVSDB_INIT([db])
-   AT_CHECK([ovsdb-server --detach --no-chdir --pidfile --remote=punix:socket 
db >/dev/null 2>&1], [0], [ignore], [ignore])
+   AT_CHECK([ovsdb-server --detach --no-chdir --pidfile --remote=punix:db.sock 
db >/dev/null 2>&1], [0], [ignore], [ignore])
on_exit 'kill `cat ovsdb-server.pid`'])
 
 dnl OVS_VSCTL_CLEANUP
@@ -16,17 +16,17 @@ dnl RUN_OVS_VSCTL(COMMAND, ...)
 dnl
 dnl Executes each ovs-vsctl COMMAND.
 m4_define([RUN_OVS_VSCTL],
-  [m4_foreach([command], [$@], [ovs-vsctl --no-wait -vreconnect:emer 
--db=unix:socket command
+  [m4_foreach([command], [$@], [ovs-vsctl --no-wait -vreconnect:emer command
 ])])
 m4_define([RUN_OVS_VSCTL_ONELINE],
-  [m4_foreach([command], [$@], [ovs-vsctl --no-wait -vreconnect:emer 
--db=unix:socket --oneline -- command
+  [m4_foreach([command], [$@], [ovs-vsctl --no-wait -vreconnect:emer --oneline 
-- command
 ])])
 
 dnl RUN_OVS_VSCTL_TOGETHER(COMMAND, ...)
 dnl
 dnl Executes each ovs-vsctl COMMAND in a single run of ovs-vsctl.
 m4_define([RUN_OVS_VSCTL_TOGETHER],
-  [ovs-vsctl --no-wait -vreconnect:emer --db=unix:socket --oneline dnl
+  [ovs-vsctl --no-wait -vreconnect:emer --oneline dnl
 m4_foreach([command], [$@], [ -- command])])
 
 dnl CHECK_BRIDGES([BRIDGE, PARENT, VLAN], ...)
@@ -875,7 +875,7 @@ AT_CHECK(
 
 ])
 m4_define([VSCTL_CHECK_FIND],
-  [AT_CHECK([echo `ovs-vsctl --bare --no-wait -vreconnect:emer 
--db=unix:socket -- --columns=name find bridge '$1' | sort`], [0], [$2
+  [AT_CHECK([echo `ovs-vsctl --bare --no-wait -vreconnect:emer -- 
--columns=name find bridge '$1' | sort`], [0], [$2
 ])])
 
 # Arithmetic relational operators without keys.
@@ -1078,19 +1078,19 @@ AT_SETUP([unreferenced record warnings])
 AT_KEYWORDS([ovs-vsctl])
 OVS_VSCTL_SETUP
 AT_CHECK(
-  [ovs-vsctl -vPATTERN:console:'%c|%p|%m' --no-wait -vreconnect:emer 
--db=unix:socket \
+  [ovs-vsctl -vPATTERN:console:'%c|%p|%m' --no-wait -vreconnect:emer \
  -- create Bridge name=br0 | uuidfilt],
   [0], [<0>
 ], [db_ctl_base|WARN|applying "create" command to table Bridge without --id 
option will have no effect
 ])
 AT_CHECK(
-  [ovs-vsctl -vPATTERN:console:'%c|%p|%m' --no-wait -vreconnect:emer 
--db=unix:socket \
+  [ovs-vsctl -vPATTERN:console:'%c|%p|%m' --no-wait -vreconnect:emer \
  -- --id=@br0 create Bridge name=br0 | uuidfilt],
   [0], [<0>
 ], [vsctl|WARN|row id "@br0" was created but no reference to it was inserted, 
so it will not actually appear in the database
 ])
 AT_CHECK(
-  [ovs-vsctl -vPATTERN:console:'%c|%p|%m' --no-wait -vreconnect:emer 
--db=unix:socket \
+  [ovs-vsctl -vPATTERN:console:'%c|%p|%m' --no-wait -vreconnect:emer \
  -- --id=@eth0_iface create Interface name=eth0 \
  -- --id=@eth0 create Port name=eth0 interfaces=@eth0_iface \
  -- --id=@m0 create Mirror name=m0 output_port=@eth0 \
-- 
2.15.1

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

Re: [ovs-dev] [PATCH v4 3/3] datapath-windows: Optimize conntrack lock implementation.

[Alin Serdean]

Acked-by: Alin Gabriel Serdean 

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-boun...@openvswitch.org] 
În numele Anand Kumar
Trimis: Monday, January 29, 2018 8:28 PM
Către: d...@openvswitch.org
Subiect: [ovs-dev] [PATCH v4 3/3] datapath-windows: Optimize conntrack lock 
implementation.

Currently, there is one global lock for conntrack module, which protects 
conntrack entries and conntrack table. All the NAT operations are performed 
holding this lock.

This becomes inefficient, as the number of conntrack entries grow.
With new implementation, we will have two PNDIS_RW_LOCK_EX locks in conntrack.

1. ovsCtBucketLock - one rw lock per bucket of the conntrack table, which is 
shared by all the ct entries that belong to the same bucket.
2. lock -  a rw lock in OVS_CT_ENTRY structure that protects the members of 
conntrack entry.

Also, OVS_CT_ENTRY structure will have a lock reference(bucketLockRef) to the 
corresponding OvsCtBucketLock of conntrack table.
We need this reference to retrieve ovsCtBucketLock from ct entry for delete 
operation.

Signed-off-by: Anand Kumar 
---
v1->v2: Address potential memory leak in conntrack initialization.
v2->v3: Fix invalid memory access after deleting ct entry.
v3->v4: Address warning "uninitialized memory"
---
 datapath-windows/ovsext/Conntrack-nat.c |   6 +
 datapath-windows/ovsext/Conntrack.c | 233 
 datapath-windows/ovsext/Conntrack.h |   3 +
 3 files changed, 157 insertions(+), 85 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack-nat.c 
b/datapath-windows/ovsext/Conntrack-nat.c
index 7975770..316c946 100644
--- a/datapath-windows/ovsext/Conntrack-nat.c
+++ b/datapath-windows/ovsext/Conntrack-nat.c
@@ -167,12 +167,16 @@ OvsNatPacket(OvsForwardingContext *ovsFwdCtx,  {
 UINT32 natFlag;
 const struct ct_endpoint* endpoint;
+LOCK_STATE_EX lockState;
+/* XXX: Move conntrack locks out of NAT after implementing lock in NAT. */
+NdisAcquireRWLockRead(entry->lock, , 0);
 /* When it is NAT, only entry->rev_key contains NATTED address;
When it is unNAT, only entry->key contains the UNNATTED address;*/
 const OVS_CT_KEY *ctKey = reverse ? >key : >rev_key;
 BOOLEAN isSrcNat;
 
 if (!(natAction & (NAT_ACTION_SRC | NAT_ACTION_DST))) {
+NdisReleaseRWLock(entry->lock, );
 return;
 }
 isSrcNat = (((natAction & NAT_ACTION_SRC) && !reverse) || @@ -202,6 +206,7 
@@ OvsNatPacket(OvsForwardingContext *ovsFwdCtx,
 }
 } else if (ctKey->dl_type == htons(ETH_TYPE_IPV6)){
 // XXX: IPv6 packet not supported yet.
+NdisReleaseRWLock(entry->lock, );
 return;
 }
 if (natAction & (NAT_ACTION_SRC_PORT | NAT_ACTION_DST_PORT)) { @@ -215,6 
+220,7 @@ OvsNatPacket(OvsForwardingContext *ovsFwdCtx,
 }
 }
 }
+NdisReleaseRWLock(entry->lock, );
 }
 
 
diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index 7d56a50..c90c000 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -31,7 +31,7 @@
 KSTART_ROUTINE OvsConntrackEntryCleaner;  static PLIST_ENTRY 
ovsConntrackTable;  static OVS_CT_THREAD_CTX ctThreadCtx; -static 
PNDIS_RW_LOCK_EX ovsConntrackLockObj;
+static PNDIS_RW_LOCK_EX *ovsCtBucketLock = NULL;
 static PNDIS_RW_LOCK_EX ovsCtNatLockObj;  extern POVS_SWITCH_CONTEXT 
gOvsSwitchContext;  static LONG ctTotalEntries; @@ -49,20 +49,14 @@ 
MapNlToCtTuple(POVS_MESSAGE msgIn, PNL_ATTR attr,  NTSTATUS  
OvsInitConntrack(POVS_SWITCH_CONTEXT context)  {
-NTSTATUS status;
+NTSTATUS status = STATUS_SUCCESS;
 HANDLE threadHandle = NULL;
 ctTotalEntries = 0;
+UINT32 numBucketLocks = CT_HASH_TABLE_SIZE;
 
 /* Init the sync-lock */
-ovsConntrackLockObj = NdisAllocateRWLock(context->NdisFilterHandle);
-if (ovsConntrackLockObj == NULL) {
-return STATUS_INSUFFICIENT_RESOURCES;
-}
-
 ovsCtNatLockObj = NdisAllocateRWLock(context->NdisFilterHandle);
 if (ovsCtNatLockObj == NULL) {
-NdisFreeRWLock(ovsConntrackLockObj);
-ovsConntrackLockObj = NULL;
 return STATUS_INSUFFICIENT_RESOURCES;
 }
 
@@ -71,15 +65,27 @@ OvsInitConntrack(POVS_SWITCH_CONTEXT context)
  * CT_HASH_TABLE_SIZE,
  OVS_CT_POOL_TAG);
 if (ovsConntrackTable == NULL) {
-NdisFreeRWLock(ovsConntrackLockObj);
-ovsConntrackLockObj = NULL;
 NdisFreeRWLock(ovsCtNatLockObj);
 ovsCtNatLockObj = NULL;
 return STATUS_INSUFFICIENT_RESOURCES;
 }
 
-for (int i = 0; i < CT_HASH_TABLE_SIZE; i++) {
+ovsCtBucketLock = OvsAllocateMemoryWithTag(sizeof(PNDIS_RW_LOCK_EX)
+   * CT_HASH_TABLE_SIZE,
+   OVS_CT_POOL_TAG);
+if 

[ovs-dev] Infracciones y sanciones

[Outsourcing y sus Consecuencias Fiscales]

 
OUTSOURCING Y SUS CONSECUENCIAS FISCALES
Febrero 07 - webinar Interactivo

Este webinar está diseñado para las empresas que prestan servicios de 
outsourcing de personal o las interesadas en contratar personal bajo la figura 
del outsourcing.

TEMARIO:

El Outsourcing en términos de la Ley Federal del Trabajo.
Tratamiento fiscal del outsourcing en impuesto sobre la renta.
Tratamiento fiscal del Outsourcing en la Ley del Seguro Social .
Tratamiento fiscal del Outsourcing en la Ley de INFONAVIT.
Tratamiento fiscal del Outsourcing en Impuesto sobre nóminas 
 
Temario e Inscripciones:

Respondiendo por este medio "Outsourcing"+TELÉFONO + NOMBRE o marcando al:

045 + 5515546630  



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

Re: [ovs-dev] [PATCH] ovn-controller: Document southbound database use and graceful termination.

[Han Zhou]

On Fri, Feb 2, 2018 at 9:44 AM, Ben Pfaff  wrote:
>
> A lot of people seem to think that "kill" gracefully terminates
> ovn-controller, but it doesn't, so this documentation at least provides
> something to point to.
>
> Signed-off-by: Ben Pfaff 
> ---
>  ovn/controller/ovn-controller.8.xml | 47
+
>  1 file changed, 47 insertions(+)
>
> diff --git a/ovn/controller/ovn-controller.8.xml
b/ovn/controller/ovn-controller.8.xml
> index 0df59acd3c8b..96a58ddf358d 100644
> --- a/ovn/controller/ovn-controller.8.xml
> +++ b/ovn/controller/ovn-controller.8.xml
> @@ -304,6 +304,53 @@
>
>  
>
> +OVN Southbound Database Usage
> +
> +
> +  ovn-controller reads from much of the
> +  OVN_Southbound database to guide its operation.
> +  ovn-controller also writes to the following tables:
> +
> +
> +
> +  Chassis
> +  
> +Upon startup, ovn-controller creates a row in this
table
> +to represent its own chassis.  Upon graceful termination, e.g.
with
> +ovs-appctl -t ovn-controller exit (but not
> +SIGTERM), ovn-controller removes its
row.
> +  
> +
> +  Encap
> +  
> +Upon startup, ovn-controller creates a row or rows
in this
> +table that represent the tunnel encapsulations by which its
chassis can
> +be reached, and points its Chassis row to them.
Upon
> +graceful termination, ovn-controller removes these
rows.
> +  
> +
> +  Port_Binding
> +  
> +At runtime, ovn-controller sets the
chassis
> +columns of ports that are resident on its chassis to point to its
> +Chassis row, and, conversely, clears the
> +chassis column of ports that point to its
> +Chassis row but are no longer resident on its
chassis.
> +The chassis column has a weak reference type, so
when
> +ovn-controller gracefully exits and removes its
> +Chassis row, the database server automatically
clears any
> +remaining references to that row.
> +  
> +
> +  MAC_Binding
> +  
> +At runtime, ovn-controller updates the
> +MAC_Binding table as instructed by
put_arp
> +and put_nd logical actions.  These changes persist
beyond
> +the lifetime of ovn-controller.
> +  
> +
> +
>  Runtime Management Commands
>  
>ovs-appctl can send commands to a running
> --
> 2.15.1
>

Thanks Ben!

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

Re: [ovs-dev] [PATCH 18/20] datapath: use ktime_get_ts64() instead of ktime_get_ts()

[Gregory Rose]

On 2/2/2018 12:13 PM, Arnd Bergmann wrote:

On Fri, Feb 2, 2018 at 7:31 PM, Gregory Rose  wrote:

On 2/2/2018 10:18 AM, Pravin Shelar wrote:

On Tue, Jan 30, 2018 at 4:40 PM, Gregory Rose 

This is done in compat code, can you move it to respective header file?


Yes - my own preference is to keep these sorts of things close to where
they're used but
I suppose there is a good chance we'll use ktime_get_ts64 elsewhere in the
future.  So
that's fine by me.

You could decide to use ktime_get_ns() divided by NSEC_PER_MSEC
instead of getting that number from ktime_get_ts64(), that would be
more portable, though a little bit slower on 32-bit architectures.

Arnd


That's an interesting suggestion Arnd.  I'm generally opposed to divide 
operations when I can avoid
them and ktime_get_ts64() avoids that SFAICT.  We do still support 32 
bit systems as well so I think I'll
just go ahead and stick with ktime_get_ts64.  Also, sometimes I have to 
compare our OOT datapath
kernel code with upstream and when I stick to the same coding as much as 
possible it helps me out.


I'll move it over into our compat layer code as suggested by Pravin but 
thanks for providing helpful

suggestion.

Regards,

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

Re: [ovs-dev] [PATCH] datapath-windows: Add trace level logs in conntrack for invalid ct state.

[aserdean]

-Mesaj original-
De la: Anand Kumar [mailto:kumaran...@vmware.com] 
Trimis: Saturday, February 3, 2018 12:28 AM
Către: Alin Serdean ; d...@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH] datapath-windows: Add trace level logs in 
conntrack for invalid ct state.

Hi Alin,

Thanks for the quick review. I will expand log messages to “Invalid XXX packet 
detected”.
I’m not sure if I follow your comment about “s/syn/SYN/g”. 

Do you want me to change it to Uppercase?
[Alin Serdean] Yes please .

Thanks,
Anand Kumar

On 2/2/18, 2:06 PM, "Alin Serdean"  wrote:

Looks good just a small nit:
s/syn/SYN/g
s/ack/ACK/g

Also I would prefer if you drop the text `Invalid!`. Either just remove it 
or expand it, i.e.:
"Invalid! ICMPhdr cannot be NULL" => "Invalid ICMP packet detected the 
header cannot be NULL"

Thanks,
Alin.

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org 
[mailto:ovs-dev-boun...@openvswitch.org] În numele Anand Kumar
Trimis: Friday, February 2, 2018 11:19 PM
Către: d...@openvswitch.org
Subiect: [ovs-dev] [PATCH] datapath-windows: Add trace level logs in 
conntrack for invalid ct state.

Signed-off-by: Anand Kumar 
---
 datapath-windows/ovsext/Conntrack-icmp.c | 1 +  
datapath-windows/ovsext/Conntrack-tcp.c  | 4 
 datapath-windows/ovsext/Conntrack.c  | 4 
 3 files changed, 9 insertions(+)

diff --git a/datapath-windows/ovsext/Conntrack-icmp.c 
b/datapath-windows/ovsext/Conntrack-icmp.c
index 4da0665..d86feed 100644
--- a/datapath-windows/ovsext/Conntrack-icmp.c
+++ b/datapath-windows/ovsext/Conntrack-icmp.c
@@ -61,6 +61,7 @@ BOOLEAN
 OvsConntrackValidateIcmpPacket(const ICMPHdr *icmp)  {
 if (!icmp) {
+OVS_LOG_TRACE("Invalid! ICMPhdr cannot be NULL");
 return FALSE;
 }
 
diff --git a/datapath-windows/ovsext/Conntrack-tcp.c 
b/datapath-windows/ovsext/Conntrack-tcp.c
index f8e85a2..65eaac5 100644
--- a/datapath-windows/ovsext/Conntrack-tcp.c
+++ b/datapath-windows/ovsext/Conntrack-tcp.c
@@ -444,12 +444,14 @@ BOOLEAN
 OvsConntrackValidateTcpPacket(const TCPHdr *tcp)  {
 if (!tcp) {
+OVS_LOG_TRACE("Invalid! TCPHdr cannot be NULL");
 return FALSE;
 }
 
 UINT16 tcp_flags = ntohs(tcp->flags);
 
 if (OvsCtInvalidTcpFlags(tcp_flags)) {
+OVS_LOG_TRACE("Invalid! tcp_flags %hu", tcp_flags);
 return FALSE;
 }
 
@@ -457,6 +459,8 @@ OvsConntrackValidateTcpPacket(const TCPHdr *tcp)
  * totally new connections (syn) or already established, not partially
  * open (syn+ack). */
 if ((tcp_flags & TCP_SYN) && (tcp_flags & TCP_ACK)) {
+OVS_LOG_TRACE("Invalid! syn+ack flags not allowed, tcp_flags %hu",
+  tcp_flags);
 return FALSE;
 }
 
diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index 43c9dd3..7e413c6 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -317,6 +317,9 @@ OvsCtEntryCreate(OvsForwardingContext *fwdCtx,
 const ICMPHdr *icmp;
 icmp = OvsGetIcmp(curNbl, l4Offset, );
 if (!OvsConntrackValidateIcmpPacket(icmp)) {
+if(icmp) {
+OVS_LOG_TRACE("Invalid! icmp->type %u", icmp->type);
+}
 state = OVS_CS_F_INVALID;
 break;
 }
@@ -334,6 +337,7 @@ OvsCtEntryCreate(OvsForwardingContext *fwdCtx,
 break;
 }
 default:
+OVS_LOG_TRACE("Invalid! Not supported protocol, ipProto %u", 
+ ipProto);
 state = OVS_CS_F_INVALID;
 break;
 }
--
2.9.3.windows.1

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

https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwIFBA=uilaK90D4TOVoH58JNXRgQ=Q5z9tBe-nAOpE7LIHSPV8uy5-437agMXvkeHHMkR8Us=yuxpORhg-xij1o9VvWANs9QOyywwfr7YO_EDT-QvqQ4=ainXigtUgyesCYog0X4639gDSFv1mrY4OTsctKNaH0M=



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

Re: [ovs-dev] [PATCH v2] datapath-windows: Add trace level logs in conntrack for invalid ct state.

[aserdean]

Much better 

Acked-by: Alin Gabriel Serdean 

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-boun...@openvswitch.org] 
În numele Anand Kumar
Trimis: Saturday, February 3, 2018 12:43 AM
Către: d...@openvswitch.org
Subiect: [ovs-dev] [PATCH v2] datapath-windows: Add trace level logs in 
conntrack for invalid ct state.

Signed-off-by: Anand Kumar 
---
 datapath-windows/ovsext/Conntrack-icmp.c | 1 +  
datapath-windows/ovsext/Conntrack-tcp.c  | 4 
 datapath-windows/ovsext/Conntrack.c  | 6 ++
 3 files changed, 11 insertions(+)

diff --git a/datapath-windows/ovsext/Conntrack-icmp.c 
b/datapath-windows/ovsext/Conntrack-icmp.c
index 4da0665..28fe2bf 100644
--- a/datapath-windows/ovsext/Conntrack-icmp.c
+++ b/datapath-windows/ovsext/Conntrack-icmp.c
@@ -61,6 +61,7 @@ BOOLEAN
 OvsConntrackValidateIcmpPacket(const ICMPHdr *icmp)  {
 if (!icmp) {
+OVS_LOG_TRACE("Invalid ICMP packet detected, header cannot be 
+ NULL");
 return FALSE;
 }
 
diff --git a/datapath-windows/ovsext/Conntrack-tcp.c 
b/datapath-windows/ovsext/Conntrack-tcp.c
index f8e85a2..8cbab24 100644
--- a/datapath-windows/ovsext/Conntrack-tcp.c
+++ b/datapath-windows/ovsext/Conntrack-tcp.c
@@ -444,12 +444,14 @@ BOOLEAN
 OvsConntrackValidateTcpPacket(const TCPHdr *tcp)  {
 if (!tcp) {
+OVS_LOG_TRACE("Invalid TCP packet detected, header cannot be 
+ NULL");
 return FALSE;
 }
 
 UINT16 tcp_flags = ntohs(tcp->flags);
 
 if (OvsCtInvalidTcpFlags(tcp_flags)) {
+OVS_LOG_TRACE("Invalid TCP packet detected, tcp_flags %hu", 
+ tcp_flags);
 return FALSE;
 }
 
@@ -457,6 +459,8 @@ OvsConntrackValidateTcpPacket(const TCPHdr *tcp)
  * totally new connections (syn) or already established, not partially
  * open (syn+ack). */
 if ((tcp_flags & TCP_SYN) && (tcp_flags & TCP_ACK)) {
+OVS_LOG_TRACE("Invalid TCP packet detected, SYN+ACK flags not allowed,"
+  "tcp_flags %hu", tcp_flags);
 return FALSE;
 }
 
diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index 43c9dd3..678bedb 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -317,6 +317,10 @@ OvsCtEntryCreate(OvsForwardingContext *fwdCtx,
 const ICMPHdr *icmp;
 icmp = OvsGetIcmp(curNbl, l4Offset, );
 if (!OvsConntrackValidateIcmpPacket(icmp)) {
+if(icmp) {
+OVS_LOG_TRACE("Invalid ICMP packet detected, icmp->type %u",
+  icmp->type);
+}
 state = OVS_CS_F_INVALID;
 break;
 }
@@ -334,6 +338,8 @@ OvsCtEntryCreate(OvsForwardingContext *fwdCtx,
 break;
 }
 default:
+OVS_LOG_TRACE("Invalid packet detected, protocol not supported"
+  " ipProto %u", ipProto);
 state = OVS_CS_F_INVALID;
 break;
 }
--
2.9.3.windows.1

___
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] ovn-controller: processing matches precedes processing action in logical flow.

[Ben Pfaff]

On Fri, Feb 02, 2018 at 10:50:29AM +0800, Guoshuai Li wrote:
> This is to fix such a problem:
> When the match field has "is_chassis_resident", and the chassis has no 
> resident,
> and the action has meter or group, the group/meter ID is assigned.
> 
> I hope to parse match field with the first, if there is no matches field,
> we do not need to deal with the action field.
> 
> Signed-off-by: Guoshuai Li 

Thank you for noticing the problem and fixing it.  I applied this to
master.  I edited the commit message and I removed the code that logged
a warning for this case; I think that this is a normal situation that
doesn't warrant logging.

Thanks,

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

[ovs-dev] Calendario del mes de Febrero

[Home Office + 9 Temas]

Buenos días

Le hacemos llegar la programación de  Febrero para que elija los temas que 
necesita usted y su equipo de trabajo.

FEBRERO

(3 hrs.) Cómo Implementar el Home Office en su Empresa – 01 de Febrero
Actualización para Compradores Profesionales – Cómo ser el Mejor – 08 de Febrero
Reclutamiento, Entrevistas y Selección de Personal – 09 de Febrero
Compensación e Incentivos para la Fuerza de Ventas – 14 de Febrero
Las 5’s de la Calidad Total Japonesa – 15 de Febrero 
(3 hrs.) Neuro-ventas – Negociando a Través de las Emociones – 16 de Febrero
Sistemas Eficaces de Control a través de Auditorías Administrativas – 22 de 
Febrero
Cómo Despedir Empleados en el Marco de la Ley – 22 de Febrero 
Dirección Avanzada de Crédito y Cobranza – 23 de Febrero
(3 hrs.) LinkedIn para su Empresa – 28 de Febrero


Si desea ampliar la información de uno o varios temas, responda a este correo 
con el Nombre del evento de su interés, Junto con los Siguientes Datos: Nombre, 
Teléfono, Empresa.
o comuníquese al 0180021293 93



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

Re: [ovs-dev] [PATCH v2] datapath-windows: Add trace level logs in conntrack for invalid ct state.

[Alin Serdean]

Applied on master. Ty!

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-boun...@openvswitch.org] 
În numele aserd...@ovn.org
Trimis: Saturday, February 3, 2018 12:48 AM
Către: 'Anand Kumar' ; d...@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH v2] datapath-windows: Add trace level logs in 
conntrack for invalid ct state.

Much better 

Acked-by: Alin Gabriel Serdean 

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-boun...@openvswitch.org] 
În numele Anand Kumar
Trimis: Saturday, February 3, 2018 12:43 AM
Către: d...@openvswitch.org
Subiect: [ovs-dev] [PATCH v2] datapath-windows: Add trace level logs in 
conntrack for invalid ct state.

Signed-off-by: Anand Kumar 
---
 datapath-windows/ovsext/Conntrack-icmp.c | 1 +  
datapath-windows/ovsext/Conntrack-tcp.c  | 4 
 datapath-windows/ovsext/Conntrack.c  | 6 ++
 3 files changed, 11 insertions(+)

diff --git a/datapath-windows/ovsext/Conntrack-icmp.c 
b/datapath-windows/ovsext/Conntrack-icmp.c
index 4da0665..28fe2bf 100644
--- a/datapath-windows/ovsext/Conntrack-icmp.c
+++ b/datapath-windows/ovsext/Conntrack-icmp.c
@@ -61,6 +61,7 @@ BOOLEAN
 OvsConntrackValidateIcmpPacket(const ICMPHdr *icmp)  {
 if (!icmp) {
+OVS_LOG_TRACE("Invalid ICMP packet detected, header cannot be 
+ NULL");
 return FALSE;
 }
 
diff --git a/datapath-windows/ovsext/Conntrack-tcp.c 
b/datapath-windows/ovsext/Conntrack-tcp.c
index f8e85a2..8cbab24 100644
--- a/datapath-windows/ovsext/Conntrack-tcp.c
+++ b/datapath-windows/ovsext/Conntrack-tcp.c
@@ -444,12 +444,14 @@ BOOLEAN
 OvsConntrackValidateTcpPacket(const TCPHdr *tcp)  {
 if (!tcp) {
+OVS_LOG_TRACE("Invalid TCP packet detected, header cannot be 
+ NULL");
 return FALSE;
 }
 
 UINT16 tcp_flags = ntohs(tcp->flags);
 
 if (OvsCtInvalidTcpFlags(tcp_flags)) {
+OVS_LOG_TRACE("Invalid TCP packet detected, tcp_flags %hu", 
+ tcp_flags);
 return FALSE;
 }
 
@@ -457,6 +459,8 @@ OvsConntrackValidateTcpPacket(const TCPHdr *tcp)
  * totally new connections (syn) or already established, not partially
  * open (syn+ack). */
 if ((tcp_flags & TCP_SYN) && (tcp_flags & TCP_ACK)) {
+OVS_LOG_TRACE("Invalid TCP packet detected, SYN+ACK flags not allowed,"
+  "tcp_flags %hu", tcp_flags);
 return FALSE;
 }
 
diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index 43c9dd3..678bedb 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -317,6 +317,10 @@ OvsCtEntryCreate(OvsForwardingContext *fwdCtx,
 const ICMPHdr *icmp;
 icmp = OvsGetIcmp(curNbl, l4Offset, );
 if (!OvsConntrackValidateIcmpPacket(icmp)) {
+if(icmp) {
+OVS_LOG_TRACE("Invalid ICMP packet detected, icmp->type %u",
+  icmp->type);
+}
 state = OVS_CS_F_INVALID;
 break;
 }
@@ -334,6 +338,8 @@ OvsCtEntryCreate(OvsForwardingContext *fwdCtx,
 break;
 }
 default:
+OVS_LOG_TRACE("Invalid packet detected, protocol not supported"
+  " ipProto %u", ipProto);
 state = OVS_CS_F_INVALID;
 break;
 }
--
2.9.3.windows.1

___
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
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] util: Use lookup table to optimize hexit_value().

[Ben Pfaff]

Daniel Alvarez Sanchez reported a significant overall speedup in ovn-northd
due to a similar patch.

Reported-by: Daniel Alvarez Sanchez 
Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-discuss/2018-February/046120.html
Signed-off-by: Ben Pfaff 
---
 lib/util.c | 39 +--
 1 file changed, 13 insertions(+), 26 deletions(-)

diff --git a/lib/util.c b/lib/util.c
index a4d22df0c359..2e7765e41f03 100644
--- a/lib/util.c
+++ b/lib/util.c
@@ -843,32 +843,19 @@ str_to_double(const char *s, double *d)
 int
 hexit_value(int c)
 {
-switch (c) {
-case '0': case '1': case '2': case '3': case '4':
-case '5': case '6': case '7': case '8': case '9':
-return c - '0';
-
-case 'a': case 'A':
-return 0xa;
-
-case 'b': case 'B':
-return 0xb;
-
-case 'c': case 'C':
-return 0xc;
-
-case 'd': case 'D':
-return 0xd;
-
-case 'e': case 'E':
-return 0xe;
-
-case 'f': case 'F':
-return 0xf;
-
-default:
-return -1;
-}
+static const signed char tbl[UCHAR_MAX + 1] = {
+#define TBL(x)  \
+(  x >= '0' && x <= '9' ? x - '0'   \
+ : x >= 'a' && x <= 'f' ? x - 'a' + 0xa \
+ : x >= 'A' && x <= 'F' ? x - 'A' + 0xa \
+ : -1)
+#define TBL0(x)  TBL(x),  TBL((x) + 1),   TBL((x) + 2),   TBL((x) + 3)
+#define TBL1(x) TBL0(x), TBL0((x) + 4),  TBL0((x) + 8),  TBL0((x) + 12)
+#define TBL2(x) TBL1(x), TBL1((x) + 16), TBL1((x) + 32), TBL1((x) + 48)
+TBL2(0), TBL2(64), TBL2(128), TBL2(192)
+};
+
+return tbl[c];
 }
 
 /* Returns the integer value of the 'n' hexadecimal digits starting at 's', or
-- 
2.15.1

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

Re: [ovs-dev] [PATCH] datapath-windows: Add trace level logs in conntrack for invalid ct state.

[Anand Kumar]

Hi Alin,

Thanks for the quick review. I will expand log messages to “Invalid XXX packet 
detected”.
I’m not sure if I follow your comment about “s/syn/SYN/g”. 

Do you want me to change it to Uppercase?

Thanks,
Anand Kumar

On 2/2/18, 2:06 PM, "Alin Serdean"  wrote:

Looks good just a small nit:
s/syn/SYN/g
s/ack/ACK/g

Also I would prefer if you drop the text `Invalid!`. Either just remove it 
or expand it, i.e.:
"Invalid! ICMPhdr cannot be NULL" => "Invalid ICMP packet detected the 
header cannot be NULL"

Thanks,
Alin.

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org 
[mailto:ovs-dev-boun...@openvswitch.org] În numele Anand Kumar
Trimis: Friday, February 2, 2018 11:19 PM
Către: d...@openvswitch.org
Subiect: [ovs-dev] [PATCH] datapath-windows: Add trace level logs in 
conntrack for invalid ct state.

Signed-off-by: Anand Kumar 
---
 datapath-windows/ovsext/Conntrack-icmp.c | 1 +  
datapath-windows/ovsext/Conntrack-tcp.c  | 4 
 datapath-windows/ovsext/Conntrack.c  | 4 
 3 files changed, 9 insertions(+)

diff --git a/datapath-windows/ovsext/Conntrack-icmp.c 
b/datapath-windows/ovsext/Conntrack-icmp.c
index 4da0665..d86feed 100644
--- a/datapath-windows/ovsext/Conntrack-icmp.c
+++ b/datapath-windows/ovsext/Conntrack-icmp.c
@@ -61,6 +61,7 @@ BOOLEAN
 OvsConntrackValidateIcmpPacket(const ICMPHdr *icmp)  {
 if (!icmp) {
+OVS_LOG_TRACE("Invalid! ICMPhdr cannot be NULL");
 return FALSE;
 }
 
diff --git a/datapath-windows/ovsext/Conntrack-tcp.c 
b/datapath-windows/ovsext/Conntrack-tcp.c
index f8e85a2..65eaac5 100644
--- a/datapath-windows/ovsext/Conntrack-tcp.c
+++ b/datapath-windows/ovsext/Conntrack-tcp.c
@@ -444,12 +444,14 @@ BOOLEAN
 OvsConntrackValidateTcpPacket(const TCPHdr *tcp)  {
 if (!tcp) {
+OVS_LOG_TRACE("Invalid! TCPHdr cannot be NULL");
 return FALSE;
 }
 
 UINT16 tcp_flags = ntohs(tcp->flags);
 
 if (OvsCtInvalidTcpFlags(tcp_flags)) {
+OVS_LOG_TRACE("Invalid! tcp_flags %hu", tcp_flags);
 return FALSE;
 }
 
@@ -457,6 +459,8 @@ OvsConntrackValidateTcpPacket(const TCPHdr *tcp)
  * totally new connections (syn) or already established, not partially
  * open (syn+ack). */
 if ((tcp_flags & TCP_SYN) && (tcp_flags & TCP_ACK)) {
+OVS_LOG_TRACE("Invalid! syn+ack flags not allowed, tcp_flags %hu",
+  tcp_flags);
 return FALSE;
 }
 
diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index 43c9dd3..7e413c6 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -317,6 +317,9 @@ OvsCtEntryCreate(OvsForwardingContext *fwdCtx,
 const ICMPHdr *icmp;
 icmp = OvsGetIcmp(curNbl, l4Offset, );
 if (!OvsConntrackValidateIcmpPacket(icmp)) {
+if(icmp) {
+OVS_LOG_TRACE("Invalid! icmp->type %u", icmp->type);
+}
 state = OVS_CS_F_INVALID;
 break;
 }
@@ -334,6 +337,7 @@ OvsCtEntryCreate(OvsForwardingContext *fwdCtx,
 break;
 }
 default:
+OVS_LOG_TRACE("Invalid! Not supported protocol, ipProto %u", 
+ ipProto);
 state = OVS_CS_F_INVALID;
 break;
 }
--
2.9.3.windows.1

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

https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwIFBA=uilaK90D4TOVoH58JNXRgQ=Q5z9tBe-nAOpE7LIHSPV8uy5-437agMXvkeHHMkR8Us=yuxpORhg-xij1o9VvWANs9QOyywwfr7YO_EDT-QvqQ4=ainXigtUgyesCYog0X4639gDSFv1mrY4OTsctKNaH0M=


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

Re: [ovs-dev] [PATCH V3] rhel: Fix support for root user using DPDK

[Ben Pfaff]

Well, is it ever useful to be able to drop unneeded capabilities while
retaining the same uid/gid?  It certainly sounds like a reasonable thing
to want to do.  I'm reluctant to apply this without at least considering
that possibility.

On Fri, Feb 02, 2018 at 12:30:50AM -0200, Marcos Felipe Schwarz wrote:
> Is there any reason why they couldn't be the same?
> 
> One reason in favor is that the option for running ovs as a non-root user
> can't be deactivated correctly by the means in the documentation [1]. It
> states that setting OVS_USER_ID to 'root:root', or commenting the variable
> out in /etc/sysconfig/openvswitch should revert the behavior back to normal.
> The patch above enforces the expected behaviour without requiring
> tinkering both
> ovs-vswitchd and ovsdb-server systemd scripts to remove the "--user
> OVS_USER_ID" parameter that is hardcoded.
> 
> [1] Non-root User Support:
> https://github.com/openvswitch/ovs/blob/master/rhl/README.RHEL.rst
> 
> On Thu, Feb 1, 2018 at 10:23 PM, Ben Pfaff  wrote:
> 
> > OK, I understand now that the goal is that passing "--user $UID" should
> > not call setuid() or setgid().  In that case, though, why pass the
> > --user option at all?  I believe that, with this patch, "--user $UID"
> > and "" yield equivalent behavior.
> >
> > Thanks,
> >
> > Ben.
> >
> > On Thu, Feb 01, 2018 at 09:34:12PM -0200, Marcos Felipe Schwarz wrote:
> > > Hi Ben,
> > >
> > > I agree that the purpose of the canges was not cleared in the patch. The
> > > full discussion is at
> > > https://mail.openvswitch.org/pipermail/ovs-discuss/2018-
> > January/046061.html.
> > > But in a nutshell, this change is a fancy way to avoid the actual
> > problem,
> > > that's why it's not intuitive. The actual problem is the current method
> > to
> > > change the ovs user at (daemon_become_new_user_linux), drops the
> > > CAP_SYS_ADMIN capability, even when you are "changing" from root to
> > itself,
> > > which occurs in rhel based distros when you use OVS_USER="root:toot". My
> > > PoC solution [2] was to force the CAP_SYS_ADMIN during the user change,
> > but
> > > this would give more permission to openvswitch process than necessary on
> > > most of the situations, for instance when DPDK and UIO are not required.
> > > The fix in the current patch was proposed by Aaron [3], which involves to
> > > avoid changing the user when it is already the desired user (and group),
> > > this fixes the problem without the downsides of the initial solution and
> > > avoids unnecessary calls.
> > >
> > > [2]
> > > https://mail.openvswitch.org/pipermail/ovs-discuss/2018-
> > January/045959.html
> > > [3]
> > > https://mail.openvswitch.org/pipermail/ovs-discuss/2018-
> > January/045973.html
> > > Regards,
> > >
> > > Marcos Schwarz
> > >
> > > On Thu, Feb 1, 2018 at 7:27 PM, Ben Pfaff  wrote:
> > >
> > > > On Thu, Feb 01, 2018 at 03:42:03PM -0200, Marcos Felipe Schwarz wrote:
> > > > >  Since 2.8.0 OVS runs as non-root user on rhel distros, but the
> > current
> > > > > implementation breaks the ability to run as root with DPDK and as a
> > > > > consequence there is no way possible to use UIO drivers on kernel
> > 4.0 and
> > > > > newer [1, 2].
> > > > > [1] http://dpdk.org/browse/dpdk/commit/?id=cdc242f260e766
> > > > > bd95a658b5e0686a62ec04f5b0
> > > > > [2] https://www.kernel.org/doc/Documentation/vm/pagemap.txt
> > > > >
> > > > > Fixes: e3e738a3d058 ("redhat: allow dpdk to also run as non-root
> > user")
> > > > > Signed-off-by: Marcos Schwarz 
> > > > > Acked-by: Aaron Conole 
> > > >
> > > > Thanks for the patch.
> > > >
> > > > I understand from the commit message the ultimate goal of this patch.
> > I
> > > > don't yet understand what change it is actually making.  In particular,
> > > > what is the purpose of the following change; what does it do?
> > > >
> > > > > --- a/lib/daemon-unix.c
> > > > > +++ b/lib/daemon-unix.c
> > > > > @@ -1047,5 +1047,6 @@ daemon_set_new_user(const char *user_spec)
> > > > >  }
> > > > >  }
> > > > >
> > > > > -switch_user = true;
> > > > > +if (!uid_verify(uid) || !gid_verify(gid))
> > > > > +switch_user = true;
> > > > >  }
> > > >
> > > > Thanks,
> > > >
> > > > Ben.
> > > >
> >
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] Makefile.am: Use correct path separator for Windows

[Shashank Ram]

Signed-off-by: Shashank Ram 
---
 Makefile.am | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 5988c02..d397f65 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -148,7 +148,7 @@ ro_shell = printf '\043 Generated automatically -- do not 
modify!-*- buffer-
 
 SUFFIXES += .in
 .in:
-   $(AM_V_GEN)PYTHONPATH=$$PYTHONPATH:$(srcdir)/python $(PYTHON) 
$(srcdir)/build-aux/soexpand.py -I$(srcdir) < $< | \
+   $(AM_V_GEN)PYTHONPATH=$$PYTHONPATH$(psep)$(srcdir)/python $(PYTHON) 
$(srcdir)/build-aux/soexpand.py -I$(srcdir) < $< | \
  $(PYTHON) $(srcdir)/build-aux/dpdkstrip.py $(DPDKSTRIP_FLAGS) | \
  sed \
-e 's,[@]PKIDIR[@],$(PKIDIR),g' \
@@ -398,7 +398,7 @@ CLEANFILES += flake8-check
 
 include $(srcdir)/manpages.mk
 $(srcdir)/manpages.mk: $(MAN_ROOTS) build-aux/sodepends.py 
python/build/soutil.py
-   @PYTHONPATH=$$PYTHONPATH:$(srcdir)/python $(PYTHON) 
$(srcdir)/build-aux/sodepends.py -I. -I$(srcdir) $(MAN_ROOTS) >$(@F).tmp
+   @PYTHONPATH=$$PYTHONPATH$(psep)$(srcdir)/python $(PYTHON) 
$(srcdir)/build-aux/sodepends.py -I. -I$(srcdir) $(MAN_ROOTS) >$(@F).tmp
@if cmp -s $(@F).tmp $@; then \
  touch $@; \
  rm -f $(@F).tmp; \
-- 
2.9.3.windows.2

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

Re: [ovs-dev] [PATCH] util: Use lookup table to optimize hexit_value().

[Yifeng Sun]

Looks good to me, thanks.

Reviewed-by: Yifeng Sun 

On Fri, Feb 2, 2018 at 3:16 PM, Ben Pfaff  wrote:

> Daniel Alvarez Sanchez reported a significant overall speedup in ovn-northd
> due to a similar patch.
>
> Reported-by: Daniel Alvarez Sanchez 
> Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-
> February/046120.html
> Signed-off-by: Ben Pfaff 
> ---
>  lib/util.c | 39 +--
>  1 file changed, 13 insertions(+), 26 deletions(-)
>
> diff --git a/lib/util.c b/lib/util.c
> index a4d22df0c359..2e7765e41f03 100644
> --- a/lib/util.c
> +++ b/lib/util.c
> @@ -843,32 +843,19 @@ str_to_double(const char *s, double *d)
>  int
>  hexit_value(int c)
>  {
> -switch (c) {
> -case '0': case '1': case '2': case '3': case '4':
> -case '5': case '6': case '7': case '8': case '9':
> -return c - '0';
> -
> -case 'a': case 'A':
> -return 0xa;
> -
> -case 'b': case 'B':
> -return 0xb;
> -
> -case 'c': case 'C':
> -return 0xc;
> -
> -case 'd': case 'D':
> -return 0xd;
> -
> -case 'e': case 'E':
> -return 0xe;
> -
> -case 'f': case 'F':
> -return 0xf;
> -
> -default:
> -return -1;
> -}
> +static const signed char tbl[UCHAR_MAX + 1] = {
> +#define TBL(x)  \
> +(  x >= '0' && x <= '9' ? x - '0'   \
> + : x >= 'a' && x <= 'f' ? x - 'a' + 0xa \
> + : x >= 'A' && x <= 'F' ? x - 'A' + 0xa \
> + : -1)
> +#define TBL0(x)  TBL(x),  TBL((x) + 1),   TBL((x) + 2),   TBL((x) + 3)
> +#define TBL1(x) TBL0(x), TBL0((x) + 4),  TBL0((x) + 8),  TBL0((x) + 12)
> +#define TBL2(x) TBL1(x), TBL1((x) + 16), TBL1((x) + 32), TBL1((x) + 48)
> +TBL2(0), TBL2(64), TBL2(128), TBL2(192)
> +};
> +
> +return tbl[c];
>  }
>
>  /* Returns the integer value of the 'n' hexadecimal digits starting at
> 's', or
> --
> 2.15.1
>
> ___
> 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] ofproto-dpif-xlate: native tunnel using valid tun_src specified by flow or port options

[Ben Pfaff]

Thank you for submitting a patch to improve Open vSwitch.  I do not yet
understand the purpose of this patch.  Is it a bug fix or a new feature?

Thanks,

Ben.

On Fri, Feb 02, 2018 at 02:08:52PM +0800, we...@ucloud.cn wrote:
> From: wenxu 
> 
> native tunnel build tunnel with tun_src only from the route src and
> not care about the options:local_ip.
> Sometimes an virtual IP using for tun_src
> dpdk-br:
> inet 10.1.1.7/24 brd 10.1.1.255 scope global dpdk-br
> inet 10.1.1.254/32 scope global dpdk-br
> 
> Interface: gre  options: {key=flow, local_ip="10.1.1.254", remote_ip=flow}
> 
> the native tunnel always using 10.1.1.7 as the tunnel_src but not 10.1.1.254.
> 
> This patch made valid tun_src specified by flow-action or gre port options
> can be used for tunnel_src of packet. It stores the rtm_type for each route
> and improve the priority RTN_LOCAL type(higher then userdef route).
> Like the kernel space when lookup the route, if there are tun_src specified
> by flow-action or port options. Check the tun_src wheather is a local
> address, then lookup the route.
> 
> Signed-off-by: wenxu 
> Signed-off-by: frank.zeng 
> ---
>  lib/ovs-router.c |   38 +++---
>  lib/ovs-router.h |2 +-
>  lib/route-table.c|   10 --
>  ofproto/ofproto-dpif-sflow.c |2 +-
>  ofproto/ofproto-dpif-xlate.c |4 
>  5 files changed, 45 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/ovs-router.c b/lib/ovs-router.c
> index 0f1103b..e1375a3 100644
> --- a/lib/ovs-router.c
> +++ b/lib/ovs-router.c
> @@ -29,6 +29,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "classifier.h"
>  #include "command-line.h"
> @@ -61,6 +62,7 @@ struct ovs_router_entry {
>  struct in6_addr nw_addr;
>  struct in6_addr src_addr;
>  uint8_t plen;
> +uint8_t rtm_type;
>  uint8_t priority;
>  uint32_t mark;
>  };
> @@ -97,13 +99,28 @@ ovs_router_lookup(uint32_t mark, const struct in6_addr 
> *ip6_dst,
>  const struct cls_rule *cr;
>  struct flow flow = {.ipv6_dst = *ip6_dst, .pkt_mark = mark};
>  
> +if (src && ipv6_addr_is_set(src)) {
> +const struct cls_rule *cr_src;
> +struct flow flow_src = {.ipv6_dst = *src, .pkt_mark = mark};
> +
> +cr_src = classifier_lookup(, OVS_VERSION_MAX, _src, NULL);
> +if (cr_src) {
> +struct ovs_router_entry *p_src = ovs_router_entry_cast(cr_src);
> +if (p_src->rtm_type != RTN_LOCAL) {
> +return false;
> +}
> +} else {
> +return false;
> +}
> +}
> +
>  cr = classifier_lookup(, OVS_VERSION_MAX, , NULL);
>  if (cr) {
>  struct ovs_router_entry *p = ovs_router_entry_cast(cr);
>  
>  ovs_strlcpy(output_bridge, p->output_bridge, IFNAMSIZ);
>  *gw = p->gw;
> -if (src) {
> +if (src && !ipv6_addr_is_set(src)) {
>  *src = p->src_addr;
>  }
>  return true;
> @@ -184,7 +201,7 @@ out:
>  }
>  
>  static int
> -ovs_router_insert__(uint32_t mark, uint8_t priority,
> +ovs_router_insert__(uint32_t mark, uint8_t priority, uint8_t rtm_type,
>  const struct in6_addr *ip6_dst,
>  uint8_t plen, const char output_bridge[],
>  const struct in6_addr *gw)
> @@ -204,6 +221,7 @@ ovs_router_insert__(uint32_t mark, uint8_t priority,
>  p->mark = mark;
>  p->nw_addr = match.flow.ipv6_dst;
>  p->plen = plen;
> +p->rtm_type = rtm_type;
>  p->priority = priority;
>  err = get_src_addr(ip6_dst, output_bridge, >src_addr);
>  if (err && ipv6_addr_is_set(gw)) {
> @@ -236,9 +254,10 @@ ovs_router_insert__(uint32_t mark, uint8_t priority,
>  
>  void
>  ovs_router_insert(uint32_t mark, const struct in6_addr *ip_dst, uint8_t plen,
> -  const char output_bridge[], const struct in6_addr *gw)
> +  uint8_t rtm_type, const char output_bridge[], 
> +  const struct in6_addr *gw)
>  {
> -ovs_router_insert__(mark, plen, ip_dst, plen, output_bridge, gw);
> +ovs_router_insert__(mark, plen, rtm_type, ip_dst, plen, output_bridge, 
> gw);
>  }
>  
>  static void
> @@ -345,7 +364,7 @@ ovs_router_add(struct unixctl_conn *conn, int argc,
>  }
>  }
>  
> -err = ovs_router_insert__(mark, plen + 32, , plen, argv[2], );
> +err = ovs_router_insert__(mark, plen + 32, RTN_UNICAST, , plen, 
> argv[2], );
>  if (err) {
>  unixctl_command_reply_error(conn, "Error while inserting route.");
>  } else {
> @@ -402,7 +421,12 @@ ovs_router_show(struct unixctl_conn *conn, int argc 
> OVS_UNUSED,
>  ipv6_format_mapped(>nw_addr, );
>  plen = rt->plen;
>  if (IN6_IS_ADDR_V4MAPPED(>nw_addr)) {
> -plen -= 96;
> +uint8_t plen_off = 96;
> +
> +if (rt->rtm_type == RTN_LOCAL) {
> +  

[ovs-dev] [PATCH v2] datapath-windows: Add trace level logs in conntrack for invalid ct state.

[Anand Kumar]

Signed-off-by: Anand Kumar 
---
 datapath-windows/ovsext/Conntrack-icmp.c | 1 +
 datapath-windows/ovsext/Conntrack-tcp.c  | 4 
 datapath-windows/ovsext/Conntrack.c  | 6 ++
 3 files changed, 11 insertions(+)

diff --git a/datapath-windows/ovsext/Conntrack-icmp.c 
b/datapath-windows/ovsext/Conntrack-icmp.c
index 4da0665..28fe2bf 100644
--- a/datapath-windows/ovsext/Conntrack-icmp.c
+++ b/datapath-windows/ovsext/Conntrack-icmp.c
@@ -61,6 +61,7 @@ BOOLEAN
 OvsConntrackValidateIcmpPacket(const ICMPHdr *icmp)
 {
 if (!icmp) {
+OVS_LOG_TRACE("Invalid ICMP packet detected, header cannot be NULL");
 return FALSE;
 }
 
diff --git a/datapath-windows/ovsext/Conntrack-tcp.c 
b/datapath-windows/ovsext/Conntrack-tcp.c
index f8e85a2..8cbab24 100644
--- a/datapath-windows/ovsext/Conntrack-tcp.c
+++ b/datapath-windows/ovsext/Conntrack-tcp.c
@@ -444,12 +444,14 @@ BOOLEAN
 OvsConntrackValidateTcpPacket(const TCPHdr *tcp)
 {
 if (!tcp) {
+OVS_LOG_TRACE("Invalid TCP packet detected, header cannot be NULL");
 return FALSE;
 }
 
 UINT16 tcp_flags = ntohs(tcp->flags);
 
 if (OvsCtInvalidTcpFlags(tcp_flags)) {
+OVS_LOG_TRACE("Invalid TCP packet detected, tcp_flags %hu", tcp_flags);
 return FALSE;
 }
 
@@ -457,6 +459,8 @@ OvsConntrackValidateTcpPacket(const TCPHdr *tcp)
  * totally new connections (syn) or already established, not partially
  * open (syn+ack). */
 if ((tcp_flags & TCP_SYN) && (tcp_flags & TCP_ACK)) {
+OVS_LOG_TRACE("Invalid TCP packet detected, SYN+ACK flags not allowed,"
+  "tcp_flags %hu", tcp_flags);
 return FALSE;
 }
 
diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index 43c9dd3..678bedb 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -317,6 +317,10 @@ OvsCtEntryCreate(OvsForwardingContext *fwdCtx,
 const ICMPHdr *icmp;
 icmp = OvsGetIcmp(curNbl, l4Offset, );
 if (!OvsConntrackValidateIcmpPacket(icmp)) {
+if(icmp) {
+OVS_LOG_TRACE("Invalid ICMP packet detected, icmp->type %u",
+  icmp->type);
+}
 state = OVS_CS_F_INVALID;
 break;
 }
@@ -334,6 +338,8 @@ OvsCtEntryCreate(OvsForwardingContext *fwdCtx,
 break;
 }
 default:
+OVS_LOG_TRACE("Invalid packet detected, protocol not supported"
+  " ipProto %u", ipProto);
 state = OVS_CS_F_INVALID;
 break;
 }
-- 
2.9.3.windows.1

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

Re: [ovs-dev] [PATCH] Makefile.am: Use correct path separator for Windows

[Shashank Ram]

From: Ben Pfaff 
Sent: Friday, February 2, 2018 2:07 PM
To: Shashank Ram
Cc: d...@openvswitch.org
Subject: Re: [ovs-dev] [PATCH] Makefile.am: Use correct path separator for 
Windows

On Thu, Feb 01, 2018 at 05:03:52PM -0800, Shashank Ram wrote:
> Signed-off-by: Shashank Ram 
> ---
>  Makefile.am | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index 5988c02..1d336b6 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -398,7 +398,7 @@ CLEANFILES += flake8-check
>
>  include $(srcdir)/manpages.mk
>  $(srcdir)/manpages.mk: $(MAN_ROOTS) build-aux/sodepends.py 
> python/build/soutil.py
> - @PYTHONPATH=$$PYTHONPATH:$(srcdir)/python $(PYTHON) 
> $(srcdir)/build-aux/sodepends.py -I. -I$(srcdir) $(MAN_ROOTS) >$(@F).tmp
> + @PYTHONPATH=$$PYTHONPATH$(psep)$(srcdir)/python $(PYTHON) 
> $(srcdir)/build-aux/sodepends.py -I. -I$(srcdir) $(MAN_ROOTS) >$(@F).tmp
>   @if cmp -s $(@F).tmp $@; then \
> touch $@; \
> rm -f $(@F).tmp; \

Thanks for the fix.

There's a very similar instance elsewhere in the same file, doesn't it
need to be corrected also?

Both of these have been there approximately forever.  I don't understand
how they didn't get found before?

diff --git a/Makefile.am b/Makefile.am
index 85a09a5c8fbd..8632d6993cd8 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -148,7 +148,7 @@ ro_shell = printf '\043 Generated automatically -- do not 
modify!-*- buffer-

 SUFFIXES += .in
 .in:
-   $(AM_V_GEN)PYTHONPATH=$$PYTHONPATH:$(srcdir)/python $(PYTHON) 
$(srcdir)/build-aux/soexpand.py -I$(srcdir) < $< | \
+   $(AM_V_GEN)PYTHONPATH=$$PYTHONPATH$(psep)$(srcdir)/python $(PYTHON) 
$(srcdir)/build-aux/soexpand.py -I$(srcdir) < $< | \
  $(PYTHON) $(srcdir)/build-aux/dpdkstrip.py $(DPDKSTRIP_FLAGS) | \
  sed \
-e 's,[@]PKIDIR[@],$(PKIDIR),g' \


Looks like this got introduced in 2.9 time frame when the commands looked like 
this:

$(AM_V_GEN)$(PERL) $(srcdir)/build-aux/soexpand.pl -I$(srcdir) < $< | \
  $(PERL) $(srcdir)/build-aux/dpdkstrip.pl $(DPDKSTRIP_FLAGS) | \

$(AM_V_GEN)$(PERL) $(srcdir)/build-aux/soexpand.pl -I$(srcdir) < $< | \
  $(PERL) $(srcdir)/build-aux/dpdkstrip.pl $(DPDKSTRIP_FLAGS) | \

Will send out a v2, thanks!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Greetings

[Sarah Adam]

My name is Mrs. Sarah Adam, a woman of 75 years, I am in the sick bed now ready 
to die soon, but before I die I have an important massage that I want to tell 
you, very urgent. 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Fidelice a sus clientes y aumente sus ganancias

[Modelo Disney®]

Haga que sus clientes vivan la experiencia inspirada en el Modelo Disney® 

Basado en el Modelo Disney®: Servicio de Excelencia para el éxito financiero
15 de Febrero- Dr. Arturo Ruiz Velasco Romero - 9am- 6pm

Muchos de los objetivos del Quality Service de Disney®, se centran en ofrecer 
al cliente la mejor experiencia posible. El servicio de atención al cliente de 
la compañía podría considerarse el estándar a seguir para cualquier otra 
organización ya que entienden que si no se trata a los jefes que pagan como se 
merecen, las cosas no se están haciendo bien. 

BENEFICIOS DE ASISTIR: 

- Conocerá las bases del éxito de Disney® a través de su calidad en el servicio.
 - Entenderá el círculo de la excelencia de Disney.
 - Implementará altos estándares de calidad para la fidelización de sus 
clientes.
 - Aprenderá cómo recuperarse en el caso de fallas en el servicio. 

¿Requiere la información a la Brevedad? responda este email con la palabra: 
Disney + nombre - teléfono - correo.


centro telefónico:018002120744 


 


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

Re: [ovs-dev] [PATCH V2] ovn-controller: Reduce the number of flows by use conjunction action

[Ben Pfaff]

On Wed, Oct 18, 2017 at 10:39:12AM +0800, wei wrote:
> This patch convert ovn-sb lflow match expr "(1 or 2) and (3 or 4)" to
> match 1 aciton connjunction(1, 1/2)
> match 2 aciton connjunction(1, 1/2)
> match 3 aciton connjunction(1, 2/2)
> match 4 aciton connjunction(1, 2/2)
> match conj_id=1, action=XXX
> 
> NOT support nested conjunction, only use conjunction action in situation "or 
> in level 0"
> Like (1 or 2) and (3 or ((4 or 5) and (6 or 7))), (4 or 5) and (6 or 7) will 
> not be converted conjunction action,
> We could call this situation as "or in level 1", in this situation (4 or 5) 
> and (6 or 7) will be crossproduct,
> so (1 or 2) and (3 or ((4 or 5) and (6 or 7))) -> (1 or 2) and (3 or (4 and 
> 6) or (4 and 7) or (5 and 6) or (5 and 7))
> 
> In openstack, security group rule will match remote security group and tcp 
> port, like
> match=(ip4.src == $as_ip4_6a8f4283_ba60_4d1c_9dec_28d027eadef2 && tcp.dst >= 
> 1 && tcp.dst <= 2))
> 
> Use this patch, the number of flows will be significantly reduced
> 
> Signed-off-by: wei 

I'm awfully sorry that I took far too long to review this.  Somehow I
missed it, even though it is a really important topic.

This patch causes numerous test failures:

2316: ovn -- 4-term mixed expression normalizationFAILED (ovn.at:483)
2321: ovn -- 4-term string expressions to flows   FAILED (ovn.at:515)
2320: ovn -- 4-term numeric expressions to flows  FAILED (ovn.at:508)
2314: ovn -- 4-term numeric expression normalization  FAILED (ovn.at:471)
2322: ovn -- 4-term mixed expressions to flowsFAILED (ovn.at:522)
2315: ovn -- 4-term string expression normalization   FAILED (ovn.at:477)
2318: ovn -- 5-term string expression normalization   FAILED (ovn.at:495)
2319: ovn -- 5-term mixed expression normalizationFAILED (ovn.at:501)
2324: ovn -- converting expressions to flows -- string fields FAILED 
(ovn.at:560)
2317: ovn -- 5-term numeric expression normalization  FAILED (ovn.at:489)
2358: ovn -- ACL logging  FAILED (ovn.at:5955)

which all take the following form:

# -*- compilation -*-
2314. ovn.at:470: testing ovn -- 4-term numeric expression normalization ...
../../tests/ovn.at:471: ovstest test-ovn exhaustive --operation=normalize 
--nvars=3 --svars=0 --bits=1 4
--- /dev/null   2017-07-26 15:46:07.674034656 -0700
+++ /home/blp/nicira/ovs/_build/tests/testsuite.dir/at-groups/2314/stderr   
2018-02-02 15:05:06.998858676 -0800
@@ -0,0 +1,2 @@
+test-ovn: ../tests/test-ovn.c:874: assertion expr_is_normalized(modified) 
failed in test_tree_shape_exhaustively()

+/home/blp/nicira/ovs/_build/tests/testsuite.dir/at-groups/2314/test-source: 
line 27: 18216 Aborted (core dumped) ovstest test-ovn 
exhaustive --operation=normalize --nvars=3 --svars=0 --bits=1 4
--- -   2018-02-02 15:05:07.018262584 -0800
+++ /home/blp/nicira/ovs/_build/tests/testsuite.dir/at-groups/2314/stdout   
2018-02-02 15:05:07.002858553 -0800
@@ -1,2 +1 @@
-Tested normalizing 1874026 expressions of 4 terminals with 3 numeric vars 
(each 1 bits) in terms of operators == != < <= > >=.

../../tests/ovn.at:471: exit code was 134, expected 0
2314. ovn.at:470: 2314. ovn -- 4-term numeric expression normalization 
(ovn.at:470): FAILED (ovn.at:471)

There is another patch that purports to improve the same thing:
https://patchwork.ozlabs.org/patch/868639/
I'm going to look at that one soon, too.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: native tunnel using valid tun_src specified by flow or port options

[wenxu]

Hi ben,


This patch can be a bugfix.  
The tunnel_src of packet maybe not  the IP address set by gre port 
options:local_ip
dpdk-br has two address 10.1.1.7/24 and 10.1.1.254/32
Interface gre  options:local_ip="10.1.1.254"
But the tunnel_src of tunnel packet is always 10.1.1.7 


Also It can be a new feature.
In the same case the kernel space Open vSwitch  can send packet with tunnel_src 
10.1.1.254.


Why we need this?
In the High-availability cluster
server1 with IP 10.1.1.7/24 and a virtual IP 10.1.1.254
server2 with IP 10.1.1.8/24  and a virtual IP 10.1.1.254


10.1.1.7 and 10.1.1.8 running for the bgp and 10.1.1.254 provide the dataplane 
forward




At 2018-02-03 06:30:51, "Ben Pfaff"  wrote:
>Thank you for submitting a patch to improve Open vSwitch.  I do not yet
>understand the purpose of this patch.  Is it a bug fix or a new feature?
>
>Thanks,
>
>Ben.
>
>On Fri, Feb 02, 2018 at 02:08:52PM +0800, we...@ucloud.cn wrote:
>> From: wenxu 
>> 
>> native tunnel build tunnel with tun_src only from the route src and
>> not care about the options:local_ip.
>> Sometimes an virtual IP using for tun_src
>> dpdk-br:
>> inet 10.1.1.7/24 brd 10.1.1.255 scope global dpdk-br
>> inet 10.1.1.254/32 scope global dpdk-br
>> 
>> Interface: gre  options: {key=flow, local_ip="10.1.1.254", remote_ip=flow}
>> 
>> the native tunnel always using 10.1.1.7 as the tunnel_src but not 10.1.1.254.
>> 
>> This patch made valid tun_src specified by flow-action or gre port options
>> can be used for tunnel_src of packet. It stores the rtm_type for each route
>> and improve the priority RTN_LOCAL type(higher then userdef route).
>> Like the kernel space when lookup the route, if there are tun_src specified
>> by flow-action or port options. Check the tun_src wheather is a local
>> address, then lookup the route.
>> 
>> Signed-off-by: wenxu 
>> Signed-off-by: frank.zeng 
>> ---
>>  lib/ovs-router.c |   38 +++---
>>  lib/ovs-router.h |2 +-
>>  lib/route-table.c|   10 --
>>  ofproto/ofproto-dpif-sflow.c |2 +-
>>  ofproto/ofproto-dpif-xlate.c |4 
>>  5 files changed, 45 insertions(+), 11 deletions(-)
>> 
>> diff --git a/lib/ovs-router.c b/lib/ovs-router.c
>> index 0f1103b..e1375a3 100644
>> --- a/lib/ovs-router.c
>> +++ b/lib/ovs-router.c
>> @@ -29,6 +29,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  
>>  #include "classifier.h"
>>  #include "command-line.h"
>> @@ -61,6 +62,7 @@ struct ovs_router_entry {
>>  struct in6_addr nw_addr;
>>  struct in6_addr src_addr;
>>  uint8_t plen;
>> +uint8_t rtm_type;
>>  uint8_t priority;
>>  uint32_t mark;
>>  };
>> @@ -97,13 +99,28 @@ ovs_router_lookup(uint32_t mark, const struct in6_addr 
>> *ip6_dst,
>>  const struct cls_rule *cr;
>>  struct flow flow = {.ipv6_dst = *ip6_dst, .pkt_mark = mark};
>>  
>> +if (src && ipv6_addr_is_set(src)) {
>> +const struct cls_rule *cr_src;
>> +struct flow flow_src = {.ipv6_dst = *src, .pkt_mark = mark};
>> +
>> +cr_src = classifier_lookup(, OVS_VERSION_MAX, _src, NULL);
>> +if (cr_src) {
>> +struct ovs_router_entry *p_src = ovs_router_entry_cast(cr_src);
>> +if (p_src->rtm_type != RTN_LOCAL) {
>> +return false;
>> +}
>> +} else {
>> +return false;
>> +}
>> +}
>> +
>>  cr = classifier_lookup(, OVS_VERSION_MAX, , NULL);
>>  if (cr) {
>>  struct ovs_router_entry *p = ovs_router_entry_cast(cr);
>>  
>>  ovs_strlcpy(output_bridge, p->output_bridge, IFNAMSIZ);
>>  *gw = p->gw;
>> -if (src) {
>> +if (src && !ipv6_addr_is_set(src)) {
>>  *src = p->src_addr;
>>  }
>>  return true;
>> @@ -184,7 +201,7 @@ out:
>>  }
>>  
>>  static int
>> -ovs_router_insert__(uint32_t mark, uint8_t priority,
>> +ovs_router_insert__(uint32_t mark, uint8_t priority, uint8_t rtm_type,
>>  const struct in6_addr *ip6_dst,
>>  uint8_t plen, const char output_bridge[],
>>  const struct in6_addr *gw)
>> @@ -204,6 +221,7 @@ ovs_router_insert__(uint32_t mark, uint8_t priority,
>>  p->mark = mark;
>>  p->nw_addr = match.flow.ipv6_dst;
>>  p->plen = plen;
>> +p->rtm_type = rtm_type;
>>  p->priority = priority;
>>  err = get_src_addr(ip6_dst, output_bridge, >src_addr);
>>  if (err && ipv6_addr_is_set(gw)) {
>> @@ -236,9 +254,10 @@ ovs_router_insert__(uint32_t mark, uint8_t priority,
>>  
>>  void
>>  ovs_router_insert(uint32_t mark, const struct in6_addr *ip_dst, uint8_t 
>> plen,
>> -  const char output_bridge[], const struct in6_addr *gw)
>> +  uint8_t rtm_type, const char output_bridge[], 
>> +  const struct in6_addr *gw)
>>  {
>> -ovs_router_insert__(mark, plen, 

Re: [ovs-dev] [PATCH] docs: Update supported DPDK versions.

[Kavanagh, Mark B]

>From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
>boun...@openvswitch.org] On Behalf Of Ian Stokes
>Sent: Tuesday, January 30, 2018 10:08 AM
>To: d...@openvswitch.org
>Subject: [ovs-dev] [PATCH] docs: Update supported DPDK versions.
>
>Update the OVS to DPDK release table to use the latest stable
>DPDK 16.11.4 for OVS 2.7.
>
>Signed-off-by: Ian Stokes 
>---
> Documentation/faq/releases.rst | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
>index 62a1957..86eefeb 100644
>--- a/Documentation/faq/releases.rst
>+++ b/Documentation/faq/releases.rst
>@@ -162,7 +162,7 @@ Q: What DPDK version does each Open vSwitch release work
>with?
> 2.4.x2.0
> 2.5.x2.2
> 2.6.x16.07.2
>-2.7.x16.11.3
>+2.7.x16.11.4
> 2.8.x17.05.2
> 2.9.x17.11
>  ===

Hey Ian,

I built OvS versions 2.7.1-.3 against DPDK v16.11.4 without issue.

However, with OvS v2.7.0, I see the following:

Warning, treated as error:
/ovs/Documentation/topics/windows.rst:506:Footnote [5] is not 
referenced.
make[2]: *** [htmldocs] Error 1
make[2]: *** Waiting for unfinished jobs
make[2]: Leaving directory `//dev/ovs'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `//dev/ovs'
make: *** [all] Error 2

It's worth noting though, that this issue is also present when building OvS 
v2.7.0 with DPDK v16.11.3. As such, I don't see this as an issues specific to 
this patch, but which should be resolved separately.

Acked-by: Mark Kavanagh 

Cheers,
Mark


>--
>2.7.5
>
>___
>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] ovn-northd: Do not add lflows in lr_in_arp_resolve stage for disabled logical ports

[Numan Siddique]

On Fri, Feb 2, 2018 at 6:02 AM, Ben Pfaff  wrote:

> On Thu, Oct 26, 2017 at 06:56:46AM +0530, Numan Siddique wrote:
> > On Thu, Oct 26, 2017 at 2:59 AM, Ben Pfaff  wrote:
> >
> > > On Wed, Sep 27, 2017 at 09:36:18PM +0530, nusid...@redhat.com wrote:
> > > > From: Numan Siddique 
> > > >
> > > > ovn-northd is adding the below logical flow for a disabled logical
> port
> > > (with mac M
> > > > and IP 'A')
> > > >
> > > > table=6 (lr_in_arp_resolve  ), match=(outport == "lrp-port" && reg0
> ==
> > > 'A'),
> > > > action=(eth.dst = 'M'; next;)
> > > >
> > > > In the case of openstack load balancer 'octavia' service, it creates
> > > logical
> > > > ports 'P1' (M1 IP1) and 'P2' (M2 IP2). It then disables logical port
> P2
> > > and
> > > > adds IP2 to P1 - (M1 IP1 IP2).
> > > >
> > > > When another port tries to reach IP2, it doesn't get delivered to
> port
> > > P1 because
> > > > of the above flow.
> > > >
> > > > Signed-off-by: Numan Siddique 
> > >
> > > Thanks a lot, I applied this to master.
> > >
> > > Please let me know if I should backport it to 2.8 (or earlier).
> > >
> >
> > Thanks for the review. It would be great if it can be backported to 2.8.
>
> Apparently I forgot to ever do that, but I did it now.
>

Thanks Ben.

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