On 9/12/22 17:58, Jon Kohler wrote: > > >> On Sep 12, 2022, at 4:17 PM, Ilya Maximets <[email protected]> wrote: >> >> On 6/24/22 22:01, Ilya Maximets wrote: >>> On 6/16/22 16:07, Jon Kohler wrote: >>>> >>>> >>>>> On Jun 3, 2022, at 1:00 PM, Ilya Maximets <[email protected]> wrote: >>>>> >>>>> On 6/3/22 16:47, Jon Kohler wrote: >>>>>> >>>>>> >>>>>>> On Jun 2, 2022, at 5:50 PM, Ilya Maximets <[email protected]> wrote: >>>>>>> >>>>>>> On 6/2/22 23:13, Jon Kohler wrote: >>>>>>>> Any takers? I’m hoping I’ve got the right mailing list, as I did see >>>>>>>> the thread get generated on the mailing list website? >>>>>>> >>>>>>> Hey, Jon. >>>>>>> >>>>>>> Yes, it's the right mailing list, though ovs-dev and dev are >>>>>>> the same thing, so you don't need to send to both. >>>>>>> >>>>>>> For the subject, you sent a patch just about a week ago. >>>>>>> Unfortunately, patches are not moving that fast. People >>>>>>> are busy with working on issues and doing reviews of other >>>>>>> patches, so please, be patient. >>>>>>> >>>>>>> Just from the patches received last month we still have >>>>>>> about 60 under review right now. You can see them in the >>>>>>> patchwork here: >>>>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_project_openvswitch_list_&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=NGPRGGo37mQiSXgHKm5rCQ&m=8oqFH3XK_nAzOxdHmUuHbWgSEPemwCFQKUdGExfEPEx3NfR3ioDpK5CZHwMdEyf_&s=Osr7hGzYKATNpHa9IvtWygokIFsGsmT4GrnDuUQ3FzY&e= >>>>>>> >>>>>>> >>>>>>> And as you can see we have much older patches too. So, it >>>>>>> will take some time until someone will get to yours. >>>>>>> >>>>>>> Best regards, Ilya Maximets. >>>>>> >>>>>> Thanks for the feedback and pointers. My apologies, I wasn’t trying to be >>>>>> ansty :) this is my first patch to this mailing list and wanted to make >>>>>> sure it >>>>>> was going to the right place. >>>>> >>>>> No problem. :) >>>>> >>>>>> >>>>>> Ok no problems, I can fully empathize with patch review backlogs :) No >>>>>> specific rush on this one. Nice CPU win whenever someone can get >>>>>> to it, but would definitely rather have quality over speed to make sure >>>>>> we haven’t missed anything! :) >>>>> >>>>> Thanks for working on improvements! I hope, someone will get >>>>> to this patch in the near future as it looks interesting indeed. >>>> >>>> Hey all, checking back on this one, its been a couple weeks and >>>> figured I’d reach out as I saw some release work happen on the >>>> mailing list recently, and hoping that may have freed up some cycles? >>>> >>>> Also, forgive the silly question, I’m not sure what this mailing lists >>>> preference >>>> is on touchpoint like this, or if it is better to do a resend on the patch >>>> instead? Didn’t want to do that in case it was frowned upon. >>> >>> No need to re-send. Just a ping is enough, unless the patch >>> needs a significant re-work due to other changes. If the patch >>> in in the patchwork, it can be picked up. Things are not moving >>> very fast around here... >>> >>>> >>>> As mentioned, no specific rush on this one, just my first patch to OVS >>>> and want to make sure I’m doing right by the list. >>> >>> No problem. >>> >>> For the patch itself: The code looks correct to me, see some >>> minor comments below. >>> >>> Best regards, Ilya Maximets. >>> >>>> >>>> Thanks again, >>>> Jon >>>> >>>>> >>>>>> >>>>>> Cheers, >>>>>> Jon >>>>>> >>>>>>> >>>>>>>> >>>>>>>>> On May 31, 2022, at 10:03 AM, Jon Kohler <[email protected]> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>>> On May 26, 2022, at 9:11 PM, Jon Kohler <[email protected]> wrote: >>>>>>>>>> >>>>>>>>>> For netdev_linux_update_via_netlink(), hint to the kernel that >>>>>>>>>> we do not need it to gather netlink internal stats when we want >>>>>>>>>> to update the netlink flags, as those stats are not rendered >>>>>>>>>> within OVS. >>>>>>>>>> >>>>>>>>>> Background: >>>>>>>>>> ovs-vswitchd can spend quite a bit of time blocked by the kernel >>>>>>>>>> during netlink calls, especially systems with many cores. This >>>>>>>>>> time is dominated by the kernel-side internal stats gathering >>>>>>>>>> mechanism in netlink, specifically: >>>>>>>>>> inet6_fill_link_af >>>>>>>>>> inet6_fill_ifla6_attrs >>>>>>>>>> __snmp6_fill_stats64 >>>>>>>>>> >>>>>>>>>> In Linux 4.4+, there exists a hint for netlink requests to not >>>>>>>>>> trigger the ipv6 stats gathering mechanism, which greatly reduces >>>>>>>>>> the amount of time that ovs-vswitchd is on CPU. >>>>>>>>>> >>>>>>>>>> Testing and Results: >>>>>>>>>> Tested booting 320 VM's and measuring OVS utilization with perf >>>>>>>>>> record, then visualized into a flamegraph using a patched version >>>>>>>>>> of ovs 2.14.2. Calls under bridge_run() seem to get hit the worst >>>>>>>>>> by this issue. >>>>>>>>>> >>>>>>>>>> Before bridge_run() == 11.3% of samples >>>>>>>>>> After bridge_run() == 3.4% of samples >>>>>>>>>> >>>>>>>>>> Note that there are at least two observed netlink calls under >>>>>>>>>> bridge_run that are still kernel stats heavy after this patch: >>>>>>>>>> >>>>>>>>>> Call 1: >>>>>>>>>> bridge_run -> netdev_run -> route_table_run -> route_table_reset -> >>>>>>>>>> ovs_router_insert -> ovs_router_insert__ -> get_src_addr -> >>>>>>>>>> netdev_ger_addr_list -> netdev_linux_get_addr_list -> getifaddrs >>>>>>>>>> >>>>>>>>>> Since the actual netlink call is coming from getifaddrs() in glibc, >>>>>>>>>> fixing would likely involve either duplicating glibc code in ovs >>>>>>>>>> source or patch glibc. >>>>>>>>>> >>>>>>>>>> Call 2: >>>>>>>>>> bridge_run -> iface_refresh_stats -> netdev_get_stats -> >>>>>>>>>> netdev_linux_get_stats -> get_stats_via_netlink >>>>>>>>>> >>>>>>>>>> This does use netlink based stats; however, it isn't immediately >>>>>>>>>> clear if just dropping the stats from inet6_fill_link_af would >>>>>>>>>> impact anything or not. Given this call is more intermittent, its >>>>>>>>>> of lesser concern. >>>>>>>>>> >>>>>>>>>> Signed-off-by: Jon Kohler <[email protected]> >>>>>>>>>> Acked-by: Greg Smith <[email protected]> >>>>>>>>> >>>>>>>>> Gentle bump >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>>> >>>>>>>>>> --- >>>>>>>>>> lib/netdev-linux.c | 9 +++++++++ >>>>>>>>>> 1 file changed, 9 insertions(+) >>>>>>>>>> >>>>>>>>>> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c >>>>>>>>>> index 2766b3f2bf..f0246d3b2b 100644 >>>>>>>>>> --- a/lib/netdev-linux.c >>>>>>>>>> +++ b/lib/netdev-linux.c >>>>>>>>>> @@ -247,6 +247,12 @@ enum { >>>>>>>>>> VALID_NUMA_ID = 1 << 8, >>>>>>>>>> }; >>>>>>>>>> >>>>>>>>>> +/* Linux 4.4 introduced the ability to skip the internal stats >>>>>>>>>> gathering >>>>>>>>>> + * that netlink does via an external filter mask that can be passed >>>>>>>>>> into >>>>>>>>>> + * a netlink request. >>>>>>>>>> + */ >>>>>>>>>> +#define RTEXT_FILTER_SKIP_STATS (1 << 3) >>> >>> Please, don't use tabs. :) >>> Also, we should, probably, avoid re-defining the value if it >>> was already defined in kernel uAPI headers. So, it's better >>> to guard with #ifndef as we do for other similar definitions. >> >> There is no reason to hold this patch, so I made above changes >> myself and applied the fix. Also backported down to 2.17 since >> it seems fairly important. >> >> Thanks! >> >> Best regards, Ilya Maximets. > > Thanks! Sorry I dropped off the face of the earth for a while, > My mother got very sick this summer, so many, many things > have slipped by over the last few months. I really appreciate > the assist in getting this done! Cheers, Jon
No problem. I hope everything is fine. > > PS - if anyone happens to be at the Open Source summit, I’ve > included this as part of the talk I’m giving at KVM forum > tomorrow. > I'm not on KVM forum, but the talk submission for OVS Conf in November is still open for a couple more days. If you want to share your experience there in a lightning talk or a full talk, if you have some more details around OVS, that would be great. (virtual is also possible) In any case would like to watch the recording from KVM forum once it is available. Thanks for the pointer! Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
