[ovs-dev] [PATCH] dpif-netdev-perf: aarch64 support for accurate timing update of TSC cycle counter

Ilya Maximets i.maximets at ovn.org
Tue Dec 3 15:19:23 UTC 2019


On 03.12.2019 15:42, Malvika Gupta wrote:
> Hi Ilya,
> 
> Please see the inline comments below. Thanks!
> 
>> -----Original Message-----
>> From: Ilya Maximets <i.maximets at ovn.org>
>> Sent: Friday, November 29, 2019 12:30 AM
>> To: Yanqin Wei (Arm Technology China) <Yanqin.Wei at arm.com>; Ilya
>> Maximets <i.maximets at ovn.org>; Malvika Gupta
>> <Malvika.Gupta at arm.com>; dev at openvswitch.org
>> Cc: nd <nd at arm.com>; Honnappa Nagarahalli
>> <Honnappa.Nagarahalli at arm.com>
>> Subject: Re: [ovs-dev] [PATCH] dpif-netdev-perf: aarch64 support for
>> accurate timing update of TSC cycle counter
>>
>> On 27.11.2019 8:38, Yanqin Wei (Arm Technology China) wrote:
>>> Hi Ilya,
>>>
>>> No, we didn't test this patch based on OVS-AF_XDP, but made a black build
>> to enable this in OVS-DPDK and test it.
>>> Currently DPDK-AF_XDP has been tested in latest kernel (not released). So I
>> think OVS-AF_XDP is close to be supported for aarch64.
>>>
>>> Furthermore, I found a document about userspace-only mode of Open
>> vSwitch without DPDK.
>>> http://docs.openvswitch.org/en/latest/intro/install/userspace/#using-t
>>> he-userspace-datapath-with-ovs-vswitchd
>>> So it seems userspace datapath should be decoupled with networking IO,
>> users can even customize this. Does it means we need implement all used
>> DPDK API inside OVS?
>>
>> Userspace datapath in OVS doesn't depend on DPDK.  DPDK only provides a
>> few port types (dpdk, dpdkvhostuser, etc.).  It's fully functional by itself.  For
>> example you may use it to forward packets with OVS-native afxdp ports:
>> http://docs.openvswitch.org/en/latest/intro/install/afxdp/
>>
>> Best regards, Ilya Maximets.
>>
> 
> How do you suggest we proceed with this patch till OVS-AF_XDP is supported for aarch64?

There is nothing to do from the OVS side, so it's totally OK.
The code is working and reachable with a userspace datapath, so
we could accept it.

Looking forward for v2.

Best regards, Ilya Maximets.

> 
>>>
>>> Best Regards,
>>> Wei Yanqin
>>>
>>>
>>>> -----Original Message-----
>>>> From: dev <ovs-dev-bounces at openvswitch.org> On Behalf Of Ilya
>>>> Maximets
>>>> Sent: Tuesday, November 26, 2019 11:38 PM
>>>> To: Malvika Gupta <Malvika.Gupta at arm.com>; dev at openvswitch.org
>>>> Cc: nd <nd at arm.com>; Honnappa Nagarahalli
>>>> <Honnappa.Nagarahalli at arm.com>
>>>> Subject: Re: [ovs-dev] [PATCH] dpif-netdev-perf: aarch64 support for
>>>> accurate timing update of TSC cycle counter
>>>>
>>>> On 13.11.2019 18:01, Malvika Gupta wrote:
>>>>> The accurate timing implementation in this patch gets the wall clock
>>>>> counter via
>>>>> cntvct_el0 register access. This call is portable to all aarch64
>>>>> architectures and has been verified on an 64-bit arm server.
>>>>>
>>>>> Suggested-by: Yanqin Wei <yanqin.wei at arm.com>
>>>>> Signed-off-by: Malvika Gupta <malvika.gupta at arm.com>
>>>>> ---
>>>>
>>>> Thanks for the patch!
>>>>
>>>> Are you trying to use AF_XDP on aarch64?  Asking because it's the
>>>> only real scenario where this patch can be useful.
>>>>
>>>> For the patch subject, I'd suggest to shorten it a little.
>>>> 'timing', 'TSC' and 'cycle counter' are kind of synonyms here and
>>>> doesn't make the sentence any clear.  Suggesting something like this:
>>>> "dpif-netdev-perf: Accurate cycle counter update on aarch64."
>>>>
>>>> What do you think?
> 
> I agree we can shorten the name; I will submit the changes in v2.
> 
>>>>
>>>> One more comment inline.
>>>>
>>>>>  lib/dpif-netdev-perf.h | 5 +++++
>>>>>  1 file changed, 5 insertions(+)
>>>>>
>>>>> diff --git a/lib/dpif-netdev-perf.h b/lib/dpif-netdev-perf.h index
>>>>> ce369375b..4ea7cc355 100644
>>>>> --- a/lib/dpif-netdev-perf.h
>>>>> +++ b/lib/dpif-netdev-perf.h
>>>>> @@ -220,6 +220,11 @@ cycles_counter_update(struct pmd_perf_stats
>> *s)
>>>>>      asm volatile("rdtsc" : "=a" (l), "=d" (h));
>>>>>
>>>>>      return s->last_tsc = ((uint64_t) h << 32) | l;
>>>>> +#elif !defined(_MSC_VER) && defined(__aarch64__)
>>>>> +    uint64_t tsc;
>>>>> +    asm volatile("mrs %0, cntvct_el0" : "=r" (tsc));
>>>>> +
>>>>> +    return s->last_tsc = tsc;
>>>>
>>>> I think we could drop the 'tsc' local variable here and write
>>>> directly to s-
>>>>> last_tsc.  Less number of variables and operations.
>>>>
> 
> Okay, I will make this change in v2.
> Best,
> Malvika
> 
>>>> Best regards, Ilya Maximets.
>>>> _______________________________________________
>>>> dev mailing list
>>>> dev at openvswitch.org
>>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
> 


More information about the dev mailing list