mbox series

[ovs-dev,v2,0/4] Mac cache handling refactor

Message ID 20240507062517.387329-1-amusil@redhat.com
Headers show
Series Mac cache handling refactor | expand

Message

Ales Musil May 7, 2024, 6:25 a.m. UTC 
There were two modules in controller mac_cache and mac-learn, both of
them did very similar thing with pretty big overlap. The goal of the
series is to consolidate and merge both of those modules into single
one. That will reduce the duplication and should make it easier for
future updates to MAC binding, FDB or packet buffering functionality.

There is also fix to properly handle tunnel_key change for LSP, LRP,
LR and LS. This was inconsistent and could lead to wrong flows being
still present even after the tunnel key change. This is not a huge
issue because the tunnel_key is rarelyt changed during runtime.

Ales Musil (4):
  northd, controller: Handle tunnel_key change consistently.
  controller: Rename mac_cache to to mac-cache.
  controller: Merge the mac-cache and mac-learn.
  controller: Use datapath key for the mac cache thresholds.

 controller/automake.mk      |   6 +-
 controller/binding.c        |  13 +-
 controller/mac-cache.c      | 745 ++++++++++++++++++++++++++++++++++++
 controller/mac-cache.h      | 210 ++++++++++
 controller/mac-learn.c      | 482 -----------------------
 controller/mac-learn.h      | 145 -------
 controller/mac_cache.c      | 547 --------------------------
 controller/mac_cache.h      | 124 ------
 controller/ovn-controller.c | 214 +++++++----
 controller/pinctrl.c        | 165 ++++----
 controller/statctrl.c       |   7 +-
 controller/statctrl.h       |   2 +-
 northd/northd.c             |   7 +
 tests/ovn.at                |  56 ++-
 14 files changed, 1253 insertions(+), 1470 deletions(-)
 create mode 100644 controller/mac-cache.c
 create mode 100644 controller/mac-cache.h
 delete mode 100644 controller/mac-learn.c
 delete mode 100644 controller/mac-learn.h
 delete mode 100644 controller/mac_cache.c
 delete mode 100644 controller/mac_cache.h

Comments

Numan Siddique May 7, 2024, 4:49 p.m. UTC | #1 
On Tue, May 7, 2024 at 2:25 AM Ales Musil <amusil@redhat.com> wrote:
>
> There were two modules in controller mac_cache and mac-learn, both of
> them did very similar thing with pretty big overlap. The goal of the
> series is to consolidate and merge both of those modules into single
> one. That will reduce the duplication and should make it easier for
> future updates to MAC binding, FDB or packet buffering functionality.
>
> There is also fix to properly handle tunnel_key change for LSP, LRP,
> LR and LS. This was inconsistent and could lead to wrong flows being
> still present even after the tunnel key change. This is not a huge
> issue because the tunnel_key is rarelyt changed during runtime.
>
> Ales Musil (4):
>   northd, controller: Handle tunnel_key change consistently.
>   controller: Rename mac_cache to to mac-cache.
>   controller: Merge the mac-cache and mac-learn.
>   controller: Use datapath key for the mac cache thresholds.

Thanks.  I applied the series to the main branch.
Do we need a backport ?  If so,  I'm inclined to backport the first
patch only as it fixes the issue.
Let me know your thoughts.

Numan


>
>  controller/automake.mk      |   6 +-
>  controller/binding.c        |  13 +-
>  controller/mac-cache.c      | 745 ++++++++++++++++++++++++++++++++++++
>  controller/mac-cache.h      | 210 ++++++++++
>  controller/mac-learn.c      | 482 -----------------------
>  controller/mac-learn.h      | 145 -------
>  controller/mac_cache.c      | 547 --------------------------
>  controller/mac_cache.h      | 124 ------
>  controller/ovn-controller.c | 214 +++++++----
>  controller/pinctrl.c        | 165 ++++----
>  controller/statctrl.c       |   7 +-
>  controller/statctrl.h       |   2 +-
>  northd/northd.c             |   7 +
>  tests/ovn.at                |  56 ++-
>  14 files changed, 1253 insertions(+), 1470 deletions(-)
>  create mode 100644 controller/mac-cache.c
>  create mode 100644 controller/mac-cache.h
>  delete mode 100644 controller/mac-learn.c
>  delete mode 100644 controller/mac-learn.h
>  delete mode 100644 controller/mac_cache.c
>  delete mode 100644 controller/mac_cache.h
>
> --
> 2.44.0
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Ales Musil May 7, 2024, 4:50 p.m. UTC | #2 
On Tue, May 7, 2024 at 6:49 PM Numan Siddique <numans@ovn.org> wrote:

> On Tue, May 7, 2024 at 2:25 AM Ales Musil <amusil@redhat.com> wrote:
> >
> > There were two modules in controller mac_cache and mac-learn, both of
> > them did very similar thing with pretty big overlap. The goal of the
> > series is to consolidate and merge both of those modules into single
> > one. That will reduce the duplication and should make it easier for
> > future updates to MAC binding, FDB or packet buffering functionality.
> >
> > There is also fix to properly handle tunnel_key change for LSP, LRP,
> > LR and LS. This was inconsistent and could lead to wrong flows being
> > still present even after the tunnel key change. This is not a huge
> > issue because the tunnel_key is rarelyt changed during runtime.
> >
> > Ales Musil (4):
> >   northd, controller: Handle tunnel_key change consistently.
> >   controller: Rename mac_cache to to mac-cache.
> >   controller: Merge the mac-cache and mac-learn.
> >   controller: Use datapath key for the mac cache thresholds.
>
> Thanks.  I applied the series to the main branch.
> Do we need a backport ?  If so,  I'm inclined to backport the first
> patch only as it fixes the issue.
> Let me know your thoughts.
>

That's correct, only the first patch should be backported.


>
> Numan
>
>
> >
> >  controller/automake.mk      |   6 +-
> >  controller/binding.c        |  13 +-
> >  controller/mac-cache.c      | 745 ++++++++++++++++++++++++++++++++++++
> >  controller/mac-cache.h      | 210 ++++++++++
> >  controller/mac-learn.c      | 482 -----------------------
> >  controller/mac-learn.h      | 145 -------
> >  controller/mac_cache.c      | 547 --------------------------
> >  controller/mac_cache.h      | 124 ------
> >  controller/ovn-controller.c | 214 +++++++----
> >  controller/pinctrl.c        | 165 ++++----
> >  controller/statctrl.c       |   7 +-
> >  controller/statctrl.h       |   2 +-
> >  northd/northd.c             |   7 +
> >  tests/ovn.at                |  56 ++-
> >  14 files changed, 1253 insertions(+), 1470 deletions(-)
> >  create mode 100644 controller/mac-cache.c
> >  create mode 100644 controller/mac-cache.h
> >  delete mode 100644 controller/mac-learn.c
> >  delete mode 100644 controller/mac-learn.h
> >  delete mode 100644 controller/mac_cache.c
> >  delete mode 100644 controller/mac_cache.h
> >
> > --
> > 2.44.0
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
>
Thanks,
Ales
Numan Siddique May 7, 2024, 9:17 p.m. UTC | #3 
On Tue, May 7, 2024 at 12:51 PM Ales Musil <amusil@redhat.com> wrote:
>
> On Tue, May 7, 2024 at 6:49 PM Numan Siddique <numans@ovn.org> wrote:
>
> > On Tue, May 7, 2024 at 2:25 AM Ales Musil <amusil@redhat.com> wrote:
> > >
> > > There were two modules in controller mac_cache and mac-learn, both of
> > > them did very similar thing with pretty big overlap. The goal of the
> > > series is to consolidate and merge both of those modules into single
> > > one. That will reduce the duplication and should make it easier for
> > > future updates to MAC binding, FDB or packet buffering functionality.
> > >
> > > There is also fix to properly handle tunnel_key change for LSP, LRP,
> > > LR and LS. This was inconsistent and could lead to wrong flows being
> > > still present even after the tunnel key change. This is not a huge
> > > issue because the tunnel_key is rarelyt changed during runtime.
> > >
> > > Ales Musil (4):
> > >   northd, controller: Handle tunnel_key change consistently.
> > >   controller: Rename mac_cache to to mac-cache.
> > >   controller: Merge the mac-cache and mac-learn.
> > >   controller: Use datapath key for the mac cache thresholds.
> >
> > Thanks.  I applied the series to the main branch.
> > Do we need a backport ?  If so,  I'm inclined to backport the first
> > patch only as it fixes the issue.
> > Let me know your thoughts.
> >
>
> That's correct, only the first patch should be backported.
The backport doesn't apply cleanly to branch-24.03.  Can you please
submit the backport patch for branch-24.03.

Thanks
Numan

>
>
> >
> > Numan
> >
> >
> > >
> > >  controller/automake.mk      |   6 +-
> > >  controller/binding.c        |  13 +-
> > >  controller/mac-cache.c      | 745 ++++++++++++++++++++++++++++++++++++
> > >  controller/mac-cache.h      | 210 ++++++++++
> > >  controller/mac-learn.c      | 482 -----------------------
> > >  controller/mac-learn.h      | 145 -------
> > >  controller/mac_cache.c      | 547 --------------------------
> > >  controller/mac_cache.h      | 124 ------
> > >  controller/ovn-controller.c | 214 +++++++----
> > >  controller/pinctrl.c        | 165 ++++----
> > >  controller/statctrl.c       |   7 +-
> > >  controller/statctrl.h       |   2 +-
> > >  northd/northd.c             |   7 +
> > >  tests/ovn.at                |  56 ++-
> > >  14 files changed, 1253 insertions(+), 1470 deletions(-)
> > >  create mode 100644 controller/mac-cache.c
> > >  create mode 100644 controller/mac-cache.h
> > >  delete mode 100644 controller/mac-learn.c
> > >  delete mode 100644 controller/mac-learn.h
> > >  delete mode 100644 controller/mac_cache.c
> > >  delete mode 100644 controller/mac_cache.h
> > >
> > > --
> > > 2.44.0
> > >
> > > _______________________________________________
> > > dev mailing list
> > > dev@openvswitch.org
> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > >
> >
> >
> Thanks,
> Ales
>
> --
>
> Ales Musil
>
> Senior Software Engineer - OVN Core
>
> Red Hat EMEA <https://www.redhat.com>
>
> amusil@redhat.com
> <https://red.ht/sig>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev