[ovs-dev] [PATCH 2/3] ofproto: Remove duplicated includes

2020-07-09 Thread wangyunjian
From: Yunjian Wang 

Remove duplicated includes.

Signed-off-by: Yunjian Wang 
---
 ofproto/ofproto-dpif.h | 1 -
 ofproto/tunnel.c   | 2 --
 2 files changed, 3 deletions(-)

diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index 4e5ae0c9e..1f5794f03 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -54,7 +54,6 @@
 #include "ovs-thread.h"
 #include "ofproto-provider.h"
 #include "util.h"
-#include "ovs-thread.h"
 
 struct dpif_flow_stats;
 struct ofproto_async_msg;
diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
index 03f0ab765..3455ed233 100644
--- a/ofproto/tunnel.c
+++ b/ofproto/tunnel.c
@@ -13,8 +13,6 @@
  * limitations under the License. */
 
 #include 
-#include "tunnel.h"
-
 #include 
 
 #include "byte-order.h"
-- 
2.23.0


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


[ovs-dev] [PATCH 3/3] datapath: Remove duplicated includes

2020-07-09 Thread wangyunjian
From: Yunjian Wang 

Remove duplicated includes.

Signed-off-by: Yunjian Wang 
---
 datapath/linux/compat/lisp.c | 1 -
 datapath/vport-stt.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/datapath/linux/compat/lisp.c b/datapath/linux/compat/lisp.c
index 6dc066de8..49c60f4ed 100644
--- a/datapath/linux/compat/lisp.c
+++ b/datapath/linux/compat/lisp.c
@@ -38,7 +38,6 @@
 #include "datapath.h"
 #include "gso.h"
 #include "vport.h"
-#include "gso.h"
 #include "vport-netdev.h"
 
 #define LISP_UDP_PORT  4341
diff --git a/datapath/vport-stt.c b/datapath/vport-stt.c
index 35c4942c5..71bbeda63 100644
--- a/datapath/vport-stt.c
+++ b/datapath/vport-stt.c
@@ -23,7 +23,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #include "datapath.h"
 #include "vport.h"
-- 
2.23.0


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


[ovs-dev] [PATCH 0/3] Remove duplicated includes

2020-07-09 Thread wangyunjian
From: Yunjian Wang 

This series include three patches for removing duplicated includes.

Yunjian Wang (3):
  lib: Remove duplicated includes
  ofproto: Remove duplicated includes
  datapath: Remove duplicated includes

 datapath/linux/compat/lisp.c | 1 -
 datapath/vport-stt.c | 1 -
 lib/netdev-native-tnl.c  | 1 -
 lib/tnl-ports.c  | 1 -
 ofproto/ofproto-dpif.h   | 1 -
 ofproto/tunnel.c | 2 --
 6 files changed, 7 deletions(-)

-- 
2.23.0


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


[ovs-dev] [PATCH 1/3] lib: Remove duplicated includes

2020-07-09 Thread wangyunjian
From: Yunjian Wang 

Remove duplicated includes.

Signed-off-by: Yunjian Wang 
---
 lib/netdev-native-tnl.c | 1 -
 lib/tnl-ports.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
index 0acc87953..b89dfdd52 100644
--- a/lib/netdev-native-tnl.c
+++ b/lib/netdev-native-tnl.c
@@ -29,7 +29,6 @@
 #include 
 #include 
 
-#include 
 #include 
 #include 
 
diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c
index 446b40763..58269d3b1 100644
--- a/lib/tnl-ports.c
+++ b/lib/tnl-ports.c
@@ -30,7 +30,6 @@
 #include "openvswitch/ofpbuf.h"
 #include "ovs-thread.h"
 #include "odp-util.h"
-#include "ovs-thread.h"
 #include "unixctl.h"
 #include "util.h"
 
-- 
2.23.0


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


[ovs-dev] Taller de ortografía y redacción

2020-07-09 Thread Importante - Paquete de 3 Cursos
El entorno laboral actual requiere de profesionistas capaces de expresarse por 
escrito de forma clara, coherente y con
impecable ortografía. Este curso en línea está enfocado en abordar las 
principales reglas ortográficas del idioma
español y en otorgar recomendaciones a los participantes para una redacción 
correcta de los textos profesionales o
de negocios más comunes tanto tradicionales como digitales.

Paquete: Taller de ortografía y redacción para profesionistas y negocios. 
(Curso en línea en vivo)


Sáb 18/07/20   | 10 am a 12:30 pm | Taller de ortografía básica para 
profesionistas y negocios.
Sáb 25/07/20   | 10 am a 12:30 pm | Taller de redacción para profesionistas y 
negocios.
Sáb 01/08/20   | 10 am a 12:00 pm | Redacción de documentos y textos ejecutivos.


Incluye: Conexión al curso para una computadora o móvil, Material del curso, 
Reconocimiento, Interacción en directo para resolución de dudas.

Proporcionaremos a los participantes guías para una correcta redacción de 
textos que sean claros, concisos y entendibles
para el ámbito profesional.

¿Te interesa este curso?
Solicita información respondiendo a este correo con la palabra Redacción, junto 
con los siguientes datos: 

Nombre:
Teléfono:
Empresa:
Correo Alterno: 

¿Dudas sobre este Evento?

Centro de atención a clientes:
Llámanos al 5530167085 - 5530167085 6630



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


[ovs-dev] [PATCH v12] AB bonding: Add "primary" interface concept

2020-07-09 Thread Jeff Squyres via dev
In AB bonding, if the current active slave becomes disabled, a
replacement slave is arbitrarily picked from the remaining set of
enabled slaves.  This commit adds the concept of a "primary" slave: an
interface that will always be (or become) the current active slave if
it is enabled.

The rationale for this functionality is to allow the designation of a
preferred interface for a given bond.  For example:

1. Bond is created with interfaces p1 (primary) and p2, both enabled.
2. p1 becomes the current active slave (because it was designated as
   the primary).
3. Later, p1 fails/becomes disabled.
4. p2 is chosen to become the current active slave.
5. Later, p1 becomes re-enabled.
6. p1 is chosen to become the current active slave (because it was
   designated as the primary)

Note that p1 becomes the active slave once it becomes re-enabled, even
if nothing has happened to p2.

This "primary" concept exists in Linux kernel network interface
bonding, but did not previously exist in OVS bonding.

Only one primary slave inteface is supported per bond, and is only
supported for active/backup bonding.

The primary slave interface is designated via
"other_config:bond-primary" when creating a bond.

Also, while adding tests for the "primary" concept, make a few small
improvements to the non-primary AB bonding test.

Signed-off-by: Jeff Squyres 
Reviewed-by: Aaron Conole 
Tested-by: Greg Rose 
Acked-by: Greg Rose 
---
v12:
- Rebased to master HEAD (fa31efd21).
- Fixed case of resetting the bond primary.  Added a test to check
  that this case works.

v11:
- Rebased to master HEAD (4e432d6f8).
- Updated wording in NEWS bullet.

v10:
- Rebased to master HEAD (714a10499).
- Fixed display issue from code shuffling in v9.
- Fixed "bond_mode" in documentation.
- Added NEWS bullet.

v9:
- Removed stale code that should have been removed in v8.
- Fixed comment punctuation.

v8:
- Many style and simplification suggestions from Ilya Maximets
- Rename "primary:" to "active-backup primary:" in ovs-appctl
  bond/show output, and make it always appear (vs. only appearing in
  explicit AB bonding scenarios).
- Properly handle fallback-to-AB-bonding cases
- Use time/warp and OVS_WAIT_UNTIL in the bonding tests to prevent
  races, and a few other improvements to the test quality
- After rebasing patch to head of tree, adjust test output as result
  of changes from other commits.

v7:
- After rebasing patch to head of tree, adjust test output as result
  of changes from 81f71381f.

v6:
- Style: bleep bloop.  Fix use of {}.

v5:
- Handle when bond is reconfigured to remove "bond-primary" config.
- Fix potential flapping between remaining slaves.
- Add a test to re-add a removed primary interface and make sure the
  bond reflects that properly.
- Add a test to remove "bond-primary" config and make sure the bond
  reflects that properly.

v4:
- Style: bleep bloop.  Trim line length below 79 characters.

v3:
- Adjusted print logic for when the primary slave is not attached

v2:
- Added "ovs-vsctl bond/show" label when primary interface is no
  longer enslaved
- Fixed strcmp() usage nits
- Moved "other_config:bond-primary" docs to the "Bonding
  Configuration" group
- Expanded commit message
- Added more test cases (including one for when the primary interface
  is no longer enslaved)

 NEWS  |   2 +
 ofproto/bond.c|  54 +++--
 ofproto/bond.h|   1 +
 tests/lacp.at |   9 ++
 tests/ofproto-dpif.at | 251 +++---
 vswitchd/bridge.c |   5 +
 vswitchd/vswitch.xml  |   8 ++
 7 files changed, 308 insertions(+), 22 deletions(-)

diff --git a/NEWS b/NEWS
index e52e862e1..017608415 100644
--- a/NEWS
+++ b/NEWS
@@ -26,6 +26,8 @@ Post-v2.13.0
  * New configuration knob 'other_config:lb-output-action' for bond ports
that enables new datapath action 'lb_output' to avoid recirculation
in balance-tcp mode.  Disabled by default.
+ * New configuration knob 'other_config:bond-primary' for AB bonds
+   that specifies interface will be the preferred port if it is active.
- Tunnels: TC Flower offload
  * Tunnel Local endpoint address masked match are supported.
  * Tunnel Romte endpoint address masked match are supported.
diff --git a/ofproto/bond.c b/ofproto/bond.c
index 2466c4d02..4606e2a7d 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -89,6 +89,7 @@ struct bond_slave {
 /* Link status. */
 bool enabled;   /* May be chosen for flows? */
 bool may_enable;/* Client considers this slave bondable. */
+bool is_primary;/* This slave is preferred over others. */
 long long delay_expires;/* Time after which 'enabled' may change. */
 
 /* Rebalancing info.  Used only by bond_rebalance(). */
@@ -124,6 +125,7 @@ struct bond {
 uint32_t basis; /* Basis for flow hash function. */
 bool use_lb_output_action;  /* Use lb_output action to avoid recirculation.

Re: [ovs-dev] [PATCH v11] AB bonding: Add "primary" interface concept

2020-07-09 Thread Jeff Squyres (jsquyres) via dev
On Jul 8, 2020, at 11:24 PM, Flavio Leitner  wrote:
> 
> I think there is still an issue with this version where one could
> change the bond-primary after adding a bond port. See the sequence
> below:
> 
>  # ovs-vsctl add-bond br0 bond0 p1 p2 \
>bond_mode=active-backup \
>other_config:bond-primary=p1
> 
> Here the bond_slave_register() will set the is_primary correctly,
> but then one could do this:
> 
>  # ovs-vsctl set port bond0 other_config:bond-primary=p2
> 
> The above will not change the primary slave. Neither this:
> 
>  # ovs-vsctl remove port bond0 other_config bond-primary
> 
> Because the slaves are initialized, so they don't change.

Excellent catch, thank you!

I applied your fix and added a test for exactly this case.

v12 coming shortly.

-- 
Jeff Squyres
jsquy...@cisco.com

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


Re: [ovs-dev] [PATCH V6 00/12] netdev datapath offload: Support IPv6 and VXLAN encap

2020-07-09 Thread Ilya Maximets
On 7/8/20 7:22 PM, Ilya Maximets wrote:
> On 7/8/20 8:38 AM, Eli Britstein wrote:
>> This patch set includes enhanced logging to increase debugability, bug
>> fixes and additional offloads - IPv6 and VXLAN encap.
>>
>> Patches #1-#2:  Enhance log prints for debugability.
>> Patches #3-#4:  Bug fixes for partial offloads and Ethernet matching.
>> Patches #5-#12: Add support for offloads of IPv6 patterns, partial
>> TCP/UDP ports, set IPv6 and encap actions
>> (clone/output).
>>
>> v2-v1:
>> - Removed redundant out label.
>> - Added a patch to fix dl_type match only.
>> v3-v2:
>> - Rebased, and more elaboration in #7 commit message.
>> v4-v3:
>> - Rebased, changed order of commits.
>> - Testpmd format replaces pervious format.
>> - Some comments from Ilya/Harsha addressed.
>> v5-v4:
>> - Removed redundant trailing '\n' from log messages.
>> - Removed 'testpmd' keyword.
>> - 'DUMP_TESTPMD_ITEM' -> 'DUMP_PATTERN_ITEM'.
>> - Removed *_mask1 variables.
>> - Changed logging to dynamic string for tunnel dumps.
>> v6-v5:
>> - Not introducing/removing 'testpmd' keyword.
>> - Minor log format fixes.
>>
>> Travis:
>> v1: https://travis-ci.org/github/elibritstein/OVS/builds/688413350
>> v2: https://travis-ci.org/github/elibritstein/OVS/builds/691375847
>> v3: https://travis-ci.org/github/elibritstein/OVS/builds/700534550
>> v4: https://travis-ci.org/github/elibritstein/OVS/builds/703556030
>> v5: https://travis-ci.org/github/elibritstein/OVS/builds/705679636
>> v6: https://travis-ci.org/github/elibritstein/OVS/builds/706044892
>>
>> Eli Britstein (10):
>>   netdev-offload-dpdk: Log testpmd format for flow create/destroy
>>   dpif-netdev: Add mega ufid in flow add/del log
>>   dpif-netdev: Don't use zero flow mark
>>   netdev-offload-dpdk: Fix Ethernet matching for type only
>>   netdev-offload-dpdk: Support partial TCP/UDP port matching
>>   netdev-offload-dpdk: Remove pre-validate of patterns function
>>   netdev-offload-dpdk: Add IPv6 pattern matching
>>   netdev-offload-dpdk: Support offload of set IPv6 actions
>>   netdev-offload-dpdk: Support offload of clone tnl_push/output actions
>>   netdev-offload-dpdk: Support tnl/push using vxlan encap attribute
>>
>> Ilya Maximets (2):
>>   netdev: Allow storing dpif type into netdev structure.
>>   netdev-offload: Use dpif type instead of class.
>>
>>  Documentation/howto/dpdk.rst  |   4 +-
>>  NEWS  |   3 +
>>  lib/dpif-netdev.c |  36 +-
>>  lib/dpif-netlink.c|  23 +-
>>  lib/dpif.c|  21 +-
>>  lib/netdev-offload-dpdk.c | 883 
>> ++
>>  lib/netdev-offload-tc.c   |   3 +-
>>  lib/netdev-offload.c  |  52 ++-
>>  lib/netdev-offload.h  |  16 +-
>>  lib/netdev-provider.h |   3 +-
>>  lib/netdev.c  |  16 +
>>  lib/netdev.h  |   2 +
>>  ofproto/ofproto-dpif-upcall.c |   5 +-
>>  tests/dpif-netdev.at  |  20 +-
>>  tests/ofproto-macros.at   |   3 +-
>>  15 files changed, 680 insertions(+), 410 deletions(-)
>>
> 
> Thanks!
> 
> I fixed the NEWS line in patch #8 as "IPv6 TCP/UDP ports" doesn't sound
> correct to me and also stripped line breaks from VLOG in patch #2.
> With that, applied to master branch.

I also backported patches #3 and #4 down to 2.10.

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


Re: [ovs-dev] [PATCH RESEND ovs-dev v1 0/2] dpif-netdev: avoid ovs-vswitchd crash

2020-07-09 Thread Ilya Maximets
On 6/30/20 5:36 AM, Tonghao Zhang wrote:
> On Fri, Jun 12, 2020 at 9:39 AM Tonghao Zhang  
> wrote:
>>
>> On Tue, Jun 9, 2020 at 8:54 AM  wrote:
>>>
>>> From: Tonghao Zhang 
>>>
>>> This patchset add more robust error handling.
>>> Tested-at: 
>>> https://travis-ci.com/github/ovn-open-virtual-networks/ovs/builds/170300796
>> ping
>>> Tonghao Zhang (2):
>>>   dpif-netdev: Add check mark to avoid ovs-vswitchd crash.
>>>   dpif-netdev: Return error code when no mark available.
>>>
>>>  lib/dpif-netdev.c | 8 
>>>  1 file changed, 8 insertions(+)
>>>
>>> --
>>> 2.26.1
>>>
>>
>>
>> --
>> Best regards, Tonghao
> Hi Ben, Ilya
> any comments about that bugfix ?

Hi.  Sorry for delay and thanks for fixes!

I changed the return value in the first patch from -1 to EINVAL just in case we
will have a more accurate error checking in the future.  And I also added missed
'Fixes' tags.

With that, applied to master and backported down to 2.10.

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


Re: [ovs-dev] [PATCH v2 ovn 2/2] ovn-northd: Fix logical flows to limit ARP/NS broadcast domain.

2020-07-09 Thread Dumitru Ceara
On 7/9/20 8:15 PM, Numan Siddique wrote:
> 
> 
> On Wed, Jul 8, 2020 at 8:34 PM Dumitru Ceara  > wrote:
> 
> Logical flows that limit the ARP/NS broadcast domain on a logical switch
> should only match on ARP requests/NS for IPs that can actually be
> replied to on the connected router port (i.e., an IP on the same network
> is configured on the router port).
> 
> Reported-by: Girish Moodalbail  >
> Reported-at:
> https://mail.openvswitch.org/pipermail/ovs-discuss/2020-June/050287.html
> Fixes: 32f5ebb06226 ("ovn-northd: Limit ARP/ND broadcast domain
> whenever possible.")
> Signed-off-by: Dumitru Ceara  >
> 
> 
> Hi Dumitru,
> 
> The patch LGTM. I've few nits.
> 
> Thanks
> Numan
>  


Hi Numan,

Thanks for the review, I sent a v3:

https://patchwork.ozlabs.org/project/openvswitch/patch/1594324577-17459-1-git-send-email-dce...@redhat.com/

Thanks,
Dumitru

> 
> ---
>  northd/ovn-northd.c |  164
> ++-
>  tests/ovn.at         |   74 +++
>  2 files changed, 210 insertions(+), 28 deletions(-)
> 
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 4c23158..e2e875a 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -6091,6 +6091,44 @@ build_lrouter_groups(struct hmap *ports,
> struct ovs_list *lr_list)
>      }
>  }
> 
> +/* Returns 'true' if the IPv4 'addr' is on the same subnet with one
> of the
> + * IPs configured on the router port.
> + */
> +static bool
> +lrouter_port_ipv4_reachable(const struct ovn_port *op, ovs_be32 addr)
> +{
> +    for (size_t i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
> +        struct ipv4_netaddr *op_addr = &op->lrp_networks.ipv4_addrs[i];
> +
> +        if ((op_addr->addr & op_addr->mask) == (addr &
> op_addr->mask)) {
> 
> 
> This can be - 
>              if ((addr & op_addr->mask) == (op_addr->network)) {
> 
> If you see lib/ovn-util.c, network is initialized as - addr & mask.
> 
> 
>               
> 
> +            return true;
> +        }
> +    }
> +    return false;
> +}
> +
> +/* Returns 'true' if the IPv6 'addr' is on the same subnet with one
> of the
> + * IPs configured on the router port.
> + */
> +static bool
> +lrouter_port_ipv6_reachable(const struct ovn_port *op,
> +                            const struct in6_addr *addr)
> +{
> +    for (size_t i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) {
> +        struct ipv6_netaddr *op_addr = &op->lrp_networks.ipv6_addrs[i];
> +
> +        struct in6_addr op_addr6_masked =
> +            ipv6_addr_bitand(&op_addr->addr, &op_addr->mask);
> 
> 
> Same as above. We don't need to do this and instead we can use -
> op_addr->network
> below in ipv6_addr_equals()
>  
> 
> +        struct in6_addr nat_addr6_masked =
> +            ipv6_addr_bitand(addr, &op_addr->mask);
> +
> +        if (ipv6_addr_equals(&op_addr6_masked, &nat_addr6_masked)) {
> 
> +            return true;
> +        }
> +    }
> +    return false;
> +}
> +
>  /*
>   * Ingress table 19: Flows that flood self originated ARP/ND
> packets in the
>   * switching domain.
> @@ -6101,8 +6139,47 @@
> build_lswitch_rport_arp_req_self_orig_flow(struct ovn_port *op,
>                                             struct ovn_datapath *od,
>                                             struct hmap *lflows)
>  {
> -    struct ds match = DS_EMPTY_INITIALIZER;
> +    struct sset all_eth_addrs = SSET_INITIALIZER(&all_eth_addrs);
>      struct ds eth_src = DS_EMPTY_INITIALIZER;
> +    struct ds match = DS_EMPTY_INITIALIZER;
> +
> +    sset_add(&all_eth_addrs, op->lrp_networks.ea_s);
> +
> +    for (size_t i = 0; i < op->od->nbr->n_nat; i++) {
> +        struct ovn_nat *nat_entry = &op->od->nat_entries[i];
> +        const struct nbrec_nat *nat = nat_entry->nb;
> +
> +        if (!nat_entry_is_valid(nat_entry)) {
> +            continue;
> +        }
> +
> +        if (!strcmp(nat->type, "snat")) {
> +            continue;
> +        }
> +
> +        if (!nat->external_mac) {
> +            continue;
> +        }
> +
> +        /* Check if there's a network configured on op on which we
> could
> 
> 
> s/op/ovn port
> or
> s/op/'op'
> 
> I couldn't understand what op is. So either putting it in quotes or
> referencing it as an
> "ovn port" would  be easier to understand.
> 
>  
> 
> +         * expect ARP requests/NS for the DNAT external_ip.
> +         */
> +        if (nat_entry_is_v6(nat_entry)) {
> +            struct in6_addr *addr =
> &nat_entry->ext_addrs.ipv6_addrs

[ovs-dev] [PATCH ovn v3] ovn-northd: Fix logical flows to limit ARP/NS broadcast domain.

2020-07-09 Thread Dumitru Ceara
Logical flows that limit the ARP/NS broadcast domain on a logical switch
should only match on ARP requests/NS for IPs that can actually be
replied to on the connected router port (i.e., an IP on the same network
is configured on the router port).

Reported-by: Girish Moodalbail 
Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-discuss/2020-June/050287.html
Fixes: 32f5ebb06226 ("ovn-northd: Limit ARP/ND broadcast domain whenever 
possible.")
Signed-off-by: Dumitru Ceara 
---
v3:
- The first patch of the series was already applied.
- Addressed Numan's comments:
  - use precomputed ipv4/ipv6_netaddr network field instead of
recomputing it.
  - reworded comments to make them more clear.
v2:
- Changed the fix into a series, such that the memory leak fix can be
  easily backported to stable branches.
- Fixed the "Fixes" tag.
---
 northd/ovn-northd.c | 162 +++-
 tests/ovn.at|  74 
 2 files changed, 208 insertions(+), 28 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 1921982..d64b467 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -6091,6 +6091,42 @@ build_lrouter_groups(struct hmap *ports, struct ovs_list 
*lr_list)
 }
 }
 
+/* Returns 'true' if the IPv4 'addr' is on the same subnet with one of the
+ * IPs configured on the router port.
+ */
+static bool
+lrouter_port_ipv4_reachable(const struct ovn_port *op, ovs_be32 addr)
+{
+for (size_t i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
+struct ipv4_netaddr *op_addr = &op->lrp_networks.ipv4_addrs[i];
+
+if ((addr & op_addr->mask) == op_addr->network) {
+return true;
+}
+}
+return false;
+}
+
+/* Returns 'true' if the IPv6 'addr' is on the same subnet with one of the
+ * IPs configured on the router port.
+ */
+static bool
+lrouter_port_ipv6_reachable(const struct ovn_port *op,
+const struct in6_addr *addr)
+{
+for (size_t i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) {
+struct ipv6_netaddr *op_addr = &op->lrp_networks.ipv6_addrs[i];
+
+struct in6_addr nat_addr6_masked =
+ipv6_addr_bitand(addr, &op_addr->mask);
+
+if (ipv6_addr_equals(&nat_addr6_masked, &op_addr->network)) {
+return true;
+}
+}
+return false;
+}
+
 /*
  * Ingress table 19: Flows that flood self originated ARP/ND packets in the
  * switching domain.
@@ -6101,8 +6137,47 @@ build_lswitch_rport_arp_req_self_orig_flow(struct 
ovn_port *op,
struct ovn_datapath *od,
struct hmap *lflows)
 {
-struct ds match = DS_EMPTY_INITIALIZER;
+struct sset all_eth_addrs = SSET_INITIALIZER(&all_eth_addrs);
 struct ds eth_src = DS_EMPTY_INITIALIZER;
+struct ds match = DS_EMPTY_INITIALIZER;
+
+sset_add(&all_eth_addrs, op->lrp_networks.ea_s);
+
+for (size_t i = 0; i < op->od->nbr->n_nat; i++) {
+struct ovn_nat *nat_entry = &op->od->nat_entries[i];
+const struct nbrec_nat *nat = nat_entry->nb;
+
+if (!nat_entry_is_valid(nat_entry)) {
+continue;
+}
+
+if (!strcmp(nat->type, "snat")) {
+continue;
+}
+
+if (!nat->external_mac) {
+continue;
+}
+
+/* Check if the ovn port has a network configured on which we could
+ * expect ARP requests/NS for the DNAT external_ip.
+ */
+if (nat_entry_is_v6(nat_entry)) {
+struct in6_addr *addr = &nat_entry->ext_addrs.ipv6_addrs[0].addr;
+
+if (!lrouter_port_ipv6_reachable(op, addr)) {
+continue;
+}
+} else {
+ovs_be32 addr = nat_entry->ext_addrs.ipv4_addrs[0].addr;
+
+if (!lrouter_port_ipv4_reachable(op, addr)) {
+continue;
+}
+}
+sset_add(&all_eth_addrs, nat->external_mac);
+}
+
 
 /* Self originated (G)ARP requests/ND need to be flooded as usual.
  * Determine that packets are self originated by also matching on
@@ -6110,15 +6185,11 @@ build_lswitch_rport_arp_req_self_orig_flow(struct 
ovn_port *op,
  * is a VLAN-backed network.
  * Priority: 80.
  */
-ds_put_format(ð_src, "{ %s, ", op->lrp_networks.ea_s);
-for (size_t i = 0; i < op->od->nbr->n_nat; i++) {
-const struct nbrec_nat *nat = op->od->nbr->nat[i];
-
-if (!nat->external_mac) {
-continue;
-}
+const char *eth_addr;
 
-ds_put_format(ð_src, "%s, ", nat->external_mac);
+ds_put_cstr(ð_src, "{");
+SSET_FOR_EACH (eth_addr, &all_eth_addrs) {
+ds_put_format(ð_src, "%s, ", eth_addr);
 }
 ds_chomp(ð_src, ' ');
 ds_chomp(ð_src, ',');
@@ -6130,8 +6201,9 @@ build_lswitch_rport_arp_req_self_orig_flow(struct 
ovn_port *op,
   ds_cstr(&match),
   "outport = \""MC_FLOOD"

[ovs-dev] Account Re-Profilling(Please Reply)

2020-07-09 Thread Gerald Johnson via dev
From: UBS Investment Bank (London)
1 Golden Lane, London.
EC 1Y 0RR, United Kingdom.
The Operations Manager,

Do accept my sincere apologies if my mail does not meet your personal
ethics, also hope that you are and your family are safe from the deadly
covid 19 pandemic?
I know that this mail will come to you as a surprise, but please do not be
discouraged with my proposal it was due to how things are moving.

However, this correspondence is unofficial and private, and it should be
treated as such. At first I will like to assure you that this transaction is
100% risk and trouble free to both parties. I am Mr.Gerald Johnson, the
Operations Manager with the UBS Investment International Bank here in
London. I am contacting you based on my personal interest to develop a
mutual business relationship with you in your country. One of our accounts
holding a balance of GBP7,549,250.00 has been dormant for many years.

Please i am asking for your partnership in re-profiling the funds.First of
all,I will like to assure you that this transaction is 100% risk and trouble
free to both parties.I solicit for your assistance to execute this
transaction.This transaction will be covered up by me with my status here in
the bank.If you are interested to know more, email me back so we could
discuss more about this great business opportunity.

Yours Sincerely,
Gerald Johnson
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH ovn v2] Fix the routing for external logical ports of bridged logical switches.

2020-07-09 Thread Ankur Sharma
Hi Numan, Daniel,

I have not looked at the patch yet. But replacing arp.sha with chassis mac is 
not the correct approach from networking perspective.
Chassic mac is NOT meant to replace the IP-MAC binding of router port, it is 
ONLY meant to ensure that for EW traffic a distributed router port mac does not 
show on multiple TOR ports.
Both for NS and EW, ARP resolution for router port ip should be responded with 
router port mac ONLY.

I am trying to understand the use case and we can discuss an alternative in 
this thread.
Can you share the repro steps, i can try the same and will try to come up with 
an alternative.

Regards,
Ankur

From: num...@ovn.org 
Sent: Thursday, July 9, 2020 2:11 AM
To: d...@openvswitch.org 
Cc: Numan Siddique ; Daniel Alvarez ; 
Ankur Sharma 
Subject: [PATCH ovn v2] Fix the routing for external logical ports of bridged 
logical switches.

From: Numan Siddique 

Routing for external logical ports is broken if these ports belonged
to bridged logical switches (with localnet port) and 'ovn-chassis-mac-mappings'
is configured. External logical ports are those which are external to OVN,
but there is a logical port for it and it is claimed by one of the HA chassis.
The claimed chassis provides routing and other native OVN serices like dhcp and 
dns.

When the external port sends ARP request for the router IP, the claimed chassis
replies for the ARP request, but the arp.sha is set to the actual router mac 
instead
of the chassis mac. This causes the traffic from external port VM/container to 
be handled
incorrectly. A ping to the router ip, is replied by all the chassis which can 
see this
packet instead of just the claimed HA chassis.

To fix this, this patch does 2 things.

1. In the table - OFTABLE_LOG_TO_PHY (65), it adds a 160 priority flow to
   modify the ARP packets arp.sha to store the chassis mac.

2. And when the packet destined to the chassis mac is received, it replaces the
   chassis mac with the actual router mac in table 0.

Reported-at: 
https://urldefense.proofpoint.com/v2/url?u=https-3A__bugzilla.redhat.com_show-5Fbug.cgi-3Fid-3D1829762&d=DwIDAg&c=s883GpUCOChKOHiocYtGcg&r=mZwX9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY&m=u_maNAEOYzfy4_tzUirBX0TdPn35ePuIddtQDl4B8fs&s=T6SxlTDjkPxA6_Lsv_KjWkOSUSfesz0LIVnovPxBXlc&e=
Reported-by: Daniel Alvarez 
CC: Ankur Sharma 
Signed-off-by: Numan Siddique 
---

v1 -> v2

  * Rebased.

 controller/chassis.c  |  48 --
 controller/chassis.h  |   2 +
 controller/physical.c | 145 +++---
 tests/ovn.at  | 131 ++
 4 files changed, 299 insertions(+), 27 deletions(-)

diff --git a/controller/chassis.c b/controller/chassis.c
index eec270ea39..25146d75f2 100644
--- a/controller/chassis.c
+++ b/controller/chassis.c
@@ -645,10 +645,11 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
 }

 bool
-chassis_get_mac(const struct sbrec_chassis *chassis_rec,
-const char *bridge_mapping,
-struct eth_addr *chassis_mac)
+chassis_get_mac_mappings(const struct sbrec_chassis *chassis_rec,
+ struct smap *chassis_mappings)
 {
+smap_init(chassis_mappings);
+
 const char *tokens
 = get_chassis_mac_mappings(&chassis_rec->other_config);
 if (!tokens[0]) {
@@ -656,7 +657,6 @@ chassis_get_mac(const struct sbrec_chassis *chassis_rec,
 }

 char *save_ptr = NULL;
-bool ret = false;
 char *tokstr = xstrdup(tokens);

 /* Format for a chassis mac configuration is:
@@ -669,24 +669,36 @@ chassis_get_mac(const struct sbrec_chassis *chassis_rec,
 char *chassis_mac_bridge = strtok_r(token, ":", &save_ptr2);
 char *chassis_mac_str = strtok_r(NULL, "", &save_ptr2);

-if (!strcmp(chassis_mac_bridge, bridge_mapping)) {
-struct eth_addr temp_mac;
+smap_replace(chassis_mappings, chassis_mac_bridge, chassis_mac_str);
+}

-/* Return the first chassis mac. */
-char *err_str = str_to_mac(chassis_mac_str, &temp_mac);
-if (err_str) {
-free(err_str);
-continue;
-}
+free(tokstr);
+return true;
+}

-ret = true;
-*chassis_mac = temp_mac;
-break;
-}
+bool
+chassis_get_mac(const struct sbrec_chassis *chassis_rec,
+const char *bridge_mapping,
+struct eth_addr *chassis_mac)
+{
+struct smap chassis_mappings;
+
+if (!chassis_get_mac_mappings(chassis_rec, &chassis_mappings)) {
+return false;
 }

-free(tokstr);
-return ret;
+const char *chassis_mac_str = smap_get_def(&chassis_mappings,
+   bridge_mapping, "");
+struct eth_addr temp_mac;
+
+char *err_str = str_to_mac(chassis_mac_str, &temp_mac);
+if (err_str) {
+free(err_str);
+return false;
+}
+
+*chassis_mac = temp_m

Re: [ovs-dev] [PATCH v2] ovsdb-tool: Add a db consistency check to the ovsdb-tool check-cluster command

2020-07-09 Thread Dumitru Ceara
On 7/9/20 6:04 PM, Federico Paolinelli wrote:
> There are some occurrences where the database ends up in an inconsistent
> state. This happened in ovn-k8s and is described in
> https://bugzilla.redhat.com/show_bug.cgi?id=1837953#c23.
> Here we are adding a supported way to check that a given db is consistent,
> which is less error prone than checking the logs.
> 
> This was only tested against a valid database, as did not manage to get a
> corrupted one.

Hi Federico,

The NB DB [0] on master-3 in the BZ [1] is an example of corrupted DB.

I tested your patch with it and I get:

ovsdb-tool check-cluster /tmp/kni1-vmaster-3-ovnnb_db.db

ovsdb-tool: /tmp/kni1-vmaster-3-ovnnb_db.db: server d5db not found in
server list

Tested-by: Dumitru Ceara 

I do have a few more minor comments on the patch itself.

Thanks,
Dumitru

[0] https://bugzilla.redhat.com/attachment.cgi?id=1697595
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1837953#c23

> 
> Signed-off-by: Federico Paolinelli 
> Suggested-by: Dumitru Ceara 
> ---
>  ovsdb/ovsdb-tool.c | 21 +
>  1 file changed, 21 insertions(+)
> 
> diff --git a/ovsdb/ovsdb-tool.c b/ovsdb/ovsdb-tool.c
> index 91662cab8..d5ada0c2d 100644
> --- a/ovsdb/ovsdb-tool.c
> +++ b/ovsdb/ovsdb-tool.c
> @@ -1497,6 +1497,27 @@ do_check_cluster(struct ovs_cmdl_context *ctx)
>  }
>  }
> 
> +/* Check for db consistency:
> + * The serverid must be in the servers list

Please add a '.' at the end of the sentence in the comment.

> + */
> +
> +for (struct server *s = c.servers; s < &c.servers[c.n_servers]; s++) {
> +struct shash *servers_obj = json_object(s->snap->servers);
> +char *server_id = xasprintf(SID_FMT, SID_ARGS(&s->header.sid));
> +bool found = false;
> +const struct shash_node *node;

Please add a blank line for readability.

> +SHASH_FOR_EACH (node, servers_obj) {
> +if (!strncmp(server_id, node->name, SID_LEN)) {
> +found = true;
> +}
> +}
> +if (!found) {
> +ovs_fatal(0, "%s: server %s not found in server list",
> +  s->filename, server_id);

This should be indented such that the arguments on the second line are
aligned right after the '(' above.

> +}
> +free(server_id);
> +}
> +
>  /* Clean up. */
> 
>  for (size_t i = 0; i < c.n_servers; i++) {
> 

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


Re: [ovs-dev] [PATCH] checkpatch: Add argument to skip gerrit change id check

2020-07-09 Thread Ilya Maximets
On 7/9/20 5:09 PM, Flavio Leitner wrote:
> On Sun, Jun 14, 2020 at 07:03:03PM +0300, Roi Dayan wrote:
>> This arg can be used internally by groups using gerrit for code reviews.
> 
> The patch looks good, but there is the condition line
> that goes up to column 85. It's the only line beyond
> column 79 in that file.
> 
> Sorry the nitpicking, but could you please fix that?

It's not a nitpicking, it's a PEP8 violation that breaks the build:
https://travis-ci.org/github/ovsrobot/ovs/jobs/698255640#L2702

> 
> fbl
> 
>>
>> Signed-off-by: Roi Dayan 
>> ---
>>  utilities/checkpatch.py | 9 +++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
>> index fc9e20bf1b5f..78c8c9ce49c7 100755
>> --- a/utilities/checkpatch.py
>> +++ b/utilities/checkpatch.py
>> @@ -182,6 +182,7 @@ __regex_if_macros = re.compile(r'^ +(%s) 
>> \([\S]([\s\S]+[\S])*\) { +\\' %
>>  
>>  skip_leading_whitespace_check = False
>>  skip_trailing_whitespace_check = False
>> +skip_gerrit_change_id_check = False
>>  skip_block_whitespace_check = False
>>  skip_signoff_check = False
>>  
>> @@ -814,7 +815,7 @@ def ovs_checkpatch_parse(text, filename, author=None, 
>> committer=None):
>>  elif is_co_author.match(line):
>>  m = is_co_author.match(line)
>>  co_authors.append(m.group(2))
>> -elif is_gerrit_change_id.match(line):
>> +elif is_gerrit_change_id.match(line) and not 
>> skip_gerrit_change_id_check:
>>  print_error(
>>  "Remove Gerrit Change-Id's before submitting upstream.")
>>  print("%d: %s\n" % (lineno, line))
>> @@ -885,7 +886,8 @@ Check options:
>>  -s|--skip-signoff-linesTolerate missing Signed-off-by line
>>  -S|--spellcheckCheck C comments and commit-message for 
>> possible
>> spelling mistakes
>> --t|--skip-trailing-whitespace  Skips the trailing whitespace test"""
>> +-t|--skip-trailing-whitespace  Skips the trailing whitespace test
>> +   --skip-gerrit-change-id Skips the gerrit change id test"""
>>% sys.argv[0])
>>  
>>  
>> @@ -942,6 +944,7 @@ if __name__ == '__main__':
>> "skip-leading-whitespace",
>> "skip-signoff-lines",
>> "skip-trailing-whitespace",
>> +   "skip-gerrit-change-id",
>> "spellcheck",
>> "quiet"])
>>  except:
>> @@ -960,6 +963,8 @@ if __name__ == '__main__':
>>  skip_signoff_check = True
>>  elif o in ("-t", "--skip-trailing-whitespace"):
>>  skip_trailing_whitespace_check = True
>> +elif o in ("--skip-gerrit-change-id"):
>> +skip_gerrit_change_id_check = True
>>  elif o in ("-f", "--check-file"):
>>  checking_file = True
>>  elif o in ("-S", "--spellcheck"):
>> -- 
>> 2.8.4
>>
>> ___
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 

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


Re: [ovs-dev] [PATCH v1 2/6] dpif-netdev: add tunnel_valid flag to skip ip/ipv6 address comparison

2020-07-09 Thread Ilya Maximets
On 7/6/20 1:27 PM, Yanqin Wei wrote:
> Hi Ilya,
> 
>>> ---
>>
>> Hi.
>> First of all, thanks for working on performance improvements!
> Thanks, I saw some slides where OVS was used to compare flow scalability with 
> other projects. It inspired me to optimize this code.
> 
>>
>> However, this doesn't look as a clean patch.
> There are some trade-off for legacy code.

What trade-offs?

>>
>> Why we need both pkt_metadata_datapath_init() and pkt_metadata_init() ?
>> Why we can't just not initialize ip_dst and use tunnel_valid flag everywhere?
> 
> This patch wants to reduce the scope of modification( only for fastpath), 
> because performance is not critical for slow path. So tunnel dst@ is set 
> before leaving fast path(upcall).
> Another reason is 'flow_tnl' member is defined in both ' pkt_metadata' and 
> 'flow'.  If tunnel_valid flag is introduced into 'flow', the layout and  
> legacy flow API also need to be modified.

I understand that you didn't want to touch anything beside the
performance critical parts.  However, dp_packet_/pkt_ API is
already heavily overloaded and having a few very similar
functions that can or can not be used in some contexts makes
things even more complicated.  It's hard to read and maintain.
And it's prone to errors in case someone will try t modify
datapath code.
I'd prefer not t have different initialization functions and
only have one variant.  This will also solve the issue that
every other part of code uses tunneling metadata without checking
'tunnel_valid' flag.  This is actually a logical mistake.
And yes, 'tunnel_valid' flag really needs a comment inside the
structure definition.

> 
>>
>> Current version complicates code making it less readable and prone to errors.
> Do you prefer to use tunnel_valid in both fast path and slow path? I could 
> send v2 for this modification.
>  
>> Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v6 4/6] dpdk: enable cpu feature detection.

2020-07-09 Thread Stokes, Ian




On 7/2/2020 6:42 PM, Harry van Haaren wrote:

This commit implements a method to retrieve the CPU ISA capabilities.
These ISA capabilities can be used in OVS to at runtime select a function
implementation to make the best use of the available ISA on the CPU.

Signed-off-by: Harry van Haaren 

LGTM, seems simple enough, I assume this gets called later in the series 
so in terms of testing will check it out there.


BR
Ian

---

v6:
- Add #if around RTE_CPUFLAG_AVX512F as it is not defined on CPU
   architectures that don't support the ISA. This check fixes compilation
   on those CPUs, as previously it had an undefined RTE_CPUFLAG_* error.
   (Reported by Travis, thanks Ian & William for running checks)

v5:
- Change API to return bool, and return to true/false (William Tu)
- Change error log to VLOG_ERR_ONCE instead of manual impl. (William Tu)

v4:
- Improve commit title and message
---
  lib/dpdk-stub.c |  9 +
  lib/dpdk.c  | 30 ++
  lib/dpdk.h  |  2 ++
  3 files changed, 41 insertions(+)

diff --git a/lib/dpdk-stub.c b/lib/dpdk-stub.c
index c332c217c..b7d577870 100644
--- a/lib/dpdk-stub.c
+++ b/lib/dpdk-stub.c
@@ -79,6 +79,15 @@ print_dpdk_version(void)
  {
  }
  
+bool

+dpdk_get_cpu_has_isa(const char *arch OVS_UNUSED,
+ const char *feature OVS_UNUSED)
+{
+VLOG_ERR_ONCE("DPDK not supported in this version of Open vSwitch, "
+  "cannot use CPU flag based optimizations");
+return false;
+}
+
  void
  dpdk_status(const struct ovsrec_open_vswitch *cfg)
  {
diff --git a/lib/dpdk.c b/lib/dpdk.c
index 31450d470..64a0d43e8 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -22,6 +22,7 @@
  #include 
  #include 
  
+#include 

  #include 
  #include 
  #include 
@@ -513,6 +514,35 @@ print_dpdk_version(void)
  puts(rte_version());
  }
  
+#define CHECK_CPU_FEATURE(feature, name_str, RTE_CPUFLAG)   \

+do {\
+if (strncmp(feature, name_str, strlen(name_str)) == 0) {\
+int has_isa = rte_cpu_get_flag_enabled(RTE_CPUFLAG);\
+VLOG_DBG("CPU flag %s, available %s\n", name_str,   \
+  has_isa ? "yes" : "no");  \
+return true;\
+}   \
+} while (0)
+
+bool
+dpdk_get_cpu_has_isa(const char *arch, const char *feature)
+{
+/* Ensure Arch is x86_64 */
+if (strncmp(arch, "x86_64", 6) != 0) {
+return false;
+}
+
+#if __x86_64__
+/* CPU flags only defined for the architecture that support it */
+CHECK_CPU_FEATURE(feature, "avx512f", RTE_CPUFLAG_AVX512F);
+CHECK_CPU_FEATURE(feature, "bmi2", RTE_CPUFLAG_BMI2);
+#endif
+
+VLOG_WARN("Unknown CPU arch,feature: %s,%s. Returning not supported.\n",
+  arch, feature);
+return false;
+}
+
  void
  dpdk_status(const struct ovsrec_open_vswitch *cfg)
  {
diff --git a/lib/dpdk.h b/lib/dpdk.h
index 736a64279..445a51d06 100644
--- a/lib/dpdk.h
+++ b/lib/dpdk.h
@@ -44,4 +44,6 @@ bool dpdk_per_port_memory(void);
  bool dpdk_available(void);
  void print_dpdk_version(void);
  void dpdk_status(const struct ovsrec_open_vswitch *);
+bool dpdk_get_cpu_has_isa(const char *arch, const char *feature);
+
  #endif /* dpdk.h */


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


Re: [ovs-dev] [PATCH v2 ovn 2/2] ovn-northd: Fix logical flows to limit ARP/NS broadcast domain.

2020-07-09 Thread Numan Siddique
On Wed, Jul 8, 2020 at 8:34 PM Dumitru Ceara  wrote:

> Logical flows that limit the ARP/NS broadcast domain on a logical switch
> should only match on ARP requests/NS for IPs that can actually be
> replied to on the connected router port (i.e., an IP on the same network
> is configured on the router port).
>
> Reported-by: Girish Moodalbail 
> Reported-at:
> https://mail.openvswitch.org/pipermail/ovs-discuss/2020-June/050287.html
> Fixes: 32f5ebb06226 ("ovn-northd: Limit ARP/ND broadcast domain whenever
> possible.")
> Signed-off-by: Dumitru Ceara 
>

Hi Dumitru,

The patch LGTM. I've few nits.

Thanks
Numan


> ---
>  northd/ovn-northd.c |  164
> ++-
>  tests/ovn.at|   74 +++
>  2 files changed, 210 insertions(+), 28 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 4c23158..e2e875a 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -6091,6 +6091,44 @@ build_lrouter_groups(struct hmap *ports, struct
> ovs_list *lr_list)
>  }
>  }
>
> +/* Returns 'true' if the IPv4 'addr' is on the same subnet with one of the
> + * IPs configured on the router port.
> + */
> +static bool
> +lrouter_port_ipv4_reachable(const struct ovn_port *op, ovs_be32 addr)
> +{
> +for (size_t i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
> +struct ipv4_netaddr *op_addr = &op->lrp_networks.ipv4_addrs[i];
> +
> +if ((op_addr->addr & op_addr->mask) == (addr & op_addr->mask)) {
>

This can be -
 if ((addr & op_addr->mask) == (op_addr->network)) {

If you see lib/ovn-util.c, network is initialized as - addr & mask.




> +return true;
> +}
> +}
> +return false;
> +}
> +
> +/* Returns 'true' if the IPv6 'addr' is on the same subnet with one of the
> + * IPs configured on the router port.
> + */
> +static bool
> +lrouter_port_ipv6_reachable(const struct ovn_port *op,
> +const struct in6_addr *addr)
> +{
> +for (size_t i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) {
> +struct ipv6_netaddr *op_addr = &op->lrp_networks.ipv6_addrs[i];
> +
> +struct in6_addr op_addr6_masked =
> +ipv6_addr_bitand(&op_addr->addr, &op_addr->mask);
>

Same as above. We don't need to do this and instead we can use -
op_addr->network
below in ipv6_addr_equals()


> +struct in6_addr nat_addr6_masked =
> +ipv6_addr_bitand(addr, &op_addr->mask);
> +
> +if (ipv6_addr_equals(&op_addr6_masked, &nat_addr6_masked)) {

+return true;
> +}
> +}
> +return false;
> +}
> +
>  /*
>   * Ingress table 19: Flows that flood self originated ARP/ND packets in
> the
>   * switching domain.
> @@ -6101,8 +6139,47 @@ build_lswitch_rport_arp_req_self_orig_flow(struct
> ovn_port *op,
> struct ovn_datapath *od,
> struct hmap *lflows)
>  {
> -struct ds match = DS_EMPTY_INITIALIZER;
> +struct sset all_eth_addrs = SSET_INITIALIZER(&all_eth_addrs);
>  struct ds eth_src = DS_EMPTY_INITIALIZER;
> +struct ds match = DS_EMPTY_INITIALIZER;
> +
> +sset_add(&all_eth_addrs, op->lrp_networks.ea_s);
> +
> +for (size_t i = 0; i < op->od->nbr->n_nat; i++) {
> +struct ovn_nat *nat_entry = &op->od->nat_entries[i];
> +const struct nbrec_nat *nat = nat_entry->nb;
> +
> +if (!nat_entry_is_valid(nat_entry)) {
> +continue;
> +}
> +
> +if (!strcmp(nat->type, "snat")) {
> +continue;
> +}
> +
> +if (!nat->external_mac) {
> +continue;
> +}
> +
> +/* Check if there's a network configured on op on which we could
>

s/op/ovn port
or
s/op/'op'

I couldn't understand what op is. So either putting it in quotes or
referencing it as an
"ovn port" would  be easier to understand.



> + * expect ARP requests/NS for the DNAT external_ip.
> + */
> +if (nat_entry_is_v6(nat_entry)) {
> +struct in6_addr *addr =
> &nat_entry->ext_addrs.ipv6_addrs[0].addr;
> +
> +if (!lrouter_port_ipv6_reachable(op, addr)) {
> +continue;
> +}
> +} else {
> +ovs_be32 addr = nat_entry->ext_addrs.ipv4_addrs[0].addr;
> +
> +if (!lrouter_port_ipv4_reachable(op, addr)) {
> +continue;
> +}
> +}
> +sset_add(&all_eth_addrs, nat->external_mac);
> +}
> +
>
>  /* Self originated (G)ARP requests/ND need to be flooded as usual.
>   * Determine that packets are self originated by also matching on
> @@ -6110,15 +6187,11 @@ build_lswitch_rport_arp_req_self_orig_flow(struct
> ovn_port *op,
>   * is a VLAN-backed network.
>   * Priority: 80.
>   */
> -ds_put_format(ð_src, "{ %s, ", op->lrp_networks.ea_s);
> -for (size_t i = 0; i < op->od->nbr->n_nat; i++) {
> -const stru

Re: [ovs-dev] [PATCH v6 0/6] DPCLS Subtable ISA Optimization

2020-07-09 Thread Ilya Maximets
On 7/2/20 7:42 PM, Harry van Haaren wrote:
> v6 work done:
> - Fix as --64 unrecognized option
> - Fix build issues with avx512 library changes
> - Fix files left in build dir after distclean
> - Fix CPU arch dependant RTE_CPUFLAG_ usage
> 
> Thanks William & Ian for review & help on v5.
> All known issues are fixed and working here.
> 
> 
> Hi All,
> 
> This patchset implements the changes as proposed during the
> OVS Conf '19, in the talk "Next steps for SW Datapath".
> Youtube link: https://youtu.be/x0bOpojnpmU
> 
> The talk raises 3 main requirements for CPU ISA Optimizations,
> each of which is addressed in some of the patches below.
> - Test & Validation (video @ 2:20)
> - Usabiliity & Debug (video @ 6:00)
> - Package & Deploy (video @ 8:45)
> 
> Patch 1/6:
> The test and validation requirements proposed above are implemented,
> with the refactor of the subtable function pointer registration,
> and the autovalidator implementation is added.
> 
> Patch 2 & 3 / 6:
> Adds the commands for usability & debug. Now improved with a "get" and
> "set" command. Get returns current priorities and a list of each lookup
> implementation. Set provides feedback to the user as to the number of
> DPCLS ports/subtables that have new lookup functions due to the command
> that was executed.
> 
> Patch 4/6:
> Enable CPU ISA detection at runtime, providing information for future
> ISA optimized functions.
> 
> Patch 5/6:
> Actual AVX-512 implementation for DPCLS subtable search. This is the
> actual SIMD vector code, which performs DPCLS miniflow iteration in
> parallel.
> 
> Patch 6/6:
> Add section in dpdk/bridges.rst on how to use the DPCLS commands, and
> what they can be used for. Testing and validation using autovalidator
> concept introduced, and command to set its priority is provided. The
> steps required to run the autovalidator as default lookup implementation
> are described. NEWS file is updated with things changed in userspace datapath.
> 
> 
> Thanks for reading, any questions please let me know.
> Regards, -Harry
> 
> 
> 
> Harry van Haaren (6):
>   dpif-netdev: implement subtable lookup validation.
>   dpif-netdev: add subtable lookup prio set command.
>   dpif-netdev: add subtable-lookup-prio-get command.
>   dpdk: enable cpu feature detection.
>   dpif-lookup: add avx512 gather implementation.
>   docs/dpdk/bridge: add datapath performance section.
> 
>  Documentation/topics/dpdk/bridge.rst   |  77 +++
>  NEWS   |   3 +
>  acinclude.m4   |  16 ++
>  configure.ac   |   4 +
>  lib/automake.mk|  24 +++
>  lib/dpdk-stub.c|   9 +
>  lib/dpdk.c |  30 +++
>  lib/dpdk.h |   2 +
>  lib/dpif-netdev-lookup-autovalidator.c | 110 ++
>  lib/dpif-netdev-lookup-avx512-gather.c | 265 +
>  lib/dpif-netdev-lookup-generic.c   |   9 +-
>  lib/dpif-netdev-lookup.c   | 124 
>  lib/dpif-netdev-lookup.h   |  79 
>  lib/dpif-netdev-private.h  |  15 --
>  lib/dpif-netdev.c  | 161 ++-
>  m4/openvswitch.m4  |  30 +++
>  16 files changed, 934 insertions(+), 24 deletions(-)
>  create mode 100644 lib/dpif-netdev-lookup-autovalidator.c
>  create mode 100644 lib/dpif-netdev-lookup-avx512-gather.c
>  create mode 100644 lib/dpif-netdev-lookup.c
>  create mode 100644 lib/dpif-netdev-lookup.h
> 

Hi.

That's an interesting feature.  Thanks for working on this!

I didn't look to the code closely yet.  Just one general note:
We're not using tabs anywhere for indentation in user-visible output, but
this patch-set does.  So this should be changed to a couple of spaces.

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


Re: [ovs-dev] Investment

2020-07-09 Thread Ms. Reem Al-Hashimi
My name is Reem E. Al-Hashimi, the Emirates Minister of State and Managing 
Director of the United Arab Emirates (Dubai) World Expo 2020 Committee. I am 
writing to you to stand as my partner to receive my share of gratification from 
foreign companies whom I helped during the bidding exercise towards the Dubai 
World Expo 2020 Committee and also i want to use this funds to assist 
Coronavirus Symptoms and Causes. 
 
Am a single Arab women and serving as a minister, there is a limit to my 
personal income and investment level and  For this reason, I cannot receive 
such a huge sum back to my country or my personal account, so an agreement was 
reached with the foreign companies to direct the gratifications to an open 
beneficiary account with a financial institution where it will be possible for 
me to instruct further transfer of the fund to a third party account for 
investment purpose which is the reason i contacted you to receive the fund as 
my partner for investment in your country. 
 
If you can handle the fund in a good investment. 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] OVN Meeting Logs 09 July, 2020

2020-07-09 Thread mmichels
Here is the IRC log for the OVN meeting for 09 July, 2020

http://eavesdrop.openstack.org/meetings//ovn_community_development_discussion/2020/ovn_community_development_discussion.2020-07-09-17.15.log.html

If you are interested in attending this meeting, it happens every
Thursday in the #openvswitch channel on the Freenode IRC server. The
next one is at 13:15 EDT (18:15 BST, 10:15 PDT) on 16 July, 2020.

Mark Michelson

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


Re: [ovs-dev] [PATCH v6 3/6] dpif-netdev: add subtable-lookup-prio-get command.

2020-07-09 Thread Stokes, Ian




On 7/2/2020 6:42 PM, Harry van Haaren wrote:

This commit adds ia new command, "dpif-netdev/subtable-lookup-prio-get"
which prints the available sutable lookup functions in this OVS binary.
Example output from the command:

Available lookup functions (priority : name)
 0 : autovalidator
 1 : generic

Signed-off-by: Harry van Haaren 
Acked-by: William Tu 


LGTM, tested without issue.

Regards
Ian


---

v5:
- Add Ack from mailing list

v4:
- Add "prio" to name for consistency with "set" command (William Tu)
- Fix typo (William Tu)
---
  lib/dpif-netdev.c | 27 +++
  1 file changed, 27 insertions(+)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 758323a0e..da344106d 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1295,6 +1295,30 @@ sorted_poll_thread_list(struct dp_netdev *dp,
  *n = k;
  }
  
+static void

+dpif_netdev_subtable_lookup_get(struct unixctl_conn *conn, int argc OVS_UNUSED,
+const char *argv[] OVS_UNUSED,
+void *aux OVS_UNUSED)
+{
+/* Get a list of all lookup functions */
+struct dpcls_subtable_lookup_info_t *lookup_funcs = NULL;
+int32_t count = dpcls_subtable_lookup_info_get(&lookup_funcs);
+if (count < 0) {
+unixctl_command_reply_error(conn, "error getting lookup names");
+return;
+}
+
+/* Add all lookup functions to reply string */
+struct ds reply = DS_EMPTY_INITIALIZER;
+ds_put_cstr(&reply, "Available lookup functions (priority : name)\n");
+for (int i = 0; i < count; i++) {
+ds_put_format(&reply, "\t%d : %s\n", lookup_funcs[i].prio,
+  lookup_funcs[i].name);
+}
+unixctl_command_reply(conn, ds_cstr(&reply));
+ds_destroy(&reply);
+}
+
  static void
  dpif_netdev_subtable_lookup_set(struct unixctl_conn *conn, int argc,
  const char *argv[], void *aux OVS_UNUSED)
@@ -1605,6 +1629,9 @@ dpif_netdev_init(void)
   "[lookup_func] [prio] [dp]",
   2, 3, dpif_netdev_subtable_lookup_set,
   NULL);
+unixctl_command_register("dpif-netdev/subtable-lookup-prio-get", "",
+ 0, 0, dpif_netdev_subtable_lookup_get,
+ NULL);
  return 0;
  }
  


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


Re: [ovs-dev] [PATCH v6 2/6] dpif-netdev: add subtable lookup prio set command.

2020-07-09 Thread Stokes, Ian




On 7/2/2020 6:42 PM, Harry van Haaren wrote:

This commit adds a command for the dpif-netdev to set a specific
lookup function to a particular priority level. The command enables
runtime switching of the dpcls subtable lookup implementation.

Selection is performed based on a priority. Higher priorities take
precedence, eg; priotity 5 will be selected instead of a priority 3.

Minor typo above 'priority'

If lookup functions have the same priority, the first one in the list
is selected.

The two options available are 'autovalidator' and 'generic'.
The below command will set a new priority for the given function:
$ ovs-appctl dpif-netdev/subtable-lookup-prio-set generic 2

The autovalidator implementation can be selected at runtime now:
$ ovs-appctl dpif-netdev/subtable-lookup-prio-set autovalidator 5

Signed-off-by: Harry van Haaren 
Acked-by: William Tu 



LGTM, tested OK, few minor typos and style comments that can be fixed 
upon commit. Few comments below.



---

v5:
- Add Ack from mailing list
- Rebase to latest master (fixing command adding conflict)

v4:
- Align prio-set command in code (William Tu)
- Improve commit title & update commit message (William Tu)

v3
- Add automatic reprobe after changing priorities
--- Refactored from previous 1-second timeout based reprobe WIP-hack
- Add VLOG entries for changed dpcls and subtable counts
--- Also return the updated counts to the issuing command for visibility
- Clarify command by adding "prio" to the name
--- New command name is "dpif-netdev/subtable-lookup-prio-set"
--- Please note this new command change - previous command is now invalid
---
  lib/dpif-netdev.c | 121 ++
  1 file changed, 121 insertions(+)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 0468064c9..758323a0e 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -259,6 +259,7 @@ struct dp_packet_flow_map {
  static void dpcls_init(struct dpcls *);
  static void dpcls_destroy(struct dpcls *);
  static void dpcls_sort_subtable_vector(struct dpcls *);
+static uint32_t dpcls_subtable_lookup_reprobe(struct dpcls *cls);
  static void dpcls_insert(struct dpcls *, struct dpcls_rule *,
   const struct netdev_flow_key *mask);
  static void dpcls_remove(struct dpcls *, struct dpcls_rule *);
@@ -891,6 +892,9 @@ dpif_netdev_xps_revalidate_pmd(const struct 
dp_netdev_pmd_thread *pmd,
 bool purge);
  static int dpif_netdev_xps_get_tx_qid(const struct dp_netdev_pmd_thread *pmd,
struct tx_port *tx);
+static inline struct dpcls *
+dp_netdev_pmd_lookup_dpcls(struct dp_netdev_pmd_thread *pmd,
+   odp_port_t in_port);
  
  static inline bool emc_entry_alive(struct emc_entry *ce);

  static void emc_clear_entry(struct emc_entry *ce);
@@ -1291,6 +1295,97 @@ sorted_poll_thread_list(struct dp_netdev *dp,
  *n = k;
  }
  
+static void

+dpif_netdev_subtable_lookup_set(struct unixctl_conn *conn, int argc,
+const char *argv[], void *aux OVS_UNUSED)
+{
+/* This function requires 2 parameters (argv[1] and argv[2]) to execute.
+ *   argv[1] is subtable name
+ *   argv[2] is priority
+ *   argv[3] is the datapath name (optional if only 1 datapath exists)
+ */
+const char *func_name = argv[1];
+
+errno = 0;
+char *err_char;
+uint32_t new_prio = strtoul(argv[2], &err_char, 10);
+if (errno != 0 || new_prio > UINT8_MAX) {
+unixctl_command_reply_error(conn,
+"error converting priority, use integer in range 0-255\n");
+return;
+}
+
+int32_t err = dpcls_subtable_set_prio(func_name, new_prio);
+if (err) {
+unixctl_command_reply_error(conn,
+"error, subtable lookup function not found\n");
+return;
+}


So in testing above I spotted that you are able to set the priority of a 
subtable lookup function even though no datapath or subtables exist for 
that lookup function to operate on. Is this expected?


I understand that the same lookup functions can have the different 
priorities on different bridges, just thinking in the case that no 
datapath exists should you be unable to set the priority?


BR
Ian

+
+/* argv[3] is optional datapath instance. If no datapath name is provided
+ * and only one datapath exists, the one existing datapath is reprobed.
+ */
+ovs_mutex_lock(&dp_netdev_mutex);
+struct dp_netdev *dp = NULL;
+
+if (argc == 4) {
+dp = shash_find_data(&dp_netdevs, argv[3]);
+} else if (shash_count(&dp_netdevs) == 1) {
+dp = shash_first(&dp_netdevs)->data;
+}
+
+if (!dp) {
+ovs_mutex_unlock(&dp_netdev_mutex);
+unixctl_command_reply_error(conn,
+"please specify an existing datapath");
+return;
+}
+
+/* Get PMD threads list, required to get DPCLS instances */
+size_t n;
+u

Re: [ovs-dev] [PATCH v8 3/4] system-dpdk: macro to set hugepages in numa nodes

2020-07-09 Thread Flavio Leitner
On Thu, Jul 09, 2020 at 10:18:18PM +0530, Gowrishankar Muthukrishnan wrote:
> Moved commands to set hugepages in every numa node into a macro
> for tests to reuse.
> 
> Signed-off-by: Gowrishankar Muthukrishnan 
> ---
LGTM
Acked-by: Flavio Leitner 

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


Re: [ovs-dev] [PATCH v8 1/4] system-dpdk: filter non-available hugepages in ovs_vswitchd_stop

2020-07-09 Thread Flavio Leitner
On Thu, Jul 09, 2020 at 10:18:16PM +0530, Gowrishankar Muthukrishnan wrote:
> While checking log for stopping ovs-vswitchd, we can ignore
> message about non-available hugepages as that is specific to
> hugepages configuration in system and not required in the scope
> log check.
> 
> Signed-off-by: Gowrishankar Muthukrishnan 
> ---

LGTM
Acked-by: Flavio Leitner 

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


Re: [ovs-dev] [PATCH v8 2/4] system-dpdk: split ovs_dpdk_start into db and daemon ops

2020-07-09 Thread Flavio Leitner
On Thu, Jul 09, 2020 at 10:18:17PM +0530, Gowrishankar Muthukrishnan wrote:
> OVS_DPDK_START could be split into two macros to start
> ovsdb-server and ovs-vswitchd individually so that, user can
> use ovs-vsctl to configure ovs tables for test requirements.
> 
> Signed-off-by: Gowrishankar Muthukrishnan 
> ---
LGTM
Acked-by: Flavio Leitner 

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


Re: [ovs-dev] [PATCH v8 4/4] system-dpdk: add negotiation check for userspace-tso

2020-07-09 Thread Flavio Leitner
On Thu, Jul 09, 2020 at 10:18:19PM +0530, Gowrishankar Muthukrishnan wrote:
> This patch adds minimal check for userspace-tso in system-dpdk
> tests, starting with verification on virtio negotiation.
> 
> Signed-off-by: Gowrishankar Muthukrishnan 
> ---
LGTM
Acked-by: Flavio Leitner 

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


[ovs-dev] [PATCH v8 3/4] system-dpdk: macro to set hugepages in numa nodes

2020-07-09 Thread Gowrishankar Muthukrishnan
Moved commands to set hugepages in every numa node into a macro
for tests to reuse.

Signed-off-by: Gowrishankar Muthukrishnan 
---
 tests/system-dpdk.at | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
index ab10f44..623bd34 100644
--- a/tests/system-dpdk.at
+++ b/tests/system-dpdk.at
@@ -1,6 +1,11 @@
 m4_define([CONFIGURE_VETH_OFFLOADS],
[AT_CHECK([ethtool -K $1 tx off], [0], [ignore], [ignore])])
 
+m4_define([SET_NUMA_NODE],
+   [
+AT_CHECK([lscpu | awk '/NUMA node\(s\)/ {c=1; while (c++<$(3)) {printf 
"$1,"}; print "$1"}' > NUMA_NODE])
+])
+
 AT_BANNER([OVS-DPDK unit tests])
 
 dnl --
@@ -96,8 +101,7 @@ OVS_DB_START()
 OVS_DPDK_START()
 
 dnl Find number of sockets
-AT_CHECK([lscpu], [], [stdout])
-AT_CHECK([cat stdout | grep "NUMA node(s)" | awk '{c=1; while (c++<$(3)) 
{printf "512,"}; print "512"}' > NUMA_NODE])
+SET_NUMA_NODE([512])
 
 dnl Add userspace bridge and attach it to OVS
 AT_CHECK([ovs-vsctl add-br br10 -- set bridge br10 datapath_type=netdev])
@@ -175,8 +179,7 @@ OVS_DB_START()
 OVS_DPDK_START()
 
 dnl Find number of sockets
-AT_CHECK([lscpu], [], [stdout])
-AT_CHECK([cat stdout | grep "NUMA node(s)" | awk '{c=1; while (c++<$(3)) 
{printf "512,"}; print "512"}' > NUMA_NODE])
+SET_NUMA_NODE([512])
 
 dnl Add userspace bridge and attach it to OVS
 AT_CHECK([ovs-vsctl add-br br10 -- set bridge br10 datapath_type=netdev])
-- 
1.8.3.1

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


[ovs-dev] [PATCH v8 4/4] system-dpdk: add negotiation check for userspace-tso

2020-07-09 Thread Gowrishankar Muthukrishnan
This patch adds minimal check for userspace-tso in system-dpdk
tests, starting with verification on virtio negotiation.

Signed-off-by: Gowrishankar Muthukrishnan 
---
v8:
 - split patch into logical units of changes (as this series)

---
 tests/system-dpdk.at | 280 +++
 1 file changed, 280 insertions(+)

diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
index 623bd34..ef03fb2 100644
--- a/tests/system-dpdk.at
+++ b/tests/system-dpdk.at
@@ -245,3 +245,283 @@ OVS_VSWITCHD_STOP(["\@does not exist. The Open vSwitch 
kernel module is probably
 \@EAL: No free hugepages reported in hugepages-1048576kB@d"])
 AT_CLEANUP
 dnl --
+
+dnl --
+dnl validate negotiation when both ovs and testpmd have tso on
+AT_SETUP([OVS-DPDK - validate negotiation when both ovs and testpmd have tso 
on])
+AT_KEYWORDS([dpdk])
+OVS_DPDK_PRE_CHECK()
+AT_SKIP_IF([! which testpmd >/dev/null 2>/dev/null])
+OVS_DB_START()
+AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch . 
other_config:userspace-tso-enable=true])
+OVS_DPDK_START()
+AT_CHECK([grep -c 'Userspace TCP Segmentation Offloading support enabled' \
+  ovs-vswitchd.log],[ignore],[dnl
+1
+])
+dnl Find number of sockets
+SET_NUMA_NODE([512])
+
+dnl Add userspace bridge and attach it to OVS
+AT_CHECK([ovs-vsctl add-br br10 -- set bridge br10 datapath_type=netdev])
+
+dnl Add vhostuser port (client mode)
+AT_CHECK([ovs-vsctl add-port br10 dpdkvhostuserclient0 -- set Interface \
+  dpdkvhostuserclient0 \
+  type=dpdkvhostuserclient \
+  options:vhost-server-path=$OVS_RUNDIR/dpdkvhostclient0], [],
+ [stdout], [stderr])
+AT_CHECK([ovs-vsctl show], [], [stdout])
+
+dnl Execute testpmd in background
+on_exit "pkill -f -x -9 'tail -f /dev/null'"
+AT_CHECK([echo "show device info all" > CMDFILE])
+AT_CHECK([echo "stop" >> CMDFILE])
+AT_CHECK([echo "port stop 0" >> CMDFILE])
+AT_CHECK([echo "tso set 1500 0" >> CMDFILE])
+AT_CHECK([echo "csum set tcp hw 0" >> CMDFILE])
+AT_CHECK([echo "port start 0" >> CMDFILE])
+AT_CHECK([echo "start" >> CMDFILE])
+AT_CHECK([echo "show port 0 tx_offload capabilities" >> CMDFILE])
+AT_CHECK([echo "show port 0 tx_offload configuration" >> CMDFILE])
+tail -f /dev/null | testpmd --socket-mem="$(cat NUMA_NODE)" --no-pci\
+   --vdev="net_virtio_user,path=$OVS_RUNDIR/dpdkvhostclient0,server=1" 
\
+   --vdev="net_tap0,iface=tap0" --file-prefix page0 \
+   --single-file-segments -- --cmdline-file=CMDFILE \
+   -a >$OVS_RUNDIR/testpmd-dpdkvhostuserclient0.log 2>&1 &
+
+dnl Give settling time to the testpmd processes - NOTE: this is bad form.
+sleep 10
+
+dnl Clean up the testpmd now
+pkill -f -x -9 'tail -f /dev/null'
+sleep 1
+
+dnl Check whether TSO is turned on (host side)
+AT_CHECK([awk '/negotiated Virtio features/ {a=$NF} END{print a}' \
+   $OVS_RUNDIR/ovs-vswitchd.log],[],[stdout])
+AT_CHECK([printf "%X" $(( $(cat stdout) & ((1<<0)|(1<<11)|(1<<12)) 
))],[],[1801])
+
+dnl Check whether TSO is turned on (guest side)
+AT_CHECK([awk 'BEGIN{n=0} /Port :/ && /TCP_CKSUM/ && /TCP_TSO/ {n++} 
END{printf n}' \
+   $OVS_RUNDIR/testpmd-dpdkvhostuserclient0.log],[0],[1])
+
+dnl Clean up
+AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout], 
[stderr])
+OVS_VSWITCHD_STOP(["
+\@EAL: No available hugepages reported in hugepages-1048576kB@d
+\@EAL:   Invalid NUMA socket, default to 0@d
+\@VHOST_CONFIG: failed to connect to $OVS_RUNDIR/dpdkvhostclient0: No such 
file or directory@d
+"])
+AT_CLEANUP
+dnl --
+
+dnl --
+dnl validate negotiation when ovs alone has tso on
+AT_SETUP([OVS-DPDK - validate negotiation when ovs alone has tso on])
+AT_KEYWORDS([dpdk])
+OVS_DPDK_PRE_CHECK()
+AT_SKIP_IF([! which testpmd >/dev/null 2>/dev/null])
+OVS_DB_START()
+AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch . 
other_config:userspace-tso-enable=true])
+OVS_DPDK_START()
+AT_CHECK([grep -c 'Userspace TCP Segmentation Offloading support enabled' \
+  ovs-vswitchd.log],[ignore],[dnl
+1
+])
+dnl Find number of sockets
+SET_NUMA_NODE([512])
+
+dnl Add userspace bridge and attach it to OVS
+AT_CHECK([ovs-vsctl add-br br10 -- set bridge br10 datapath_type=netdev])
+
+dnl Add vhostuser port (client mode)
+AT_CHECK([ovs-vsctl add-port br10 dpdkvhostuserclient0 -- set Interface \
+  dpdkvhostuserclient0 \
+  type=dpdkvhostuserclient \
+  options:vhost-server-path=$OVS_RUNDIR/dpdkvhostclient0], [],
+ [stdout], [stderr])
+AT_CHECK([ovs-vsctl show], [], [stdout])
+
+dnl Execute testpmd in background
+on_exit "pkill -f -x -9 'tail -f /dev/null'"
+AT_CHECK([echo "show device info all" > CMDFILE])
+AT_CHECK([echo "stop" >> CMDFILE])
+AT

[ovs-dev] [PATCH v8 2/4] system-dpdk: split ovs_dpdk_start into db and daemon ops

2020-07-09 Thread Gowrishankar Muthukrishnan
OVS_DPDK_START could be split into two macros to start
ovsdb-server and ovs-vswitchd individually so that, user can
use ovs-vsctl to configure ovs tables for test requirements.

Signed-off-by: Gowrishankar Muthukrishnan 
---
 tests/system-dpdk-macros.at | 17 -
 tests/system-dpdk.at|  5 +
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/tests/system-dpdk-macros.at b/tests/system-dpdk-macros.at
index c6708ca..9e55f10 100644
--- a/tests/system-dpdk-macros.at
+++ b/tests/system-dpdk-macros.at
@@ -33,13 +33,11 @@ m4_define([OVS_DPDK_PRE_PHY_SKIP],
 ])
 
 
-# OVS_DPDK_START()
+# OVS_DB_START()
 #
-# Create an empty database and start ovsdb-server. Add special configuration
-# dpdk-init to enable DPDK functionality. Start ovs-vswitchd connected to that
-# database using system devices (no dummies).
+# Create an empty database and start ovsdb-server.
 #
-m4_define([OVS_DPDK_START],
+m4_define([OVS_DB_START],
   [dnl Create database.
AT_CHECK([touch .conf.db.~lock~])
AT_CHECK([ovsdb-tool create conf.db 
$abs_top_srcdir/vswitchd/vswitch.ovsschema])
@@ -54,7 +52,16 @@ m4_define([OVS_DPDK_START],
 
dnl Initialize database.
AT_CHECK([ovs-vsctl --no-wait init])
+])
+
 
+# OVS_DPDK_START()
+#
+# Add special configuration dpdk-init to enable DPDK functionality.
+# Start ovs-vswitchd connected to that database using system devices (no 
dummies).
+#
+m4_define([OVS_DPDK_START],
+  [
dnl Enable DPDK functionality
AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch . 
other_config:dpdk-init=true])
 
diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
index 21424c2..ab10f44 100644
--- a/tests/system-dpdk.at
+++ b/tests/system-dpdk.at
@@ -8,6 +8,7 @@ dnl Check if EAL init is successful
 AT_SETUP([OVS-DPDK - EAL init])
 AT_KEYWORDS([dpdk])
 OVS_DPDK_PRE_CHECK()
+OVS_DB_START()
 OVS_DPDK_START()
 AT_CHECK([grep "DPDK Enabled - initializing..." ovs-vswitchd.log], [], 
[stdout])
 AT_CHECK([grep "EAL" ovs-vswitchd.log], [], [stdout])
@@ -28,6 +29,7 @@ AT_SETUP([OVS-DPDK - add standard DPDK port])
 AT_KEYWORDS([dpdk])
 
 OVS_DPDK_PRE_PHY_SKIP()
+OVS_DB_START()
 OVS_DPDK_START()
 
 dnl Add userspace bridge and attach it to OVS
@@ -55,6 +57,7 @@ dnl Add vhost-user-client port
 AT_SETUP([OVS-DPDK - add vhost-user-client port])
 AT_KEYWORDS([dpdk])
 OVS_DPDK_PRE_CHECK()
+OVS_DB_START()
 OVS_DPDK_START()
 
 dnl Add userspace bridge and attach it to OVS
@@ -89,6 +92,7 @@ AT_SETUP([OVS-DPDK - ping vhost-user ports])
 AT_KEYWORDS([dpdk])
 OVS_DPDK_PRE_CHECK()
 AT_SKIP_IF([! which testpmd >/dev/null 2>/dev/null])
+OVS_DB_START()
 OVS_DPDK_START()
 
 dnl Find number of sockets
@@ -167,6 +171,7 @@ AT_SETUP([OVS-DPDK - ping vhost-user-client ports])
 AT_KEYWORDS([dpdk])
 OVS_DPDK_PRE_CHECK()
 AT_SKIP_IF([! which testpmd >/dev/null 2>/dev/null])
+OVS_DB_START()
 OVS_DPDK_START()
 
 dnl Find number of sockets
-- 
1.8.3.1

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


[ovs-dev] [PATCH v8 0/4] system-dpdk: add tso negotiation tests

2020-07-09 Thread Gowrishankar Muthukrishnan
This patch series enables system-dpdk test to validate TSO
negotiation.

Gowrishankar Muthukrishnan (4):
  system-dpdk: filter non-available hugepages in ovs_vswitchd_stop
  system-dpdk: split ovs_dpdk_start into db and daemon ops
  system-dpdk: macro to set hugepages in numa nodes
  system-dpdk: add negotiation check for userspace-tso

 tests/system-dpdk-macros.at |  17 ++-
 tests/system-dpdk.at| 301 +++-
 2 files changed, 309 insertions(+), 9 deletions(-)

-- 
1.8.3.1

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


[ovs-dev] [PATCH v8 1/4] system-dpdk: filter non-available hugepages in ovs_vswitchd_stop

2020-07-09 Thread Gowrishankar Muthukrishnan
While checking log for stopping ovs-vswitchd, we can ignore
message about non-available hugepages as that is specific to
hugepages configuration in system and not required in the scope
log check.

Signed-off-by: Gowrishankar Muthukrishnan 
---
 tests/system-dpdk.at | 5 +
 1 file changed, 5 insertions(+)

diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
index a015d52..21424c2 100644
--- a/tests/system-dpdk.at
+++ b/tests/system-dpdk.at
@@ -15,6 +15,7 @@ AT_CHECK([grep "DPDK Enabled - initialized" 
ovs-vswitchd.log], [], [stdout])
 OVS_VSWITCHD_STOP(["/Global register is changed during/d
 /EAL:   Invalid NUMA socket, default to 0/d
 /EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable 
clock cycles !/d
+/EAL: No available hugepages reported in hugepages-1048576kB/d
 /EAL: No free hugepages reported in hugepages-1048576kB/d"])
 AT_CLEANUP
 dnl --
@@ -41,6 +42,7 @@ OVS_VSWITCHD_STOP("/does not exist. The Open vSwitch kernel 
module is probably n
 /Failed to enable flow control/d
 /Global register is changed during/d
 /EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable 
clock cycles !/d
+/EAL: No available hugepages reported in hugepages-1048576kB/d
 /EAL: No free hugepages reported in hugepages-1048576kB/d
 ")
 AT_CLEANUP
@@ -74,6 +76,7 @@ OVS_VSWITCHD_STOP(["\@does not exist. The Open vSwitch kernel 
module is probably
 \@Global register is changed during@d
 \@EAL:   Invalid NUMA socket, default to 0@d
 \@EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable 
clock cycles !@d
+\@EAL: No available hugepages reported in hugepages-1048576kB@d
 \@EAL: No free hugepages reported in hugepages-1048576kB@d"])
 AT_CLEANUP
 dnl --
@@ -153,6 +156,7 @@ OVS_VSWITCHD_STOP(["\@does not exist. The Open vSwitch 
kernel module is probably
 \@failed to enumerate system datapaths: No such file or directory@d
 \@EAL:   Invalid NUMA socket, default to 0@d
 \@EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable 
clock cycles !@d
+\@EAL: No available hugepages reported in hugepages-1048576kB@d
 \@EAL: No free hugepages reported in hugepages-1048576kB@d"])
 AT_CLEANUP
 dnl --
@@ -229,6 +233,7 @@ OVS_VSWITCHD_STOP(["\@does not exist. The Open vSwitch 
kernel module is probably
 \@failed to enumerate system datapaths: No such file or directory@d
 \@EAL:   Invalid NUMA socket, default to 0@d
 \@EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable 
clock cycles !@d
+\@EAL: No available hugepages reported in hugepages-1048576kB@d
 \@EAL: No free hugepages reported in hugepages-1048576kB@d"])
 AT_CLEANUP
 dnl --
-- 
1.8.3.1

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


[ovs-dev] [PATCH v2] ovsdb-tool: Add a db consistency check to the ovsdb-tool check-cluster command

2020-07-09 Thread Federico Paolinelli
There are some occurrences where the database ends up in an inconsistent
state. This happened in ovn-k8s and is described in
https://bugzilla.redhat.com/show_bug.cgi?id=1837953#c23.
Here we are adding a supported way to check that a given db is consistent,
which is less error prone than checking the logs.

This was only tested against a valid database, as did not manage to get a
corrupted one.

Signed-off-by: Federico Paolinelli 
Suggested-by: Dumitru Ceara 
---
 ovsdb/ovsdb-tool.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/ovsdb/ovsdb-tool.c b/ovsdb/ovsdb-tool.c
index 91662cab8..d5ada0c2d 100644
--- a/ovsdb/ovsdb-tool.c
+++ b/ovsdb/ovsdb-tool.c
@@ -1497,6 +1497,27 @@ do_check_cluster(struct ovs_cmdl_context *ctx)
 }
 }

+/* Check for db consistency:
+ * The serverid must be in the servers list
+ */
+
+for (struct server *s = c.servers; s < &c.servers[c.n_servers]; s++) {
+struct shash *servers_obj = json_object(s->snap->servers);
+char *server_id = xasprintf(SID_FMT, SID_ARGS(&s->header.sid));
+bool found = false;
+const struct shash_node *node;
+SHASH_FOR_EACH (node, servers_obj) {
+if (!strncmp(server_id, node->name, SID_LEN)) {
+found = true;
+}
+}
+if (!found) {
+ovs_fatal(0, "%s: server %s not found in server list",
+  s->filename, server_id);
+}
+free(server_id);
+}
+
 /* Clean up. */

 for (size_t i = 0; i < c.n_servers; i++) {
-- 
2.26.2

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


Re: [ovs-dev] [PATCH] dpif-netdev: Avoid deadlock with offloading during PMD thread deletion.

2020-07-09 Thread 0-day Robot
Bleep bloop.  Greetings Ilya Maximets, 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: Comment with 'xxx' marker
#157 FILE: lib/dpif-netdev.c:3217:
 * XXX: Main thread will try to pause/stop all revalidators during datapath

Lines checked: 194, 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


[ovs-dev] [PATCH] dpif-netdev: Avoid deadlock with offloading during PMD thread deletion.

2020-07-09 Thread Ilya Maximets
Main thread will try to pause/stop all revalidators during datapath
reconfiguration via datapath purge callback (dp_purge_cb) while
holding 'dp->port_mutex'.  And deadlock happens in case any of
revalidator threads is already waiting on 'dp->port_mutex' while
dumping offloaded flows:

   main thread   revalidator
 -  --

 ovs_mutex_lock(&dp->port_mutex)

dpif_netdev_flow_dump_next()
-> dp_netdev_flow_to_dpif_flow
-> get_dpif_flow_status
-> dpif_netdev_get_flow_offload_status()
-> ovs_mutex_lock(&dp->port_mutex)
   

 reconfigure_datapath()
 -> reconfigure_pmd_threads()
 -> dp_netdev_del_pmd()
 -> dp_purge_cb()
 -> udpif_pause_revalidators()
 -> ovs_barrier_block(&udpif->pause_barrier)


  

We're not allowed to call offloading API without holding global
port mutex from the userspace datapath due to thread safety
restrictions on netdev-ofload-dpdk module.  And it's also not easy
to rework datapath reconfiguration process in order to move actual
PMD removal and datapath purge out of the port mutex.

So, for now, not sleeping on a mutex if it's not immediately available
seem like an easiest workaround.  This will have impact on flow
statistics update rate and on ability to get the latest statistics
before removing the flow (latest stats will be lost in case we were
not able to take the mutex).  However, this will allow us to operate
normally avoiding the deadlock.

The last bit is that to avoid flapping of flow attributes and
statistics we're not failing the operation, but returning last
statistics and attributes returned by offload provider.  Since those
might be updated in different threads, stores and reads are atomic.

Reported-by: Frank Wang (王培辉) 
Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-dev/2020-June/371753.html
Fixes: a309e4f52660 ("dpif-netdev: Update offloaded flows statistics.")
Signed-off-by: Ilya Maximets 
---

Tested with unit tests only.  Needs checking with the real setup.
Especially stats/attrs update parts.

 AUTHORS.rst   |  1 +
 lib/dpif-netdev.c | 78 +++
 2 files changed, 73 insertions(+), 6 deletions(-)

diff --git a/AUTHORS.rst b/AUTHORS.rst
index 8e6a0769f..eb36a01d0 100644
--- a/AUTHORS.rst
+++ b/AUTHORS.rst
@@ -504,6 +504,7 @@ Edwin Chiu  ec...@vmware.com
 Eivind Bulie Haanaes
 Enas Ahmad  enas.ah...@kaust.edu.sa
 Eric Lopez
+Frank Wang (王培辉) wangpei...@inspur.com
 Frido Roose fr.ro...@gmail.com
 Gaetano Catalli gaetano.cata...@gmail.com
 Gavin Remaley   gavin_rema...@selinc.com
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 629a0cb53..2eb7f32ff 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -490,6 +490,12 @@ struct dp_netdev_flow_stats {
 atomic_uint16_t tcp_flags; /* Bitwise-OR of seen tcp_flags values. */
 };
 
+/* Contained by struct dp_netdev_flow's 'last_attrs' member.  */
+struct dp_netdev_flow_attrs {
+atomic_bool offloaded; /* True if flow is offloaded to HW. */
+ATOMIC(const char *) dp_layer; /* DP layer the flow is handled in. */
+};
+
 /* A flow in 'dp_netdev_pmd_thread's 'flow_table'.
  *
  *
@@ -550,6 +556,10 @@ struct dp_netdev_flow {
 /* Statistics. */
 struct dp_netdev_flow_stats stats;
 
+/* Statistics and attributes received from the netdev offload provider. */
+struct dp_netdev_flow_stats last_stats;
+struct dp_netdev_flow_attrs last_attrs;
+
 /* Actions. */
 OVSRCU_TYPE(struct dp_netdev_actions *) actions;
 
@@ -3150,9 +3160,43 @@ dp_netdev_pmd_find_flow(const struct 
dp_netdev_pmd_thread *pmd,
 return NULL;
 }
 
+static void
+dp_netdev_flow_set_last_stats_attrs(struct dp_netdev_flow *netdev_flow,
+const struct dpif_flow_stats *stats,
+const struct dpif_flow_attrs *attrs)
+{
+struct dp_netdev_flow_stats *last_stats = &netdev_flow->last_stats;
+struct dp_netdev_flow_attrs *last_attrs = &netdev_flow->last_attrs;
+
+atomic_store_relaxed(&last_stats->used, stats->used);
+atomic_store_relaxed(&last_stats->packet_count, stats->n_packets);
+atomic_store_relaxed(&last_stats->byte_count,   stats->n_bytes);
+atomic_store_relaxed(&last_stats->tcp_flags,stats->tcp_flags);
+
+atomic_store_relaxed(&last_attrs->offloaded,attrs->offloaded);
+atomic_store_relaxed(&last_attrs->dp_layer, attrs->dp_layer);
+}
+
+static void
+dp_netdev_flow_get_last_stats_attrs(struct dp_netdev_flow *netdev_flow,
+struct dpif_flow_stats *stats,
+

Re: [ovs-dev] [PATCH] checkpatch: Add argument to skip gerrit change id check

2020-07-09 Thread Flavio Leitner
On Sun, Jun 14, 2020 at 07:03:03PM +0300, Roi Dayan wrote:
> This arg can be used internally by groups using gerrit for code reviews.

The patch looks good, but there is the condition line
that goes up to column 85. It's the only line beyond
column 79 in that file.

Sorry the nitpicking, but could you please fix that?

fbl

> 
> Signed-off-by: Roi Dayan 
> ---
>  utilities/checkpatch.py | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
> index fc9e20bf1b5f..78c8c9ce49c7 100755
> --- a/utilities/checkpatch.py
> +++ b/utilities/checkpatch.py
> @@ -182,6 +182,7 @@ __regex_if_macros = re.compile(r'^ +(%s) 
> \([\S]([\s\S]+[\S])*\) { +\\' %
>  
>  skip_leading_whitespace_check = False
>  skip_trailing_whitespace_check = False
> +skip_gerrit_change_id_check = False
>  skip_block_whitespace_check = False
>  skip_signoff_check = False
>  
> @@ -814,7 +815,7 @@ def ovs_checkpatch_parse(text, filename, author=None, 
> committer=None):
>  elif is_co_author.match(line):
>  m = is_co_author.match(line)
>  co_authors.append(m.group(2))
> -elif is_gerrit_change_id.match(line):
> +elif is_gerrit_change_id.match(line) and not 
> skip_gerrit_change_id_check:
>  print_error(
>  "Remove Gerrit Change-Id's before submitting upstream.")
>  print("%d: %s\n" % (lineno, line))
> @@ -885,7 +886,8 @@ Check options:
>  -s|--skip-signoff-linesTolerate missing Signed-off-by line
>  -S|--spellcheckCheck C comments and commit-message for 
> possible
> spelling mistakes
> --t|--skip-trailing-whitespace  Skips the trailing whitespace test"""
> +-t|--skip-trailing-whitespace  Skips the trailing whitespace test
> +   --skip-gerrit-change-id Skips the gerrit change id test"""
>% sys.argv[0])
>  
>  
> @@ -942,6 +944,7 @@ if __name__ == '__main__':
> "skip-leading-whitespace",
> "skip-signoff-lines",
> "skip-trailing-whitespace",
> +   "skip-gerrit-change-id",
> "spellcheck",
> "quiet"])
>  except:
> @@ -960,6 +963,8 @@ if __name__ == '__main__':
>  skip_signoff_check = True
>  elif o in ("-t", "--skip-trailing-whitespace"):
>  skip_trailing_whitespace_check = True
> +elif o in ("--skip-gerrit-change-id"):
> +skip_gerrit_change_id_check = True
>  elif o in ("-f", "--check-file"):
>  checking_file = True
>  elif o in ("-S", "--spellcheck"):
> -- 
> 2.8.4
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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


Re: [ovs-dev] [PATCH] ovsdb-tool: Add a db consistency check to the ovsdb-tool check-cluster command

2020-07-09 Thread 0-day Robot
Bleep bloop.  Greetings Federico Paolinelli, I am a robot and I have tried out 
your patch.
Thanks for your contribution.

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


git-am:
error: corrupt patch at line 10
error: could not build fake ancestor
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0001 ovsdb-tool: Add a db consistency check to the ovsdb-tool 
check-cluster command
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


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

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


Re: [ovs-dev] [PATCH v7] system-dpdk: add negotiation check for userspace-tso

2020-07-09 Thread Flavio Leitner
On Thu, Jul 09, 2020 at 07:14:02PM +0530, Gowrishankar Muthukrishnan wrote:
> Hi Flavio,
> I think it should be fine as we even do a similar adjustment of splitting
> OVS_DPDK_START into DB and daemon start ops for existing tests as well.
> What do you think?.

That is required to have TSO negotiation checks, so it's part
of the patch. On another hand, those new filtering messages are
unrelated.

A patch should make one logical change.

fbl


> 
> Thanks,
> Gowrishankar
> 
> On Thu, Jul 9, 2020 at 6:26 PM Flavio Leitner  wrote:
> 
> >
> > Hi Gowrishankar,
> >
> > On Thu, Jul 09, 2020 at 11:34:33AM +0530, Gowrishankar Muthukrishnan wrote:
> > > This patch adds minimal check for userspace-tso in system-dpdk
> > > tests, starting with verification on virtio negotiation.
> > >
> > > Signed-off-by: Gowrishankar Muthukrishnan 
> > > ---
> > > v7:
> > >  - fixed ovs stop log check for added tests.
> > >  - test #8 is skipped using AT_ macro.
> > >
> > > Sample test output:
> > > ## --- ##
> > > ## openvswitch 2.13.90 test suite. ##
> > > ## --- ##
> > >
> > > OVS-DPDK unit tests
> > >
> > >   1: OVS-DPDK - EAL init ok
> > >   2: OVS-DPDK - add standard DPDK port   skipped (
> > system-dpdk.at:36)
> > >   3: OVS-DPDK - add vhost-user-client port   ok
> > >   4: OVS-DPDK - ping vhost-user portsok
> > >   5: OVS-DPDK - ping vhost-user-client ports ok
> > >   6: OVS-DPDK - validate negotiation when both ovs and testpmd have tso
> > on ok
> > >   7: OVS-DPDK - validate negotiation when ovs alone has tso on ok
> > >   8: OVS-DPDK - validate negotiation when ovs alone has tso off skipped (
> > system-dpdk.at:448)
> > >   9: OVS-DPDK - validate negotiation when both ovs and testpmd have tso
> > off ok
> > >
> > > ## - ##
> > > ## Test results. ##
> > > ## - ##
> > >
> > > 7 tests were successful.
> > > 2 tests were skipped.
> > >
> > > ---
> > >  tests/system-dpdk-macros.at |  17 ++-
> > >  tests/system-dpdk.at| 301
> > +++-
> > >  2 files changed, 309 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/tests/system-dpdk-macros.at b/tests/system-dpdk-macros.at
> > > index c6708ca..9e55f10 100644
> > > --- a/tests/system-dpdk-macros.at
> > > +++ b/tests/system-dpdk-macros.at
> > > @@ -33,13 +33,11 @@ m4_define([OVS_DPDK_PRE_PHY_SKIP],
> > >  ])
> > >
> > >
> > > -# OVS_DPDK_START()
> > > +# OVS_DB_START()
> > >  #
> > > -# Create an empty database and start ovsdb-server. Add special
> > configuration
> > > -# dpdk-init to enable DPDK functionality. Start ovs-vswitchd connected
> > to that
> > > -# database using system devices (no dummies).
> > > +# Create an empty database and start ovsdb-server.
> > >  #
> > > -m4_define([OVS_DPDK_START],
> > > +m4_define([OVS_DB_START],
> > >[dnl Create database.
> > > AT_CHECK([touch .conf.db.~lock~])
> > > AT_CHECK([ovsdb-tool create conf.db
> > $abs_top_srcdir/vswitchd/vswitch.ovsschema])
> > > @@ -54,7 +52,16 @@ m4_define([OVS_DPDK_START],
> > >
> > > dnl Initialize database.
> > > AT_CHECK([ovs-vsctl --no-wait init])
> > > +])
> > > +
> > >
> > > +# OVS_DPDK_START()
> > > +#
> > > +# Add special configuration dpdk-init to enable DPDK functionality.
> > > +# Start ovs-vswitchd connected to that database using system devices
> > (no dummies).
> > > +#
> > > +m4_define([OVS_DPDK_START],
> > > +  [
> > > dnl Enable DPDK functionality
> > > AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch .
> > other_config:dpdk-init=true])
> > >
> > > diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
> > > index a015d52..ef03fb2 100644
> > > --- a/tests/system-dpdk.at
> > > +++ b/tests/system-dpdk.at
> > > @@ -1,6 +1,11 @@
> > >  m4_define([CONFIGURE_VETH_OFFLOADS],
> > > [AT_CHECK([ethtool -K $1 tx off], [0], [ignore], [ignore])])
> > >
> > > +m4_define([SET_NUMA_NODE],
> > > +   [
> > > +AT_CHECK([lscpu | awk '/NUMA node\(s\)/ {c=1; while (c++<$(3))
> > {printf "$1,"}; print "$1"}' > NUMA_NODE])
> > > +])
> > > +
> > >  AT_BANNER([OVS-DPDK unit tests])
> > >
> > >  dnl
> > --
> > > @@ -8,6 +13,7 @@ dnl Check if EAL init is successful
> > >  AT_SETUP([OVS-DPDK - EAL init])
> > >  AT_KEYWORDS([dpdk])
> > >  OVS_DPDK_PRE_CHECK()
> > > +OVS_DB_START()
> > >  OVS_DPDK_START()
> > >  AT_CHECK([grep "DPDK Enabled - initializing..." ovs-vswitchd.log], [],
> > [stdout])
> > >  AT_CHECK([grep "EAL" ovs-vswitchd.log], [], [stdout])
> > > @@ -15,6 +21,7 @@ AT_CHECK([grep "DPDK Enabled - initialized"
> > ovs-vswitchd.log], [], [stdout])
> > >  OVS_VSWITCHD_STOP(["/Global register is changed during/d
> > >  /EAL:   Invalid NUMA socket, default to 0/d
> > >  /EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using
> > unreliable clock cycles !/d
> > > +/EAL: No available hugepages reported in hu

[ovs-dev] [PATCH] ovsdb-tool: Add a db consistency check to the ovsdb-tool check-cluster command

2020-07-09 Thread Federico Paolinelli
There are some occurrences where the database ends up in an inconsistent
state. This happened in ovn-k8s and is described in
https://bugzilla.redhat.com/show_bug.cgi?id=1837953#c23.
Here we are adding a supported way to check that a given db is consistent,
which is less error prone than checking the logs.

This was only tested against a valid database, as did not manage to get a
corrupted one.

Signed-off-by: Federico Paolinelli 
Suggested-by: Dumitru Ceara 
---
ovsdb/ovsdb-tool.c | 21 +
1 file changed, 21 insertions(+)

diff --git a/ovsdb/ovsdb-tool.c b/ovsdb/ovsdb-tool.c
index 91662cab8..d5ada0c2d 100644
--- a/ovsdb/ovsdb-tool.c
+++ b/ovsdb/ovsdb-tool.c
@@ -1497,6 +1497,27 @@ do_check_cluster(struct ovs_cmdl_context *ctx)
}
}
+ /* Check for db consistency:
+ * The serverid must be in the servers list
+ */
+
+ for (struct server *s = c.servers; s < &c.servers[c.n_servers]; s++) {
+ struct shash *servers_obj = json_object(s->snap->servers);
+ char *server_id = xasprintf(SID_FMT, SID_ARGS(&s->header.sid));
+ bool found = false;
+ const struct shash_node *node;
+ SHASH_FOR_EACH (node, servers_obj) {
+ if (!strncmp(server_id, node->name, SID_LEN)) {
+ found = true;
+ }
+ }
+ if (!found) {
+ ovs_fatal(0, "%s: server %s not found in server list",
+ s->filename, server_id);
+ }
+ free(server_id);
+ }
+
/* Clean up. */
for (size_t i = 0; i < c.n_servers; i++) {
-- 
2.26.2

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


Re: [ovs-dev] [PATCH v7] system-dpdk: add negotiation check for userspace-tso

2020-07-09 Thread Gowrishankar Muthukrishnan
Hi Flavio,
I think it should be fine as we even do a similar adjustment of splitting
OVS_DPDK_START into DB and daemon start ops for existing tests as well.
What do you think?.

Thanks,
Gowrishankar

On Thu, Jul 9, 2020 at 6:26 PM Flavio Leitner  wrote:

>
> Hi Gowrishankar,
>
> On Thu, Jul 09, 2020 at 11:34:33AM +0530, Gowrishankar Muthukrishnan wrote:
> > This patch adds minimal check for userspace-tso in system-dpdk
> > tests, starting with verification on virtio negotiation.
> >
> > Signed-off-by: Gowrishankar Muthukrishnan 
> > ---
> > v7:
> >  - fixed ovs stop log check for added tests.
> >  - test #8 is skipped using AT_ macro.
> >
> > Sample test output:
> > ## --- ##
> > ## openvswitch 2.13.90 test suite. ##
> > ## --- ##
> >
> > OVS-DPDK unit tests
> >
> >   1: OVS-DPDK - EAL init ok
> >   2: OVS-DPDK - add standard DPDK port   skipped (
> system-dpdk.at:36)
> >   3: OVS-DPDK - add vhost-user-client port   ok
> >   4: OVS-DPDK - ping vhost-user portsok
> >   5: OVS-DPDK - ping vhost-user-client ports ok
> >   6: OVS-DPDK - validate negotiation when both ovs and testpmd have tso
> on ok
> >   7: OVS-DPDK - validate negotiation when ovs alone has tso on ok
> >   8: OVS-DPDK - validate negotiation when ovs alone has tso off skipped (
> system-dpdk.at:448)
> >   9: OVS-DPDK - validate negotiation when both ovs and testpmd have tso
> off ok
> >
> > ## - ##
> > ## Test results. ##
> > ## - ##
> >
> > 7 tests were successful.
> > 2 tests were skipped.
> >
> > ---
> >  tests/system-dpdk-macros.at |  17 ++-
> >  tests/system-dpdk.at| 301
> +++-
> >  2 files changed, 309 insertions(+), 9 deletions(-)
> >
> > diff --git a/tests/system-dpdk-macros.at b/tests/system-dpdk-macros.at
> > index c6708ca..9e55f10 100644
> > --- a/tests/system-dpdk-macros.at
> > +++ b/tests/system-dpdk-macros.at
> > @@ -33,13 +33,11 @@ m4_define([OVS_DPDK_PRE_PHY_SKIP],
> >  ])
> >
> >
> > -# OVS_DPDK_START()
> > +# OVS_DB_START()
> >  #
> > -# Create an empty database and start ovsdb-server. Add special
> configuration
> > -# dpdk-init to enable DPDK functionality. Start ovs-vswitchd connected
> to that
> > -# database using system devices (no dummies).
> > +# Create an empty database and start ovsdb-server.
> >  #
> > -m4_define([OVS_DPDK_START],
> > +m4_define([OVS_DB_START],
> >[dnl Create database.
> > AT_CHECK([touch .conf.db.~lock~])
> > AT_CHECK([ovsdb-tool create conf.db
> $abs_top_srcdir/vswitchd/vswitch.ovsschema])
> > @@ -54,7 +52,16 @@ m4_define([OVS_DPDK_START],
> >
> > dnl Initialize database.
> > AT_CHECK([ovs-vsctl --no-wait init])
> > +])
> > +
> >
> > +# OVS_DPDK_START()
> > +#
> > +# Add special configuration dpdk-init to enable DPDK functionality.
> > +# Start ovs-vswitchd connected to that database using system devices
> (no dummies).
> > +#
> > +m4_define([OVS_DPDK_START],
> > +  [
> > dnl Enable DPDK functionality
> > AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch .
> other_config:dpdk-init=true])
> >
> > diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
> > index a015d52..ef03fb2 100644
> > --- a/tests/system-dpdk.at
> > +++ b/tests/system-dpdk.at
> > @@ -1,6 +1,11 @@
> >  m4_define([CONFIGURE_VETH_OFFLOADS],
> > [AT_CHECK([ethtool -K $1 tx off], [0], [ignore], [ignore])])
> >
> > +m4_define([SET_NUMA_NODE],
> > +   [
> > +AT_CHECK([lscpu | awk '/NUMA node\(s\)/ {c=1; while (c++<$(3))
> {printf "$1,"}; print "$1"}' > NUMA_NODE])
> > +])
> > +
> >  AT_BANNER([OVS-DPDK unit tests])
> >
> >  dnl
> --
> > @@ -8,6 +13,7 @@ dnl Check if EAL init is successful
> >  AT_SETUP([OVS-DPDK - EAL init])
> >  AT_KEYWORDS([dpdk])
> >  OVS_DPDK_PRE_CHECK()
> > +OVS_DB_START()
> >  OVS_DPDK_START()
> >  AT_CHECK([grep "DPDK Enabled - initializing..." ovs-vswitchd.log], [],
> [stdout])
> >  AT_CHECK([grep "EAL" ovs-vswitchd.log], [], [stdout])
> > @@ -15,6 +21,7 @@ AT_CHECK([grep "DPDK Enabled - initialized"
> ovs-vswitchd.log], [], [stdout])
> >  OVS_VSWITCHD_STOP(["/Global register is changed during/d
> >  /EAL:   Invalid NUMA socket, default to 0/d
> >  /EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using
> unreliable clock cycles !/d
> > +/EAL: No available hugepages reported in hugepages-1048576kB/d
> >  /EAL: No free hugepages reported in hugepages-1048576kB/d"])
> >  AT_CLEANUP
> >  dnl
> --
>
> The addition of the hugepages message in the filter is good
> in this and other tests as well, but it should be on a
> separate patch altogether because this one is only about
> tso negotiation, correct?
>
> Otherwise it looks good to me.
> Thanks
> fbl
>
>
>
>
> > @@ -27,6 +34,7 @@ AT_SETUP([OVS-DPDK - add standard DPDK port])
> >  AT_

Re: [ovs-dev] [PATCH v5 0/7] netdev datapath: Partial action offload

2020-07-09 Thread Sriharsha Basavapatna via dev
Hi Ilya,

I removed the RFC tag on this partial-offload patchset and it is
rebased to the latest master branch. We are targeting this for 2.14.
So just a gentle reminder on this offload patchset.

Thanks,
-Harsha

On Thu, Jul 9, 2020 at 12:17 PM Sriharsha Basavapatna
 wrote:
>
> Hi,
>
> This patchset extends the "Partial HW acceleration" mode to offload a
> part of the action processing to HW, instead of offloading just lookup
> (MARK/RSS), for "vhost-user" ports. This is referred to as "Partial Action
> Offload". This mode does not require SRIOV/switchdev configuration. In this
> mode, forwarding (output) action is still performed by OVS-DPDK SW datapath.
>
> Thanks,
> -Harsha
>
> **
>
> v4-->v5:
>
> - Rebased to the current ovs-master (includes vxlan-encap full offload)
> - Added 2 patches to support partial offload of vlan push/pop actions
>
> v3-->v4:
>
> - Removed mega-ufid mapping code from dpif-netdev (patch #5) and updated the
>   existing mega-ufid mapping code in netdev-offload-dpdk to support partial
>   action offload.
>
> v2-->v3:
>
> - Added more commit log comments in the refactoring patch (#1).
> - Removed wrapper function for should_partial_offload_egress().
> - Removed partial offload check for output action in parse_flow_actions().
> - Changed patch sequence (skip-encap and get-stats before offload patch).
> - Removed locking code (details in email), added inline comments.
> - Moved mega-ufid mapping code from skip-encap (#3) to the offload patch (#5).
>
> v1-->v2:
>
> Fixed review comments from Eli:
> - Revamped action list parsing to reject multiple clone/output actions
> - Updated comments to reflect support for single clone/output action
> - Removed place-holder function for ingress partial action offload
> - Created a separate patch (#2) to query dpdk-vhost netdevs
> - Set transfer attribute to 0 for partial action offload
> - Updated data type of 'dp_flow' in 'dp_netdev_execute_aux'
> - Added a mutex to synchronize between offload and datapath threads
> - Avoid fall back to mark/rss when egress partial offload fails
> - Drop count action for partial action offload
>
> Other changes:
> - Avoid duplicate offload requests for the same mega-ufid (from PMD threads)
> - Added a coverage counter to track pkts with tnl-push partial offloaded
> - Fixed dp_netdev_pmd_remove_flow() to delete partial offloaded flow
>
> **
>
> Sriharsha Basavapatna (7):
>   dpif-netdev: Refactor dp_netdev_flow_offload_put()
>   netdev-dpdk: provide a function to identify dpdk-vhost netdevs
>   dpif-netdev: Skip encap action during datapath execution
>   dpif-netdev: Support flow_get() with partial-action-offload
>   dpif-netdev: Support partial-action-offload of VXLAN encap flow
>   dpif-netdev: Support partial offload of PUSH_VLAN action
>   dpif-netdev: Support partial offload of POP_VLAN action
>
>  lib/dpif-netdev.c | 463 ++
>  lib/netdev-dpdk.c |   5 +
>  lib/netdev-dpdk.h |   1 +
>  lib/netdev-offload-dpdk.c |  96 ++--
>  lib/netdev-offload.h  |   3 +
>  lib/odp-execute.c |  19 +-
>  6 files changed, 502 insertions(+), 85 deletions(-)
>
> --
> 2.25.0.rc2
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v6 1/6] dpif-netdev: implement subtable lookup validation.

2020-07-09 Thread Stokes, Ian




On 7/2/2020 6:42 PM, Harry van Haaren wrote:

This commit refactors the existing dpif subtable function pointer
infrastructure, and implements an autovalidator component.

The refactoring of the existing dpcls subtable lookup function
handling, making it more generic, and cleaning up how to enable
more implementations in future.

In order to ensure all implementations provide identical results,
the autovalidator is added. The autovalidator itself implements
the subtable lookup function prototype, but internally iterates
over all other available implementations. The end result is that
testing of each implementation becomes automatic, when the auto-
validator implementation is selected.

Signed-off-by: Harry van Haaren 


Thasnk for this Harry,

a few comments below, more for discussion.

Theres a few minor typos and style fixes required, nothing major that 
cannot be fixed on commit so no need for revision.




---

v4:
- Fix automake .c file order (William Tu)
- Fix include file ordering: netdev-lookup before netdev-perf (William Tu)
- Fix Typos (William Tu)
- Add --enable-autovalidator compile time flag to set the DPCLS Autovalidator
   to the highest priority by default. This is required to run the unit tests
   with all DPCLS implementations without changing every test-case.
   Backwards compatibility is kept with OVS 2.13.
- Add check to ensure autovalidator is 0th item in lookup array
- Improve comments around autovalidator lookup iteration

v3:
- Fix compile error by adding errno.h include (William Tu)
- Improve vlog prints by using hex not int for bitmasks
- Update license years adding 2020
- Fix 0 used as NULL pointer
---
  acinclude.m4   |  16 
  configure.ac   |   1 +
  lib/automake.mk|   3 +
  lib/dpif-netdev-lookup-autovalidator.c | 110 +
  lib/dpif-netdev-lookup-generic.c   |   9 +-
  lib/dpif-netdev-lookup.c   | 104 +++
  lib/dpif-netdev-lookup.h   |  75 +
  lib/dpif-netdev-private.h  |  15 
  lib/dpif-netdev.c  |  13 ++-
  9 files changed, 322 insertions(+), 24 deletions(-)
  create mode 100644 lib/dpif-netdev-lookup-autovalidator.c
  create mode 100644 lib/dpif-netdev-lookup.c
  create mode 100644 lib/dpif-netdev-lookup.h

diff --git a/acinclude.m4 b/acinclude.m4
index 8847b8145..9b28222f6 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -14,6 +14,22 @@
  # See the License for the specific language governing permissions and
  # limitations under the License.
  
+dnl Set OVS DPCLS Autovalidator as default subtable search at compile time?

+dnl This enables automatically running all unit tests with all DPCLS
+dnl implementations.
+AC_DEFUN([OVS_CHECK_DPCLS_AUTOVALIDATOR], [
+  AC_ARG_ENABLE([autovalidator],
+[AC_HELP_STRING([--enable-autovalidator], [Enable DPCLS 
autovalidator as default subtable search implementation.])],
+[autovalidator=yes],[autovalidator=no])
+  AC_MSG_CHECKING([whether DPCLS Autovalidator is default implementation])
+  if test "$autovalidator" != yes; then
+AC_MSG_RESULT([no])
+  else
+OVS_CFLAGS="$OVS_CFLAGS -DDPCLS_AUTOVALIDATOR_DEFAULT"
+AC_MSG_RESULT([yes])
+  fi
+])
+
  dnl OVS_ENABLE_WERROR
  AC_DEFUN([OVS_ENABLE_WERROR],
[AC_ARG_ENABLE(
diff --git a/configure.ac b/configure.ac
index 1877aae56..81893e56e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -181,6 +181,7 @@ OVS_CONDITIONAL_CC_OPTION([-Wno-unused-parameter], 
[HAVE_WNO_UNUSED_PARAMETER])
  OVS_ENABLE_WERROR
  OVS_ENABLE_SPARSE
  OVS_CTAGS_IDENTIFIERS
+OVS_CHECK_DPCLS_AUTOVALIDATOR
  
  AC_ARG_VAR(KARCH, [Kernel Architecture String])

  AC_SUBST(KARCH)
diff --git a/lib/automake.mk b/lib/automake.mk
index 86940ccd2..1fc1a209e 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -81,6 +81,9 @@ lib_libopenvswitch_la_SOURCES = \
lib/dp-packet.h \
lib/dp-packet.c \
lib/dpdk.h \
+   lib/dpif-netdev-lookup.h \
+   lib/dpif-netdev-lookup.c \
+   lib/dpif-netdev-lookup-autovalidator.c \
lib/dpif-netdev-lookup-generic.c \
lib/dpif-netdev.c \
lib/dpif-netdev.h \
diff --git a/lib/dpif-netdev-lookup-autovalidator.c 
b/lib/dpif-netdev-lookup-autovalidator.c
new file mode 100644
index 0..a027321ab
--- /dev/null
+++ b/lib/dpif-netdev-lookup-autovalidator.c
@@ -0,0 +1,110 @@
+/*
+ * Copyright (c) 2020 Intel Corporation.
+ *
+ * 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 speci

Re: [ovs-dev] [PATCH v7] system-dpdk: add negotiation check for userspace-tso

2020-07-09 Thread Flavio Leitner


Hi Gowrishankar,

On Thu, Jul 09, 2020 at 11:34:33AM +0530, Gowrishankar Muthukrishnan wrote:
> This patch adds minimal check for userspace-tso in system-dpdk
> tests, starting with verification on virtio negotiation.
> 
> Signed-off-by: Gowrishankar Muthukrishnan 
> ---
> v7:
>  - fixed ovs stop log check for added tests.
>  - test #8 is skipped using AT_ macro.
> 
> Sample test output:
> ## --- ##
> ## openvswitch 2.13.90 test suite. ##
> ## --- ##
> 
> OVS-DPDK unit tests
> 
>   1: OVS-DPDK - EAL init ok
>   2: OVS-DPDK - add standard DPDK port   skipped 
> (system-dpdk.at:36)
>   3: OVS-DPDK - add vhost-user-client port   ok
>   4: OVS-DPDK - ping vhost-user portsok
>   5: OVS-DPDK - ping vhost-user-client ports ok
>   6: OVS-DPDK - validate negotiation when both ovs and testpmd have tso on ok
>   7: OVS-DPDK - validate negotiation when ovs alone has tso on ok
>   8: OVS-DPDK - validate negotiation when ovs alone has tso off skipped 
> (system-dpdk.at:448)
>   9: OVS-DPDK - validate negotiation when both ovs and testpmd have tso off ok
> 
> ## - ##
> ## Test results. ##
> ## - ##
> 
> 7 tests were successful.
> 2 tests were skipped.
> 
> ---
>  tests/system-dpdk-macros.at |  17 ++-
>  tests/system-dpdk.at| 301 
> +++-
>  2 files changed, 309 insertions(+), 9 deletions(-)
> 
> diff --git a/tests/system-dpdk-macros.at b/tests/system-dpdk-macros.at
> index c6708ca..9e55f10 100644
> --- a/tests/system-dpdk-macros.at
> +++ b/tests/system-dpdk-macros.at
> @@ -33,13 +33,11 @@ m4_define([OVS_DPDK_PRE_PHY_SKIP],
>  ])
>  
>  
> -# OVS_DPDK_START()
> +# OVS_DB_START()
>  #
> -# Create an empty database and start ovsdb-server. Add special configuration
> -# dpdk-init to enable DPDK functionality. Start ovs-vswitchd connected to 
> that
> -# database using system devices (no dummies).
> +# Create an empty database and start ovsdb-server.
>  #
> -m4_define([OVS_DPDK_START],
> +m4_define([OVS_DB_START],
>[dnl Create database.
> AT_CHECK([touch .conf.db.~lock~])
> AT_CHECK([ovsdb-tool create conf.db 
> $abs_top_srcdir/vswitchd/vswitch.ovsschema])
> @@ -54,7 +52,16 @@ m4_define([OVS_DPDK_START],
>  
> dnl Initialize database.
> AT_CHECK([ovs-vsctl --no-wait init])
> +])
> +
>  
> +# OVS_DPDK_START()
> +#
> +# Add special configuration dpdk-init to enable DPDK functionality.
> +# Start ovs-vswitchd connected to that database using system devices (no 
> dummies).
> +#
> +m4_define([OVS_DPDK_START],
> +  [
> dnl Enable DPDK functionality
> AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch . 
> other_config:dpdk-init=true])
>  
> diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
> index a015d52..ef03fb2 100644
> --- a/tests/system-dpdk.at
> +++ b/tests/system-dpdk.at
> @@ -1,6 +1,11 @@
>  m4_define([CONFIGURE_VETH_OFFLOADS],
> [AT_CHECK([ethtool -K $1 tx off], [0], [ignore], [ignore])])
>  
> +m4_define([SET_NUMA_NODE],
> +   [
> +AT_CHECK([lscpu | awk '/NUMA node\(s\)/ {c=1; while (c++<$(3)) {printf 
> "$1,"}; print "$1"}' > NUMA_NODE])
> +])
> +
>  AT_BANNER([OVS-DPDK unit tests])
>  
>  dnl 
> --
> @@ -8,6 +13,7 @@ dnl Check if EAL init is successful
>  AT_SETUP([OVS-DPDK - EAL init])
>  AT_KEYWORDS([dpdk])
>  OVS_DPDK_PRE_CHECK()
> +OVS_DB_START()
>  OVS_DPDK_START()
>  AT_CHECK([grep "DPDK Enabled - initializing..." ovs-vswitchd.log], [], 
> [stdout])
>  AT_CHECK([grep "EAL" ovs-vswitchd.log], [], [stdout])
> @@ -15,6 +21,7 @@ AT_CHECK([grep "DPDK Enabled - initialized" 
> ovs-vswitchd.log], [], [stdout])
>  OVS_VSWITCHD_STOP(["/Global register is changed during/d
>  /EAL:   Invalid NUMA socket, default to 0/d
>  /EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable 
> clock cycles !/d
> +/EAL: No available hugepages reported in hugepages-1048576kB/d
>  /EAL: No free hugepages reported in hugepages-1048576kB/d"])
>  AT_CLEANUP
>  dnl 
> --

The addition of the hugepages message in the filter is good
in this and other tests as well, but it should be on a
separate patch altogether because this one is only about
tso negotiation, correct?

Otherwise it looks good to me.
Thanks
fbl




> @@ -27,6 +34,7 @@ AT_SETUP([OVS-DPDK - add standard DPDK port])
>  AT_KEYWORDS([dpdk])
>  
>  OVS_DPDK_PRE_PHY_SKIP()
> +OVS_DB_START()
>  OVS_DPDK_START()
>  
>  dnl Add userspace bridge and attach it to OVS
> @@ -41,6 +49,7 @@ OVS_VSWITCHD_STOP("/does not exist. The Open vSwitch kernel 
> module is probably n
>  /Failed to enable flow control/d
>  /Global register is changed during/d
>  /EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable 
> clock cycles !/d
> +/EAL: No available hugepages reported in hugepages-

[ovs-dev] お支払い方法の情報を更新

2020-07-09 Thread amazon via dev
お支払い方法の情報を更新してください。Update default card for your membership.

 
 マイストア? タイムセール? ギフト券 

 


Amazonプライムをご利用頂きありがとうございます。お客様のAmazonプライム会員資格は、2020/07/09に更新を迎えます。お調べしたところ、会費のお支払いに使用できる有効なクレジットカードがアカウントに登録されていません。クレジットカード情報の更新、新しいクレジットカードの追加については以下の手順をご確認ください。



1. アカウントサービスからAmazonプライム会員情報を管理するにアクセスします。


2. Amazonプライムに登録したAmazon.co.jpのアカウントを使用してサインインします。


3. 左側に表示されている「現在の支払方法」の下にある「支払方法を変更する」のリンクをクリックします。


4. 有効期限の更新または新しいクレジットカード情報を入力してください。



Amazonプライムを継続してご利用いただくために、会費のお支払いにご指定いただいたクレジットカードが使用できない場合は、アカウントに登録されている別 
のクレジットカードに会費を請求させて頂きます。会費の請求が出来ない場合は、お客様のAmazonプライム会員資格は失効し、特典をご利用できなくなります。


Amazon.co.jpカスタマーサービス 




 
支払方法の情報を更新する 



 

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


[ovs-dev] [PATCH ovn v2] Fix the routing for external logical ports of bridged logical switches.

2020-07-09 Thread numans
From: Numan Siddique 

Routing for external logical ports is broken if these ports belonged
to bridged logical switches (with localnet port) and 'ovn-chassis-mac-mappings'
is configured. External logical ports are those which are external to OVN,
but there is a logical port for it and it is claimed by one of the HA chassis.
The claimed chassis provides routing and other native OVN serices like dhcp and 
dns.

When the external port sends ARP request for the router IP, the claimed chassis
replies for the ARP request, but the arp.sha is set to the actual router mac 
instead
of the chassis mac. This causes the traffic from external port VM/container to 
be handled
incorrectly. A ping to the router ip, is replied by all the chassis which can 
see this
packet instead of just the claimed HA chassis.

To fix this, this patch does 2 things.

1. In the table - OFTABLE_LOG_TO_PHY (65), it adds a 160 priority flow to
   modify the ARP packets arp.sha to store the chassis mac.

2. And when the packet destined to the chassis mac is received, it replaces the
   chassis mac with the actual router mac in table 0.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1829762
Reported-by: Daniel Alvarez 
CC: Ankur Sharma 
Signed-off-by: Numan Siddique 
---

v1 -> v2

  * Rebased.

 controller/chassis.c  |  48 --
 controller/chassis.h  |   2 +
 controller/physical.c | 145 +++---
 tests/ovn.at  | 131 ++
 4 files changed, 299 insertions(+), 27 deletions(-)

diff --git a/controller/chassis.c b/controller/chassis.c
index eec270ea39..25146d75f2 100644
--- a/controller/chassis.c
+++ b/controller/chassis.c
@@ -645,10 +645,11 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
 }
 
 bool
-chassis_get_mac(const struct sbrec_chassis *chassis_rec,
-const char *bridge_mapping,
-struct eth_addr *chassis_mac)
+chassis_get_mac_mappings(const struct sbrec_chassis *chassis_rec,
+ struct smap *chassis_mappings)
 {
+smap_init(chassis_mappings);
+
 const char *tokens
 = get_chassis_mac_mappings(&chassis_rec->other_config);
 if (!tokens[0]) {
@@ -656,7 +657,6 @@ chassis_get_mac(const struct sbrec_chassis *chassis_rec,
 }
 
 char *save_ptr = NULL;
-bool ret = false;
 char *tokstr = xstrdup(tokens);
 
 /* Format for a chassis mac configuration is:
@@ -669,24 +669,36 @@ chassis_get_mac(const struct sbrec_chassis *chassis_rec,
 char *chassis_mac_bridge = strtok_r(token, ":", &save_ptr2);
 char *chassis_mac_str = strtok_r(NULL, "", &save_ptr2);
 
-if (!strcmp(chassis_mac_bridge, bridge_mapping)) {
-struct eth_addr temp_mac;
+smap_replace(chassis_mappings, chassis_mac_bridge, chassis_mac_str);
+}
 
-/* Return the first chassis mac. */
-char *err_str = str_to_mac(chassis_mac_str, &temp_mac);
-if (err_str) {
-free(err_str);
-continue;
-}
+free(tokstr);
+return true;
+}
 
-ret = true;
-*chassis_mac = temp_mac;
-break;
-}
+bool
+chassis_get_mac(const struct sbrec_chassis *chassis_rec,
+const char *bridge_mapping,
+struct eth_addr *chassis_mac)
+{
+struct smap chassis_mappings;
+
+if (!chassis_get_mac_mappings(chassis_rec, &chassis_mappings)) {
+return false;
 }
 
-free(tokstr);
-return ret;
+const char *chassis_mac_str = smap_get_def(&chassis_mappings,
+   bridge_mapping, "");
+struct eth_addr temp_mac;
+
+char *err_str = str_to_mac(chassis_mac_str, &temp_mac);
+if (err_str) {
+free(err_str);
+return false;
+}
+
+*chassis_mac = temp_mac;
+return true;
 }
 
 /* Returns true if the database is all cleaned up, false if more work is
diff --git a/controller/chassis.h b/controller/chassis.h
index 178d2957e8..dae761312d 100644
--- a/controller/chassis.h
+++ b/controller/chassis.h
@@ -42,6 +42,8 @@ bool chassis_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn,
 bool chassis_get_mac(const struct sbrec_chassis *chassis,
  const char *bridge_mapping,
  struct eth_addr *chassis_mac);
+bool chassis_get_mac_mappings(const struct sbrec_chassis *,
+  struct smap *chassis_mappings);
 const char *chassis_get_id(void);
 const char * get_chassis_mac_mappings(const struct smap *ext_ids);
 
diff --git a/controller/physical.c b/controller/physical.c
index 6d7d8e93bc..b43a157b94 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -62,7 +62,8 @@ load_logical_ingress_metadata(const struct sbrec_port_binding 
*binding,
 /* UUID to identify OF flows not associated with ovsdb rows. */
 static struct uuid *hc_uuid = NULL;
 
-#define CHASSIS_MAC_TO_ROUTER_MAC_CONJID100
+#define CHASSIS_MA

Re: [ovs-dev] [PATCH v5 3/7] dpif-netdev: Skip encap action during datapath execution

2020-07-09 Thread 0-day Robot
Bleep bloop.  Greetings Sriharsha Basavapatna, 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.


build:
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. 
   -I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall 
-Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security 
-Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align 
-Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes 
-Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g 
-O2 -MT lib/dpctl.lo -MD -MP -MF $depbase.Tpo -c -o lib/dpctl.lo lib/dpctl.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include 
-I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare 
-Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter 
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition 
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow 
-Werror -Werror -g -O2 -MT lib/dpctl.lo -MD -MP -MF lib/.deps/dpctl.Tpo -c 
lib/dpctl.c -o lib/dpctl.o
depbase=`echo lib/dp-packet.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. 
   -I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall 
-Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security 
-Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align 
-Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes 
-Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g 
-O2 -MT lib/dp-packet.lo -MD -MP -MF $depbase.Tpo -c -o lib/dp-packet.lo 
lib/dp-packet.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include 
-I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare 
-Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter 
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition 
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow 
-Werror -Werror -g -O2 -MT lib/dp-packet.lo -MD -MP -MF lib/.deps/dp-packet.Tpo 
-c lib/dp-packet.c -o lib/dp-packet.o
depbase=`echo lib/dpif-netdev-lookup-generic.lo | sed 
's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. 
   -I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall 
-Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security 
-Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align 
-Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes 
-Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g 
-O2 -MT lib/dpif-netdev-lookup-generic.lo -MD -MP -MF $depbase.Tpo -c -o 
lib/dpif-netdev-lookup-generic.lo lib/dpif-netdev-lookup-generic.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include 
-I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare 
-Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter 
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition 
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow 
-Werror -Werror -g -O2 -MT lib/dpif-netdev-lookup-generic.lo -MD -MP -MF 
lib/.deps/dpif-netdev-lookup-generic.Tpo -c lib/dpif-netdev-lookup-generic.c -o 
lib/dpif-netdev-lookup-generic.o
depbase=`echo lib/dpif-netdev.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. 
   -I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall 
-Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security 
-Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align 
-Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes 
-Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g 
-O2 -MT lib/dpif-netdev.lo -MD -MP -MF $depbase.Tpo -c -o lib/dpif-netdev.lo 
lib/dpif-netdev.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include 
-I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare 
-Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter 
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition 
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow 
-Werror -Werror -g -O2 -MT lib/dpif-netdev.lo -MD -MP -MF 
lib/.deps/dpif-netdev.Tpo -c lib/dpif-netdev.c -o lib/dpif-netdev.o
lib/dpif-netdev.c: In function 'dp_execute_cb':
lib/dpif-netdev.c:7493:38: error: initialization discards 'const' qualifier 
from pointer target type [-Werror]