>-----Original Message----- >From: Eelco Chaudron <[email protected]> >Sent: Thursday, 23 October 2025 11:23 >To: Eli Britstein <[email protected]>; Gaetan Rivet <[email protected]> >Cc: [email protected]; Eelco Chaudron <[email protected]> >Subject: Re: [ovs-dev] [RFC PATCH v4 39/43] dpif-offload: Move flow_mark >from dpif-netdev to the providers. > >External email: Use caution opening links or attachments > > >On 23 Oct 2025, at 9:44, Eelco Chaudron via dev wrote: > >> This change moves the flow_mark handling from dpif-netdev down to the >> specific offload providers. >> >> Note that this patch should be merged with the previous one once this >> approach is accepted and we move to a non-RFC series. > >Hi Eli, > >This is the patch that changes how the flow mark is handled, as mentioned in >the reply to the cover letter of the v3 series. > >Could you let me know if this patch aligns with what you had in mind? If so, >please confirm, and I’ll roll this patch in with the previous one and send out >a >non-RFC version of the series. [Eli Britstein] Not my meaning. In this series, there is still the mapping in dpif-netdev.c, yes, for "mufid" and not mark, but still. Why do we need this at this layer at all? My meaning was to provide the flow pointer to dpif_offload_datapath_flow_put() (it is already provided in .cb_data.callback_aux_flow). Dpif-offload-dpdk will hold the map, and perform all the associate/disassociate/etc functions. dpif_offload_netdev_hw_miss_packet_postprocess() can return the flow hint so the find function is also implemented in dpif-offload-dpdk layer (and later enhanced with skipping actions). WDYT?
Another note is that the modify flow bug re-appears in v4. > >Cheers, > >Eelco > >> Signed-off-by: Eelco Chaudron <[email protected]> >> --- >> lib/dpif-netdev-private-flow.h | 4 +- >> lib/dpif-netdev.c | 207 ++++++++++++++------------------- >> lib/dpif-offload-dpdk.c | 73 +++++++++--- >> lib/dpif-offload-provider.h | 16 ++- >> lib/dpif-offload.c | 29 ++--- >> lib/dpif-offload.h | 17 ++- >> lib/netdev-offload-dpdk.c | 86 ++++++++++++-- >> lib/netdev-offload-dpdk.h | 3 +- >> 8 files changed, 251 insertions(+), 184 deletions(-) >> >> diff --git a/lib/dpif-netdev-private-flow.h >> b/lib/dpif-netdev-private-flow.h index 2d76c5e32..080c9d85e 100644 >> --- a/lib/dpif-netdev-private-flow.h >> +++ b/lib/dpif-netdev-private-flow.h >> @@ -89,7 +89,7 @@ struct dp_netdev_flow { >> /* 'flow_table'. */ >> const struct cmap_node simple_match_node; /* In >dp_netdev_pmd_thread's >> 'simple_match_table'. */ >> - const struct cmap_node mark_node; /* In owning flow_mark's >mark_to_flow */ >> + const struct cmap_node mufid_node; /* In dp_netdev's >> + 'mufid_to_flow'. */ >> const ovs_u128 ufid; /* Unique flow identifier. */ >> const ovs_u128 mega_ufid; /* Unique mega flow identifier. */ >> const unsigned pmd_id; /* The 'core_id' of pmd thread owning this >> */ >> @@ -102,7 +102,7 @@ struct dp_netdev_flow { >> struct ovs_refcount ref_cnt; >> >> bool dead; >> - uint32_t mark; /* Unique flow mark for netdev offloading. >> */ >> + bool offloaded; >> uint64_t simple_match_mark; /* Unique flow mark for the simple match. >*/ >> odp_port_t orig_in_port; >> >> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index >> 908880e39..2f1b8ad0a 100644 >> --- a/lib/dpif-netdev.c >> +++ b/lib/dpif-netdev.c >> @@ -121,9 +121,7 @@ COVERAGE_DEFINE(datapath_drop_invalid_port); >> 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_postprocess); >> -#endif >> >> /* Protects against changes to 'dp_netdevs'. */ struct ovs_mutex >> dp_netdev_mutex = OVS_MUTEX_INITIALIZER; @@ -290,9 +288,9 @@ >struct >> dp_netdev { >> struct ovs_mutex meters_lock; >> struct cmap meters OVS_GUARDED; >> >> - /* Flow Mark to flow mapping. */ >> - struct ovs_mutex mark_to_flow_lock; >> - struct cmap mark_to_flow OVS_GUARDED; >> + /* Flow mega ufid to flow mapping. */ >> + struct ovs_mutex mufid_to_flow_lock; >> + struct cmap mufid_to_flow OVS_GUARDED; >> >> /* Probability of EMC insertions is a factor of 'emc_insert_min'.*/ >> atomic_uint32_t emc_insert_min; >> @@ -1815,9 +1813,9 @@ create_dp_netdev(const char *name, const struct >dpif_class *class, >> cmap_init(&dp->meters); >> ovs_mutex_init(&dp->meters_lock); >> >> - /* Init flow mark resources. */ >> - cmap_init(&dp->mark_to_flow); >> - ovs_mutex_init(&dp->mark_to_flow_lock); >> + /* Initialize mega ufid-to-flow mapping resources. */ >> + cmap_init(&dp->mufid_to_flow); >> + ovs_mutex_init(&dp->mufid_to_flow_lock); >> >> /* Disable upcalls by default. */ >> dp_netdev_disable_upcall(dp); >> @@ -1961,8 +1959,8 @@ dp_netdev_free(struct dp_netdev *dp) >> cmap_destroy(&dp->tx_bonds); >> ovs_mutex_destroy(&dp->bond_mutex); >> >> - cmap_destroy(&dp->mark_to_flow); >> - ovs_mutex_destroy(&dp->mark_to_flow_lock); >> + cmap_destroy(&dp->mufid_to_flow); >> + ovs_mutex_destroy(&dp->mufid_to_flow_lock); >> >> /* Upcalls must be disabled at this point */ >> dp_netdev_destroy_upcall_lock(dp); >> @@ -2413,31 +2411,29 @@ dp_netdev_pmd_find_dpcls(struct >> dp_netdev_pmd_thread *pmd, >> >> /* Associate mark with a flow, which is 1:N mapping */ static void >> -mark_to_flow_associate(struct dp_netdev *dp, const uint32_t mark, >> - struct dp_netdev_flow *flow) >> +mufid_to_flow_associate(struct dp_netdev *dp, struct dp_netdev_flow >> +*flow) >> { >> dp_netdev_flow_ref(flow); >> >> - ovs_mutex_lock(&dp->mark_to_flow_lock); >> - cmap_insert(&dp->mark_to_flow, >> - CONST_CAST(struct cmap_node *, &flow->mark_node), >> - hash_int(mark, 0)); >> - ovs_mutex_unlock(&dp->mark_to_flow_lock); >> - >> - flow->mark = mark; >> + ovs_mutex_lock(&dp->mufid_to_flow_lock); >> + cmap_insert(&dp->mufid_to_flow, >> + CONST_CAST(struct cmap_node *, &flow->mufid_node), >> + dp_netdev_flow_hash(&flow->mega_ufid)); >> + flow->offloaded = true; >> + ovs_mutex_unlock(&dp->mufid_to_flow_lock); >> >> - VLOG_DBG("Associated dp_netdev flow %p with mark %u mega_ufid >"UUID_FMT, >> - flow, mark, UUID_ARGS((struct uuid *) &flow->mega_ufid)); >> + VLOG_DBG("Associated dp_netdev flow %p with mega_ufid "UUID_FMT, >> + flow, UUID_ARGS((struct uuid *) &flow->mega_ufid)); >> } >> >> static bool >> -flow_mark_has_no_ref(struct dp_netdev *dp, uint32_t mark) >> +mufid_has_no_references(struct dp_netdev *dp, const ovs_u128 *ufid) >> { >> struct dp_netdev_flow *flow; >> >> - CMAP_FOR_EACH_WITH_HASH (flow, mark_node, hash_int(mark, 0), >> - &dp->mark_to_flow) { >> - if (flow->mark == mark) { >> + CMAP_FOR_EACH_WITH_HASH (flow, mufid_node, >dp_netdev_flow_hash(ufid), >> + &dp->mufid_to_flow) { >> + if (flow->offloaded && ovs_u128_equals(flow->mega_ufid, >> + *ufid)) { >> return false; >> } >> } >> @@ -2446,59 +2442,57 @@ flow_mark_has_no_ref(struct dp_netdev *dp, >> uint32_t mark) } >> >> static void >> -mark_to_flow_disassociate(struct dp_netdev *dp, struct dp_netdev_flow >> *flow) >> +mufid_to_flow_disassociate(struct dp_netdev *dp, struct >> +dp_netdev_flow *flow) >> { >> - uint32_t mark = flow->mark; >> - >> - if (OVS_UNLIKELY(mark == INVALID_FLOW_MARK)) { >> + if (OVS_UNLIKELY(!flow->offloaded)) { >> return; >> } >> >> - flow->mark = INVALID_FLOW_MARK; >> - >> - ovs_mutex_lock(&dp->mark_to_flow_lock); >> - cmap_remove(&dp->mark_to_flow, >> - CONST_CAST(struct cmap_node *, &flow->mark_node), >> - hash_int(mark, 0)); >> - ovs_mutex_unlock(&dp->mark_to_flow_lock); >> + ovs_mutex_lock(&dp->mufid_to_flow_lock); >> + flow->offloaded = false; >> + cmap_remove(&dp->mufid_to_flow, >> + CONST_CAST(struct cmap_node *, &flow->mufid_node), >> + dp_netdev_flow_hash(&flow->mega_ufid)); >> + ovs_mutex_unlock(&dp->mufid_to_flow_lock); >> >> dp_netdev_flow_unref(flow); >> } >> >> static void >> -mark_to_flow_disassociate_all(struct dp_netdev *dp, const uint32_t >> mark) >> +mufid_to_flow_disassociate_all(struct dp_netdev *dp, const ovs_u128 >> +*ufid) >> { >> - size_t hash = hash_int(mark, 0); >> + size_t hash = dp_netdev_flow_hash(ufid); >> struct dp_netdev_flow *flow; >> >> - if (OVS_UNLIKELY(mark == INVALID_FLOW_MARK)) { >> - return; >> - } >> + ovs_mutex_lock(&dp->mufid_to_flow_lock); >> + CMAP_FOR_EACH_WITH_HASH (flow, mufid_node, hash, &dp- >>mufid_to_flow) { >> + if (flow->offloaded && ovs_u128_equals(flow->mega_ufid, *ufid)) >> { >> + flow->offloaded = false; >> + cmap_remove(&dp->mufid_to_flow, >> + CONST_CAST(struct cmap_node *, >> &flow->mufid_node), >> + hash); >> >> - ovs_mutex_lock(&dp->mark_to_flow_lock); >> - CMAP_FOR_EACH_WITH_HASH (flow, mark_node, hash, &dp- >>mark_to_flow) { >> - if (flow->mark == mark) { >> - flow->mark = INVALID_FLOW_MARK; >> - cmap_remove(&dp->mark_to_flow, >> - CONST_CAST(struct cmap_node *, >> &flow->mark_node), >> - hash_int(mark, 0)); >> dp_netdev_flow_unref(flow); >> } >> } >> - ovs_mutex_unlock(&dp->mark_to_flow_lock); >> + ovs_mutex_unlock(&dp->mufid_to_flow_lock); >> } >> >> static struct dp_netdev_flow * >> -mark_to_flow_find(const struct dp_netdev_pmd_thread *pmd, >> - const uint32_t mark) >> +mufid_to_flow_find(const struct dp_netdev_pmd_thread *pmd, >> + const ovs_u128 *ufid) >> { >> struct dp_netdev_flow *flow; >> >> - CMAP_FOR_EACH_WITH_HASH (flow, mark_node, hash_int(mark, 0), >> - &pmd->dp->mark_to_flow) { >> + if (!ufid) { >> + return NULL; >> + } >> + >> + CMAP_FOR_EACH_WITH_HASH (flow, mufid_node, >dp_netdev_flow_hash(ufid), >> + &pmd->dp->mufid_to_flow) { >> >> - if (flow->mark == mark && flow->pmd_id == pmd->core_id && >> - flow->dead == false) { >> + if (flow->offloaded && ovs_u128_equals(flow->mega_ufid, *ufid) >> + && flow->pmd_id == pmd->core_id && !flow->dead) { >> return flow; >> } >> } >> @@ -2571,21 +2565,19 @@ log_netdev_flow_change(const struct >> dp_netdev_flow *flow, } >> >> static void >> -offload_flow_del_resume(int error, uint32_t flow_mark) >> +offload_flow_del_resume(int error) >> { >> if (error == EINPROGRESS) { >> return; >> } >> - dpif_offload_free_flow_mark(flow_mark); >> + /* Currently there is no work to be done on flow deletion. */ >> } >> >> static void >> offload_flow_del_resume_cb(void *aux_dp OVS_UNUSED, void *aux_flow >OVS_UNUSED, >> - struct dpif_flow_stats *stats OVS_UNUSED, >> - uint32_t flow_mark, >> - int error) >> + struct dpif_flow_stats *stats OVS_UNUSED, >> + int error) >> { >> - offload_flow_del_resume(error, flow_mark); >> + offload_flow_del_resume(error); >> } >> >> static void >> @@ -2593,13 +2585,13 @@ offload_flow_del(struct dp_netdev *dp, struct >> dp_netdev_flow *flow) { >> odp_port_t in_port = flow->flow.in_port.odp_port; >> >> - if (flow->mark == INVALID_FLOW_MARK) { >> + if (!flow->offloaded) { >> return; >> } >> >> - mark_to_flow_disassociate(dp, flow); >> + mufid_to_flow_disassociate(dp, flow); >> >> - if (flow_mark_has_no_ref(dp, flow->mark) >> + if (mufid_has_no_references(dp, &flow->mega_ufid) >> && dpif_offload_is_offload_enabled()) { >> struct dpif_offload_flow_del del = { >> .in_port = in_port, >> @@ -2607,16 +2599,15 @@ offload_flow_del(struct dp_netdev *dp, struct >dp_netdev_flow *flow) >> .stats = NULL, >> .cb_data = { .callback=offload_flow_del_resume_cb }, >> }; >> - uint32_t mark; >> int ret; >> >> - ret = dpif_offload_datapath_flow_del(dp->full_name, &del, &mark); >> + ret = dpif_offload_datapath_flow_del(dp->full_name, &del); >> if (ret && ret != EINPROGRESS) { >> VLOG_DBG("Failed removing offload flow ufid "UUID_FMT >> " from port %d", >> UUID_ARGS((struct uuid *)&flow->mega_ufid), in_port); >> } >> - offload_flow_del_resume(ret, mark); >> + offload_flow_del_resume(ret); >> } >> } >> >> @@ -3478,8 +3469,7 @@ dp_netdev_flow_is_simple_match(const struct >> match *match) } >> >> static void >> -offload_flow_put_resume(struct dp_netdev *dp, >> - struct dp_netdev_flow *flow, uint32_t flow_mark, >> +offload_flow_put_resume(struct dp_netdev *dp, struct dp_netdev_flow >> +*flow, >> int error) >> { >> if (error == EINPROGRESS) { >> @@ -3487,29 +3477,21 @@ offload_flow_put_resume(struct dp_netdev >*dp, >> } >> >> if (!error) { >> - if (flow_mark != INVALID_FLOW_MARK) { >> - if (flow->mark == INVALID_FLOW_MARK) { >> - mark_to_flow_associate(dp, flow_mark, flow); >> - } else if (flow->mark != flow_mark) { >> - /* The flow mark has changed. */ >> - mark_to_flow_disassociate_all(dp, flow->mark); >> - dpif_offload_free_flow_mark(flow->mark); >> - mark_to_flow_associate(dp, flow_mark, flow); >> - } >> + if (!flow->offloaded) { >> + mufid_to_flow_associate(dp, flow); >> + } >> >> - if (flow->dead) { >> - /* If flows are processed asynchronously, modifications >> might >> - * still be queued up while the flow is being removed. In >> this >> - * case we need to go through the delete process here. */ >> - offload_flow_del(dp, flow); >> - } >> + if (flow->dead) { >> + /* If flows are processed asynchronously, modifications might >> + * still be queued up while the flow is being removed. In this >> + * case we need to go through the delete process here. */ >> + offload_flow_del(dp, flow); >> } >> } else { >> - /* On error, no flow should be associated with this flow mark, >> - * and we should free it. */ >> - if (flow->mark != INVALID_FLOW_MARK) { >> - mark_to_flow_disassociate_all(dp, flow_mark); >> - dpif_offload_free_flow_mark(flow_mark); >> + /* On error, no flows should be associated with this mega_ufid >> offload, >> + * so disassociate all dp_flow structures. */ >> + if (flow->offloaded) { >> + mufid_to_flow_disassociate_all(dp, &flow->mega_ufid); >> } >> } >> dp_netdev_flow_unref(flow); >> @@ -3517,14 +3499,12 @@ offload_flow_put_resume(struct dp_netdev >*dp, >> >> static void >> offload_flow_put_resume_cb(void *aux_dp, void *aux_flow, >> - struct dpif_flow_stats *stats OVS_UNUSED, >> - uint32_t flow_mark, >> - int error) >> + struct dpif_flow_stats *stats OVS_UNUSED, >> + int error) >> { >> struct dp_netdev *dp = aux_dp; >> struct dp_netdev_flow *flow = aux_flow; >> >> - offload_flow_put_resume(dp, flow, flow_mark, error); >> + offload_flow_put_resume(dp, flow, error); >> } >> >> static void >> @@ -3545,7 +3525,6 @@ offload_flow_put(struct dp_netdev *dp, struct >dp_netdev_flow *flow, >> .callback_aux_dp = dp, >> .callback_aux_flow = flow }, >> }; >> - uint32_t flow_mark; >> int error; >> >> if (!dpif_offload_is_offload_enabled()) { @@ -3553,9 +3532,8 @@ >> offload_flow_put(struct dp_netdev *dp, struct dp_netdev_flow *flow, >> } >> >> dp_netdev_flow_ref(flow); >> - error = dpif_offload_datapath_flow_put(dp->full_name, &put, >> - &flow_mark); >> - offload_flow_put_resume(dp, flow, flow_mark, error); >> + error = dpif_offload_datapath_flow_put(dp->full_name, &put); >> + offload_flow_put_resume(dp, flow, error); >> } >> >> static struct dp_netdev_flow * >> @@ -3595,8 +3573,8 @@ dp_netdev_flow_add(struct >dp_netdev_pmd_thread *pmd, >> flow = xmalloc(sizeof *flow - sizeof flow->cr.flow.mf + mask.len); >> memset(&flow->stats, 0, sizeof flow->stats); >> flow->dead = false; >> + flow->offloaded = false; >> flow->batch = NULL; >> - flow->mark = INVALID_FLOW_MARK; >> flow->orig_in_port = orig_in_port; >> *CONST_CAST(unsigned *, &flow->pmd_id) = pmd->core_id; >> *CONST_CAST(struct flow *, &flow->flow) = match->flow; @@ >> -7731,35 +7709,30 @@ dp_netdev_hw_flow(const struct >dp_netdev_pmd_thread *pmd, >> struct dp_packet *packet, >> struct dp_netdev_flow **flow) { >> - uint32_t mark; >> - >> -#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 postprocess_api_supported; >> + ovs_u128 *ufid; >> + int err; >> >> 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); >> - >> - if (err && err != EOPNOTSUPP) { >> - if (err != ECANCELED) { >> - COVERAGE_INC(datapath_drop_hw_miss_postprocess); >> - } >> - return -1; >> - } >> - } >> -#endif >> >> - /* If no mark, no flow to find. */ >> - if (!dp_packet_has_flow_mark(packet, &mark)) { >> + if (!postprocess_api_supported) { >> *flow = NULL; >> return 0; >> } >> >> - *flow = mark_to_flow_find(pmd, mark); >> + err = dpif_offload_netdev_hw_miss_packet_postprocess(rxq->port- >>netdev, >> + packet, >> + &ufid); >> + >> + if (err && err != EOPNOTSUPP) { >> + if (err != ECANCELED) { >> + COVERAGE_INC(datapath_drop_hw_miss_postprocess); >> + } >> + return -1; >> + } >> + >> + *flow = mufid_to_flow_find(pmd, ufid); >> return 0; >> } >> >> diff --git a/lib/dpif-offload-dpdk.c b/lib/dpif-offload-dpdk.c index >> 49aeb6dc8..9393e9324 100644 >> --- a/lib/dpif-offload-dpdk.c >> +++ b/lib/dpif-offload-dpdk.c >> @@ -100,6 +100,8 @@ struct dpif_offload_dpdk { >> atomic_bool offload_thread_shutdown; >> struct dpdk_offload_thread *offload_threads; >> >> + struct id_fpool *flow_mark_pool; >> + >> /* Configuration specific variables. */ >> struct ovsthread_once once_enable; /* Track first-time enablement. */ >> unsigned int offload_thread_count; /* Number of offload threads. >> */ @@ -115,6 +117,40 @@ dpif_offload_dpdk_cast(const struct >> dpif_offload *offload) DECLARE_EXTERN_PER_THREAD_DATA(unsigned int, >> dpdk_offload_thread_id); >> DEFINE_EXTERN_PER_THREAD_DATA(dpdk_offload_thread_id, >> OVSTHREAD_ID_UNSET); >> >> +static uint32_t >> +dpif_offload_dpdk_allocate_flow_mark(struct dpif_offload_dpdk >> +*offload) { >> + static struct ovsthread_once init_once = >OVSTHREAD_ONCE_INITIALIZER; >> + unsigned int uid = dpdk_offload_thread_id() \ >> + % offload->offload_thread_count; >> + uint32_t flow_mark; >> + >> + if (ovsthread_once_start(&init_once)) { >> + /* Haven't initiated yet, do it here. */ >> + offload->flow_mark_pool = id_fpool_create( >> + offload->offload_thread_count, 1, UINT32_MAX - 1); >> + ovsthread_once_done(&init_once); >> + } >> + >> + if (id_fpool_new_id(offload->flow_mark_pool, uid, &flow_mark)) { >> + return flow_mark; >> + } >> + >> + return INVALID_FLOW_MARK; >> +} >> + >> +static void >> +dpif_offload_dpdk_free_flow_mark(struct dpif_offload_dpdk *offload, >> + uint32_t flow_mark) { >> + if (flow_mark != INVALID_FLOW_MARK) { >> + unsigned int uid = dpdk_offload_thread_id() \ >> + % offload->offload_thread_count; >> + >> + id_fpool_free_id(offload->flow_mark_pool, uid, flow_mark); >> + } >> +} >> + >> unsigned int >> dpdk_offload_thread_id(void) >> { >> @@ -312,7 +348,7 @@ dpif_offload_dpdk_offload_del(struct >dpdk_offload_thread *thread, >> struct dpdk_offload_thread_item *item) >> { >> struct dpdk_offload_flow_item *flow = &item->data->flow; >> - uint32_t mark = INVALID_FLOW_MARK; >> + uint32_t mark; >> struct dpif_flow_stats stats; >> struct netdev *netdev; >> int error = 0; >> @@ -334,13 +370,14 @@ dpif_offload_dpdk_offload_del(struct >dpdk_offload_thread *thread, >> error = netdev_offload_dpdk_flow_del(netdev, &flow->ufid, >> flow->requested_stats ? &stats >> : >> NULL); >> + dpif_offload_dpdk_free_flow_mark(thread->offload, mark); >> } >> >> do_callback: >> dpif_offload_datapath_flow_op_continue(&flow->callback, >> flow->requested_stats ? &stats >> : NULL, >> - mark, error); >> + error); >> return error; >> } >> >> @@ -368,7 +405,7 @@ dpif_offload_dpdk_offload_put(struct >dpdk_offload_thread *thread, >> goto do_callback; >> } >> >> - mark = dpif_offload_allocate_flow_mark(); >> + mark = dpif_offload_dpdk_allocate_flow_mark(thread->offload); >> if (mark == INVALID_FLOW_MARK) { >> VLOG_ERR("Failed to allocate flow mark!"); >> error = ENOSPC; >> @@ -407,15 +444,14 @@ do_callback: >> } >> } else if (mark != INVALID_FLOW_MARK) { >> /* We allocated a mark, but it was not used. */ >> - dpif_offload_free_flow_mark(mark); >> - mark = INVALID_FLOW_MARK; >> + dpif_offload_dpdk_free_flow_mark(thread->offload, mark); >> } >> } >> >> dpif_offload_datapath_flow_op_continue(&flow->callback, >> flow->requested_stats ? &stats >> : NULL, >> - mark, error); >> + error); >> return error; >> } >> >> @@ -795,6 +831,7 @@ dpif_offload_dpdk_open(const struct >dpif_offload_class *offload_class, >> offload->offload_threads = NULL; >> atomic_count_init(&offload->next_offload_thread_id, 0); >> atomic_init(&offload->offload_thread_shutdown, false); >> + offload->flow_mark_pool = NULL; >> >> *offload_ = &offload->offload; >> return 0; >> @@ -819,8 +856,6 @@ dpif_offload_dpdk_close(struct dpif_offload >*offload_) >> dpif_offload_dpdk_cleanup_port, >> offload_); >> >> - dpif_offload_port_mgr_uninit(offload->port_mgr); >> - >> atomic_store_relaxed(&offload->offload_thread_shutdown, true); >> if (offload->offload_threads) { >> for (int i = 0; i < offload->offload_thread_count; i++) { @@ >> -829,6 +864,12 @@ dpif_offload_dpdk_close(struct dpif_offload *offload_) >> cmap_destroy(&offload->offload_threads[i].megaflow_to_mark); >> } >> } >> + >> + dpif_offload_port_mgr_uninit(offload->port_mgr); >> + if (offload->flow_mark_pool) { >> + id_fpool_destroy(offload->flow_mark_pool); >> + } >> + >> free(offload); >> } >> >> @@ -1020,19 +1061,18 @@ >> dpif_offload_dpdk_get_n_offloaded_by_thread(struct dpif_offload_dpdk >> *offload, static int >dpif_offload_dpdk_netdev_hw_miss_packet_postprocess( >> const struct dpif_offload *offload_, struct netdev *netdev, >> - struct dp_packet *packet) >> - >> + struct dp_packet *packet, ovs_u128 **ufid) >> { >> struct dpif_offload_dpdk *offload = >> dpif_offload_dpdk_cast(offload_); >> >> - return netdev_offload_dpdk_hw_miss_packet_recover(offload, netdev, >packet); >> + return netdev_offload_dpdk_hw_miss_packet_recover(offload, netdev, >packet, >> + ufid); >> } >> >> static int >> dpif_offload_dpdk_netdev_flow_put(const struct dpif_offload *offload_, >> struct netdev *netdev OVS_UNUSED, >> - struct dpif_offload_flow_put *put, >> - uint32_t *flow_mark) >> + struct dpif_offload_flow_put *put) >> { >> struct dpif_offload_dpdk *offload = dpif_offload_dpdk_cast(offload_); >> struct dpdk_offload_thread_item *item; @@ -1055,16 +1095,13 @@ >> dpif_offload_dpdk_netdev_flow_put(const struct dpif_offload *offload_, >> flow_offload->callback = put->cb_data; >> >> dpif_offload_dpdk_offload_flow_enqueue(offload, item); >> - >> - *flow_mark = INVALID_FLOW_MARK; >> return EINPROGRESS; >> } >> >> static int >> dpif_offload_dpdk_netdev_flow_del(const struct dpif_offload *offload_, >> struct netdev *netdev OVS_UNUSED, >> - struct dpif_offload_flow_del *del, >> - uint32_t *flow_mark) >> + struct dpif_offload_flow_del *del) >> { >> struct dpif_offload_dpdk *offload = dpif_offload_dpdk_cast(offload_); >> struct dpdk_offload_thread_item *item; @@ -1081,8 +1118,6 @@ >> dpif_offload_dpdk_netdev_flow_del(const struct dpif_offload *offload_, >> flow_offload->callback = del->cb_data; >> >> dpif_offload_dpdk_offload_flow_enqueue(offload, item); >> - >> - *flow_mark = INVALID_FLOW_MARK; >> return EINPROGRESS; >> } >> >> diff --git a/lib/dpif-offload-provider.h b/lib/dpif-offload-provider.h >> index 8d64b98f5..1b9614852 100644 >> --- a/lib/dpif-offload-provider.h >> +++ b/lib/dpif-offload-provider.h >> @@ -292,10 +292,16 @@ struct dpif_offload_class { >> * 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. */ >> + * fully consumed by the provider for non-error conditions. >> + * >> + * When zero (0) is returned, the 'ufid' pointer may reference the ufid >> + * associated with this packet. This can be used to support partial >> + * offloads. The returned pointer must remain valid until the end of >> + * the next RCU grace period. */ >> int (*netdev_hw_miss_packet_postprocess)(const struct dpif_offload *, >> struct netdev *, >> - struct dp_packet *); >> + struct dp_packet *, >> + ovs_u128 **ufid); >> >> /* Add or modify the specified flow directly in the offload datapath. >> * The actual implementation may choose to handle the offload @@ >> -304,14 +310,12 @@ struct dpif_offload_class { >> * callback must not be called, and 0 should be returned. If this call >> is >> * not successful, a positive errno value should be returned. */ >> int (*netdev_flow_put)(const struct dpif_offload *, struct netdev *, >> - struct dpif_offload_flow_put *, >> - uint32_t *flow_mark); >> + struct dpif_offload_flow_put *); >> >> /* Delete the specified flow directly from the offloaded datapath. See >> the >> * above 'netdev_flow_put' for implementation details. */ >> int (*netdev_flow_del)(const struct dpif_offload *, struct netdev *, >> - struct dpif_offload_flow_del *, >> - uint32_t *flow_mark); >> + struct dpif_offload_flow_del *); >> >> /* Get offload statistics based on the flows 'ufid'. Note that this API >> * does NOT support asynchronous handling. Returns 'true' if the >> flow was diff --git a/lib/dpif-offload.c b/lib/dpif-offload.c index >> d4ed1ba10..30f2946b9 100644 >> --- a/lib/dpif-offload.c >> +++ b/lib/dpif-offload.c >> @@ -1419,8 +1419,7 @@ dpif_offload_netdev_same_offload(const struct >> netdev *a, >> >> int >> dpif_offload_datapath_flow_put(const char *dpif_name, >> - struct dpif_offload_flow_put *put, >> - uint32_t *flow_mark) >> + struct dpif_offload_flow_put *put) >> { >> struct dpif_offload *offload; >> struct dp_offload *dp_offload; >> @@ -1432,9 +1431,6 @@ dpif_offload_datapath_flow_put(const char >*dpif_name, >> ovs_mutex_unlock(&dpif_offload_mutex); >> >> if (OVS_UNLIKELY(!dp_offload)) { >> - if (flow_mark) { >> - *flow_mark = INVALID_FLOW_MARK; >> - } >> return 0; >> } >> >> @@ -1442,20 +1438,15 @@ dpif_offload_datapath_flow_put(const char >*dpif_name, >> put->in_port); >> >> if (OVS_LIKELY(netdev && offload->class->netdev_flow_put)) { >> - return offload->class->netdev_flow_put(offload, netdev, put, >> - flow_mark); >> + return offload->class->netdev_flow_put(offload, netdev, put); >> } >> >> - if (flow_mark) { >> - *flow_mark = INVALID_FLOW_MARK; >> - } >> return 0; >> } >> >> int >> dpif_offload_datapath_flow_del(const char *dpif_name, >> - struct dpif_offload_flow_del *del, >> - uint32_t *flow_mark) >> + struct dpif_offload_flow_del *del) >> { >> struct dpif_offload *offload; >> struct dp_offload *dp_offload; >> @@ -1467,9 +1458,6 @@ dpif_offload_datapath_flow_del(const char >*dpif_name, >> ovs_mutex_unlock(&dpif_offload_mutex); >> >> if (OVS_UNLIKELY(!dp_offload)) { >> - if (flow_mark) { >> - *flow_mark = INVALID_FLOW_MARK; >> - } >> return 0; >> } >> >> @@ -1477,13 +1465,9 @@ dpif_offload_datapath_flow_del(const char >*dpif_name, >> del->in_port); >> >> if (OVS_LIKELY(netdev && offload->class->netdev_flow_del)) { >> - return offload->class->netdev_flow_del(offload, netdev, del, >> - flow_mark); >> + return offload->class->netdev_flow_del(offload, netdev, del); >> } >> >> - if (flow_mark) { >> - *flow_mark = INVALID_FLOW_MARK; >> - } >> return 0; >> } >> >> @@ -1522,7 +1506,8 @@ dpif_offload_datapath_flow_stats(const char >> *dpif_name, odp_port_t in_port, >> >> int >> dpif_offload_netdev_hw_miss_packet_postprocess(struct netdev *netdev, >> - struct dp_packet *packet) >> + struct dp_packet *packet, >> + ovs_u128 **ufid) >> { >> const struct dpif_offload *offload; >> bool postprocess_api_supported; >> @@ -1547,7 +1532,7 @@ >dpif_offload_netdev_hw_miss_packet_postprocess(struct netdev *netdev, >> } >> >> rc = offload->class->netdev_hw_miss_packet_postprocess(offload, >netdev, >> - packet); >> + packet, >> + ufid); >> if (rc == EOPNOTSUPP) { >> /* API unsupported by the port; avoid subsequent calls. */ >> >> atomic_store_relaxed(&netdev->hw_info.postprocess_api_supported, >> false); diff --git a/lib/dpif-offload.h b/lib/dpif-offload.h index >> a18c530dd..38e2699bb 100644 >> --- a/lib/dpif-offload.h >> +++ b/lib/dpif-offload.h >> @@ -155,13 +155,13 @@ int dpif_offload_stats_get(struct dpif *, struct >> netdev_custom_stats **stats, bool dpif_offload_netdev_same_offload(const >struct netdev *, >> const struct netdev *); int >> dpif_offload_netdev_hw_miss_packet_postprocess(struct netdev *, >> - struct dp_packet *); >> + struct dp_packet *, >> + ovs_u128 **ufid); >> >> >> /* Flow modification callback definitions. */ typedef void >> dpif_offload_flow_op_cb(void *aux_dp, void *aux_flow, >> - struct dpif_flow_stats *stats, >> - uint32_t flow_mark, int error); >> + struct dpif_flow_stats *stats, >> + int error); >> >> /* Supporting structures for flow modification functions. */ struct >> dpif_offload_flow_cb_data { @@ -191,11 +191,9 @@ struct >> dpif_offload_flow_del { >> >> /* Flow modification functions, which can be used in the fast path. >> */ int dpif_offload_datapath_flow_put(const char *dpif_name, >> - struct dpif_offload_flow_put *, >> - uint32_t *flow_mark); >> + struct dpif_offload_flow_put *); >> int dpif_offload_datapath_flow_del(const char *dpif_name, >> - struct dpif_offload_flow_del *, >> - uint32_t *flow_mark); >> + struct dpif_offload_flow_del *); >> bool dpif_offload_datapath_flow_stats(const char *dpif_name, >> odp_port_t in_port, const ovs_u128 >> *ufid, >> struct dpif_flow_stats *, @@ >> -203,11 +201,10 @@ bool dpif_offload_datapath_flow_stats(const char >> *dpif_name, >> >> static inline void dpif_offload_datapath_flow_op_continue( >> struct dpif_offload_flow_cb_data *cb, struct dpif_flow_stats *stats, >> - uint32_t flow_mark, int error) >> + int error) >> { >> if (cb && cb->callback) { >> - cb->callback(cb->callback_aux_dp, cb->callback_aux_flow, >> - stats, flow_mark, error); >> + cb->callback(cb->callback_aux_dp, cb->callback_aux_flow, >> + stats, error); >> } >> } >> >> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c >> index 0512fd1d1..ea99a375f 100644 >> --- a/lib/netdev-offload-dpdk.c >> +++ b/lib/netdev-offload-dpdk.c >> @@ -60,6 +60,7 @@ static struct vlog_rate_limit rl = >> VLOG_RATE_LIMIT_INIT(600, 600); >> >> struct ufid_to_rte_flow_data { >> struct cmap_node node; >> + struct cmap_node mark_node; >> ovs_u128 ufid; >> struct netdev *netdev; >> struct rte_flow *rte_flow; >> @@ -68,11 +69,13 @@ struct ufid_to_rte_flow_data { >> struct netdev *physdev; >> struct ovs_mutex lock; >> unsigned int creation_tid; >> + uint32_t flow_mark; >> bool dead; >> }; >> >> struct netdev_offload_dpdk_data { >> struct cmap ufid_to_rte_flow; >> + struct cmap mark_to_rte_flow; >> uint64_t *rte_flow_counters; >> struct ovs_mutex map_lock; >> }; >> @@ -85,6 +88,7 @@ offload_data_init(struct netdev *netdev, unsigned int >offload_thread_count) >> data = xzalloc(sizeof *data); >> ovs_mutex_init(&data->map_lock); >> cmap_init(&data->ufid_to_rte_flow); >> + cmap_init(&data->mark_to_rte_flow); >> data->rte_flow_counters = xcalloc(offload_thread_count, >> sizeof >> *data->rte_flow_counters); >> >> @@ -124,6 +128,7 @@ offload_data_destroy(struct netdev *netdev) >> } >> >> cmap_destroy(&data->ufid_to_rte_flow); >> + cmap_destroy(&data->mark_to_rte_flow); >> ovsrcu_postpone(offload_data_destroy__, data); >> >> ovsrcu_set(&netdev->hw_info.offload_data, NULL); @@ -168,6 >> +173,35 @@ offload_data_map(struct netdev *netdev) >> return data ? &data->ufid_to_rte_flow : NULL; } >> >> +static bool >> +offload_data_maps(struct netdev *netdev, struct cmap **ufid_map, >> + struct cmap **mark_map) { >> + struct netdev_offload_dpdk_data *data; >> + >> + data = (struct netdev_offload_dpdk_data *) >> + ovsrcu_get(void *, &netdev->hw_info.offload_data); >> + >> + if (!data) { >> + return false; >> + } >> + >> + *ufid_map = &data->ufid_to_rte_flow; >> + *mark_map = &data->mark_to_rte_flow; >> + return true; >> +} >> + >> +static struct cmap * >> +offload_data_mark_map(struct netdev *netdev) { >> + struct netdev_offload_dpdk_data *data; >> + >> + data = (struct netdev_offload_dpdk_data *) >> + ovsrcu_get(void *, &netdev->hw_info.offload_data); >> + >> + return data ? &data->mark_to_rte_flow : NULL; } >> + >> /* Find rte_flow with @ufid. */ >> static struct ufid_to_rte_flow_data * >> ufid_to_rte_flow_data_find(struct netdev *netdev, @@ -213,17 +247,38 >> @@ ufid_to_rte_flow_data_find_protected(struct netdev *netdev, >> return NULL; >> } >> >> +/* Find rte_flow with @flow_mark. */ >> +static struct ufid_to_rte_flow_data * >> +mark_to_rte_flow_data_find(struct netdev *netdev, uint32_t flow_mark) >> +{ >> + size_t hash = hash_int(flow_mark, 0); >> + struct ufid_to_rte_flow_data *data; >> + struct cmap *mark_map = offload_data_mark_map(netdev); >> + >> + if (!mark_map) { >> + return NULL; >> + } >> + >> + CMAP_FOR_EACH_WITH_HASH (data, mark_node, hash, mark_map) { >> + if (data->flow_mark == flow_mark) { >> + return data; >> + } >> + } >> + return NULL; >> +} >> + >> static inline struct ufid_to_rte_flow_data * >> ufid_to_rte_flow_associate(const ovs_u128 *ufid, struct netdev *netdev, >> struct netdev *physdev, struct rte_flow >> *rte_flow, >> - bool actions_offloaded) >> + bool actions_offloaded, uint32_t >> + flow_mark) >> { >> size_t hash = hash_bytes(ufid, sizeof *ufid, 0); >> - struct cmap *map = offload_data_map(netdev); >> struct ufid_to_rte_flow_data *data_prev; >> struct ufid_to_rte_flow_data *data; >> + struct cmap *map, *mark_map; >> >> - if (!map) { >> + >> + if (!offload_data_maps(netdev, &map, &mark_map)) { >> return NULL; >> } >> >> @@ -248,9 +303,12 @@ ufid_to_rte_flow_associate(const ovs_u128 *ufid, >struct netdev *netdev, >> data->rte_flow = rte_flow; >> data->actions_offloaded = actions_offloaded; >> data->creation_tid = dpdk_offload_thread_id(); >> + data->flow_mark = flow_mark; >> ovs_mutex_init(&data->lock); >> >> cmap_insert(map, CONST_CAST(struct cmap_node *, &data->node), >> hash); >> + cmap_insert(mark_map, CONST_CAST(struct cmap_node *, &data- >>mark_node), >> + hash_int(flow_mark, 0)); >> >> offload_data_unlock(netdev); >> return data; >> @@ -268,14 +326,16 @@ ufid_to_rte_flow_disassociate(struct >ufid_to_rte_flow_data *data) >> OVS_REQUIRES(data->lock) >> { >> size_t hash = hash_bytes(&data->ufid, sizeof data->ufid, 0); >> - struct cmap *map = offload_data_map(data->netdev); >> + struct cmap *map, *mark_map; >> >> - if (!map) { >> + if (!offload_data_maps(data->netdev, &map, &mark_map)) { >> return; >> } >> >> offload_data_lock(data->netdev); >> cmap_remove(map, CONST_CAST(struct cmap_node *, &data->node), >> hash); >> + cmap_remove(mark_map, CONST_CAST(struct cmap_node *, &data- >>mark_node), >> + hash_int(data->flow_mark, 0)); >> offload_data_unlock(data->netdev); >> >> if (data->netdev != data->physdev) { @@ -2348,7 +2408,8 @@ >> netdev_offload_dpdk_add_flow(struct dpif_offload_dpdk *offload, >> goto out; >> } >> flows_data = ufid_to_rte_flow_associate(ufid, netdev, patterns.physdev, >> - flow, actions_offloaded); >> + flow, actions_offloaded, >> + flow_mark); >> VLOG_DBG("%s/%s: installed flow %p by ufid "UUID_FMT, >> netdev_get_name(netdev), netdev_get_name(patterns.physdev), >flow, >> UUID_ARGS((struct uuid *) ufid)); @@ -2701,16 +2762,27 >> @@ get_vport_netdev(struct dpif_offload_dpdk *offload, >> >> int netdev_offload_dpdk_hw_miss_packet_recover( >> struct dpif_offload_dpdk *offload, struct netdev *netdev, >> - struct dp_packet *packet) >> + struct dp_packet *packet, ovs_u128 **ufid) >> { >> struct rte_flow_restore_info rte_restore_info; >> + struct ufid_to_rte_flow_data *data = NULL; >> struct rte_flow_tunnel *rte_tnl; >> struct netdev *vport_netdev; >> struct pkt_metadata *md; >> struct flow_tnl *md_tnl; >> odp_port_t vport_odp; >> + uint32_t flow_mark; >> int ret = 0; >> >> + if (dp_packet_has_flow_mark(packet, &flow_mark)) { >> + data = mark_to_rte_flow_data_find(netdev, flow_mark); >> + } >> + if (data) { >> + *ufid = &data->ufid; >> + } else { >> + *ufid = NULL; >> + } >> + >> ret = netdev_dpdk_rte_flow_get_restore_info(netdev, packet, >> &rte_restore_info, NULL); >> if (ret) { >> diff --git a/lib/netdev-offload-dpdk.h b/lib/netdev-offload-dpdk.h >> index c3b63290d..c9a081df6 100644 >> --- a/lib/netdev-offload-dpdk.h >> +++ b/lib/netdev-offload-dpdk.h >> @@ -34,7 +34,8 @@ uint64_t >netdev_offload_dpdk_flow_get_n_offloaded_by_thread( >> struct netdev *, unsigned int tid); int >> netdev_offload_dpdk_hw_miss_packet_recover(struct dpif_offload_dpdk *, >> struct netdev *, >> - struct dp_packet *); >> + struct dp_packet *, >> + ovs_u128 **ufid); >> int netdev_offload_dpdk_flow_put(struct dpif_offload_dpdk *, >> struct netdev *, struct match *, >> struct nlattr *actions, size_t >> actions_len, >> -- >> 2.50.1 >> >> _______________________________________________ >> dev mailing list >> [email protected] >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail >> .openvswitch.org%2Fmailman%2Flistinfo%2Fovs- >dev&data=05%7C02%7Celibr%4 >> >0nvidia.com%7Cc7ebcd00c8ee46f988f508de120d5a40%7C43083d15727340c1 >b7db3 >> >9efd9ccc17a%7C0%7C0%7C638968045820001189%7CUnknown%7CTWFpbGZ >sb3d8eyJFb >> >XB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWF >pbCI >> >sIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=2kllZaQdxcEmoOH%2F%2B0xn >Sa094a3HU >> r7%2FaQ5IplOvF%2Fg%3D&reserved=0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
