Hi Dumitru, sorry for the late response! I would like to use my changes to enable a smoother upgrade process: currently the ovn-match-northd-version check depends on the south database version, actions, northd_stages, and the minor and global OVN versions.
With my change, I want to allow for more flexible upgrading that isn't tied to ovn-match-northd-version. In my opinion, this change makes sense because most often, out of all the factors considered by ovn-match-northd-version, it's the south database version that changes, while the controllers can continue working with it. If my option is set to true = the controller will terminate if it finds a difference in actions, which is the same as when ovn-match-northd-version == true, but only checking for action matching, which relaxes the upgrade requirements. This looks like we could parameterize the ovn-match-northd-version option, but that would break backward compatibility. In this situation - controller itself would work without those flows that it could not parse, but only if there's no difference in the logical pipeline length between northd and the controller. I tested the following case - I built northd on one version (25.03 in my case), and controllers on version 24.03. Initially, the controllers didn't allow even basic traffic cases. Then I shifted the openflow table definitions (specified in lflow.h) to match what the controller has in version 25.03, but that didn't solve the problem. The issue turned out to be that the controller was populating openflow tables from table 8 up to the length of its pipeline (LOG_PIPELINE_**_LEN), while northd already had a longer pipeline, so tables with numbers higher than 8 + the old pipeline length didn't have openflow rules. After this, the controllers started working for cases that don't involve unparsed rules. I hope I explained what I meant clearly. Based on the what i said above, wouldn't it be better to move the OpenFlow table number definitions from macros to variables stored in memory? I don't see significant drawbacks to changing this behavior. We could store the northd pipeline length in the south database and bump it along with the table numbers in the controller. This would, for example, enable a scenario where logical flow tables are added to northd, and controllers of older versions could still work with this. I will send a patch with this change in advance for further discussion. On 27.08.2025 12:56, Dumitru Ceara wrote: > On 6/18/25 1:43 PM, Dumitru Ceara wrote: >> On 6/9/25 7:09 PM, Rukomoinikova Aleksandra wrote: >>> Hi Dumitru, sorry for the long wait =) >> Hi Alexandra, >> >> No worries, I'm also slow in responding, I apologize. >> > Hi Alexandra, > > This patch needs a rebase and it won't make it into 25.09.0 as it missed > the branch date. But more importantly, it would be nice to understand > better the use case below. > >>> On 02.06.2025 18:27, Dumitru Ceara wrote: >>>> Hi Alexandra, >>>> >>>> On 6/2/25 4:31 PM, Rukomoinikova Aleksandra wrote: >>>>> Hi Dumitru! Thanks for your time. I understand your questions, they are >>>>> reasonable, but I cannot assess the criticality of adding new actions >>>>> without stupidly reading the code and verifying the supported actions, I >>>>> wanted to do this in an automated process. it seems to me that my >>>>> approach makes updating the controller more transparent to the user in >>>>> terms of understanding the changes that have occurred in the code. I >>>>> understand that storing actions in the sb global database does not look >>>>> like the cleanest way, but I have not found another one. If you have any >>>>> suggestions on how to improve my approach, then I will be glad to hear >>>>> it and send a new version. >>>>> >>> Currently, the version verification is tied to the |northd| version, >>> which isn’t always relevant. The northd version is linked to the >>> database and minor version, and checksums are calculated for stages in >>> both |northd| and actions. However, version changes don’t always >>> correlate with changes in actions. I believe my approach provides >>> greater flexibility for updates, without being dependent on versioning. >>> >>> The only changes that could critically break compatibility during an >>> update (whether in |northd| or the controller itself) are modifications >>> to actions or shifts in the ingress/egress pipeline’s OpenFlow table >>> starting points—please correct me if I’m wrong. It might also make sense >>> to add the ability to compare these values, making updates more transparent. >>> >> I'm afraid that if we do something like that and relax the condition to >> match on northd-version (which includes the SB schema version too) we >> might make it easier to miss cases that cause incompatibility between >> ovn-controllers and ovn-northd. I might be wrong though. >> >> >>>> I think I'm failing to understand the use case, so please bear with me. >>>> >>>> As far as I can tell your patch does the following: >>>> 1. ovn-northd stores the action names it's aware of as a string set in >>>> the SB >>>> 2. ovn-controller compares that with the action names it knows of >>>> 3. if the sets of actions are equal it's all fine >>>> 4. else, if there's a difference: >>>> 4a. if validate-actions-fail-mode is not set then ovn-controller just >>>> logs the mismatch, tries to process all logical flows, potentially fails >>>> to parse some >>>> 4b. if validate-actions-fail-mode=fail then ovn-controller logs the >>>> mismatch and triggers a full incremental processing recompute (this >>>> continues until the mismatch goes away) >>>> >>>> How will the configuration of ovn-controllers look like in your >>>> deployment? Is it: >>>> - ovn-match-northd-version=false (or not set) >>>> - validate-actions-fail-mode not set (i.e., log and continue) >>> in this case the controller will continue to work, it will not be able >>> to parse some actions, this gives reverse compatibility with the current >>> version of work in this case (when >>> >>> ovn-match-northd-version is set to false) >>> >> Right, so, that was my question too, maybe I didn't express it properly. >> Let me try again: >> >> Even without your patch, if we don't set ovn-match-northd-version=true, >> the behavior of ovn-controller will be to try to process the contents of >> the Southbound database. >> >> In case the Southbound (and northd) have already been upgraded to a >> version that includes logical actions that ovn-controller cannot >> process, ovn-controller will just fail parsing those logical actions >> (and flow) but will process the rest of the Southbound and reconcile >> what OpenFlows it can. >> >> How is that different from the case with your patch applied and >> "validate-actions-fail-mode=continue"? >> > I'm still not sure about the use case you're trying to address, it would > be nice to get some clarification on my queries above. > > In any case, I'm marking the patch as "changes requested" for now in > patchwork as it needs at least a rebase. > >>>> Because, unless I'm missing something, the generated OpenFlow rules and >>>> configuration is exactly the same as when you use the current OVN code >>>> with ovn-match-northd-version=false. >>>> >>>> ovn-controllers will still fail to parse logical flows that use >>>> unsupported actions and will log that failure. >>>> >>>> Or is the goal of your patch just to get the exact diff between the sets >>>> of actions (supported by ovn-northd vs supported by ovn-controller)? In >>>> this latter case, wouldn't it make sense to just add an appctl command >>>> that dumps the sets of actions for both ovn-northd and ovn-controller >>>> and do the diff offline? >>> I think it's not very convenient to make 2 handles, dump the output and >>> get a diff >>> >> Why not? With your approach you'd still have to iterate through all >> chassis and check all ovn-controller logs on each chassis to see if >> there's a mismatch in supported actions. >> >> An ovn-appctl command to list supported actions for norhtd and >> ovn-controller actually is a bit cleaner in my opinion because it >> doesn't force the CMS to rely on log scraping. >> >> Regards, >> Dumitru >> > Thanks, > Dumitru > -- regards, Alexandra. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
