netdev - Re: [PATCH net v3] bonding: fix ad_actor_system option setting to default

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 20 Dec 2021 15:53:27 +0100
From:   "Fernando F. Mancera" <ffmancera@...eup.net>
To:     Jay Vosburgh <jay.vosburgh@...onical.com>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH net v3] bonding: fix ad_actor_system option setting to
 default

On 12/18/21 03:34, Jay Vosburgh wrote:
> Fernando Fernandez Mancera <ffmancera@...eup.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 all-zeroes MAC as actors'
>> system address it was failing with EINVAL.
>>
>> An all-zeroes ethernet address is valid, only multicast addresses are not
>> valid values.
>>
>> Signed-off-by: Fernando Fernandez Mancera <ffmancera@...eup.net>
> 
> Acked-by: Jay Vosburgh <jay.vosburgh@...onical.com>
> 
>> ---
>> v2: added documentation changes and modified commit message
>> v3: fixed format warning on commit message
>> ---
>> Documentation/networking/bonding.rst | 11 ++++++-----
>> drivers/net/bonding/bond_options.c   |  2 +-
>> 2 files changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/networking/bonding.rst b/Documentation/networking/bonding.rst
>> index 31cfd7d674a6..c0a789b00806 100644
>> --- a/Documentation/networking/bonding.rst
>> +++ b/Documentation/networking/bonding.rst
>> @@ -196,11 +196,12 @@ ad_actor_sys_prio
>> ad_actor_system
>>
>> 	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.
>> +	protocol packet exchanges (LACPDUs). The value cannot be a multicast
>> +	address. If the all-zeroes MAC is specified, bonding will internally
>> +	use the MAC of the bond itself. 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.
>>
>> 	This parameter has effect only in 802.3ad mode and is available through
>> 	SysFs interface.
>> 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
>>

I noticed this patch state in patchwork is "changes requested"[1]. But I 
didn't get any reply or request. Is the state wrong? Should I ignore it? 
If not, what is missing?

Thanks,
Fernando.

[1] 
https://patchwork.kernel.org/project/netdevbpf/patch/20211218015001.1740-1-ffmancera@riseup.net/