Re: [ovs-dev] NDP for ipv6 in OVS 2.10.0

2019-09-10 Thread kai xi fan 
You can use multicast forwarding. Because NDP are unicast and multicast
packets.

Thomas Goirand  于2019年9月10日周二 下午10:09写道：

> Hi Ben and others,
>
> I just want to know: does OVS 2.10 has support for IPv6 mac learning /
> NDP, so that we can avoid the ipv6 broadcast storm in our cloud?
>
> Cheers,
>
> Thomas Goirand (zigo)
>
> P.S: I'm not registered to this list, please CC me.
> ___
> 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] NDP for ipv6 in OVS 2.10.0

2019-09-10 Thread Ben Pfaff 
On Tue, Sep 10, 2019 at 04:02:36PM +0200, Thomas Goirand wrote:
> I just want to know: does OVS 2.10 has support for IPv6 mac learning /
> NDP, so that we can avoid the ipv6 broadcast storm in our cloud?

Hi Thomas.

MAC learning is an L2 feature that applies to IPv4 and IPv6 in the
same way.  All versions of OVS support MAC learning.

Neighbor discovery (NDP) is an IPv6 feature used to obtain the
Ethernet address associated with an IPv6 address.  In its default
configuration, run without a controller, OVS is an L2 switch, so it
does not get involved in NDP, it only passes the packets along.  When
it is configured with a controller, the controller can inspect the NDP
packets and suppress them in an intelligent way.  OVN, for example,
does this, and other controllers might do it too.

OVS does have the ability to do multicast snooping for IPv4 and IPv6.
Maybe that is really what you are asking about.  I don't understand
this feature set very well.  The author of the IPv6 multicast snooping
code, Thadeu, whom I have CC, may be able to help answer your
questions about that if you have any.

I hope this helps.

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

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

2019-09-10 Thread Ankur Sharma 
Hi Numan,

Thanks for calling out the failure.
Fixed the same in v5.

Did not realize that we have removed OVS tests from repo, hence based on test 
number, I ruled it out as an unrelated tests.

Regards,
Ankur

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



On Mon, Sep 9, 2019 at 1:58 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,

Thanks for v4.

The below test case  is failing

## --- ##
116: ovn -- 2 HVs, 2 lports/HV, localnet ports, DVR N-S Ping FAILED 
(ovn.at:15773 
[ovn.at])

## - ##
## Test results. ##
## - ##

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, 243 insertions(+), 12 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",

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

2019-09-10 Thread 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|  14 ++-
 7 files changed, 244 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] [RFC v2] Document process for compatibility between OVS and OVN.

2019-09-10 Thread Russell Bryant 
On Fri, Sep 6, 2019 at 5:08 PM Mark Michelson  wrote:
>
> This document serves to provide an explanation for how OVN will remain
> compatible with OVS. It provides instructions for OVN contributors for
> how to maintain compatibility even across older versions of OVS when
> possible.
>
> Note that the document currently makes reference to some non-existent
> items. For instance, it refers to an ovs-compat directory in the OVN
> project. Also, it refers to some macros for testing the OVS version.
> These will be added in a future commit if the process documented here is
> approved.
>
> I'm submitting this as an RFC, since I imagine there are some details I
> have not thought about, and there will also be some great suggestions
> from others on this matter.
>
> Signed-off-by: Mark Michelson 
> ---
>  Documentation/automake.mk  |   1 +
>  Documentation/internals/contributing/index.rst |   1 +
>  .../contributing/ovs-ovn-compatibility.rst | 129 
> +
>  3 files changed, 131 insertions(+)
>  create mode 100644 
> Documentation/internals/contributing/ovs-ovn-compatibility.rst
>
> diff --git a/Documentation/automake.mk b/Documentation/automake.mk
> index f7e1d2628..d9bd4be1f 100644
> --- a/Documentation/automake.mk
> +++ b/Documentation/automake.mk
> @@ -56,6 +56,7 @@ DOC_SOURCE = \
> Documentation/internals/contributing/documentation-style.rst \
> Documentation/internals/contributing/libopenvswitch-abi.rst \
> Documentation/internals/contributing/submitting-patches.rst \
> +   Documentation/internals/contributing/ovs-ovn-compatibility.rst \
> Documentation/requirements.txt \
> $(addprefix Documentation/ref/,$(RST_MANPAGES) $(RST_MANPAGES_NOINST))
>  FLAKE8_PYFILES += Documentation/conf.py
> diff --git a/Documentation/internals/contributing/index.rst 
> b/Documentation/internals/contributing/index.rst
> index a46cb046a..7ef57a1e2 100644
> --- a/Documentation/internals/contributing/index.rst
> +++ b/Documentation/internals/contributing/index.rst
> @@ -36,3 +36,4 @@ The below guides provide information on contributing to 
> Open vSwitch itself.
> coding-style-windows
> documentation-style
> libopenvswitch-abi
> +   ovs-ovn-compatibility
> diff --git a/Documentation/internals/contributing/ovs-ovn-compatibility.rst 
> b/Documentation/internals/contributing/ovs-ovn-compatibility.rst
> new file mode 100644
> index 0..d40b58c32
> --- /dev/null
> +++ b/Documentation/internals/contributing/ovs-ovn-compatibility.rst
> @@ -0,0 +1,129 @@
> +..
> +  Copyright (c) 2019 Nicira, Inc.

Copy/paste oversight?  I don't think a Copyright notice is necessary,
but if you leave it, s/Nicira/Red Hat/

> +
> +  Licensed under the Apache License, Version 2.0 (the "License"); you may
> +  not use this file except in compliance with the License. You may obtain
> +  a copy of the License at
> +
> +  http://www.apache.org/licenses/LICENSE-2.0
> +
> +  Unless required by applicable law or agreed to in writing, software
> +  distributed under the License is distributed on an "AS IS" BASIS, 
> WITHOUT
> +  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See 
> the
> +  License for the specific language governing permissions and limitations
> +  under the License.
> +
> +  Convention for heading levels in Open vSwitch documentation:
> +
> +  ===  Heading 0 (reserved for the title in a document)
> +  ---  Heading 1
> +  ~~~  Heading 2
> +  +++  Heading 3
> +  '''  Heading 4
> +
> +  Avoid deeper levels because they do not render well.
> +
> +===
> +Keeping OVN Compatible with OVS
> +===
> +
> +OVN has split from OVS. Prior to this split, there was no issue with
> +compatibility. All code changes happened at the same time and in the same 
> repo.

I think some compatibility issues were always there since it was still
possible to run separate versions of OVN vs OVS, or even separate
versions within the same deployment during upgrades.  I'd say it's now
important to be even more explicit about compatibility since OVN and
OVS are now released and versioned independently.

> +New releases contained the latest OVS and OVN changes. But now these 
> assumptions
> +are no longer valid, and so care must be taken to maintain compatibility.
> +
> +OVS and OVN versions
> +
> +
> +Prior to the split, the release schedule for OVN was bound to the release
> +schedule for OVS. OVS releases a new version approximately every 6 months. 
> OVN
> +has not yet determined a release schedule, but it is entirely possible that 
> it
> +will be different from OVS. Eventually, this will lead to a situation where 
> it
> +is very important that we publish which versions of OVN are compatible with
> +which versions of OVS. When incompatibilities are discovered, it is 
> important

Re: [ovs-dev] [PATCH ovn v2 1/2] Add ovn-appctl utility

2019-09-10 Thread Mark Michelson 

For the series,
Acked-by: Mark Michelson 

On 9/10/19 3:42 AM, nusid...@redhat.com wrote:

From: Numan Siddique 

Now that OVN has it's own rundir, "ovs-appctl -t ovn-controller/ovn-northd"
doesn't work. To fix this, ovn-appctl utility is added which
looks for the OVN pid/ctl files in the ovn rundir.

The code is taken from ovs-appctl.c and modified to use ovn_rundir()
instead of ovs_rundir().

Signed-off-by: Numan Siddique 
---
  rhel/ovn-fedora.spec.in|   2 +
  utilities/.gitignore   |   2 +
  utilities/automake.mk  |  13 +-
  utilities/ovn-appctl.8.xml | 352 +
  utilities/ovn-appctl.c | 239 +
  utilities/ovn-ctl  |  18 +-
  6 files changed, 615 insertions(+), 11 deletions(-)
  create mode 100644 utilities/ovn-appctl.8.xml
  create mode 100644 utilities/ovn-appctl.c

diff --git a/rhel/ovn-fedora.spec.in b/rhel/ovn-fedora.spec.in
index 14035de9a..9ee807fab 100644
--- a/rhel/ovn-fedora.spec.in
+++ b/rhel/ovn-fedora.spec.in
@@ -430,6 +430,7 @@ fi
  %{_bindir}/ovn-sbctl
  %{_bindir}/ovn-trace
  %{_bindir}/ovn-detrace
+%{_bindir}/ovn-appctl
  %{_datadir}/ovn/scripts/ovn-ctl
  %{_datadir}/ovn/scripts/ovn-lib
  %{_datadir}/ovn/scripts/ovndb-servers.ocf
@@ -440,6 +441,7 @@ fi
  %{_mandir}/man8/ovn-nbctl.8*
  %{_mandir}/man8/ovn-trace.8*
  %{_mandir}/man1/ovn-detrace.1*
+%{_mandir}/man8/ovn-appctl.8*
  #%{_mandir}/man7/ovn-architecture.7* - Uncomment this once the manpage is 
fixed
  %{_mandir}/man8/ovn-sbctl.8*
  #%{_mandir}/man5/ovn-nb.5* - Uncomment this once the manpage is fixed
diff --git a/utilities/.gitignore b/utilities/.gitignore
index 1d01e0b28..b319e8366 100644
--- a/utilities/.gitignore
+++ b/utilities/.gitignore
@@ -3,6 +3,8 @@
  /ovn-nbctl.8
  /ovn-sbctl
  /ovn-sbctl.8
+/ovn-appctl
+/ovn-appctl.8
  /ovn-trace
  /ovn-trace.8
  /ovn-detrace
diff --git a/utilities/automake.mk b/utilities/automake.mk
index 21dd8ccdf..ab0f6003a 100644
--- a/utilities/automake.mk
+++ b/utilities/automake.mk
@@ -8,7 +8,8 @@ man_MANS += \
  utilities/ovn-nbctl.8 \
  utilities/ovn-sbctl.8 \
  utilities/ovn-trace.8 \
-utilities/ovn-detrace.1
+utilities/ovn-detrace.1 \
+utilities/ovn-appctl.8
  
  MAN_ROOTS += \

  utilities/ovn-sbctl.8.in \
@@ -27,6 +28,7 @@ EXTRA_DIST += \
  utilities/ovn-docker-overlay-driver.in \
  utilities/ovn-docker-underlay-driver.in \
  utilities/ovn-nbctl.8.xml \
+utilities/ovn-appctl.8.xml \
  utilities/ovn-trace.8.xml \
  utilities/ovn-detrace.in \
  utilities/ovndb-servers.ocf \
@@ -49,7 +51,9 @@ CLEANFILES += \
  utilities/ovn-sbctl.8 \
  utilities/ovn-trace.8 \
  utilities/ovn-detrace.1 \
-utilities/ovn-detrace
+utilities/ovn-detrace \
+utilities/ovn-appctl.8 \
+utilities/ovn-appctl
  
  utilities/ovn-lib: $(top_builddir)/config.status
  
@@ -68,4 +72,9 @@ bin_PROGRAMS += utilities/ovn-trace

  utilities_ovn_trace_SOURCES = utilities/ovn-trace.c
  utilities_ovn_trace_LDADD = lib/libovn.la $(OVSDB_LIBDIR)/libovsdb.la 
$(OVS_LIBDIR)/libopenvswitch.la
  
+# ovn-nbctl

+bin_PROGRAMS += utilities/ovn-appctl
+utilities_ovn_appctl_SOURCES = utilities/ovn-appctl.c
+utilities_ovn_appctl_LDADD = lib/libovn.la $(OVSDB_LIBDIR)/libovsdb.la 
$(OVS_LIBDIR)/libopenvswitch.la
+
  include utilities/bugtool/automake.mk
diff --git a/utilities/ovn-appctl.8.xml b/utilities/ovn-appctl.8.xml
new file mode 100644
index 0..32a42a766
--- /dev/null
+++ b/utilities/ovn-appctl.8.xml
@@ -0,0 +1,352 @@
+
+
+Name
+ovn-appctl -- utility for configuring running OVN daemons
+
+Synopsis
+
+   ovn-appctl [--target=target | -t target]
+  [-T secs | --timeout=secs] command [arg...]
+
+ovn-appctl --help 
+ovn-appctl --version 
+
+Description
+
+  OVN daemons accept certain commands at runtime to control their behavior
+  and query their settings. Every daemon accepts a common set of commands
+  documented under COMMON COMMANDS below. Some daemons support additional
+  commands documented in their own manpages.
+
+
+
+   The ovn-appctl program provides a simple way to invoke
+   these commands. The command to be sent is specified on
+   ovn-appctl's command line as non-option arguments.
+   ovn-appctl sends the command and prints the daemon's
+   response on standard output.
+
+
+
+  ovn-ctl is exactly similar to Open vSwitch
+  ovs-appctl utility.
+
+
+Command Commands
+
+  Every OVN daemon supports a common set of commands, which are documented
+  in this section.
+
+
+General Commands
+
+  These commands display daemon-specific commands and the running version.
+  Note that these commands are different from the --help and --version
+  options that return information about the ovn-appctl
+  utility itself.
+
+
+
+  list-commands
+  
+Lists the commands supported by the target.
+  
+

Re: [ovs-dev] [PATCH v8] netdev-dpdk:Detailed packet drop statistics

2019-09-10 Thread Sriram Vatala via dev 
-Original Message-
From: Ilya Maximets 
Sent: 10 September 2019 19:29
To: Sriram Vatala ; ovs-dev@openvswitch.org
Cc: ktray...@redhat.com; ian.sto...@intel.com
Subject: Re: [PATCH v8] netdev-dpdk:Detailed packet drop statistics

On 08.09.2019 19:01, Sriram Vatala wrote:
> OVS may be unable to transmit packets for multiple reasons and today
> there is a single counter to track packets dropped due to any of those
> reasons. The most common reason is that a VM is unable to read packets
> fast enough causing the vhostuser port transmit queue on the OVS side
> to become full. This manifests as a problem with VNFs not receiving
> all packets. Having a separate drop counter to track packets dropped
> because the transmit queue is full will clearly indicate that the
> problem is on the VM side and not in OVS. Similarly maintaining
> separate counters for all possible drops helps in indicating sensible
> cause for packet drops.
>
> This patch adds custom software stats counters to track packets
> dropped at port level and these counters are displayed along with
> other stats in "ovs-vsctl get interface  statistics"
> command. The detailed stats will be available for both dpdk and
> vhostuser ports.
>
> Signed-off-by: Sriram Vatala 
> ---
> Changes since v7 :
> Defined structure netdev_dpdk_sw_stats and moved all custom stats into
> it.
> Placed a pointer to netdev_dpdk_sw_stats structure in netdev_dpdk
> strucure.
> stats are reported with prefix with netdev_dpdk
> ---
>  include/openvswitch/netdev.h  |  14 +++
>  lib/netdev-dpdk.c | 109 +++---
>  utilities/bugtool/automake.mk |   3 +-
>  utilities/bugtool/ovs-bugtool-get-port-stats  |  25 
>  .../plugins/network-status/openvswitch.xml|   1 +
>  vswitchd/vswitch.xml  |  24 
>  6 files changed, 156 insertions(+), 20 deletions(-)  create mode
> 100755 utilities/bugtool/ovs-bugtool-get-port-stats
>
> diff --git a/include/openvswitch/netdev.h
> b/include/openvswitch/netdev.h index 0c10f7b48..c17e6a97d 100644
> --- a/include/openvswitch/netdev.h
> +++ b/include/openvswitch/netdev.h
> @@ -89,6 +89,20 @@ struct netdev_stats {
>  uint64_t rx_jabber_errors;
>  };
>
> +/* Custom software stats for dpdk ports */ struct
> +netdev_dpdk_sw_stats {
> +/* No. of retries when unable to transmit. */
> +uint64_t tx_retries;
> +/* Pkt drops when unable to transmit; Probably Tx queue is full */
> +uint64_t tx_failure_drops;
> +/* Pkt len greater than max dev MTU */
> +uint64_t tx_mtu_exceeded_drops;
> +/* Pkt drops in egress policier processing */
> +uint64_t tx_qos_drops;
> +/* Pkt drops in ingress policier processing */
> +uint64_t rx_qos_drops;
> +};

This should not be in a common header since the only user is netdev-dpdk.c.
Regarding this code itself:
1. s/policier/policer/
2. s/Pkt/Packet/, s/len/length/
3. s/max dev MTU/device MTU/ (MTU already has 'maximum' word).
4. All comments should end with a period.

Sorry I missed to check spell. I will fix this in my next patch v9
I will move this structure definition to netdev-dpdk.c  and make it static.

> +
>  /* Structure representation of custom statistics counter */  struct
> netdev_custom_counter {
>  uint64_t value;
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
> bc20d6843..39aab4672 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -413,11 +413,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,
> @@ -1171,7 +1170,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 = 0;
> +dev->sw_stats = (struct netdev_dpdk_sw_stats *)
> +xcalloc(1,sizeof(struct
> + netdev_dpdk_sw_stats));

This should be:
   dev->sw_stats = xzalloc(sizeof *dev->sw_stats); Or
   dev->sw_stats = dpdk_rte_mzalloc(sizeof *dev->sw_stats);

The later variant will require proper error handling as it could return NULL.
I will change this in next patch v9.

See the Documentation/internals/contributing/coding-style.rst for details.

>
>  return 0;
>  }
> @@ -1357,6 +1357,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);
>  ovs_mutex_destroy(>mutex);
>  }
>
> @@ -2171,6 +2172,7 @@ netdev_dpdk_vhost_rxq_recv(struct netdev_rxq *rxq,
>  struct ingress_policer *policer =

[ovs-dev] [PATCH v3 ovn] northd: add empty_lb controller_event for logical router

2019-09-10 Thread Lorenzo Bianconi 
Add empty load balancer controller_event support to logical router
pipeline. Update northd documentation even for logical switch pipeline

Signed-off-by: Lorenzo Bianconi 
---
Changes since v2:
- improve code readability
Changes since v1:
- rebase on-top of current ovn master branch
---
 northd/ovn-northd.8.xml | 10 
 northd/ovn-northd.c | 24 --
 tests/ovn.at| 56 +++--
 3 files changed, 76 insertions(+), 14 deletions(-)

diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index b34ef687a..0f4f1c112 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -1785,6 +1785,16 @@ icmp6 {
 
 
 
+  
+If controller_event has been enabled for all the configured load
+balancing rules for a Gateway router or Router with gateway port
+in OVN_Northbound database that does not have configured
+backends, a priority-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
+  
+
   
 For all the configured load balancing rules for a Gateway router or
 Router with gateway port in OVN_Northbound database that
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index c24e4d864..f393cebb8 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -6148,9 +6148,17 @@ get_force_snat_ip(struct ovn_datapath *od, const char 
*key_type, ovs_be32 *ip)
 static void
 add_router_lb_flow(struct hmap *lflows, struct ovn_datapath *od,
struct ds *match, struct ds *actions, int priority,
-   const char *lb_force_snat_ip, char *backend_ips,
-   bool is_udp, int addr_family)
+   const char *lb_force_snat_ip, struct smap_node *lb_info,
+   bool is_udp, int addr_family, char *ip_addr,
+   uint16_t l4_port, struct nbrec_load_balancer *lb,
+   struct shash *meter_groups)
 {
+char *backend_ips = lb_info->value;
+
+build_empty_lb_event_flow(od, lflows, lb_info, ip_addr, lb,
+  l4_port, addr_family, S_ROUTER_IN_DNAT,
+  meter_groups);
+
 /* A match and actions for new connections. */
 char *new_match = xasprintf("ct.new && %s", ds_cstr(match));
 if (lb_force_snat_ip) {
@@ -6308,7 +6316,7 @@ copy_ra_to_sb(struct ovn_port *op, const char 
*address_mode)
 
 static void
 build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
-struct hmap *lflows)
+struct hmap *lflows, 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. */
@@ -7525,7 +7533,6 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
 ds_put_format(, "ip && ip6.dst == %s",
 ip_address);
 }
-free(ip_address);
 
 int prio = 110;
 bool is_udp = lb->protocol && !strcmp(lb->protocol, "udp") ?
@@ -7546,8 +7553,11 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
   od->l3redirect_port->json_key);
 }
 add_router_lb_flow(lflows, od, , , prio,
-   lb_force_snat_ip, node->value, is_udp,
-   addr_family);
+   lb_force_snat_ip, node, is_udp,
+   addr_family, ip_address, port, lb,
+   meter_groups);
+
+free(ip_address);
 }
 }
 sset_destroy(_ips);
@@ -8328,7 +8338,7 @@ build_lflows(struct northd_context *ctx, struct hmap 
*datapaths,
 
 build_lswitch_flows(datapaths, ports, port_groups, , mcgroups,
 igmp_groups, meter_groups);
-build_lrouter_flows(datapaths, ports, );
+build_lrouter_flows(datapaths, ports, , meter_groups);
 
 /* Push changes to the Logical_Flow table to database. */
 const struct sbrec_logical_flow *sbflow, *next_sbflow;
diff --git a/tests/ovn.at b/tests/ovn.at
index de1b3b3ba..2749184eb 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -14683,9 +14683,22 @@ ovn_start
 # Create hypervisors hv[12].
 # Add vif1[12] to hv1, vif2[12] to hv2
 # Add all of the vifs to a single logical switch sw0.
+# Create logical router lr0
 
 net_add n1
-ovn-nbctl ls-add sw0
+
+ovn-nbctl create Logical_Router name=lr0 options:chassis=hv1
+for i in 0 1; do
+idx=$((i+1))
+ovn-nbctl ls-add sw$i
+ovn-nbctl lrp-add lr0 lrp$i 00:00:00:00:ff:0$idx 192.168.$idx.254/24
+ovn-nbctl \
+-- lsp-add sw$i lrp$i-attachment \
+-- set Logical_Switch_Port

Re: [ovs-dev] [PATCHv2] netdev-afxdp: Add need_wakeup supprt.

2019-09-10 Thread William Tu 
Hi Eelco,

Thanks for the review.

On Tue, Sep 10, 2019 at 7:24 AM Eelco Chaudron  wrote:
>
> Hi William,
>
> One comment below, but I’m also wondering if we should warn/log a
> message if OVS is compiled without this feature and we explicitly
> configure it? Same for the reverse what if the kernel does not support
> this option, will it fail?
>
yes, it's a little confusing now. Considering the following cases

1) kernel support && libbpf support && user enable
--> works

2) kernel support && libbpf NOT support && user enable
--> works, the need_wakeup is no ops, I will add warning

3) kernel NOT support && libbpf support && user enable
--> fails, the AF_XDP packet rx/tx sees no packets (libbpf is not
backward compatible?)

4) kernel NOT && libbpf NOT support && user enable
--> works, the need_wakeup is no ops, I will add warning

will address the above cases in next version.

>
> //Eelco
>
>
> On 5 Sep 2019, at 22:51, William Tu wrote:
>
> > The patch adds support for using need_wakeup flag in AF_XDP rings.
> > A new option, use_need_wakeup, is added. When this option is used,
> > it means that OVS has to explicitly wake up the kernel RX, using
> > poll()
> > syscall and wake up TX, using sendto() syscall. This feature improves
> > the performance by avoiding unnecessary sendto syscalls for TX.
> > For RX, instead of kernel always busy-spinning on fille queue, OVS
> > wakes
> > up the kernel RX processing when fill queue is replenished.
> >
> > The need_wakeup feature is merged into Linux kernel 5.3.0-rc1 and OVS
> > enables it by default. Running the feature before this version causes
> > xsk bind fails, please use options:use_need_wakeup=false to disable
> > it.
> >
> > For virtual interface, it's better set use_need_wakeup=false, since
> > the virtual device's AF_XDP xmit is synchronous: the sendto syscall
> > enters kernel and process the TX packet on tx queue directly.
> >
> > I tested on kernel 5.3.0-rc3 using its libbpf.  On Intel Xeon E5-2620
> > v3 2.4GHz system, performance of physical port to physical port
> > improves
> > from 6.1Mpps to 7.3Mpps. Testing on 5.2.0-rc6 using libbpf from
> > 5.3.0-rc3
> > does not work due to libbpf API change. Users have to use the older
> > libbpf for older kernel.
> >
> > Suggested-by: Ilya Maximets 
> > Signed-off-by: William Tu 
> > ---
> > v2:
> > - address feedbacks from Ilya and Eelco
> > - add options:use_need_wakeup, default to true
> > - remove poll timeout=1sec, make poll() return immediately
> > - naming change: rename to xsk_rx_wakeup_if_needing
> > - fix indents and return value for errno
> > ---
> >  Documentation/intro/install/afxdp.rst | 14 --
> >  acinclude.m4  |  5 ++
> >  lib/netdev-afxdp.c| 94
> > +++
> >  lib/netdev-linux-private.h|  2 +
> >  vswitchd/vswitch.xml  | 13 +
> >  5 files changed, 114 insertions(+), 14 deletions(-)
> >
> > diff --git a/Documentation/intro/install/afxdp.rst
> > b/Documentation/intro/install/afxdp.rst
> > index 820e9d993d8f..b7a3d0b7f57b 100644
> > --- a/Documentation/intro/install/afxdp.rst
> > +++ b/Documentation/intro/install/afxdp.rst
> > @@ -176,9 +176,17 @@ in :doc:`general`::
> >ovs-vswitchd ...
> >ovs-vsctl -- add-br br0 -- set Bridge br0 datapath_type=netdev
> >
> > -Make sure your device driver support AF_XDP, and to use 1 PMD (on
> > core 4)
> > -on 1 queue (queue 0) device, configure these options: **pmd-cpu-mask,
> > -pmd-rxq-affinity, and n_rxq**. The **xdpmode** can be "drv" or
> > "skb"::
> > +Make sure your device driver support AF_XDP, netdev-afxdp supports
> > +the following additional options (see man ovs-vswitchd.conf.db for
> > +more details):
> > +
> > + * **xdpmode**: use "drv" for driver mode, or "skb" for skb mode.
> > +
> > + * **use_need_wakeup**: enable by setting to "true", otherwise
> > "false"
>
> Bit confusing here, as the default is true?

Yes, thanks. I will fix it in next version.

Regards,
William


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

Re: [ovs-dev] NDP for ipv6 in OVS 2.10.0

2019-09-10 Thread William Tu 
On Tue, Sep 10, 2019 at 9:06 AM Gregory Rose  wrote:
>
>
> On 9/10/2019 7:02 AM, Thomas Goirand wrote:
> > Hi Ben and others,
> >
> > I just want to know: does OVS 2.10 has support for IPv6 mac learning /
> > NDP, so that we can avoid the ipv6 broadcast storm in our cloud?
>
> OVS will switch/forward IPv6 NDP traffic generated by network stations
> with IPv6 enabled.
> Is there some other support you're wondering about?

I'm checking the lib/mac-learning.c and ofproto/ofproto-dpif-xlate.c
I think OVS only learns MAC-port mapping from ARP packet, not IPv6 ND.

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

Re: [ovs-dev] NDP for ipv6 in OVS 2.10.0

2019-09-10 Thread Gregory Rose 



On 9/10/2019 7:02 AM, Thomas Goirand wrote:

Hi Ben and others,

I just want to know: does OVS 2.10 has support for IPv6 mac learning /
NDP, so that we can avoid the ipv6 broadcast storm in our cloud?


OVS will switch/forward IPv6 NDP traffic generated by network stations 
with IPv6 enabled.

Is there some other support you're wondering about?

- Greg



Cheers,

Thomas Goirand (zigo)

P.S: I'm not registered to this list, please CC me.
___
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] netdev-dpdk: Track vhost tx contention.

2019-09-10 Thread Eelco Chaudron 



On 26 Aug 2019, at 16:33, David Marchand wrote:

> Add a coverage counter to help diagnose contention on the vhost txqs.
> This is seen as dropped packets on the physical ports for rates that
> are usually handled fine by OVS.
>
> Signed-off-by: David Marchand 

LGTM, Acked-by: Eelco Chaudron 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCHv2] netdev-afxdp: Add need_wakeup supprt.

2019-09-10 Thread Eelco Chaudron 

Hi William,

One comment below, but I’m also wondering if we should warn/log a 
message if OVS is compiled without this feature and we explicitly 
configure it? Same for the reverse what if the kernel does not support 
this option, will it fail?



//Eelco


On 5 Sep 2019, at 22:51, William Tu wrote:


The patch adds support for using need_wakeup flag in AF_XDP rings.
A new option, use_need_wakeup, is added. When this option is used,
it means that OVS has to explicitly wake up the kernel RX, using 
poll()

syscall and wake up TX, using sendto() syscall. This feature improves
the performance by avoiding unnecessary sendto syscalls for TX.
For RX, instead of kernel always busy-spinning on fille queue, OVS 
wakes

up the kernel RX processing when fill queue is replenished.

The need_wakeup feature is merged into Linux kernel 5.3.0-rc1 and OVS
enables it by default. Running the feature before this version causes
xsk bind fails, please use options:use_need_wakeup=false to disable 
it.


For virtual interface, it's better set use_need_wakeup=false, since
the virtual device's AF_XDP xmit is synchronous: the sendto syscall
enters kernel and process the TX packet on tx queue directly.

I tested on kernel 5.3.0-rc3 using its libbpf.  On Intel Xeon E5-2620
v3 2.4GHz system, performance of physical port to physical port 
improves
from 6.1Mpps to 7.3Mpps. Testing on 5.2.0-rc6 using libbpf from 
5.3.0-rc3

does not work due to libbpf API change. Users have to use the older
libbpf for older kernel.

Suggested-by: Ilya Maximets 
Signed-off-by: William Tu 
---
v2:
- address feedbacks from Ilya and Eelco
- add options:use_need_wakeup, default to true
- remove poll timeout=1sec, make poll() return immediately
- naming change: rename to xsk_rx_wakeup_if_needing
- fix indents and return value for errno
---
 Documentation/intro/install/afxdp.rst | 14 --
 acinclude.m4  |  5 ++
 lib/netdev-afxdp.c| 94 
+++

 lib/netdev-linux-private.h|  2 +
 vswitchd/vswitch.xml  | 13 +
 5 files changed, 114 insertions(+), 14 deletions(-)

diff --git a/Documentation/intro/install/afxdp.rst 
b/Documentation/intro/install/afxdp.rst

index 820e9d993d8f..b7a3d0b7f57b 100644
--- a/Documentation/intro/install/afxdp.rst
+++ b/Documentation/intro/install/afxdp.rst
@@ -176,9 +176,17 @@ in :doc:`general`::
   ovs-vswitchd ...
   ovs-vsctl -- add-br br0 -- set Bridge br0 datapath_type=netdev

-Make sure your device driver support AF_XDP, and to use 1 PMD (on 
core 4)

-on 1 queue (queue 0) device, configure these options: **pmd-cpu-mask,
-pmd-rxq-affinity, and n_rxq**. The **xdpmode** can be "drv" or 
"skb"::

+Make sure your device driver support AF_XDP, netdev-afxdp supports
+the following additional options (see man ovs-vswitchd.conf.db for
+more details):
+
+ * **xdpmode**: use "drv" for driver mode, or "skb" for skb mode.
+
+ * **use_need_wakeup**: enable by setting to "true", otherwise 
"false"


Bit confusing here, as the default is true?


+For example, to use 1 PMD (on core 4) on 1 queue (queue 0) device,
+configure these options: **pmd-cpu-mask, pmd-rxq-affinity, and 
n_rxq**.

+The **xdpmode** can be "drv" or "skb"::

   ethtool -L enp2s0 combined 1
   ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x10
diff --git a/acinclude.m4 b/acinclude.m4
index f0e38898b17a..3f24475c96b5 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -276,6 +276,11 @@ AC_DEFUN([OVS_CHECK_LINUX_AF_XDP], [
   [Define to 1 if AF_XDP support is available and 
enabled.])

 LIBBPF_LDADD=" -lbpf -lelf"
 AC_SUBST([LIBBPF_LDADD])
+
+AC_CHECK_DECL([xsk_ring_prod__needs_wakeup], [
+  AC_DEFINE([HAVE_XDP_NEED_WAKEUP], [1],
+[XDP need wakeup support detected in xsk.h.])
+], [], [#include ])
   fi
   AM_CONDITIONAL([HAVE_AF_XDP], test "$AF_XDP_ENABLE" = true)
 ])
diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
index e5b058d08a09..5169ed3ff801 100644
--- a/lib/netdev-afxdp.c
+++ b/lib/netdev-afxdp.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -82,7 +83,7 @@ BUILD_ASSERT_DECL(PROD_NUM_DESCS == CONS_NUM_DESCS);
 #define UMEM2DESC(elem, base) ((uint64_t)((char *)elem - (char 
*)base))


 static struct xsk_socket_info *xsk_configure(int ifindex, int 
xdp_queue_id,

- int mode);
+ int mode, bool 
use_need_wakeup);

 static void xsk_remove_xdp_program(uint32_t ifindex, int xdpmode);
 static void xsk_destroy(struct xsk_socket_info *xsk);
 static int xsk_configure_all(struct netdev *netdev);
@@ -117,6 +118,49 @@ struct xsk_socket_info {
 atomic_uint64_t tx_dropped;
 };

+#ifdef HAVE_XDP_NEED_WAKEUP
+static inline void
+xsk_rx_wakeup_if_needed(struct xsk_umem_info *umem,
+struct netdev *netdev, int fd)
+{
+struct pollfd pfd;
+int ret;
+
+if

[ovs-dev] NDP for ipv6 in OVS 2.10.0

2019-09-10 Thread Thomas Goirand 
Hi Ben and others,

I just want to know: does OVS 2.10 has support for IPv6 mac learning /
NDP, so that we can avoid the ipv6 broadcast storm in our cloud?

Cheers,

Thomas Goirand (zigo)

P.S: I'm not registered to this list, please CC me.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] netdev-dpdk: add support for the RTE_ETH_EVENT_INTR_RESET event

2019-09-10 Thread Ilya Maximets 
On 10.09.2019 15:12, Eelco Chaudron wrote:
> Currently, OVS does not register and therefore not handle the
> interface reset event from the DPDK framework. This would cause a
> problem in cases where a VF is used as an interface, and its
> configuration changes.
> 
> As an example in the following scenario the MAC change is not
> detected/acted upon until OVS is restarted without the patch applied:
> 
>   $ echo 1 > /sys/bus/pci/devices/:05:00.1/sriov_numvfs
>   $ ovs-vsctl add-port ovs_pvp_br0 dpdk0 -- \
> set Interface dpdk0 type=dpdk -- \
> set Interface dpdk0 options:dpdk-devargs=:05:0a.0
> 
>   $ ip link set p5p2 vf 0 mac 52:54:00:92:d3:33
> 
> Signed-off-by: Eelco Chaudron 
> ---
> v1 -> v2:
>   - Fixed Ilya's comments
> 
>  lib/netdev-dpdk.c |   53 
> +++--
>  1 file changed, 51 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 4805783..37a431e 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -396,6 +396,8 @@ struct netdev_dpdk {
>  bool attached;
>  /* If true, rte_eth_dev_start() was successfully called */
>  bool started;
> +bool reset_needed;
> +/* 1 pad byte here. */
>  struct eth_addr hwaddr;
>  int mtu;
>  int socket_id;
> @@ -1173,6 +1175,8 @@ common_construct(struct netdev *netdev, dpdk_port_t 
> port_no,
>  ovsrcu_index_init(>vid, -1);
>  dev->vhost_reconfigured = false;
>  dev->attached = false;
> +dev->started = false;

Thanks.

> +dev->reset_needed = false;
>  
>  ovsrcu_init(>qos_conf, NULL);
>  
> @@ -1769,6 +1773,34 @@ netdev_dpdk_process_devargs(struct netdev_dpdk *dev,
>  return new_port_id;
>  }
>  
> +static int
> +dpdk_eth_event_callback(dpdk_port_t port_id, enum rte_eth_event_type type,
> +void *param OVS_UNUSED, void *ret_param OVS_UNUSED)
> +{
> +struct netdev_dpdk *dev;
> +
> +switch ((int) type) {
> +case RTE_ETH_EVENT_INTR_RESET:
> +ovs_mutex_lock(_mutex);
> +dev = netdev_dpdk_lookup_by_port_id(port_id);
> +if (dev) {
> +ovs_mutex_lock(>mutex);
> +dev->reset_needed = true;
> +netdev_request_reconfigure(>up);
> +VLOG_DBG_RL(, "Port "DPDK_PORT_ID_FMT" received a reset 
> request",
> +dev->port_id);

It'll be better to use a netdev name here. This will also allow to avoid 
checkpatch
warning.  Something like this:

   VLOG_DBG_RL(, "%s: Device reset requested.",
   netdev_get_name(>up));

> +ovs_mutex_unlock(>mutex);
> +}
> +ovs_mutex_unlock(_mutex);
> +break;
> +
> +default:
> +/* Ignore all other types */

A period is needed at the end of the comment.

> +break;
> +   }
> +   return 0;
> +}
> +
>  static void
>  dpdk_set_rxq_config(struct netdev_dpdk *dev, const struct smap *args)
>  OVS_REQUIRES(dev->mutex)
> @@ -3807,6 +3839,8 @@ netdev_dpdk_class_init(void)
>  /* This function can be called for different classes.  The initialization
>   * needs to be done only once */
>  if (ovsthread_once_start()) {
> +int ret;
> +
>  ovs_thread_create("dpdk_watchdog", dpdk_watchdog, NULL);
>  unixctl_command_register("netdev-dpdk/set-admin-state",
>   "[netdev] up|down", 1, 2,
> @@ -3820,6 +3854,15 @@ netdev_dpdk_class_init(void)
>   "[netdev]", 0, 1,
>   netdev_dpdk_get_mempool_info, NULL);
>  
> +ret = rte_eth_dev_callback_register(RTE_ETH_ALL,
> +RTE_ETH_EVENT_INTR_RESET,
> +dpdk_eth_event_callback, NULL);
> +
> +if (ret != 0) {
> +VLOG_ERR("Ethernet device callback register error: %s",
> + rte_strerror(-ret));
> +}
> +
>  ovsthread_once_done();
>  }
>  
> @@ -4180,13 +4223,19 @@ netdev_dpdk_reconfigure(struct netdev *netdev)
>  && dev->rxq_size == dev->requested_rxq_size
>  && dev->txq_size == dev->requested_txq_size
>  && dev->socket_id == dev->requested_socket_id
> -&& dev->started) {
> +&& dev->started && !dev->reset_needed) {
>  /* Reconfiguration is unnecessary */
>  
>  goto out;
>  }
>  
> -rte_eth_dev_stop(dev->port_id);
> +if (dev->reset_needed) {
> +rte_eth_dev_reset(dev->port_id);
> +dev->reset_needed = false;
> +} else {
> +rte_eth_dev_stop(dev->port_id);
> +}
> +
>  dev->started = false;
>  
>  err = netdev_dpdk_mempool_configure(dev);
> 
> 
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v8] netdev-dpdk:Detailed packet drop statistics

2019-09-10 Thread Ilya Maximets 
On 08.09.2019 19:01, Sriram Vatala wrote:
> OVS may be unable to transmit packets for multiple reasons and
> today there is a single counter to track packets dropped due to
> any of those reasons. The most common reason is that a VM is
> unable to read packets fast enough causing the vhostuser port
> transmit queue on the OVS side to become full. This manifests
> as a problem with VNFs not receiving all packets. Having a
> separate drop counter to track packets dropped because the
> transmit queue is full will clearly indicate that the problem
> is on the VM side and not in OVS. Similarly maintaining separate
> counters for all possible drops helps in indicating sensible
> cause for packet drops.
> 
> This patch adds custom software stats counters to track packets
> dropped at port level and these counters are displayed along with
> other stats in "ovs-vsctl get interface  statistics"
> command. The detailed stats will be available for both dpdk and
> vhostuser ports.
> 
> Signed-off-by: Sriram Vatala 
> ---
> Changes since v7 :
> Defined structure netdev_dpdk_sw_stats and moved all custom stats
> into it.
> Placed a pointer to netdev_dpdk_sw_stats structure in netdev_dpdk
> strucure.
> stats are reported with prefix with netdev_dpdk
> ---
>  include/openvswitch/netdev.h  |  14 +++
>  lib/netdev-dpdk.c | 109 +++---
>  utilities/bugtool/automake.mk |   3 +-
>  utilities/bugtool/ovs-bugtool-get-port-stats  |  25 
>  .../plugins/network-status/openvswitch.xml|   1 +
>  vswitchd/vswitch.xml  |  24 
>  6 files changed, 156 insertions(+), 20 deletions(-)
>  create mode 100755 utilities/bugtool/ovs-bugtool-get-port-stats
> 
> diff --git a/include/openvswitch/netdev.h b/include/openvswitch/netdev.h
> index 0c10f7b48..c17e6a97d 100644
> --- a/include/openvswitch/netdev.h
> +++ b/include/openvswitch/netdev.h
> @@ -89,6 +89,20 @@ struct netdev_stats {
>  uint64_t rx_jabber_errors;
>  };
>  
> +/* Custom software stats for dpdk ports */
> +struct netdev_dpdk_sw_stats {
> +/* No. of retries when unable to transmit. */
> +uint64_t tx_retries;
> +/* Pkt drops when unable to transmit; Probably Tx queue is full */
> +uint64_t tx_failure_drops;
> +/* Pkt len greater than max dev MTU */
> +uint64_t tx_mtu_exceeded_drops;
> +/* Pkt drops in egress policier processing */
> +uint64_t tx_qos_drops;
> +/* Pkt drops in ingress policier processing */
> +uint64_t rx_qos_drops;
> +};

This should not be in a common header since the only user is netdev-dpdk.c.
Regarding this code itself:
1. s/policier/policer/
2. s/Pkt/Packet/, s/len/length/
3. s/max dev MTU/device MTU/ (MTU already has 'maximum' word).
4. All comments should end with a period.

> +
>  /* Structure representation of custom statistics counter */
>  struct netdev_custom_counter {
>  uint64_t value;
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index bc20d6843..39aab4672 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -413,11 +413,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,
> @@ -1171,7 +1170,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 = 0;
> +dev->sw_stats = (struct netdev_dpdk_sw_stats *)
> +xcalloc(1,sizeof(struct netdev_dpdk_sw_stats));

This should be:
   dev->sw_stats = xzalloc(sizeof *dev->sw_stats);
Or
   dev->sw_stats = dpdk_rte_mzalloc(sizeof *dev->sw_stats);

The later variant will require proper error handling as it could return NULL.

See the Documentation/internals/contributing/coding-style.rst for details.

>  
>  return 0;
>  }
> @@ -1357,6 +1357,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);
>  ovs_mutex_destroy(>mutex);
>  }
>  
> @@ -2171,6 +2172,7 @@ netdev_dpdk_vhost_rxq_recv(struct netdev_rxq *rxq,
>  struct ingress_policer *policer = netdev_dpdk_get_ingress_policer(dev);
>  uint16_t nb_rx = 0;
>  uint16_t dropped = 0;
> +uint16_t qos_drops = 0;
>  int qid = rxq->queue_id * VIRTIO_QNUM + VIRTIO_TXQ;
>  int vid = netdev_dpdk_get_vid(dev);
>  
> @@ -2202,11 +2204,13 @@ netdev_dpdk_vhost_rxq_recv(struct netdev_rxq *rxq,
>  (struct rte_mbuf **) batch->packets,
>

Re: [ovs-dev] [PATCH v2] netdev-dpdk: add support for the RTE_ETH_EVENT_INTR_RESET event

2019-09-10 Thread 0-day Robot 
Bleep bloop.  Greetings Eelco Chaudron, 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 80 characters long (recommended limit is 79)
#66 FILE: lib/netdev-dpdk.c:1756:
VLOG_DBG_RL(, "Port "DPDK_PORT_ID_FMT" received a reset request",

Lines checked: 132, 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 v1] netdev-dpdk: Fix flow control configuration.

2019-09-10 Thread Ilya Maximets 
On 10.09.2019 13:43, Konieczny, TomaszX wrote:
> -Original Message-
> From: Ilya Maximets  
> Sent: 10 September 2019 11:44
> To: Konieczny, TomaszX ; d...@openvswitch.org
> Cc: Stokes, Ian 
> Subject: Re: [ovs-dev] [PATCH v1] netdev-dpdk: Fix flow control configuration.
> 
> On 10.09.2019 12:29, Ilya Maximets wrote:
>> On 09.09.2019 14:39, Tomasz Konieczny wrote:
>>> Currently OVS is unable to change flow control configuration in DPDK 
>>> because new settings are being overwritten by current settings with 
>>> rte_eth_dev_flow_ctrl_get(). The fix restores correct order of 
>>> operations and at the same time does not trigger error on devices 
>>> without flow control support when flow control not requested.
>>>
>>> Fixes: d95a8d2b0bd6 ("netdev-dpdk: Fix flow control configuration.")
>>> Signed-off-by: Tomasz Konieczny 
>>> ---
>>
>> Hi Tomasz,
>> Thanks for the fix!
>>
>> I agree that current code in master doesn't make any sense. :) It 
>> seems that no-one uses this functionality since it's broken for more 
>> than a year already.
>>
>> Fixes tag should point to the commit from master branch, so it should 
>> be:
>> Fixes: 7e1de65e8dfb ("netdev-dpdk: Fix failure to configure flow 
>> control at netdev-init.")
>>
>> Regarding the fix itself:
>> Can we just move following two lines:
>>
>> dev->fc_conf.mode = fc_mode;
>> dev->fc_conf.autoneg = autoneg;
>>
>> below the rte_eth_dev_flow_ctrl_get() ?
>> Current version of the patch will re-setup flow control on each call 
>> if it is not in initial state.
> 
> One more nit is that it's, probably, better to re-name this patch to be more 
> specific. Especially because we already have equally named patch on other 
> branch. (The patch still needs to have v2 in a subject prefix with re-naming 
> mentioned in a version history.)
> 
>>
>> Best regards, Ilya Maximets.
> 
> Hi Ilya,
> Thank you for your comments.
> 
> I will adhere to tag and name issues.
> 
> Regarding the fix itself comment:
> These lines need to be run separately from get() in order to allow disabling 
> already enabled
> flow control - get() will not run in this case.

I looked at the code once more and to the dpdk drivers' implementation.
You're partially right, but we have to get() flow control configuration
in all cases because some drivers (e.g. ixgbe) has flow control enabled
by default and since we have 'false' as a default value, we need to disable
the flow control for it. Not calling the get() will result in setting
zeroized fc_config, which may be discarded by the driver since those
values are validated inside.

So, I think that we need to call get() function unconditionally.
Avoiding of failure for devices that doesn't support flow control could
be implemented like this:

/* Get the Flow control configuration. */
err = rte_eth_dev_flow_ctrl_get(dev->port_id, >fc_conf);
if (err) {
if (err == -ENOTSUP) {
VLOG_INFO_ONCE("%s: Flow control is not supported.",
   netdev_get_name(netdev));
err = 0; /* Not fatal. */
} else {
VLOG_WARN("%s: Cannot get flow control parameters: %s",
  netdev_get_name(netdev), rte_strerror(-err));
}
goto out;
}

What do you think?


One minor nit is that it's better to keep an empty line here:
---
 autoneg = smap_get_bool(args, "flow-ctrl-autoneg", false);
-
 fc_mode = fc_mode_set[tx_fc_en][rx_fc_en];
---

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

Re: [ovs-dev] [PATCH v8] netdev-dpdk:Detailed packet drop statistics

2019-09-10 Thread Sriram Vatala via dev 
Hi All,
I have addressed the comments given in patch v7. Please review the patch v8
and let me know your comments if any.

Thanks & Regards,
Sriram.

-Original Message-
From: Sriram Vatala  
Sent: 08 September 2019 21:32
To: ovs-dev@openvswitch.org; i.maxim...@samsung.com
Cc: ktray...@redhat.com; ian.sto...@intel.com; Sriram Vatala

Subject: [PATCH v8] netdev-dpdk:Detailed packet drop statistics

OVS may be unable to transmit packets for multiple reasons and today there
is a single counter to track packets dropped due to any of those reasons.
The most common reason is that a VM is unable to read packets fast enough
causing the vhostuser port transmit queue on the OVS side to become full.
This manifests as a problem with VNFs not receiving all packets. Having a
separate drop counter to track packets dropped because the transmit queue is
full will clearly indicate that the problem is on the VM side and not in
OVS. Similarly maintaining separate counters for all possible drops helps in
indicating sensible cause for packet drops.

This patch adds custom software stats counters to track packets dropped at
port level and these counters are displayed along with other stats in
"ovs-vsctl get interface  statistics"
command. The detailed stats will be available for both dpdk and vhostuser
ports.

Signed-off-by: Sriram Vatala 
---
Changes since v7 :
Defined structure netdev_dpdk_sw_stats and moved all custom stats into it.
Placed a pointer to netdev_dpdk_sw_stats structure in netdev_dpdk strucure.
stats are reported with prefix with netdev_dpdk
---
 include/openvswitch/netdev.h  |  14 +++
 lib/netdev-dpdk.c | 109 +++---
 utilities/bugtool/automake.mk |   3 +-
 utilities/bugtool/ovs-bugtool-get-port-stats  |  25 
 .../plugins/network-status/openvswitch.xml|   1 +
 vswitchd/vswitch.xml  |  24 
 6 files changed, 156 insertions(+), 20 deletions(-)  create mode 100755
utilities/bugtool/ovs-bugtool-get-port-stats

diff --git a/include/openvswitch/netdev.h b/include/openvswitch/netdev.h
index 0c10f7b48..c17e6a97d 100644
--- a/include/openvswitch/netdev.h
+++ b/include/openvswitch/netdev.h
@@ -89,6 +89,20 @@ struct netdev_stats {
 uint64_t rx_jabber_errors;
 };
 
+/* Custom software stats for dpdk ports */ struct netdev_dpdk_sw_stats 
+{
+/* No. of retries when unable to transmit. */
+uint64_t tx_retries;
+/* Pkt drops when unable to transmit; Probably Tx queue is full */
+uint64_t tx_failure_drops;
+/* Pkt len greater than max dev MTU */
+uint64_t tx_mtu_exceeded_drops;
+/* Pkt drops in egress policier processing */
+uint64_t tx_qos_drops;
+/* Pkt drops in ingress policier processing */
+uint64_t rx_qos_drops;
+};
+
 /* Structure representation of custom statistics counter */  struct
netdev_custom_counter {
 uint64_t value;
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
bc20d6843..39aab4672 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -413,11 +413,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,
@@ -1171,7 +1170,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 = 0;
+dev->sw_stats = (struct netdev_dpdk_sw_stats *)
+xcalloc(1,sizeof(struct netdev_dpdk_sw_stats));
 
 return 0;
 }
@@ -1357,6 +1357,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);
 ovs_mutex_destroy(>mutex);
 }
 
@@ -2171,6 +2172,7 @@ netdev_dpdk_vhost_rxq_recv(struct netdev_rxq *rxq,
 struct ingress_policer *policer = netdev_dpdk_get_ingress_policer(dev);
 uint16_t nb_rx = 0;
 uint16_t dropped = 0;
+uint16_t qos_drops = 0;
 int qid = rxq->queue_id * VIRTIO_QNUM + VIRTIO_TXQ;
 int vid = netdev_dpdk_get_vid(dev);
 
@@ -2202,11 +2204,13 @@ netdev_dpdk_vhost_rxq_recv(struct netdev_rxq *rxq,
 (struct rte_mbuf **) batch->packets,
 nb_rx, true);
 dropped -= nb_rx;
+qos_drops = dropped;
 }
 
 rte_spinlock_lock(>stats_lock);
 netdev_dpdk_vhost_update_rx_counters(>stats, batch->packets,
  nb_rx, dropped);
+dev->sw_stats->rx_qos_drops += qos_drops;
 rte_spinlock_unlock(>stats_lock);
 
 batch->count = nb_rx;
@@ -2232,6 +2236,7 @@

[ovs-dev] [PATCH v2] netdev-dpdk: add support for the RTE_ETH_EVENT_INTR_RESET event

2019-09-10 Thread Eelco Chaudron 
Currently, OVS does not register and therefore not handle the
interface reset event from the DPDK framework. This would cause a
problem in cases where a VF is used as an interface, and its
configuration changes.

As an example in the following scenario the MAC change is not
detected/acted upon until OVS is restarted without the patch applied:

  $ echo 1 > /sys/bus/pci/devices/:05:00.1/sriov_numvfs
  $ ovs-vsctl add-port ovs_pvp_br0 dpdk0 -- \
set Interface dpdk0 type=dpdk -- \
set Interface dpdk0 options:dpdk-devargs=:05:0a.0

  $ ip link set p5p2 vf 0 mac 52:54:00:92:d3:33

Signed-off-by: Eelco Chaudron 
---
v1 -> v2:
  - Fixed Ilya's comments

 lib/netdev-dpdk.c |   53 +++--
 1 file changed, 51 insertions(+), 2 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 4805783..37a431e 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -396,6 +396,8 @@ struct netdev_dpdk {
 bool attached;
 /* If true, rte_eth_dev_start() was successfully called */
 bool started;
+bool reset_needed;
+/* 1 pad byte here. */
 struct eth_addr hwaddr;
 int mtu;
 int socket_id;
@@ -1173,6 +1175,8 @@ common_construct(struct netdev *netdev, dpdk_port_t 
port_no,
 ovsrcu_index_init(>vid, -1);
 dev->vhost_reconfigured = false;
 dev->attached = false;
+dev->started = false;
+dev->reset_needed = false;
 
 ovsrcu_init(>qos_conf, NULL);
 
@@ -1769,6 +1773,34 @@ netdev_dpdk_process_devargs(struct netdev_dpdk *dev,
 return new_port_id;
 }
 
+static int
+dpdk_eth_event_callback(dpdk_port_t port_id, enum rte_eth_event_type type,
+void *param OVS_UNUSED, void *ret_param OVS_UNUSED)
+{
+struct netdev_dpdk *dev;
+
+switch ((int) type) {
+case RTE_ETH_EVENT_INTR_RESET:
+ovs_mutex_lock(_mutex);
+dev = netdev_dpdk_lookup_by_port_id(port_id);
+if (dev) {
+ovs_mutex_lock(>mutex);
+dev->reset_needed = true;
+netdev_request_reconfigure(>up);
+VLOG_DBG_RL(, "Port "DPDK_PORT_ID_FMT" received a reset 
request",
+dev->port_id);
+ovs_mutex_unlock(>mutex);
+}
+ovs_mutex_unlock(_mutex);
+break;
+
+default:
+/* Ignore all other types */
+break;
+   }
+   return 0;
+}
+
 static void
 dpdk_set_rxq_config(struct netdev_dpdk *dev, const struct smap *args)
 OVS_REQUIRES(dev->mutex)
@@ -3807,6 +3839,8 @@ netdev_dpdk_class_init(void)
 /* This function can be called for different classes.  The initialization
  * needs to be done only once */
 if (ovsthread_once_start()) {
+int ret;
+
 ovs_thread_create("dpdk_watchdog", dpdk_watchdog, NULL);
 unixctl_command_register("netdev-dpdk/set-admin-state",
  "[netdev] up|down", 1, 2,
@@ -3820,6 +3854,15 @@ netdev_dpdk_class_init(void)
  "[netdev]", 0, 1,
  netdev_dpdk_get_mempool_info, NULL);
 
+ret = rte_eth_dev_callback_register(RTE_ETH_ALL,
+RTE_ETH_EVENT_INTR_RESET,
+dpdk_eth_event_callback, NULL);
+
+if (ret != 0) {
+VLOG_ERR("Ethernet device callback register error: %s",
+ rte_strerror(-ret));
+}
+
 ovsthread_once_done();
 }
 
@@ -4180,13 +4223,19 @@ netdev_dpdk_reconfigure(struct netdev *netdev)
 && dev->rxq_size == dev->requested_rxq_size
 && dev->txq_size == dev->requested_txq_size
 && dev->socket_id == dev->requested_socket_id
-&& dev->started) {
+&& dev->started && !dev->reset_needed) {
 /* Reconfiguration is unnecessary */
 
 goto out;
 }
 
-rte_eth_dev_stop(dev->port_id);
+if (dev->reset_needed) {
+rte_eth_dev_reset(dev->port_id);
+dev->reset_needed = false;
+} else {
+rte_eth_dev_stop(dev->port_id);
+}
+
 dev->started = false;
 
 err = netdev_dpdk_mempool_configure(dev);

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

Re: [ovs-dev] [PATCH] netdev-dpdk: add support for the RTE_ETH_EVENT_INTR_RESET event

2019-09-10 Thread Ilya Maximets 
On 10.09.2019 14:11, Eelco Chaudron wrote:
> 
> 
> On 5 Sep 2019, at 14:40, Ilya Maximets wrote:
> 
>> Hi Eelco,
> 
> 
>>> , 2 deletions(-)
>>>
>>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>>> index bc20d6843..a23150387 100644
>>> --- a/lib/netdev-dpdk.c
>>> +++ b/lib/netdev-dpdk.c
>>> @@ -362,6 +362,7 @@ struct netdev_dpdk {
>>>  bool attached;
>>>  /* If true, rte_eth_dev_start() was successfully called */
>>>  bool started;
>>> +    bool reset_needed;
>>
>> This will produce a hole in the structure as members will no longer
>> fit into a single cacheline.  And cacheline markers will be misaligned.
>>
>> See details for similar issue here:
>> https://protect2.fireeye.com/url?k=66886626-3bef2200-6689ed69-0cc47a31384a-d13032efc289ec6c=https://mail.openvswitch.org/pipermail/ovs-dev/2019-September/362349.html
>>
>> Possible solution is to turn existing flags into bit fields or stop
>> avoiding the problem and drop most of padding in the structure.
>>
>>
>>>  struct eth_addr hwaddr;
>>>  int mtu;
>>>  int socket_id;
> 
> I did a pahole check, as my counting is always off, and it look ok, even with 
> the change:
> 
> struct netdev_dpdk {
> union {
>     OVS_CACHE_LINE_MARKER cacheline0;    /* 0 1 */
>     struct {
>     dpdk_port_t port_id; /* 0 2 */
>     _Bool  attached; /* 2 1 */
>     _Bool  started;  /* 3 1 */
>     _Bool  reset_needed; /* 4 1 */
> 
>     /* XXX 1 byte hole, try to pack */
> 
>     struct eth_addr hwaddr;  /* 6 6 */
>     int    mtu;  /*    12 4 */

OK. I see. There was a 2 byte hole after 'hwaddr' and with your patch
it moved earlier moving the 'hwaddr' from byte 4 to byte 6.
Maybe you could add a /* 1 pad byte here. */ after the new flag?

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

Re: [ovs-dev] [PATCH] netdev-dpdk: add support for the RTE_ETH_EVENT_INTR_RESET event

2019-09-10 Thread Eelco Chaudron 




On 5 Sep 2019, at 14:40, Ilya Maximets wrote:


Hi Eelco,




, 2 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index bc20d6843..a23150387 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -362,6 +362,7 @@ struct netdev_dpdk {
 bool attached;
 /* If true, rte_eth_dev_start() was successfully called */
 bool started;
+bool reset_needed;


This will produce a hole in the structure as members will no longer
fit into a single cacheline.  And cacheline markers will be 
misaligned.


See details for similar issue here:
https://mail.openvswitch.org/pipermail/ovs-dev/2019-September/362349.html

Possible solution is to turn existing flags into bit fields or stop
avoiding the problem and drop most of padding in the structure.



 struct eth_addr hwaddr;
 int mtu;
 int socket_id;


I did a pahole check, as my counting is always off, and it look ok, even 
with the change:


struct netdev_dpdk {
union {
OVS_CACHE_LINE_MARKER cacheline0;/* 0 1 */
struct {
dpdk_port_t port_id; /* 0 2 */
_Bool  attached; /* 2 1 */
_Bool  started;  /* 3 1 */
_Bool  reset_needed; /* 4 1 */

/* XXX 1 byte hole, try to pack */

struct eth_addr hwaddr;  /* 6 6 */
intmtu;  /*12 4 */
intsocket_id;/*16 4 */
intbuf_size; /*20 4 */
intmax_packet_len;   /*24 4 */
enum dpdk_dev_type type; /*28 4 */
enum netdev_flags flags; /*32 4 */
intlink_reset_cnt;   /*36 4 */
union {
char * devargs;  /*40 8 */
char * vhost_id; /*40 8 */
};   /*40 8 */
struct dpdk_tx_queue * tx_q; /*48 8 */
struct rte_eth_link link;/*56 8 */

/* XXX last struct has 2 bytes of padding */
};   /* 064 */
uint8_tpad49[64];/* 064 */
};



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

Re: [ovs-dev] [PATCH v1] netdev-dpdk: Fix flow control configuration.

2019-09-10 Thread Konieczny, TomaszX 
-Original Message-
From: Ilya Maximets  
Sent: 10 September 2019 11:44
To: Konieczny, TomaszX ; d...@openvswitch.org
Cc: Stokes, Ian 
Subject: Re: [ovs-dev] [PATCH v1] netdev-dpdk: Fix flow control configuration.

On 10.09.2019 12:29, Ilya Maximets wrote:
> On 09.09.2019 14:39, Tomasz Konieczny wrote:
>> Currently OVS is unable to change flow control configuration in DPDK 
>> because new settings are being overwritten by current settings with 
>> rte_eth_dev_flow_ctrl_get(). The fix restores correct order of 
>> operations and at the same time does not trigger error on devices 
>> without flow control support when flow control not requested.
>>
>> Fixes: d95a8d2b0bd6 ("netdev-dpdk: Fix flow control configuration.")
>> Signed-off-by: Tomasz Konieczny 
>> ---
> 
> Hi Tomasz,
> Thanks for the fix!
> 
> I agree that current code in master doesn't make any sense. :) It 
> seems that no-one uses this functionality since it's broken for more 
> than a year already.
> 
> Fixes tag should point to the commit from master branch, so it should 
> be:
> Fixes: 7e1de65e8dfb ("netdev-dpdk: Fix failure to configure flow 
> control at netdev-init.")
> 
> Regarding the fix itself:
> Can we just move following two lines:
> 
> dev->fc_conf.mode = fc_mode;
> dev->fc_conf.autoneg = autoneg;
> 
> below the rte_eth_dev_flow_ctrl_get() ?
> Current version of the patch will re-setup flow control on each call 
> if it is not in initial state.

One more nit is that it's, probably, better to re-name this patch to be more 
specific. Especially because we already have equally named patch on other 
branch. (The patch still needs to have v2 in a subject prefix with re-naming 
mentioned in a version history.)

> 
> Best regards, Ilya Maximets.

Hi Ilya,
Thank you for your comments.

I will adhere to tag and name issues.

Regarding the fix itself comment:
These lines need to be run separately from get() in order to allow disabling 
already enabled
flow control - get() will not run in this case. Also, as far as I understand, 
these lines
and re-setup will only run when there is some change in flow control settings.
if (dev->fc_conf.mode != fc_mode || autoneg != dev->fc_conf.autoneg)



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

Re: [ovs-dev] [PATCH v6 1/1] Avoid dp_hash recirculation for balance-tcp bond selection mode

2019-09-10 Thread 0-day Robot 
Bleep bloop.  Greetings Vishal Deep Ajmera 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:
WARNING: Line is 81 characters long (recommended limit is 79)
#218 FILE: lib/dpif-netdev.c:732:
 * Note: This flag is decoupled from 
'reload'

WARNING: Line is 81 characters long (recommended limit is 79)
#219 FILE: lib/dpif-netdev.c:733:
 * flag otherwise full pmd reload will 
become

Lines checked: 1627, Warnings: 2, 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

[ovs-dev] [PATCH v6 1/1] Avoid dp_hash recirculation for balance-tcp bond selection mode

2019-09-10 Thread Vishal Deep Ajmera via dev 
Problem:

In OVS-DPDK, flows with output over a bond interface of type “balance-tcp”
(using a hash on TCP/UDP 5-tuple) get translated by the ofproto layer into
"HASH" and "RECIRC" datapath actions. After recirculation, the packet is
forwarded to the bond member port based on 8-bits of the datapath hash
value computed through dp_hash. This causes performance degradation in the
following ways:

1. The recirculation of the packet implies another lookup of the packet’s
flow key in the exact match cache (EMC) and potentially Megaflow classifier
(DPCLS). This is the biggest cost factor.

2. The recirculated packets have a new “RSS” hash and compete with the
original packets for the scarce number of EMC slots. This implies more
EMC misses and potentially EMC thrashing causing costly DPCLS lookups.

3. The 256 extra megaflow entries per bond for dp_hash bond selection put
additional load on the revalidation threads.

Owing to this performance degradation, deployments stick to “balance-slb”
bond mode even though it does not do active-active load balancing for
VXLAN- and GRE-tunnelled traffic because all tunnel packet have the same
source MAC address.

Proposed optimization:
--
This proposal introduces a new load-balancing output action instead of
recirculation.

Maintain one table per-bond (could just be an array of uint16's) and
program it the same way internal flows are created today for each possible
hash value(256 entries) from ofproto layer. Use this table to load-balance
flows as part of output action processing.

Currently xlate_normal() -> output_normal() -> bond_update_post_recirc_rules()
-> bond_may_recirc() and compose_output_action__() generate
“dp_hash(hash_l4(0))” and “recirc()” actions. In this case the
RecircID identifies the bond. For the recirculated packets the ofproto layer
installs megaflow entries that match on RecircID and masked dp_hash and send
them to the corresponding output port.

Instead, we will now generate actions as
"hash(l4(0)),lb_output(bond,)"

This combines hash computation (only if needed, else re-use RSS hash) and
inline load-balancing over the bond. This action is used *only* for balance-tcp
bonds in OVS-DPDK datapath (the OVS kernel datapath remains unchanged).

Example:

Current scheme:
---
With 1 IP-UDP flow:

flow-dump from pmd on cpu core: 2
recirc_id(0),in_port(7),packet_type(ns=0,id=0),eth(src=02:00:00:02:14:01,dst=0c:c4:7a:58:f0:2b),eth_type(0x0800),ipv4(frag=no),
 packets:2828969, bytes:181054016, used:0.000s, 
actions:hash(hash_l4(0)),recirc(0x1)

recirc_id(0x1),dp_hash(0x113683bd/0xff),in_port(7),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
 packets:2828937, bytes:181051968, used:0.000s, actions:2

With 8 IP-UDP flows (with random UDP src port): (all hitting same DPCL):

flow-dump from pmd on cpu core: 2
recirc_id(0),in_port(7),packet_type(ns=0,id=0),eth(src=02:00:00:02:14:01,dst=0c:c4:7a:58:f0:2b),eth_type(0x0800),ipv4(frag=no),
 packets:2674009, bytes:171136576, used:0.000s, 
actions:hash(hash_l4(0)),recirc(0x1)

recirc_id(0x1),dp_hash(0xf8e02b7e/0xff),in_port(7),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
 packets:377395, bytes:24153280, used:0.000s, actions:2
recirc_id(0x1),dp_hash(0xb236c260/0xff),in_port(7),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
 packets:333486, bytes:21343104, used:0.000s, actions:1
recirc_id(0x1),dp_hash(0x7d89eb18/0xff),in_port(7),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
 packets:348461, bytes:22301504, used:0.000s, actions:1
recirc_id(0x1),dp_hash(0xa78d75df/0xff),in_port(7),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
 packets:633353, bytes:40534592, used:0.000s, actions:2
recirc_id(0x1),dp_hash(0xb58d846f/0xff),in_port(7),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
 packets:319901, bytes:20473664, used:0.001s, actions:2
recirc_id(0x1),dp_hash(0x24534406/0xff),in_port(7),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
 packets:334985, bytes:21439040, used:0.001s, actions:1
recirc_id(0x1),dp_hash(0x3cf32550/0xff),in_port(7),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
 packets:326404, bytes:20889856, used:0.001s, actions:1

New scheme:
---
We can do with a single flow entry (for any number of new flows):

in_port(7),packet_type(ns=0,id=0),eth(src=02:00:00:02:14:01,dst=0c:c4:7a:58:f0:2b),eth_type(0x0800),ipv4(frag=no),
 packets:2674009, bytes:171136576, used:0.000s, 
actions:hash(l4(0)),lb_output(bond,1)

A new CLI has been added to dump the datapath bond cache as given below.

“sudo ovs-appctl dpif-netdev/dp-bond-show [dp]”

root@ubuntu-190:performance_scripts # sudo ovs-appctl dpif-netdev/dp-bond-show
Bond cache:
bond-id 1 :
bucket 0 - slave 2
bucket 1 - slave 1
bucket 2 - slave 2
bucket 3 - slave 1

Performance improvement:

With a prototype of the proposed idea, the following perf improvement

[ovs-dev] [PATCH v6 0/1] Balance-tcp bond mode optimization

2019-09-10 Thread Vishal Deep Ajmera via dev 
v5->v6:
 Addressed comments from Ilya Maximets.
 https://mail.openvswitch.org/pipermail/ovs-dev/2019-August/362001.html
 Rebased to OVS master.

v4->v5:
 Support for stats per hash bucket.
 Support for dynamic load balancing.
 Rebased to OVS Master.

v3->v4:
 Addressed Ilya Maximets comments.
 https://mail.openvswitch.org/pipermail/ovs-dev/2019-July/360452.html

v2->v3:
 Rebased to OVS master.
 Fixed git merge issue.

v1->v2:
 Updated datapath action to hash + lb-output.
 Updated throughput test observations.
 Rebased to OVS master.

Vishal Deep Ajmera (1):
  Avoid dp_hash recirculation for balance-tcp bond selection mode

 datapath/linux/compat/include/linux/openvswitch.h |   2 +
 lib/dpif-netdev.c | 515 --
 lib/dpif-netlink.c|   3 +
 lib/dpif-provider.h   |   8 +
 lib/dpif.c|  48 ++
 lib/dpif.h|   7 +
 lib/odp-execute.c |   2 +
 lib/odp-util.c|   4 +
 ofproto/bond.c|  52 ++-
 ofproto/bond.h|   9 +
 ofproto/ofproto-dpif-ipfix.c  |   1 +
 ofproto/ofproto-dpif-sflow.c  |   1 +
 ofproto/ofproto-dpif-xlate.c  |  39 +-
 ofproto/ofproto-dpif.c|  32 ++
 ofproto/ofproto-dpif.h|  12 +-
 tests/lacp.at |   9 +
 vswitchd/bridge.c |   4 +
 vswitchd/vswitch.xml  |  10 +
 18 files changed, 698 insertions(+), 60 deletions(-)

-- 
1.9.1

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

Re: [ovs-dev] [PATCH v1] netdev-dpdk: Fix flow control configuration.

2019-09-10 Thread Ilya Maximets 
On 10.09.2019 12:29, Ilya Maximets wrote:
> On 09.09.2019 14:39, Tomasz Konieczny wrote:
>> Currently OVS is unable to change flow control configuration in DPDK
>> because new settings are being overwritten by current settings
>> with rte_eth_dev_flow_ctrl_get(). The fix restores correct order
>> of operations and at the same time does not trigger error on devices
>> without flow control support when flow control not requested.
>>
>> Fixes: d95a8d2b0bd6 ("netdev-dpdk: Fix flow control configuration.")
>> Signed-off-by: Tomasz Konieczny 
>> ---
> 
> Hi Tomasz,
> Thanks for the fix!
> 
> I agree that current code in master doesn't make any sense. :)
> It seems that no-one uses this functionality since it's broken
> for more than a year already.
> 
> Fixes tag should point to the commit from master branch, so it
> should be:
> Fixes: 7e1de65e8dfb ("netdev-dpdk: Fix failure to configure flow control at 
> netdev-init.")
> 
> Regarding the fix itself:
> Can we just move following two lines:
> 
> dev->fc_conf.mode = fc_mode;
> dev->fc_conf.autoneg = autoneg;
> 
> below the rte_eth_dev_flow_ctrl_get() ?
> Current version of the patch will re-setup flow control on each call
> if it is not in initial state.

One more nit is that it's, probably, better to re-name this patch to be
more specific. Especially because we already have equally named patch
on other branch. (The patch still needs to have v2 in a subject prefix
with re-naming mentioned in a version history.)

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

[ovs-dev] [PATCH] Exclude ovn-nb/ovn-sb man and OVN schema files during compilation.

2019-09-10 Thread nusiddiq 
From: Numan Siddique 

The commit [1] removed OVN, but had to leave out some OVN bits
for the ovsdb-server raft testing. But "make install" is installing
ovn-nb/ovn-sb man entries and OVN schema files.

This patch excludes these.

"make install" is also installing ovn-nbctl/ovn-sbctl and this still needs to
be addressed.

[1] - f3e24610ea8("Remove OVN.")

Signed-off-by: Numan Siddique 
---
 ovn/automake.mk   | 83 ++-
 ovn/utilities/automake.mk | 10 +
 2 files changed, 6 insertions(+), 87 deletions(-)

diff --git a/ovn/automake.mk b/ovn/automake.mk
index afaf0688c..7d16c6036 100644
--- a/ovn/automake.mk
+++ b/ovn/automake.mk
@@ -1,82 +1,7 @@
-# OVN southbound schema and IDL
-EXTRA_DIST += ovn/ovn-sb.ovsschema
-pkgdata_DATA += ovn/ovn-sb.ovsschema
-
-# OVN southbound E-R diagram
-#
-# If "python" or "dot" is not available, then we do not add graphical diagram
-# to the documentation.
-if HAVE_PYTHON
-if HAVE_DOT
-ovn/ovn-sb.gv: ovsdb/ovsdb-dot.in ovn/ovn-sb.ovsschema
-   $(AM_V_GEN)$(OVSDB_DOT) --no-arrows $(srcdir)/ovn/ovn-sb.ovsschema > $@
-ovn/ovn-sb.pic: ovn/ovn-sb.gv ovsdb/dot2pic
-   $(AM_V_GEN)(dot -T plain < ovn/ovn-sb.gv | $(PYTHON) 
$(srcdir)/ovsdb/dot2pic -f 3) > $@.tmp && \
-   mv $@.tmp $@
-OVN_SB_PIC = ovn/ovn-sb.pic
-OVN_SB_DOT_DIAGRAM_ARG = --er-diagram=$(OVN_SB_PIC)
-CLEANFILES += ovn/ovn-sb.gv ovn/ovn-sb.pic
-endif
-endif
-
-# OVN southbound schema documentation
-EXTRA_DIST += ovn/ovn-sb.xml
-CLEANFILES += ovn/ovn-sb.5
-man_MANS += ovn/ovn-sb.5
-ovn/ovn-sb.5: \
-   ovsdb/ovsdb-doc ovn/ovn-sb.xml ovn/ovn-sb.ovsschema $(OVN_SB_PIC)
-   $(AM_V_GEN)$(OVSDB_DOC) \
-   $(OVN_SB_DOT_DIAGRAM_ARG) \
-   --version=$(VERSION) \
-   $(srcdir)/ovn/ovn-sb.ovsschema \
-   $(srcdir)/ovn/ovn-sb.xml > $@.tmp && \
-   mv $@.tmp $@
-
-# OVN northbound schema and IDL
-EXTRA_DIST += ovn/ovn-nb.ovsschema
-pkgdata_DATA += ovn/ovn-nb.ovsschema
-
-# OVN northbound E-R diagram
-#
-# If "python" or "dot" is not available, then we do not add graphical diagram
-# to the documentation.
-if HAVE_PYTHON
-if HAVE_DOT
-ovn/ovn-nb.gv: ovsdb/ovsdb-dot.in ovn/ovn-nb.ovsschema
-   $(AM_V_GEN)$(OVSDB_DOT) --no-arrows $(srcdir)/ovn/ovn-nb.ovsschema > $@
-ovn/ovn-nb.pic: ovn/ovn-nb.gv ovsdb/dot2pic
-   $(AM_V_GEN)(dot -T plain < ovn/ovn-nb.gv | $(PYTHON) 
$(srcdir)/ovsdb/dot2pic -f 3) > $@.tmp && \
-   mv $@.tmp $@
-OVN_NB_PIC = ovn/ovn-nb.pic
-OVN_NB_DOT_DIAGRAM_ARG = --er-diagram=$(OVN_NB_PIC)
-CLEANFILES += ovn/ovn-nb.gv ovn/ovn-nb.pic
-endif
-endif
-
-# OVN northbound schema documentation
-EXTRA_DIST += ovn/ovn-nb.xml
-CLEANFILES += ovn/ovn-nb.5
-man_MANS += ovn/ovn-nb.5
-ovn/ovn-nb.5: \
-   ovsdb/ovsdb-doc ovn/ovn-nb.xml ovn/ovn-nb.ovsschema $(OVN_NB_PIC)
-   $(AM_V_GEN)$(OVSDB_DOC) \
-   $(OVN_NB_DOT_DIAGRAM_ARG) \
-   --version=$(VERSION) \
-   $(srcdir)/ovn/ovn-nb.ovsschema \
-   $(srcdir)/ovn/ovn-nb.xml > $@.tmp && \
-   mv $@.tmp $@
-
-# Version checking for ovn-nb.ovsschema.
-ALL_LOCAL += ovn/ovn-nb.ovsschema.stamp
-ovn/ovn-nb.ovsschema.stamp: ovn/ovn-nb.ovsschema
-   $(srcdir)/build-aux/cksum-schema-check $? $@
-CLEANFILES += ovn/ovn-nb.ovsschema.stamp
-
-# Version checking for ovn-sb.ovsschema.
-ALL_LOCAL += ovn/ovn-sb.ovsschema.stamp
-ovn/ovn-sb.ovsschema.stamp: ovn/ovn-sb.ovsschema
-   $(srcdir)/build-aux/cksum-schema-check $? $@
-CLEANFILES += ovn/ovn-sb.ovsschema.stamp
+EXTRA_DIST += ovn/ovn-sb.ovsschema \
+ ovn/ovn-sb.xml \
+ ovn/ovn-nb.ovsschema \
+ ovn/ovn-nb.xml
 
 include ovn/lib/automake.mk
 include ovn/utilities/automake.mk
diff --git a/ovn/utilities/automake.mk b/ovn/utilities/automake.mk
index c2c3b7d5c..d2e2675c0 100644
--- a/ovn/utilities/automake.mk
+++ b/ovn/utilities/automake.mk
@@ -1,12 +1,6 @@
-man_MANS += \
-ovn/utilities/ovn-nbctl.8 \
-ovn/utilities/ovn-sbctl.8
-
-MAN_ROOTS += \
-ovn/utilities/ovn-sbctl.8.in
-
 EXTRA_DIST += \
-ovn/utilities/ovn-nbctl.8.xml
+ovn/utilities/ovn-nbctl.8.xml \
+ovn/utilities/ovn-sbctl.8.in
 
 CLEANFILES += \
 ovn/utilities/ovn-nbctl.8 \
-- 
2.21.0

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

Re: [ovs-dev] [PATCH v1] netdev-dpdk: Fix flow control configuration.

2019-09-10 Thread Ilya Maximets 
On 09.09.2019 14:39, Tomasz Konieczny wrote:
> Currently OVS is unable to change flow control configuration in DPDK
> because new settings are being overwritten by current settings
> with rte_eth_dev_flow_ctrl_get(). The fix restores correct order
> of operations and at the same time does not trigger error on devices
> without flow control support when flow control not requested.
> 
> Fixes: d95a8d2b0bd6 ("netdev-dpdk: Fix flow control configuration.")
> Signed-off-by: Tomasz Konieczny 
> ---

Hi Tomasz,
Thanks for the fix!

I agree that current code in master doesn't make any sense. :)
It seems that no-one uses this functionality since it's broken
for more than a year already.

Fixes tag should point to the commit from master branch, so it
should be:
Fixes: 7e1de65e8dfb ("netdev-dpdk: Fix failure to configure flow control at 
netdev-init.")

Regarding the fix itself:
Can we just move following two lines:

dev->fc_conf.mode = fc_mode;
dev->fc_conf.autoneg = autoneg;

below the rte_eth_dev_flow_ctrl_get() ?
Current version of the patch will re-setup flow control on each call
if it is not in initial state.

Best regards, Ilya Maximets.

>  lib/netdev-dpdk.c | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index bc20d68..c02dea1 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1856,17 +1856,19 @@ netdev_dpdk_set_config(struct netdev *netdev, const 
> struct smap *args,
>  rx_fc_en = smap_get_bool(args, "rx-flow-ctrl", false);
>  tx_fc_en = smap_get_bool(args, "tx-flow-ctrl", false);
>  autoneg = smap_get_bool(args, "flow-ctrl-autoneg", false);
> -
>  fc_mode = fc_mode_set[tx_fc_en][rx_fc_en];
> -if (dev->fc_conf.mode != fc_mode || autoneg != dev->fc_conf.autoneg) {
> -dev->fc_conf.mode = fc_mode;
> -dev->fc_conf.autoneg = autoneg;
> +
> +if (fc_mode != RTE_FC_NONE || autoneg != false) {
>  /* Get the Flow control configuration for DPDK-ETH */
>  err = rte_eth_dev_flow_ctrl_get(dev->port_id, >fc_conf);
>  if (err) {
>  VLOG_WARN("Cannot get flow control parameters on port "
>  DPDK_PORT_ID_FMT", err=%d", dev->port_id, err);
>  }
> +}
> +if (dev->fc_conf.mode != fc_mode || autoneg != dev->fc_conf.autoneg) {
> +dev->fc_conf.mode = fc_mode;
> +dev->fc_conf.autoneg = autoneg;
>  dpdk_eth_flow_ctrl_setup(dev);
>  }
>  
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Toto je moje poslední vɑrování Dev!

2019-09-10 Thread Aʼnonymní Hʌcker 
POSLEDNÍ VAROVÁNÍ d...@openvswitch.org!

Máte konečnou příležitost zachránit svůj
společenský život - já si srandu !!

Dávám vám posledních 72 hodin, abych provedl platbu,
než pošlu vίdeo s vaší ʍasturbací všem svým
přátelům a spolupracovníkům.


Naposledy jste navštívili Ƿornografickou stránku s
mladými teenagery, stáhli jste a automaticky nainstalovali
Şpyware, který jsem vytvořil.

Můj program zapnul fotoaparát a zaznamenal akt vaší
ʍasturbace a vίdeo, které jste sledovali při ʍasturbování.
Můj software také stáhl seznam e-mailových kontaktů
a seznam vašich přátel na Facebooku ze zařízení.

Mám oba- Dev.mp4 - s vaší ʍasturbací
a soubor se všemi kontakty na pevném disku.

Jste velmi zvrhlí!


Pokud chcete, abych odstranil oba soubory a uchoval vaše
tajemství, musíte mi poslat platbu Bitcoinem.
Dávám vám posledních 72 hodin na převod prostředků.

Pokud nevíte, jak s Bitcoinem platit, navštivte
Google a hledejte - jak nakupovat bitcoin.


Okamžitě odešlete na tuto
adresu Bitcoin 50.000 CZK = 0.2080321 BTC:

3KyCFSHTBMYNo7eqZFyoZu3jKg3qqn2Yqc

(zkopírujte a vložte)


1 BTC = 240.120 CZK právě teď, takže odešlete přesně 0.2080321 BTC
na výše uvedenou adresu.


Nesnaž se mě podvádět!
Jakmile otevřete tento e-mail, budu vědět, že jste jej otevřeli.

Tato adresa Bitcoinu je propojena pouze s vámi, takže budu
vědět, zda jste odeslali správnou částku.
Když zaplatíte v plné výši, odstraním oba
soubory a deaktivuji software.

Pokud platbu neposíláte, pošlu vaše vίdeo s ʍasturbací všem
přátelům a spolupracovníkům ze seznamu kontaktů,
který jsem si stáhnul.


Zde jsou opět platební údaje:



Odeslat:

0.2080321 BTC

této adrese Bitcoin:

3KyCFSHTBMYNo7eqZFyoZu3jKg3qqn2Yqc

(zkopírujte a vložte)



Můžete navštívit policii, ale nikdo vám nepomůže.
Vím, co dělám.
Nežiji ve vaší zemi a vím, jak zůstat anonymní.

Nesnažte se oklamat mě - budu vědět okamžitě - můj Şpysoft
nahrává všechny webové stránky, které navštívíte,
a všechny klávesy, které stisknete.
Pokud ano - pošlu tento ošklivý záznam všem,
koho znáte, včetně vaší rodiny!

Nepodváděj mě! Nezapomeňte na hanbu a pokud
tuto zprávu ignorujete, váš život bude zničen.

Čekám na vaši platbu Bitcoinem.

Aʼnonymní Hʌcker


P.S. Pokud potřebujete více času na nákup a odeslání BTC,
otevřete svůj poznámkový blok a napište - 48H ++ - a uložte.
Tímto způsobem mě můžete kontaktovat.
Uvažuji o tom, že vám ještě 48 hodin před odesláním vίdea
vašim kontaktům, ale pouze tehdy, když vίdím,
že se opravdu snažíte koupit bitcoin.



(
Jsme anonymní.
My neodpouštíme. Nezapomínáme.
Očekávejte nás.
))

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

[ovs-dev] Решаем вопросы выгодной закупки и доставки из Китая

2019-09-10 Thread акулина via dev 
Здравствуйте!
Перейдем сразу к сути, чтобы не занимать Ваше время.
Доставим Ваш товар из Китая в максимально сжатые сроки.
По необходимости, поможем с закупкой, проверим качество товара перед отправкой, 
проконтролируем безопасность доставки.
Заинтересовало?
Напишите нам!
Email: stiloszavi1...@mail.ru 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] Virtio Crypto with OVS-DPDK

2019-09-10 Thread Ilya Maximets 
> Hi All,
> 
> I am working on Virtio Crypto and could run DPDK virtio crypto use cases on 
> both x86 and ARM.
> 
> Now ,  I am trying to integrate virtio crypto with ovs-dpdk , is there any 
> work done in this direction . Could someone please help with inputs.

Hi Harish,
Why do you want to integrate virtio-crypto into OVS?
OVS is an OpenFlow Network Switch, but crypto devices are not network devices.
They will not fit in OVS purposes nor OVS architecture.

Best regards, Ilya Maximets.

> 
> Below is the DPDK sample application we tried.
> 
> https://doc.dpdk.org/guides/sample_app_ug/vhost_crypto.html
> 
> https://spp-tmp.readthedocs.io/en/stable/cryptodevs/virtio.html
> 
> 
> Regards,
> Harish
> Software R
> Hyderabad Design Center (HDC), NXP India
> Mob:9885996745 work: 91-4033504056
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] ovn: Fix broken dist* targets.

2019-09-10 Thread Ilya Maximets 
Redundant line continuation was missed while removing OVN
that broke distdir target and subsequently all the dependent
targets, e.g. distcheck:

  make[1]:
  *** No rule to make target 'nodist_ovn_lib_libovn_la_SOURCES',
  needed by 'distdir'.  Stop.

CC: Mark Michelson 
Fixes: f3e24610ea18 ("Remove OVN.")
Signed-off-by: Ilya Maximets 
---
 ovn/lib/automake.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ovn/lib/automake.mk b/ovn/lib/automake.mk
index c74430005..53a10e211 100644
--- a/ovn/lib/automake.mk
+++ b/ovn/lib/automake.mk
@@ -7,7 +7,7 @@ ovn_lib_libovn_la_SOURCES = \
ovn/lib/acl-log.c \
ovn/lib/acl-log.h \
ovn/lib/ovn-util.c \
-   ovn/lib/ovn-util.h \
+   ovn/lib/ovn-util.h
 nodist_ovn_lib_libovn_la_SOURCES = \
ovn/lib/ovn-nb-idl.c \
ovn/lib/ovn-nb-idl.h \
-- 
2.17.1

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

Re: [ovs-dev] [PATCH ovn v2 1/2] Add ovn-appctl utility

2019-09-10 Thread 0-day Robot 
Bleep bloop.  Greetings Numan Siddique, 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 lacks whitespace around operator
WARNING: Line lacks whitespace around operator
#555 FILE: utilities/ovn-appctl.c:90:
  -t, --target=TARGET  pidfile or socket to contact\n\

WARNING: Line lacks whitespace around operator
#557 FILE: utilities/ovn-appctl.c:92:
  list-commands  List commands supported by the target\n\

WARNING: Line lacks whitespace around operator
#559 FILE: utilities/ovn-appctl.c:94:
  vlog/list  List current logging levels\n\

WARNING: Line lacks whitespace around operator
WARNING: Line lacks whitespace around operator
#560 FILE: utilities/ovn-appctl.c:95:
  vlog/list-pattern  List logging patterns for each destination.\n\

WARNING: Line lacks whitespace around operator
#561 FILE: utilities/ovn-appctl.c:96:
  vlog/set [SPEC]\n\

WARNING: Line lacks whitespace around operator
#566 FILE: utilities/ovn-appctl.c:101:
  vlog/reopenMake the program reopen its log file\n\

WARNING: Line lacks whitespace around operator
WARNING: Line lacks whitespace around operator
#568 FILE: utilities/ovn-appctl.c:103:
  --timeout=SECS wait at most SECS seconds for a response\n\

WARNING: Line lacks whitespace around operator
#570 FILE: utilities/ovn-appctl.c:105:
  -V, --version  Display ovn-appctl version information\n",

WARNING: Line is 113 characters long (recommended limit is 79)
#731 FILE: utilities/ovn-ctl:68:
ovn-appctl -t $OVN_RUNDIR/ovnnb_db.ctl 
ovsdb-server/set-active-ovsdb-server `cat $ovnnb_active_conf_file`

WARNING: Line is 87 characters long (recommended limit is 79)
#732 FILE: utilities/ovn-ctl:69:
ovn-appctl -t $OVN_RUNDIR/ovnnb_db.ctl 
ovsdb-server/connect-active-ovsdb-server

WARNING: Line is 113 characters long (recommended limit is 79)
#742 FILE: utilities/ovn-ctl:82:
ovn-appctl -t $OVN_RUNDIR/ovnsb_db.ctl 
ovsdb-server/set-active-ovsdb-server `cat $ovnsb_active_conf_file`

WARNING: Line is 87 characters long (recommended limit is 79)
#743 FILE: utilities/ovn-ctl:83:
ovn-appctl -t $OVN_RUNDIR/ovnsb_db.ctl 
ovsdb-server/connect-active-ovsdb-server

WARNING: Line is 86 characters long (recommended limit is 79)
#752 FILE: utilities/ovn-ctl:92:
ovn-appctl -t $OVN_RUNDIR/ovnnb_db.ctl 
ovsdb-server/disconnect-active-ovsdb-server

WARNING: Line is 86 characters long (recommended limit is 79)
#758 FILE: utilities/ovn-ctl:97:
ovn-appctl -t $OVN_RUNDIR/ovnsb_db.ctl 
ovsdb-server/disconnect-active-ovsdb-server

WARNING: Line is 98 characters long (recommended limit is 79)
#767 FILE: utilities/ovn-ctl:277:
ovn-appctl -t $OVN_RUNDIR/ovn${1}_db.ctl ovsdb-server/sync-status | awk 
'{if(NR==1) print $2}'

Lines checked: 773, Warnings: 18, 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 v4 ovn] Replace chassis mac with router port mac on destination chassis

2019-09-10 Thread Numan Siddique 
On Mon, Sep 9, 2019 at 1:58 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,

Thanks for v4.

The below test case  is failing

## --- ##
116: ovn -- 2 HVs, 2 lports/HV, localnet ports, DVR N-S Ping FAILED (
ovn.at:15773)

## - ##
## Test results. ##
## - ##

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, 243 insertions(+), 12 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 *

Re: [ovs-dev] [PATCH ovn 1/2] Add ovn-appctl utility

2019-09-10 Thread Numan Siddique 
On Wed, Sep 4, 2019 at 11:21 PM Mark Michelson  wrote:

> On 9/2/19 1:09 PM, nusid...@redhat.com wrote:
> > From: Numan Siddique 
> >
> > Now that OVN has it's own rundir, "ovs-appctl -t
> ovn-controller/ovn-northd"
> > doesn't work. To fix this, ovn-appctl utility is added which
> > looks for the OVN pid/ctl files in the ovn rundir.
> >
> > The code is taken from ovs-appctl.c and modified to use ovn_rundir()
> > instead of ovs_rundir().
> >
> > Signed-off-by: Numan Siddique 
> > ---
> >   rhel/ovn-fedora.spec.in|   2 +
> >   utilities/.gitignore   |   2 +
> >   utilities/automake.mk  |  13 +-
> >   utilities/ovn-appctl.8.xml | 352 +
> >   utilities/ovn-appctl.c | 239 +
> >   utilities/ovn-ctl  |  18 +-
> >   6 files changed, 615 insertions(+), 11 deletions(-)
> >   create mode 100644 utilities/ovn-appctl.8.xml
> >   create mode 100644 utilities/ovn-appctl.c
> >
> > diff --git a/rhel/ovn-fedora.spec.in b/rhel/ovn-fedora.spec.in
> > index 14035de9a..9ee807fab 100644
> > --- a/rhel/ovn-fedora.spec.in
> > +++ b/rhel/ovn-fedora.spec.in
> > @@ -430,6 +430,7 @@ fi
> >   %{_bindir}/ovn-sbctl
> >   %{_bindir}/ovn-trace
> >   %{_bindir}/ovn-detrace
> > +%{_bindir}/ovn-appctl
> >   %{_datadir}/ovn/scripts/ovn-ctl
> >   %{_datadir}/ovn/scripts/ovn-lib
> >   %{_datadir}/ovn/scripts/ovndb-servers.ocf
> > @@ -440,6 +441,7 @@ fi
> >   %{_mandir}/man8/ovn-nbctl.8*
> >   %{_mandir}/man8/ovn-trace.8*
> >   %{_mandir}/man1/ovn-detrace.1*
> > +%{_mandir}/man8/ovn-appctl.8*
> >   #%{_mandir}/man7/ovn-architecture.7* - Uncomment this once the manpage
> is fixed
> >   %{_mandir}/man8/ovn-sbctl.8*
> >   #%{_mandir}/man5/ovn-nb.5* - Uncomment this once the manpage is fixed
> > diff --git a/utilities/.gitignore b/utilities/.gitignore
> > index 1d01e0b28..b319e8366 100644
> > --- a/utilities/.gitignore
> > +++ b/utilities/.gitignore
> > @@ -3,6 +3,8 @@
> >   /ovn-nbctl.8
> >   /ovn-sbctl
> >   /ovn-sbctl.8
> > +/ovn-appctl
> > +/ovn-appctl.8
> >   /ovn-trace
> >   /ovn-trace.8
> >   /ovn-detrace
> > diff --git a/utilities/automake.mk b/utilities/automake.mk
> > index 21dd8ccdf..ab0f6003a 100644
> > --- a/utilities/automake.mk
> > +++ b/utilities/automake.mk
> > @@ -8,7 +8,8 @@ man_MANS += \
> >   utilities/ovn-nbctl.8 \
> >   utilities/ovn-sbctl.8 \
> >   utilities/ovn-trace.8 \
> > -utilities/ovn-detrace.1
> > +utilities/ovn-detrace.1 \
> > +utilities/ovn-appctl.8
> >
> >   MAN_ROOTS += \
> >   utilities/ovn-sbctl.8.in \
> > @@ -27,6 +28,7 @@ EXTRA_DIST += \
> >   utilities/ovn-docker-overlay-driver.in \
> >   utilities/ovn-docker-underlay-driver.in \
> >   utilities/ovn-nbctl.8.xml \
> > +utilities/ovn-appctl.8.xml \
> >   utilities/ovn-trace.8.xml \
> >   utilities/ovn-detrace.in \
> >   utilities/ovndb-servers.ocf \
> > @@ -49,7 +51,9 @@ CLEANFILES += \
> >   utilities/ovn-sbctl.8 \
> >   utilities/ovn-trace.8 \
> >   utilities/ovn-detrace.1 \
> > -utilities/ovn-detrace
> > +utilities/ovn-detrace \
> > +utilities/ovn-appctl.8 \
> > +utilities/ovn-appctl
> >
> >   utilities/ovn-lib: $(top_builddir)/config.status
> >
> > @@ -68,4 +72,9 @@ bin_PROGRAMS += utilities/ovn-trace
> >   utilities_ovn_trace_SOURCES = utilities/ovn-trace.c
> >   utilities_ovn_trace_LDADD = lib/libovn.la $(OVSDB_LIBDIR)/libovsdb.la
> $(OVS_LIBDIR)/libopenvswitch.la
> >
> > +# ovn-nbctl
> > +bin_PROGRAMS += utilities/ovn-appctl
> > +utilities_ovn_appctl_SOURCES = utilities/ovn-appctl.c
> > +utilities_ovn_appctl_LDADD = lib/libovn.la $(OVSDB_LIBDIR)/libovsdb.la
> $(OVS_LIBDIR)/libopenvswitch.la
> > +
> >   include utilities/bugtool/automake.mk
> > diff --git a/utilities/ovn-appctl.8.xml b/utilities/ovn-appctl.8.xml
> > new file mode 100644
> > index 0..32a42a766
> > --- /dev/null
> > +++ b/utilities/ovn-appctl.8.xml
> > @@ -0,0 +1,352 @@
> > +
> > +
> > +Name
> > +ovn-appctl -- utility for configuring running OVN daemons
> > +
> > +Synopsis
> > +
> > +   ovn-appctl [--target=target | -t target]
> > +  [-T secs | --timeout=secs] command [arg...]
> > +
> > +ovn-appctl --help 
> > +ovn-appctl --version 
> > +
> > +Description
> > +
> > +  OVN daemons accept certain commands at runtime to control their
> behavior
> > +  and query their settings. Every daemon accepts a common set of
> commands
> > +  documented under COMMON COMMANDS below. Some daemons support
> additional
> > +  commands documented in their own manpages.
> > +
> > +
> > +
> > +   The ovn-appctl program provides a simple way to
> invoke
> > +   these commands. The command to be sent is specified on
> > +   ovn-appctl's command line as non-option arguments.
> > +   ovn-appctl sends the command and prints the daemon's
> > +   response on standard output.
> > +
> > +
> > +
> > +  ovn-ctl is exactly similar to Open vSwitch
> > +

[ovs-dev] [PATCH ovn v2 2/2] Generate documentation and manpages for ovn-archicture and ovn-nb/ovn-sb

2019-09-10 Thread nusiddiq 
From: Numan Siddique 

This was missing when OVN was split from OVS.

Signed-off-by: Numan Siddique 
---
 Makefile.am | 17 +
 TODO_SPLIT.rst  |  6 ---
 automake.mk | 84 +
 ovn-nb.xml  |  2 +-
 rhel/ovn-fedora.spec.in |  6 +--
 5 files changed, 89 insertions(+), 26 deletions(-)
 create mode 100644 automake.mk

diff --git a/Makefile.am b/Makefile.am
index f3df733a1..97dc309e3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -488,22 +488,7 @@ dist-docs:
 .PHONY: dist-docs
 
 
-# Version checking for ovn-nb.ovsschema.
-ALL_LOCAL += ovn-nb.ovsschema.stamp
-ovn-nb.ovsschema.stamp: ovn-nb.ovsschema
-   $(srcdir)/build-aux/cksum-schema-check $? $@
-CLEANFILES += ovn-nb.ovsschema.stamp
-
-# Version checking for ovn-sb.ovsschema.
-ALL_LOCAL += ovn-sb.ovsschema.stamp
-ovn-sb.ovsschema.stamp: ovn-sb.ovsschema
-   $(srcdir)/build-aux/cksum-schema-check $? $@
-
-pkgdata_DATA += ovn-nb.ovsschema
-pkgdata_DATA += ovn-sb.ovsschema
-
-CLEANFILES += ovn-sb.ovsschema.stamp
-
+include automake.mk
 include Documentation/automake.mk
 include m4/automake.mk
 include lib/automake.mk
diff --git a/TODO_SPLIT.rst b/TODO_SPLIT.rst
index bb8657eb1..933534084 100644
--- a/TODO_SPLIT.rst
+++ b/TODO_SPLIT.rst
@@ -35,12 +35,6 @@ Immediate tasks
 * Someone with a decent ability to write should give the README.rst file some
   polish (or even just rewrite it. I won't be offended).
 
-* After the split, the below things are missing during compilation
- - OVN northbound/southbound E-R diagram
- - OVN northbound/southbound schema documentation
- - ovn-architecture manpage generation.
-  This needs to be fixed.
-
 * Cleanup the acinclude.m4 and m4 folder
 
 Immediate to Short-term tasks
diff --git a/automake.mk b/automake.mk
new file mode 100644
index 0..ad801f1e5
--- /dev/null
+++ b/automake.mk
@@ -0,0 +1,84 @@
+man_MANS += ovn-architecture.7
+EXTRA_DIST += ovn-architecture.7.xml
+CLEANFILES += ovn-architecture.7
+
+# OVN northbound E-R diagram
+#
+# If "python" or "dot" is not available, then we do not add graphical diagram
+# to the documentation.
+if HAVE_PYTHON
+if HAVE_DOT
+OVSDB_DOT = $(run_python) ${OVSDIR}/ovsdb/ovsdb-dot.in
+ovn-nb.gv: ${OVSDIR}/ovsdb/ovsdb-dot.in $(srcdir)/ovn-nb.ovsschema
+   $(AM_V_GEN)$(OVSDB_DOT) --no-arrows $(srcdir)/ovn-nb.ovsschema > $@
+ovn-nb.pic: ovn-nb.gv ${OVSDIR}/ovsdb/dot2pic
+   $(AM_V_GEN)(dot -T plain < ovn-nb.gv | $(PYTHON) 
${OVSDIR}/ovsdb/dot2pic -f 3) > $@.tmp && \
+   mv $@.tmp $@
+OVN_NB_PIC = ovn-nb.pic
+OVN_NB_DOT_DIAGRAM_ARG = --er-diagram=$(OVN_NB_PIC)
+CLEANFILES += ovn-nb.gv ovn-nb.pic
+endif
+endif
+
+# OVN northbound schema documentation
+EXTRA_DIST += ovn-nb.xml
+CLEANFILES += ovn-nb.5
+man_MANS += ovn-nb.5
+
+OVSDB_DOC = $(run_python) ${OVSDIR}/ovsdb/ovsdb-doc
+ovn-nb.5: \
+   ${OVSDIR}/ovsdb/ovsdb-doc $(srcdir)/ovn-nb.xml 
$(srcdir)/ovn-nb.ovsschema $(OVN_NB_PIC)
+   $(AM_V_GEN)$(OVSDB_DOC) \
+   $(OVN_NB_DOT_DIAGRAM_ARG) \
+   --version=$(VERSION) \
+   $(srcdir)/ovn-nb.ovsschema \
+   $(srcdir)/ovn-nb.xml > $@.tmp && \
+   mv $@.tmp $@
+
+# OVN southbound E-R diagram
+#
+# If "python" or "dot" is not available, then we do not add graphical diagram
+# to the documentation.
+if HAVE_PYTHON
+if HAVE_DOT
+ovn-sb.gv: ${OVSDIR}/ovsdb/ovsdb-dot.in $(srcdir)/ovn-sb.ovsschema
+   $(AM_V_GEN)$(OVSDB_DOT) --no-arrows $(srcdir)/ovn-sb.ovsschema > $@
+ovn-sb.pic: ovn-sb.gv ${OVSDIR}/ovsdb/dot2pic
+   $(AM_V_GEN)(dot -T plain < ovn-sb.gv | $(PYTHON) 
${OVSDIR}/ovsdb/dot2pic -f 3) > $@.tmp && \
+   mv $@.tmp $@
+OVN_SB_PIC = ovn-sb.pic
+OVN_SB_DOT_DIAGRAM_ARG = --er-diagram=$(OVN_SB_PIC)
+CLEANFILES += ovn-sb.gv ovn-sb.pic
+endif
+endif
+
+# OVN southbound schema documentation
+EXTRA_DIST += ovn-sb.xml
+CLEANFILES += ovn-sb.5
+man_MANS += ovn-sb.5
+
+ovn-sb.5: \
+   ${OVSDIR}/ovsdb/ovsdb-doc $(srcdir)/ovn-sb.xml 
$(srcdir)/ovn-sb.ovsschema $(OVN_SB_PIC)
+   $(AM_V_GEN)$(OVSDB_DOC) \
+   $(OVN_SB_DOT_DIAGRAM_ARG) \
+   --version=$(VERSION) \
+   $(srcdir)/ovn-sb.ovsschema \
+   $(srcdir)/ovn-sb.xml > $@.tmp && \
+   mv $@.tmp $@
+
+
+# Version checking for ovn-nb.ovsschema.
+ALL_LOCAL += ovn-nb.ovsschema.stamp
+ovn-nb.ovsschema.stamp: ovn-nb.ovsschema
+   $(srcdir)/build-aux/cksum-schema-check $? $@
+CLEANFILES += ovn-nb.ovsschema.stamp
+
+# Version checking for ovn-sb.ovsschema.
+ALL_LOCAL += ovn-sb.ovsschema.stamp
+ovn-sb.ovsschema.stamp: ovn-sb.ovsschema
+   $(srcdir)/build-aux/cksum-schema-check $? $@
+
+pkgdata_DATA += ovn-nb.ovsschema
+pkgdata_DATA += ovn-sb.ovsschema
+
+CLEANFILES += ovn-sb.ovsschema.stamp
diff --git a/ovn-nb.xml b/ovn-nb.xml
index 442e5cb60..b41b57906 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -1535,7 +1535,7 @@
   address.
 
   
-  
+  
 
   Enables/disables IP

[ovs-dev] [PATCH ovn v2 1/2] Add ovn-appctl utility

2019-09-10 Thread nusiddiq 
From: Numan Siddique 

Now that OVN has it's own rundir, "ovs-appctl -t ovn-controller/ovn-northd"
doesn't work. To fix this, ovn-appctl utility is added which
looks for the OVN pid/ctl files in the ovn rundir.

The code is taken from ovs-appctl.c and modified to use ovn_rundir()
instead of ovs_rundir().

Signed-off-by: Numan Siddique 
---
 rhel/ovn-fedora.spec.in|   2 +
 utilities/.gitignore   |   2 +
 utilities/automake.mk  |  13 +-
 utilities/ovn-appctl.8.xml | 352 +
 utilities/ovn-appctl.c | 239 +
 utilities/ovn-ctl  |  18 +-
 6 files changed, 615 insertions(+), 11 deletions(-)
 create mode 100644 utilities/ovn-appctl.8.xml
 create mode 100644 utilities/ovn-appctl.c

diff --git a/rhel/ovn-fedora.spec.in b/rhel/ovn-fedora.spec.in
index 14035de9a..9ee807fab 100644
--- a/rhel/ovn-fedora.spec.in
+++ b/rhel/ovn-fedora.spec.in
@@ -430,6 +430,7 @@ fi
 %{_bindir}/ovn-sbctl
 %{_bindir}/ovn-trace
 %{_bindir}/ovn-detrace
+%{_bindir}/ovn-appctl
 %{_datadir}/ovn/scripts/ovn-ctl
 %{_datadir}/ovn/scripts/ovn-lib
 %{_datadir}/ovn/scripts/ovndb-servers.ocf
@@ -440,6 +441,7 @@ fi
 %{_mandir}/man8/ovn-nbctl.8*
 %{_mandir}/man8/ovn-trace.8*
 %{_mandir}/man1/ovn-detrace.1*
+%{_mandir}/man8/ovn-appctl.8*
 #%{_mandir}/man7/ovn-architecture.7* - Uncomment this once the manpage is fixed
 %{_mandir}/man8/ovn-sbctl.8*
 #%{_mandir}/man5/ovn-nb.5* - Uncomment this once the manpage is fixed
diff --git a/utilities/.gitignore b/utilities/.gitignore
index 1d01e0b28..b319e8366 100644
--- a/utilities/.gitignore
+++ b/utilities/.gitignore
@@ -3,6 +3,8 @@
 /ovn-nbctl.8
 /ovn-sbctl
 /ovn-sbctl.8
+/ovn-appctl
+/ovn-appctl.8
 /ovn-trace
 /ovn-trace.8
 /ovn-detrace
diff --git a/utilities/automake.mk b/utilities/automake.mk
index 21dd8ccdf..ab0f6003a 100644
--- a/utilities/automake.mk
+++ b/utilities/automake.mk
@@ -8,7 +8,8 @@ man_MANS += \
 utilities/ovn-nbctl.8 \
 utilities/ovn-sbctl.8 \
 utilities/ovn-trace.8 \
-utilities/ovn-detrace.1
+utilities/ovn-detrace.1 \
+utilities/ovn-appctl.8
 
 MAN_ROOTS += \
 utilities/ovn-sbctl.8.in \
@@ -27,6 +28,7 @@ EXTRA_DIST += \
 utilities/ovn-docker-overlay-driver.in \
 utilities/ovn-docker-underlay-driver.in \
 utilities/ovn-nbctl.8.xml \
+utilities/ovn-appctl.8.xml \
 utilities/ovn-trace.8.xml \
 utilities/ovn-detrace.in \
 utilities/ovndb-servers.ocf \
@@ -49,7 +51,9 @@ CLEANFILES += \
 utilities/ovn-sbctl.8 \
 utilities/ovn-trace.8 \
 utilities/ovn-detrace.1 \
-utilities/ovn-detrace
+utilities/ovn-detrace \
+utilities/ovn-appctl.8 \
+utilities/ovn-appctl
 
 utilities/ovn-lib: $(top_builddir)/config.status
 
@@ -68,4 +72,9 @@ bin_PROGRAMS += utilities/ovn-trace
 utilities_ovn_trace_SOURCES = utilities/ovn-trace.c
 utilities_ovn_trace_LDADD = lib/libovn.la $(OVSDB_LIBDIR)/libovsdb.la 
$(OVS_LIBDIR)/libopenvswitch.la
 
+# ovn-nbctl
+bin_PROGRAMS += utilities/ovn-appctl
+utilities_ovn_appctl_SOURCES = utilities/ovn-appctl.c
+utilities_ovn_appctl_LDADD = lib/libovn.la $(OVSDB_LIBDIR)/libovsdb.la 
$(OVS_LIBDIR)/libopenvswitch.la
+
 include utilities/bugtool/automake.mk
diff --git a/utilities/ovn-appctl.8.xml b/utilities/ovn-appctl.8.xml
new file mode 100644
index 0..32a42a766
--- /dev/null
+++ b/utilities/ovn-appctl.8.xml
@@ -0,0 +1,352 @@
+
+
+Name
+ovn-appctl -- utility for configuring running OVN daemons
+
+Synopsis
+
+   ovn-appctl [--target=target | -t target]
+  [-T secs | --timeout=secs] command [arg...]
+
+ovn-appctl --help 
+ovn-appctl --version 
+
+Description
+
+  OVN daemons accept certain commands at runtime to control their behavior
+  and query their settings. Every daemon accepts a common set of commands
+  documented under COMMON COMMANDS below. Some daemons support additional
+  commands documented in their own manpages.
+
+
+
+   The ovn-appctl program provides a simple way to invoke
+   these commands. The command to be sent is specified on
+   ovn-appctl's command line as non-option arguments.
+   ovn-appctl sends the command and prints the daemon's
+   response on standard output.
+
+
+
+  ovn-ctl is exactly similar to Open vSwitch
+  ovs-appctl utility.
+
+
+Command Commands
+
+  Every OVN daemon supports a common set of commands, which are documented
+  in this section.
+
+
+General Commands
+
+  These commands display daemon-specific commands and the running version.
+  Note that these commands are different from the --help and --version
+  options that return information about the ovn-appctl
+  utility itself.
+
+
+
+  list-commands
+  
+Lists the commands supported by the target.
+  
+
+  version
+  
+Displays the version and compilation date of the target.
+  
+
+
+Logging Commands
+
+  OVN

Re: [ovs-dev] [RFC v2] Document process for compatibility between OVS and OVN.

2019-09-10 Thread Numan Siddique 
Hi Mark,

Few comments below.


On Sat, Sep 7, 2019 at 2:38 AM Mark Michelson  wrote:

> This document serves to provide an explanation for how OVN will remain
> compatible with OVS. It provides instructions for OVN contributors for
> how to maintain compatibility even across older versions of OVS when
> possible.
>
> Note that the document currently makes reference to some non-existent
> items. For instance, it refers to an ovs-compat directory in the OVN
>

I think you need to update the commit message as "ovs-compat" directory
approach is removed in RFC v2.


> project. Also, it refers to some macros for testing the OVS version.
> These will be added in a future commit if the process documented here is
> approved.
>
> I'm submitting this as an RFC, since I imagine there are some details I
> have not thought about, and there will also be some great suggestions
> from others on this matter.
>
> Signed-off-by: Mark Michelson 
> ---
>  Documentation/automake.mk  |   1 +
>  Documentation/internals/contributing/index.rst |   1 +
>  .../contributing/ovs-ovn-compatibility.rst | 129
> +
>  3 files changed, 131 insertions(+)
>  create mode 100644
> Documentation/internals/contributing/ovs-ovn-compatibility.rst
>
> diff --git a/Documentation/automake.mk b/Documentation/automake.mk
> index f7e1d2628..d9bd4be1f 100644
> --- a/Documentation/automake.mk
> +++ b/Documentation/automake.mk
> @@ -56,6 +56,7 @@ DOC_SOURCE = \
> Documentation/internals/contributing/documentation-style.rst \
> Documentation/internals/contributing/libopenvswitch-abi.rst \
> Documentation/internals/contributing/submitting-patches.rst \
> +   Documentation/internals/contributing/ovs-ovn-compatibility.rst \
> Documentation/requirements.txt \
> $(addprefix Documentation/ref/,$(RST_MANPAGES)
> $(RST_MANPAGES_NOINST))
>  FLAKE8_PYFILES += Documentation/conf.py
> diff --git a/Documentation/internals/contributing/index.rst
> b/Documentation/internals/contributing/index.rst
> index a46cb046a..7ef57a1e2 100644
> --- a/Documentation/internals/contributing/index.rst
> +++ b/Documentation/internals/contributing/index.rst
> @@ -36,3 +36,4 @@ The below guides provide information on contributing to
> Open vSwitch itself.
> coding-style-windows
> documentation-style
> libopenvswitch-abi
> +   ovs-ovn-compatibility
> diff --git
> a/Documentation/internals/contributing/ovs-ovn-compatibility.rst
> b/Documentation/internals/contributing/ovs-ovn-compatibility.rst
> new file mode 100644
> index 0..d40b58c32
> --- /dev/null
> +++ b/Documentation/internals/contributing/ovs-ovn-compatibility.rst
> @@ -0,0 +1,129 @@
> +..
> +  Copyright (c) 2019 Nicira, Inc.
> +
> +  Licensed under the Apache License, Version 2.0 (the "License"); you
> may
> +  not use this file except in compliance with the License. You may
> obtain
> +  a copy of the License at
> +
> +  http://www.apache.org/licenses/LICENSE-2.0
> +
> +  Unless required by applicable law or agreed to in writing, software
> +  distributed under the License is distributed on an "AS IS" BASIS,
> WITHOUT
> +  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> See the
> +  License for the specific language governing permissions and
> limitations
> +  under the License.
> +
> +  Convention for heading levels in Open vSwitch documentation:
> +
> +  ===  Heading 0 (reserved for the title in a document)
> +  ---  Heading 1
> +  ~~~  Heading 2
> +  +++  Heading 3
> +  '''  Heading 4
> +
> +  Avoid deeper levels because they do not render well.
> +
> +===
> +Keeping OVN Compatible with OVS
> +===
> +
> +OVN has split from OVS. Prior to this split, there was no issue with
> +compatibility. All code changes happened at the same time and in the same
> repo.
> +New releases contained the latest OVS and OVN changes. But now these
> assumptions
> +are no longer valid, and so care must be taken to maintain compatibility.
> +
> +OVS and OVN versions
> +
> +
> +Prior to the split, the release schedule for OVN was bound to the release
> +schedule for OVS. OVS releases a new version approximately every 6
> months. OVN
> +has not yet determined a release schedule, but it is entirely possible
> that it
> +will be different from OVS. Eventually, this will lead to a situation
> where it
> +is very important that we publish which versions of OVN are compatible
> with
> +which versions of OVS. When incompatibilities are discovered, it is
> important to
> +ensure that these are clearly stated.
> +
> +The split of OVS and OVN happened in the run-up to the release of OVS
> 2.12. As a
> +result, all versions of OVN *must* be compiled against OVS version 2.12 or
> +later. Before going further into compatibility, let's explore the ways
> that OVN
>