[ovs-dev] [PATCH v3] ovn-controller: Fix parsing of OVN tunnel IDs
Ben Pfaff
blp at ovn.org
Fri Jun 7 16:41:48 UTC 2019
On Mon, May 27, 2019 at 10:55:52AM +0200, Dumitru Ceara wrote:
> Encap tunnel-ids are of the form:
> <chassis-id><OVN_MVTEP_CHASSISID_DELIM><encap-ip>.
> 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 <iyervl at ymail.com>
> Reported-at: https://bugzilla.redhat.com/1708131
> Reported-by: Haidong Li <haili at redhat.com>
> Fixes: b520ca7 ("Support for multiple VTEP in OVN")
> Signed-off-by: Dumitru Ceara <dceara at redhat.com>
> ---
>
> v3: Rebase
> v2: Update commit message
This seems about right.
Here's an incremental with some suggestions for encaps_tunnel_id_parse()
and encaps_tunnel_id_match(). I'm pretty happy with the
encaps_tunnel_id_parse() change; the encaps_tunnel_id_match() one might
be going over the top.
Let me know what you think.
diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
index 61b3eab3971f..3b3921a736e2 100644
--- a/ovn/controller/encaps.c
+++ b/ovn/controller/encaps.c
@@ -102,64 +102,51 @@ encaps_tunnel_id_create(const char *chassis_id, const char *encap_ip)
* 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)
+bool
+encaps_tunnel_id_parse(const char *tunnel_id, char **chassis_id,
+ char **encap_ip)
{
- const char *match = strchr(tunnel_id, OVN_MVTEP_CHASSISID_DELIM);
-
- if (!match) {
+ /* Find the delimiter. Fail if there is no delimiter or if <chassis_name>
+ * or <IP> is the empty string.*/
+ const char *d = strchr(tunnel_id, OVN_MVTEP_CHASSISID_DELIM);
+ if (d == tunnel_id || !d || !d[1]) {
return false;
}
if (chassis_id) {
- size_t chassis_id_len = (match - tunnel_id);
-
- *chassis_id = xmemdup0(tunnel_id, chassis_id_len);
+ *chassis_id = xmemdup0(tunnel_id, d - tunnel_id);
}
-
- /* Consume the tunnel-id delimiter. */
- match++;
-
if (encap_ip) {
- /*
- * If the value has morphed into something other than
- * <chassis-id><delim><encap-ip>, fail and free already allocated
- * memory (i.e., chassis_id).
- */
- if (*match == 0) {
- if (chassis_id) {
- free(*chassis_id);
- }
- return false;
- }
- *encap_ip = xstrdup(match);
+ *encap_ip = xstrdup(d + 1);
}
-
return true;
}
/*
- * Returns true if a given tunnel_id contains 'chassis_id' and, if specified,
- * the given 'encap_ip'. Returns false otherwise.
+ * Returns true if 'tunnel_id' contains 'chassis_id' and, if specified, the
+ * given 'encap_ip'. Returns false otherwise.
*/
-bool encaps_tunnel_id_match(const char *tunnel_id, const char *chassis_id,
- const char *encap_ip)
+bool
+encaps_tunnel_id_match(const char *tunnel_id, const char *chassis_id,
+ const char *encap_ip)
{
- if (strstr(tunnel_id, chassis_id) != tunnel_id) {
- return false;
- }
-
- size_t chassis_id_len = strlen(chassis_id);
-
- if (tunnel_id[chassis_id_len] != OVN_MVTEP_CHASSISID_DELIM) {
- return false;
- }
-
- if (encap_ip && strcmp(&tunnel_id[chassis_id_len + 1], encap_ip)) {
- return false;
+ while (*tunnel_id == *chassis_id) {
+ if (!*tunnel_id) {
+ /* 'tunnel_id' and 'chassis_id' are equal strings. This is a
+ * mismatch because 'tunnel_id' is missing the delimiter and IP. */
+ return false;
+ }
+ tunnel_id++;
+ chassis_id++;
}
- return true;
+ /* We found the first byte that disagrees between 'tunnel_id' and
+ * 'chassis_id'. If we consumed all of 'chassis_id' and arrived at the
+ * delimiter in 'tunnel_id' (and if 'encap_ip' is correct, if it was
+ * supplied), it's a match. */
+ return (*tunnel_id == OVN_MVTEP_CHASSISID_DELIM
+ && *chassis_id == '\0'
+ && (!encap_ip || !strcmp(tunnel_id + 1, encap_ip)));
}
static void
More information about the dev
mailing list