On 27 Oct 2025, at 16:17, Eli Britstein wrote:
>> -----Original Message----- >> From: Eelco Chaudron <[email protected]> >> Sent: Monday, 27 October 2025 15:21 >> To: Eli Britstein <[email protected]> >> Cc: [email protected]; Gaetan Rivet <[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 26 Oct 2025, at 12:02, Eli Britstein wrote: >> >>>> -----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? >> >> The reason I still keep this mapping in dpif-netdev is that it maintains a >> one-to- >> many relationship between the mufid and the corresponding dp_netdev_flow >> objects. Flow offload tracking is therefore inherently done in dpif-netdev, >> and it >> feels odd to delegate this responsibility entirely to the individual offload >> providers. >> >> Even if we were to shift the tracking into the offload provider, it would >> still need >> to track which PMDs have the mufid offloaded and which dp_netdev_flow >> objects correspond to it. When removing a flow, it can only be deleted once >> the >> last PMD has removed its instance of the flow. Handling all of this within >> the >> offload provider layer feels misplaced. Moreover, managing the >> dp_netdev_flow reference counting from that layer would introduce additional >> complexity and risk around lifecycle management. >> >> Thoughts? > [Eli Britstein] > > To comment about the 1:N mapping, it is another issue that I didn't want to > open now, but as it's brought up, and may affect the arch, I'll elaborate: > In case there are multiple (N) PMDs, it is not guaranteed to have N u-flows. > There might be just one. Then, a packet hitting the HW flow might reach to > another PMD, in which there is no u-flow (same mega-ufid). > In case of just mark partial offload, it is harmless. This PMD won't find any > u-flow associated, but the processing is done correctly as the packet is not > modified by the HW so it is just processed without any offload, creating its > own u-flow. > With partial offload that allows HW modification (and processing after > skipping some actions) this is wrong. The PMD cannot just process as if there > wasn't any offload since the packet is already modified by the HW. Instead, > there should be some awareness of it. > In our downstream, we did it by searching the mark->flow of other PMDs and > cloning the foreign PMD's u-flow to the current one. > > The concept of flow<->mark(/mufid) mapping is related to offload. The purpose > of this RFC (/patch-set) is to have a clean architecture for offloads. > Keeping dpif-netdev doing has a taste of missing that purpose. > I understand your points, including the risk in it. Still, I think we should > figure it out. > I don't think it is impossible. What do you think about trying to make the > API for it, and try to detect the potential bugs in code review and testing? > > For the 1:N issue above, I think the dpif-offload can provide the foreign > u-flow, marking it as foreign but still have the clone logic in dpif-netdev. > > WDYT? Let me think about this a bit more, and see if I can cook up something that does not put too much backpressure on the flow offload. >>> Another note is that the modify flow bug re-appears in v4. >> >> Can you explain the steps you use to reproduce this? >> I’m asking because my previous test that showed this issue is still passing >> on >> RFCv4. >> If you can share the reproduction steps, I’ll add a unit test for it to the >> last >> patch in the series. > [Eli Britstein] > Sure. Quite simple: > Configuration: > * One bridge, with p0, pf0vf0, pf0vf1. > * Each VF is in a separated namespace (ip netns). > Scenario: > * Restart OVS. > * Ping from VF0 to VF1. > * Watch the flows. > > The flows are: > 1. VF0->flood. Flood since OVS didn't learn VF1's MAC yet. This is not > offloaded. > 2. VF1->VF0. OVS already learned VF0's MAC, so just one forwarding. This is > offloaded. > 3. VF1's MAC is learned. The flow from #1 is modified to be VF0->VF1. This > should be offloaded. Works in upstream and v3, but fails in v4. I tested this in my setup, and it’s working fine here. I also added a unit test for it, and it’s passing. Are you doing anything differently, or can you try the test below? //Eelco diff --git a/tests/system-dpdk-offloads.at b/tests/system-dpdk-offloads.at index e78876149..7b61dbf92 100644 --- a/tests/system-dpdk-offloads.at +++ b/tests/system-dpdk-offloads.at @@ -148,4 +148,60 @@ AT_CHECK([test $(ovs-appctl dpif-netdev/pmd-stats-show | \ awk '/phwol hits:/ {sum += $3} END {print sum}') -ge 8]) OVS_TRAFFIC_VSWITCHD_STOP -AT_CLEANUP \ No newline at end of file +AT_CLEANUP + +AT_SETUP([dpdk offload - flow modification non-offload -> offload]) +OVS_DPDK_OFFLOAD_PRE_CHECK() +OVS_TRAFFIC_VSWITCHD_START() + +ADD_NAMESPACES(at_ns0, at_ns1, at_ns2) +ADD_VF(p0, at_ns0, br0, "10.1.1.1/24") +ADD_VF(p1, at_ns1, br0, "10.1.1.2/24") +ADD_VF(p2, at_ns2, br0, "10.1.1.3/24") + +NS_CHECK_EXEC([at_ns0], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], [0], + [ignore]) +NS_CHECK_EXEC([at_ns1], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], [0], + [ignore]) +NS_CHECK_EXEC([at_ns2], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], [0], + [ignore]) + +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) + +m4_define([ICMP_PKT], [m4_join([,], + [eth_src=f0:00:00:01:01:01,eth_dst=f0:00:00:01:01:02,eth_type=0x0800], + [nw_src=10.1.1.1,nw_dst=10.1.1.2], + [nw_proto=1,nw_ttl=64,nw_frag=no], + [icmp_type=8,icmp_code=0])]) + +m4_define([ICMP_PKT_REPLY], [m4_join([,], + [eth_src=f0:00:00:01:01:02,eth_dst=f0:00:00:01:01:01,eth_type=0x0800], + [nw_src=10.1.1.2,nw_dst=10.1.1.1], + [nw_proto=1,nw_ttl=64,nw_frag=no], + [icmp_type=8,icmp_code=0])]) + +# Sent a packet in one direction; this should create a non-offloadable +# flow between br0 and ovs-p1. +NS_CHECK_EXEC([at_ns0], [$PYTHON3 $srcdir/sendpkt.py p0 \ + $(ovs-ofctl compose-packet --bare 'ICMP_PKT' '')], [0], [ignore]) + +OVS_WAIT_UNTIL_EQUAL( + [ovs-appctl dpctl/dump-flows --names type=partially-offloaded \ + | grep "f0:00:00:01:01:01" | DUMP_DP_IP_CLEAN_SORTED], [dnl +in_port(ovs-p0),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions:br0,ovs-p1,ovs-p2]) + +# Sent the reply packet. A new flow should be inserted in hardware for the +# reverse direction (since the FDB entry for the destination is known). However, +# the revalidator should update the originating flow, causing it to be fully +# offloaded. +NS_CHECK_EXEC([at_ns1], [$PYTHON3 $srcdir/sendpkt.py p1 \ + $(ovs-ofctl compose-packet --bare 'ICMP_PKT_REPLY' '')], [0], [ignore]) + +OVS_WAIT_UNTIL_EQUAL( + [ovs-appctl dpctl/dump-flows --names type=dpdk,offloaded \ + | grep "f0:00:00:01:01:01" | DUMP_DP_IP_CLEAN_SORTED], [dnl +in_port(ovs-p0),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions:ovs-p1 +in_port(ovs-p1),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions:ovs-p0]) + +OVS_TRAFFIC_VSWITCHD_STOP +AT_CLEANUP >> >> Cheers, >> >> Eelco >> >>>> 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%2Fma >>>>> >> il%2F&data=05%7C02%7Celibr%40nvidia.com%7Ca55c7369b1034194d01b08d >> e15 >>>>> >> 5baabe%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C6389716806 >> 954188 >>>>> >> 84%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAu >> MDAwM >>>>> >> CIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&s >> da >>>>> >> ta=b0c%2FzcJFF8UXGV7aJI9ufWqApXhMRYwtDXSORqkNrI8%3D&reserved=0 >>>>> .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
