On 14 Jan 2026, at 14:48, Aaron Conole wrote:
> Eelco Chaudron <[email protected]> writes: > >> This patch introduces a common port management layer for >> offload providers and integrates it into the dpif-offload >> subsystem. Existing dummy offload provider is updated to >> use the new APIs. >> >> Acked-by: Eli Britstein <elibr.nvidia.com> >> Signed-off-by: Eelco Chaudron <[email protected]> >> --- > > I've started going back through with an eye towards memory ordering and > lifecycle management. I have a comment here. [...] >> >> +struct dpif_offload_port_mgr_port * >> +dpif_offload_port_mgr_find_by_odp_port(struct dpif_offload_port_mgr *mgr, >> + odp_port_t port_no) >> +{ >> + struct dpif_offload_port_mgr_port *port; >> + >> + CMAP_FOR_EACH_WITH_HASH (port, odp_port_node, >> + hash_int(odp_to_u32(port_no), 0), >> + &mgr->odp_port_to_port) { >> + if (port->port_no == port_no) { >> + return port; >> + } >> + } >> + return NULL; >> +} >> + >> +struct dpif_offload_port_mgr_port * >> +dpif_offload_port_mgr_remove(struct dpif_offload_port_mgr *mgr, >> + odp_port_t port_no, bool keep_netdev_ref) > > This API allows specifying whether or not to release the port's netdev > object - but from what I can tell (and maybe I missed something?) no one > ever calls it with a 'false' parameter. I'd rather remove the > keep_netdev_ref argument and just force the caller to schedule release > of the port->netdev at a later time. I accidentally inverted the test > in my head while reviewing something and thought I saw a double-free > (but it was just my own confusion). Thanks, Aaron, for taking another stab at this series :) I’ll remove the boolean in v6 and add some comments to the functions to clarify the netdev reference behavior. >> +{ >> + struct dpif_offload_port_mgr_port *port; >> + >> + ovs_mutex_lock(&mgr->cmap_mod_lock); >> + >> + port = dpif_offload_port_mgr_find_by_odp_port(mgr, port_no); >> + >> + if (port) { >> + cmap_remove(&mgr->odp_port_to_port, &port->odp_port_node, >> + hash_int(odp_to_u32(port_no), 0)); >> + cmap_remove(&mgr->netdev_to_port, &port->netdev_node, >> + hash_pointer(port->netdev, 0)); >> + >> + if (port->ifindex >= 0) { >> + cmap_remove(&mgr->ifindex_to_port, &port->ifindex_node, >> + hash_int(port->ifindex, 0)); >> + } >> + if (!keep_netdev_ref) { >> + netdev_close(port->netdev); >> + } >> + } >> + >> + ovs_mutex_unlock(&mgr->cmap_mod_lock); >> + return port; >> +} [...] _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
