From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andy Gospodarek Subject: Re: [patch net-next] bonding: create netlink event when bonding option is changed Date: Wed, 20 Aug 2014 10:00:51 -0400 Message-ID: <20140820140051.GC1954@gospo.home.greyhouse.net> References: <1408456932-10318-1-git-send-email-jiri@resnulli.us> <20140819203401.GA1954@gospo.home.greyhouse.net> <20140819203748.GA4326@nanopsycho.orion> <20140819204634.GB1954@gospo.home.greyhouse.net> <20140820064458.GA1916@nanopsycho.orion> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: netdev@vger.kernel.org, davem@davemloft.net, j.vosburgh@gmail.com, vfalico@gmail.com, andy@greyhouse.net, nikolay@redhat.com, dingtianhong@huawei.com, sfeldma@cumulusnetworks.com To: Jiri Pirko Return-path: Received: from ext3.cumulusnetworks.com ([198.211.106.187]:36509 "EHLO ext3.cumulusnetworks.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751747AbaHTOA4 (ORCPT ); Wed, 20 Aug 2014 10:00:56 -0400 Content-Disposition: inline In-Reply-To: <20140820064458.GA1916@nanopsycho.orion> Sender: netdev-owner@vger.kernel.org List-ID: On Wed, Aug 20, 2014 at 08:44:58AM +0200, Jiri Pirko wrote: > Tue, Aug 19, 2014 at 10:46:34PM CEST, gospo@cumulusnetworks.com wrote: > >On Tue, Aug 19, 2014 at 10:37:48PM +0200, Jiri Pirko wrote: > >> Tue, Aug 19, 2014 at 10:34:01PM CEST, gospo@cumulusnetworks.com wrote: > >> >On Tue, Aug 19, 2014 at 04:02:12PM +0200, Jiri Pirko wrote: > >> >> Userspace needs to be notified if one changes some option. > >> >> > >> >> Signed-off-by: Jiri Pirko > >> >> --- > >> >> drivers/net/bonding/bond_options.c | 2 ++ > >> >> include/linux/netdevice.h | 1 + > >> >> 2 files changed, 3 insertions(+) > >> >> > >> >> diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c > >> >> index dc73463..d8dc17f 100644 > >> >> --- a/drivers/net/bonding/bond_options.c > >> >> +++ b/drivers/net/bonding/bond_options.c > >> >> @@ -625,6 +625,8 @@ int __bond_opt_set(struct bonding *bond, > >> >> out: > >> >> if (ret) > >> >> bond_opt_error_interpret(bond, opt, ret, val); > >> >> + else > >> >> + call_netdevice_notifiers(NETDEV_CHANGEINFODATA, bond->dev); > >> >Is this valuable where the configuration change to the bonding driver > >> >came from netlink. Do you have a good example why this useful in the > >> >netlink path as well as when using sysfs? It would seem more logical to > >> >add call_netdevice_notifiers to bonding_sysfs_store_option so anything > >> >listening to netlink messages or the netlink cache can know to check for > >> >changes. > >> > >> But this is called for sysfs as well. > >Sorry for the confusion. I was suggesting that this call is *only* used > >for sysfs changes not for changes from sysfs and netlink as it would > >with your patch. > > > >> Example of use is that application can monitor rtnetlink to see if for > >> example active slave changed, or whatever other option changed. > >Any application monitoring netlink messages will already know about the > >configuration change if it came down via netlink. There would not be a > >need for an extra message. > > > >My question was: Is there a need for 2 netlink messages to indicate > >that a bonding configuration change was needed when the config came over > >netlink. I did not see the need for 2 netlink messages in this case and > >was asking for clarification from you. > > Imagine 2 applications. One is just monitoring netlink events ("ip > mon"), the second is setting up bond via netlink. Now, the second sends > a message to kernel, that is unicast to kernel. The first application > does not see that message. Therefore there is need to generate the event > message in kernel and send it back to userspace via multicast. > That message the first application will see. And that is exactly what > my patch is doing. Agreed. Thanks for having the discussion on this. Acked-by: Andy Gospodarek > > > > >> > >> > > >> >> > >> >> return ret; > >> >> } > >> >> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > >> >> index 967ddcc..44bc4bd 100644 > >> >> --- a/include/linux/netdevice.h > >> >> +++ b/include/linux/netdevice.h > >> >> @@ -2017,6 +2017,7 @@ struct pcpu_sw_netstats { > >> >> #define NETDEV_CHANGEUPPER 0x0015 > >> >> #define NETDEV_RESEND_IGMP 0x0016 > >> >> #define NETDEV_PRECHANGEMTU 0x0017 /* notify before mtu change happened */ > >> >> +#define NETDEV_CHANGEINFODATA 0x0018 > >> >> > >> >> int register_netdevice_notifier(struct notifier_block *nb); > >> >> int unregister_netdevice_notifier(struct notifier_block *nb); > >> >> -- > >> >> 1.9.3 > >> >> > >> >> -- > >> >> To unsubscribe from this list: send the line "unsubscribe netdev" in > >> >> the body of a message to majordomo@vger.kernel.org > >> >> More majordomo info at http://vger.kernel.org/majordomo-info.html > >-- > >To unsubscribe from this list: send the line "unsubscribe netdev" in > >the body of a message to majordomo@vger.kernel.org > >More majordomo info at http://vger.kernel.org/majordomo-info.html