[ovs-dev] [PATCH ovn 2/3] Refactor install scripts for ovn container

[amginwal]

From: Aliasgar Ginwala 

Signed-off-by: Aliasgar Ginwala 
---
 utilities/automake.mk  |  3 ++-
 utilities/docker/Makefile  |  2 +-
 utilities/docker/debian/Dockerfile |  1 +
 utilities/docker/debian/build.sh   | 24 +-
 utilities/docker/install_ovn.sh| 40 ++
 5 files changed, 45 insertions(+), 25 deletions(-)
 create mode 100755 utilities/docker/install_ovn.sh

diff --git a/utilities/automake.mk b/utilities/automake.mk
index 197cc7011..0b7e38dc0 100644
--- a/utilities/automake.mk
+++ b/utilities/automake.mk
@@ -39,7 +39,8 @@ EXTRA_DIST += \
 utilities/docker/ovn_default_sb_port \
 utilities/docker/ovn_default_northd_host \
 utilities/docker/debian/Dockerfile \
-utilities/docker/debian/build.sh
+utilities/docker/debian/build.sh \
+utilities/docker/install_ovn.sh
 
 CLEANFILES += \
 utilities/ovn-ctl.8 \
diff --git a/utilities/docker/Makefile b/utilities/docker/Makefile
index 304723f8f..57e95651c 100644
--- a/utilities/docker/Makefile
+++ b/utilities/docker/Makefile
@@ -9,7 +9,7 @@
 #   make push
 
 REPO = ${DOCKER_REPO}
-tag = ${OVN_VERSION}_${KERNEL_VERSION}
+tag = ${OVN_VERSION}_${DISTRO}_${OVN_BRANCH}
 
 build: ;docker build -t ${REPO}:${tag} --build-arg DISTRO=${DISTRO} \
 --build-arg OVN_BRANCH=${OVN_BRANCH} \
diff --git a/utilities/docker/debian/Dockerfile 
b/utilities/docker/debian/Dockerfile
index 9c35f6b16..366ad6d4f 100644
--- a/utilities/docker/debian/Dockerfile
+++ b/utilities/docker/debian/Dockerfile
@@ -6,6 +6,7 @@ ARG GITHUB_SRC
 ARG DISTRO
 
 copy $DISTRO/build.sh /build.sh
+copy install_ovn.sh /install_ovn.sh
 RUN /build.sh $OVN_BRANCH $GITHUB_SRC
 
 COPY ovn_default_nb_port /etc/ovn/ovn_default_nb_port
diff --git a/utilities/docker/debian/build.sh b/utilities/docker/debian/build.sh
index 8e492bf76..0d0c8cfd8 100755
--- a/utilities/docker/debian/build.sh
+++ b/utilities/docker/debian/build.sh
@@ -23,29 +23,7 @@ dh-autoreconf openssl"
 apt-get update
 apt-get install -y ${build_deps}
 
-# get ovs source always from master as its needed as dependency
-mkdir /build; cd /build
-git clone --depth 1 -b master https://github.com/openvswitch/ovs.git
-cd ovs;
-mkdir _gcc;
-
-# build and install
-./boot.sh
-cd _gcc
-../configure --localstatedir="/var" --sysconfdir="/etc" --prefix="/usr" \
---enable-ssl
-cd ..; make -C _gcc install; cd ..
-
-
-# get ovn source
-git clone --depth 1 -b $OVN_BRANCH $GITHUB_SRC
-cd ovn
-
-# build and install
-./boot.sh
-./configure --localstatedir="/var" --sysconfdir="/etc" --prefix="/usr" \
---enable-ssl --with-ovs-source=/build/ovs/ --with-ovs-build=/build/ovs/_gcc
-make -j8; make install
+./install_ovn.sh $OVN_BRANCH $GITHUB_SRC
 
 # remove deps to make the container light weight.
 apt-get remove --purge -y ${build_deps}
diff --git a/utilities/docker/install_ovn.sh b/utilities/docker/install_ovn.sh
new file mode 100755
index 0..55c189aae
--- /dev/null
+++ b/utilities/docker/install_ovn.sh
@@ -0,0 +1,40 @@
+#!/bin/sh
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at:
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+OVN_BRANCH=$1
+GITHUB_SRC=$2
+
+# get ovs source always from master as its needed as dependency
+mkdir /build; cd /build
+git clone --depth 1 -b master https://github.com/openvswitch/ovs.git
+cd ovs;
+mkdir _gcc;
+
+# build and install
+./boot.sh
+cd _gcc
+../configure --localstatedir="/var" --sysconfdir="/etc" --prefix="/usr" \
+--enable-ssl
+cd ..; make -C _gcc install; cd ..
+
+
+# get ovn source
+git clone --depth 1 -b $OVN_BRANCH $GITHUB_SRC
+cd ovn
+
+# build and install
+./boot.sh
+./configure --localstatedir="/var" --sysconfdir="/etc" --prefix="/usr" \
+--enable-ssl --with-ovs-source=/build/ovs/ --with-ovs-build=/build/ovs/_gcc
+make -j8; make install
-- 
2.20.1 (Apple Git-117)

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

[ovs-dev] [PATCH ovn 3/3] rhel support for ovn container

[amginwal]

From: Aliasgar Ginwala 

Current code only had support for starting ovn in ubuntu containers.
This patch adds supprt for rhel using centos7 as a base image

Signed-off-by: Aliasgar Ginwala 
---
 Documentation/intro/install/general.rst |  2 +-
 utilities/automake.mk   |  2 ++
 utilities/docker/rhel/Dockerfile| 20 +
 utilities/docker/rhel/build.sh  | 38 +
 4 files changed, 61 insertions(+), 1 deletion(-)
 create mode 100755 utilities/docker/rhel/Dockerfile
 create mode 100755 utilities/docker/rhel/build.sh

diff --git a/Documentation/intro/install/general.rst 
b/Documentation/intro/install/general.rst
index cdd78f7f7..52bfd7d18 100644
--- a/Documentation/intro/install/general.rst
+++ b/Documentation/intro/install/general.rst
@@ -471,7 +471,7 @@ Start OVN containers using unix socket::
 
 User can use any other base image for debian, e.g. u14.04, etc.
 
-RHEL based docker build support needs to be added.
+RHEL based docker support is now added with centos7 as a base image.
 
 Starting OVN host service
 
diff --git a/utilities/automake.mk b/utilities/automake.mk
index 0b7e38dc0..73018ca66 100644
--- a/utilities/automake.mk
+++ b/utilities/automake.mk
@@ -40,6 +40,8 @@ EXTRA_DIST += \
 utilities/docker/ovn_default_northd_host \
 utilities/docker/debian/Dockerfile \
 utilities/docker/debian/build.sh \
+utilities/docker/rhel/Dockerfile \
+utilities/docker/rhel/build.sh \
 utilities/docker/install_ovn.sh
 
 CLEANFILES += \
diff --git a/utilities/docker/rhel/Dockerfile b/utilities/docker/rhel/Dockerfile
new file mode 100755
index 0..e4f5cfece
--- /dev/null
+++ b/utilities/docker/rhel/Dockerfile
@@ -0,0 +1,20 @@
+FROM centos:7
+MAINTAINER "Aliasgar Ginwala" 
+
+ARG OVN_BRANCH
+ARG GITHUB_SRC
+ARG DISTRO
+
+copy $DISTRO/build.sh /build.sh
+copy install_ovn.sh /install_ovn.sh
+RUN /build.sh $OVN_BRANCH $GITHUB_SRC
+
+COPY ovn_default_nb_port /etc/ovn/ovn_default_nb_port
+COPY ovn_default_sb_port /etc/ovn/ovn_default_sb_port
+COPY ovn_default_northd_host /etc/ovn/ovn_default_northd_host
+
+COPY start-ovn /bin/start-ovn
+VOLUME ["/var/log/openvswitch", \
+"/var/lib/openvswitch", "/var/run/openvswitch", "/etc/openvswitch", \
+"/var/log/ovn", "/var/lib/ovn", "/var/run/ovn", "/etc/ovn"]
+ENTRYPOINT ["start-ovn"]
diff --git a/utilities/docker/rhel/build.sh b/utilities/docker/rhel/build.sh
new file mode 100755
index 0..eb6f85a4a
--- /dev/null
+++ b/utilities/docker/rhel/build.sh
@@ -0,0 +1,38 @@
+#!/bin/sh
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at:
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+OVN_BRANCH=$1
+GITHUB_SRC=$2
+
+# Install deps
+build_deps="rpm-build yum-utils yum-builddep automake autoconf openssl-devel \
+epel-release python3 gdb libtool git bzip2 perl-core zlib-devel openssl git \
+libtool"
+
+yum update -y
+yum install @'Development Tools'  ${build_deps} -y
+pip3 install six
+
+./install_ovn.sh $OVN_BRANCH $GITHUB_SRC
+
+# remove unused packages to make the container light weight.
+for i in $(package-cleanup --leaves --all);
+do yum remove -y $i; yum autoremove -y;
+done
+yum remove ${build_deps} -y
+cd ..; rm -rf ovs; rm -rf ovn
+
+# Install basic utils
+basic_utils="vim-minimal.x86_64 net-tools.x86_64 uuid.x86_64 iproute.x86_64"
+yum install -y ${basic_utils}
-- 
2.20.1 (Apple Git-117)

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

[ovs-dev] [PATCH 2/3] Combine conjunctions with identical matches into one flow.

[Mark Michelson]

As stated in previous commits, conjunctive matches have an issue where
it is possible to install multiple flows that have identical matches.
This results in ambiguity, and can lead to features (such as ACLs) not
functioning properly.

This change fixes the problem by combining conjunctions with identical
matches into a single flow. As an example, in the past we may have had
something like:

nw_dst=10.0.0.1 actions=conjunction(2,1/2)
nw_dst=10.0.0.1 actions=conjunction(3,1/2)

This commit changes this into

nw_dst=10.0.0.1 actions=conjunction(2,1/2),conjunction(3,1/2)

This way, there is only a single flow with the proscribed match, and
there is no ambiguity.

Signed-off-by: Mark Michelson 
---
 controller/lflow.c  |  5 ++--
 controller/ofctrl.c | 73 +
 controller/ofctrl.h |  6 +
 tests/ovn.at| 17 +
 4 files changed, 79 insertions(+), 22 deletions(-)

diff --git a/controller/lflow.c b/controller/lflow.c
index e3ed20cd4..34b7c36a6 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -736,8 +736,9 @@ consider_logical_flow(
 dst->clause = src->clause;
 dst->n_clauses = src->n_clauses;
 }
-ofctrl_add_flow(flow_table, ptable, lflow->priority, 0, >match,
-, >header_.uuid);
+
+ofctrl_add_or_append_flow(flow_table, ptable, lflow->priority, 0,
+  >match, , >header_.uuid);
 ofpbuf_uninit();
 }
 }
diff --git a/controller/ofctrl.c b/controller/ofctrl.c
index 3131baff0..afb0036f1 100644
--- a/controller/ofctrl.c
+++ b/controller/ofctrl.c
@@ -69,6 +69,11 @@ struct ovn_flow {
 uint64_t cookie;
 };
 
+static struct ovn_flow *ovn_flow_alloc(uint8_t table_id, uint16_t priority,
+   uint64_t cookie,
+   const struct match *match,
+   const struct ofpbuf *actions,
+   const struct uuid *sb_uuid);
 static uint32_t ovn_flow_match_hash(const struct ovn_flow *);
 static struct ovn_flow *ovn_flow_lookup(struct hmap *flow_table,
 const struct ovn_flow *target,
@@ -657,16 +662,8 @@ ofctrl_check_and_add_flow(struct ovn_desired_flow_table 
*flow_table,
   const struct uuid *sb_uuid,
   bool log_duplicate_flow)
 {
-struct ovn_flow *f = xmalloc(sizeof *f);
-f->table_id = table_id;
-f->priority = priority;
-minimatch_init(>match, match);
-f->ofpacts = xmemdup(actions->data, actions->size);
-f->ofpacts_len = actions->size;
-f->sb_uuid = *sb_uuid;
-f->match_hmap_node.hash = ovn_flow_match_hash(f);
-f->uuid_hindex_node.hash = uuid_hash(>sb_uuid);
-f->cookie = cookie;
+struct ovn_flow *f = ovn_flow_alloc(table_id, priority, cookie, match,
+actions, sb_uuid);
 
 ovn_flow_log(f, "ofctrl_add_flow");
 
@@ -721,9 +718,65 @@ ofctrl_add_flow(struct ovn_desired_flow_table 
*desired_flows,
 ofctrl_check_and_add_flow(desired_flows, table_id, priority, cookie,
   match, actions, sb_uuid, true);
 }
+
+void
+ofctrl_add_or_append_flow(struct ovn_desired_flow_table *desired_flows,
+  uint8_t table_id, uint16_t priority, uint64_t cookie,
+  const struct match *match,
+  const struct ofpbuf *actions,
+  const struct uuid *sb_uuid)
+{
+struct ovn_flow *f = ovn_flow_alloc(table_id, priority, cookie, match,
+actions, sb_uuid);
+
+ovn_flow_log(f, "ofctrl_add_or_append_flow");
+
+struct ovn_flow *existing;
+existing = ovn_flow_lookup(_flows->match_flow_table, f, false);
+if (existing) {
+/* There's already a flow with this particular match. Append the
+ * action to that flow rather than adding a new flow
+ */
+uint64_t compound_stub[64 / 8];
+struct ofpbuf compound;
+ofpbuf_use_stub(, compound_stub, sizeof(compound_stub));
+ofpbuf_put(, existing->ofpacts, existing->ofpacts_len);
+ofpbuf_put(, f->ofpacts, f->ofpacts_len);
+
+free(existing->ofpacts);
+existing->ofpacts = xmemdup(compound.data, compound.size);
+existing->ofpacts_len = compound.size;
+
+ovn_flow_destroy(f);
+} else {
+hmap_insert(_flows->match_flow_table, >match_hmap_node,
+f->match_hmap_node.hash);
+hindex_insert(_flows->uuid_flow_table, >uuid_hindex_node,
+  f->uuid_hindex_node.hash);
+}
+}
 
 /* ovn_flow. */
 
+static struct ovn_flow *
+ovn_flow_alloc(uint8_t table_id, uint16_t priority, uint64_t cookie,
+   const struct match *match, const struct ofpbuf *actions,
+   const 

[ovs-dev] [PATCH 1/3] Revert conjunctive match removal patches.

[Mark Michelson]

This partially reverts commits 6f914327a55aaab11507db0911b08d695e17ce2a
and 298701dbc99645700be41680a43d049cb061847a. This restores the behavior
of making conjunctive matches, and it restores the tests to their state.

The one thing it does not revert is the addition of the
force_cross_product variable in expr.c. This variable will be used in a
later commit in this series. Instead of removing it, we just set it
"false" to restore previous behavior.

With this commit, the conjunctive match misbehavior described in commit
298701dbc99645700be41680a43d049cb061847a is restored. However, this will
be fixed in an upcoming commit in this series.

Signed-off-by: Mark Michelson 
---
 TODO.rst |   10 -
 lib/expr.c   |9 +-
 tests/ovn.at | 1250 +++---
 3 files changed, 61 insertions(+), 1208 deletions(-)

diff --git a/TODO.rst b/TODO.rst
index ed55ea236..943d9bf81 100644
--- a/TODO.rst
+++ b/TODO.rst
@@ -145,13 +145,3 @@ OVN To-do List
   * Support FTP ALGs.
 
   * Support reject action.
-
-* Conjunction: Conjunction is disabled in OVN. This needs to be revisisted
-  to enable conjunction again after addressing the issues related to it.
-  Like, if there are multiple ACLs with overlapping Conjunction matches,
-  conjunction flows are not added properly.
-  Eg. match(ip4.src == {IP1, IP2, IP3} && ip4.dst == {IP4, IP5, IP6} &&
-  tcp.dst >= 800 && tcp.dst <= 900) actions=drop
-
-  match(ip4.src == {IP1, IP2, IP3} && ip4.dst == {IP4, IP5, IP6} &&
-  tcp.dst >= 1000 && tcp.dst <= 2000) actions=allow
diff --git a/lib/expr.c b/lib/expr.c
index 9b9b6bcca..71de61543 100644
--- a/lib/expr.c
+++ b/lib/expr.c
@@ -42,13 +42,8 @@ VLOG_DEFINE_THIS_MODULE(expr);
  *
  * match 2 - ip4.src == {IP1, IP2} && tcp.dst >=700 && tcp.src <=800
  * action - allow.
- *
- * To handle this issue temporarily force crossproduct so that conjunction
- * flows are not generated.
- *
- * Remove this once fixed.
- * */
-static bool force_crossproduct = true;
+ */
+static bool force_crossproduct = false;
 
 static struct expr *parse_and_annotate(const char *s,
const struct shash *symtab,
diff --git a/tests/ovn.at b/tests/ovn.at
index 22b272a60..641a646fc 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -589,22 +589,6 @@ ip,reg14=0x6
 ipv6,reg14=0x5
 ipv6,reg14=0x6
 ])
-AT_CHECK([expr_to_flow 'inport == {"eth0", "eth1", "eth2"} && ip4 && tcp && 
tcp.dst == {500, 501}'], [0], [dnl
-tcp,reg14=0x5,tp_dst=500
-tcp,reg14=0x5,tp_dst=501
-tcp,reg14=0x6,tp_dst=500
-tcp,reg14=0x6,tp_dst=501
-])
-AT_CHECK([expr_to_flow 'outport == {"eth0", "eth1", "eth2"} && ip4 && tcp && 
tcp.src == {400, 401} && tcp.dst == {500, 501}'], [0], [dnl
-tcp,reg15=0x5,tp_src=400,tp_dst=500
-tcp,reg15=0x5,tp_src=400,tp_dst=501
-tcp,reg15=0x5,tp_src=401,tp_dst=500
-tcp,reg15=0x5,tp_src=401,tp_dst=501
-tcp,reg15=0x6,tp_src=400,tp_dst=500
-tcp,reg15=0x6,tp_src=400,tp_dst=501
-tcp,reg15=0x6,tp_src=401,tp_dst=500
-tcp,reg15=0x6,tp_src=401,tp_dst=501
-])
 AT_CHECK([expr_to_flow 'inport == "eth0" && inport == "eth1"'], [0], [dnl
 (no flows)
 ])
@@ -709,14 +693,6 @@ reg15=0x11
 reg15=0x12
 reg15=0x13
 ])
-AT_CHECK([expr_to_flow 'outport == @pg1 && ip4.src == {10.0.0.4, 10.0.0.5}'], 
[0], [dnl
-ip,reg15=0x11,nw_src=10.0.0.4
-ip,reg15=0x11,nw_src=10.0.0.5
-ip,reg15=0x12,nw_src=10.0.0.4
-ip,reg15=0x12,nw_src=10.0.0.5
-ip,reg15=0x13,nw_src=10.0.0.4
-ip,reg15=0x13,nw_src=10.0.0.5
-])
 AT_CHECK([expr_to_flow 'outport == {@pg_empty}'], [0], [dnl
 (no flows)
 ])
@@ -725,27 +701,22 @@ reg15=0x11
 ])
 AT_CLEANUP
 
-AT_SETUP([ovn -- converting expressions to flows -- no conjunction])
-AT_KEYWORDS([no conjunction])
+AT_SETUP([ovn -- converting expressions to flows -- conjunction])
+AT_KEYWORDS([conjunction])
 expr_to_flow () {
 echo "$1" | ovstest test-ovn expr-to-flows | sort
 }
 
-# conjunction is disabled in OVN until some of the issues
-# related to conjunction flows are fixed.
-# expr-to-flows should not generate any conjunction flows.
 lflow="ip4.src == {10.0.0.1, 10.0.0.2, 10.0.0.3} && \
 ip4.dst == {20.0.0.1, 20.0.0.2, 20.0.0.3}"
 AT_CHECK([expr_to_flow "$lflow"], [0], [dnl
-ip,nw_src=10.0.0.1,nw_dst=20.0.0.1
-ip,nw_src=10.0.0.1,nw_dst=20.0.0.2
-ip,nw_src=10.0.0.1,nw_dst=20.0.0.3
-ip,nw_src=10.0.0.2,nw_dst=20.0.0.1
-ip,nw_src=10.0.0.2,nw_dst=20.0.0.2
-ip,nw_src=10.0.0.2,nw_dst=20.0.0.3
-ip,nw_src=10.0.0.3,nw_dst=20.0.0.1
-ip,nw_src=10.0.0.3,nw_dst=20.0.0.2
-ip,nw_src=10.0.0.3,nw_dst=20.0.0.3
+conj_id=1,ip
+ip,nw_dst=20.0.0.1: conjunction(1, 0/2)
+ip,nw_dst=20.0.0.2: conjunction(1, 0/2)
+ip,nw_dst=20.0.0.3: conjunction(1, 0/2)
+ip,nw_src=10.0.0.1: conjunction(1, 1/2)
+ip,nw_src=10.0.0.2: conjunction(1, 1/2)
+ip,nw_src=10.0.0.3: conjunction(1, 1/2)
 ])
 
 lflow="ip && (!ct.est || (ct.est && ct_label.blocked == 1))"
@@ -759,12 +730,12 @@ ct_state=-est+trk,ipv6
 lflow="ip4.src == {10.0.0.1, 10.0.0.2, 10.0.0.3} && \
 ip4.dst == {20.0.0.1, 20.0.0.2}"
 AT_CHECK([expr_to_flow "$lflow"], [0], 

Re: [ovs-dev] [PATCH 1/3] Revert conjunctive match removal patches.

[0-day Robot]

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

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


git-am:
fatal: sha1 information is lacking or useless (TODO.rst).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Revert conjunctive match removal patches.
The copy of the patch that failed is found in:
   
/var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
When you have resolved this problem, run "git am --resolved".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


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

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

Re: [ovs-dev] [PATCH v2] Modify release document for OVN.

[Mark Michelson]

I made the suggested changes and pushed the updated document to master.

On 10/2/19 5:17 PM, Han Zhou wrote:

Hi Mark,

Thanks for updating this. Please see my comments below.


On Thu, Sep 26, 2019 at 12:25 PM Mark Michelson > wrote:

 >
 > This is an RFC to discuss the modified release cycle of OVN compared to
 > OVS. The document is mostly unchanged, with two exceptions:
 >
 > * The release cycle for OVN is modified to be 3 months instead of 6.
 > * The version numbering for OVN is modified to use the year and month of
 >   the release instead of other numbers.
 >
 > Signed-off-by: Mark Michelson >

 > ---
 > v1 -> v2: Rebased
 > ---
 >  Documentation/internals/release-process.rst | 60 
++---

 >  1 file changed, 30 insertions(+), 30 deletions(-)
 >
 > diff --git a/Documentation/internals/release-process.rst 
b/Documentation/internals/release-process.rst

 > index 3396177b8..8af962b49 100644
 > --- a/Documentation/internals/release-process.rst
 > +++ b/Documentation/internals/release-process.rst
 > @@ -25,19 +25,19 @@
 >  OVN Release Process
 >  ===
 >
 > -This document describes the process ordinarily used for OVN
 > -development and release.  Exceptions are sometimes necessary, so all 
of the
 > -statements here should be taken as subject to change through rough 
consensus of

 > -OVN contributors, obtained through public discussion on, e.g., ovs-dev
 > -or the #openvswitch IRC channel.
 > +This document describes the process ordinarily used for OVN 
development and
 > +release.  Exceptions are sometimes necessary, so all of the 
statements here

 > +should be taken as subject to change through rough consensus of OVN
 > +contributors, obtained through public discussion on, e.g., ovs-dev 
or the

 > +#openvswitch IRC channel.
 >
 >  Release Strategy
 >  
 >
 > -OVN feature development takes place on the "master" branch.
 > -Ordinarily, new features are rebased against master and applied 
directly.  For
 > -features that take significant development, sometimes it is more 
appropriate to
 > -merge a separate branch into master; please discuss this on ovs-dev 
in advance.
 > +OVN feature development takes place on the "master" branch. 
Ordinarily, new
 > +features are rebased against master and applied directly.  For 
features that
 > +take significant development, sometimes it is more appropriate to 
merge a

 > +separate branch into master; please discuss this on ovs-dev in advance.
 >
 >  The process of making a release has the following stages.  See `Release
 >  Scheduling`_ for the timing of each stage:
 > @@ -50,7 +50,7 @@ Scheduling`_ for the timing of each stage:
 >     Please propose and discuss exceptions on ovs-dev.
 >
 >  2. Fork a release branch from master, named for the expected release 
number,

 > -   e.g. "branch-2.3" for the branch that will yield OVN 2.3.x.
 > +   e.g. "branch-2019.10" for the branch that will yield OVN 2019.10.x.
 >
 >     Release branches are intended for testing and stabilization.  At 
this stage
 >     and in later stages, they should receive only bug fixes, not new 
features.

 > @@ -65,15 +65,15 @@ Scheduling`_ for the timing of each stage:
 >     and risk and discussed on ovs-dev before creating the branch.
 >
 >  3. When committers come to rough consensus that the release is 
ready, they
 > -   release the .0 release on its branch, e.g. 2.3.0 for branch-2.3.  
To make
 > -   the actual release, a committer pushes a signed tag named, e.g. 
v2.3.0, to

 > -   the OVN repository, makes a release tarball available on
 > +   release the .0 release on its branch, e.g. 2019.10.0 for 
branch-2019.10.  To

 > +   make the actual release, a committer pushes a signed tag named, e.g.
 > +   v2019.10.0, to the OVN repository, makes a release tarball 
available on
 > openvswitch.org , and posts a release 
announcement to ovs-announce.

 >
 >  4. As bug fixes accumulate, or after important bugs or 
vulnerabilities are
 > -   fixed, committers may make additional releases from a branch: 
2.3.1, 2.3.2,
 > -   and so on.  The process is the same for these additional release 
as for a .0

 > -   release.
 > +   fixed, committers may make additional releases from a branch: 
2019.10.1,
 > +   2019.10.2, and so on.  The process is the same for these 
additional release

 > +   as for a .0 release.
 >
 >  At most two release branches are formally maintained at any given 
time: the
 >  latest release and the latest release designed as LTS.  An LTS 
release is one
 > @@ -97,33 +97,33 @@ titled "Prepare for x.y.0" and increments the 
version number to x.y.  This is
 >  the initial commit on branch-x.y.  The second is titled "Prepare for 
post-x.y.0

 >  (x.y.90)" and increments the version number to x.y.90.
 >
 > -The version number on a release branch is x.y.z, where z is initially 0.
 > -Making a release requires two commits.  The first is titled 

[ovs-dev] [PATCH 0/3] Reintroduce conjunctive matches

[Mark Michelson]

Conjunctive matches are currently disabled in OVN. Use of constructs
such as port groups and address sets in ACLs can potentially result in
flows with identical matches being generated. These flows, when added to
OVS, result in unpredictable behavior.

This patch set aims to fix this by consolidating conjunctions with
identical matches to a single flow. In addition, this patch series
introduces behavior to limit the total number of conjunction flows
installed. This hopefully should result in less risk of reaching the
maximum number of resubmits during packet processing.

Mark Michelson (3):
  Revert conjunctive match removal patches.
  Combine conjunctions with identical matches into one flow.
  Limit the number of generated conjunctions.

 TODO.rst|   10 -
 controller/lflow.c  |   16 +-
 controller/ofctrl.c |  142 +-
 controller/ofctrl.h |   12 +
 include/ovn/expr.h  |2 +
 lib/expr.c  |   15 +-
 tests/ovn.at| 1265 +++
 7 files changed, 233 insertions(+), 1229 deletions(-)

-- 
2.14.5

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

[ovs-dev] [PATCH 3/3] Limit the number of generated conjunctions.

[Mark Michelson]

There is a maximum number of resubmits that can occur during packet
processing. Deliberately creating a flow table that might cross this
threshold is irresponsible.

This commit causes the ofctrl code to track the maximum width and depth
of conjunctions in the desired flow table. By multiplying them, we have
a possible worst case for number of resubmits required. If this number
exceeds a quarter of the maximum resubmits allowed, then we fall back to
forcing crossproducts.

The resulting flow table can end up being a mixture of conjunction and
non-conjunction flows if the limit is exceeded.

Signed-off-by: Mark Michelson 
---
 controller/lflow.c  | 11 +
 controller/ofctrl.c | 69 +
 controller/ofctrl.h |  6 +
 include/ovn/expr.h  |  2 ++
 lib/expr.c  |  6 +
 5 files changed, 94 insertions(+)

diff --git a/controller/lflow.c b/controller/lflow.c
index 34b7c36a6..318066465 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -37,6 +37,13 @@
 VLOG_DEFINE_THIS_MODULE(lflow);
 
 COVERAGE_DEFINE(lflow_run);
+
+/* Limit maximum conjunctions to a quarter of the max
+ * resubmits. Unfortunatley, max resubmits is not publicly
+ * defined in a header file, so if max resubmits is changed
+ * from 4096, we have to make sure to update this as well
+ */
+#define MAX_CONJUNCTIONS (4096 / 4)
 
 /* Symbol table. */
 
@@ -602,6 +609,10 @@ consider_logical_flow(
 struct expr *prereqs;
 char *error;
 
+uint32_t conj_worst_case;
+conj_worst_case = flow_table->max_conj_width * flow_table->max_conj_depth;
+expr_set_force_crossproduct(conj_worst_case >= MAX_CONJUNCTIONS);
+
 error = ovnacts_parse_string(lflow->actions, , , );
 if (error) {
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
diff --git a/controller/ofctrl.c b/controller/ofctrl.c
index afb0036f1..2b2fde3c9 100644
--- a/controller/ofctrl.c
+++ b/controller/ofctrl.c
@@ -686,6 +686,64 @@ ofctrl_check_and_add_flow(struct ovn_desired_flow_table 
*flow_table,
   f->uuid_hindex_node.hash);
 }
 
+static void
+get_conjunction_dimensions(const struct ovn_flow *f, uint32_t *conj_width_p,
+   uint32_t *conj_depth_p)
+{
+struct ofpact *ofpact;
+uint32_t conj_width = 0;
+uint32_t conj_depth = 0;
+OFPACT_FOR_EACH (ofpact, f->ofpacts, f->ofpacts_len) {
+if (ofpact->type == OFPACT_CONJUNCTION) {
+struct ofpact_conjunction *conj = ofpact_get_CONJUNCTION(ofpact);
+if (conj->n_clauses > conj_depth) {
+conj_depth = conj->n_clauses;
+}
+conj_width++;
+}
+}
+
+*conj_width_p = conj_width;
+*conj_depth_p = conj_depth;
+}
+
+static void
+adjust_conjunction_count(struct ovn_desired_flow_table *desired_flows,
+ const struct ovn_flow *f, bool add)
+{
+uint32_t conj_width;
+uint32_t conj_depth;
+
+get_conjunction_dimensions(f, _width, _depth);
+
+if (add) {
+if (conj_width > desired_flows->max_conj_width) {
+desired_flows->max_conj_width = conj_width;
+}
+if (conj_depth > desired_flows->max_conj_depth) {
+desired_flows->max_conj_depth = conj_depth;
+}
+} else if (desired_flows->max_conj_width <= conj_width ||
+   desired_flows->max_conj_depth <= conj_depth) {
+/* Removing either the widest or deepest conjunction.
+ * Walk the table and recalculate maximums
+ */
+struct ovn_flow *iter;
+desired_flows->max_conj_width = 0;
+desired_flows->max_conj_depth = 0;
+HMAP_FOR_EACH (iter, match_hmap_node,
+   _flows->match_flow_table) {
+get_conjunction_dimensions(iter, _width, _depth);
+if (conj_width > desired_flows->max_conj_width) {
+desired_flows->max_conj_width = conj_width;
+}
+if (conj_depth > desired_flows->max_conj_depth) {
+desired_flows->max_conj_depth = conj_depth;
+}
+}
+}
+}
+
 /* Removes a bundles of flows from the flow table. */
 void
 ofctrl_remove_flows(struct ovn_desired_flow_table *flow_table,
@@ -697,6 +755,7 @@ ofctrl_remove_flows(struct ovn_desired_flow_table 
*flow_table,
 _table->uuid_flow_table) {
 if (uuid_equals(>sb_uuid, sb_uuid)) {
 ovn_flow_log(f, "ofctrl_remove_flow");
+adjust_conjunction_count(flow_table, f, false);
 hmap_remove(_table->match_flow_table,
 >match_hmap_node);
 hindex_remove(_table->uuid_flow_table, >uuid_hindex_node);
@@ -747,6 +806,12 @@ ofctrl_add_or_append_flow(struct ovn_desired_flow_table 
*desired_flows,
 existing->ofpacts = xmemdup(compound.data, compound.size);
 existing->ofpacts_len = compound.size;
 
+/* It's a bit of a cheat that we only call 

Re: [ovs-dev] [PATCH v3 ovn] ovn-nb/sbctl.c: Use env variables for passing options.

[Ginwala, Aliasgar via dev]

Thanks Ben. I tested it with the command-line changes that you pushed to ovs.

From: Ben Pfaff 
Sent: Friday, October 25, 2019 1:32 PM
To: amgin...@gmail.com 
Cc: d...@openvswitch.org ; Ginwala, Aliasgar 

Subject: Re: [ovs-dev] [PATCH v3 ovn] ovn-nb/sbctl.c: Use env variables for 
passing options.

On Fri, Oct 25, 2019 at 01:10:57PM -0700, amgin...@gmail.com wrote:
> From: Aliasgar Ginwala 
>
> Add new env variables OVN_NBCTL_OPTIONS and OVN_SBCTL_OPTIONS for
> ovn-nbctl and ovn-sbctl respectively where user can set
> supported ovn-nb/sbctl options using environment variable.
> e.g. OVN_SBCTL_OPTIONS="--db=unix:sb1.ovsdb --no-leader-only"
>
> Signed-off-by: Aliasgar Ginwala 

I didn't test this.

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

Re: [ovs-dev] [PATCH v3 ovn] ovn-nb/sbctl.c: Use env variables for passing options.

[Ben Pfaff]

On Fri, Oct 25, 2019 at 01:10:57PM -0700, amgin...@gmail.com wrote:
> From: Aliasgar Ginwala 
> 
> Add new env variables OVN_NBCTL_OPTIONS and OVN_SBCTL_OPTIONS for
> ovn-nbctl and ovn-sbctl respectively where user can set
> supported ovn-nb/sbctl options using environment variable.
> e.g. OVN_SBCTL_OPTIONS="--db=unix:sb1.ovsdb --no-leader-only"
> 
> Signed-off-by: Aliasgar Ginwala 

I didn't test this.

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

Re: [ovs-dev] [PATCH v2 ovn] ovn-nb/sbctl.c: Use env variables for passing options.

[Ginwala, Aliasgar via dev]

Yeah makes sense since we are doing null check in command-line. Thanks for the 
review and suggestions. Sent v3 https://patchwork.ozlabs.org/patch/1184436/ 
PTAL.

From: Ben Pfaff 
Sent: Friday, October 25, 2019 11:24 AM
To: amgin...@gmail.com 
Cc: d...@openvswitch.org ; Ginwala, Aliasgar 

Subject: Re: [ovs-dev] [PATCH v2 ovn] ovn-nb/sbctl.c: Use env variables for 
passing options.

On Tue, Oct 15, 2019 at 06:52:52PM -0700, amgin...@gmail.com wrote:
> From: Aliasgar Ginwala 
>
> Add new env variables OVN_NBCTL_OPTIONS and OVN_SBCTL_OPTIONS for
> ovn-nbctl and ovn-sbctl respectively where user can set
> supported ovn-nb/sbctl options using environment variable.
> e.g. OVN_SBCTL_OPTIONS="--db=unix:sb1.ovsdb --no-leader-only"
>
> Signed-off-by: Aliasgar Ginwala 
> +/* Check if options are set via env var. */
> +char *ctl_options = getenv("OVN_NBCTL_OPTIONS");
> +char **argv;
> +int *argcp;
> +argcp = xmalloc(sizeof(int));
> +*argcp = argc;
> +argv = ovs_cmdl_env_parse_all(argcp, argv_,
> +  ctl_options);
> +if (!argv) {
> +/* This situation should never occur, but... */
> +ctl_fatal("Unable to read argv");
> +}
> +argc = *argcp;
> +free(argcp);

This seems to me to be more complicated than necessary.  Is the
following sufficient?

argv = ovs_cmdl_env_parse_all(, argv, getenv("OVN_NBCTL_OPTIONS"));

I don't know why the check for null argv is needed.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3 ovn] ovn-nb/sbctl.c: Use env variables for passing options.

[amginwal]

From: Aliasgar Ginwala 

Add new env variables OVN_NBCTL_OPTIONS and OVN_SBCTL_OPTIONS for
ovn-nbctl and ovn-sbctl respectively where user can set
supported ovn-nb/sbctl options using environment variable.
e.g. OVN_SBCTL_OPTIONS="--db=unix:sb1.ovsdb --no-leader-only"

Signed-off-by: Aliasgar Ginwala 
---
 utilities/ovn-nbctl.8.xml | 25 +
 utilities/ovn-nbctl.c |  3 +++
 utilities/ovn-sbctl.8.in  |  8 
 utilities/ovn-sbctl.c |  3 +++
 4 files changed, 39 insertions(+)

diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml
index fd75c0e44..b207dac46 100644
--- a/utilities/ovn-nbctl.8.xml
+++ b/utilities/ovn-nbctl.8.xml
@@ -1178,6 +1178,31 @@
   wait at all.  Use the sync command to override this
   behavior.
 
+
+
+  User can set one or more OVN_NBCTL_OPTIONS options in
+  environment variable. Under the Bourne shell this might be
+  done like this:
+
+
+
+  OVN_NBCTL_OPTIONS="--db=unix:nb1.ovsdb --no-leader-only"
+
+
+
+  When OVN_NBCTL_OPTIONS is set, ovn-nbctl
+  automatically and transparently uses the environment variable to
+  execute its commands. However user can still over-ride environment
+  options by passing different in cli.
+
+
+
+  When the environment variable is no longer needed, unset it, e.g.:
+
+
+
+  unset OVN_NBCTL_OPTIONS
+
   
 
 --db database
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index a89a9cb4d..9bc465209 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -136,6 +136,9 @@ main(int argc, char *argv[])
 
 nbctl_cmd_init();
 
+/* Check if options are set via env var. */
+argv = ovs_cmdl_env_parse_all(, argv, getenv("OVN_NBCTL_OPTIONS"));
+
 /* ovn-nbctl has three operation modes:
  *
  *- Direct: Executes commands by contacting ovsdb-server directly.
diff --git a/utilities/ovn-sbctl.8.in b/utilities/ovn-sbctl.8.in
index 2aaa457e8..b3c21d625 100644
--- a/utilities/ovn-sbctl.8.in
+++ b/utilities/ovn-sbctl.8.in
@@ -93,6 +93,14 @@ to approximately \fIsecs\fR seconds.  If the timeout expires,
 would normally happen only if the database cannot be contacted, or if
 the system is overloaded.)
 .
+.IP "\fBOVN_SBCTL_OPTIONS\fR"
+.User can set one or more options using \fBOVN_SBCTL_OPTIONS\fR environment
+.variable. Under the Bourne shell this might be done like this:
+.export \fBOVN_SBCTL_OPTIONS\fR"="--db=unix:sb1.ovsdb --no-leader-only".
+.However user can still over-ride environment options by passing different
+.options in cli. When the environment variable is no longer needed, unset it,
+.e.g.: unset \fBOVN_SBCTL_OPTIONS\fR"
+.
 .so lib/vlog.man
 .so lib/common.man
 .
diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c
index 9a9b6f0ec..ffcaee2c4 100644
--- a/utilities/ovn-sbctl.c
+++ b/utilities/ovn-sbctl.c
@@ -108,6 +108,9 @@ main(int argc, char *argv[])
 
 sbctl_cmd_init();
 
+/* Check if options are set via env var. */
+argv = ovs_cmdl_env_parse_all(, argv, getenv("OVN_SBCTL_OPTIONS"));
+
 /* Parse command line. */
 char *args = process_escape_args(argv);
 shash_init(_options);
-- 
2.20.1 (Apple Git-117)

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

Re: [ovs-dev] [PATCH v3] command-line.c: Support parsing ctl options via env variable

[0-day Robot]

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

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


git-am:
Failed to merge in the changes.
Patch failed at 0001 command-line.c: Support parsing ctl options via env 
variable
The copy of the patch that failed is found in:
   
/var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
When you have resolved this problem, run "git am --resolved".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


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

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

Re: [ovs-dev] [PATCH v3] command-line.c: Support parsing ctl options via env variable

[Ben Pfaff]

On Fri, Oct 25, 2019 at 12:33:52PM -0700, amgin...@gmail.com wrote:
> From: Aliasgar Ginwala 
> 
> Signed-off-by: Aliasgar Ginwala 

I tweaked this a little further and applied it to master as follows.

Thank you!

-8<--cut here-->8--

From: Aliasgar Ginwala 
Date: Fri, 25 Oct 2019 12:33:52 -0700
Subject: [PATCH] command-line: New function ovs_cmdl_env_parse_all().

This function allows an environment variable to be included in
command-line parsing.  It will receive its first user in an
upcoming commit.

Signed-off-by: Aliasgar Ginwala 
Signed-off-by: Ben Pfaff 
---
 lib/command-line.c | 24 
 lib/command-line.h |  3 +++
 2 files changed, 27 insertions(+)

diff --git a/lib/command-line.c b/lib/command-line.c
index 9e000bd28d1a..967f4f5d5846 100644
--- a/lib/command-line.c
+++ b/lib/command-line.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include "svec.h"
 #include "openvswitch/dynamic-string.h"
 #include "ovs-thread.h"
 #include "util.h"
@@ -77,6 +78,29 @@ find_option_by_value(const struct option *options, int value)
 return NULL;
 }
 
+/* Parses options set using environment variable.  The caller specifies the
+ * supported options in environment variable.  On success, adds the parsed
+ * env variables in 'argv', the number of options in 'argc', and returns argv.
+ *  */
+char **
+ovs_cmdl_env_parse_all(int *argcp, char *argv[], const char *env_options)
+{
+ovs_assert(*argcp > 0);
+
+struct svec args = SVEC_EMPTY_INITIALIZER;
+svec_add(, argv[0]);
+if (env_options) {
+svec_parse_words(, env_options);
+}
+for (int i = 1; i < *argcp; i++) {
+svec_add(, argv[i]);
+}
+svec_terminate();
+
+*argcp = args.n;
+return args.names;
+}
+
 /* Parses the command-line options in 'argc' and 'argv'.  The caller specifies
  * the supported options in 'options'.  On success, stores the parsed options
  * in '*pop', the number of options in '*n_pop', and returns NULL.  On failure,
diff --git a/lib/command-line.h b/lib/command-line.h
index 9d62dc2501c5..fc2a2690f688 100644
--- a/lib/command-line.h
+++ b/lib/command-line.h
@@ -54,6 +54,9 @@ char *ovs_cmdl_parse_all(int argc, char *argv[], const struct 
option *,
  struct ovs_cmdl_parsed_option **, size_t *)
 OVS_WARN_UNUSED_RESULT;
 
+char **ovs_cmdl_env_parse_all(int *argcp, char *argv_[],
+  const char *env_options);
+
 void ovs_cmdl_print_options(const struct option *options);
 void ovs_cmdl_print_commands(const struct ovs_cmdl_command *commands);
 
-- 
2.21.0

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

[ovs-dev] [PATCH v3] command-line.c: Support parsing ctl options via env variable

[amginwal]

From: Aliasgar Ginwala 

Signed-off-by: Aliasgar Ginwala 
---
 lib/command-line.c | 29 +
 lib/command-line.h |  3 +++
 2 files changed, 32 insertions(+)

diff --git a/lib/command-line.c b/lib/command-line.c
index 9e000bd28..2fc8b6e48 100644
--- a/lib/command-line.c
+++ b/lib/command-line.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include "svec.h"
 #include "openvswitch/dynamic-string.h"
 #include "ovs-thread.h"
 #include "util.h"
@@ -77,6 +78,34 @@ find_option_by_value(const struct option *options, int value)
 return NULL;
 }
 
+/* Parses options set using environment variable.  The caller specifies the
+ * supported options in environment variable.  On success, adds the parsed
+ * env variables in 'argv', the number of options in 'argc', and returns argv.
+ *  */
+char **
+ovs_cmdl_env_parse_all(int *argcp, char *argv[],
+   char *env_options)
+{
+struct svec args = SVEC_EMPTY_INITIALIZER;
+
+/* argv[0] stays in place. */
+ovs_assert(*argcp > 0);
+svec_add(, argv[0]);
+
+/* Anything from the environment variable goes next. */
+if (env_options) {
+svec_parse_words(, env_options);
+}
+
+/* Remaining command-line options go at the end. */
+for (int i = 1; i < *argcp; i++) {
+svec_add(, argv[i]);
+}
+
+*argcp = args.n;
+return args.names;
+}
+
 /* Parses the command-line options in 'argc' and 'argv'.  The caller specifies
  * the supported options in 'options'.  On success, stores the parsed options
  * in '*pop', the number of options in '*n_pop', and returns NULL.  On failure,
diff --git a/lib/command-line.h b/lib/command-line.h
index 9d62dc250..4b8f76da7 100644
--- a/lib/command-line.h
+++ b/lib/command-line.h
@@ -54,6 +54,9 @@ char *ovs_cmdl_parse_all(int argc, char *argv[], const struct 
option *,
  struct ovs_cmdl_parsed_option **, size_t *)
 OVS_WARN_UNUSED_RESULT;
 
+char **ovs_cmdl_env_parse_all(int *argcp, char *argv_[],
+  char *env_options);
+
 void ovs_cmdl_print_options(const struct option *options);
 void ovs_cmdl_print_commands(const struct ovs_cmdl_command *commands);
 
-- 
2.20.1 (Apple Git-117)

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

Re: [ovs-dev] [PATCH v2] command-line.c: Support parsing ctl options via env variable

[Ginwala, Aliasgar via dev]

Sure, let me change and test it. Will send out v3 addressing the same.

From: Ben Pfaff 
Sent: Friday, October 25, 2019 11:47 AM
To: Ilya Maximets 
Cc: ovs-dev@openvswitch.org ; Ginwala, Aliasgar 

Subject: Re: Re: [ovs-dev] [PATCH v2] command-line.c: Support parsing ctl 
options via env variable

On Fri, Oct 25, 2019 at 08:42:46PM +0200, Ilya Maximets wrote:
> > Thanks Ben: It works fine. Just a minor typo. It's much neat and trimmed 
> > now.
> >
> > I tried with multiple options e.g.
> > export OVN_NBCTL_OPTIONS="--db=unix:nb1.ovsdb --no-leader-only 
> > --leader-only --no-leader-only"
> >
> > and it gets called as expected:
> > Called as ovn-nbctl --db=unix:nb1.ovsdb --no-leader-only --leader-only 
> > --no-leader-only set-connection pssl:6641
> >
> > 
> > From: Ben Pfaff 
> > Sent: Friday, October 25, 2019 11:12 AM
> > To: amginwal at gmail.com 
> > Cc: dev at openvswitch.org ; Ginwala, Aliasgar 
> > 
> > Subject: Re: [ovs-dev] [PATCH v2] command-line.c: Support parsing ctl 
> > options via env variable
> >
> > On Thu, Oct 24, 2019 at 10:43:14PM -0700, amginwal at gmail.com wrote:
> > > From: Aliasgar Ginwala 
> > >
> > > Signed-off-by: Aliasgar Ginwala 
> >
> > To me, this looks more complicated than necessary, and thus harder to
> > reason about regarding correctness.  What about the following?  It does
> > not make me think as hard about correctness.  However, I have not tested
> > it.
> >
> > -8<--cut here-->8--
> >
> > From: Aliasgar Ginwala 
> > Date: Thu, 24 Oct 2019 22:43:14 -0700
> > Subject: [PATCH] command-line.c: Support parsing ctl options via env 
> > variable
> >
> > Signed-off-by: Aliasgar Ginwala 
> > Signed-off-by: Ben Pfaff 
> > ---
> >  lib/command-line.c | 34 ++
> >  lib/command-line.h |  3 +++
> >  2 files changed, 37 insertions(+)
> >
> > diff --git a/lib/command-line.c b/lib/command-line.c
> > index 9e000bd28d1a..fa02b49152e5 100644
> > --- a/lib/command-line.c
> > +++ b/lib/command-line.c
> > @@ -19,6 +19,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include "svec.h"
> >  #include "openvswitch/dynamic-string.h"
> >  #include "ovs-thread.h"
> >  #include "util.h"
> > @@ -77,6 +78,39 @@ find_option_by_value(const struct option *options, int 
> > value)
> >  return NULL;
> >  }
> >
> > +/* Parses options set using environment variable.  The caller specifies the
> > + * supported options in environment variable.  On success, adds the parsed
> > + * env variables in 'argv', the number of options in 'argc', and returns 
> > argv.
> > + */
> > +char **
> > +ovs_cmdl_env_parse_all(int *argcp, char *argv[], const char *env_options)
> > +{
> > +struct svec args = SVEC_EMPTY_INITIALIZER;
> > +
> > +/* argv[0] stays in place. */
> > +ovs_assert(*argcp > 0);
> > +svec_add(, argv[0]);
> > Should be argv_
> > +
> > +/* Anything from the environment variable goes next. */
> > +if (env_options) {
> > +char *env_copy = xstrdup(env_options);
> > +char *save_ptr = NULL;
> > +for (char *option = strtok_r(env_copy, " ", _ptr);
> > + option; option = strtok_r(NULL, " ", _ptr)) {
> > +svec_add(, option);
> > +}
> > +free(env_copy);
>
> Just curious, can we use svec_parse_words() instead of above loop?

That would actually be much better, since it handles quoting.

(I wrote svec_parse_words() but I forgot about it!)

Ali, can you make that change?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] command-line.c: Support parsing ctl options via env variable

[Ben Pfaff]

On Fri, Oct 25, 2019 at 08:42:46PM +0200, Ilya Maximets wrote:
> > Thanks Ben: It works fine. Just a minor typo. It's much neat and trimmed 
> > now.
> > 
> > I tried with multiple options e.g.
> > export OVN_NBCTL_OPTIONS="--db=unix:nb1.ovsdb --no-leader-only 
> > --leader-only --no-leader-only"
> > 
> > and it gets called as expected:
> > Called as ovn-nbctl --db=unix:nb1.ovsdb --no-leader-only --leader-only 
> > --no-leader-only set-connection pssl:6641
> > 
> > 
> > From: Ben Pfaff 
> > Sent: Friday, October 25, 2019 11:12 AM
> > To: amginwal at gmail.com 
> > Cc: dev at openvswitch.org ; Ginwala, Aliasgar 
> > 
> > Subject: Re: [ovs-dev] [PATCH v2] command-line.c: Support parsing ctl 
> > options via env variable
> > 
> > On Thu, Oct 24, 2019 at 10:43:14PM -0700, amginwal at gmail.com wrote:
> > > From: Aliasgar Ginwala 
> > > 
> > > Signed-off-by: Aliasgar Ginwala 
> > 
> > To me, this looks more complicated than necessary, and thus harder to
> > reason about regarding correctness.  What about the following?  It does
> > not make me think as hard about correctness.  However, I have not tested
> > it.
> > 
> > -8<--cut here-->8--
> > 
> > From: Aliasgar Ginwala 
> > Date: Thu, 24 Oct 2019 22:43:14 -0700
> > Subject: [PATCH] command-line.c: Support parsing ctl options via env 
> > variable
> > 
> > Signed-off-by: Aliasgar Ginwala 
> > Signed-off-by: Ben Pfaff 
> > ---
> >  lib/command-line.c | 34 ++
> >  lib/command-line.h |  3 +++
> >  2 files changed, 37 insertions(+)
> > 
> > diff --git a/lib/command-line.c b/lib/command-line.c
> > index 9e000bd28d1a..fa02b49152e5 100644
> > --- a/lib/command-line.c
> > +++ b/lib/command-line.c
> > @@ -19,6 +19,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include "svec.h"
> >  #include "openvswitch/dynamic-string.h"
> >  #include "ovs-thread.h"
> >  #include "util.h"
> > @@ -77,6 +78,39 @@ find_option_by_value(const struct option *options, int 
> > value)
> >  return NULL;
> >  }
> > 
> > +/* Parses options set using environment variable.  The caller specifies the
> > + * supported options in environment variable.  On success, adds the parsed
> > + * env variables in 'argv', the number of options in 'argc', and returns 
> > argv.
> > + */
> > +char **
> > +ovs_cmdl_env_parse_all(int *argcp, char *argv[], const char *env_options)
> > +{
> > +struct svec args = SVEC_EMPTY_INITIALIZER;
> > +
> > +/* argv[0] stays in place. */
> > +ovs_assert(*argcp > 0);
> > +svec_add(, argv[0]);
> > Should be argv_
> > +
> > +/* Anything from the environment variable goes next. */
> > +if (env_options) {
> > +char *env_copy = xstrdup(env_options);
> > +char *save_ptr = NULL;
> > +for (char *option = strtok_r(env_copy, " ", _ptr);
> > + option; option = strtok_r(NULL, " ", _ptr)) {
> > +svec_add(, option);
> > +}
> > +free(env_copy);
> 
> Just curious, can we use svec_parse_words() instead of above loop?

That would actually be much better, since it handles quoting.

(I wrote svec_parse_words() but I forgot about it!)

Ali, can you make that change?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] Avoid indeterminate statistics in offload implementations.

[Ben Pfaff]

A lot of the offload implementations didn't bother to initialize the
statistics they were supposed to return.  I don't know whether any of
the callers actually use them, but it looked wrong.

Found by inspection.

Signed-off-by: Ben Pfaff 
---
 lib/netdev-dummy.c| 10 --
 lib/netdev-offload-dpdk.c | 10 --
 lib/netdev-offload-tc.c   |  5 -
 3 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index 95e1a329a908..71df29184d9b 100644
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -1434,7 +1434,7 @@ netdev_dummy_flow_put(struct netdev *netdev, struct match 
*match,
   struct nlattr *actions OVS_UNUSED,
   size_t actions_len OVS_UNUSED,
   const ovs_u128 *ufid, struct offload_info *info,
-  struct dpif_flow_stats *stats OVS_UNUSED)
+  struct dpif_flow_stats *stats)
 {
 struct netdev_dummy *dev = netdev_dummy_cast(netdev);
 struct offloaded_flow *off_flow;
@@ -1476,12 +1476,15 @@ netdev_dummy_flow_put(struct netdev *netdev, struct 
match *match,
 ds_destroy();
 }
 
+if (stats) {
+memset(stats, 0, sizeof *stats);
+}
 return 0;
 }
 
 static int
 netdev_dummy_flow_del(struct netdev *netdev, const ovs_u128 *ufid,
-  struct dpif_flow_stats *stats OVS_UNUSED)
+  struct dpif_flow_stats *stats)
 {
 struct netdev_dummy *dev = netdev_dummy_cast(netdev);
 struct offloaded_flow *off_flow;
@@ -1521,6 +1524,9 @@ exit:
 ds_destroy();
 }
 
+if (stats) {
+memset(stats, 0, sizeof *stats);
+}
 return error ? -1 : 0;
 }
 
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 01e900461adf..96794dc4dc8b 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -710,7 +710,7 @@ static int
 netdev_offload_dpdk_flow_put(struct netdev *netdev, struct match *match,
  struct nlattr *actions, size_t actions_len,
  const ovs_u128 *ufid, struct offload_info *info,
- struct dpif_flow_stats *stats OVS_UNUSED)
+ struct dpif_flow_stats *stats)
 {
 struct rte_flow *rte_flow;
 int ret;
@@ -732,13 +732,16 @@ netdev_offload_dpdk_flow_put(struct netdev *netdev, 
struct match *match,
 return ret;
 }
 
+if (stats) {
+memset(stats, 0, sizeof *stats);
+}
 return netdev_offload_dpdk_add_flow(netdev, match, actions,
 actions_len, ufid, info);
 }
 
 static int
 netdev_offload_dpdk_flow_del(struct netdev *netdev, const ovs_u128 *ufid,
- struct dpif_flow_stats *stats OVS_UNUSED)
+ struct dpif_flow_stats *stats)
 {
 struct rte_flow *rte_flow = ufid_to_rte_flow_find(ufid);
 
@@ -746,6 +749,9 @@ netdev_offload_dpdk_flow_del(struct netdev *netdev, const 
ovs_u128 *ufid,
 return -1;
 }
 
+if (stats) {
+memset(stats, 0, sizeof *stats);
+}
 return netdev_offload_dpdk_destroy_flow(netdev, ufid, rte_flow);
 }
 
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index f6d1abb2e695..c5b9cbdb2bfe 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -1143,7 +1143,7 @@ static int
 netdev_tc_flow_put(struct netdev *netdev, struct match *match,
struct nlattr *actions, size_t actions_len,
const ovs_u128 *ufid, struct offload_info *info,
-   struct dpif_flow_stats *stats OVS_UNUSED)
+   struct dpif_flow_stats *stats)
 {
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
 enum tc_qdisc_hook hook = get_tc_qdisc_hook(netdev);
@@ -1448,6 +1448,9 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
*match,
 
 err = tc_replace_flower(ifindex, prio, handle, , block_id, hook);
 if (!err) {
+if (stats) {
+memset(stats, 0, sizeof *stats);
+}
 add_ufid_tc_mapping(ufid, flower.prio, flower.handle, netdev, ifindex);
 }
 
-- 
2.21.0

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

Re: [ovs-dev] [PATCH v2] command-line.c: Support parsing ctl options via env variable

[Ilya Maximets]

Thanks Ben: It works fine. Just a minor typo. It's much neat and trimmed now.

I tried with multiple options e.g.
export OVN_NBCTL_OPTIONS="--db=unix:nb1.ovsdb --no-leader-only --leader-only 
--no-leader-only"

and it gets called as expected:
Called as ovn-nbctl --db=unix:nb1.ovsdb --no-leader-only --leader-only 
--no-leader-only set-connection pssl:6641


From: Ben Pfaff 
Sent: Friday, October 25, 2019 11:12 AM
To: amginwal at gmail.com 
Cc: dev at openvswitch.org ; Ginwala, Aliasgar 
Subject: Re: [ovs-dev] [PATCH v2] command-line.c: Support parsing ctl options 
via env variable

On Thu, Oct 24, 2019 at 10:43:14PM -0700, amginwal at gmail.com wrote:

From: Aliasgar Ginwala 

Signed-off-by: Aliasgar Ginwala 


To me, this looks more complicated than necessary, and thus harder to
reason about regarding correctness.  What about the following?  It does
not make me think as hard about correctness.  However, I have not tested
it.

-8<--cut here-->8--

From: Aliasgar Ginwala 
Date: Thu, 24 Oct 2019 22:43:14 -0700
Subject: [PATCH] command-line.c: Support parsing ctl options via env variable

Signed-off-by: Aliasgar Ginwala 
Signed-off-by: Ben Pfaff 
---
 lib/command-line.c | 34 ++
 lib/command-line.h |  3 +++
 2 files changed, 37 insertions(+)

diff --git a/lib/command-line.c b/lib/command-line.c
index 9e000bd28d1a..fa02b49152e5 100644
--- a/lib/command-line.c
+++ b/lib/command-line.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include "svec.h"
 #include "openvswitch/dynamic-string.h"
 #include "ovs-thread.h"
 #include "util.h"
@@ -77,6 +78,39 @@ find_option_by_value(const struct option *options, int value)
 return NULL;
 }

+/* Parses options set using environment variable.  The caller specifies the
+ * supported options in environment variable.  On success, adds the parsed
+ * env variables in 'argv', the number of options in 'argc', and returns argv.
+ */
+char **
+ovs_cmdl_env_parse_all(int *argcp, char *argv[], const char *env_options)
+{
+struct svec args = SVEC_EMPTY_INITIALIZER;
+
+/* argv[0] stays in place. */
+ovs_assert(*argcp > 0);
+svec_add(, argv[0]);
Should be argv_
+
+/* Anything from the environment variable goes next. */
+if (env_options) {
+char *env_copy = xstrdup(env_options);
+char *save_ptr = NULL;
+for (char *option = strtok_r(env_copy, " ", _ptr);
+ option; option = strtok_r(NULL, " ", _ptr)) {
+svec_add(, option);
+}
+free(env_copy);


Just curious, can we use svec_parse_words() instead of above loop?

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

Re: [ovs-dev] [PATCH v2] command-line.c: Support parsing ctl options via env variable

[Ginwala, Aliasgar via dev]

Thanks Ben: It works fine. Just a minor typo. It's much neat and trimmed now.

I tried with multiple options e.g.
export OVN_NBCTL_OPTIONS="--db=unix:nb1.ovsdb --no-leader-only --leader-only 
--no-leader-only"

and it gets called as expected:
Called as ovn-nbctl --db=unix:nb1.ovsdb --no-leader-only --leader-only 
--no-leader-only set-connection pssl:6641


From: Ben Pfaff 
Sent: Friday, October 25, 2019 11:12 AM
To: amgin...@gmail.com 
Cc: d...@openvswitch.org ; Ginwala, Aliasgar 

Subject: Re: [ovs-dev] [PATCH v2] command-line.c: Support parsing ctl options 
via env variable

On Thu, Oct 24, 2019 at 10:43:14PM -0700, amgin...@gmail.com wrote:
> From: Aliasgar Ginwala 
>
> Signed-off-by: Aliasgar Ginwala 

To me, this looks more complicated than necessary, and thus harder to
reason about regarding correctness.  What about the following?  It does
not make me think as hard about correctness.  However, I have not tested
it.

-8<--cut here-->8--

From: Aliasgar Ginwala 
Date: Thu, 24 Oct 2019 22:43:14 -0700
Subject: [PATCH] command-line.c: Support parsing ctl options via env variable

Signed-off-by: Aliasgar Ginwala 
Signed-off-by: Ben Pfaff 
---
 lib/command-line.c | 34 ++
 lib/command-line.h |  3 +++
 2 files changed, 37 insertions(+)

diff --git a/lib/command-line.c b/lib/command-line.c
index 9e000bd28d1a..fa02b49152e5 100644
--- a/lib/command-line.c
+++ b/lib/command-line.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include "svec.h"
 #include "openvswitch/dynamic-string.h"
 #include "ovs-thread.h"
 #include "util.h"
@@ -77,6 +78,39 @@ find_option_by_value(const struct option *options, int value)
 return NULL;
 }

+/* Parses options set using environment variable.  The caller specifies the
+ * supported options in environment variable.  On success, adds the parsed
+ * env variables in 'argv', the number of options in 'argc', and returns argv.
+ */
+char **
+ovs_cmdl_env_parse_all(int *argcp, char *argv[], const char *env_options)
+{
+struct svec args = SVEC_EMPTY_INITIALIZER;
+
+/* argv[0] stays in place. */
+ovs_assert(*argcp > 0);
+svec_add(, argv[0]);
Should be argv_
+
+/* Anything from the environment variable goes next. */
+if (env_options) {
+char *env_copy = xstrdup(env_options);
+char *save_ptr = NULL;
+for (char *option = strtok_r(env_copy, " ", _ptr);
+ option; option = strtok_r(NULL, " ", _ptr)) {
+svec_add(, option);
+}
+free(env_copy);
+}
+
+/* Remaining command-line options go at the end. */
+for (int i = 1; i < *argcp; i++) {
+svec_add(, argv[i]);
should be argv_
+}
+
+*argcp = args.n;
+return args.names;
+}
+
 /* Parses the command-line options in 'argc' and 'argv'.  The caller specifies
  * the supported options in 'options'.  On success, stores the parsed options
  * in '*pop', the number of options in '*n_pop', and returns NULL.  On failure,
diff --git a/lib/command-line.h b/lib/command-line.h
index 9d62dc2501c5..dc7ec36eae6c 100644
--- a/lib/command-line.h
+++ b/lib/command-line.h
@@ -54,6 +54,9 @@ char *ovs_cmdl_parse_all(int argc, char *argv[], const struct 
option *,
  struct ovs_cmdl_parsed_option **, size_t *)
 OVS_WARN_UNUSED_RESULT;

+char **ovs_cmdl_env_parse_all(int *argcp, char *argv[],
+  const char *env_options);
+
 void ovs_cmdl_print_options(const struct option *options);
 void ovs_cmdl_print_commands(const struct ovs_cmdl_command *commands);

--
2.21.0

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

Re: [ovs-dev] [PATCH v3 ovn 1/2] Add RDNSS support to OVN

[Ben Pfaff]

On Fri, Oct 25, 2019 at 11:51:42PM +0530, Numan Siddique wrote:
> On Fri, Oct 25, 2019 at 11:15 PM Ben Pfaff  wrote:
> 
> > On Fri, Oct 25, 2019 at 03:45:49PM +0200, Lorenzo Bianconi wrote:
> > > +/* RDNSS option RFC 6106 */
> > > +#define ND_RDNSS_OPT_LEN8
> > > +#define ND_OPT_RDNSS25
> > > +struct nd_rdnss_opt {
> > > +uint8_t type; /* ND_OPT_RDNSS. */
> > > +uint8_t len;  /* >= 3. */
> > > +ovs_be16 reserved;/* Always 0. */
> > > +ovs_16aligned_be32 lifetime;
> > > +const ovs_be128 dns[0];
> > > +};
> > > +BUILD_ASSERT_DECL(ND_RDNSS_OPT_LEN == sizeof(struct nd_rdnss_opt));
> >
> > This structure is a little odd.  The use of ovs_16aligned_be32 implies
> > that it can be 16-bit aligned, but ovs_be128 implies that it must be
> > 64-bit aligned.
> >
> 
> Would it work if we remove ovs_be128 from the struct and then when adding
> this option
> to the IPv6 RA packet, copy the IPv6 address directly to dp_packet buffer ?

That seems fine.

If the addresses are going at the end of the buffer, you can use
dp_packet_put() to append them directly.

> How ever if you think we need to have ovs_16aligned_be128 type, does it
> makes sense to add this
> structure and corresponding function - packet_put_ra_rdnss_opt to
> lib/packets.c of ovs repo ?

ovs_16aligned_be128 would probably go naturally in openvswitch/types.h
in OVS, alongside the other similar types.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 ovn] ovn-nb/sbctl.c: Use env variables for passing options.

[Ben Pfaff]

On Tue, Oct 15, 2019 at 06:52:52PM -0700, amgin...@gmail.com wrote:
> From: Aliasgar Ginwala 
> 
> Add new env variables OVN_NBCTL_OPTIONS and OVN_SBCTL_OPTIONS for
> ovn-nbctl and ovn-sbctl respectively where user can set
> supported ovn-nb/sbctl options using environment variable.
> e.g. OVN_SBCTL_OPTIONS="--db=unix:sb1.ovsdb --no-leader-only"
> 
> Signed-off-by: Aliasgar Ginwala 
> +/* Check if options are set via env var. */
> +char *ctl_options = getenv("OVN_NBCTL_OPTIONS");
> +char **argv;
> +int *argcp;
> +argcp = xmalloc(sizeof(int));
> +*argcp = argc;
> +argv = ovs_cmdl_env_parse_all(argcp, argv_,
> +  ctl_options);
> +if (!argv) {
> +/* This situation should never occur, but... */
> +ctl_fatal("Unable to read argv");
> +}
> +argc = *argcp;
> +free(argcp);

This seems to me to be more complicated than necessary.  Is the
following sufficient?

argv = ovs_cmdl_env_parse_all(, argv, getenv("OVN_NBCTL_OPTIONS"));

I don't know why the check for null argv is needed.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 ovn 1/2] Add RDNSS support to OVN

[Numan Siddique]

On Fri, Oct 25, 2019 at 11:15 PM Ben Pfaff  wrote:

> On Fri, Oct 25, 2019 at 03:45:49PM +0200, Lorenzo Bianconi wrote:
> > +/* RDNSS option RFC 6106 */
> > +#define ND_RDNSS_OPT_LEN8
> > +#define ND_OPT_RDNSS25
> > +struct nd_rdnss_opt {
> > +uint8_t type; /* ND_OPT_RDNSS. */
> > +uint8_t len;  /* >= 3. */
> > +ovs_be16 reserved;/* Always 0. */
> > +ovs_16aligned_be32 lifetime;
> > +const ovs_be128 dns[0];
> > +};
> > +BUILD_ASSERT_DECL(ND_RDNSS_OPT_LEN == sizeof(struct nd_rdnss_opt));
>
> This structure is a little odd.  The use of ovs_16aligned_be32 implies
> that it can be 16-bit aligned, but ovs_be128 implies that it must be
> 64-bit aligned.
>

Would it work if we remove ovs_be128 from the struct and then when adding
this option
to the IPv6 RA packet, copy the IPv6 address directly to dp_packet buffer ?

How ever if you think we need to have ovs_16aligned_be128 type, does it
makes sense to add this
structure and corresponding function - packet_put_ra_rdnss_opt to
lib/packets.c of ovs repo ?

Right now only OVN needs this function.

Thanks
Numan


> We might need a new ovs_16aligned_be128 type.  It seems to be missing
> from our menagerie so far.
> ___
> 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] command-line.c: Support parsing ctl options via env variable

[Ben Pfaff]

On Thu, Oct 24, 2019 at 10:43:14PM -0700, amgin...@gmail.com wrote:
> From: Aliasgar Ginwala 
> 
> Signed-off-by: Aliasgar Ginwala 

To me, this looks more complicated than necessary, and thus harder to
reason about regarding correctness.  What about the following?  It does
not make me think as hard about correctness.  However, I have not tested
it.

-8<--cut here-->8--

From: Aliasgar Ginwala 
Date: Thu, 24 Oct 2019 22:43:14 -0700
Subject: [PATCH] command-line.c: Support parsing ctl options via env variable

Signed-off-by: Aliasgar Ginwala 
Signed-off-by: Ben Pfaff 
---
 lib/command-line.c | 34 ++
 lib/command-line.h |  3 +++
 2 files changed, 37 insertions(+)

diff --git a/lib/command-line.c b/lib/command-line.c
index 9e000bd28d1a..fa02b49152e5 100644
--- a/lib/command-line.c
+++ b/lib/command-line.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include "svec.h"
 #include "openvswitch/dynamic-string.h"
 #include "ovs-thread.h"
 #include "util.h"
@@ -77,6 +78,39 @@ find_option_by_value(const struct option *options, int value)
 return NULL;
 }
 
+/* Parses options set using environment variable.  The caller specifies the
+ * supported options in environment variable.  On success, adds the parsed
+ * env variables in 'argv', the number of options in 'argc', and returns argv.
+ */
+char **
+ovs_cmdl_env_parse_all(int *argcp, char *argv[], const char *env_options)
+{
+struct svec args = SVEC_EMPTY_INITIALIZER;
+
+/* argv[0] stays in place. */
+ovs_assert(*argcp > 0);
+svec_add(, argv[0]);
+
+/* Anything from the environment variable goes next. */
+if (env_options) {
+char *env_copy = xstrdup(env_options);
+char *save_ptr = NULL;
+for (char *option = strtok_r(env_copy, " ", _ptr);
+ option; option = strtok_r(NULL, " ", _ptr)) {
+svec_add(, option);
+}
+free(env_copy);
+}
+
+/* Remaining command-line options go at the end. */
+for (int i = 1; i < *argcp; i++) {
+svec_add(, argv[i]);
+}
+
+*argcp = args.n;
+return args.names;
+}
+
 /* Parses the command-line options in 'argc' and 'argv'.  The caller specifies
  * the supported options in 'options'.  On success, stores the parsed options
  * in '*pop', the number of options in '*n_pop', and returns NULL.  On failure,
diff --git a/lib/command-line.h b/lib/command-line.h
index 9d62dc2501c5..dc7ec36eae6c 100644
--- a/lib/command-line.h
+++ b/lib/command-line.h
@@ -54,6 +54,9 @@ char *ovs_cmdl_parse_all(int argc, char *argv[], const struct 
option *,
  struct ovs_cmdl_parsed_option **, size_t *)
 OVS_WARN_UNUSED_RESULT;
 
+char **ovs_cmdl_env_parse_all(int *argcp, char *argv[],
+  const char *env_options);
+
 void ovs_cmdl_print_options(const struct option *options);
 void ovs_cmdl_print_commands(const struct ovs_cmdl_command *commands);
 
-- 
2.21.0

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

Re: [ovs-dev] [PATCH branch-2.10] compat: Remove unused function

[Ben Pfaff]

On Fri, Oct 25, 2019 at 12:13:36PM +0200, Simon Horman wrote:
> From: Greg Rose 
> 
> The compat function rpl_nf_conntrack_in() does not appear to be used
> anywhere and emits warnings as such during builds < 4.10.
> 
> The patch passes Travis:
> 
> https://travis-ci.org/gvrose8192/ovs-experimental/builds/423097292
> 
> Remove it.

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

Re: [ovs-dev] [PATCH v2] ovsdb-server: fix memory leak while converting database

[Ben Pfaff]

On Fri, Oct 25, 2019 at 02:22:58PM +0200, Damijan Skvarc wrote:
> Hi Ben & thanks for looking into this.
> 
> I can confirm your suggestion works ok and prevents freeing the same
> memory twice. Thanks for finding this possibility. Otherwise I would do
> this in some other way, but not all people are the same :).

Thanks for confirming.  I applied this to master and backported as far
as branch-2.9.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 ovn 1/2] Add RDNSS support to OVN

[Ben Pfaff]

On Fri, Oct 25, 2019 at 03:45:49PM +0200, Lorenzo Bianconi wrote:
> +/* RDNSS option RFC 6106 */
> +#define ND_RDNSS_OPT_LEN8
> +#define ND_OPT_RDNSS25
> +struct nd_rdnss_opt {
> +uint8_t type; /* ND_OPT_RDNSS. */
> +uint8_t len;  /* >= 3. */
> +ovs_be16 reserved;/* Always 0. */
> +ovs_16aligned_be32 lifetime;
> +const ovs_be128 dns[0];
> +};
> +BUILD_ASSERT_DECL(ND_RDNSS_OPT_LEN == sizeof(struct nd_rdnss_opt));

This structure is a little odd.  The use of ovs_16aligned_be32 implies
that it can be 16-bit aligned, but ovs_be128 implies that it must be
64-bit aligned.

We might need a new ovs_16aligned_be128 type.  It seems to be missing
from our menagerie so far.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [RFC ovn 2/2] northd: add logical flows for dhcpv6 pfd parsing

[Lorenzo Bianconi]

Introduce logical flows in ovn router pipeline in order to parse dhcpv6
advertise/reply from IPv6 prefix delegation router.

Signed-off-by: Lorenzo Bianconi 
---
 northd/ovn-northd.c | 61 -
 1 file changed, 60 insertions(+), 1 deletion(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index ea8ad7c2d..d2a545b2b 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -2570,6 +2570,8 @@ ovn_port_update_sbrec(struct northd_context *ctx,
   struct sset *active_ha_chassis_grps)
 {
 sbrec_port_binding_set_datapath(op->sb, op->od->sb);
+const char *ipv6_pd_list = NULL;
+
 if (op->nbrp) {
 /* If the router is for l3 gateway, it resides on a chassis
  * and its port type is "l3gateway". */
@@ -2692,6 +2694,12 @@ ovn_port_update_sbrec(struct northd_context *ctx,
 smap_add(, "l3gateway-chassis", chassis_name);
 }
 }
+
+ipv6_pd_list = smap_get(>sb->options, "ipv6_ra_pd_list");
+if (ipv6_pd_list) {
+smap_add(, "ipv6_ra_pd_list", ipv6_pd_list);
+}
+
 sbrec_port_binding_set_options(op->sb, );
 smap_destroy();
 
@@ -2741,6 +2749,12 @@ ovn_port_update_sbrec(struct northd_context *ctx,
 smap_add_format(,
 "qdisc_queue_id", "%d", queue_id);
 }
+
+ipv6_pd_list = smap_get(>sb->options, "ipv6_ra_pd_list");
+if (ipv6_pd_list) {
+smap_add(, "ipv6_ra_pd_list", ipv6_pd_list);
+}
+
 sbrec_port_binding_set_options(op->sb, );
 smap_destroy();
 if (ovn_is_known_nb_lsp_type(op->nbsp->type)) {
@@ -2790,6 +2804,12 @@ ovn_port_update_sbrec(struct northd_context *ctx,
 if (chassis) {
 smap_add(, "l3gateway-chassis", chassis);
 }
+
+ipv6_pd_list = smap_get(>sb->options, "ipv6_ra_pd_list");
+if (ipv6_pd_list) {
+smap_add(, "ipv6_ra_pd_list", ipv6_pd_list);
+}
+
 sbrec_port_binding_set_options(op->sb, );
 smap_destroy();
 } else {
@@ -5385,7 +5405,8 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,
 }
 
 if (!op->nbsp->dhcpv4_options && !op->nbsp->dhcpv6_options) {
-/* CMS has disabled both native DHCPv4 and DHCPv6 for this lport.
+/* CMS has disabled native DHCPv4, DHCPv6 and prefix
+ * delegation for this lport.
  */
 continue;
 }
@@ -7055,6 +7076,31 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
 free(snat_ips);
 }
 
+/* DHCPv6 reply handling */
+HMAP_FOR_EACH (op, key_node, ports) {
+if (!op->nbrp) {
+continue;
+}
+
+struct lport_addresses lrp_networks;
+if (!extract_lrp_networks(op->nbrp, _networks)) {
+continue;
+}
+
+for (size_t i = 0; i < lrp_networks.n_ipv6_addrs; i++) {
+ds_clear();
+ds_clear();
+ds_put_format(, "inport == %s && ip6.dst == %s"
+  " && udp.src == 547 && udp.dst == 546",
+  op->json_key, lrp_networks.ipv6_addrs[i].addr_s);
+ds_put_format(, "reg0 = 0; dhcp6_server_pkt { "
+  "eth.dst <-> eth.src; ip6.dst <-> ip6.src; "
+  "outport <-> inport; output; };");
+ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 100,
+  ds_cstr(), ds_cstr());
+}
+}
+
 /* Logical router ingress table 1: IP Input for IPv6. */
 HMAP_FOR_EACH (op, key_node, ports) {
 if (!op->nbrp) {
@@ -7774,6 +7820,19 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
 continue;
 }
 
+/* enable IPv6 prefix delegation */
+bool prefix_delegation = smap_get_bool(>nbrp->options,
+   "prefix_delegation", false);
+if (prefix_delegation) {
+struct smap options;
+
+smap_clone(, >sb->options);
+smap_add(, "ipv6_prefix_delegation", "true");
+
+sbrec_port_binding_set_options(op->sb, );
+smap_destroy();
+}
+
 const char *address_mode = smap_get(
 >nbrp->ipv6_ra_configs, "address_mode");
 
-- 
2.21.0

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

[ovs-dev] [RFC ovn 1/2] controller: add ipv6 prefix delegation state machine

[Lorenzo Bianconi]

Introduce Ipv6 Prefix delegation state machine according to RFC 3633
https://tools.ietf.org/html/rfc3633.
Add dhcp6_server_pkt controller action to parse advertise/reply from
IPv6 delegation server. Advertise/reply are parsed running respectively:
- pinctrl_parse_dhcv6_advt
- pinctrl_parse_dhcv6_reply
The IPv6 requesting router starts sending dhcpv6 solicit through the logical
router ported marked with ipv6_prefix_delegationn set to true.
Save IPv6 prefix received by IPv6 delegation router in the options columns of
SB port binding table in order to be reused by Router Advertisement framework
run by ovn logical router pipeline

Signed-off-by: Lorenzo Bianconi 
---
 controller/pinctrl.c  | 543 ++
 include/ovn/actions.h |   9 +-
 lib/actions.c |  22 ++
 lib/ovn-l7.h  |  19 ++
 tests/ovn.at  |   6 +
 utilities/ovn-trace.c |   2 +
 6 files changed, 600 insertions(+), 1 deletion(-)

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index d826da186..7907ea8e0 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -269,6 +269,17 @@ static void pinctrl_ip_mcast_handle_igmp(
 const struct match *md,
 struct ofpbuf *userdata);
 
+static void init_ipv6_prefixd(void);
+static void destroy_ipv6_prefixd(void);
+static void ipv6_prefixd_wait(long long int timeout);
+static void
+prepare_ipv6_prefix_req(struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
+struct ovsdb_idl_index *sbrec_port_binding_by_name,
+const struct hmap *local_datapaths)
+OVS_REQUIRES(pinctrl_mutex);
+static void
+send_ipv6_prefix_msg(struct rconn *swconn, long long int *send_prefixd_time)
+OVS_REQUIRES(pinctrl_mutex);
 static bool may_inject_pkts(void);
 
 static void init_put_vport_bindings(void);
@@ -440,6 +451,7 @@ pinctrl_init(void)
 init_put_mac_bindings();
 init_send_garps_rarps();
 init_ipv6_ras();
+init_ipv6_prefixd();
 init_buffered_packets_map();
 init_event_table();
 ip_mcast_snoop_init();
@@ -526,6 +538,55 @@ set_actions_and_enqueue_msg(struct rconn *swconn,
 ofpbuf_uninit();
 }
 
+static struct shash ipv6_prefixd;
+
+enum {
+PREFIX_SOLICIT,
+PREFIX_PENDING,
+PREFIX_DONE,
+};
+
+struct ipv6_prefixd_state {
+long long int next_announce;
+struct in6_addr ipv6_addr;
+struct eth_addr ea;
+int64_t port_key;
+int64_t metadata;
+struct in6_addr prefix;
+unsigned plife_time;
+unsigned vlife_time;
+unsigned t1;
+unsigned t2;
+int8_t plen;
+int state;
+};
+
+static void
+init_ipv6_prefixd(void)
+{
+shash_init(_prefixd);
+}
+
+static void
+ipv6_prefixd_delete(struct ipv6_prefixd_state *pfd)
+{
+if (pfd) {
+free(pfd);
+}
+}
+
+static void
+destroy_ipv6_prefixd(void)
+{
+struct shash_node *iter, *next;
+SHASH_FOR_EACH_SAFE (iter, next, _prefixd) {
+struct ipv6_prefixd_state *pfd = iter->data;
+ipv6_prefixd_delete(pfd);
+shash_delete(_prefixd, iter);
+}
+shash_destroy(_prefixd);
+}
+
 struct buffer_info {
 struct ofpbuf ofpacts;
 struct dp_packet *p;
@@ -949,6 +1010,255 @@ pinctrl_handle_tcp_reset(struct rconn *swconn, const 
struct flow *ip_flow,
 dp_packet_uninit();
 }
 
+static void
+pinctrl_parse_dhcv6_advt(struct rconn *swconn, const struct flow *ip_flow,
+ struct dp_packet *pkt_in, const struct match *md,
+ struct ofpbuf *userdata)
+{
+struct udp_header *udp_in = dp_packet_l4(pkt_in);
+unsigned char *in_dhcpv6_data = (unsigned char *)(udp_in + 1);
+size_t dlen = MIN(ntohs(udp_in->udp_len), dp_packet_l4_size(pkt_in));
+uint8_t *data, *end = (uint8_t *)udp_in + dlen;
+int len = 0;
+
+data = xmalloc(dlen);
+if (!data) {
+return;
+}
+
+in_dhcpv6_data += 4;
+
+while (in_dhcpv6_data < end) {
+struct dhcpv6_opt_header *in_opt =
+ (struct dhcpv6_opt_header *)in_dhcpv6_data;
+int opt_len = sizeof *in_opt + ntohs(in_opt->len);
+
+if (dlen < opt_len + len) {
+goto out;
+}
+
+switch (ntohs(in_opt->code)) {
+case DHCPV6_OPT_IA_PD: {
+int orig_len = len, hdr_len = 0, size = sizeof *in_opt + 12;
+
+memcpy([len], in_opt, size);
+in_opt = (struct dhcpv6_opt_header *)(in_dhcpv6_data + size);
+len += size;
+
+while (size < opt_len) {
+int flen = sizeof *in_opt + ntohs(in_opt->len);
+
+if (dlen < flen + len) {
+goto out;
+}
+
+if (ntohs(in_opt->code) == DHCPV6_OPT_IA_PREFIX) {
+memcpy([len], in_opt, flen);
+hdr_len += flen;
+len += flen;
+}
+if (ntohs(in_opt->code) == DHCPV6_OPT_STATUS_CODE) {
+   struct dhcpv6_opt_status *status;
+
+  

[ovs-dev] [RFC ovn 0/2] Add IPv6 Prefix delegation (RFC3633)

[Lorenzo Bianconi]

Introduce Ipv6 Prefix delegation state machine according to RFC 3633
https://tools.ietf.org/html/rfc3633.
Add dhcp6_server_pkt controller action to parse advertise/reply from
IPv6 delegation server.
Introduce logical flows in ovn router pipeline in order to parse dhcpv6
advertise/reply from IPv6 prefix delegation router.

Lorenzo Bianconi (2):
  controller: add ipv6 prefix delegation state machine
  northd: add logical flows for dhcpv6 pfd parsing

 controller/pinctrl.c  | 543 ++
 include/ovn/actions.h |   9 +-
 lib/actions.c |  22 ++
 lib/ovn-l7.h  |  19 ++
 northd/ovn-northd.c   |  61 -
 tests/ovn.at  |   6 +
 utilities/ovn-trace.c |   2 +
 7 files changed, 660 insertions(+), 2 deletions(-)

-- 
2.21.0

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

Re: [ovs-dev] [PATCH] docs: To build OVS on RHEL7 EPEL is needed

[Ben Pfaff]

On Fri, Oct 25, 2019 at 05:41:22PM +0200, Timothy Redaelli wrote:
> Since Python 3 is now mandatory, Extra Packages for Enterprise Linux
> (EPEL) repository is needed in order to build OVS on RHEL7.
> 
> Signed-off-by: Timothy Redaelli 

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

Re: [ovs-dev] [PATCH] flow: Fix crash on vlan packets with partial offloading.

[Ilya Maximets]

On 24.10.2019 22:00, Ben Pfaff wrote:

On Thu, Oct 24, 2019 at 12:09:16PM +0200, Ilya Maximets wrote:

parse_tcp_flags() does not care about vlan tags in a packet thus
not able to parse them.  As a result, if partial offloading is
enabled in userspace datapath vlan packets are not parsed, i.e.
has no initialized offsets.  This causes OVS crash on any attempt
to access/modify packet header fields.


Thanks!  Good catch!

Acked-by: Ben Pfaff 


Thanks! Applied to master and backported down to 2.10.

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

Re: [ovs-dev] [PATCH] tests: Fix indentation in userspace packet type aware test.

[Ilya Maximets]

On 24.10.2019 17:40, Ben Pfaff wrote:

On Thu, Oct 24, 2019 at 02:41:48PM +0200, Ilya Maximets wrote:

CC: Ben Pfaff 
Fixes: 7be29a47576d ("ofproto-dpif: Remove tabs from output.")
Signed-off-by: Ilya Maximets 


If you've run the test and this makes it pass:
Acked-by: Ben Pfaff 


Yes, it does. Thanks!

Applied to master. Tested and backported down to 2.10.

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

Re: [ovs-dev] [PATCH] docs: To build OVS on RHEL7 EPEL is needed

[0-day Robot]

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

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


checkpatch:
WARNING: Line is 88 characters long (recommended limit is 79)
#28 FILE: Documentation/intro/install/fedora.rst:71:
$ yum install 
https://dl.fedoraproject.org/pub/epel/epel-release-latest-7.noarch.rpm

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


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

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

Re: [ovs-dev] [PATCH] dpif-netdev: Fix time delta overflow in case of race for meter lock.

[William Tu]

On Fri, Oct 25, 2019 at 4:44 AM Ilya Maximets  wrote:
>
> There is a race window between getting the time and getting the meter
> lock.  This could lead to situation where the thread with larger
> current time (this thread called time_{um}sec() later than others)
> will acquire meter lock first and update meter->used to the large
> value.  Next threads will try to calculate time delta by subtracting
> the large meter->used from their lower time getting the negative value
> which will be converted to a big unsigned delta.
>
> Fix that by assuming that all these threads received packets in the
> same time in this case, i.e. dropping negative delta to 0.
>
> CC: Jarno Rajahalme 
> Fixes: 4b27db644a8c ("dpif-netdev: Simple DROP meter implementation.")
> Reported-at: 
> https://mail.openvswitch.org/pipermail/ovs-dev/2019-September/363126.html
> Signed-off-by: Ilya Maximets 
> ---

LGTM.
Thanks for the fix
Acked-by: William Tu 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] docs: To build OVS on RHEL7 EPEL is needed

[Timothy Redaelli]

Since Python 3 is now mandatory, Extra Packages for Enterprise Linux
(EPEL) repository is needed in order to build OVS on RHEL7.

Signed-off-by: Timothy Redaelli 
---
 Documentation/intro/install/fedora.rst | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/intro/install/fedora.rst 
b/Documentation/intro/install/fedora.rst
index 6fe1fb5b2..c6d1d83ae 100644
--- a/Documentation/intro/install/fedora.rst
+++ b/Documentation/intro/install/fedora.rst
@@ -63,11 +63,12 @@ The command below will create a temporary SPEC file::
   > /tmp/ovs.spec
 
 And to install specific dependencies, use the corresponding tool below.
-For some of the dependencies on RHEL you may need to add two additional
+For some of the dependencies on RHEL 7 you may need to add three additional
 repositories to help yum-builddep, e.g.::
 
 $ subscription-manager repos --enable=rhel-7-server-extras-rpms
 $ subscription-manager repos --enable=rhel-7-server-optional-rpms
+$ yum install 
https://dl.fedoraproject.org/pub/epel/epel-release-latest-7.noarch.rpm
 
 DNF::
 
-- 
2.21.0

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

Re: [ovs-dev] [PATCH v10 3/3] ovs-bugtool: Script to collect the port statistics

[Kevin Traynor]

On 24/10/2019 19:21, Sriram Vatala wrote:
> Sometimes, analysing the drop statistics of the ports
> will be helpful in debugging. This patch adds script
> to collect all supported port stats which also includes
> the drop counters in userspace datapath. The output of
> this scirpt is included in the bugtool output.
> 

s/scirpt/script/

> Signed-off-by: Sriram Vatala 

Acked-by: Kevin Traynor 

> ---
>  utilities/bugtool/automake.mk |  3 ++-
>  utilities/bugtool/ovs-bugtool-get-port-stats  | 15 +++
>  .../plugins/network-status/openvswitch.xml|  1 +
>  3 files changed, 18 insertions(+), 1 deletion(-)
>  create mode 100755 utilities/bugtool/ovs-bugtool-get-port-stats
> 
> diff --git a/utilities/bugtool/automake.mk b/utilities/bugtool/automake.mk
> index 4c85b9cba..0a9b93088 100644
> --- a/utilities/bugtool/automake.mk
> +++ b/utilities/bugtool/automake.mk
> @@ -21,7 +21,8 @@ bugtool_scripts = \
>   utilities/bugtool/ovs-bugtool-ovs-bridge-datapath-type \
>   utilities/bugtool/ovs-bugtool-ovs-vswitchd-threads-affinity \
>   utilities/bugtool/ovs-bugtool-qos-configs \
> - utilities/bugtool/ovs-bugtool-get-dpdk-nic-numa
> + utilities/bugtool/ovs-bugtool-get-dpdk-nic-numa \
> + utilities/bugtool/ovs-bugtool-get-port-stats
>  
>  scripts_SCRIPTS += $(bugtool_scripts)
>  
> diff --git a/utilities/bugtool/ovs-bugtool-get-port-stats 
> b/utilities/bugtool/ovs-bugtool-get-port-stats
> new file mode 100755
> index 0..23e61034e
> --- /dev/null
> +++ b/utilities/bugtool/ovs-bugtool-get-port-stats
> @@ -0,0 +1,15 @@
> +#! /bin/bash
> +
> +#Iterate through each port of every bridge and print
> +#the port statistics
> +
> +for bridge in `ovs-vsctl -- --real list-br`
> +do
> +echo "${bridge} : "
> +echo "  ${bridge} : `ovs-vsctl get interface ${bridge} statistics`"
> +for iface in `ovs-vsctl list-ifaces ${bridge}`
> +do
> +echo "  ${iface} : `ovs-vsctl get interface ${iface} statistics`"
> +done
> +echo -e "\n"
> +done
> diff --git a/utilities/bugtool/plugins/network-status/openvswitch.xml 
> b/utilities/bugtool/plugins/network-status/openvswitch.xml
> index b0e7a1510..72aa44930 100644
> --- a/utilities/bugtool/plugins/network-status/openvswitch.xml
> +++ b/utilities/bugtool/plugins/network-status/openvswitch.xml
> @@ -41,4 +41,5 @@
>   repeat="2">/usr/share/openvswitch/scripts/ovs-bugtool-ovs-ofctl-loop-over-bridges
>  "dump-group-stats"
>   filters="ovs">/usr/share/openvswitch/scripts/ovs-bugtool-get-dpdk-nic-numa
>  ip -s -s link 
> show
> + filters="ovs">/usr/share/openvswitch/scripts/ovs-bugtool-get-port-stats
>  
> 

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

Re: [ovs-dev] [PATCH v10 2/3] netdev-dpdk : Detailed packet drop statistics

[Kevin Traynor]

On 24/10/2019 19:21, Sriram Vatala wrote:
> OVS may be unable to transmit packets for multiple reasons on
> the userspace datapath and today there is a single counter to
> track packets dropped due to any of those reasons. This patch
> adds custom software stats for the different reasons packets
> may be dropped during tx/rx on the userspace datapath in OVS.
> 
> - MTU drops : drops that occur due to a too large packet size
> - Qos drops : drops that occur due to egress/ingress QOS
> - Tx failures: drops as returned by the DPDK PMD send function
> 
> Note that the reason for tx failures is not specificied in OVS.

s/specificied/specified/

> In practice for vhost ports it is most common that tx failures
> are because there are not enough available descriptors,
> which is usually caused by misconfiguration of the guest queues
> and/or because the guest is not consuming packets fast enough
> from the queues.
> 
> These counters are displayed along with other stats in
> "ovs-vsctl get interface  statistics" command and are
> available for dpdk and vhostuser/vhostuserclient ports.
> 
> Also the existing "tx_retries" counter for vhost ports has been
> renamed to "ovs_tx_retries", so that all the custom statistics
> that OVS accumulates itself will have the prefix "ovs_". This
> will prevent any custom stats names overlapping with
> driver/HW stats.
> 
> Signed-off-by: Sriram Vatala 

Acked-by: Kevin Traynor 

> ---
>  Documentation/topics/dpdk/bridge.rst |  6 ++
>  Documentation/topics/dpdk/vhost-user.rst |  2 +-
>  lib/netdev-dpdk.c| 82 +++-
>  3 files changed, 72 insertions(+), 18 deletions(-)
> 
> diff --git a/Documentation/topics/dpdk/bridge.rst 
> b/Documentation/topics/dpdk/bridge.rst
> index d9bc7eba4..f0ef42ecc 100644
> --- a/Documentation/topics/dpdk/bridge.rst
> +++ b/Documentation/topics/dpdk/bridge.rst
> @@ -75,6 +75,12 @@ OpenFlow14`` option::
>  
>  $ ovs-ofctl -O OpenFlow14 dump-ports br0
>  
> +There are custom statistics that OVS accumulates itself and these stats has
> +``ovs_`` as prefix. These custom stats are shown along with other stats
> +using the following command::
> +
> +$ ovs-vsctl get Interface  statistics
> +
>  EMC Insertion Probability
>  -
>  
> diff --git a/Documentation/topics/dpdk/vhost-user.rst 
> b/Documentation/topics/dpdk/vhost-user.rst
> index cda5b122f..ec0caeb16 100644
> --- a/Documentation/topics/dpdk/vhost-user.rst
> +++ b/Documentation/topics/dpdk/vhost-user.rst
> @@ -551,7 +551,7 @@ processing packets at the required rate.
>  The amount of Tx retries on a vhost-user or vhost-user-client interface can 
> be
>  shown with::
>  
> -  $ ovs-vsctl get Interface dpdkvhostclient0 statistics:tx_retries
> +  $ ovs-vsctl get Interface dpdkvhostclient0 statistics:ovs_tx_retries
>  
>  vhost-user Dequeue Zero Copy (experimental)
>  ---
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 2cc2516a9..6922e61ca 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -174,6 +174,20 @@ static const struct vhost_device_ops 
> virtio_net_device_ops =
>  .destroy_connection = destroy_connection,
>  };
>  
> +/* Custom software stats for dpdk ports */
> +struct netdev_dpdk_sw_stats {
> +/* No. of retries when unable to transmit. */
> +uint64_t tx_retries;
> +/* Packet drops when unable to transmit; Probably Tx queue is full. */
> +uint64_t tx_failure_drops;
> +/* Packet length greater than device MTU. */
> +uint64_t tx_mtu_exceeded_drops;
> +/* Packet drops in egress policer processing. */
> +uint64_t tx_qos_drops;
> +/* Packet drops in ingress policer processing. */
> +uint64_t rx_qos_drops;
> +};
> +
>  enum { DPDK_RING_SIZE = 256 };
>  BUILD_ASSERT_DECL(IS_POW2(DPDK_RING_SIZE));
>  enum { DRAIN_TSC = 20ULL };
> @@ -416,11 +430,10 @@ struct netdev_dpdk {
>  
>  PADDED_MEMBERS(CACHE_LINE_SIZE,
>  struct netdev_stats stats;
> -/* Custom stat for retries when unable to transmit. */
> -uint64_t tx_retries;
> +struct netdev_dpdk_sw_stats *sw_stats;
>  /* Protects stats */
>  rte_spinlock_t stats_lock;
> -/* 4 pad bytes here. */
> +/* 36 pad bytes here. */
>  );
>  
>  PADDED_MEMBERS(CACHE_LINE_SIZE,
> @@ -1176,7 +1189,8 @@ common_construct(struct netdev *netdev, dpdk_port_t 
> port_no,
>  dev->rte_xstats_ids = NULL;
>  dev->rte_xstats_ids_size = 0;
>  
> -dev->tx_retries = (dev->type == DPDK_DEV_VHOST) ? 0 : UINT64_MAX;
> +dev->sw_stats = xzalloc(sizeof *dev->sw_stats);
> +dev->sw_stats->tx_retries = (dev->type == DPDK_DEV_VHOST) ? 0 : 
> UINT64_MAX;
>  
>  return 0;
>  }
> @@ -1362,6 +1376,7 @@ common_destruct(struct netdev_dpdk *dev)
>  ovs_list_remove(>list_node);
>  free(ovsrcu_get_protected(struct ingress_policer *,
>>ingress_policer));
> +free(dev->sw_stats);

Re: [ovs-dev] [PATCH v10 1/3] netdev-dpdk: Reuse vhost function for dpdk ETH custom stats.

[Kevin Traynor]

On 24/10/2019 19:21, Sriram Vatala wrote:
> From: Ilya Maximets 
> 
> This is yet another refactoring for upcoming detailed drop stats.
> It allowes to use single function for all the software calculated

s/allowes/allows

> statistics in netdev-dpdk for both vhost and ETH ports.
> 
> UINT64_MAX used as a marker for non-supported statistics in a
> same way as it's done in bridge.c for common netdev stats.
> 
> Co-authored-by: Sriram Vatala 
> Cc: Sriram Vatala 
> Signed-off-by: Ilya Maximets 
> Signed-off-by: Sriram Vatala 

Acked-by: Kevin Traynor 

> ---
>  lib/netdev-dpdk.c | 69 +++
>  1 file changed, 40 insertions(+), 29 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 04e1a2d1b..2cc2516a9 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -474,6 +474,8 @@ struct netdev_rxq_dpdk {
>  static void netdev_dpdk_destruct(struct netdev *netdev);
>  static void netdev_dpdk_vhost_destruct(struct netdev *netdev);
>  
> +static int netdev_dpdk_get_sw_custom_stats(const struct netdev *,
> +   struct netdev_custom_stats *);
>  static void netdev_dpdk_clear_xstats(struct netdev_dpdk *dev);
>  
>  int netdev_dpdk_get_vid(const struct netdev_dpdk *dev);
> @@ -1174,7 +1176,7 @@ common_construct(struct netdev *netdev, dpdk_port_t 
> port_no,
>  dev->rte_xstats_ids = NULL;
>  dev->rte_xstats_ids_size = 0;
>  
> -dev->tx_retries = 0;
> +dev->tx_retries = (dev->type == DPDK_DEV_VHOST) ? 0 : UINT64_MAX;
>  
>  return 0;
>  }
> @@ -2775,7 +2777,9 @@ netdev_dpdk_get_custom_stats(const struct netdev 
> *netdev,
>  
>  uint32_t i;
>  struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> -int rte_xstats_ret;
> +int rte_xstats_ret, sw_stats_size;
> +
> +netdev_dpdk_get_sw_custom_stats(netdev, custom_stats);
>  
>  ovs_mutex_lock(>mutex);
>  
> @@ -2790,23 +2794,22 @@ netdev_dpdk_get_custom_stats(const struct netdev 
> *netdev,
>  if (rte_xstats_ret > 0 &&
>  rte_xstats_ret <= dev->rte_xstats_ids_size) {
>  
> -custom_stats->size = rte_xstats_ret;
> -custom_stats->counters =
> -(struct netdev_custom_counter *) xcalloc(rte_xstats_ret,
> -sizeof(struct netdev_custom_counter));
> +sw_stats_size = custom_stats->size;
> +custom_stats->size += rte_xstats_ret;
> +custom_stats->counters = xrealloc(custom_stats->counters,
> +  custom_stats->size *
> +  sizeof 
> *custom_stats->counters);
>  
>  for (i = 0; i < rte_xstats_ret; i++) {
> -ovs_strlcpy(custom_stats->counters[i].name,
> +ovs_strlcpy(custom_stats->counters[sw_stats_size + i].name,
>  netdev_dpdk_get_xstat_name(dev,
> 
> dev->rte_xstats_ids[i]),
>  NETDEV_CUSTOM_STATS_NAME_SIZE);
> -custom_stats->counters[i].value = values[i];
> +custom_stats->counters[sw_stats_size + i].value = values[i];
>  }
>  } else {
>  VLOG_WARN("Cannot get XSTATS values for port: "DPDK_PORT_ID_FMT,
>dev->port_id);
> -custom_stats->counters = NULL;
> -custom_stats->size = 0;
>  /* Let's clear statistics cache, so it will be
>   * reconfigured */
>  netdev_dpdk_clear_xstats(dev);
> @@ -2821,39 +2824,47 @@ netdev_dpdk_get_custom_stats(const struct netdev 
> *netdev,
>  }
>  
>  static int
> -netdev_dpdk_vhost_get_custom_stats(const struct netdev *netdev,
> -   struct netdev_custom_stats *custom_stats)
> +netdev_dpdk_get_sw_custom_stats(const struct netdev *netdev,
> +struct netdev_custom_stats *custom_stats)
>  {
>  struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> -int i;
> +int i, n;
>  
> -#define VHOST_CSTATS \
> -VHOST_CSTAT(tx_retries)
> +#define SW_CSTATS \
> +SW_CSTAT(tx_retries)
>  
> -#define VHOST_CSTAT(NAME) + 1
> -custom_stats->size = VHOST_CSTATS;
> -#undef VHOST_CSTAT
> +#define SW_CSTAT(NAME) + 1
> +custom_stats->size = SW_CSTATS;
> +#undef SW_CSTAT
>  custom_stats->counters = xcalloc(custom_stats->size,
>   sizeof *custom_stats->counters);
> -i = 0;
> -#define VHOST_CSTAT(NAME) \
> -ovs_strlcpy(custom_stats->counters[i++].name, #NAME, \
> -NETDEV_CUSTOM_STATS_NAME_SIZE);
> -VHOST_CSTATS;
> -#undef VHOST_CSTAT
>  
>  ovs_mutex_lock(>mutex);
>  
>  rte_spinlock_lock(>stats_lock);
>  i = 0;
> -#define VHOST_CSTAT(NAME) \
> +#define SW_CSTAT(NAME) \
>  custom_stats->counters[i++].value = dev->NAME;
> -VHOST_CSTATS;
> -#undef VHOST_CSTAT
> 

Re: [ovs-dev] [dpdk-latest PATCH] netdev-dpdk: add custom vhost statistics to count IRQs

[Ilya Maximets]

On 25.10.2019 15:51, Eelco Chaudron wrote:



On 16 Oct 2019, at 15:03, Ilya Maximets wrote:


On 15.10.2019 13:20, Eelco Chaudron wrote:

When the dpdk vhost library executes an eventfd_write() call,
i.e. waking up the guest, a new callback will be called.

This patch adds the callback to count the number of
interrupts sent to the VM to track the number of times
interrupts where generated.

This might be of interest to find out system-calls were
called in the DPDK fast path.

The custom statistics can be seen with the following command:

   ovs-ofctl -O OpenFlow14 dump-ports  {}

Signed-off-by: Eelco Chaudron 
---
  lib/netdev-dpdk.c |   27 ++-
  1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index ba92e89..7ebbcdc 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -165,6 +165,8 @@ static int new_device(int vid);
  static void destroy_device(int vid);
  static int vring_state_changed(int vid, uint16_t queue_id, int enable);
  static void destroy_connection(int vid);
+static void vhost_guest_notified(int vid);
+
  static const struct vhost_device_ops virtio_net_device_ops =
  {
  .new_device =  new_device,
@@ -173,6 +175,7 @@ static const struct vhost_device_ops virtio_net_device_ops =
  .features_changed = NULL,
  .new_connection = NULL,
  .destroy_connection = destroy_connection,
+    .guest_notified = vhost_guest_notified,
  };
   enum { DPDK_RING_SIZE = 256 };
@@ -416,6 +419,8 @@ struct netdev_dpdk {
  struct netdev_stats stats;
  /* Custom stat for retries when unable to transmit. */
  uint64_t tx_retries;
+    /* Custom stat counting number of times a vhost remote was woken up */
+    uint64_t vhost_irqs;
  /* Protects stats */
  rte_spinlock_t stats_lock;
  /* 4 pad bytes here. */
@@ -2826,7 +2831,8 @@ netdev_dpdk_vhost_get_custom_stats(const struct netdev 
*netdev,
  int i;
   #define VHOST_CSTATS \
-    VHOST_CSTAT(tx_retries)
+    VHOST_CSTAT(tx_retries) \
+    VHOST_CSTAT(vhost_irqs)
   #define VHOST_CSTAT(NAME) + 1
  custom_stats->size = VHOST_CSTATS;
@@ -3746,6 +3752,25 @@ destroy_connection(int vid)
  }
  }
 +static
+void vhost_guest_notified(int vid)
+{
+    struct netdev_dpdk *dev;
+
+    ovs_mutex_lock(_mutex);
+    LIST_FOR_EACH (dev, list_node, _list) {
+    if (netdev_dpdk_get_vid(dev) == vid) {
+    ovs_mutex_lock(>mutex);
+    rte_spinlock_lock(>stats_lock);
+    dev->vhost_irqs++;
+    rte_spinlock_unlock(>stats_lock);
+    ovs_mutex_unlock(>mutex);
+    break;
+    }
+    }
+    ovs_mutex_unlock(_mutex);


So, for every single eventfd_write() we're taking the global mutex,
traversing list of all the devices, taking one more mutex and finally
taking the spinlock just to increment a single counter.
I think, it's too much.


Yes you are right this might be a bit of overkill…


Wouldn't it significantly affect interrupt based virtio drivers in
terms of performance?  How frequently 2 vhost-user ports will lock
each other on the global device mutex?  How frequently 2 PMD threads
will lock each other on rx/tx to different vrings of the same vhost
port?


I used iperf3 and did not show any negative effects (maybe I should add more 
than 4 physical queues, 1 had one VM queue), but also the results show large 
deviations.


From my experience, I'd say that iperf via kernel virtio driver is not able
to saturate a PMD thread.  They are likely relaxed and have time to wait
on the mutex.  Also, You, probably, have only couple of ports, so the critical
section is not large enough to produce frequent interlocking.

I'm just pointing that to count number of eventfd syscalls we're probably
making 2 other syscalls and probably sleeping in them.

For testing purposes, I'd suggest to add 10-20(100?) dummy pmd ports (e.g.
vhost without VMs) and assign them to different thread. Probably, more threads
in iperf for traffic generation, switching to small udp packets.
You could also replace lock() with trylock() for dpdk_mutex and count
contentions in a same way as it done for tx_lock in David's patch.




I see that 'guest_notified' patch is already in dpdk master, but
wouldn't it be better to accumulate this statistics inside DPDK
and return it with some new API like rte_vhost_get_vring_xstats()
if required?  I don't see any usecase for this callback beside
counting some stats.


I agree, however, the vhost library has no internal counters (only the PMD 
implementation which we do not use), so hence they liked the callback.


One more possible issue is that this not-so-useful callback hijacked
the reserved space in the structure and we could suffer in the
future while adding new callbacks due to ABI breakage.
(In context of DPDK API/ABI stability discussions)




For the current patch I could only suggest to convert it to simple
coverage counter like it done in 'vhost_tx_contention' 

Re: [ovs-dev] [PATCH ovn] controller: Downgrade a warning log message

[Russell Bryant]

On Fri, Oct 25, 2019 at 10:17 AM Numan Siddique  wrote:
>
>
>
> On Fri, Oct 25, 2019 at 7:07 PM Russell Bryant  wrote:
>>
>> This log message was introduced in commit 5344f24ecb.  It gets hit
>> under normal circumstances, so it would be better as a debug message
>> instead of a warning.  I also expanded it to clarify that the next
>> step will be to create the chassis record.
>>
>> This was found by trying to run the system-ovn.at tests, and they
>> failed because of these unexpected warning log messages.
>>
>> Signed-off-by: Russell Bryant 
>
>
> Acked-by: Numan Siddique 

Thanks!  I've applied this to master.

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

Re: [ovs-dev] [PATCH ovn] controller: Downgrade a warning log message

[Numan Siddique]

On Fri, Oct 25, 2019 at 7:07 PM Russell Bryant  wrote:

> This log message was introduced in commit 5344f24ecb.  It gets hit
> under normal circumstances, so it would be better as a debug message
> instead of a warning.  I also expanded it to clarify that the next
> step will be to create the chassis record.
>
> This was found by trying to run the system-ovn.at tests, and they
> failed because of these unexpected warning log messages.
>
> Signed-off-by: Russell Bryant 
>

Acked-by: Numan Siddique 


Thanks
Numan

---
>  controller/chassis.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/controller/chassis.c b/controller/chassis.c
> index 699b66281..978273e19 100644
> --- a/controller/chassis.c
> +++ b/controller/chassis.c
> @@ -484,8 +484,9 @@ chassis_get_record(struct ovsdb_idl_txn *ovnsb_idl_txn,
>  chassis_rec = chassis_lookup_by_name(sbrec_chassis_by_name,
>
> chassis_info_id(_state));
>  if (!chassis_rec) {
> -VLOG_WARN("Could not find Chassis : stored (%s) ovs (%s)",
> -  chassis_info_id(_state), chassis_id);
> +VLOG_DBG("Could not find Chassis, will create it"
> + ": stored (%s) ovs (%s)",
> + chassis_info_id(_state), chassis_id);
>  if (ovnsb_idl_txn) {
>  /* Recreate the chassis record.  */
>  chassis_rec = sbrec_chassis_insert(ovnsb_idl_txn);
> --
> 2.21.0
>
> ___
> 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] [dpdk-latest PATCH v2] netdev-dpdk: add coverage counter to count vhost IRQs

[Eelco Chaudron]

When the dpdk vhost library executes an eventfd_write() call,
i.e. waking up the guest, a new callback will be called.

This patch adds the callback to count the number of
interrupts sent to the VM to track the number of times
interrupts where generated.

This might be of interest to find out system-calls were
called in the DPDK fast path.

The coverage counter is called "dpdk_vhost_irqs" and can be
read with:

  $ ovs-appctl coverage/show | grep dpdk_vhost_irqs
  dpdk_vhost_irqs  275880.6/sec 129962.683/sec 6561.7250/sec   
total: 23684319

  $ ovs-appctl coverage/read-counter dpdk_vhost_irqs
  23684319

Signed-off-by: Eelco Chaudron 
---
 lib/netdev-dpdk.c |   12 
 1 file changed, 12 insertions(+)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index ba92e89..8c6aaeb 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -46,6 +46,7 @@
 #include "dpdk.h"
 #include "dpif-netdev.h"
 #include "fatal-signal.h"
+#include "coverage.h"
 #include "netdev-provider.h"
 #include "netdev-vport.h"
 #include "odp-util.h"
@@ -72,6 +73,8 @@ enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM};
 VLOG_DEFINE_THIS_MODULE(netdev_dpdk);
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
 
+COVERAGE_DEFINE(dpdk_vhost_irqs);
+
 #define DPDK_PORT_WATCHDOG_INTERVAL 5
 
 #define OVS_CACHE_LINE_SIZE CACHE_LINE_SIZE
@@ -165,6 +168,8 @@ static int new_device(int vid);
 static void destroy_device(int vid);
 static int vring_state_changed(int vid, uint16_t queue_id, int enable);
 static void destroy_connection(int vid);
+static void vhost_guest_notified(int vid);
+
 static const struct vhost_device_ops virtio_net_device_ops =
 {
 .new_device =  new_device,
@@ -173,6 +178,7 @@ static const struct vhost_device_ops virtio_net_device_ops =
 .features_changed = NULL,
 .new_connection = NULL,
 .destroy_connection = destroy_connection,
+.guest_notified = vhost_guest_notified,
 };
 
 enum { DPDK_RING_SIZE = 256 };
@@ -3746,6 +3752,12 @@ destroy_connection(int vid)
 }
 }
 
+static
+void vhost_guest_notified(int vid OVS_UNUSED)
+{
+COVERAGE_INC(dpdk_vhost_irqs);
+}
+
 /*
  * Retrieve the DPDK virtio device ID (vid) associated with a vhostuser
  * or vhostuserclient netdev.

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

Re: [ovs-dev] [dpdk-latest PATCH] netdev-dpdk: add custom vhost statistics to count IRQs

[Eelco Chaudron]

On 16 Oct 2019, at 15:03, Ilya Maximets wrote:


On 15.10.2019 13:20, Eelco Chaudron wrote:

When the dpdk vhost library executes an eventfd_write() call,
i.e. waking up the guest, a new callback will be called.

This patch adds the callback to count the number of
interrupts sent to the VM to track the number of times
interrupts where generated.

This might be of interest to find out system-calls were
called in the DPDK fast path.

The custom statistics can be seen with the following command:

   ovs-ofctl -O OpenFlow14 dump-ports  {}

Signed-off-by: Eelco Chaudron 
---
  lib/netdev-dpdk.c |   27 ++-
  1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index ba92e89..7ebbcdc 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -165,6 +165,8 @@ static int new_device(int vid);
  static void destroy_device(int vid);
  static int vring_state_changed(int vid, uint16_t queue_id, int 
enable);

  static void destroy_connection(int vid);
+static void vhost_guest_notified(int vid);
+
  static const struct vhost_device_ops virtio_net_device_ops =
  {
  .new_device =  new_device,
@@ -173,6 +175,7 @@ static const struct vhost_device_ops 
virtio_net_device_ops =

  .features_changed = NULL,
  .new_connection = NULL,
  .destroy_connection = destroy_connection,
+.guest_notified = vhost_guest_notified,
  };
   enum { DPDK_RING_SIZE = 256 };
@@ -416,6 +419,8 @@ struct netdev_dpdk {
  struct netdev_stats stats;
  /* Custom stat for retries when unable to transmit. */
  uint64_t tx_retries;
+/* Custom stat counting number of times a vhost remote was 
woken up */

+uint64_t vhost_irqs;
  /* Protects stats */
  rte_spinlock_t stats_lock;
  /* 4 pad bytes here. */
@@ -2826,7 +2831,8 @@ netdev_dpdk_vhost_get_custom_stats(const struct 
netdev *netdev,

  int i;
   #define VHOST_CSTATS \
-VHOST_CSTAT(tx_retries)
+VHOST_CSTAT(tx_retries) \
+VHOST_CSTAT(vhost_irqs)
   #define VHOST_CSTAT(NAME) + 1
  custom_stats->size = VHOST_CSTATS;
@@ -3746,6 +3752,25 @@ destroy_connection(int vid)
  }
  }
 +static
+void vhost_guest_notified(int vid)
+{
+struct netdev_dpdk *dev;
+
+ovs_mutex_lock(_mutex);
+LIST_FOR_EACH (dev, list_node, _list) {
+if (netdev_dpdk_get_vid(dev) == vid) {
+ovs_mutex_lock(>mutex);
+rte_spinlock_lock(>stats_lock);
+dev->vhost_irqs++;
+rte_spinlock_unlock(>stats_lock);
+ovs_mutex_unlock(>mutex);
+break;
+}
+}
+ovs_mutex_unlock(_mutex);


So, for every single eventfd_write() we're taking the global mutex,
traversing list of all the devices, taking one more mutex and finally
taking the spinlock just to increment a single counter.
I think, it's too much.


Yes you are right this might be a bit of overkill…


Wouldn't it significantly affect interrupt based virtio drivers in
terms of performance?  How frequently 2 vhost-user ports will lock
each other on the global device mutex?  How frequently 2 PMD threads
will lock each other on rx/tx to different vrings of the same vhost
port?


I used iperf3 and did not show any negative effects (maybe I should add 
more than 4 physical queues, 1 had one VM queue), but also the results 
show large deviations.



I see that 'guest_notified' patch is already in dpdk master, but
wouldn't it be better to accumulate this statistics inside DPDK
and return it with some new API like rte_vhost_get_vring_xstats()
if required?  I don't see any usecase for this callback beside
counting some stats.


I agree, however, the vhost library has no internal counters (only the 
PMD implementation which we do not use), so hence they liked the 
callback.



For the current patch I could only suggest to convert it to simple
coverage counter like it done in 'vhost_tx_contention' patch here:
https://patchwork.ozlabs.org/patch/1175849/


This is what I’ll do (and will send a patch soon). Although from a 
user perspective a per vhost user would make more sense as it could 
easily indicate which VM is configured wrongly…


Cheers,

Eelco

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

[ovs-dev] [PATCH v3 ovn 2/2] Add DNSSL support to OVN

[Lorenzo Bianconi]

Introduce the possibility to specify a DNSSL option to Router
Advertisement packets. DNS Search list can be specified using
'dnssl' tag in the ipv6_ra_configs column of logical router
port table

Signed-off-by: Lorenzo Bianconi 
---
 controller/pinctrl.c | 52 +++-
 lib/ovn-l7.h | 12 ++
 northd/ovn-northd.c  |  4 
 ovn-nb.xml   |  5 +
 tests/ovn.at | 30 -
 5 files changed, 92 insertions(+), 11 deletions(-)

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 1eaa53ad1..67cc257c0 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -2194,6 +2194,7 @@ struct ipv6_ra_config {
 struct lport_addresses prefixes;
 struct in6_addr rdnss;
 bool has_rdnss;
+struct ds dnssl;
 };
 
 struct ipv6_ra_state {
@@ -2215,6 +2216,7 @@ ipv6_ra_config_delete(struct ipv6_ra_config *config)
 {
 if (config) {
 destroy_lport_addresses(>prefixes);
+ds_destroy(>dnssl);
 free(config);
 }
 }
@@ -2253,6 +2255,7 @@ ipv6_ra_update_config(const struct sbrec_port_binding *pb)
 nd_ra_min_interval_default(config->max_interval));
 config->mtu = smap_get_int(>options, "ipv6_ra_mtu", ND_MTU_DEFAULT);
 config->la_flags = IPV6_ND_RA_OPT_PREFIX_ON_LINK;
+ds_init(>dnssl);
 
 const char *address_mode = smap_get(>options, "ipv6_ra_address_mode");
 if (!address_mode) {
@@ -2298,6 +2301,11 @@ ipv6_ra_update_config(const struct sbrec_port_binding 
*pb)
 }
 config->has_rdnss = !!rdnss;
 
+const char *dnssl = smap_get(>options, "ipv6_ra_dnssl");
+if (dnssl) {
+ds_put_buffer(>dnssl, dnssl, strlen(dnssl));
+}
+
 return config;
 
 fail:
@@ -2356,6 +2364,44 @@ packet_put_ra_rdnss_opt(struct dp_packet *b, uint8_t num,
   prev_l4_size + size));
 }
 
+static void
+packet_put_ra_dnssl_opt(struct dp_packet *b, ovs_be32 lifetime,
+char *dnssl_list)
+{
+char *t0, *r0 = dnssl_list, dnssl[255] = {};
+size_t prev_l4_size = dp_packet_l4_size(b);
+struct ip6_hdr *nh = dp_packet_l3(b);
+size_t size = 8;
+int i = 0;
+
+while ((t0 = strtok_r(r0, ",", ))) {
+char *t1, *r1 = t0;
+
+size += strlen(t0) + 2;
+while ((t1 = strtok_r(r1, ".", ))) {
+dnssl[i++] = strlen(t1);
+memcpy([i], t1, strlen(t1));
+i += strlen(t1);
+}
+dnssl[i++] = 0;
+}
+size = ROUND_UP(size, 8);
+nh->ip6_plen = htons(prev_l4_size + size);
+
+struct ovs_nd_dnssl *nd_dnssl = dp_packet_put_uninit(b, size);
+nd_dnssl->type = ND_OPT_DNSSL;
+nd_dnssl->len = size / 8;
+nd_dnssl->reserved = 0;
+put_16aligned_be32(_dnssl->lifetime, lifetime);
+memcpy(_dnssl->dnssl[0], dnssl, size);
+
+struct ovs_ra_msg *ra = dp_packet_l4(b);
+ra->icmph.icmp6_cksum = 0;
+uint32_t icmp_csum = packet_csum_pseudoheader6(dp_packet_l3(b));
+ra->icmph.icmp6_cksum = csum_finish(csum_continue(icmp_csum, ra,
+  prev_l4_size + size));
+}
+
 /* Called with in the pinctrl_handler thread context. */
 static long long int
 ipv6_ra_send(struct rconn *swconn, struct ipv6_ra_state *ra)
@@ -2364,7 +2410,7 @@ ipv6_ra_send(struct rconn *swconn, struct ipv6_ra_state 
*ra)
 return ra->next_announce;
 }
 
-uint64_t packet_stub[128 / 8];
+uint64_t packet_stub[512 / 8];
 struct dp_packet packet;
 dp_packet_use_stub(, packet_stub, sizeof packet_stub);
 compose_nd_ra(, ra->config->eth_src, ra->config->eth_dst,
@@ -2384,6 +2430,10 @@ ipv6_ra_send(struct rconn *swconn, struct ipv6_ra_state 
*ra)
 packet_put_ra_rdnss_opt(, 1, htonl(0x),
 >config->rdnss);
 }
+if (ra->config->dnssl.length) {
+packet_put_ra_dnssl_opt(, htonl(0x),
+ra->config->dnssl.string);
+}
 
 uint64_t ofpacts_stub[4096 / 8];
 struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
diff --git a/lib/ovn-l7.h b/lib/ovn-l7.h
index 44ad02949..73058 100644
--- a/lib/ovn-l7.h
+++ b/lib/ovn-l7.h
@@ -232,6 +232,18 @@ struct nd_rdnss_opt {
 };
 BUILD_ASSERT_DECL(ND_RDNSS_OPT_LEN == sizeof(struct nd_rdnss_opt));
 
+/* DNSSL option RFC 6106 */
+#define ND_OPT_DNSSL31
+#define ND_DNSSL_OPT_LEN8
+struct ovs_nd_dnssl {
+u_int8_t type;  /* ND_OPT_DNSSL */
+u_int8_t len;   /* >= 2 */
+ovs_be16 reserved;
+ovs_16aligned_be32 lifetime;
+char dnssl[0];
+};
+BUILD_ASSERT_DECL(ND_DNSSL_OPT_LEN == sizeof(struct ovs_nd_dnssl));
+
 #define DHCPV6_DUID_LL  3
 #define DHCPV6_HW_TYPE_ETH  1
 
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index d1de36e08..dbb279052 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -6489,6 +6489,10 @@ copy_ra_to_sb(struct ovn_port *op, const char 
*address_mode)
 if 

[ovs-dev] [PATCH v3 ovn 0/2] add RDNSS/DNSSL support to OVN

[Lorenzo Bianconi]

Changes since v2:
- move option definition in ovn-l7.h
- use ovs_16aligned_be32 for option lifetime field

Changes since v1:
- fix sparse warnings
- rebase DNSSL patch on top of RDNSS one


Lorenzo Bianconi (2):
  Add RDNSS support to OVN
  Add DNSSL support to OVN

 controller/pinctrl.c | 93 +++-
 lib/ovn-l7.h | 24 
 northd/ovn-northd.c  |  9 +
 ovn-nb.xml   | 10 +
 tests/ovn.at | 39 ++-
 5 files changed, 165 insertions(+), 10 deletions(-)

-- 
2.21.0

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

[ovs-dev] [PATCH v3 ovn 1/2] Add RDNSS support to OVN

[Lorenzo Bianconi]

Introduce the possibility to specify a RDNSS option to Router
Advertisement packets. DNS IPv6 address can be specified using
'rdnss' tag in the ipv6_ra_configs column of logical router
port table

Acked-by: Mark Michelson 
Signed-off-by: Lorenzo Bianconi 
---
 controller/pinctrl.c | 41 +
 lib/ovn-l7.h | 12 
 northd/ovn-northd.c  |  5 +
 ovn-nb.xml   |  5 +
 tests/ovn.at | 27 +++
 5 files changed, 82 insertions(+), 8 deletions(-)

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index d826da186..1eaa53ad1 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -2192,6 +2192,8 @@ struct ipv6_ra_config {
 uint8_t mo_flags; /* Managed/Other flags for RAs */
 uint8_t la_flags; /* On-link/autonomous flags for address prefixes */
 struct lport_addresses prefixes;
+struct in6_addr rdnss;
+bool has_rdnss;
 };
 
 struct ipv6_ra_state {
@@ -2289,6 +2291,12 @@ ipv6_ra_update_config(const struct sbrec_port_binding 
*pb)
 VLOG_WARN("Invalid IP source %s", ip_addr);
 goto fail;
 }
+const char *rdnss = smap_get(>options, "ipv6_ra_rdnss");
+if (rdnss && !ipv6_parse(rdnss, >rdnss)) {
+VLOG_WARN("Invalid RDNSS source %s", rdnss);
+goto fail;
+}
+config->has_rdnss = !!rdnss;
 
 return config;
 
@@ -2319,6 +2327,35 @@ put_load(uint64_t value, enum mf_field_id dst, int ofs, 
int n_bits,
 bitwise_one(ofpact_set_field_mask(sf), sf->field->n_bytes, ofs, n_bits);
 }
 
+static void
+packet_put_ra_rdnss_opt(struct dp_packet *b, uint8_t num,
+ovs_be32 lifetime, const struct in6_addr *dns)
+{
+size_t prev_l4_size = dp_packet_l4_size(b);
+struct ip6_hdr *nh = dp_packet_l3(b);
+size_t len = 2 * num + 1;
+size_t size = len * 8;
+
+nh->ip6_plen = htons(prev_l4_size + len * 8);
+
+struct nd_rdnss_opt *nd_rdnss = dp_packet_put_uninit(b, size);
+nd_rdnss->type = ND_OPT_RDNSS;
+nd_rdnss->len = len;
+nd_rdnss->reserved = 0;
+put_16aligned_be32(_rdnss->lifetime, lifetime);
+
+ovs_be128 *addr = (ovs_be128 *)(nd_rdnss + 1);
+for (int i = 0; i < num; i++) {
+memcpy(addr + i, dns, sizeof(ovs_be32[4]));
+}
+
+struct ovs_ra_msg *ra = dp_packet_l4(b);
+ra->icmph.icmp6_cksum = 0;
+uint32_t icmp_csum = packet_csum_pseudoheader6(dp_packet_l3(b));
+ra->icmph.icmp6_cksum = csum_finish(csum_continue(icmp_csum, ra,
+  prev_l4_size + size));
+}
+
 /* Called with in the pinctrl_handler thread context. */
 static long long int
 ipv6_ra_send(struct rconn *swconn, struct ipv6_ra_state *ra)
@@ -2343,6 +2380,10 @@ ipv6_ra_send(struct rconn *swconn, struct ipv6_ra_state 
*ra)
 ra->config->la_flags, htonl(IPV6_ND_RA_OPT_PREFIX_VALID_LIFETIME),
 htonl(IPV6_ND_RA_OPT_PREFIX_PREFERRED_LIFETIME), addr);
 }
+if (ra->config->has_rdnss) {
+packet_put_ra_rdnss_opt(, 1, htonl(0x),
+>config->rdnss);
+}
 
 uint64_t ofpacts_stub[4096 / 8];
 struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
diff --git a/lib/ovn-l7.h b/lib/ovn-l7.h
index c93def450..44ad02949 100644
--- a/lib/ovn-l7.h
+++ b/lib/ovn-l7.h
@@ -220,6 +220,18 @@ struct dhcpv6_opt_ia_na {
 ovs_be32 t2;
 });
 
+/* RDNSS option RFC 6106 */
+#define ND_RDNSS_OPT_LEN8
+#define ND_OPT_RDNSS25
+struct nd_rdnss_opt {
+uint8_t type; /* ND_OPT_RDNSS. */
+uint8_t len;  /* >= 3. */
+ovs_be16 reserved;/* Always 0. */
+ovs_16aligned_be32 lifetime;
+const ovs_be128 dns[0];
+};
+BUILD_ASSERT_DECL(ND_RDNSS_OPT_LEN == sizeof(struct nd_rdnss_opt));
+
 #define DHCPV6_DUID_LL  3
 #define DHCPV6_HW_TYPE_ETH  1
 
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index ea8ad7c2d..d1de36e08 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -6485,6 +6485,11 @@ copy_ra_to_sb(struct ovn_port *op, const char 
*address_mode)
 smap_add(, "ipv6_ra_prefixes", ds_cstr());
 ds_destroy();
 
+const char *rdnss = smap_get(>nbrp->ipv6_ra_configs, "rdnss");
+if (rdnss) {
+smap_add(, "ipv6_ra_rdnss", rdnss);
+}
+
 smap_add(, "ipv6_ra_src_eth", op->lrp_networks.ea_s);
 
 sbrec_port_binding_set_options(op->sb, );
diff --git a/ovn-nb.xml b/ovn-nb.xml
index 1504f8fca..2faf9390b 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -1885,6 +1885,11 @@
 is one-third of ,
 i.e. 200 seconds if that key is unset.
   
+
+  
+IPv6 address of RDNSS server announced in RA packets. At the moment
+OVN supports just one RDNSS server.
+  
 
 
 
diff --git a/tests/ovn.at b/tests/ovn.at
index 22b272a60..3e7692895 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -12537,14 +12537,15 @@ construct_expected_ra() {
 
 local mtu=$1
 local ra_mo=$2
-local ra_prefix_la=$3

[ovs-dev] [PATCH ovn] controller: Downgrade a warning log message

[Russell Bryant]

This log message was introduced in commit 5344f24ecb.  It gets hit
under normal circumstances, so it would be better as a debug message
instead of a warning.  I also expanded it to clarify that the next
step will be to create the chassis record.

This was found by trying to run the system-ovn.at tests, and they
failed because of these unexpected warning log messages.

Signed-off-by: Russell Bryant 
---
 controller/chassis.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/controller/chassis.c b/controller/chassis.c
index 699b66281..978273e19 100644
--- a/controller/chassis.c
+++ b/controller/chassis.c
@@ -484,8 +484,9 @@ chassis_get_record(struct ovsdb_idl_txn *ovnsb_idl_txn,
 chassis_rec = chassis_lookup_by_name(sbrec_chassis_by_name,
  chassis_info_id(_state));
 if (!chassis_rec) {
-VLOG_WARN("Could not find Chassis : stored (%s) ovs (%s)",
-  chassis_info_id(_state), chassis_id);
+VLOG_DBG("Could not find Chassis, will create it"
+ ": stored (%s) ovs (%s)",
+ chassis_info_id(_state), chassis_id);
 if (ovnsb_idl_txn) {
 /* Recreate the chassis record.  */
 chassis_rec = sbrec_chassis_insert(ovnsb_idl_txn);
-- 
2.21.0

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

Re: [ovs-dev] [PATCH ovn] tests/system-ovn: Ignore some ovn-controller warnings

[Russell Bryant]

On Thu, Oct 24, 2019 at 10:56 PM Numan Siddique  wrote:
>
>
>
> On Fri, Oct 25, 2019, 7:59 AM Numan Siddique  wrote:
>>
>>
>>
>> On Fri, Oct 25, 2019, 6:56 AM Russell Bryant  wrote:
>>>
>>> This log message was introduced in commit 5344f24ecb.  It may be more
>>> appropriate as a deubg message, but as a warning, it breaks this test suite.
>>> Filtering it out of the logs gets these tests passing for me.
>>>
>>> A sample of the messages encountered in a test run are:
>>>
>>> 2019-10-25T01:06:53.026Z|00010|chassis|WARN|Could not find Chassis : stored 
>>> (hv1) ovs (hv1)
>>> 2019-10-25T01:06:53.026Z|00011|chassis|WARN|Could not find Chassis : stored 
>>> (hv1) ovs (hv1)
>>> 2019-10-25T01:06:53.026Z|00013|chassis|WARN|Could not find Chassis : stored 
>>> (hv1) ovs (hv1)
>>> 2019-10-25T01:06:53.026Z|00014|chassis|WARN|Could not find Chassis : stored 
>>> (hv1) ovs (hv1)
>>
>>
>> Hi Russell,
>>
>> Could you please provide the signed off tag
>
>
> Acked-by: Numan Siddique 
>
> Forgot that  will take care of the tag while committing.

Thanks for the review!  Sorry I didn't see that you had submitted the
same patch earlier.

I think I'm not going to apply this though in favor of just removing
this log message, since I don't think it's really helpful, at least as
a warning.

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

[ovs-dev] [PATCH v2] ovsdb-server: fix memory leak while converting database

[Damijan Skvarc]

Hi Ben & thanks for looking into this.

I can confirm your suggestion works ok and prevents freeing the same
memory twice. Thanks for finding this possibility. Otherwise I would do
this in some other way, but not all people are the same :).

Signed-off-by: Damijan Skvarc 
---
 ovsdb/file.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/ovsdb/file.c b/ovsdb/file.c
index 8d16b09..0af077f 100644
--- a/ovsdb/file.c
+++ b/ovsdb/file.c
@@ -235,10 +235,14 @@ ovsdb_convert_table(struct ovsdb_txn *txn,
 continue;
 }
 
+ovsdb_datum_destroy(_row->fields[dst_column->index],
+_column->type);
+
 struct ovsdb_error *error = ovsdb_datum_convert(
 _row->fields[dst_column->index], _column->type,
 _row->fields[src_column->index], _column->type);
 if (error) {
+ovsdb_datum_init_empty(_row->fields[dst_column->index]);
 ovsdb_row_destroy(dst_row);
 return error;
 }
-- 
2.7.4

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

Re: [ovs-dev] [PATCH v2 ovn 1/2] Add RDNSS support to OVN

[Numan Siddique]

On Fri, Oct 25, 2019 at 4:18 PM Lorenzo Bianconi <
lorenzo.bianc...@redhat.com> wrote:

> > On Wed, Oct 23, 2019 at 8:49 PM Lorenzo Bianconi <
> > lorenzo.bianc...@redhat.com> wrote:
> >
> > > Introduce the possibility to specify a RDNSS option to Router
> > > Advertisement packets. DNS IPv6 address can be specified using
> > > 'rdnss' tag in the ipv6_ra_configs column of logical router
> > > port table
> > >
> > > Acked-by: Mark Michelson 
> > > Signed-off-by: Lorenzo Bianconi 
> > > ---
> > >  controller/pinctrl.c | 51 
> > >  northd/ovn-northd.c  |  5 +
> > >  ovn-nb.xml   |  4 
> > >  tests/ovn.at | 27 ---
> > >  4 files changed, 79 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> > > index d826da186..1125f2ef3 100644
> > > --- a/controller/pinctrl.c
> > > +++ b/controller/pinctrl.c
> > > @@ -2192,6 +2192,8 @@ struct ipv6_ra_config {
> > >  uint8_t mo_flags; /* Managed/Other flags for RAs */
> > >  uint8_t la_flags; /* On-link/autonomous flags for address
> prefixes */
> > >  struct lport_addresses prefixes;
> > > +struct in6_addr rdnss;
> > > +bool has_rdnss;
> > >  };
> > >
> > >  struct ipv6_ra_state {
> > > @@ -2202,6 +2204,16 @@ struct ipv6_ra_state {
> > >  bool delete_me;
> > >  };
> > >
> > > +#define ND_RDNSS_OPT_LEN8
> > > +#define ND_OPT_RDNSS25
> > > +struct nd_rdnss_opt {
> > > +uint8_t type; /* ND_OPT_RDNSS. */
> > > +uint8_t len;  /* >= 3. */
> > > +ovs_be16 reserved;/* Always 0. */
> > > +ovs_be32 lifetime;
> > > +const ovs_be128 dns[0];
> > > +};
> > > +
> > >
> >
> > I think this structure definition can be moved to lib/ovn-l7.h.
> > Can you please change the lifetime type to ovs_16aligned_be32, the way it
> > is done here -
> > https://github.com/openvswitch/ovs/blob/master/lib/packets.h#L1032
> > And also a macro - BUILD_ASSERT_DECL like it is done for the struct
> > - ovs_nd_prefix_opt.
> > If this function is later required in OVS, we can easily move these to
> the
> > ovs repo.
> >
> > Also can you please add the RFC number for RDNSS in the comments ? It
> would
> > be easier for some one to
> > look into the RFC if required.
>
> Hi Numan,
>
> thx for the review. Will do in v3
>
> >
> > As per the RFC, I can see that we can include multiple RDNSS addressed in
> > the RA message, where as this patch restricts to just 1 address.
> > Any particular reason for this ?
>
> packet_put_ra_rdnss_opt supports 'multiple' rdnss addresses but for the
> moment
> northd (just for simplicity) adds a single server address. I think we can
> add
> multiple server support in the future if needed. What do you think?
>

I am fine with it. Can you please add this limitation to the documentation.

Thanks
Numan


>
> Regards,
> Lorenzo
>
> >
> > Thanks
> > Numan
> >
> >
> >
> > >  static void
> > >  init_ipv6_ras(void)
> > >  {
> > > @@ -2289,6 +2301,12 @@ ipv6_ra_update_config(const struct
> > > sbrec_port_binding *pb)
> > >  VLOG_WARN("Invalid IP source %s", ip_addr);
> > >  goto fail;
> > >  }
> > > +const char *rdnss = smap_get(>options, "ipv6_ra_rdnss");
> > > +if (rdnss && !ipv6_parse(rdnss, >rdnss)) {
> > > +VLOG_WARN("Invalid RDNSS source %s", rdnss);
> > > +goto fail;
> > > +}
> > > +config->has_rdnss = !!rdnss;
> > >
> > >  return config;
> > >
> > > @@ -2319,6 +2337,35 @@ put_load(uint64_t value, enum mf_field_id dst,
> int
> > > ofs, int n_bits,
> > >  bitwise_one(ofpact_set_field_mask(sf), sf->field->n_bytes, ofs,
> > > n_bits);
> > >  }
> > >
> > > +static void
> > > +packet_put_ra_rdnss_opt(struct dp_packet *b, uint8_t num,
> > > +ovs_be32 lifetime, const struct in6_addr *dns)
> > > +{
> > > +size_t prev_l4_size = dp_packet_l4_size(b);
> > > +struct ip6_hdr *nh = dp_packet_l3(b);
> > > +size_t len = 2 * num + 1;
> > > +size_t size = len * 8;
> > > +
> > > +nh->ip6_plen = htons(prev_l4_size + len * 8);
> > > +
> > > +struct nd_rdnss_opt *nd_rdnss = dp_packet_put_uninit(b, size);
> > > +nd_rdnss->type = ND_OPT_RDNSS;
> > > +nd_rdnss->len = len;
> > > +nd_rdnss->reserved = 0;
> > > +nd_rdnss->lifetime = lifetime;
> > > +
> > > +ovs_be128 *addr = (ovs_be128 *)(nd_rdnss + 1);
> > > +for (int i = 0; i < num; i++) {
> > > +memcpy(addr + i, dns, sizeof(ovs_be32[4]));
> > > +}
> > > +
> > > +struct ovs_ra_msg *ra = dp_packet_l4(b);
> > > +ra->icmph.icmp6_cksum = 0;
> > > +uint32_t icmp_csum = packet_csum_pseudoheader6(dp_packet_l3(b));
> > > +ra->icmph.icmp6_cksum = csum_finish(csum_continue(icmp_csum, ra,
> > > +  prev_l4_size +
> > > size));
> > > +}
> > > +
> > >  /* Called with in the pinctrl_handler thread context. */
> > >  static long long int
> > >  ipv6_ra_send(struct 

Re: [ovs-dev] Meter measures incorrect when use multi-pmd in ovs2.10

[Ilya Maximets]

Hi,
   I meet a problem that when send packets using netperf in multi-thread mode.
   The reproducing condition is like this:
   1. ovs 2.10 in dpdk usermode with 2 pmd
   2. Set Meter with rate=100,000pps, burst=20,000packet
   3. when use single-thread mode for netperf, Meter behaves correct, and 
packets larger than 100,pps is dropped
   when use multi-thread mode for netperf, we noticed packets come from 
both 2 pmds, and in this time, Meter measure does not work.


   But Meter behaves correctly in ovs2.8 whether using single pmd or multi-pmd .
   Also, we have merged the patch 42697ca7757b594cc841d944e43ffc17905e3188
  (long_delta_t = now / 1000 - meter->used / 1000)  but problem still exists.




   We researched the meter code, found that meter use time_usec() to compute 
the delta time in ovs2.10.
   but when call the function dp_netdev_run_meter(),  the input parameter 'now' 
variable come from pmd->ctx.now
   and pmd->ctx.now may updated when receive packet in function 
dp_netdev_process_rxq_port()
   so let's suppose the following condition:
   pmd1 receives packet in T1
   pmd2 receives packet in T2 (T2used changed to pmd1->ctx.now
   than handle Meter in pmd2, now = pmd2->ctx.now
   and long_delta_t = now / 1000 - meter->used / 1000 = T2/1000 - T1/1000  
which will be a negative value!!!
   then delta time changed with clause:
  delta_t = (long_delta_t > (long long int)meter->max_delta_t)
? meter->max_delta_t : (uint32_t)long_delta_t;
 in this time, delta_t = (uint32_t)(T2/1000 - T1/1000)  will be overflow???


now, we just fix this problem by modifing the input parameter 'now' with 
time_usec().
we don't know whether this code modify has any side effect(performance issue?)


Hi. Thanks for reporting the issue!

Regarding the solution, there is still a small race window between time_usec() 
and
acquiring the meter lock, so it's still possible to get a negative value.

I prepared a patch with alternative solution:
https://patchwork.ozlabs.org/patch/1184065/

Please, check and reply if it fixes your issue.

Best regards, Ilya Maximets.

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

[ovs-dev] [PATCH] dpif-netdev: Fix time delta overflow in case of race for meter lock.

[Ilya Maximets]

There is a race window between getting the time and getting the meter
lock.  This could lead to situation where the thread with larger
current time (this thread called time_{um}sec() later than others)
will acquire meter lock first and update meter->used to the large
value.  Next threads will try to calculate time delta by subtracting
the large meter->used from their lower time getting the negative value
which will be converted to a big unsigned delta.

Fix that by assuming that all these threads received packets in the
same time in this case, i.e. dropping negative delta to 0.

CC: Jarno Rajahalme 
Fixes: 4b27db644a8c ("dpif-netdev: Simple DROP meter implementation.")
Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-dev/2019-September/363126.html
Signed-off-by: Ilya Maximets 
---
 lib/dpif-netdev.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 4546b55e8..13586206a 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -5646,6 +5646,14 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct 
dp_packet_batch *packets_,
 /* All packets will hit the meter at the same time. */
 long_delta_t = now / 1000 - meter->used / 1000; /* msec */
 
+if (long_delta_t < 0) {
+/* This condition means that we have several threads fighting for a
+   meter lock, and the one who received the packets a bit later wins.
+   Assuming that all racing threads received packets at the same time
+   to avoid overflow. */
+long_delta_t = 0;
+}
+
 /* Make sure delta_t will not be too large, so that bucket will not
  * wrap around below. */
 delta_t = (long_delta_t > (long long int)meter->max_delta_t)
-- 
2.17.1

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

Re: [ovs-dev] [PATCH branch-2.10] compat: Remove unused function

[0-day Robot]

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

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


checkpatch:
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Ben Pfaff , Simon Horman 

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


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

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

Re: [ovs-dev] [PATCH v2 ovn 1/2] Add RDNSS support to OVN

[Lorenzo Bianconi]

> On Wed, Oct 23, 2019 at 8:49 PM Lorenzo Bianconi <
> lorenzo.bianc...@redhat.com> wrote:
> 
> > Introduce the possibility to specify a RDNSS option to Router
> > Advertisement packets. DNS IPv6 address can be specified using
> > 'rdnss' tag in the ipv6_ra_configs column of logical router
> > port table
> >
> > Acked-by: Mark Michelson 
> > Signed-off-by: Lorenzo Bianconi 
> > ---
> >  controller/pinctrl.c | 51 
> >  northd/ovn-northd.c  |  5 +
> >  ovn-nb.xml   |  4 
> >  tests/ovn.at | 27 ---
> >  4 files changed, 79 insertions(+), 8 deletions(-)
> >
> > diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> > index d826da186..1125f2ef3 100644
> > --- a/controller/pinctrl.c
> > +++ b/controller/pinctrl.c
> > @@ -2192,6 +2192,8 @@ struct ipv6_ra_config {
> >  uint8_t mo_flags; /* Managed/Other flags for RAs */
> >  uint8_t la_flags; /* On-link/autonomous flags for address prefixes */
> >  struct lport_addresses prefixes;
> > +struct in6_addr rdnss;
> > +bool has_rdnss;
> >  };
> >
> >  struct ipv6_ra_state {
> > @@ -2202,6 +2204,16 @@ struct ipv6_ra_state {
> >  bool delete_me;
> >  };
> >
> > +#define ND_RDNSS_OPT_LEN8
> > +#define ND_OPT_RDNSS25
> > +struct nd_rdnss_opt {
> > +uint8_t type; /* ND_OPT_RDNSS. */
> > +uint8_t len;  /* >= 3. */
> > +ovs_be16 reserved;/* Always 0. */
> > +ovs_be32 lifetime;
> > +const ovs_be128 dns[0];
> > +};
> > +
> >
> 
> I think this structure definition can be moved to lib/ovn-l7.h.
> Can you please change the lifetime type to ovs_16aligned_be32, the way it
> is done here -
> https://github.com/openvswitch/ovs/blob/master/lib/packets.h#L1032
> And also a macro - BUILD_ASSERT_DECL like it is done for the struct
> - ovs_nd_prefix_opt.
> If this function is later required in OVS, we can easily move these to the
> ovs repo.
> 
> Also can you please add the RFC number for RDNSS in the comments ? It would
> be easier for some one to
> look into the RFC if required.

Hi Numan,

thx for the review. Will do in v3

> 
> As per the RFC, I can see that we can include multiple RDNSS addressed in
> the RA message, where as this patch restricts to just 1 address.
> Any particular reason for this ?

packet_put_ra_rdnss_opt supports 'multiple' rdnss addresses but for the moment
northd (just for simplicity) adds a single server address. I think we can add
multiple server support in the future if needed. What do you think?

Regards,
Lorenzo

> 
> Thanks
> Numan
> 
> 
> 
> >  static void
> >  init_ipv6_ras(void)
> >  {
> > @@ -2289,6 +2301,12 @@ ipv6_ra_update_config(const struct
> > sbrec_port_binding *pb)
> >  VLOG_WARN("Invalid IP source %s", ip_addr);
> >  goto fail;
> >  }
> > +const char *rdnss = smap_get(>options, "ipv6_ra_rdnss");
> > +if (rdnss && !ipv6_parse(rdnss, >rdnss)) {
> > +VLOG_WARN("Invalid RDNSS source %s", rdnss);
> > +goto fail;
> > +}
> > +config->has_rdnss = !!rdnss;
> >
> >  return config;
> >
> > @@ -2319,6 +2337,35 @@ put_load(uint64_t value, enum mf_field_id dst, int
> > ofs, int n_bits,
> >  bitwise_one(ofpact_set_field_mask(sf), sf->field->n_bytes, ofs,
> > n_bits);
> >  }
> >
> > +static void
> > +packet_put_ra_rdnss_opt(struct dp_packet *b, uint8_t num,
> > +ovs_be32 lifetime, const struct in6_addr *dns)
> > +{
> > +size_t prev_l4_size = dp_packet_l4_size(b);
> > +struct ip6_hdr *nh = dp_packet_l3(b);
> > +size_t len = 2 * num + 1;
> > +size_t size = len * 8;
> > +
> > +nh->ip6_plen = htons(prev_l4_size + len * 8);
> > +
> > +struct nd_rdnss_opt *nd_rdnss = dp_packet_put_uninit(b, size);
> > +nd_rdnss->type = ND_OPT_RDNSS;
> > +nd_rdnss->len = len;
> > +nd_rdnss->reserved = 0;
> > +nd_rdnss->lifetime = lifetime;
> > +
> > +ovs_be128 *addr = (ovs_be128 *)(nd_rdnss + 1);
> > +for (int i = 0; i < num; i++) {
> > +memcpy(addr + i, dns, sizeof(ovs_be32[4]));
> > +}
> > +
> > +struct ovs_ra_msg *ra = dp_packet_l4(b);
> > +ra->icmph.icmp6_cksum = 0;
> > +uint32_t icmp_csum = packet_csum_pseudoheader6(dp_packet_l3(b));
> > +ra->icmph.icmp6_cksum = csum_finish(csum_continue(icmp_csum, ra,
> > +  prev_l4_size +
> > size));
> > +}
> > +
> >  /* Called with in the pinctrl_handler thread context. */
> >  static long long int
> >  ipv6_ra_send(struct rconn *swconn, struct ipv6_ra_state *ra)
> > @@ -2343,6 +2390,10 @@ ipv6_ra_send(struct rconn *swconn, struct
> > ipv6_ra_state *ra)
> >  ra->config->la_flags,
> > htonl(IPV6_ND_RA_OPT_PREFIX_VALID_LIFETIME),
> >  htonl(IPV6_ND_RA_OPT_PREFIX_PREFERRED_LIFETIME), addr);
> >  }
> > +if (ra->config->has_rdnss) {
> > +packet_put_ra_rdnss_opt(, 1, htonl(0x),
> > +

Re: [ovs-dev] [PATCH v10 1/3] netdev-dpdk: Reuse vhost function for dpdk ETH custom stats.

[Sriram Vatala via dev]

Hi,

Please review the patch set v10.

Changes since v9 :
Patch - 1:
Fixed the issue with index inside the for loop for copying the dpdk extended
stats  inside 'netdev_dpdk_get_custom_stats' function.

Patch - 2:
Modified the commit message.
Changed the custom stats prefix from 'netdev_dpdk_' to 'ovs_'
Added note about the prefix 'ovs_' in dpdk documentation. 
Addressed other comments given by Kevin.

Thanks & Regards,
Sriram.

-Original Message-
From: Sriram Vatala  
Sent: 24 October 2019 23:51
To: ovs-dev@openvswitch.org; i.maxim...@ovn.org; ktray...@redhat.com
Cc: Ilya Maximets ; Sriram Vatala

Subject: [PATCH v10 1/3] netdev-dpdk: Reuse vhost function for dpdk ETH
custom stats.

From: Ilya Maximets 

This is yet another refactoring for upcoming detailed drop stats.
It allowes to use single function for all the software calculated statistics
in netdev-dpdk for both vhost and ETH ports.

UINT64_MAX used as a marker for non-supported statistics in a same way as
it's done in bridge.c for common netdev stats.

Co-authored-by: Sriram Vatala 
Cc: Sriram Vatala 
Signed-off-by: Ilya Maximets 
Signed-off-by: Sriram Vatala 
---
 lib/netdev-dpdk.c | 69 +++
 1 file changed, 40 insertions(+), 29 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
04e1a2d1b..2cc2516a9 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -474,6 +474,8 @@ struct netdev_rxq_dpdk {  static void
netdev_dpdk_destruct(struct netdev *netdev);  static void
netdev_dpdk_vhost_destruct(struct netdev *netdev);
 
+static int netdev_dpdk_get_sw_custom_stats(const struct netdev *,
+   struct netdev_custom_stats 
+*);
 static void netdev_dpdk_clear_xstats(struct netdev_dpdk *dev);
 
 int netdev_dpdk_get_vid(const struct netdev_dpdk *dev); @@ -1174,7 +1176,7
@@ common_construct(struct netdev *netdev, dpdk_port_t port_no,
 dev->rte_xstats_ids = NULL;
 dev->rte_xstats_ids_size = 0;
 
-dev->tx_retries = 0;
+dev->tx_retries = (dev->type == DPDK_DEV_VHOST) ? 0 : UINT64_MAX;
 
 return 0;
 }
@@ -2775,7 +2777,9 @@ netdev_dpdk_get_custom_stats(const struct netdev
*netdev,
 
 uint32_t i;
 struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
-int rte_xstats_ret;
+int rte_xstats_ret, sw_stats_size;
+
+netdev_dpdk_get_sw_custom_stats(netdev, custom_stats);
 
 ovs_mutex_lock(>mutex);
 
@@ -2790,23 +2794,22 @@ netdev_dpdk_get_custom_stats(const struct netdev
*netdev,
 if (rte_xstats_ret > 0 &&
 rte_xstats_ret <= dev->rte_xstats_ids_size) {
 
-custom_stats->size = rte_xstats_ret;
-custom_stats->counters =
-(struct netdev_custom_counter *)
xcalloc(rte_xstats_ret,
-sizeof(struct netdev_custom_counter));
+sw_stats_size = custom_stats->size;
+custom_stats->size += rte_xstats_ret;
+custom_stats->counters = xrealloc(custom_stats->counters,
+  custom_stats->size *
+  sizeof 
+ *custom_stats->counters);
 
 for (i = 0; i < rte_xstats_ret; i++) {
-ovs_strlcpy(custom_stats->counters[i].name,
+ovs_strlcpy(custom_stats->counters[sw_stats_size + 
+ i].name,
 netdev_dpdk_get_xstat_name(dev,
 
dev->rte_xstats_ids[i]),
 NETDEV_CUSTOM_STATS_NAME_SIZE);
-custom_stats->counters[i].value = values[i];
+custom_stats->counters[sw_stats_size + i].value = 
+ values[i];
 }
 } else {
 VLOG_WARN("Cannot get XSTATS values for port:
"DPDK_PORT_ID_FMT,
   dev->port_id);
-custom_stats->counters = NULL;
-custom_stats->size = 0;
 /* Let's clear statistics cache, so it will be
  * reconfigured */
 netdev_dpdk_clear_xstats(dev); @@ -2821,39 +2824,47 @@
netdev_dpdk_get_custom_stats(const struct netdev *netdev,  }
 
 static int
-netdev_dpdk_vhost_get_custom_stats(const struct netdev *netdev,
-   struct netdev_custom_stats
*custom_stats)
+netdev_dpdk_get_sw_custom_stats(const struct netdev *netdev,
+struct netdev_custom_stats 
+*custom_stats)
 {
 struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
-int i;
+int i, n;
 
-#define VHOST_CSTATS \
-VHOST_CSTAT(tx_retries)
+#define SW_CSTATS \
+SW_CSTAT(tx_retries)
 
-#define VHOST_CSTAT(NAME) + 1
-custom_stats->size = VHOST_CSTATS;
-#undef VHOST_CSTAT
+#define SW_CSTAT(NAME) + 1
+custom_stats->size = SW_CSTATS;
+#undef SW_CSTAT
 custom_stats->counters = xcalloc(custom_stats->size,
  sizeof *custom_stats->counters);
-i = 0;
-#define VHOST_CSTAT(NAME) \
-ovs_strlcpy(custom_stats->counters[i++].name, #NAME, \
-  

Re: [ovs-dev] [PATCH] tc: Limit the max action number to 16

[Simon Horman]

On Thu, Oct 24, 2019 at 10:43:00AM +0200, Simon Horman wrote:
> On Mon, Oct 21, 2019 at 07:01:38AM +, Roi Dayan wrote:
> > 
> > 
> > On 2019-10-18 1:00 PM, Simon Horman wrote:
> > > On Wed, Oct 16, 2019 at 11:53:52AM +, Roi Dayan wrote:
> > >>
> > >>
> > >> On 2019-10-16 2:40 PM, Simon Horman wrote:
> > >>> On Wed, Oct 16, 2019 at 11:37:14AM +0300, Roi Dayan wrote:
> >  From: Chris Mi 
> > 
> >  Currently, ovs supports to offload max TCA_ACT_MAX_PRIO(32) actions.
> >  But net sched api has a limit of 4K message size which is not enough
> >  for 32 actions when echo flag is set.
> > 
> >  After a lot of testing, we find that 16 actions is a reasonable number.
> >  So in this commit, we introduced a new define to limit the max actions.
> > 
> >  Fixes: 0c70132cd288("tc: Make the actions order consistent")
> >  Signed-off-by: Chris Mi 
> >  Reviewed-by: Roi Dayan 
> > >>>
> > >>> Hi Chris,
> > >>>
> > >>> I'm unclear on what problem is this patch solving.
> > >>
> > >> Hi Simon,
> > >>
> > >> I can help with the answer here.
> > >> When ovs send netlink msg to tc to add a filter we also add
> > >> the echo flag to get a reply data. this reply can be big as
> > >> it contains everything from the request and if it pass the 4K
> > >> size then the kernel will just not fill/send it but the return
> > >> status of the tc command is still 0 for success.
> > >>
> > >> In ovs we use that reply to get back the handle id on success but
> > >> for this case will fail.
> > >> To make sure this ack reply always exists for successful tc calls
> > >> we limit the number of actions here to avoid reaching the 4K size limit.
> > > 
> > > Thanks,
> > > 
> > > It sounds like it would be good to develop a mechanism where
> > > the information required can be retrieved in a less verbose manner,
> > > thus allowing a higher limit.
> > > 
> > > But I do agree that this resolves a problem and I have applied it to
> > > master. Let me know if you think it is also appropriate for older 
> > > branches.
> > > 
> > > Kind regards,
> > > Simon
> > > 
> > 
> > thanks Simon.
> > this patch can be applied also to branches 2.10, 2.11, 2.12.
> 
> Thanks,
> 
> I have pushed a backport to branch-2.12.
> And I intend to do likewise for branch-2.11 if travis-ci passes.

I have now pushed this patch to branch-2.11.

> 
> I am however holding back on branch-2.10 as it seems broken
> with respect to travis-ci. And I do not like to add patches
> to broken branches.
> 
> I am trying to investigate the cause of that breakage.
> I would also welcome any help in this area.

I have proposed a fix for this problem.

https://mail.openvswitch.org/pipermail/ovs-dev/2019-October/363868.html
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH branch-2.10] compat: Remove unused function

[Simon Horman]

From: Greg Rose 

The compat function rpl_nf_conntrack_in() does not appear to be used
anywhere and emits warnings as such during builds < 4.10.

The patch passes Travis:

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

Remove it.

A backport of this patch to branch-2.10 seems to be required
to fix build errors when compiling against v3.16.54

  before: https://travis-ci.org/openvswitch/ovs/builds/602518689
  after: https://travis-ci.org/horms2/ovs/builds/602665800

Signed-off-by: Greg Rose 
Signed-off-by: Ben Pfaff 
Signed-off-by: Simon Horman 
Reviewed-by: John Hurley 
---
 .../include/net/netfilter/nf_conntrack_core.h | 21 ---
 1 file changed, 21 deletions(-)

diff --git a/datapath/linux/compat/include/net/netfilter/nf_conntrack_core.h 
b/datapath/linux/compat/include/net/netfilter/nf_conntrack_core.h
index cd55843c3605..10158011fd4d 100644
--- a/datapath/linux/compat/include/net/netfilter/nf_conntrack_core.h
+++ b/datapath/linux/compat/include/net/netfilter/nf_conntrack_core.h
@@ -67,27 +67,6 @@ static inline bool rpl_nf_ct_get_tuple(const struct sk_buff 
*skb,
 #define nf_ct_get_tuple rpl_nf_ct_get_tuple
 #endif /* HAVE_NF_CT_GET_TUPLEPR_TAKES_STRUCT_NET */
 
-/* Commit 08733a0cb7de ("netfilter: handle NF_REPEAT from nf_conntrack_in()")
- * introduced behavioural changes to this function which cannot be detected
- * in the headers. Unconditionally backport to kernels older than the one which
- * contains this commit. */
-#if LINUX_VERSION_CODE < KERNEL_VERSION(4,10,0)
-static unsigned int rpl_nf_conntrack_in(struct net *net, u_int8_t pf,
-   unsigned int hooknum,
-   struct sk_buff *skb)
-{
-   int err;
-
-   /* Repeat if requested, see nf_iterate(). */
-   do {
-   err = nf_conntrack_in(net, pf, hooknum, skb);
-   } while (err == NF_REPEAT);
-
-   return err;
-}
-#define nf_conntrack_in rpl_nf_conntrack_in
-#endif /* < 4.10 */
-
 #ifdef HAVE_NF_CONN_TIMER
 
 #ifndef HAVE_NF_CT_DELETE
-- 
2.20.1

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

[ovs-dev] INTESA TRANSFER

[Pilar Sanchez]

Today we processed as attached 30% payment of invoice sent as regards
our order,

kindly confirm the receipt of this payment and inform us the exact date
of dispatch.

Best regards,

Pilar Sanchez

From: Gent Tabaku [mailto:gent.tab...@intesasenpaolobank.al [1]] 
Sent: Thursday, October 24, 2019 11:39 AM
To: Pilar Sanchez

Subject: Fwd: INTESA TRANSFER

Gent Tabaku 

Intesa Sanpaolo Bank Albania 

Joint Teller Senior 

Branch " Dega Rr Bardhyl " 

Tel.: 0687573535 

gent.tab...@intesasanpaolobank.al [1] 

  

Links:
--
[1] http://webmail.auditum.pt/Mondo/lang/sys/Forms/MAI/#NOP___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev