| Message ID | 1587145415-16372-1-git-send-email-pavel.contrib@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Skip setting the same hwaddr to a lag port if not needed | expand |
Fri, Apr 17, 2020 at 07:43:35PM CEST, pavel.contrib@gmail.com 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 <pavel.contrib@gmail.com> >--- > 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 >
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 <jiri@resnulli.us> wrote: > Fri, Apr 17, 2020 at 07:43:35PM CEST, pavel.contrib@gmail.com 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 <pavel.contrib@gmail.com> > >--- > > 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 > > >
Mon, Apr 27, 2020 at 09:58:45PM CEST, pavel.contrib@gmail.com wrote: >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. 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 <jiri@resnulli.us> wrote: > >> Fri, Apr 17, 2020 at 07:43:35PM CEST, pavel.contrib@gmail.com 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 <pavel.contrib@gmail.com> >> >--- >> > 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 >> > >>
Hi Jiri, Sure. Done. I've sent the patch to the list. Thanks On Mon, May 4, 2020 at 6:40 AM Jiri Pirko <jiri@resnulli.us> wrote: > Mon, Apr 27, 2020 at 09:58:45PM CEST, pavel.contrib@gmail.com wrote: > >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. > > > 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 <jiri@resnulli.us> wrote: > > > >> Fri, Apr 17, 2020 at 07:43:35PM CEST, pavel.contrib@gmail.com 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 <pavel.contrib@gmail.com> > >> >--- > >> > 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 > >> > > >> >
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:
Avoid setting the same mac address to a LAG port, if the LAG port already has the right one. Signed-off-by: Pavel Shirshov <pavel.contrib@gmail.com> --- teamd/teamd.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)