[ovs-dev] [PATCH net-next v3] openvswitch: Make metadata_dst tunnel work in IP_TUNNEL_INFO_BRIDGE mode

[wenxu]

From: wenxu 

There is currently no support for the multicast/broadcast aspects
of VXLAN in ovs. In the datapath flow the tun_dst must specific.
But in the IP_TUNNEL_INFO_BRIDGE mode the tun_dst can not be specific.
And the packet can forward through the fdb table of vxlan devcice. In
this mode the broadcast/multicast packet can be sent through the
following ways in ovs.

ovs-vsctl add-port br0 vxlan -- set in vxlan type=vxlan \
options:key=1000 options:remote_ip=flow
ovs-ofctl add-flow br0 in_port=LOCAL,dl_dst=ff:ff:ff:ff:ff:ff, \
action=output:vxlan

bridge fdb append ff:ff:ff:ff:ff:ff dev vxlan_sys_4789 dst 172.168.0.1 \
src_vni 1000 vni 1000 self
bridge fdb append ff:ff:ff:ff:ff:ff dev vxlan_sys_4789 dst 172.168.0.2 \
src_vni 1000 vni 1000 self

Signed-off-by: wenxu 
---
 include/uapi/linux/openvswitch.h |  1 +
 net/openvswitch/flow_netlink.c   | 29 +
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index dbe0cbe..b8913b9 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -364,6 +364,7 @@ enum ovs_tunnel_key_attr {
OVS_TUNNEL_KEY_ATTR_IPV6_DST,   /* struct in6_addr dst IPv6 
address. */
OVS_TUNNEL_KEY_ATTR_PAD,
OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS,/* struct erspan_metadata */
+   OVS_TUNNEL_KEY_ATTR_IPV4_INFO_BRIDGE,   /* No argument. 
IPV4_INFO_BRIDGE mode.*/
__OVS_TUNNEL_KEY_ATTR_MAX
 };
 
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index 691da85..ed12b3f 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -403,6 +403,7 @@ size_t ovs_key_attr_size(void)
[OVS_TUNNEL_KEY_ATTR_IPV6_SRC]  = { .len = sizeof(struct in6_addr) 
},
[OVS_TUNNEL_KEY_ATTR_IPV6_DST]  = { .len = sizeof(struct in6_addr) 
},
[OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS]   = { .len = OVS_ATTR_VARIABLE },
+   [OVS_TUNNEL_KEY_ATTR_IPV4_INFO_BRIDGE]   = { .len = 0 },
 };
 
 static const struct ovs_len_tbl
@@ -666,6 +667,7 @@ static int ip_tun_from_nlattr(const struct nlattr *attr,
  bool log)
 {
bool ttl = false, ipv4 = false, ipv6 = false;
+   bool info_bridge_mode = false;
__be16 tun_flags = 0;
int opts_type = 0;
struct nlattr *a;
@@ -782,6 +784,10 @@ static int ip_tun_from_nlattr(const struct nlattr *attr,
tun_flags |= TUNNEL_ERSPAN_OPT;
opts_type = type;
break;
+   case OVS_TUNNEL_KEY_ATTR_IPV4_INFO_BRIDGE:
+   info_bridge_mode = true;
+   ipv4 = true;
+   break;
default:
OVS_NLERR(log, "Unknown IP tunnel attribute %d",
  type);
@@ -812,16 +818,29 @@ static int ip_tun_from_nlattr(const struct nlattr *attr,
OVS_NLERR(log, "IP tunnel dst address not specified");
return -EINVAL;
}
-   if (ipv4 && !match->key->tun_key.u.ipv4.dst) {
-   OVS_NLERR(log, "IPv4 tunnel dst address is zero");
-   return -EINVAL;
+   if (ipv4) {
+   if (info_bridge_mode) {
+   if (match->key->tun_key.u.ipv4.src ||
+   match->key->tun_key.u.ipv4.dst ||
+   match->key->tun_key.tp_src ||
+   match->key->tun_key.tp_dst ||
+   match->key->tun_key.ttl ||
+   match->key->tun_key.tos ||
+   tun_flags & ~TUNNEL_KEY) {
+   OVS_NLERR(log, "IPv4 tun info is not 
correct");
+   return -EINVAL;
+   }
+   } else if (!match->key->tun_key.u.ipv4.dst) {
+   OVS_NLERR(log, "IPv4 tunnel dst address is 
zero");
+   return -EINVAL;
+   }
}
if (ipv6 && ipv6_addr_any(>key->tun_key.u.ipv6.dst)) {
OVS_NLERR(log, "IPv6 tunnel dst address is zero");
return -EINVAL;
}
 
-   if (!ttl) {
+   if (!ttl && !info_bridge_mode) {
OVS_NLERR(log, "IP tunnel TTL not specified.");
return -EINVAL;
}
@@ -2605,6 +2624,8 @@ static int validate_and_copy_set_tun(const struct nlattr 
*attr,
tun_info->mode = IP_TUNNEL_INFO_TX;
if (key.tun_proto == AF_INET6)
tun_info->mode |= IP_TUNNEL_INFO_IPV6;
+   else if (key.tun_proto == AF_INET && 

Re: [ovs-dev] [PATCH 2/2] ovs: tests: Adjust the test cases about group stats

[solomon]

Roi Dayan wrote> 
> 
> Hi,
> 
> the title of the patch has non utf8 chars after the word 'tests'.
> can you check please.
> 
> I see it as
> 
> ovs: tests???Adjust the test cases about group stats

yes, there is a Chinese colon in original title.

> 
> Thanks,
> Roi
> ___
> 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 v4 2/5] ovn: Add generic HA chassis group

[Han Zhou]

Mostly looks good to me, just minor comments.

On Thu, Mar 14, 2019 at 12:32 PM  wrote:
>
> +static void
> +build_lrouter_groups__(struct hmap *ports, struct ovn_datapath *od)
> +{
> +ovs_assert((od && od->nbr && od->lr_group));
> +
> +if (od->l3dgw_port && od->l3redirect_port) {
> +/* It's a logical router with gateway port. If it
> + * has HA_Chassis_Group associated to it in SB DB, then store the
> + * ha chassis group name. */
> +if (od->l3redirect_port->sb->ha_chassis_group) {
> +sset_add(>lr_group->ha_chassis_groups,
> + od->l3redirect_port->sb->ha_chassis_group->name);
> +}
> +}
> +
> +for (size_t i = 0; i < od->nbr->n_ports; i++) {
> +struct ovn_port *router_port =
> +ovn_port_find(ports, od->nbr->ports[i]->name);
> +
> +if (!router_port || !router_port->peer) {
> +continue;
> +}
> +
> +/* Get the peer logical switch/logical router datapath. */
> +struct ovn_datapath *peer_dp = router_port->peer->od;
> +if (peer_dp->nbr) {
> +if (!peer_dp->lr_group) {
> +peer_dp->lr_group = od->lr_group;
> +od->lr_group->router_dps[od->lr_group->n_router_dps++]
> += peer_dp;
> +build_lrouter_groups__(ports, peer_dp);
> +}
> +} else {
> +for (size_t j = 0; j < peer_dp->n_router_ports; j++) {
> +if (!peer_dp->router_ports[j]->peer) {
> +/* If there is no peer port connecting to the
> +* router datapath, ignore it. */

s/router datapath/router port

> +continue;
> +}
> +

> +static void
> +build_lrouter_groups(struct hmap *ports, struct ovs_list *lr_list)
> +{
> +struct ovn_datapath *od;
> +size_t n_router_dps = ovs_list_size(lr_list);
> +
> +LIST_FOR_EACH (od, lr_list, lr_list) {
> +if (!od->lr_group) {
> +od->lr_group = xzalloc(sizeof *od->lr_group);
> +/* Each logical router group can have max
> + * 'n_router_dps'. So allocate enough memory. */
> +od->lr_group->router_dps = xcalloc(sizeof *od, n_router_dps);
> +od->lr_group->router_dps[od->lr_group->n_router_dps] = od;

Here it is more clear to be: od->lr_group->router_dps[0] = od;

> +od->lr_group->n_router_dps = 1;
> +sset_init(>lr_group->ha_chassis_groups);
> +build_lrouter_groups__(ports, od);
> +}
> +}
> +}
> +

>  static void
> -destroy_datapaths_and_ports(struct hmap *datapaths, struct hmap *ports)
> +destroy_datapaths_and_ports(struct hmap *datapaths, struct hmap *ports,
> +struct ovs_list *lr_list)
>  {
> +struct ovn_datapath *router_dp;
> +LIST_FOR_EACH_POP (router_dp, lr_list, lr_list) {
> +if (router_dp->lr_group) {
> +struct lrouter_group *lr_group = router_dp->lr_group;
> +
> +for (size_t i = 0; i < router_dp->lr_group->n_router_dps; i++) {
> +if (router_dp->lr_group->router_dps[i] != router_dp) {

This if-condition seems not needed.

> +router_dp->lr_group->router_dps[i]->lr_group = NULL;
> +}
> +}

s/router_dp->lr_group/lr_group in above for-loop.

> +
> +free(lr_group->router_dps);
> +sset_destroy(_group->ha_chassis_groups);
> +free(lr_group);
> +router_dp->lr_group = NULL;
> +}
> +}
> +

> diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
> index 10a59649a..48d27b960 100644
> --- a/ovn/ovn-nb.ovsschema
> +++ b/ovn/ovn-nb.ovsschema
> @@ -1,7 +1,7 @@
>  {
>  "name": "OVN_Northbound",
> -"version": "5.14.1",
> -"cksum": "3758097843 20509",
> +"version": "5.15.0",
> +"cksum": "1078795414 21917",
>  "tables": {
>  "NB_Global": {
>  "columns": {
> @@ -271,6 +271,12 @@
>   "refType": "strong"},
>   "min": 0,
>   "max": "unlimited"}},
> +"ha_chassis_group": {
> +"type": {"key": {"type": "uuid",
> + "refTable": "HA_Chassis_Group",
> + "refType": "weak"},

Shall this be strong ref instead? In normal case logical ports hosted
by a HA-chassis-group should be deleted before the HA-chassis-group is
removed. (same for SB schema)

> + "min": 0,
> + "max": 1}},
>  "options": {
>  "type": {"key": "string",
>   "value": "string",
> @@ -392,5 +398,29 @@
>  "type": {"key": "string", "value": "string",
>   "min": 0, "max": "unlimited"}}},
>  "indexes": [["name"]],
> -

Re: [ovs-dev] [PATCH] ovs: fix the bug of bucket counter is not updated

[solomon]

Ben Pfaff wrote:
> On Thu, Mar 21, 2019 at 10:41:05AM +0800, solomon wrote:
>> Ben Pfaff wrote:
>>> On Wed, Mar 20, 2019 at 08:16:18PM +0800, Li Wei wrote:

 After inserting/removing a bucket, we don't update the bucket counter.
 When we call ovs-ofctl dump-group-stats br-int, a panic happened.
>>>
>>> Thanks for the patch!  It looks correct to me.  Thank you for adding a
>>> test, too.
>>>
>>> I took a closer look and I saw that 'n_buckets' is not very useful,
>>> because it is only used in cases where the code is already
>>> O(n_buckets).  I think that we can just remove it.  Then it cannot get
>>> out-of-sync.  What do you think of this variation of your patch?
>>
>>
>> ovs_list_size() will traversing the list to get the total length.
>>
>> In our custom scheduling algorithms (eg wrr, least-connection), 
>> we need to know the total number of buckets before traversing the bucket 
>> list to hit target bucket. 
>> so, it is traversed twice.
>>
>> If the number of buckets reaches 100+, there are tens of thousands of 
>> groups, don't this modification affect performance?
>>
>> I hope to keep n_buckets in struct ofgroup.
> 
> OK.
> 
> I applied the original to master and backported as far as branch-2.6.

hi Ben，this patch depends on commit 0b4caa2eba22a516a312e7b404107eaebe618ee1
（ofp-group: support to insert bucket with weight value for select type）on 
branch-2.10.

Could you pick commit 0b4caa2eba22 into brnach-2.10 also？

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

[ovs-dev] [PATCH v2 net-next] net: openvswitch: Add a new action check_pkt_len

[nusiddiq]

From: Numan Siddique 

This patch adds a new action - 'check_pkt_len' which checks the
packet length and executes a set of actions if the packet
length is greater than the specified length or executes
another set of actions if the packet length is lesser or equal to.

This action takes below nlattrs
  * OVS_CHECK_PKT_LEN_ATTR_PKT_LEN - 'pkt_len' to check for

  * OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER - Nested actions
to apply if the packet length is greater than the specified 'pkt_len'

  * OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL - Nested
actions to apply if the packet length is lesser or equal to the
specified 'pkt_len'.

The main use case for adding this action is to solve the packet
drops because of MTU mismatch in OVN virtual networking solution.
When a VM (which belongs to a logical switch of OVN) sends a packet
destined to go via the gateway router and if the nic which provides
external connectivity, has a lesser MTU, OVS drops the packet
if the packet length is greater than this MTU.

With the help of this action, OVN will check the packet length
and if it is greater than the MTU size, it will generate an
ICMP packet (type 3, code 4) and includes the next hop mtu in it
so that the sender can fragment the packets.

Reported-at:
https://mail.openvswitch.org/pipermail/ovs-discuss/2018-July/047039.html
Suggested-by: Ben Pfaff 
Signed-off-by: Numan Siddique 
CC: Gregory Rose 
CC: Pravin B Shelar 
---
v1 -> v2
-
   * Addressed the review comments.
 - Removed the vlan-tag length when checking the packet length
 - Reordered the netlink attributes
 - Changed the comments to use 'attribute' instead of 'action'

Corresponding OVS patch (submitted as RFC for now)  which makes use of this 
action can be
found here - https://patchwork.ozlabs.org/patch/1059081/

 include/uapi/linux/openvswitch.h |  42 
 net/openvswitch/actions.c|  48 +
 net/openvswitch/flow_netlink.c   | 171 +++
 3 files changed, 261 insertions(+)

diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index dbe0cbe4f1b7..dfabacee6903 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -798,6 +798,44 @@ struct ovs_action_push_eth {
struct ovs_key_ethernet addresses;
 };
 
+/*
+ * enum ovs_check_pkt_len_attr - Attributes for %OVS_ACTION_ATTR_CHECK_PKT_LEN.
+ *
+ * @OVS_CHECK_PKT_LEN_ATTR_PKT_LEN: u16 Packet length to check for.
+ * @OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER: Nested OVS_ACTION_ATTR_*
+ * actions to apply if the packer length is greater than the specified
+ * length in the attr - OVS_CHECK_PKT_LEN_ATTR_PKT_LEN.
+ * @OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL - Nested OVS_ACTION_ATTR_*
+ * actions to apply if the packer length is lesser or equal to the specified
+ * length in the attr - OVS_CHECK_PKT_LEN_ATTR_PKT_LEN.
+ */
+enum ovs_check_pkt_len_attr {
+   OVS_CHECK_PKT_LEN_ATTR_UNSPEC,
+   OVS_CHECK_PKT_LEN_ATTR_PKT_LEN,
+   OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER,
+   OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL,
+   __OVS_CHECK_PKT_LEN_ATTR_MAX,
+
+#ifdef __KERNEL__
+   OVS_CHECK_PKT_LEN_ATTR_ARG  /* struct check_pkt_len_arg  */
+#endif
+};
+
+#define OVS_CHECK_PKT_LEN_ATTR_MAX (__OVS_CHECK_PKT_LEN_ATTR_MAX - 1)
+
+#ifdef __KERNEL__
+struct check_pkt_len_arg {
+   u16 pkt_len;/* Same value as OVS_CHECK_PKT_LEN_ATTR_PKT_LEN'. */
+   bool exec_for_greater;  /* When true, actions in IF_GREATER will
+* not change flow keys. False otherwise.
+*/
+   bool exec_for_lesser_equal; /* When true, actions in IF_LESS_EQUAL
+* will not change flow keys. False
+* otherwise.
+*/
+};
+#endif
+
 /**
  * enum ovs_action_attr - Action types.
  *
@@ -842,6 +880,9 @@ struct ovs_action_push_eth {
  * packet, or modify the packet (e.g., change the DSCP field).
  * @OVS_ACTION_ATTR_CLONE: make a copy of the packet and execute a list of
  * actions without affecting the original packet and key.
+ * @OVS_ACTION_ATTR_CHECK_PKT_LEN: Check the packet length and execute a set
+ * of actions if greater than the specified packet length, else execute
+ * another set of actions.
  *
  * Only a single header can be set with a single %OVS_ACTION_ATTR_SET.  Not all
  * fields within a header are modifiable, e.g. the IPv4 protocol and fragment
@@ -876,6 +917,7 @@ enum ovs_action_attr {
OVS_ACTION_ATTR_POP_NSH,  /* No argument. */
OVS_ACTION_ATTR_METER,/* u32 meter ID. */
OVS_ACTION_ATTR_CLONE,/* Nested OVS_CLONE_ATTR_*.  */
+   OVS_ACTION_ATTR_CHECK_PKT_LEN, /* Nested OVS_CHECK_PKT_LEN_ATTR_*. */
 
__OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
   * from userspace. */
diff --git 

[ovs-dev] [PATCH] hmap: Improve debug log message when reporting unusually large buckets.

[Ben Pfaff]

I was seeing a lot of these messages, including a lot of them suppressed
by rate-limiting, and I wondered whether any really big messages were
being suppressed.  By reporting the largest bucket, instead of just every
large bucket, it becomes more likely that the truly too-large buckets get
reported.

(The problem I saw was a false alarm.)

Signed-off-by: Ben Pfaff 
---
 lib/hmap.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/lib/hmap.c b/lib/hmap.c
index 1ba4a5716a78..20161698af5d 100644
--- a/lib/hmap.c
+++ b/lib/hmap.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2012, 2013, 2015 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2012, 2013, 2015, 2019 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -103,6 +103,7 @@ resize(struct hmap *hmap, size_t new_mask, const char 
*where)
 tmp.buckets[i] = NULL;
 }
 }
+int max_count = 0;
 for (i = 0; i <= hmap->mask; i++) {
 struct hmap_node *node, *next;
 int count = 0;
@@ -111,15 +112,20 @@ resize(struct hmap *hmap, size_t new_mask, const char 
*where)
 hmap_insert_fast(, node, node->hash);
 count++;
 }
-if (count > 5) {
-static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(10, 10);
-COVERAGE_INC(hmap_pathological);
-VLOG_DBG_RL(, "%s: %d nodes in bucket (%"PRIuSIZE" nodes, 
%"PRIuSIZE" buckets)",
-where, count, hmap->n, hmap->mask + 1);
+if (count > max_count) {
+max_count = count;
 }
 }
 hmap_swap(hmap, );
 hmap_destroy();
+
+if (max_count > 5) {
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(10, 10);
+COVERAGE_INC(hmap_pathological);
+VLOG_DBG_RL(, "%s: %d nodes in bucket "
+"(%"PRIuSIZE" nodes, %"PRIuSIZE" buckets)",
+where, max_count, hmap->n, hmap->mask + 1);
+}
 }
 
 static size_t
-- 
2.20.1

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

Re: [ovs-dev] [PATCH V7 2/2] odp-util: Do not rewrite fields with the same values as matched

[Ben Pfaff]

On Thu, Mar 21, 2019 at 07:44:16AM +, Eli Britstein wrote:
> To improve performance and avoid wasting resources for HW offloaded
> flows, do not rewrite fields that are matched with the same value.
> 
> Signed-off-by: Eli Britstein 
> Reviewed-by: Roi Dayan 

Thanks.  I applied this series to master.

I folded in an improvement to the comment on keycmp_mask() and minor
style fixes to the same function:

diff --git a/lib/odp-util.c b/lib/odp-util.c
index 291c05f84a48..b6552c5c208a 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -7367,29 +7367,27 @@ struct offsetof_sizeof {
 int size;
 };
 
-/* Compare the keys similary to memcmp, but each field separately.
- * The offsets and sizes of each field is provided by offsetof_sizeof_arr
- * argument.
- * For fields with the same value, zero out their mask part in order not to
- * rewrite them as it's unnecessary */
+/* Compares each of the fields in 'key0' and 'key1'.  The fields are specified
+ * in 'offsetof_sizeof_arr', which is an array terminated by a 0-size field.
+ * Returns true if all of the fields are equal, false if at least one differs.
+ * As a side effect, for each field that is the same in 'key0' and 'key1',
+ * zeros the corresponding bytes in 'mask'. */
 static bool
 keycmp_mask(const void *key0, const void *key1,
 struct offsetof_sizeof *offsetof_sizeof_arr, void *mask)
 {
-int field;
 bool differ = false;
 
-for (field = 0 ; ; field++) {
+for (int field = 0 ; ; field++) {
 int size = offsetof_sizeof_arr[field].size;
 int offset = offsetof_sizeof_arr[field].offset;
-char *pkey0 = ((char *)key0) + offset;
-char *pkey1 = ((char *)key1) + offset;
-char *pmask = ((char *)mask) + offset;
-
 if (size == 0) {
 break;
 }
 
+char *pkey0 = ((char *)key0) + offset;
+char *pkey1 = ((char *)key1) + offset;
+char *pmask = ((char *)mask) + offset;
 if (memcmp(pkey0, pkey1, size) == 0) {
 memset(pmask, 0, size);
 } else {
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v5] ovsdb raft: Sync commit index to followers without delay.

[Han Zhou]

On Mon, Mar 25, 2019 at 2:00 PM Ben Pfaff  wrote:
>
> On Sat, Mar 23, 2019 at 09:44:26AM -0700, Han Zhou wrote:
> > From: Han Zhou 
> >
> > When update is requested from follower, the leader sends AppendRequest
> > to all followers and wait until AppendReply received from majority, and
> > then it will update commit index - the new entry is regarded as committed
> > in raft log. However, this commit will not be notified to followers
> > (including the one initiated the request) until next heartbeat (ping
> > timeout), if no other pending requests. This results in long latency
> > for updates made through followers, especially when a batch of updates
> > are requested through the same follower.
>
> The tests pass now, but each one of them ends up with several ovn-sbctl
> and ovsdb-server processes all trying to use 100% of a CPU.  If I run
> the tests with 10-way parallelism, the load average goes above 100.
> Surely something has to be wrong in the implementation here.
>
> Each of the ovn-sbctl processes is trying to push through only a single
> transaction; after that, it exits.  If we think of ovsdb-server as
> giving each of its clients one chance to execute an RPC in round-robin
> order, which is approximately correct, then one of those transactions
> should succeed per round.  I don't understand why, if this model is
> correct, the ovn-sbctls would burn so much CPU.  If the model is wrong,
> then we need to understand why it is wrong and how we can fix it.  Maybe
> the ovn-sbctl processes are retrying blindly without waiting for an
> update from the server; if so, that's a bug and it should be fixed.

Hi Ben, to make the test case more effective, in v4/v5 I enlarged the
size of each transaction 50 times, specified by the var "n3". This is
just to create the load in test and slow down the transaction
execution, to ensure there are enough parallel requests ongoing, and
to ensure we can test the server membership changes during this
period. Without this change (in test case only), the CPU was low, but
it just making the torture test not torturing at all (either skipped
because the execution was too fast, or if put sleep there it loses the
parallelism). And the test case change was proved to be more effective
- it found an existing bug, as mentioned in the notes.

If your question is why increasing the size of transaction causing
high CPU load, it seems expected if we see what's being done by each
client:
- download initial data, which increases as more and more transaction
executed, each adding 50 entries (10 * 5 * 50 = 2500), later clients
will take more CPU because of this.
- send transaction with operations: wait + update, which has double
size of the original data, plus newly added 50 entries.
- there are some chances that clients got conflict update (wait
operation fails), which triggers retry. This doesn't happen a lot (by
checking the logs), and I think it is expected due to the purpose of
the testing - to increase parallelism.
- process updates, at least once for its own update, but also could
process other's update. (This step is not CPU costly since each update
notification is not big)

So I think increasing the size of transaction causing higher CPU is
expected due to these operations, considering the JsonRPC cost. I ran
the tests a lot a times and I didn't observe any client occupying CPU
for long time - there were short spikes (not 100%) shown by "top", but
I never see anyone there twice. If you see any behavior like it is
continously retrying, could you please share the x-y.log file of that
client?

I agree there could be improvement to the RPC performance of
ovsdb/IDL, but it is not related to this patch (or maybe we can say
the test in this patch utilized this slowness).

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

[ovs-dev] Camaras de seguridad y reloj checadores a el mejor precio

[Seguridad Patrimonial]

 




 










Promoción Cámaras de Seguridad, 8 Cámaras de Seguridad todo Incluido.

No Incluye Instalación

















Precio Original$10,953.00

20%

Costo Actual $8,764.80




Sistema TURBOHD 1080p / DVR 8 Canales / 8 Cámaras Bala (exterior 2.8 mm) / 
Transceptores / Conectores / Fuente de Poder Profesional hasta 15 Vcd para 
Larga Distancia










Funciones principales






















































Otras Ofertas

















Lector De Huella Con Teclado Para Control De Asistencia, 500 Huellas, Genera 
Reporte Por USB En Excel, Descarga Mediante Memoria USB

$1,059.75

 


















Checador Biométrico / Reconocimiento Facial / Huella / Control de Acceso 1 
Puerta / 1500 Rostros / 2000 Huellas / Incluye Función ADMS

$2,433.40

 









Contactanos:
i...@seguridadingenia.com
CdMx: 01 558 751 5260
Monterrey 01 812 968 1730
Guadalajara 01 332 712 8183
Toluca 01 722 917 7930
Puebla ??01 222 941 7215??
Veracruz 2293865465
Xalapa 2283527000
Merida  9995654986
Cancun 9983617832
Playa del Carmen 9842220439
Orizaba 2722010012
Queretaro 4426299047
San Luis 4446033106
Saltillo 8443781055
Chihuahua 6145667468
Tijuana 6648389220
Cabo San lucas 6242327243
Aguascalientes 4494734128
Celaya 4612871946
Acapulco 7443300895
Hermosillo 6624376077
Pachuca 7714730696
Cuernavaca 7776086307
Ensenada 6462641969
Cd Juarez 6567905254

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

Re: [ovs-dev] [PATCH 0/3] OVN: Fixes regarding RA prefixes

[Ben Pfaff]

On Mon, Mar 25, 2019 at 05:29:53PM -0400, Mark Michelson wrote:
> This is a series of patches that fix some errors with router
> advertisements. Each individual patch will explain the issues more
> completely.
> 
> This patch series is built against OVS master, but it will need to be
> backported as far back as 2.9

Thanks.  I applied this and backported it as far as 2.9.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 0/2] ovn-controller: Add a new thread in pinctrl module to process packet-ins

[Ben Pfaff]

On Sat, Mar 16, 2019 at 11:27:06AM +0530, nusid...@redhat.com wrote:
> From: Numan Siddique 
> 
> This series attempts to add a new thread in pinctrl module. This thread
> will handle the packet-ins.
> 
> 
> v2 -> v3
> -
>* Fixed the clang errors.
> 
> v1 -> v2
> --
>   * Added a new patch p1 to the series suggessted by Mark.
>   * Addressed the review comments from Han and Mark.

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

[ovs-dev] [PATCH 3/3] OVN: Make periodic RAs consistent with RA responder.

[Mark Michelson]

This commit makes periodic RAs from OVN consistent with the RAs sent in
response to RSs. Specifically, this ensures that prefix flags are set
correctly for each address mode.

This commit also gets rid of some redundant definitions for RA prefix
option flags from packets.h in favor of the ones in ovn-l7.h.

Signed-off-by: Mark Michelson 
---
 lib/packets.h| 3 ---
 ovn/controller/pinctrl.c | 5 +++--
 tests/ovn.at | 2 +-
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/lib/packets.h b/lib/packets.h
index e20a70a66..d293b3542 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -978,9 +978,6 @@ struct ovs_nd_prefix_opt {
 };
 BUILD_ASSERT_DECL(ND_PREFIX_OPT_LEN == sizeof(struct ovs_nd_prefix_opt));
 
-#define ND_PREFIX_ON_LINK0x80
-#define ND_PREFIX_AUTONOMOUS_ADDRESS 0x40
-
 /* Neighbor Discovery option: MTU. */
 #define ND_MTU_OPT_LEN 8
 #define ND_MTU_DEFAULT 0
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 100a20ff2..91952700d 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -1638,7 +1638,7 @@ ipv6_ra_update_config(const struct sbrec_port_binding *pb)
 config->min_interval = smap_get_int(>options, "ipv6_ra_min_interval",
 nd_ra_min_interval_default(config->max_interval));
 config->mtu = smap_get_int(>options, "ipv6_ra_mtu", ND_MTU_DEFAULT);
-config->la_flags = ND_PREFIX_ON_LINK;
+config->la_flags = IPV6_ND_RA_OPT_PREFIX_ON_LINK;
 
 const char *address_mode = smap_get(>options, "ipv6_ra_address_mode");
 if (!address_mode) {
@@ -1647,10 +1647,11 @@ ipv6_ra_update_config(const struct sbrec_port_binding 
*pb)
 }
 if (!strcmp(address_mode, "dhcpv6_stateless")) {
 config->mo_flags = IPV6_ND_RA_FLAG_OTHER_ADDR_CONFIG;
+config->la_flags |= IPV6_ND_RA_OPT_PREFIX_AUTONOMOUS;
 } else if (!strcmp(address_mode, "dhcpv6_stateful")) {
 config->mo_flags = IPV6_ND_RA_FLAG_MANAGED_ADDR_CONFIG;
 } else if (!strcmp(address_mode, "slaac")) {
-config->la_flags |= ND_PREFIX_AUTONOMOUS_ADDRESS;
+config->la_flags |= IPV6_ND_RA_OPT_PREFIX_AUTONOMOUS;
 } else {
 VLOG_WARN("Invalid address mode %s", address_mode);
 goto fail;
diff --git a/tests/ovn.at b/tests/ovn.at
index 069d3bb30..e7746cb0f 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -10646,7 +10646,7 @@ ra_test 05dc 80 80 40 
aef0 30 fd0f00
 
 # And the other address mode
 ovn-nbctl --wait=hv set Logical_Router_Port ro-sw 
ipv6_ra_configs:address_mode=dhcpv6_stateless
-ra_test 05dc 40 80 40 aef0 30 
fd0f
+ra_test 05dc 40 c0 40 aef0 30 
fd0f
 
 OVN_CLEANUP([hv1],[hv2])
 AT_CLEANUP
-- 
2.14.5

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

[ovs-dev] [PATCH 2/3] OVN: Always send prefix option in RAs

[Mark Michelson]

OVN's behavior when sending router advertisements has been to include IP
prefix information only if the address mode is set to "slaac" or
"dhcp_stateless". In these modes, sending the prefix to the client is
necessary so that it may automatically provision its IP address. We do
not send the prefix option when the address mode is set to
"dhcp_stateful" since there is no need for the client to automatically
provision an IP address.

This logic is flawed, however. When using dhcp_stateful, we provide a
managed IPv6 address for a client. However, because we do not provide
prefix information in our RAs, the client does not know the prefix
length for the address it has been allocated. With dhclient, we have
seen it assume either /64 or /128, depending on which version is being
used. This may not accurately reflect the prefix length being used by
the DHCP server though.

The fix here is to always send prefix information in our RAs, regardless
of address mode. The key difference lies in how we set the A
(autonomous addressing) flag. For slaac and dhcp_stateless address
modes, we will set this flag, indicating the client should provision its
own address based on the prefix we have sent. For dhcp_stateful, we will
not set this flag. This way, it is clear the prefix is informational,
and the client should not try to provision its own IPv6 address.

Signed-off-by: Mark Michelson 
---
 ovn/lib/actions.c   | 12 +---
 ovn/lib/ovn-l7.h|  3 ++-
 ovn/northd/ovn-northd.c | 11 ---
 tests/ovn.at|  8 +---
 4 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 56e1ab2ae..eb7e5badd 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -1954,12 +1954,6 @@ parse_put_nd_ra_opts(struct action_context *ctx, const 
struct expr_field *dst,
 return;
 }
 
-if (addr_mode_stateful && prefix_set) {
-lexer_error(ctx->lexer, "prefix option can't be"
-" set when address mode is dhcpv6_stateful.");
-return;
-}
-
 if (!addr_mode_stateful && !prefix_set) {
 lexer_error(ctx->lexer, "prefix option needs "
 "to be set when address mode is slaac/dhcpv6_stateless.");
@@ -2020,10 +2014,14 @@ encode_put_nd_ra_option(const struct ovnact_gen_option 
*o,
 struct ovs_nd_prefix_opt *prefix_opt =
 ofpbuf_put_uninit(ofpacts, sizeof *prefix_opt);
 uint8_t prefix_len = ipv6_count_cidr_bits(>mask.ipv6);
+struct ovs_ra_msg *ra = ofpbuf_at(ofpacts, ra_offset, sizeof *ra);
 prefix_opt->type = ND_OPT_PREFIX_INFORMATION;
 prefix_opt->len = 4;
 prefix_opt->prefix_len = prefix_len;
-prefix_opt->la_flags = IPV6_ND_RA_OPT_PREFIX_FLAGS;
+prefix_opt->la_flags = IPV6_ND_RA_OPT_PREFIX_ON_LINK;
+if (!(ra->mo_flags & IPV6_ND_RA_FLAG_MANAGED_ADDR_CONFIG)) {
+prefix_opt->la_flags |= IPV6_ND_RA_OPT_PREFIX_AUTONOMOUS;
+}
 put_16aligned_be32(_opt->valid_lifetime,
htonl(IPV6_ND_RA_OPT_PREFIX_VALID_LIFETIME));
 put_16aligned_be32(_opt->preferred_lifetime,
diff --git a/ovn/lib/ovn-l7.h b/ovn/lib/ovn-l7.h
index fe3104e1f..c24201ef0 100644
--- a/ovn/lib/ovn-l7.h
+++ b/ovn/lib/ovn-l7.h
@@ -258,7 +258,8 @@ nd_ra_opts_destroy(struct hmap *nd_ra_opts)
 #define IPV6_ND_RA_REACHABLE_TIME   0
 #define IPV6_ND_RA_RETRANSMIT_TIMER 0
 
-#define IPV6_ND_RA_OPT_PREFIX_FLAGS 0xc0
+#define IPV6_ND_RA_OPT_PREFIX_ON_LINK   0x80
+#define IPV6_ND_RA_OPT_PREFIX_AUTONOMOUS0x40
 #define IPV6_ND_RA_OPT_PREFIX_VALID_LIFETIME0x
 #define IPV6_ND_RA_OPT_PREFIX_PREFERRED_LIFETIME0x
 
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 2843969f4..05b8aad4f 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -6312,13 +6312,10 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
 continue;
 }
 
-/* Add the prefix option if the address mode is slaac or
- * dhcpv6_stateless. */
-if (strcmp(address_mode, "dhcpv6_stateful")) {
-ds_put_format(, ", prefix = %s/%u",
-  op->lrp_networks.ipv6_addrs[i].network_s,
-  op->lrp_networks.ipv6_addrs[i].plen);
-}
+ds_put_format(, ", prefix = %s/%u",
+  op->lrp_networks.ipv6_addrs[i].network_s,
+  op->lrp_networks.ipv6_addrs[i].plen);
+
 add_rs_response_flow = true;
 }
 
diff --git a/tests/ovn.at b/tests/ovn.at
index f2f2bc405..069d3bb30 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1260,9 +1260,11 @@ reg1[0] = put_nd_ra_opts(addr_mode = "dhcpv6_stateless", 
slla = ae:01:02:03:04:0
 reg1[0] = put_nd_ra_opts(addr_mode = "slaac", mtu = 1500, prefix = aef0::/64);
 slla option 

[ovs-dev] [PATCH 1/3] OVN: Use offset instead of pointer into ofpbuf

[Mark Michelson]

In general, maintaining a pointer into an ofpbuf is risky. As the ofpbuf
grows, it can reallocate its data. If this happens, then pointers into
the data will become invalid.

A safer practice is to track an offset into the ofpbuf's data where a
structure you are interested in is kept. This way, if the ofpbuf data is
reallocated, you can find your structure again by using the offset.

In practice, this patch is not fixing any issues with OVN. Even though
the ra pointer is pointing to ofpbuf data that can be reallocated, it
will never actually happen. ovn-northd and all test cases always encode
the address mode first, meaning we will only ever read from the ra
pointer before the ofpbuf has a chance to expand.

However, this base work is essential for an upcoming patch in this series.

Signed-off-by: Mark Michelson 
---
 ovn/lib/actions.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 7b7a89478..56e1ab2ae 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -1978,18 +1978,21 @@ format_PUT_ND_RA_OPTS(const struct ovnact_put_opts *po,
 
 static void
 encode_put_nd_ra_option(const struct ovnact_gen_option *o,
-struct ofpbuf *ofpacts, struct ovs_ra_msg *ra)
+struct ofpbuf *ofpacts, ptrdiff_t ra_offset)
 {
 const union expr_constant *c = o->value.values;
 
 switch (o->option->code) {
 case ND_RA_FLAG_ADDR_MODE:
+{
+struct ovs_ra_msg *ra = ofpbuf_at(ofpacts, ra_offset, sizeof *ra);
 if (!strcmp(c->string, "dhcpv6_stateful")) {
 ra->mo_flags = IPV6_ND_RA_FLAG_MANAGED_ADDR_CONFIG;
 } else if (!strcmp(c->string, "dhcpv6_stateless")) {
 ra->mo_flags = IPV6_ND_RA_FLAG_OTHER_ADDR_CONFIG;
 }
 break;
+}
 
 case ND_OPT_SOURCE_LINKADDR:
 {
@@ -2051,6 +2054,7 @@ encode_PUT_ND_RA_OPTS(const struct ovnact_put_opts *po,
  * pinctrl module receives the ICMPv6 Router Solicitation packet
  * it can copy the userdata field AS IS and resume the packet.
  */
+size_t ra_offset = ofpacts->size;
 struct ovs_ra_msg *ra = ofpbuf_put_zeros(ofpacts, sizeof *ra);
 ra->icmph.icmp6_type = ND_ROUTER_ADVERT;
 ra->cur_hop_limit = IPV6_ND_RA_CUR_HOP_LIMIT;
@@ -2059,7 +2063,7 @@ encode_PUT_ND_RA_OPTS(const struct ovnact_put_opts *po,
 
 for (const struct ovnact_gen_option *o = po->options;
  o < >options[po->n_options]; o++) {
-encode_put_nd_ra_option(o, ofpacts, ra);
+encode_put_nd_ra_option(o, ofpacts, ra_offset);
 }
 
 encode_finish_controller_op(oc_offset, ofpacts);
-- 
2.14.5

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

[ovs-dev] [PATCH 0/3] OVN: Fixes regarding RA prefixes

[Mark Michelson]

This is a series of patches that fix some errors with router
advertisements. Each individual patch will explain the issues more
completely.

This patch series is built against OVS master, but it will need to be
backported as far back as 2.9

Mark Michelson (3):
  OVN: Use offset instead of pointer into ofpbuf
  OVN: Always send prefix option in RAs
  OVN: Make periodic RAs consistent with RA responder.

 lib/packets.h|  3 ---
 ovn/controller/pinctrl.c |  5 +++--
 ovn/lib/actions.c| 20 +++-
 ovn/lib/ovn-l7.h |  3 ++-
 ovn/northd/ovn-northd.c  | 11 ---
 tests/ovn.at | 10 ++
 6 files changed, 26 insertions(+), 26 deletions(-)

-- 
2.14.5

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

Re: [ovs-dev] [patch v1] tests: Remove maximum version kernel check.

[Ben Pfaff]

On Sat, Mar 16, 2019 at 11:50:24AM -0700, Darrell Ball wrote:
> The macro 'OVS_CHECK_KERNEL' was checking for maximum supported kernel
> version.  This means checks like 'OVS_CHECK_KERNEL(3, 10, 4, 18)'
> in various tests need to be updated when each new kernel version is
> supported. This is unnecessary as these tests are expected to continue
> to work in later kernel versions.
> 
> Fix this by changing the macro to check for minimum version only and
> update the macro name accordingly.
> 
> This patch does not make any possible corrections to the indicated minimum
> kernel version specified by the callers of the macro.
> 
> Signed-off-by: Darrell Ball 

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

Re: [ovs-dev] [PATCH v5] ovsdb raft: Sync commit index to followers without delay.

[Ben Pfaff]

On Sat, Mar 23, 2019 at 09:44:26AM -0700, Han Zhou wrote:
> From: Han Zhou 
> 
> When update is requested from follower, the leader sends AppendRequest
> to all followers and wait until AppendReply received from majority, and
> then it will update commit index - the new entry is regarded as committed
> in raft log. However, this commit will not be notified to followers
> (including the one initiated the request) until next heartbeat (ping
> timeout), if no other pending requests. This results in long latency
> for updates made through followers, especially when a batch of updates
> are requested through the same follower.

The tests pass now, but each one of them ends up with several ovn-sbctl
and ovsdb-server processes all trying to use 100% of a CPU.  If I run
the tests with 10-way parallelism, the load average goes above 100.
Surely something has to be wrong in the implementation here.

Each of the ovn-sbctl processes is trying to push through only a single
transaction; after that, it exits.  If we think of ovsdb-server as
giving each of its clients one chance to execute an RPC in round-robin
order, which is approximately correct, then one of those transactions
should succeed per round.  I don't understand why, if this model is
correct, the ovn-sbctls would burn so much CPU.  If the model is wrong,
then we need to understand why it is wrong and how we can fix it.  Maybe
the ovn-sbctl processes are retrying blindly without waiting for an
update from the server; if so, that's a bug and it should be fixed.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovs: fix the bug of bucket counter is not updated

[Ben Pfaff]

On Thu, Mar 21, 2019 at 10:41:05AM +0800, solomon wrote:
> Ben Pfaff wrote:
> > On Wed, Mar 20, 2019 at 08:16:18PM +0800, Li Wei wrote:
> >>
> >> After inserting/removing a bucket, we don't update the bucket counter.
> >> When we call ovs-ofctl dump-group-stats br-int, a panic happened.
> > 
> > Thanks for the patch!  It looks correct to me.  Thank you for adding a
> > test, too.
> > 
> > I took a closer look and I saw that 'n_buckets' is not very useful,
> > because it is only used in cases where the code is already
> > O(n_buckets).  I think that we can just remove it.  Then it cannot get
> > out-of-sync.  What do you think of this variation of your patch?
> 
> 
> ovs_list_size() will traversing the list to get the total length.
> 
> In our custom scheduling algorithms (eg wrr, least-connection), 
> we need to know the total number of buckets before traversing the bucket list 
> to hit target bucket. 
> so, it is traversed twice.
> 
> If the number of buckets reaches 100+, there are tens of thousands of groups, 
> don't this modification affect performance?
> 
> I hope to keep n_buckets in struct ofgroup.

OK.

I applied the original to master and backported as far as branch-2.6.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] explanation and workaround (was: Re: OVS mailing lists *sometimes* replace the From: address by the list address)

[Ben Pfaff]

I obtained an explanation from LF about this issue.  It is not due to an
mailing list configuration change.  It results from DMARC, which is a
setting for email sender domains that causes receivers to reject email
that is allegedly from the domain if it cannot be verified that it
really came from it.  Since mail to mailing lists break these rules,
Mailman and other mailing list software rewrites From headers with DMARC
senders so that the messages do not appear to originate from them.
Otherwise, the receiver would probably discard the email, since it
breaks the DMARC rules.

The most likely reason that we are seeing this often now is that some
new domains have turned on DMARC.

We can't do anything about this directly, because we don't control DMARC
on senders' domains and we don't control email processing on receivers.

I wrote the following script to un-rewrite the From: header before
passing it to git-am.  It isn't perfect but it worked on the few
examples I tried.

#! /bin/sh
tmp=$(mktemp)
cat >$tmp
if grep '^From:.*via dev.*' "$tmp" >/dev/null 2>&1; then
   sed '/^From:.*via dev.*/d
s/^[Rr]eply-[tT]o:/From:/' $tmp
else
   cat "$tmp"
fi | git am "$@"
rm "$tmp"
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH net-next v2] openvswitch: Make metadata_dst tunnel work in IP_TUNNEL_INFO_BRIDGE mode

[Pravin Shelar]

On Sat, Mar 23, 2019 at 4:03 AM  wrote:
>
> From: wenxu 
>
> There is currently no support for the multicast/broadcast aspects
> of VXLAN in ovs. In the datapath flow the tun_dst must specific.
> But in the IP_TUNNEL_INFO_BRIDGE mode the tun_dst can not be specific.
> And the packet can forward through the fdb table of vxlan devcice. In
> this mode the broadcast/multicast packet can be sent through the
> following ways in ovs.
>
> ovs-vsctl add-port br0 vxlan -- set in vxlan type=vxlan \
> options:key=1000 options:remote_ip=flow
> ovs-ofctl add-flow br0 in_port=LOCAL,dl_dst=ff:ff:ff:ff:ff:ff,\
> action=output:vxlan
>
> bridge fdb append ff:ff:ff:ff:ff:ff dev vxlan_sys_4789 dst 172.168.0.1 \
> src_vni 1000 vni 1000 self
> bridge fdb append ff:ff:ff:ff:ff:ff dev vxlan_sys_4789 dst 172.168.0.2 \
> src_vni 1000 vni 1000 self
>
> Signed-off-by: wenxu 
> ---
>  include/uapi/linux/openvswitch.h |  1 +
>  net/openvswitch/flow_netlink.c   | 19 ---
>  2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/include/uapi/linux/openvswitch.h 
> b/include/uapi/linux/openvswitch.h
> index dbe0cbe..696a308 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -364,6 +364,7 @@ enum ovs_tunnel_key_attr {
> OVS_TUNNEL_KEY_ATTR_IPV6_DST,   /* struct in6_addr dst IPv6 
> address. */
> OVS_TUNNEL_KEY_ATTR_PAD,
> OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS,/* struct erspan_metadata */
> +   OVS_TUNNEL_KEY_ATTR_NO_IPV4_DST,/* No argument. No dst IP 
> address. */
This should be explicitly named to indicate its IP_TUNNEL_INFO_BRIDGE mode.

> __OVS_TUNNEL_KEY_ATTR_MAX
>  };
>
> diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
> index 691da85..7abea44 100644
> --- a/net/openvswitch/flow_netlink.c
> +++ b/net/openvswitch/flow_netlink.c
> @@ -403,6 +403,7 @@ size_t ovs_key_attr_size(void)
> [OVS_TUNNEL_KEY_ATTR_IPV6_SRC]  = { .len = sizeof(struct 
> in6_addr) },
> [OVS_TUNNEL_KEY_ATTR_IPV6_DST]  = { .len = sizeof(struct 
> in6_addr) },
> [OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS]   = { .len = OVS_ATTR_VARIABLE },
> +   [OVS_TUNNEL_KEY_ATTR_NO_IPV4_DST]   = { .len = 0 },
>  };
>
>  static const struct ovs_len_tbl
> @@ -666,6 +667,7 @@ static int ip_tun_from_nlattr(const struct nlattr *attr,
>   bool log)
>  {
> bool ttl = false, ipv4 = false, ipv6 = false;
> +   bool no_ipv4_dst = false;
> __be16 tun_flags = 0;
> int opts_type = 0;
> struct nlattr *a;
> @@ -782,6 +784,10 @@ static int ip_tun_from_nlattr(const struct nlattr *attr,
> tun_flags |= TUNNEL_ERSPAN_OPT;
> opts_type = type;
> break;
> +   case OVS_TUNNEL_KEY_ATTR_NO_IPV4_DST:
> +   no_ipv4_dst = true;
> +   ipv4 = true;
> +   break;
> default:
> OVS_NLERR(log, "Unknown IP tunnel attribute %d",
>   type);
> @@ -812,9 +818,14 @@ static int ip_tun_from_nlattr(const struct nlattr *attr,
> OVS_NLERR(log, "IP tunnel dst address not specified");
> return -EINVAL;
> }
> -   if (ipv4 && !match->key->tun_key.u.ipv4.dst) {
> -   OVS_NLERR(log, "IPv4 tunnel dst address is zero");
> -   return -EINVAL;
> +   if (ipv4) {
> +   if (no_ipv4_dst && match->key->tun_key.u.ipv4.dst) {
> +   OVS_NLERR(log, "IPv4 tunnel dst address is 
> not zero");
> +   return -EINVAL;
I am not sure why dst-ip is only validated here, in
IP_TUNNEL_INFO_BRIDGE mode all parameters except VNI are ignored.
either we need to check entire tunnel key or skip check for dst-ip, I
would prefer checking entire tun-key.


> +   } else if (!no_ipv4_dst && 
> !match->key->tun_key.u.ipv4.dst) {
> +   OVS_NLERR(log, "IPv4 tunnel dst address is 
> zero");
> +   return -EINVAL;
> +   }
> }
> if (ipv6 && ipv6_addr_any(>key->tun_key.u.ipv6.dst)) {
> OVS_NLERR(log, "IPv6 tunnel dst address is zero");
> @@ -2605,6 +2616,8 @@ static int validate_and_copy_set_tun(const struct 
> nlattr *attr,
> tun_info->mode = IP_TUNNEL_INFO_TX;
> if (key.tun_proto == AF_INET6)
> tun_info->mode |= IP_TUNNEL_INFO_IPV6;
> +   else if (key.tun_proto == AF_INET && key.tun_key.u.ipv4.dst == 0)
> +   tun_info->mode |= IP_TUNNEL_INFO_BRIDGE;
> tun_info->key = key.tun_key;
>
> /* We need to store the options in the action itself since
> --
> 1.8.3.1
>

Re: [ovs-dev] [PATCH net-next] openvswitch: Make metadata_dst tunnel work in IP_TUNNEL_INFO_BRIDGE mode

[Pravin Shelar]

On Sun, Mar 24, 2019 at 6:24 PM wenxu  wrote:
>
> On 2019/3/25 上午2:46, Pravin Shelar wrote:
> > On Sun, Mar 24, 2019 at 12:03 AM wenxu  wrote:
> >> On 2019/3/24 上午5:39, Pravin Shelar wrote:
> >>> On Sat, Mar 23, 2019 at 2:18 AM wenxu  wrote:
>  On 2019/3/23 下午3:50, Pravin Shelar wrote:
> 
>  On Thu, Mar 21, 2019 at 3:34 AM  wrote:
> 
>  From: wenxu 
> 
>  There is currently no support for the multicasti/broadcst aspects
>  of VXLAN in ovs. In the datapath flow the tun_dst must specific.
>  But in the IP_TUNNEL_INFO_BRIDGE mode the tun_dst can not be specific.
>  And the packet can forward through the fdb of vxlan devcice. In
>  this mode the broadcast/multicast packet can be sent through the
>  following ways in ovs.
> 
>  ovs-vsctl add-port br0 vxlan -- set in vxlan type=vxlan \
>  options:key=1000 options:remote_ip=flow
>  ovs-ofctl add-flow br0 in_port=LOCAL,dl_dst=ff:ff:ff:ff:ff:ff,\
>  action=output:vxlan
> 
>  bridge fdb append ff:ff:ff:ff:ff:ff dev vxlan_sys_4789 dst 172.168.0.1 \
>  src_vni 1000 vni 1000 self
>  bridge fdb append ff:ff:ff:ff:ff:ff dev vxlan_sys_4789 dst 172.168.0.2 \
>  src_vni 1000 vni 1000 self
> 
>  This would make datapath bit complicated, can you give example of such 
>  use-case?
> 
>  There is currently no support for the multicast/broadcast aspects
>  of VXLAN in ovs. To get around the lack of multicast support, it is 
>  possible to
>  pre-provision MAC to IP address mappings either manually or from a 
>  controller.
> 
>  With this patch we can achieve this through the fdb of the lower vxlan
>  device.
> 
>  For example. three severs connects with vxlan.
>  server1 IP 10.0.0.1 tunnel IP  172.168.0.1 vni 1000
>  server2 IP 10.0.0.2 tunnel IP  172.168.0.2 vni 1000
>  server3 IP 10.0.0.3 tunnel IP  172.168.0.3 vni 1000
> 
>  All the broadcast arp request from server1, can be send to vxlan_sys_4789
>  in IP_TUNNEL_INFO_BRIDGE mode. Then the broadcast packet can send through
>  the fdb table in the vxlan device as following:
> 
>  bridge fdb append ff:ff:ff:ff:ff:ff dev vxlan_sys_4789 dst 172.168.0.1 \
>  src_vni 1000 vni 1000 self
>  bridge fdb append ff:ff:ff:ff:ff:ff dev vxlan_sys_4789 dst 172.168.0.2 \
>  src_vni 1000 vni 1000 self
> 
> 
>  Not any for multicast case. This patch make ovs vxlan tunnel using the 
>  fdb
>  table of lower vxlan device.
> >>> Have you tried OVS mac learning?
> >>>
> >> The key point is that it makes ovs vxlan tunnel can make use of the fdb 
> >> table of lower vxlan device.
> >>
> >> The fdb table can be configurable or mac learning from outside.
> >>
> >> For the broadcast example.  In the ovs, it can only achieve this through 
> >> multiple output actions to simulate the broadcast.
> >>
> >> ovs-ofctl add-flow br0 
> >> in_port=server1,dl_dst=ff:ff:ff:ff:ff:ff,actions=set_field:172.168.0.1->tun_dst,output:vxlan,\
> >>
> >> set_field:172.168.0.2->tun_dst,output:vxlan.
> >>
> >> But there are some limits for the number of output actions.
> >>
> > I was referring to mac-learning feature in OVS i.e. using learn
> > action. I wanted to see if there is something that you are not able to
> > do with OVS learn action.
> >
> Ovs mac learn action is only work for the specific vxlan tunnel port（ fixed 
> tun_dst, tun_id) like following.
>
> ovs-vsctl set in vxlan options:remote_ip=172.168.0.1 options:key=1000
>
> ( This is the same problem for Linux bridge, It achieve this through 
> IP_TUNNEL_INFO_BRIDGE mode work
>
> with the fdb of lower vxlan device)
>
>
> But it is not work for the flow based tunnel (remote_ip=flow),  There will be 
> huge number of the tunnel peer.
>
> It' hard to manage the tunnel port with the specific mode.
>
>
OK, So it is hard to use ovs learn action, but it is doable. Given
IP_TUNNEL_INFO_BRIDGE is not adding too much complexity to OVS, I am
fine adding this feature.

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

[ovs-dev] [PATCH net-next v2] openvswitch: add seqadj extension when NAT is used.

[Flavio Leitner]

When the conntrack is initialized, there is no helper attached
yet so the nat info initialization (nf_nat_setup_info) skips
adding the seqadj ext.

A helper is attached later when the conntrack is not confirmed
but is going to be committed. In this case, if NAT is needed then
adds the seqadj ext as well.

Fixes: 16ec3d4fbb96 ("openvswitch: Fix cached ct with helper.")
Signed-off-by: Flavio Leitner 
---
 net/openvswitch/conntrack.c | 6 ++
 1 file changed, 6 insertions(+)

Changelog:
v2 - removed nfct_help(ct) check as it is not necessary.

diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 51080004677e..845b83598e0d 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -990,6 +990,12 @@ static int __ovs_ct_lookup(struct net *net, struct 
sw_flow_key *key,
GFP_ATOMIC);
if (err)
return err;
+
+   /* helper installed, add seqadj if NAT is required */
+   if (info->nat && !nfct_seqadj(ct)) {
+   if (!nfct_seqadj_ext_add(ct))
+   return -EINVAL;
+   }
}
 
/* Call the helper only if:
-- 
2.20.1



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

[ovs-dev] [PATCH v2] OVN: Add support for Transport Zones

[lmartins]

From: Lucas Alvares Gomes 

This patch is adding support for Transport Zones. Transport zones (a.k.a
TZs) is way to enable users of OVN to separate Chassis into different
logical groups that will form tunnels only between members of the same
group(s).

Each Chassis can belong to one or more Transport Zones. If not set,
the Chassis will be considered part of a default group; this feature
is backward compatible and did not require any changes to the database
schemas.

Configuring Transport Zones is done by creating a key called
"ovn-transport-zones" in the external_ids of the Open_vSwitch table of the
local OVS instance. The value is a string with the name of the Transport
Zone that this instance is part of. Multiple TZs may be specified with
a comma-separated list. For example:

$ sudo ovs-vsctl set open . external-ids:ovn-transport-zones=tz1

or

$ sudo ovs-vsctl set open . external-ids:ovn-transport-zones=tz1,tz2,tz3

This configuration will be also exposed in the Chassis table of the OVN
Southbound Database so that external systems can see what TZ(s) each
Chassis are part of and make decisions based those values.

The use for Transport Zones includes but are not limited to:

* Edge computing: As a way to preventing edge sites from trying to create
  tunnels with every node on every other edge site while still allowing
  these sites to create tunnels with the central node.

* Extra security layer: Where users wants to create "trust zones"
  and prevent computes in a more secure zone to communicate with a less
  secure zone.

Reported-by: Daniel Alvarez Sanchez 
Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-discuss/2019-February/048255.html
Signed-off-by: Lucas Alvares Gomes 
---

v1 -> v2
 * Rename the function check_chassis_tzones to chassis_tzones_overlap.
 * Fix a memory leak in chassis_tzones_overlap.
 * Pass the transport_zones to encaps_run() as a "const char *"
   instead of "struct sbrec_chassis". With this we can also avoid not
   running the function in case the Chassis entry is not yet created.

 NEWS|  3 +
 ovn/controller/chassis.c|  8 ++-
 ovn/controller/encaps.c | 58 +-
 ovn/controller/encaps.h |  3 +-
 ovn/controller/ovn-controller.8.xml |  9 +++
 ovn/controller/ovn-controller.c | 14 -
 tests/ovn.at| 93 +
 7 files changed, 183 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index 1e4744dbd..4adf49f57 100644
--- a/NEWS
+++ b/NEWS
@@ -24,6 +24,9 @@ Post-v2.11.0
protocol extension.
- OVN:
  * Select IPAM mac_prefix in a random manner if not provided by the user
+ * Support for Transport Zones, a way to separate chassis into
+   logical groups which results in tunnels only been formed between
+   members of the same transport zone(s).
- New QoS type "linux-netem" on Linux.
 
 v2.11.0 - 19 Feb 2019
diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c
index 3ea908d18..34c260410 100644
--- a/ovn/controller/chassis.c
+++ b/ovn/controller/chassis.c
@@ -139,6 +139,8 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
 const char *datapath_type =
 br_int && br_int->datapath_type ? br_int->datapath_type : "";
 const char *cms_options = get_cms_options(>external_ids);
+const char *transport_zones = smap_get_def(>external_ids,
+   "ovn-transport-zones", "");
 
 struct ds iface_types = DS_EMPTY_INITIALIZER;
 ds_put_cstr(_types, "");
@@ -167,18 +169,22 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
 = smap_get_def(_rec->external_ids, "iface-types", "");
 const char *chassis_cms_options
 = get_cms_options(_rec->external_ids);
+const char *chassis_transport_zones = smap_get_def(
+_rec->external_ids, "ovn-transport-zones", "");
 
 /* If any of the external-ids should change, update them. */
 if (strcmp(bridge_mappings, chassis_bridge_mappings) ||
 strcmp(datapath_type, chassis_datapath_type) ||
 strcmp(iface_types_str, chassis_iface_types) ||
-strcmp(cms_options, chassis_cms_options)) {
+strcmp(cms_options, chassis_cms_options) ||
+strcmp(transport_zones, chassis_transport_zones)) {
 struct smap new_ids;
 smap_clone(_ids, _rec->external_ids);
 smap_replace(_ids, "ovn-bridge-mappings", bridge_mappings);
 smap_replace(_ids, "datapath-type", datapath_type);
 smap_replace(_ids, "iface-types", iface_types_str);
 smap_replace(_ids, "ovn-cms-options", cms_options);
+smap_replace(_ids, "ovn-transport-zones", transport_zones);
 sbrec_chassis_verify_external_ids(chassis_rec);
 sbrec_chassis_set_external_ids(chassis_rec, _ids);
   

Re: [ovs-dev] [PATCH v2] datapath-windows: Address memory allocation issues for OVS_BUFFER_CONTEXT

[aserdean]

-Original Message-
From: ovs-dev-boun...@openvswitch.org  On
Behalf Of Anand Kumar via dev
Sent: Thursday, March 21, 2019 1:55 AM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH v2] datapath-windows: Address memory allocation
issues for OVS_BUFFER_CONTEXT

With current implementation, when nbl pool is allocated, context size is
specified as 64 bytes, while the OVS_BUFFER_CONTEXT size is only 32 bytes.
Since context size is never changed, additional memory is not required.

This patch makes it simpler to allocate memory for OVS_BUFFER_CONTEXT so
that it is always aligned to MEMORY_ALLOCATION_ALIGNMENT.
This is acheived by updating "value" field in the context structure, so that
number of elements in array is always a multiple of
MEMORY_ALLOCATION_ALIGNMENT.

Also change the DEFAULT_CONTEXT_SIZE to accomodate OVS_BUFFER_CONTEXT size.

Signed-off-by: Anand Kumar 
---
 
Thanks a lot for implementing this!

Acked-by: Alin Gabriel Serdean 

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

Re: [ovs-dev] [PATCH] OVN: Add support for Transport Zones

[Lucas Alvares Gomes Martins]

Hi,

Thanks a lot Mark for the quick review.

On Mon, Mar 25, 2019 at 3:40 PM Mark Michelson  wrote:
>
> Hi Lucas,
>
> Thanks for the patch. It's mostly good but I have a few issues with it.
> See below.
>
> On 3/25/19 10:36 AM, lmart...@redhat.com wrote:
> > From: Lucas Alvares Gomes 
> >
> > This patch is adding support for Transport Zones. Transport zones (a.k.a
> > TZs) is way to enable users of OVN to separate Chassis into different
> > logical groups that will form tunnels only between members of the same
> > group(s).
> >
> > Each Chassis can belong to one or more Transport Zones. If not set,
> > the Chassis will be considered part of a default group; this feature
> > is backward compatible and did not require any changes to the database
> > schemas.
> >
> > Configuring Transport Zones is done by creating a key called
> > "ovn-transport-zones" in the external_ids of the Open_vSwitch table of the
> > local OVS instance. The value is a string with the name of the Transport
> > Zone that this instance is part of. Multiple TZs may be specified with
> > a comma-separated list. For example:
> >
> > $ sudo ovs-vsctl set open . external-ids:ovn-transport-zones=tz1
> >
> > or
> >
> > $ sudo ovs-vsctl set open . external-ids:ovn-transport-zones=tz1,tz2,tz3
> >
> > This configuration will be also exposed in the Chassis table of the OVN
> > Southbound Database so that external systems can see what TZ(s) each
> > Chassis are part of and make decisions based those values.
> >
> > The use for Transport Zones includes but are not limited to:
> >
> > * Edge computing: As a way to preventing edge sites from trying to create
> >tunnels with every node on every other edge site while still allowing
> >these sites to create tunnels with the central node.
> >
> > * Extra security layer: Where users wants to create "trust zones"
> >and prevent computes in a more secure zone to communicate with a less
> >secure zone.
> >
> > Reported-by: Daniel Alvarez Sanchez 
> > Reported-at: 
> > https://mail.openvswitch.org/pipermail/ovs-discuss/2019-February/048255.html
> > Signed-off-by: Lucas Alvares Gomes 
> > ---
> >   NEWS|  3 +
> >   ovn/controller/chassis.c|  8 ++-
> >   ovn/controller/encaps.c | 54 +++--
> >   ovn/controller/encaps.h |  3 +-
> >   ovn/controller/ovn-controller.8.xml |  9 +++
> >   ovn/controller/ovn-controller.c |  2 +-
> >   tests/ovn.at| 93 +
> >   7 files changed, 164 insertions(+), 8 deletions(-)
> >
> > diff --git a/NEWS b/NEWS
> > index 1e4744dbd..4adf49f57 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -24,6 +24,9 @@ Post-v2.11.0
> >  protocol extension.
> >  - OVN:
> >* Select IPAM mac_prefix in a random manner if not provided by the 
> > user
> > + * Support for Transport Zones, a way to separate chassis into
> > +   logical groups which results in tunnels only been formed between
> > +   members of the same transport zone(s).
> >  - New QoS type "linux-netem" on Linux.
> >
> >   v2.11.0 - 19 Feb 2019
> > diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c
> > index 3ea908d18..34c260410 100644
> > --- a/ovn/controller/chassis.c
> > +++ b/ovn/controller/chassis.c
> > @@ -139,6 +139,8 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
> >   const char *datapath_type =
> >   br_int && br_int->datapath_type ? br_int->datapath_type : "";
> >   const char *cms_options = get_cms_options(>external_ids);
> > +const char *transport_zones = smap_get_def(>external_ids,
> > +   "ovn-transport-zones", "");
> >
> >   struct ds iface_types = DS_EMPTY_INITIALIZER;
> >   ds_put_cstr(_types, "");
> > @@ -167,18 +169,22 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
> >   = smap_get_def(_rec->external_ids, "iface-types", "");
> >   const char *chassis_cms_options
> >   = get_cms_options(_rec->external_ids);
> > +const char *chassis_transport_zones = smap_get_def(
> > +_rec->external_ids, "ovn-transport-zones", "");
> >
> >   /* If any of the external-ids should change, update them. */
> >   if (strcmp(bridge_mappings, chassis_bridge_mappings) ||
> >   strcmp(datapath_type, chassis_datapath_type) ||
> >   strcmp(iface_types_str, chassis_iface_types) ||
> > -strcmp(cms_options, chassis_cms_options)) {
> > +strcmp(cms_options, chassis_cms_options) ||
> > +strcmp(transport_zones, chassis_transport_zones)) {
> >   struct smap new_ids;
> >   smap_clone(_ids, _rec->external_ids);
> >   smap_replace(_ids, "ovn-bridge-mappings", 
> > bridge_mappings);
> >   smap_replace(_ids, "datapath-type", datapath_type);
> >   smap_replace(_ids, "iface-types", iface_types_str);
> >   

Re: [ovs-dev] [RFC v4 1/1] datapath: Add a new action check_pkt_len

[Gregory Rose]

On 3/25/2019 5:30 AM, Numan Siddique wrote:



On Mon, Mar 25, 2019 at 12:07 AM Numan Siddique > wrote:




On Fri, Mar 22, 2019 at 9:10 PM Gregory Rose mailto:gvrose8...@gmail.com>> wrote:


On 3/22/2019 2:58 AM, Numan Siddique wrote:



On Fri, Mar 22, 2019 at 6:16 AM Gregory Rose
mailto:gvrose8...@gmail.com>> wrote:



On 3/21/2019 5:38 PM, Gregory Rose wrote:
>
>
> On 3/21/2019 10:37 AM, Numan Siddique wrote:
>> This is the datapath patch -
https://patchwork.ozlabs.org/patch/1046370/
>> and this is the corresponding ovs-vswitchd patch -
>> https://patchwork.ozlabs.org/patch/1059081/ (this is
part of the
>> series -
>>
https://patchwork.ozlabs.org/project/openvswitch/list/?series=98190,

>> but probably you would be interested in only ovs patch)
>>
>> Sharing the links so that you can find it easily.
>>
>> Thanks
>> Numan
>>
>>
>
> This patch:
>
> https://patchwork.ozlabs.org/patch/1059081/
>
> shows this when applied:
>
> Applying: Add a new OVS action check_pkt_larger
> .git/rebase-apply/patch:1097: new blank line at EOF.
> +
> warning: 1 line adds whitespace errors.
>
> In regards to the datapath patch 1046370
> 
>
> In execute_check_pkt_len():
>
> +
> +   actual_pkt_len = skb->len +
(skb_vlan_tag_present(skb) ?
> VLAN_HLEN : 0);
> +
>
> This doesn't seem right to me - the skb length should
include the
> length of the entire packet, including any
> VLAN tags, or at least that is my understanding. 
Please check it.



Hi Greg,

I checked and tested it it. I can confirm that skb->len doesn't
include the vlan header.
Existing code in flow.c also uses the same way to get the packet
len -
https://github.com/openvswitch/ovs/blob/master/datapath/flow.c#L77


I submitted the patch to net-dev ML and I forgot to CC - ovs-dev - 
https://patchwork.ozlabs.org/patch/1063378/


Yes, I'm subscribed to netdev as well and already saw it.  :)

Thanks!

- Greg



Thanks
Numan

Thanks
Numan


>
> In validate_and_copy_check_pkt_len() in flow_netlink.c:
>
> +   static const struct nla_policy
pol[OVS_CHECK_PKT_LEN_ATTR_MAX
> + 1] = {
> + [OVS_CHECK_PKT_LEN_ATTR_PKT_LEN] = {.type = NLA_U16 },
> + [OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER] = {
> +   .type = NLA_NESTED },
> + [OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL] = {
> +   .type = NLA_NESTED },
> +   };
>
> I don't care for declaring these things within function
scope and it
> is not generally done.  I see that
> flow_netlink.c has one other instance of the nla_policy
structure
> statically declared within the function scope
> but if you look at datapath.c none of them are.  I
prefer the way it's
> done in datapath.c.  I also grepped around
> in other kernel code in the ./net tree and that is also
the way it's
> done there, i.e. I didn't see any other
> instances of it declared within function scope.
>
> I compiled both the ovs-vswitchd and openvswitch kernel
module
> components with no issues.  I wanted to use
> clang but the version of clang on Ubuntu right now
doesn't have
> retpoline support so it won't compile
> kernel modules.
>
> :-/
>
> I did some quick regression testing and found no
problems.  If you can
> address the two coding issues I brought up
> then I'd be glad to add my reviewed and tested by tags.

Oh wait, this is just an RFC.

I'll review and test the patches again when they
officially come out.
Maybe clang will have retpoline support
by then.


Thank you for the review. I will address them. I am planning
to submit the patch
to net-next ML. Looks like the window is open now -
http://vger.kernel.org/~davem/net-next.html

Please let me know in case you prefer to submit another RFC
version here before submitting 

Re: [ovs-dev] [RFC v4 1/1] datapath: Add a new action check_pkt_len

[Gregory Rose]

On 3/24/2019 11:37 AM, Numan Siddique wrote:



On Fri, Mar 22, 2019 at 9:10 PM Gregory Rose > wrote:



On 3/22/2019 2:58 AM, Numan Siddique wrote:



On Fri, Mar 22, 2019 at 6:16 AM Gregory Rose
mailto:gvrose8...@gmail.com>> wrote:



On 3/21/2019 5:38 PM, Gregory Rose wrote:
>
>
> On 3/21/2019 10:37 AM, Numan Siddique wrote:
>> This is the datapath patch -
https://patchwork.ozlabs.org/patch/1046370/
>> and this is the corresponding ovs-vswitchd patch -
>> https://patchwork.ozlabs.org/patch/1059081/ (this is part
of the
>> series -
>>
https://patchwork.ozlabs.org/project/openvswitch/list/?series=98190,

>> but probably you would be interested in only ovs patch)
>>
>> Sharing the links so that you can find it easily.
>>
>> Thanks
>> Numan
>>
>>
>
> This patch:
>
> https://patchwork.ozlabs.org/patch/1059081/
>
> shows this when applied:
>
> Applying: Add a new OVS action check_pkt_larger
> .git/rebase-apply/patch:1097: new blank line at EOF.
> +
> warning: 1 line adds whitespace errors.
>
> In regards to the datapath patch 1046370
> 
>
> In execute_check_pkt_len():
>
> +
> +   actual_pkt_len = skb->len +
(skb_vlan_tag_present(skb) ?
> VLAN_HLEN : 0);
> +
>
> This doesn't seem right to me - the skb length should
include the
> length of the entire packet, including any
> VLAN tags, or at least that is my understanding.  Please
check it.



Hi Greg,

I checked and tested it it. I can confirm that skb->len doesn't 
include the vlan header.
Existing code in flow.c also uses the same way to get the packet len - 
https://github.com/openvswitch/ovs/blob/master/datapath/flow.c#L77


Thanks
Numan


Thanks for clearing that up for me Numan.

- Greg




>
> In validate_and_copy_check_pkt_len() in flow_netlink.c:
>
> +   static const struct nla_policy
pol[OVS_CHECK_PKT_LEN_ATTR_MAX
> + 1] = {
> + [OVS_CHECK_PKT_LEN_ATTR_PKT_LEN] = {.type = NLA_U16 },
> + [OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER] = {
> +   .type = NLA_NESTED },
> + [OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL] = {
> +   .type = NLA_NESTED },
> +   };
>
> I don't care for declaring these things within function
scope and it
> is not generally done.  I see that
> flow_netlink.c has one other instance of the nla_policy
structure
> statically declared within the function scope
> but if you look at datapath.c none of them are.  I prefer
the way it's
> done in datapath.c.  I also grepped around
> in other kernel code in the ./net tree and that is also the
way it's
> done there, i.e. I didn't see any other
> instances of it declared within function scope.
>
> I compiled both the ovs-vswitchd and openvswitch kernel module
> components with no issues.  I wanted to use
> clang but the version of clang on Ubuntu right now doesn't
have
> retpoline support so it won't compile
> kernel modules.
>
> :-/
>
> I did some quick regression testing and found no problems. 
If you can
> address the two coding issues I brought up
> then I'd be glad to add my reviewed and tested by tags.

Oh wait, this is just an RFC.

I'll review and test the patches again when they officially
come out.
Maybe clang will have retpoline support
by then.


Thank you for the review. I will address them. I am planning to
submit the patch
to net-next ML. Looks like the window is open now -
http://vger.kernel.org/~davem/net-next.html

Please let me know in case you prefer to submit another RFC
version here before submitting to net-dev ML.


I think you're good to go.

Thanks,

- Greg



Thanks
Numan

- Greg





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

Re: [ovs-dev] [PATCH v5 3/3] dpif-netdev: split out generic lookup function

[Van Haaren, Harry]

> -Original Message-
> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
> Sent: Friday, March 22, 2019 1:19 PM
> To: Van Haaren, Harry ; Eelco Chaudron
> 
> Cc: ovs-dev@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH v5 3/3] dpif-netdev: split out generic lookup
> function
> 
> On 22.03.2019 16:08, Van Haaren, Harry wrote:
> > Hey Eelco,
> >
> >> -Original Message-



> > I'll work on addressing your comments early next week - thanks again! -H
> 
> Hi Harry,
> 
> I didn't look at this series closely yet. But if you're going to prepare new
> version, it'll be good to fix checkpatch issues reported by 0-day robot too:
> 
>   https://mail.openvswitch.org/pipermail/ovs-dev/2019-February/356615.html
>   https://mail.openvswitch.org/pipermail/ovs-dev/2019-February/356616.html
> 
> BTW, commented out counters looks suspicious.
> 
> Best regards, Ilya Maximets.

Thanks Ilya, fixing checkpatch issues now, no problems.

Correct the lookups_match counter isn't implemented yet - will add it in v6 
implementation.

The name of the variable is a bit misleading - it really counts the number of 
subtables searched for a match, per packet that it found a hit for.

Eg: a packet that matched the 3rd subtable searched, will += counter by 3.

So really, it counts "subtable-effort per packet match". Does this align with 
you understanding of it?

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

Re: [ovs-dev] [PATCH] OVN: Add support for Transport Zones

[Mark Michelson]

Hi Lucas,

Thanks for the patch. It's mostly good but I have a few issues with it. 
See below.


On 3/25/19 10:36 AM, lmart...@redhat.com wrote:

From: Lucas Alvares Gomes 

This patch is adding support for Transport Zones. Transport zones (a.k.a
TZs) is way to enable users of OVN to separate Chassis into different
logical groups that will form tunnels only between members of the same
group(s).

Each Chassis can belong to one or more Transport Zones. If not set,
the Chassis will be considered part of a default group; this feature
is backward compatible and did not require any changes to the database
schemas.

Configuring Transport Zones is done by creating a key called
"ovn-transport-zones" in the external_ids of the Open_vSwitch table of the
local OVS instance. The value is a string with the name of the Transport
Zone that this instance is part of. Multiple TZs may be specified with
a comma-separated list. For example:

$ sudo ovs-vsctl set open . external-ids:ovn-transport-zones=tz1

or

$ sudo ovs-vsctl set open . external-ids:ovn-transport-zones=tz1,tz2,tz3

This configuration will be also exposed in the Chassis table of the OVN
Southbound Database so that external systems can see what TZ(s) each
Chassis are part of and make decisions based those values.

The use for Transport Zones includes but are not limited to:

* Edge computing: As a way to preventing edge sites from trying to create
   tunnels with every node on every other edge site while still allowing
   these sites to create tunnels with the central node.

* Extra security layer: Where users wants to create "trust zones"
   and prevent computes in a more secure zone to communicate with a less
   secure zone.

Reported-by: Daniel Alvarez Sanchez 
Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-discuss/2019-February/048255.html
Signed-off-by: Lucas Alvares Gomes 
---
  NEWS|  3 +
  ovn/controller/chassis.c|  8 ++-
  ovn/controller/encaps.c | 54 +++--
  ovn/controller/encaps.h |  3 +-
  ovn/controller/ovn-controller.8.xml |  9 +++
  ovn/controller/ovn-controller.c |  2 +-
  tests/ovn.at| 93 +
  7 files changed, 164 insertions(+), 8 deletions(-)

diff --git a/NEWS b/NEWS
index 1e4744dbd..4adf49f57 100644
--- a/NEWS
+++ b/NEWS
@@ -24,6 +24,9 @@ Post-v2.11.0
 protocol extension.
 - OVN:
   * Select IPAM mac_prefix in a random manner if not provided by the user
+ * Support for Transport Zones, a way to separate chassis into
+   logical groups which results in tunnels only been formed between
+   members of the same transport zone(s).
 - New QoS type "linux-netem" on Linux.
  
  v2.11.0 - 19 Feb 2019

diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c
index 3ea908d18..34c260410 100644
--- a/ovn/controller/chassis.c
+++ b/ovn/controller/chassis.c
@@ -139,6 +139,8 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
  const char *datapath_type =
  br_int && br_int->datapath_type ? br_int->datapath_type : "";
  const char *cms_options = get_cms_options(>external_ids);
+const char *transport_zones = smap_get_def(>external_ids,
+   "ovn-transport-zones", "");
  
  struct ds iface_types = DS_EMPTY_INITIALIZER;

  ds_put_cstr(_types, "");
@@ -167,18 +169,22 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
  = smap_get_def(_rec->external_ids, "iface-types", "");
  const char *chassis_cms_options
  = get_cms_options(_rec->external_ids);
+const char *chassis_transport_zones = smap_get_def(
+_rec->external_ids, "ovn-transport-zones", "");
  
  /* If any of the external-ids should change, update them. */

  if (strcmp(bridge_mappings, chassis_bridge_mappings) ||
  strcmp(datapath_type, chassis_datapath_type) ||
  strcmp(iface_types_str, chassis_iface_types) ||
-strcmp(cms_options, chassis_cms_options)) {
+strcmp(cms_options, chassis_cms_options) ||
+strcmp(transport_zones, chassis_transport_zones)) {
  struct smap new_ids;
  smap_clone(_ids, _rec->external_ids);
  smap_replace(_ids, "ovn-bridge-mappings", bridge_mappings);
  smap_replace(_ids, "datapath-type", datapath_type);
  smap_replace(_ids, "iface-types", iface_types_str);
  smap_replace(_ids, "ovn-cms-options", cms_options);
+smap_replace(_ids, "ovn-transport-zones", transport_zones);
  sbrec_chassis_verify_external_ids(chassis_rec);
  sbrec_chassis_set_external_ids(chassis_rec, _ids);
  smap_destroy(_ids);
diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
index 610b833de..072be4ac2 100644
--- a/ovn/controller/encaps.c
+++ b/ovn/controller/encaps.c
@@ -195,20 +195,52 @@ 

[ovs-dev] Rotación Cero - ultimos lugares

[Estrategias de fidelización de personal.]

Cursos TOP - Webinar Interactivo – Miércoles 03 de Abril

Rotación Cero - Abril 03

La rotación de personal es un fenómeno que afecta a la mayoría de las 
organizaciones y merma el rendimiento de los colaboradores.

Presentaremos los fundamentos para identificar el impacto que la rotación de 
personal tiene en nuestra empresa así como de las
estrategias a aplicar para sobrellevarla. 
 

OBJETIVOS ESPECÍFICOS:

• Causas de la rotación de personal..
• Medición de la rotación de personal..
• Acciones preventivas.
• Estrategias de fidelización de personal. 


Para mayor información, responder sobre este correo con la palabra CERO + los 
siguientes datos:


NOMBRE:
TELÉFONO:
EMPRSA:
CORREO ALTERNO: 

Llamanos al (045) 55 1554 6630


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

Re: [ovs-dev] [PATCH net-next] openvswitch: add seqadj extension when NAT is used.

[Flavio Leitner]

On Sat, Mar 23, 2019 at 12:32:37PM -0700, Pravin Shelar wrote:
> On Thu, Mar 21, 2019 at 9:52 AM Flavio Leitner  wrote:
> >
> > When the conntrack is initialized, there is no helper attached
> > yet so the nat info initialization (nf_nat_setup_info) skips
> > adding the seqadj ext.
> >
> > A helper is attached later when the conntrack is not confirmed
> > but is going to be committed. In this case, if NAT is needed then
> > adds the seqadj ext as well.
> >
> > Fixes: 16ec3d4fbb96 ("openvswitch: Fix cached ct with helper.")
> > Signed-off-by: Flavio Leitner 
> > ---
> >  net/openvswitch/conntrack.c | 5 +
> >  1 file changed, 5 insertions(+)
> >
> I am not able to apply this patch.

This is for Davem net-next. I got the patch from ML and it worked for
me.


> > diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
> > index 1b6896896fff..a7664515c943 100644
> > --- a/net/openvswitch/conntrack.c
> > +++ b/net/openvswitch/conntrack.c
> > @@ -990,6 +990,11 @@ static int __ovs_ct_lookup(struct net *net, struct 
> > sw_flow_key *key,
> > GFP_ATOMIC);
> > if (err)
> > return err;
> > +
> > +   if (info->nat && nfct_help(ct) && !nfct_seqadj(ct)) 
> > {
> Given helper is just assigned, is nfct_help() check required here?

I tried to be very clear, but you're right that it's not required.
I will post a v2 removing that.
Thanks
fbl

> 
> > +   if (!nfct_seqadj_ext_add(ct))
> > +   return -EINVAL;
> > +   }
> > }
> >
> > /* Call the helper only if:
> > --
> > 2.20.1
> >
> >
> >

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

[ovs-dev] [PATCH] OVN: Add support for Transport Zones

[lmartins]

From: Lucas Alvares Gomes 

This patch is adding support for Transport Zones. Transport zones (a.k.a
TZs) is way to enable users of OVN to separate Chassis into different
logical groups that will form tunnels only between members of the same
group(s).

Each Chassis can belong to one or more Transport Zones. If not set,
the Chassis will be considered part of a default group; this feature
is backward compatible and did not require any changes to the database
schemas.

Configuring Transport Zones is done by creating a key called
"ovn-transport-zones" in the external_ids of the Open_vSwitch table of the
local OVS instance. The value is a string with the name of the Transport
Zone that this instance is part of. Multiple TZs may be specified with
a comma-separated list. For example:

$ sudo ovs-vsctl set open . external-ids:ovn-transport-zones=tz1

or

$ sudo ovs-vsctl set open . external-ids:ovn-transport-zones=tz1,tz2,tz3

This configuration will be also exposed in the Chassis table of the OVN
Southbound Database so that external systems can see what TZ(s) each
Chassis are part of and make decisions based those values.

The use for Transport Zones includes but are not limited to:

* Edge computing: As a way to preventing edge sites from trying to create
  tunnels with every node on every other edge site while still allowing
  these sites to create tunnels with the central node.

* Extra security layer: Where users wants to create "trust zones"
  and prevent computes in a more secure zone to communicate with a less
  secure zone.

Reported-by: Daniel Alvarez Sanchez 
Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-discuss/2019-February/048255.html
Signed-off-by: Lucas Alvares Gomes 
---
 NEWS|  3 +
 ovn/controller/chassis.c|  8 ++-
 ovn/controller/encaps.c | 54 +++--
 ovn/controller/encaps.h |  3 +-
 ovn/controller/ovn-controller.8.xml |  9 +++
 ovn/controller/ovn-controller.c |  2 +-
 tests/ovn.at| 93 +
 7 files changed, 164 insertions(+), 8 deletions(-)

diff --git a/NEWS b/NEWS
index 1e4744dbd..4adf49f57 100644
--- a/NEWS
+++ b/NEWS
@@ -24,6 +24,9 @@ Post-v2.11.0
protocol extension.
- OVN:
  * Select IPAM mac_prefix in a random manner if not provided by the user
+ * Support for Transport Zones, a way to separate chassis into
+   logical groups which results in tunnels only been formed between
+   members of the same transport zone(s).
- New QoS type "linux-netem" on Linux.
 
 v2.11.0 - 19 Feb 2019
diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c
index 3ea908d18..34c260410 100644
--- a/ovn/controller/chassis.c
+++ b/ovn/controller/chassis.c
@@ -139,6 +139,8 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
 const char *datapath_type =
 br_int && br_int->datapath_type ? br_int->datapath_type : "";
 const char *cms_options = get_cms_options(>external_ids);
+const char *transport_zones = smap_get_def(>external_ids,
+   "ovn-transport-zones", "");
 
 struct ds iface_types = DS_EMPTY_INITIALIZER;
 ds_put_cstr(_types, "");
@@ -167,18 +169,22 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
 = smap_get_def(_rec->external_ids, "iface-types", "");
 const char *chassis_cms_options
 = get_cms_options(_rec->external_ids);
+const char *chassis_transport_zones = smap_get_def(
+_rec->external_ids, "ovn-transport-zones", "");
 
 /* If any of the external-ids should change, update them. */
 if (strcmp(bridge_mappings, chassis_bridge_mappings) ||
 strcmp(datapath_type, chassis_datapath_type) ||
 strcmp(iface_types_str, chassis_iface_types) ||
-strcmp(cms_options, chassis_cms_options)) {
+strcmp(cms_options, chassis_cms_options) ||
+strcmp(transport_zones, chassis_transport_zones)) {
 struct smap new_ids;
 smap_clone(_ids, _rec->external_ids);
 smap_replace(_ids, "ovn-bridge-mappings", bridge_mappings);
 smap_replace(_ids, "datapath-type", datapath_type);
 smap_replace(_ids, "iface-types", iface_types_str);
 smap_replace(_ids, "ovn-cms-options", cms_options);
+smap_replace(_ids, "ovn-transport-zones", transport_zones);
 sbrec_chassis_verify_external_ids(chassis_rec);
 sbrec_chassis_set_external_ids(chassis_rec, _ids);
 smap_destroy(_ids);
diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
index 610b833de..072be4ac2 100644
--- a/ovn/controller/encaps.c
+++ b/ovn/controller/encaps.c
@@ -195,20 +195,52 @@ chassis_tunnel_add(const struct sbrec_chassis 
*chassis_rec, const struct sbrec_s
 return tuncnt;
 }
 
+/*
+ * Check if both chassis are in the same transport zone.
+ */
+static bool

Re: [ovs-dev] [RFC v4 1/1] datapath: Add a new action check_pkt_len

[Numan Siddique]

On Mon, Mar 25, 2019 at 12:07 AM Numan Siddique  wrote:

>
>
> On Fri, Mar 22, 2019 at 9:10 PM Gregory Rose  wrote:
>
>>
>> On 3/22/2019 2:58 AM, Numan Siddique wrote:
>>
>>
>>
>> On Fri, Mar 22, 2019 at 6:16 AM Gregory Rose 
>> wrote:
>>
>>>
>>>
>>> On 3/21/2019 5:38 PM, Gregory Rose wrote:
>>> >
>>> >
>>> > On 3/21/2019 10:37 AM, Numan Siddique wrote:
>>> >> This is the datapath patch -
>>> https://patchwork.ozlabs.org/patch/1046370/
>>> >> and this is the corresponding ovs-vswitchd patch -
>>> >> https://patchwork.ozlabs.org/patch/1059081/ (this is part of the
>>> >> series -
>>> >> https://patchwork.ozlabs.org/project/openvswitch/list/?series=98190,
>>> >> but probably you would be interested in only ovs patch)
>>> >>
>>> >> Sharing the links so that you can find it easily.
>>> >>
>>> >> Thanks
>>> >> Numan
>>> >>
>>> >>
>>> >
>>> > This patch:
>>> >
>>> > https://patchwork.ozlabs.org/patch/1059081/
>>> >
>>> > shows this when applied:
>>> >
>>> > Applying: Add a new OVS action check_pkt_larger
>>> > .git/rebase-apply/patch:1097: new blank line at EOF.
>>> > +
>>> > warning: 1 line adds whitespace errors.
>>> >
>>> > In regards to the datapath patch 1046370
>>> > 
>>> >
>>> > In execute_check_pkt_len():
>>> >
>>> > +
>>> > +   actual_pkt_len = skb->len + (skb_vlan_tag_present(skb) ?
>>> > VLAN_HLEN : 0);
>>> > +
>>> >
>>> > This doesn't seem right to me - the skb length should include the
>>> > length of the entire packet, including any
>>> > VLAN tags, or at least that is my understanding.  Please check it.
>>>
>>
> Hi Greg,
>
> I checked and tested it it. I can confirm that skb->len doesn't include
> the vlan header.
> Existing code in flow.c also uses the same way to get the packet len -
> https://github.com/openvswitch/ovs/blob/master/datapath/flow.c#L77
>
>
I submitted the patch to net-dev ML and I forgot to CC - ovs-dev -
https://patchwork.ozlabs.org/patch/1063378/

Thanks
Numan


> Thanks
> Numan
>
> >
>>> > In validate_and_copy_check_pkt_len() in flow_netlink.c:
>>> >
>>> > +   static const struct nla_policy pol[OVS_CHECK_PKT_LEN_ATTR_MAX
>>> > + 1] = {
>>> > +   [OVS_CHECK_PKT_LEN_ATTR_PKT_LEN] = {.type = NLA_U16 },
>>> > +   [OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER] = {
>>> > +   .type = NLA_NESTED },
>>> > +   [OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL] = {
>>> > +   .type = NLA_NESTED },
>>> > +   };
>>> >
>>> > I don't care for declaring these things within function scope and it
>>> > is not generally done.  I see that
>>> > flow_netlink.c has one other instance of the nla_policy structure
>>> > statically declared within the function scope
>>> > but if you look at datapath.c none of them are.  I prefer the way it's
>>> > done in datapath.c.  I also grepped around
>>> > in other kernel code in the ./net tree and that is also the way it's
>>> > done there, i.e. I didn't see any other
>>> > instances of it declared within function scope.
>>> >
>>> > I compiled both the ovs-vswitchd and openvswitch kernel module
>>> > components with no issues.  I wanted to use
>>> > clang but the version of clang on Ubuntu right now doesn't have
>>> > retpoline support so it won't compile
>>> > kernel modules.
>>> >
>>> > :-/
>>> >
>>> > I did some quick regression testing and found no problems.  If you can
>>> > address the two coding issues I brought up
>>> > then I'd be glad to add my reviewed and tested by tags.
>>>
>>> Oh wait, this is just an RFC.
>>>
>>> I'll review and test the patches again when they officially come out.
>>> Maybe clang will have retpoline support
>>> by then.
>>>
>>>
>> Thank you for the review. I will address them. I am planning to submit
>> the patch
>> to net-next ML. Looks like the window is open now -
>> http://vger.kernel.org/~davem/net-next.html
>>
>> Please let me know in case you prefer to submit another RFC version here
>> before submitting to  net-dev ML.
>>
>>
>> I think you're good to go.
>>
>> Thanks,
>>
>> - Greg
>>
>>
>> Thanks
>> Numan
>>
>> - Greg
>>>
>>>
>>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] bridge: Propagate patch port pairing errors to db.

[Eelco Chaudron]

On 22 Mar 2019, at 13:58, Ilya Maximets wrote:

> Virtual ports like 'patch' ports that almost fully implemented on
> 'ofproto' layer could have internal to 'ofproto' statuses that
> could not be retrieved from 'netdev' or other layers. For example,
> in current implementation there is no way to get the patch port
> pairing status (i.e. if it has usable peer?).
>
> New 'ofproto-provider' API function 'vport_get_status' introduced to
> cover this gap. It allowes 'bridge' layer to retrive current status
> of ofproto virtual ports and propagate it to DB.
> For now we're only interested in pairing errors of 'patch' ports.
> That are propagated to the 'error' column of the 'Interface' table.
>
> Ex.:
>
>   $ ovs-vsctl show
> ...
> Bridge "br1"
>   ...
>   Port "patch1"
> Interface "patch1"
>   type: patch
>   options: {peer="patch0"}
>   error: "No usable peer 'patch0' exists in 'system' datapath."
>
> Bridge "br0"
>   datapath_type: netdev
>   ...
>   Port "patch0"
> Interface "patch0"
>   type: patch
>   options: {peer="patch1"}
>   error: "No usable peer 'patch1' exists in 'netdev' datapath."
>
> Signed-off-by: Ilya Maximets 

Thanks for taking care of this Ilya!

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

Re: [ovs-dev] [ovs-dev, RFC] vswitchd: warn about misconfigured interface type on netdev bridge

[Eelco Chaudron]

On 20 Mar 2019, at 10:28, Ilya Maximets wrote:


On 20.03.2019 12:18, Ilya Maximets wrote:

On 20.03.2019 11:34, Ilya Maximets wrote:

On 20.03.2019 11:01, Eelco Chaudron wrote:



On 19 Mar 2019, at 16:51, Ilya Maximets wrote:


On 19.03.2019 12:23, Eelco Chaudron wrote:
When debugging an issue we noticed that by accident someone has 
changes the
bridge datapath_type to netdev, where it's clearly a kernel 
(system bridge)
with physical and tap devices. Unfortunately, this is not 
something you
will easily spot, as the bridge datapath type value is not shown 
by default.


In addition, OVS is not warning you about this potential mismatch 
in

interface and bridge datapath.

I'm sending out this patch as an RFC as I could not find a clear
demarcation between bridge datapath types and interface datapath 
types.

The patch below will at least warn for netdev bridges with system
interfaces. But no warning will be given for some unsupported 
virtual
interfaces. For system bridges, the dpdk types will no be 
recognized as
system/virtual interfaces (unless the name exists) which will 
result in

an error.

Signed-off-by: Eelco Chaudron 
---


Hi.

Hmm. What do you mean under misconfiguration?


Good question, as the documentation and code are not clear about 
this part…


My interpretation is that for the netdev datapath bridges 
DPDK/vhost ports, internal ports, patch-ports, and user space 
tunnel ports are the once supported. But any other kernel ports are 
not, however, your text below suggests otherwise.


See below.



In practice, userspace datapath is able to open "system" type 
netdevs.
It's supported by netdev-linux to open system network devices via 
socket.

And these devices has "system" type.
On 'datapath_type' changes, bridge/ofproto will be destroyed and 
re-created.
All the ports from kernel datapath will be destroyed and new ones 
created

in userspace. All the ports should still work as usual.


With the below statement in mind, I configure the following:

  ovs-vsctl add-br test
  ip link add name right1 type veth peer name left1
  ip link add name right2 type veth peer name left2
  ovs-vsctl add-port test left1
  ovs-vsctl add-port test left2
  ovs-vsctl add-port test eth0
  ip link set dev left1 up
  ip link set dev left2 up
  ip netns add netns1
  ip netns add netns2
  ip link set dev right1 netns netns1
  ip link set dev right2 netns netns2
  ip netns exec netns1 ip link set dev lo up
  ip netns exec netns1 ip link set dev right1 up
  ip netns exec netns2 ip link set dev right2 up
  ip netns exec netns2 ip link set dev lo up
  ip netns exec netns1 ip  a a dev right1 192.168.0.1/24
  ip netns exec netns2 ip a a dev right2 192.168.0.2/24

This is all working fine now, and now some accidentally does this:

 ovs-vsctl set bridge test datapath_type=netdev

And you suggest all continues to work as is?


In general, yes.



The only possible issue will be that this bridge will no longer 
communicate

with other bridges, because they're in different datapaths now.

So, below warning will be printed on any attempt to add legitimate
system network interface to the userspace bridge.


"system" devices supported by both datapaths, but "dpdk" devices 
supported
only by userspace, that is why you see the error in case of 
changing to

'datapath_type=system'.


Are you saying ALL system devices are supported by the netdev 
datapath? Or only a specific set? If the later we should check for 
those (and maybe add some infrastructure to identify easily which 
devices are supported by which datapath. If it exists please point 
me to it, as I might have overlooked it).


OVS netdev datapath could open any linux network interfacce that 
could be

opened with the raw socket. There is nothing device specific here.

BTW, tests/system-userspace-testsuite.at completely relies on 
ability to

open and forward traffic through the veth pairs by netdev datapath.



Maybe I'm missing something? What is the issue you're trying to 
address?


The above example stops working due to checksum offloading on veth.
But if you are right this is a valid configuration I’ll further 
investigate this.


Configuration is valid. The issue here is that OVS netdev datapath 
doesn't
support TX checksum offloading (this is not easy task with arguable 
profit).
i.e. if packet arrives with bad/no checksum it will be sent to the 
output port
with same bad/no checksum. Everything works in case of kernel 
datapth because
the packet doesn't leave the kernel space. In case of netdev 
datapath some
information (like CHECKSUM_VALID skb flags) is lost while receiving 
via


I meant CHECKSUM_UNNECESSARY.


Thanks Ilya for all the details above, this helped me to quickly get 
trough the bottom of this.


socket in userspace and subsequently kernel expects valid checksum 
while
receiving the packet from userspace because TX offloading is not 
enabled.


This kind of issues usually mitigated by disabling TX offloading on 
the
"right*" interfaces, or 

[ovs-dev] [PATCH] [windows][wmi] Switch from internal port to all ports defined

[Alin Gabriel Serdean]

This patch changes the way we try to figure out if a port is defined on a given 
switch.

Instead of looking only in the internal ports defined switch to all ports 
defined.

This caused issues when trying to add a Hyper-V container port to a given OVS 
bridge.

Reported-by: Danting Liu 
Signed-off-by: Alin Gabriel Serdean 
---
 lib/wmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/wmi.c b/lib/wmi.c
index e6dc63cde..44c1d75e9 100644
--- a/lib/wmi.c
+++ b/lib/wmi.c
@@ -686,7 +686,7 @@ create_wmi_port(char *name) {
 
 /* Check if the element already exists on the switch. */
 wchar_t internal_port_query[WMI_QUERY_COUNT] = L"SELECT * FROM "
-L"Msvm_InternalEthernetPort WHERE ElementName = \"";
+L"CIM_EthernetPort WHERE ElementName = \"";
 
 wide_name = xmalloc((strlen(name) + 1) * sizeof(wchar_t));
 
-- 
2.21.0.windows.1

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

Re: [ovs-dev] [PATCH v3] dpif-netdev: dfc_process optimization by prefetching EMC entry.

[Yanqin Wei (Arm Technology China)]

Hi Ilya,

Thanks for your reply. We could have a look at test results in x86 by then.
I can understand patch 1054571. If both patches are apply to master, I could 
rebase prefetch EMC patch for it.
Mandatory hash computing in the ingress makes logic simple a lot, and it only 
costs a small price even in worst case(EMC/SMC disable & no hash-feature/load 
balance enable).

Best Regards,
Wei Yanqin

-Original Message-
From: Ilya Maximets  
Sent: Friday, March 22, 2019 9:12 PM
To: Yanqin Wei (Arm Technology China) ; 
d...@openvswitch.org; ian.sto...@intel.com
Cc: nd 
Subject: Re: [ovs-dev][PATCH v3] dpif-netdev: dfc_process optimization by 
prefetching EMC entry.

On 22.03.2019 11:44, Yanqin Wei (Arm Technology China) wrote:
> Hi , OVS Maintainers,
> 
> Could you help to have a look at this patch? Thanks a lot.

Hi. Thanks for improving performance and sorry for delay. Review process here 
in OVS is a bit slow due to lack of reviewers.

I have a plan to test this patch a bit on a next week. Want to check the 
performance impact on PVP cases on x86.

BTW, I have a patch that affects same code. Maybe it'll be interesting to you: 
https://patchwork.ozlabs.org/patch/1054571/

Best regards, Ilya Maximets.

> 
> Best Regards,
> Wei Yanqin
> 
> -Original Message-
> From: Yanqin Wei 
> Sent: Wednesday, March 13, 2019 1:28 PM
> To: d...@openvswitch.org
> Cc: nd ; Gavin Hu (Arm Technology China) 
> ; Yanqin Wei (Arm Technology China) 
> 
> Subject: [ovs-dev][PATCH v3] dpif-netdev: dfc_process optimization by 
> prefetching EMC entry.
> 
> It is observed that the throughput of multi-flow is worse than single-flow in 
> the EMC NIC to NIC cases. It is because CPU cache-miss increasing in EMC 
> lookup. Each flow need load at least one EMC entry to CPU cache(several cache 
> lines) and compare it with packet miniflow.
> This patch improve it by prefetching EMC entry in advance. Hash value 
> can be obtained from dpdk rss hash, so this step can be advanced ahead 
> of
> miniflow_extract() and prefetch EMC entry there. The prefetching size is 
> defined as ROUND_UP(128,CACHE_LINE_SIZE), which can cover majority traffic 
> including TCP/UDP protocol and need 2 cache lines in most modern CPU.
> Performance test was run in some arm platform. 1000/1 flows NIC2NIC test 
> achieved around 10% throughput improvement in thunderX2(aarch64 platform).
> 
> Signed-off-by: Yanqin Wei 
> Reviewed-by: Gavin Hu 
> ---
>  lib/dpif-netdev.c | 80 
> ---
>  1 file changed, 52 insertions(+), 28 deletions(-)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 
> 4d6d0c3..982082c 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -189,6 +189,10 @@ struct netdev_flow_key {
>  #define DEFAULT_EM_FLOW_INSERT_MIN (UINT32_MAX / \
>  DEFAULT_EM_FLOW_INSERT_INV_PROB)
>  
> +/* DEFAULT_EMC_PREFETCH_SIZE can cover majority traffic including 
> +TCP/UDP
> + * protocol. */
> +#define DEFAULT_EMC_PREFETCH_SIZE ROUND_UP(128,CACHE_LINE_SIZE)
> +
>  struct emc_entry {
>  struct dp_netdev_flow *flow;
>  struct netdev_flow_key key;   /* key.hash used for emc hash value. */
> @@ -6166,15 +6170,20 @@ dp_netdev_upcall(struct dp_netdev_pmd_thread 
> *pmd, struct dp_packet *packet_,  }
>  
>  static inline uint32_t
> -dpif_netdev_packet_get_rss_hash_orig_pkt(struct dp_packet *packet,
> -const struct miniflow *mf)
> +dpif_netdev_packet_get_packet_rss_hash(struct dp_packet *packet,
> +bool md_is_valid)
>  {
> -uint32_t hash;
> +uint32_t hash,recirc_depth;
>  
> -if (OVS_LIKELY(dp_packet_rss_valid(packet))) {
> -hash = dp_packet_get_rss_hash(packet);
> -} else {
> -hash = miniflow_hash_5tuple(mf, 0);
> +hash = dp_packet_get_rss_hash(packet);
> +
> +if (md_is_valid) {
> +/* The RSS hash must account for the recirculation depth to avoid
> + * collisions in the exact match cache */
> +recirc_depth = *recirc_depth_get_unsafe();
> +if (OVS_UNLIKELY(recirc_depth)) {
> +hash = hash_finish(hash, recirc_depth);
> +}
>  dp_packet_set_rss_hash(packet, hash);
>  }
>  
> @@ -6182,24 +6191,23 @@ 
> dpif_netdev_packet_get_rss_hash_orig_pkt(struct dp_packet *packet,  }
>  
>  static inline uint32_t
> -dpif_netdev_packet_get_rss_hash(struct dp_packet *packet,
> -const struct miniflow *mf)
> +dpif_netdev_packet_get_hash_5tuple(struct dp_packet *packet,
> +const struct miniflow *mf,
> +bool md_is_valid)
>  {
> -uint32_t hash, recirc_depth;
> +uint32_t hash,recirc_depth;
>  
> -if (OVS_LIKELY(dp_packet_rss_valid(packet))) {
> -hash = dp_packet_get_rss_hash(packet);
> -} else {
> -hash = miniflow_hash_5tuple(mf, 0);
> -