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."
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
