On 16 Jan 2026, at 12:13, Ilya Maximets wrote:
> 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. Thanks for taking care of this Ilya!! I’ll start working on the followup patches on Monday. //Eelco _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
