| Message ID | 20140829195339.GA9780@kria |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Fri, Aug 29, 2014 at 12:53 PM, Sabrina Dubroca <sd@queasysnail.net> wrote: > 2014-08-29, 11:14:48 -0700, Cong Wang wrote: >> On Fri, Aug 29, 2014 at 8:26 AM, Tommi Rantala <tt.rantala@gmail.com> wrote: >> > [ 77.297196] RTNL: assertion failed at net/ipv6/addrconf.c (1699) >> > [ 77.298080] CPU: 0 PID: 4842 Comm: trinity-main Not tainted 3.17.0-rc2+ #30 >> > [ 77.299039] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011 >> > [ 77.299789] ffff88003d76a618 ffff880026133c50 ffffffff8238ba79 >> > ffff880037c84520 >> > [ 77.300829] ffff880026133c90 ffffffff820bd52b 0000000000000000 >> > ffffffff82d86c40 >> > [ 77.301869] 0000000000000000 00000000f76fd1e1 ffff8800382d8000 >> > ffff8800382d8220 >> > [ 77.302906] Call Trace: >> > [ 77.303246] [<ffffffff8238ba79>] dump_stack+0x4d/0x66 >> > [ 77.303928] [<ffffffff820bd52b>] addrconf_join_solict+0x4b/0xb0 >> > [ 77.304731] [<ffffffff820b031b>] ipv6_dev_ac_inc+0x2bb/0x330 >> > [ 77.305498] [<ffffffff820b0060>] ? ac6_seq_start+0x260/0x260 >> > [ 77.306257] [<ffffffff820b05fe>] ipv6_sock_ac_join+0x26e/0x360 >> > [ 77.307046] [<ffffffff820b0429>] ? ipv6_sock_ac_join+0x99/0x360 >> > [ 77.307798] [<ffffffff820cdd60>] do_ipv6_setsockopt.isra.5+0xa70/0xf20 >> >> >> I think we should just use rtnl_lock() instead of rcu_read_lock() there, >> it is not a hot path worth optimization. >> >> Please try the attached patch. > > note: it doesn't build as it is now, it needs: > > -EXPORT_SYMBOL(dev_get_by_flags_rcu); > +EXPORT_SYMBOL(dev_get_by_flags); > > > I just tried your patch with a basic test program (open > socket/join/leave/close and open socket/join/close). > > I think you need to modify ipv6_sock_ac_close as well, or you can still > trigger the assertion when closing the socket without leaving first. You are absolutely right here. Can I have your Signed-off-by and Tested-by before sending the patch formally? Thanks! -- 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
Hi Sabrina, On Fr, 2014-08-29 at 21:53 +0200, Sabrina Dubroca wrote: > 2014-08-29, 11:14:48 -0700, Cong Wang wrote: > > On Fri, Aug 29, 2014 at 8:26 AM, Tommi Rantala <tt.rantala@gmail.com> wrote: > > > [ 77.297196] RTNL: assertion failed at net/ipv6/addrconf.c (1699) > > > [ 77.298080] CPU: 0 PID: 4842 Comm: trinity-main Not tainted 3.17.0-rc2+ #30 > > > [ 77.299039] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011 > > > [ 77.299789] ffff88003d76a618 ffff880026133c50 ffffffff8238ba79 > > > ffff880037c84520 > > > [ 77.300829] ffff880026133c90 ffffffff820bd52b 0000000000000000 > > > ffffffff82d86c40 > > > [ 77.301869] 0000000000000000 00000000f76fd1e1 ffff8800382d8000 > > > ffff8800382d8220 > > > [ 77.302906] Call Trace: > > > [ 77.303246] [<ffffffff8238ba79>] dump_stack+0x4d/0x66 > > > [ 77.303928] [<ffffffff820bd52b>] addrconf_join_solict+0x4b/0xb0 > > > [ 77.304731] [<ffffffff820b031b>] ipv6_dev_ac_inc+0x2bb/0x330 > > > [ 77.305498] [<ffffffff820b0060>] ? ac6_seq_start+0x260/0x260 > > > [ 77.306257] [<ffffffff820b05fe>] ipv6_sock_ac_join+0x26e/0x360 > > > [ 77.307046] [<ffffffff820b0429>] ? ipv6_sock_ac_join+0x99/0x360 > > > [ 77.307798] [<ffffffff820cdd60>] do_ipv6_setsockopt.isra.5+0xa70/0xf20 > > > > > > I think we should just use rtnl_lock() instead of rcu_read_lock() there, > > it is not a hot path worth optimization. > > > > Please try the attached patch. > > note: it doesn't build as it is now, it needs: > > -EXPORT_SYMBOL(dev_get_by_flags_rcu); > +EXPORT_SYMBOL(dev_get_by_flags); > > > I just tried your patch with a basic test program (open > socket/join/leave/close and open socket/join/close). > > I think you need to modify ipv6_sock_ac_close as well, or you can still > trigger the assertion when closing the socket without leaving first. > > Modified patch attached. Sorry, just had time to look at this. The reason is not to have list corruption but that the calls down to ndo_set_rx_mode expect rtnl to be locked by the drivers. Filter lists are locked by addr_list_lock and that's why I think we never saw any problems with that, but drivers expect rtnl locked for those calls. But this problem also affects multicast join, so patch seems incomplete to me (and for that matter ssm multicast join, too). Also rtnl_lock and rcu_read_lock compose in that order, so we don't need to change dev_get_by_flags, but as this is the only user it sure is possible. RCU locked version is just easier composeable, so I wouldn't touch that if needed in future, just also take rcu lock as before. So just adding rtnl_lock add appropriate places seems to be ok to me, but still need to review parts of the ssm code. Also we should move ASSERT_RTNL checks from addrconf_join_solict to ipv6_dev_mc_inc/dec. Thanks, Hannes -- 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
2014-08-29, 15:54:48 -0700, Cong Wang wrote: > [...] > > You are absolutely right here. > > Can I have your Signed-off-by and Tested-by before sending the patch > formally? > > Thanks! Sure: Signed-off-by: Sabrina Dubroca <sd@queasysnail.net> Tested-by: Sabrina Dubroca <sd@queasysnail.net> Thanks,
On Fri, Aug 29, 2014 at 6:51 PM, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote: > > Also rtnl_lock and rcu_read_lock compose in that order, so we don't need > to change dev_get_by_flags, but as this is the only user it sure is > possible. RCU locked version is just easier composeable, so I wouldn't > touch that if needed in future, just also take rcu lock as before. There is no point to keep RCU read lock if we have rtnl lock, I don't know why you don't want to change dev_get_by_flags(), it is pretty easy to do since it only has one caller. Even if you really need RCU in future, you are always welcome to bring it back when you do, sorry we should never be blocked by code NOT merged yet. > > Also we should move ASSERT_RTNL checks from addrconf_join_solict to > ipv6_dev_mc_inc/dec. > Make it another patch. -- 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
Hi Cong, On Tue, Sep 2, 2014, at 18:50, Cong Wang wrote: > On Fri, Aug 29, 2014 at 6:51 PM, Hannes Frederic Sowa > <hannes@stressinduktion.org> wrote: > > > > Also rtnl_lock and rcu_read_lock compose in that order, so we don't need > > to change dev_get_by_flags, but as this is the only user it sure is > > possible. RCU locked version is just easier composeable, so I wouldn't > > touch that if needed in future, just also take rcu lock as before. > > There is no point to keep RCU read lock if we have rtnl lock, > I don't know why you don't want to change dev_get_by_flags(), > it is pretty easy to do since it only has one caller. I definitely don't have a problem cleaning this up in net-next. I wanted a minimal patch for stable because I didn't check history where and when additional users of dev_get_by_flags_rcu were removed. > Even if you really need RCU in future, you are always welcome > to bring it back when you do, sorry we should never be blocked by > code NOT merged yet. > > > > > Also we should move ASSERT_RTNL checks from addrconf_join_solict to > > ipv6_dev_mc_inc/dec. > > > > Make it another patch. It is just one logical change, moving ASSERT_RTNLs to places where they better catch invalid callstacks. Bye, Hannes -- 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
On Tue, Sep 2, 2014 at 10:58 AM, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote: > Hi Cong, > > On Tue, Sep 2, 2014, at 18:50, Cong Wang wrote: >> On Fri, Aug 29, 2014 at 6:51 PM, Hannes Frederic Sowa >> <hannes@stressinduktion.org> wrote: >> > >> > Also rtnl_lock and rcu_read_lock compose in that order, so we don't need >> > to change dev_get_by_flags, but as this is the only user it sure is >> > possible. RCU locked version is just easier composeable, so I wouldn't >> > touch that if needed in future, just also take rcu lock as before. >> >> There is no point to keep RCU read lock if we have rtnl lock, >> I don't know why you don't want to change dev_get_by_flags(), >> it is pretty easy to do since it only has one caller. > > I definitely don't have a problem cleaning this up in net-next. I wanted > a minimal patch for stable because I didn't check history where and when > additional users of dev_get_by_flags_rcu were removed. `git grep` should show you we only have one caller. Apparently we don't care about any out-of-tree module. > >> Even if you really need RCU in future, you are always welcome >> to bring it back when you do, sorry we should never be blocked by >> code NOT merged yet. >> >> > >> > Also we should move ASSERT_RTNL checks from addrconf_join_solict to >> > ipv6_dev_mc_inc/dec. >> > >> >> Make it another patch. > > It is just one logical change, moving ASSERT_RTNLs to places where they > better catch invalid callstacks. > Conflicts with what you claimed above. :) -- 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
On Tue, 2014-09-02 at 11:04 -0700, Cong Wang wrote: > On Tue, Sep 2, 2014 at 10:58 AM, Hannes Frederic Sowa > > I definitely don't have a problem cleaning this up in net-next. I wanted > > a minimal patch for stable because I didn't check history where and when > > additional users of dev_get_by_flags_rcu were removed. > > `git grep` should show you we only have one caller. Apparently we don't > care about any out-of-tree module. Point is : you did not check if some stable versions had more callers. Its very nice you checked current version, but it is not enough for a stable candidate. -- 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
On Tue, Sep 2, 2014 at 11:11 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > On Tue, 2014-09-02 at 11:04 -0700, Cong Wang wrote: >> On Tue, Sep 2, 2014 at 10:58 AM, Hannes Frederic Sowa > >> > I definitely don't have a problem cleaning this up in net-next. I wanted >> > a minimal patch for stable because I didn't check history where and when >> > additional users of dev_get_by_flags_rcu were removed. >> >> `git grep` should show you we only have one caller. Apparently we don't >> care about any out-of-tree module. > > Point is : you did not check if some stable versions had more callers. > > Its very nice you checked current version, but it is not enough for a > stable candidate. That is what we do when backporting patches, I can do that if David asks me to backport it, but you know for netdev that is David's work. (I am not saying I don't want to help him, I just want to point out the fact. I am very pleased to help David for stable backports as long as he asks) -- 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
On Tue, Sep 2, 2014, at 20:04, Cong Wang wrote: > On Tue, Sep 2, 2014 at 10:58 AM, Hannes Frederic Sowa > <hannes@stressinduktion.org> wrote: > > Hi Cong, > > > > On Tue, Sep 2, 2014, at 18:50, Cong Wang wrote: > >> On Fri, Aug 29, 2014 at 6:51 PM, Hannes Frederic Sowa > >> <hannes@stressinduktion.org> wrote: > >> > > >> > Also rtnl_lock and rcu_read_lock compose in that order, so we don't need > >> > to change dev_get_by_flags, but as this is the only user it sure is > >> > possible. RCU locked version is just easier composeable, so I wouldn't > >> > touch that if needed in future, just also take rcu lock as before. > >> > >> There is no point to keep RCU read lock if we have rtnl lock, > >> I don't know why you don't want to change dev_get_by_flags(), > >> it is pretty easy to do since it only has one caller. > > > > I definitely don't have a problem cleaning this up in net-next. I wanted > > a minimal patch for stable because I didn't check history where and when > > additional users of dev_get_by_flags_rcu were removed. > > `git grep` should show you we only have one caller. Apparently we don't > care about any out-of-tree module. Sure, I don't care about out-of-tree modules either. I just wanted to make it easier to backport. Current patch is almost headache free to backport. > >> Even if you really need RCU in future, you are always welcome > >> to bring it back when you do, sorry we should never be blocked by > >> code NOT merged yet. > >> > >> > > >> > Also we should move ASSERT_RTNL checks from addrconf_join_solict to > >> > ipv6_dev_mc_inc/dec. > >> > > >> > >> Make it another patch. > > > > It is just one logical change, moving ASSERT_RTNLs to places where they > > better catch invalid callstacks. > > > > Conflicts with what you claimed above. :) Those ASSERT_RTNLs were misplaced and only caught the callers mostly from addrconf.c. I don't mind getting reports from stable kernel users and fixing those, too (or help fixing those). ASSERT_RTNL is not dangerous. We had a long history in not correctly using rtnl lock in ipv6/multicast code and those wrongfully placed ASSERT_RTNLs were my bad when I fixed the duplicate address detection handling. If enough multicast addresses are subscribed to an interface we might again get those splats because enabling promisc mode on an interface will also check for rtnl lock. Bye, Hannes -- 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
On Tue, 2014-09-02 at 11:15 -0700, Cong Wang wrote: > That is what we do when backporting patches, I can do that if David asks > me to backport it, but you know for netdev that is David's work. > > (I am not saying I don't want to help him, I just want to point out the fact. > I am very pleased to help David for stable backports as long as he asks) Problem is : your patch submission do not identify bug origin. You claim you want to help, but you do not provide the basic thing that _really_ helps. The proper way to identify bug origin is to add in the headers one line : Fixes: 12-digit-SHA1 ("patch title") -- 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
On Tue, Sep 2, 2014 at 11:21 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > On Tue, 2014-09-02 at 11:15 -0700, Cong Wang wrote: > >> That is what we do when backporting patches, I can do that if David asks >> me to backport it, but you know for netdev that is David's work. >> >> (I am not saying I don't want to help him, I just want to point out the fact. >> I am very pleased to help David for stable backports as long as he asks) > > Problem is : your patch submission do not identify bug origin. > > You claim you want to help, but you do not provide the basic thing that > _really_ helps. > > The proper way to identify bug origin is to add in the headers one > line : > > Fixes: 12-digit-SHA1 ("patch title") > Since when "Fixes:" tag becomes mandatory for a stable patch? At least netdev-FAQ is not updated. ;-) I 100% agree "Fixes:" is helpful when backporting patches, but it is not mandatory currently. For this patch, I was too lazy to dig the history, it looks like this is caused by the following commit: commit c15b1ccadb323ea50023e8f1cca2954129a62b51 Author: Hannes Frederic Sowa <hannes@stressinduktion.org> Date: Thu Mar 27 18:28:07 2014 +0100 ipv6: move DAD and addrconf_verify processing to workqueue Thanks. -- 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
On Tue, Sep 2, 2014 at 11:18 AM, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote: > Those ASSERT_RTNLs were misplaced and only caught the callers mostly > from addrconf.c. I don't mind getting reports from stable kernel users > and fixing those, too (or help fixing those). ASSERT_RTNL is not > dangerous. > > We had a long history in not correctly using rtnl lock in ipv6/multicast > code and those wrongfully placed ASSERT_RTNLs were my bad when I fixed > the duplicate address detection handling. > > If enough multicast addresses are subscribed to an interface we might > again get those splats because enabling promisc mode on an interface > will also check for rtnl lock. > Sure, I never doubt adding ASSERT_RTNL() is helpful, I just still think this should be for net-next, or at least a separated patch. I don't want my patch to be blamed in others' "Fixes:". :) -- 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
On Di, 2014-09-02 at 11:40 -0700, Cong Wang wrote: > On Tue, Sep 2, 2014 at 11:18 AM, Hannes Frederic Sowa > <hannes@stressinduktion.org> wrote: > > Those ASSERT_RTNLs were misplaced and only caught the callers mostly > > from addrconf.c. I don't mind getting reports from stable kernel users > > and fixing those, too (or help fixing those). ASSERT_RTNL is not > > dangerous. > > > > We had a long history in not correctly using rtnl lock in ipv6/multicast > > code and those wrongfully placed ASSERT_RTNLs were my bad when I fixed > > the duplicate address detection handling. > > > > If enough multicast addresses are subscribed to an interface we might > > again get those splats because enabling promisc mode on an interface > > will also check for rtnl lock. > > > > Sure, I never doubt adding ASSERT_RTNL() is helpful, I just still think > this should be for net-next, or at least a separated patch. I don't want > my patch to be blamed in others' "Fixes:". :) Come on, that's why we have community review. Nobody blames anyone because of added regressions. It's more a fault of the community then, and it works out fairly good I think! Even others are keen on fixing your bugs sometimes. ;) If fixes tag is well researched, it won't point to the addition of ASSERT_RTNL() but your patch would help to discover a bug somewhere else in the stack. I think for this patch a fixes-tag is hard to find because it is hard to find because it dates back to the beginning of the git history IMHO. Bye, Hannes -- 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
On 09/02/2014 02:15 PM, Cong Wang wrote: > On Tue, Sep 2, 2014 at 11:11 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote: >> On Tue, 2014-09-02 at 11:04 -0700, Cong Wang wrote: >>> On Tue, Sep 2, 2014 at 10:58 AM, Hannes Frederic Sowa >> >>>> I definitely don't have a problem cleaning this up in net-next. I wanted >>>> a minimal patch for stable because I didn't check history where and when >>>> additional users of dev_get_by_flags_rcu were removed. >>> >>> `git grep` should show you we only have one caller. Apparently we don't >>> care about any out-of-tree module. >> >> Point is : you did not check if some stable versions had more callers. >> >> Its very nice you checked current version, but it is not enough for a >> stable candidate. > > That is what we do when backporting patches, I can do that if David asks > me to backport it, but you know for netdev that is David's work. > > (I am not saying I don't want to help him, I just want to point out the fact. > I am very pleased to help David for stable backports as long as he asks) Instead of helping after the fact, why not arrange the patches so that it's not such a big issue. Leave the _rcu variant alone. Add an _rtnl variant of the function and use that in the patch. Have a follow-on patch that removes the _rcu variant all by itself. This way backports become easier, and if anyone wants the _rcu variant back, all they have to do is revert a very simple commit. -vlad -- 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
On Tue, Sep 2, 2014 at 12:02 PM, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote: > > If fixes tag is well researched, it won't point to the addition of > ASSERT_RTNL() but your patch would help to discover a bug somewhere else > in the stack. > > I think for this patch a fixes-tag is hard to find because it is hard to > find because it dates back to the beginning of the git history IMHO. > As I replied to Eric, this warning is probably caused by the following commit: commit c15b1ccadb323ea50023e8f1cca2954129a62b51 Author: Hannes Frederic Sowa <hannes@stressinduktion.org> Date: Thu Mar 27 18:28:07 2014 +0100 ipv6: move DAD and addrconf_verify processing to workqueue HOWEVER, you can definitely argue that the code without your ASSERT_RTNL() was already broken, it's a little hard to tell without digging more, that might date back to the beginning of git as you said. For safety, I think we can simply assume it's that commit to be fixed so that we don't fix older kernels until someone really reports a bug. -- 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
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 429801370d0c..1ae0e745b1b1 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -2077,8 +2077,8 @@ void __dev_remove_pack(struct packet_type *pt); void dev_add_offload(struct packet_offload *po); void dev_remove_offload(struct packet_offload *po); -struct net_device *dev_get_by_flags_rcu(struct net *net, unsigned short flags, - unsigned short mask); +struct net_device *dev_get_by_flags(struct net *net, unsigned short flags, + unsigned short mask); struct net_device *dev_get_by_name(struct net *net, const char *name); struct net_device *dev_get_by_name_rcu(struct net *net, const char *name); struct net_device *__dev_get_by_name(struct net *net, const char *name); diff --git a/net/core/dev.c b/net/core/dev.c index 443b814db05b..8fede6ef4a39 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -897,23 +897,24 @@ struct net_device *dev_getfirstbyhwtype(struct net *net, unsigned short type) EXPORT_SYMBOL(dev_getfirstbyhwtype); /** - * dev_get_by_flags_rcu - find any device with given flags + * dev_get_by_flags - find any device with given flags * @net: the applicable net namespace * @if_flags: IFF_* values * @mask: bitmask of bits in if_flags to check * * Search for any interface with the given flags. Returns NULL if a device * is not found or a pointer to the device. Must be called inside - * rcu_read_lock(), and result refcount is unchanged. + * rtnl_lock(), and result refcount is unchanged. */ -struct net_device *dev_get_by_flags_rcu(struct net *net, unsigned short if_flags, +struct net_device *dev_get_by_flags(struct net *net, unsigned short if_flags, unsigned short mask) { struct net_device *dev, *ret; + ASSERT_RTNL(); ret = NULL; - for_each_netdev_rcu(net, dev) { + for_each_netdev(net, dev) { if (((dev->flags ^ if_flags) & mask) == 0) { ret = dev; break; @@ -921,7 +922,7 @@ struct net_device *dev_get_by_flags_rcu(struct net *net, unsigned short if_flags } return ret; } -EXPORT_SYMBOL(dev_get_by_flags_rcu); +EXPORT_SYMBOL(dev_get_by_flags); /** * dev_valid_name - check if name is okay for network device diff --git a/net/ipv6/anycast.c b/net/ipv6/anycast.c index 210183244689..6de5caa26ea4 100644 --- a/net/ipv6/anycast.c +++ b/net/ipv6/anycast.c @@ -77,7 +77,7 @@ int ipv6_sock_ac_join(struct sock *sk, int ifindex, const struct in6_addr *addr) pac->acl_next = NULL; pac->acl_addr = *addr; - rcu_read_lock(); + rtnl_lock(); if (ifindex == 0) { struct rt6_info *rt; @@ -90,11 +90,11 @@ int ipv6_sock_ac_join(struct sock *sk, int ifindex, const struct in6_addr *addr) goto error; } else { /* router, no matching interface: just pick one */ - dev = dev_get_by_flags_rcu(net, IFF_UP, + dev = dev_get_by_flags(net, IFF_UP, IFF_UP | IFF_LOOPBACK); } } else - dev = dev_get_by_index_rcu(net, ifindex); + dev = __dev_get_by_index(net, ifindex); if (dev == NULL) { err = -ENODEV; @@ -136,7 +136,7 @@ int ipv6_sock_ac_join(struct sock *sk, int ifindex, const struct in6_addr *addr) } error: - rcu_read_unlock(); + rtnl_unlock(); if (pac) sock_kfree_s(sk, pac, sizeof(*pac)); return err; @@ -171,13 +171,15 @@ int ipv6_sock_ac_drop(struct sock *sk, int ifindex, const struct in6_addr *addr) spin_unlock_bh(&ipv6_sk_ac_lock); - rcu_read_lock(); - dev = dev_get_by_index_rcu(net, pac->acl_ifindex); + rtnl_lock(); + dev = __dev_get_by_index(net, pac->acl_ifindex); if (dev) ipv6_dev_ac_dec(dev, &pac->acl_addr); - rcu_read_unlock(); + rtnl_unlock(); sock_kfree_s(sk, pac, sizeof(*pac)); + if (!dev) + return -ENODEV; return 0; } @@ -198,12 +200,12 @@ void ipv6_sock_ac_close(struct sock *sk) spin_unlock_bh(&ipv6_sk_ac_lock); prev_index = 0; - rcu_read_lock(); + rtnl_lock(); while (pac) { struct ipv6_ac_socklist *next = pac->acl_next; if (pac->acl_ifindex != prev_index) { - dev = dev_get_by_index_rcu(net, pac->acl_ifindex); + dev = __dev_get_by_index(net, pac->acl_ifindex); prev_index = pac->acl_ifindex; } if (dev) @@ -211,7 +213,7 @@ void ipv6_sock_ac_close(struct sock *sk) sock_kfree_s(sk, pac, sizeof(*pac)); pac = next; } - rcu_read_unlock(); + rtnl_unlock(); } static void aca_put(struct ifacaddr6 *ac)