Skip to content

Commit

Permalink
net/sfc: fix reading adapter state without locking
Browse files Browse the repository at this point in the history
[ upstream commit 17b0d7b ]

Update MAC stats function reads adapter state with MAC stats locking
but without adapter locking. Add adapter locking before calling this
function and remove MAC stats locking since there's no point to have
it together with adapter locking. The second place MAC stats locking
is used is MAC stats reset function. It's called with adapter being
already locked so there's no point to use MAC stats locking anymore.

Fixes: 1caab2f ("net/sfc: add basic statistics")

Signed-off-by: Ivan Ilchenko <ivan.ilchenko@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Reviewed-by: Andy Moreton <amoreton@xilinx.com>
  • Loading branch information
ol-vanyaio authored and bluca committed Jul 26, 2021
1 parent e5e8e0a commit 99bcdae
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 15 deletions.
1 change: 0 additions & 1 deletion drivers/net/sfc/sfc.h
Expand Up @@ -128,7 +128,6 @@ struct sfc_port {
unsigned int nb_mcast_addrs;
uint8_t *mcast_addrs;

rte_spinlock_t mac_stats_lock;
uint64_t *mac_stats_buf;
unsigned int mac_stats_nb_supported;
efsys_mem_t mac_stats_dma_mem;
Expand Down
28 changes: 20 additions & 8 deletions drivers/net/sfc/sfc_ethdev.c
Expand Up @@ -604,7 +604,7 @@ sfc_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
uint64_t *mac_stats;
int ret;

rte_spinlock_lock(&port->mac_stats_lock);
sfc_adapter_lock(sa);

ret = sfc_port_update_mac_stats(sa);
if (ret != 0)
Expand Down Expand Up @@ -677,7 +677,7 @@ sfc_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
}

unlock:
rte_spinlock_unlock(&port->mac_stats_lock);
sfc_adapter_unlock(sa);
SFC_ASSERT(ret >= 0);
return -ret;
}
Expand All @@ -689,19 +689,24 @@ sfc_stats_reset(struct rte_eth_dev *dev)
struct sfc_port *port = &sa->port;
int rc;

sfc_adapter_lock(sa);

if (sa->state != SFC_ADAPTER_STARTED) {
/*
* The operation cannot be done if port is not started; it
* will be scheduled to be done during the next port start
*/
port->mac_stats_reset_pending = B_TRUE;
sfc_adapter_unlock(sa);
return 0;
}

rc = sfc_port_reset_mac_stats(sa);
if (rc != 0)
sfc_err(sa, "failed to reset statistics (rc = %d)", rc);

sfc_adapter_unlock(sa);

SFC_ASSERT(rc >= 0);
return -rc;
}
Expand All @@ -717,7 +722,7 @@ sfc_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
unsigned int i;
int nstats = 0;

rte_spinlock_lock(&port->mac_stats_lock);
sfc_adapter_lock(sa);

rc = sfc_port_update_mac_stats(sa);
if (rc != 0) {
Expand All @@ -739,7 +744,7 @@ sfc_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
}

unlock:
rte_spinlock_unlock(&port->mac_stats_lock);
sfc_adapter_unlock(sa);

return nstats;
}
Expand Down Expand Up @@ -780,7 +785,7 @@ sfc_xstats_get_by_id(struct rte_eth_dev *dev, const uint64_t *ids,
int ret;
int rc;

rte_spinlock_lock(&port->mac_stats_lock);
sfc_adapter_lock(sa);

if (unlikely(values == NULL) ||
unlikely(ids == NULL && n < port->mac_stats_nb_supported)) {
Expand Down Expand Up @@ -810,7 +815,7 @@ sfc_xstats_get_by_id(struct rte_eth_dev *dev, const uint64_t *ids,
ret = nb_written;

unlock:
rte_spinlock_unlock(&port->mac_stats_lock);
sfc_adapter_unlock(sa);

return ret;
}
Expand All @@ -826,9 +831,14 @@ sfc_xstats_get_names_by_id(struct rte_eth_dev *dev,
unsigned int nb_written = 0;
unsigned int i;

sfc_adapter_lock(sa);

if (unlikely(xstats_names == NULL) ||
unlikely((ids == NULL) && (size < port->mac_stats_nb_supported)))
return port->mac_stats_nb_supported;
unlikely((ids == NULL) && (size < port->mac_stats_nb_supported))) {
nb_supported = port->mac_stats_nb_supported;
sfc_adapter_unlock(sa);
return nb_supported;
}

for (i = 0; (i < EFX_MAC_NSTATS) && (nb_written < size); ++i) {
if (!EFX_MAC_STAT_SUPPORTED(port->mac_stats_mask, i))
Expand All @@ -844,6 +854,8 @@ sfc_xstats_get_names_by_id(struct rte_eth_dev *dev,
++nb_supported;
}

sfc_adapter_unlock(sa);

return nb_written;
}

Expand Down
9 changes: 3 additions & 6 deletions drivers/net/sfc/sfc_port.c
Expand Up @@ -43,7 +43,7 @@ sfc_port_update_mac_stats(struct sfc_adapter *sa)
unsigned int nb_attempts = 0;
int rc;

SFC_ASSERT(rte_spinlock_is_locked(&port->mac_stats_lock));
SFC_ASSERT(sfc_adapter_is_locked(sa));

if (sa->state != SFC_ADAPTER_STARTED)
return EINVAL;
Expand Down Expand Up @@ -103,14 +103,13 @@ sfc_port_reset_sw_stats(struct sfc_adapter *sa)
int
sfc_port_reset_mac_stats(struct sfc_adapter *sa)
{
struct sfc_port *port = &sa->port;
int rc;

rte_spinlock_lock(&port->mac_stats_lock);
SFC_ASSERT(sfc_adapter_is_locked(sa));

rc = efx_mac_stats_clear(sa->nic);
if (rc == 0)
sfc_port_reset_sw_stats(sa);
rte_spinlock_unlock(&port->mac_stats_lock);

return rc;
}
Expand Down Expand Up @@ -416,8 +415,6 @@ sfc_port_attach(struct sfc_adapter *sa)
goto fail_mcast_addr_list_buf_alloc;
}

rte_spinlock_init(&port->mac_stats_lock);

rc = ENOMEM;
port->mac_stats_buf = rte_calloc_socket("mac_stats_buf", EFX_MAC_NSTATS,
sizeof(uint64_t), 0,
Expand Down

0 comments on commit 99bcdae

Please sign in to comment.