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 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
