> On Sep 12, 2022, at 4:17 PM, Ilya Maximets <i.maxim...@ovn.org> 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 <i.maxim...@ovn.org> wrote: >>>> >>>> On 6/3/22 16:47, Jon Kohler wrote: >>>>> >>>>> >>>>>> On Jun 2, 2022, at 5:50 PM, Ilya Maximets <i.maxim...@ovn.org> 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 <j...@nutanix.com> wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>>> On May 26, 2022, at 9:11 PM, Jon Kohler <j...@nutanix.com> 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 <j...@nutanix.com> >>>>>>>>> Acked-by: Greg Smith <gasm...@nutanix.com> >>>>>>>> >>>>>>>> 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 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. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
