On 3/4/21 4:32 PM, Michal Kazior wrote:
> From: Michal Kazior <mic...@plume.com>
>
> Some older wireless drivers - ones relying on the
> old and long deprecated wireless extension ioctl
> system - can generate quite a bit of IFLA_WIRELESS
> events depending on their configuration and
> runtime conditions. These are delivered as
> RTNLGRP_LINK via RTM_NEWLINK messages.
>
> These tend to be relatively easily identifiable
> because they report the change mask being 0. This
> isn't guaranteed but in practice it shouldn't be a
> problem. None of the wireless events that I ever
> observed actually carry any unique information
> about netdev states that ovs-vswitchd is
> interested in. Hence ignoring these shouldn't
> cause any problems.
>
> These events can be responsible for a significant
> CPU churn as ovs-vswitchd attempts to do plenty of
> work for each and every netlink message regardless
> of what that message carries. On low-end devices
> such as consumer-grade routers these can lead to a
> lot of CPU cycles being wasted, adding up to heat
> output and reducing performance.
>
> It could be argued that wireless drivers in
> question should be fixed, but that isn't exactly a
> trivial thing to do. Patching ovs seems far more
> viable while still making sense.
>
> Signed-off-by: Michal Kazior <mic...@plume.com>
> ---
>
> Notes:
> v4:
> - fixed comment-too-long checkpatch warnin [0day robot]
>
> v3:
> - dont change rtnetlink_parse() semantics, instead
> extend rtnetlink_change struct and update its
> consumers [Ilya]
> - adjusted commit log to reflect different approach
> [Ilya]
>
> v2:
> - fix bracing style [0day robot / checkpatch]
>
> lib/if-notifier.c | 7 ++++++-
> lib/netdev-linux.c | 9 +++++++++
> lib/route-table.c | 4 ++++
> lib/rtnetlink.c | 18 ++++++++++++++++++
> lib/rtnetlink.h | 3 +++
> 5 files changed, 40 insertions(+), 1 deletion(-)
Hi. Thanks for the new version!
And sorry again for slow reviews.
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index 6be23dbee..388288f71 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -663,6 +663,10 @@ netdev_linux_update_lag(struct rtnetlink_change *change)
> {
> struct linux_lag_member *lag;
>
> + if (change->irrelevant) {
> + return;
> + }
> +
> if (change->sub && netdev_linux_kind_is_lag(change->sub)) {
> lag = shash_find_data(&lag_shash, change->ifname);
>
> @@ -887,6 +891,10 @@ netdev_linux_update(struct netdev_linux *dev, int nsid,
> const struct rtnetlink_change *change)
> OVS_REQUIRES(dev->mutex)
> {
> + if (change->irrelevant) {
> + return;
> + }
> +
> if (netdev_linux_netnsid_is_eq(dev, nsid)) {
> netdev_linux_update__(dev, change);
> }
It's unclear why we need to check inside these functions.
I mean, there is only one place where these functions called
and there is no any useful work done there beside calling them.
I think, it's better to just check right after receiving
the change in a same way as in netdev_linux_update_via_netlink().
Something like this:
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index e9ce41d10..ef90fc44c 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -663,10 +663,6 @@ netdev_linux_update_lag(struct rtnetlink_change *change)
{
struct linux_lag_member *lag;
- if (change->irrelevant) {
- return;
- }
-
if (change->sub && netdev_linux_kind_is_lag(change->sub)) {
lag = shash_find_data(&lag_shash, change->ifname);
@@ -746,7 +742,7 @@ netdev_linux_run(const struct netdev_class *netdev_class
OVS_UNUSED)
if (!error) {
struct rtnetlink_change change;
- if (rtnetlink_parse(&buf, &change)) {
+ if (rtnetlink_parse(&buf, &change) && !change->irrelevant) {
struct netdev *netdev_ = NULL;
char dev_name[IFNAMSIZ];
@@ -891,10 +887,6 @@ netdev_linux_update(struct netdev_linux *dev, int nsid,
const struct rtnetlink_change *change)
OVS_REQUIRES(dev->mutex)
{
- if (change->irrelevant) {
- return;
- }
-
if (netdev_linux_netnsid_is_eq(dev, nsid)) {
netdev_linux_update__(dev, change);
}
---
What do you think?
If it looks good to you, I can squash above diff with your patch and
apply to master.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
