| Message ID | 0d08acdefc7630df9a236f6bf7e731b823b3d1e3.1762950453.git.echaudro@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Architectural refactoring of hardware offload infrastructure | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
I need to analyze this patch but first I need to get the actual commit information and unified diff. Let me retrieve that. ``` git sha: 0d08acdefc7630df9a236f6bf7e731b823b3d1e3 Author: Eelco Chaudron <echaudro@redhat.com> Subject: dpif-offload: Move the netdev_any_oor() API to dpif-offload. This patch moves the netdev_any_oor() functionality from netdev-offload to dpif-offload as a new dpif_offload_netdevs_out_of_resources() API. > diff --git a/lib/dpif-offload.c b/lib/dpif-offload.c > index 5473dd2ea..0cff74a5d 100644 > --- a/lib/dpif-offload.c > +++ b/lib/dpif-offload.c > @@ -1213,6 +1213,29 @@ dpif_offload_get_netdev_by_port_id(struct dpif *dpif, > return netdev; > } > > +bool > +dpif_offload_netdevs_out_of_resources(struct dpif *dpif) > +{ > + struct dp_offload *dp_offload = dpif_offload_get_dp_offload(dpif); > + struct dpif_offload_port_dump dump; > + struct dpif_offload_port port; > + bool oor = false; > + > + if (!dp_offload || !dpif_offload_is_offload_enabled()) { > + return false; > + } > + > + DPIF_OFFLOAD_PORT_FOR_EACH (&port, &dump, dpif) { > + if (port.netdev->hw_info.oor) { > + oor = true; > + dpif_offload_port_dump_done(&dump); > + break; > + } > + } > + > + return oor; > +} Is the dpif_offload_port_dump_done() call needed when breaking out of DPIF_OFFLOAD_PORT_FOR_EACH? Typically, for_each macros handle cleanup automatically when the loop completes normally, but manual cleanup might be required when breaking early. > [ ... ] > diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c > index b324793f0..666c14da7 100644 > --- a/ofproto/ofproto-dpif-upcall.c > +++ b/ofproto/ofproto-dpif-upcall.c > @@ -974,7 +974,7 @@ udpif_run_flow_rebalance(struct udpif *udpif) > return; > } > > - if (!netdev_any_oor()) { > + if (!dpif_offload_netdevs_out_of_resources(udpif->dpif)) { > return; > } Could this change affect the behavior in udpif_run_flow_rebalance()? The original netdev_any_oor() checked all netdevs globally, while the new function only checks netdevs associated with a specific dpif. ```
diff --git a/lib/dpif-offload.c b/lib/dpif-offload.c index 5473dd2ea..0cff74a5d 100644 --- a/lib/dpif-offload.c +++ b/lib/dpif-offload.c @@ -1213,6 +1213,29 @@ dpif_offload_get_netdev_by_port_id(struct dpif *dpif, return netdev; } +bool +dpif_offload_netdevs_out_of_resources(struct dpif *dpif) +{ + struct dp_offload *dp_offload = dpif_offload_get_dp_offload(dpif); + struct dpif_offload_port_dump dump; + struct dpif_offload_port port; + bool oor = false; + + if (!dp_offload || !dpif_offload_is_offload_enabled()) { + return false; + } + + DPIF_OFFLOAD_PORT_FOR_EACH (&port, &dump, dpif) { + if (port.netdev->hw_info.oor) { + oor = true; + dpif_offload_port_dump_done(&dump); + break; + } + } + + return oor; +} + /* This function tries to offload the operations to the dpif-offload * providers. It will return the number of operations not handled, whose * pointers are re-arranged and available in **ops. */ diff --git a/lib/dpif-offload.h b/lib/dpif-offload.h index d67e7bc68..0de077919 100644 --- a/lib/dpif-offload.h +++ b/lib/dpif-offload.h @@ -84,6 +84,7 @@ struct netdev *dpif_offload_offload_get_netdev_by_port_id( struct dpif_offload *, odp_port_t); struct dpif_offload *dpif_offload_port_offloaded_by(const struct dpif *, odp_port_t); +bool dpif_offload_netdevs_out_of_resources(struct dpif *); enum dpif_offload_impl_type dpif_offload_get_impl_type( const struct dpif_offload *); enum dpif_offload_impl_type dpif_offload_get_impl_type_by_class( diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c index bd8e58206..fff49240d 100644 --- a/lib/netdev-offload.c +++ b/lib/netdev-offload.c @@ -326,31 +326,6 @@ struct port_to_netdev_data { int ifindex; }; -/* - * Find if any netdev is in OOR state. Return true if there's at least - * one netdev that's in OOR state; otherwise return false. - */ -bool -netdev_any_oor(void) - OVS_EXCLUDED(port_to_netdev_rwlock) -{ - struct port_to_netdev_data *data; - bool oor = false; - - ovs_rwlock_rdlock(&port_to_netdev_rwlock); - HMAP_FOR_EACH (data, portno_node, &port_to_netdev) { - struct netdev *dev = data->netdev; - - if (dev->hw_info.oor) { - oor = true; - break; - } - } - ovs_rwlock_unlock(&port_to_netdev_rwlock); - - return oor; -} - void netdev_ports_traverse(const char *dpif_type, bool (*cb)(struct netdev *, odp_port_t, void *), diff --git a/lib/netdev-offload.h b/lib/netdev-offload.h index 0882214a9..b8701bad6 100644 --- a/lib/netdev-offload.h +++ b/lib/netdev-offload.h @@ -83,7 +83,6 @@ int netdev_init_flow_api(struct netdev *); void netdev_uninit_flow_api(struct netdev *); int netdev_get_hw_info(struct netdev *, int); void netdev_set_hw_info(struct netdev *, int, int); -bool netdev_any_oor(void); void netdev_set_flow_api_enabled(const struct smap *ovs_other_config); struct dpif_port; diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index b324793f0..666c14da7 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -974,7 +974,7 @@ udpif_run_flow_rebalance(struct udpif *udpif) return; } - if (!netdev_any_oor()) { + if (!dpif_offload_netdevs_out_of_resources(udpif->dpif)) { return; }
Move the netdev_any_oor() API to a new dpif_offload_netdevs_out_of_resource() API. Signed-off-by: Eelco Chaudron <echaudro@redhat.com> --- lib/dpif-offload.c | 23 +++++++++++++++++++++++ lib/dpif-offload.h | 1 + lib/netdev-offload.c | 25 ------------------------- lib/netdev-offload.h | 1 - ofproto/ofproto-dpif-upcall.c | 2 +- 5 files changed, 25 insertions(+), 27 deletions(-)