On 11 Dec 2025, at 11:05, Eli Britstein wrote:
>> -----Original Message----- >> From: dev <[email protected]> On Behalf Of Eelco Chaudron >> Sent: Tuesday, 2 December 2025 16:05 >> To: [email protected] >> Subject: [ovs-dev] [PATCH v2 16/41] dpif-offload: Move >> hw_miss_packet_postprocess netdev API to dpif. >> >> This patch moves the hardware packet miss recovery API from the netdev- >> offload to the dpif-offload provider. The API name has been changed from >> hw_miss_packet_recover() to hw_miss_packet_postprocess() to reflect that it >> may also be used for other tasks in the future, such as conntrack post- >> processing. > I think "recover" is still a better name than "postprocess". > If a packet was partially processed in the HW and missed to SW, need to set > its state to be as if it was processed in SW from the beginning. > In current netdev-offload-dpdk, the only scenario currently supported is with > tunnel decap. > With CT we will have to recover more metadata of the packet, like recirc-id, > ct-state/mark/label. > With dp-hash we will have to recover the hash value calculated by the HW. > The term "postprocess" might imply some processing done after further SW > processing but it's the not. > I also agree "recover" might not be the perfect term, but still better than > "postprocess". > In the API definition comment, you also mention "recover": > " * Recover and/or set the packet state ..." > If you still stick with renaming, need to be consistent and also change > function names like netdev_offload_dpdk_hw_miss_packet_recover. > WDYT? Hi Eli, Thanks for the feedback. I understand your concern about the naming. Here are some points on why I chose hw_miss_packet_postprocess() and my thoughts on alternatives: - I used “postprocess” in the context of post-hardware processing, not general SW processing. - In a later patch, this function will be used for flow lookup as part of partial HW offload, so it’s more general than just “recovering” a packet. - This is a general callback for anything that needs HW post-processing, hence the name (not SW processing). - The name already contains hw, so it conveys the hardware context. - We could consider removing packet_miss from the name entirely to emphasize the generality, e.g., hw_post_processing(). WDYT? I’m happy to settle on a name that is both general enough for future use and clear about its purpose. //Eelco >> >> Signed-off-by: Eelco Chaudron <[email protected]> >> --- >> lib/dpif-netdev.c | 18 +++++++++++------- >> lib/dpif-offload-dpdk.c | 10 ++++++++++ >> lib/dpif-offload-provider.h | 12 ++++++++++++ >> lib/dpif-offload.c | 36 +++++++++++++++++++++++++++++++++++ >> lib/dpif-offload.h | 2 ++ >> lib/netdev-offload-dpdk.c | 3 +-- >> lib/netdev-offload-dpdk.h | 2 ++ >> lib/netdev-offload-provider.h | 6 ------ >> lib/netdev-offload.c | 33 +++----------------------------- >> lib/netdev-offload.h | 4 ++-- >> lib/netdev.c | 2 +- >> 11 files changed, 80 insertions(+), 48 deletions(-) >> >> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 82df85823..5d32a47b3 >> 100644 >> --- a/lib/dpif-netdev.c >> +++ b/lib/dpif-netdev.c >> @@ -122,7 +122,7 @@ COVERAGE_DEFINE(datapath_drop_invalid_bond); >> COVERAGE_DEFINE(datapath_drop_invalid_tnl_port); >> COVERAGE_DEFINE(datapath_drop_rx_invalid_packet); >> #ifdef ALLOW_EXPERIMENTAL_API /* Packet restoration API required. */ - >> COVERAGE_DEFINE(datapath_drop_hw_miss_recover); >> +COVERAGE_DEFINE(datapath_drop_hw_miss_postprocess); >> #endif >> >> /* Protects against changes to 'dp_netdevs'. */ @@ -8423,14 +8423,18 @@ >> dp_netdev_hw_flow(const struct dp_netdev_pmd_thread *pmd, #ifdef >> ALLOW_EXPERIMENTAL_API /* Packet restoration API required. */ >> /* Restore the packet if HW processing was terminated before completion. >> */ >> struct dp_netdev_rxq *rxq = pmd->ctx.last_rxq; >> - bool miss_api_supported; >> + bool postprocess_api_supported; >> + >> + atomic_read_relaxed(&rxq->port->netdev- >>> hw_info.postprocess_api_supported, >> + &postprocess_api_supported); >> + if (postprocess_api_supported) { >> + int err = dpif_offload_netdev_hw_miss_packet_postprocess( >> + rxq->port->netdev, packet); > Blank line Already has a blank line. >> >> - atomic_read_relaxed(&rxq->port->netdev->hw_info.miss_api_supported, >> - &miss_api_supported); >> - if (miss_api_supported) { >> - int err = netdev_hw_miss_packet_recover(rxq->port->netdev, packet); >> if (err && err != EOPNOTSUPP) { >> - COVERAGE_INC(datapath_drop_hw_miss_recover); >> + if (err != ECANCELED) { >> + COVERAGE_INC(datapath_drop_hw_miss_postprocess); >> + } >> return -1; >> } >> } >> diff --git a/lib/dpif-offload-dpdk.c b/lib/dpif-offload-dpdk.c index >> 76a8946c9..c8ae1adb2 100644 >> --- a/lib/dpif-offload-dpdk.c >> +++ b/lib/dpif-offload-dpdk.c >> @@ -286,6 +286,14 @@ dpif_offload_dpdk_netdev_flow_flush(const struct >> dpif_offload *offload >> return netdev_offload_dpdk_flow_flush(netdev); >> } >> >> +static int >> +dpif_offload_dpdk_netdev_hw_miss_packet_postprocess( >> + const struct dpif_offload *offload_ OVS_UNUSED, struct netdev *netdev, >> + struct dp_packet *packet) >> +{ >> + return netdev_offload_dpdk_hw_miss_packet_recover(netdev, packet); >> +} >> + >> struct dpif_offload_class dpif_offload_dpdk_class = { >> .type = "dpdk", >> .supported_dpif_types = (const char *const[]) { @@ -300,6 +308,8 @@ >> struct dpif_offload_class dpif_offload_dpdk_class = { >> .port_del = dpif_offload_dpdk_port_del, >> .flow_get_n_offloaded = dpif_offload_dpdk_get_n_offloaded, >> .netdev_flow_flush = dpif_offload_dpdk_netdev_flow_flush, >> + .netdev_hw_miss_packet_postprocess = \ >> + dpif_offload_dpdk_netdev_hw_miss_packet_postprocess, >> }; >> >> /* XXX: Temporary functions below, which will be removed once fully diff >> --git >> a/lib/dpif-offload-provider.h b/lib/dpif-offload-provider.h index >> 60d6c5321..78cf5d7fa 100644 >> --- a/lib/dpif-offload-provider.h >> +++ b/lib/dpif-offload-provider.h >> @@ -168,6 +168,18 @@ struct dpif_offload_class { >> /* Deletes all offloaded flows on this netdev. Return 0 if successful, >> * otherwise returns a positive errno value. */ >> int (*netdev_flow_flush)(const struct dpif_offload *, struct netdev *); >> + >> + /* Recover and/or set the packet state (contents and metadata) for >> + * continued processing in software, and perform any additional >> + * post-processing required by the offload provider. >> + * >> + * Return 0 if successful and the packet requires further processing; >> + * otherwise, return a positive errno value and take ownership of the >> + * packet if errno != EOPNOTSUPP. Return ECANCELED if the packet was >> + * fully consumed by the provider for non-error conditions. */ >> + int (*netdev_hw_miss_packet_postprocess)(const struct dpif_offload *, >> + struct netdev *, >> + struct dp_packet *); >> }; >> >> >> diff --git a/lib/dpif-offload.c b/lib/dpif-offload.c index >> 99cfe2aac..8ac0f8f96 >> 100644 >> --- a/lib/dpif-offload.c >> +++ b/lib/dpif-offload.c >> @@ -846,6 +846,42 @@ dpif_offload_netdev_flush_flows(struct netdev >> *netdev) >> return EOPNOTSUPP; >> } >> >> +int >> +dpif_offload_netdev_hw_miss_packet_postprocess(struct netdev *netdev, >> + struct dp_packet >> +*packet) { >> + const struct dpif_offload *offload; >> + bool postprocess_api_supported; >> + int rc; >> + >> + atomic_read_relaxed(&netdev->hw_info.postprocess_api_supported, >> + &postprocess_api_supported); >> + if (!postprocess_api_supported) { >> + return EOPNOTSUPP; >> + } >> + >> + offload = ovsrcu_get(const struct dpif_offload *, >> + &netdev->dpif_offload); >> + >> + if (!offload || !offload->class->netdev_hw_miss_packet_postprocess) { >> + if (offload) { >> + /* Offload is configured and API unsupported by the port; >> + * avoid subsequent calls. */ >> + atomic_store_relaxed(&netdev->hw_info.postprocess_api_supported, >> + false); >> + } >> + return EOPNOTSUPP; >> + } >> + >> + rc = offload->class->netdev_hw_miss_packet_postprocess(offload, netdev, >> + packet); >> + if (rc == EOPNOTSUPP) { >> + /* API unsupported by the port; avoid subsequent calls. */ >> + atomic_store_relaxed(&netdev->hw_info.postprocess_api_supported, >> + false); >> + } >> + return rc; >> +} >> + >> >> >> >> struct dpif_offload_port_mgr * >> dpif_offload_port_mgr_init(void) >> diff --git a/lib/dpif-offload.h b/lib/dpif-offload.h index >> fb7edf269..fe73b278d >> 100644 >> --- a/lib/dpif-offload.h >> +++ b/lib/dpif-offload.h >> @@ -75,5 +75,7 @@ void dpif_offload_meter_del(const struct dpif *dpif, >> ofproto_meter_id meter_id, >> >> >> >> /* Netdev specific function, which can be used in the fast path. */ int >> dpif_offload_netdev_flush_flows(struct netdev *); >> +int dpif_offload_netdev_hw_miss_packet_postprocess(struct netdev *, >> + struct dp_packet *); >> >> #endif /* DPIF_OFFLOAD_H */ >> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c index >> d1781d8e4..c3d5e83f5 100644 >> --- a/lib/netdev-offload-dpdk.c >> +++ b/lib/netdev-offload-dpdk.c >> @@ -2685,7 +2685,7 @@ get_vport_netdev(const char *dpif_type, >> return aux.vport; >> } >> >> -static int >> +int >> netdev_offload_dpdk_hw_miss_packet_recover(struct netdev *netdev, >> struct dp_packet *packet) { @@ >> -2803,5 +2803,4 >> @@ const struct netdev_flow_api netdev_offload_dpdk = { >> .init_flow_api = netdev_offload_dpdk_init_flow_api, >> .uninit_flow_api = netdev_offload_dpdk_uninit_flow_api, >> .flow_get = netdev_offload_dpdk_flow_get, >> - .hw_miss_packet_recover = >> netdev_offload_dpdk_hw_miss_packet_recover, >> }; >> diff --git a/lib/netdev-offload-dpdk.h b/lib/netdev-offload-dpdk.h index >> d5061b40c..475822e1b 100644 >> --- a/lib/netdev-offload-dpdk.h >> +++ b/lib/netdev-offload-dpdk.h >> @@ -24,5 +24,7 @@ struct netdev; >> * associated dpif offload provider. */ int >> netdev_offload_dpdk_flow_flush(struct netdev *); uint64_t >> netdev_offload_dpdk_flow_get_n_offloaded(struct netdev *); >> +int netdev_offload_dpdk_hw_miss_packet_recover(struct netdev *, >> + struct dp_packet *); >> >> #endif /* NETDEV_OFFLOAD_DPDK_H */ >> diff --git a/lib/netdev-offload-provider.h b/lib/netdev-offload-provider.h >> index >> 898df8333..f762af19a 100644 >> --- a/lib/netdev-offload-provider.h >> +++ b/lib/netdev-offload-provider.h >> @@ -80,12 +80,6 @@ struct netdev_flow_api { >> int (*flow_del)(struct netdev *, const ovs_u128 *ufid, >> struct dpif_flow_stats *); >> >> - /* Recover the packet state (contents and data) for continued processing >> - * in software. >> - * Return 0 if successful, otherwise returns a positive errno value and >> - * takes ownership of a packet if errno != EOPNOTSUPP. */ >> - int (*hw_miss_packet_recover)(struct netdev *, struct dp_packet *); >> - >> /* Initializies the netdev flow api. >> * Return 0 if successful, otherwise returns a positive errno value. */ >> int (*init_flow_api)(struct netdev *); diff --git a/lib/netdev-offload.c >> b/lib/netdev-offload.c index 0c4209290..be5787f4d 100644 >> --- a/lib/netdev-offload.c >> +++ b/lib/netdev-offload.c >> @@ -186,7 +186,8 @@ netdev_assign_flow_api(struct netdev *netdev) >> CMAP_FOR_EACH (rfa, cmap_node, &netdev_flow_apis) { >> if (!rfa->flow_api->init_flow_api(netdev)) { >> ovs_refcount_ref(&rfa->refcnt); >> - atomic_store_relaxed(&netdev->hw_info.miss_api_supported, true); >> + atomic_store_relaxed(&netdev->hw_info.postprocess_api_supported, >> + true); >> ovsrcu_set(&netdev->flow_api, rfa->flow_api); >> VLOG_INFO("%s: Assigned flow API '%s'.", >> netdev_get_name(netdev), rfa->flow_api->type); @@ >> -195,7 >> +196,7 @@ netdev_assign_flow_api(struct netdev *netdev) >> VLOG_DBG("%s: flow API '%s' is not suitable.", >> netdev_get_name(netdev), rfa->flow_api->type); >> } >> - atomic_store_relaxed(&netdev->hw_info.miss_api_supported, false); >> + atomic_store_relaxed(&netdev->hw_info.postprocess_api_supported, >> + false); >> VLOG_INFO("%s: No suitable flow API found.", netdev_get_name(netdev)); >> >> return -1; >> @@ -254,34 +255,6 @@ netdev_flow_put(struct netdev *netdev, struct match >> *match, >> : EOPNOTSUPP; >> } >> >> -int >> -netdev_hw_miss_packet_recover(struct netdev *netdev, >> - struct dp_packet *packet) >> -{ >> - const struct netdev_flow_api *flow_api; >> - bool miss_api_supported; >> - int rv; >> - >> - atomic_read_relaxed(&netdev->hw_info.miss_api_supported, >> - &miss_api_supported); >> - if (!miss_api_supported) { >> - return EOPNOTSUPP; >> - } >> - >> - flow_api = ovsrcu_get(const struct netdev_flow_api *, >> &netdev->flow_api); >> - if (!flow_api || !flow_api->hw_miss_packet_recover) { >> - return EOPNOTSUPP; >> - } >> - >> - rv = flow_api->hw_miss_packet_recover(netdev, packet); >> - if (rv == EOPNOTSUPP) { >> - /* API unsupported by the port; avoid subsequent calls. */ >> - atomic_store_relaxed(&netdev->hw_info.miss_api_supported, false); >> - } >> - >> - return rv; >> -} >> - >> int >> netdev_flow_get(struct netdev *netdev, struct match *match, >> struct nlattr **actions, const ovs_u128 *ufid, diff --git >> a/lib/netdev- >> offload.h b/lib/netdev-offload.h index 2b32179ec..6006396b9 100644 >> --- a/lib/netdev-offload.h >> +++ b/lib/netdev-offload.h >> @@ -47,7 +47,8 @@ struct ovs_action_push_tnl; >> /* Offload-capable (HW) netdev information */ struct netdev_hw_info { >> bool oor; /* Out of Offload Resources ? */ >> - atomic_bool miss_api_supported; /* hw_miss_packet_recover() >> supported.*/ >> + /* Is hw_miss_packet_postprocess() supported.*/ >> + atomic_bool postprocess_api_supported; >> int offload_count; /* Offloaded flow count */ >> int pending_count; /* Pending (non-offloaded) flow count >> */ >> OVSRCU_TYPE(void *) offload_data; /* Offload metadata. */ @@ -110,7 >> +111,6 @@ bool netdev_flow_dump_next(struct netdev_flow_dump *, struct >> match *, int netdev_flow_put(struct netdev *, struct match *, struct nlattr >> *actions, >> size_t actions_len, const ovs_u128 *, >> struct offload_info *, struct dpif_flow_stats *); -int >> netdev_hw_miss_packet_recover(struct netdev *, struct dp_packet *); int >> netdev_flow_get(struct netdev *, struct match *, struct nlattr **actions, >> const ovs_u128 *, struct dpif_flow_stats *, >> struct dpif_flow_attrs *, struct ofpbuf *wbuffer); diff >> --git >> a/lib/netdev.c b/lib/netdev.c index 6a05e9a7e..13f3d707e 100644 >> --- a/lib/netdev.c >> +++ b/lib/netdev.c >> @@ -435,7 +435,7 @@ netdev_open(const char *name, const char *type, >> struct netdev **netdevp) >> seq_read(netdev->reconfigure_seq); >> ovsrcu_set(&netdev->flow_api, NULL); >> netdev->hw_info.oor = false; >> - atomic_init(&netdev->hw_info.miss_api_supported, false); >> + atomic_init(&netdev->hw_info.postprocess_api_supported, >> + false); >> netdev->node = shash_add(&netdev_shash, name, netdev); >> >> /* By default enable one tx and rx queue per netdev. */ _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
