[PATCH net] ipvs: call ip_vs_dst_notifier before ipv6_dev_notf

[PATCH net] ipvs: call ip_vs_dst_notifier before ipv6_dev_notf

[Xin Long]

ip_vs_dst_event is supposed to clean up all dst used in ipvs'
destinations when a net dev is going down. But it works only
when the dst's dev is the same as the dev from the event.

Now with the same priority but late registration,
ip_vs_dst_notifier is always called after ipv6_dev_notf where
the dst's dev is set to lo for NETDEV_DOWN event.

As the dst's dev lo is not the same as the dev from the event
in ip_vs_dst_event(), ipv6_dev_notf can actually never work.
Also as these dst have to wait for dest_trash_timer to clean
them up. It would cause some non-permanent kernel warnings:

  unregister_netdevice: waiting for br0 to become free. Usage count = 3

To fix it, call ip_vs_dst_notifier before ipv6_dev_notf by
increasing its priority to ADDRCONF_NOTIFY_PRIORITY + 5.

Fixes: 7a4f0761fce3 ("IPVS: init and cleanup restructuring")
Reported-by: Li Shuang <shuali@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/netfilter/ipvs/ip_vs_ctl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 83395bf6..aded477 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -3980,6 +3980,7 @@ static void __net_exit ip_vs_control_net_cleanup_sysctl(struct netns_ipvs *ipvs)
 
 static struct notifier_block ip_vs_dst_notifier = {
 	.notifier_call = ip_vs_dst_event,
+	.priority = ADDRCONF_NOTIFY_PRIORITY + 5,
 };
 
 int __net_init ip_vs_control_net_init(struct netns_ipvs *ipvs)
-- 
2.1.0

Re: [PATCH net] ipvs: call ip_vs_dst_notifier before ipv6_dev_notf

[Julian Anastasov]

	Hello,

On Wed, 14 Nov 2018, Xin Long wrote:

> ip_vs_dst_event is supposed to clean up all dst used in ipvs'
> destinations when a net dev is going down. But it works only
> when the dst's dev is the same as the dev from the event.
> 
> Now with the same priority but late registration,
> ip_vs_dst_notifier is always called after ipv6_dev_notf where
> the dst's dev is set to lo for NETDEV_DOWN event.
> 
> As the dst's dev lo is not the same as the dev from the event
> in ip_vs_dst_event(), ipv6_dev_notf can actually never work.
> Also as these dst have to wait for dest_trash_timer to clean
> them up. It would cause some non-permanent kernel warnings:
> 
>   unregister_netdevice: waiting for br0 to become free. Usage count = 3
> 
> To fix it, call ip_vs_dst_notifier before ipv6_dev_notf by
> increasing its priority to ADDRCONF_NOTIFY_PRIORITY + 5.
> 
> Fixes: 7a4f0761fce3 ("IPVS: init and cleanup restructuring")
> Reported-by: Li Shuang <shuali@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

	OK,

Acked-by: Julian Anastasov <ja@ssi.bg>

	But this fix may hide another problem...

	Isn't that racy to change dst.dev during NETDEV_DOWN?
IPv4 calls rt_flush_dev() only on NETDEV_UNREGISTER, while
rt6_uncached_list_flush_dev() is called in rt6_disable_ip(),
even on NETDEV_DOWN. How fatal is to call it only on
NETDEV_UNREGISTER? By this way, we rely on synchronize_net()
before the NETDEV_UNREGISTER event to avoid any dst.dev access.

> ---
>  net/netfilter/ipvs/ip_vs_ctl.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index 83395bf6..aded477 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -3980,6 +3980,7 @@ static void __net_exit ip_vs_control_net_cleanup_sysctl(struct netns_ipvs *ipvs)
>  
>  static struct notifier_block ip_vs_dst_notifier = {
>  	.notifier_call = ip_vs_dst_event,
> +	.priority = ADDRCONF_NOTIFY_PRIORITY + 5,
>  };
>  
>  int __net_init ip_vs_control_net_init(struct netns_ipvs *ipvs)
> -- 
> 2.1.0

Regards

Re: [PATCH net] ipvs: call ip_vs_dst_notifier before ipv6_dev_notf

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1113 bytes --]

Hi Xin,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net/master]

url:    https://github.com/0day-ci/linux/commits/Xin-Long/ipvs-call-ip_vs_dst_notifier-before-ipv6_dev_notf/20181114-055727
config: i386-randconfig-x005-11131818 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

>> net/netfilter/ipvs/ip_vs_ctl.c:3983:14: error: 'ADDRCONF_NOTIFY_PRIORITY' undeclared here (not in a function); did you mean 'DEVCONF_NDISC_NOTIFY'?
     .priority = ADDRCONF_NOTIFY_PRIORITY + 5,
                 ^~~~~~~~~~~~~~~~~~~~~~~~
                 DEVCONF_NDISC_NOTIFY

vim +3983 net/netfilter/ipvs/ip_vs_ctl.c

  3980	
  3981	static struct notifier_block ip_vs_dst_notifier = {
  3982		.notifier_call = ip_vs_dst_event,
> 3983		.priority = ADDRCONF_NOTIFY_PRIORITY + 5,
  3984	};
  3985	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33999 bytes --]

Re: [PATCH net] ipvs: call ip_vs_dst_notifier before ipv6_dev_notf

[Xin Long]

On Wed, Nov 14, 2018 at 1:38 PM kbuild test robot <lkp@intel.com> wrote:
>
> Hi Xin,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on net/master]
>
> url:    https://github.com/0day-ci/linux/commits/Xin-Long/ipvs-call-ip_vs_dst_notifier-before-ipv6_dev_notf/20181114-055727
> config: i386-randconfig-x005-11131818 (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386
>
> All errors (new ones prefixed by >>):
>
> >> net/netfilter/ipvs/ip_vs_ctl.c:3983:14: error: 'ADDRCONF_NOTIFY_PRIORITY' undeclared here (not in a function); did you mean 'DEVCONF_NDISC_NOTIFY'?
>      .priority = ADDRCONF_NOTIFY_PRIORITY + 5,
>                  ^~~~~~~~~~~~~~~~~~~~~~~~
>                  DEVCONF_NDISC_NOTIFY
>
> vim +3983 net/netfilter/ipvs/ip_vs_ctl.c
>
>   3980
>   3981  static struct notifier_block ip_vs_dst_notifier = {
>   3982          .notifier_call = ip_vs_dst_event,
> > 3983          .priority = ADDRCONF_NOTIFY_PRIORITY + 5,
>   3984  };
>   3985
This issue only exist on ipv6, I will fix it by simply using
#ifdef CONFIG_IP_VS_IPV6

>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation