On 1/6/22 14:11, Van Haaren, Harry wrote: >> -----Original Message----- >> From: Finn, Emma <[email protected]> >> Sent: Wednesday, January 5, 2022 4:54 PM >> To: [email protected]; Van Haaren, Harry <[email protected]>; >> Amber, Kumar <[email protected]> >> Cc: Finn, Emma <[email protected]> >> Subject: [PATCH v4 0/9] Actions Infrastructure + Optimizations >> >> --- >> v4: >> - Rebase to master >> - Add ISA implementation of push_vlan action > > Thanks for the updated patchset Emma & Amber. > > Overall, this is working as expected and I've only had some minor > comments throughout the patchset. I've added my Acked-by to most > patches, some small open questions remain to be addressed in a v5. > > +CC Ian/Ilya , I'd like to see the v5 get merged, so let's continue to work > towards that.
Hi, Harry, Ian, others. Following up from a brief conversation during today's upstream meeting. It was brought to my attention that you're expecting this series and the 'hash' one to be accepted into 2.17. Though there are few issues with that: 1. This review for v4 was actually very first review of the patch set. The other one as of today doesn't have any reviews at all. Looking at the change log for this patch set it doesn't seem that internal reviews behind the closed doors (if there were any) requested any significant changes. In any case, internal reviews is not the way how open-source projects work. 2. The soft freeze for 2.17 began on Jan 3 in accordance with our release schedule (even a bit later), and as you know, during the soft freeze we're not normally accepting patches that wasn't already reviewed before the soft freeze begun. https://mail.openvswitch.org/pipermail/ovs-dev/2022-January/390487.html That's not the end of a world, but you need to request an exception in reply to the email linked above. But I have a few high-level concerns regarding the patch set itself, and that's a bigger problem for me: 1. What are the benefits of these patch sets? A lot of infrastructure changes are made, but the benefits of them are unclear. Why these changes are needed in the end? I believe, that was the main reason why community had no interest in reviewing these patches. 2.17 is supposed to be a new LTS, so infrastructure changes without clear benefits might not be a good fit taking into account time constraints and lack of reviews. 2. The same concern that I already brought to you attention in other conversations, e.g. on the ovs-security list about a month ago. It's related to all developments in that area: why this is tied to the userspace datapath? i.e. why execution of actions depends on the datapath? This seems artificial and complicates testing a lot. Like current autovalidator is not able to test most of the packet parsing cases, the same way autovalidator will not be able to test execution of actions. I have some more comments, but they are more related to the actual code and above 2 are the most important for now. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
