On 03.12.2019 15:42, Malvika Gupta wrote: > Hi Ilya, > > Please see the inline comments below. Thanks! > >> -----Original Message----- >> From: Ilya Maximets <[email protected]> >> Sent: Friday, November 29, 2019 12:30 AM >> To: Yanqin Wei (Arm Technology China) <[email protected]>; Ilya >> Maximets <[email protected]>; Malvika Gupta >> <[email protected]>; [email protected] >> Cc: nd <[email protected]>; Honnappa Nagarahalli >> <[email protected]> >> 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 <[email protected]> On Behalf Of Ilya >>>> Maximets >>>> Sent: Tuesday, November 26, 2019 11:38 PM >>>> To: Malvika Gupta <[email protected]>; [email protected] >>>> Cc: nd <[email protected]>; Honnappa Nagarahalli >>>> <[email protected]> >>>> 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 <[email protected]> >>>>> Signed-off-by: Malvika Gupta <[email protected]> >>>>> --- >>>> >>>> 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 >>>> [email protected] >>>> 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 [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
