[ovs-dev] [PATCH] Add a __str__ method to idl.Row

[Terry Wilson]

It's sometimes handy to log an entire Row object, so this just
adds a string representation of the object as:

   Tablename(col1=val1, col2=val2, ..., coln=valn)

Signed-off-by: Terry Wilson 
---
 python/ovs/db/idl.py | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
index 84af978..d1d9155 100644
--- a/python/ovs/db/idl.py
+++ b/python/ovs/db/idl.py
@@ -997,6 +997,12 @@ class Row(object):
 def __hash__(self):
 return int(self.__dict__['uuid'])
 
+def __str__(self):
+return "{table}({data})".format(
+table=self._table.name,
+data=", ".join("{col}={val}".format(col=c, val=getattr(self, c))
+   for c in sorted(self._table.columns)))
+
 def __getattr__(self, column_name):
 assert self._changes is not None
 assert self._mutations is not None
-- 
1.8.3.1

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

[ovs-dev] [branch-2.9 2/2] Prepare for 2.9.7.

[Justin Pettit]

Signed-off-by: Justin Pettit 
---
 NEWS | 3 +++
 configure.ac | 2 +-
 debian/changelog | 6 ++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index 8f6469fd4dbc..0e708efd64c4 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,6 @@
+v2.9.7 - xx xxx 
+
+
 v2.9.6 - 03 Sep 2019
 
- Bug fixes
diff --git a/configure.ac b/configure.ac
index ad4e8138c334..4e8591caa081 100644
--- a/configure.ac
+++ b/configure.ac
@@ -13,7 +13,7 @@
 # limitations under the License.
 
 AC_PREREQ(2.63)
-AC_INIT(openvswitch, 2.9.6, b...@openvswitch.org)
+AC_INIT(openvswitch, 2.9.7, b...@openvswitch.org)
 AC_CONFIG_SRCDIR([datapath/datapath.c])
 AC_CONFIG_MACRO_DIR([m4])
 AC_CONFIG_AUX_DIR([build-aux])
diff --git a/debian/changelog b/debian/changelog
index 8746d837632f..d0c5afce2d58 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+openvswitch (2.9.7-1) unstable; urgency=low
+   [ Open vSwitch team ]
+   * New upstream version
+
+ -- Open vSwitch team   Tue, 03 Sep 2019 15:14:43 -0700
+
 openvswitch (2.9.6-1) unstable; urgency=low
[ Open vSwitch team ]
* New upstream version
-- 
2.17.1

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

[ovs-dev] [branch-2.9 1/2] Set release date for 2.9.6.

[Justin Pettit]

Signed-off-by: Justin Pettit 
---
 NEWS | 3 ++-
 debian/changelog | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index c1bb90fe03af..8f6469fd4dbc 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,6 @@
-v2.9.6 - xx xxx 
+v2.9.6 - 03 Sep 2019
 
+   - Bug fixes
 
 v2.9.5 - 30 Mar 2019
 
diff --git a/debian/changelog b/debian/changelog
index 2b61be238444..8746d837632f 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -2,7 +2,7 @@ openvswitch (2.9.6-1) unstable; urgency=low
[ Open vSwitch team ]
* New upstream version
 
- -- Open vSwitch team   Sat, 30 Mar 2019 11:00:25 -0700
+ -- Open vSwitch team   Tue, 03 Sep 2019 15:13:45 -0700
 
 openvswitch (2.9.5-1) unstable; urgency=low
[ Open vSwitch team ]
-- 
2.17.1

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

[ovs-dev] [branch-2.10 2/2] Prepare for 2.10.4.

[Justin Pettit]

Signed-off-by: Justin Pettit 
---
 NEWS | 3 +++
 configure.ac | 2 +-
 debian/changelog | 6 ++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index 6233984fa1e0..109d809d6a61 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,6 @@
+v2.10.4 - xx xxx 
+
+
 v2.10.3 - 03 Sep 2019
 
- Bug fixes
diff --git a/configure.ac b/configure.ac
index d47b32f86b2c..f2a0352671f2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -13,7 +13,7 @@
 # limitations under the License.
 
 AC_PREREQ(2.63)
-AC_INIT(openvswitch, 2.10.3, b...@openvswitch.org)
+AC_INIT(openvswitch, 2.10.4, b...@openvswitch.org)
 AC_CONFIG_SRCDIR([datapath/datapath.c])
 AC_CONFIG_MACRO_DIR([m4])
 AC_CONFIG_AUX_DIR([build-aux])
diff --git a/debian/changelog b/debian/changelog
index 0c8939550ff8..4654845c89ef 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+openvswitch (2.10.4-1) unstable; urgency=low
+   [ Open vSwitch team ]
+   * New upstream version
+
+ -- Open vSwitch team   Tue, 03 Sep 2019 15:12:30 -0700
+
 openvswitch (2.10.3-1) unstable; urgency=low
[ Open vSwitch team ]
* New upstream version
-- 
2.17.1

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

[ovs-dev] [branch-2.10 1/2] Set release date for 2.10.3.

[Justin Pettit]

Signed-off-by: Justin Pettit 
---
 NEWS | 3 ++-
 debian/changelog | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index 064550b038f9..6233984fa1e0 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,6 @@
-v2.10.3 - xx xxx 
+v2.10.3 - 03 Sep 2019
 
+   - Bug fixes
 
 v2.10.2 - 30 Mar 2019
 
diff --git a/debian/changelog b/debian/changelog
index 7752f1c250f7..0c8939550ff8 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -2,7 +2,7 @@ openvswitch (2.10.3-1) unstable; urgency=low
[ Open vSwitch team ]
* New upstream version
 
- -- Open vSwitch team   Sat, 30 Mar 2019 10:50:37 -0700
+ -- Open vSwitch team   Tue, 03 Sep 2019 15:10:28 -0700
 
 openvswitch (2.10.2-1) unstable; urgency=low
[ Open vSwitch team ]
-- 
2.17.1

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

[ovs-dev] [branch-2.11 2/2] Prepare for 2.11.3.

[Justin Pettit]

Signed-off-by: Justin Pettit 
---
 NEWS | 3 +++
 configure.ac | 2 +-
 debian/changelog | 6 ++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index f2d44e4630bb..a21d475628d0 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,6 @@
+v2.11.3 - xx xxx 
+
+
 v2.11.2 - 03 Sep 2019
 
- Bug fixes
diff --git a/configure.ac b/configure.ac
index 0c19739220b1..45ff8cb07e30 100644
--- a/configure.ac
+++ b/configure.ac
@@ -13,7 +13,7 @@
 # limitations under the License.
 
 AC_PREREQ(2.63)
-AC_INIT(openvswitch, 2.11.2, b...@openvswitch.org)
+AC_INIT(openvswitch, 2.11.3, b...@openvswitch.org)
 AC_CONFIG_SRCDIR([datapath/datapath.c])
 AC_CONFIG_MACRO_DIR([m4])
 AC_CONFIG_AUX_DIR([build-aux])
diff --git a/debian/changelog b/debian/changelog
index 25763e831b68..b7a466b55aff 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+openvswitch (2.11.3-1) unstable; urgency=low
+   [ Open vSwitch team ]
+   * New upstream version
+
+ -- Open vSwitch team   Tue, 03 Sep 2019 15:06:54 -0700
+
 openvswitch (2.11.2-1) unstable; urgency=low
[ Open vSwitch team ]
* New upstream version
-- 
2.17.1

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

[ovs-dev] [branch-2.11 1/2] Set release date for 2.11.2.

[Justin Pettit]

Signed-off-by: Justin Pettit 
---
 NEWS | 3 ++-
 debian/changelog | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index ff2647739375..f2d44e4630bb 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,6 @@
-v2.11.2 - xx xxx 
+v2.11.2 - 03 Sep 2019
 
+   - Bug fixes
- DPDK
  * OVS validated with DPDK 18.11.2 which is recommended to be used.
 
diff --git a/debian/changelog b/debian/changelog
index 1c3f40bad58d..25763e831b68 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -2,7 +2,7 @@ openvswitch (2.11.2-1) unstable; urgency=low
[ Open vSwitch team ]
* New upstream version
 
- -- Open vSwitch team   Sat, 30 Mar 2019 10:46:10 -0700
+ -- Open vSwitch team   Tue, 03 Sep 2019 15:05:42 -0700
 
 openvswitch (2.11.1-1) unstable; urgency=low
[ Open vSwitch team ]
-- 
2.17.1

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

[ovs-dev] [branch-2.12 2/2] Prepare for 2.12.1.

[Justin Pettit]

Signed-off-by: Justin Pettit 
---
 NEWS | 3 +++
 configure.ac | 2 +-
 debian/changelog | 6 ++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index 9f4845d8788e..877d965e8691 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,6 @@
+v2.12.1 - xx xxx 
+
+
 v2.12.0 - 03 Sep 2019
 
- DPDK:
diff --git a/configure.ac b/configure.ac
index 5fee8e9eb134..a4322176c1d7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -13,7 +13,7 @@
 # limitations under the License.
 
 AC_PREREQ(2.63)
-AC_INIT(openvswitch, 2.12.0, b...@openvswitch.org)
+AC_INIT(openvswitch, 2.12.1, b...@openvswitch.org)
 AC_CONFIG_SRCDIR([datapath/datapath.c])
 AC_CONFIG_MACRO_DIR([m4])
 AC_CONFIG_AUX_DIR([build-aux])
diff --git a/debian/changelog b/debian/changelog
index b6d0568ef077..bb9ea31a030c 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+openvswitch (2.12.1-1) unstable; urgency=low
+   [ Open vSwitch team ]
+   * New upstream version
+
+ -- Open vSwitch team   Tue, 03 Sep 2019 14:57:37 -0700
+
 openvswitch (2.12.0-1) unstable; urgency=low
[ Open vSwitch team ]
* New upstream version
-- 
2.17.1

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

[ovs-dev] [branch-2.12 1/2] Set release date for 2.12.0.

[Justin Pettit]

Signed-off-by: Justin Pettit 
---
 NEWS | 4 ++--
 debian/changelog | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/NEWS b/NEWS
index be3ea42b4c91..9f4845d8788e 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,5 @@
-v2.12.0 - xx xxx 
--
+v2.12.0 - 03 Sep 2019
+
- DPDK:
  * New option 'other_config:dpdk-socket-limit' to limit amount of
hugepage memory that can be used by DPDK.
diff --git a/debian/changelog b/debian/changelog
index 638a69e4f682..b6d0568ef077 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,8 +1,8 @@
 openvswitch (2.12.0-1) unstable; urgency=low
-
+   [ Open vSwitch team ]
* New upstream version
 
- -- Open vSwitch team   Mon, 22 Jul 2019 09:36:56 -0700
+ -- Open vSwitch team   Tue, 03 Sep 2019 14:56:53 -0700
 
 openvswitch (2.11.0-1) unstable; urgency=low
 
-- 
2.17.1

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

Re: [ovs-dev] [PATCH] tests: Add track-origins flag to valgrind.

[William Tu]

On Tue, Sep 03, 2019 at 06:07:26PM +0300, Ilya Maximets wrote:
> Useful for tracking where the uninitialized memory came from.
> Report example:
> 
>   Thread 13 revalidator11:
>   Conditional jump or move depends on uninitialised value(s)
>  at 0x4C35D96: __memcmp_sse4_1 (in vgpreload_memcheck.so)
>  by 0x9D4404: ofpbuf_equal (ofpbuf.h:273)
>  by 0x9D4404: revalidate_ukey__ (ofproto-dpif-upcall.c:2219)
>  <...>
>  by 0x6AF488E: clone (clone.S:95)
>Uninitialised value was created by a stack allocation
>  at 0x9D4450: compose_slow_path (ofproto-dpif-upcall.c:1062)
> 
> Signed-off-by: Ilya Maximets 
> ---
Looks good to me.
Tested-by: William Tu 

>  tests/automake.mk | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/automake.mk b/tests/automake.mk
> index d6ab51732..908eb 100644
> --- a/tests/automake.mk
> +++ b/tests/automake.mk
> @@ -264,7 +264,8 @@ $(valgrind_wrappers): tests/valgrind-wrapper.in
>  CLEANFILES += $(valgrind_wrappers)
>  EXTRA_DIST += tests/valgrind-wrapper.in
>  
> -VALGRIND = valgrind --log-file=valgrind.%p --leak-check=full \
> +VALGRIND = valgrind --log-file=valgrind.%p \
> + --leak-check=full --track-origins=yes \
>   --suppressions=$(abs_top_srcdir)/tests/glibc.supp \
>   --suppressions=$(abs_top_srcdir)/tests/openssl.supp --num-callers=20
>  HELGRIND = valgrind --log-file=helgrind.%p --tool=helgrind \
> -- 
> 2.17.1
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] treewide: Use packet batch APIs

[William Tu]

On Tue, Sep 3, 2019 at 10:39 AM Paul Chaignon  wrote:
>
> On Tue, Sep 03, 2019 at 06:37PM +, William Tu  wrote:
> > On Sun, Sep 01, 2019 at 03:10:05PM +0200, Paul Chaignon wrote:
> > > This patch replaces direct accesses to dp_packet_batch and dp_packet
> > > internal components by the appropriate API calls.  It extends commit
> > > 1270b6e52 (treewide: Wider use of packet batch APIs).
> > >
> > > This patch was generated using the following semantic patch (cf.
> > > http://coccinelle.lip6.fr).
> >
> > Looks very interesting, I spent some time learning it.
>
> Glad you find it interesting!  I'm actually trying to practice using it
> myself.  (If you have any usage ideas for Open vSwitch, I'm interested; I
> already went through all the treewide patches.)

I think we can create something like scripts/coccinelle in linux kernel,
and try to match/replace some buggy patterns, and occasionally in ovs
we run 'make check-coccinelle'.
Maybe Ilya has more thoughts about what to match?

>
> > If you have time, can you show us how to run it?
> > I installed coccinelle on ubuntu and I can run on linux kernel
> >  make coccicheck MODE=report
> > but for OVS, do we provide the semantic patch below and run
> > it manually?
>
> I executed the semantic patch below with:
>   spatch --sp-file semantic-patch.cocci --very-quiet \
>  --dir ./ > output.diff
>
> Note that there will be some incorrect changes in lib/dp-packet.h which
> you'll need to remove (e.g., Coccinelle tries to use dp_packet_is_eth()
> inside dp_packet_is_eth()).  It's possible to avoid that, but afaik, only
> at the cost of a slightly more verbose semantic patch.
>

Thanks, now I'm able to get it running.

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

Re: [ovs-dev] [PATCH v1 ovn] OVN: Send RARP for vif ports for which OVN does not know the IP.

[Ankur Sharma]

Hi Numan,

Thanks a lot for trying the patch.
Yes, as of now i kept RARP and GARP separate entities and that lead to code 
duplication.
Sure, I will try to unify them.

I will rebase, make the unification changes and will submit a v2 soon.

Regards,
Ankur

From: Numan Siddique 
Sent: Tuesday, September 3, 2019 3:47 AM
To: Ankur Sharma 
Cc: ovs-dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v1 ovn] OVN: Send RARP for vif ports for which 
OVN does not know the IP.



On Fri, Aug 2, 2019 at 3:12 AM Ankur Sharma 
mailto:ankur.sha...@nutanix.com>> wrote:
ISSUE:
For a VIF port (on a bridged logical switch), OVN sends out
GARPs, advertising port's mac and IP.

However, if a VIF port (on a bridged logical switch) has not
been assigned an IP, then OVN does not advertise anything.
As a result, for such VIF ports basic operations like VM
migration will not work, since TOR will direct the packets
destined to this VIF to old chassis.

This patch addresses the same by advertising reverse ARP (RARP)
for non ip assigned bridged logical switch connected VIF ports.

Signed-off-by: Ankur Sharma 
mailto:ankur.sha...@nutanix.com>>

Hi Ankur,

Can you please rebase this patch too.

Looking the code at a very high level, it seems it has some duplicate code with
garp. Can you please check if it is possible to have common code between
garp and rarp sending.

Thanks
Numan

---
 controller/pinctrl.c | 238 +++
 tests/ovn.at 
[ovn.at]
 |  68 +++
 2 files changed, 306 insertions(+)

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index ebbb52a..191556f 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -209,6 +209,20 @@ static void send_garp_prepare(
 OVS_REQUIRES(pinctrl_mutex);
 static void send_garp_run(struct rconn *swconn, long long int *send_garp_time)
 OVS_REQUIRES(pinctrl_mutex);
+
+static void init_send_rarps(void);
+static void destroy_send_rarps(void);
+static void send_rarp_wait(long long int send_rarp_time);
+static void send_rarp_prepare(
+struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
+struct ovsdb_idl_index *sbrec_port_binding_by_name,
+const struct ovsrec_bridge *,
+const struct sbrec_chassis *,
+const struct hmap *local_datapaths)
+OVS_REQUIRES(pinctrl_mutex);
+static void send_rarp_run(struct rconn *swconn, long long int *send_rarp_time)
+OVS_REQUIRES(pinctrl_mutex);
+
 static void pinctrl_handle_nd_na(struct rconn *swconn,
  const struct flow *ip_flow,
  const struct match *md,
@@ -428,6 +442,7 @@ pinctrl_init(void)
 {
 init_put_mac_bindings();
 init_send_garps();
+init_send_rarps();
 init_ipv6_ras();
 init_buffered_packets_map();
 init_event_table();
@@ -2024,6 +2039,8 @@ pinctrl_handler(void *arg_)
 static long long int send_ipv6_ra_time = LLONG_MAX;
 /* Next GARP announcement in ms. */
 static long long int send_garp_time = LLONG_MAX;
+/* Next RARP announcement in ms. */
+static long long int send_rarp_time = LLONG_MAX;
 /* Next multicast query (IGMP) in ms. */
 static long long int send_mcast_query_time = LLONG_MAX;

@@ -2078,6 +2095,7 @@ pinctrl_handler(void *arg_)
 if (may_inject_pkts()) {
 ovs_mutex_lock(_mutex);
 send_garp_run(swconn, _garp_time);
+send_rarp_run(swconn, _rarp_time);
 send_ipv6_ras(swconn, _ipv6_ra_time);
 send_mac_binding_buffered_pkts(swconn);
 ovs_mutex_unlock(_mutex);
@@ -2089,6 +2107,7 @@ pinctrl_handler(void *arg_)
 rconn_run_wait(swconn);
 rconn_recv_wait(swconn);
 send_garp_wait(send_garp_time);
+send_rarp_wait(send_rarp_time);
 ipv6_ra_wait(send_ipv6_ra_time);
 ip_mcast_querier_wait(send_mcast_query_time);

@@ -2138,6 +2157,9 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
 send_garp_prepare(sbrec_port_binding_by_datapath,
   sbrec_port_binding_by_name, br_int, chassis,
   local_datapaths, active_tunnels);
+send_rarp_prepare(sbrec_port_binding_by_datapath,
+  sbrec_port_binding_by_name, br_int, chassis,
+  local_datapaths);
 prepare_ipv6_ras(sbrec_port_binding_by_datapath,
  sbrec_port_binding_by_name, local_datapaths);
 sync_dns_cache(dns_table);
@@ -2494,6 +2516,7 @@ pinctrl_destroy(void)
 latch_destroy(_thread_exit);
 free(pinctrl.br_int_name);
 destroy_send_garps();
+destroy_send_rarps();
 destroy_ipv6_ras();
 destroy_buffered_packets_map();
 event_table_destroy();
@@ -2785,6 +2808,9 @@ struct garp_data {
 /* 

Re: [ovs-dev] [PATCH v2 ovn] Replace chassis mac with router port mac on destination chassis

[Ankur Sharma]

Hi Numan,

Thanks a lot for trying the patch.
Just submitted the v3.

Regards,
Ankur

From: Numan Siddique 
Sent: Tuesday, September 3, 2019 3:42 AM
To: Ankur Sharma 
Cc: ovs-dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v2 ovn] Replace chassis mac with router port mac 
on destination chassis



On Tue, Aug 27, 2019 at 7:31 AM Ankur Sharma 
mailto:ankur.sha...@nutanix.com>> wrote:
During E-W routing for vlan backed networks, we replace router port
mac with chassis mac, when packet leaves the source hypervisor.

As a result, the destination VM (on remote hypervisor) will see
chassis mac as source mac in received packet.

Although, functionality wise this does not cause any issue,
however chassis mac being see as source on VM, will
lead to following:
a. INCONSISTENT SOURCE MAC:
   If the destination VM moves to same hypervisor as sender,
   then it will see router port mac as source mac. Whereas, on
   a remote hypervisor, source mac will be the sender chassis mac.

   This will cause inconsistency in packet headers for the same
   flow and could be confusing for someone looking at packet
   captures inside the vm.

b. SYSTEM MAC BEING EXPOSED TO VM:
   Chassis mac is a CMS provided mac, i.e it is an infrastructure
   mac. It is not a good practice to expose such values to VM,
   which should not be seeing them in first place.

In order to replace chassis mac with router port mac, we will
do following.

a. Create conjunction for each chassis mac and router port vlan
   id combination. For example, for a 2 node chassis setup, where
   we have a logical router, connected to 4 logical switches with
   vlan ids: 2000, 1000, 0 and 24, the conjunction flow will look
   like following:

   cookie=0x0, duration=9094.608s, table=0, n_packets=0, n_bytes=0, 
idle_age=9094, priority=180,dl_src=aa:bb:cc:dd:ee:22 
actions=conjunction(100,1/2)
   cookie=0x0, duration=9094.608s, table=0, n_packets=0, n_bytes=0, 
idle_age=9094, priority=180,dl_src=aa:bb:cc:dd:ff:ee 
actions=conjunction(100,1/2)

   cookie=0x0, duration=9094.552s, table=0, n_packets=0, n_bytes=0, 
idle_age=9094, priority=180,dl_vlan=2000 actions=conjunction(100,2/2)
   cookie=0x0, duration=9094.552s, table=0, n_packets=0, n_bytes=0, 
idle_age=9094, priority=180,dl_vlan=1000 actions=conjunction(100,2/2)
   cookie=0x0, duration=9094.552s, table=0, n_packets=0, n_bytes=0, 
idle_age=9094, priority=180,vlan_tci=0x/0x1fff actions=conjunction(100,2/2)
   cookie=0x0, duration=9094.552s, table=0, n_packets=0, n_bytes=0, 
idle_age=9094, priority=180,dl_vlan=24 actions=conjunction(100,2/2)

b. Using this conjunction as match, we can identify if packet entering 
destination
   hypervisor was routed at the source or not. This will be done in table=0 
(Physical to logical)
   at priority=180.
   For example:
   cookie=0x0, duration=9795.957s, table=0, n_packets=1391, n_bytes=141882, 
idle_age=8396, priority=180,conj_id=100,in_port=146,dl_vlan=1000 
actions=.,mod_dl_src:00:00:01:01:02:03,...

c. We use conjunction, as it will ensure that we do not end up having lot of 
flows
   as we scale up. If we do not use conjunction, then we will have
   N (number of chassis macs) X M (number of router vlans) number of ovs flows.
   Conjunction converts it to N + M.
   Consider a setup, with 500 Chassis and 500 routed vlans.
   Without conjunction we will need 25000 (500 * 500) flows,
   whereas with conjunction that number comes down to 1000 (500 + 500).

Signed-off-by: Ankur Sharma 
mailto:ankur.sha...@nutanix.com>>

Hi Ankur,

Looks like you have to rebase this patch again.

Can you please resubmit again.

Thanks
Numan

---
 controller/chassis.c|   2 +-
 controller/chassis.h|   3 +
 controller/ovn-controller.c |   5 +
 controller/physical.c   | 222 ++--
 controller/physical.h   |   1 +
 ovn-architecture.7.xml  |  10 +-
 tests/ovn.at 
[ovn.at]
|  12 ++-
 7 files changed, 242 insertions(+), 13 deletions(-)

diff --git a/controller/chassis.c b/controller/chassis.c
index 937c557..699b662 100644
--- a/controller/chassis.c
+++ b/controller/chassis.c
@@ -144,7 +144,7 @@ get_bridge_mappings(const struct smap *ext_ids)
 return smap_get_def(ext_ids, "ovn-bridge-mappings", "");
 }

-static const char *
+const char *
 get_chassis_mac_mappings(const struct smap *ext_ids)
 {
 return smap_get_def(ext_ids, "ovn-chassis-mac-mappings", "");
diff --git a/controller/chassis.h b/controller/chassis.h
index eb46ca3..178d295 100644
--- a/controller/chassis.h
+++ b/controller/chassis.h
@@ -27,6 +27,7 @@ struct sbrec_chassis;
 struct sbrec_chassis_table;
 struct sset;
 struct eth_addr;
+struct smap;

 void chassis_register_ovs_idl(struct ovsdb_idl *);
 const struct sbrec_chassis 

[ovs-dev] [PATCH v3 ovn] Replace chassis mac with router port mac on destination chassis

[Ankur Sharma]

During E-W routing for vlan backed networks, we replace router port
mac with chassis mac, when packet leaves the source hypervisor.

As a result, the destination VM (on remote hypervisor) will see
chassis mac as source mac in received packet.

Although, functionality wise this does not cause any issue,
however chassis mac being see as source on VM, will
lead to following:
a. INCONSISTENT SOURCE MAC:
   If the destination VM moves to same hypervisor as sender,
   then it will see router port mac as source mac. Whereas, on
   a remote hypervisor, source mac will be the sender chassis mac.

   This will cause inconsistency in packet headers for the same
   flow and could be confusing for someone looking at packet
   captures inside the vm.

b. SYSTEM MAC BEING EXPOSED TO VM:
   Chassis mac is a CMS provided mac, i.e it is an infrastructure
   mac. It is not a good practice to expose such values to VM,
   which should not be seeing them in first place.

In order to replace chassis mac with router port mac, we will
do following.

a. Create conjunction for each chassis mac and router port vlan
   id combination. For example, for a 2 node chassis setup, where
   we have a logical router, connected to 4 logical switches with
   vlan ids: 2000, 1000, 0 and 24, the conjunction flow will look
   like following:

   cookie=0x0, duration=9094.608s, table=0, n_packets=0, n_bytes=0, 
idle_age=9094, priority=180,dl_src=aa:bb:cc:dd:ee:22 
actions=conjunction(100,1/2)
   cookie=0x0, duration=9094.608s, table=0, n_packets=0, n_bytes=0, 
idle_age=9094, priority=180,dl_src=aa:bb:cc:dd:ff:ee 
actions=conjunction(100,1/2)

   cookie=0x0, duration=9094.552s, table=0, n_packets=0, n_bytes=0, 
idle_age=9094, priority=180,dl_vlan=2000 actions=conjunction(100,2/2)
   cookie=0x0, duration=9094.552s, table=0, n_packets=0, n_bytes=0, 
idle_age=9094, priority=180,dl_vlan=1000 actions=conjunction(100,2/2)
   cookie=0x0, duration=9094.552s, table=0, n_packets=0, n_bytes=0, 
idle_age=9094, priority=180,vlan_tci=0x/0x1fff actions=conjunction(100,2/2)
   cookie=0x0, duration=9094.552s, table=0, n_packets=0, n_bytes=0, 
idle_age=9094, priority=180,dl_vlan=24 actions=conjunction(100,2/2)

b. Using this conjunction as match, we can identify if packet entering 
destination
   hypervisor was routed at the source or not. This will be done in table=0 
(Physical to logical)
   at priority=180.
   For example:
   cookie=0x0, duration=9795.957s, table=0, n_packets=1391, n_bytes=141882, 
idle_age=8396, priority=180,conj_id=100,in_port=146,dl_vlan=1000 
actions=.,mod_dl_src:00:00:01:01:02:03,...

c. We use conjunction, as it will ensure that we do not end up having lot of 
flows
   as we scale up. If we do not use conjunction, then we will have
   N (number of chassis macs) X M (number of router vlans) number of ovs flows.
   Conjunction converts it to N + M.
   Consider a setup, with 500 Chassis and 500 routed vlans.
   Without conjunction we will need 25000 (500 * 500) flows,
   whereas with conjunction that number comes down to 1000 (500 + 500).

Signed-off-by: Ankur Sharma 
---
 controller/chassis.c|   2 +-
 controller/chassis.h|   3 +
 controller/ovn-controller.c |   5 +
 controller/physical.c   | 222 ++--
 controller/physical.h   |   1 +
 ovn-architecture.7.xml  |  10 +-
 tests/ovn.at|  12 ++-
 7 files changed, 242 insertions(+), 13 deletions(-)

diff --git a/controller/chassis.c b/controller/chassis.c
index 937c557..699b662 100644
--- a/controller/chassis.c
+++ b/controller/chassis.c
@@ -144,7 +144,7 @@ get_bridge_mappings(const struct smap *ext_ids)
 return smap_get_def(ext_ids, "ovn-bridge-mappings", "");
 }
 
-static const char *
+const char *
 get_chassis_mac_mappings(const struct smap *ext_ids)
 {
 return smap_get_def(ext_ids, "ovn-chassis-mac-mappings", "");
diff --git a/controller/chassis.h b/controller/chassis.h
index eb46ca3..178d295 100644
--- a/controller/chassis.h
+++ b/controller/chassis.h
@@ -27,6 +27,7 @@ struct sbrec_chassis;
 struct sbrec_chassis_table;
 struct sset;
 struct eth_addr;
+struct smap;
 
 void chassis_register_ovs_idl(struct ovsdb_idl *);
 const struct sbrec_chassis *chassis_run(
@@ -42,5 +43,7 @@ bool chassis_get_mac(const struct sbrec_chassis *chassis,
  const char *bridge_mapping,
  struct eth_addr *chassis_mac);
 const char *chassis_get_id(void);
+const char * get_chassis_mac_mappings(const struct smap *ext_ids);
+
 
 #endif /* controller/chassis.h */
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index e27b56b..b5449b8 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -1268,9 +1268,14 @@ en_flow_output_run(struct engine_node *node)
 (struct sbrec_port_binding_table *)EN_OVSDB_GET(
 engine_get_input("SB_port_binding", node));
 
+struct sbrec_chassis_table *chassis_table =
+(struct 

Re: [ovs-dev] [PATCH] rhel: Revert RHEL 7.4 comp_ver change

[Gregory Rose]

On 9/3/2019 10:02 AM, Guru Shetty wrote:



On Tue, 3 Sep 2019 at 08:50, Greg Rose > wrote:


I looked at the wrong list of kernels when I changed the value for the
RHEL 7.4 comp_ver variable.  Revert that part of commit e64c2c1
("rhel: Fix ovs-kmod-manage.sh to work with RHEL 7.3").

Fixes: e64c2c1 ("rhel: Fix ovs-kmod-manage.sh to work with RHEL 7.3")
Signed-off-by: Greg Rose mailto:gvrose8...@gmail.com>>

Thanks. I pushed this to master, 2.12 and 2.11




Thanks Guru!


---
 rhel/usr_share_openvswitch_scripts_ovs-kmod-manage.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rhel/usr_share_openvswitch_scripts_ovs-kmod-manage.sh
b/rhel/usr_share_openvswitch_scripts_ovs-kmod-manage.sh
index c5b1d2d..693fb0b 100644
--- a/rhel/usr_share_openvswitch_scripts_ovs-kmod-manage.sh
+++ b/rhel/usr_share_openvswitch_scripts_ovs-kmod-manage.sh
@@ -87,7 +87,7 @@ if [ "$mainline_major" = "3" ] && [
"$mainline_minor" = "10" ]; then
         installed_ver="$minor_rev"
     elif [ "$major_rev" = "693" ]; then
 #        echo "rhel74"
-        comp_ver=21
+        comp_ver=11
         ver_offset=4
         installed_ver="$minor_rev"
     elif [ "$major_rev" = "862" ]; then
-- 
1.8.3.1


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



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

Re: [ovs-dev] [PATCH v2] treewide: Use packet batch APIs

[Paul Chaignon]

On Tue, Sep 03, 2019 at 06:37PM +, William Tu  wrote:
> On Sun, Sep 01, 2019 at 03:10:05PM +0200, Paul Chaignon wrote:
> > This patch replaces direct accesses to dp_packet_batch and dp_packet
> > internal components by the appropriate API calls.  It extends commit
> > 1270b6e52 (treewide: Wider use of packet batch APIs).
> >
> > This patch was generated using the following semantic patch (cf.
> > http://coccinelle.lip6.fr).
>
> Looks very interesting, I spent some time learning it.

Glad you find it interesting!  I'm actually trying to practice using it
myself.  (If you have any usage ideas for Open vSwitch, I'm interested; I
already went through all the treewide patches.)

> If you have time, can you show us how to run it?
> I installed coccinelle on ubuntu and I can run on linux kernel
>  make coccicheck MODE=report
> but for OVS, do we provide the semantic patch below and run
> it manually?

I executed the semantic patch below with:
  spatch --sp-file semantic-patch.cocci --very-quiet \
 --dir ./ > output.diff

Note that there will be some incorrect changes in lib/dp-packet.h which
you'll need to remove (e.g., Coccinelle tries to use dp_packet_is_eth()
inside dp_packet_is_eth()).  It's possible to avoid that, but afaik, only
at the cost of a slightly more verbose semantic patch.

>
> >
> > // 
> > @ dp_packet @
> > struct dp_packet_batch *b1;
> > struct dp_packet_batch b2;
> > struct dp_packet *p;
> > expression e;
> > @@
> >
> > (
> > - b1->packets[b1->count++] = p;
> > + dp_packet_batch_add(b1, p);
> > |
> > - b2.packets[b2.count++] = p;
> > + dp_packet_batch_add(, p);
> > |
> > - p->packet_type == htonl(PT_ETH)
> > + dp_packet_is_eth(p)
> > |
> > - p->packet_type != htonl(PT_ETH)
> > + !dp_packet_is_eth(p)
> > |
> > - b1->count == 0
> > + dp_packet_batch_is_empty(b1)
> > |
> > - !b1->count
> > + dp_packet_batch_is_empty(b1)
>
> I understand the above using + and -
> But can you explain the lines below?

If I were to remove the four following matches on count and keep only the
last two, Coccinelle would attempt to replace count assignments and
increments with dp_packet_batch_size().  I might end up with e.g.:

  - batch->count = 0;
  + dp_packet_batch_size(batch) = 0;

In short, I'm using these four matches to ensure the last two matches
replace only rvalues.

> > |
> >   b1->count = e;
> > |
> >   b1->count++
> > |
> >   b2.count = e;
> > |
> >   b2.count++
> > |
> Why do we need "expression e" and are they match and replace something?

The four preceding matches don't replace code as there's no -/+ in the
leftmost column.
Expression e represents anything that might be on the right-hand side.
The semantic patch doesn't run without it, I'm guessing because the
statement is incomplete.

>
> > - b1->count
> > + dp_packet_batch_size(b1)
> > |
> > - b2.count
> > + dp_packet_batch_size()
> > )
> > // 
> >
> > Signed-off-by: Paul Chaignon 
>
> Looks good to me, thanks for the patch.
> Acked-by: William Tu 
>
>
> > ---
> > Changelogs:
> >   Changes in v2:
> > - Rebased on master branch.
> > - Re-applied the semantic patch (new changes in lib/netdev-afxdp.c).
> >
> >  lib/dpif-netdev.c  |  7 ---
> >  lib/flow.c |  2 +-
> >  lib/netdev-afxdp.c | 20 
> >  lib/netdev-dpdk.c  |  3 ++-
> >  lib/netdev-dummy.c |  2 +-
> >  lib/packets.c  |  4 ++--
> >  lib/pcap-file.c|  2 +-
> >  7 files changed, 23 insertions(+), 17 deletions(-)
> >
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index 75d85b2fd..d24f9502f 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -4261,7 +4261,7 @@ dp_netdev_process_rxq_port(struct 
> > dp_netdev_pmd_thread *pmd,
> >  /* At least one packet received. */
> >  *recirc_depth_get() = 0;
> >  pmd_thread_ctx_time_update(pmd);
> > -batch_cnt = batch.count;
> > +batch_cnt = dp_packet_batch_size();
> >  if (pmd_perf_metrics_enabled(pmd)) {
> >  /* Update batch histogram. */
> >  s->current.batches++;
> > @@ -6292,7 +6292,7 @@ packet_batch_per_flow_update(struct 
> > packet_batch_per_flow *batch,
> >  {
> >  batch->byte_count += dp_packet_size(packet);
> >  batch->tcp_flags |= tcp_flags;
> > -batch->array.packets[batch->array.count++] = packet;
> > +dp_packet_batch_add(>array, packet);
> >  }
> >
> >  static inline void
> > @@ -6314,7 +6314,8 @@ packet_batch_per_flow_execute(struct 
> > packet_batch_per_flow *batch,
> >  struct dp_netdev_actions *actions;
> >  struct dp_netdev_flow *flow = batch->flow;
> >
> > -dp_netdev_flow_used(flow, batch->array.count, batch->byte_count,
> > +dp_netdev_flow_used(flow, dp_packet_batch_size(>array),
> > +batch->byte_count,
> >  batch->tcp_flags, pmd->ctx.now / 1000);
> >
> >  actions = dp_netdev_flow_get_actions(flow);
> > diff --git a/lib/flow.c b/lib/flow.c
> > index ac6a4e121..393243309 100644
> > --- a/lib/flow.c
> 

Re: [ovs-dev] [PATCH] rhel: Revert RHEL 7.4 comp_ver change

[Guru Shetty]

On Tue, 3 Sep 2019 at 08:50, Greg Rose  wrote:

> I looked at the wrong list of kernels when I changed the value for the
> RHEL 7.4 comp_ver variable.  Revert that part of commit e64c2c1
> ("rhel: Fix ovs-kmod-manage.sh to work with RHEL 7.3").
>
> Fixes: e64c2c1 ("rhel: Fix ovs-kmod-manage.sh to work with RHEL 7.3")
> Signed-off-by: Greg Rose 
>
Thanks. I pushed this to master, 2.12 and 2.11




> ---
>  rhel/usr_share_openvswitch_scripts_ovs-kmod-manage.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/rhel/usr_share_openvswitch_scripts_ovs-kmod-manage.sh
> b/rhel/usr_share_openvswitch_scripts_ovs-kmod-manage.sh
> index c5b1d2d..693fb0b 100644
> --- a/rhel/usr_share_openvswitch_scripts_ovs-kmod-manage.sh
> +++ b/rhel/usr_share_openvswitch_scripts_ovs-kmod-manage.sh
> @@ -87,7 +87,7 @@ if [ "$mainline_major" = "3" ] && [ "$mainline_minor" =
> "10" ]; then
>  installed_ver="$minor_rev"
>  elif [ "$major_rev" = "693" ]; then
>  #echo "rhel74"
> -comp_ver=21
> +comp_ver=11
>  ver_offset=4
>  installed_ver="$minor_rev"
>  elif [ "$major_rev" = "862" ]; then
> --
> 1.8.3.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] SIGILL ovs branch-2.12/arm64/DPDK

[Ilya Maximets]

> Hi
> 
> I've been testing non-x86 architecture builds for the upcoming 2.12 release
> of OVS and I'm hitting an issue with DPDK enabled builds on the arm64
> architecture.
> 
> branch-2.12 includes improvements for native hashing under arm64; these
> appear to work fine when DPDK is not in the mix, but with DPDK enabled, I
> get a SIGILL in lib/hash.c:

Hi.

What is your target platform?
One explanation I could imagine is that DPDK blindly forces -march=armv8-a+crc
defining __ARM_FEATURE_CRC32 while your cpu doesn't support crc32 extension.

Do you have crc32 in the list of cpu features in /proc/cpuinfo ?

Best regards, Ilya Maximets.

> 
> Program received signal SIGILL, Illegal instruction.
> hash_bytes (p_=p_ at entry=0xaac36e50, n=7, basis=basis at entry=0) at
> ../lib/hash.c:38
> 38 ../lib/hash.c: No such file or directory.
> 
> (gdb) backtrace full
> #0  hash_bytes (p_=p_ at entry=0xaac36e50, n=7, basis=basis at entry=0) at
> ../lib/hash.c:38
> p = 0xaac36e50
> orig_n = 7
> hash = 0
> #1  0xaab4238c in hash_string (basis=0, s=0xaac36e50 "comment")
> at ../lib/hash.h:342
> No locals.
> #2  hash_name (name=name at entry=0xaac36e50 "comment") at
> ../lib/shash.c:28
> No locals.
> #3  0xaab42a1c in shash_find (sh=sh at entry=0xaaceb6f8
> , name=name at entry=0xaac36e50 "comment")
> at ../lib/shash.c:231
> No locals.
> #4  0xaab42a60 in shash_add_once (sh=0xaaceb6f8 ,
> name=0xaac36e50 "comment",
> data=0xaacdb0d8 ) at ../lib/shash.c:135
> No locals.
> #5  0xaab42ab4 in shash_add_assert (sh=sh at entry=0xaaceb6f8
> , name=,
> data=data at entry=0xaacdb0d8 ) at ../lib/shash.c:146
> __func__ = "shash_add_assert"
> #6  0xaab05900 in ctl_register_command (command=0xaacdb0d8
> ) at ../lib/db-ctl-base.c:2470
> No locals.
> #7  ctl_register_commands (commands=0xaacdb0d8 ) at
> ../lib/db-ctl-base.c:2470
> p = 0xaacdb0d8 
> p = 
> #8  ctl_init__ (idl_class_=, ctl_classes_=,
> cmd_show_tables_=0x0, ctl_exit_func_=)
> at ../lib/db-ctl-base.c:2486
> No locals.
> #9  0xaaad5768 in nbctl_cmd_init () at
> ../ovn/utilities/ovn-nbctl.c:5695
> No locals.
> #10 main (argc=1, argv=0xf4d8) at ../ovn/utilities/ovn-nbctl.c:137
> idl = 
> local_options = {map = {buckets = 0x0, one = 0xf390, mask =
> 281473881441200, n = 187649986051872}}
> args = 
> parsed_options = 0xf350
> n_parsed_options = 187649986051968
> error_s = 
> socket_name = 
> daemon_mode = 
> 
> This effects pretty much all of the binaries built with DPDK enabled.
> 
> gcc version is 9.2.1 on Ubuntu eoan; I've not tested on earlier gcc
> versions (yet).
> 
> Any help/pointers much appreciated.
> 
> Cheers
> 
> James

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

Re: [ovs-dev] [PATCH v2] treewide: Use packet batch APIs

[William Tu]

On Sun, Sep 01, 2019 at 03:10:05PM +0200, Paul Chaignon wrote:
> This patch replaces direct accesses to dp_packet_batch and dp_packet
> internal components by the appropriate API calls.  It extends commit
> 1270b6e52 (treewide: Wider use of packet batch APIs).
> 
> This patch was generated using the following semantic patch (cf.
> http://coccinelle.lip6.fr).

Looks very interesting, I spent some time learning it.
If you have time, can you show us how to run it?
I installed coccinelle on ubuntu and I can run on linux kernel
 make coccicheck MODE=report
but for OVS, do we provide the semantic patch below and run
it manually?

> 
> // 
> @ dp_packet @
> struct dp_packet_batch *b1;
> struct dp_packet_batch b2;
> struct dp_packet *p;
> expression e;
> @@
> 
> (
> - b1->packets[b1->count++] = p;
> + dp_packet_batch_add(b1, p);
> |
> - b2.packets[b2.count++] = p;
> + dp_packet_batch_add(, p);
> |
> - p->packet_type == htonl(PT_ETH)
> + dp_packet_is_eth(p)
> |
> - p->packet_type != htonl(PT_ETH)
> + !dp_packet_is_eth(p)
> |
> - b1->count == 0
> + dp_packet_batch_is_empty(b1)
> |
> - !b1->count
> + dp_packet_batch_is_empty(b1)

I understand the above using + and -
But can you explain the lines below?
> |
>   b1->count = e;
> |
>   b1->count++
> |
>   b2.count = e;
> |
>   b2.count++
> |
Why do we need "expression e" and are they match and replace something?

> - b1->count
> + dp_packet_batch_size(b1)
> |
> - b2.count
> + dp_packet_batch_size()
> )
> // 
> 
> Signed-off-by: Paul Chaignon 

Looks good to me, thanks for the patch.
Acked-by: William Tu 


> ---
> Changelogs:
>   Changes in v2:
> - Rebased on master branch.
> - Re-applied the semantic patch (new changes in lib/netdev-afxdp.c).
> 
>  lib/dpif-netdev.c  |  7 ---
>  lib/flow.c |  2 +-
>  lib/netdev-afxdp.c | 20 
>  lib/netdev-dpdk.c  |  3 ++-
>  lib/netdev-dummy.c |  2 +-
>  lib/packets.c  |  4 ++--
>  lib/pcap-file.c|  2 +-
>  7 files changed, 23 insertions(+), 17 deletions(-)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 75d85b2fd..d24f9502f 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -4261,7 +4261,7 @@ dp_netdev_process_rxq_port(struct dp_netdev_pmd_thread 
> *pmd,
>  /* At least one packet received. */
>  *recirc_depth_get() = 0;
>  pmd_thread_ctx_time_update(pmd);
> -batch_cnt = batch.count;
> +batch_cnt = dp_packet_batch_size();
>  if (pmd_perf_metrics_enabled(pmd)) {
>  /* Update batch histogram. */
>  s->current.batches++;
> @@ -6292,7 +6292,7 @@ packet_batch_per_flow_update(struct 
> packet_batch_per_flow *batch,
>  {
>  batch->byte_count += dp_packet_size(packet);
>  batch->tcp_flags |= tcp_flags;
> -batch->array.packets[batch->array.count++] = packet;
> +dp_packet_batch_add(>array, packet);
>  }
>  
>  static inline void
> @@ -6314,7 +6314,8 @@ packet_batch_per_flow_execute(struct 
> packet_batch_per_flow *batch,
>  struct dp_netdev_actions *actions;
>  struct dp_netdev_flow *flow = batch->flow;
>  
> -dp_netdev_flow_used(flow, batch->array.count, batch->byte_count,
> +dp_netdev_flow_used(flow, dp_packet_batch_size(>array),
> +batch->byte_count,
>  batch->tcp_flags, pmd->ctx.now / 1000);
>  
>  actions = dp_netdev_flow_get_actions(flow);
> diff --git a/lib/flow.c b/lib/flow.c
> index ac6a4e121..393243309 100644
> --- a/lib/flow.c
> +++ b/lib/flow.c
> @@ -1098,7 +1098,7 @@ parse_tcp_flags(struct dp_packet *packet)
>  ovs_be16 dl_type;
>  uint8_t nw_frag = 0, nw_proto = 0;
>  
> -if (packet->packet_type != htonl(PT_ETH)) {
> +if (!dp_packet_is_eth(packet)) {
>  return 0;
>  }
>  
> diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
> index e5b058d08..6e0180327 100644
> --- a/lib/netdev-afxdp.c
> +++ b/lib/netdev-afxdp.c
> @@ -765,7 +765,7 @@ free_afxdp_buf_batch(struct dp_packet_batch *batch)
>  addr = (uintptr_t)base & (~FRAME_SHIFT_MASK);
>  elems[i] = (void *)addr;
>  }
> -umem_elem_push_n(xpacket->mpool, batch->count, elems);
> +umem_elem_push_n(xpacket->mpool, dp_packet_batch_size(batch), elems);
>  dp_packet_batch_init(batch);
>  }
>  
> @@ -860,9 +860,11 @@ __netdev_afxdp_batch_send(struct netdev *netdev, int qid,
>  free_batch = check_free_batch(batch);
>  
>  umem = xsk_info->umem;
> -ret = umem_elem_pop_n(>mpool, batch->count, elems_pop);
> +ret = umem_elem_pop_n(>mpool, dp_packet_batch_size(batch),
> +  elems_pop);
>  if (OVS_UNLIKELY(ret)) {
> -atomic_add_relaxed(_info->tx_dropped, batch->count, );
> +atomic_add_relaxed(_info->tx_dropped, 
> dp_packet_batch_size(batch),
> +   );
>  VLOG_WARN_RL(, "%s: send failed due to exhausted memory pool.",
>   netdev_get_name(netdev));
>  error = ENOMEM;
> @@ -870,10 +872,12 @@ 

Re: [ovs-dev] [PATCH v3] flow: fix incorrect padding length checking in ipv6_sanity_check

[William Tu]

On Mon, Sep 02, 2019 at 04:36:47PM +0800, Yanqin Wei wrote:
> The padding length is (packet size - ipv6 header length - ipv6 plen).  This
> patch fixes incorrect padding size checking in ipv6_sanity_check.
> 
> Reviewed-by: Gavin Hu 
> Signed-off-by: Yanqin Wei 

Looks good to me.

Acked-by: William Tu 

> ---
>  lib/flow.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/flow.c b/lib/flow.c
> index ac6a4e1..0413c67 100644
> --- a/lib/flow.c
> +++ b/lib/flow.c
> @@ -699,7 +699,7 @@ ipv6_sanity_check(const struct ovs_16aligned_ip6_hdr *nh, 
> size_t size)
>  return false;
>  }
>  /* Jumbo Payload option not supported yet. */
> -if (OVS_UNLIKELY(size - plen > UINT8_MAX)) {
> +if (OVS_UNLIKELY(size - (plen + IPV6_HEADER_LEN) > UINT8_MAX)) {
>  return false;
>  }
>  
> -- 
> 2.7.4
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] rhel: Revert RHEL 7.4 comp_ver change

[Greg Rose]

I looked at the wrong list of kernels when I changed the value for the
RHEL 7.4 comp_ver variable.  Revert that part of commit e64c2c1
("rhel: Fix ovs-kmod-manage.sh to work with RHEL 7.3").

Fixes: e64c2c1 ("rhel: Fix ovs-kmod-manage.sh to work with RHEL 7.3")
Signed-off-by: Greg Rose 
---
 rhel/usr_share_openvswitch_scripts_ovs-kmod-manage.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rhel/usr_share_openvswitch_scripts_ovs-kmod-manage.sh 
b/rhel/usr_share_openvswitch_scripts_ovs-kmod-manage.sh
index c5b1d2d..693fb0b 100644
--- a/rhel/usr_share_openvswitch_scripts_ovs-kmod-manage.sh
+++ b/rhel/usr_share_openvswitch_scripts_ovs-kmod-manage.sh
@@ -87,7 +87,7 @@ if [ "$mainline_major" = "3" ] && [ "$mainline_minor" = "10" 
]; then
 installed_ver="$minor_rev"
 elif [ "$major_rev" = "693" ]; then
 #echo "rhel74"
-comp_ver=21
+comp_ver=11
 ver_offset=4
 installed_ver="$minor_rev"
 elif [ "$major_rev" = "862" ]; then
-- 
1.8.3.1

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

[ovs-dev] [PATCH] tests: Add track-origins flag to valgrind.

[Ilya Maximets]

Useful for tracking where the uninitialized memory came from.
Report example:

  Thread 13 revalidator11:
  Conditional jump or move depends on uninitialised value(s)
 at 0x4C35D96: __memcmp_sse4_1 (in vgpreload_memcheck.so)
 by 0x9D4404: ofpbuf_equal (ofpbuf.h:273)
 by 0x9D4404: revalidate_ukey__ (ofproto-dpif-upcall.c:2219)
 <...>
 by 0x6AF488E: clone (clone.S:95)
   Uninitialised value was created by a stack allocation
 at 0x9D4450: compose_slow_path (ofproto-dpif-upcall.c:1062)

Signed-off-by: Ilya Maximets 
---
 tests/automake.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/automake.mk b/tests/automake.mk
index d6ab51732..908eb 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -264,7 +264,8 @@ $(valgrind_wrappers): tests/valgrind-wrapper.in
 CLEANFILES += $(valgrind_wrappers)
 EXTRA_DIST += tests/valgrind-wrapper.in
 
-VALGRIND = valgrind --log-file=valgrind.%p --leak-check=full \
+VALGRIND = valgrind --log-file=valgrind.%p \
+   --leak-check=full --track-origins=yes \
--suppressions=$(abs_top_srcdir)/tests/glibc.supp \
--suppressions=$(abs_top_srcdir)/tests/openssl.supp --num-callers=20
 HELGRIND = valgrind --log-file=helgrind.%p --tool=helgrind \
-- 
2.17.1

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

[ovs-dev] [PATCH v5 ovn 3/3] northd: add rate limiting support for SB controller events

[Lorenzo Bianconi]

Introduce the capability to associate a meter to each controller event
type in order to not overload the pinctrl thread under heavy load.
Each event type relies on a meter with a defined name:
- empty_lb_backends: event-elb

Acked-by: Mark Michelson 
Signed-off-by: Lorenzo Bianconi 
---
 northd/ovn-northd.8.xml |  5 
 northd/ovn-northd.c | 55 ++---
 ovn-nb.xml  |  8 ++
 tests/ovn.at|  1 +
 4 files changed, 55 insertions(+), 14 deletions(-)

diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 442e899bc..c0a54bcce 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -301,6 +301,11 @@
   Pre-stateful to send IP packets to the connection tracker
   for packet de-fragmentation before eventually advancing to ingress table
   LB.
+  If controller_event has been enabled and load balancing rules with
+  empty backends have been added in OVN_Northbound, a 130 flow
+  is added to trigger ovn-controller events whenever the chassis receives a
+  packet for that particular VIP. If event-elb meter has been
+  previously created, it will be associated to the empty_lb logical flow
 
 
 Ingress Table 5: Pre-stateful
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 679d36d88..df7a5eca9 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -4018,14 +4018,18 @@ static void
 build_empty_lb_event_flow(struct ovn_datapath *od, struct hmap *lflows,
   struct smap_node *node, char *ip_address,
   struct nbrec_load_balancer *lb, uint16_t port,
-  int addr_family, int pl)
+  int addr_family, int pl, struct shash *meter_groups)
 {
 if (!controller_event_en || node->value[0]) {
 return;
 }
 
 struct ds match = DS_EMPTY_INITIALIZER;
-char *action;
+char *meter = "", *action;
+
+if (meter_groups && shash_find(meter_groups, "event-elb")) {
+meter = "event-elb";
+}
 
 if (addr_family == AF_INET) {
 ds_put_format(, "ip4.dst == %s && %s",
@@ -4039,18 +4043,20 @@ build_empty_lb_event_flow(struct ovn_datapath *od, 
struct hmap *lflows,
   port);
 }
 action = xasprintf("trigger_event(event = \"%s\", "
-   "vip = \"%s\", protocol = \"%s\", "
-   "load_balancer = \"" UUID_FMT "\");",
-   event_to_string(OVN_EVENT_EMPTY_LB_BACKENDS),
-   node->key, lb->protocol,
-   UUID_ARGS(>header_.uuid));
+   "meter = \"%s\", vip = \"%s\", "
+   "protocol = \"%s\", "
+   "load_balancer = \"" UUID_FMT "\");",
+   event_to_string(OVN_EVENT_EMPTY_LB_BACKENDS),
+   meter, node->key, lb->protocol,
+   UUID_ARGS(>header_.uuid));
 ovn_lflow_add(lflows, od, pl, 130, ds_cstr(), action);
 ds_destroy();
 free(action);
 }
 
 static void
-build_pre_lb(struct ovn_datapath *od, struct hmap *lflows)
+build_pre_lb(struct ovn_datapath *od, struct hmap *lflows,
+ struct shash *meter_groups)
 {
 /* Do not send ND packets to conntrack */
 ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110,
@@ -4087,7 +4093,8 @@ build_pre_lb(struct ovn_datapath *od, struct hmap *lflows)
 }
 
 build_empty_lb_event_flow(od, lflows, node, ip_address, lb,
-  port, addr_family, S_SWITCH_IN_PRE_LB);
+  port, addr_family, S_SWITCH_IN_PRE_LB,
+  meter_groups);
 
 free(ip_address);
 
@@ -4907,7 +4914,8 @@ build_lrouter_groups(struct hmap *ports, struct ovs_list 
*lr_list)
 static void
 build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
 struct hmap *port_groups, struct hmap *lflows,
-struct hmap *mcgroups, struct hmap *igmp_groups)
+struct hmap *mcgroups, struct hmap *igmp_groups,
+struct shash *meter_groups)
 {
 /* This flow table structure is documented in ovn-northd(8), so please
  * update ovn-northd.8.xml if you change anything. */
@@ -4924,7 +4932,7 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,
 }
 
 build_pre_acls(od, lflows);
-build_pre_lb(od, lflows);
+build_pre_lb(od, lflows, meter_groups);
 build_pre_stateful(od, lflows);
 build_acls(od, lflows, port_groups);
 build_qos(od, lflows);
@@ -8278,12 +8286,13 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
 static void
 build_lflows(struct northd_context *ctx, struct hmap *datapaths,
  struct hmap *ports, struct hmap *port_groups,
- struct hmap *mcgroups, struct hmap *igmp_groups)
+ struct hmap *mcgroups, struct 

[ovs-dev] [PATCH v5 ovn 2/3] add meter support to trigger_event action

[Lorenzo Bianconi]

Introduce meter support to trigger_event action in order to not
overload pinctrl thread under heavy load

Signed-off-by: Lorenzo Bianconi 
---
 include/ovn/actions.h |  1 +
 lib/actions.c | 43 +--
 ovn-sb.xml|  5 -
 tests/ovn.at  |  4 
 4 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 0ca06537c..145f27f25 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -327,6 +327,7 @@ struct ovnact_controller_event {
 int event_type;   /* controller event type */
 struct ovnact_gen_option *options;
 size_t n_options;
+char *meter;
 };
 
 /* OVNACT_BIND_VPORT. */
diff --git a/lib/actions.c b/lib/actions.c
index 08c589ab3..6a5907e1b 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -1273,6 +1273,9 @@ format_TRIGGER_EVENT(const struct ovnact_controller_event 
*event,
 {
 ds_put_format(s, "trigger_event(event = \"%s\"",
   event_to_string(event->event_type));
+if (event->meter) {
+ds_put_format(s, ", meter = \"%s\"", event->meter);
+}
 for (const struct ovnact_gen_option *o = event->options;
  o < >options[event->n_options]; o++) {
 ds_put_cstr(s, ", ");
@@ -1420,10 +1423,21 @@ encode_TRIGGER_EVENT(const struct 
ovnact_controller_event *event,
  const struct ovnact_encode_params *ep OVS_UNUSED,
  struct ofpbuf *ofpacts)
 {
+uint32_t meter_id = NX_CTLR_NO_METER;
 size_t oc_offset;
 
+if (event->meter) {
+meter_id = ovn_extend_table_assign_id(ep->meter_table, event->meter,
+  ep->lflow_uuid);
+if (meter_id == EXT_TABLE_ID_INVALID) {
+VLOG_WARN("Unable to assign id for trigger meter: %s",
+  event->meter);
+return;
+}
+}
+
 oc_offset = encode_start_controller_op(ACTION_OPCODE_EVENT, false,
-   NX_CTLR_NO_METER, ofpacts);
+   meter_id, ofpacts);
 ovs_be32 ofs = htonl(event->event_type);
 ofpbuf_put(ofpacts, , sizeof ofs);
 
@@ -1738,11 +1752,27 @@ parse_trigger_event(struct action_context *ctx,
  sizeof *event->options);
 }
 
-struct ovnact_gen_option *o = >options[event->n_options++];
-memset(o, 0, sizeof *o);
-parse_gen_opt(ctx, o,
-  >pp->controller_event_opts->event_opts[event_type],
-  event_to_string(event_type));
+if (lexer_match_id(ctx->lexer, "meter")) {
+if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) {
+return;
+}
+/* If multiple meters are given, use the most recent. */
+if (ctx->lexer->token.type == LEX_T_STRING &&
+strlen(ctx->lexer->token.s)) {
+free(event->meter);
+event->meter = xstrdup(ctx->lexer->token.s);
+} else if (ctx->lexer->token.type != LEX_T_STRING) {
+lexer_syntax_error(ctx->lexer, "expecting string");
+return;
+}
+lexer_get(ctx->lexer);
+} else {
+struct ovnact_gen_option *o = >options[event->n_options++];
+memset(o, 0, sizeof *o);
+parse_gen_opt(ctx, o,
+>pp->controller_event_opts->event_opts[event_type],
+event_to_string(event_type));
+}
 if (ctx->lexer->error) {
 return;
 }
@@ -1763,6 +1793,7 @@ static void
 ovnact_controller_event_free(struct ovnact_controller_event *event)
 {
 free_gen_options(event->options, event->n_options);
+free(event->meter);
 }
 
 static void
diff --git a/ovn-sb.xml b/ovn-sb.xml
index 02691bbd3..477e7bc7a 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -1996,7 +1996,9 @@ tcp.flags = RST;
   
 This action is used to allow ovs-vswitchd to report CMS related
 events writing them in  table.
-Supported event:
+It is possible to associate a meter to a each event in order to
+not overload pinctrl thread under heavy load; each meter is
+identified though a defined naming convention. Supported events:
   
 
   
@@ -2007,6 +2009,7 @@ tcp.flags = RST;
 no configured backend destinations. For this event, the event
 info includes the load balancer VIP, the load balancer UUID,
 and the transport protocol.
+Associated meter: event-elb
   
 
   
diff --git a/tests/ovn.at b/tests/ovn.at
index 0f0775922..5352a750a 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1353,6 +1353,10 @@ tcp_reset { };
 trigger_event(event = "empty_lb_backends", vip = "10.0.0.1:80", protocol = 
"tcp", load_balancer = 

[ovs-dev] [PATCH v5 ovn 1/3] northd: introduce build_empty_lb_event_flow routine

[Lorenzo Bianconi]

Signed-off-by: Lorenzo Bianconi 
---
 northd/ovn-northd.c | 63 ++---
 1 file changed, 37 insertions(+), 26 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 9a282..679d36d88 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -4014,6 +4014,41 @@ ls_has_dns_records(const struct nbrec_logical_switch 
*nbs)
 return false;
 }
 
+static void
+build_empty_lb_event_flow(struct ovn_datapath *od, struct hmap *lflows,
+  struct smap_node *node, char *ip_address,
+  struct nbrec_load_balancer *lb, uint16_t port,
+  int addr_family, int pl)
+{
+if (!controller_event_en || node->value[0]) {
+return;
+}
+
+struct ds match = DS_EMPTY_INITIALIZER;
+char *action;
+
+if (addr_family == AF_INET) {
+ds_put_format(, "ip4.dst == %s && %s",
+  ip_address, lb->protocol);
+} else {
+ds_put_format(, "ip6.dst == %s && %s",
+  ip_address, lb->protocol);
+}
+if (port) {
+ds_put_format(, " && %s.dst == %u", lb->protocol,
+  port);
+}
+action = xasprintf("trigger_event(event = \"%s\", "
+   "vip = \"%s\", protocol = \"%s\", "
+   "load_balancer = \"" UUID_FMT "\");",
+   event_to_string(OVN_EVENT_EMPTY_LB_BACKENDS),
+   node->key, lb->protocol,
+   UUID_ARGS(>header_.uuid));
+ovn_lflow_add(lflows, od, pl, 130, ds_cstr(), action);
+ds_destroy();
+free(action);
+}
+
 static void
 build_pre_lb(struct ovn_datapath *od, struct hmap *lflows)
 {
@@ -4051,32 +4086,8 @@ build_pre_lb(struct ovn_datapath *od, struct hmap 
*lflows)
 sset_add(_ips, ip_address);
 }
 
-if (controller_event_en && !node->value[0]) {
-struct ds match = DS_EMPTY_INITIALIZER;
-char *action;
-
-if (addr_family == AF_INET) {
-ds_put_format(, "ip4.dst == %s && %s",
-  ip_address, lb->protocol);
-} else {
-ds_put_format(, "ip6.dst == %s && %s",
-  ip_address, lb->protocol);
-}
-if (port) {
-ds_put_format(, " && %s.dst == %u", lb->protocol,
-  port);
-}
-action = xasprintf("trigger_event(event = \"%s\", "
-   "vip = \"%s\", protocol = \"%s\", "
-   "load_balancer = \"" UUID_FMT "\");",
-   event_to_string(OVN_EVENT_EMPTY_LB_BACKENDS),
-   node->key, lb->protocol,
-   UUID_ARGS(>header_.uuid));
-ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 120,
-  ds_cstr(), action);
-ds_destroy();
-free(action);
-}
+build_empty_lb_event_flow(od, lflows, node, ip_address, lb,
+  port, addr_family, S_SWITCH_IN_PRE_LB);
 
 free(ip_address);
 
-- 
2.21.0

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

[ovs-dev] [PATCH v5 ovn 0/3] Add rate limiting support for controller_events

[Lorenzo Bianconi]

Introduce the capability to associate a meter to each controller event
type in order to not overload the pinctrl thread under heavy load.
For each event type we need to define a naming convention for the related
meter:
- empty_lb_backends: event-elb

Changes since v4:
- rebase ontop of current ovn master branch
- fold patch 1/3: northd: introduce build_empty_lb_event_flow routine
Changes since v3:
- rebase on top of 'add empty_lb controller_event for logical router pipeline'
  https://patchwork.ozlabs.org/cover/1153918/
- update documentation
- added a unit test for the meter option in the "action parsing"
Changes since v2:
- fold patch 'Repair memory leak for OVN controller events'
- fix memory leak of meter string
Changes since v1:
- fix subject

Lorenzo Bianconi (3):
  northd: introduce build_empty_lb_event_flow routine
  add meter support to trigger_event action
  northd: add rate limiting support for SB controller events

 include/ovn/actions.h   |   1 +
 lib/actions.c   |  43 ++---
 northd/ovn-northd.8.xml |   5 ++
 northd/ovn-northd.c | 102 +++-
 ovn-nb.xml  |   8 
 ovn-sb.xml  |   5 +-
 tests/ovn.at|   5 ++
 7 files changed, 130 insertions(+), 39 deletions(-)

-- 
2.21.0

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

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

[Eelco Chaudron]

On 20 Aug 2019, at 11:57, Anju Thomas wrote:


Thanks for the comments Eelco. I will address them in the next patch.

On a similar note, are these the final set of comments for this patch 
?


Did you get any update, and are you planning on sending out an update?

Thanks,

Eelco


Regards
Anju

-Original Message-
From: Eelco Chaudron 
Sent: Monday, August 12, 2019 5:48 PM
To: Anju Thomas 
Cc: d...@openvswitch.org; Keshav Gupta 
Subject: Re: [ovs-dev] [PATCH v12] Improved Packet Drop Statistics in 
OVS


Hi Anju,

See comments inline…

//Eelco


On 25 Jul 2019, at 14:16, Anju Thomas wrote:


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

Furthermore, there are numerous other reasons for which packets can 
be

dropped by OVS slow path that are not related to the OpenFlow
pipeline.
The generated datapath flow entries include a drop action to avoid
further expensive upcalls to the slow path, but subsequent packets
dropped by the datapath are not accounted anywhere.

Finally, the datapath itself drops packets in certain error
situations.
Also, these drops are today not accounted for.This makes it difficult
for OVS users to monitor packet drop in an OVS instance and to alert 
a

management system in case of a unexpected increase of such drops.
Also OVS trouble-shooters face difficulties in analysing packet 
drops.


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

1. Identify and account all the silent packet drop scenarios

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

v11->v12 Addresses comments from Ben Pfaff

Co-authored-by: Rohith Basavaraja 
Co-authored-by: Keshav Gupta 
Signed-off-by: Anju Thomas 
Signed-off-by: Rohith Basavaraja 
Signed-off-by: Keshav Gupta 
---
 datapath/linux/compat/include/linux/openvswitch.h |   1 +
 lib/dpif-netdev.c |  46 -
 lib/dpif.c|   7 +
 lib/dpif.h|   4 +
 lib/odp-execute.c |  79 
 lib/odp-util.c|   9 +
 ofproto/ofproto-dpif-ipfix.c  |   1 +
 ofproto/ofproto-dpif-sflow.c  |   1 +
 ofproto/ofproto-dpif-xlate.c  |  40 -
 ofproto/ofproto-dpif-xlate.h  |   3 +
 ofproto/ofproto-dpif.c|   8 +
 ofproto/ofproto-dpif.h|   8 +-
 tests/automake.mk |   3 +-
 tests/dpif-netdev.at  |   8 +
 tests/drop-stats.at   | 210
++
 tests/ofproto-dpif.at |   2 +-
 tests/tunnel-push-pop.at  |  29 ++-
 tests/tunnel.at   |  16 +-
 18 files changed, 464 insertions(+), 11 deletions(-)  create mode
100644 tests/drop-stats.at

diff --git a/datapath/linux/compat/include/linux/openvswitch.h
b/datapath/linux/compat/include/linux/openvswitch.h
index 65a003a..415c053 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -989,6 +989,7 @@ enum ovs_action_attr {  #ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH,   /* struct ovs_action_push_tnl*/
OVS_ACTION_ATTR_TUNNEL_POP,/* u32 port number. */
+   OVS_ACTION_ATTR_DROP,
 #endif
__OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
   * from userspace. */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index
6b99a3c..3878b8d 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -101,6 +101,17 @@ enum { MAX_METERS = 65536 };/* Maximum 
number

of meters. */
 enum { MAX_BANDS = 8 }; /* Maximum number of bands / meter.
*/
 enum { N_METER_LOCKS = 64 };/* Maximum number of meters. */

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

@@ -5762,7 +5773,7 @@ dp_netdev_run_meter(struct dp_netdev 

Re: [ovs-dev] [PATCH] show "rx_missed_errors" counter in interface statisics

[0-day Robot]

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

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


checkpatch:
ERROR: Author txfh2007  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Liu Chang 
Lines checked: 31, Warnings: 1, Errors: 1


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

[ovs-dev] [PATCH] show "rx_missed_errors" counter in interface statisics

[txfh2007 via dev]

Hi all:

Currently OVS maintains several Statistics counters per interface. 
"rx_missed_errors" counter is amount them and collects pkts not received due to 
local resource constaints. Many ovs netdevs support collecting this counter, 
such as netdev-linux, netdev-dpdk, netdev-bsd and so on. But as far as I know, 
this counter can't be read by command "ovs-vsctl list interface |grep 
statistics". I have found the root cause(may be I was wrong) is in task 
"iface_refresh_stats", the "rx_missed_errors" is not in the macro IFACE_STATS. 
So even if this counter is updated by netdev, it woundn't be read by users.

This simple patch tries to solve this problem, many thanks for your kindly 
reminder. 
   
Signed-off-by: Liu Chang 

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index d921c4e..55d30bf 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -2393,6 +2393,7 @@ iface_refresh_stats(struct iface *iface)
 IFACE_STAT(rx_frame_errors, "rx_frame_err") \
 IFACE_STAT(rx_over_errors,  "rx_over_err")  \
 IFACE_STAT(rx_crc_errors,   "rx_crc_err")   \
+IFACE_STAT(rx_missed_errors,"rx_missed_errors") \
 IFACE_STAT(collisions,  "collisions")   \
 IFACE_STAT(rx_1_to_64_packets,  "rx_1_to_64_packets")   \
 IFACE_STAT(rx_65_to_127_packets,"rx_65_to_127_packets") \





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

Re: [ovs-dev] [PATCH v1 ovn] OVN: Send RARP for vif ports for which OVN does not know the IP.

[Numan Siddique]

On Fri, Aug 2, 2019 at 3:12 AM Ankur Sharma 
wrote:

> ISSUE:
> For a VIF port (on a bridged logical switch), OVN sends out
> GARPs, advertising port's mac and IP.
>
> However, if a VIF port (on a bridged logical switch) has not
> been assigned an IP, then OVN does not advertise anything.
> As a result, for such VIF ports basic operations like VM
> migration will not work, since TOR will direct the packets
> destined to this VIF to old chassis.
>
> This patch addresses the same by advertising reverse ARP (RARP)
> for non ip assigned bridged logical switch connected VIF ports.
>
> Signed-off-by: Ankur Sharma 
>

Hi Ankur,

Can you please rebase this patch too.

Looking the code at a very high level, it seems it has some duplicate code
with
garp. Can you please check if it is possible to have common code between
garp and rarp sending.

Thanks
Numan


> ---
>  controller/pinctrl.c | 238
> +++
>  tests/ovn.at |  68 +++
>  2 files changed, 306 insertions(+)
>
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index ebbb52a..191556f 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -209,6 +209,20 @@ static void send_garp_prepare(
>  OVS_REQUIRES(pinctrl_mutex);
>  static void send_garp_run(struct rconn *swconn, long long int
> *send_garp_time)
>  OVS_REQUIRES(pinctrl_mutex);
> +
> +static void init_send_rarps(void);
> +static void destroy_send_rarps(void);
> +static void send_rarp_wait(long long int send_rarp_time);
> +static void send_rarp_prepare(
> +struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
> +struct ovsdb_idl_index *sbrec_port_binding_by_name,
> +const struct ovsrec_bridge *,
> +const struct sbrec_chassis *,
> +const struct hmap *local_datapaths)
> +OVS_REQUIRES(pinctrl_mutex);
> +static void send_rarp_run(struct rconn *swconn, long long int
> *send_rarp_time)
> +OVS_REQUIRES(pinctrl_mutex);
> +
>  static void pinctrl_handle_nd_na(struct rconn *swconn,
>   const struct flow *ip_flow,
>   const struct match *md,
> @@ -428,6 +442,7 @@ pinctrl_init(void)
>  {
>  init_put_mac_bindings();
>  init_send_garps();
> +init_send_rarps();
>  init_ipv6_ras();
>  init_buffered_packets_map();
>  init_event_table();
> @@ -2024,6 +2039,8 @@ pinctrl_handler(void *arg_)
>  static long long int send_ipv6_ra_time = LLONG_MAX;
>  /* Next GARP announcement in ms. */
>  static long long int send_garp_time = LLONG_MAX;
> +/* Next RARP announcement in ms. */
> +static long long int send_rarp_time = LLONG_MAX;
>  /* Next multicast query (IGMP) in ms. */
>  static long long int send_mcast_query_time = LLONG_MAX;
>
> @@ -2078,6 +2095,7 @@ pinctrl_handler(void *arg_)
>  if (may_inject_pkts()) {
>  ovs_mutex_lock(_mutex);
>  send_garp_run(swconn, _garp_time);
> +send_rarp_run(swconn, _rarp_time);
>  send_ipv6_ras(swconn, _ipv6_ra_time);
>  send_mac_binding_buffered_pkts(swconn);
>  ovs_mutex_unlock(_mutex);
> @@ -2089,6 +2107,7 @@ pinctrl_handler(void *arg_)
>  rconn_run_wait(swconn);
>  rconn_recv_wait(swconn);
>  send_garp_wait(send_garp_time);
> +send_rarp_wait(send_rarp_time);
>  ipv6_ra_wait(send_ipv6_ra_time);
>  ip_mcast_querier_wait(send_mcast_query_time);
>
> @@ -2138,6 +2157,9 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
>  send_garp_prepare(sbrec_port_binding_by_datapath,
>sbrec_port_binding_by_name, br_int, chassis,
>local_datapaths, active_tunnels);
> +send_rarp_prepare(sbrec_port_binding_by_datapath,
> +  sbrec_port_binding_by_name, br_int, chassis,
> +  local_datapaths);
>  prepare_ipv6_ras(sbrec_port_binding_by_datapath,
>   sbrec_port_binding_by_name, local_datapaths);
>  sync_dns_cache(dns_table);
> @@ -2494,6 +2516,7 @@ pinctrl_destroy(void)
>  latch_destroy(_thread_exit);
>  free(pinctrl.br_int_name);
>  destroy_send_garps();
> +destroy_send_rarps();
>  destroy_ipv6_ras();
>  destroy_buffered_packets_map();
>  event_table_destroy();
> @@ -2785,6 +2808,9 @@ struct garp_data {
>  /* Contains GARPs to be sent. Protected by pinctrl_mutex*/
>  static struct shash send_garp_data;
>
> +/* Contains RARPs to be sent. Protected by pinctrl_mutex*/
> +static struct shash send_rarp_data;
> +
>  static void
>  init_send_garps(void)
>  {
> @@ -2797,6 +2823,34 @@ destroy_send_garps(void)
>  shash_destroy_free_data(_garp_data);
>  }
>
> +/*
> + * Send reverse ARP (RARP) for vif on localnet.
> + *
> + * When a new vif on localnet is added and IP is unknown,
> + * RARPs are sent announcing the port's mac.
> + * On localnet, such announcements are needed for
> + * 

Re: [ovs-dev] [PATCH v2 ovn] Replace chassis mac with router port mac on destination chassis

[Numan Siddique]

On Tue, Aug 27, 2019 at 7:31 AM Ankur Sharma 
wrote:

> During E-W routing for vlan backed networks, we replace router port
> mac with chassis mac, when packet leaves the source hypervisor.
>
> As a result, the destination VM (on remote hypervisor) will see
> chassis mac as source mac in received packet.
>
> Although, functionality wise this does not cause any issue,
> however chassis mac being see as source on VM, will
> lead to following:
> a. INCONSISTENT SOURCE MAC:
>If the destination VM moves to same hypervisor as sender,
>then it will see router port mac as source mac. Whereas, on
>a remote hypervisor, source mac will be the sender chassis mac.
>
>This will cause inconsistency in packet headers for the same
>flow and could be confusing for someone looking at packet
>captures inside the vm.
>
> b. SYSTEM MAC BEING EXPOSED TO VM:
>Chassis mac is a CMS provided mac, i.e it is an infrastructure
>mac. It is not a good practice to expose such values to VM,
>which should not be seeing them in first place.
>
> In order to replace chassis mac with router port mac, we will
> do following.
>
> a. Create conjunction for each chassis mac and router port vlan
>id combination. For example, for a 2 node chassis setup, where
>we have a logical router, connected to 4 logical switches with
>vlan ids: 2000, 1000, 0 and 24, the conjunction flow will look
>like following:
>
>cookie=0x0, duration=9094.608s, table=0, n_packets=0, n_bytes=0,
> idle_age=9094, priority=180,dl_src=aa:bb:cc:dd:ee:22
> actions=conjunction(100,1/2)
>cookie=0x0, duration=9094.608s, table=0, n_packets=0, n_bytes=0,
> idle_age=9094, priority=180,dl_src=aa:bb:cc:dd:ff:ee
> actions=conjunction(100,1/2)
>
>cookie=0x0, duration=9094.552s, table=0, n_packets=0, n_bytes=0,
> idle_age=9094, priority=180,dl_vlan=2000 actions=conjunction(100,2/2)
>cookie=0x0, duration=9094.552s, table=0, n_packets=0, n_bytes=0,
> idle_age=9094, priority=180,dl_vlan=1000 actions=conjunction(100,2/2)
>cookie=0x0, duration=9094.552s, table=0, n_packets=0, n_bytes=0,
> idle_age=9094, priority=180,vlan_tci=0x/0x1fff
> actions=conjunction(100,2/2)
>cookie=0x0, duration=9094.552s, table=0, n_packets=0, n_bytes=0,
> idle_age=9094, priority=180,dl_vlan=24 actions=conjunction(100,2/2)
>
> b. Using this conjunction as match, we can identify if packet entering
> destination
>hypervisor was routed at the source or not. This will be done in
> table=0 (Physical to logical)
>at priority=180.
>For example:
>cookie=0x0, duration=9795.957s, table=0, n_packets=1391,
> n_bytes=141882, idle_age=8396,
> priority=180,conj_id=100,in_port=146,dl_vlan=1000
> actions=.,mod_dl_src:00:00:01:01:02:03,...
>
> c. We use conjunction, as it will ensure that we do not end up having lot
> of flows
>as we scale up. If we do not use conjunction, then we will have
>N (number of chassis macs) X M (number of router vlans) number of ovs
> flows.
>Conjunction converts it to N + M.
>Consider a setup, with 500 Chassis and 500 routed vlans.
>Without conjunction we will need 25000 (500 * 500) flows,
>whereas with conjunction that number comes down to 1000 (500 + 500).
>
> Signed-off-by: Ankur Sharma 
>

Hi Ankur,

Looks like you have to rebase this patch again.

Can you please resubmit again.

Thanks
Numan


> ---
>  controller/chassis.c|   2 +-
>  controller/chassis.h|   3 +
>  controller/ovn-controller.c |   5 +
>  controller/physical.c   | 222
> ++--
>  controller/physical.h   |   1 +
>  ovn-architecture.7.xml  |  10 +-
>  tests/ovn.at|  12 ++-
>  7 files changed, 242 insertions(+), 13 deletions(-)
>
> diff --git a/controller/chassis.c b/controller/chassis.c
> index 937c557..699b662 100644
> --- a/controller/chassis.c
> +++ b/controller/chassis.c
> @@ -144,7 +144,7 @@ get_bridge_mappings(const struct smap *ext_ids)
>  return smap_get_def(ext_ids, "ovn-bridge-mappings", "");
>  }
>
> -static const char *
> +const char *
>  get_chassis_mac_mappings(const struct smap *ext_ids)
>  {
>  return smap_get_def(ext_ids, "ovn-chassis-mac-mappings", "");
> diff --git a/controller/chassis.h b/controller/chassis.h
> index eb46ca3..178d295 100644
> --- a/controller/chassis.h
> +++ b/controller/chassis.h
> @@ -27,6 +27,7 @@ struct sbrec_chassis;
>  struct sbrec_chassis_table;
>  struct sset;
>  struct eth_addr;
> +struct smap;
>
>  void chassis_register_ovs_idl(struct ovsdb_idl *);
>  const struct sbrec_chassis *chassis_run(
> @@ -42,5 +43,7 @@ bool chassis_get_mac(const struct sbrec_chassis *chassis,
>   const char *bridge_mapping,
>   struct eth_addr *chassis_mac);
>  const char *chassis_get_id(void);
> +const char * get_chassis_mac_mappings(const struct smap *ext_ids);
> +
>
>  #endif /* controller/chassis.h */
> diff --git a/controller/ovn-controller.c 

[ovs-dev] SIGILL ovs branch-2.12/arm64/DPDK

[James Page]

Hi

I've been testing non-x86 architecture builds for the upcoming 2.12 release
of OVS and I'm hitting an issue with DPDK enabled builds on the arm64
architecture.

branch-2.12 includes improvements for native hashing under arm64; these
appear to work fine when DPDK is not in the mix, but with DPDK enabled, I
get a SIGILL in lib/hash.c:

Program received signal SIGILL, Illegal instruction.
hash_bytes (p_=p_@entry=0xaac36e50, n=7, basis=basis@entry=0) at
../lib/hash.c:38
38 ../lib/hash.c: No such file or directory.

(gdb) backtrace full
#0  hash_bytes (p_=p_@entry=0xaac36e50, n=7, basis=basis@entry=0) at
../lib/hash.c:38
p = 0xaac36e50
orig_n = 7
hash = 0
#1  0xaab4238c in hash_string (basis=0, s=0xaac36e50 "comment")
at ../lib/hash.h:342
No locals.
#2  hash_name (name=name@entry=0xaac36e50 "comment") at
../lib/shash.c:28
No locals.
#3  0xaab42a1c in shash_find (sh=sh@entry=0xaaceb6f8
, name=name@entry=0xaac36e50 "comment")
at ../lib/shash.c:231
No locals.
#4  0xaab42a60 in shash_add_once (sh=0xaaceb6f8 ,
name=0xaac36e50 "comment",
data=0xaacdb0d8 ) at ../lib/shash.c:135
No locals.
#5  0xaab42ab4 in shash_add_assert (sh=sh@entry=0xaaceb6f8
, name=,
data=data@entry=0xaacdb0d8 ) at ../lib/shash.c:146
__func__ = "shash_add_assert"
#6  0xaab05900 in ctl_register_command (command=0xaacdb0d8
) at ../lib/db-ctl-base.c:2470
No locals.
#7  ctl_register_commands (commands=0xaacdb0d8 ) at
../lib/db-ctl-base.c:2470
p = 0xaacdb0d8 
p = 
#8  ctl_init__ (idl_class_=, ctl_classes_=,
cmd_show_tables_=0x0, ctl_exit_func_=)
at ../lib/db-ctl-base.c:2486
No locals.
#9  0xaaad5768 in nbctl_cmd_init () at
../ovn/utilities/ovn-nbctl.c:5695
No locals.
#10 main (argc=1, argv=0xf4d8) at ../ovn/utilities/ovn-nbctl.c:137
idl = 
local_options = {map = {buckets = 0x0, one = 0xf390, mask =
281473881441200, n = 187649986051872}}
args = 
parsed_options = 0xf350
n_parsed_options = 187649986051968
error_s = 
socket_name = 
daemon_mode = 

This effects pretty much all of the binaries built with DPDK enabled.

gcc version is 9.2.1 on Ubuntu eoan; I've not tested on earlier gcc
versions (yet).

Any help/pointers much appreciated.

Cheers

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

[ovs-dev] Ваш заказ сформирован

[Зайнаб via dev]

Добрый день!

Мы обеспечим ваш офис всем необходимым!   

Компания «Море рюкзаков» занимается оптовыми продажами канцелярских товаров, 
нам уже более 15 лет. Мы работаем только с самыми надежными поставщиками как из 
Российской Федерации так и иностранными партнерами.
   Мы обеспечиваем ваш офис всем необходимым, начиная от канцелярских 
принадлежностей, заканчивая одноразовой посудой и офисной кухней.

C нашим ассортиментом вы можете ознакомиться на нашем сайте:
https://www.morerukzakov.ru/

   Для нашего клиента и его комфорта, мы предоставляем следующий спектр услуг:
- прием заказов любым способом, как вам удобно
- полное ведение заказа менеджером
- резервирование товара сроком до недели
- гибкая система скидок
- консультирование клиента об особенностях того или иного товара
- возможность отсрочки платежа
- доставка до офиса
И самое главное широкий ассортимент качественных товаров.

С Уважением, «Море рюкзаков»
https://www.morerukzakov.ru/
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn] Fix configure help string for ovs source/build path

[Numan Siddique]

On Fri, Aug 30, 2019 at 6:53 PM Lorenzo Bianconi <
lorenzo.bianc...@redhat.com> wrote:

> Signed-off-by: Lorenzo Bianconi 
>

Thanks. I pushed this to master.

Numan


> ---
>  acinclude.m4 | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/acinclude.m4 b/acinclude.m4
> index d5552d708..31e4f0b54 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -1212,10 +1212,10 @@ dnl
>  dnl Check for OVS sources
>  AC_DEFUN([OVN_CHECK_OVS], [
>AC_ARG_WITH([ovs-source],
> -  [AC_HELP_STRING([--ovs-source=/path/to/ovs/src/dir],
> +  [AC_HELP_STRING([--with-ovs-source=/path/to/ovs/src/dir],
>[Specify the OVS src directory])])
>AC_ARG_WITH([ovs-build],
> -  [AC_HELP_STRING([--ovs-build=/path/to/ovs/build/dir],
> +  [AC_HELP_STRING([--with-ovs-build=/path/to/ovs/build/dir],
>[Specify the OVS build directory])])
>
>AC_MSG_CHECKING([for OVS source directory])
> --
> 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

Re: [ovs-dev] [PATCH v1] ofproto: Fix OVS crash when packets hit Openflow rules with certain combinations of nested actions

[Anil Kumar Koli via dev]

Hi Ilya,

Sorry for late reply and thanks for the review comments.
Yes, the issue happens only in the userspace datapath and not in kernel 
datapath. I have tested it on kernel datapath and could not see this issue for 
the same build.

Regarding the suggested solution, I still feel like going with recursive mutex 
approach is more cleaner then releasing the lock for 
ofproto_packet_out_finish().  We have the ofproto_dpif_xcache_execute() which 
may lead to race when a flow mod learn is called from another thread. May be 
it could also be possible that in certain combination we may result the crash 
in ofproto_packet_out_start().  May be Ben can share his inputs as well.

Best Regards,Anil 
Kumar

-Original Message-
From: Ilya Maximets 
Sent: Thursday, 29 August, 2019 05:10 PM
To: Anil Kumar Koli ; 
ovs-dev@openvswitch.org
Cc: 'Ben Pfaff' 
Subject: Re: [ovs-dev] [PATCH v1] ofproto: Fix OVS crash when packets hit 
Openflow rules with certain combinations of nested actions

On 29.08.2019 12:52, Anil Kumar Koli wrote:
> Hi Ilya,
>
> Thanks for the review.
> Please find below the stack trace of the crash
>
>  (gdb) bt
> #0  0x7f0a3da46c37 in __GI_raise (sig=sig@entry=6) at
> ../nptl/sysdeps/unix/sysv/linux/raise.c:56
> #1  0x7f0a3da4a028 in __GI_abort () at abort.c:89
> #2  0x0094262e in ovs_abort_valist (err_no=,
> format=, args=args@entry=0x7fffaf59e308) at
> lib/util.c:419
> #3  0x009426b7 in ovs_abort (err_no=,
> format=format@entry=0xb0289f "%s: pthread_%s_%s failed") at
> lib/util.c:411
> #4  0x009104f9 in ovs_mutex_lock_at (l_=l_@entry=0xe47cc0
> , where=where@entry=0xad4199 "ofproto/ofproto.c:5391")
> at lib/ovs-thread.c:75
> #5  0x0083fb1f in ofproto_flow_mod_learn
> (ofm=ofm@entry=0x7fffaf59e620, keep_ref=,
> limit=, below_limitp=below_limitp@entry=0x7fffaf59e540)
> at ofproto/ofproto.c:5391
> #6  0x0085e5d0 in xlate_learn_action
> (ctx=ctx@entry=0x7fffaf5a02e0, learn=learn@entry=0x2b18308) at
> ofproto/ofproto-dpif-xlate.c:5378
> #7  0x008615c3 in do_xlate_actions (ofpacts=,
> ofpacts_len=, ctx=,
> is_last_action=, group_bucket_action=)
> at ofproto/ofproto-dpif-xlate.c:6893
> #8  0x0085edba in xlate_recursively (actions_xlator=0x860730
> , is_last_action=false, deepens=,
> rule=0x2b8e440, ctx=0x7fffaf5a02e0) at
> ofproto/ofproto-dpif-xlate.c:4233
> #9  xlate_table_action (ctx=0x7fffaf5a02e0, in_port=, 
> table_id=, may_packet_in=, 
> honor_table_miss=, with_ct_orig=, 
> is_last_action=false,
> xlator=0x860730 ) at
> ofproto/ofproto-dpif-xlate.c:4361
> #10 0x00861aaa in xlate_ofpact_resubmit (resubmit= out>, resubmit=, resubmit=,
> is_last_action=, ctx=0x7fffaf5a02e0) at
> ofproto/ofproto-dpif-xlate.c:4672
> #11 do_xlate_actions (ofpacts=ofpacts@entry=0x2b654e8, 
> ofpacts_len=ofpacts_len@entry=48, ctx=ctx@entry=0x7fffaf5a02e0, 
> is_last_action=is_last_action@entry=true, 
> group_bucket_action=group_bucket_action@entry=false)
> at ofproto/ofproto-dpif-xlate.c:6773
> #12 0x008696d6 in xlate_actions (xin=xin@entry=0x7fffaf5a0da0,
> xout=xout@entry=0x7fffaf5a11b0) at ofproto/ofproto-dpif-xlate.c:7570
> #13 0x00859b56 in upcall_xlate (wc=0x7fffaf5a23f0,
> odp_actions=0x7fffaf5a1bc0, upcall=0x7fffaf5a1150, udpif=0x2b10670) at
> ofproto/ofproto-dpif-upcall.c:1197
> #14 process_upcall (udpif=udpif@entry=0x2b10670,
> upcall=upcall@entry=0x7fffaf5a1150,
> odp_actions=odp_actions@entry=0x7fffaf5a1bc0,
> wc=wc@entry=0x7fffaf5a23f0) at ofproto/ofproto-dpif-upcall.c:1413
> #15 0x0085a9eb in upcall_cb (packet=, 
> flow=0x7fffaf5a2150, ufid=, pmd_id=, 
> type=, userdata=, actions=0x7fffaf5a1bc0, 
> wc=0x7fffaf5a23f0,
> put_actions=0x7fffaf5a1c00, aux=0x2b10670) at
> ofproto/ofproto-dpif-upcall.c:1315
> #16 0x0088419c in dp_netdev_upcall (pmd=pmd@entry=0x7f0a35f34010, 
> packet_=packet_@entry=0x2b68930, flow=flow@entry=0x7fffaf5a2150, 
> wc=wc@entry=0x7fffaf5a23f0, ufid=ufid@entry=0x7fffaf5a1ba0, 
> type=type@entry=DPIF_UC_MISS,
> userdata=userdata@entry=0x0, actions=actions@entry=0x7fffaf5a1bc0,
> put_actions=put_actions@entry=0x7fffaf5a1c00) at
> lib/dpif-netdev.c:6236
> #17 0x0088d053 in handle_packet_upcall
> (put_actions=0x7fffaf5a1c00, actions=0x7fffaf5a1bc0,
> key=0x7fffaf5a3080, packet=0x2b68930, pmd=0x7f0a35f34010) at
> lib/dpif-netdev.c:6591
> #18 fast_path_processing (pmd=pmd@entry=0x7f0a35f34010, 
> packets_=packets_@entry=0x7fffaf5a35f0, keys=keys@entry=0x7fffaf5a3040, 
> flow_map=flow_map@entry=0x7fffaf5a2ef0, 
> index_map=index_map@entry=0x7fffaf5a2ee0 "",
> in_port=) at lib/dpif-netdev.c:6709
> #19 0x0088e9ce in dp_netdev_input__
> (pmd=pmd@entry=0x7f0a35f34010, packets=packets@entry=0x7fffaf5a35f0,
> md_is_valid=md_is_valid@entry=true, port_no=port_no@entry=0) at
> lib/dpif-netdev.c:6797
> #20 0x00890710 in dp_netdev_recirculate
> (packets=0x7fffaf5a35f0, pmd=0x7f0a35f34010) at lib/dpif-netdev.c:6842
> #21 dp_execute_cb