From: Nikolay Aleksandrov <nikolay@cumulusnetworks•com>
To: Xin Long <lucien.xin@gmail•com>, network dev <netdev@vger•kernel.org>
Cc: davem@davemloft•net, hannes@stressinduktion•org
Subject: Re: [PATCH net] bridge: a netlink notification should be sent whenever those attributes change
Date: Thu, 3 Mar 2016 13:29:54 +0100 [thread overview]
Message-ID: <56D82E42.4060401@cumulusnetworks.com> (raw)
In-Reply-To: <2e80304ba6a0021637e362c83f931dc596f72d89.1457007418.git.lucien.xin@gmail.com>
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.xin@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();
> }
>
next prev parent reply other threads:[~2016-03-03 12:29 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-03 12:16 [PATCH net] bridge: a netlink notification should be sent whenever those attributes change Xin Long
2016-03-03 12:29 ` Nikolay Aleksandrov [this message]
2016-03-05 14:44 ` Xin Long
2016-03-05 19:43 ` Nikolay Aleksandrov
2016-03-06 6:08 ` Xin Long
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=56D82E42.4060401@cumulusnetworks.com \
--to=nikolay@cumulusnetworks$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=hannes@stressinduktion$(echo .)org \
--cc=lucien.xin@gmail$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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