From 965fa8e600d28d98e7e0c7340517a9c139843ee3 Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Wed, 19 Dec 2018 06:08:48 +0000 Subject: [PATCH] mlxsw: spectrum_router: Make RIF deletion more robust In the past we had multiple instances where RIFs were not properly deleted. One of the reasons for leaking a RIF was that at the time when IP addresses were flushed from the respective netdev (prompting the destruction of the RIF), the netdev was no longer a mlxsw upper. This caused the inet{,6}addr notification blocks to ignore the NETDEV_DOWN event and leak the RIF. Instead of checking whether the netdev is our upper when an IP address is removed, we can instead check if the netdev has a RIF configured. To look up a RIF we need to access mlxsw private data, so the patch stores the notification blocks inside a mlxsw struct. This then allows us to use container_of() and extract the required private data. Signed-off-by: Ido Schimmel Reviewed-by: Petr Machata Signed-off-by: David S. Miller --- .../net/ethernet/mellanox/mlxsw/spectrum.c | 14 ------ .../net/ethernet/mellanox/mlxsw/spectrum.h | 4 -- .../ethernet/mellanox/mlxsw/spectrum_router.c | 49 ++++++++++++------- 3 files changed, 31 insertions(+), 36 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c index d0235aa2aebc..4197b29a9bda 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c @@ -5414,18 +5414,10 @@ static struct notifier_block mlxsw_sp_inetaddr_valid_nb __read_mostly = { .notifier_call = mlxsw_sp_inetaddr_valid_event, }; -static struct notifier_block mlxsw_sp_inetaddr_nb __read_mostly = { - .notifier_call = mlxsw_sp_inetaddr_event, -}; - static struct notifier_block mlxsw_sp_inet6addr_valid_nb __read_mostly = { .notifier_call = mlxsw_sp_inet6addr_valid_event, }; -static struct notifier_block mlxsw_sp_inet6addr_nb __read_mostly = { - .notifier_call = mlxsw_sp_inet6addr_event, -}; - static const struct pci_device_id mlxsw_sp1_pci_id_table[] = { {PCI_VDEVICE(MELLANOX, PCI_DEVICE_ID_MELLANOX_SPECTRUM), 0}, {0, }, @@ -5451,9 +5443,7 @@ static int __init mlxsw_sp_module_init(void) int err; register_inetaddr_validator_notifier(&mlxsw_sp_inetaddr_valid_nb); - register_inetaddr_notifier(&mlxsw_sp_inetaddr_nb); register_inet6addr_validator_notifier(&mlxsw_sp_inet6addr_valid_nb); - register_inet6addr_notifier(&mlxsw_sp_inet6addr_nb); err = mlxsw_core_driver_register(&mlxsw_sp1_driver); if (err) @@ -5480,9 +5470,7 @@ err_sp1_pci_driver_register: err_sp2_core_driver_register: mlxsw_core_driver_unregister(&mlxsw_sp1_driver); err_sp1_core_driver_register: - unregister_inet6addr_notifier(&mlxsw_sp_inet6addr_nb); unregister_inet6addr_validator_notifier(&mlxsw_sp_inet6addr_valid_nb); - unregister_inetaddr_notifier(&mlxsw_sp_inetaddr_nb); unregister_inetaddr_validator_notifier(&mlxsw_sp_inetaddr_valid_nb); return err; } @@ -5493,9 +5481,7 @@ static void __exit mlxsw_sp_module_exit(void) mlxsw_pci_driver_unregister(&mlxsw_sp1_pci_driver); mlxsw_core_driver_unregister(&mlxsw_sp2_driver); mlxsw_core_driver_unregister(&mlxsw_sp1_driver); - unregister_inet6addr_notifier(&mlxsw_sp_inet6addr_nb); unregister_inet6addr_validator_notifier(&mlxsw_sp_inet6addr_valid_nb); - unregister_inetaddr_notifier(&mlxsw_sp_inetaddr_nb); unregister_inetaddr_validator_notifier(&mlxsw_sp_inetaddr_valid_nb); } diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h index cb1d37df87ea..f022e9e24085 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h @@ -458,12 +458,8 @@ int mlxsw_sp_netdevice_router_port_event(struct net_device *dev, unsigned long event, void *ptr); void mlxsw_sp_rif_macvlan_del(struct mlxsw_sp *mlxsw_sp, const struct net_device *macvlan_dev); -int mlxsw_sp_inetaddr_event(struct notifier_block *unused, - unsigned long event, void *ptr); int mlxsw_sp_inetaddr_valid_event(struct notifier_block *unused, unsigned long event, void *ptr); -int mlxsw_sp_inet6addr_event(struct notifier_block *unused, - unsigned long event, void *ptr); int mlxsw_sp_inet6addr_valid_event(struct notifier_block *unused, unsigned long event, void *ptr); int mlxsw_sp_netdevice_vrf_event(struct net_device *l3_dev, unsigned long event, diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c index a80e15b98937..08b60d4b9492 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c @@ -71,6 +71,8 @@ struct mlxsw_sp_router { bool aborted; struct notifier_block fib_nb; struct notifier_block netevent_nb; + struct notifier_block inetaddr_nb; + struct notifier_block inet6addr_nb; const struct mlxsw_sp_rif_ops **rif_ops_arr; const struct mlxsw_sp_ipip_ops **ipip_ops_arr; }; @@ -6778,12 +6780,12 @@ static int __mlxsw_sp_inetaddr_event(struct mlxsw_sp *mlxsw_sp, return 0; } -int mlxsw_sp_inetaddr_event(struct notifier_block *unused, - unsigned long event, void *ptr) +static int mlxsw_sp_inetaddr_event(struct notifier_block *nb, + unsigned long event, void *ptr) { struct in_ifaddr *ifa = (struct in_ifaddr *) ptr; struct net_device *dev = ifa->ifa_dev->dev; - struct mlxsw_sp *mlxsw_sp; + struct mlxsw_sp_router *router; struct mlxsw_sp_rif *rif; int err = 0; @@ -6791,15 +6793,12 @@ int mlxsw_sp_inetaddr_event(struct notifier_block *unused, if (event == NETDEV_UP) goto out; - mlxsw_sp = mlxsw_sp_lower_get(dev); - if (!mlxsw_sp) - goto out; - - rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev); + router = container_of(nb, struct mlxsw_sp_router, inetaddr_nb); + rif = mlxsw_sp_rif_find_by_dev(router->mlxsw_sp, dev); if (!mlxsw_sp_rif_should_config(rif, dev, event)) goto out; - err = __mlxsw_sp_inetaddr_event(mlxsw_sp, dev, event, NULL); + err = __mlxsw_sp_inetaddr_event(router->mlxsw_sp, dev, event, NULL); out: return notifier_from_errno(err); } @@ -6833,6 +6832,7 @@ out: struct mlxsw_sp_inet6addr_event_work { struct work_struct work; + struct mlxsw_sp *mlxsw_sp; struct net_device *dev; unsigned long event; }; @@ -6841,15 +6841,12 @@ static void mlxsw_sp_inet6addr_event_work(struct work_struct *work) { struct mlxsw_sp_inet6addr_event_work *inet6addr_work = container_of(work, struct mlxsw_sp_inet6addr_event_work, work); + struct mlxsw_sp *mlxsw_sp = inet6addr_work->mlxsw_sp; struct net_device *dev = inet6addr_work->dev; unsigned long event = inet6addr_work->event; - struct mlxsw_sp *mlxsw_sp; struct mlxsw_sp_rif *rif; rtnl_lock(); - mlxsw_sp = mlxsw_sp_lower_get(dev); - if (!mlxsw_sp) - goto out; rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev); if (!mlxsw_sp_rif_should_config(rif, dev, event)) @@ -6863,25 +6860,25 @@ out: } /* Called with rcu_read_lock() */ -int mlxsw_sp_inet6addr_event(struct notifier_block *unused, - unsigned long event, void *ptr) +static int mlxsw_sp_inet6addr_event(struct notifier_block *nb, + unsigned long event, void *ptr) { struct inet6_ifaddr *if6 = (struct inet6_ifaddr *) ptr; struct mlxsw_sp_inet6addr_event_work *inet6addr_work; struct net_device *dev = if6->idev->dev; + struct mlxsw_sp_router *router; /* NETDEV_UP event is handled by mlxsw_sp_inet6addr_valid_event */ if (event == NETDEV_UP) return NOTIFY_DONE; - if (!mlxsw_sp_port_dev_lower_find_rcu(dev)) - return NOTIFY_DONE; - inet6addr_work = kzalloc(sizeof(*inet6addr_work), GFP_ATOMIC); if (!inet6addr_work) return NOTIFY_BAD; + router = container_of(nb, struct mlxsw_sp_router, inet6addr_nb); INIT_WORK(&inet6addr_work->work, mlxsw_sp_inet6addr_event_work); + inet6addr_work->mlxsw_sp = router->mlxsw_sp; inet6addr_work->dev = dev; inet6addr_work->event = event; dev_hold(dev); @@ -7657,6 +7654,16 @@ int mlxsw_sp_router_init(struct mlxsw_sp *mlxsw_sp) mlxsw_sp->router = router; router->mlxsw_sp = mlxsw_sp; + router->inetaddr_nb.notifier_call = mlxsw_sp_inetaddr_event; + err = register_inetaddr_notifier(&router->inetaddr_nb); + if (err) + goto err_register_inetaddr_notifier; + + router->inet6addr_nb.notifier_call = mlxsw_sp_inet6addr_event; + err = register_inet6addr_notifier(&router->inet6addr_nb); + if (err) + goto err_register_inet6addr_notifier; + INIT_LIST_HEAD(&mlxsw_sp->router->nexthop_neighs_list); err = __mlxsw_sp_router_init(mlxsw_sp); if (err) @@ -7742,6 +7749,10 @@ err_ipips_init: err_rifs_init: __mlxsw_sp_router_fini(mlxsw_sp); err_router_init: + unregister_inet6addr_notifier(&router->inet6addr_nb); +err_register_inet6addr_notifier: + unregister_inetaddr_notifier(&router->inetaddr_nb); +err_register_inetaddr_notifier: kfree(mlxsw_sp->router); return err; } @@ -7759,5 +7770,7 @@ void mlxsw_sp_router_fini(struct mlxsw_sp *mlxsw_sp) mlxsw_sp_ipips_fini(mlxsw_sp); mlxsw_sp_rifs_fini(mlxsw_sp); __mlxsw_sp_router_fini(mlxsw_sp); + unregister_inet6addr_notifier(&mlxsw_sp->router->inet6addr_nb); + unregister_inetaddr_notifier(&mlxsw_sp->router->inetaddr_nb); kfree(mlxsw_sp->router); } -- 2.30.2