On 1/12/26 12:20 PM, Eelco Chaudron wrote:
> 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_post_process()
> to reflect that it may also be used for other tasks in the future,
> such as conntrack post-processing.
>
> Acked-by: Eli Britstein <elibr.nvidia.com>
> Signed-off-by: Eelco Chaudron <[email protected]>
> ---
>
> v3 changes:
> - Changed API name from hw_miss_packet_postprocess()
> to hw_post_process()
> ---
> lib/dpif-netdev.c | 18 +++++++++++-------
> lib/dpif-offload-dpdk.c | 9 +++++++++
> lib/dpif-offload-provider.h | 11 +++++++++++
> lib/dpif-offload.c | 35 +++++++++++++++++++++++++++++++++++
> lib/dpif-offload.h | 1 +
> 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 | 3 ++-
> 11 files changed, 77 insertions(+), 48 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 82df85823..07e94f5cb 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_post_process);
> #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 post_process_api_supported;
> +
> +
> atomic_read_relaxed(&rxq->port->netdev->hw_info.post_process_api_supported,
> + &post_process_api_supported);
> + if (post_process_api_supported) {
> + int err = dpif_offload_netdev_hw_post_process(rxq->port->netdev,
> + packet);
>
> - 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_post_process);
Should we have a separate counter for the ECANCELED case?
> + }
> return -1;
> }
> }
> diff --git a/lib/dpif-offload-dpdk.c b/lib/dpif-offload-dpdk.c
> index 2ded4111d..9763e19c6 100644
> --- a/lib/dpif-offload-dpdk.c
> +++ b/lib/dpif-offload-dpdk.c
> @@ -288,6 +288,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_post_process(
> + 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[]) {
> @@ -302,6 +310,7 @@ 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_post_process = dpif_offload_dpdk_netdev_hw_post_process,
> };
>
> /* 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 5e403894e..8da7b12cd 100644
> --- a/lib/dpif-offload-provider.h
> +++ b/lib/dpif-offload-provider.h
> @@ -169,6 +169,17 @@ 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/or 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_post_process)(const struct dpif_offload *, struct netdev
> *,
> + struct dp_packet *);
> };
>
>
> diff --git a/lib/dpif-offload.c b/lib/dpif-offload.c
> index 1c5494d8e..544744c88 100644
> --- a/lib/dpif-offload.c
> +++ b/lib/dpif-offload.c
> @@ -852,6 +852,41 @@ dpif_offload_netdev_flush_flows(struct netdev *netdev)
> return EOPNOTSUPP;
> }
>
> +int
> +dpif_offload_netdev_hw_post_process(struct netdev *netdev,
> + struct dp_packet *packet)
> +{
> + const struct dpif_offload *offload;
> + bool post_process_api_supported;
> + int rc;
> +
> + atomic_read_relaxed(&netdev->hw_info.post_process_api_supported,
> + &post_process_api_supported);
> + if (!post_process_api_supported) {
> + return EOPNOTSUPP;
> + }
> +
> + offload = ovsrcu_get(const struct dpif_offload *, &netdev->dpif_offload);
> +
> + if (!offload || !offload->class->netdev_hw_post_process) {
> + if (offload) {
> + /* Offload is configured and API unsupported by the port;
> + * avoid subsequent calls. */
> + atomic_store_relaxed(&netdev->hw_info.post_process_api_supported,
> + false);
> + }
> + return EOPNOTSUPP;
> + }
> +
> + rc = offload->class->netdev_hw_post_process(offload, netdev, packet);
> + if (rc == EOPNOTSUPP) {
> + /* API unsupported by the port; avoid subsequent calls. */
> + atomic_store_relaxed(&netdev->hw_info.post_process_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..acef15832 100644
> --- a/lib/dpif-offload.h
> +++ b/lib/dpif-offload.h
> @@ -75,5 +75,6 @@ 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_post_process(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..453bb20a0 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.post_process_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.post_process_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..6939fcd35 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_post_process() supported. */
> + atomic_bool post_process_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 5413ca61d..c5182fe8f 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -435,7 +435,8 @@ 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.post_process_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
