From: Mahesh Bandewar <mahe...@google.com> The patch 009146d117b ("ipvlan: assign unique dev-id for each slave device.") used ida_simple_get() to generate dev_ids assigned to the slave devices. However (Eric has pointed out that) there is a shortcoming with that approach as it always uses the first available ID. This becomes a problem when a slave gets deleted and a new slave gets added. The ID gets reassigned causing the new slave to get the same link-local address. This side-effect is undesirable.
This patch replaces IDR logic with a simple per-port variable that keeps incrementing and wraps around when the MAX (0xFFFE) is reached. The only downside is that this is an inefficient (n^2) search if there are 64k (or close to 64k) slaves in the system, the dev-id search takes time. However having these many devices in the system has it's own challenges. Fixes: 009146d117b ("ipvlan: assign unique dev-id for each slave device.") Signed-off-by: Mahesh Bandewar <mahe...@google.com> CC: Eric Dumazet <eduma...@google.com> --- drivers/net/ipvlan/ipvlan.h | 2 +- drivers/net/ipvlan/ipvlan_main.c | 69 ++++++++++++++++++++++++++++++++++------ 2 files changed, 61 insertions(+), 10 deletions(-) diff --git a/drivers/net/ipvlan/ipvlan.h b/drivers/net/ipvlan/ipvlan.h index 0a9068fdee0f..535f254324ba 100644 --- a/drivers/net/ipvlan/ipvlan.h +++ b/drivers/net/ipvlan/ipvlan.h @@ -94,10 +94,10 @@ struct ipvl_port { struct hlist_head hlhead[IPVLAN_HASH_SIZE]; struct list_head ipvlans; u16 mode; + u16 dev_id_base; struct work_struct wq; struct sk_buff_head backlog; int count; - struct ida ida; }; struct ipvl_skb_cb { diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c index ce7ca6a5aa8a..691662e02abb 100644 --- a/drivers/net/ipvlan/ipvlan_main.c +++ b/drivers/net/ipvlan/ipvlan_main.c @@ -119,7 +119,7 @@ static int ipvlan_port_create(struct net_device *dev) skb_queue_head_init(&port->backlog); INIT_WORK(&port->wq, ipvlan_process_multicast); - ida_init(&port->ida); + port->dev_id_base = 1; err = netdev_rx_handler_register(dev, ipvlan_handle_frame, port); if (err) @@ -151,7 +151,6 @@ static void ipvlan_port_destroy(struct net_device *dev) dev_put(skb->dev); kfree_skb(skb); } - ida_destroy(&port->ida); kfree(port); } @@ -496,6 +495,60 @@ static int ipvlan_nl_fillinfo(struct sk_buff *skb, return ret; } +static u16 ipvlan_get_dev_id(struct ipvl_port *port) +{ + struct ipvl_dev *ipvlan; + u16 tid, dev_id = 0; + bool found, exhausted = false, second_round = false; + + /* Idea here is to get the next available ID while avoiding + * reuse of already used IDs. Per port variable (dev_id_base) + * is used to get the next available ID. + * + * Possibilities are from 0x1 to 0xfffe so finding next available + * ID is not be that difficult. However if the system is up for a + * long time and these slave devices are getting added / deleted + * routinely and it reaches the MAX, then only the assignment will + * attempt to recycle previously used IDs starting from 0x1 (Provided + * these are not currently in use). + * + * In case of a failure, dev-id search will return 0. All IDs in-use + * case is very inefficient (n^2 search) but having close to 64k + * slaves has it's own system limitations. + */ + while (!exhausted) { + tid = port->dev_id_base++; + + if (tid == 0xFFFE) { + /* Reached MAX; restart from 1 */ + port->dev_id_base = 1; + if (second_round) + exhausted = true; + else + second_round = true; + continue; + } + /* Should not be same as the masters' (if present) */ + if (tid == port->dev->dev_id) + continue; + /* Make sure that it's already not assigned to any of the + * existing slaves. + */ + found = false; + list_for_each_entry(ipvlan, &port->ipvlans, pnode) { + if (ipvlan->dev->dev_id == tid) { + found = true; + break; + } + } + if (!found) { + dev_id = tid; + break; + } + } + return dev_id; +} + static int ipvlan_link_new(struct net *src_net, struct net_device *dev, struct nlattr *tb[], struct nlattr *data[]) { @@ -540,10 +593,11 @@ static int ipvlan_link_new(struct net *src_net, struct net_device *dev, * Assign IDs between 0x1 and 0xFFFE (used by the master) to each * slave link [see addrconf_ifid_eui48()]. */ - err = ida_simple_get(&port->ida, 1, 0xFFFE, GFP_KERNEL); - if (err < 0) + dev->dev_id = ipvlan_get_dev_id(port); + if (dev->dev_id == 0) { + err = -ENOSPC; goto destroy_ipvlan_port; - dev->dev_id = err; + } /* TODO Probably put random address here to be presented to the * world but keep using the physical-dev address for the outgoing @@ -555,7 +609,7 @@ static int ipvlan_link_new(struct net *src_net, struct net_device *dev, err = register_netdevice(dev); if (err < 0) - goto remove_ida; + goto destroy_ipvlan_port; err = netdev_upper_dev_link(phy_dev, dev); if (err) { @@ -574,8 +628,6 @@ static int ipvlan_link_new(struct net *src_net, struct net_device *dev, netdev_upper_dev_unlink(phy_dev, dev); unregister_netdev: unregister_netdevice(dev); -remove_ida: - ida_simple_remove(&port->ida, dev->dev_id); destroy_ipvlan_port: if (create) ipvlan_port_destroy(phy_dev); @@ -593,7 +645,6 @@ static void ipvlan_link_delete(struct net_device *dev, struct list_head *head) kfree_rcu(addr, rcu); } - ida_simple_remove(&ipvlan->port->ida, dev->dev_id); list_del_rcu(&ipvlan->pnode); unregister_netdevice_queue(dev, head); netdev_upper_dev_unlink(ipvlan->phy_dev, dev); -- 2.11.0.390.gc69c2f50cf-goog