Skip to content

Commit

Permalink
ice: Accumulate ring statistics over reset
Browse files Browse the repository at this point in the history 
Resets may occur with or without user interaction. For example, a TX hang
or reconfiguration of parameters will result in a reset. During reset, the
VSI is freed, freeing any statistics structures inside as well. This would
create an issue for the user where a reset happens in the background,
statistics set to zero, and the user checks ring statistics expecting them
to be populated.

To ensure this doesn't happen, accumulate ring statistics over reset.

Define a new ring statistics structure, ice_ring_stats. The new structure
lives in the VSI's parent, preserving ring statistics when VSI is freed.

1. Define a new structure vsi_ring_stats in the PF scope
2. Allocate/free stats only during probe, unload, or change in ring size
3. Replace previous ring statistics functionality with new structure

Signed-off-by: Benjamin Mikailenko <benjamin.mikailenko@intel.com>
Tested-by: Gurucharan G <gurucharanx.g@intel.com> (A Contingent worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
  • Loading branch information
Benjamin Mikailenko authored and Tony Nguyen committed Nov 23, 2022
1 parent 2fd5e43 commit 288ecf4
Show file tree
Hide file tree
Showing 9 changed files with 351 additions and 61 deletions.
6 changes: 6 additions & 0 deletions drivers/net/ethernet/intel/ice/ice.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,11 @@ enum ice_vsi_state {
ICE_VSI_STATE_NBITS /* must be last */
};

struct ice_vsi_stats {
struct ice_ring_stats **tx_ring_stats; /* Tx ring stats array */
struct ice_ring_stats **rx_ring_stats; /* Rx ring stats array */
};

/* struct that defines a VSI, associated with a dev */
struct ice_vsi {
struct net_device *netdev;
Expand Down Expand Up @@ -541,6 +546,7 @@ struct ice_pf {
u16 ctrl_vsi_idx; /* control VSI index in pf->vsi array */

struct ice_vsi **vsi; /* VSIs created by the driver */
struct ice_vsi_stats **vsi_stats;
struct ice_sw *first_sw; /* first switch created by firmware */
u16 eswitch_mode; /* current mode of eswitch */
struct ice_vfs vfs;
Expand Down
12 changes: 6 additions & 6 deletions drivers/net/ethernet/intel/ice/ice_ethtool.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1544,9 +1544,9 @@ __ice_get_ethtool_stats(struct net_device *netdev,

ice_for_each_alloc_txq(vsi, j) {
tx_ring = READ_ONCE(vsi->tx_rings[j]);
if (tx_ring) {
data[i++] = tx_ring->stats.pkts;
data[i++] = tx_ring->stats.bytes;
if (tx_ring && tx_ring->ring_stats) {
data[i++] = tx_ring->ring_stats->stats.pkts;
data[i++] = tx_ring->ring_stats->stats.bytes;
} else {
data[i++] = 0;
data[i++] = 0;
Expand All @@ -1555,9 +1555,9 @@ __ice_get_ethtool_stats(struct net_device *netdev,

ice_for_each_alloc_rxq(vsi, j) {
rx_ring = READ_ONCE(vsi->rx_rings[j]);
if (rx_ring) {
data[i++] = rx_ring->stats.pkts;
data[i++] = rx_ring->stats.bytes;
if (rx_ring && rx_ring->ring_stats) {
data[i++] = rx_ring->ring_stats->stats.pkts;
data[i++] = rx_ring->ring_stats->stats.bytes;
} else {
data[i++] = 0;
data[i++] = 0;
Expand Down
233 changes: 225 additions & 8 deletions drivers/net/ethernet/intel/ice/ice_lib.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,49 @@ static irqreturn_t ice_eswitch_msix_clean_rings(int __always_unused irq, void *d
return IRQ_HANDLED;
}

/**
* ice_vsi_alloc_stat_arrays - Allocate statistics arrays
* @vsi: VSI pointer
*/
static int ice_vsi_alloc_stat_arrays(struct ice_vsi *vsi)
{
struct ice_vsi_stats *vsi_stat;
struct ice_pf *pf = vsi->back;

if (vsi->type == ICE_VSI_CHNL)
return 0;
if (!pf->vsi_stats)
return -ENOENT;

vsi_stat = kzalloc(sizeof(*vsi_stat), GFP_KERNEL);
if (!vsi_stat)
return -ENOMEM;

vsi_stat->tx_ring_stats =
kcalloc(vsi->alloc_txq, sizeof(*vsi_stat->tx_ring_stats),
GFP_KERNEL);
if (!vsi_stat->tx_ring_stats)
goto err_alloc_tx;

vsi_stat->rx_ring_stats =
kcalloc(vsi->alloc_rxq, sizeof(*vsi_stat->rx_ring_stats),
GFP_KERNEL);
if (!vsi_stat->rx_ring_stats)
goto err_alloc_rx;

pf->vsi_stats[vsi->idx] = vsi_stat;

return 0;

err_alloc_rx:
kfree(vsi_stat->rx_ring_stats);
err_alloc_tx:
kfree(vsi_stat->tx_ring_stats);
kfree(vsi_stat);
pf->vsi_stats[vsi->idx] = NULL;
return -ENOMEM;
}

/**
* ice_vsi_alloc - Allocates the next available struct VSI in the PF
* @pf: board private structure
Expand Down Expand Up @@ -560,6 +603,11 @@ ice_vsi_alloc(struct ice_pf *pf, enum ice_vsi_type vsi_type,

if (vsi->type == ICE_VSI_CTRL && vf)
vf->ctrl_vsi_idx = vsi->idx;

/* allocate memory for Tx/Rx ring stat pointers */
if (ice_vsi_alloc_stat_arrays(vsi))
goto err_rings;

goto unlock_pf;

err_rings:
Expand Down Expand Up @@ -1535,6 +1583,106 @@ static int ice_vsi_alloc_rings(struct ice_vsi *vsi)
return -ENOMEM;
}

/**
* ice_vsi_free_stats - Free the ring statistics structures
* @vsi: VSI pointer
*/
static void ice_vsi_free_stats(struct ice_vsi *vsi)
{
struct ice_vsi_stats *vsi_stat;
struct ice_pf *pf = vsi->back;
int i;

if (vsi->type == ICE_VSI_CHNL)
return;
if (!pf->vsi_stats)
return;

vsi_stat = pf->vsi_stats[vsi->idx];
if (!vsi_stat)
return;

ice_for_each_alloc_txq(vsi, i) {
if (vsi_stat->tx_ring_stats[i]) {
kfree_rcu(vsi_stat->tx_ring_stats[i], rcu);
WRITE_ONCE(vsi_stat->tx_ring_stats[i], NULL);
}
}

ice_for_each_alloc_rxq(vsi, i) {
if (vsi_stat->rx_ring_stats[i]) {
kfree_rcu(vsi_stat->rx_ring_stats[i], rcu);
WRITE_ONCE(vsi_stat->rx_ring_stats[i], NULL);
}
}

kfree(vsi_stat->tx_ring_stats);
kfree(vsi_stat->rx_ring_stats);
kfree(vsi_stat);
pf->vsi_stats[vsi->idx] = NULL;
}

/**
* ice_vsi_alloc_ring_stats - Allocates Tx and Rx ring stats for the VSI
* @vsi: VSI which is having stats allocated
*/
static int ice_vsi_alloc_ring_stats(struct ice_vsi *vsi)
{
struct ice_ring_stats **tx_ring_stats;
struct ice_ring_stats **rx_ring_stats;
struct ice_vsi_stats *vsi_stats;
struct ice_pf *pf = vsi->back;
u16 i;

vsi_stats = pf->vsi_stats[vsi->idx];
tx_ring_stats = vsi_stats->tx_ring_stats;
rx_ring_stats = vsi_stats->rx_ring_stats;

/* Allocate Tx ring stats */
ice_for_each_alloc_txq(vsi, i) {
struct ice_ring_stats *ring_stats;
struct ice_tx_ring *ring;

ring = vsi->tx_rings[i];
ring_stats = tx_ring_stats[i];

if (!ring_stats) {
ring_stats = kzalloc(sizeof(*ring_stats), GFP_KERNEL);
if (!ring_stats)
goto err_out;

WRITE_ONCE(tx_ring_stats[i], ring_stats);
}

ring->ring_stats = ring_stats;
}

/* Allocate Rx ring stats */
ice_for_each_alloc_rxq(vsi, i) {
struct ice_ring_stats *ring_stats;
struct ice_rx_ring *ring;

ring = vsi->rx_rings[i];
ring_stats = rx_ring_stats[i];

if (!ring_stats) {
ring_stats = kzalloc(sizeof(*ring_stats), GFP_KERNEL);
if (!ring_stats)
goto err_out;

WRITE_ONCE(rx_ring_stats[i], ring_stats);
}

ring->ring_stats = ring_stats;
}

return 0;

err_out:
ice_vsi_free_stats(vsi);
return -ENOMEM;
}

/**
* ice_vsi_manage_rss_lut - disable/enable RSS
* @vsi: the VSI being changed
Expand Down Expand Up @@ -2580,6 +2728,10 @@ ice_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi,
if (ret)
goto unroll_vector_base;

ret = ice_vsi_alloc_ring_stats(vsi);
if (ret)
goto unroll_vector_base;

ice_vsi_map_rings_to_vectors(vsi);

/* ICE_VSI_CTRL does not need RSS so skip RSS processing */
Expand Down Expand Up @@ -2618,6 +2770,9 @@ ice_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi,
if (ret)
goto unroll_vector_base;

ret = ice_vsi_alloc_ring_stats(vsi);
if (ret)
goto unroll_vector_base;
/* Do not exit if configuring RSS had an issue, at least
* receive traffic on first queue. Hence no need to capture
* return value
Expand All @@ -2631,6 +2786,11 @@ ice_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi,
ret = ice_vsi_alloc_rings(vsi);
if (ret)
goto unroll_vsi_init;

ret = ice_vsi_alloc_ring_stats(vsi);
if (ret)
goto unroll_vector_base;

break;
default:
/* clean up the resources and exit */
Expand Down Expand Up @@ -2690,6 +2850,7 @@ ice_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi,
unroll_alloc_q_vector:
ice_vsi_free_q_vectors(vsi);
unroll_vsi_init:
ice_vsi_free_stats(vsi);
ice_vsi_delete(vsi);
unroll_get_qs:
ice_vsi_put_qs(vsi);
Expand Down Expand Up @@ -3081,7 +3242,7 @@ int ice_vsi_release(struct ice_vsi *vsi)
vsi->agg_node && vsi->agg_node->valid)
vsi->agg_node->num_vsis--;
ice_vsi_clear_rings(vsi);

ice_vsi_free_stats(vsi);
ice_vsi_put_qs(vsi);

/* retain SW VSI data structure since it is needed to unregister and
Expand Down Expand Up @@ -3208,6 +3369,47 @@ ice_vsi_rebuild_set_coalesce(struct ice_vsi *vsi,
}
}

/**
* ice_vsi_realloc_stat_arrays - Frees unused stat structures
* @vsi: VSI pointer
* @prev_txq: Number of Tx rings before ring reallocation
* @prev_rxq: Number of Rx rings before ring reallocation
*/
static int
ice_vsi_realloc_stat_arrays(struct ice_vsi *vsi, int prev_txq, int prev_rxq)
{
struct ice_vsi_stats *vsi_stat;
struct ice_pf *pf = vsi->back;
int i;

if (!prev_txq || !prev_rxq)
return 0;
if (vsi->type == ICE_VSI_CHNL)
return 0;

vsi_stat = pf->vsi_stats[vsi->idx];

if (vsi->num_txq < prev_txq) {
for (i = vsi->num_txq; i < prev_txq; i++) {
if (vsi_stat->tx_ring_stats[i]) {
kfree_rcu(vsi_stat->tx_ring_stats[i], rcu);
WRITE_ONCE(vsi_stat->tx_ring_stats[i], NULL);
}
}
}

if (vsi->num_rxq < prev_rxq) {
for (i = vsi->num_rxq; i < prev_rxq; i++) {
if (vsi_stat->rx_ring_stats[i]) {
kfree_rcu(vsi_stat->rx_ring_stats[i], rcu);
WRITE_ONCE(vsi_stat->rx_ring_stats[i], NULL);
}
}
}

return 0;
}

/**
* ice_vsi_rebuild - Rebuild VSI after reset
* @vsi: VSI to be rebuild
Expand All @@ -3219,10 +3421,10 @@ int ice_vsi_rebuild(struct ice_vsi *vsi, bool init_vsi)
{
u16 max_txqs[ICE_MAX_TRAFFIC_CLASS] = { 0 };
struct ice_coalesce_stored *coalesce;
int ret, i, prev_txq, prev_rxq;
int prev_num_q_vectors = 0;
enum ice_vsi_type vtype;
struct ice_pf *pf;
int ret, i;

if (!vsi)
return -EINVAL;
Expand All @@ -3241,6 +3443,9 @@ int ice_vsi_rebuild(struct ice_vsi *vsi, bool init_vsi)

prev_num_q_vectors = ice_vsi_rebuild_get_coalesce(vsi, coalesce);

prev_txq = vsi->num_txq;
prev_rxq = vsi->num_rxq;

ice_rm_vsi_lan_cfg(vsi->port_info, vsi->idx);
ret = ice_rm_vsi_rdma_cfg(vsi->port_info, vsi->idx);
if (ret)
Expand Down Expand Up @@ -3307,6 +3512,10 @@ int ice_vsi_rebuild(struct ice_vsi *vsi, bool init_vsi)
if (ret)
goto err_vectors;

ret = ice_vsi_alloc_ring_stats(vsi);
if (ret)
goto err_vectors;

ice_vsi_map_rings_to_vectors(vsi);

vsi->stat_offsets_loaded = false;
Expand Down Expand Up @@ -3346,6 +3555,10 @@ int ice_vsi_rebuild(struct ice_vsi *vsi, bool init_vsi)
if (ret)
goto err_vectors;

ret = ice_vsi_alloc_ring_stats(vsi);
if (ret)
goto err_vectors;

vsi->stat_offsets_loaded = false;
break;
case ICE_VSI_CHNL:
Expand Down Expand Up @@ -3394,6 +3607,10 @@ int ice_vsi_rebuild(struct ice_vsi *vsi, bool init_vsi)
return ice_schedule_reset(pf, ICE_RESET_PFR);
}
}

if (ice_vsi_realloc_stat_arrays(vsi, prev_txq, prev_rxq))
goto err_vectors;

ice_vsi_rebuild_set_coalesce(vsi, coalesce, prev_num_q_vectors);
kfree(coalesce);

Expand Down Expand Up @@ -3735,9 +3952,9 @@ static void ice_update_ring_stats(struct ice_q_stats *stats, u64 pkts, u64 bytes
*/
void ice_update_tx_ring_stats(struct ice_tx_ring *tx_ring, u64 pkts, u64 bytes)
{
u64_stats_update_begin(&tx_ring->syncp);
ice_update_ring_stats(&tx_ring->stats, pkts, bytes);
u64_stats_update_end(&tx_ring->syncp);
u64_stats_update_begin(&tx_ring->ring_stats->syncp);
ice_update_ring_stats(&tx_ring->ring_stats->stats, pkts, bytes);
u64_stats_update_end(&tx_ring->ring_stats->syncp);
}

/**
Expand All @@ -3748,9 +3965,9 @@ void ice_update_tx_ring_stats(struct ice_tx_ring *tx_ring, u64 pkts, u64 bytes)
*/
void ice_update_rx_ring_stats(struct ice_rx_ring *rx_ring, u64 pkts, u64 bytes)
{
u64_stats_update_begin(&rx_ring->syncp);
ice_update_ring_stats(&rx_ring->stats, pkts, bytes);
u64_stats_update_end(&rx_ring->syncp);
u64_stats_update_begin(&rx_ring->ring_stats->syncp);
ice_update_ring_stats(&rx_ring->ring_stats->stats, pkts, bytes);
u64_stats_update_end(&rx_ring->ring_stats->syncp);
}

/**
Expand Down
Loading

0 comments on commit 288ecf4

Please sign in to comment.