netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jay Vosburgh <jay.vosburgh@canonical.com>
To: Fernando Fernandez Mancera <ffmancera@riseup.net>
Cc: netdev@vger.kernel.org
Subject: Re: [PATCH net] bonding: fix ad_actor_system option setting to default
Date: Fri, 17 Dec 2021 14:22:40 -0800	[thread overview]
Message-ID: <14022.1639779760@famine> (raw)
In-Reply-To: <20211217182846.6910-1-ffmancera@riseup.net>

Fernando Fernandez Mancera <ffmancera@riseup.net> wrote:

>When 802.3ad bond mode is configured the ad_actor_system option is set
>to "00:00:00:00:00:00". But when trying to set the default value
>manually it was failing with EINVAL.
>
>A zero ethernet address is valid, only multicast addresses are not valid
>values.

	Your intent here by setting ad_actor_system to all zeroes is to
induce bonding to actually set ad_actor_system to the MAC of the bond
itself?

	If so, please also update Documentation/networking/bonding.rst,
as the current text says

        In an AD system, this specifies the mac-address for the actor in
        protocol packet exchanges (LACPDUs). The value cannot be NULL or
        multicast. It is preferred to have the local-admin bit set for this
        mac but driver does not enforce it. If the value is not given then
        system defaults to using the masters' mac address as actors' system
        address.

	I'd suggest something like "The value cannot be a multicast
address.  If the all-zeroes MAC is specified, bonding will internally
use the MAC of the bond itself."

	-J

>Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
>---
> drivers/net/bonding/bond_options.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>index a8fde3bc458f..b93337b5a721 100644
>--- a/drivers/net/bonding/bond_options.c
>+++ b/drivers/net/bonding/bond_options.c
>@@ -1526,7 +1526,7 @@ static int bond_option_ad_actor_system_set(struct bonding *bond,
> 		mac = (u8 *)&newval->value;
> 	}
> 
>-	if (!is_valid_ether_addr(mac))
>+	if (is_multicast_ether_addr(mac))
> 		goto err;
> 
> 	netdev_dbg(bond->dev, "Setting ad_actor_system to %pM\n", mac);
>-- 
>2.30.2
>

---
	-Jay Vosburgh, jay.vosburgh@canonical.com

  reply	other threads:[~2021-12-17 22:22 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-17 18:28 [PATCH net] bonding: fix ad_actor_system option setting to default Fernando Fernandez Mancera
2021-12-17 22:22 ` Jay Vosburgh [this message]
2021-12-18  1:28   ` Fernando F. Mancera

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=14022.1639779760@famine \
    --to=jay.vosburgh@canonical.com \
    --cc=ffmancera@riseup.net \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).