Re: [ovs-dev] [PATCH] dpif-netdev-perf: aarch64 support for accurate timing update of TSC cycle counter
On 03.12.2019 15:42, Malvika Gupta wrote:
> Hi Ilya,
>
> Please see the inline comments below. Thanks!
>
>> -Original Message-
>> From: Ilya Maximets
>> Sent: Friday, November 29, 2019 12:30 AM
>> To: Yanqin Wei (Arm Technology China) ; Ilya
>> Maximets ; Malvika Gupta
>> ; d...@openvswitch.org
>> Cc: nd ; Honnappa Nagarahalli
>>
>> 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 On Behalf Of Ilya
>>>> Maximets
>>>> Sent: Tuesday, November 26, 2019 11:38 PM
>>>> To: Malvika Gupta ; d...@openvswitch.org
>>>> Cc: nd ; Honnappa Nagarahalli
>>>>
>>>> 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
>>>>> Signed-off-by: Malvika Gupta
>>>>> ---
>>>>
>>>> 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
>>>> d...@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.
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif-netdev-perf: aarch64 support for accurate timing update of TSC cycle counter
Hi Ilya,
Please see the inline comments below. Thanks!
> -Original Message-
> From: Ilya Maximets
> Sent: Friday, November 29, 2019 12:30 AM
> To: Yanqin Wei (Arm Technology China) ; Ilya
> Maximets ; Malvika Gupta
> ; d...@openvswitch.org
> Cc: nd ; Honnappa Nagarahalli
>
> 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?
> >
> > Best Regards,
> > Wei Yanqin
> >
> >
> >> -Original Message-
> >> From: dev On Behalf Of Ilya
> >> Maximets
> >> Sent: Tuesday, November 26, 2019 11:38 PM
> >> To: Malvika Gupta ; d...@openvswitch.org
> >> Cc: nd ; Honnappa Nagarahalli
> >>
> >> 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
> >>> Signed-off-by: Malvika Gupta
> >>> ---
> >>
> >> 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
> >> d...@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.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
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-the-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.
>
> Best Regards,
> Wei Yanqin
>
>
>> -Original Message-
>> From: dev On Behalf Of Ilya Maximets
>> Sent: Tuesday, November 26, 2019 11:38 PM
>> To: Malvika Gupta ; d...@openvswitch.org
>> Cc: nd ; Honnappa Nagarahalli
>>
>> 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
>>> Signed-off-by: Malvika Gupta
>>> ---
>>
>> 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?
>>
>> 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.
>>
>> Best regards, Ilya Maximets.
>> ___
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif-netdev-perf: aarch64 support for accurate timing update of TSC cycle counter
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-the-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?
Best Regards,
Wei Yanqin
> -Original Message-
> From: dev On Behalf Of Ilya Maximets
> Sent: Tuesday, November 26, 2019 11:38 PM
> To: Malvika Gupta ; d...@openvswitch.org
> Cc: nd ; Honnappa Nagarahalli
>
> 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
> > Signed-off-by: Malvika Gupta
> > ---
>
> 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?
>
> 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.
>
> Best regards, Ilya Maximets.
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
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
> Signed-off-by: Malvika Gupta
> ---
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?
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.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] dpif-netdev-perf: aarch64 support for accurate timing update of TSC cycle counter
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
Signed-off-by: Malvika Gupta
---
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;
#elif defined(__linux__)
return rdtsc_syscall(s);
#else
--
2.17.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
