[PATCH net 0/4] nfp: fix a warning, stats, naming and route leak

[PATCH net 0/4] nfp: fix a warning, stats, naming and route leak

[Jakub Kicinski]

Hi!

Various fixes for the NFP.  Patch 1 fixes a harmless GCC 8 warning.
Patch 2 ensures statistics are correct after users decrease the number
of channels/rings.  Patch 3 restores phy_port_name behaviour for flower,
ndo_get_phy_port_name used to return -EOPNOTSUPP on one of the netdevs,
and we need to keep it that way otherwise interface names may change.
Patch 4 fixes refcnt leak in flower tunnel offload code.

Jakub Kicinski (3):
  nfp: don't pad strings in nfp_cpp_resource_find() to avoid gcc 8
    warning
  nfp: include all ring counters in interface stats
  nfp: remove phys_port_name on flower's vNIC

Pieter Jansen van Vuuren (1):
  nfp: flower: free dst_entry in route table

 drivers/net/ethernet/netronome/nfp/flower/main.c          | 1 +
 drivers/net/ethernet/netronome/nfp/flower/tunnel_conf.c   | 2 ++
 drivers/net/ethernet/netronome/nfp/nfp_net.h              | 4 ++++
 drivers/net/ethernet/netronome/nfp/nfp_net_common.c       | 4 ++--
 drivers/net/ethernet/netronome/nfp/nfpcore/nfp_resource.c | 7 ++-----
 5 files changed, 11 insertions(+), 7 deletions(-)

-- 
2.17.1

[PATCH net 1/4] nfp: don't pad strings in nfp_cpp_resource_find() to avoid gcc 8 warning

[Jakub Kicinski]

Once upon a time nfp_cpp_resource_find() took a name parameter,
which could be any user-chosen string.  Resources are identified
by a CRC32 hash of a 8 byte string, so we had to pad user input
with zeros to make sure CRC32 gave the correct result.

Since then nfp_cpp_resource_find() was made to operate on allocated
resources only (struct nfp_resource).  We kzalloc those so there is
no need to pad the strings and use memcmp.

This avoids a GCC 8 stringop-truncation warning:

In function ‘nfp_cpp_resource_find’,
    inlined from ‘nfp_resource_try_acquire’ at .../nfpcore/nfp_resource.c:153:8,
    inlined from ‘nfp_resource_acquire’ at .../nfpcore/nfp_resource.c:206:9:
    .../nfpcore/nfp_resource.c:108:2: warning:  strncpy’ output may be truncated copying 8 bytes from a string of length 8 [-Wstringop-truncation]
      strncpy(name_pad, res->name, sizeof(name_pad));
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/nfpcore/nfp_resource.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_resource.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_resource.c
index 2dd89dba9311..d32af598da90 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_resource.c
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_resource.c
@@ -98,21 +98,18 @@ struct nfp_resource {
 
 static int nfp_cpp_resource_find(struct nfp_cpp *cpp, struct nfp_resource *res)
 {
-	char name_pad[NFP_RESOURCE_ENTRY_NAME_SZ] = {};
 	struct nfp_resource_entry entry;
 	u32 cpp_id, key;
 	int ret, i;
 
 	cpp_id = NFP_CPP_ID(NFP_RESOURCE_TBL_TARGET, 3, 0);  /* Atomic read */
 
-	strncpy(name_pad, res->name, sizeof(name_pad));
-
 	/* Search for a matching entry */
-	if (!memcmp(name_pad, NFP_RESOURCE_TBL_NAME "\0\0\0\0\0\0\0\0", 8)) {
+	if (!strcmp(res->name, NFP_RESOURCE_TBL_NAME)) {
 		nfp_err(cpp, "Grabbing device lock not supported\n");
 		return -EOPNOTSUPP;
 	}
-	key = crc32_posix(name_pad, sizeof(name_pad));
+	key = crc32_posix(res->name, NFP_RESOURCE_ENTRY_NAME_SZ);
 
 	for (i = 0; i < NFP_RESOURCE_TBL_ENTRIES; i++) {
 		u64 addr = NFP_RESOURCE_TBL_BASE +
-- 
2.17.1

[PATCH net 2/4] nfp: include all ring counters in interface stats

[Jakub Kicinski]

We are gathering software statistics on per-ring basis.
.ndo_get_stats64 handler adds the rings up.  Unfortunately
we are currently only adding up active rings, which means
that if user decreases the number of active rings the
statistics from deactivated rings will no longer be counted
and total interface statistics may go backwards.

Always sum all possible rings, the stats are allocated
statically for max number of rings, so we don't have to
worry about them being removed.  We could add the stats
up when user changes the ring count, but it seems unnecessary..
Adding up inactive rings will be very quick since no datapath
will be touching them.

Fixes: 164d1e9e5d52 ("nfp: add support for ethtool .set_channels")
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index 75110c8d6a90..ed27176c2bce 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -3121,7 +3121,7 @@ static void nfp_net_stat64(struct net_device *netdev,
 	struct nfp_net *nn = netdev_priv(netdev);
 	int r;
 
-	for (r = 0; r < nn->dp.num_r_vecs; r++) {
+	for (r = 0; r < nn->max_r_vecs; r++) {
 		struct nfp_net_r_vector *r_vec = &nn->r_vecs[r];
 		u64 data[3];
 		unsigned int start;
-- 
2.17.1

[PATCH net 3/4] nfp: remove phys_port_name on flower's vNIC

[Jakub Kicinski]

.ndo_get_phys_port_name was recently extended to support multi-vNIC
FWs.  These are firmwares which can have more than one vNIC per PF
without associated port (e.g. Adaptive Buffer Management FW), therefore
we need a way of distinguishing the vNICs.  Unfortunately, it's too
late to make flower use the same naming.  Flower users may depend on
.ndo_get_phys_port_name returning -EOPNOTSUPP, for example the name
udev gave the PF vNIC was just the bare PCI device-based name before
the change, and will have 'nn0' appended after.

To ensure flower's vNIC doesn't have phys_port_name attribute, add
a flag to vNIC struct and set it in flower code.  New projects will
not set the flag adhere to the naming scheme from the start.

Fixes: 51c1df83e35c ("nfp: assign vNIC id as phys_port_name of vNICs which are not ports")
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/flower/main.c    | 1 +
 drivers/net/ethernet/netronome/nfp/nfp_net.h        | 4 ++++
 drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 2 +-
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/netronome/nfp/flower/main.c b/drivers/net/ethernet/netronome/nfp/flower/main.c
index 19cfa162ac65..1decf3a1cad3 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/main.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/main.c
@@ -455,6 +455,7 @@ static int nfp_flower_vnic_alloc(struct nfp_app *app, struct nfp_net *nn,
 
 	eth_hw_addr_random(nn->dp.netdev);
 	netif_keep_dst(nn->dp.netdev);
+	nn->vnic_no_name = true;
 
 	return 0;
 
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net.h b/drivers/net/ethernet/netronome/nfp/nfp_net.h
index 57cb035dcc6d..2a71a9ffd095 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net.h
@@ -590,6 +590,8 @@ struct nfp_net_dp {
  * @vnic_list:		Entry on device vNIC list
  * @pdev:		Backpointer to PCI device
  * @app:		APP handle if available
+ * @vnic_no_name:	For non-port PF vNIC make ndo_get_phys_port_name return
+ *			-EOPNOTSUPP to keep backwards compatibility (set by app)
  * @port:		Pointer to nfp_port structure if vNIC is a port
  * @app_priv:		APP private data for this vNIC
  */
@@ -663,6 +665,8 @@ struct nfp_net {
 	struct pci_dev *pdev;
 	struct nfp_app *app;
 
+	bool vnic_no_name;
+
 	struct nfp_port *port;
 
 	void *app_priv;
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index ed27176c2bce..d4c27f849f9b 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -3286,7 +3286,7 @@ nfp_net_get_phys_port_name(struct net_device *netdev, char *name, size_t len)
 	if (nn->port)
 		return nfp_port_get_phys_port_name(netdev, name, len);
 
-	if (nn->dp.is_vf)
+	if (nn->dp.is_vf || nn->vnic_no_name)
 		return -EOPNOTSUPP;
 
 	n = snprintf(name, len, "n%d", nn->id);
-- 
2.17.1

[PATCH net 4/4] nfp: flower: free dst_entry in route table

[Jakub Kicinski]

From: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com>

We need to release the refcnt on dst_entry in the route table, otherwise
we will leak the route.

Fixes: 8e6a9046b66a ("nfp: flower vxlan neighbour offload")
Signed-off-by: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com>
Signed-off-by: Louis Peens <louis.peens@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/flower/tunnel_conf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/netronome/nfp/flower/tunnel_conf.c b/drivers/net/ethernet/netronome/nfp/flower/tunnel_conf.c
index ec524d97869d..78afe75129ab 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/tunnel_conf.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/tunnel_conf.c
@@ -381,6 +381,8 @@ nfp_tun_neigh_event_handler(struct notifier_block *nb, unsigned long event,
 	err = PTR_ERR_OR_ZERO(rt);
 	if (err)
 		return NOTIFY_DONE;
+
+	ip_rt_put(rt);
 #else
 	return NOTIFY_DONE;
 #endif
-- 
2.17.1

Re: [PATCH net 0/4] nfp: fix a warning, stats, naming and route leak

[David Miller]

From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Mon, 11 Jun 2018 21:33:34 -0700

> Various fixes for the NFP.  Patch 1 fixes a harmless GCC 8 warning.
> Patch 2 ensures statistics are correct after users decrease the number
> of channels/rings.  Patch 3 restores phy_port_name behaviour for flower,
> ndo_get_phy_port_name used to return -EOPNOTSUPP on one of the netdevs,
> and we need to keep it that way otherwise interface names may change.
> Patch 4 fixes refcnt leak in flower tunnel offload code.

Series applied.