On 1/15/26 10:01 PM, Eelco Chaudron wrote: > > > On 15 Jan 2026, at 18:44, Ilya Maximets wrote: >> On 1/15/26 10:19 AM, Eelco Chaudron wrote: > [...] > >> Thanks, Eelco for the update! I read through the set again and it >> reads much easier after the renames. There are still a few small >> style issues throughout the set, but they are all minor like an >> extra empty line at the EOF of dpif-offload.c in patch 7, a few >> more places with a missing double space or underscore or the thread >> safety annotations not on a separate line. There is also a missed >> rename of dpif_offload_flow_get_n_offloaded() into something like >> dpif_ofload_flow_count(), since the underlying callback was renamed. >> And the provider_collection_add() should return a positive errno >> after all, since it's passed directly into ovs_strerror(). >> >> As discussed off-list, instead of me writing all of these nits down >> in the emails and then you fixing them, it's simpler if I just >> handle the merge and fix the nits on commit. >> >> Also, found a couple more issues that we should address: >> >> 1. In the first patch, the dpif_offload_dump_next() function is >> describing a case that should be impossible, but still tries >> to handle it. We shouldn't do that. And instead we need to >> just take the collection reference in dpif_offload_dump_start() >> to make sure the collection doesn't go away during the dump. >> And then we can fully rely on the LIST_FOR_EACH_CONTINUE. >> >> 2. The per-port priority configuration should be per-interface >> instead as datapath doesn't deal with ports, it only works >> with interfaces. And some ports, like bonds can have multiple >> interfaces. It may not be a big deal as in a normal case >> I don't think we would need different offload for interfaces >> in the same port, but it's not really correct to configure >> ports, when it is applied on the interface. >> >> And there are two things that I flagged in v5, but we decided to >> handle separately: >> >> 3. Mass-rename of offload provider structures and functions to >> make them coherent and shorter than they are. Most of them >> are not exported and don't need extensive prefixes. And these >> prefixes need unification after moving stuff between netdev* >> and dpif* modules. >> >> 4. Move of the port manager instances from provider-specific >> structures to the common struct dpif_offload. This should >> eliminate some duplication and make the module boundaries >> more clear. This should also eliminate the need for the >> port dump API or at least significantly simplify it, e.g. >> by just iterating over port cmap with a cmap_cursor. >> >> >> So, the situation is the following: We have branching planned for >> tomorrow. Full CI run takes about 24 hours. So, we don't really >> have time for v7. We could postpone branching, but it also seems >> unreasonable as we only really need changes in about 4 patches >> out of 40, and none of the issues listed above are critical or >> breaking any functionality. They are mostly internal code movements. >> It would be much easier to get the set merged and then work on the >> 4 items above next week and backport to the new branch. >> >> As mentioned earlier, to save some time, I can handle the merging >> and fix all the small nits on commit instead of wasting a lot of >> time on writing them down on the mailing list and then Eelco >> fixing them. >> >> Eelco, Aaron, does that sound like a good plan? Or did I miss >> anything in my summary? > > Thanks, Ilya, for summarizing our offline discussion! This seems like > the best way forward, and I will start working on those patches once > I get back on Monday.
Thanks, Eelco, Eli, and Aaron! As agreed, I fixed all the small nits that I saw throughout the set, added ACKs from Aaron, and applied the set to main. Also backported patch 34 down to 3.3. Let's try to get the patches for the remaining 4 items above soon after branching. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
