* [PATCH net] bonding: fix ad_actor_system option setting to default
@ 2021-12-17 18:28 Fernando Fernandez Mancera
2021-12-17 22:22 ` Jay Vosburgh
0 siblings, 1 reply; 3+ messages in thread
From: Fernando Fernandez Mancera @ 2021-12-17 18:28 UTC (permalink / raw)
To: netdev; +Cc: Fernando Fernandez Mancera
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.
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net] bonding: fix ad_actor_system option setting to default
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
2021-12-18 1:28 ` Fernando F. Mancera
0 siblings, 1 reply; 3+ messages in thread
From: Jay Vosburgh @ 2021-12-17 22:22 UTC (permalink / raw)
To: Fernando Fernandez Mancera; +Cc: netdev
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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH net] bonding: fix ad_actor_system option setting to default
2021-12-17 22:22 ` Jay Vosburgh
@ 2021-12-18 1:28 ` Fernando F. Mancera
0 siblings, 0 replies; 3+ messages in thread
From: Fernando F. Mancera @ 2021-12-18 1:28 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: netdev
On 12/17/21 23:22, Jay Vosburgh wrote:
> 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?
Yes, as the user should be able to set it back to the MAC of the bond
itself. Basically, restore it to the default value.
>
> 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."
Sure, that would be good. I will do it. Thank you!
>
> -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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-12-18 1:28 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2021-12-18 1:28 ` Fernando F. Mancera
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).