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

[liwei]

On Thu, Apr 5, 2018 at 1:16 AM, Ben Pfaff <blp at ovn.org> wrote:
> 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?

Yes, it works.
Will you apply this patch directly or need I repost this patch as your
suggestion? will this patch be backported to old stable versions?

Thanks

>
> 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;