On 6/29/20 7:33 AM, Sriharsha Basavapatna wrote: > On Mon, Jun 29, 2020 at 5:30 AM Ilya Maximets <[email protected]> wrote: >> >> On 6/21/20 1:19 PM, Eli Britstein wrote: >>> As offload is done using the mega ufid of a flow, for better >>> debugability, add it in the log message. >> >> Could you, please, tell me why we need this extra ufid generated >> at all? Why the usual flow ufid is not enough? I'm a bit confused. > > I believe this comes from the mark/rss (partial offload) > implementation. The rationale behind this design is explained in this > commit: > > 241bad15d99a422dce71a6ec6116a2d7b9c31796 > (dpif-netdev: associate flow with a mark id) > > I'm pasting the relevant section here for quick reference: > > " One tricky thing in OVS-DPDK is, the flow tables is per-PMD. For the > case there is only one phys port but with 2 queues, there could be 2 > PMDs. In other words, even for a single mega flow (i.e. udp,tp_src=1000), > there could be 2 different dp_netdev flows, one for each PMD. That could > results to the same mega flow being offloaded twice in the hardware, > worse, we may get 2 different marks and only the last one will work. > > To avoid that, a megaflow_to_mark CMAP is created. An entry will be > added for the first PMD that wants to offload a flow. For later PMDs, > it will see such megaflow is already offloaded, then the flow will not > be offloaded to HW twice."
OK. Thanks for the pointer! That is because we're mixing pmd_id into flow ufid. I see. There should be a more elegant solution, but let it be this way for now. > > Thanks, > -Harsha > >> >>> >>> Signed-off-by: Eli Britstein <[email protected]> >>> Reviewed-by: Roni Bar Yanai <[email protected]> >>> --- >>> lib/dpif-netdev.c | 7 +++++-- >>> tests/dpif-netdev.at | 2 ++ >>> tests/ofproto-macros.at | 3 ++- >>> 3 files changed, 9 insertions(+), 3 deletions(-) >>> >>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c >>> index 57565802a..da0c48ef5 100644 >>> --- a/lib/dpif-netdev.c >>> +++ b/lib/dpif-netdev.c >>> @@ -2510,8 +2510,9 @@ dp_netdev_flow_offload_main(void *data OVS_UNUSED) >>> OVS_NOT_REACHED(); >>> } >>> >>> - VLOG_DBG("%s to %s netdev flow\n", >>> - ret == 0 ? "succeed" : "failed", op); >>> + VLOG_DBG("%s to %s netdev flow "UUID_FMT"\n", >>> + ret == 0 ? "succeed" : "failed", op, >>> + UUID_ARGS((struct uuid *) &offload->flow->mega_ufid)); >>> dp_netdev_free_flow_offload(offload); >>> ovsrcu_quiesce(); >>> } >>> @@ -3383,6 +3384,8 @@ dp_netdev_flow_add(struct dp_netdev_pmd_thread *pmd, >>> >>> ds_put_cstr(&ds, "flow_add: "); >>> odp_format_ufid(ufid, &ds); >>> + ds_put_cstr(&ds, " mega_"); >>> + odp_format_ufid(&flow->mega_ufid, &ds); >>> ds_put_cstr(&ds, " "); >>> odp_flow_format(key_buf.data, key_buf.size, >>> mask_buf.data, mask_buf.size, >>> diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at >>> index 21f0c8d24..ec5ffc290 100644 >>> --- a/tests/dpif-netdev.at >>> +++ b/tests/dpif-netdev.at >>> @@ -13,6 +13,7 @@ strip_timers () { >>> >>> strip_xout () { >>> sed ' >>> + s/mega_ufid:[-0-9a-f]* // >>> s/ufid:[-0-9a-f]* // >>> s/used:[0-9]*\.[0-9]*/used:0.0/ >>> s/actions:.*/actions: <del>/ >>> @@ -23,6 +24,7 @@ strip_xout () { >>> >>> strip_xout_keep_actions () { >>> sed ' >>> + s/mega_ufid:[-0-9a-f]* // >>> s/ufid:[-0-9a-f]* // >>> s/used:[0-9]*\.[0-9]*/used:0.0/ >>> s/packets:[0-9]*/packets:0/ >>> diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at >>> index b2b17eed3..87f9ae280 100644 >>> --- a/tests/ofproto-macros.at >>> +++ b/tests/ofproto-macros.at >>> @@ -131,7 +131,8 @@ strip_duration () { >>> # Strips 'ufid:...' from output, to make it easier to compare. >>> # (ufids are random.) >>> strip_ufid () { >>> - sed 's/ufid:[[-0-9a-f]]* //' >>> + sed 's/mega_ufid:[[-0-9a-f]]* // >>> + s/ufid:[[-0-9a-f]]* //' >>> } >>> m4_divert_pop([PREPARE_TESTS]) >>> >>> >> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
