[ovs-dev] [PATCH] ovn: fix tunnel id overflow

[Ben Pfaff]

On Wed, Apr 04, 2018 at 02:57:38PM +0800, Wei Li wrote:
> Need to check tnlid dosen't exceed max and wrap it when it dose.
> 
> Signed-off-by: Wei Li <liwei at anbutu.com>
> ---
>  ovn/northd/ovn-northd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 396381049..f286322b1 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -298,7 +298,7 @@ allocate_tnlid(struct hmap *set, const char *name, uint32_t max,
>  {
>      for (uint32_t tnlid = *hint + 1; tnlid != *hint;
>           tnlid = tnlid + 1 <= max ? tnlid + 1 : 1) {
> -        if (!tnlid_in_use(set, tnlid)) {
> +        if (tnlid <= max && !tnlid_in_use(set, tnlid)) {
>              add_tnlid(set, tnlid);
>              *hint = tnlid;
>              return tnlid;

Thanks for the bug fix.

This code has an inconsistency: the third clause of the "for" statement
wraps around 'max' properly but the first clause does not.  I think that
a better fix would make them consistent.  My suggestion is below.  Does
it also fix the problem?

Thanks,

Ben.

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 07896be84d8f..9ca15bc2ff4b 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -292,12 +292,18 @@ tnlid_in_use(const struct hmap *set, uint32_t tnlid)
     return false;
 }
 
+static uint32_t
+next_tnlid(uint32_t tnlid, uint32_t max)
+{
+    return tnlid + 1 <= max ? tnlid + 1 : 1;
+}
+
 static uint32_t
 allocate_tnlid(struct hmap *set, const char *name, uint32_t max,
                uint32_t *hint)
 {
-    for (uint32_t tnlid = *hint + 1; tnlid != *hint;
-         tnlid = tnlid + 1 <= max ? tnlid + 1 : 1) {
+    for (uint32_t tnlid = next_tnlid(*hint, max); tnlid != *hint;
+         tnlid = next_tnlid(tnlid, max)) {
         if (!tnlid_in_use(set, tnlid)) {
             add_tnlid(set, tnlid);
             *hint = tnlid;