On 11 Jan 2026, at 16:54, Aaron Conole wrote:
> Eelco Chaudron via dev <[email protected]> writes: > [...] >> REQUEST FOR COMMENTS >> ------------------------------------------------------------- >> This is a significant architectural change that affects >> core OVS infrastructure. We welcome feedback on: >> >> - The overall architectural approach and the >> `dpif-offload-provider` concept. >> - The API design, particularly the new asynchronous model >> for `dpif-netdev`. >> - The migration strategy and any potential backward >> compatibility concerns. >> - Performance implications of the new framework. > > Just a few quick thoughts while I'm finishing up (I'm on patch 29): > > 1. There are a few groups that might make more sense to squash together > while applying. For example, 23-25 is particularly agregious example > where it introduces some temp change, and then deletes it with a > different change. While seeing the steps is nice, it is *really* > confusing - even on the review side, it's a bit jarring to see. > Typically we don't allow patches that clean up earlier patches in the > same series. It may be worth spending the time to squash some of > these. Regardless of how messy it can be. I don’t mind squashing some patches in the series, but on the other hand, keeping smaller change sets can make it easier to bisect if we do run into problems. One of the reasons for doing these intermediate moves was to allow running the full regression suite at each step. For v5, I’ll leave it as is for now and wait for Ilya to share his feedback before making changes. > 2. The dpdk offload stuff - my opinion is it should be named with > rte_flow rather than dpdk. It could make things difficult in the > future if a second offload type becomes available for DPDK ports. > WDYT (I realize that's a rename that takes a bit of time to do)? We already discussed this during the RFC series. At that time, the code used rte_flow throughout (and sometimes just rte, as some function names were getting quite long), but we ultimately decided to rename everything to dpdk. The main motivation was to avoid renaming things again within the series, which would be more confusing for existing users. Using dpdk also better reflects the scope of the code: it is DPDK-specific, not a generic RTE interface. The rte_ prefix doesn’t add much clarity here. If we ever introduce another DPDK-specific offload mechanism in the future, we can still distinguish it cleanly using a suffix (for example, dpdk_async), without overloading the naming now. > 3. During my read-ahead, I got the heebee jeebees with > [34/40] dpif_netdev: Fix nullable memcpy in queue_netdev_flow_put(). > I haven't completed my review to that portion, but I suspect that if > it's a real issue, it could be first in the series or even separate > and applied separately. WDYT? I’m not entirely sure why this only surfaced with my changes, and I haven’t spent too much time digging into it yet. If we manage to get the series in this week, we can backport this fix and should be fine. If, for some reason, that doesn’t happen, I’ll send it as a separate patch. Aaron, even though you’re at v29, I’ll be sending out a v5 soon. It doesn’t change any of the patches you’re still reviewing, but it will allow you to start ACKing the earlier patches. This should help speed up the review, as we need this completed by the end of the week before branching (and I’m on PTO this Friday). [...] _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
