On 4 Feb 2026, at 23:50, Ilya Maximets wrote:
> On 2/2/26 11:50 AM, Eelco Chaudron wrote: >> Move port manager instances from provider-specific data structures >> into the common struct dpif_offload. This removes duplication across >> providers and clarifies ownership of port-related state. >> >> With the port manager owned by dpif_offload, offload providers are >> required to use the common port management APIs. This simplifies >> external port management and eliminates the need for provider-specific >> port enumeration logic. >> >> The port dump functions were also simplified using the >> CMAP_CURSOR_FOR_EACH_CONTINUE() API. >> >> Signed-off-by: Eelco Chaudron <[email protected]> >> --- [...] >> +/* Iterates through each DPIF_OFFLOAD_PORT in DPIF, using DUMP as state. >> + * >> + * If you break out of the loop, then you need to free the dump structure by >> + * hand using dpif_offload_port_dump_done(). >> + * >> + * IMPORTANT: You must not RCU quiesce during the FOR_EACH loop, or even >> + * between port dump function calls if used standalone. */ >> +#define DPIF_OFFLOAD_PROVIDERS_PORT_FOR_EACH(DPIF_OFFLOAD_PORT, DUMP, DPIF) >> \ >> + for (dpif_offload_port_dump_start(DUMP, DPIF); >> \ >> + (dpif_offload_port_dump_next(DUMP, DPIF_OFFLOAD_PORT) >> \ >> + ? true >> \ >> + : (dpif_offload_port_dump_done(DUMP), false)); >> \ >> + ) > > Still not sure why we need this macro and any of the dpif_offload_port_dump_* > functions... See below. >> + >> static struct netdev * >> dpif_offload_get_netdev_by_port_id__( >> struct dpif_offload_provider_collection *collection, >> @@ -1312,7 +1300,7 @@ dpif_offload_netdevs_out_of_resources(struct dpif >> *dpif) >> { >> struct dpif_offload_provider_collection *collection; >> struct dpif_offload_port_dump dump; >> - struct dpif_offload_port port; >> + struct dpif_offload_port *port; >> bool oor = false; >> >> collection = dpif_get_offload_provider_collection(dpif); >> @@ -1320,8 +1308,8 @@ dpif_offload_netdevs_out_of_resources(struct dpif >> *dpif) >> return false; >> } >> >> - DPIF_OFFLOAD_PORT_FOR_EACH (&port, &dump, dpif) { >> - if (port.netdev->hw_info.oor) { >> + DPIF_OFFLOAD_PROVIDERS_PORT_FOR_EACH (&port, &dump, dpif) { > > Can this be just two nested loops instead? > > LIST_FOR_EACH (offload, dpif_list_node, providers_list) { > DPIF_OFFLOAD_PORT_FOR_EACH (port, offload) { > ... > } > } > > Or do we have some other use case for this dpif_offload_port_dump_* interface? I think I had a bit of tunnel vision trying to keep the API alive ;) I thought we might need it for conntrack, but that could just use the same approach you suggested above. In addition, it turns out the cursor approach for CMAPs is not RCU safe across quiescent states. Anyway, I’ve removed this API and used your suggestion instead. If we feel like we need this abstraction in the future, we can always re-add it. I’ll send out a v4. //Eelco _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
