[ovs-dev] [PATCH V3 10/12] dpif-netdev: Add mega ufid in flow add log

Sriharsha Basavapatna sriharsha.basavapatna at broadcom.com
Mon Jun 29 05:33:55 UTC 2020


On Mon, Jun 29, 2020 at 5:30 AM Ilya Maximets <i.maximets at ovn.org> 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 <elibr at mellanox.com>
> > Reviewed-by: Roni Bar Yanai <roniba at mellanox.com>
> > ---
> >  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])
> >
> >
>


More information about the dev mailing list