On 03/03/2016 01:16 PM, Xin Long wrote:
> Now when we change the attributes of bridge or br_port by netlink,
> a relevant netlink notification will be sent, but if we change them
> by ioctl or sysfs, no notification will be sent.
>
> we should ensure that whenever those attributes change internally or from
> sysfs/ioctl, that a netlink notification is sent out to listeners.
>
> Also, NetworkManager will use this in the future to listen for out-of-band
> bridge master attribute updates and incorporate them into the runtime
> configuration.
>
> Signed-off-by: Xin Long <lucien....@gmail.com>
> ---
> net/bridge/br_ioctl.c | 40 ++++++++++++++++++++++++----------------
> net/bridge/br_sysfs_br.c | 2 ++
> net/bridge/br_sysfs_if.c | 4 +++-
> 3 files changed, 29 insertions(+), 17 deletions(-)
>
[snip]
> static int old_deviceless(struct net *net, void __user *uarg)
> diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
> index 6b80914..231e093 100644
> --- a/net/bridge/br_sysfs_br.c
> +++ b/net/bridge/br_sysfs_br.c
> @@ -44,6 +44,8 @@ static ssize_t store_bridge_parm(struct device *d,
> return -EINVAL;
>
> err = (*set)(br, val);
> + if (!err)
> + netdev_state_change(br->dev);
This is incorrect because you don't have rtnl here, bridge device sysfs
options take care of rtnl only on per-option basis and they obtain and
release it themselves, so you won't have rtnl held when you call
netdev_state_change. While I agree that this is needed, a larger change
would be necessary for br_sysfs_br.c.
Off-topic: I've been looking into factoring out the bond option API and reusing
it here as it already has all of this handled, but I won't have time to finish
it before the next merge window, so if you fix the issue here I'm okay with
this as interim solution.
Cheers,
Nik
> return err ? err : len;
> }
>
> diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
> index efe415a..0ad8592 100644
> --- a/net/bridge/br_sysfs_if.c
> +++ b/net/bridge/br_sysfs_if.c
> @@ -253,8 +253,10 @@ static ssize_t brport_store(struct kobject *kobj,
> spin_lock_bh(&p->br->lock);
> ret = brport_attr->store(p, val);
> spin_unlock_bh(&p->br->lock);
> - if (ret == 0)
> + if (ret == 0) {
> + br_ifinfo_notify(RTM_NEWLINK, p);
> ret = count;
> + }
> }
> rtnl_unlock();
> }
>
