mbox series

[ovs-dev,v2,0/3] Use newest OVS version to fix Undefined Behavior

Message ID 20220420055537.445495-1-amorenoz@redhat.com
Headers show
Series Use newest OVS version to fix Undefined Behavior | expand

Message

Adrián Moreno April 20, 2022, 5:55 a.m. UTC
A series has been recently introduced in OVS that has two main benefits:

- Undefined Behavior in iterator loops is removed. This enforces some
  restrictions on how this macros can be used, namely:
  * User-provided iterator variable is set to NULL after the loop ends
    normally
  * In _SAFE version of the loop, it's not always safe to use the 'next'
    variable. When it would point to the list's head, it is instead set
    to NULL by the iteration macros.

- _SAFE version of the iterator macros now do not require the user to
  provide a 'next' variable leading to cleaner and less error prone
  code.

This series bumps ovs to the latest HEAD in master branch and adapts the
code accordingly.

v1->v2:
- Rebase
- Use ovs branch-2.17

Adrian Moreno (3):
  treewide: bump ovs and fix problematic loops
  parallel-hmap: rewrite iterator using multivar helpers
  treewide: remove next variable in _SAFE loops

 controller-vtep/binding.c   |   4 +-
 controller-vtep/gateway.c   |   4 +-
 controller-vtep/vtep.c      |   4 +-
 controller/binding.c        |  21 +++---
 controller/encaps.c         |   4 +-
 controller/if-status.c      |  17 ++---
 controller/lflow-cache.c    |   3 +-
 controller/lflow-conj-ids.c |  18 +++--
 controller/lflow.c          |  36 +++++-----
 controller/ofctrl-seqno.c   |   4 +-
 controller/ofctrl.c         |  84 +++++++++++-----------
 controller/ovn-controller.c |  20 +++---
 controller/patch.c          |   4 +-
 controller/physical.c       |   4 +-
 controller/pinctrl.c        |  69 +++++++++---------
 controller/vif-plug.c       |   8 +--
 ic/ovn-ic.c                 |  12 ++--
 lib/actions.c               |   2 +-
 lib/expr.c                  |  51 +++++++------
 lib/extend-table.c          |  20 +++---
 lib/extend-table.h          |   4 +-
 lib/ovn-parallel-hmap.h     |  10 +--
 lib/ovn-util.c              |   2 +-
 lib/vif-plug-provider.c     |   8 +--
 northd/northd.c             | 139 +++++++++++++++++-------------------
 northd/ovn-northd-ddlog.c   |   4 +-
 northd/ovn-northd.c         |  16 ++---
 ovs                         |   2 +-
 utilities/ovn-ic-nbctl.c    |   4 +-
 utilities/ovn-ic-sbctl.c    |   4 +-
 utilities/ovn-nbctl.c       |  14 ++--
 utilities/ovn-sbctl.c       |   4 +-
 utilities/ovn-trace.c       |  12 ++--
 33 files changed, 299 insertions(+), 313 deletions(-)

Comments

Mark Michelson April 21, 2022, 7:24 p.m. UTC | #1
Thanks Dumitru and Adrian. I rolled Dumitru's edit into patch 3 and 
pushed the series to the main branch.

On 4/20/22 01:55, Adrian Moreno wrote:
> A series has been recently introduced in OVS that has two main benefits:
> 
> - Undefined Behavior in iterator loops is removed. This enforces some
>    restrictions on how this macros can be used, namely:
>    * User-provided iterator variable is set to NULL after the loop ends
>      normally
>    * In _SAFE version of the loop, it's not always safe to use the 'next'
>      variable. When it would point to the list's head, it is instead set
>      to NULL by the iteration macros.
> 
> - _SAFE version of the iterator macros now do not require the user to
>    provide a 'next' variable leading to cleaner and less error prone
>    code.
> 
> This series bumps ovs to the latest HEAD in master branch and adapts the
> code accordingly.
> 
> v1->v2:
> - Rebase
> - Use ovs branch-2.17
> 
> Adrian Moreno (3):
>    treewide: bump ovs and fix problematic loops
>    parallel-hmap: rewrite iterator using multivar helpers
>    treewide: remove next variable in _SAFE loops
> 
>   controller-vtep/binding.c   |   4 +-
>   controller-vtep/gateway.c   |   4 +-
>   controller-vtep/vtep.c      |   4 +-
>   controller/binding.c        |  21 +++---
>   controller/encaps.c         |   4 +-
>   controller/if-status.c      |  17 ++---
>   controller/lflow-cache.c    |   3 +-
>   controller/lflow-conj-ids.c |  18 +++--
>   controller/lflow.c          |  36 +++++-----
>   controller/ofctrl-seqno.c   |   4 +-
>   controller/ofctrl.c         |  84 +++++++++++-----------
>   controller/ovn-controller.c |  20 +++---
>   controller/patch.c          |   4 +-
>   controller/physical.c       |   4 +-
>   controller/pinctrl.c        |  69 +++++++++---------
>   controller/vif-plug.c       |   8 +--
>   ic/ovn-ic.c                 |  12 ++--
>   lib/actions.c               |   2 +-
>   lib/expr.c                  |  51 +++++++------
>   lib/extend-table.c          |  20 +++---
>   lib/extend-table.h          |   4 +-
>   lib/ovn-parallel-hmap.h     |  10 +--
>   lib/ovn-util.c              |   2 +-
>   lib/vif-plug-provider.c     |   8 +--
>   northd/northd.c             | 139 +++++++++++++++++-------------------
>   northd/ovn-northd-ddlog.c   |   4 +-
>   northd/ovn-northd.c         |  16 ++---
>   ovs                         |   2 +-
>   utilities/ovn-ic-nbctl.c    |   4 +-
>   utilities/ovn-ic-sbctl.c    |   4 +-
>   utilities/ovn-nbctl.c       |  14 ++--
>   utilities/ovn-sbctl.c       |   4 +-
>   utilities/ovn-trace.c       |  12 ++--
>   33 files changed, 299 insertions(+), 313 deletions(-)
>
Dumitru Ceara April 26, 2022, 1:03 p.m. UTC | #2
On 4/21/22 21:24, Mark Michelson wrote:
> Thanks Dumitru and Adrian. I rolled Dumitru's edit into patch 3 and
> pushed the series to the main branch.
> 

Hi Mark,

Shouldn't patches 1/3 and 2/3 go to branch-22.03 too?

I'm not sure anymore if we ever decided on 22.03 to be the official LTS
branch but your comment here seems to indicate that:

https://mail.openvswitch.org/pipermail/ovs-dev/2022-April/393381.html

Thanks,
Dumitru

> On 4/20/22 01:55, Adrian Moreno wrote:
>> A series has been recently introduced in OVS that has two main benefits:
>>
>> - Undefined Behavior in iterator loops is removed. This enforces some
>>    restrictions on how this macros can be used, namely:
>>    * User-provided iterator variable is set to NULL after the loop ends
>>      normally
>>    * In _SAFE version of the loop, it's not always safe to use the 'next'
>>      variable. When it would point to the list's head, it is instead set
>>      to NULL by the iteration macros.
>>
>> - _SAFE version of the iterator macros now do not require the user to
>>    provide a 'next' variable leading to cleaner and less error prone
>>    code.
>>
>> This series bumps ovs to the latest HEAD in master branch and adapts the
>> code accordingly.
>>
>> v1->v2:
>> - Rebase
>> - Use ovs branch-2.17
>>
>> Adrian Moreno (3):
>>    treewide: bump ovs and fix problematic loops
>>    parallel-hmap: rewrite iterator using multivar helpers
>>    treewide: remove next variable in _SAFE loops
>>
>>   controller-vtep/binding.c   |   4 +-
>>   controller-vtep/gateway.c   |   4 +-
>>   controller-vtep/vtep.c      |   4 +-
>>   controller/binding.c        |  21 +++---
>>   controller/encaps.c         |   4 +-
>>   controller/if-status.c      |  17 ++---
>>   controller/lflow-cache.c    |   3 +-
>>   controller/lflow-conj-ids.c |  18 +++--
>>   controller/lflow.c          |  36 +++++-----
>>   controller/ofctrl-seqno.c   |   4 +-
>>   controller/ofctrl.c         |  84 +++++++++++-----------
>>   controller/ovn-controller.c |  20 +++---
>>   controller/patch.c          |   4 +-
>>   controller/physical.c       |   4 +-
>>   controller/pinctrl.c        |  69 +++++++++---------
>>   controller/vif-plug.c       |   8 +--
>>   ic/ovn-ic.c                 |  12 ++--
>>   lib/actions.c               |   2 +-
>>   lib/expr.c                  |  51 +++++++------
>>   lib/extend-table.c          |  20 +++---
>>   lib/extend-table.h          |   4 +-
>>   lib/ovn-parallel-hmap.h     |  10 +--
>>   lib/ovn-util.c              |   2 +-
>>   lib/vif-plug-provider.c     |   8 +--
>>   northd/northd.c             | 139 +++++++++++++++++-------------------
>>   northd/ovn-northd-ddlog.c   |   4 +-
>>   northd/ovn-northd.c         |  16 ++---
>>   ovs                         |   2 +-
>>   utilities/ovn-ic-nbctl.c    |   4 +-
>>   utilities/ovn-ic-sbctl.c    |   4 +-
>>   utilities/ovn-nbctl.c       |  14 ++--
>>   utilities/ovn-sbctl.c       |   4 +-
>>   utilities/ovn-trace.c       |  12 ++--
>>   33 files changed, 299 insertions(+), 313 deletions(-)
>>
>
Mark Michelson April 26, 2022, 2:52 p.m. UTC | #3
On 4/26/22 09:03, Dumitru Ceara wrote:
> On 4/21/22 21:24, Mark Michelson wrote:
>> Thanks Dumitru and Adrian. I rolled Dumitru's edit into patch 3 and
>> pushed the series to the main branch.
>>
> 
> Hi Mark,
> 
> Shouldn't patches 1/3 and 2/3 go to branch-22.03 too?
> 
> I'm not sure anymore if we ever decided on 22.03 to be the official LTS
> branch but your comment here seems to indicate that:
> 
> https://mail.openvswitch.org/pipermail/ovs-dev/2022-April/393381.html
> 
> Thanks,
> Dumitru

Yes 22.03 is an LTS. I backported patches 1 and 2 to branch-22.03.

> 
>> On 4/20/22 01:55, Adrian Moreno wrote:
>>> A series has been recently introduced in OVS that has two main benefits:
>>>
>>> - Undefined Behavior in iterator loops is removed. This enforces some
>>>     restrictions on how this macros can be used, namely:
>>>     * User-provided iterator variable is set to NULL after the loop ends
>>>       normally
>>>     * In _SAFE version of the loop, it's not always safe to use the 'next'
>>>       variable. When it would point to the list's head, it is instead set
>>>       to NULL by the iteration macros.
>>>
>>> - _SAFE version of the iterator macros now do not require the user to
>>>     provide a 'next' variable leading to cleaner and less error prone
>>>     code.
>>>
>>> This series bumps ovs to the latest HEAD in master branch and adapts the
>>> code accordingly.
>>>
>>> v1->v2:
>>> - Rebase
>>> - Use ovs branch-2.17
>>>
>>> Adrian Moreno (3):
>>>     treewide: bump ovs and fix problematic loops
>>>     parallel-hmap: rewrite iterator using multivar helpers
>>>     treewide: remove next variable in _SAFE loops
>>>
>>>    controller-vtep/binding.c   |   4 +-
>>>    controller-vtep/gateway.c   |   4 +-
>>>    controller-vtep/vtep.c      |   4 +-
>>>    controller/binding.c        |  21 +++---
>>>    controller/encaps.c         |   4 +-
>>>    controller/if-status.c      |  17 ++---
>>>    controller/lflow-cache.c    |   3 +-
>>>    controller/lflow-conj-ids.c |  18 +++--
>>>    controller/lflow.c          |  36 +++++-----
>>>    controller/ofctrl-seqno.c   |   4 +-
>>>    controller/ofctrl.c         |  84 +++++++++++-----------
>>>    controller/ovn-controller.c |  20 +++---
>>>    controller/patch.c          |   4 +-
>>>    controller/physical.c       |   4 +-
>>>    controller/pinctrl.c        |  69 +++++++++---------
>>>    controller/vif-plug.c       |   8 +--
>>>    ic/ovn-ic.c                 |  12 ++--
>>>    lib/actions.c               |   2 +-
>>>    lib/expr.c                  |  51 +++++++------
>>>    lib/extend-table.c          |  20 +++---
>>>    lib/extend-table.h          |   4 +-
>>>    lib/ovn-parallel-hmap.h     |  10 +--
>>>    lib/ovn-util.c              |   2 +-
>>>    lib/vif-plug-provider.c     |   8 +--
>>>    northd/northd.c             | 139 +++++++++++++++++-------------------
>>>    northd/ovn-northd-ddlog.c   |   4 +-
>>>    northd/ovn-northd.c         |  16 ++---
>>>    ovs                         |   2 +-
>>>    utilities/ovn-ic-nbctl.c    |   4 +-
>>>    utilities/ovn-ic-sbctl.c    |   4 +-
>>>    utilities/ovn-nbctl.c       |  14 ++--
>>>    utilities/ovn-sbctl.c       |   4 +-
>>>    utilities/ovn-trace.c       |  12 ++--
>>>    33 files changed, 299 insertions(+), 313 deletions(-)
>>>
>>
>