[PATCH] Skip setting the same hwaddr to a lag port if not needed

[Pavel Shirshov]

Avoid setting the same mac address to a LAG port,
if the LAG port already has the right one.

Signed-off-by: Pavel Shirshov 
---
 teamd/teamd.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/teamd/teamd.c b/teamd/teamd.c
index 8cdc16d..f955b19 100644
--- a/teamd/teamd.c
+++ b/teamd/teamd.c
@@ -837,7 +837,14 @@ static int teamd_set_hwaddr(struct teamd_context *ctx)
    	err = -EINVAL;
    	goto free_hwaddr;
    }
-	err = team_hwaddr_set(ctx->th, ctx->ifindex, hwaddr, hwaddr_len);
+
+	if (memcmp(hwaddr, ctx->hwaddr, hwaddr_len))
+		err = team_hwaddr_set(ctx->th, ctx->ifindex, hwaddr, hwaddr_len);
+	else {
+		err = 0;
+		teamd_log_dbg(ctx, "Skip setting same hwaddr string: "%s".", hwaddr_str);
+	}
+
    if (!err)
    	ctx->hwaddr_explicit = true;
 free_hwaddr:

[Jiri Pirko]

Fri, Apr 17, 2020 at 07:43:35PM CEST, [e-mail] wrote:
> Avoid setting the same mac address to a LAG port,
> if the LAG port already has the right one.

What's the benefit added by this patch?
> Signed-off-by: Pavel Shirshov
>
> teamd/teamd.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/teamd/teamd.c b/teamd/teamd.c
> index 8cdc16d..f955b19 100644
> --- a/teamd/teamd.c
> +++ b/teamd/teamd.c
> @@ -837,7 +837,14 @@ static int teamd_set_hwaddr(struct teamd_context *ctx)
>    	err = -EINVAL;
>    	goto free_hwaddr;
>    }
> err = team_hwaddr_set(ctx->th, ctx->ifindex, hwaddr, hwaddr_len);
> if (memcmp(hwaddr, ctx->hwaddr, hwaddr_len))
> err = team_hwaddr_set(ctx->th, ctx->ifindex, hwaddr, hwaddr_len);
> else {
> err = 0;
> teamd_log_dbg(ctx, "Skip setting same hwaddr string: \"%s\".", hwaddr_str);
> }
> if (!err)
>   ctx->hwaddr_explicit = true;
> free_hwaddr:
>
> 2.7.4

[Pavel Shirshov]

Hi Jiri,

Thank you, for reviewing the patch.

This patch has the following benefits:

1. We avoid changing settings of a link. Which prevents the kernel from
   sending multiply Netlink messages. That will benefit Netlink message
   listeners. They will not need to react to the Netlink messages.
   2. It makes libteam faster. We don't need to use any syscalls and go
   deep into libteam functions when we don't need to. One simple check and
   libteam can avoid a lot of work.

In our case, when we have hundreds of ports, and up to a hundred LAGs, this
patch saves us significant time.

Please let me know if you have additional questions.

Thanks

On Fri, Apr 24, 2020 at 11:01 PM Jiri Pirko  wrote:
> Fri, Apr 17, 2020 at 07:43:35PM CEST, [e-mail] wrote:
> > Avoid setting the same mac address to a LAG port,
> > if the LAG port already has the right one.
>
> What's the benefit added by this patch?
> > Signed-off-by: Pavel Shirshov
> >
> > teamd/teamd.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/teamd/teamd.c b/teamd/teamd.c
> > index 8cdc16d..f955b19 100644
> > --- a/teamd/teamd.c
> > +++ b/teamd/teamd.c
> > @@ -837,7 +837,14 @@ static int teamd_set_hwaddr(struct teamd_context
>
> *ctx)
> >           err = -EINVAL;
> >           goto free_hwaddr;
> >   }
> >  err = team_hwaddr_set(ctx->th, ctx->ifindex, hwaddr, hwaddr_len);
> >  if (memcmp(hwaddr, ctx->hwaddr, hwaddr_len))
> >          err = team_hwaddr_set(ctx->th, ctx->ifindex, hwaddr,
>
> hwaddr_len);
> >  else {
> >          err = 0;
> >          teamd_log_dbg(ctx, "Skip setting same hwaddr string:
>
> "%s".", hwaddr_str);
> >  }
> > if (!err)
> >         ctx->hwaddr_explicit = true;
> > free_hwaddr:
> >
> > 2.7.4

[Jiri Pirko]

Mon, Apr 27, 2020 at 09:58:45PM CEST, [e-mail] wrote:
> Hi Jiri,
>
> Thank you, for reviewing the patch.
>
> This patch has the following benefits:
> We avoid changing settings of a link. Which prevents the kernel from
>
> sending multiply Netlink messages. That will benefit Netlink message
>   listeners. They will not need to react to the Netlink messages.
>   2. It makes libteam faster. We don't need to use any syscalls and go
>   deep into libteam functions when we don't need to. One simple check and
>   libteam can avoid a lot of work.
>
> In our case, when we have hundreds of ports, and up to a hundred LAGs, this
> patch saves us significant time.

Could you please add this info into the patch description?

Thanks!
> Please let me know if you have additional questions.
>
> Thanks
>
> On Fri, Apr 24, 2020 at 11:01 PM Jiri Pirko  wrote:
> > Fri, Apr 17, 2020 at 07:43:35PM CEST, [e-mail] wrote:
> > > Avoid setting the same mac address to a LAG port,
> > > if the LAG port already has the right one.
> >
> > What's the benefit added by this patch?
> > > Signed-off-by: Pavel Shirshov
> > >
> > > teamd/teamd.c | 9 ++++++++-
> > > 1 file changed, 8 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/teamd/teamd.c b/teamd/teamd.c
> > > index 8cdc16d..f955b19 100644
> > > --- a/teamd/teamd.c
> > > +++ b/teamd/teamd.c
> > > @@ -837,7 +837,14 @@ static int teamd_set_hwaddr(struct teamd_context
> >
> > *ctx)
> > >           err = -EINVAL;
> > >           goto free_hwaddr;
> > >   }
> > >  err = team_hwaddr_set(ctx->th, ctx->ifindex, hwaddr, hwaddr_len);
> > >  if (memcmp(hwaddr, ctx->hwaddr, hwaddr_len))
> > >          err = team_hwaddr_set(ctx->th, ctx->ifindex, hwaddr,
> >
> > hwaddr_len);
> > >  else {
> > >          err = 0;
> > >          teamd_log_dbg(ctx, "Skip setting same hwaddr string:
> >
> > "%s".", hwaddr_str);
> > >  }
> > > if (!err)
> > >         ctx->hwaddr_explicit = true;
> > > free_hwaddr:
> > >
> > > 2.7.4

[Pavel Shirshov]

Hi Jiri,

Sure. Done. I've sent the patch to the list.

Thanks

On Mon, May 4, 2020 at 6:40 AM Jiri Pirko  wrote:
> Mon, Apr 27, 2020 at 09:58:45PM CEST, [e-mail] wrote:
> > Hi Jiri,
> >
> > Thank you, for reviewing the patch.
> >
> > This patch has the following benefits:
> > We avoid changing settings of a link. Which prevents the kernel from
> >
> > sending multiply Netlink messages. That will benefit Netlink message
> >   listeners. They will not need to react to the Netlink messages.
> >   2. It makes libteam faster. We don't need to use any syscalls and go
> >   deep into libteam functions when we don't need to. One simple check and
> >   libteam can avoid a lot of work.
> >
> > In our case, when we have hundreds of ports, and up to a hundred LAGs,
>
> this
> > patch saves us significant time.
>
> Could you please add this info into the patch description?
>
> Thanks!
> > Please let me know if you have additional questions.
> >
> > Thanks
> >
> > On Fri, Apr 24, 2020 at 11:01 PM Jiri Pirko  wrote:
> > > Fri, Apr 17, 2020 at 07:43:35PM CEST, [e-mail] wrote:
> > > > Avoid setting the same mac address to a LAG port,
> > > > if the LAG port already has the right one.
> > >
> > > What's the benefit added by this patch?
> > > > Signed-off-by: Pavel Shirshov
> > > >
> > > > teamd/teamd.c | 9 ++++++++-
> > > > 1 file changed, 8 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/teamd/teamd.c b/teamd/teamd.c
> > > > index 8cdc16d..f955b19 100644
> > > > --- a/teamd/teamd.c
> > > > +++ b/teamd/teamd.c
> > > > @@ -837,7 +837,14 @@ static int teamd_set_hwaddr(struct teamd_context
> > >
> > > *ctx)
> > > >           err = -EINVAL;
> > > >           goto free_hwaddr;
> > > >   }
> > > >  err = team_hwaddr_set(ctx->th, ctx->ifindex, hwaddr,
>
> hwaddr_len);
> > > >  if (memcmp(hwaddr, ctx->hwaddr, hwaddr_len))
> > > >          err = team_hwaddr_set(ctx->th, ctx->ifindex, hwaddr,
> > >
> > > hwaddr_len);
> > > >  else {
> > > >          err = 0;
> > > >          teamd_log_dbg(ctx, "Skip setting same hwaddr string:
> > >
> > > "%s".", hwaddr_str);
> > > >  }
> > > > if (!err)
> > > >         ctx->hwaddr_explicit = true;
> > > > free_hwaddr:
> > > >
> > > > 2.7.4