netdev - Re: [patch net-next 16/26] bonding: implement lower state change propagation

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]

Message-ID: <565ED8C3.3090709@cumulusnetworks.com>
Date:	Wed, 2 Dec 2015 12:40:51 +0100
From:	Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
To:	Jiri Pirko <jiri@...nulli.us>, netdev@...r.kernel.org
Cc:	davem@...emloft.net, idosch@...lanox.com, eladr@...lanox.com,
	yotamg@...lanox.com, ogerlitz@...lanox.com
Subject: Re: [patch net-next 16/26] bonding: implement lower state change
 propagation

On 12/01/2015 02:48 PM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@...lanox.com>
> 
> Let netdev notifier listeners know about link and slave state change.
> 
> Signed-off-by: Jiri Pirko <jiri@...lanox.com>
> ---
>  drivers/net/bonding/bond_main.c | 10 ++++++++++
>  include/net/bonding.h           |  7 +++++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index c9943fc..e153a87 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1315,6 +1315,16 @@ void bond_queue_slave_event(struct slave *slave)
>  	queue_delayed_work(slave->bond->wq, &nnw->work, 0);
>  }
>  
> +void bond_lower_state_changed(struct slave *slave)
> +{
> +	struct netdev_lag_lower_state_info info;
> +
> +	info.link_up = slave->link == BOND_LINK_UP ||
> +		       slave->link == BOND_LINK_FAIL;
> +	info.tx_enabled = bond_is_active_slave(slave);
> +	netdev_lower_state_changed(slave->dev, &info);
> +}
> +

Hmm, but does this tell the listeners what changed ? I think it just
sends the current slave state and the listener has to decide what has
changed.
For example, right now it's possible for multiple identical events to
be sent (e.g. set_inactive_flags called two times on release, once because
of curr_active_slave change and second because of your set), the listeners
should be able to cope with that. For this same example I see that the mlxsw
will call mlxsw_sp_port_lag_changed() two times with the same values in that
case. I'm not saying this is necessarily bad, just noting it. :-)


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html