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

Ilya Maximets i.maximets at ovn.org
Mon Jun 29 09:22:44 UTC 2020


On 6/29/20 7:33 AM, Sriharsha Basavapatna wrote:
> 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."

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