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.
Cheers,
Eelco
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
