Re: [ovs-dev] [PATCH net-next v14] openvswitch: enable NSH support

[Yang, Yi]

On Wed, Nov 01, 2017 at 04:08:22AM +0800, Jiri Benc wrote:
> On Mon, 30 Oct 2017 09:29:34 +0800, Yi Yang wrote:
> > +static int set_nsh(struct sk_buff *skb, struct sw_flow_key *flow_key,
> > +  const struct nlattr *a)
> > +{
> > +   struct nshhdr *nh;
> > +   size_t length;
> > +   int err;
> > +   u8 flags;
> > +   u8 ttl;
> > +   int i;
> > +
> > +   struct ovs_key_nsh key;
> > +   struct ovs_key_nsh mask;
> > +
> > +   err = nsh_key_from_nlattr(a, , );
> > +   if (err)
> > +   return err;
> > +
> > +   /* Make sure the NSH base header is there */
> > +   if (!pskb_may_pull(skb, NSH_BASE_HDR_LEN))
> 
> This should be skb_network_offset(skb) + NSH_BASE_HDR_LEN.
> 

Fixed in v15.

> > +size_t ovs_nsh_key_attr_size(void)
> > +{
> > +   /* Whenever adding new OVS_NSH_KEY_ FIELDS, we should consider
> > +* updating this function.
> > +*/
> > +   return  nla_total_size(NSH_BASE_HDR_LEN) /* OVS_NSH_KEY_ATTR_BASE */
> > +   /* OVS_NSH_KEY_ATTR_MD1 and OVS_NSH_KEY_ATTR_MD2 are
> > +* mutually exclusive, so the bigger one can cover
> > +* the small one.
> > +*
> > +* OVS_NSH_KEY_ATTR_MD2
> > +*/
> 
> A nit, not important but since you'll need to respin anyway: the last
> line in the comment above seems to be a left over from some previous
> version of the comment. This should be enough:
> 
>   /* OVS_NSH_KEY_ATTR_MD1 and OVS_NSH_KEY_ATTR_MD2 are
>* mutually exclusive, so the bigger one can cover
>* the small one.
>*/
> 
> Or maybe I misunderstood what you meant.
> 
Fixed it per the above one.

> > +int nsh_hdr_from_nlattr(const struct nlattr *attr,
> > +   struct nshhdr *nh, size_t size)
> > +{
> > +   struct nlattr *a;
> > +   int rem;
> > +   u8 flags = 0;
> > +   u8 ttl = 0;
> > +   int mdlen = 0;
> > +
> > +   /* validate_nsh has check this, so we needn't do duplicate check here
> > +*/
> > +   nla_for_each_nested(a, attr, rem) {
> > +   int type = nla_type(a);
> > +
> > +   switch (type) {
> > +   case OVS_NSH_KEY_ATTR_BASE: {
> > +   const struct ovs_nsh_key_base *base = nla_data(a);
> > +
> > +   flags = base->flags;
> > +   ttl = base->ttl;
> > +   nh->np = base->np;
> > +   nh->mdtype = base->mdtype;
> > +   nh->path_hdr = base->path_hdr;
> > +   break;
> > +   }
> > +   case OVS_NSH_KEY_ATTR_MD1:
> > +   mdlen = nla_len(a);
> > +   memcpy(>md1, nla_data(a), mdlen);
> 
> The check for 'size' disappeared from here somehow.
> 
> > +   break;
> > +
> > +   case OVS_NSH_KEY_ATTR_MD2:
> > +   mdlen = nla_len(a);
> > +   memcpy(>md2, nla_data(a), mdlen);
> 
> And here.

validate_nsh checked netlink attributes but can't check size, yes, we
should add size check for mdlen, v15 has had them. Please check v15,
thanks a lot.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH net-next v14] openvswitch: enable NSH support

[Yang, Yi]

On Wed, Nov 01, 2017 at 03:57:41AM +0800, Eric Garver wrote:
> On Mon, Oct 30, 2017 at 09:29:34AM +0800, Yi Yang wrote:
> [...]
> > +int nsh_pop(struct sk_buff *skb)
> > +{
> > +   struct nshhdr *nh;
> > +   size_t length;
> > +   __be16 inner_proto;
> > +
> > +   if (!pskb_may_pull(skb, NSH_BASE_HDR_LEN))
> > +   return -ENOMEM;
> > +   nh = (struct nshhdr *)(skb->data);
> > +   length = nsh_hdr_len(nh);
> > +   if (!pskb_may_pull(skb, length))
> > +   return -ENOMEM;
> > +
> > +   nh = (struct nshhdr *)(skb->data);
> > +   inner_proto = tun_p_to_eth_p(nh->np);
> 
> If you fetch inner_proto before the second pskb_may_pull then there is
> no need to reload the nh pointer as you won't use it later.
> 
> > +   if (!inner_proto)
> > +   return -EAFNOSUPPORT;
> > +
> > +   length = nsh_hdr_len(nh);
> 
> You already have the length from above. No need to get it again.

Good catch, fixed in v15.

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

[ovs-dev] [PATCH net-next v15] openvswitch: enable NSH support

[Yi Yang]

v14->v15
 - Check size in nsh_hdr_from_nlattr
 - Fixed four small issues pointed out By Jiri and Eric

v13->v14
 - Rename skb_push_nsh to nsh_push per Dave's comment
 - Rename skb_pop_nsh to nsh_pop per Dave's comment

v12->v13
 - Fix NSH header length check in set_nsh

v11->v12
 - Fix missing changes old comments pointed out
 - Fix new comments for v11

v10->v11
 - Fix the left three disputable comments for v9
   but not fixed in v10.

v9->v10
 - Change struct ovs_key_nsh to
   struct ovs_nsh_key_base base;
   __be32 context[NSH_MD1_CONTEXT_SIZE];
 - Fix new comments for v9

v8->v9
 - Fix build error reported by daily intel build
   because nsh module isn't selected by openvswitch

v7->v8
 - Rework nested value and mask for OVS_KEY_ATTR_NSH
 - Change pop_nsh to adapt to nsh kernel module
 - Fix many issues per comments from Jiri Benc

v6->v7
 - Remove NSH GSO patches in v6 because Jiri Benc
   reworked it as another patch series and they have
   been merged.
 - Change it to adapt to nsh kernel module added by NSH
   GSO patch series

v5->v6
 - Fix the rest comments for v4.
 - Add NSH GSO support for VxLAN-gpe + NSH and
   Eth + NSH.

v4->v5
 - Fix many comments by Jiri Benc and Eric Garver
   for v4.

v3->v4
 - Add new NSH match field ttl
 - Update NSH header to the latest format
   which will be final format and won't change
   per its author's confirmation.
 - Fix comments for v3.

v2->v3
 - Change OVS_KEY_ATTR_NSH to nested key to handle
   length-fixed attributes and length-variable
   attriubte more flexibly.
 - Remove struct ovs_action_push_nsh completely
 - Add code to handle nested attribute for SET_MASKED
 - Change PUSH_NSH to use the nested OVS_KEY_ATTR_NSH
   to transfer NSH header data.
 - Fix comments and coding style issues by Jiri and Eric

v1->v2
 - Change encap_nsh and decap_nsh to push_nsh and pop_nsh
 - Dynamically allocate struct ovs_action_push_nsh for
   length-variable metadata.

OVS master and 2.8 branch has merged NSH userspace
patch series, this patch is to enable NSH support
in kernel data path in order that OVS can support
NSH in compat mode by porting this.

Signed-off-by: Yi Yang 
---
 include/net/nsh.h|   3 +
 include/uapi/linux/openvswitch.h |  29 
 net/nsh/nsh.c|  59 
 net/openvswitch/Kconfig  |   1 +
 net/openvswitch/actions.c| 119 +++
 net/openvswitch/flow.c   |  51 +++
 net/openvswitch/flow.h   |   7 +
 net/openvswitch/flow_netlink.c   | 315 ++-
 net/openvswitch/flow_netlink.h   |   5 +
 9 files changed, 588 insertions(+), 1 deletion(-)

diff --git a/include/net/nsh.h b/include/net/nsh.h
index a1eaea2..350b1ad 100644
--- a/include/net/nsh.h
+++ b/include/net/nsh.h
@@ -304,4 +304,7 @@ static inline void nsh_set_flags_ttl_len(struct nshhdr 
*nsh, u8 flags,
NSH_FLAGS_MASK | NSH_TTL_MASK | NSH_LEN_MASK);
 }
 
+int nsh_push(struct sk_buff *skb, const struct nshhdr *pushed_nh);
+int nsh_pop(struct sk_buff *skb);
+
 #endif /* __NET_NSH_H */
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index 0cd6f88..ac2623b 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -333,6 +333,7 @@ enum ovs_key_attr {
OVS_KEY_ATTR_CT_LABELS, /* 16-octet connection tracking label */
OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV4,   /* struct ovs_key_ct_tuple_ipv4 */
OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV6,   /* struct ovs_key_ct_tuple_ipv6 */
+   OVS_KEY_ATTR_NSH,   /* Nested set of ovs_nsh_key_* */
 
 #ifdef __KERNEL__
OVS_KEY_ATTR_TUNNEL_INFO,  /* struct ip_tunnel_info */
@@ -492,6 +493,30 @@ struct ovs_key_ct_tuple_ipv6 {
__u8   ipv6_proto;
 };
 
+enum ovs_nsh_key_attr {
+   OVS_NSH_KEY_ATTR_UNSPEC,
+   OVS_NSH_KEY_ATTR_BASE,  /* struct ovs_nsh_key_base. */
+   OVS_NSH_KEY_ATTR_MD1,   /* struct ovs_nsh_key_md1. */
+   OVS_NSH_KEY_ATTR_MD2,   /* variable-length octets for MD type 2. */
+   __OVS_NSH_KEY_ATTR_MAX
+};
+
+#define OVS_NSH_KEY_ATTR_MAX (__OVS_NSH_KEY_ATTR_MAX - 1)
+
+struct ovs_nsh_key_base {
+   __u8 flags;
+   __u8 ttl;
+   __u8 mdtype;
+   __u8 np;
+   __be32 path_hdr;
+};
+
+#define NSH_MD1_CONTEXT_SIZE 4
+
+struct ovs_nsh_key_md1 {
+   __be32 context[NSH_MD1_CONTEXT_SIZE];
+};
+
 /**
  * enum ovs_flow_attr - attributes for %OVS_FLOW_* commands.
  * @OVS_FLOW_ATTR_KEY: Nested %OVS_KEY_ATTR_* attributes specifying the flow
@@ -808,6 +833,8 @@ struct ovs_action_push_eth {
  * @OVS_ACTION_ATTR_POP_ETH: Pop the outermost Ethernet header off the
  * packet.
  * @OVS_ACTION_ATTR_CT_CLEAR: Clear conntrack state from the packet.
+ * @OVS_ACTION_ATTR_PUSH_NSH: push NSH header to the packet.
+ * @OVS_ACTION_ATTR_POP_NSH: pop the outermost NSH header off the packet.
  *
  * Only a single header can be set with a single %OVS_ACTION_ATTR_SET.  Not all
  * fields within a 

Re: [ovs-dev] [PKG-Openstack-devel] Bug#878249: recent OVS upload

[Thomas Goirand]

On 10/27/2017 08:52 AM, Ben Pfaff wrote:
> On Thu, Oct 26, 2017 at 03:45:48PM -0700, Ben Pfaff wrote:
>> I see a number of failed builds here:
>> https://buildd.debian.org/status/package.php?p=openvswitch=experimental
>>
>> Let me analyze them:
>>
>> * mips, powerpc, and ppc64 should be fixed by this commit that is
>>   already on branch-2.8:
>>   https://github.com/openvswitch/ovs/commit/2906ff5e7eb1fb39b497dc05e471
>>
>> * m68k is because of looser alignment rules than on other platforms.  I
>>   don't care much about m68k, and it's not a Debian required platform,
>>   so I don't plan to fix this.
>>
>> * sparc64 failures are due to bus errors.  I would like to investigate,
>>   but I don't know how, because there is only one sparc64 machine listed
>>   at https://db.debian.org/machines.cgi, and that machine appears to be
>>   down (it is not accepting SSH connections at least when I tried just
>>   now).
>>
>> * The ppc64el failure is a hang during the testsuite.  Test 2332, which
>>   appears to be "ovn -- icmp_reply: 1 HVs, 2 LSs, 1 lport/LS, 1 LR",
>>   hung.  I will try to reproduce and fix this.  Even if we do not fix
>>   it, it might not recur in later runs, because it indicates a race
>>   condition in the testsuite.  (This is almost certainly a bug in the
>>   testsuite rather than in OVS itself.)
> 
> There's now a failure on hppa, too, which bears investigation.  I'll try
> to look at that soon.

Ben, could you investigate? I've uploaded to Unstable a version with the
patch above, because it's the last blocker to have the last OpenStack
dependency approved in the NEW queue. Let me know if you think we need
to work more on the package.

Cheers,

Thomas Goirand (zigo)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCHv2 2/2] ovn-sbctl: Fix possible null pointer to qsort.

[William Tu]

Clang reports possible null pointer 'lflows' passed to qsort.
This is due to the checker unable to make sure whether 'lflows'
gets malloc or not in the previous loop.  Fix it by checking the
'n_flows' before calling qsort.

Signed-off-by: William Tu 
---
 ovn/utilities/ovn-sbctl.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c
index c5ec4e6eaf24..6f3743b55632 100644
--- a/ovn/utilities/ovn-sbctl.c
+++ b/ovn/utilities/ovn-sbctl.c
@@ -860,7 +860,10 @@ cmd_lflow_list(struct ctl_context *ctx)
 lflows[n_flows] = lflow;
 n_flows++;
 }
-qsort(lflows, n_flows, sizeof *lflows, lflow_cmp);
+
+if (n_flows) {
+qsort(lflows, n_flows, sizeof *lflows, lflow_cmp);
+}
 
 bool print_uuid = shash_find(>options, "--uuid") != NULL;
 
-- 
2.7.4

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

[ovs-dev] [PATCHv2 1/2] ofproto-dpif-xlate: Fix bad memory free.

[William Tu]

Clang reports possibly bad free of 'ofm' when it comes from the stack
instead of malloc because Clang is not able to verify whether the previous
if condition 'ctx->xin->xcache' still hold the same.  Fix it by
adding additional condition.

Signed-off-by: William Tu 
---
 ofproto/ofproto-dpif-xlate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index d0b45d233e69..667960d70389 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -5131,7 +5131,7 @@ xlate_learn_action(struct xlate_ctx *ctx, const struct 
ofpact_learn *learn)
 }
 }
 
-if (ctx->xin->xcache) {
+if (ofm != __) {
 free(ofm);
 }
 
-- 
2.7.4

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

Re: [ovs-dev] [PATCH] travis: Fix OSX build on travis

[Yi-Hung Wei]

On Mon, Oct 23, 2017 at 12:40 PM, Guru Shetty  wrote:
> On 23 October 2017 at 09:39, William Tu  wrote:
>
>> Run "brew update" before any installs.
>> This yields a clean build:
>> https://travis-ci.org/williamtu/ovs-travis/builds/291616874
>>
>> Signed-off-by: William Tu 
>> Cc: Yi-Hung Wei 
>>
>
> Applied to master, thanks!

Hi Guru,

Can you help to cherry pick this fix to older branchs? We have similar
errors in MAC OSX build on other older branches.

ex: https://travis-ci.org/openvswitch/ovs/jobs/295462097 (branch 2.7)
https://travis-ci.org/openvswitch/ovs/builds/295482886 (branch 2.8)

Thanks,

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

[ovs-dev] [PATCHv2] acinclude: Fix SKB_GSO_UDP check.

[William Tu]

The HAVE_SKB_GSO_UDP checks whether skbuff.h defines SKB_GSO_UDP.
However, it falsely returns yes because grep matches SKB_GSO_UDP_TUNNEL.
Thus, add space character '[:space:]' before and after it.

Fixes: ad283644f0e4 ("acinclude: Check for SKB_GSO_UDP")
Signed-off-by: William Tu 
Cc: Greg Rose 
---
v1->v2
  remove using "grep -w" since -w is not POSIX
---
 acinclude.m4 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 89f88ca8de75..c653a75a356b 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -768,7 +768,7 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   OVS_GREP_IFELSE([$KSRC/include/net/netfilter/nf_conntrack_helper.h],
   [nf_conntrack_helper_put],
   [OVS_DEFINE(HAVE_NF_CONNTRACK_HELPER_PUT)])
-  OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h],[SKB_GSO_UDP],
+  
OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h],:space:]]]SKB_GSO_UDP[[[:space:,
   [OVS_DEFINE([HAVE_SKB_GSO_UDP])])
   OVS_GREP_IFELSE([$KSRC/include/net/dst.h],[DST_NOCACHE],
   [OVS_DEFINE([HAVE_DST_NOCACHE])])
-- 
2.7.4

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

[ovs-dev] [PATCH v2 3/5] dpif-netdev: Add CD statistics

[Yipeng Wang]

This patch adds CD hit and miss statistics to dp_stat_type. PMD stats will
show the total CD hit and miss counts. This patch depends on the first patch.

CC: Darrell Ball 
CC: Jan Scheurich 
Signed-off-by: Yipeng Wang 
Signed-off-by: Antonio Fischetti 
Co-authored-by: Antonio Fischetti 
---
 lib/dpif-netdev.c | 33 -
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 78219ba..5245cb5 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -278,7 +278,7 @@ static void dpcls_remove(struct dpcls *, struct dpcls_rule 
*);
 static bool dpcls_lookup(struct dpcls *cls,
  const struct netdev_flow_key keys[],
  struct dpcls_rule **rules, size_t cnt,
- int *num_lookups_p);
+ int *num_lookups_p, int *cd_hit);
 static inline struct dpcls_subtable *
 dpcls_find_subtable(struct dpcls *cls, const struct netdev_flow_key *mask);
 
@@ -405,6 +405,8 @@ enum dp_stat_type {
 DP_STAT_LOST,   /* Packets not passed up to the client. */
 DP_STAT_LOOKUP_HIT, /* Number of subtable lookups for flow table
hits */
+DP_STAT_CD_HIT, /* Packets that hit CD. */
+DP_STAT_CD_MISS,/* Packets that miss CD. */
 DP_N_STATS
 };
 
@@ -938,6 +940,10 @@ pmd_info_show_stats(struct ds *reply,
   : 0,
   stats[DP_STAT_MISS], stats[DP_STAT_LOST]);
 
+ds_put_format(reply,
+  "\tCD hits:%llu\n\tCD miss:%llu\n",
+  stats[DP_STAT_CD_HIT], stats[DP_STAT_CD_MISS]);
+
 if (total_cycles == 0) {
 return;
 }
@@ -2576,7 +2582,7 @@ find_index_in_sub_ptrs(struct dpcls *cls,
 static struct dp_netdev_flow *
 dp_netdev_pmd_lookup_flow(struct dp_netdev_pmd_thread *pmd,
   const struct netdev_flow_key *key,
-  int *lookup_num_p)
+  int *lookup_num_p, int *cd_hit)
 {
 struct dpcls *cls;
 struct dpcls_rule *rule;
@@ -2585,7 +2591,7 @@ dp_netdev_pmd_lookup_flow(struct dp_netdev_pmd_thread 
*pmd,
 
 cls = dp_netdev_pmd_lookup_dpcls(pmd, in_port);
 if (OVS_LIKELY(cls)) {
-dpcls_lookup(cls, key, , 1, lookup_num_p);
+dpcls_lookup(cls, key, , 1, lookup_num_p, cd_hit);
 netdev_flow = dp_netdev_flow_cast(rule);
 }
 return netdev_flow;
@@ -2932,7 +2938,7 @@ flow_put_on_pmd(struct dp_netdev_pmd_thread *pmd,
 }
 
 ovs_mutex_lock(>flow_mutex);
-netdev_flow = dp_netdev_pmd_lookup_flow(pmd, key, NULL);
+netdev_flow = dp_netdev_pmd_lookup_flow(pmd, key, NULL, NULL);
 if (!netdev_flow) {
 if (put->flags & DPIF_FP_CREATE) {
 if (cmap_count(>flow_table) < MAX_FLOWS) {
@@ -5412,7 +5418,7 @@ handle_packet_upcall(struct dp_netdev_pmd_thread *pmd,
  * to be locking everyone out of making flow installs.  If we
  * move to a per-core classifier, it would be reasonable. */
 ovs_mutex_lock(>flow_mutex);
-netdev_flow = dp_netdev_pmd_lookup_flow(pmd, key, NULL);
+netdev_flow = dp_netdev_pmd_lookup_flow(pmd, key, NULL, NULL);
 if (OVS_LIKELY(!netdev_flow)) {
 netdev_flow = dp_netdev_flow_add(pmd, key, , ,
  add_actions->data,
@@ -5444,6 +5450,7 @@ fast_path_processing(struct dp_netdev_pmd_thread *pmd,
 struct dp_netdev *dp = pmd->dp;
 int miss_cnt = 0, lost_cnt = 0;
 int lookup_cnt = 0, add_lookup_cnt;
+int cd_hit = 0, add_cd_hit;
 bool any_miss;
 size_t i;
 
@@ -5454,7 +5461,7 @@ fast_path_processing(struct dp_netdev_pmd_thread *pmd,
 /* Get the classifier for the in_port */
 cls = dp_netdev_pmd_lookup_dpcls(pmd, in_port);
 if (OVS_LIKELY(cls)) {
-any_miss = !dpcls_lookup(cls, keys, rules, cnt, _cnt);
+any_miss = !dpcls_lookup(cls, keys, rules, cnt, _cnt, _hit);
 } else {
 any_miss = true;
 memset(rules, 0, sizeof(rules));
@@ -5477,9 +5484,10 @@ fast_path_processing(struct dp_netdev_pmd_thread *pmd,
  * a rule covering this flow.  In this case, it's a lot cheaper
  * to catch it here than execute a miss. */
 netdev_flow = dp_netdev_pmd_lookup_flow(pmd, [i],
-_lookup_cnt);
+_lookup_cnt, _cd_hit);
 if (netdev_flow) {
 lookup_cnt += add_lookup_cnt;
+cd_hit += add_cd_hit;
 rules[i] = _flow->cr;
 continue;
 }
@@ -5519,6 +5527,9 @@ fast_path_processing(struct dp_netdev_pmd_thread *pmd,
 dp_netdev_count_packet(pmd, DP_STAT_LOOKUP_HIT, lookup_cnt);
 dp_netdev_count_packet(pmd, DP_STAT_MISS, miss_cnt);
 dp_netdev_count_packet(pmd, DP_STAT_LOST, lost_cnt);
+

[ovs-dev] [PATCH v2 1/5] dpif-netdev: Basic CD feature with scalar lookup.

[Yipeng Wang]

Cuckoo distributor (CD) is a double-hash function hash table, that helps
redirect packets to their corresponding subtables to avoid the sequential
search of megaflow subtables.  This is another layer of cache to cache flows
and their corresponding subtable indexes.  Different from a hash table, CD
can have certain false positive rate (since the full key is not stored for
space efficiency).  Our CD design was partially inspired by earlier concepts
proposed in "simTable"[1] and "Cuckoo Filter"[2], and DPDK's cuckoo hash
implementation.

For the current implementation, the design does not allow displacing items
when a bucket is full, which is different from the behavior of a cuckoo hash
table.  The advantage is that we do not need to store two signatures so that
the struct is more compact.  We use 16 entries per bucket for the
convenience of vector lookup.

Each classifier has its own cuckoo distributor.

[1] H. Lee and B. Lee, Approaches for improving tuple space search-based
table lookup, ICTC '15
[2] B. Fan, D. G. Andersen, M. Kaminsky, and M. D. Mitzenmacher,
Cuckoo Filter: Practically Better Than Bloom, CoNEXT '14

CC: Darrell Ball 
CC: Jan Scheurich 
Signed-off-by: Yipeng Wang 
Signed-off-by: Antonio Fischetti 
Co-authored-by: Antonio Fischetti 
---
Evaluation:
We create set of rules with various src IP. We feed traffic containing 1M
flows with various src IP and dst IP. All the flows hit 10/20/30
rules creating 10/20/30 subtables.

The table below shows the preliminary continuous testing results (full line
speed test) we collected with a uni-directional phy-to-phy setup. OvS
runs with 1 PMD. We use Spirent as the hardware traffic generator.

Scalar CD results:
1M flows:
no.subtable: 10  20  30
cd-ovs   3658328 3028111 2863329
orig_ovs 2683455 1646227 1240501
speedup  1.36x   1.84x   2.31x

---
 lib/dpif-netdev.c | 421 +-
 1 file changed, 419 insertions(+), 2 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index d5eb830..ea1d625 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -176,6 +176,66 @@ struct emc_cache {
  i__ < EM_FLOW_HASH_SEGS;\
  i__++, srch_hash__ >>= EM_FLOW_HASH_SHIFT)
 
+
+/* Cuckoo distributor (CD) is a double-hash function hash table, that helps
+ * redirect packets to their corresponding subtables to avoid the sequential
+ * search of megaflow subtables.  This is another layer of cache to cache flows
+ * and their corresponding subtable indexes.  Different from a hash table, CD
+ * can have certain false positive rate (since the full key is not stored for
+ * space efficiency).  Our CD design was partially inspired by earlier concepts
+ * proposed in "simTable"[1] and "Cuckoo Filter"[2], and DPDK's cuckoo hash
+ * implementation.
+ *
+ * For the current implementation, the design does not allow displacing items
+ * when a bucket is full, which is different from the behavior of a cuckoo hash
+ * table.  The advantage is that we do not need to store two signatures so that
+ * the struct is more compact.  We use 16 entries per bucket for the
+ * convenience of vector lookup.
+ *
+ * Each classifier has its own cuckoo distributor.
+ *
+ * [1] H. Lee and B. Lee, Approaches for improving tuple space search-based
+ * table lookup, ICTC '15
+ * [2] B. Fan, D. G. Andersen, M. Kaminsky, and M. D. Mitzenmacher,
+ * Cuckoo Filter: Practically Better Than Bloom, CoNEXT '14
+ */
+
+#define CD_NUM_BUCKETS (1 << 16)
+#define CD_BUCKET_MASK (CD_NUM_BUCKETS - 1)
+/* Number of entries per bucket. */
+#define CD_ENTRIES 16
+#define CD_ENTRY_MASK (CD_ENTRIES - 1)
+
+/* These two seeds are used for hashing out two bucket locations. */
+#define CD_PRIM_SEED 10
+#define CD_SEC_SEED 20
+
+/* This bit is used to choose which bucket to replace CD's entry
+ * in cd_insert. */
+#define CD_BKT_BIT (1 << CD_ENTRIES)
+
+/* Two byte signature and same length of mask. */
+typedef uint16_t cd_sig_t;
+#define CD_SIG_MASK 0x
+
+/* Length of Subtable pointer array for cuckoo distributor to index subtables.
+ * The size of the table is at most 2^16-1 entires because the CD's entry
+ * provides 2 bytes for indexing currently.
+ */
+#define SUB_PTR_LEN 256
+
+/* The bucket struct for cuckoo distributor*/
+struct cd_bucket {
+cd_sig_t sig[CD_ENTRIES];   /* 2-byte long signature. */
+uint16_t table_index[CD_ENTRIES];   /* index to subtable pointer array. */
+} __attribute__ ((packed));
+
+
+struct cd_cache {
+struct cd_bucket buckets[CD_NUM_BUCKETS]; /* buckets array. */
+} __attribute__ ((aligned (64)));
+
+
 /* Simple non-wildcarding single-priority classifier. */
 
 /* Time in ms between successive optimizations of the dpcls subtable vector */
@@ -194,6 +254,8 @@ struct dpcls {
 odp_port_t in_port;
 struct cmap subtables_map;
 struct pvector subtables;
+struct cd_cache *cdtable;   /* The cuckoo 

[ovs-dev] [PATCH v2 4/5] dpif-netdev: Add adaptive CD mechanism

[Yipeng Wang]

When there are only one subtable in the megaflow cache, CD does not benefit.
In such case, CD actually hurts the performance because of the extra CD lookup
process.

This patch implements an adaptive turn on/off CD mechanism. The average
iterated subtable count will be collected for every 5 seconds, and depending
on this count, CD will be turned on or off during run-time.

This patch depends on previous patches.

CC: Darrell Ball 
CC: Jan Scheurich 
Signed-off-by: Yipeng Wang 
Signed-off-by: Antonio Fischetti 
Co-authored-by: Antonio Fischetti 
---
 lib/dpif-netdev.c | 160 +++---
 1 file changed, 105 insertions(+), 55 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 5245cb5..425dfc4 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -227,6 +227,9 @@ typedef uint16_t cd_sig_t;
  */
 #define SUB_PTR_LEN 256
 
+/* Time in ms between the decisions of turning on or off CD. */
+#define DPCLS_CD_OPTIMIZATION_INTERVAL 5000
+
 /* The bucket struct for cuckoo distributor*/
 struct cd_bucket {
 cd_sig_t sig[CD_ENTRIES];   /* 2-byte long signature. */
@@ -258,6 +261,7 @@ struct dpcls {
 struct cmap subtables_map;
 struct pvector subtables;
 struct cd_cache *cdtable;   /* The cuckoo distributor. */
+uint8_t cd_on;/* Turn on or off CD during runtime. */
 struct dpcls_subtable *sub_ptrs[SUB_PTR_LEN]; /* Subtable pointer array. */
 };
 
@@ -643,6 +647,8 @@ struct dp_netdev_pmd_thread {
 struct cmap classifiers;
 /* Periodically sort subtable vectors according to hit frequencies */
 long long int next_optimization;
+/* Periodically decide if we should turn on/off CD */
+long long int next_cd_optimization;
 /* End of the next time interval for which processing cycles
are stored for each polled rxq. */
 long long int rxq_interval;
@@ -2866,12 +2872,14 @@ dp_netdev_flow_add(struct dp_netdev_pmd_thread *pmd,
 cmap_insert(>flow_table, CONST_CAST(struct cmap_node *, >node),
 dp_netdev_flow_hash(>ufid));
 /* Insert to CD here. */
-if (OVS_LIKELY(key != NULL)) {
-struct dpcls_subtable *subtable = dpcls_find_subtable(cls, );
-int index = find_index_in_sub_ptrs(cls, subtable);
+if (cls->cd_on) {
+if (OVS_LIKELY(key != NULL)) {
+struct dpcls_subtable *subtable = dpcls_find_subtable(cls, );
+int index = find_index_in_sub_ptrs(cls, subtable);
 
-if (index != 0) {
-cd_insert(cls->cdtable, key, index);
+if (index != 0) {
+cd_insert(cls->cdtable, key, index);
+}
 }
 }
 
@@ -6290,6 +6298,12 @@ struct dpcls_subtable {
 struct cmap rules;   /* Contains "struct dpcls_rule"s. */
 uint32_t hit_cnt;/* Number of match hits in subtable in current
 optimization interval. */
+uint32_t access_cnt; /* With CD implemented, hit_cnt should be subtable
+  * hits that miss in CD, so the ranking mechanism
+  * which is based on hit_cnt still works properly.
+  * We have the access_cnt as total access count to
+  * each subtable to consider if we should turn on
+  * or turn off CD. */
 struct netdev_flow_key mask; /* Wildcards for fields (const). */
 /* 'mask' must be the last field, additional space is allocated here. */
 };
@@ -6309,6 +6323,7 @@ dpcls_init(struct dpcls *cls)
 VLOG_ERR("Create cuckoo distributor failed");
 }
 cd_init(cls->cdtable);
+cls->cd_on = 1;
 for (i = 0; i < SUB_PTR_LEN; i++) {
 cls->sub_ptrs[i] = 0;
 }
@@ -6356,6 +6371,7 @@ dpcls_create_subtable(struct dpcls *cls, const struct 
netdev_flow_key *mask)
- sizeof subtable->mask.mf + mask->len);
 cmap_init(>rules);
 subtable->hit_cnt = 0;
+subtable->access_cnt = 0;
 netdev_flow_key_clone(>mask, mask);
 cmap_insert(>subtables_map, >cmap_node, mask->hash);
 /* Add the new subtable at the end of the pvector (with no hits yet) */
@@ -6432,6 +6448,34 @@ dp_netdev_pmd_try_optimize(struct dp_netdev_pmd_thread 
*pmd,
 pmd->next_optimization = now + DPCLS_OPTIMIZATION_INTERVAL;
 }
 }
+if (now > pmd->next_cd_optimization) {
+CMAP_FOR_EACH (cls, node, >classifiers) {
+struct pvector *pvec = >subtables;
+struct dpcls_subtable *subtable;
+float avg_table_cnt = 0;
+int cnt = 0;
+uint32_t total = 0;
+uint32_t sum = 0;
+PVECTOR_FOR_EACH (subtable,pvec) {
+sum += subtable->access_cnt * cnt;
+total += subtable->access_cnt;
+subtable->access_cnt = 0;
+cnt++;
+}
+/* If total access is too 

[ovs-dev] [PATCH v2 5/5] unit-test: Add a delay for CD initialization.

[Yipeng Wang]

This patch adds a delay during test 1215 for considering CD
initialization time.

CC: Darrell Ball 
CC: Jan Scheurich 
Signed-off-by: Yipeng Wang 
Signed-off-by: Antonio Fischetti 
Co-authored-by: Antonio Fischetti 
---
 tests/ofproto-dpif.at | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index c75a1ac..8b55454 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -9596,6 +9596,9 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
 dnl Start a new connection from port 1.
 AT_CHECK([ovs-appctl netdev-dummy/receive p1 
'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.1.1.1,dst=10.1.1.2,proto=17,tos=0,ttl=64,frag=no),udp(src=1,dst=2)'])
 
+# cuckoo distributor requires time for initilization, add sleep
+sleep 2
+
 AT_CHECK([cat ovs-vswitchd.log | strip_ufid | filter_flow_install], [0], [dnl
 
recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(proto=17,frag=no),
 actions:ct(commit)
 ])
-- 
2.7.4

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

[ovs-dev] [PATCH v2 0/5] dpif-netdev: Cuckoo-Distributor implementation

[Yipeng Wang]

The Datapath Classifier uses tuple space search for flow classification.
The rules are arranged into a set of tuples/subtables (each with a
distinct mask).  Each subtable is implemented as a hash table and lookup
is done with flow keys formed by selecting the bits from the packet header
based on each subtable's mask. Tuple space search will sequentially search
each subtable until a match is found. With a large number of subtables, a
sequential search of the subtables could consume a lot of CPU cycles. In
a testbench with a uniform traffic pattern equally distributed across 20
subtables, we measured that up to 65% of total execution time is attributed
to the megaflow cache lookup.

This patch presents the idea of the two-layer hierarchical lookup, where a
low overhead first level of indirection is accessed first, we call this
level cuckoo distributor (CD). If a flow key has been inserted in the flow
table the first level will indicate with high probability that which
subtable to look into. A lookup is performed on the second level (the
target subtable) to retrieve the result. If the key doesn’t have a match,
then we revert back to the sequential search of subtables. The patch is
partially inspired by earlier concepts proposed in "simTable"[1] and
"Cuckoo Filter"[2], and DPDK's Cuckoo Hash implementation.

This patch can improve the already existing Subtable Ranking when traffic
data has high entropy. Subtable Ranking helps minimize the number of
traversed subtables when most of the traffic hit the same subtable.
However, in the case of high entropy traffic such as traffic coming from
a physical port, multiple subtables could be hit with a similar frequency.
In this case the average subtable lookups per hit would be much greater
than 1. In addition, CD can adaptively turn off when it finds the traffic
mostly hit one subtable. Thus, CD will not be an overhead when Subtable
Ranking works well.

Scheme:
CD is in front of the subtables. Packets are directed to corresponding subtable
if hit in CD instead of searching each subtable sequentially.
 ---
|  CD   |
 ---
   \
\
 -  - -
|sub  ||sub  |...|sub  |
|table||table|   |table|
 -  - -

 Evaluation:
 --
We create a set of rules with various src IP. We feed traffic containing various
numbers of flows with various src IP and dst IP. All the flows hit 10/20/30
rules creating 10/20/30 subtables. We will explain the rule/traffic setup
in detail later.

The table below shows the preliminary continuous testing results (full line
speed test) we collected with a uni-directional phy-to-phy setup. OvS
runs with 1 PMD. We use Spirent as the hardware traffic generator.

 Before v2 rebase:
 
AVX2 data:
20k flows:
no.subtable: 10  20  30
cd-ovs   4267332 3478251 3126763
orig-ovs 3260883 2174551 1689981
speedup  1.31x   1.60x   1.85x

100k flows:
no.subtable: 10  20  30
cd-ovs   4015783 3276100 2970645
orig-ovs 2692882 1711955 1302321
speedup  1.49x   1.91x   2.28x

1M flows:
no.subtable: 10  20  30
cd-ovs   3895961 3170530 2968555
orig-ovs 2683455 1646227 1240501
speedup  1.45x   1.92x   2.39x

Scalar data:
1M flows:
no.subtable: 10  20  30
cd-ovs   3658328 3028111 2863329
orig_ovs 2683455 1646227 1240501
speedup  1.36x   1.84x   2.31x

 After v2 rebase:
 
After rebase for v1, we tested 1M flows, 20 table cases, the results still hold.
1M flows:
no.subtable:   20
cd-ovs 3066483
orig-ovs   1588049
speedup1.93x


 Test rules/traffic setup:
 
To setup a test case with 20 subtables, the rule set we use is like below:
tcp,nw_src=1.0.0.0/8, actions=output:1
udp,nw_src=2.0.0.0/9, actions=output:1
udp,nw_src=3.0.0.0/10,actions=output:1
udp,nw_src=4.0.0.0/11,actions=output:1
...
udp,nw_src=18.0.0.0/25,actions=output:1
udp,nw_src=19.0.0.0/26,actions=output:1
udp,nw_src=20.0.0.0/27,actions=output:1

Then for the traffic generator, we generate corresponding traffics with
src_ip varying from 1.0.0.0 to 20.0.0.0. For each src_ip, we change
dst_ip for 5 different values. This will effectively generate 1M
different flows hitting the 20 rules we created. And because the different
wildcarding bits in nw_src, the 20 rules will belong to 20 subtables.
We use 64 Bytes packet across all tests.

How to check if CD works or not for your use case:
 
CD cannot improve throughput for all use cases. It targets on use cases when
multiple subtables exist and when the top-ranked subtable is not hit by the
vast majority of the traffic.

One can use $OVS_DIR/utilities/ovs-appctl dpif-netdev/pmd-stats-show
command to check CD statistics: hit/miss.
Another statistic also shown is: "avg. subtable lookups per hit".
In our test case, the original OvS will have an average subtable lookups value
as 10, because 

[ovs-dev] [PATCH v2 2/5] dpif-netdev: Add AVX2 implementation for CD lookup.

[Yipeng Wang]

This patch adds the AVX2 implementation during CD lookup. 16 entries of a
bucket will be compared together with the lookup key. This patch depends
on the first patch.

CC: Darrell Ball 
CC: Jan Scheurich 
Signed-off-by: Yipeng Wang 
Signed-off-by: Antonio Fischetti 
Co-authored-by: Antonio Fischetti 
---
evaluation:
We setup the testing enviornment same to the previous patch. The AVX2
CD implementation's results are shown below.

AVX2 data:
1M flows:
no.subtable: 10  20  30
cd-ovs   3895961 3170530 2968555
orig-ovs 2683455 1646227 1240501
speedup  1.45x   1.92x   2.39x
---
 lib/dpif-netdev.c | 67 ++-
 1 file changed, 66 insertions(+), 1 deletion(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index ea1d625..78219ba 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -30,6 +30,9 @@
 #include 
 #include 
 #include 
+#if defined(__AVX2__)
+#include 
+#endif
 
 #ifdef DPDK_NETDEV
 #include 
@@ -2378,7 +2381,37 @@ cd_lookup_bulk_pipe(struct dpcls *cls, const struct 
netdev_flow_key keys[],
 
 OVS_PREFETCH(prim_bkt1);
 OVS_PREFETCH(sec_bkt1);
+#ifdef __AVX2__
+prim_hitmask = _mm256_movemask_epi8((__m256i)_mm256_cmpeq_epi16(
+_mm256_load_si256((__m256i const *)prim_bkt0->sig),
+_mm256_set1_epi16(temp_sig0)));
+
+
+sec_hitmask = _mm256_movemask_epi8((__m256i)_mm256_cmpeq_epi16(
+_mm256_load_si256((__m256i const *)sec_bkt0->sig),
+_mm256_set1_epi16(temp_sig0)));
 
+if (prim_hitmask) {
+loc = raw_ctz(prim_hitmask) >> 1;
+data[i-1] =
+ prim_bkt0->table_index[loc];
+if (data[i-1] != 0 && cls->sub_ptrs[data[i-1]] != 0) {
+hits |= 1 << (i - 1);
+prim_bkt0 = prim_bkt1;
+sec_bkt0 = sec_bkt1;
+temp_sig0 = temp_sig1;
+continue;
+}
+}
+
+if (sec_hitmask) {
+loc = raw_ctz(sec_hitmask) >> 1;
+data[i-1] = sec_bkt0->table_index[loc];
+if (data[i-1] != 0 && cls->sub_ptrs[data[i-1]] != 0) {
+   hits |= 1 << (i - 1);
+}
+}
+#else
 unsigned int j;
 prim_hitmask = 0;
 sec_hitmask = 0;
@@ -2407,12 +2440,42 @@ cd_lookup_bulk_pipe(struct dpcls *cls, const struct 
netdev_flow_key keys[],
 hits |= 1 << (i - 1);
 }
 }
-
+#endif
 prim_bkt0 = prim_bkt1;
 sec_bkt0 = sec_bkt1;
 temp_sig0 = temp_sig1;
 }
 
+#ifdef __AVX2__
+prim_hitmask = _mm256_movemask_epi8((__m256i)_mm256_cmpeq_epi16(
+_mm256_load_si256((__m256i const *)prim_bkt0->sig),
+_mm256_set1_epi16(temp_sig0)));
+
+
+sec_hitmask = _mm256_movemask_epi8((__m256i)_mm256_cmpeq_epi16(
+_mm256_load_si256((__m256i const *)sec_bkt0->sig),
+_mm256_set1_epi16(temp_sig0)));
+
+if (prim_hitmask) {
+loc = raw_ctz(prim_hitmask) >> 1;
+data[i-1] = prim_bkt0->table_index[loc];
+if (data[i-1] != 0 && cls->sub_ptrs[data[i-1]] != 0) {
+hits |= 1 << (i - 1);
+if (hit_mask != NULL) {
+*hit_mask = hits;
+}
+return;
+}
+ }
+
+if (sec_hitmask) {
+loc = raw_ctz(sec_hitmask) >> 1;
+data[i-1] = sec_bkt0->table_index[loc];
+if (data[i-1] != 0 && cls->sub_ptrs[data[i-1]] != 0) {
+   hits |= 1 << (i - 1);
+}
+}
+#else
 unsigned int j;
 prim_hitmask = 0;
 sec_hitmask = 0;
@@ -2442,9 +2505,11 @@ cd_lookup_bulk_pipe(struct dpcls *cls, const struct 
netdev_flow_key keys[],
 }
 }
 
+#endif
 if (hit_mask != NULL) {
 *hit_mask = hits;
 }
+
 }
 
 static int
-- 
2.7.4

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

[ovs-dev] [PATCHv3 3/3] ovsdb-server.c: Fix memory leak

[Yifeng Sun]

Valgrind testcase 2349 (ovn -- DSCP marking check) reports the leak below:
21 bytes in 21 blocks are definitely lost in loss record 24 of 362
at 0x4C2DB8F: malloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x436FD4: xmalloc (util.c:120)
by 0x437044: xmemdup0 (util.c:150)
by 0x408C97: add_manager_options (ovsdb-server.c:709)
by 0x408C97: query_db_remotes (ovsdb-server.c:765)
by 0x408C97: reconfigure_remotes (ovsdb-server.c:926)
by 0x406273: main_loop (ovsdb-server.c:194)
by 0x406273: main (ovsdb-server.c:434)

When options are freed, options->role need to be freed explicitly.

v1->v3: Amend valgrind report.

Signed-off-by: Yifeng Sun 
---
 ovsdb/ovsdb-server.c | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index 030d86ba467f..cd30dd48425b 100644
--- a/ovsdb/ovsdb-server.c
+++ b/ovsdb/ovsdb-server.c
@@ -674,6 +674,21 @@ add_remote(struct shash *remotes, const char *target)
 return options;
 }
 
+static void
+free_remotes(struct shash *remotes)
+{
+struct ovsdb_jsonrpc_options *options;
+struct shash_node *node;
+
+if (remotes) {
+SHASH_FOR_EACH(node, remotes) {
+options = node->data;
+free(options->role);
+}
+shash_destroy_free_data(remotes);
+}
+}
+
 /* Adds a remote and options to 'remotes', based on the Manager table row in
  * 'row'. */
 static void
@@ -929,7 +944,7 @@ reconfigure_remotes(struct ovsdb_jsonrpc_server *jsonrpc,
 }
 }
 ovsdb_jsonrpc_server_set_remotes(jsonrpc, _remotes);
-shash_destroy_free_data(_remotes);
+free_remotes(_remotes);
 
 return errors.string;
 }
-- 
2.7.4

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

[ovs-dev] [PATCHv3 1/3] ovsdb-idl: Fix memory leak

[Yifeng Sun]

Valgrind testcase 2339 (ovn -- ipam connectivity) reports the leak below:
45 (32 direct, 13 indirect) bytes in 1 blocks are definitely lost in loss 
record 65 of 83
at 0x4C2DB8F: malloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x4A6D64: xmalloc (util.c:120)
by 0x49C847: shash_add_nocopy__ (shash.c:109)
by 0x49C847: shash_add_nocopy (shash.c:121)
by 0x49CA85: shash_add (shash.c:129)
by 0x49CA85: shash_add_once (shash.c:136)
by 0x4914B5: ovsdb_idl_create_index (ovsdb-idl.c:2067)
by 0x406C98: create_ovnsb_indexes (ovn-controller.c:568)
by 0x406C98: main (ovn-controller.c:619)

The leak happens when vsdb_idl_table is freed but its indexes are not freed.

v1->v2: Amend comments.
v2->v3: Fix error in patch.

Signed-off-by: Yifeng Sun 
---
 lib/ovsdb-idl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 5617e08d633c..be29c92957c0 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -2163,6 +2163,7 @@ ovsdb_idl_destroy_indexes(struct ovsdb_idl_table *table)
 skiplist_destroy(index->skiplist, NULL);
 free(index->columns);
 }
+shash_destroy_free_data(>indexes);
 }
 
 static void
-- 
2.7.4

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

Re: [ovs-dev] [PATCH 1/2] acinclude: Add support for grep option.

[William Tu]

On Mon, Oct 30, 2017 at 1:04 PM, Ben Pfaff  wrote:
> On Mon, Oct 16, 2017 at 07:26:44AM -0700, William Tu wrote:
>> Allow to pass grep's option to OVS_GREP_IFELSE.
>> One use case is to pass '-w' for exact match.
>>
>> Signed-off-by: William Tu 
>
> POSIX doesn't mention a -w option, and the Autoconf manual says that it
> is not portable in practice.  It also says that \b is not portable in
> practice.
>
> Is there another way to accomplish what you want to do?  For example,
> how about HAVE_SKB_GSO_UDP[^_]?  Since this is Autoconf, probably it's
> necessary to double the [], as: HAVE_SKB_GSO_UDP[[^_]]
>
> (We don't really care that much about portability to everything that
> Autoconf supports, so probably we could really use -w or \b in
> practice.)
>
> This is what the Autoconf manual says:
>
> 'grep'
>  Portable scripts can rely on the 'grep' options '-c', '-l', '-n',
>  and '-v', but should avoid other options.  For example, don't use
>  '-w', as Posix does not require it and Irix 6.5.16m's 'grep' does
>  not support it.  Also, portable scripts should not combine '-c'
>  with '-l', as Posix does not allow this.
>
>  Some of the options required by Posix are not portable in practice.
>  Don't use 'grep -q' to suppress output, because many 'grep'
>  implementations (e.g., Solaris) do not support '-q'.  Don't use
>  'grep -s' to suppress output either, because Posix says '-s' does
>  not suppress output, only some error messages; also, the '-s'
>  option of traditional 'grep' behaved like '-q' does in most modern
>  implementations.  Instead, redirect the standard output and
>  standard error (in case the file doesn't exist) of 'grep' to
>  '/dev/null'.  Check the exit status of 'grep' to determine whether
>  it found a match.
>
>  The QNX4 implementation fails to count lines with 'grep -c '$'',
>  but works with 'grep -c '^''.  Other alternatives for counting
>  lines are to use 'sed -n '$='' or 'wc -l'.
>
>  Some traditional 'grep' implementations do not work on long input
>  lines.  On AIX the default 'grep' silently truncates long lines on
>  the input before matching.
>
>  Also, many implementations do not support multiple regexps with
>  '-e': they either reject '-e' entirely (e.g., Solaris) or honor
>  only the last pattern (e.g., IRIX 6.5 and NeXT). To work around
>  these problems, invoke 'AC_PROG_GREP' and then use '$GREP'.
>
>  Another possible workaround for the multiple '-e' problem is to
>  separate the patterns by newlines, for example:
>
>   grep 'foo
>   bar' in.txt
>
>  except that this fails with traditional 'grep' implementations and
>  with OpenBSD 3.8 'grep'.
>
>  Traditional 'grep' implementations (e.g., Solaris) do not support
>  the '-E' or '-F' options.  To work around these problems, invoke
>  'AC_PROG_EGREP' and then use '$EGREP', and similarly for
>  'AC_PROG_FGREP' and '$FGREP'.  Even if you are willing to require
>  support for Posix 'grep', your script should not use both '-E' and
>  '-F', since Posix does not allow this combination.
>
>  Portable 'grep' regular expressions should use '\' only to escape
>  characters in the string '$()*.0123456789[\^{}'.  For example,
>  alternation, '\|', is common but Posix does not require its support
>  in basic regular expressions, so it should be avoided in portable
>  scripts.  Solaris and HP-UX 'grep' do not support it.  Similarly,
>  the following escape sequences should also be avoided: '\<', '\>',
>  '\+', '\?', '\`', '\'', '\B', '\b', '\S', '\s', '\W', and '\w'.
>
>  Posix does not specify the behavior of 'grep' on binary files.  An
>  example where this matters is using BSD 'grep' to search text that
>  includes embedded ANSI escape sequences for colored output to
>  terminals ('\033[m' is the sequence to restore normal output); the
>  behavior depends on whether input is seekable:
>
>   $ printf 'esc\033[mape\n' > sample
>   $ grep . sample
>   Binary file sample matches
>   $ cat sample | grep .
>   escape

Thanks for the feedback. I will submit v2 patch to use
HAVE_SKB_GSO_UDP[[^_]] instead.
William
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH, RFC] tests: Add a default timeout for control utilities

[Ben Pfaff]

On Sat, Sep 02, 2017 at 08:52:38AM -0700, Ben Pfaff wrote:
> On Mon, Aug 28, 2017 at 08:14:59PM +0300, Alin Gabriel Serdean wrote:
> > Let's suppose that ovsdb-server is running properly, but ovs-vswitchd
> > is not responsive/crashed. We try to add a port via ovs-vsctl and it will
> > hang.
> > This patch aims at that scenario and tries to make life easier when
> > debugging hanging tests.
> > 
> > Some shells do not allow dashes in function names (default behavior),
> > we shall try to define an alias to overcome dashes if the shell allows it.
> > 
> > Signed-off-by: Alin Gabriel Serdean 
> > Suggested-by: Ben Pfaff 
> 
> Acked-by: Ben Pfaff 

I expected that you'd apply this, but, anyway, I've done it just now.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] system-traffic: Fix conntrack tests

[Ben Pfaff]

On Mon, Oct 30, 2017 at 10:31:14AM -0700, William Tu wrote:
> On Thu, Oct 26, 2017 at 2:24 PM, Yi-Hung Wei  wrote:
> > Three conntrack system-traffic tests are broken because of a recent
> > change 7827edcaebd8 ("Add dl_type to flow metadata for correct
> > interpretation of conntrack metadata"). It can be reproduced by
> >   $ make check-system-userspace TESTSUITEFLAGS='18 19 37'
> >
> > This patch modifies the check messages to fix the breakage.
> >
> > Fixes: 7827edcaebd8 ("Add dl_type to flow metadata for correct 
> > interpretation of conntrack metadata")
> > CC: Daniel Alvarez 
> > Signed-off-by: Yi-Hung Wei 
> > ---
> 
> Looks good to me.
> 
> Tested-by: William Tu 

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

Re: [ovs-dev] [PATCH 7/9] flow: Refactor parse_ct_state()

[Ben Pfaff]

On Fri, Aug 25, 2017 at 03:51:17PM -0700, Yi-Hung Wei wrote:
> Refactor parse_ct_state() to support different delimiters.
> 
> Signed-off-by: Yi-Hung Wei 
> ---
>  lib/flow.c   | 6 +++---
>  lib/flow.h   | 2 +-
>  ofproto/ofproto-dpif-trace.c | 2 +-
>  ovn/utilities/ovn-trace.c| 2 +-
>  4 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/flow.c b/lib/flow.c
> index b2b10aa488be..34bc176e8b6e 100644
> --- a/lib/flow.c
> +++ b/lib/flow.c
> @@ -1129,14 +1129,14 @@ ct_state_from_string(const char *s)
>   * returns false, and reports error message in 'ds'. */
>  bool
>  parse_ct_state(const char *state_str, uint32_t default_state,
> -   uint32_t *ct_state, struct ds *ds)
> +   const char *delimiters, uint32_t *ct_state, struct ds *ds)

Thanks for working on this.

parse_ct_state() has a pretty good function-level comment, so would you
mind updating it to mention the new parameter?

Thanks,

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

Re: [ovs-dev] [PATCH 4/9] ofproto/trace: Query ct_state for conntrack recirc from DP

[Ben Pfaff]

On Fri, Aug 25, 2017 at 03:51:14PM -0700, Yi-Hung Wei wrote:
> Instead of using fixed default conntrack state 'trk|new' in
> ofproto/trace for conntrack recirculation, this patch queries the
> conntrack state from datapath using ct_dpif_get_info().
> 
> Signed-off-by: Yi-Hung Wei 

I'm getting a patch reject trying to apply this.  Will you rebase and
re-post the series?

Thanks,

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

Re: [ovs-dev] [PATCH 2/9] ofproto/trace: Propagate ct_zone in recirculation

[Ben Pfaff]

On Thu, Aug 31, 2017 at 02:38:35PM -0700, Greg Rose wrote:
> On 08/25/2017 03:51 PM, Yi-Hung Wei wrote:
> >This patch propagates ct_zone when ofproto/trace automatically runs
> >through the recirculation process.
> >
> >Fixes: e6bc8e749381 ("ofproto/trace: Add support for tracing conntrack 
> >recirculation")
> >Signed-off-by: Yi-Hung Wei 
> >---
> >  ofproto/ofproto-dpif-trace.c |  4 +++-
> >  ofproto/ofproto-dpif-trace.h |  3 ++-
> >  ofproto/ofproto-dpif-xlate.c |  7 ---
> >  tests/ofproto-dpif.at| 14 +++---
> >  4 files changed, 16 insertions(+), 12 deletions(-)
> >
> >diff --git a/ofproto/ofproto-dpif-trace.c b/ofproto/ofproto-dpif-trace.c
> >index a45c9cfd9619..c3c929520a2d 100644
> >--- a/ofproto/ofproto-dpif-trace.c
> >+++ b/ofproto/ofproto-dpif-trace.c
> >@@ -91,7 +91,8 @@ oftrace_node_destroy(struct oftrace_node *node)
> >  bool
> >  oftrace_add_recirc_node(struct ovs_list *recirc_queue,
> >  enum oftrace_recirc_type type, const struct flow 
> > *flow,
> >-const struct dp_packet *packet, uint32_t recirc_id)
> >+const struct dp_packet *packet, uint32_t recirc_id,
> >+const uint16_t zone)
> 
> This function is beginning to get a lot of parameters.  As a suggestion 
> perhaps you might create a helper struct
> to contain all the parameters and just pass a pointer.  Personally I start 
> looking for ways to cut down on parameter
> passing when a function gets to 4 or more parameters.  Again - just a 
> personal predilection.
> 
> Otherwise the patch LGTM.
> 
> Reviewed-by: Greg Rose 

Thanks Yi-Hung and Greg.

I applied this to master.  Greg, I think that your comment is fair, but
it's not a crisis yet so I'll leave that for a possible later
improvement.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/9] ofproto/trace: Fix memory leak in oftrace_push_ct_state()

[Ben Pfaff]

On Fri, Aug 25, 2017 at 03:51:11PM -0700, Yi-Hung Wei wrote:
> Free the allocated memory in the pop function.
> 
> Fixes: 0f2f05bbcf743 ("ofproto/trace: Add --ct-next option to ofproto/trace")
> Signed-off-by: Yi-Hung Wei 
> ---
>  ofproto/ofproto-dpif-trace.c | 13 -
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/ofproto/ofproto-dpif-trace.c b/ofproto/ofproto-dpif-trace.c
> index 38d11002f290..a45c9cfd9619 100644
> --- a/ofproto/ofproto-dpif-trace.c
> +++ b/ofproto/ofproto-dpif-trace.c
> @@ -128,12 +128,14 @@ oftrace_push_ct_state(struct ovs_list *next_ct_states, 
> uint32_t ct_state)
>  ovs_list_push_back(next_ct_states, _ct_state->node);
>  }
>  
> -static uint32_t
> -oftrace_pop_ct_state(struct ovs_list *next_ct_states)
> +static void
> +oftrace_pop_ct_state(struct ovs_list *next_ct_states, uint32_t *ct_state)
>  {
>  struct oftrace_next_ct_state *s;
>  LIST_FOR_EACH_POP (s, node, next_ct_states) {
> -return s->state;
> +*ct_state = s->state;
> +free(s);
> +return;
>  }
>  OVS_NOT_REACHED();
>  }

Thanks for the fix!

I don't understand why the function return type needs to change.  Can
you change this to preserve the return type, while fixing the memory
leak?

Thanks,

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

Re: [ovs-dev] [RFC PATCH v2 08/10] vswitch.xml: Detail vxlanipsec user interface.

[Ben Pfaff]

On Fri, Aug 25, 2017 at 05:40:30PM +0100, Ian Stokes wrote:
> This commit adds details to the vswitch xml regarding the use of the
> vxlanipsec interface type. This patch is not intended for upstreaming
> and simply seeks to solicit feedback on the user interface design of
> the vxlanipsec port type as described in the vswitch.xml.
> 
> This modifies the vswitch.xml with a proposed vxlanipsec interface.
> It also provides details for the proposed interface options such as
> SPD creation, SA creation and modification, Policy entries for the
> SPD as well as traffic selector options for the policy.
> 
> Signed-off-by: Ian Stokes 

Thanks for adding documentation.

Would you mind adding the documentation in the same commit that adds the
documented feature?  We find that this makes what's going on a little
easier to understand.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC PATCH v2 06/10] vxlanipsec: Add userspace support for vxlan ipsec.

[Ben Pfaff]

On Fri, Aug 25, 2017 at 05:40:28PM +0100, Ian Stokes wrote:
> This patch introduces a new tunnel port type 'vxlanipsec'. This port
> combines vxlan tunnelling with IPsec operating in transport mode.
> 
> Ciphering and authentication actions ares provided by a DPDK cryptodev.
> The cryptodev operates as a vdev and is associated with the vxlan tunnel
> port. Upon tunnel encapsulation packets are encrypted and a hash digest
> attached to the packet as per RFC4303. Upon decapsulation a packet is
> first verified via the hash and then decrypted.
> 
> The cipher algorithm used is 128 AES-CBC and the authentication algorithm
> is HMAC-SHA1-96. Note this work is in progress and is not meant for
> upstream. It's purpose is to solicit feedback on the approach and known
> issues flagged in the accompanying cover letter to the patch series.
> 
> Signed-off-by: Ian Stokes 

Thanks a lot for working on this feature!

When I compile without dpdk enabled, I now get:

../lib/netdev-vport.c:31:10: fatal error: 'rte_config.h' file not found
../lib/netdev-native-tnl.c:35:10: fatal error: 'rte_config.h' file not found
"sparse" complains:

../lib/netdev-vport.h:40:22: warning: symbol 'spi_map' was not declared. Should 
it be static?

There is obviously a lot of code here to review, but I have not started
on that yet.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC PATCH v2 04/10] flow: Add ESP spi value to flow struct.

[Ben Pfaff]

On Tue, Oct 31, 2017 at 02:46:15PM -0700, Ben Pfaff wrote:
> On Fri, Aug 25, 2017 at 05:40:26PM +0100, Ian Stokes wrote:
> > This patch adds a field to the flow struct to represent the ESP security
> > parameter index of a packet.
> > 
> > Signed-off-by: Ian Stokes 
> > ---
> >  include/openvswitch/flow.h |5 +
> >  1 files changed, 5 insertions(+), 0 deletions(-)
> > 
> > diff --git a/include/openvswitch/flow.h b/include/openvswitch/flow.h
> > index a658a58..d986929 100644
> > --- a/include/openvswitch/flow.h
> > +++ b/include/openvswitch/flow.h
> > @@ -156,6 +156,11 @@ struct flow {
> >  ovs_be32 igmp_group_ip4;/* IGMP group IPv4 address.
> >   * Keep last for BUILD_ASSERT_DECL below. 
> > */
> >  ovs_be32 pad3;  /* Pad to 64 bits. */
> > +
> > +/* SPI for ESP (64-bit aligned) */
> > +/* XXX TO DO: move this to the l3 layer */
> > +ovs_be32 spi;
> > +ovs_be32 pad4;
> >  };
> 
> I'd like to see this moved to the L3 layer (as your comment says).

Actually, I think I take that back.  Isn't this logically part of L4?
(If so, then 'spi' can replace 'pad3' instead of being a doubleword of
its own.)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC PATCH v2 05/10] flow: Modify minflow extract to handle SPI.

[Ben Pfaff]

On Fri, Aug 25, 2017 at 05:40:27PM +0100, Ian Stokes wrote:
> The patch modifies the miniflow extract function to hande the case where
> the network protocol for a packet is ESP. In this case the SPI value in
> the ESP header is extracted and set in the minflow map.
> 
> Signed-off-by: Ian Stokes 
> ---
>  lib/flow.c |   11 ++-
>  1 files changed, 10 insertions(+), 1 deletions(-)
> 
> diff --git a/lib/flow.c b/lib/flow.c
> index b2b10aa..428ff7a 100644
> --- a/lib/flow.c
> +++ b/lib/flow.c
> @@ -643,6 +643,7 @@ miniflow_extract(struct dp_packet *packet, struct 
> miniflow *dst)
>  uint8_t nw_frag, nw_tos, nw_ttl, nw_proto;
>  uint8_t *ct_nw_proto_p = NULL;
>  ovs_be16 ct_tp_src = 0, ct_tp_dst = 0;
> +ovs_be32 esp_spi = 0;
>  
>  /* Metadata. */
>  if (flow_tnl_dst_is_set(>tunnel)) {
> @@ -920,7 +921,15 @@ miniflow_extract(struct dp_packet *packet, struct 
> miniflow *dst)
>  miniflow_push_be16(mf, ct_tp_src, ct_tp_src);
>  miniflow_push_be16(mf, ct_tp_dst, ct_tp_dst);
>  }
> -} else if (OVS_LIKELY(nw_proto == IPPROTO_SCTP)) {
> +} else if (OVS_LIKELY(nw_proto == IPPROTO_ESP)) {
> +if (OVS_LIKELY(size >= ESP_HEADER_LEN)) {
> +const struct esp_header *esp = data;
> +
> +esp_spi = esp->spi;
> +miniflow_push_be32(mf, spi, esp_spi);
> +miniflow_push_be32(mf, pad4, 0); /* Pad for ESP */
> +}
> +}else if (OVS_LIKELY(nw_proto == IPPROTO_SCTP)) {
>  if (OVS_LIKELY(size >= SCTP_HEADER_LEN)) {
>  const struct sctp_header *sctp = data;

This removes a space from the SCTP 'if' statement, it would be better
without that change.

Thanks,

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

Re: [ovs-dev] [RFC PATCH v2 04/10] flow: Add ESP spi value to flow struct.

[Ben Pfaff]

On Fri, Aug 25, 2017 at 05:40:26PM +0100, Ian Stokes wrote:
> This patch adds a field to the flow struct to represent the ESP security
> parameter index of a packet.
> 
> Signed-off-by: Ian Stokes 
> ---
>  include/openvswitch/flow.h |5 +
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/include/openvswitch/flow.h b/include/openvswitch/flow.h
> index a658a58..d986929 100644
> --- a/include/openvswitch/flow.h
> +++ b/include/openvswitch/flow.h
> @@ -156,6 +156,11 @@ struct flow {
>  ovs_be32 igmp_group_ip4;/* IGMP group IPv4 address.
>   * Keep last for BUILD_ASSERT_DECL below. */
>  ovs_be32 pad3;  /* Pad to 64 bits. */
> +
> +/* SPI for ESP (64-bit aligned) */
> +/* XXX TO DO: move this to the l3 layer */
> +ovs_be32 spi;
> +ovs_be32 pad4;
>  };

I'd like to see this moved to the L3 layer (as your comment says).
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC PATCH v2 03/10] packets: Add ESP header and trailer.

[Ben Pfaff]

On Fri, Aug 25, 2017 at 05:40:25PM +0100, Ian Stokes wrote:
> This patch introduces structs for both ESP headers and ESP trailers
> along with expected size assertions.
> 
> Signed-off-by: Ian Stokes 

Applied to master, thanks!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC PATCH v2 02/10] openvswitch.h: add vport to ovs_action_push_tnl.

[Ben Pfaff]

On Fri, Aug 25, 2017 at 05:40:24PM +0100, Ian Stokes wrote:
> Upon callback for building/pushing a packet header when encapsulating
> for tunneling a reference to the vport in question is required to access
> associated devices such as cryptodevs. This patch adds this pointer and
> will enable future work with cryptodevs that are associated with a
> vport.
> 
> Signed-off-by: Ian Stokes 
> ---
>  datapath/linux/compat/include/linux/openvswitch.h |3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
> b/datapath/linux/compat/include/linux/openvswitch.h
> index bc6c94b..afa7faf 100644
> --- a/datapath/linux/compat/include/linux/openvswitch.h
> +++ b/datapath/linux/compat/include/linux/openvswitch.h
> @@ -723,6 +723,8 @@ struct ovs_action_hash {
>   * @tnl_port: To identify tunnel port to pass header info.
>   * @out_port: Physical port to send encapsulated packet.
>   * @header_len: Length of the header to be pushed.
> + * @dev: Pointer to vport so that the cryptodev parameters associated with 
> the
> + * vport can be accessed at the callback function.
>   * @tnl_type: This is only required to format this header.  Otherwise
>   * ODP layer can not parse %header.
>   * @header: Partial header for the tunnel. Tunnel push action can use
> @@ -732,6 +734,7 @@ struct ovs_action_push_tnl {
>   odp_port_t tnl_port;
>   odp_port_t out_port;
>   uint32_t header_len;
> +struct netdev_vport *dev;
>   uint32_t tnl_type; /* For logging. */
>   uint32_t header[TNL_PUSH_HEADER_SIZE / 4];
>  };

Maybe this is safe for some reason, but I worry that there's the
possibility of a use-after-free error.  Is 'dev' supposed to hold a
reference to the netdev (with netdev_ref())?  If so, it would be good to
document that in the comment.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC PATCH v2 01/10] acinclude.m4: Support compilation of libIPsec.

[Ben Pfaff]

On Fri, Aug 25, 2017 at 05:40:23PM +0100, Ian Stokes wrote:
> LibIpsecMB is required to enable the use of vdev cryptodev devices in
> DPDK. This patch adds a condition to check for the library when it is
> detected that ONFIG_RTE_LIBRTE_PMD_AESNI_MB=y is enabled in the DPDK
> config.
> 
> Signed-off-by: Ian Stokes 
> ---
>  acinclude.m4 |   13 +
>  1 files changed, 13 insertions(+), 0 deletions(-)
> 
> diff --git a/acinclude.m4 b/acinclude.m4
> index aeb594a..8c14367 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -271,6 +271,19 @@ AC_DEFUN([OVS_CHECK_DPDK], [
> ], [],
> [AC_DEFINE([DPDK_PDUMP], [1], [DPDK pdump enabled in OVS.])])
>   ])
> +AC_COMPILE_IFELSE([
> +  AC_LANG_PROGRAM(
> +[
> +  #include 
> +#if RTE_LIBRTE_PMD_AESNI_MB
> +#error
> +#endif
> +], [])
> +  ], [],
> +  [AC_SEARCH_LIBS([init_mb_mgr_sse],[IPSec_MB],[],[AC_MSG_ERROR([unable 
> to find lib_IPSec_MB in ${LDFLAGS}, install the dependency package])])
> +   DPDK_EXTRA_LIB="-lIPSec_MB"
> +   AC_DEFINE([PMD_AESNI_MB], [1], [PMD_AESNI_MB support detected in 
> DPDK.])])
> +

It is a little unusual to make a test fail when a feature
(RTE_LIBRTE_PMD_AESNI_MB in this case) is detected.  This makes the
feature prone to being detected if something unrelated fails in the
toolchain.  Usually, one would either use the opposite approach--that
is, fail if RTE_LIBRTE_PMD_AESNI_MB is not declared--or something based
on AC_CHECK_DECL or AC_LINK_IFELSE using some symbol that
RTE_LIBRTE_PMD_AESNI_MB makes available.

Also, I recommend adding an explanation of this dependency to the
documentation, otherwise this will be confusing to users.

Thanks,

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

[ovs-dev] [PATCHv2 2/2] test-ovsdb: Fix memory leak (Amend comments)

[Yifeng Sun]

v1 -> v2:

range_end_atom is allocated in ovsdb_atom_from_string__() and no one is holding 
a reference to it at the end of do_parse_atom_strings(). It should be freed, as 
also pointed out by ovsdb_atom_destroy().

Valgrind report is as below:

16 bytes in 1 blocks are definitely lost in loss record 2 of 5
at 0x4C2DB8F: malloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x43F5F4: xmalloc (util.c:120)
by 0x424AC6: alloc_default_atoms (ovsdb-data.c:315)
by 0x4271E0: ovsdb_atom_from_string__ (ovsdb-data.c:508)
by 0x4271E0: ovsdb_atom_from_string (ovsdb-data.c:632)
by 0x40ADCC: do_parse_atom_strings (test-ovsdb.c:566)
by 0x41BA73: ovs_cmdl_run_command__ (command-line.c:115)
by 0x4051C9: main (test-ovsdb.c:72)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCHv2 1/2] ovsdb-idl: Fix memory leak (Amend comments)

[Yifeng Sun]

v1 -> v2:

When ovsdb_idl_table is freed, its indexes are not freed.

Valgrind report is as below:

45 (32 direct, 13 indirect) bytes in 1 blocks are definitely lost in loss 
record 65 of 83
at 0x4C2DB8F: malloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x4A6D64: xmalloc (util.c:120)
by 0x49C847: shash_add_nocopy__ (shash.c:109)
by 0x49C847: shash_add_nocopy (shash.c:121)
by 0x49CA85: shash_add (shash.c:129)
by 0x49CA85: shash_add_once (shash.c:136)
by 0x4914B5: ovsdb_idl_create_index (ovsdb-idl.c:2067)
by 0x406C98: create_ovnsb_indexes (ovn-controller.c:568)
by 0x406C98: main (ovn-controller.c:619)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC PATCH 1/2] sflow: introduce egress flow sampling

[Ben Pfaff]

It's been a long time since there's been activity in this series.  I'm
not entirely following the discussion, so I'd like some guidance on how
to proceed with it.  For now, I'm just marking it "Deferred" in
patchwork.

On Mon, Sep 04, 2017 at 09:28:45AM +, Weglicki, MichalX wrote:
> Hello Neil, 
> 
> NAT can be configured as range of addresses, so it is impossible to 
> Get this information before actual translation happens. 
> 
> In general I don't see any problem with creating egress action 
> as it works in ipfix as default configuration along ingress 
> action. When it comes to caching packet, I don't know yet exactly 
> how it should implemented, but as long as this is optional sFLOW
> feature which would enable only during specific NAT case, I don't 
> really see big impact as well. Also it gives us possible improvements 
> in the future, as there could be other cases where some of the 
> data fields can't be read on ingress. As you confirmed that it would
> work according to sFLOW specs, I really think it is worth it. 
> 
> Br, 
> Michal. 
> 
> > -Original Message-
> > From: Neil McKee [mailto:neil.mc...@inmon.com]
> > Sent: Thursday, August 31, 2017 8:10 PM
> > To: Weglicki, MichalX 
> > Cc: d...@openvswitch.org
> > Subject: Re: [ovs-dev] [RFC PATCH 1/2] sflow: introduce egress flow sampling
> > 
> > Yes.  Any solution that samples the original packet and annotates it
> > with accurate information about its forwarding will conform to the
> > spec.   But anything you do that touches the chain of actions in more
> > than one place is likely to be problematic...
> > 
> > For example,  one possible approach that OVS *might* have taken from
> > the start was to (1) mark the packet for sampling at ingress (2)
> > accumulate forwarding information in a buffer as it goes through each
> > action,  then (3) send an upcall with original-packet+forwarding-info
> > at the point when all the actions were completed.  At first glance
> > this seems clean because there is only one upcall and no awkward
> > rendez-vous,  but actually it is making assumptions that the datapath
> > is happy to buffer information at any stage,  and that it will process
> > each packet to completion.  Suppose there were a datapath
> > implementation (in hardware or software) where it made sense to
> > pipeline the datapath with batches of packets?  It's not hard to
> > imagine scenarios where those assumptions would turn into serious
> > constraints.
> > 
> > I completely agree with you that the NAT detail is a high-value
> > measurement,  but the radical simplicity of the current implementation
> > is important,  so making changes to the datapath should be weighed
> > against that.  One question is:  would it help if the sFlow feed
> > included at least the outline of the NAT translation that is known
> > from the actions-list? Or is there some other way to look up the NAT
> > translation table from user-space?  Perhaps similar to the way the
> > host-sflow agent annotates samples with TCP performance metrics here:
> > https://github.com/sflow/host-sflow/blob/v2.0.11/src/Linux/mod_tcp.c#L512-L631
> > 
> > Neil
> > 
> > --
> > Neil McKee
> > InMon Corp.
> > http://www.inmon.com
> > 
> > 
> > On Thu, Aug 31, 2017 at 5:38 AM, Weglicki, MichalX
> >  wrote:
> > > Hello Neil,
> > >
> > > The problem is that to fill NAT translation correctly through
> > > extended_nat we need sample packet before and after the translation.
> > > I understand that such information (possibly) could be analyzed by
> > > collector based on information from two switches, however I think that
> > > correctly getting this information at one point is beneficial.
> > >
> > > The approach which Przemek took is similar to ipfix, where default
> > > configuration per bridge is to have packet sampled on ingress and egress.
> > > This allows to support all the middlebox functions (e.g. NAT) that ipfix
> > > defines. I think it should also be supported in OVS-sFLOW.
> > > I wasn't aware that same packet can't be sampled on ingreess
> > > and egress, according to specification it can have to be sampled on
> > > igress OR egress - I didn't find any clear statement forbidding it,
> > > but I'm sure it is there, as you said.
> > >
> > > What if I would go into some hybrid approach, when all sampling
> > > would happen on ingress, but there would be optional egress action
> > > which fills additional counters (like extended_nat) and sent it when 
> > > needed.
> > > So the logic would be:
> > > - When ingress action is executed, all counters are calculated, if any
> > >   of the counters can't be retrieved at this point, packet is buffered, 
> > > and
> > >   marked for egress sampling with additional information what information
> > >  is missing.
> > > - Then on egress when action is executed, requested function is
> > >   calculated, filled, and then sent to collector.
> > > - In all other cases, egress 

Re: [ovs-dev] [PATCH v3 0/9] OVS-DPDK flow offload with rte_flow

[Ben Pfaff]

I see that this series appears to have stalled.  Should we expect a new
version sometime soon?  Do you need more reviews on the current version?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/2] xlate: normalize the actions after translation

[Ben Pfaff]

On Mon, Oct 30, 2017 at 12:22:50PM +0100, Zoltan Balogh wrote:
> When all OF actions have been translated, there could be actions at
> the end of list of odp actions which are not needed to be executed.
> So, the list can be normalized at the end of xlate_actions().
> 
> Signed-off-by: Zoltan Balogh 
> Signed-off-by: Sugesh Chandran 
> Co-authored-by: Sugesh Chandran 
> Tested-by: Sugesh Chandran 

Thanks for working on this!  It will be helpful in some cases.

In is_valid_last_action(), I recommend assigning nl_attr_type(nla) to a
variable of type enum ovs_action_attr, then using that for the switch
statement.  That will ensure that, as new kinds of actions are added, we
remember to add new items to the switch statement.

Here are some minor simplifications that you might consider folding in
also.  They compile, but I have not tested them.

--8<--cut here-->8--

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 5e400e091ac6..d08cfddc55cb 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -6943,7 +6943,7 @@ xlate_wc_finish(struct xlate_ctx *ctx)
 /* Returns true if the action stored in 'nla' can be a valid last action of a
  * datapath flow. */
 static bool
-is_valid_last_action(struct nlattr *nla)
+is_valid_last_action(const struct nlattr *nla)
 {
 switch (nl_attr_type(nla)) {
 case OVS_ACTION_ATTR_USERSPACE:
@@ -6965,35 +6965,27 @@ is_valid_last_action(struct nlattr *nla)
  * 'data'. Execution of actions beyond this last attribute does not make sense.
  */
 static size_t
-last_action_offset(struct nlattr *data, const size_t data_len)
+last_action_offset(const struct nlattr *data, const size_t data_len)
 {
-uint16_t left;
-struct nlattr *a, *b = NULL;
+const struct nlattr *last = data;
 
+uint16_t left;
+const struct nlattr *a;
 NL_ATTR_FOR_EACH (a, left, data, data_len) {
 if (is_valid_last_action(a)) {
-b = a;
+last = nl_attr_next(a);
 }
 }
 
-if (b) {
-return NLA_ALIGN(((char *)b - (char *)data) + b->nla_len);
-} else {
-return 0;
-}
+return (char *) last - (char *) data;
 }
 
+/* Get rid of any unneeded actions at the tail end. */
 static void
 normalize_odp_actions(struct xlate_ctx *ctx)
 {
-struct nlattr *data = ctx->odp_actions->data;
-size_t size = ctx->odp_actions->size;
-size_t new_size = last_action_offset(data, size);
-
-/* Get rid of any unneeded actions at the tail end. */
-if (OVS_UNLIKELY(new_size != size)) {
-ctx->odp_actions->size = new_size;
-}
+struct ofpbuf *oa = ctx->odp_actions;
+oa->size = last_action_offset(oa->data, oa->size);
 }
 
 /* Translates the flow, actions, or rule in 'xin' into datapath actions in
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 2/3] test-ovsdb: Fix memory leak

[Yifeng Sun]

Reported by `make check-valgrind`.
This patch was tested by `make check` and `make check-valgrind`.

Signed-off-by: Yifeng Sun 
---
 tests/test-ovsdb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index 451172cdcc34..b5147fc055e3 100644
--- a/tests/test-ovsdb.c
+++ b/tests/test-ovsdb.c
@@ -582,6 +582,7 @@ do_parse_atom_strings(struct ovs_cmdl_context *ctx)
 ovsdb_atom_destroy(, base.type);
 if (range_end_atom) {
 ovsdb_atom_destroy(range_end_atom, base.type);
+   free(range_end_atom);
 }
 }
 ovsdb_base_type_destroy();
-- 
2.7.4

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

[ovs-dev] [PATCH 1/3] ovsdb-idl: Fix memory leak

[Yifeng Sun]

Reported by `make check-valgrind`.
This patch was tested by `make check` and `make check-valgrind`.

Signed-off-by: Yifeng Sun 
---
 lib/ovsdb-idl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 5617e08d633c..be29c92957c0 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -2163,6 +2163,7 @@ ovsdb_idl_destroy_indexes(struct ovsdb_idl_table *table)
 skiplist_destroy(index->skiplist, NULL);
 free(index->columns);
 }
+shash_destroy_free_data(>indexes);
 }
 
 static void
-- 
2.7.4

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

Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: use xlate error enum for unsupported packet type

[Ben Pfaff]

On Mon, Aug 21, 2017 at 08:34:41AM +, Zoltán Balogh wrote:
> Instead of using the value 1 a new enum should be used for indicating
> translation error which occurs because of unsupported packet type.
> 
> Signed-off-by: Zoltan Balogh 
> Signed-off-by: Jan Scheurich 
> Co-authored-by: Jan Scheurich 
> Fixes: f839892a206a ("OF support and translation of generic encap and
> decap")
> CC: Jan Scheurich 

Thanks, applied to master and branch-2.8.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH net-next v14] openvswitch: enable NSH support

[Jiri Benc]

On Mon, 30 Oct 2017 09:29:34 +0800, Yi Yang wrote:
> +static int set_nsh(struct sk_buff *skb, struct sw_flow_key *flow_key,
> +const struct nlattr *a)
> +{
> + struct nshhdr *nh;
> + size_t length;
> + int err;
> + u8 flags;
> + u8 ttl;
> + int i;
> +
> + struct ovs_key_nsh key;
> + struct ovs_key_nsh mask;
> +
> + err = nsh_key_from_nlattr(a, , );
> + if (err)
> + return err;
> +
> + /* Make sure the NSH base header is there */
> + if (!pskb_may_pull(skb, NSH_BASE_HDR_LEN))

This should be skb_network_offset(skb) + NSH_BASE_HDR_LEN.

> +size_t ovs_nsh_key_attr_size(void)
> +{
> + /* Whenever adding new OVS_NSH_KEY_ FIELDS, we should consider
> +  * updating this function.
> +  */
> + return  nla_total_size(NSH_BASE_HDR_LEN) /* OVS_NSH_KEY_ATTR_BASE */
> + /* OVS_NSH_KEY_ATTR_MD1 and OVS_NSH_KEY_ATTR_MD2 are
> +  * mutually exclusive, so the bigger one can cover
> +  * the small one.
> +  *
> +  * OVS_NSH_KEY_ATTR_MD2
> +  */

A nit, not important but since you'll need to respin anyway: the last
line in the comment above seems to be a left over from some previous
version of the comment. This should be enough:

/* OVS_NSH_KEY_ATTR_MD1 and OVS_NSH_KEY_ATTR_MD2 are
 * mutually exclusive, so the bigger one can cover
 * the small one.
 */

Or maybe I misunderstood what you meant.

> +int nsh_hdr_from_nlattr(const struct nlattr *attr,
> + struct nshhdr *nh, size_t size)
> +{
> + struct nlattr *a;
> + int rem;
> + u8 flags = 0;
> + u8 ttl = 0;
> + int mdlen = 0;
> +
> + /* validate_nsh has check this, so we needn't do duplicate check here
> +  */
> + nla_for_each_nested(a, attr, rem) {
> + int type = nla_type(a);
> +
> + switch (type) {
> + case OVS_NSH_KEY_ATTR_BASE: {
> + const struct ovs_nsh_key_base *base = nla_data(a);
> +
> + flags = base->flags;
> + ttl = base->ttl;
> + nh->np = base->np;
> + nh->mdtype = base->mdtype;
> + nh->path_hdr = base->path_hdr;
> + break;
> + }
> + case OVS_NSH_KEY_ATTR_MD1:
> + mdlen = nla_len(a);
> + memcpy(>md1, nla_data(a), mdlen);

The check for 'size' disappeared from here somehow.

> + break;
> +
> + case OVS_NSH_KEY_ATTR_MD2:
> + mdlen = nla_len(a);
> + memcpy(>md2, nla_data(a), mdlen);

And here.

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

Re: [ovs-dev] [PATCH] rhel.rst: Add python-sphinx as a dependency.

[Aaron Conole]

Ben Pfaff  writes:

> On Tue, Oct 31, 2017 at 03:47:35PM -0400, Aaron Conole wrote:
>> Ben Pfaff  writes:
>> 
>> > On Fri, Oct 20, 2017 at 12:39:10AM -0700, Gurucharan Shetty wrote:
>> >> Signed-off-by: Gurucharan Shetty 
>> >> ---
>> >>  Documentation/intro/install/rhel.rst | 2 +-
>> >>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >> 
>> >> diff --git a/Documentation/intro/install/rhel.rst 
>> >> b/Documentation/intro/install/rhel.rst
>> >> index 86c5cf3..aff6ccf 100644
>> >> --- a/Documentation/intro/install/rhel.rst
>> >> +++ b/Documentation/intro/install/rhel.rst
>> >> @@ -76,7 +76,7 @@ the below command::
>> >>  
>> >>  $ yum install gcc make python-devel openssl-devel kernel-devel 
>> >> graphviz \
>> >>  kernel-debug-devel autoconf automake rpm-build redhat-rpm-config 
>> >> \
>> >> -libtool checkpolicy selinux-policy-devel
>> >> +libtool checkpolicy selinux-policy-devel python-sphinx
>> >
>> > For Debian, we just recommend installing the build-dependencies listed
>> > in debian/control.  That has the advantage that it can't get out of
>> > date.  It has the disadvantage, though, that it's not easy to cut and
>> > paste (although "apt-get build-dep openvswitch" usually does the trick).
>> > Maybe "yum" has some mode that installs dependencies from a spec file?
>> 
>> For 'yum' distributions:
>> 
>>   yum-builddep
>> 
>> For 'dnf' distributions (newer Fedora, and future RHEL versions):
>> 
>>   dnf builddep
>
> Would it be reasonable to change rhel.rst to recommend using one of
> those tools, to ease future maintenance?

Sure.  Something like below?  I don't know about the wordsmithing, so
I'll defer that to Guru.

---

diff --git a/Documentation/intro/install/rhel.rst b/Documentation/intro/install/
rhel.rst
index 86c5cf3..36bb661 100644
--- a/Documentation/intro/install/rhel.rst
+++ b/Documentation/intro/install/rhel.rst
@@ -72,11 +72,14 @@ Build Requirements
 
 To compile the RPMs, you will need to install the packages described in the
 :doc:`general` along with some additional packages. These can be installed with
-the below command::
+the below command for ``yum`` based distributions (but note that the
+openvswitch source RPM must be available somewhere)::
 
-$ yum install gcc make python-devel openssl-devel kernel-devel graphviz \
-kernel-debug-devel autoconf automake rpm-build redhat-rpm-config \
-libtool checkpolicy selinux-policy-devel
+$ yum-builddep openvswitch
+
+For ``dnf`` based distributions, use the following command::
+
+$ dnf builddep rhel/openvswitch-fedora.spec
 
 .. _rhel-bootstrapping:
--
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH net-next v14] openvswitch: enable NSH support

[Jiri Benc]

On Tue, 31 Oct 2017 15:57:41 -0400, Eric Garver wrote:
> On Mon, Oct 30, 2017 at 09:29:34AM +0800, Yi Yang wrote:
> > +   if (WARN_ON(is_push_nsh && is_mask))
> > +   return -EINVAL;
> 
> OVS_NLERR() is probably more appropriate.

No, not here. If this happens, it's a bug in the kernel and WARN_ON is
what we need. This is not triggerable from user space and user space
has no way to fix it if this happens.

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

Re: [ovs-dev] [PATCH] rhel.rst: Add python-sphinx as a dependency.

[Ben Pfaff]

On Tue, Oct 31, 2017 at 03:47:35PM -0400, Aaron Conole wrote:
> Ben Pfaff  writes:
> 
> > On Fri, Oct 20, 2017 at 12:39:10AM -0700, Gurucharan Shetty wrote:
> >> Signed-off-by: Gurucharan Shetty 
> >> ---
> >>  Documentation/intro/install/rhel.rst | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> diff --git a/Documentation/intro/install/rhel.rst 
> >> b/Documentation/intro/install/rhel.rst
> >> index 86c5cf3..aff6ccf 100644
> >> --- a/Documentation/intro/install/rhel.rst
> >> +++ b/Documentation/intro/install/rhel.rst
> >> @@ -76,7 +76,7 @@ the below command::
> >>  
> >>  $ yum install gcc make python-devel openssl-devel kernel-devel 
> >> graphviz \
> >>  kernel-debug-devel autoconf automake rpm-build redhat-rpm-config \
> >> -libtool checkpolicy selinux-policy-devel
> >> +libtool checkpolicy selinux-policy-devel python-sphinx
> >
> > For Debian, we just recommend installing the build-dependencies listed
> > in debian/control.  That has the advantage that it can't get out of
> > date.  It has the disadvantage, though, that it's not easy to cut and
> > paste (although "apt-get build-dep openvswitch" usually does the trick).
> > Maybe "yum" has some mode that installs dependencies from a spec file?
> 
> For 'yum' distributions:
> 
>   yum-builddep
> 
> For 'dnf' distributions (newer Fedora, and future RHEL versions):
> 
>   dnf builddep

Would it be reasonable to change rhel.rst to recommend using one of
those tools, to ease future maintenance?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH net-next v14] openvswitch: enable NSH support

[Eric Garver]

On Mon, Oct 30, 2017 at 09:29:34AM +0800, Yi Yang wrote:
[...]
> +int nsh_pop(struct sk_buff *skb)
> +{
> + struct nshhdr *nh;
> + size_t length;
> + __be16 inner_proto;
> +
> + if (!pskb_may_pull(skb, NSH_BASE_HDR_LEN))
> + return -ENOMEM;
> + nh = (struct nshhdr *)(skb->data);
> + length = nsh_hdr_len(nh);
> + if (!pskb_may_pull(skb, length))
> + return -ENOMEM;
> +
> + nh = (struct nshhdr *)(skb->data);
> + inner_proto = tun_p_to_eth_p(nh->np);

If you fetch inner_proto before the second pskb_may_pull then there is
no need to reload the nh pointer as you won't use it later.

> + if (!inner_proto)
> + return -EAFNOSUPPORT;
> +
> + length = nsh_hdr_len(nh);

You already have the length from above. No need to get it again.

> + skb_pull(skb, length);
> + skb_reset_mac_header(skb);
> + skb_reset_network_header(skb);
> + skb_reset_mac_len(skb);
> + skb->protocol = inner_proto;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(nsh_pop);
[...]
> +static int nsh_key_put_from_nlattr(const struct nlattr *attr,
> +struct sw_flow_match *match, bool is_mask,
> +bool is_push_nsh, bool log)
> +{
> + struct nlattr *a;
> + int rem;
> + bool has_base = false;
> + bool has_md1 = false;
> + bool has_md2 = false;
> + u8 mdtype = 0;
> + int mdlen = 0;
> +
> + if (WARN_ON(is_push_nsh && is_mask))
> + return -EINVAL;

OVS_NLERR() is probably more appropriate.

> +
> + nla_for_each_nested(a, attr, rem) {
> + int type = nla_type(a);
> + int i;
> +
[...]
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v1 1/1] Build the JSON C extension for the Python lib

[Ben Pfaff]

On Thu, Aug 17, 2017 at 02:14:13PM -0500, Terry Wilson wrote:
> The JSON C extensions performs much better than the pure Python
> version, so build it when producing RPMs.
> 
> Signed-off-by: Terry Wilson 

Hi Russell, would you mind taking a look at this?  It is Pythonic and
touches only the RHEL directory, so I don't feel entirely qualified to
review it.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/2] lib/netlink: Use correct netlink max message size

[Ben Pfaff]

On Fri, Sep 22, 2017 at 07:44:53AM -0700, Greg Rose wrote:
> The maximum message size for recent Linux kernels is 32Kb and in older
> kernels it is 16KB.
> 
> See http://www.spinics.net/lists/netdev/msg431592.html
> 
> Adjust the size checked and update a comment.
> 
> Signed-off-by: Greg Rose 

...

> diff --git a/lib/netlink.c b/lib/netlink.c
> index de3ebcd..04310ff 100644
> --- a/lib/netlink.c
> +++ b/lib/netlink.c
> @@ -570,7 +570,7 @@ nl_msg_next(struct ofpbuf *buffer, struct ofpbuf *msg)
>  bool
>  nl_attr_oversized(size_t payload_size)
>  {
> -return payload_size > UINT16_MAX - NLA_HDRLEN;
> +return payload_size > INT16_MAX - NLA_HDRLEN;
>  }

Thanks for the patch!

I am confused by a difference between the commit message and the code.
Before this patch, nl_attr_oversized() considered an attribute of about
64 kB to be oversize; after this patch, about 32 kB.  Shouldn't the new
value be about 16 kB?

Thanks,

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

Re: [ovs-dev] [PATCH 1/2] datapath: Check maximum netlink message size

[Ben Pfaff]

On Fri, Sep 22, 2017 at 07:44:52AM -0700, Greg Rose wrote:
> In kernels < 4.9 the maximum netlink message size is 16KB.
> 
> See http://www.spinics.net/lists/netdev/msg431592.html
> 
> Signed-off-by: Greg Rose 

Thanks, applied to master, branch-2.8, and branch-2.7.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Comunicación y negociación eficaz

[Cobranza Altamente Eficaces]

Las mejores técnicas para la recuperación de su cartera vencida

Habilidades de cobranza altamente eficaces
10 de noviembre - MCE. Abdón Guzmán Santana Zárate9am-6pm

El arte de la cobranza requiere de técnicas de negociación y habilidades para 
la interacción y el trato humano. El éxito de esta actividad, depende en gran 
parte de las habilidades y destrezas alcanzadas en el campo de la negociación. 
Un ejecutivo de cobranza exitoso, debe saber llevar un diálogo con el deudor y 
atraer el pago, a través del uso de palabras adecuadas sin llegar a las 
amenazas, buscando un acuerdo entre la empresa y los deudores.

BENEFICIOS DE ASISTIR: 

- Aprenderá a manejar excusas, mentiras y quejas de los deudores, así como a 
clientes difíciles. - Conocerá cuáles son los límites que marca la ley en 
relación a la cobranza. - Sabrá como tomar el control de la llamada telefónica 
con deudores que intentan desviar la conversación. - Identificará cuándo y cómo 
es posible llevar un proceso judicial con su cartera vencida. 

¿Requiere la información a la Brevedad? responda este email con la palabra: 
Cobranza + nombre - teléfono - correo.


centro telefónico:018002120744


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

Re: [ovs-dev] [PATCH 2/2] lib: Move lib/poll-loop.h to include/openvswitch

[Ben Pfaff]

On Thu, Aug 17, 2017 at 12:06:25AM +0800, Xiao Liang wrote:
> Poll-loop is the core to implement main loop. It should be available in
> libopenvswitch.
> 
> Signed-off-by: Xiao Liang 

I'm concerned about the way that this adds a definition of HANDLE in a
public header.  That seems unfriendly to code that might want to include
both this header and Win32 headers that properly define HANDLE.

Alin, what's the right thing to do here?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] datapath-windows: Update OvsCompleteNbl argument to match definition

[aserdean]

Thanks Shashank and Sai. I pushed this on master.

Alin.

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Shashank Ram
> Sent: Wednesday, October 25, 2017 10:53 PM
> To: Sairam Venugopal ; d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH] datapath-windows: Update OvsCompleteNbl
> argument to match definition
> 
> 
> Update the OvsCompleteNbl to take in a PVOID and explicitly cast to
> POVS_SWITCH_CONTEXT. This is useful when finding declarations in Visual
> Studio. The mismatch breaks this functionality.
> 
> Found by inspection.
> 
> Signed-off-by: Sairam Venugopal 
> ---
> 
> Acked-by: Shashank Ram 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

Re: [ovs-dev] [PATCH 2/2] ipfix: Update Timestamp when flow updated

[Ben Pfaff]

On Tue, Oct 31, 2017 at 10:08:08AM -0700, Greg Rose wrote:
> On 10/24/2017 03:19 PM, Ben Pfaff wrote:
> >On Tue, Jun 06, 2017 at 01:42:08PM -0700, Greg Rose wrote:
> >>On 06/06/2017 08:22 AM, Ben Pfaff wrote:
> >>>On Mon, Jun 05, 2017 at 08:34:32PM -0700, Greg Rose wrote:
> On 06/05/2017 06:04 PM, Ben Pfaff wrote:
> >From: Greg Rose 
> >
> >Reported-by: Felix Konstantin Maurer 
> >Signed-off-by: Greg Rose 
> >[b...@ovn.org changed this to use ipfix_now()]
> >Signed-off-by: Ben Pfaff 
> >---
> >   ofproto/ofproto-dpif-ipfix.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> >diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c
> >index 5589b0ea05e1..bc63b7b0294b 100644
> >--- a/ofproto/ofproto-dpif-ipfix.c
> >+++ b/ofproto/ofproto-dpif-ipfix.c
> >@@ -1643,6 +1643,7 @@ ipfix_cache_update(struct dpif_ipfix_exporter 
> >*exporter,
> >   ipfix_cache_aggregate_entries(entry, old_entry);
> >   free(entry);
> >   ipfix_update_stats(exporter, false, current_flows, 
> > sampled_pkt_type);
> >+old_entry->flow_end_timestamp_usec = ipfix_now();
> >   }
> >   }
> >
> >
> Looks good, thanks Ben!
> >>>
> >>>Thanks for the review!
> >>>
> >>>If I recall correctly, Felix reported that your original patch didn't
> >>>help, though, so probably this one doesn't either.  We should track that
> >>>down before we go farther, I guess.
> >>>
> >>Yes, I'm am following up.  Having all sorts of problems getting a working 
> >>ipfix flow collector to work though.  The ManageEngine netflow collector 
> >>claims to work with ipfix but SFAICT it does not so I'm not going to waste 
> >>more time on it.  Once I can start
> >>collecting and analyzing the work should proceed quickly.
> >
> >I never applied this patch (from June) because we were continuing to
> >look deeper.  I don't know whether that ever bore fruit.  Should I apply
> >this patch now?
> >
> >Thanks,
> >
> >Ben.
> >
> 
> I think this one got lost.  I never followed up on it and got busy with other 
> more pressing issues.

Should it get revived?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 6/6] installer-windows: Add x64 installer build via command line

[Alin Gabriel Serdean]

Add a new variable to know on which platform we are compiling.

Make the msbuild command to be aware of the platform we want to build.

Shorter the msbuild parameters from `property:`->`p:`. Change slashes
to double slashes so msys does not get confused.

Signed-off-by: Alin Gabriel Serdean 
---
 Makefile.am | 1 +
 m4/openvswitch.m4   | 3 +++
 windows/automake.mk | 2 +-
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index ebbc045..5d19f08 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -20,6 +20,7 @@ AM_CPPFLAGS += $(PTHREAD_INCLUDES)
 AM_CPPFLAGS += $(MSVC_CFLAGS)
 AM_LDFLAGS += $(PTHREAD_LDFLAGS)
 AM_LDFLAGS += $(MSVC64_LDFLAGS)
+PLATFORM = $(MSVC_PLATFORM)
 endif
 
 AM_CPPFLAGS += -I $(top_srcdir)/include
diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4
index 59e1352..5b13baa 100644
--- a/m4/openvswitch.m4
+++ b/m4/openvswitch.m4
@@ -79,11 +79,14 @@ AC_DEFUN([OVS_CHECK_WIN64],
  if (cl) 2>&1 | grep 'x64' >/dev/null 2>&1; then
cl_cv_x64=yes
MSVC64_LDFLAGS=" /MACHINE:X64 "
+   MSVC_PLATFORM="x64"
  else
cl_cv_x64=no
MSVC64_LDFLAGS=""
+   MSVC_PLATFORM="x86"
  fi])
  AC_SUBST([MSVC64_LDFLAGS])
+ AC_SUBST([MSVC_PLATFORM])
 ])
 
 dnl Checks for WINDOWS.
diff --git a/windows/automake.mk b/windows/automake.mk
index 11ab4c7..80dca14 100644
--- a/windows/automake.mk
+++ b/windows/automake.mk
@@ -35,7 +35,7 @@ windows_installer: all
cp -f 
$(top_srcdir)/datapath-windows/x64/Win8.1$(VSTUDIO_CONFIG)/package/ovsext.cat 
windows/ovs-windows-installer/Driver/Win8.1/ovsext.cat
cp -f 
$(top_srcdir)/datapath-windows/x64/Win8.1$(VSTUDIO_CONFIG)/package/ovsext.inf 
windows/ovs-windows-installer/Driver/Win8.1/ovsext.inf
cp -f 
$(top_srcdir)/datapath-windows/x64/Win8.1$(VSTUDIO_CONFIG)/package/ovsext.sys 
windows/ovs-windows-installer/Driver/Win8.1/ovsext.sys
-   MSBuild.exe windows/ovs-windows-installer.sln //nologo /target:Build 
/property:Configuration="Release" /property:Version="$(PACKAGE_VERSION)"
+   MSBuild.exe windows/ovs-windows-installer.sln //nologo //target:Build 
//p:Configuration="Release" //p:Version="$(PACKAGE_VERSION)" 
//p:Platform=$(PLATFORM)
 
 EXTRA_DIST += \
windows/automake.mk \
-- 
2.10.2.windows.1

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

[ovs-dev] [PATCH 4/6] installer-windows: Call WIX binaries outside of MSBuild on x64

[Alin Gabriel Serdean]

Unfortunately all WIX binaries (candle, heat, etc) are only 32 bit (up to
the latest version 3.11).

For performance reasons they are run as .NET assemblies inside the MSBuild
process. Running 32 bit assemblies inside a 64 bit process (MSBuild) makes
them segfault.

Add a new option for heat to be run as an individual process when the
platform is not x86.

Signed-off-by: Alin Gabriel Serdean 
---
 windows/ovs-windows-installer/ovs-windows-installer.wixproj | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/windows/ovs-windows-installer/ovs-windows-installer.wixproj 
b/windows/ovs-windows-installer/ovs-windows-installer.wixproj
index a8256ed..241d605 100644
--- a/windows/ovs-windows-installer/ovs-windows-installer.wixproj
+++ b/windows/ovs-windows-installer/ovs-windows-installer.wixproj
@@ -13,6 +13,7 @@
 1.0.0.0
   
   
+true
 bin\$(Configuration)\
 obj\$(Configuration)\
 
BinariesPath=Binaries;SymbolsPath=Symbols;Version=$(Version)
@@ -21,6 +22,7 @@
 1076;
   
   
+true
 
BinariesPath=Binaries;SymbolsPath=Symbols;Version=$(Version)
 False
 False
@@ -67,9 +69,9 @@
   
   
   
-
+
 
-
+
 
   
   

[ovs-dev] [PATCH 3/6] installer-windows: Resolve WIX solution build type

[Alin Gabriel Serdean]

Until now the x64 build of the installer solution was pointing to the
x86 build of the WIX project.

This patch changes for them to match.

Signed-off-by: Alin Gabriel Serdean 
---
 windows/ovs-windows-installer.sln | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/windows/ovs-windows-installer.sln 
b/windows/ovs-windows-installer.sln
index 09311f9..f563438 100644
--- a/windows/ovs-windows-installer.sln
+++ b/windows/ovs-windows-installer.sln
@@ -10,8 +10,8 @@ Global
Release|x86 = Release|x86
EndGlobalSection
GlobalSection(ProjectConfigurationPlatforms) = postSolution
-   {259905A2-7434-4190-8A33-8FBA67171DD6}.Release|x64.ActiveCfg = 
Release|x86
-   {259905A2-7434-4190-8A33-8FBA67171DD6}.Release|x64.Build.0 = 
Release|x86
+   {259905A2-7434-4190-8A33-8FBA67171DD6}.Release|x64.ActiveCfg = 
Release|x64
+   {259905A2-7434-4190-8A33-8FBA67171DD6}.Release|x64.Build.0 = 
Release|x64
{259905A2-7434-4190-8A33-8FBA67171DD6}.Release|x86.ActiveCfg = 
Release|x86
{259905A2-7434-4190-8A33-8FBA67171DD6}.Release|x86.Build.0 = 
Release|x86
EndGlobalSection
-- 
2.10.2.windows.1

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

[ovs-dev] [PATCH 2/6] installer-windows: Remove unused entries from WIX project

[Alin Gabriel Serdean]

Remove duplicate and obsolete entries from the installer WIX project.

Found by inspection.

Signed-off-by: Alin Gabriel Serdean 
---
 .../ovs-windows-installer.wixproj  | 36 --
 1 file changed, 36 deletions(-)

diff --git a/windows/ovs-windows-installer/ovs-windows-installer.wixproj 
b/windows/ovs-windows-installer/ovs-windows-installer.wixproj
index f0e8f50..a8256ed 100644
--- a/windows/ovs-windows-installer/ovs-windows-installer.wixproj
+++ b/windows/ovs-windows-installer/ovs-windows-installer.wixproj
@@ -12,11 +12,6 @@
 $(MSBuildExtensionsPath)\Microsoft\WiX\v3.x\Wix.targets
 1.0.0.0
   
-  
-bin\$(Configuration)\
-obj\$(Configuration)\
-Debug;Version=$(Version)
-  
   
 bin\$(Configuration)\
 obj\$(Configuration)\
@@ -25,37 +20,6 @@
 False
 1076;
   
-  
-Debug
-bin\$(Platform)\$(Configuration)\
-
obj\$(Platform)\$(Configuration)\
-  
-  
-
BinariesPath=Binaries;SymbolsPath=Symbols;Version=$(Version)
-False
-False
-1076;
-bin\$(Platform)\$(Configuration)\
-
obj\$(Platform)\$(Configuration)\
-  
-  
-Debug;Version=$(Version)
-bin\$(Platform)\$(Configuration)\
-
obj\$(Platform)\$(Configuration)\
-  
-  
-
BinariesPath=Binaries;SymbolsPath=Symbols;Version=$(Version)
-False
-False
-1076;
-bin\$(Platform)\$(Configuration)\
-
obj\$(Platform)\$(Configuration)\
-  
-  
-Debug;Version=$(Version)
-bin\$(Platform)\$(Configuration)\
-
obj\$(Platform)\$(Configuration)\
-  
   
 
BinariesPath=Binaries;SymbolsPath=Symbols;Version=$(Version)
 False
-- 
2.10.2.windows.1

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

[ovs-dev] [PATCH 0/6] installer fixes on msbuild64

[Alin Gabriel Serdean]

Installer fixes to allow the MSI build using msbuild(64bit variant).

Alin Gabriel Serdean (6):
  build-windows: Suppress output from MSBuild
  installer-windows: Remove unused entries from WIX project
  installer-windows: Resolve WIX solution build type
  installer-windows: Call WIX binaries outside of MSBuild on x64
  installer-windows: Modify installer so it can be compiled on x64
  installer-windows: Add x64 installer build via command line

 Makefile.am|  9 ++---
 datapath-windows/automake.mk   |  4 +--
 m4/openvswitch.m4  |  3 ++
 windows/automake.mk|  2 +-
 windows/ovs-windows-installer.sln  |  4 +--
 windows/ovs-windows-installer/Product.wxs  | 12 ++-
 .../ovs-windows-installer.wixproj  | 42 +++---
 7 files changed, 28 insertions(+), 48 deletions(-)

-- 
2.10.2.windows.1

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

Re: [ovs-dev] [PATCH 2/2] ipfix: Update Timestamp when flow updated

[Greg Rose]

On 10/24/2017 03:19 PM, Ben Pfaff wrote:

On Tue, Jun 06, 2017 at 01:42:08PM -0700, Greg Rose wrote:

On 06/06/2017 08:22 AM, Ben Pfaff wrote:

On Mon, Jun 05, 2017 at 08:34:32PM -0700, Greg Rose wrote:

On 06/05/2017 06:04 PM, Ben Pfaff wrote:

From: Greg Rose 

Reported-by: Felix Konstantin Maurer 
Signed-off-by: Greg Rose 
[b...@ovn.org changed this to use ipfix_now()]
Signed-off-by: Ben Pfaff 
---
   ofproto/ofproto-dpif-ipfix.c | 1 +
   1 file changed, 1 insertion(+)

diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c
index 5589b0ea05e1..bc63b7b0294b 100644
--- a/ofproto/ofproto-dpif-ipfix.c
+++ b/ofproto/ofproto-dpif-ipfix.c
@@ -1643,6 +1643,7 @@ ipfix_cache_update(struct dpif_ipfix_exporter *exporter,
   ipfix_cache_aggregate_entries(entry, old_entry);
   free(entry);
   ipfix_update_stats(exporter, false, current_flows, sampled_pkt_type);
+old_entry->flow_end_timestamp_usec = ipfix_now();
   }
   }



Looks good, thanks Ben!


Thanks for the review!

If I recall correctly, Felix reported that your original patch didn't
help, though, so probably this one doesn't either.  We should track that
down before we go farther, I guess.


Yes, I'm am following up.  Having all sorts of problems getting a working ipfix 
flow collector to work though.  The ManageEngine netflow collector claims to 
work with ipfix but SFAICT it does not so I'm not going to waste more time on 
it.  Once I can start
collecting and analyzing the work should proceed quickly.


I never applied this patch (from June) because we were continuing to
look deeper.  I don't know whether that ever bore fruit.  Should I apply
this patch now?

Thanks,

Ben.



I think this one got lost.  I never followed up on it and got busy with other 
more pressing issues.

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

Re: [ovs-dev] [PATCH 3/2] vswitchd: Document netdev-dpdk commands.

[Fischetti, Antonio]

Thanks, LGTM.
-Antonio

Acked-by: Antonio Fischetti 

> -Original Message-
> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
> Sent: Tuesday, October 31, 2017 3:18 PM
> To: ovs-dev@openvswitch.org
> Cc: Heetae Ahn ; Fischetti, Antonio
> ; Loftus, Ciara ;
> Kavanagh, Mark B ; Stokes, Ian
> ; Wojciechowicz, RobertX
> ; Ilya Maximets 
> Subject: [PATCH 3/2] vswitchd: Document netdev-dpdk commands.
> 
> Signed-off-by: Ilya Maximets 
> ---
>  NEWS|  3 +++
>  lib/automake.mk |  1 +
>  lib/netdev-dpdk-unixctl.man | 13 +
>  manpages.mk |  2 ++
>  vswitchd/ovs-vswitchd.8.in  |  1 +
>  5 files changed, 20 insertions(+)
>  create mode 100644 lib/netdev-dpdk-unixctl.man
> 
> diff --git a/NEWS b/NEWS
> index 1325d31..6c09d71 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -5,6 +5,9 @@ Post-v2.8.0
> chassis "hostname" in addition to a chassis "name".
> - Linux kernel 4.13
>   * Add support for compiling OVS with the latest Linux 4.13 kernel
> +   - DPDK:
> + * New debug appctl command 'netdev-dpdk/get-mempool-info'.
> + * All the netdev-dpdk appctl commands described in ovs-vswitchd man 
> page.
> 
>  v2.8.0 - 31 Aug 2017
>  
> diff --git a/lib/automake.mk b/lib/automake.mk
> index ca1cf5d..f6a82d5 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -467,6 +467,7 @@ MAN_FRAGMENTS += \
>   lib/db-ctl-base.man \
>   lib/dpctl.man \
>   lib/memory-unixctl.man \
> + lib/netdev-dpdk-unixctl.man \
>   lib/ofp-version.man \
>   lib/ovs.tmac \
>   lib/service.man \
> diff --git a/lib/netdev-dpdk-unixctl.man b/lib/netdev-dpdk-unixctl.man
> new file mode 100644
> index 000..73b2e10
> --- /dev/null
> +++ b/lib/netdev-dpdk-unixctl.man
> @@ -0,0 +1,13 @@
> +.SS "NETDEV-DPDK COMMANDS"
> +These commands manage DPDK related ports (\fItype=dpdk*\fR).
> +.IP "\fBnetdev-dpdk/set-admin-state\fR [\fIinterface\fR] \fIstate\fR"
> +Sets admin state for DPDK \fIinterface\fR (or all interfaces if none is 
> given)
> +to \fIstate\fR.  \fIstate\fR can be "up" or "down".
> +.IP "\fBnetdev-dpdk/detach\fR \fIpci-address\fR"
> +Detaches device with corresponding \fIpci-address\fR from DPDK.  This command
> +can be used to detach device if it wasn't detached automatically after port
> +deletion. Refer to the documentation for details and instructions.
> +.IP "\fBnetdev-dpdk/get-mempool-info\fR [\fIinterface\fR]"
> +Prints the debug information about memory pool used by DPDK \fIinterface\fR.
> +If called without arguments, information of all the available mempools will
> +be printed.
> diff --git a/manpages.mk b/manpages.mk
> index d610d88..c89bc45 100644
> --- a/manpages.mk
> +++ b/manpages.mk
> @@ -279,6 +279,7 @@ vswitchd/ovs-vswitchd.8: \
>   lib/daemon.man \
>   lib/dpctl.man \
>   lib/memory-unixctl.man \
> + lib/netdev-dpdk-unixctl.man \
>   lib/service.man \
>   lib/ssl-bootstrap.man \
>   lib/ssl.man \
> @@ -296,6 +297,7 @@ lib/coverage-unixctl.man:
>  lib/daemon.man:
>  lib/dpctl.man:
>  lib/memory-unixctl.man:
> +lib/netdev-dpdk-unixctl.man:
>  lib/service.man:
>  lib/ssl-bootstrap.man:
>  lib/ssl.man:
> diff --git a/vswitchd/ovs-vswitchd.8.in b/vswitchd/ovs-vswitchd.8.in
> index c18baf6..76ccfcb 100644
> --- a/vswitchd/ovs-vswitchd.8.in
> +++ b/vswitchd/ovs-vswitchd.8.in
> @@ -283,6 +283,7 @@ port names, which this thread polls.
>  .IP "\fBdpif-netdev/pmd-rxq-rebalance\fR [\fIdp\fR]"
>  Reassigns rxqs to pmds in the datapath \fIdp\fR based on their current usage.
>  .
> +.so lib/netdev-dpdk-unixctl.man
>  .so ofproto/ofproto-dpif-unixctl.man
>  .so ofproto/ofproto-unixctl.man
>  .so lib/vlog-unixctl.man
> --
> 2.7.4

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

[ovs-dev] [PATCH 3/2] vswitchd: Document netdev-dpdk commands.

[Ilya Maximets]

Signed-off-by: Ilya Maximets 
---
 NEWS|  3 +++
 lib/automake.mk |  1 +
 lib/netdev-dpdk-unixctl.man | 13 +
 manpages.mk |  2 ++
 vswitchd/ovs-vswitchd.8.in  |  1 +
 5 files changed, 20 insertions(+)
 create mode 100644 lib/netdev-dpdk-unixctl.man

diff --git a/NEWS b/NEWS
index 1325d31..6c09d71 100644
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,9 @@ Post-v2.8.0
chassis "hostname" in addition to a chassis "name".
- Linux kernel 4.13
  * Add support for compiling OVS with the latest Linux 4.13 kernel
+   - DPDK:
+ * New debug appctl command 'netdev-dpdk/get-mempool-info'.
+ * All the netdev-dpdk appctl commands described in ovs-vswitchd man page.
 
 v2.8.0 - 31 Aug 2017
 
diff --git a/lib/automake.mk b/lib/automake.mk
index ca1cf5d..f6a82d5 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -467,6 +467,7 @@ MAN_FRAGMENTS += \
lib/db-ctl-base.man \
lib/dpctl.man \
lib/memory-unixctl.man \
+   lib/netdev-dpdk-unixctl.man \
lib/ofp-version.man \
lib/ovs.tmac \
lib/service.man \
diff --git a/lib/netdev-dpdk-unixctl.man b/lib/netdev-dpdk-unixctl.man
new file mode 100644
index 000..73b2e10
--- /dev/null
+++ b/lib/netdev-dpdk-unixctl.man
@@ -0,0 +1,13 @@
+.SS "NETDEV-DPDK COMMANDS"
+These commands manage DPDK related ports (\fItype=dpdk*\fR).
+.IP "\fBnetdev-dpdk/set-admin-state\fR [\fIinterface\fR] \fIstate\fR"
+Sets admin state for DPDK \fIinterface\fR (or all interfaces if none is given)
+to \fIstate\fR.  \fIstate\fR can be "up" or "down".
+.IP "\fBnetdev-dpdk/detach\fR \fIpci-address\fR"
+Detaches device with corresponding \fIpci-address\fR from DPDK.  This command
+can be used to detach device if it wasn't detached automatically after port
+deletion. Refer to the documentation for details and instructions.
+.IP "\fBnetdev-dpdk/get-mempool-info\fR [\fIinterface\fR]"
+Prints the debug information about memory pool used by DPDK \fIinterface\fR.
+If called without arguments, information of all the available mempools will
+be printed.
diff --git a/manpages.mk b/manpages.mk
index d610d88..c89bc45 100644
--- a/manpages.mk
+++ b/manpages.mk
@@ -279,6 +279,7 @@ vswitchd/ovs-vswitchd.8: \
lib/daemon.man \
lib/dpctl.man \
lib/memory-unixctl.man \
+   lib/netdev-dpdk-unixctl.man \
lib/service.man \
lib/ssl-bootstrap.man \
lib/ssl.man \
@@ -296,6 +297,7 @@ lib/coverage-unixctl.man:
 lib/daemon.man:
 lib/dpctl.man:
 lib/memory-unixctl.man:
+lib/netdev-dpdk-unixctl.man:
 lib/service.man:
 lib/ssl-bootstrap.man:
 lib/ssl.man:
diff --git a/vswitchd/ovs-vswitchd.8.in b/vswitchd/ovs-vswitchd.8.in
index c18baf6..76ccfcb 100644
--- a/vswitchd/ovs-vswitchd.8.in
+++ b/vswitchd/ovs-vswitchd.8.in
@@ -283,6 +283,7 @@ port names, which this thread polls.
 .IP "\fBdpif-netdev/pmd-rxq-rebalance\fR [\fIdp\fR]"
 Reassigns rxqs to pmds in the datapath \fIdp\fR based on their current usage.
 .
+.so lib/netdev-dpdk-unixctl.man
 .so ofproto/ofproto-dpif-unixctl.man
 .so ofproto/ofproto-unixctl.man
 .so lib/vlog-unixctl.man
-- 
2.7.4

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

Re: [ovs-dev] [PATCH 2/2] netdev-dpdk: Add debug appctl to get mempool information.

[Ilya Maximets]

On 31.10.2017 17:01, Fischetti, Antonio wrote:
> Thanks Ilya, looks a useful debugging command, I gave it a try.
> Agree with Raymond, there should be some reference in the doc somewhere.

Thanks for review and testing. I've sent patch with documentation update
in reply to cover-letter (with 3/2 tag).

Best regards, Ilya Maximets.

> Beside that LGTM.
> 
> Acked-by: Antonio Fischetti 
> 
>> -Original Message-
>> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
>> Sent: Tuesday, October 31, 2017 11:35 AM
>> To: ovs-dev@openvswitch.org
>> Cc: Heetae Ahn ; Fischetti, Antonio
>> ; Loftus, Ciara ;
>> Kavanagh, Mark B ; Stokes, Ian
>> ; Wojciechowicz, RobertX
>> ; Ilya Maximets 
>> Subject: [PATCH 2/2] netdev-dpdk: Add debug appctl to get mempool 
>> information.
>>
>> New appctl 'netdev-dpdk/get-mempool-info' implemented to get result
>> of 'rte_mempool_list_dump()' function if no arguments passed and
>> 'rte_mempool_dump()' if DPDK netdev passed as argument.
>>
>> Could be used for debugging mbuf leaks and other mempool related
>> issues. Most useful in pair with `grep -v "cache_count.*=0"`.
>>
>> Signed-off-by: Ilya Maximets 
>> ---
>>  lib/netdev-dpdk.c | 54 
>> ++
>>  1 file changed, 54 insertions(+)
>>
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index 4ec536d..0e4a08c 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -2550,6 +2550,56 @@ error:
>>  free(response);
>>  }
>>
>> +static void
>> +netdev_dpdk_get_mempool_info(struct unixctl_conn *conn,
>> + int argc, const char *argv[],
>> + void *aux OVS_UNUSED)
>> +{
>> +size_t size;
>> +FILE *stream;
>> +char *response = NULL;
>> +struct netdev *netdev = NULL;
>> +
>> +if (argc == 2) {
>> +netdev = netdev_from_name(argv[1]);
>> +if (!netdev || !is_dpdk_class(netdev->netdev_class)) {
>> +unixctl_command_reply_error(conn, "Not a DPDK Interface");
>> +goto out;
>> +}
>> +}
>> +
>> +stream = open_memstream(, );
>> +if (!stream) {
>> +response = xasprintf("Unable to open memstream: %s.",
>> + ovs_strerror(errno));
>> +unixctl_command_reply_error(conn, response);
>> +goto out;
>> +}
>> +
>> +if (netdev) {
>> +struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
>> +
>> +ovs_mutex_lock(>mutex);
>> +ovs_mutex_lock(_mp_mutex);
>> +
>> +rte_mempool_dump(stream, dev->mp);
>> +
>> +ovs_mutex_unlock(_mp_mutex);
>> +ovs_mutex_unlock(>mutex);
>> +} else {
>> +ovs_mutex_lock(_mp_mutex);
>> +rte_mempool_list_dump(stream);
>> +ovs_mutex_unlock(_mp_mutex);
>> +}
>> +
>> +fclose(stream);
>> +
>> +unixctl_command_reply(conn, response);
>> +out:
>> +free(response);
>> +netdev_close(netdev);
>> +}
>> +
>>  /*
>>   * Set virtqueue flags so that we do not receive interrupts.
>>   */
>> @@ -2806,6 +2856,10 @@ netdev_dpdk_class_init(void)
>>   "pci address of device", 1, 1,
>>   netdev_dpdk_detach, NULL);
>>
>> +unixctl_command_register("netdev-dpdk/get-mempool-info",
>> + "[netdev]", 0, 1,
>> + netdev_dpdk_get_mempool_info, NULL);
>> +
>>  ovsthread_once_done();
>>  }
>>
>> --
>> 2.7.4
> 
> 
> 
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH V2 3/4] tc: Add header rewrite using tc pedit action

[Simon Horman]

On Tue, Oct 31, 2017 at 09:20:55AM +0200, Paul Blakey wrote:
> 
> 
> On 30/10/2017 15:42, Simon Horman wrote:
> > On Wed, Oct 25, 2017 at 02:24:15PM +0300, Roi Dayan wrote:
> > > 
> > > 
> > > On 27/09/2017 12:08, Simon Horman wrote:
> > > > On Mon, Sep 25, 2017 at 04:31:42PM +0300, Paul Blakey wrote:
> > > > > 
> > > > > 
> > > > > On 18/09/2017 18:01, Simon Horman wrote:
> > > > > > On Mon, Sep 18, 2017 at 07:16:03AM +0300, Roi Dayan wrote:
> > > > > > > From: Paul Blakey 
> > > > > > > 
> > > > > > > To be later used to implement ovs action set offloading.
> > > > > > > 
> > > > > > > Signed-off-by: Paul Blakey 
> > > > > > > Reviewed-by: Roi Dayan 
> > > > > > > ---
> > > > > > >lib/tc.c | 372 
> > > > > > > ++-
> > > > > > >lib/tc.h |  16 +++
> > > > > > >2 files changed, 385 insertions(+), 3 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/lib/tc.c b/lib/tc.c
> > > > > > > index c9cada2..743b2ee 100644
> > > > > > > --- a/lib/tc.c
> > > > > > > +++ b/lib/tc.c
> > > > > > > @@ -21,8 +21,10 @@
> > > > > > >#include 
> > > > > > >#include 
> > > > > > >#include 
> > > > > > > +#include 
> > > > > > >#include 
> > > > > > >#include 
> > > > > > > +#include 
> > > > > > >#include 
> > > > > > >#include 
> > > > > > >#include 
> > > > > > > @@ -33,11 +35,14 @@
> > > > > > >#include "netlink-socket.h"
> > > > > > >#include "netlink.h"
> > > > > > >#include "openvswitch/ofpbuf.h"
> > > > > > > +#include "openvswitch/util.h"
> > > > > > >#include "openvswitch/vlog.h"
> > > > > > >#include "packets.h"
> > > > > > >#include "timeval.h"
> > > > > > >#include "unaligned.h"
> > > > > > > +#define MAX_PEDIT_OFFSETS 8
> > > > > > 
> > > > > > Why 8?
> > > > > We don't expect anything more right now (ipv6 src/dst rewrite 
> > > > > requires 8
> > > > > pedits iirc). I can't think of a larger use case, maybe ipv6 + macs if
> > > > > that's makes sens. do you suggest we increase it? to what?
> > > > 
> > > > It seems strange to me to place a somewhat arbitrary small limit
> > > > when none exists in the pedit API being used. I would at prefer if
> > > > it was at least a bigger, say 16 or 32.
> > > 
> > > 
> > > Hi Simon,
> > > 
> > > Sorry for the late reply due to holidays and vacations.
> > > Me & Paul going to go over this and do the fixes needed and
> > > also rebase over latest master and run tests again.
> > 
> > Likewise, sorry for not responding earlier (same reason).
> > 
> > > I'll answer what I'm more familiar with now and Paul will continue.
> > > The 8 here is too low and you right. We used this definition
> > > for allocation of the pedit keys on the stack in
> > > nl_msg_put_flower_rewrite_pedits()
> > > 
> > > It was for convenience instead of calculating the maximum possible
> > > keys that could exists and allocating it there and freeing it at
> > > the end.
> > > 
> > > Increasing it to 32 is probably more than enough and wont waste much.
> > 
> > Thanks, that sounds good.
> > 
> > > > > > >VLOG_DEFINE_THIS_MODULE(tc);
> > > > > > >static struct vlog_rate_limit error_rl = 
> > > > > > > VLOG_RATE_LIMIT_INIT(60, 5);
> > > > > > > @@ -50,6 +55,82 @@ enum tc_offload_policy {
> > > > > > >static enum tc_offload_policy tc_policy = TC_POLICY_NONE;
> > > > > > > +struct tc_pedit_key_ex {
> > > > > > > +enum pedit_header_type htype;
> > > > > > > +enum pedit_cmd cmd;
> > > > > > > +};
> > > > > > > +
> > > > > > > +struct flower_key_to_pedit {
> > > > > > > +enum pedit_header_type htype;
> > > > > > > +int flower_offset;
> > > > > > > +int offset;
> > > > > > > +int size;
> > > > > > > +};
> > > > > > > +
> > > > > > > +static struct flower_key_to_pedit flower_pedit_map[] = {
> > > > > > > +{
> > > > > > > +TCA_PEDIT_KEY_EX_HDR_TYPE_IP4,
> > > > > > > +12,
> > > > > > > +offsetof(struct tc_flower_key, ipv4.ipv4_src),
> > > > > > > +MEMBER_SIZEOF(struct tc_flower_key, ipv4.ipv4_src)
> > > > > > > +}, {
> > > > > > > +TCA_PEDIT_KEY_EX_HDR_TYPE_IP4,
> > > > > > > +16,
> > > > > > > +offsetof(struct tc_flower_key, ipv4.ipv4_dst),
> > > > > > > +MEMBER_SIZEOF(struct tc_flower_key, ipv4.ipv4_dst)
> > > > > > > +}, {
> > > > > > > +TCA_PEDIT_KEY_EX_HDR_TYPE_IP4,
> > > > > > > +8,
> > > > > > > +offsetof(struct tc_flower_key, ipv4.rewrite_ttl),
> > > > > > > +MEMBER_SIZEOF(struct tc_flower_key, ipv4.rewrite_ttl)
> > > > > > > +}, {
> > > > > > > +TCA_PEDIT_KEY_EX_HDR_TYPE_IP6,
> > > > > > > +8,
> > > > > > > +offsetof(struct tc_flower_key, ipv6.ipv6_src),
> > > > > > > +MEMBER_SIZEOF(struct tc_flower_key, ipv6.ipv6_src)
> > > > > > > +}, {
> > > > > > > +TCA_PEDIT_KEY_EX_HDR_TYPE_IP6,
> > > > > > > +24,
> > > > > > > +   

[ovs-dev] [PATCH v2 4/4] ovn: Add IPv6 capability to ovn-nbctl lb-add

[Mark Michelson]

ovn-nbctl will now accept IPv6 addresses for load balancer VIPs and
desetination addresses.

In addition, the ovn-nbctl lb-list, lr-lb-list, and ls-lb-list have been
modified to be able to fit IPv6 addresses on screen.

Signed-off-by: Mark Michelson 
---
 ovn/utilities/ovn-nbctl.8.xml |  14 +-
 ovn/utilities/ovn-nbctl.c | 175 ++-
 tests/ovn-nbctl.at| 379 +++---
 3 files changed, 459 insertions(+), 109 deletions(-)

diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
index a20828088..3688d35b3 100644
--- a/ovn/utilities/ovn-nbctl.8.xml
+++ b/ovn/utilities/ovn-nbctl.8.xml
@@ -600,13 +600,15 @@
  Creates a new load balancer named lb with the provided
  vip and ips or adds the vip to
  an existing lb.  vip should be a
- virtual IPv4 address (or an IPv4 address and a port number with
+ virtual IP address (or an IP address and a port number with
  : as a separator).  Examples for vip are
- 192.168.1.4 and 192.168.1.5:8080.
- ips should be comma separated IPv4 endpoints (or comma
- separated IPv4 addresses and port numbers with : as a
- separator).  Examples for ips are 10.0.0.1,10.0.0.2
- or 20.0.0.10:8800,20.0.0.11:8800.
+ 192.168.1.4, fd0f::1, and
+ 192.168.1.5:8080. ips should be comma
+ separated IP endpoints (or comma separated IP addresses and port
+ numbers with : as a separator).  ips must
+ be the same address family as vip.  Examples for
+ ips are 10.0.0.1,10.0.0.2or
+ [fdef::1]:8800,[fdef::2]:8800.
 
 
 
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 8e5c1a440..252e4c904 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -1548,40 +1548,76 @@ nbctl_lb_add(struct ctl_context *ctx)
 }
 }
 
-ovs_be32 ipv4 = 0;
-ovs_be16 port = 0;
-char *error = ip_parse_port(lb_vip, , );
+struct sockaddr_storage ss_vip;
+char *error;
+error = ipv46_parse(lb_vip, PORT_OPTIONAL, _vip);
 if (error) {
 free(error);
-if (!ip_parse(lb_vip, )) {
-ctl_fatal("%s: should be an IPv4 address (or an IPv4 address "
-"and a port number with : as a separator).", lb_vip);
+ctl_fatal("%s: should be an IP address (or an IP address "
+  "and a port number with : as a separator).", lb_vip);
+}
+
+char lb_vip_normalized[INET6_ADDRSTRLEN + 8];
+char normalized_ip[INET6_ADDRSTRLEN];
+if (ss_vip.ss_family == AF_INET) {
+struct sockaddr_in *sin = ALIGNED_CAST(struct sockaddr_in *, _vip);
+inet_ntop(AF_INET, >sin_addr, normalized_ip,
+  sizeof normalized_ip);
+if (sin->sin_port) {
+is_vip_with_port = true;
+snprintf(lb_vip_normalized, sizeof lb_vip_normalized, "%s:%d",
+ normalized_ip, ntohs(sin->sin_port));
+} else {
+is_vip_with_port = false;
+ovs_strlcpy(lb_vip_normalized, normalized_ip,
+sizeof lb_vip_normalized);
 }
-
-if (is_update_proto) {
-ctl_fatal("Protocol is unnecessary when no port of vip "
-"is given.");
+} else {
+struct sockaddr_in6 *sin6 = ALIGNED_CAST(struct sockaddr_in6 *,
+ _vip);
+inet_ntop(AF_INET6, >sin6_addr, normalized_ip,
+  sizeof normalized_ip);
+if (sin6->sin6_port) {
+is_vip_with_port = true;
+snprintf(lb_vip_normalized, sizeof lb_vip_normalized, "[%s]:%d",
+ normalized_ip, ntohs(sin6->sin6_port));
+} else {
+is_vip_with_port = false;
+ovs_strlcpy(lb_vip_normalized, normalized_ip,
+sizeof lb_vip_normalized);
 }
-is_vip_with_port = false;
+}
+
+if (!is_vip_with_port && is_update_proto) {
+ctl_fatal("Protocol is unnecessary when no port of vip "
+  "is given.");
 }
 
 char *token = NULL, *save_ptr = NULL;
 struct ds lb_ips_new = DS_EMPTY_INITIALIZER;
 for (token = strtok_r(lb_ips, ",", _ptr);
 token != NULL; token = strtok_r(NULL, ",", _ptr)) {
-if (is_vip_with_port) {
-error = ip_parse_port(token, , );
-if (error) {
-free(error);
-ds_destroy(_ips_new);
-ctl_fatal("%s: should be an IPv4 address and a port "
+struct sockaddr_storage ss_dst;
+
+error = ipv46_parse(token, is_vip_with_port
+? PORT_REQUIRED
+: PORT_FORBIDDEN,
+_dst);
+
+if (error) {
+free(error);
+if (is_vip_with_port) {
+ctl_fatal("%s: should be an IP address and a port "
  

[ovs-dev] [PATCH v2 2/4] ovn: Allow ct_lb actions to take IPv6 address arguments.

[Mark Michelson]

The ct_lb action previously assumed that any address arguments were
IPv4. This patch expands the parsing, formatting, and encoding of ct_lb
to be amenable to IPv6 addresses as well.

Signed-off-by: Mark Michelson 
---
 include/ovn/actions.h |  6 +++-
 ovn/lib/actions.c | 99 ---
 tests/ovn.at  |  8 -
 3 files changed, 91 insertions(+), 22 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 0a04af7aa..63885da3c 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -200,7 +200,11 @@ struct ovnact_ct_nat {
 };
 
 struct ovnact_ct_lb_dst {
-ovs_be32 ip;
+int family;
+union {
+struct in6_addr ipv6;
+ovs_be32 ipv4;
+};
 uint16_t port;
 };
 
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index c9876436d..3c21eb382 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -883,23 +883,63 @@ parse_ct_lb_action(struct action_context *ctx)
 
 if (lexer_match(ctx->lexer, LEX_T_LPAREN)) {
 while (!lexer_match(ctx->lexer, LEX_T_RPAREN)) {
-if (ctx->lexer->token.type != LEX_T_INTEGER
-|| mf_subvalue_width(>lexer->token.value) > 32) {
-free(dsts);
-lexer_syntax_error(ctx->lexer, "expecting IPv4 address");
-return;
-}
+struct ovnact_ct_lb_dst dst;
+if (lexer_match(ctx->lexer, LEX_T_LSQUARE)) {
+/* IPv6 address and port */
+if (ctx->lexer->token.type != LEX_T_INTEGER
+|| ctx->lexer->token.format != LEX_F_IPV6) {
+free(dsts);
+lexer_syntax_error(ctx->lexer, "expecting IPv6 address");
+return;
+}
+dst.family = AF_INET6;
+dst.ipv6 = ctx->lexer->token.value.ipv6;
 
-/* Parse IP. */
-ovs_be32 ip = ctx->lexer->token.value.ipv4;
-lexer_get(ctx->lexer);
+lexer_get(ctx->lexer);
+if (!lexer_match(ctx->lexer, LEX_T_RSQUARE)) {
+free(dsts);
+lexer_syntax_error(ctx->lexer, "no closing square "
+   "bracket");
+return;
+}
+dst.port = 0;
+if (lexer_match(ctx->lexer, LEX_T_COLON)
+&& !action_parse_port(ctx, )) {
+free(dsts);
+return;
+}
+} else {
+if (ctx->lexer->token.type != LEX_T_INTEGER
+|| (ctx->lexer->token.format != LEX_F_IPV4
+&& ctx->lexer->token.format != LEX_F_IPV6)) {
+free(dsts);
+lexer_syntax_error(ctx->lexer, "expecting IP address");
+return;
+}
 
-/* Parse optional port. */
-uint16_t port = 0;
-if (lexer_match(ctx->lexer, LEX_T_COLON)
-&& !action_parse_port(ctx, )) {
-free(dsts);
-return;
+/* Parse IP. */
+struct ovnact_ct_lb_dst dst;
+if (ctx->lexer->token.format == LEX_F_IPV4) {
+dst.family = AF_INET;
+dst.ipv4 = ctx->lexer->token.value.ipv4;
+} else {
+dst.family = AF_INET6;
+dst.ipv6 = ctx->lexer->token.value.ipv6;
+}
+
+lexer_get(ctx->lexer);
+dst.port = 0;
+if (lexer_match(ctx->lexer, LEX_T_COLON)) {
+if (dst.family == AF_INET6) {
+free(dsts);
+lexer_syntax_error(ctx->lexer, "IPv6 address needs "
+"square brackets if port is included");
+return;
+} else if (!action_parse_port(ctx, )) {
+free(dsts);
+return;
+}
+}
 }
 lexer_match(ctx->lexer, LEX_T_COMMA);
 
@@ -907,7 +947,7 @@ parse_ct_lb_action(struct action_context *ctx)
 if (n_dsts >= allocated_dsts) {
 dsts = x2nrealloc(dsts, _dsts, sizeof *dsts);
 }
-dsts[n_dsts++] = (struct ovnact_ct_lb_dst) { ip, port };
+dsts[n_dsts++] = dst;
 }
 }
 
@@ -929,9 +969,19 @@ format_CT_LB(const struct ovnact_ct_lb *cl, struct ds *s)
 }
 
 const struct ovnact_ct_lb_dst *dst = >dsts[i];
-ds_put_format(s, IP_FMT, IP_ARGS(dst->ip));
-if (dst->port) {
-ds_put_format(s, ":%"PRIu16, dst->port);
+if (dst->family == AF_INET) {
+ds_put_format(s, IP_FMT, IP_ARGS(dst->ipv4));
+if (dst->port) {
+

[ovs-dev] [PATCH v2 0/4] Add support for IPv6 load balancers

[Mark Michelson]

This patchset adds the necessary items in order to support IPv6 load
balancers in OVN. No syntax has changed in ovn-nbctl or in the
northbound database to support this. Appropriate tests have been
added to the testsuite as well.

v1 -> v2:
* The patchset has been rebased and conflicts resolved. The only
  patch with noticeable differences is patch 3 of 4. This is because
  add_router_lb_flow had to be modified to not attempt to add
  undnat flows for IPv6 load balancers.
* ovn-northd manpage has been updated to detail flows that are installed
  for IPv6 load balancers. This change is in patch 3 as well.

Mark Michelson (4):
  Add general-purpose IP/port parsing function.
  ovn: Allow ct_lb actions to take IPv6 address arguments.
  ovn: Allow northd to install IPv6 ct_lb logical flows.
  ovn: Add IPv6 capability to ovn-nbctl lb-add

 include/ovn/actions.h |   6 +-
 lib/packets.c |  78 +
 lib/packets.h |  10 ++
 ovn/lib/actions.c |  99 ---
 ovn/northd/ovn-northd.8.xml   |  68 +---
 ovn/northd/ovn-northd.c   | 182 
 ovn/ovn-nb.xml|  22 ++-
 ovn/utilities/ovn-nbctl.8.xml |  14 +-
 ovn/utilities/ovn-nbctl.c | 175 ++-
 tests/ovn-nbctl.at| 379 +++---
 tests/ovn.at  |   8 +-
 11 files changed, 801 insertions(+), 240 deletions(-)

-- 
2.13.5

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

Re: [ovs-dev] [PATCH 2/2] netdev-dpdk: Add debug appctl to get mempool information.

[Fischetti, Antonio]

Thanks Ilya, looks a useful debugging command, I gave it a try.
Agree with Raymond, there should be some reference in the doc somewhere.
Beside that LGTM.

Acked-by: Antonio Fischetti 

> -Original Message-
> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
> Sent: Tuesday, October 31, 2017 11:35 AM
> To: ovs-dev@openvswitch.org
> Cc: Heetae Ahn ; Fischetti, Antonio
> ; Loftus, Ciara ;
> Kavanagh, Mark B ; Stokes, Ian
> ; Wojciechowicz, RobertX
> ; Ilya Maximets 
> Subject: [PATCH 2/2] netdev-dpdk: Add debug appctl to get mempool information.
> 
> New appctl 'netdev-dpdk/get-mempool-info' implemented to get result
> of 'rte_mempool_list_dump()' function if no arguments passed and
> 'rte_mempool_dump()' if DPDK netdev passed as argument.
> 
> Could be used for debugging mbuf leaks and other mempool related
> issues. Most useful in pair with `grep -v "cache_count.*=0"`.
> 
> Signed-off-by: Ilya Maximets 
> ---
>  lib/netdev-dpdk.c | 54 ++
>  1 file changed, 54 insertions(+)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 4ec536d..0e4a08c 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -2550,6 +2550,56 @@ error:
>  free(response);
>  }
> 
> +static void
> +netdev_dpdk_get_mempool_info(struct unixctl_conn *conn,
> + int argc, const char *argv[],
> + void *aux OVS_UNUSED)
> +{
> +size_t size;
> +FILE *stream;
> +char *response = NULL;
> +struct netdev *netdev = NULL;
> +
> +if (argc == 2) {
> +netdev = netdev_from_name(argv[1]);
> +if (!netdev || !is_dpdk_class(netdev->netdev_class)) {
> +unixctl_command_reply_error(conn, "Not a DPDK Interface");
> +goto out;
> +}
> +}
> +
> +stream = open_memstream(, );
> +if (!stream) {
> +response = xasprintf("Unable to open memstream: %s.",
> + ovs_strerror(errno));
> +unixctl_command_reply_error(conn, response);
> +goto out;
> +}
> +
> +if (netdev) {
> +struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> +
> +ovs_mutex_lock(>mutex);
> +ovs_mutex_lock(_mp_mutex);
> +
> +rte_mempool_dump(stream, dev->mp);
> +
> +ovs_mutex_unlock(_mp_mutex);
> +ovs_mutex_unlock(>mutex);
> +} else {
> +ovs_mutex_lock(_mp_mutex);
> +rte_mempool_list_dump(stream);
> +ovs_mutex_unlock(_mp_mutex);
> +}
> +
> +fclose(stream);
> +
> +unixctl_command_reply(conn, response);
> +out:
> +free(response);
> +netdev_close(netdev);
> +}
> +
>  /*
>   * Set virtqueue flags so that we do not receive interrupts.
>   */
> @@ -2806,6 +2856,10 @@ netdev_dpdk_class_init(void)
>   "pci address of device", 1, 1,
>   netdev_dpdk_detach, NULL);
> 
> +unixctl_command_register("netdev-dpdk/get-mempool-info",
> + "[netdev]", 0, 1,
> + netdev_dpdk_get_mempool_info, NULL);
> +
>  ovsthread_once_done();
>  }
> 
> --
> 2.7.4

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

Re: [ovs-dev] [PATCH 4/4] netdev-dpdk: Remove unused MAX_NB_MBUF.

[Fischetti, Antonio]

Hi Ilya, I've tested all this patch-series by 
 - running some PVP test, 
 - checking the NUMA-awareness works and 
 - MTU small/big changes that causes reuse/creation of a new mp

It works fine.

LGTM

Acked-by: Antonio Fischetti 

> -Original Message-
> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
> Sent: Monday, October 30, 2017 12:53 PM
> To: ovs-dev@openvswitch.org
> Cc: Heetae Ahn ; Fischetti, Antonio
> ; Loftus, Ciara ;
> Kavanagh, Mark B ; Stokes, Ian
> ; Wojciechowicz, RobertX
> ; Ilya Maximets 
> Subject: [PATCH 4/4] netdev-dpdk: Remove unused MAX_NB_MBUF.
> 
> CC: Robert Wojciechowicz 
> Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each 
> port.")
> Signed-off-by: Ilya Maximets 
> ---
>  lib/netdev-dpdk.c | 18 --
>  1 file changed, 4 insertions(+), 14 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index cdb3244..0b40966 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -89,23 +89,13 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5,
> 20);
>  #define NETDEV_DPDK_MBUF_ALIGN  1024
>  #define NETDEV_DPDK_MAX_PKT_LEN 9728
> 
> -/* Max and min number of packets in the mempool.  OVS tries to allocate a
> - * mempool with MAX_NB_MBUF: if this fails (because the system doesn't have
> - * enough hugepages) we keep halving the number until the allocation succeeds
> - * or we reach MIN_NB_MBUF */
> -
> -#define MAX_NB_MBUF  (4096 * 64)
> +/* Min number of packets in the mempool.  OVS tries to allocate a mempool 
> with
> + * roughly estimated number of mbufs: if this fails (because the system
> doesn't
> + * have enough hugepages) we keep halving the number until the allocation
> + * succeeds or we reach MIN_NB_MBUF */
>  #define MIN_NB_MBUF  (4096 * 4)
>  #define MP_CACHE_SZ  RTE_MEMPOOL_CACHE_MAX_SIZE
> 
> -/* MAX_NB_MBUF can be divided by 2 many times, until MIN_NB_MBUF */
> -BUILD_ASSERT_DECL(MAX_NB_MBUF % ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF) ==
> 0);
> -
> -/* The smallest possible NB_MBUF that we're going to try should be a multiple
> - * of MP_CACHE_SZ. This is advised by DPDK documentation. */
> -BUILD_ASSERT_DECL((MAX_NB_MBUF / ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF))
> -  % MP_CACHE_SZ == 0);
> -
>  /*
>   * DPDK XSTATS Counter names definition
>   */
> --
> 2.7.4

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

Re: [ovs-dev] [PATCH 3/4] netdev-dpdk: Factor out struct dpdk_mp.

[Fischetti, Antonio]

Thanks Ilya, it's a good rework especially for netdev_dpdk_mempool_configure() 
fn.

LGTM

Acked-by: Antonio Fischetti 



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

Re: [ovs-dev] [PATCH 2/4] netdev-dpdk: Fix dpdk_mp leak in case of EEXIST.

[Fischetti, Antonio]

LGTM 
Acked-by: Antonio Fischetti 

> -Original Message-
> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
> Sent: Monday, October 30, 2017 12:53 PM
> To: ovs-dev@openvswitch.org
> Cc: Heetae Ahn ; Fischetti, Antonio
> ; Loftus, Ciara ;
> Kavanagh, Mark B ; Stokes, Ian
> ; Wojciechowicz, RobertX
> ; Ilya Maximets 
> Subject: [PATCH 2/4] netdev-dpdk: Fix dpdk_mp leak in case of EEXIST.
> 
> CC: Robert Wojciechowicz 
> CC: Antonio Fischetti 
> Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each 
> port.")
> Fixes: b6b26021d2e2 ("netdev-dpdk: fix management of pre-existing mempools.")
> Signed-off-by: Ilya Maximets 
> ---
>  lib/netdev-dpdk.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 1e9d78f..ba6add2 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -649,6 +649,12 @@ netdev_dpdk_mempool_configure(struct netdev_dpdk *dev)
>   * Update dev with the new values. */
>  dev->mtu = dev->requested_mtu;
>  dev->max_packet_len = MTU_TO_FRAME_LEN(dev->mtu);
> +/* 'mp' should contain pointer to the mempool already owned by 
> netdev.
> + * Otherwise something went completely wrong. */
> +ovs_assert(dev->dpdk_mp);
> +ovs_assert(dev->dpdk_mp->mp == mp->mp);
> +/* Free the returned struct dpdk_mp because it will not be used. */
> +rte_free(mp);
>  return EEXIST;
>  } else {
>  /* A new mempool was created, release the previous one. */
> --
> 2.7.4

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

Re: [ovs-dev] [PATCH 2/2] netdev-dpdk: Add debug appctl to get mempool information.

[Ilya Maximets]

On 31.10.2017 14:41, Raymond Burkholder wrote:
>>
>> New appctl 'netdev-dpdk/get-mempool-info' implemented to get result of
>> 'rte_mempool_list_dump()' function if no arguments passed and
>> 'rte_mempool_dump()' if DPDK netdev passed as argument.
>>
>> Could be used for debugging mbuf leaks and other mempool related issues.
>> Most useful in pair with `grep -v "cache_count.*=0"`.
> 
> Pardon my ignorance, but do commands like these get put into the
> documentation, command-line help at some point?
> 
> Looks like I should peruse source code and commits for other interesting
> commands.

Hm.. Many appctl commands are described in man for ovs-vswitchd.
I guess, it's kind of historical issue that netdev-dpdk, upcall, autoattach
and maybe some other commands was never referenced there.

So, maybe we need to create a section in vswitchd/ovs-vswitchd.8.in for
netdev-dpdk commands later.
We can make it as a separate commit after accepting of this change to
add all the missing netdev-dpdk appctl calls at once.

>>
>> Signed-off-by: Ilya Maximets 
>> ---
>>  lib/netdev-dpdk.c | 54
>> ++
>>  1 file changed, 54 insertions(+)
>>
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 4ec536d..0e4a08c
>> 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -2550,6 +2550,56 @@ error:
>>  free(response);
>>  }
>>
>> +static void
>> +netdev_dpdk_get_mempool_info(struct unixctl_conn *conn,
>> + int argc, const char *argv[],
>> + void *aux OVS_UNUSED) {
>> +size_t size;
>> +FILE *stream;
>> +char *response = NULL;
>> +struct netdev *netdev = NULL;
>> +
>> +if (argc == 2) {
>> +netdev = netdev_from_name(argv[1]);
>> +if (!netdev || !is_dpdk_class(netdev->netdev_class)) {
>> +unixctl_command_reply_error(conn, "Not a DPDK Interface");
>> +goto out;
>> +}
>> +}
>> +
>> +stream = open_memstream(, );
>> +if (!stream) {
>> +response = xasprintf("Unable to open memstream: %s.",
>> + ovs_strerror(errno));
>> +unixctl_command_reply_error(conn, response);
>> +goto out;
>> +}
>> +
>> +if (netdev) {
>> +struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
>> +
>> +ovs_mutex_lock(>mutex);
>> +ovs_mutex_lock(_mp_mutex);
>> +
>> +rte_mempool_dump(stream, dev->mp);
>> +
>> +ovs_mutex_unlock(_mp_mutex);
>> +ovs_mutex_unlock(>mutex);
>> +} else {
>> +ovs_mutex_lock(_mp_mutex);
>> +rte_mempool_list_dump(stream);
>> +ovs_mutex_unlock(_mp_mutex);
>> +}
>> +
>> +fclose(stream);
>> +
>> +unixctl_command_reply(conn, response);
>> +out:
>> +free(response);
>> +netdev_close(netdev);
>> +}
>> +
>>  /*
>>   * Set virtqueue flags so that we do not receive interrupts.
>>   */
>> @@ -2806,6 +2856,10 @@ netdev_dpdk_class_init(void)
>>   "pci address of device", 1, 1,
>>   netdev_dpdk_detach, NULL);
>>
>> +unixctl_command_register("netdev-dpdk/get-mempool-info",
>> + "[netdev]", 0, 1,
>> + netdev_dpdk_get_mempool_info, NULL);
>> +
>>  ovsthread_once_done();
>>  }
>>
>> --
>> 2.7.4
> 
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/2] netdev-dpdk: Add debug appctl to get mempool information.

[Raymond Burkholder]

> 
> New appctl 'netdev-dpdk/get-mempool-info' implemented to get result of
> 'rte_mempool_list_dump()' function if no arguments passed and
> 'rte_mempool_dump()' if DPDK netdev passed as argument.
> 
> Could be used for debugging mbuf leaks and other mempool related issues.
> Most useful in pair with `grep -v "cache_count.*=0"`.

Pardon my ignorance, but do commands like these get put into the
documentation, command-line help at some point?

Looks like I should peruse source code and commits for other interesting
commands.



> 
> Signed-off-by: Ilya Maximets 
> ---
>  lib/netdev-dpdk.c | 54
> ++
>  1 file changed, 54 insertions(+)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 4ec536d..0e4a08c
> 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -2550,6 +2550,56 @@ error:
>  free(response);
>  }
> 
> +static void
> +netdev_dpdk_get_mempool_info(struct unixctl_conn *conn,
> + int argc, const char *argv[],
> + void *aux OVS_UNUSED) {
> +size_t size;
> +FILE *stream;
> +char *response = NULL;
> +struct netdev *netdev = NULL;
> +
> +if (argc == 2) {
> +netdev = netdev_from_name(argv[1]);
> +if (!netdev || !is_dpdk_class(netdev->netdev_class)) {
> +unixctl_command_reply_error(conn, "Not a DPDK Interface");
> +goto out;
> +}
> +}
> +
> +stream = open_memstream(, );
> +if (!stream) {
> +response = xasprintf("Unable to open memstream: %s.",
> + ovs_strerror(errno));
> +unixctl_command_reply_error(conn, response);
> +goto out;
> +}
> +
> +if (netdev) {
> +struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> +
> +ovs_mutex_lock(>mutex);
> +ovs_mutex_lock(_mp_mutex);
> +
> +rte_mempool_dump(stream, dev->mp);
> +
> +ovs_mutex_unlock(_mp_mutex);
> +ovs_mutex_unlock(>mutex);
> +} else {
> +ovs_mutex_lock(_mp_mutex);
> +rte_mempool_list_dump(stream);
> +ovs_mutex_unlock(_mp_mutex);
> +}
> +
> +fclose(stream);
> +
> +unixctl_command_reply(conn, response);
> +out:
> +free(response);
> +netdev_close(netdev);
> +}
> +
>  /*
>   * Set virtqueue flags so that we do not receive interrupts.
>   */
> @@ -2806,6 +2856,10 @@ netdev_dpdk_class_init(void)
>   "pci address of device", 1, 1,
>   netdev_dpdk_detach, NULL);
> 
> +unixctl_command_register("netdev-dpdk/get-mempool-info",
> + "[netdev]", 0, 1,
> + netdev_dpdk_get_mempool_info, NULL);
> +
>  ovsthread_once_done();
>  }
> 
> --
> 2.7.4


-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

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

[ovs-dev] [PATCH 2/2] netdev-dpdk: Add debug appctl to get mempool information.

[Ilya Maximets]

New appctl 'netdev-dpdk/get-mempool-info' implemented to get result
of 'rte_mempool_list_dump()' function if no arguments passed and
'rte_mempool_dump()' if DPDK netdev passed as argument.

Could be used for debugging mbuf leaks and other mempool related
issues. Most useful in pair with `grep -v "cache_count.*=0"`.

Signed-off-by: Ilya Maximets 
---
 lib/netdev-dpdk.c | 54 ++
 1 file changed, 54 insertions(+)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 4ec536d..0e4a08c 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2550,6 +2550,56 @@ error:
 free(response);
 }
 
+static void
+netdev_dpdk_get_mempool_info(struct unixctl_conn *conn,
+ int argc, const char *argv[],
+ void *aux OVS_UNUSED)
+{
+size_t size;
+FILE *stream;
+char *response = NULL;
+struct netdev *netdev = NULL;
+
+if (argc == 2) {
+netdev = netdev_from_name(argv[1]);
+if (!netdev || !is_dpdk_class(netdev->netdev_class)) {
+unixctl_command_reply_error(conn, "Not a DPDK Interface");
+goto out;
+}
+}
+
+stream = open_memstream(, );
+if (!stream) {
+response = xasprintf("Unable to open memstream: %s.",
+ ovs_strerror(errno));
+unixctl_command_reply_error(conn, response);
+goto out;
+}
+
+if (netdev) {
+struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
+
+ovs_mutex_lock(>mutex);
+ovs_mutex_lock(_mp_mutex);
+
+rte_mempool_dump(stream, dev->mp);
+
+ovs_mutex_unlock(_mp_mutex);
+ovs_mutex_unlock(>mutex);
+} else {
+ovs_mutex_lock(_mp_mutex);
+rte_mempool_list_dump(stream);
+ovs_mutex_unlock(_mp_mutex);
+}
+
+fclose(stream);
+
+unixctl_command_reply(conn, response);
+out:
+free(response);
+netdev_close(netdev);
+}
+
 /*
  * Set virtqueue flags so that we do not receive interrupts.
  */
@@ -2806,6 +2856,10 @@ netdev_dpdk_class_init(void)
  "pci address of device", 1, 1,
  netdev_dpdk_detach, NULL);
 
+unixctl_command_register("netdev-dpdk/get-mempool-info",
+ "[netdev]", 0, 1,
+ netdev_dpdk_get_mempool_info, NULL);
+
 ovsthread_once_done();
 }
 
-- 
2.7.4

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

[ovs-dev] [PATCH 1/2] netdev-dpdk: Fix mempool creation with large MTU.

[Ilya Maximets]

Currently mempool name size limited to 25 characters by
RTE_MEMPOOL_NAMESIZE. netdev-dpdk tries to create mempool with the
following name pattern: "ovs_%{hash}_%{socket}_%{mtu}_%{n_mbuf}".

We have 3 chars for "ovs" + 4 chars for delimiters + 8 chars for
hash (because it's the 32 bit integer printed in hex) + 1 char for
socket_id (mostly 1, but it could be 2 on some systems; larger?) = 16.

Only 25 - 16 = 9 characters remains for mtu + n_mbufs.
Minimum usual value for mtu is 1500 --> 2030 (4 chars) after
dpdk_buf_size conversion and the minimum value for n_mbufs is 16384
(5 chars). So, all the 9 characters are used.

If we'll try to create port with mtu = 9500, mempool creation will
fail, because FRAME_LEN_TO_MTU(dpdk_buf_size(9500)) = 10222 (5 chars)
and this value will overflow the RTE_MEMPOOL_NAMESIZE limit.

Same issue will happen if we'll try to create port with big enough
number of queues or will try to create big enough number of PMD
threads (number of tx queues will enlarge the mempool requirements).

Fix that by removing the delimiters. To keep the readability (at least
partial) of the mempool names exact field sizes with zero padding
are used.

Following limits should be suitable for now:
 - Hash length: 8 chars (uint32_t in hex)
 - Socket ID  : 2 chars (For systems with up to 10 sockets)
 - MTU: 5 chars (MTU (10^5 - 1) should be enough for now)
 - n_mbufs: 7 chars (Up to 10^7 of mbufs)

   Total  : 22 + 3 (for "ovs") = 25

CC: Antonio Fischetti 
CC: Robert Wojciechowicz 
Fixes: f06546a51dd8 ("Fix mempool names to reflect socket id.")
Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each port.")
Signed-off-by: Ilya Maximets 
---
 lib/netdev-dpdk.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 0b40966..4ec536d 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -500,7 +500,8 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu)
 do {
 /* Full DPDK memory pool name must be unique and cannot be
  * longer than RTE_MEMPOOL_NAMESIZE. */
-int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%d_%u",
+int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
+   "ovs%08x%02d%05d%07u",
hash, socket_id, mtu, n_mbufs);
 if (ret < 0 || ret >= RTE_MEMPOOL_NAMESIZE) {
 VLOG_DBG("snprintf returned %d. "
-- 
2.7.4

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

[ovs-dev] [PATCH 0/2] netdev-dpdk: Mempool creation failure + Appctl

[Ilya Maximets]

This series implemented on top of my previous patch-set:
 * [PATCH 0/4] netdev-dpdk: mempool management: Leaks & Refactoring.
   https://mail.openvswitch.org/pipermail/ovs-dev/2017-October/340144.html

I can rebase it on top of current master easily if needed.

Patches are independent, sent together only because they are based on
the same patch-set pointed above.

First patch fixes mempool creation failure in case of large MTU or
big number of queues/threads. Second patch adds debug appctl to
obtain mempool information from DPDK including names, numbers of
available mbufs, object sizes and memory pointers.

Ilya Maximets (2):
  netdev-dpdk: Fix mempool creation with large MTU.
  netdev-dpdk: Add debug appctl to get mempool information.

 lib/netdev-dpdk.c | 57 ++-
 1 file changed, 56 insertions(+), 1 deletion(-)

-- 
2.7.4

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

[ovs-dev] [PATCH] rhel: Add support for "systemctl reload openvswitch"

[Timothy Redaelli]

The reload procedure will trigger a script that saves the flows and tlv
maps (using ovs-save) then it restarts ovsdb-server, it stops ovs-vswitchd,
it sets other_config:flow-restore-wait=true (to wait till flow restore is
finished), it starts ovs-vswitchd, it restore the backupped flows/tlv
maps and it removes other_config:flow-restore-wait=true (logic mostly ripped
from ovs-ctl).

It uses systemctl with --job-mode=ignore-dependencies to restart ovsdb-server
and stop and start ovs-vswitchd in order to avoid systemd to restart the other
components due to dependencies (as explained in rhel/README.RHEL.rst).

Signed-off-by: Timothy Redaelli 
---
 rhel/automake.mk |  1 +
 rhel/openvswitch-fedora.spec.in  |  5 
 rhel/usr_lib_systemd_system_openvswitch.service  |  2 +-
 rhel/usr_lib_systemd_system_ovsdb-server.service |  1 -
 rhel/usr_share_openvswitch_scripts_ovs-reload| 36 
 5 files changed, 43 insertions(+), 2 deletions(-)
 create mode 100755 rhel/usr_share_openvswitch_scripts_ovs-reload

diff --git a/rhel/automake.mk b/rhel/automake.mk
index 9336f0912..0955dceed 100644
--- a/rhel/automake.mk
+++ b/rhel/automake.mk
@@ -24,6 +24,7 @@ EXTRA_DIST += \
rhel/openvswitch.spec.in \
rhel/openvswitch-fedora.spec \
rhel/openvswitch-fedora.spec.in \
+   rhel/usr_share_openvswitch_scripts_ovs-reload \
rhel/usr_share_openvswitch_scripts_sysconfig.template \
rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template \
rhel/usr_lib_udev_rules.d_91-vfio.rules \
diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index fb7d918c6..87bec39c9 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -314,6 +314,10 @@ install -d -m 0755 
$RPM_BUILD_ROOT%{_prefix}/lib/ocf/resource.d/ovn
 ln -s %{_datadir}/openvswitch/scripts/ovndb-servers.ocf \
   $RPM_BUILD_ROOT%{_prefix}/lib/ocf/resource.d/ovn/ovndb-servers
 
+install -p -D -m 0755 \
+rhel/usr_share_openvswitch_scripts_ovs-reload \
+$RPM_BUILD_ROOT%{_datadir}/openvswitch/scripts/ovs-reload
+
 # remove unpackaged files
 rm -f $RPM_BUILD_ROOT%{_bindir}/ovs-parse-backtrace \
 $RPM_BUILD_ROOT%{_sbindir}/ovs-vlan-bug-workaround \
@@ -539,6 +543,7 @@ fi
 %{_datadir}/openvswitch/scripts/ovs-save
 %{_datadir}/openvswitch/scripts/ovs-vtep
 %{_datadir}/openvswitch/scripts/ovs-ctl
+%{_datadir}/openvswitch/scripts/ovs-reload
 %config %{_datadir}/openvswitch/vswitch.ovsschema
 %config %{_datadir}/openvswitch/vtep.ovsschema
 %{_bindir}/ovs-appctl
diff --git a/rhel/usr_lib_systemd_system_openvswitch.service 
b/rhel/usr_lib_systemd_system_openvswitch.service
index faca44b54..2cf29f0e9 100644
--- a/rhel/usr_lib_systemd_system_openvswitch.service
+++ b/rhel/usr_lib_systemd_system_openvswitch.service
@@ -9,7 +9,7 @@ Requires=ovs-vswitchd.service
 [Service]
 Type=oneshot
 ExecStart=/bin/true
-ExecReload=/bin/true
+ExecReload=/usr/share/openvswitch/scripts/ovs-reload
 ExecStop=/bin/true
 RemainAfterExit=yes
 
diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service 
b/rhel/usr_lib_systemd_system_ovsdb-server.service
index 5baac822d..234d39355 100644
--- a/rhel/usr_lib_systemd_system_ovsdb-server.service
+++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
@@ -3,7 +3,6 @@ Description=Open vSwitch Database Unit
 After=syslog.target network-pre.target
 Before=network.target network.service
 Wants=ovs-delete-transient-ports.service
-ReloadPropagatedFrom=openvswitch.service
 PartOf=openvswitch.service
 
 [Service]
diff --git a/rhel/usr_share_openvswitch_scripts_ovs-reload 
b/rhel/usr_share_openvswitch_scripts_ovs-reload
new file mode 100755
index 0..3ac1a46c6
--- /dev/null
+++ b/rhel/usr_share_openvswitch_scripts_ovs-reload
@@ -0,0 +1,36 @@
+#! /bin/sh
+
+# 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.
+
+# Save flows
+bridges=$(ovs-vsctl -- --real list-br)
+flows=$(/usr/share/openvswitch/scripts/ovs-save save-flows $bridges)
+
+# Restart the database first, since a large database may take a
+# while to load, and we want to minimize forwarding disruption.
+systemctl --job-mode=ignore-dependencies restart ovsdb-server
+
+# Stop ovs-vswitchd.
+systemctl --job-mode=ignore-dependencies stop ovs-vswitchd
+
+# Start vswitchd by asking it to wait till flow restore is finished.
+ovs-vsctl --no-wait set open_vswitch . 

Re: [ovs-dev] [PATCH] netdev-dpdk: replace uint8_t with dpdk_port_t

[Kavanagh, Mark B]

>From: Ilya Maximets [mailto:i.maxim...@samsung.com]
>Sent: Tuesday, October 31, 2017 6:46 AM
>To: Kavanagh, Mark B ; d...@openvswitch.org
>Subject: Re: [ovs-dev][PATCH] netdev-dpdk: replace uint8_t with dpdk_port_t
>
>Thanks. I wanted to remove this function initially, that's why I forget
>to replace the type here.
>
>This is important because dpdk changes the type of port_id to uint16_t
>for upcoming release.

Yes, exactly - that's how I detected this issue.
>
>Acked-by: Ilya Maximets 

Thanks Ilya!
Mark

>
>On 20.10.2017 15:37, Mark Kavanagh wrote:
>> netdev_dpdk_detach() declares a 'port_id' variable, of type uint8_t.
>> This variable should instead be of type dpdk_port_t.
>>
>> Fixes: bb37956ac ("netdev-dpdk: Use uint8_t for port_id.")
>> CC: Ilya Maximets 
>> Signed-off-by: Mark Kavanagh 
>> ---
>>  lib/netdev-dpdk.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index c60f46f..1f6345d 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -2549,7 +2549,7 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc
>OVS_UNUSED,
>>  {
>>  int ret;
>>  char *response;
>> -uint8_t port_id;
>> +dpdk_port_t port_id;
>>  char devname[RTE_ETH_NAME_MAX_LEN];
>>  struct netdev_dpdk *dev;
>>
>> --
>> 1.9.3
>>
>>
>>
>>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] tests: fix PTAP system test to check only OF stats

[Zoltán Balogh]

Hi Ben,

I rebased the patch and sent v3 to the dev list:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-October/340225.html
https://patchwork.ozlabs.org/patch/832300/

Best regards,
Zoltan

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Monday, October 30, 2017 10:33 PM
> To: Zoltán Balogh 
> Cc: 'd...@openvswitch.org' 
> Subject: Re: [ovs-dev] [PATCH v2] tests: fix PTAP system test to check only 
> OF stats
> 
> On Wed, Jul 12, 2017 at 07:22:58AM +, Zoltán Balogh wrote:
> >
> > It turned out, checking datapath flow statistics during system-userspace
> > test is not reliable. Unwanted packets can be injected depending on
> > system configuration. As a workaround, this commit removes checking
> > statistics of datapath flows and does check OpenFlow statistics of the
> > integrator bridges. Datapath flows can be checked in normal PTAP unit
> > tests by running 'make check'.
> >
> > Reported-by: Darrell Ball 
> > Suggested-by: Jan Scheurich 
> > Tested-by: Darrell Ball 
> > Signed-off-by: Zoltán Balogh 
> 
> It seems that this patch was never properly reviewed and applied, but it
> no longer applies cleanly.  Is there any chance you'd be willing to
> rebase and re-post it?
> 
> Thanks,
> 
> Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3] tests: fix PTAP system test to check only OF stats

[Zoltan Balogh]

It turned out, checking datapath flow statistics during system-userspace
test is not reliable. Unwanted packets can be injected depending on
system configuration. As a workaround, this commit removes checking
statistics of datapath flows and does check OpenFlow statistics of the
integrator bridges. Datapath flows can be checked in normal PTAP unit
tests by running 'make check'.

Reported-by: Darrell Ball 
Suggested-by: Jan Scheurich 
Tested-by: Darrell Ball 
Signed-off-by: Zoltan Balogh 
---
 tests/system-userspace-packet-type-aware.at | 119 
 1 file changed, 67 insertions(+), 52 deletions(-)

diff --git a/tests/system-userspace-packet-type-aware.at 
b/tests/system-userspace-packet-type-aware.at
index 3aa2de5..24a7698 100644
--- a/tests/system-userspace-packet-type-aware.at
+++ b/tests/system-userspace-packet-type-aware.at
@@ -33,9 +33,9 @@ AT_SETUP([ptap - triangle bridge setup with L2 and L3 GRE 
tunnels])
 #  1030   br-in1  gre-13  l2  br-in3 3010 (l2)
 #  2010   br-in2  gre-21  ptapbr-in1 1020 (l2), 1021 (l3)
 #  2030   br-in2  gre-23  ptapbr-in3 3020 (l2), 3021 (l3)
-#  3010   br-in1  gre-31  l2  br-in1 1030 (l2)
-#  3020   br-in1  gre-32  l2  br-in2 2010 (ptap)
-#  3021   br-in1  gre-32_l3   l3same
+#  3010   br-in3  gre-31  l2  br-in1 1030 (l2)
+#  3020   br-in3  gre-32  l2  br-in2 2010 (ptap)
+#  3021   br-in3  gre-32_l3   l3same
 
 
 AT_SKIP_IF([test $HAVE_NC = no])
@@ -176,15 +176,15 @@ AT_CHECK([
 
 ### Flows in br-pto twist TEP IP addresses in tunnel IP headers
 AT_CHECK([
-ovs-ofctl add-flow br-p1 in_port:LOCAL,actions=2
+ovs-ofctl add-flow br-p1 in_port:LOCAL,ip,actions=2
 ovs-ofctl add-flow br-p1 
in_port:2,ip,nw_dst:20.0.0.1,actions=mod_nw_dst:10.0.0.1,mod_nw_src:10.0.0.2,LOCAL
 ovs-ofctl add-flow br-p1 
in_port:2,ip,nw_dst:30.0.0.1,actions=mod_nw_dst:10.0.0.1,mod_nw_src:10.0.0.3,LOCAL
 
-ovs-ofctl add-flow br-p2 in_port:LOCAL,actions=2
+ovs-ofctl add-flow br-p2 in_port:LOCAL,ip,actions=2
 ovs-ofctl add-flow br-p2 
in_port:2,ip,nw_dst:10.0.0.2,actions=mod_nw_dst:20.0.0.2,mod_nw_src:20.0.0.1,LOCAL
 ovs-ofctl add-flow br-p2 
in_port:2,ip,nw_dst:30.0.0.2,actions=mod_nw_dst:20.0.0.2,mod_nw_src:20.0.0.3,LOCAL
 
-ovs-ofctl add-flow br-p3 in_port:LOCAL,actions=2
+ovs-ofctl add-flow br-p3 in_port:LOCAL,ip,actions=2
 ovs-ofctl add-flow br-p3 
in_port:2,ip,nw_dst:10.0.0.3,actions=mod_nw_dst:30.0.0.3,mod_nw_src:30.0.0.1,LOCAL
 ovs-ofctl add-flow br-p3 
in_port:2,ip,nw_dst:20.0.0.3,actions=mod_nw_dst:30.0.0.3,mod_nw_src:30.0.0.2,LOCAL
 ], [0])
@@ -204,15 +204,15 @@ AT_CHECK([
 ovs-ofctl dump-flows br-p2 | ofctl_strip | strip_n_packets | strip_n_bytes 
| sort | grep actions
 ovs-ofctl dump-flows br-p3 | ofctl_strip | strip_n_packets | strip_n_bytes 
| sort | grep actions
 ], [0], [dnl
- in_port=LOCAL actions=output:2
  ip,in_port=2,nw_dst=20.0.0.1 
actions=mod_nw_dst:10.0.0.1,mod_nw_src:10.0.0.2,LOCAL
  ip,in_port=2,nw_dst=30.0.0.1 
actions=mod_nw_dst:10.0.0.1,mod_nw_src:10.0.0.3,LOCAL
- in_port=LOCAL actions=output:2
+ ip,in_port=LOCAL actions=output:2
  ip,in_port=2,nw_dst=10.0.0.2 
actions=mod_nw_dst:20.0.0.2,mod_nw_src:20.0.0.1,LOCAL
  ip,in_port=2,nw_dst=30.0.0.2 
actions=mod_nw_dst:20.0.0.2,mod_nw_src:20.0.0.3,LOCAL
- in_port=LOCAL actions=output:2
+ ip,in_port=LOCAL actions=output:2
  ip,in_port=2,nw_dst=10.0.0.3 
actions=mod_nw_dst:30.0.0.3,mod_nw_src:30.0.0.1,LOCAL
  ip,in_port=2,nw_dst=20.0.0.3 
actions=mod_nw_dst:30.0.0.3,mod_nw_src:30.0.0.2,LOCAL
+ ip,in_port=LOCAL actions=output:2
 ])
 
 ### Setup test ports for traffic injection
@@ -331,9 +331,6 @@ AT_CHECK([
 ])
 
 
-# Clear up megaflow cache
-sleep 10
-
 # Ping between N1 and N3, via the L2 GRE tunnel between br-in1 and br-in3
 NS_CHECK_EXEC([ns1], [ping -q -c 3 -i 0.3 -w 2 $N3_IP | FORMAT_PING], [0], [dnl
 3 packets transmitted, 3 received, 0% packet loss, time 0ms
@@ -342,23 +339,25 @@ NS_CHECK_EXEC([ns1], [ping -q -c 3 -i 0.3 -w 2 $N3_IP | 
FORMAT_PING], [0], [dnl
 sleep 1
 
 AT_CHECK([
-ovs-appctl dpctl/dump-flows | strip_used | grep -v ipv6 | grep -v arp |sort
-], [0], [flow-dump from non-dpdk interfaces:
-recirc_id(0),in_port(10),packet_type(ns=0,id=0),eth(dst=aa:55:00:00:00:03),eth_type(0x0800),ipv4(src=10.0.0.1,dst=10.0.0.3,proto=47,frag=no),
 packets:2, bytes:272, used:0.0s, 
actions:set(ipv4(src=30.0.0.1,dst=30.0.0.3)),tnl_pop(14)
-recirc_id(0),in_port(11),packet_type(ns=0,id=0),eth(dst=aa:55:00:00:00:03),eth_type(0x0800),ipv4(frag=no),
 packets:2, bytes:272, used:0.0s, actions:13
-recirc_id(0),in_port(12),packet_type(ns=0,id=0),eth(dst=aa:55:00:00:00:01),eth_type(0x0800),ipv4(frag=no),
 packets:2, bytes:244, used:0.0s, actions:11

Re: [ovs-dev] [PATCH V2 3/4] tc: Add header rewrite using tc pedit action

[Paul Blakey]

On 30/10/2017 15:42, Simon Horman wrote:

On Wed, Oct 25, 2017 at 02:24:15PM +0300, Roi Dayan wrote:



On 27/09/2017 12:08, Simon Horman wrote:

On Mon, Sep 25, 2017 at 04:31:42PM +0300, Paul Blakey wrote:



On 18/09/2017 18:01, Simon Horman wrote:

On Mon, Sep 18, 2017 at 07:16:03AM +0300, Roi Dayan wrote:

From: Paul Blakey 

To be later used to implement ovs action set offloading.

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 
---
   lib/tc.c | 372 
++-
   lib/tc.h |  16 +++
   2 files changed, 385 insertions(+), 3 deletions(-)

diff --git a/lib/tc.c b/lib/tc.c
index c9cada2..743b2ee 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -21,8 +21,10 @@
   #include 
   #include 
   #include 
+#include 
   #include 
   #include 
+#include 
   #include 
   #include 
   #include 
@@ -33,11 +35,14 @@
   #include "netlink-socket.h"
   #include "netlink.h"
   #include "openvswitch/ofpbuf.h"
+#include "openvswitch/util.h"
   #include "openvswitch/vlog.h"
   #include "packets.h"
   #include "timeval.h"
   #include "unaligned.h"
+#define MAX_PEDIT_OFFSETS 8


Why 8?

We don't expect anything more right now (ipv6 src/dst rewrite requires 8
pedits iirc). I can't think of a larger use case, maybe ipv6 + macs if
that's makes sens. do you suggest we increase it? to what?


It seems strange to me to place a somewhat arbitrary small limit
when none exists in the pedit API being used. I would at prefer if
it was at least a bigger, say 16 or 32.



Hi Simon,

Sorry for the late reply due to holidays and vacations.
Me & Paul going to go over this and do the fixes needed and
also rebase over latest master and run tests again.


Likewise, sorry for not responding earlier (same reason).


I'll answer what I'm more familiar with now and Paul will continue.
The 8 here is too low and you right. We used this definition
for allocation of the pedit keys on the stack in
nl_msg_put_flower_rewrite_pedits()

It was for convenience instead of calculating the maximum possible
keys that could exists and allocating it there and freeing it at
the end.

Increasing it to 32 is probably more than enough and wont waste much.


Thanks, that sounds good.


   VLOG_DEFINE_THIS_MODULE(tc);
   static struct vlog_rate_limit error_rl = VLOG_RATE_LIMIT_INIT(60, 5);
@@ -50,6 +55,82 @@ enum tc_offload_policy {
   static enum tc_offload_policy tc_policy = TC_POLICY_NONE;
+struct tc_pedit_key_ex {
+enum pedit_header_type htype;
+enum pedit_cmd cmd;
+};
+
+struct flower_key_to_pedit {
+enum pedit_header_type htype;
+int flower_offset;
+int offset;
+int size;
+};
+
+static struct flower_key_to_pedit flower_pedit_map[] = {
+{
+TCA_PEDIT_KEY_EX_HDR_TYPE_IP4,
+12,
+offsetof(struct tc_flower_key, ipv4.ipv4_src),
+MEMBER_SIZEOF(struct tc_flower_key, ipv4.ipv4_src)
+}, {
+TCA_PEDIT_KEY_EX_HDR_TYPE_IP4,
+16,
+offsetof(struct tc_flower_key, ipv4.ipv4_dst),
+MEMBER_SIZEOF(struct tc_flower_key, ipv4.ipv4_dst)
+}, {
+TCA_PEDIT_KEY_EX_HDR_TYPE_IP4,
+8,
+offsetof(struct tc_flower_key, ipv4.rewrite_ttl),
+MEMBER_SIZEOF(struct tc_flower_key, ipv4.rewrite_ttl)
+}, {
+TCA_PEDIT_KEY_EX_HDR_TYPE_IP6,
+8,
+offsetof(struct tc_flower_key, ipv6.ipv6_src),
+MEMBER_SIZEOF(struct tc_flower_key, ipv6.ipv6_src)
+}, {
+TCA_PEDIT_KEY_EX_HDR_TYPE_IP6,
+24,
+offsetof(struct tc_flower_key, ipv6.ipv6_dst),
+MEMBER_SIZEOF(struct tc_flower_key, ipv6.ipv6_dst)
+}, {
+TCA_PEDIT_KEY_EX_HDR_TYPE_ETH,
+6,
+offsetof(struct tc_flower_key, src_mac),
+MEMBER_SIZEOF(struct tc_flower_key, src_mac)
+}, {
+TCA_PEDIT_KEY_EX_HDR_TYPE_ETH,
+0,
+offsetof(struct tc_flower_key, dst_mac),
+MEMBER_SIZEOF(struct tc_flower_key, dst_mac)
+}, {
+TCA_PEDIT_KEY_EX_HDR_TYPE_ETH,
+12,
+offsetof(struct tc_flower_key, eth_type),
+MEMBER_SIZEOF(struct tc_flower_key, eth_type)
+}, {
+TCA_PEDIT_KEY_EX_HDR_TYPE_TCP,
+0,
+offsetof(struct tc_flower_key, tcp_src),
+MEMBER_SIZEOF(struct tc_flower_key, tcp_src)
+}, {
+TCA_PEDIT_KEY_EX_HDR_TYPE_TCP,
+2,
+offsetof(struct tc_flower_key, tcp_dst),
+MEMBER_SIZEOF(struct tc_flower_key, tcp_dst)
+}, {
+TCA_PEDIT_KEY_EX_HDR_TYPE_UDP,
+0,
+offsetof(struct tc_flower_key, udp_src),
+MEMBER_SIZEOF(struct tc_flower_key, udp_src)
+}, {
+TCA_PEDIT_KEY_EX_HDR_TYPE_UDP,
+2,
+offsetof(struct tc_flower_key, udp_dst),
+MEMBER_SIZEOF(struct tc_flower_key, udp_dst)
+},
+};
+
   struct tcmsg *
   tc_make_request(int ifindex, int type, unsigned int flags,
   struct ofpbuf *request)
@@ -365,6 +446,96 @@ 

Re: [ovs-dev] [PATCH] netdev-dpdk: replace uint8_t with dpdk_port_t

[Ilya Maximets]

Thanks. I wanted to remove this function initially, that's why I forget
to replace the type here.

This is important because dpdk changes the type of port_id to uint16_t
for upcoming release.

Acked-by: Ilya Maximets 

On 20.10.2017 15:37, Mark Kavanagh wrote:
> netdev_dpdk_detach() declares a 'port_id' variable, of type uint8_t.
> This variable should instead be of type dpdk_port_t.
> 
> Fixes: bb37956ac ("netdev-dpdk: Use uint8_t for port_id.")
> CC: Ilya Maximets 
> Signed-off-by: Mark Kavanagh 
> ---
>  lib/netdev-dpdk.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index c60f46f..1f6345d 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -2549,7 +2549,7 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc 
> OVS_UNUSED,
>  {
>  int ret;
>  char *response;
> -uint8_t port_id;
> +dpdk_port_t port_id;
>  char devname[RTE_ETH_NAME_MAX_LEN];
>  struct netdev_dpdk *dev;
> 
> --
> 1.9.3
> 
> 
> 
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev