Skip to content

Commit

Permalink
mlxsw: spectrum_router: Make RIF deletion more robust
Browse files Browse the repository at this point in the history 
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 <idosch@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
@idosch @davem330
idosch authored and davem330 committed Dec 19, 2018
1 parent 21ffedb commit 965fa8e
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 36 deletions.
14 changes: 0 additions & 14 deletions drivers/net/ethernet/mellanox/mlxsw/spectrum.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, },
Expand All @@ -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)
Expand All @@ -5480,9 +5470,7 @@ static int __init mlxsw_sp_module_init(void)
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;
}
Expand All @@ -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);
}

Expand Down
4 changes: 0 additions & 4 deletions drivers/net/ethernet/mellanox/mlxsw/spectrum.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
49 changes: 31 additions & 18 deletions drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand Down Expand Up @@ -6778,28 +6780,25 @@ 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;

/* NETDEV_UP event is handled by mlxsw_sp_inetaddr_valid_event */
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);
}
Expand Down Expand Up @@ -6833,6 +6832,7 @@ int mlxsw_sp_inetaddr_valid_event(struct notifier_block *unused,

struct mlxsw_sp_inet6addr_event_work {
struct work_struct work;
struct mlxsw_sp *mlxsw_sp;
struct net_device *dev;
unsigned long event;
};
Expand All @@ -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))
Expand All @@ -6863,25 +6860,25 @@ static void mlxsw_sp_inet6addr_event_work(struct work_struct *work)
}

/* 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);
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -7742,6 +7749,10 @@ int mlxsw_sp_router_init(struct mlxsw_sp *mlxsw_sp)
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;
}
Expand All @@ -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);
}

0 comments on commit 965fa8e

Please sign in to comment.