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.
AFAICT, kernel will just ignore unknown flag, e.g. if it's
older than 4.4, is that correct?
>>>>>>> +
>>>>>>> /* Use one for the packet buffer and another for the aux buffer to
>>>>>>> receive
>>>>>>> * TSO packets. */
>>>>>>> #define IOV_STD_SIZE 1
>>>>>>> @@ -6418,6 +6424,9 @@ netdev_linux_update_via_netlink(struct
>>>>>>> netdev_linux *netdev)
>>>>>>> if (netdev_linux_netnsid_is_remote(netdev)) {
>>>>>>> nl_msg_put_u32(&request, IFLA_IF_NETNSID, netdev->netnsid);
>>>>>>> }
>>>>>>> +
>>>>>>> + nl_msg_put_u32(&request, IFLA_EXT_MASK, RTEXT_FILTER_SKIP_STATS);
>>>>>>> +
>>>>>>> error = nl_transact(NETLINK_ROUTE, &request, &reply);
>>>>>>> ofpbuf_uninit(&request);
>>>>>>> if (error) {
>>>>>>> --
>>>>>>> 2.30.1 (Apple Git-130)
>>>>>
>>>>> _______________________________________________
>>>>> dev mailing list
>>>>> d...@openvswitch.org
>>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=NGPRGGo37mQiSXgHKm5rCQ&m=8oqFH3XK_nAzOxdHmUuHbWgSEPemwCFQKUdGExfEPEx3NfR3ioDpK5CZHwMdEyf_&s=h-hQ3onb9ZZoJwMrpOdPVN0uZVoTPJtZ8ZHFKTCknZ0&e=
>>>
>>
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
