mbox series

[ovs-dev,v2,0/3] northd: Optimize parsing of LSP addresses.

Message ID 20230302131110.1665580-1-i.maximets@ovn.org
Headers show
Series northd: Optimize parsing of LSP addresses. | expand

Message

Ilya Maximets March 2, 2023, 1:11 p.m. UTC 
This patch set optimizes usage of string operations and avoids parsing
same LSP addresses multiple times.

Performance tests with ovn-heater show 5-10% improvement in high-scale
density-light scenarios.

Version 2:
  - Reduced code duplication in is_dynamic_lsp_address(). [Mark]

Ilya Maximets (3):
  treewide: Remove unnecessary strlen() calls.
  ovn-util: Optimize is_dynamic_lsp_address.
  northd: Don't parse LSP addresses twice.

 controller/chassis.c  |  4 +--
 controller/encaps.c   |  2 +-
 controller/physical.c |  2 +-
 controller/pinctrl.c  |  6 ++--
 ic/ovn-ic.c           |  4 +--
 lib/ovn-util.c        | 45 ++++++++++++++++++++----------
 northd/northd.c       | 64 +++++++++++++++----------------------------
 utilities/ovn-nbctl.c | 14 +++++-----
 8 files changed, 68 insertions(+), 73 deletions(-)

Comments

Dumitru Ceara March 2, 2023, 1:14 p.m. UTC | #1 
On 3/2/23 14:11, Ilya Maximets wrote:
> This patch set optimizes usage of string operations and avoids parsing
> same LSP addresses multiple times.
> 
> Performance tests with ovn-heater show 5-10% improvement in high-scale
> density-light scenarios.
> 
> Version 2:
>   - Reduced code duplication in is_dynamic_lsp_address(). [Mark]
> 

Thanks for this revision, Ilya!

I'm thinking of applying this to the main branch and branch-23.03 before
the release tomorrow.  Mark, Simon, do you guys agree with that?  In
theory this is not a bug fix but it's safe enough IMO and the benefits
are significant.

Regards,
Dumitru
Simon Horman March 2, 2023, 2:45 p.m. UTC | #2 
On Thu, Mar 02, 2023 at 02:14:24PM +0100, Dumitru Ceara wrote:
> On 3/2/23 14:11, Ilya Maximets wrote:
> > This patch set optimizes usage of string operations and avoids parsing
> > same LSP addresses multiple times.
> > 
> > Performance tests with ovn-heater show 5-10% improvement in high-scale
> > density-light scenarios.
> > 
> > Version 2:
> >   - Reduced code duplication in is_dynamic_lsp_address(). [Mark]
> > 
> 
> Thanks for this revision, Ilya!
> 
> I'm thinking of applying this to the main branch and branch-23.03 before
> the release tomorrow.  Mark, Simon, do you guys agree with that?  In
> theory this is not a bug fix but it's safe enough IMO and the benefits
> are significant.

Sounds reasonable to me, though I'm not clear how
strict the backporting rules are.
Mark Michelson March 2, 2023, 2:50 p.m. UTC | #3 
On 3/2/23 09:45, Simon Horman wrote:
> On Thu, Mar 02, 2023 at 02:14:24PM +0100, Dumitru Ceara wrote:
>> On 3/2/23 14:11, Ilya Maximets wrote:
>>> This patch set optimizes usage of string operations and avoids parsing
>>> same LSP addresses multiple times.
>>>
>>> Performance tests with ovn-heater show 5-10% improvement in high-scale
>>> density-light scenarios.
>>>
>>> Version 2:
>>>    - Reduced code duplication in is_dynamic_lsp_address(). [Mark]
>>>
>>
>> Thanks for this revision, Ilya!
>>
>> I'm thinking of applying this to the main branch and branch-23.03 before
>> the release tomorrow.  Mark, Simon, do you guys agree with that?  In
>> theory this is not a bug fix but it's safe enough IMO and the benefits
>> are significant.
> 
> Sounds reasonable to me, though I'm not clear how
> strict the backporting rules are.
> 

I'm also fine with this since this is a pretty minor changeset and its 
probability of causing problems is extremely low.
Dumitru Ceara March 3, 2023, 11:07 a.m. UTC | #4 
On 3/2/23 15:50, Mark Michelson wrote:
> On 3/2/23 09:45, Simon Horman wrote:
>> On Thu, Mar 02, 2023 at 02:14:24PM +0100, Dumitru Ceara wrote:
>>> On 3/2/23 14:11, Ilya Maximets wrote:
>>>> This patch set optimizes usage of string operations and avoids parsing
>>>> same LSP addresses multiple times.
>>>>
>>>> Performance tests with ovn-heater show 5-10% improvement in high-scale
>>>> density-light scenarios.
>>>>
>>>> Version 2:
>>>>    - Reduced code duplication in is_dynamic_lsp_address(). [Mark]
>>>>
>>>
>>> Thanks for this revision, Ilya!
>>>
>>> I'm thinking of applying this to the main branch and branch-23.03 before
>>> the release tomorrow.  Mark, Simon, do you guys agree with that?  In
>>> theory this is not a bug fix but it's safe enough IMO and the benefits
>>> are significant.
>>
>> Sounds reasonable to me, though I'm not clear how
>> strict the backporting rules are.
>>

committer-responsibilities.rst says:

  Feature development should be done only on master. Occasionally it makes sense
  to add a feature to the most recent release branch, before the first actual
  release of that branch. These should be handled in the same way as bug fixes,
  that is, first implemented on master and then backported.

We're kind of in that situation now.  Although this series is not a
feature but at the same time not really a bugfix either.

> 
> I'm also fine with this since this is a pretty minor changeset and its
> probability of causing problems is extremely low.
> 

Thanks for your inputs on this and for the patch and reviews!

I pushed this to main and branch-23.03.

Regards,
Dumitru