Re: [ovs-dev] [PATCH v2] ovn-controller: Fix parsing of OVN tunnel IDs

2019-05-24 Thread 0-day Robot
Bleep bloop.  Greetings Dumitru Ceara, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

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


git-am:
fatal: sha1 information is lacking or useless (ovn/controller/pinctrl.c).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 ovn-controller: Fix parsing of OVN tunnel IDs
The copy of the patch that failed is found in:
   
/var/lib/jenkins/jobs/upstream_build_from_pw/workspace/OVN/.git/rebase-apply/patch
When you have resolved this problem, run "git am --resolved".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


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

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


[ovs-dev] [PATCH v2] ovn-controller: Fix parsing of OVN tunnel IDs

2019-05-24 Thread Dumitru Ceara
Encap tunnel-ids are of the form:
.
In physical_run we were checking if a tunnel-id corresponds
to the local chassis-id by searching if the chassis-id string
is included in the tunnel-id (strstr). This can break quite
easily, for example, if the local chassis-id is a substring
of a remote chassis-id. In that case we were wrongfully
skipping the tunnel creation.

To fix that new tunnel-id creation and parsing functions are added in
encaps.[ch]. These functions are now used everywhere where applicable.

Acked-by: Venu Iyer 
Reported-at: https://bugzilla.redhat.com/1708131
Reported-by: Haidong Li 
Fixes: b520ca7 ("Support for multiple VTEP in OVN")
Signed-off-by: Dumitru Ceara 
---

v2: Update commit message
---
 ovn/controller/bfd.c| 31 ---
 ovn/controller/encaps.c | 87 -
 ovn/controller/encaps.h |  6 +++
 ovn/controller/ovn-controller.h |  7 
 ovn/controller/physical.c   | 51 +---
 ovn/controller/pinctrl.c|  4 +-
 6 files changed, 130 insertions(+), 56 deletions(-)

diff --git a/ovn/controller/bfd.c b/ovn/controller/bfd.c
index d016e27..b6aef04 100644
--- a/ovn/controller/bfd.c
+++ b/ovn/controller/bfd.c
@@ -15,6 +15,7 @@
 
 #include 
 #include "bfd.h"
+#include "encaps.h"
 #include "lport.h"
 #include "ovn-controller.h"
 
@@ -72,14 +73,16 @@ bfd_calculate_active_tunnels(const struct ovsrec_bridge 
*br_int,
 const char *id = smap_get(_rec->external_ids,
   "ovn-chassis-id");
 if (id) {
-char *chassis_name;
-char *save_ptr = NULL;
-char *tokstr = xstrdup(id);
-chassis_name = strtok_r(tokstr, 
OVN_MVTEP_CHASSISID_DELIM, _ptr);
-if (chassis_name && !sset_contains(active_tunnels, 
chassis_name)) {
-sset_add(active_tunnels, chassis_name);
+char *chassis_name = NULL;
+
+if (encaps_tunnel_id_parse(id, _name,
+   NULL)) {
+if (!sset_contains(active_tunnels,
+   chassis_name)) {
+sset_add(active_tunnels, chassis_name);
+}
+free(chassis_name);
 }
-free(tokstr);
 }
 }
 }
@@ -193,17 +196,17 @@ bfd_run(const struct ovsrec_interface_table 
*interface_table,
 const char *tunnel_id = smap_get(_int->ports[k]->external_ids,
   "ovn-chassis-id");
 if (tunnel_id) {
-char *chassis_name;
-char *save_ptr = NULL;
-char *tokstr = xstrdup(tunnel_id);
+char *chassis_name = NULL;
 char *port_name = br_int->ports[k]->name;
 
 sset_add(, port_name);
-chassis_name = strtok_r(tokstr, OVN_MVTEP_CHASSISID_DELIM, 
_ptr);
-if (chassis_name && sset_contains(_chassis, chassis_name)) {
-sset_add(_ifaces, port_name);
+
+if (encaps_tunnel_id_parse(tunnel_id, _name, NULL)) {
+if (sset_contains(_chassis, chassis_name)) {
+sset_add(_ifaces, port_name);
+}
+free(chassis_name);
 }
-free(tokstr);
 }
 }
 
diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
index dcf7810..d467540 100644
--- a/ovn/controller/encaps.c
+++ b/ovn/controller/encaps.c
@@ -26,6 +26,13 @@
 
 VLOG_DEFINE_THIS_MODULE(encaps);
 
+/*
+ * Given there could be multiple tunnels with different IPs to the same
+ * chassis we annotate the ovn-chassis-id with
+ * OVN_MVTEP_CHASSISID_DELIM.
+ */
+#defineOVN_MVTEP_CHASSISID_DELIM '@'
+
 void
 encaps_register_ovs_idl(struct ovsdb_idl *ovs_idl)
 {
@@ -78,6 +85,83 @@ tunnel_create_name(struct tunnel_ctx *tc, const char 
*chassis_id)
 return NULL;
 }
 
+/*
+ * Returns a tunnel-id of the form 'chassis_id'-delimiter-'encap_ip'.
+ */
+char *
+encaps_tunnel_id_create(const char *chassis_id, const char *encap_ip)
+{
+return xasprintf("%s%c%s", chassis_id, OVN_MVTEP_CHASSISID_DELIM,
+ encap_ip);
+}
+
+/*
+ * Parses a 'tunnel_id' of the form .
+ * If the 'chassis_id' argument is not NULL the function will allocate memory
+ * and store the chassis-id part of the tunnel-id at '*chassis_id'.
+ * If the 'encap_ip' argument is not NULL the function will allocate memory
+ * and store the encapsulation IP part of the tunnel-id at '*encap_ip'.
+ */
+bool encaps_tunnel_id_parse(const char *tunnel_id, char **chassis_id,
+char **encap_ip)