Re: [ovs-dev] [PATCH 4/4] ofp-flow: Reduce memory consumption for ofputil_flow_mod, using minimatch.

[Armando M.]

On 20 March 2018 at 13:46, Ben Pfaff  wrote:

> Until now, struct ofputil_flow_mod, which represents an OpenFlow flow table
> modification request, has incorporated a struct match, which made the
> overall ofputil_flow_mod about 2.5 kB.  This is OK for a small number of
> flows, but absurdly inflates memory requirements when there are hundreds of
> thousands of flows.  This commit fixes the problem by changing struct match
> to struct minimatch inside ofputil_flow_mod, which reduces its size to
> about 100 bytes plus the actual size of the flow match (usually a few dozen
> bytes).
>
> This affects memory usage of ovs-ofctl (when it adds a large number of
> flows) more than ovs-vswitchd.
>
> Reported-by: Michael Ben-Ami 
> Signed-off-by: Ben Pfaff 
> ---
>  AUTHORS.rst|  1 +
>  include/openvswitch/ofp-flow.h |  2 +-
>  lib/learn.c| 12 --
>  lib/learning-switch.c  | 14 +--
>  lib/ofp-bundle.c   |  1 +
>  lib/ofp-flow.c | 89 ++
> 
>  ofproto/ofproto-dpif-xlate.c   |  6 ++-
>  ofproto/ofproto-dpif.c | 17 
>  ofproto/ofproto-dpif.h |  2 +-
>  ofproto/ofproto-provider.h |  2 +-
>  ofproto/ofproto.c  | 49 ++-
>  ovn/controller/ofctrl.c| 22 ++-
>  utilities/ovs-ofctl.c  | 81 +++---
> 
>  13 files changed, 185 insertions(+), 113 deletions(-)
>
> diff --git a/AUTHORS.rst b/AUTHORS.rst
> index dc69ba3f3c1f..81f9b6f28b88 100644
> --- a/AUTHORS.rst
> +++ b/AUTHORS.rst
> @@ -506,6 +506,7 @@ Marvin Pascual  mar...@pascual.com.ph
>  Maxime Brun m.b...@alphalink.fr
>  Madhu Venugopal maven...@gmail.com
>  Michael A. Collins  mike.a.coll...@ark-net.org
> +Michael Ben-Ami mben...@digitalocean.com
>  Michael Hu  m...@nicira.com
>  Michael J. Smalley  michaeljsmal...@gmail.com
>  Michael Mao m...@nicira.com
> diff --git a/include/openvswitch/ofp-flow.h b/include/openvswitch/ofp-flow
> .h
> index 17d48f12e060..2ff2e45b66c4 100644
> --- a/include/openvswitch/ofp-flow.h
> +++ b/include/openvswitch/ofp-flow.h
> @@ -73,7 +73,7 @@ void ofputil_flow_mod_flags_format(struct ds *, enum
> ofputil_flow_mod_flags);
>  struct ofputil_flow_mod {
>  struct ovs_list list_node; /* For queuing flow_mods. */
>
> -struct match match;
> +struct minimatch match;
>  int priority;
>
>  /* Cookie matching.  The flow_mod affects only flows that have
> cookies that
> diff --git a/lib/learn.c b/lib/learn.c
> index f5d15503fe79..c4d5b3e0c449 100644
> --- a/lib/learn.c
> +++ b/lib/learn.c
> @@ -91,14 +91,17 @@ learn_check(const struct ofpact_learn *learn, const
> struct match *src_match)
>   * 'ofpacts' and retains ownership of it.  'fm->ofpacts' will point into
> the
>   * 'ofpacts' buffer.
>   *
> + * The caller must eventually destroy fm->match.
> + *
>   * The caller has to actually execute 'fm'. */
>  void
>  learn_execute(const struct ofpact_learn *learn, const struct flow *flow,
>struct ofputil_flow_mod *fm, struct ofpbuf *ofpacts)
>  {
>  const struct ofpact_learn_spec *spec;
> +struct match match;
>
> -match_init_catchall(&fm->match);
> +match_init_catchall(&match);
>  fm->priority = learn->priority;
>  fm->cookie = htonll(0);
>  fm->cookie_mask = htonll(0);
> @@ -140,10 +143,10 @@ learn_execute(const struct ofpact_learn *learn,
> const struct flow *flow,
>
>  switch (spec->dst_type) {
>  case NX_LEARN_DST_MATCH:
> -mf_write_subfield(&spec->dst, &value, &fm->match);
> -match_add_ethernet_prereq(&fm->match, spec->dst.field);
> +mf_write_subfield(&spec->dst, &value, &match);
> +match_add_ethernet_prereq(&match, spec->dst.field);
>  mf_vl_mff_set_tlv_bitmap(
> -spec->dst.field, &fm->match.flow.tunnel.metadat
> a.present.map);
> +spec->dst.field, &match.flow.tunnel.metadata.pr
> esent.map);
>  break;
>
>  case NX_LEARN_DST_LOAD:
> @@ -173,6 +176,7 @@ learn_execute(const struct ofpact_learn *learn, const
> struct flow *flow,
>  }
>  }
>
> +minimatch_init(&fm->match, &match);
>  fm->ofpacts = ofpacts->data;
>  fm->ofpacts_len = ofpacts->size;
>  }
> diff --git a/lib/learning-switch.c b/lib/learning-switch.c
> index 3a9e015bbe9c..04eaa6e8e73f 100644
> --- a/lib/learning-switch.c
> +++ b/lib/learning-switch.c
> @@ -206,7 +206,6 @@ lswitch_handshake(struct lswitch *sw)
>  output.max_len = OFP_DEFAULT_MISS_SEND_LEN;
>
>  struct ofputil_flow_mod fm = {
> -.match = MATCH_CATCHALL_INITIALIZER,
>  .priority = 0,
>  .table_id = 0,
>  .command = OFPFC_ADD,
> @@ -216,8 +215,10 @@ lswitch_handshake(struct lswitch *sw)
>  

Re: [ovs-dev] [PATCH 3/4] flow, match, classifier: Add new functions for miniflow and minimatch.

[Armando M.]

On 20 March 2018 at 13:46, Ben Pfaff  wrote:

> The miniflow and minimatch APIs lack several of the features of the flow
> and match APIs.  This commit adds a few of the missing functions.
>
> These functions will be used for the first time in an upcoming commit.
>
> Signed-off-by: Ben Pfaff 
> ---
>  include/openvswitch/match.h |  4 
>  lib/classifier.c| 19 +++
>  lib/classifier.h|  4 
>  lib/flow.h  | 27 +++
>  lib/match.c | 44 ++
> ++
>  5 files changed, 98 insertions(+)
>
> diff --git a/include/openvswitch/match.h b/include/openvswitch/match.h
> index e5cf1a0d7c14..49333ae8f7a1 100644
> --- a/include/openvswitch/match.h
> +++ b/include/openvswitch/match.h
> @@ -246,6 +246,7 @@ struct minimatch {
>  };
>
>  void minimatch_init(struct minimatch *, const struct match *);
> +void minimatch_init_catchall(struct minimatch *);
>  void minimatch_clone(struct minimatch *, const struct minimatch *);
>  void minimatch_move(struct minimatch *dst, struct minimatch *src);
>  void minimatch_destroy(struct minimatch *);
> @@ -253,6 +254,7 @@ void minimatch_destroy(struct minimatch *);
>  void minimatch_expand(const struct minimatch *, struct match *);
>
>  bool minimatch_equal(const struct minimatch *a, const struct minimatch
> *b);
> +uint32_t minimatch_hash(const struct minimatch *, uint32_t basis);
>
>  bool minimatch_matches_flow(const struct minimatch *, const struct flow
> *);
>
> @@ -262,6 +264,8 @@ void minimatch_format(const struct minimatch *, const
> struct tun_table *,
>  char *minimatch_to_string(const struct minimatch *,
>const struct ofputil_port_map *, int priority);
>
> +bool minimatch_has_default_hidden_fields(const struct minimatch *);
> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/lib/classifier.c b/lib/classifier.c
> index cea9053b6f67..edb40c89c608 100644
> --- a/lib/classifier.c
> +++ b/lib/classifier.c
> @@ -1223,6 +1223,25 @@ classifier_find_match_exactly(const struct
> classifier *cls,
>  return retval;
>  }
>
> +/* Finds and returns a rule in 'cls' with priority 'priority' and exactly
> the
> + * same matching criteria as 'target', and that is visible in 'version'.
> + * Returns a null pointer if 'cls' doesn't contain an exact match visible
> in
> + * 'version'. */
> +const struct cls_rule *
> +classifier_find_minimatch_exactly(const struct classifier *cls,
> +  const struct minimatch *target, int
> priority,
> +  ovs_version_t version)
> +{
> +const struct cls_rule *retval;
> +struct cls_rule cr;
> +
> +cls_rule_init_from_minimatch(&cr, target, priority);
> +retval = classifier_find_rule_exactly(cls, &cr, version);
> +cls_rule_destroy(&cr);
> +
> +return retval;
> +}
> +
>  /* Checks if 'target' would overlap any other rule in 'cls' in
> 'version'.  Two
>   * rules are considered to overlap if both rules have the same priority
> and a
>   * packet could match both, and if both rules are visible in the same
> version.
> diff --git a/lib/classifier.h b/lib/classifier.h
> index 1263afdfd1a9..d1bd4aa12a78 100644
> --- a/lib/classifier.h
> +++ b/lib/classifier.h
> @@ -406,6 +406,10 @@ const struct cls_rule 
> *classifier_find_match_exactly(const
> struct classifier *,
>   const struct match *,
>   int priority,
>   ovs_version_t);
> +const struct cls_rule *classifier_find_minimatch_exactly(
> +const struct classifier *, const struct minimatch *,
> +int priority, ovs_version_t);
> +
>  bool classifier_is_empty(const struct classifier *);
>  int classifier_count(const struct classifier *);
>
> diff --git a/lib/flow.h b/lib/flow.h
> index 3331e2068ed8..5e41567b2668 100644
> --- a/lib/flow.h
> +++ b/lib/flow.h
> @@ -728,6 +728,11 @@ static inline ovs_be32 miniflow_get_be32(const struct
> miniflow *,
>  static inline uint16_t miniflow_get_vid(const struct miniflow *, size_t);
>  static inline uint16_t miniflow_get_tcp_flags(const struct miniflow *);
>  static inline ovs_be64 miniflow_get_metadata(const struct miniflow *);
> +static inline uint64_t miniflow_get_tun_metadata_present_map(
> +const struct miniflow *);
> +static inline uint32_t miniflow_get_recirc_id(const struct miniflow *);
> +static inline uint32_t miniflow_get_dp_hash(const struct miniflow *);
> +static inline ovs_be32 miniflow_get_ports(const struct miniflow *);
>
>  bool miniflow_equal(const struct miniflow *a, const struct miniflow *b);
>  bool miniflow_equal_in_minimask(const struct miniflow *a,
> @@ -857,6 +862,27 @@ miniflow_get_metadata(const struct miniflow *flow)
>  return MINIFLOW_GET_BE64(flow, metadata);
>  }
>
> +/* Returns the bitmap that indicates which tunnel metadata fields are
> present
>

Re: [ovs-dev] [PATCH 2/4] flow: Improve type-safety of MINIFLOW_GET_TYPE.

[Armando M.]

On 20 March 2018 at 13:46, Ben Pfaff  wrote:

> Until mow, this macro has blindly read the passed-in type's size, but
> that's unnecessarily risky.  This commit changes it to verify that the
> passed-in type is the same size as the field and, on GCC and Clang, that
> the types are compatible.  It also adds a version that does not check,
> for the one case where (currently) we deliberately read the wrong size,
> and updates a few uses to use more precise field names.
>
> Signed-off-by: Ben Pfaff 
> ---
>  lib/classifier.c  |  8 
>  lib/dpif-netdev.c |  3 ++-
>  lib/flow.c|  2 +-
>  lib/flow.h| 17 -
>  4 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/lib/classifier.c b/lib/classifier.c
> index ef7e3de0728c..cea9053b6f67 100644
> --- a/lib/classifier.c
> +++ b/lib/classifier.c
> @@ -495,8 +495,8 @@ classifier_count(const struct classifier *cls)
>  static inline ovs_be32 minimatch_get_ports(const struct minimatch *match)
>  {
>  /* Could optimize to use the same map if needed for fast path. */
> -return MINIFLOW_GET_BE32(match->flow, tp_src)
> -& MINIFLOW_GET_BE32(&match->mask->masks, tp_src);
> +return (miniflow_get_ports(match->flow)
> +& miniflow_get_ports(&match->mask->masks));
>  }
>
>  /* Inserts 'rule' into 'cls' in 'version'.  Until 'rule' is removed from
> 'cls',
> @@ -1501,7 +1501,7 @@ insert_subtable(struct classifier *cls, const struct
> minimask *mask)
>  /* Ports trie. */
>  ovsrcu_set_hidden(&subtable->ports_trie, NULL);
>  *CONST_CAST(int *, &subtable->ports_mask_len)
> -= 32 - ctz32(ntohl(MINIFLOW_GET_BE32(&mask->masks, tp_src)));
> += 32 - ctz32(ntohl(miniflow_get_ports(&mask->masks)));
>
>  /* List of rules. */
>  rculist_init(&subtable->rules_list);
> @@ -1696,7 +1696,7 @@ find_match_wc(const struct cls_subtable *subtable,
> ovs_version_t version,
>  unsigned int mbits;
>  ovs_be32 value, plens, mask;
>
> -mask = MINIFLOW_GET_BE32(&subtable->mask.masks, tp_src);
> +mask = miniflow_get_ports(&subtable->mask.masks);
>  value = ((OVS_FORCE ovs_be32 *)flow)[TP_PORTS_OFS32] & mask;
>  mbits = trie_lookup_value(&subtable->ports_trie, &value, &plens,
> 32);
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index b07fc6b8b327..be31fd0922b0 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -2200,7 +2200,8 @@ dp_netdev_pmd_lookup_flow(struct
> dp_netdev_pmd_thread *pmd,
>  {
>  struct dpcls *cls;
>  struct dpcls_rule *rule;
> -odp_port_t in_port = u32_to_odp(MINIFLOW_GET_U32(&key->mf, in_port));
> +odp_port_t in_port = u32_to_odp(MINIFLOW_GET_U32(&key->mf,
> + in_port.odp_port));
>  struct dp_netdev_flow *netdev_flow = NULL;
>
>  cls = dp_netdev_pmd_lookup_dpcls(pmd, in_port);
> diff --git a/lib/flow.c b/lib/flow.c
> index 38ff29c8cd14..09b66b81858f 100644
> --- a/lib/flow.c
> +++ b/lib/flow.c
> @@ -2007,7 +2007,7 @@ miniflow_hash_5tuple(const struct miniflow *flow,
> uint32_t basis)
>  }
>
>  /* Add both ports at once. */
> -hash = hash_add(hash, MINIFLOW_GET_U32(flow, tp_src));
> +hash = hash_add(hash, (OVS_FORCE uint32_t)
> miniflow_get_ports(flow));
>  }
>  out:
>  return hash_finish(hash, 42);
> diff --git a/lib/flow.h b/lib/flow.h
> index 770a07a62778..3331e2068ed8 100644
> --- a/lib/flow.h
> +++ b/lib/flow.h
> @@ -684,6 +684,14 @@ miniflow_get__(const struct miniflow *mf, size_t idx)
>  /* Get the value of the struct flow 'FIELD' as up to 8 byte wide integer
> type
>   * 'TYPE' from miniflow 'MF'. */
>  #define MINIFLOW_GET_TYPE(MF, TYPE, FIELD)  \
> +(BUILD_ASSERT(sizeof(TYPE) == sizeof(((struct flow *)0)->FIELD)),   \
> + BUILD_ASSERT_GCCONLY(__builtin_types_compatible_p(TYPE,
> typeof(((struct flow *)0)->FIELD))), \
> + MINIFLOW_GET_TYPE__(MF, TYPE, FIELD))
> +
> +/* Like MINIFLOW_GET_TYPE, but without checking that TYPE is the correct
> width
> + * for FIELD.  (This is useful for deliberately reading adjacent fields
> in one
> + * go.)  */
> +#define MINIFLOW_GET_TYPE__(MF, TYPE, FIELD)\
>  (MINIFLOW_IN_MAP(MF, FLOW_U64_OFFSET(FIELD))\
>   ? ((OVS_FORCE const TYPE *)miniflow_get__(MF,
> FLOW_U64_OFFSET(FIELD))) \
>   [FLOW_U64_OFFREM(FIELD) / sizeof(TYPE)]\
> @@ -806,7 +814,7 @@ miniflow_get_vid(const struct miniflow *flow, size_t n)
>  {
>  if (n < FLOW_MAX_VLAN_HEADERS) {
>  union flow_vlan_hdr hdr = {
> -.qtag = MINIFLOW_GET_BE32(flow, vlans[n])
> +.qtag = MINIFLOW_GET_BE32(flow, vlans[n].qtag)
>  };
>  return vlan_tci_to_vid(hdr.tci);
>  }
> @@ -849,6 +857,13 @@ miniflow_get_metadata(const struct miniflow *flow)
>  return MINIFLOW_GET_BE64(flow, metadata);
>  }
>
> +
> +/* Returns the 'tp_src

Re: [ovs-dev] [PATCH 1/4] match: Add 'tun_md' member to struct minimatch.

[Armando M.]

On 20 March 2018 at 13:46, Ben Pfaff  wrote:

> struct match has had a 'tun_md' member for a long time, but struct
> minimatch has never had one.  This doesn't matter for the purposes for
> which minimatch is currently used, but it means that a minimatch is not
> completely substitutable for a match and therefore blocks some new uses.
> This patch adds the member.
>
> Signed-off-by: Ben Pfaff 
> ---
>  include/openvswitch/match.h|  1 +
>  include/openvswitch/tun-metadata.h |  5 +
>  lib/match.c|  7 ++-
>  lib/tun-metadata.c | 17 +
>  4 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/include/openvswitch/match.h b/include/openvswitch/match.h
> index 61a67de2c413..e5cf1a0d7c14 100644
> --- a/include/openvswitch/match.h
> +++ b/include/openvswitch/match.h
> @@ -242,6 +242,7 @@ struct minimatch {
>  };
>  struct miniflow *flows[2];
>  };
> +struct tun_metadata_allocation *tun_md;
>  };
>
>  void minimatch_init(struct minimatch *, const struct match *);
> diff --git a/include/openvswitch/tun-metadata.h
> b/include/openvswitch/tun-metadata.h
> index 935c5c495027..dc0312ecb724 100644
> --- a/include/openvswitch/tun-metadata.h
> +++ b/include/openvswitch/tun-metadata.h
> @@ -101,6 +101,11 @@ struct tun_metadata_allocation {
>  bool valid; /* Set to true after any allocation
> occurs. */
>  };
>
> +struct tun_metadata_allocation *tun_metadata_allocation_clone(
> +const struct tun_metadata_allocation *);
> +void tun_metadata_allocation_copy(struct tun_metadata_allocation *,
> +  const struct tun_metadata_allocation *);
> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/lib/match.c b/lib/match.c
> index 97a5282997b0..3eab0fd5dec9 100644
> --- a/lib/match.c
> +++ b/lib/match.c
> @@ -1663,6 +1663,8 @@ minimatch_init(struct minimatch *dst, const struct
> match *src)
>  miniflow_alloc(dst->flows, 2, &tmp);
>  miniflow_init(dst->flow, &src->flow);
>  minimask_init(dst->mask, &src->wc);
> +
> +dst->tun_md = tun_metadata_allocation_clone(&src->tun_md);
>  }
>
>  /* Initializes 'dst' as a copy of 'src'.  The caller must eventually free
> 'dst'
> @@ -1677,6 +1679,7 @@ minimatch_clone(struct minimatch *dst, const struct
> minimatch *src)
> miniflow_get_values(src->flow), data_size);
>  memcpy(miniflow_values(&dst->mask->masks),
> miniflow_get_values(&src->mask->masks), data_size);
> +dst->tun_md = tun_metadata_allocation_clone(src->tun_md);
>  }
>
>  /* Initializes 'dst' with the data in 'src', destroying 'src'.  The
> caller must
> @@ -1686,6 +1689,7 @@ minimatch_move(struct minimatch *dst, struct
> minimatch *src)
>  {
>  dst->flow = src->flow;
>  dst->mask = src->mask;
> +dst->tun_md = src->tun_md;
>  }
>
>  /* Frees any memory owned by 'match'.  Does not free the storage in which
> @@ -1694,6 +1698,7 @@ void
>  minimatch_destroy(struct minimatch *match)
>  {
>  free(match->flow);
> +free(match->tun_md);
>  }
>
>  /* Initializes 'dst' as a copy of 'src'. */
> @@ -1702,7 +1707,7 @@ minimatch_expand(const struct minimatch *src, struct
> match *dst)
>  {
>  miniflow_expand(src->flow, &dst->flow);
>  minimask_expand(src->mask, &dst->wc);
> -memset(&dst->tun_md, 0, sizeof dst->tun_md);
> +tun_metadata_allocation_copy(&dst->tun_md, src->tun_md);
>  }
>
>  /* Returns true if 'a' and 'b' match the same packets, false otherwise.
> */
> diff --git a/lib/tun-metadata.c b/lib/tun-metadata.c
> index c0d9180e020e..f8a0e19524e9 100644
> --- a/lib/tun-metadata.c
> +++ b/lib/tun-metadata.c
> @@ -924,3 +924,20 @@ tun_metadata_match_format(struct ds *s, const struct
> match *match)
>  ds_put_char(s, ',');
>  }
>  }
> +
> +struct tun_metadata_allocation *
> +tun_metadata_allocation_clone(const struct tun_metadata_allocation *src)
> +{
> +return src && src->valid ? xmemdup(src, sizeof *src) : NULL;
> +}
> +
> +void
> +tun_metadata_allocation_copy(struct tun_metadata_allocation *dst,
> + const struct tun_metadata_allocation *src)
> +{
> +if (src && src->valid) {
> +*dst = *src;
> +} else {
> +memset(dst, 0, sizeof *dst);
> +}
> +}
> --
> 2.16.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>

LGTM. I wonder if this and the related patches are worth considering for a
backport.

Thanks!

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

[ovs-dev] [PATCH] bridge: fix crash when more than 32767 ports added

[lidejun1]

From: l00397770 

When creating a port using ovs-vsctl, its Openflow port number is automatically
assigned, and the max value is 32767. After adding 32767 ports, subsequent 
ports'
number are all 65535, and they are all inserted into a bridge's ifaces hmap,
but when these ports are deleted, their hamp_nodes are not removed by 
iface_destroy__,
this can cause ovs crash in the following call path:
bridge_reconfigure ---> bridge_add_ports ---> bridge_add_ports__ ---> 
iface_create
---> hmap_insert_at ---> hmap_expand_at ---> resize ---> hmap_insert_fast,
the bridge's ifaces hmap bucket is corrupted.

To fix the above issue, we check Openflow port number in iface_do_create: if 
the port
number is 65535, report an error and do not add this port into a bridge.
---
 vswitchd/bridge.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index d90997e..6f6314c 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1802,6 +1802,11 @@ iface_do_create(const struct bridge *br,
   iface_cfg->name, ovs_strerror(error));
 goto error;
 }
+if (*ofp_portp == OFPP_NONE) {
+VLOG_WARN("could not add network device %s to ofproto", 
iface_cfg->name);
+error = ERANGE;
+goto error;
+}
 
 VLOG_INFO("bridge %s: added interface %s on port %d",
   br->name, iface_cfg->name, *ofp_portp);
-- 
2.6.4.windows.1

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

[ovs-dev] [PATCH] bridge: fix crash when more than 32767 ports added

[lidejun1]

From: l00397770 

When creating a port using ovs-vsctl, its Openflow port number is automatically
assigned, and the max value is 32767. After adding 32767 ports, subsequent 
ports'
number are all 65535, and they are all inserted into a bridge's ifaces hmap,
but when these ports are deleted, their hamp_nodes are not removed by 
iface_destroy__,
this can cause ovs crash in the following call path:
bridge_reconfigure ---> bridge_add_ports ---> bridge_add_ports__ ---> 
iface_create
---> hmap_insert_at ---> hmap_expand_at ---> resize ---> hmap_insert_fast,
the bridge's ifaces hmap bucket is corrupted.

To fix the above issue, we check Openflow port number in iface_do_create: if 
the port
number is 65535, report an error and do not add this port into a bridge.

Signed-off-by: lidej...@huawei.com
Acked-By: jerry.lili...@huawei.com
---
 vswitchd/bridge.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index d90997e..6f6314c 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1802,6 +1802,11 @@ iface_do_create(const struct bridge *br,
   iface_cfg->name, ovs_strerror(error));
 goto error;
 }
+if (*ofp_portp == OFPP_NONE) {
+VLOG_WARN("could not add network device %s to ofproto", 
iface_cfg->name);
+error = ERANGE;
+goto error;
+}
 
 VLOG_INFO("bridge %s: added interface %s on port %d",
   br->name, iface_cfg->name, *ofp_portp);
-- 
2.6.4.windows.1

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

[ovs-dev] Updates on Interop ITX 2018

[Jessica Jones]

Hi,

 

Hope you're doing well!

 

Wishing to you all the best for Interop ITX 2018!

 

Would you be interested in acquiring an Attendees list of Interop ITX 2018?

 

FYI: We have updated 2,742 contacts of those who're attending at show like -

 

. IT Manager/Director

*   CIO/CTO
*   Business Leader
*   IT Operations
*   IT Architect
*   Software Developer/Engineer
*   Business/IT/Data Analyst
*   Network/Storage Engineer
*   Systems Admin
*   Application Development Manager
*   Infrastructure Manager
*   IT Support/System Support Lead

.

 

List contains: Contact Name, Company's Name, Phone Number, Fax Number,
Title, Email address, Complete Mailing Address, Company revenue, size, Web
address etc.

 

If you're interested to purchase the list, please let me know to send cost
and more information.

 

Thank you and I look forward to hear from you.

 

Regards,

Audrey Black

Marketing Manager

 

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

[ovs-dev] [RFC v2 2/2] ingress scheduling: Provide per interface ingress priority

[Billy O'Mahony]

Allow configuration to specify an ingress priority for interfaces.
Modify ovs-netdev datapath to act on this configuration so that packets
on interfaces with a higher priority will tend be processed ahead of
packets on lower priority interfaces.  This protects traffic on higher
priority interfaces from loss and latency as PMDs get overloaded.

Signed-off-by: Billy O'Mahony 
---
 include/openvswitch/ofp-parse.h |   3 ++
 lib/dpif-netdev.c   | 103 +++-
 lib/netdev-bsd.c|   1 +
 lib/netdev-dpdk.c   |  13 -
 lib/netdev-dummy.c  |   1 +
 lib/netdev-linux.c  |   1 +
 lib/netdev-provider.h   |  11 -
 lib/netdev-vport.c  |   1 +
 lib/netdev.c|  42 
 lib/netdev.h|   2 +
 vswitchd/bridge.c   |   2 +
 11 files changed, 165 insertions(+), 15 deletions(-)

diff --git a/include/openvswitch/ofp-parse.h b/include/openvswitch/ofp-parse.h
index 3fdd468..d77ab8f 100644
--- a/include/openvswitch/ofp-parse.h
+++ b/include/openvswitch/ofp-parse.h
@@ -33,6 +33,9 @@ extern "C" {
 struct match;
 struct mf_field;
 struct ofputil_port_map;
+struct tun_table;
+struct flow_wildcards;
+struct ofputil_port_map;
 
 struct ofp_protocol {
 const char *name;
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index b07fc6b..736d0b6 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -42,6 +43,7 @@
 #include "dpif.h"
 #include "dpif-netdev-perf.h"
 #include "dpif-provider.h"
+#include "netdev-provider.h"
 #include "dummy.h"
 #include "fat-rwlock.h"
 #include "flow.h"
@@ -487,6 +489,7 @@ static void dp_netdev_actions_free(struct dp_netdev_actions 
*);
 struct polled_queue {
 struct dp_netdev_rxq *rxq;
 odp_port_t port_no;
+uint8_t priority;
 };
 
 /* Contained by struct dp_netdev_pmd_thread's 'poll_list' member. */
@@ -626,6 +629,12 @@ struct dpif_netdev {
 uint64_t last_port_seq;
 };
 
+static void
+dp_netdev_process_rxq_port(struct dp_netdev_pmd_thread *pmd,
+   struct dp_netdev_rxq *rxq,
+   odp_port_t port_no,
+   unsigned int *rxd_cnt,
+   unsigned int *txd_cnt);
 static int get_port_by_number(struct dp_netdev *dp, odp_port_t port_no,
   struct dp_netdev_port **portp)
 OVS_REQUIRES(dp->port_mutex);
@@ -3259,15 +3268,16 @@ dp_netdev_pmd_flush_output_packets(struct 
dp_netdev_pmd_thread *pmd,
 return output_cnt;
 }
 
-static int
+static void
 dp_netdev_process_rxq_port(struct dp_netdev_pmd_thread *pmd,
struct dp_netdev_rxq *rxq,
-   odp_port_t port_no)
+   odp_port_t port_no,
+   unsigned int *rxd_cnt,
+   unsigned int *txd_cnt)
 {
 struct dp_packet_batch batch;
 struct cycle_timer timer;
 int error;
-int batch_cnt = 0, output_cnt = 0;
 uint64_t cycles;
 
 /* Measure duration for polling and processing rx burst. */
@@ -3279,17 +3289,17 @@ dp_netdev_process_rxq_port(struct dp_netdev_pmd_thread 
*pmd,
 error = netdev_rxq_recv(rxq->rx, &batch);
 if (!error) {
 /* At least one packet received. */
+*rxd_cnt = batch.count;
 *recirc_depth_get() = 0;
 pmd_thread_ctx_time_update(pmd);
 
-batch_cnt = batch.count;
 dp_netdev_input(pmd, &batch, port_no);
 
 /* Assign processing cycles to rx queue. */
 cycles = cycle_timer_stop(&pmd->perf_stats, &timer);
 dp_netdev_rxq_add_cycles(rxq, RXQ_CYCLES_PROC_CURR, cycles);
 
-output_cnt = dp_netdev_pmd_flush_output_packets(pmd, false);
+*txd_cnt = dp_netdev_pmd_flush_output_packets(pmd, false);
 } else {
 /* Discard cycles. */
 cycle_timer_stop(&pmd->perf_stats, &timer);
@@ -3299,11 +3309,11 @@ dp_netdev_process_rxq_port(struct dp_netdev_pmd_thread 
*pmd,
 VLOG_ERR_RL(&rl, "error receiving data from %s: %s",
 netdev_rxq_get_name(rxq->rx), ovs_strerror(error));
 }
+*txd_cnt = 0;
 }
 
 pmd->ctx.last_rxq = NULL;
 
-return batch_cnt + output_cnt;
 }
 
 static struct tx_port *
@@ -3935,11 +3945,16 @@ dpif_netdev_run(struct dpif *dpif)
 HMAP_FOR_EACH (port, node, &dp->ports) {
 if (!netdev_is_pmd(port->netdev)) {
 int i;
+unsigned int rxd_cnt;
+unsigned int txd_cnt;
 
 for (i = 0; i < port->n_rxq; i++) {
-if (dp_netdev_process_rxq_port(non_pmd,
-   &port->rxqs[i],
-   port->port_no)) {
+dp_netdev_process_rxq_port(non_pmd,
+

[ovs-dev] [RFC v2 1/2] ingress scheduling: schema and docs

[Billy O'Mahony]

Signed-off-by: Billy O'Mahony 
---
 Documentation/howto/dpdk.rst | 18 ++
 vswitchd/vswitch.ovsschema   |  9 +++--
 vswitchd/vswitch.xml | 40 
 3 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index 79b626c..fca353a 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -237,6 +237,24 @@ respective parameter. To disable the flow control at tx 
side, run::
 
 $ ovs-vsctl set Interface dpdk-p0 options:tx-flow-ctrl=false
 
+Ingress Scheduling
+--
+
+The ingress scheduling feature is described in general in
+``ovs-vswitchd.conf.db (5)``.
+
+Ingress scheduling currently only supports setting a priority for incoming
+packets for an entire interface. Priority levels 0 (lowest) to 3 (highest) are
+supported.  The default priority is 0.
+
+Interfaces of type ``dpdk`` and ``dpdkvhostuserclient`` support ingress
+scheduling.
+
+To prioritize packets on a particular port:
+
+$ ovs-vsctl set Interface dpdk0 \
+ingress_sched=port_prio=3
+
 pdump
 -
 
diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
index 90e50b6..5552d87 100644
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@@ -1,6 +1,6 @@
 {"name": "Open_vSwitch",
- "version": "7.15.1",
- "cksum": "3682332033 23608",
+ "version": "7.15.2",
+ "cksum": "2390903851 23814",
  "tables": {
"Open_vSwitch": {
  "columns": {
@@ -352,6 +352,11 @@
"minInteger": 1},
"min": 0,
"max": 1}},
+   "ingress_sched": {
+ "type": {"key": {"type": "string",
+  "enum": ["set", ["port_prio"]]},
+  "value": "string",
+  "min": 0, "max": 1}},
"error": {
  "type": {"key": "string", "min": 0, "max": 1}}},
  "indexes": [["name"]]},
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index f899a19..9ab3960 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -3071,6 +3071,33 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch 
options:peer=p1 \
   
 
 
+
+  
+   Configuration to allow certain traffic to be prioritized.  This means
+   some combination of:
+  
+  
+
+ prioritized packets are forwarded to their destination port before
+ non-prioritized
+
+
+ prioritized packets are less likely to be dropped in an overloaded
+ situation than prioritized packets
+
+  
+  
+   Ingress scheduling is supported with the best effort of the Interface
+   type and datapath.  Currently the only field supported is port_prio
+   which applies a priority to all incoming packets on the Interface.
+  
+  
+
+ Priority levels are 0 (lowest) to 3 (highest).
+
+  
+
+
 
   
 BFD, defined in RFC 5880 and RFC 5881, allows point-to-point
@@ -3627,6 +3654,19 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch 
options:peer=p1 \
   
 
 
+
+  
+Ingress Scheduling allows XXX TODO BOM
+  
+
+  
+The ingress priority of the port: 0 lowest to 3 highest. Higher
+priority ports are read more frequently than lower priority ports. This
+providing enhanced protection against being dropped due to Rx queue
+overflow to packets ingressing on those ports.
+  
+
+
 
   The overall purpose of these columns is described under Common
   Columns at the beginning of this document.
-- 
2.7.4

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

[ovs-dev] [RFC v2 0/2] Ingress Scheduling

[Billy O'Mahony]

Hi All,

I've updated the original RFC patch to account for two sets of comments. See
below.

I had originally intended to make this a v1 patch but two items are
outstanding:
* further testing needs to be completed to determine the full effect on vports
* re-configuration of the priorities at run-time is not supported in RFCv2

I want to allow re-configuration of the port priorities without calling
netdev_request_reconfigure as that will, I understand, restart the
netdevs and drop packets.  I am thinking that adding the ingress_prio config
smap to ofproto_port_set_config call in bridge_reconfigure and then calling
dp_netdev_reload_pmd__ from dpif_netdev_port_set_config will end up forcing the
pmd to reload_queues_and_ports and giving effect to the new priorities.  But
I'd appreciate guidance on that approach.

Original Description:
This patch set implements the 'preferential read' part of the feature of
ingress scheduling described at OvS 2017 Fall Conference
https://www.slideshare.net/LF_OpenvSwitch/lfovs17ingress-scheduling-82280320.

It allows configuration to specify an ingress priority for an entire
interface. This protects traffic on higher priority interfaces from loss and
latency as PMDs get overloaded.

Results so far a are very promising; For a uniform traffic distribution as
total offered load increases loss starts on the lowest priority port first and
the highest priority port last.

History:

RFCv1:
Initial version.

RFCv2:
* Keep ingress prio config in netdev base rather than in each netdev type.
* Account for differing rxq lengths
* Applies clean to 4299145

Billy O'Mahony (2):
  ingress scheduling: schema and docs
  ingress scheduling: Provide per interface ingress priority

 Documentation/howto/dpdk.rst|  18 +++
 include/openvswitch/ofp-parse.h |   3 ++
 lib/dpif-netdev.c   | 103 +++-
 lib/netdev-bsd.c|   1 +
 lib/netdev-dpdk.c   |  13 -
 lib/netdev-dummy.c  |   1 +
 lib/netdev-linux.c  |   1 +
 lib/netdev-provider.h   |  11 -
 lib/netdev-vport.c  |   1 +
 lib/netdev.c|  42 
 lib/netdev.h|   2 +
 vswitchd/bridge.c   |   2 +
 vswitchd/vswitch.ovsschema  |   9 +++-
 vswitchd/vswitch.xml|  40 
 14 files changed, 230 insertions(+), 17 deletions(-)

-- 
2.7.4

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

[ovs-dev] [PATCH v7 5/5] tests: Add tests for stopwatch module

[Mark Michelson]

From: Jakub Sitnicki 

Check if stopwatch module is calculating statistics as expected.

Signed-off-by: Jakub Sitnicki 
Signed-off-by: Mark Michelson 
---
 lib/stopwatch.c|   2 +-
 tests/automake.mk  |   3 +-
 tests/library.at   |   5 ++
 tests/test-stopwatch.c | 195 +
 4 files changed, 203 insertions(+), 2 deletions(-)
 create mode 100644 tests/test-stopwatch.c

diff --git a/lib/stopwatch.c b/lib/stopwatch.c
index 4cce7d911..80677d000 100644
--- a/lib/stopwatch.c
+++ b/lib/stopwatch.c
@@ -245,7 +245,7 @@ stopwatch_get_stats_protected(const char *name,
 return false;
 }
 
-stats->count = perf->samples;
+stats->count = perf->n_samples;
 stats->unit = perf->units;
 stats->max = perf->max;
 stats->min = perf->min;
diff --git a/tests/automake.mk b/tests/automake.mk
index 780d3b830..bab91a466 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -365,7 +365,8 @@ tests_ovstest_SOURCES = \
tests/test-uuid.c \
tests/test-bitmap.c \
tests/test-vconn.c \
-   tests/test-aa.c
+   tests/test-aa.c \
+   tests/test-stopwatch.c
 
 if !WIN32
 tests_ovstest_SOURCES += \
diff --git a/tests/library.at b/tests/library.at
index 5bfea2f69..99541b0c9 100644
--- a/tests/library.at
+++ b/tests/library.at
@@ -251,3 +251,8 @@ AT_CLEANUP
 AT_SETUP([rcu])
 AT_CHECK([ovstest test-rcu-quiesce], [0], [])
 AT_CLEANUP
+
+AT_SETUP([stopwatch module])
+AT_CHECK([ovstest test-stopwatch], [0], [..
+], [ignore])
+AT_CLEANUP
diff --git a/tests/test-stopwatch.c b/tests/test-stopwatch.c
new file mode 100644
index 0..1270cd936
--- /dev/null
+++ b/tests/test-stopwatch.c
@@ -0,0 +1,195 @@
+/*
+ * Copyright (c) 2018 Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include 
+#undef NDEBUG
+#include "stopwatch.h"
+#include 
+#include 
+#include 
+#include "ovstest.h"
+#include "util.h"
+
+#define MAX_SAMPLES 100
+#define UNIT SW_MS
+
+struct test_data {
+const char *name;
+unsigned long long samples[MAX_SAMPLES];
+size_t num_samples;
+struct stopwatch_stats expected_stats;
+};
+
+static struct test_data data_sets[] = {
+{
+.name = "1-interval-zero-length",
+.samples = { 0, 0 },
+.num_samples = 2,
+.expected_stats = {
+.count = 1,
+.unit = UNIT,
+.max = 0,
+.min = 0,
+.pctl_95 = 0,
+.ewma_50 = 0,
+.ewma_1 = 0,
+},
+},
+{
+.name = "1-interval-unit-length",
+.samples = { 0, 1 },
+.num_samples = 2,
+.expected_stats = {
+.count = 1,
+.unit = UNIT,
+.max = 1,
+.min = 1,
+.pctl_95 = 0,
+.ewma_50 = 1,
+.ewma_1 = 1,
+},
+},
+{
+.name = "10-intervals-unit-length",
+.samples = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11 },
+.num_samples = 11,
+.expected_stats = {
+.count = 10,
+.unit = UNIT,
+.max = 1,
+.min = 1,
+.pctl_95 = 1,
+.ewma_50 = 1,
+.ewma_1 = 1,
+},
+},
+{
+.name = "10-intervals-linear-growth",
+.samples = { 1, 2, 4, 7, 11, 16, 22, 29, 37, 46, 56 },
+.num_samples = 11,
+.expected_stats = {
+.count = 10,
+.unit = UNIT,
+.max = 10,
+.min = 1,
+.pctl_95 = 10.0,
+.ewma_50 = 9.0,
+.ewma_1 = 1.4,
+},
+},
+{
+.name = "60-intervals-unit-length",
+.samples = { 1,  2,  3,  4,  5,  6,  7,  8,  9, 10,
+11, 12, 13, 14, 15, 16, 17, 18, 19, 20,
+21, 22, 23, 24, 25, 26, 27, 28, 29, 30,
+31, 32, 33, 34, 35, 36, 37, 38, 39, 40,
+41, 42, 43, 44, 45, 46, 47, 48, 49, 50,
+51, 52, 53, 54, 55, 56, 57, 58, 59, 60,
+61, },
+.num_samples = 61,
+.expected_stats = {
+.count = 60,
+.unit = UNIT,
+.max = 1,
+.min = 1,
+.pctl_95 = 1,
+.ewma_50 = 1,
+.ewma_1 = 1,
+},
+},
+{
+.name = "60-intervals-linear-growth",
+.samples = {   1,2,4,7,   11,   16,   22,   29,   37,   46,
+  56,   6

[ovs-dev] [PATCH v6 2/5] Measure timing of ovn-controller loops.

[Mark Michelson]

This modifies ovn-controller to measure the amount of time it takes to
detect a change in the southbound database and generate the resulting
flow table. This may require multiple iterations of the ovn-controller
loop.

The statistics can be queried using:

ovs-appctl -t ovn-controller stopwatch/show ovn-controller-loop

The statistics can be reset using:

ovs-appctl -t ovn-controller stopwatch/reset ovn-controller-loop

Signed-off-by: Mark Michelson 
---
 ovn/controller/ovn-controller.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 7592bda25..abd253fca 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -57,6 +57,9 @@
 #include "stream.h"
 #include "unixctl.h"
 #include "util.h"
+#include "timeval.h"
+#include "timer.h"
+#include "stopwatch.h"
 
 VLOG_DEFINE_THIS_MODULE(main);
 
@@ -67,6 +70,8 @@ static unixctl_cb_func inject_pkt;
 #define DEFAULT_BRIDGE_NAME "br-int"
 #define DEFAULT_PROBE_INTERVAL_MSEC 5000
 
+#define CONTROLLER_LOOP_STOPWATCH_NAME "ovn-controller-loop"
+
 static void update_probe_interval(struct controller_ctx *,
   const char *ovnsb_remote);
 static void parse_options(int argc, char *argv[]);
@@ -639,8 +644,10 @@ main(int argc, char *argv[])
 unixctl_command_register("inject-pkt", "MICROFLOW", 1, 1, inject_pkt,
  &pending_pkt);
 
+stopwatch_create(CONTROLLER_LOOP_STOPWATCH_NAME, SW_MS);
 /* Main loop. */
 exiting = false;
+unsigned int our_seqno = 0;
 while (!exiting) {
 /* Check OVN SB database. */
 char *new_ovnsb_remote = get_ovnsb_remote(ovs_idl_loop.idl);
@@ -659,6 +666,12 @@ main(int argc, char *argv[])
 .ovnsb_idl_txn = ovsdb_idl_loop_run(&ovnsb_idl_loop),
 };
 
+if (our_seqno != ovsdb_idl_get_seqno(ctx.ovnsb_idl)) {
+stopwatch_start(CONTROLLER_LOOP_STOPWATCH_NAME,
+time_msec());
+our_seqno = ovsdb_idl_get_seqno(ctx.ovnsb_idl);
+}
+
 update_probe_interval(&ctx, ovnsb_remote);
 
 update_ssl_config(ctx.ovs_idl);
@@ -728,6 +741,9 @@ main(int argc, char *argv[])
 ofctrl_put(&flow_table, &pending_ct_zones,
get_nb_cfg(ctx.ovnsb_idl));
 
+stopwatch_stop(CONTROLLER_LOOP_STOPWATCH_NAME,
+   time_msec());
+
 hmap_destroy(&flow_table);
 }
 if (ctx.ovnsb_idl_txn) {
@@ -792,6 +808,7 @@ main(int argc, char *argv[])
 ofctrl_wait();
 pinctrl_wait(&ctx);
 }
+
 ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop);
 
 if (ovsdb_idl_loop_commit_and_wait(&ovs_idl_loop) == 1) {
-- 
2.14.3

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

[ovs-dev] [PATCH v7 1/5] Add stopwatch timing API

[Mark Michelson]

This is similar to the existing coverage and perf-counter APIs in OVS.
However, rather than keeping counters, this is aimed at timing how long
operations take to perform. "Operations" in this case can be anything
from a loop iteration, to a function, to something more complex.

The library allows for named stopwatches to be created. From there, the
stopwatch can be started and stopped via stopwatch_start() and
stopwatch_stop(). After each run, statistics for the stopwatch will be
calculated.

Statistics for a particular stopwatch can be queried from the command
line by using ovs-appctl -t  stopwatch/show .

Statistics can be reset from the command line using
ovs-appctl -t  stopwatch/reset 

Signed-off-by: Mark Michelson 
---
 lib/automake.mk |   2 +
 lib/stopwatch.c | 483 
 lib/stopwatch.h |  41 +
 3 files changed, 526 insertions(+)
 create mode 100644 lib/stopwatch.c
 create mode 100644 lib/stopwatch.h

diff --git a/lib/automake.mk b/lib/automake.mk
index c7eda6e31..0161a946b 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -224,6 +224,8 @@ lib_libopenvswitch_la_SOURCES = \
lib/pcap-file.h \
lib/perf-counter.h \
lib/perf-counter.c \
+   lib/stopwatch.h \
+   lib/stopwatch.c \
lib/poll-loop.c \
lib/process.c \
lib/process.h \
diff --git a/lib/stopwatch.c b/lib/stopwatch.c
new file mode 100644
index 0..d33976c28
--- /dev/null
+++ b/lib/stopwatch.c
@@ -0,0 +1,483 @@
+/* Copyright (c) 2017 Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include 
+
+#include "stopwatch.h"
+#include "openvswitch/shash.h"
+#include "openvswitch/vlog.h"
+#include "unixctl.h"
+#include "openvswitch/dynamic-string.h"
+#include "openvswitch/poll-loop.h"
+#include "ovs-thread.h"
+#include 
+#include "socket-util.h"
+
+VLOG_DEFINE_THIS_MODULE(stopwatch);
+
+struct average {
+double average; /* Moving average */
+double alpha;   /* Weight given to new samples */
+};
+
+#define MARKERS 5
+
+/* Number of samples to collect before reporting P-square calculated
+ * percentile
+ */
+#define P_SQUARE_MIN 50
+
+/* The naming of these fields is based on the naming used in the
+ * P-square algorithm paper.
+ */
+struct percentile {
+int n[MARKERS];
+double n_prime[MARKERS];
+double q[MARKERS];
+double dn[MARKERS];
+unsigned long long samples[P_SQUARE_MIN];
+double percentile;
+};
+
+struct stopwatch {
+enum stopwatch_units units;
+unsigned long long n_samples;
+unsigned long long max;
+unsigned long long min;
+struct percentile pctl;
+struct average short_term;
+struct average long_term;
+unsigned long long sample_start;
+bool sample_in_progress;
+};
+
+enum stopwatch_op {
+OP_START_SAMPLE,
+OP_END_SAMPLE,
+OP_RESET,
+OP_SHUTDOWN,
+};
+
+struct stopwatch_packet {
+enum stopwatch_op op;
+char name[32];
+unsigned long long time;
+};
+
+static struct shash stopwatches = SHASH_INITIALIZER(&stopwatches);
+static struct ovs_mutex stopwatches_lock = OVS_MUTEX_INITIALIZER;
+
+static int stopwatch_pipe[2];
+static pthread_t stopwatch_thread_id;
+
+static const char *unit_name[] = {
+[SW_MS] = "msec",
+[SW_US] = "usec",
+[SW_NS] = "nsec",
+};
+
+/* Percentile value we are calculating */
+#define P 0.95
+
+static int
+comp_samples(const void *left, const void *right)
+{
+const double *left_d = left;
+const double *right_d = right;
+
+return (int) *right_d - *left_d;
+}
+
+/* Calculate the percentile using the P-square algorithm. For more
+ * information, see https://www1.cse.wustl.edu/~jain/papers/ftp/psqr.pdf
+ */
+static void
+calc_percentile(unsigned long long n_samples, struct percentile *pctl,
+unsigned long long new_sample)
+{
+
+if (n_samples < P_SQUARE_MIN) {
+pctl->samples[n_samples - 1] = new_sample;
+}
+
+/* For the first MARKERS samples, we calculate the percentile
+ * in the traditional way in the pct->q array.
+ */
+if (n_samples <= MARKERS) {
+pctl->q[n_samples - 1] = new_sample;
+qsort(pctl->q, n_samples, sizeof *pctl->q, comp_samples);
+if (n_samples == MARKERS) {
+pctl->n[0] = 0;
+pctl->n[1] = 1;
+pctl->n[2] = 2;
+pctl->n[3] = 3;
+pctl->n[4] = 4;
+
+pctl->n_prime[0] = 0;
+pctl->n_prime[1] = 2 * P;
+  

[ovs-dev] [PATCH v7 4/5] stopwatch: Add API for waiting until samples have been processed

[Mark Michelson]

From: Jakub Sitnicki 

Will be used for testing the module.

Signed-off-by: Jakub Sitnicki 
Signed-off-by: Mark Michelson 
---
 lib/stopwatch.c | 34 ++
 lib/stopwatch.h |  3 +++
 2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/lib/stopwatch.c b/lib/stopwatch.c
index 7f036e894..4cce7d911 100644
--- a/lib/stopwatch.c
+++ b/lib/stopwatch.c
@@ -66,6 +66,7 @@ struct stopwatch {
 enum stopwatch_op {
 OP_START_SAMPLE,
 OP_END_SAMPLE,
+OP_SYNC,
 OP_RESET,
 OP_SHUTDOWN,
 };
@@ -78,6 +79,7 @@ struct stopwatch_packet {
 
 static struct shash stopwatches = SHASH_INITIALIZER(&stopwatches);
 static struct ovs_mutex stopwatches_lock = OVS_MUTEX_INITIALIZER;
+static pthread_cond_t stopwatches_sync = PTHREAD_COND_INITIALIZER;
 
 static int stopwatch_pipe[2];
 static pthread_t stopwatch_thread_id;
@@ -233,12 +235,12 @@ add_sample(struct stopwatch *sw, unsigned long long 
new_sample)
 }
 
 static bool
-performance_get_stats_protected(const char *name,
-struct performance_stats *stats)
+stopwatch_get_stats_protected(const char *name,
+struct stopwatch_stats *stats)
 {
-struct performance *perf;
+struct stopwatch *perf;
 
-perf = shash_find_data(&performances, name);
+perf = shash_find_data(&stopwatches, name);
 if (!perf) {
 return false;
 }
@@ -255,13 +257,13 @@ performance_get_stats_protected(const char *name,
 }
 
 bool
-performance_get_stats(const char *name, struct performance_stats *stats)
+stopwatch_get_stats(const char *name, struct stopwatch_stats *stats)
 {
 bool found = false;
 
-ovs_mutex_lock(&performances_lock);
-found = performance_get_stats_protected(name, stats);
-ovs_mutex_unlock(&performances_lock);
+ovs_mutex_lock(&stopwatches_lock);
+found = stopwatch_get_stats_protected(name, stats);
+ovs_mutex_unlock(&stopwatches_lock);
 
 return found;
 }
@@ -413,6 +415,9 @@ stopwatch_thread(void *ign OVS_UNUSED)
 case OP_END_SAMPLE:
 stopwatch_end_sample_protected(&pkt);
 break;
+case OP_SYNC:
+xpthread_cond_signal(&stopwatches_sync);
+break;
 case OP_RESET:
 stopwatch_reset_protected(&pkt);
 break;
@@ -515,3 +520,16 @@ stopwatch_stop(const char *name, unsigned long long ts)
 ovs_strlcpy(pkt.name, name, sizeof(pkt.name));
 write(stopwatch_pipe[1], &pkt, sizeof(pkt));
 }
+
+void
+stopwatch_sync(void)
+{
+struct stopwatch_packet pkt = {
+.op = OP_SYNC,
+};
+
+ovs_mutex_lock(&stopwatches_lock);
+write(stopwatch_pipe[1], &pkt, sizeof(pkt));
+ovs_mutex_cond_wait(&stopwatches_sync, &stopwatches_lock);
+ovs_mutex_unlock(&stopwatches_lock);
+}
diff --git a/lib/stopwatch.h b/lib/stopwatch.h
index fac5de2c6..6ee7291d9 100644
--- a/lib/stopwatch.h
+++ b/lib/stopwatch.h
@@ -51,4 +51,7 @@ void stopwatch_stop(const char *name, unsigned long long ts);
 /* Retrieve statistics calculated from collected samples */
 bool stopwatch_get_stats(const char *name, struct stopwatch_stats *stats);
 
+/* Block until all enqueued samples have been processed. */
+void stopwatch_sync(void);
+
 #endif /* stopwatch.h */
-- 
2.14.3

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

[ovs-dev] [PATCH v7 3/5] stopwatch: Add API for retrieving calculated statistics

[Mark Michelson]

From: Jakub Sitnicki 

Will be used for testing the module.

Signed-off-by: Jakub Sitnicki 
Signed-off-by: Mark Michelson 
---
 lib/stopwatch.c | 34 ++
 lib/stopwatch.h | 13 +
 2 files changed, 47 insertions(+)

diff --git a/lib/stopwatch.c b/lib/stopwatch.c
index d33976c28..7f036e894 100644
--- a/lib/stopwatch.c
+++ b/lib/stopwatch.c
@@ -232,6 +232,40 @@ add_sample(struct stopwatch *sw, unsigned long long 
new_sample)
 calc_average(&sw->long_term, new_sample);
 }
 
+static bool
+performance_get_stats_protected(const char *name,
+struct performance_stats *stats)
+{
+struct performance *perf;
+
+perf = shash_find_data(&performances, name);
+if (!perf) {
+return false;
+}
+
+stats->count = perf->samples;
+stats->unit = perf->units;
+stats->max = perf->max;
+stats->min = perf->min;
+stats->pctl_95 = perf->pctl.percentile;
+stats->ewma_50 = perf->short_term.average;
+stats->ewma_1 = perf->long_term.average;
+
+return true;
+}
+
+bool
+performance_get_stats(const char *name, struct performance_stats *stats)
+{
+bool found = false;
+
+ovs_mutex_lock(&performances_lock);
+found = performance_get_stats_protected(name, stats);
+ovs_mutex_unlock(&performances_lock);
+
+return found;
+}
+
 static void
 stopwatch_print(struct stopwatch *sw, const char *name,
   struct ds *s)
diff --git a/lib/stopwatch.h b/lib/stopwatch.h
index 61f814523..fac5de2c6 100644
--- a/lib/stopwatch.h
+++ b/lib/stopwatch.h
@@ -24,6 +24,16 @@ enum stopwatch_units {
 SW_NS,
 };
 
+struct stopwatch_stats {
+unsigned long long count;/* Total number of samples. */
+enum stopwatch_units unit; /* Unit of following values. */
+unsigned long long max;  /* Maximum value. */
+unsigned long long min;  /* Minimum value. */
+double pctl_95;  /* 95th percentile. */
+double ewma_50;  /* Exponentially weighted moving average 
(alpha 0.50). */
+double ewma_1;   /* Exponentially weighted moving average 
(alpha 0.01). */
+};
+
 /* Create a new stopwatch.
  * The "units" are not used for any calculations but are printed when
  * statistics are requested.
@@ -38,4 +48,7 @@ void stopwatch_start(const char *name, unsigned long long ts);
  */
 void stopwatch_stop(const char *name, unsigned long long ts);
 
+/* Retrieve statistics calculated from collected samples */
+bool stopwatch_get_stats(const char *name, struct stopwatch_stats *stats);
+
 #endif /* stopwatch.h */
-- 
2.14.3

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

[ovs-dev] [PATCH v7 0/5] Stopwatch Library

[Mark Michelson]

This set of commits adds a new library for OVS that allows for measuring
the performance of operations in OVS and compiling statistics from these
measurements.

For developers, this can provide a measurement of something that is
either finer or coarser-grained than what is easily measured with a
profiler.

v6 -> v7:
* Changed the measurement in ovn-controller to be more narrow, and
  changed the name of the measurement as well.
* Changed return type of some internal stopwatch functions to 'void'
  since return type was irrelevant.

v5 -> v6:
* Fixed mixed-case constant name in ovn-controller.c
* Removed FIXME comments in test-stopwatch.c
* Fixed clang warning about uninitialized fields in initializer.

v4 -> v5:
* The library has changed from focusing on "performance" to being a
  stopwatch.
* Jakub Sitnicki wrote some tests, and these are included here.
* Based on tests, the 95th percentile is now calculated in the
  traditional way for the first 50 measurements and then switches
  to P-square.
* The sample structure has been removed since storing the end time
  was not necessary.
* A new boolean indicating a sample is in progress has been added. This
  corrects an issue that prevented a timestamp of 0 from being used as a
  start time.
* A bug was fixed where the minimum time was always 0.

v3 -> v4:
* The samples vector has been removed in favor of using moving
  calculations. The average is calculated using a common exponential
  weighted moving average. The 95th percentile is calculated using the
  P-square algorithm. This greatly reduces the memory overhead since
  there is no longer a need to maintain any sort of history of samples.

* A performance/reset CLI command has been added.

* The wording of the commit in patch 2 has been altered to more
  accurately describe what is being measured.

v2 -> v3:
* A background thread has been introduced that maintains the performance
  structures.
* To reduce contention in the threads that are reporting performance
  measurements, they no longer acquire a mutex. Instead, they pass
  information over a pipe to the background thread.
* To reduce memory usage, the sample vectors have a maximum size they
  can reach. In addition, the measured intervals are now smaller. Rather
  than one minute, five minute, and ten minute measurements, we now do
  ten second, thirty second, and one minute measurements.

v1 -> v2:
In version 1, there was a patch included that would save statistics to
the OVS database. Based on feedback from that, I decided to forgo
database support for the time being. If database support were to be
added, using a time series database rather than the OVS database would
be the way to go.

Removal of the database patch had a snowball effect that has reduced the
overall size of the patchset.

Jakub Sitnicki (3):
  stopwatch: Add API for retrieving calculated statistics
  stopwatch: Add API for waiting until samples have been processed
  tests: Add tests for stopwatch module

Mark Michelson (2):
  Add stopwatch timing API
  Measure timing of ovn-controller flow creation.

 lib/automake.mk |   2 +
 lib/stopwatch.c | 535 
 lib/stopwatch.h |  57 +
 ovn/controller/ovn-controller.c |  13 +
 tests/automake.mk   |   3 +-
 tests/library.at|   5 +
 tests/test-stopwatch.c  | 195 +++
 7 files changed, 809 insertions(+), 1 deletion(-)
 create mode 100644 lib/stopwatch.c
 create mode 100644 lib/stopwatch.h
 create mode 100644 tests/test-stopwatch.c

-- 
2.14.3

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

[ovs-dev] [PATCH v7 2/5] Measure timing of ovn-controller flow creation.

[Mark Michelson]

This modifies ovn-controller to measure the amount of time it takes to
generate flows.

The statistics can be queried using:

ovs-appctl -t ovn-controller stopwatch/show
ovn-controller-flow-generation

The statistics can be reset using:

ovs-appctl -t ovn-controller stopwatch/reset
ovn-controller-flow-generation

Signed-off-by: Mark Michelson 
---
 ovn/controller/ovn-controller.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index cfd529459..fdefd3d55 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -57,6 +57,9 @@
 #include "stream.h"
 #include "unixctl.h"
 #include "util.h"
+#include "timeval.h"
+#include "timer.h"
+#include "stopwatch.h"
 
 VLOG_DEFINE_THIS_MODULE(main);
 
@@ -67,6 +70,8 @@ static unixctl_cb_func inject_pkt;
 #define DEFAULT_BRIDGE_NAME "br-int"
 #define DEFAULT_PROBE_INTERVAL_MSEC 5000
 
+#define CONTROLLER_LOOP_STOPWATCH_NAME "ovn-controller-flow-generation"
+
 static void update_probe_interval(struct controller_ctx *,
   const char *ovnsb_remote);
 static void parse_options(int argc, char *argv[]);
@@ -640,6 +645,7 @@ main(int argc, char *argv[])
 unixctl_command_register("inject-pkt", "MICROFLOW", 1, 1, inject_pkt,
  &pending_pkt);
 
+stopwatch_create(CONTROLLER_LOOP_STOPWATCH_NAME, SW_MS);
 /* Main loop. */
 exiting = false;
 while (!exiting) {
@@ -709,6 +715,9 @@ main(int argc, char *argv[])
 ct_zone_bitmap, &pending_ct_zones);
 if (ctx.ovs_idl_txn) {
 if (ofctrl_can_put()) {
+stopwatch_start(CONTROLLER_LOOP_STOPWATCH_NAME,
+time_msec());
+
 commit_ct_zones(br_int, &pending_ct_zones);
 
 struct hmap flow_table = HMAP_INITIALIZER(&flow_table);
@@ -726,6 +735,9 @@ main(int argc, char *argv[])
  &flow_table, &local_datapaths, &local_lports,
  &chassis_index, &active_tunnels);
 
+stopwatch_stop(CONTROLLER_LOOP_STOPWATCH_NAME,
+   time_msec());
+
 ofctrl_put(&flow_table, &pending_ct_zones,
get_nb_cfg(ctx.ovnsb_idl));
 
@@ -793,6 +805,7 @@ main(int argc, char *argv[])
 ofctrl_wait();
 pinctrl_wait(&ctx);
 }
+
 ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop);
 
 if (ovsdb_idl_loop_commit_and_wait(&ovs_idl_loop) == 1) {
-- 
2.14.3

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

Re: [ovs-dev] [PATCH] rhel: don't drop capabilities when running as root

[Aaron Conole]

Russell Bryant  writes:

> On Tue, Mar 27, 2018 at 9:26 AM, Aaron Conole  wrote:
>> Aaron Conole  writes:
>>
>>> Currently, regardless of which user is being set as the running user,
>>> Open vSwitch daemons on RHEL systems drop capabilities.  This means the
>>> very powerful CAP_SYS_ADMIN is dropped, even when the user is 'root'.
>>>
>>> For the majority of use cases this behavior works, as the user can
>>> enable or disable various configurations, regardless of which datapath
>>> functions are desired.  However, when using certain DPDK PMDs, the
>>> enablement and configuration calls require CAP_SYS_ADMIN.
>>>
>>> Instead of retaining CAP_SYS_ADMIN in all cases, which would practically
>>> nullify the uid/gid and privilege drop, we don't pass the --ovs-user
>>> option to the daemons.  This shunts the capability and privilege
>>> dropping code.
>>>
>>> Reported-by: Marcos Felipe Schwarz 
>>> Reported-at:
>>> https://mail.openvswitch.org/pipermail/ovs-discuss/2018-January/045955.html
>>> Fixes: e3e738a3d058 ("redhat: allow dpdk to also run as non-root user")
>>> Signed-off-by: Aaron Conole 
>>> ---
>>
>> Ping?
>
> Applied to master and branch-2.9.
>
> Please continue to CC me on rhel patches like this that have been
> reviewed by someone and you feel are ready to be applied.

Cool, will do.  Thanks Russell!

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

[ovs-dev] using pki other than ovs-pki for SSL in openvswitch

[Manish Regmi]

Hi all,
   We are trying to use SSL using set-controller to communicate
between ovs-ofctl and ovs daemon.
Is it possible to use keys and certs generated by other PKI
infrastructure (say vault). I could not find any info on this. is it
possible? is there a tutorial or other info about this?

Thank you very much for help.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] question about dp_packet lifetime

[Alessandro Rosetti]

Thank you Darrell and Ilya!

yes, I thought about wrapping pthread_spin_ using a generic API.

I'll review my patch with your tips and I'll send it again correctly,
thanks again!

Alessandro

2018-03-28 18:36 GMT+02:00 Darrell Ball :

> I hit send too quick Alessandro; one clarification inline
>
> On Wed, Mar 28, 2018 at 9:13 AM, Darrell Ball  wrote:
>
>> Another aspect (besides what Ilya mentioned) you might want to check is
>> to look at OVS patchwork for your patches,
>> after you submit, and check that they are there, firstly.
>> Also check that they look like other accepted patches overall and for
>> chunks of similar code constructs.
>>
>> https://patchwork.ozlabs.org/project/openvswitch/list/
>>
>> Check that your patches can be applied on top of an updated master branch
>> of OVS.
>>
>> I did a quick pass over the raw diff and noticed that in many cases you
>> are already using lots of OVS apis which good.
>>
>> A few pointers:
>> 1/ Try to use inline functions as much as possible, instead of macros
>> 2/ Think about portability - Don't use direct calls to pthread_ apis for
>> example
>>
>
> I am specifically referring to the locking apis, like pthread_spin_
>
> 3/ Create wrappers for new locks that use generic OVS lock apis
>> 4/ Clearly describe any build dependencies, if any, in the install guide
>> documentation.
>> 5/ Think about portability for parts of the code and look how that is
>> handled in other cases.
>> 6/ I think it would be helpful for you to describe one or more use cases
>> for netmap, for the general user.
>> 7/ Think about testing and see what we can do to automate - we have
>> system tests that run with
>> make check-kmod and make check-system-userspace
>> Existing files are  tests/system-traffic.at and tests/system-ovn.at,
>> which is shared for Linux and userspace datapath
>> 8/ You might want to describe some tests results, including performance
>> numbers in the cover letter.
>>
>> Cheers Darrell
>>
>>
>> On Wed, Mar 28, 2018 at 1:50 AM, Alessandro Rosetti <
>> alessandro.rose...@gmail.com> wrote:
>>
>>> Hi Darrell, Ilya and everyone else,
>>>
>>> I'm contacting you since you were interested.
>>> I've posted the patch that implements netmap in OVS attaching the file
>>> in the mail, did I do it wrong?
>>> https://mail.openvswitch.org/pipermail/ovs-dev/2018-March/345371.html
>>>
>>> I'm posting it inline now,
>>> sorry for the mess!
>>>
>>> Alessandro.
>>>
>>> --
>>>
>>> diff --git a/acinclude.m4 b/acinclude.m4
>>> index d61e37a5e..d9dd9fbd1 100644
>>> --- a/acinclude.m4
>>> +++ b/acinclude.m4
>>> @@ -341,6 +341,36 @@ AC_DEFUN([OVS_CHECK_DPDK], [
>>>AM_CONDITIONAL([DPDK_NETDEV], test "$DPDKLIB_FOUND" = true)
>>>  ])
>>>
>>> +dnl OVS_CHECK_NETMAP
>>> +dnl
>>> +dnl Check netmap
>>> +AC_DEFUN([OVS_CHECK_NETMAP], [
>>> +  AC_ARG_WITH([netmap],
>>> +  [AC_HELP_STRING([--with-netmap], [Enable NETMAP])],
>>> +  [have_netmap=true])
>>> +  AC_MSG_CHECKING([whether netmap datapath is enabled])
>>> +
>>> +  if test "$have_netmap" != true || test "$with_netmap" = no; then
>>> +AC_MSG_RESULT([no])
>>> +  else
>>> +AC_MSG_RESULT([yes])
>>> +NETMAP_FOUND=false
>>> +AC_LINK_IFELSE(
>>> +   [AC_LANG_PROGRAM([#include 
>>> + #include
>>> + #include
>>> + #include], [])],
>>> +[NETMAP_FOUND=true])
>>> +if $NETMAP_FOUND; then
>>> +AC_DEFINE([NETMAP_NETDEV], [1], [NETMAP datapath is enabled.])
>>> +else
>>> +AC_MSG_ERROR([Could not find NETMAP headers])
>>> +fi
>>> +  fi
>>> +
>>> +  AM_CONDITIONAL([NETMAP_NETDEV], test "$NETMAP_FOUND" = true)
>>> +])
>>> +
>>>  dnl OVS_GREP_IFELSE(FILE, REGEX, [IF-MATCH], [IF-NO-MATCH])
>>>  dnl
>>>  dnl Greps FILE for REGEX.  If it matches, runs IF-MATCH, otherwise
>>> IF-NO-MATCH.
>>> @@ -900,7 +930,7 @@ dnl with or without modifications, as long as this
>>> notice is preserved.
>>>
>>>  AC_DEFUN([_OVS_CHECK_CC_OPTION], [dnl
>>>m4_define([ovs_cv_name], [ovs_cv_[]m4_translit([$1], [-= ], [__])])dnl
>>> -  AC_CACHE_CHECK([whether $CC accepts $1], [ovs_cv_name],
>>> +  AC_CACHE_CHECK([whether $CC accepts $1], [ovs_cv_name],
>>>  [ovs_save_CFLAGS="$CFLAGS"
>>>   dnl Include -Werror in the compiler options, because without
>>> -Werror
>>>   dnl clang's GCC-compatible compiler driver does not return a
>>> failure
>>> @@ -951,7 +981,7 @@ dnl OVS_ENABLE_OPTION([OPTION])
>>>  dnl Check whether the given C compiler OPTION is accepted.
>>>  dnl If so, add it to WARNING_FLAGS.
>>>  dnl Example: OVS_ENABLE_OPTION([-Wdeclaration-after-statement])
>>> -AC_DEFUN([OVS_ENABLE_OPTION],
>>> +AC_DEFUN([OVS_ENABLE_OPTION],
>>>[OVS_CHECK_CC_OPTION([$1], [WARNING_FLAGS="$WARNING_FLAGS $1"])
>>> AC_SUBST([WARNING_FLAGS])])
>>>
>>> diff --git a/configure.ac b/configure.ac
>>> index 9940a1a45..24cd4718c 100644
>>> ---

Re: [ovs-dev] [PATCH v2 1/1] userspace: Add IPv6 extension header parsing for tunnels

[Eelco Chaudron]

On 03/09/2018 03:59 PM, William Tu wrote:

On Thu, Feb 8, 2018 at 10:42 AM, Eric Garver  wrote:

On Thu, Feb 08, 2018 at 04:40:38PM +0100, Eelco Chaudron wrote:

While OVS userspace datapath (OVS-DPDK) supports GREv6, it does not
inter-operate with a native Linux ip6gretap tunnel. This is because
the Linux driver uses IPv6 optional headers for the Tunnel
Encapsulation Limit (RFC 2473, section 6.6).

Maybe worth noting that the kernel started adding these headers in
4.12. Specifically, it was introduced by 89a23c8b528b ("ip6_tunnel: Fix
missing tunnel encapsulation limit option")


OVS userspace simply does not parse these IPv6 extension headers
inside netdev_tnl_ip_extract_tnl_md(), as such popping the tunnel
leaves extra bytes resulting in a mangled decapsulated frame.

The change below will parse the IPv6 "next header" chain and return
the offset to the upper layer protocol.

v1->v2
  - Remove netdev_tnl_ip6_get_upperlayer_offset() and reused existing
parse_ipv6_ext_hdrs() function.


I also hit this issue recent. I tested this patch and it works OK.
However, do you think its simpler if we do below:
--- a/lib/netdev-native-tnl.c
+++ b/lib/netdev-native-tnl.c
@@ -146,8 +146,8 @@ netdev_tnl_ip_extract_tnl_md(struct dp_packet
*packet, struct flow_tnl *tnl,
  tnl->ip_tos = ntohl(tc_flow) >> 20;
  tnl->ip_ttl = ip6->ip6_hlim;

-*hlen += IPV6_HEADER_LEN;
+*hlen += packet->l4_ofs - packet->l3_ofs;

Since parse_ipv6_ext_hdrs() already called at packet parsing stage,
the l4_ofs has the extension header's length.
Are you sure this path is taken for all packets? Quickly looking at the 
code I see this path only with connection tracking actions. Or did I 
miss something?



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

diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
index fb5eab033..4f520a0f9 100644
--- a/lib/netdev-native-tnl.c
+++ b/lib/netdev-native-tnl.c
@@ -115,6 +115,10 @@ netdev_tnl_ip_extract_tnl_md(struct dp_packet *packet, 
struct flow_tnl *tnl,
  *hlen += IP_HEADER_LEN;

  } else if (IP_VER(ip->ip_ihl_ver) == 6) {
+const void *ip6_data;
+size_t ip6_size;
+uint8_t nw_proto;
+uint8_t nw_frag;
  ovs_be32 tc_flow = get_16aligned_be32(&ip6->ip6_flow);

  memcpy(tnl->ipv6_src.s6_addr, ip6->ip6_src.be16, sizeof ip6->ip6_src);
@@ -125,6 +129,20 @@ netdev_tnl_ip_extract_tnl_md(struct dp_packet *packet, 
struct flow_tnl *tnl,

  *hlen += IPV6_HEADER_LEN;

+ip6_data = ip6 + 1;
+ip6_size = l3_size - IPV6_HEADER_LEN;
+nw_proto = ip6->ip6_nxt;
+nw_frag = 0;
+
+if (!parse_ipv6_ext_hdrs(&ip6_data, &ip6_size, &nw_proto, &nw_frag) ||
+nw_frag != 0) {
+VLOG_WARN_RL(&err_rl,
+ "ipv6 packet has unsupported extension headers");
+return NULL;
+}
+
+*hlen += l3_size - IPV6_HEADER_LEN - ip6_size;
+
  } else {
  VLOG_WARN_RL(&err_rl, "ipv4 packet has invalid version (%d)",
   IP_VER(ip->ip_ihl_ver));
--
2.14.3


Thanks Eelco. I tested this by applying it on top of a non-upstream gre6
test case I had previously written [0]. If this gets applied I can post
that testcase to the list.

Tested-by: Eric Garver 

[0] 
https://github.com/erig0/ovs/commits/ff2e39278b66b3fe937dba63b32e341098901c50
___
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] question about dp_packet lifetime

[Darrell Ball]

I hit send too quick Alessandro; one clarification inline

On Wed, Mar 28, 2018 at 9:13 AM, Darrell Ball  wrote:

> Another aspect (besides what Ilya mentioned) you might want to check is to
> look at OVS patchwork for your patches,
> after you submit, and check that they are there, firstly.
> Also check that they look like other accepted patches overall and for
> chunks of similar code constructs.
>
> https://patchwork.ozlabs.org/project/openvswitch/list/
>
> Check that your patches can be applied on top of an updated master branch
> of OVS.
>
> I did a quick pass over the raw diff and noticed that in many cases you
> are already using lots of OVS apis which good.
>
> A few pointers:
> 1/ Try to use inline functions as much as possible, instead of macros
> 2/ Think about portability - Don't use direct calls to pthread_ apis for
> example
>

I am specifically referring to the locking apis, like pthread_spin_

3/ Create wrappers for new locks that use generic OVS lock apis
> 4/ Clearly describe any build dependencies, if any, in the install guide
> documentation.
> 5/ Think about portability for parts of the code and look how that is
> handled in other cases.
> 6/ I think it would be helpful for you to describe one or more use cases
> for netmap, for the general user.
> 7/ Think about testing and see what we can do to automate - we have system
> tests that run with
> make check-kmod and make check-system-userspace
> Existing files are  tests/system-traffic.at and tests/system-ovn.at,
> which is shared for Linux and userspace datapath
> 8/ You might want to describe some tests results, including performance
> numbers in the cover letter.
>
> Cheers Darrell
>
>
> On Wed, Mar 28, 2018 at 1:50 AM, Alessandro Rosetti <
> alessandro.rose...@gmail.com> wrote:
>
>> Hi Darrell, Ilya and everyone else,
>>
>> I'm contacting you since you were interested.
>> I've posted the patch that implements netmap in OVS attaching the file in
>> the mail, did I do it wrong?
>> https://mail.openvswitch.org/pipermail/ovs-dev/2018-March/345371.html
>>
>> I'm posting it inline now,
>> sorry for the mess!
>>
>> Alessandro.
>>
>> --
>>
>> diff --git a/acinclude.m4 b/acinclude.m4
>> index d61e37a5e..d9dd9fbd1 100644
>> --- a/acinclude.m4
>> +++ b/acinclude.m4
>> @@ -341,6 +341,36 @@ AC_DEFUN([OVS_CHECK_DPDK], [
>>AM_CONDITIONAL([DPDK_NETDEV], test "$DPDKLIB_FOUND" = true)
>>  ])
>>
>> +dnl OVS_CHECK_NETMAP
>> +dnl
>> +dnl Check netmap
>> +AC_DEFUN([OVS_CHECK_NETMAP], [
>> +  AC_ARG_WITH([netmap],
>> +  [AC_HELP_STRING([--with-netmap], [Enable NETMAP])],
>> +  [have_netmap=true])
>> +  AC_MSG_CHECKING([whether netmap datapath is enabled])
>> +
>> +  if test "$have_netmap" != true || test "$with_netmap" = no; then
>> +AC_MSG_RESULT([no])
>> +  else
>> +AC_MSG_RESULT([yes])
>> +NETMAP_FOUND=false
>> +AC_LINK_IFELSE(
>> +   [AC_LANG_PROGRAM([#include 
>> + #include
>> + #include
>> + #include], [])],
>> +[NETMAP_FOUND=true])
>> +if $NETMAP_FOUND; then
>> +AC_DEFINE([NETMAP_NETDEV], [1], [NETMAP datapath is enabled.])
>> +else
>> +AC_MSG_ERROR([Could not find NETMAP headers])
>> +fi
>> +  fi
>> +
>> +  AM_CONDITIONAL([NETMAP_NETDEV], test "$NETMAP_FOUND" = true)
>> +])
>> +
>>  dnl OVS_GREP_IFELSE(FILE, REGEX, [IF-MATCH], [IF-NO-MATCH])
>>  dnl
>>  dnl Greps FILE for REGEX.  If it matches, runs IF-MATCH, otherwise
>> IF-NO-MATCH.
>> @@ -900,7 +930,7 @@ dnl with or without modifications, as long as this
>> notice is preserved.
>>
>>  AC_DEFUN([_OVS_CHECK_CC_OPTION], [dnl
>>m4_define([ovs_cv_name], [ovs_cv_[]m4_translit([$1], [-= ], [__])])dnl
>> -  AC_CACHE_CHECK([whether $CC accepts $1], [ovs_cv_name],
>> +  AC_CACHE_CHECK([whether $CC accepts $1], [ovs_cv_name],
>>  [ovs_save_CFLAGS="$CFLAGS"
>>   dnl Include -Werror in the compiler options, because without -Werror
>>   dnl clang's GCC-compatible compiler driver does not return a failure
>> @@ -951,7 +981,7 @@ dnl OVS_ENABLE_OPTION([OPTION])
>>  dnl Check whether the given C compiler OPTION is accepted.
>>  dnl If so, add it to WARNING_FLAGS.
>>  dnl Example: OVS_ENABLE_OPTION([-Wdeclaration-after-statement])
>> -AC_DEFUN([OVS_ENABLE_OPTION],
>> +AC_DEFUN([OVS_ENABLE_OPTION],
>>[OVS_CHECK_CC_OPTION([$1], [WARNING_FLAGS="$WARNING_FLAGS $1"])
>> AC_SUBST([WARNING_FLAGS])])
>>
>> diff --git a/configure.ac b/configure.ac
>> index 9940a1a45..24cd4718c 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -180,6 +180,7 @@ AC_SUBST(KARCH)
>>  OVS_CHECK_LINUX
>>  OVS_CHECK_LINUX_TC
>>  OVS_CHECK_DPDK
>> +OVS_CHECK_NETMAP
>>  OVS_CHECK_PRAGMA_MESSAGE
>>  AC_SUBST([OVS_CFLAGS])
>>  AC_SUBST([OVS_LDFLAGS])
>> diff --git a/lib/automake.mk b/lib/automake.mk
>> index 5c26e0f33..4ccd9e22a 100644
>> --- a/lib/automake.mk
>> +++ b/lib/aut

Re: [ovs-dev] [PATCH v6] Configurable Link State Change (LSC) detection mode

[Eelco Chaudron]

On 03/27/2018 04:07 PM, Stokes, Ian wrote:

On 27.03.2018 13:19, Stokes, Ian wrote:

It is possible to change LSC detection mode to polling or interrupt
mode for DPDK interfaces. The default is polling mode. To set
interrupt mode, option dpdk-lsc-interrupt has to be set to true.

In polling mode more processor time is needed, since the OVS
repeatedly reads the link state with a short period. It can lead to
packet loss for certain systems.

In interrupt mode the hardware itself triggers an interrupt when link
state change happens, so less processing time needs for the OVS.

For detailed description and usage see the dpdk install documentation.

Could you, please, better describe why we need this change?
Because we're not removing the polling thread. OVS will still poll the
link states periodically. This config option has no effect on that side.
Also, link state polling in OVS uses 'rte_eth_link_get_nowait()' function
which will be called in both cases and should not wait for hardware reply
in any implementation.


rte_eth_link_get_nowait() on Intel XL710 could take an excessive time to 
respond. The following patch, 
https://dpdk.org/ml/archives/dev/2018-March/092156.html is taking care 
of it from a DPDK side.


There might be other drivers that also take a long time, hence this 
patch might still be useful in the future.



I believe it was related to a case where bonded mode in active back was causing 
packet drops due to the frequency that the LSC was being polled. Using 
interrupt based approach alleviated the issue. (I'm open to correction on this 
:))

@Robert/Eelco You may be able to provide some more light here and whether the 
patches below in DPDK resolve the issue?

This long delay can be an issue in bonding mode, as the links checks for 
bonding interfaces is holding the RW lock in bond_run(). This same lock 
is taken in the PMD thread when calling the bond_check_admissibility() 
for upcall traffic.

There was recent bug fix for intel NICs that fixes waiting of an admin
queue on link state requests despite of 'no_wait' flag:
 http://dpdk.org/ml/archives/dev/2018-March/092156.html
Will this fix your target case?

So, the difference of execution time of 'rte_eth_link_get_nowait()'
with enabled and disabled interrupts should be not so significant.
Do you have performance measurements? Measurement with above fix applied?
I do not have delay numbers but I know that we were no longer seeing 
dropped traffic compared to other NICs under the same load with upcall 
traffic present.

Thanks for working on this Robert.

I've completed some testing including the case where LSC is not

supported, in which case the port will remain in a down state and fail
rx/tx traffic. This behavior conforms to the netdev_reconfigure
expectations in the fail case so that's ok.

I'm not sure if this is acceptable. For example, we're not failing
reconfiguration in case of issues with number of queues. We're trying
different numbers until we have working configuration.
Maybe we need the same fall-back mechanism in case of not supported LSC
interrupts? (MTU setup errors are really uncommon unlike LSC interrupts'
support in PMDs).

Thanks for raising this Ilya.

I thought of this as well. I'd like to see a fall back to the PMD but didn’t 
see how it could be done in a clean way.

Unfortunately rte_eth_dev_configure() returns -EINVAL when lsc mode is 
requested but not supported.

It doesn't give us a clue if the error is related to lsc mode as it could also 
relate to a number of other configure issues such as nb_rxq/nb_txq/portid etc.

It would be better if we could query the device via ethdev api to see if it 
supports lsc interrupt mode but that’s not available currently.

Maybe a DPDK patch before we continue?

The only way I have seen lsc support queries in DPDK is by querying the device 
data itself which doesn't look great, this was discussed in an earlier thread I 
think for this patch.

The alternative could be to introduce a generic retry for 
rte_eth_dev_configure() when configure fails but with lsc configured to PMD 
instead but I'd prefer to see an indication that the lsc mode was the cause.

If we have a cleaner way to fall back to PMD that’s ok by me but I think we 
have to allow for a possible failure in during a reconfigure and follow the 
prescribed behavior as per netdev_reconfigure() which this code currently does.


I'm a bit late to the thread but I have a few other comments below.

I'd like to get this patch in the next pull request if possible so I'd

appreciate if others can give any comments on the patch also.

Thanks
Ian


Signed-off-by: Robert Mulik 
---
v5 -> v6:
- DPDK install documentation updated.
- Status of lsc_interrupt_mode of DPDK interfaces can be read by

command

   ovs-appctl dpif/show.
- It was suggested to check if the HW supports interrupt mode, but it
is not
   possible to do without DPDK code change, so it is skipped from this
patch.
---
  Documentation/intro/install/dpdk.rst |

Re: [ovs-dev] question about dp_packet lifetime

[Darrell Ball]

Another aspect (besides what Ilya mentioned) you might want to check is to
look at OVS patchwork for your patches,
after you submit, and check that they are there, firstly.
Also check that they look like other accepted patches overall and for
chunks of similar code constructs.

https://patchwork.ozlabs.org/project/openvswitch/list/

Check that your patches can be applied on top of an updated master branch
of OVS.

I did a quick pass over the raw diff and noticed that in many cases you are
already using lots of OVS apis which good.

A few pointers:
1/ Try to use inline functions as much as possible, instead of macros
2/ Think about portability - Don't use direct calls to pthread_ apis for
example
3/ Create wrappers for new locks that use generic OVS lock apis
4/ Clearly describe any build dependencies, if any, in the install guide
documentation.
5/ Think about portability for parts of the code and look how that is
handled in other cases.
6/ I think it would be helpful for you to describe one or more use cases
for netmap, for the general user.
7/ Think about testing and see what we can do to automate - we have system
tests that run with
make check-kmod and make check-system-userspace
Existing files are  tests/system-traffic.at and tests/system-ovn.at,
which is shared for Linux and userspace datapath
8/ You might want to describe some tests results, including performance
numbers in the cover letter.

Cheers Darrell


On Wed, Mar 28, 2018 at 1:50 AM, Alessandro Rosetti <
alessandro.rose...@gmail.com> wrote:

> Hi Darrell, Ilya and everyone else,
>
> I'm contacting you since you were interested.
> I've posted the patch that implements netmap in OVS attaching the file in
> the mail, did I do it wrong?
> https://mail.openvswitch.org/pipermail/ovs-dev/2018-March/345371.html
>
> I'm posting it inline now,
> sorry for the mess!
>
> Alessandro.
>
> --
>
> diff --git a/acinclude.m4 b/acinclude.m4
> index d61e37a5e..d9dd9fbd1 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -341,6 +341,36 @@ AC_DEFUN([OVS_CHECK_DPDK], [
>AM_CONDITIONAL([DPDK_NETDEV], test "$DPDKLIB_FOUND" = true)
>  ])
>
> +dnl OVS_CHECK_NETMAP
> +dnl
> +dnl Check netmap
> +AC_DEFUN([OVS_CHECK_NETMAP], [
> +  AC_ARG_WITH([netmap],
> +  [AC_HELP_STRING([--with-netmap], [Enable NETMAP])],
> +  [have_netmap=true])
> +  AC_MSG_CHECKING([whether netmap datapath is enabled])
> +
> +  if test "$have_netmap" != true || test "$with_netmap" = no; then
> +AC_MSG_RESULT([no])
> +  else
> +AC_MSG_RESULT([yes])
> +NETMAP_FOUND=false
> +AC_LINK_IFELSE(
> +   [AC_LANG_PROGRAM([#include 
> + #include
> + #include
> + #include], [])],
> +[NETMAP_FOUND=true])
> +if $NETMAP_FOUND; then
> +AC_DEFINE([NETMAP_NETDEV], [1], [NETMAP datapath is enabled.])
> +else
> +AC_MSG_ERROR([Could not find NETMAP headers])
> +fi
> +  fi
> +
> +  AM_CONDITIONAL([NETMAP_NETDEV], test "$NETMAP_FOUND" = true)
> +])
> +
>  dnl OVS_GREP_IFELSE(FILE, REGEX, [IF-MATCH], [IF-NO-MATCH])
>  dnl
>  dnl Greps FILE for REGEX.  If it matches, runs IF-MATCH, otherwise
> IF-NO-MATCH.
> @@ -900,7 +930,7 @@ dnl with or without modifications, as long as this
> notice is preserved.
>
>  AC_DEFUN([_OVS_CHECK_CC_OPTION], [dnl
>m4_define([ovs_cv_name], [ovs_cv_[]m4_translit([$1], [-= ], [__])])dnl
> -  AC_CACHE_CHECK([whether $CC accepts $1], [ovs_cv_name],
> +  AC_CACHE_CHECK([whether $CC accepts $1], [ovs_cv_name],
>  [ovs_save_CFLAGS="$CFLAGS"
>   dnl Include -Werror in the compiler options, because without -Werror
>   dnl clang's GCC-compatible compiler driver does not return a failure
> @@ -951,7 +981,7 @@ dnl OVS_ENABLE_OPTION([OPTION])
>  dnl Check whether the given C compiler OPTION is accepted.
>  dnl If so, add it to WARNING_FLAGS.
>  dnl Example: OVS_ENABLE_OPTION([-Wdeclaration-after-statement])
> -AC_DEFUN([OVS_ENABLE_OPTION],
> +AC_DEFUN([OVS_ENABLE_OPTION],
>[OVS_CHECK_CC_OPTION([$1], [WARNING_FLAGS="$WARNING_FLAGS $1"])
> AC_SUBST([WARNING_FLAGS])])
>
> diff --git a/configure.ac b/configure.ac
> index 9940a1a45..24cd4718c 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -180,6 +180,7 @@ AC_SUBST(KARCH)
>  OVS_CHECK_LINUX
>  OVS_CHECK_LINUX_TC
>  OVS_CHECK_DPDK
> +OVS_CHECK_NETMAP
>  OVS_CHECK_PRAGMA_MESSAGE
>  AC_SUBST([OVS_CFLAGS])
>  AC_SUBST([OVS_LDFLAGS])
> diff --git a/lib/automake.mk b/lib/automake.mk
> index 5c26e0f33..4ccd9e22a 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -134,12 +134,14 @@ lib_libopenvswitch_la_SOURCES = \
>   lib/namemap.c \
>   lib/netdev-dpdk.h \
>   lib/netdev-dummy.c \
> + lib/netdev-netmap.h \
>   lib/netdev-provider.h \
>   lib/netdev-vport.c \
>   lib/netdev-vport.h \
>   lib/netdev-vport-private.h \
>   lib/netdev.c \
>   lib/netdev.h \
> + lib/netmap.h \
>   lib/netflow

Re: [ovs-dev] Clustered DB commits causes sandbox errors

[Mark Michelson]

Thanks for figuring this out! I tested with this patch and all works as 
expected in the sandbox.


On 03/27/2018 05:34 PM, aginwala wrote:

Hi Mark:


The thing is clustering db uses new sockets nb1.ovsdb and sb1.ovsdb. 
However, northd was still trying to use old ovnsb_db.sock and 
ovnnb_db.sock .


I was able to fix the issue as per below patch

diff --git a/tutorial/ovs-sandbox b/tutorial/ovs-sandbox
index babc032..c3e9f12 100755
--- a/tutorial/ovs-sandbox
+++ b/tutorial/ovs-sandbox
@@ -510,8 +510,8 @@ if $ovn; then
      fi
      rungdb $gdb_ovn_northd $gdb_ovn_northd_ex ovn-northd --detach \
          --no-chdir --pidfile -vconsole:off --log-file \
-        --ovnsb-db=unix:"$sandbox"/ovnsb_db.sock \
-        --ovnnb-db=unix:"$sandbox"/ovnnb_db.sock
+        --ovnsb-db=unix:"$sandbox"/sb1.ovsdb \
+        --ovnnb-db=unix:"$sandbox"/nb1.ovsdb

test:

#/ovs/tutorial/sandbox# ovn-nbctl --wait=hv sync
#/ovs/tutorial/sandbox#
#/ovs/tutorial/sandbox#ovn-nbctl --wait=hv ls-add ls1
#/ovs/tutorial/sandbox# ovn-nbctl --wait=hv ls-del ls1
#/ovs/tutorial/sandbox

Will submit the formal patch and add --wait=hv in ovn-setup.sh so that 
its easier to trace this condition too to ensure northd is always 
working fine.



Let me know if someone else has any comments.


Regards,


On Tue, Mar 27, 2018 at 10:34 AM, aginwala > wrote:


Hi Mark:

I did reset to HEAD~6 and the sandbox still crashes . So took
commit: cb8cbbbe97b56401c399fa261b9670eb1698bf14 that Han recently
used to rebase his patches and it works fine. So the diff is
somewhere from this commit to the master.


Also, I noticed one thing that if we are using ssl by default for
sandbox, it's better to configure northd to use ssl too by generating :
e.g. I restarted northd as below in the current master sandbox and
it worked fine:
ovn-northd --detach --no-chdir --pidfile -vconsole:off --log-file
--ovnsb-db=ssl:127.0.0.1:6642 
--ovnnb-db=ssl:127.0.0.1:6641  -p
/root/ovs/tutorial/sandbox/chassis-1-privkey.pem -c
/root/ovs/tutorial/sandbox/chassis-1-cert.pem -C
/root/ovs/tutorial/sandbox/pki/switchca/cacert.pem

Will try to see if I can get the fix real quick.  Ofcourse the diff
is super big! :)

On Mon, Mar 26, 2018 at 2:42 PM, Mark Michelson mailto:mmich...@redhat.com>> wrote:

Hi,

I'm currently on the master branch of OVS, commit "1b1d2e6da
ovsdb: Introduce experimental support for clustered databases."
I started the OVS sandbox using `make sandbox
SANDBOXFLAGS="--ovn"` . I tried to run some tests to add some
logical switch ports to OVN. Running `ovn-nbctl --wait=hv
lsp-add ls0 lsp0` blocks forever. I found that ovn-northd.log
was peppered with lines like the following:


2018-03-26T21:21:06.509Z|00018|reconnect|INFO|unix:/home/putnopvut/ovs/tutorial/sandbox/ovnnb_db.sock:
connecting...

2018-03-26T21:21:06.509Z|00019|reconnect|INFO|unix:/home/putnopvut/ovs/tutorial/sandbox/ovnnb_db.sock:
connection attempt failed (No such file or directory)

2018-03-26T21:21:06.509Z|00020|reconnect|INFO|unix:/home/putnopvut/ovs/tutorial/sandbox/ovnnb_db.sock:
continuing to reconnect in the background but suppressing
further logging

2018-03-26T21:21:06.509Z|00021|reconnect|INFO|unix:/home/putnopvut/ovs/tutorial/sandbox/ovnsb_db.sock:
connecting...

2018-03-26T21:21:06.509Z|00022|reconnect|INFO|unix:/home/putnopvut/ovs/tutorial/sandbox/ovnsb_db.sock:
connection attempt failed (No such file or directory)

2018-03-26T21:21:06.509Z|00023|reconnect|INFO|unix:/home/putnopvut/ovs/tutorial/sandbox/ovnsb_db.sock:
continuing to reconnect in the background but suppressing
further logging

And ovn-controller.log has lines like:


2018-03-26T21:21:00.202Z|00021|rconn|INFO|unix:/home/putnopvut/ovs/tutorial/sandbox/br-int.mgmt:
connected
2018-03-26T21:21:00.203Z|00022|ovsdb_idl|WARN|transaction error:
{"details":"RBAC rules for client \"chassis-1\" role
\"ovn-controller\" prohibit row insertion into table
\"Encap\".","error":"permission error"}

I attempted to bisect to see what commit introduced the problem,
but I ran into problems here, too. If I revert to HEAD~6
(077f03028 jsonrpc-server: Separate changing read_only status
from reconnecting.), then the ovs-sandbox works as expected. If
I revert to HEAD~5, HEAD~4, HEAD~3, HEAD~2, or HEAD~, I hit a
compilation error:

In file included from lib/ovsdb-idl.c:45:0:
lib/ovsdb-idl.c: In function ‘ovsdb_idl_send_monitor_request’:
lib/ovsdb-idl.c:1638:34: error: ‘struct ovsdb_idl’ has no member
named ‘class_’
                                idl->class_->database,
column->name);
   

Re: [ovs-dev] [PATCH] lib/netdev-tc-offloads: Fix frag first/later translation

[Simon Horman]

On Sun, Mar 25, 2018 at 12:53:25PM +0300, Roi Dayan wrote:
> 
> 
> On 25/03/2018 12:11, Roi Dayan wrote:
> > Fragment mask (any and later) always exists so we need to test
> > for FLOW_NW_FRAG_LATER only if the state is FLOW_NW_FRAG_ANY.
> > Before this fix we could pass frag no and first at the same time to TC
> > which is also not tested there for bad frag state.
> > This fix make sure we only pass frag first/later if is frag.
> > 
> > Fixes: 83e866067ea6 ("netdev-tc-offloads: Add support for IP fragmentation")
> > Signed-off-by: Roi Dayan 
> > Reviewed-by: Paul Blakey 
> > ---
> >   lib/netdev-tc-offloads.c | 19 +--
> >   1 file changed, 13 insertions(+), 6 deletions(-)
> > 
> > diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
> > index f22415ee11bd..6db76801fd0e 100644
> > --- a/lib/netdev-tc-offloads.c
> > +++ b/lib/netdev-tc-offloads.c
> > @@ -948,14 +948,21 @@ netdev_tc_flow_put(struct netdev *netdev, struct 
> > match *match,
> >   flower.key.ip_ttl = key->nw_ttl;
> >   flower.mask.ip_ttl = mask->nw_ttl;
> > -if (mask->nw_frag) {
> > -if (key->nw_frag & FLOW_NW_FRAG_ANY)
> > +if (mask->nw_frag & FLOW_NW_FRAG_ANY) {
> > +flower.mask.flags |= TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT;
> > +
> > +if (key->nw_frag & FLOW_NW_FRAG_ANY) {
> >   flower.key.flags |= TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT;
> > -if (!(key->nw_frag & FLOW_NW_FRAG_LATER))
> > -flower.key.flags |= TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST;
> > -flower.mask.flags |= TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT;
> > -flower.mask.flags |= TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST;
> > +if (mask->nw_frag & FLOW_NW_FRAG_LATER) {
> > +flower.mask.flags |= 
> > TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST;
> > +
> > +if (!(key->nw_frag & FLOW_NW_FRAG_LATER)) {
> > +flower.key.flags |= 
> > TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST;
> > +}
> > +}
> > +}
> > +
> 
> Hi Simon,
> 
> This fix OVS to send TC nofrag without frag first/later.
> Though for frag it will set TC frag+fragfirst flags.
> This is working fine though from your patch to iproute user-space
> you showed an example of using only firstfrag flag without the need
> for frag flag.
> I'm not sure if we need to minimize it from OVS as well or just let
> this be frag+firstfrag.
> let me know what you think.

Hi Roi,

thanks for reporting this. I'd like a little more time to investigate this
and follow-up properly.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] Adding cli for displaying LACP counters

[Nitin Katiyar]

Currently OVS does not provide any command to display stats for LACP
without which it is difficult to debug LACP issues. Here we propose
to display various statistics about LACP PDUs and slave state change.

Sample output:

ovs_lacp # ovs-appctl lacp/stats-show
 bond-prv statistics 

slave: dpdk0:
RX PDUs: 128
RX Bad PDUs: 0
TX PDUs: 5
Link Expired: 2
Link Defaulted: 1
Carrier Status Changed: 0

Signed-off-by: Nitin Katiyar 
---
 lib/lacp.c | 79 +++---
 1 file changed, 76 insertions(+), 3 deletions(-)

diff --git a/lib/lacp.c b/lib/lacp.c
index fdefeda..8353746 100644
--- a/lib/lacp.c
+++ b/lib/lacp.c
@@ -127,9 +127,12 @@ struct slave {
 struct timer tx;  /* Next message transmission timer. */
 struct timer rx;  /* Expected message receive timer. */
 
-uint32_t count_rx_pdus;   /* dot3adAggPortStatsLACPDUsRx */
-uint32_t count_rx_pdus_bad;   /* dot3adAggPortStatsIllegalRx */
-uint32_t count_tx_pdus;   /* dot3adAggPortStatsLACPDUsTx */
+uint32_t count_rx_pdus; /* dot3adAggPortStatsLACPDUsRx */
+uint32_t count_rx_pdus_bad; /* dot3adAggPortStatsIllegalRx */
+uint32_t count_tx_pdus; /* dot3adAggPortStatsLACPDUsTx */
+uint32_t count_link_expired;/* Num of times link expired */
+uint32_t count_link_defaulted;  /* Num of times link defaulted */
+uint32_t count_carrier_changed; /* Num of times link status changed */
 };
 
 static struct ovs_mutex mutex;
@@ -153,6 +156,7 @@ static bool info_tx_equal(struct lacp_info *, struct 
lacp_info *)
 OVS_REQUIRES(mutex);
 
 static unixctl_cb_func lacp_unixctl_show;
+static unixctl_cb_func lacp_unixctl_show_stats;
 
 /* Populates 'pdu' with a LACP PDU comprised of 'actor' and 'partner'. */
 static void
@@ -206,6 +210,8 @@ lacp_init(void)
 {
 unixctl_command_register("lacp/show", "[port]", 0, 1,
  lacp_unixctl_show, NULL);
+unixctl_command_register("lacp/show-stats", "[port]", 0, 1,
+ lacp_unixctl_show_stats, NULL);
 }
 
 static void
@@ -459,6 +465,7 @@ lacp_slave_carrier_changed(const struct lacp *lacp, const 
void *slave_)
 if (slave->status == LACP_CURRENT || slave->lacp->active) {
 slave_set_expired(slave);
 }
+slave->count_carrier_changed++;
 
 out:
 lacp_unlock();
@@ -525,8 +532,10 @@ lacp_run(struct lacp *lacp, lacp_send_pdu *send_pdu) 
OVS_EXCLUDED(mutex)
 
 if (slave->status == LACP_CURRENT) {
 slave_set_expired(slave);
+slave->count_link_expired++;
 } else if (slave->status == LACP_EXPIRED) {
 slave_set_defaulted(slave);
+slave->count_link_defaulted++;
 }
 if (slave->status != old_status) {
 seq_change(connectivity_seq_get());
@@ -994,6 +1003,40 @@ lacp_print_details(struct ds *ds, struct lacp *lacp) 
OVS_REQUIRES(mutex)
 }
 
 static void
+lacp_print_stats(struct ds *ds, struct lacp *lacp) OVS_REQUIRES(mutex)
+{
+struct shash slave_shash = SHASH_INITIALIZER(&slave_shash);
+const struct shash_node **sorted_slaves = NULL;
+
+struct slave *slave;
+int i;
+
+ds_put_format(ds, " %s statistics \n", lacp->name);
+
+HMAP_FOR_EACH (slave, node, &lacp->slaves) {
+shash_add(&slave_shash, slave->name, slave);
+}
+sorted_slaves = shash_sort(&slave_shash);
+
+for (i = 0; i < shash_count(&slave_shash); i++) {
+slave = sorted_slaves[i]->data;
+ds_put_format(ds, "\nslave: %s:\n", slave->name);
+ds_put_format(ds, "\tRX PDUs: %u\n", slave->count_rx_pdus);
+ds_put_format(ds, "\tRX Bad PDUs: %u\n", slave->count_rx_pdus_bad);
+ds_put_format(ds, "\tTX PDUs: %u\n", slave->count_tx_pdus);
+ds_put_format(ds, "\tLink Expired: %u\n",
+  slave->count_link_expired);
+ds_put_format(ds, "\tLink Defaulted: %u\n",
+  slave->count_link_defaulted);
+ds_put_format(ds, "\tCarrier Status Changed: %u\n",
+  slave->count_carrier_changed);
+}
+
+shash_destroy(&slave_shash);
+free(sorted_slaves);
+}
+
+static void
 lacp_unixctl_show(struct unixctl_conn *conn, int argc, const char *argv[],
   void *aux OVS_UNUSED) OVS_EXCLUDED(mutex)
 {
@@ -1021,6 +1064,36 @@ out:
 lacp_unlock();
 }
 
+static void
+lacp_unixctl_show_stats(struct unixctl_conn *conn,
+  int argc,
+  const char *argv[],
+  void *aux OVS_UNUSED) OVS_EXCLUDED(mutex)
+{
+struct ds ds = DS_EMPTY_INITIALIZER;
+struct lacp *lacp;
+
+lacp_lock();
+if (argc > 1) {
+lacp = lacp_find(argv[1]);
+if (!lacp) {
+unixctl_command_reply_error(conn, "no such lacp object");
+goto out;
+}
+lacp_print_stats(&ds, lacp);
+ 

[ovs-dev] [PATCH] ovn-ctl: Support starting clustered OVN dbs

[nusiddiq]

From: Numan Siddique 

This patch adds the options to start clustered OVN db servers.
To support this following options are added - '--db-nb-cluster-local-addr',
'--db-nb-cluster-remote-addr', '--db-sb-cluster-local-addr' and
'--db-sb-cluster-remote-addr'. If only '--db-(nb/sb)-cluster-local-addr' is 
defined
then clustered db is created (using ovsdb-tool create-cluster). If both are 
defined,
then the db is added to the cluster (using ovsdb-tool join-cluster)

Presently this patch doesn't handle the schema update scenario when restarting 
the
clustered ovsdb-servers. This will be handled in a separate patch.

(There are 2 checkpatch warnings 'Line length is >79-characters long' in 
ovn-ctl.8.xml)

Signed-off-by: Numan Siddique 
---
 ovn/utilities/ovn-ctl   | 175 ++--
 ovn/utilities/ovn-ctl.8.xml |  70 ++
 utilities/ovs-lib.in|  71 +-
 3 files changed, 259 insertions(+), 57 deletions(-)

diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
index dc0c26159..ed8f41081 100755
--- a/ovn/utilities/ovn-ctl
+++ b/ovn/utilities/ovn-ctl
@@ -95,80 +95,131 @@ promote_ovnsb() {
 
 start_nb_ovsdb() {
 # Check and eventually start ovsdb-server for Northbound DB
-if ! pidfile_is_running $DB_NB_PID; then
-upgrade_db "$DB_NB_FILE" "$DB_NB_SCHEMA" 1>/dev/null 2>/dev/null
+if pidfile_is_running $DB_SB_PID; then
+return
+fi
 
-set ovsdb-server
+mode="standalone"
+if test ! -z "$DB_NB_CLUSTER_LOCAL_ADDR"; then
+mode="cluster"
+elif test ! -z "$DB_NB_SYNC_FROM_ADDR"; then
+mode="active_passive"
+echo "$DB_NB_SYNC_FROM_PROTO:$DB_NB_SYNC_FROM_ADDR:\
+$DB_NB_SYNC_FROM_PORT" > $ovnnb_active_conf_file
+fi
 
-if test X"$DB_NB_DETACH" != Xno; then
-set "$@" --detach --monitor
+if test X$mode = "Xcluster"; then
+if test -z "$DB_NB_CLUSTER_REMOTE_ADDR"; then
+create_cluster "$DB_NB_FILE" "$DB_NB_SCHEMA" \
+"$DB_NB_CLUSTER_LOCAL_ADDR"
 else
-set exec "$@"
+join_cluster "$DB_NB_FILE" "OVN_Northbound" \
+"$DB_NB_CLUSTER_LOCAL_ADDR" "$DB_NB_CLUSTER_REMOTE_ADDR"
 fi
+else
+upgrade_db "$DB_NB_FILE" "$DB_NB_SCHEMA" 1>/dev/null 2>/dev/null
+fi
 
-set "$@" $OVN_NB_LOG --log-file=$OVN_NB_LOGFILE
-set "$@" --remote=punix:$DB_NB_SOCK --pidfile=$DB_NB_PID
-set "$@" --remote=db:OVN_Northbound,NB_Global,connections
-set "$@" --unixctl=ovnnb_db.ctl
-set "$@" --private-key=db:OVN_Northbound,SSL,private_key
-set "$@" --certificate=db:OVN_Northbound,SSL,certificate
-set "$@" --ca-cert=db:OVN_Northbound,SSL,ca_cert
-set "$@" --ssl-protocols=db:OVN_Northbound,SSL,ssl_protocols
-set "$@" --ssl-ciphers=db:OVN_Northbound,SSL,ssl_ciphers
-
-if test X"$DB_NB_CREATE_INSECURE_REMOTE" = Xyes; then
-set "$@" --remote=ptcp:$DB_NB_PORT:$DB_NB_ADDR
-fi
+set ovsdb-server
 
-if test ! -z "$DB_NB_SYNC_FROM_ADDR"; then
-echo 
"$DB_NB_SYNC_FROM_PROTO:$DB_NB_SYNC_FROM_ADDR:$DB_NB_SYNC_FROM_PORT" > 
$ovnnb_active_conf_file
-fi
+if test X"$DB_NB_DETACH" != Xno; then
+set "$@" --detach --monitor
+else
+set exec "$@"
+fi
 
-if test -e $ovnnb_active_conf_file; then
-set "$@" --sync-from=`cat $ovnnb_active_conf_file`
-fi
+set "$@" $OVN_NB_LOG --log-file=$OVN_NB_LOGFILE
+set "$@" --remote=punix:$DB_NB_SOCK --pidfile=$DB_NB_PID
+set "$@" --unixctl=ovnnb_db.ctl
+
+set "$@" --remote=db:OVN_Northbound,NB_Global,connections
+set "$@" --private-key=db:OVN_Northbound,SSL,private_key
+set "$@" --certificate=db:OVN_Northbound,SSL,certificate
+set "$@" --ca-cert=db:OVN_Northbound,SSL,ca_cert
+set "$@" --ssl-protocols=db:OVN_Northbound,SSL,ssl_protocols
+set "$@" --ssl-ciphers=db:OVN_Northbound,SSL,ssl_ciphers
+
+if test X"$DB_NB_CREATE_INSECURE_REMOTE" = Xyes; then
+set "$@" --remote=ptcp:$DB_NB_PORT:$DB_NB_ADDR
+fi
+
+# Set '--sync-from' if ovnnb_active_conf_file is present and mode
+# is 'active_passive'
+if test -e $ovnnb_active_conf_file && test X$mode = "Xactive_passive"; then
+set "$@" --sync-from=`cat $ovnnb_active_conf_file`
+fi
 
-$@ $DB_NB_FILE
+$@ $DB_NB_FILE
+
+# Initialize the database if it's running standalone, active-passive,
+# or is the first server in a cluster.
+if test -z "$DB_NB_CLUSTER_REMOTE_ADDR"; then
 ovn-nbctl init
 fi
 }
 
 start_sb_ovsdb() {
 # Check and eventually start ovsdb-server for Southbound DB
-if ! pidfile_is_running $DB_SB_PID; then
-upgrade_db "$DB_SB_FILE" "$DB_SB_SCHEMA" 1>/dev/null 2>/dev/null
+if pidfile_is_running $DB_SB_PID; then
+return
+fi
+
+mode="standalone"
 
-set ovsdb-server
+if test ! -z "$DB_SB_CLUSTER_LOCAL_ADDR"; then
+m

Re: [ovs-dev] dev Digest, Vol 103, Issue 179

[Satyavalli Rama]

Hi Ben 




Colud you plese provide  your valuable inputs.


Thanks & Regards
Satya Valli
Tata Consultancy Services
Mailto: satyavalli.r...@tcs.com
Website: http://www.tcs.com

Experience certainty. IT Services
Business Solutions
Consulting




-ovs-dev-boun...@openvswitch.org wrote: -

To: ovs-dev@openvswitch.org
From: ovs-dev-requ...@openvswitch.org
Sent by: ovs-dev-boun...@openvswitch.org
Date: 02/28/2018 08:07PM
Subject: dev Digest, Vol 103, Issue 179


Send dev mailing list submissions to
ovs-dev@openvswitch.org

To subscribe or unsubscribe via the World Wide Web, visit
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
or, via email, send a message with subject or body 'help' to
ovs-dev-requ...@openvswitch.org

You can reach the person managing the list at
ovs-dev-ow...@openvswitch.org

When replying, please edit your Subject line so it is more specific
than "Re: Contents of dev digest..."


Today's Topics:

   1. [PATCH] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics
  Support (SatyaValli)


--

Message: 1
Date: Wed, 28 Feb 2018 20:05:56 +0530
From: SatyaValli 
To: d...@openvswitch.org
Cc: Manasa Cherukupally , Surya
Muttamsetty , Pavani Panthagada
, Satya Valli , Lavanya
Harivelam 
Subject: [ovs-dev] [PATCH] OF1.5/EXT-334 OXS/Extensible Flow Entry
Statistics Support
Message-ID: <1519828556-3535-1-git-send-email-satyavalli.r...@tcs.com>

From: SatyaValli 

This Patch provides implementation Existing flow entry statistics are
redefined as standard OXS(OpenFlow Extensible Statistics) fields for
displaying the arbitrary flow stats.The existing Flow Stats were renamed
as Flow Description.

To support this implementation below messages are newly added

OFPRAW_OFPT15_FLOW_REMOVED,
OFPRAW_OFPST15_FLOW_REQUEST,
OFPRAW_OFPST15_FLOW_DESC_REQUEST,
OFPRAW_OFPST15_AGGREGATE_REQUEST,
OFPRAW_OFPST15_FLOW_REPLY,
OFPRAW_OFPST15_FLOW_DESC_REPLY,
OFPRAW_OFPST15_AGGREGATE_REPLY,

The current commit adds support for the new feature in flow statistics
multipart messages,aggregate multipart messages and OXS support for flow
removal message, individual flow description messages.

"ovs-ofctl dump-flows" & "ovs-ofctl dump-aggregate" now accepts a new option
"--oxs-stats" provided with the arbitrary OXS fields for displaying the
desired flow stats.

Below are Commands to display OXS stats field wise

Flow Statistics Multipart
ovs-ofctl dump-flows -O OpenFlow15  --oxs-stats=idle_time
ovs-ofctl dump-flows -O OpenFlow15  --oxs-stats=packet_count
ovs-ofctl dump-flows -O OpenFlow15  --oxs-stats=byte_count
ovs-ofctl dump-flows -O OpenFlow15  --oxs-stats=duration

Aggregate Flow Statistics Multipart
ovs-ofctl dump-aggregate -O OpenFlow15  --oxs-stats=packet_count
ovs-ofctl dump-aggregate -O OpenFlow15  --oxs-stats=byte_count
ovs-ofctl dump-aggregate -O OpenFlow15  --oxs-stats=flow_count

Signed-off-by: Satya Valli 
Co-authored-by: Lavanya Harivelam 
Signed-off-by: Lavanya Harivelam 
Co-authored-by: Surya Muttamsetty 
Signed-off-by: Surya Muttamsetty 
Co-authored-by: Manasa Cherukupally 
Signed-off-by: Manasa Cherukupally 
Co-authored-by: Pavani Panthagada 
Signed-off-by: Pavani Panthagada 

---
 NEWS|7 +
 include/openflow/openflow-1.5.h |   81 +++
 include/openvswitch/ofp-msgs.h  |   31 +-
 include/openvswitch/ofp-print.h |9 +-
 lib/automake.mk |2 +
 lib/ofp-bundle.c|2 +
 lib/ofp-flow.c  |  198 +++-
 lib/ofp-monitor.c   |   46 +-
 lib/ofp-parse.c |1 +
 lib/ofp-print.c |   52 ++
 lib/ox-stat.c   | 1040 +++
 lib/ox-stat.h   |   60 +++
 lib/rconn.c |2 +
 lib/vconn.c |7 +-
 ofproto/ofproto.c   |3 +-
 tests/ofp-print.at  |   80 +++
 tests/ofproto-dpif.at   |   85 
 tests/ofproto.at|3 +-
 utilities/ovs-ofctl.8.in|   31 +-
 utilities/ovs-ofctl.c   |  123 -
 20 files changed, 1815 insertions(+), 48 deletions(-)
 create mode 100644 lib/ox-stat.c
 create mode 100644 lib/ox-stat.h

diff --git a/NEWS b/NEWS
index 4230189..0704960 100644
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,13 @@ Post-v2.9.0
"table#".  These are not helpful names for the purpose of accepting
and displaying table names, so now tables by default have no names.
- ovs-ofctl:
+ * Existing flow entry statistics are redefined as standard OXS(OpenFlow
+   Extensible Statistics) fields for displaying the arbitrary flow stats.
+ * The existing flow statistics are renamed as Flow Description.
+ * Now "ovs-ofctl dump-flows" and "ovs-ofctl dump-aggregate" accepts a new
+   option

Re: [ovs-dev] question about dp_packet lifetime

[Ilya Maximets]

On 28.03.2018 11:50, Alessandro Rosetti wrote:
> Hi Darrell, Ilya and everyone else,
> 
> I'm contacting you since you were interested.
> I've posted the patch that implements netmap in OVS attaching the file in the 
> mail, did I do it wrong?
> https://mail.openvswitch.org/pipermail/ovs-dev/2018-March/345371.html

It looks like only cover letter was sent there. If the patch was
sent as attachment it could probably be stripped by the mail-list
engine.

You could use following documentation for contribution process:

http://docs.openvswitch.org/en/latest/internals/contributing/submitting-patches/

Linux kernel docs contains a lot of useful information about common
opensource practices:
https://www.kernel.org/doc/html/v4.13/process/submitting-patches.html

IMHO, Preferred way to send a patch is to use 'git send-email'.

'man git-send-email' is a good place to start. Also, many examples
and tutorials could be found in web.

Best regards, Ilya Maximets.

> 
> I'm posting it inline now,
> sorry for the mess!
> 
> Alessandro.
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] v2, python-windows: Fix unicode python tests on Windows

[Alin Gabriel Serdean]

This patch changes the default filesystem encodings to the values used
before python3.6 to ensure compatibility with older versions.

Signed-off-by: Alin Gabriel Serdean 
Co-authored-by: Alin Balutoiu 
---
v2: update commit message
---
 tests/atlocal.in | 9 +
 1 file changed, 9 insertions(+)

diff --git a/tests/atlocal.in b/tests/atlocal.in
index 55f9333ee..0df504be7 100644
--- a/tests/atlocal.in
+++ b/tests/atlocal.in
@@ -106,6 +106,15 @@ FreeBSD|NetBSD)
 ;;
 esac
 
+if test x"$PYTHON3" != x && test "$IS_WIN32" = yes; then
+# enables legacy windows unicode printing needed for Python3 compatibility
+# with the Python2 tests
+PYTHONLEGACYWINDOWSFSENCODING=true
+export PYTHONLEGACYWINDOWSFSENCODING
+PYTHONLEGACYWINDOWSSTDIO=true
+export PYTHONLEGACYWINDOWSSTDIO
+fi
+
 # Check whether to run IPv6 tests.
 if $PYTHON -c '
 import socket
-- 
2.16.1.windows.1

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

Re: [ovs-dev] question about dp_packet lifetime

[Alessandro Rosetti]

Hi Darrell, Ilya and everyone else,

I'm contacting you since you were interested.
I've posted the patch that implements netmap in OVS attaching the file in
the mail, did I do it wrong?
https://mail.openvswitch.org/pipermail/ovs-dev/2018-March/345371.html

I'm posting it inline now,
sorry for the mess!

Alessandro.

--

diff --git a/acinclude.m4 b/acinclude.m4
index d61e37a5e..d9dd9fbd1 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -341,6 +341,36 @@ AC_DEFUN([OVS_CHECK_DPDK], [
   AM_CONDITIONAL([DPDK_NETDEV], test "$DPDKLIB_FOUND" = true)
 ])

+dnl OVS_CHECK_NETMAP
+dnl
+dnl Check netmap
+AC_DEFUN([OVS_CHECK_NETMAP], [
+  AC_ARG_WITH([netmap],
+  [AC_HELP_STRING([--with-netmap], [Enable NETMAP])],
+  [have_netmap=true])
+  AC_MSG_CHECKING([whether netmap datapath is enabled])
+
+  if test "$have_netmap" != true || test "$with_netmap" = no; then
+AC_MSG_RESULT([no])
+  else
+AC_MSG_RESULT([yes])
+NETMAP_FOUND=false
+AC_LINK_IFELSE(
+   [AC_LANG_PROGRAM([#include 
+ #include
+ #include
+ #include], [])],
+[NETMAP_FOUND=true])
+if $NETMAP_FOUND; then
+AC_DEFINE([NETMAP_NETDEV], [1], [NETMAP datapath is enabled.])
+else
+AC_MSG_ERROR([Could not find NETMAP headers])
+fi
+  fi
+
+  AM_CONDITIONAL([NETMAP_NETDEV], test "$NETMAP_FOUND" = true)
+])
+
 dnl OVS_GREP_IFELSE(FILE, REGEX, [IF-MATCH], [IF-NO-MATCH])
 dnl
 dnl Greps FILE for REGEX.  If it matches, runs IF-MATCH, otherwise
IF-NO-MATCH.
@@ -900,7 +930,7 @@ dnl with or without modifications, as long as this
notice is preserved.

 AC_DEFUN([_OVS_CHECK_CC_OPTION], [dnl
   m4_define([ovs_cv_name], [ovs_cv_[]m4_translit([$1], [-= ], [__])])dnl
-  AC_CACHE_CHECK([whether $CC accepts $1], [ovs_cv_name],
+  AC_CACHE_CHECK([whether $CC accepts $1], [ovs_cv_name],
 [ovs_save_CFLAGS="$CFLAGS"
  dnl Include -Werror in the compiler options, because without -Werror
  dnl clang's GCC-compatible compiler driver does not return a failure
@@ -951,7 +981,7 @@ dnl OVS_ENABLE_OPTION([OPTION])
 dnl Check whether the given C compiler OPTION is accepted.
 dnl If so, add it to WARNING_FLAGS.
 dnl Example: OVS_ENABLE_OPTION([-Wdeclaration-after-statement])
-AC_DEFUN([OVS_ENABLE_OPTION],
+AC_DEFUN([OVS_ENABLE_OPTION],
   [OVS_CHECK_CC_OPTION([$1], [WARNING_FLAGS="$WARNING_FLAGS $1"])
AC_SUBST([WARNING_FLAGS])])

diff --git a/configure.ac b/configure.ac
index 9940a1a45..24cd4718c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -180,6 +180,7 @@ AC_SUBST(KARCH)
 OVS_CHECK_LINUX
 OVS_CHECK_LINUX_TC
 OVS_CHECK_DPDK
+OVS_CHECK_NETMAP
 OVS_CHECK_PRAGMA_MESSAGE
 AC_SUBST([OVS_CFLAGS])
 AC_SUBST([OVS_LDFLAGS])
diff --git a/lib/automake.mk b/lib/automake.mk
index 5c26e0f33..4ccd9e22a 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -134,12 +134,14 @@ lib_libopenvswitch_la_SOURCES = \
  lib/namemap.c \
  lib/netdev-dpdk.h \
  lib/netdev-dummy.c \
+ lib/netdev-netmap.h \
  lib/netdev-provider.h \
  lib/netdev-vport.c \
  lib/netdev-vport.h \
  lib/netdev-vport-private.h \
  lib/netdev.c \
  lib/netdev.h \
+ lib/netmap.h \
  lib/netflow.h \
  lib/netlink.c \
  lib/netlink.h \
@@ -403,6 +405,15 @@ lib_libopenvswitch_la_SOURCES += \
  lib/dpdk-stub.c
 endif

+if NETMAP_NETDEV
+lib_libopenvswitch_la_SOURCES += \
+ lib/netmap.c \
+ lib/netdev-netmap.c
+else
+lib_libopenvswitch_la_SOURCES += \
+ lib/netmap-stub.c
+endif
+
 if WIN32
 lib_libopenvswitch_la_SOURCES += \
  lib/dpif-netlink.c \
diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index 443c22504..e917e6d6a 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -92,6 +92,7 @@ dp_packet_use_const(struct dp_packet *b, const void
*data, size_t size)
 dp_packet_set_size(b, size);
 }

+
 /* Initializes 'b' as an empty dp_packet that contains the 'allocated'
bytes.
  * DPDK allocated dp_packet and *data is allocated from one continous
memory
  * region as part of memory pool, so in memory data start right after
@@ -105,6 +106,19 @@ dp_packet_init_dpdk(struct dp_packet *b, size_t
allocated)
 b->source = DPBUF_DPDK;
 }

+/* Initializes 'b' as a dp_packet whose data points to a netmap buffer of
size
+ * 'size' bytes. */
+#ifdef NETMAP_NETDEV
+void
+dp_packet_init_netmap(struct dp_packet *b, void *data, size_t size)
+{
+b->source = DPBUF_NETMAP;
+dp_packet_set_base(b, data);
+dp_packet_set_data(b, data);
+dp_packet_set_size(b, size);
+}
+#endif
+
 /* Initializes 'b' as an empty dp_packet with an initial capacity of 'size'
  * bytes. */
 void
@@ -125,6 +139,11 @@ dp_packet_uninit(struct dp_packet *b)
 /* If this dp_packet was allocated by DPDK it must have been
  * created as a dp_packet */
 free_dpdk_buf((struct dp_packet*) b);
+#endif
+} else if (b->source == DPBUF_NETMAP) {
+#ifdef NETMAP_NETDEV
+/* If this dp_packet was