nfp: protect each repr pointer individually with RCU

Representors are grouped in sets by type.  Currently the whole
sets are under RCU protection, but individual representor pointers
are not.  This causes some inconveniences when representors have
to be destroyed, because we have to allocate new sets to remove
any representors.  Protect the individual pointers with RCU.
Signed-off-by: NJakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: NDirk van der Merwe <dirk.vandermerwe@netronome.com>
Signed-off-by: NDavid S. Miller <davem@davemloft.net>

drivers/net/ethernet/netronome/nfp/flower/main.c

@@ -99,7 +99,7 @@ nfp_flower_repr_get(struct nfp_app *app, u32 port_id)
 	if (port >= reprs->num_reprs)
 		return NULL;
 
-	return reprs->reprs[port];
+	return rcu_dereference(reprs->reprs[port]);
 }
 
 static int
@@ -114,15 +114,19 @@ nfp_flower_reprs_reify(struct nfp_app *app, enum nfp_repr_type type,
 	if (!reprs)
 		return 0;
 
-	for (i = 0; i < reprs->num_reprs; i++)
-		if (reprs->reprs[i]) {
-			struct nfp_repr *repr = netdev_priv(reprs->reprs[i]);
+	for (i = 0; i < reprs->num_reprs; i++) {
+		struct net_device *netdev;
+
+		netdev = nfp_repr_get_locked(app, reprs, i);
+		if (netdev) {
+			struct nfp_repr *repr = netdev_priv(netdev);
 
 			err = nfp_flower_cmsg_portreify(repr, exists);
 			if (err)
 				return err;
 			count++;
 		}
+	}
 
 	return count;
 }
@@ -234,19 +238,21 @@ nfp_flower_spawn_vnic_reprs(struct nfp_app *app,
 		return -ENOMEM;
 
 	for (i = 0; i < cnt; i++) {
+		struct net_device *repr;
 		struct nfp_port *port;
 		u32 port_id;
 
-		reprs->reprs[i] = nfp_repr_alloc(app);
-		if (!reprs->reprs[i]) {
+		repr = nfp_repr_alloc(app);
+		if (!repr) {
 			err = -ENOMEM;
 			goto err_reprs_clean;
 		}
+		RCU_INIT_POINTER(reprs->reprs[i], repr);
 
 		/* For now we only support 1 PF */
 		WARN_ON(repr_type == NFP_REPR_TYPE_PF && i);
 
-		port = nfp_port_alloc(app, port_type, reprs->reprs[i]);
+		port = nfp_port_alloc(app, port_type, repr);
 		if (repr_type == NFP_REPR_TYPE_PF) {
 			port->pf_id = i;
 			port->vnic = priv->nn->dp.ctrl_bar;
@@ -257,11 +263,11 @@ nfp_flower_spawn_vnic_reprs(struct nfp_app *app,
 				app->pf->vf_cfg_mem + i * NFP_NET_CFG_BAR_SZ;
 		}
 
-		eth_hw_addr_random(reprs->reprs[i]);
+		eth_hw_addr_random(repr);
 
 		port_id = nfp_flower_cmsg_pcie_port(nfp_pcie, vnic_type,
 						    i, queue);
-		err = nfp_repr_init(app, reprs->reprs[i],
+		err = nfp_repr_init(app, repr,
 				    port_id, port, priv->nn->dp.netdev);
 		if (err) {
 			nfp_port_free(port);
@@ -270,7 +276,7 @@ nfp_flower_spawn_vnic_reprs(struct nfp_app *app,
 
 		nfp_info(app->cpp, "%s%d Representor(%s) created\n",
 			 repr_type == NFP_REPR_TYPE_PF ? "PF" : "VF", i,
-			 reprs->reprs[i]->name);
+			 repr->name);
 	}
 
 	nfp_app_reprs_set(app, repr_type, reprs);
@@ -291,7 +297,7 @@ nfp_flower_spawn_vnic_reprs(struct nfp_app *app,
 err_reprs_remove:
 	reprs = nfp_app_reprs_set(app, repr_type, NULL);
 err_reprs_clean:
-	nfp_reprs_clean_and_free(reprs);
+	nfp_reprs_clean_and_free(app, reprs);
 	return err;
 }
 
@@ -329,17 +335,18 @@ nfp_flower_spawn_phy_reprs(struct nfp_app *app, struct nfp_flower_priv *priv)
 
 	for (i = 0; i < eth_tbl->count; i++) {
 		unsigned int phys_port = eth_tbl->ports[i].index;
+		struct net_device *repr;
 		struct nfp_port *port;
 		u32 cmsg_port_id;
 
-		reprs->reprs[phys_port] = nfp_repr_alloc(app);
-		if (!reprs->reprs[phys_port]) {
+		repr = nfp_repr_alloc(app);
+		if (!repr) {
 			err = -ENOMEM;
 			goto err_reprs_clean;
 		}
+		RCU_INIT_POINTER(reprs->reprs[phys_port], repr);
 
-		port = nfp_port_alloc(app, NFP_PORT_PHYS_PORT,
-				      reprs->reprs[phys_port]);
+		port = nfp_port_alloc(app, NFP_PORT_PHYS_PORT, repr);
 		if (IS_ERR(port)) {
 			err = PTR_ERR(port);
 			goto err_reprs_clean;
@@ -350,11 +357,11 @@ nfp_flower_spawn_phy_reprs(struct nfp_app *app, struct nfp_flower_priv *priv)
 			goto err_reprs_clean;
 		}
 
-		SET_NETDEV_DEV(reprs->reprs[phys_port], &priv->nn->pdev->dev);
+		SET_NETDEV_DEV(repr, &priv->nn->pdev->dev);
 		nfp_net_get_mac_addr(app->pf, port);
 
 		cmsg_port_id = nfp_flower_cmsg_phys_port(phys_port);
-		err = nfp_repr_init(app, reprs->reprs[phys_port],
+		err = nfp_repr_init(app, repr,
 				    cmsg_port_id, port, priv->nn->dp.netdev);
 		if (err) {
 			nfp_port_free(port);
@@ -367,7 +374,7 @@ nfp_flower_spawn_phy_reprs(struct nfp_app *app, struct nfp_flower_priv *priv)
 					     phys_port);
 
 		nfp_info(app->cpp, "Phys Port %d Representor(%s) created\n",
-			 phys_port, reprs->reprs[phys_port]->name);
+			 phys_port, repr->name);
 	}
 
 	nfp_app_reprs_set(app, NFP_REPR_TYPE_PHYS_PORT, reprs);
@@ -397,7 +404,7 @@ nfp_flower_spawn_phy_reprs(struct nfp_app *app, struct nfp_flower_priv *priv)
 err_reprs_remove:
 	reprs = nfp_app_reprs_set(app, NFP_REPR_TYPE_PHYS_PORT, NULL);
 err_reprs_clean:
-	nfp_reprs_clean_and_free(reprs);
+	nfp_reprs_clean_and_free(app, reprs);
 err_free_ctrl_skb:
 	kfree_skb(ctrl_skb);
 	return err;

drivers/net/ethernet/netronome/nfp/nfp_net_repr.c

@@ -46,6 +46,13 @@
 #include "nfp_net_sriov.h"
 #include "nfp_port.h"
 
+struct net_device *
+nfp_repr_get_locked(struct nfp_app *app, struct nfp_reprs *set, unsigned int id)
+{
+	return rcu_dereference_protected(set->reprs[id],
+					 lockdep_is_held(&app->pf->lock));
+}
+
 static void
 nfp_repr_inc_tx_stats(struct net_device *netdev, unsigned int len,
 		      int tx_status)
@@ -369,21 +376,24 @@ static void nfp_repr_clean_and_free(struct nfp_repr *repr)
 	nfp_repr_free(repr);
 }
 
-void nfp_reprs_clean_and_free(struct nfp_reprs *reprs)
+void nfp_reprs_clean_and_free(struct nfp_app *app, struct nfp_reprs *reprs)
 {
+	struct net_device *netdev;
 	unsigned int i;
 
-	for (i = 0; i < reprs->num_reprs; i++)
-		if (reprs->reprs[i])
-			nfp_repr_clean_and_free(netdev_priv(reprs->reprs[i]));
+	for (i = 0; i < reprs->num_reprs; i++) {
+		netdev = nfp_repr_get_locked(app, reprs, i);
+		if (netdev)
+			nfp_repr_clean_and_free(netdev_priv(netdev));
+	}
 
 	kfree(reprs);
 }
 
 void
-nfp_reprs_clean_and_free_by_type(struct nfp_app *app,
-				 enum nfp_repr_type type)
+nfp_reprs_clean_and_free_by_type(struct nfp_app *app, enum nfp_repr_type type)
 {
+	struct net_device *netdev;
 	struct nfp_reprs *reprs;
 	int i;
 
@@ -395,14 +405,16 @@ nfp_reprs_clean_and_free_by_type(struct nfp_app *app,
 	/* Preclean must happen before we remove the reprs reference from the
 	 * app below.
 	 */
-	for (i = 0; i < reprs->num_reprs; i++)
-		if (reprs->reprs[i])
-			nfp_app_repr_preclean(app, reprs->reprs[i]);
+	for (i = 0; i < reprs->num_reprs; i++) {
+		netdev = nfp_repr_get_locked(app, reprs, i);
+		if (netdev)
+			nfp_app_repr_preclean(app, netdev);
+	}
 
 	reprs = nfp_app_reprs_set(app, type, NULL);
 
 	synchronize_rcu();
-	nfp_reprs_clean_and_free(reprs);
+	nfp_reprs_clean_and_free(app, reprs);
 }
 
 struct nfp_reprs *nfp_reprs_alloc(unsigned int num_reprs)
@@ -420,46 +432,29 @@ struct nfp_reprs *nfp_reprs_alloc(unsigned int num_reprs)
 
 int nfp_reprs_resync_phys_ports(struct nfp_app *app)
 {
-	struct nfp_reprs *reprs, *old_reprs;
+	struct net_device *netdev;
+	struct nfp_reprs *reprs;
 	struct nfp_repr *repr;
 	int i;
 
-	old_reprs = nfp_reprs_get_locked(app, NFP_REPR_TYPE_PHYS_PORT);
-	if (!old_reprs)
-		return 0;
-
-	reprs = nfp_reprs_alloc(old_reprs->num_reprs);
+	reprs = nfp_reprs_get_locked(app, NFP_REPR_TYPE_PHYS_PORT);
 	if (!reprs)
-		return -ENOMEM;
-
-	for (i = 0; i < old_reprs->num_reprs; i++) {
-		if (!old_reprs->reprs[i])
-			continue;
-
-		repr = netdev_priv(old_reprs->reprs[i]);
-		if (repr->port->type == NFP_PORT_INVALID) {
-			nfp_app_repr_preclean(app, old_reprs->reprs[i]);
-			continue;
-		}
-
-		reprs->reprs[i] = old_reprs->reprs[i];
-	}
-
-	old_reprs = nfp_app_reprs_set(app, NFP_REPR_TYPE_PHYS_PORT, reprs);
-	synchronize_rcu();
+		return 0;
 
-	/* Now we free up removed representors */
-	for (i = 0; i < old_reprs->num_reprs; i++) {
-		if (!old_reprs->reprs[i])
+	for (i = 0; i < reprs->num_reprs; i++) {
+		netdev = nfp_repr_get_locked(app, reprs, i);
+		if (!netdev)
 			continue;
 
-		repr = netdev_priv(old_reprs->reprs[i]);
+		repr = netdev_priv(netdev);
 		if (repr->port->type != NFP_PORT_INVALID)
 			continue;
 
+		nfp_app_repr_preclean(app, netdev);
+		rcu_assign_pointer(reprs->reprs[i], NULL);
+		synchronize_rcu();
 		nfp_repr_clean(repr);
 	}
 
-	kfree(old_reprs);
 	return 0;
 }

drivers/net/ethernet/netronome/nfp/nfp_net_repr.h

@@ -35,6 +35,7 @@
 #define NFP_NET_REPR_H
 
 struct metadata_dst;
+struct nfp_app;
 struct nfp_net;
 struct nfp_port;
 
@@ -47,7 +48,7 @@ struct nfp_port;
  */
 struct nfp_reprs {
 	unsigned int num_reprs;
-	struct net_device *reprs[0];
+	struct net_device __rcu *reprs[0];
 };
 
 /**
@@ -114,16 +115,18 @@ static inline int nfp_repr_get_port_id(struct net_device *netdev)
 	return priv->dst->u.port_info.port_id;
 }
 
+struct net_device *
+nfp_repr_get_locked(struct nfp_app *app, struct nfp_reprs *set,
+		    unsigned int id);
+
 void nfp_repr_inc_rx_stats(struct net_device *netdev, unsigned int len);
 int nfp_repr_init(struct nfp_app *app, struct net_device *netdev,
 		  u32 cmsg_port_id, struct nfp_port *port,
 		  struct net_device *pf_netdev);
 struct net_device *nfp_repr_alloc(struct nfp_app *app);
-void
-nfp_reprs_clean_and_free(struct nfp_reprs *reprs);
-void
-nfp_reprs_clean_and_free_by_type(struct nfp_app *app,
-				 enum nfp_repr_type type);
+void nfp_reprs_clean_and_free(struct nfp_app *app, struct nfp_reprs *reprs);
+void nfp_reprs_clean_and_free_by_type(struct nfp_app *app,
+				      enum nfp_repr_type type);
 struct nfp_reprs *nfp_reprs_alloc(unsigned int num_reprs);
 int nfp_reprs_resync_phys_ports(struct nfp_app *app);