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. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
