Re: [ovs-dev] [PATCH v3] ovn-controller: Fix parsing of OVN tunnel IDs
On Wed, Jun 12, 2019 at 5:47 PM Ben Pfaff wrote:
>
> On Wed, Jun 12, 2019 at 10:05:36AM +0200, Dumitru Ceara wrote:
> > On Tue, Jun 11, 2019 at 12:08 PM Dumitru Ceara wrote:
> > >
> > > On Fri, Jun 7, 2019 at 6:42 PM Ben Pfaff wrote:
> > > >
> > > > On Mon, May 27, 2019 at 10:55:52AM +0200, Dumitru Ceara wrote:
> > > > > 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
> > > > > ---
> > > > >
> > > > > 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.
> > >
> > > Thanks for the suggestions.
> > > I like the new encaps_tunnel_id_parse() as it's more succinct and even
> > > though encaps_tunnel_id_match() might be going a bit over the top it's
> > > the most efficient way to do it indeed.
> > >
> > > Thanks,
> > > Dumitru
> >
> > Hi Ben,
> >
> > Shall I send a v4 incorporating your suggestions or will you directly
> > apply your incremental patch with the changes on top of mine?
>
> If you like the changes, or some of them, you can send a v4
> incorporating them. If you don't like them, let me know and I'll look
> at v3 again.
Done, just sent a v4:
https://patchwork.ozlabs.org/patch/1114661/
Thanks,
Dumitru
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] ovn-controller: Fix parsing of OVN tunnel IDs
On Wed, Jun 12, 2019 at 10:05:36AM +0200, Dumitru Ceara wrote:
> On Tue, Jun 11, 2019 at 12:08 PM Dumitru Ceara wrote:
> >
> > On Fri, Jun 7, 2019 at 6:42 PM Ben Pfaff wrote:
> > >
> > > On Mon, May 27, 2019 at 10:55:52AM +0200, Dumitru Ceara wrote:
> > > > 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
> > > > ---
> > > >
> > > > 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.
> >
> > Thanks for the suggestions.
> > I like the new encaps_tunnel_id_parse() as it's more succinct and even
> > though encaps_tunnel_id_match() might be going a bit over the top it's
> > the most efficient way to do it indeed.
> >
> > Thanks,
> > Dumitru
>
> Hi Ben,
>
> Shall I send a v4 incorporating your suggestions or will you directly
> apply your incremental patch with the changes on top of mine?
If you like the changes, or some of them, you can send a v4
incorporating them. If you don't like them, let me know and I'll look
at v3 again.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] ovn-controller: Fix parsing of OVN tunnel IDs
On Tue, Jun 11, 2019 at 12:08 PM Dumitru Ceara wrote:
>
> On Fri, Jun 7, 2019 at 6:42 PM Ben Pfaff wrote:
> >
> > On Mon, May 27, 2019 at 10:55:52AM +0200, Dumitru Ceara wrote:
> > > 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
> > > ---
> > >
> > > 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.
>
> Thanks for the suggestions.
> I like the new encaps_tunnel_id_parse() as it's more succinct and even
> though encaps_tunnel_id_match() might be going a bit over the top it's
> the most efficient way to do it indeed.
>
> Thanks,
> Dumitru
Hi Ben,
Shall I send a v4 incorporating your suggestions or will you directly
apply your incremental patch with the changes on top of mine?
Thanks,
Dumitru
>
> >
> > 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
> >
> > + * or 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
> > - * , 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(_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
Re: [ovs-dev] [PATCH v3] ovn-controller: Fix parsing of OVN tunnel IDs
On Fri, Jun 7, 2019 at 6:42 PM Ben Pfaff wrote:
>
> On Mon, May 27, 2019 at 10:55:52AM +0200, Dumitru Ceara wrote:
> > 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
> > ---
> >
> > 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.
Thanks for the suggestions.
I like the new encaps_tunnel_id_parse() as it's more succinct and even
though encaps_tunnel_id_match() might be going a bit over the top it's
the most efficient way to do it indeed.
Thanks,
Dumitru
>
> 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
>
> + * or 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
> - * , 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(_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
___
dev
Re: [ovs-dev] [PATCH v3] ovn-controller: Fix parsing of OVN tunnel IDs
On Mon, May 27, 2019 at 10:55:52AM +0200, Dumitru Ceara wrote:
> 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
> ---
>
> 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
+ * or 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
- * , 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(_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
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] ovn-controller: Fix parsing of OVN tunnel IDs
Aaron Conole writes: > Numan Siddique writes: > >> On Mon, May 27, 2019, 4:01 PM Dumitru Ceara wrote: >> >> On Mon, May 27, 2019 at 11:57 AM 0-day Robot wrote: >> > >> > 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/bfd.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 >> >> Hi Aaron, >> >> I'm not sure what the problem is here. Can it be related to what the >> bot is doing? >> >> For the v3 version I had updated the master branch and rebased the dev >> branch; then I generated the patch from my dev branch with: >> git format-patch -M master --subject-prefix="PATCH v3" -o _patch/ >> >> If i apply the patch with "git am" everything seems to work fine: >> $ git checkout -b test origin/master >> $ git am _patch/0001-ovn-controller-Fix-parsing-of-OVN-tunnel-IDs.patch >> Applying: ovn-controller: Fix parsing of OVN tunnel IDs >> $ git log --oneline -1 >> 8c559d4 ovn-controller: Fix parsing of OVN tunnel IDs >> $ git --version >> git version 1.8.3.1 >> >> Looks like the bot is trying to apply the patches to the ovn-org/ovn repo. > > I'll double check - I thought I had a regex that was correctly > separating it. Should be all set. >> Aaron is it possible to change the bot to apply the patches to the >> new repo only if "ovn" is with in the [*ovn*] ? > > That's the intention. > >> Right now we use - "[PATCH ..]ovn: ... " for OVN related patches in the >> openvswitch repo. > > Right. > >> Thanks >> Numan >> >> If I use git-pw it also seems to work fine: >> $ git checkout -b test2 origin/master >> Branch test2 set up to track remote branch master from origin. >> Switched to a new branch 'test2' >> $ git-pw patch apply 1105697 >> Starting new HTTPS connection (1): patchwork.ozlabs.org >> Starting new HTTPS connection (1): patchwork.ozlabs.org >> Applying: ovn-controller: Fix parsing of OVN tunnel IDs >> $ git log --oneline -1 >> aa83849 ovn-controller: Fix parsing of OVN tunnel IDs >> >> Thanks, >> Dumitru >> ___ >> 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 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] ovn-controller: Fix parsing of OVN tunnel IDs
Numan Siddique writes: > On Mon, May 27, 2019, 4:01 PM Dumitru Ceara wrote: > > On Mon, May 27, 2019 at 11:57 AM 0-day Robot wrote: > > > > 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/bfd.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 > > Hi Aaron, > > I'm not sure what the problem is here. Can it be related to what the > bot is doing? > > For the v3 version I had updated the master branch and rebased the dev > branch; then I generated the patch from my dev branch with: > git format-patch -M master --subject-prefix="PATCH v3" -o _patch/ > > If i apply the patch with "git am" everything seems to work fine: > $ git checkout -b test origin/master > $ git am _patch/0001-ovn-controller-Fix-parsing-of-OVN-tunnel-IDs.patch > Applying: ovn-controller: Fix parsing of OVN tunnel IDs > $ git log --oneline -1 > 8c559d4 ovn-controller: Fix parsing of OVN tunnel IDs > $ git --version > git version 1.8.3.1 > > Looks like the bot is trying to apply the patches to the ovn-org/ovn repo. I'll double check - I thought I had a regex that was correctly separating it. > Aaron is it possible to change the bot to apply the patches to the new repo > only if "ovn" is with in the [*ovn*] ? That's the intention. > Right now we use - "[PATCH ..]ovn: ... " for OVN related patches in the > openvswitch repo. Right. > Thanks > Numan > > If I use git-pw it also seems to work fine: > $ git checkout -b test2 origin/master > Branch test2 set up to track remote branch master from origin. > Switched to a new branch 'test2' > $ git-pw patch apply 1105697 > Starting new HTTPS connection (1): patchwork.ozlabs.org > Starting new HTTPS connection (1): patchwork.ozlabs.org > Applying: ovn-controller: Fix parsing of OVN tunnel IDs > $ git log --oneline -1 > aa83849 ovn-controller: Fix parsing of OVN tunnel IDs > > Thanks, > Dumitru > ___ > 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 v3] ovn-controller: Fix parsing of OVN tunnel IDs
On Mon, May 27, 2019, 4:01 PM Dumitru Ceara wrote: > On Mon, May 27, 2019 at 11:57 AM 0-day Robot wrote: > > > > 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/bfd.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 > > Hi Aaron, > > I'm not sure what the problem is here. Can it be related to what the > bot is doing? > > For the v3 version I had updated the master branch and rebased the dev > branch; then I generated the patch from my dev branch with: > git format-patch -M master --subject-prefix="PATCH v3" -o _patch/ > > If i apply the patch with "git am" everything seems to work fine: > $ git checkout -b test origin/master > $ git am _patch/0001-ovn-controller-Fix-parsing-of-OVN-tunnel-IDs.patch > Applying: ovn-controller: Fix parsing of OVN tunnel IDs > $ git log --oneline -1 > 8c559d4 ovn-controller: Fix parsing of OVN tunnel IDs > $ git --version > git version 1.8.3.1 > Looks like the bot is trying to apply the patches to the ovn-org/ovn repo. Aaron is it possible to change the bot to apply the patches to the new repo only if "ovn" is with in the [*ovn*] ? Right now we use - "[PATCH ..]ovn: ... " for OVN related patches in the openvswitch repo. Thanks Numan > If I use git-pw it also seems to work fine: > $ git checkout -b test2 origin/master > Branch test2 set up to track remote branch master from origin. > Switched to a new branch 'test2' > $ git-pw patch apply 1105697 > Starting new HTTPS connection (1): patchwork.ozlabs.org > Starting new HTTPS connection (1): patchwork.ozlabs.org > Applying: ovn-controller: Fix parsing of OVN tunnel IDs > $ git log --oneline -1 > aa83849 ovn-controller: Fix parsing of OVN tunnel IDs > > Thanks, > Dumitru > ___ > 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 v3] ovn-controller: Fix parsing of OVN tunnel IDs
On Mon, May 27, 2019 at 11:57 AM 0-day Robot wrote: > > 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/bfd.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 Hi Aaron, I'm not sure what the problem is here. Can it be related to what the bot is doing? For the v3 version I had updated the master branch and rebased the dev branch; then I generated the patch from my dev branch with: git format-patch -M master --subject-prefix="PATCH v3" -o _patch/ If i apply the patch with "git am" everything seems to work fine: $ git checkout -b test origin/master $ git am _patch/0001-ovn-controller-Fix-parsing-of-OVN-tunnel-IDs.patch Applying: ovn-controller: Fix parsing of OVN tunnel IDs $ git log --oneline -1 8c559d4 ovn-controller: Fix parsing of OVN tunnel IDs $ git --version git version 1.8.3.1 If I use git-pw it also seems to work fine: $ git checkout -b test2 origin/master Branch test2 set up to track remote branch master from origin. Switched to a new branch 'test2' $ git-pw patch apply 1105697 Starting new HTTPS connection (1): patchwork.ozlabs.org Starting new HTTPS connection (1): patchwork.ozlabs.org Applying: ovn-controller: Fix parsing of OVN tunnel IDs $ git log --oneline -1 aa83849 ovn-controller: Fix parsing of OVN tunnel IDs Thanks, Dumitru ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] ovn-controller: Fix parsing of OVN tunnel IDs
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/bfd.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 v3] ovn-controller: Fix parsing of OVN tunnel IDs
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
---
v3: Rebase
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 a2f590d..22db00a 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 7bd52d1..61b3eab 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
