Bug 1897201

Summary:	ovn-controller: Remove logical datapath dependency from expression parsing code
Product:	Red Hat Enterprise Linux Fast Datapath	Reporter:	Ilya Maximets <i.maximets>
Component:	OVN	Assignee:	Dumitru Ceara <dceara>
Status:	CLOSED WONTFIX	QA Contact:	Jianlin Shi <jishi>
Severity:	high	Docs Contact:
Priority:	medium
Version:	FDP 20.I	CC:	ctrautma, dcbw, dceara, mmichels, nusiddiq
Target Milestone:	---
Target Release:	---
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:		Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2024-02-14 21:11:42 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

Description Ilya Maximets 2020-11-12 14:57:01 UTC

Expression parsing code in ovn-controller depends on the logical datapath.
This is because of optimization made for BZ 1818128:
https://github.com/ovn-org/ovn/commit/bff01d49d6f27424a2bef197ef50bae8b03cbd00
Parser reconstructs the name of a port group using the datapath tunnel key.

This makes it really hard to implement solution for BZ 1877002, because
expression parsing requires 1:1 lfow to logical datapath relation.

Need to remove this dependency from expression parsing code to unblock
BZ 1877002 and actually untangle the code, i.e. make it cleaner and better
logically separated.

Comment 1 Dan Williams 2021-05-25 17:15:18 UTC

Ilya, is this bug still relevant now that bug 1877002 is finished?

Comment 2 Dumitru Ceara 2021-05-26 07:24:28 UTC

(In reply to Dan Williams from comment #1)
> Ilya, is this bug still relevant now that bug 1877002 is finished?

I think it still is.  Right now we reparse the complete match/action
expression for each logical datapath in the datapath group.  We can
optimize lflow processing in ovn-controller by:
1. pre-parsing the match and action expressions regardless of logical
   datapath/datapath group the flow is applied to.
2. In a second step we can refine the pre-parsed expression for each
   logical datapath in the lflow's datapath group.

Comment 3 Mark Michelson 2022-10-05 19:21:21 UTC

I'm going to echo Dan's question from last year and ask if this is still relevant. I suppose that the optimization potential may still exist, but the bigger question is whether it's worth pursuing. We're in a different position than we were 2 years ago with regards to OVN performance. These days, ovn-controller is typically not the performance bottleneck anymore, and I'm curious if we think this sort of optimization would show real gains, performance-wise.

Comment 4 Dumitru Ceara 2022-10-06 10:04:57 UTC

(In reply to Mark Michelson from comment #3)
> I'm going to echo Dan's question from last year and ask if this is still
> relevant. I suppose that the optimization potential may still exist, but the
> bigger question is whether it's worth pursuing. We're in a different
> position than we were 2 years ago with regards to OVN performance. These
> days, ovn-controller is typically not the performance bottleneck anymore,
> and I'm curious if we think this sort of optimization would show real gains,
> performance-wise.

Not sure whether this is a good argument as it's quite subjective but I think the code will also get significantly more complex than it already is.

Comment 5 Ilya Maximets 2022-10-21 08:03:47 UTC

(In reply to Mark Michelson from comment #3)
> I'm going to echo Dan's question from last year and ask if this is still
> relevant. I suppose that the optimization potential may still exist, but the
> bigger question is whether it's worth pursuing. We're in a different
> position than we were 2 years ago with regards to OVN performance. These
> days, ovn-controller is typically not the performance bottleneck anymore,
> and I'm curious if we think this sort of optimization would show real gains,
> performance-wise.

I think, at this stage this is more of a tech-debt task.  Even though the
code may become a bit more complex from one side, it should be much better
isolated providing more flexibility to developers on how to use it.

So, I'm not sure if this BZ should stay open or how much benefit it may
provide if actually implemented.  But that's a common thing with this kind
of tech-debt tasks.

Comment 6 OVN Bot 2024-02-14 21:11:40 UTC

This issue is being closed as an automatic process due to the issue's age. If you wish to re-open this issue, please do so in Jira (https://issues.redhat.com) in the 'FDP' project. Please be sure to set the component to the latest OVN version where this issue is known to occur. If this is a feature request or improvement, please set the component to 'OVN'.