Re: [ovs-dev] [PATCH RFC 5/5] Tunnel: Add self tests for MAC learning and ageing.

2021-10-25 Thread Flavio Leitner 
On Thu, Oct 07, 2021 at 02:35:40PM +0200, Paolo Valerio wrote:
> Tests for both ipv4 and ipv6 have been added.

Thanks for writing the unit tests.
Could you please add them as part of the patch adding the commands?

Thanks
fbl

> 
> Signed-off-by: Paolo Valerio 
> ---
>  tests/tunnel-push-pop-ipv6.at |   66 
> +
>  tests/tunnel-push-pop.at  |   65 
>  2 files changed, 131 insertions(+)
> 
> diff --git a/tests/tunnel-push-pop-ipv6.at b/tests/tunnel-push-pop-ipv6.at
> index 59723e63b..5c4dd248b 100644
> --- a/tests/tunnel-push-pop-ipv6.at
> +++ b/tests/tunnel-push-pop-ipv6.at
> @@ -255,6 +255,36 @@ AT_CHECK([cat p0.pcap.txt | grep 
> 93aa55aa5586dd60203aff2001cafe | un
>  
> ff93aa55aa5586dd60203aff2001cafe0088ff020001ff9387004d462001cafe00930101aa55aa55
>  ])
>  
> +dnl Set the ageing time to 5 seconds
> +AT_CHECK([ovs-appctl tnl/neigh/ageing 5], [0], [OK
> +])
> +
> +dnl Read the current ageing time
> +AT_CHECK([ovs-appctl tnl/neigh/ageing], [0], [5
> +])
> +
> +dnl Add an entry
> +AT_CHECK([ovs-appctl tnl/neigh/set br0 2001:cafe::92 aa:bb:cc:00:00:01], 
> [0], [OK
> +])
> +
> +AT_CHECK([ovs-appctl tnl/neigh/show | grep br0 | sort], [0], [dnl
> +2001:cafe::92 aa:bb:cc:00:00:01   br0
> +])
> +
> +ovs-appctl time/warp 5000
> +
> +dnl Check the entry has been removed
> +AT_CHECK([ovs-appctl tnl/neigh/show | grep br0 | sort], [0], [dnl
> +])
> +
> +dnl Restore the ageing time to 900s (default)
> +AT_CHECK([ovs-appctl tnl/neigh/ageing 900], [0], [OK
> +])
> +
> +dnl Read the current ageing time
> +AT_CHECK([ovs-appctl tnl/neigh/ageing], [0], [900
> +])
> +
>  dnl Check ARP Snoop
>  AT_CHECK([ovs-appctl netdev-dummy/receive p0 
> 'in_port(1),eth(src=f8:bc:12:44:34:c8,dst=aa:55:aa:55:00:00),eth_type(0x86dd),ipv6(src=2001:cafe::92,dst=2001:cafe::88,label=0,proto=58,tclass=0,hlimit=255,frag=no),icmpv6(type=136,code=0),nd(target=2001:cafe::92,sll=00:00:00:00:00:00,tll=f8:bc:12:44:34:c8)'])
>  
> @@ -432,6 +462,42 @@ AT_CHECK([ovs-appctl dpif/dump-flows int-br | grep 
> 'in_port(6081)'], [0], [dnl
>  
> tunnel(tun_id=0x7b,ipv6_src=2001:cafe::92,ipv6_dst=2001:cafe::88,geneve({class=0x,type=0x80,len=4,0xa/0xf}{class=0x,type=0,len=4}),flags(-df-csum+key)),recirc_id(0),in_port(6081),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
>  packets:0, bytes:0, used:never, 
> actions:userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=3,rule_cookie=0,controller_id=0,max_len=65535))
>  ])
>  
> +dnl Receive VXLAN with different MAC and verify that the neigh cache gets 
> updated
> +AT_CHECK([ovs-appctl netdev-dummy/receive p0 
> 'aa55aa55f8bc1244cafe86dd603a11402001cafe00922001cafe0088c85312b5003abc700c037b000800451c000140117cce7f017f010035003500080172'])
> +
> +ovs-appctl time/warp 1000
> +ovs-appctl time/warp 1000
> +
> +dnl Check VXLAN tunnel push
> +AT_CHECK([ovs-ofctl add-flow int-br action=2])
> +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
> 'in_port(2),eth(src=36:b1:ee:7c:01:01,dst=36:b1:ee:7c:01:02),eth_type(0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)'],
>  [0], [stdout])
> +AT_CHECK([tail -1 stdout], [0],
> +  [Datapath actions: 
> clone(tnl_push(tnl_port(4789),header(size=70,type=4,eth(dst=f8:bc:12:44:ca:fe,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=2001:cafe::92,label=0,proto=17,tclass=0x0,hlimit=64),udp(src=0,dst=4789,csum=0x),vxlan(flags=0x800,vni=0x7b)),out_port(100)),1)
> +])
> +
> +AT_CHECK([ovs-appctl tnl/arp/show | tail -n+3 | sort], [0], [dnl
> +2001:cafe::92 f8:bc:12:44:ca:fe   br0
> +2001:cafe::93 f8:bc:12:44:34:b7   br0
> +])
> +
> +dnl Restore and check the cache entries
> +AT_CHECK([ovs-appctl netdev-dummy/receive p0 
> 'aa55aa55f8bc124434b686dd603a11402001cafe00922001cafe0088c85312b5003abc700c037b000800451c000140117cce7f017f010035003500080172'])
> +
> +ovs-appctl time/warp 1000
> +ovs-appctl time/warp 1000
> +
> +dnl Check VXLAN tunnel push
> +AT_CHECK([ovs-ofctl add-flow int-br action=2])
> +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
> 'in_port(2),eth(src=36:b1:ee:7c:01:01,dst=36:b1:ee:7c:01:02),eth_type(0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)'],
>  [0], [stdout])
> +AT_CHECK([tail -1 stdout], [0],
> +  [Datapath actions: 
> clone(tnl_push(tnl_port(4789),header(size=70,type=4,eth(dst=f8:bc:12:44:34:b6,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=2001:cafe::92,label=0,proto=17,tclass=0x0,hlimit=64),udp(src=0,dst=4789,csum=0x),vxlan(flags=0x800,vni=0x7b)),out_port(100)),1)
>

Re: [ovs-dev] [PATCH RFC 4/5] Native tunnel: Do not refresh the entry while revalidating.

2021-10-25 Thread Flavio Leitner 
Hi Paolo,

On Thu, Oct 07, 2021 at 02:35:34PM +0200, Paolo Valerio wrote:
> This is a minor issue but visible e.g. when you try to flush the neigh
> cache while the ARP flow is still present in the datapath, triggering
> the revalidation of the datapath flows which subsequntly

Typo

> refreshes/adds the entry in the cache.

Otherwise it looks ok.
fbl


> 
> Signed-off-by: Paolo Valerio 
> ---
>  lib/tnl-neigh-cache.c|   20 +---
>  lib/tnl-neigh-cache.h|2 +-
>  ofproto/ofproto-dpif-xlate.c |3 ++-
>  3 files changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/tnl-neigh-cache.c b/lib/tnl-neigh-cache.c
> index 9d3f00ad9..df8de48eb 100644
> --- a/lib/tnl-neigh-cache.c
> +++ b/lib/tnl-neigh-cache.c
> @@ -173,7 +173,7 @@ tnl_arp_set(const char name[IFNAMSIZ], ovs_be32 dst,
>  
>  static int
>  tnl_arp_snoop(const struct flow *flow, struct flow_wildcards *wc,
> -  const char name[IFNAMSIZ])
> +  const char name[IFNAMSIZ], bool update)
>  {
>  /* Snoop normal ARP replies and gratuitous ARP requests/replies only */
>  if (!is_arp(flow)
> @@ -183,13 +183,17 @@ tnl_arp_snoop(const struct flow *flow, struct 
> flow_wildcards *wc,
>  return EINVAL;
>  }
>  
> -tnl_arp_set(name, FLOW_WC_GET_AND_MASK_WC(flow, wc, nw_src), 
> flow->arp_sha);
> +memset(>masks.nw_src, 0xff, sizeof wc->masks.nw_src);
> +
> +if (update) {
> +tnl_arp_set(name, flow->nw_src, flow->arp_sha);
> +}
>  return 0;
>  }
>  
>  static int
>  tnl_nd_snoop(const struct flow *flow, struct flow_wildcards *wc,
> - const char name[IFNAMSIZ])
> + const char name[IFNAMSIZ], bool update)
>  {
>  if (!is_nd(flow, wc) || flow->tp_src != htons(ND_NEIGHBOR_ADVERT)) {
>  return EINVAL;
> @@ -208,20 +212,22 @@ tnl_nd_snoop(const struct flow *flow, struct 
> flow_wildcards *wc,
>  memset(>masks.ipv6_dst, 0xff, sizeof wc->masks.ipv6_dst);
>  memset(>masks.nd_target, 0xff, sizeof wc->masks.nd_target);
>  
> -tnl_neigh_set(name, >nd_target, flow->arp_tha);
> +if (update) {
> +tnl_neigh_set(name, >nd_target, flow->arp_tha);
> +}
>  return 0;
>  }
>  
>  int
>  tnl_neigh_snoop(const struct flow *flow, struct flow_wildcards *wc,
> -const char name[IFNAMSIZ])
> +const char name[IFNAMSIZ], bool update)
>  {
>  int res;
> -res = tnl_arp_snoop(flow, wc, name);
> +res = tnl_arp_snoop(flow, wc, name, update);
>  if (res != EINVAL) {
>  return res;
>  }
> -return tnl_nd_snoop(flow, wc, name);
> +return tnl_nd_snoop(flow, wc, name, update);
>  }
>  
>  void
> diff --git a/lib/tnl-neigh-cache.h b/lib/tnl-neigh-cache.h
> index 92fdf5a93..a2fd9f4ae 100644
> --- a/lib/tnl-neigh-cache.h
> +++ b/lib/tnl-neigh-cache.h
> @@ -32,7 +32,7 @@
>  #include "util.h"
>  
>  int tnl_neigh_snoop(const struct flow *flow, struct flow_wildcards *wc,
> -const char dev_name[IFNAMSIZ]);
> +const char dev_name[IFNAMSIZ], bool update);
>  void
>  tnl_neigh_set(const char name[IFNAMSIZ], const struct in6_addr *dst,
>const struct eth_addr mac);
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 4430ac073..44a49dae8 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -4097,7 +4097,8 @@ terminate_native_tunnel(struct xlate_ctx *ctx, struct 
> flow *flow,
>  (flow->dl_type == htons(ETH_TYPE_ARP) ||
>   flow->nw_proto == IPPROTO_ICMPV6) &&
>   is_neighbor_reply_correct(ctx, flow)) {
> -tnl_neigh_snoop(flow, wc, ctx->xbridge->name);
> +tnl_neigh_snoop(flow, wc, ctx->xbridge->name,
> +ctx->xin->allow_side_effects);
>  } else if (*tnl_port != ODPP_NONE &&
> ctx->xin->allow_side_effects &&
> (flow->dl_type == htons(ETH_TYPE_IP) ||
> 
> ___
> 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 RFC 3/5] Tunnel: Snoop ingress packets and update neigh cache if needed.

2021-10-25 Thread Flavio Leitner 
On Thu, Oct 07, 2021 at 02:35:28PM +0200, Paolo Valerio wrote:
> In case of native tunnel with bfd enabled, if the MAC address of the
> remote end's interface changes (e.g. because it got rebooted, and the
> MAC address is allocated dinamically), the BFD session will never be
> re-established.
> 
> This happens because the local tunnel neigh entry doesn't get updated,
> and the local end keeps sending BFD packets with the old destination
> MAC address. This was not an issue until
> b23ddcc57d41 ("tnl-neigh-cache: tighten arp and nd snooping.")
> because ARP requests were snooped as well avoiding the problem.
> 
> Fix this by snooping the incoming packets, and updating the neigh
> cache accordingly.


Can we add a mention that this only affects slow path?
Otherwise it may suggests a performance impact.


> Signed-off-by: Paolo Valerio 
> Fixes: b23ddcc57d41 ("tnl-neigh-cache: tighten arp and nd snooping.")
> ---
>  lib/tnl-neigh-cache.c|   12 ++--
>  lib/tnl-neigh-cache.h|3 +++
>  ofproto/ofproto-dpif-xlate.c |   14 ++
>  3 files changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/tnl-neigh-cache.c b/lib/tnl-neigh-cache.c
> index c8a7b60cd..9d3f00ad9 100644
> --- a/lib/tnl-neigh-cache.c
> +++ b/lib/tnl-neigh-cache.c
> @@ -135,9 +135,9 @@ tnl_neigh_delete(struct tnl_neigh_entry *neigh)
>  ovsrcu_postpone(neigh_entry_free, neigh);
>  }
>  
> -static void
> -tnl_neigh_set__(const char name[IFNAMSIZ], const struct in6_addr *dst,
> -const struct eth_addr mac)
> +void
> +tnl_neigh_set(const char name[IFNAMSIZ], const struct in6_addr *dst,
> +  const struct eth_addr mac)
>  {
>  ovs_mutex_lock();
>  struct tnl_neigh_entry *neigh = tnl_neigh_lookup__(name, dst);
> @@ -168,7 +168,7 @@ tnl_arp_set(const char name[IFNAMSIZ], ovs_be32 dst,
>  const struct eth_addr mac)
>  {
>  struct in6_addr dst6 = in6_addr_mapped_ipv4(dst);
> -tnl_neigh_set__(name, , mac);
> +tnl_neigh_set(name, , mac);
>  }
>  
>  static int
> @@ -208,7 +208,7 @@ tnl_nd_snoop(const struct flow *flow, struct 
> flow_wildcards *wc,
>  memset(>masks.ipv6_dst, 0xff, sizeof wc->masks.ipv6_dst);
>  memset(>masks.nd_target, 0xff, sizeof wc->masks.nd_target);
>  
> -tnl_neigh_set__(name, >nd_target, flow->arp_tha);
> +tnl_neigh_set(name, >nd_target, flow->arp_tha);
>  return 0;
>  }
>  
> @@ -355,7 +355,7 @@ tnl_neigh_cache_add(struct unixctl_conn *conn, int argc 
> OVS_UNUSED,
>  return;
>  }
>  
> -tnl_neigh_set__(br_name, , mac);
> +tnl_neigh_set(br_name, , mac);
>  unixctl_command_reply(conn, "OK");
>  }
>  
> diff --git a/lib/tnl-neigh-cache.h b/lib/tnl-neigh-cache.h
> index e4b42b059..92fdf5a93 100644
> --- a/lib/tnl-neigh-cache.h
> +++ b/lib/tnl-neigh-cache.h
> @@ -33,6 +33,9 @@
>  
>  int tnl_neigh_snoop(const struct flow *flow, struct flow_wildcards *wc,
>  const char dev_name[IFNAMSIZ]);
> +void
> +tnl_neigh_set(const char name[IFNAMSIZ], const struct in6_addr *dst,
> +  const struct eth_addr mac);
>  int tnl_neigh_lookup(const char dev_name[IFNAMSIZ], const struct in6_addr 
> *dst,
>   struct eth_addr *mac);
>  void tnl_neigh_cache_init(void);
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 8723cb4e8..4430ac073 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -4098,6 +4098,20 @@ terminate_native_tunnel(struct xlate_ctx *ctx, struct 
> flow *flow,
>   flow->nw_proto == IPPROTO_ICMPV6) &&
>   is_neighbor_reply_correct(ctx, flow)) {
>  tnl_neigh_snoop(flow, wc, ctx->xbridge->name);
> +} else if (*tnl_port != ODPP_NONE &&
> +   ctx->xin->allow_side_effects &&
> +   (flow->dl_type == htons(ETH_TYPE_IP) ||
> +flow->dl_type == htons(ETH_TYPE_IPV6))) {
> +struct eth_addr mac = flow->dl_src;
> +struct in6_addr s_ip6;
> +
> +if (flow->nw_src) {

I don't think we will have zeros as valid source IP addr at this
point, but this looks odd. Why not repeating the same check?
   if (flow->dl_type == htons(ETH_TYPE_IP)) {


> +in6_addr_set_mapped_ipv4(_ip6, flow->nw_src);
> +} else {
> +s_ip6 = flow->ipv6_src;
> +}
> +
> +tnl_neigh_set(ctx->xbridge->name, _ip6, mac);
>  }
>  }
>  
> 
> ___
> 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 RFC 2/5] Native tunnel: Add tnl/neigh/ageing command.

2021-10-25 Thread Flavio Leitner 
On Thu, Oct 07, 2021 at 02:35:21PM +0200, Paolo Valerio wrote:
> with the command is now possible to change the ageing time of the
> cache entries.

Please start with a normal sentence using a capital letter.

> For the existing entries the ageing time is updated only if the
> current expiration is greater than the new one. In any case, the next
> refresh will set it to the new value.
> 
> This is intended mostly for debugging purpose.
> 
> Signed-off-by: Paolo Valerio 
> ---
>  NEWS|3 ++
>  lib/tnl-neigh-cache.c   |   77 
> ++-
>  ofproto/ofproto-tnl-unixctl.man |5 +++
>  3 files changed, 76 insertions(+), 9 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index 90f4b1590..148dd5d61 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -10,6 +10,9 @@ Post-v2.16.0
> limiting behavior.
>   * Add hardware offload support for matching IPv4/IPv6 frag types
> (experimental).
> +   - Native tunnel:
> + * Added new ovs-appctl tnl/neigh/ageing to read/write the neigh ageing
> +   time.
>  
>  
>  v2.16.0 - 16 Aug 2021
> diff --git a/lib/tnl-neigh-cache.c b/lib/tnl-neigh-cache.c
> index a37456e6d..c8a7b60cd 100644
> --- a/lib/tnl-neigh-cache.c
> +++ b/lib/tnl-neigh-cache.c
> @@ -42,6 +42,7 @@
>  #include "unixctl.h"
>  #include "util.h"
>  #include "openvswitch/vlog.h"
> +#include "ovs-atomic.h"

The previous patch is using atomic ops, so this shouldn't be needed
here.

>  
>  
>  /* In milliseconds */
> @@ -57,6 +58,7 @@ struct tnl_neigh_entry {
>  
>  static struct cmap table = CMAP_INITIALIZER;
>  static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER;
> +static atomic_uint32_t neigh_ageing;
>  
>  static uint32_t
>  tnl_neigh_hash(const struct in6_addr *ip)
> @@ -74,6 +76,15 @@ tnl_neigh_expired(struct tnl_neigh_entry *neigh)
>  return expired <= time_msec();
>  }
>  
> +static uint32_t
> +tnl_neigh_get_ageing(void)
> +{
> +unsigned int ageing;
> +
> +atomic_read_relaxed(_ageing, );
> +return ageing;
> +}
> +
>  static struct tnl_neigh_entry *
>  tnl_neigh_lookup__(const char br_name[IFNAMSIZ], const struct in6_addr *dst)
>  {
> @@ -88,7 +99,7 @@ tnl_neigh_lookup__(const char br_name[IFNAMSIZ], const 
> struct in6_addr *dst)
>  }
>  
>  atomic_store_relaxed(>expires, time_msec() +
> - NEIGH_ENTRY_DEFAULT_IDLE_TIME);
> + tnl_neigh_get_ageing());
>  return neigh;
>  }
>  }
> @@ -133,7 +144,7 @@ tnl_neigh_set__(const char name[IFNAMSIZ], const struct 
> in6_addr *dst,
>  if (neigh) {
>  if (eth_addr_equals(neigh->mac, mac)) {
>  atomic_store_relaxed(>expires, time_msec() +
> - NEIGH_ENTRY_DEFAULT_IDLE_TIME);
> + tnl_neigh_get_ageing());
>  ovs_mutex_unlock();
>  return;
>  }
> @@ -146,7 +157,7 @@ tnl_neigh_set__(const char name[IFNAMSIZ], const struct 
> in6_addr *dst,
>  neigh->ip = *dst;
>  neigh->mac = mac;
>  atomic_store_relaxed(>expires, time_msec() +
> - NEIGH_ENTRY_DEFAULT_IDLE_TIME);
> + tnl_neigh_get_ageing());
>  ovs_strlcpy(neigh->br_name, name, sizeof neigh->br_name);
>  cmap_insert(, >cmap_node, tnl_neigh_hash(>ip));
>  ovs_mutex_unlock();
> @@ -272,6 +283,43 @@ tnl_neigh_cache_flush(struct unixctl_conn *conn, int 
> argc OVS_UNUSED,
>  unixctl_command_reply(conn, "OK");
>  }
>  
> +static void
> +tnl_neigh_cache_ageing(struct unixctl_conn *conn, int argc,
> +const char *argv[], void *aux OVS_UNUSED)
> +{
> +long long int new_exp, curr_exp;
> +struct tnl_neigh_entry *neigh;
> +uint32_t ageing;
> +
> +if (argc == 1) {
> +struct ds ds = DS_EMPTY_INITIALIZER;
> +ds_put_format(, "%u", tnl_neigh_get_ageing() / 1000);

Shouldn't that be PRIu32?

> +unixctl_command_reply(conn, ds_cstr());
> +ds_destroy();
> +
> +return;
> +}
> +
> +if (!ovs_scan(argv[1], "%"SCNu32, ) ||
> +!ageing || ageing > 1) {

Why 1? Perhaps it needs to be defined and mentioned in the
documentation. I suggest 3600 secs (1 hour) as an arbitrary upper
limit.

> +unixctl_command_reply_error(conn, "bad ageing value");
> +return;
> +}
> +
> +ageing *= 1000;
> +atomic_store_relaxed(_ageing, ageing);
> +new_exp = time_msec() + ageing;
> +
> +CMAP_FOR_EACH (neigh, cmap_node, ) {
> +atomic_read_relaxed(>expires, _exp);
> +if (new_exp < curr_exp) {
> +atomic_store_relaxed(>expires, new_exp);
> +}
> +}
> +
> +unixctl_command_reply(conn, "OK");
> +}
> +
>  static int
>  lookup_any(const char *host_name, struct in6_addr *address)
>  {
> @@ -346,10 +394,21 @@ tnl_neigh_cache_show(struct unixctl_conn *conn, int 
> argc OVS_UNUSED,
>  void
>

Re: [ovs-dev] [PATCH RFC 1/5] Native tunnel: Read/write expires atomically.

2021-10-25 Thread Flavio Leitner 


Hi Paolo,

The lookup does not change cmap, but it changes the entry which can
be used by multiple threads. In that case, we would need a mutex to
modify the entry. However, in this specific case only 'expires' is
required to change, and other fields are static. Therefore, going
with atomic op makes sense to me.

Since you're using atomic op, it would be great to include
"ovs-atomic.h", though it is indirectly included by thread
or rcu headers.

What do you think?

fbl


On Thu, Oct 07, 2021 at 02:35:15PM +0200, Paolo Valerio wrote:
> Signed-off-by: Paolo Valerio 
> ---
>  lib/tnl-neigh-cache.c |   31 ++-
>  1 file changed, 22 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/tnl-neigh-cache.c b/lib/tnl-neigh-cache.c
> index 5bda4af7e..a37456e6d 100644
> --- a/lib/tnl-neigh-cache.c
> +++ b/lib/tnl-neigh-cache.c
> @@ -44,14 +44,14 @@
>  #include "openvswitch/vlog.h"
>  
>  
> -/* In seconds */
> -#define NEIGH_ENTRY_DEFAULT_IDLE_TIME  (15 * 60)
> +/* In milliseconds */
> +#define NEIGH_ENTRY_DEFAULT_IDLE_TIME  (15 * 60 * 1000)
>  
>  struct tnl_neigh_entry {
>  struct cmap_node cmap_node;
>  struct in6_addr ip;
>  struct eth_addr mac;
> -time_t expires; /* Expiration time. */
> +atomic_llong expires;   /* Expiration time in ms. */
>  char br_name[IFNAMSIZ];
>  };
>  
> @@ -64,6 +64,16 @@ tnl_neigh_hash(const struct in6_addr *ip)
>  return hash_bytes(ip->s6_addr, 16, 0);
>  }
>  
> +static bool
> +tnl_neigh_expired(struct tnl_neigh_entry *neigh)
> +{
> +long long expired;
> +
> +atomic_read_relaxed(>expires, );
> +
> +return expired <= time_msec();
> +}
> +
>  static struct tnl_neigh_entry *
>  tnl_neigh_lookup__(const char br_name[IFNAMSIZ], const struct in6_addr *dst)
>  {
> @@ -73,11 +83,12 @@ tnl_neigh_lookup__(const char br_name[IFNAMSIZ], const 
> struct in6_addr *dst)
>  hash = tnl_neigh_hash(dst);
>  CMAP_FOR_EACH_WITH_HASH (neigh, cmap_node, hash, ) {
>  if (ipv6_addr_equals(>ip, dst) && !strcmp(neigh->br_name, 
> br_name)) {
> -if (neigh->expires <= time_now()) {
yy> +if (tnl_neigh_expired(neigh)) {
>  return NULL;
>  }
>  
> -neigh->expires = time_now() + NEIGH_ENTRY_DEFAULT_IDLE_TIME;
> +atomic_store_relaxed(>expires, time_msec() +
> + NEIGH_ENTRY_DEFAULT_IDLE_TIME);
>  return neigh;
>  }
>  }
> @@ -121,7 +132,8 @@ tnl_neigh_set__(const char name[IFNAMSIZ], const struct 
> in6_addr *dst,
>  struct tnl_neigh_entry *neigh = tnl_neigh_lookup__(name, dst);
>  if (neigh) {
>  if (eth_addr_equals(neigh->mac, mac)) {
> -neigh->expires = time_now() + NEIGH_ENTRY_DEFAULT_IDLE_TIME;
> +atomic_store_relaxed(>expires, time_msec() +
> + NEIGH_ENTRY_DEFAULT_IDLE_TIME);
>  ovs_mutex_unlock();
>  return;
>  }
> @@ -133,7 +145,8 @@ tnl_neigh_set__(const char name[IFNAMSIZ], const struct 
> in6_addr *dst,
>  
>  neigh->ip = *dst;
>  neigh->mac = mac;
> -neigh->expires = time_now() + NEIGH_ENTRY_DEFAULT_IDLE_TIME;
> +atomic_store_relaxed(>expires, time_msec() +
> + NEIGH_ENTRY_DEFAULT_IDLE_TIME);
>  ovs_strlcpy(neigh->br_name, name, sizeof neigh->br_name);
>  cmap_insert(, >cmap_node, tnl_neigh_hash(>ip));
>  ovs_mutex_unlock();
> @@ -208,7 +221,7 @@ tnl_neigh_cache_run(void)
>  
>  ovs_mutex_lock();
>  CMAP_FOR_EACH(neigh, cmap_node, ) {
> -if (neigh->expires <= time_now()) {
> +if (tnl_neigh_expired(neigh)) {
>  tnl_neigh_delete(neigh);
>  changed = true;
>  }
> @@ -319,7 +332,7 @@ tnl_neigh_cache_show(struct unixctl_conn *conn, int argc 
> OVS_UNUSED,
>  
>  ds_put_format(, ETH_ADDR_FMT"   %s",
>ETH_ADDR_ARGS(neigh->mac), neigh->br_name);
> -if (neigh->expires <= time_now()) {
> +if (tnl_neigh_expired(neigh)) {
>  ds_put_format(, " STALE");
>  }
>  ds_put_char(, '\n');
> 
> ___
> 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] [ovn] bug: ovn-northd was blocked for changes handling

2021-10-25 Thread Han Zhou 
On Mon, Oct 4, 2021 at 3:02 PM Vladislav Odintsov  wrote:
>
> Hi,
>
> I’ve faced with a next issue using latest OVN (master branch) with OVN-IC
enabled:
> ovn-northd CPU utilisation was at ~70-80% and ovsdb-server serving
OVN_Southbound DB was also under heavy load.
>
> In ovn-northd.log file there were warnings:
> 2021-10-01T11:14:43.548Z|18845|northd|INFO|deleting Datapath_Binding
dd6af7f7-ea46-4496-988a-e7f9828924d0 with duplicate
external-ids:logical-switch/router ec35e3e0-2674-47e7-b645-2c9b8b31865b
> 2021-10-01T11:14:44.148Z|18846|poll_loop|INFO|Dropped 32 log messages in
last 6 seconds (most recently, 0 seconds ago) due to excessive rate
> 2021-10-01T11:14:44.148Z|18847|poll_loop|INFO|wakeup due to [POLLIN] on
fd 19 (172.20.33.110:55202<->172.20.33.102:16642) at lib/stream-ssl.c:832
(69% CPU usage)
> 2021-10-01T11:14:48.336Z|18848|ovsdb_idl|WARN|Dropped 307 log messages in
last 60 seconds (most recently, 0 seconds ago) due to excessive rate
> 2021-10-01T11:14:48.336Z|18849|ovsdb_idl|WARN|transaction error:
{"details":"Deletion of 1 weak reference(s) to deleted (or never-existing)
rows from column \"datapath\" in \"IP_Multicast\" row
72bac803-e484-4358-9e48-11911c8aa16f caused this column to become empty,
but constraints on this column disallow an empty
column.","error":"constraint violation"}
>
> I checked datapath by reported logical-switch id:
>
>  ~]# ovn-sbctl find datap
external_ids:logical-switch=ec35e3e0-2674-47e7-b645-2c9b8b31865b
> _uuid   : 7e045551-7700-4a50-b0aa-02cb4e1be59d
> external_ids: {interconn-ts=vpc-CCF01DF6-rtb-216BABB1-global,
logical-switch="ec35e3e0-2674-47e7-b645-2c9b8b31865b",
name=vpc-CCF01DF6-rtb-216BABB1-global}
> load_balancers  : []
> tunnel_key  : 16712519
>
> _uuid   : dd6af7f7-ea46-4496-988a-e7f9828924d0
> external_ids: {interconn-ts=vpc-CCF01DF6-rtb-216BABB1-global,
logical-switch="ec35e3e0-2674-47e7-b645-2c9b8b31865b",
name=vpc-CCF01DF6-rtb-216BABB1-global}
> load_balancers  : []
> tunnel_key  : 5
>
>
> It refers to ovn-ic transit switch:
>
>  ~]# ovn-ic-sbctl list datapath vpc-CCF01DF6-rtb-216BABB1-global
> _uuid   : fc159fa4-d5ba-46ed-a54c-d00745091021
> external_ids: {}
> transit_switch  : vpc-CCF01DF6-rtb-216BABB1-global
> tunnel_key  : 16712519
>
>  ~]# ovn-ic-nbctl list transit vpc-CCF01DF6-rtb-216BABB1-global
> _uuid   : b5312889-92f9-40fe-98f9-2ea7ce3debcc
> external_ids: {}
> name: vpc-CCF01DF6-rtb-216BABB1-global
> other_config: {}
>
> The problem ip-multicast document:
>
>  ~]# ovn-sbctl find ip-mul datapath=dd6af7f7-ea46-4496-988a-e7f9828924d0
> _uuid   : 72bac803-e484-4358-9e48-11911c8aa16f
> datapath: dd6af7f7-ea46-4496-988a-e7f9828924d0
> enabled : false
> eth_src : ""
> idle_timeout: 300
> ip4_src : ""
> ip6_src : ""
> querier : true
> query_interval  : 150
> query_max_resp  : 1
> seq_no  : 0
> table_size  : 2048
>
> I tried manually destroy this ip_multicast document, which blocked
deletion of datapath document:
>  ~]# ovn-sbctl destroy ip-mul 72bac803-e484-4358-9e48-11911c8aa16f
>
>  ~]# ovn-sbctl list ip-mul 72bac803-e484-4358-9e48-11911c8aa16f
> ovn-sbctl: no row "72bac803-e484-4358-9e48-11911c8aa16f" in table
IP_Multicast
>
>
> And problem was resolved. ovn-northd daemon stopped consuming CPU and
excess datapath was deleted automatically.
>
>  ~]# ovn-sbctl find datapath
external_ids:interconn-ts=vpc-CCF01DF6-rtb-216BABB1-global
> _uuid   : 7e045551-7700-4a50-b0aa-02cb4e1be59d
> external_ids: {interconn-ts=vpc-CCF01DF6-rtb-216BABB1-global,
logical-switch="ec35e3e0-2674-47e7-b645-2c9b8b31865b",
name=vpc-CCF01DF6-rtb-216BABB1-global}
> load_balancers  : []
> tunnel_key  : 16712519
>
> The main problem here is that northd was entirely blocked. No processing
to SB was done. Even ovn-controllers couldn’t claim new ports. No changes
were propagated, so this is critial issue.
>
> Can anybody help understand the reason for this?
>
> Regards,
> Vladislav Odintsov
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Thanks for reporting the problem, and sorry for the slow response.
This is indeed a critical issue, but I am not sure how the duplicated
record is generated. What I can think of now is that when there is SB-DB
failover and there is a race during that period that at least two northd
processes both regard themselves as active so they both insert the
datapath_binding record when they see a new LS (in your case it happens to
be a TS) during this period. If this is how it happens, the race period
needs to be long enough (or the change rate in NB must be high enough),
otherwise it is not easy to reproduce. So the question is, did you see this
after a SB-DB

[ovs-dev] [PATCH ovn] northd.c: Fix north blocking on deleting duplicated SB datapath.

2021-10-25 Thread Han Zhou 
Duplicated datapaths shouldn't be created in the first place, but in
case it is created because of either bug or dirty data, it should be
properly deleted instead of causing permanent failure in northd.

Currently, when there is a duplicated datapath record and a ip_multicast
record referencing it created, northd tries to delete the duplicated
datapath but the transaction fails due to schema constraint:

transaction error: {"details":"Deletion of 1 weak reference(s) to deleted (or 
never-existing) rows from column \"datapath\" in \"IP_Multicast\" row 
72bac803-e484-4358-9e48-11911c8aa16f caused this column to become empty, but 
constraints on this column disallow an empty column.","error":"constraint 
violation"}

Northd would be blocked forever until manual deletion of the
ip_multicast record.

The problem is that in the same transaction only datapath is deleted but
not the ip_multicast record that references the datapath. In
build_ip_mcast() there is logic to delete ip_multicast records with
non-exist datapath, but the logic of ovn_datapath_from_sbrec() can find
the datapath and regard it as valid whenever the
external-ids:logical-switch/router matches. This patch fixes that by
comparing the sbrec of the datapath afterwards, and regards it as valid
only if the sbrec matches, too. This way, both ip_multicast and datapath
records are deleted in the same transaction and won't cause any trouble.

Reported-by: Vladislav Odintsov 
Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-dev/2021-October/388269.html
Signed-off-by: Han Zhou 
---
 northd/northd.c |  7 ++-
 tests/ovn-northd.at | 20 
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/northd/northd.c b/northd/northd.c
index f71121486..da4f9cd14 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -1045,7 +1045,12 @@ ovn_datapath_from_sbrec(struct hmap *datapaths,
 !smap_get_uuid(>external_ids, "logical-router", )) {
 return NULL;
 }
-return ovn_datapath_find(datapaths, );
+struct ovn_datapath *od = ovn_datapath_find(datapaths, );
+if (od && (od->sb == sb)) {
+return od;
+}
+
+return NULL;
 }
 
 static bool
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 544820764..e2b9924b6 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -5446,3 +5446,23 @@ wait_row_count Port_binding 1 logical-port=S1-vm2 
requested_chassis=$ch2_uuid
 
 AT_CLEANUP
 ])
+
+# Duplicated datapaths shouldn't be created, but in case it is created because
+# of bug or dirty data, it should be properly deleted instead of causing
+# permanent failure in northd.
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([handling duplicated datapaths])
+ovn_start
+
+check ovn-nbctl --wait=sb ls-add ls1
+ls1_uuid=$(fetch_column nb:Logical_Switch _uuid)
+
+# create a duplicated sb datapath (and an IP_Mulicast record that references
+# it) on purpose.
+AT_CHECK([ovn-sbctl --id=@dp create Datapath_Binding 
external_ids:logical-switch=$ls1_uuid external_ids:name=ls1 tunnel_key=123 -- 
create IP_Multicast datapath=@dp], [0], [ignore])
+
+# northd should delete one of the datapaths in the end
+wait_row_count Datapath_Binding 1
+
+AT_CLEANUP
+])
-- 
2.30.2

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

[ovs-dev] [PATCH dpdk-latest 2/2] netdev-dpdk: Fix build with 21.11-rc1.

2021-10-25 Thread David Marchand 
PKT_[RT]X_* and other mbuf macros have been prefixed with RTE_MBUF_ [1].
Update accordingly.

1: https://git.dpdk.org/dpdk/commit/?id=daa02b5cddbb

Signed-off-by: David Marchand 
---
 lib/dp-packet.h   | 26 ++
 lib/netdev-dpdk.c | 18 +-
 2 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index 3dc582fbfd..ee0805ae69 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -58,29 +58,31 @@ enum OVS_PACKED_ENUM dp_packet_source {
 enum dp_packet_offload_mask {
 /* Value 0 is not used. */
 /* Is the 'rss_hash' valid? */
-DEF_OL_FLAG(DP_PACKET_OL_RSS_HASH, PKT_RX_RSS_HASH, 0x1),
+DEF_OL_FLAG(DP_PACKET_OL_RSS_HASH, RTE_MBUF_F_RX_RSS_HASH, 0x1),
 /* Is the 'flow_mark' valid? */
-DEF_OL_FLAG(DP_PACKET_OL_FLOW_MARK, PKT_RX_FDIR_ID, 0x2),
+DEF_OL_FLAG(DP_PACKET_OL_FLOW_MARK, RTE_MBUF_F_RX_FDIR_ID, 0x2),
 /* Bad L4 checksum in the packet. */
-DEF_OL_FLAG(DP_PACKET_OL_RX_L4_CKSUM_BAD, PKT_RX_L4_CKSUM_BAD, 0x4),
+DEF_OL_FLAG(DP_PACKET_OL_RX_L4_CKSUM_BAD, RTE_MBUF_F_RX_L4_CKSUM_BAD, 0x4),
 /* Bad IP checksum in the packet. */
-DEF_OL_FLAG(DP_PACKET_OL_RX_IP_CKSUM_BAD, PKT_RX_IP_CKSUM_BAD, 0x8),
+DEF_OL_FLAG(DP_PACKET_OL_RX_IP_CKSUM_BAD, RTE_MBUF_F_RX_IP_CKSUM_BAD, 0x8),
 /* Valid L4 checksum in the packet. */
-DEF_OL_FLAG(DP_PACKET_OL_RX_L4_CKSUM_GOOD, PKT_RX_L4_CKSUM_GOOD, 0x10),
+DEF_OL_FLAG(DP_PACKET_OL_RX_L4_CKSUM_GOOD, RTE_MBUF_F_RX_L4_CKSUM_GOOD,
+0x10),
 /* Valid IP checksum in the packet. */
-DEF_OL_FLAG(DP_PACKET_OL_RX_IP_CKSUM_GOOD, PKT_RX_IP_CKSUM_GOOD, 0x20),
+DEF_OL_FLAG(DP_PACKET_OL_RX_IP_CKSUM_GOOD, RTE_MBUF_F_RX_IP_CKSUM_GOOD,
+0x20),
 /* TCP Segmentation Offload. */
-DEF_OL_FLAG(DP_PACKET_OL_TX_TCP_SEG, PKT_TX_TCP_SEG, 0x40),
+DEF_OL_FLAG(DP_PACKET_OL_TX_TCP_SEG, RTE_MBUF_F_TX_TCP_SEG, 0x40),
 /* Offloaded packet is IPv4. */
-DEF_OL_FLAG(DP_PACKET_OL_TX_IPV4, PKT_TX_IPV4, 0x80),
+DEF_OL_FLAG(DP_PACKET_OL_TX_IPV4, RTE_MBUF_F_TX_IPV4, 0x80),
 /* Offloaded packet is IPv6. */
-DEF_OL_FLAG(DP_PACKET_OL_TX_IPV6, PKT_TX_IPV6, 0x100),
+DEF_OL_FLAG(DP_PACKET_OL_TX_IPV6, RTE_MBUF_F_TX_IPV6, 0x100),
 /* Offload TCP checksum. */
-DEF_OL_FLAG(DP_PACKET_OL_TX_TCP_CKSUM, PKT_TX_TCP_CKSUM, 0x200),
+DEF_OL_FLAG(DP_PACKET_OL_TX_TCP_CKSUM, RTE_MBUF_F_TX_TCP_CKSUM, 0x200),
 /* Offload UDP checksum. */
-DEF_OL_FLAG(DP_PACKET_OL_TX_UDP_CKSUM, PKT_TX_UDP_CKSUM, 0x400),
+DEF_OL_FLAG(DP_PACKET_OL_TX_UDP_CKSUM, RTE_MBUF_F_TX_UDP_CKSUM, 0x400),
 /* Offload SCTP checksum. */
-DEF_OL_FLAG(DP_PACKET_OL_TX_SCTP_CKSUM, PKT_TX_SCTP_CKSUM, 0x800),
+DEF_OL_FLAG(DP_PACKET_OL_TX_SCTP_CKSUM, RTE_MBUF_F_TX_SCTP_CKSUM, 0x800),
 /* Adding new field requires adding to DP_PACKET_OL_SUPPORTED_MASK. */
 };
 
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 6d3fd6beda..db08aec440 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2165,14 +2165,14 @@ netdev_dpdk_prep_hwol_packet(struct netdev_dpdk *dev, 
struct rte_mbuf *mbuf)
 {
 struct dp_packet *pkt = CONTAINER_OF(mbuf, struct dp_packet, mbuf);
 
-if (mbuf->ol_flags & PKT_TX_L4_MASK) {
+if (mbuf->ol_flags & RTE_MBUF_F_TX_L4_MASK) {
 mbuf->l2_len = (char *)dp_packet_l3(pkt) - (char *)dp_packet_eth(pkt);
 mbuf->l3_len = (char *)dp_packet_l4(pkt) - (char *)dp_packet_l3(pkt);
 mbuf->outer_l2_len = 0;
 mbuf->outer_l3_len = 0;
 }
 
-if (mbuf->ol_flags & PKT_TX_TCP_SEG) {
+if (mbuf->ol_flags & RTE_MBUF_F_TX_TCP_SEG) {
 struct tcp_header *th = dp_packet_l4(pkt);
 
 if (!th) {
@@ -2182,11 +2182,11 @@ netdev_dpdk_prep_hwol_packet(struct netdev_dpdk *dev, 
struct rte_mbuf *mbuf)
 }
 
 mbuf->l4_len = TCP_OFFSET(th->tcp_ctl) * 4;
-mbuf->ol_flags |= PKT_TX_TCP_CKSUM;
+mbuf->ol_flags |= RTE_MBUF_F_TX_TCP_CKSUM;
 mbuf->tso_segsz = dev->mtu - mbuf->l3_len - mbuf->l4_len;
 
-if (mbuf->ol_flags & PKT_TX_IPV4) {
-mbuf->ol_flags |= PKT_TX_IP_CKSUM;
+if (mbuf->ol_flags & RTE_MBUF_F_TX_IPV4) {
+mbuf->ol_flags |= RTE_MBUF_F_TX_IP_CKSUM;
 }
 }
 return true;
@@ -2533,7 +2533,7 @@ netdev_dpdk_filter_packet_len(struct netdev_dpdk *dev, 
struct rte_mbuf **pkts,
 for (i = 0; i < pkt_cnt; i++) {
 pkt = pkts[i];
 if (OVS_UNLIKELY((pkt->pkt_len > dev->max_packet_len)
-&& !(pkt->ol_flags & PKT_TX_TCP_SEG))) {
+&& !(pkt->ol_flags & RTE_MBUF_F_TX_TCP_SEG))) {
 VLOG_WARN_RL(, "%s: Too big size %" PRIu32 " "
  "max_packet_len %d", dev->up.name, pkt->pkt_len,
  dev->max_packet_len);
@@ -2755,12 +2755,12 @@ dpdk_copy_dp_packet_to_mbuf(struct rte_mempool *mp, 
struct dp_packet *pkt_orig)
 mbuf_dest->tx_offload = pkt_orig->mbuf.tx_offload;
 mbuf_dest->packet_type =

[ovs-dev] [PATCH dpdk-latest 1/2] netdev-dpdk: Remove unused attribute from rte_flow rule.

2021-10-25 Thread David Marchand 
The shared attribute has been deprecated some time ago and will be
removed in 21.11 [1].
Since its value was 0, let's simply rely on implicit init by not
mentionning it.

1: https://git.dpdk.org/dpdk/commit/?id=92ef4b8f1688

Signed-off-by: David Marchand 
---
Note: this patch could go to the master branch too.
---
 lib/netdev-dpdk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index ee5e528f75..6d3fd6beda 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -5279,7 +5279,7 @@ netdev_dpdk_rte_flow_query_count(struct netdev *netdev,
  struct rte_flow_query_count *query,
  struct rte_flow_error *error)
 {
-struct rte_flow_action_count count = { .shared = 0, .id = 0 };
+struct rte_flow_action_count count = { .id = 0, };
 const struct rte_flow_action actions[] = {
 {
 .type = RTE_FLOW_ACTION_TYPE_COUNT,
-- 
2.23.0

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

Re: [ovs-dev] [OVN Patch v4 2/2] Add support for configuring parallelization via unixctl

2021-10-25 Thread Anton Ivanov 
Hi Han,

I had requests to move this to cli. This also allows a unified format if we add 
parallelization to ovsdb or controller. 

On 25 October 2021 04:05:39 BST, Han Zhou  wrote:
>On Tue, Sep 21, 2021 at 8:48 AM  wrote:
>>
>> From: Anton Ivanov 
>>
>> libs: add configuration support to parallel-hmap.[c,h]
>> northd: add support for configuring parallelization to northd
>
>Hi Anton,
>
>This patch seems to replace the NB option use_parallel_build with unix
>command configuration. Could you explain the motivation of this? I feel
>that NB option is better, because with HA we only need to set in one place
>for all northds.
>BTW, there is no documentation change for the NB options, if it is supposed
>to be removed.
>
>Thanks,
>Han
>
>>
>> Signed-off-by: Anton Ivanov 
>> ---
>>  lib/ovn-parallel-hmap.c | 185 ++--
>>  lib/ovn-parallel-hmap.h |  63 +-
>>  northd/northd.c |  30 +++
>>  northd/northd.h |   2 -
>>  northd/ovn-northd.c |   5 +-
>>  tests/ovn-macros.at |  16 +++-
>>  6 files changed, 263 insertions(+), 38 deletions(-)
>>
>> diff --git a/lib/ovn-parallel-hmap.c b/lib/ovn-parallel-hmap.c
>> index 1b3883441..6a6488a17 100644
>> --- a/lib/ovn-parallel-hmap.c
>> +++ b/lib/ovn-parallel-hmap.c
>> @@ -33,6 +33,7 @@
>>  #include "ovs-thread.h"
>>  #include "ovs-numa.h"
>>  #include "random.h"
>> +#include "unixctl.h"
>>
>>  VLOG_DEFINE_THIS_MODULE(ovn_parallel_hmap);
>>
>> @@ -46,6 +47,7 @@ VLOG_DEFINE_THIS_MODULE(ovn_parallel_hmap);
>>   */
>>  static atomic_bool initial_pool_setup = ATOMIC_VAR_INIT(false);
>>  static bool can_parallelize = false;
>> +static bool should_parallelize = false;
>>
>>  /* This is set only in the process of exit and the set is
>>   * accompanied by a fence. It does not need to be atomic or be
>> @@ -83,7 +85,7 @@ static void *standard_helper_thread(void *arg);
>>
>>  struct worker_pool *ovn_add_standard_pool(int size)
>>  {
>> -return add_worker_pool(standard_helper_thread, size);
>> +return add_worker_pool(standard_helper_thread, size, "default",
>true);
>>  }
>>
>>  bool
>> @@ -92,6 +94,19 @@ ovn_stop_parallel_processing(struct worker_pool *pool)
>>  return pool->workers_must_exit;
>>  }
>>
>> +bool
>> +ovn_set_parallel_processing(bool enable)
>> +{
>> +should_parallelize = enable;
>> +return can_parallelize;
>> +}
>> +
>> +bool
>> +ovn_get_parallel_processing(void)
>> +{
>> +return can_parallelize && should_parallelize;
>> +}
>> +
>>  bool
>>  ovn_can_parallelize_hashes(bool force_parallel)
>>  {
>> @@ -117,6 +132,7 @@ destroy_pool(struct worker_pool *pool) {
>>  sem_close(pool->done);
>>  sprintf(sem_name, MAIN_SEM_NAME, sembase, pool);
>>  sem_unlink(sem_name);
>> +free(pool->name);
>>  free(pool);
>>  }
>>
>> @@ -127,6 +143,10 @@ ovn_resize_pool(struct worker_pool *pool, int size)
>>
>>  ovs_assert(pool != NULL);
>>
>> +if (!pool->is_mutable) {
>> +return false;
>> +}
>> +
>>  if (!size) {
>>  size = pool_size;
>>  }
>> @@ -166,7 +186,8 @@ cleanup:
>>
>>
>>  struct worker_pool *
>> -ovn_add_worker_pool(void *(*start)(void *), int size)
>> +ovn_add_worker_pool(void *(*start)(void *), int size, char *name,
>> +bool is_mutable)
>>  {
>>  struct worker_pool *new_pool = NULL;
>>  bool test = false;
>> @@ -194,6 +215,8 @@ ovn_add_worker_pool(void *(*start)(void *), int size)
>>  new_pool = xmalloc(sizeof(struct worker_pool));
>>  new_pool->size = size;
>>  new_pool->start = start;
>> +new_pool->is_mutable = is_mutable;
>> +new_pool->name = xstrdup(name);
>>  sprintf(sem_name, MAIN_SEM_NAME, sembase, new_pool);
>>  new_pool->done = sem_open(sem_name, O_CREAT, S_IRWXU, 0);
>>  if (new_pool->done == SEM_FAILED) {
>> @@ -226,6 +249,7 @@ cleanup:
>>  sprintf(sem_name, MAIN_SEM_NAME, sembase, new_pool);
>>  sem_unlink(sem_name);
>>  }
>> +free(new_pool->name);
>>  ovs_mutex_unlock(_mutex);
>>  return NULL;
>>  }
>> @@ -342,8 +366,7 @@ ovn_complete_pool_callback(struct worker_pool *pool,
>>  }
>>  } while (completed < pool->size);
>>  }
>> -
>> -/* Complete a thread pool which uses a callback function to process
>results
>> +/* Run a thread pool which uses a callback function to process results
>>   */
>>  void
>>  ovn_run_pool_callback(struct worker_pool *pool,
>> @@ -352,8 +375,8 @@ ovn_run_pool_callback(struct worker_pool *pool,
>>void *fin_result,
>>void *result_frags, int index))
>>  {
>> -ovn_start_pool(pool);
>> -ovn_complete_pool_callback(pool, fin_result, result_frags,
>helper_func);
>> +start_pool(pool);
>> +complete_pool_callback(pool, fin_result, result_frags, helper_func);
>>  }
>>
>>  /* Run a thread pool - basic, does not do results processing.
>> @@ -401,6 +424,28 @@

Re: [ovs-dev] [PATCH] netlink-socket: Check for null sock in nl_sock_recv__()

2021-10-25 Thread 0-day Robot 
Bleep bloop.  Greetings David Christensen, I am a robot and I have tried out 
your patch.
Thanks for your contribution.

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


checkpatch:
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Murilo Opsfelder Araujo 
Lines checked: 43, 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] netlink-socket: Check for null sock in nl_sock_recv__()

2021-10-25 Thread David Christensen 
In certain high load situations, such as when creating a large number of
ports on a switch, the parameter 'sock' may be passed to nl_sock_recv__()
as null, resulting in a segmentation fault when 'sock' is later
dereferenced, such as when calling recvmsg().

The ovs-vswitchd.log will display something like this:

fatal_signal(revalidator138)|WARN|terminating with signal 11 (signal 11)

Tested this change under the same circumstances that originally generated
the segmentation fault and it ran successfully for four days without any
issue.

Signed-off-by: Murilo Opsfelder Araujo 
Signed-off-by: David Christensen 
IBM-BZ: #193057
---
 lib/netlink-socket.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/lib/netlink-socket.c b/lib/netlink-socket.c
index 5867de564..3ab4d8485 100644
--- a/lib/netlink-socket.c
+++ b/lib/netlink-socket.c
@@ -653,6 +653,10 @@ nl_sock_recv__(struct nl_sock *sock, struct ofpbuf *buf, 
int *nsid, bool wait)
 int *ptr;
 int error;
 
+if (sock == NULL) {
+return ECONNRESET;
+}
+
 ovs_assert(buf->allocated >= sizeof *nlmsghdr);
 ofpbuf_clear(buf);
 
-- 
2.27.0

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

Re: [ovs-dev] [PATCH] ovsdb-idl: Add memory report function.

2021-10-25 Thread Han Zhou 
On Mon, Oct 25, 2021 at 5:39 AM Ilya Maximets  wrote:
>
> On 10/23/21 08:51, Han Zhou wrote:
> >
> >
> > On Thu, Oct 14, 2021 at 4:46 AM Ilya Maximets mailto:i.maxim...@ovn.org>> wrote:
> >>
> >> Added new function to return memory usage statistics for database
> >> objects inside IDL.  Statistics similar to what ovsdb-server reports.
> >> Not counting _Server database as it should be small, hence doesn't
> >> worth adding extra code to the ovsdb-cs module.  Can be added later
> >> if needed.
> >>
> >> ovs-vswitchd is a user in OVS, but this API will be mostly useful for
> >> OVN daemons.
> >>
> >> Signed-off-by: Ilya Maximets >
> >> ---
>
> 
>
> >
> > Hi Ilya, I tested in sandbox:
> >
> > [hanzhou@fedora tutorial]$ ovs-vsctl add-br br-int
> > [hanzhou@fedora tutorial]$ ovs-appctl memory/show
> > handlers:17 ports:1 revalidators:7 rules:5
> >
> > It didn't show idl-xxx memory for vswitchd. Did I miss something?
>
> Weird.  Are you sure that patch was correctly applied?

My bad! I forgot that this patch is not in a series :P

Acked-by: Han Zhou 

>
> On my machine:
>
> [ovs]# wget -c
https://patchwork.ozlabs.org/project/openvswitch/patch/20211014114614.829400-1-i.maxim...@ovn.org/mbox/
--content-disposition
> [ovs]# git am -3 ovs-dev-ovsdb-idl-Add-memory-report-function..patch
> [ovs]# make -j8 sandbox
> [tutorial]# ovs-appctl memory/show
> idl-cells:17
> [tutorial]# ovs-vsctl add-br br-int
> [tutorial]# ovs-appctl memory/show
> handlers:11 idl-cells:96 ports:1 revalidators:5 rules:5
> [tutorial]# ovs-vsctl del-br br-int
> [tutorial]# ovs-appctl memory/show
> idl-cells:17
>
> Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [External] : Re: " Add support for DHCP Option 12 (Hostname)"

2021-10-25 Thread Brendan Doyle 
OK thanks, It was going to be a relatively easy backport if we need to 
do it.

But great that it is already done.


On 25/10/2021 15:04, Numan Siddique wrote:

On Fri, Oct 22, 2021 at 9:23 AM Brendan Doyle  wrote:


Just wondering was this patch backported, if so could someone provide a
pointer


I see the commit [1] in the main branch and branch-21.09.

https://urldefense.com/v3/__https://github.com/ovn-org/ovn/commit/0e4d0f1fbf70ddfc347781129451fb61097d559e__;!!ACWV5N9M2RV99hQ!amElLcjYZZwvrE3Arm25gxq2Fqhed7WSKhfXoSinx8dyZlQLuBjBT9G1mR7HHfDVqzk$

Thanks
Numan


Thanks

___
dev mailing list
d...@openvswitch.org
https://urldefense.com/v3/__https://mail.openvswitch.org/mailman/listinfo/ovs-dev__;!!ACWV5N9M2RV99hQ!amElLcjYZZwvrE3Arm25gxq2Fqhed7WSKhfXoSinx8dyZlQLuBjBT9G1mR7He6P7wbM$



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

Re: [ovs-dev] [PATCH ovn 13/12 v7] NEWS: Add note on infrastructure for plug providers.

2021-10-25 Thread Frode Nordahl 
On Mon, Oct 25, 2021 at 5:59 PM Frode Nordahl
 wrote:
>
> Signed-off-by: Frode Nordahl 
> ---
>  NEWS | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/NEWS b/NEWS
> index 5f448e67d..7a993a0be 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -6,6 +6,13 @@ Post v21.09.0
>  if not desired.
>- Added Load_Balancer_Group support, which simplifies large scale
>  configurations of load balancers.
> +  - Introduced infrastructure for plug providers.  When OVN is linked with an
> +appropriate plug implementation, such as OVN VIF [0], CMS can request OVN
> +to plug lports.  This is particularly useful in topologies where the
> +ovn-controller process is running on SmartNIC control plane CPUs.  Please
> +refer to Documentation/topics/plug_providers/plug-providers.rst for more
> +information.
> +[0] https://github.com/ovn-org/ovn-vif
>
>  OVN v21.09.0 - 01 Oct 2021
>  --
> --
> 2.32.0

As promised I added a patch with a NEWS addition at the tip of the series.

I have also staged a repository that should be suitable for
upstreaming at https://github.com/fnordahl/ovn-vif

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

[ovs-dev] [PATCH ovn 13/12 v7] NEWS: Add note on infrastructure for plug providers.

2021-10-25 Thread Frode Nordahl 
Signed-off-by: Frode Nordahl 
---
 NEWS | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/NEWS b/NEWS
index 5f448e67d..7a993a0be 100644
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,13 @@ Post v21.09.0
 if not desired.
   - Added Load_Balancer_Group support, which simplifies large scale
 configurations of load balancers.
+  - Introduced infrastructure for plug providers.  When OVN is linked with an
+appropriate plug implementation, such as OVN VIF [0], CMS can request OVN
+to plug lports.  This is particularly useful in topologies where the
+ovn-controller process is running on SmartNIC control plane CPUs.  Please
+refer to Documentation/topics/plug_providers/plug-providers.rst for more
+information.
+[0] https://github.com/ovn-org/ovn-vif
 
 OVN v21.09.0 - 01 Oct 2021
 --
-- 
2.32.0

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

[ovs-dev] [PATCH] TCP Stream: Use TCP keepalive by default

2021-10-25 Thread Michael Santana 
In the case that a client disables jsonrpc probes the client would fail
to detect if the connection to the server has dropped. To workaround
such case TCP keepalive is enabled.

Signed-off-by: Michael Santana 
---
 lib/socket-util.c | 13 +
 lib/socket-util.h |  1 +
 lib/stream-fd.c   |  1 +
 lib/stream-ssl.c  |  2 ++
 lib/stream-tcp.c  |  1 +
 5 files changed, 18 insertions(+)

diff --git a/lib/socket-util.c b/lib/socket-util.c
index 4f1ffecf5..dbf35f9bb 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -114,6 +114,19 @@ setsockopt_tcp_nodelay(int fd)
 }
 }
 
+void
+setsockopt_tcp_keepalive(int fd)
+{
+int on = 1;
+int retval;
+
+retval = setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, , sizeof on);
+if (retval) {
+retval = sock_errno();
+VLOG_ERR("setsockopt(SO_KEEPALIVE): %s", sock_strerror(retval));
+}
+}
+
 /* Sets the DSCP value of socket 'fd' to 'dscp', which must be 63 or less.
  * 'family' must indicate the socket's address family (AF_INET or AF_INET6, to
  * do anything useful). */
diff --git a/lib/socket-util.h b/lib/socket-util.h
index 9ccb7d4cc..f6cf00a32 100644
--- a/lib/socket-util.h
+++ b/lib/socket-util.h
@@ -33,6 +33,7 @@ struct ds;
 int set_nonblocking(int fd);
 void xset_nonblocking(int fd);
 void setsockopt_tcp_nodelay(int fd);
+void setsockopt_tcp_keepalive(int fd);
 int set_dscp(int fd, int family, uint8_t dscp);
 
 bool addr_is_ipv6(const char *host_name);
diff --git a/lib/stream-fd.c b/lib/stream-fd.c
index 46ee7ae27..df609579c 100644
--- a/lib/stream-fd.c
+++ b/lib/stream-fd.c
@@ -93,6 +93,7 @@ fd_connect(struct stream *stream)
 int retval = check_connection_completion(s->fd);
 if (retval == 0 && s->fd_type == AF_INET) {
 setsockopt_tcp_nodelay(s->fd);
+setsockopt_tcp_keepalive(s->fd);
 }
 return retval;
 }
diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
index 0ea3f2c08..0a47208da 100644
--- a/lib/stream-ssl.c
+++ b/lib/stream-ssl.c
@@ -267,6 +267,7 @@ new_ssl_stream(char *name, char *server_name, int fd, enum 
session_type type,
  */
 if (state == STATE_SSL_CONNECTING) {
 setsockopt_tcp_nodelay(fd);
+setsockopt_tcp_keepalive(fd);
 }
 
 /* Create and configure OpenSSL stream. */
@@ -521,6 +522,7 @@ ssl_connect(struct stream *stream)
 }
 sslv->state = STATE_SSL_CONNECTING;
 setsockopt_tcp_nodelay(sslv->fd);
+setsockopt_tcp_keepalive(sslv->fd);
 /* Fall through. */
 
 case STATE_SSL_CONNECTING:
diff --git a/lib/stream-tcp.c b/lib/stream-tcp.c
index e8dc2bfaa..48443b9be 100644
--- a/lib/stream-tcp.c
+++ b/lib/stream-tcp.c
@@ -43,6 +43,7 @@ new_tcp_stream(char *name, int fd, int connect_status, struct 
stream **streamp)
 {
 if (connect_status == 0) {
 setsockopt_tcp_nodelay(fd);
+setsockopt_tcp_keepalive(fd);
 }
 
 return new_fd_stream(name, fd, connect_status, AF_INET, streamp);
-- 
2.31.1

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

Re: [ovs-dev] " Add support for DHCP Option 12 (Hostname)"

2021-10-25 Thread Numan Siddique 
On Fri, Oct 22, 2021 at 9:23 AM Brendan Doyle  wrote:
>
>
> Just wondering was this patch backported, if so could someone provide a
> pointer
>

I see the commit [1] in the main branch and branch-21.09.

https://github.com/ovn-org/ovn/commit/0e4d0f1fbf70ddfc347781129451fb61097d559e

Thanks
Numan

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

[ovs-dev] [PATCH v3 2/2] python: replace pyOpenSSL with ssl

2021-10-25 Thread Timothy Redaelli 
Currently, pyOpenSSL is half-deprecated upstream and so it's removed on
some distributions (for example on CentOS Stream 9,
https://issues.redhat.com/browse/CS-336), but since OVS only
supports Python 3 it's possible to replace pyOpenSSL with "import ssl"
included in base Python 3.

Stream recv and send had to be splitted as _recv and _send, since SSLError
is a subclass of socket.error and so it was not possible to except for
SSLWantReadError and SSLWantWriteError in recv and send of SSLStream.

TCPstream._open cannot be used in SSLStream, since Python ssl module
requires the SSL socket to be created before connecting it, so
SSLStream._open needs to create the socket, create SSL socket and then
connect the SSL socket.

Reported-by: Timothy Redaelli 
Reported-at: https://bugzilla.redhat.com/1988429
Signed-off-by: Timothy Redaelli 
---
 .ci/linux-prepare.sh |  2 +-
 .cirrus.yml  |  2 +-
 .travis.yml  |  1 -
 python/ovs/poller.py |  6 +--
 python/ovs/stream.py | 91 ++--
 tests/ovsdb-idl.at   |  2 +-
 6 files changed, 60 insertions(+), 44 deletions(-)

diff --git a/.ci/linux-prepare.sh b/.ci/linux-prepare.sh
index c55125cf7..b9b499bad 100755
--- a/.ci/linux-prepare.sh
+++ b/.ci/linux-prepare.sh
@@ -21,7 +21,7 @@ make -j4 HAVE_LLVM= HAVE_SQLITE= install
 cd ..
 
 pip3 install --disable-pip-version-check --user \
-flake8 hacking sphinx pyOpenSSL wheel setuptools
+flake8 hacking sphinx wheel setuptools
 pip3 install --user --upgrade docutils
 pip3 install --user  'meson==0.47.1'
 
diff --git a/.cirrus.yml b/.cirrus.yml
index 480fea242..a7ae793bc 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -9,7 +9,7 @@ freebsd_build_task:
 
   env:
 DEPENDENCIES: automake libtool gmake gcc wget openssl python3
-PY_DEPS:  sphinx|openssl
+PY_DEPS:  sphinx
 matrix:
   COMPILER: gcc
   COMPILER: clang
diff --git a/.travis.yml b/.travis.yml
index 51d051108..c7aeede06 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -17,7 +17,6 @@ addons:
   - libjemalloc-dev
   - libnuma-dev
   - libpcap-dev
-  - python3-openssl
   - python3-pip
   - python3-sphinx
   - libelf-dev
diff --git a/python/ovs/poller.py b/python/ovs/poller.py
index 3624ec865..157719c3a 100644
--- a/python/ovs/poller.py
+++ b/python/ovs/poller.py
@@ -26,9 +26,9 @@ if sys.platform == "win32":
 import ovs.winutils as winutils
 
 try:
-from OpenSSL import SSL
+import ssl
 except ImportError:
-SSL = None
+ssl = None
 
 try:
 from eventlet import patcher as eventlet_patcher
@@ -73,7 +73,7 @@ class _SelectSelect(object):
 def register(self, fd, events):
 if isinstance(fd, socket.socket):
 fd = fd.fileno()
-if SSL and isinstance(fd, SSL.Connection):
+if ssl and isinstance(fd, ssl.SSLSocket):
 fd = fd.fileno()
 
 if sys.platform != 'win32':
diff --git a/python/ovs/stream.py b/python/ovs/stream.py
index f5a520862..40d484827 100644
--- a/python/ovs/stream.py
+++ b/python/ovs/stream.py
@@ -22,9 +22,9 @@ import ovs.socket_util
 import ovs.vlog
 
 try:
-from OpenSSL import SSL
+import ssl
 except ImportError:
-SSL = None
+ssl = None
 
 if sys.platform == 'win32':
 import ovs.winutils as winutils
@@ -322,6 +322,12 @@ class Stream(object):
 The recv function will not block waiting for data to arrive.  If no
 data have been received, it returns (errno.EAGAIN, "") immediately."""
 
+try:
+return self._recv(n)
+except socket.error as e:
+return (ovs.socket_util.get_exception_errno(e), "")
+
+def _recv(self, n):
 retval = self.connect()
 if retval != 0:
 return (retval, "")
@@ -331,10 +337,7 @@ class Stream(object):
 if sys.platform == 'win32' and self.socket is None:
 return self.__recv_windows(n)
 
-try:
-return (0, self.socket.recv(n))
-except socket.error as e:
-return (ovs.socket_util.get_exception_errno(e), "")
+return (0, self.socket.recv(n))
 
 def __recv_windows(self, n):
 if self._read_pending:
@@ -396,6 +399,12 @@ class Stream(object):
 Will not block.  If no bytes can be immediately accepted for
 transmission, returns -errno.EAGAIN immediately."""
 
+try:
+return self._send(buf)
+except socket.error as e:
+return -ovs.socket_util.get_exception_errno(e)
+
+def _send(self, buf):
 retval = self.connect()
 if retval != 0:
 return -retval
@@ -409,10 +418,7 @@ class Stream(object):
 if sys.platform == 'win32' and self.socket is None:
 return self.__send_windows(buf)
 
-try:
-return self.socket.send(buf)
-except socket.error as e:
-return -ovs.socket_util.get_exception_errno(e)
+return self.socket.send(buf)
 
 def __send_windows(self, buf):

[ovs-dev] [PATCH v3 1/2] socket-util: split inet_open_active function and use connect_ex

2021-10-25 Thread Timothy Redaelli 
In an upcoming patch, PyOpenSSL will be replaced with Python ssl module,
but in order to do an async connection with Python ssl module the ssl
socket must be created when the socket is created, but before the
socket is connected.

So, inet_open_active function is splitted in 3 parts:
- inet_create_socket_active: creates the socket and returns the family and
  the socket, or (error, None) if some error needs to be returned.
- inet_connect_active: connect the socket and returns the errno (it
  returns 0 if errno is EINPROGRESS or EWOULDBLOCK).

connect is replaced by connect_ex, since Python suggest to use it for
asynchronous connects and it's also cleaner since inet_connect_active
returns errno that connect_ex already returns, moreover due to a Python
limitation connect cannot not be used with ssl module.

inet_open_active function is changed in order to use the new functions
inet_create_socket_active and inet_connect_active.

Signed-off-by: Timothy Redaelli 
---
 python/ovs/socket_util.py | 36 ++--
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/python/ovs/socket_util.py b/python/ovs/socket_util.py
index 3faa64e9d..651012bf0 100644
--- a/python/ovs/socket_util.py
+++ b/python/ovs/socket_util.py
@@ -222,8 +222,7 @@ def inet_parse_active(target, default_port):
 return (host_name, port)
 
 
-def inet_open_active(style, target, default_port, dscp):
-address = inet_parse_active(target, default_port)
+def inet_create_socket_active(style, address):
 try:
 is_addr_inet = is_valid_ipv4_address(address[0])
 if is_addr_inet:
@@ -235,23 +234,32 @@ def inet_open_active(style, target, default_port, dscp):
 except socket.error as e:
 return get_exception_errno(e), None
 
+return family, sock
+
+
+def inet_connect_active(sock, address, family, dscp):
 try:
 set_nonblocking(sock)
 set_dscp(sock, family, dscp)
-try:
-sock.connect(address)
-except socket.error as e:
-error = get_exception_errno(e)
-if sys.platform == 'win32' and error == errno.WSAEWOULDBLOCK:
-# WSAEWOULDBLOCK would be the equivalent on Windows
-# for EINPROGRESS on Unix.
-error = errno.EINPROGRESS
-if error != errno.EINPROGRESS:
-raise
-return 0, sock
+error = sock.connect_ex(address)
+if error not in (0, errno.EINPROGRESS, errno.EWOULDBLOCK):
+sock.close()
+return error
+return 0
 except socket.error as e:
 sock.close()
-return get_exception_errno(e), None
+return get_exception_errno(e)
+
+
+def inet_open_active(style, target, default_port, dscp):
+address = inet_parse_active(target, default_port)
+family, sock = inet_create_socket_active(style, address)
+if sock is None:
+return family, sock
+error = inet_connect_active(sock, address, family, dscp)
+if error:
+return error, None
+return 0, sock
 
 
 def get_exception_errno(e):
-- 
2.31.1

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

[ovs-dev] [PATCH v3 0/2] replace pyOpenSSL with ssl

2021-10-25 Thread Timothy Redaelli 
Currently, pyOpenSSL is half-deprecated upstream and so it's removed on
some distributions (for example on CentOS Stream 9,
https://issues.redhat.com/browse/CS-336), but since OVS only
supports Python 3 it's possible to replace pyOpenSSL with "import ssl"
included in base Python 3.

v2 -> v3:
As suggested by Ilya, split up inet_open_active and use connect_ex.
connect_ex is suggested upstream to be used for asynchronous connects
and so it should be used also for "standard" TCP sockets.

Removed raises in connect_ex, since from Python 3.3 (and OVS supports
3.4+) any errors returned by connect are a subclass of socket.error, or
better OSError, so the errno can be used directly.

Added a comment in "python: replace pyOpenSSL with ssl" patch that
explains why TCPStream._open cannot be used.

Timothy Redaelli (2):
  socket-util: split inet_open_active function and use connect_ex
  python: replace pyOpenSSL with ssl

 .ci/linux-prepare.sh  |  2 +-
 .cirrus.yml   |  2 +-
 .travis.yml   |  1 -
 python/ovs/poller.py  |  6 +--
 python/ovs/socket_util.py | 36 ++--
 python/ovs/stream.py  | 91 +++
 tests/ovsdb-idl.at|  2 +-
 7 files changed, 82 insertions(+), 58 deletions(-)

-- 
2.31.1

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

Re: [ovs-dev] [PATCH] ovsdb-idl: Add memory report function.

2021-10-25 Thread Ilya Maximets 
On 10/23/21 08:51, Han Zhou wrote:
> 
> 
> On Thu, Oct 14, 2021 at 4:46 AM Ilya Maximets  > wrote:
>>
>> Added new function to return memory usage statistics for database
>> objects inside IDL.  Statistics similar to what ovsdb-server reports.
>> Not counting _Server database as it should be small, hence doesn't
>> worth adding extra code to the ovsdb-cs module.  Can be added later
>> if needed.
>>
>> ovs-vswitchd is a user in OVS, but this API will be mostly useful for
>> OVN daemons.
>>
>> Signed-off-by: Ilya Maximets mailto:i.maxim...@ovn.org>>
>> ---



> 
> Hi Ilya, I tested in sandbox:
> 
> [hanzhou@fedora tutorial]$ ovs-vsctl add-br br-int
> [hanzhou@fedora tutorial]$ ovs-appctl memory/show
> handlers:17 ports:1 revalidators:7 rules:5
> 
> It didn't show idl-xxx memory for vswitchd. Did I miss something?

Weird.  Are you sure that patch was correctly applied?

On my machine:

[ovs]# wget -c 
https://patchwork.ozlabs.org/project/openvswitch/patch/20211014114614.829400-1-i.maxim...@ovn.org/mbox/
 --content-disposition
[ovs]# git am -3 ovs-dev-ovsdb-idl-Add-memory-report-function..patch
[ovs]# make -j8 sandbox
[tutorial]# ovs-appctl memory/show
idl-cells:17
[tutorial]# ovs-vsctl add-br br-int
[tutorial]# ovs-appctl memory/show
handlers:11 idl-cells:96 ports:1 revalidators:5 rules:5
[tutorial]# ovs-vsctl del-br br-int
[tutorial]# ovs-appctl memory/show
idl-cells:17

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

Re: [ovs-dev] [PATCH] system-traffic: Fix typo in conntrack zones tests.

2021-10-25 Thread 0-day Robot 
Bleep bloop.  Greetings lin huang, 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 13
error: could not build fake ancestor
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0001 system-traffic: Fix typo in conntrack zones tests.
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".


Patch skipped due to previous failure.

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] system-traffic: Fix typo in conntrack zones tests.

2021-10-25 Thread lin huang 
Signed-off-by: Lin Huang 
---
tests/system-traffic.at | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 092de308b..528d22cfc 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -3915,7 +3915,7 @@ zone=4,limit=10,count=0
zone=5,limit=10,count=0
])

-dnl Test ct-get-limits for all zoens
+dnl Test ct-get-limits for all zones
AT_CHECK([ovs-appctl dpctl/ct-get-limits], [0], [dnl
default limit=10
zone=0,limit=5,count=5
--
2.12.2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn v2] Add a northbound interface to program MAC_Binding table

2021-10-25 Thread Lorenzo Bianconi 
> From: Karthik Chandrashekar 
> 

[...]
>  
> +static void
> +build_mac_binding_table(struct northd_context *ctx, struct hmap *ports)
> +{
> +const struct nbrec_mac_binding *nb_mac_binding;
> +NBREC_MAC_BINDING_FOR_EACH (nb_mac_binding, ctx->ovnnb_idl) {
> +struct ovn_port *op = ovn_port_find(
> +ports, nb_mac_binding->logical_port);
> +if (op && op->nbrp) {
> +struct ovn_datapath *od = op->od;
> +if (od && od->sb) {
> +const struct sbrec_mac_binding *mb =
> +mac_binding_lookup(ctx->sbrec_mac_binding_by_lport_ip,
> +   nb_mac_binding->logical_port,
> +   nb_mac_binding->ip);
> +if (!mb) {
> +mb = sbrec_mac_binding_insert(ctx->ovnsb_txn);
> +sbrec_mac_binding_set_logical_port(
> +mb, nb_mac_binding->logical_port);
> +sbrec_mac_binding_set_ip(mb, nb_mac_binding->ip);
> +sbrec_mac_binding_set_mac(mb, nb_mac_binding->mac);
> +sbrec_mac_binding_set_datapath(mb, od->sb);
> +} else {
> +sbrec_mac_binding_set_mac(mb, nb_mac_binding->mac);
> +}

I guess we should add some flags here to consider the case both the user/CMS
adds a static mac binding and pinctrl discovers the same IP address. What do
you think?

> +}
> +}
> +}
> +}
> +
>  /* Returns a string of the IP address of the router port 'op' that
>   * overlaps with 'ip_s".  If one is not found, returns NULL.
>   *
> @@ -14465,6 +14495,7 @@ ovnnb_db_run(struct northd_context *ctx,
>  build_mcast_groups(ctx, datapaths, ports, _groups, _groups);
>  build_meter_groups(ctx, _groups);
>  build_bfd_table(ctx, _connections, ports);
> +build_mac_binding_table(ctx, ports);
>  stopwatch_stop(BUILD_LFLOWS_CTX_STOPWATCH_NAME, time_msec());
>  stopwatch_start(BUILD_LFLOWS_STOPWATCH_NAME, time_msec());
>  build_lflows(ctx, datapaths, ports, _groups, _groups,
> diff --git a/northd/northd.h b/northd/northd.h
> index ffa2bbb4e..59f407332 100644
> --- a/northd/northd.h
> +++ b/northd/northd.h
> @@ -27,6 +27,7 @@ struct northd_context {
>  struct ovsdb_idl_index *sbrec_ha_chassis_grp_by_name;
>  struct ovsdb_idl_index *sbrec_mcast_group_by_name_dp;
>  struct ovsdb_idl_index *sbrec_ip_mcast_by_dp;
> +struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip;
>  
>  bool use_parallel_build;
>  };
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 39aa96055..5fba98812 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -23,6 +23,7 @@
>  #include "daemon.h"
>  #include "fatal-signal.h"
>  #include "lib/ip-mcast-index.h"
> +#include "lib/mac-binding-index.h"
>  #include "lib/mcast-group-index.h"
>  #include "memory.h"
>  #include "northd.h"
> @@ -904,6 +905,9 @@ main(int argc, char *argv[])
>  struct ovsdb_idl_index *sbrec_ip_mcast_by_dp
>  = ip_mcast_index_create(ovnsb_idl_loop.idl);
>  
> +struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip
> += mac_binding_index_create(ovnsb_idl_loop.idl);
> +
>  unixctl_command_register("sb-connection-status", "", 0, 0,
>   ovn_conn_show, ovnsb_idl_loop.idl);
>  
> @@ -959,6 +963,7 @@ main(int argc, char *argv[])
>  .sbrec_ha_chassis_grp_by_name = sbrec_ha_chassis_grp_by_name,
>  .sbrec_mcast_group_by_name_dp = sbrec_mcast_group_by_name_dp,
>  .sbrec_ip_mcast_by_dp = sbrec_ip_mcast_by_dp,
> +.sbrec_mac_binding_by_lport_ip = 
> sbrec_mac_binding_by_lport_ip,
>  .use_parallel_build = use_parallel_build,
>  };
>  
> diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
> index 99772932d..0e96b5de1 100644
> --- a/northd/ovn_northd.dl
> +++ b/northd/ovn_northd.dl
> @@ -1018,6 +1018,15 @@ sb::Out_MAC_Binding (._uuid= mb._uuid,
>  sb::Out_Port_Binding(.logical_port = mb.logical_port),
>  sb::Out_Datapath_Binding(._uuid = mb.datapath).
>  
> +sb::Out_MAC_Binding (._uuid= hash,
> +.logical_port = nb.logical_port,
> +.ip   = nb.ip,
> +.mac  = nb.mac,
> +.datapath = router._uuid) :-
> +nb in nb::MAC_Binding(),
> +rp in (.router = router, .json_name = 
> json_escape(nb.logical_port)),
> +var hash = hash128((nb.logical_port, nb.ip)).
> +
>  /*
>   * DHCP options: fixed table
>   */
> diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
> index 5dee04fe9..27dc91438 100644
> --- a/ovn-nb.ovsschema
> +++ b/ovn-nb.ovsschema
> @@ -1,7 +1,7 @@
>  {
>  "name": "OVN_Northbound",
> -"version": "5.33.1",
> -"cksum": "1931852754 30731",
> +"version": "5.34.0",
> +"cksum": "1872933846 30998",
>  "tables": {
>