Skip to content

Commit

Permalink
i40e: separate PF and VSI state flags
Browse files Browse the repository at this point in the history 
Avoid using the same named flags for both vsi->state and pf->state. This
makes code review easier, as it is more likely that future authors will
use the correct state field when checking bits. Previous commits already
found issues with at least one check, and possibly others may be
incorrect.

This reduces confusion as it is more clear what each flag represents,
and which flags are valid for which state field.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
  • Loading branch information
Jacob Keller authored and Jeff Kirsher committed Apr 30, 2017
1 parent 2318b40 commit d19cb64
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 55 deletions.
12 changes: 10 additions & 2 deletions drivers/net/ethernet/intel/i40e/i40e.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ enum i40e_state_t {
__I40E_CONFIG_BUSY,
__I40E_CONFIG_DONE,
__I40E_DOWN,
__I40E_NEEDS_RESTART,
__I40E_SERVICE_SCHED,
__I40E_ADMINQ_EVENT_PENDING,
__I40E_MDD_EVENT_PENDING,
Expand All @@ -138,7 +137,6 @@ enum i40e_state_t {
__I40E_GLOBAL_RESET_REQUESTED,
__I40E_EMP_RESET_REQUESTED,
__I40E_EMP_RESET_INTR_RECEIVED,
__I40E_FILTER_OVERFLOW_PROMISC,
__I40E_SUSPENDED,
__I40E_PTP_TX_IN_PROGRESS,
__I40E_BAD_EEPROM,
Expand All @@ -149,6 +147,16 @@ enum i40e_state_t {
__I40E_VF_DISABLE,
};

/* VSI state flags */
enum i40e_vsi_state_t {
__I40E_VSI_DOWN,
__I40E_VSI_NEEDS_RESTART,
__I40E_VSI_SYNCING_FILTERS,
__I40E_VSI_OVERFLOW_PROMISC,
__I40E_VSI_REINIT_REQUESTED,
__I40E_VSI_DOWN_REQUESTED,
};

enum i40e_interrupt_policy {
I40E_INTERRUPT_BEST_CASE,
I40E_INTERRUPT_MEDIUM,
Expand Down
4 changes: 2 additions & 2 deletions drivers/net/ethernet/intel/i40e/i40e_client.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ void i40e_client_subtask(struct i40e_pf *pf)
* the netdev is up, then open the client.
*/
if (!test_bit(__I40E_CLIENT_INSTANCE_OPENED, &cdev->state)) {
if (!test_bit(__I40E_DOWN, &vsi->state) &&
if (!test_bit(__I40E_VSI_DOWN, &vsi->state) &&
client->ops && client->ops->open) {
set_bit(__I40E_CLIENT_INSTANCE_OPENED, &cdev->state);
ret = client->ops->open(&cdev->lan_info, client);
Expand All @@ -397,7 +397,7 @@ void i40e_client_subtask(struct i40e_pf *pf)
/* Likewise for client close. If the client is up, but the netdev
* is down, then close the client.
*/
if (test_bit(__I40E_DOWN, &vsi->state) &&
if (test_bit(__I40E_VSI_DOWN, &vsi->state) &&
client->ops && client->ops->close) {
clear_bit(__I40E_CLIENT_INSTANCE_OPENED, &cdev->state);
client->ops->close(&cdev->lan_info, client, false);
Expand Down
4 changes: 2 additions & 2 deletions drivers/net/ethernet/intel/i40e/i40e_debugfs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ static void i40e_dbg_dump_vsi_seid(struct i40e_pf *pf, int seid)
}
dev_info(&pf->pdev->dev, " active_filters %u, promisc_threshold %u, overflow promisc %s\n",
vsi->active_filters, vsi->promisc_threshold,
(test_bit(__I40E_FILTER_OVERFLOW_PROMISC, &vsi->state) ?
(test_bit(__I40E_VSI_OVERFLOW_PROMISC, &vsi->state) ?
"ON" : "OFF"));
nstat = i40e_get_vsi_stats_struct(vsi);
dev_info(&pf->pdev->dev,
Expand Down Expand Up @@ -1706,7 +1706,7 @@ static ssize_t i40e_dbg_netdev_ops_write(struct file *filp,
} else if (!vsi->netdev) {
dev_info(&pf->pdev->dev, "tx_timeout: no netdev for VSI %d\n",
vsi_seid);
} else if (test_bit(__I40E_DOWN, &vsi->state)) {
} else if (test_bit(__I40E_VSI_DOWN, &vsi->state)) {
dev_info(&pf->pdev->dev, "tx_timeout: VSI %d not UP\n",
vsi_seid);
} else if (rtnl_trylock()) {
Expand Down
66 changes: 32 additions & 34 deletions drivers/net/ethernet/intel/i40e/i40e_main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ static void i40e_get_netdev_stats_struct(struct net_device *netdev,
struct rtnl_link_stats64 *vsi_stats = i40e_get_vsi_stats_struct(vsi);
int i;

if (test_bit(__I40E_DOWN, &vsi->state))
if (test_bit(__I40E_VSI_DOWN, &vsi->state))
return;

if (!vsi->tx_rings)
Expand Down Expand Up @@ -753,7 +753,7 @@ static void i40e_update_vsi_stats(struct i40e_vsi *vsi)
u64 tx_p, tx_b;
u16 q;

if (test_bit(__I40E_DOWN, &vsi->state) ||
if (test_bit(__I40E_VSI_DOWN, &vsi->state) ||
test_bit(__I40E_CONFIG_BUSY, &pf->state))
return;

Expand Down Expand Up @@ -1346,7 +1346,7 @@ struct i40e_mac_filter *i40e_add_filter(struct i40e_vsi *vsi,
* to failed, so we don't bother to try sending the filter
* to the hardware.
*/
if (test_bit(__I40E_FILTER_OVERFLOW_PROMISC, &vsi->state))
if (test_bit(__I40E_VSI_OVERFLOW_PROMISC, &vsi->state))
f->state = I40E_FILTER_FAILED;
else
f->state = I40E_FILTER_NEW;
Expand Down Expand Up @@ -1525,7 +1525,7 @@ static int i40e_set_mac(struct net_device *netdev, void *p)
return 0;
}

if (test_bit(__I40E_DOWN, &vsi->back->state) ||
if (test_bit(__I40E_VSI_DOWN, &vsi->back->state) ||
test_bit(__I40E_RESET_RECOVERY_PENDING, &vsi->back->state))
return -EADDRNOTAVAIL;

Expand Down Expand Up @@ -1920,7 +1920,7 @@ void i40e_aqc_add_filters(struct i40e_vsi *vsi, const char *vsi_name,

if (fcnt != num_add) {
*promisc_changed = true;
set_bit(__I40E_FILTER_OVERFLOW_PROMISC, &vsi->state);
set_bit(__I40E_VSI_OVERFLOW_PROMISC, &vsi->state);
dev_warn(&vsi->back->pdev->dev,
"Error %s adding RX filters on %s, promiscuous mode forced on\n",
i40e_aq_str(hw, aq_err),
Expand Down Expand Up @@ -2003,7 +2003,7 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
struct i40e_aqc_add_macvlan_element_data *add_list;
struct i40e_aqc_remove_macvlan_element_data *del_list;

while (test_and_set_bit(__I40E_CONFIG_BUSY, &vsi->state))
while (test_and_set_bit(__I40E_VSI_SYNCING_FILTERS, &vsi->state))
usleep_range(1000, 2000);
pf = vsi->back;

Expand Down Expand Up @@ -2139,7 +2139,7 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)

num_add = 0;
hlist_for_each_entry_safe(new, h, &tmp_add_list, hlist) {
if (test_bit(__I40E_FILTER_OVERFLOW_PROMISC,
if (test_bit(__I40E_VSI_OVERFLOW_PROMISC,
&vsi->state)) {
new->state = I40E_FILTER_FAILED;
continue;
Expand Down Expand Up @@ -2227,20 +2227,20 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
* safely exit if we didn't just enter, we no longer have any failed
* filters, and we have reduced filters below the threshold value.
*/
if (test_bit(__I40E_FILTER_OVERFLOW_PROMISC, &vsi->state) &&
if (test_bit(__I40E_VSI_OVERFLOW_PROMISC, &vsi->state) &&
!promisc_changed && !failed_filters &&
(vsi->active_filters < vsi->promisc_threshold)) {
dev_info(&pf->pdev->dev,
"filter logjam cleared on %s, leaving overflow promiscuous mode\n",
vsi_name);
clear_bit(__I40E_FILTER_OVERFLOW_PROMISC, &vsi->state);
clear_bit(__I40E_VSI_OVERFLOW_PROMISC, &vsi->state);
promisc_changed = true;
vsi->promisc_threshold = 0;
}

/* if the VF is not trusted do not do promisc */
if ((vsi->type == I40E_VSI_SRIOV) && !pf->vf[vsi->vf_id].trusted) {
clear_bit(__I40E_FILTER_OVERFLOW_PROMISC, &vsi->state);
clear_bit(__I40E_VSI_OVERFLOW_PROMISC, &vsi->state);
goto out;
}

Expand All @@ -2265,11 +2265,11 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
}
if ((changed_flags & IFF_PROMISC) ||
(promisc_changed &&
test_bit(__I40E_FILTER_OVERFLOW_PROMISC, &vsi->state))) {
test_bit(__I40E_VSI_OVERFLOW_PROMISC, &vsi->state))) {
bool cur_promisc;

cur_promisc = (!!(vsi->current_netdev_flags & IFF_PROMISC) ||
test_bit(__I40E_FILTER_OVERFLOW_PROMISC,
test_bit(__I40E_VSI_OVERFLOW_PROMISC,
&vsi->state));
if ((vsi->type == I40E_VSI_MAIN) &&
(pf->lan_veb != I40E_NO_VEB) &&
Expand Down Expand Up @@ -2353,7 +2353,7 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
if (retval)
vsi->flags |= I40E_VSI_FLAG_FILTER_CHANGED;

clear_bit(__I40E_CONFIG_BUSY, &vsi->state);
clear_bit(__I40E_VSI_SYNCING_FILTERS, &vsi->state);
return retval;

err_no_memory:
Expand All @@ -2365,7 +2365,7 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
spin_unlock_bh(&vsi->mac_filter_hash_lock);

vsi->flags |= I40E_VSI_FLAG_FILTER_CHANGED;
clear_bit(__I40E_CONFIG_BUSY, &vsi->state);
clear_bit(__I40E_VSI_SYNCING_FILTERS, &vsi->state);
return -ENOMEM;
}

Expand Down Expand Up @@ -3907,7 +3907,7 @@ static void i40e_netpoll(struct net_device *netdev)
int i;

/* if interface is down do nothing */
if (test_bit(__I40E_DOWN, &vsi->state))
if (test_bit(__I40E_VSI_DOWN, &vsi->state))
return;

if (pf->flags & I40E_FLAG_MSIX_ENABLED) {
Expand Down Expand Up @@ -4436,7 +4436,7 @@ static void i40e_napi_disable_all(struct i40e_vsi *vsi)
static void i40e_vsi_close(struct i40e_vsi *vsi)
{
struct i40e_pf *pf = vsi->back;
if (!test_and_set_bit(__I40E_DOWN, &vsi->state))
if (!test_and_set_bit(__I40E_VSI_DOWN, &vsi->state))
i40e_down(vsi);
i40e_vsi_free_irq(vsi);
i40e_vsi_free_tx_resources(vsi);
Expand All @@ -4453,10 +4453,10 @@ static void i40e_vsi_close(struct i40e_vsi *vsi)
**/
static void i40e_quiesce_vsi(struct i40e_vsi *vsi)
{
if (test_bit(__I40E_DOWN, &vsi->state))
if (test_bit(__I40E_VSI_DOWN, &vsi->state))
return;

set_bit(__I40E_NEEDS_RESTART, &vsi->state);
set_bit(__I40E_VSI_NEEDS_RESTART, &vsi->state);
if (vsi->netdev && netif_running(vsi->netdev))
vsi->netdev->netdev_ops->ndo_stop(vsi->netdev);
else
Expand All @@ -4469,10 +4469,9 @@ static void i40e_quiesce_vsi(struct i40e_vsi *vsi)
**/
static void i40e_unquiesce_vsi(struct i40e_vsi *vsi)
{
if (!test_bit(__I40E_NEEDS_RESTART, &vsi->state))
if (!test_and_clear_bit(__I40E_VSI_NEEDS_RESTART, &vsi->state))
return;

clear_bit(__I40E_NEEDS_RESTART, &vsi->state);
if (vsi->netdev && netif_running(vsi->netdev))
vsi->netdev->netdev_ops->ndo_open(vsi->netdev);
else
Expand Down Expand Up @@ -4638,7 +4637,7 @@ static void i40e_detect_recover_hung(struct i40e_pf *pf)
return;

/* Make sure, VSI state is not DOWN/RECOVERY_PENDING */
if (test_bit(__I40E_DOWN, &vsi->back->state) ||
if (test_bit(__I40E_VSI_DOWN, &vsi->back->state) ||
test_bit(__I40E_RESET_RECOVERY_PENDING, &vsi->back->state))
return;

Expand Down Expand Up @@ -5354,7 +5353,7 @@ static int i40e_up_complete(struct i40e_vsi *vsi)
if (err)
return err;

clear_bit(__I40E_DOWN, &vsi->state);
clear_bit(__I40E_VSI_DOWN, &vsi->state);
i40e_napi_enable_all(vsi);
i40e_vsi_enable_irq(vsi);

Expand Down Expand Up @@ -5435,7 +5434,7 @@ void i40e_down(struct i40e_vsi *vsi)
int i;

/* It is assumed that the caller of this function
* sets the vsi->state __I40E_DOWN bit.
* sets the vsi->state __I40E_VSI_DOWN bit.
*/
if (vsi->netdev) {
netif_carrier_off(vsi->netdev);
Expand Down Expand Up @@ -5787,10 +5786,9 @@ void i40e_do_reset(struct i40e_pf *pf, u32 reset_flags, bool lock_acquired)
struct i40e_vsi *vsi = pf->vsi[v];

if (vsi != NULL &&
test_bit(__I40E_REINIT_REQUESTED, &vsi->state)) {
test_and_clear_bit(__I40E_VSI_REINIT_REQUESTED,
&vsi->state))
i40e_vsi_reinit_locked(pf->vsi[v]);
clear_bit(__I40E_REINIT_REQUESTED, &vsi->state);
}
}
} else if (reset_flags & BIT_ULL(__I40E_DOWN_REQUESTED)) {
int v;
Expand All @@ -5801,10 +5799,10 @@ void i40e_do_reset(struct i40e_pf *pf, u32 reset_flags, bool lock_acquired)
struct i40e_vsi *vsi = pf->vsi[v];

if (vsi != NULL &&
test_bit(__I40E_DOWN_REQUESTED, &vsi->state)) {
set_bit(__I40E_DOWN, &vsi->state);
test_and_clear_bit(__I40E_VSI_DOWN_REQUESTED,
&vsi->state)) {
set_bit(__I40E_VSI_DOWN, &vsi->state);
i40e_down(vsi);
clear_bit(__I40E_DOWN_REQUESTED, &vsi->state);
}
}
} else {
Expand Down Expand Up @@ -6223,7 +6221,7 @@ static void i40e_fdir_reinit_subtask(struct i40e_pf *pf)
**/
static void i40e_vsi_link_event(struct i40e_vsi *vsi, bool link_up)
{
if (!vsi || test_bit(__I40E_DOWN, &vsi->state))
if (!vsi || test_bit(__I40E_VSI_DOWN, &vsi->state))
return;

switch (vsi->type) {
Expand Down Expand Up @@ -6316,11 +6314,11 @@ static void i40e_link_event(struct i40e_pf *pf)

if (new_link == old_link &&
new_link_speed == old_link_speed &&
(test_bit(__I40E_DOWN, &vsi->state) ||
(test_bit(__I40E_VSI_DOWN, &vsi->state) ||
new_link == netif_carrier_ok(vsi->netdev)))
return;

if (!test_bit(__I40E_DOWN, &vsi->state))
if (!test_bit(__I40E_VSI_DOWN, &vsi->state))
i40e_print_link_message(vsi, new_link);

/* Notify the base of the switch tree connected to
Expand Down Expand Up @@ -7591,7 +7589,7 @@ static int i40e_vsi_mem_alloc(struct i40e_pf *pf, enum i40e_vsi_type type)
}
vsi->type = type;
vsi->back = pf;
set_bit(__I40E_DOWN, &vsi->state);
set_bit(__I40E_VSI_DOWN, &vsi->state);
vsi->flags = 0;
vsi->idx = vsi_idx;
vsi->int_rate_limit = 0;
Expand Down Expand Up @@ -9718,7 +9716,7 @@ static int i40e_add_vsi(struct i40e_vsi *vsi)
}

vsi->active_filters = 0;
clear_bit(__I40E_FILTER_OVERFLOW_PROMISC, &vsi->state);
clear_bit(__I40E_VSI_OVERFLOW_PROMISC, &vsi->state);
spin_lock_bh(&vsi->mac_filter_hash_lock);
/* If macvlan filters already exist, force them to get loaded */
hash_for_each_safe(vsi->mac_filter_hash, bkt, h, f, hlist) {
Expand Down
8 changes: 4 additions & 4 deletions drivers/net/ethernet/intel/i40e/i40e_txrx.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -850,7 +850,7 @@ static bool i40e_clean_tx_irq(struct i40e_vsi *vsi,

if (budget &&
((j / WB_STRIDE) == 0) && (j > 0) &&
!test_bit(__I40E_DOWN, &vsi->state) &&
!test_bit(__I40E_VSI_DOWN, &vsi->state) &&
(I40E_DESC_UNUSED(tx_ring) != tx_ring->count))
tx_ring->arm_wb = true;
}
Expand All @@ -868,7 +868,7 @@ static bool i40e_clean_tx_irq(struct i40e_vsi *vsi,
smp_mb();
if (__netif_subqueue_stopped(tx_ring->netdev,
tx_ring->queue_index) &&
!test_bit(__I40E_DOWN, &vsi->state)) {
!test_bit(__I40E_VSI_DOWN, &vsi->state)) {
netif_wake_subqueue(tx_ring->netdev,
tx_ring->queue_index);
++tx_ring->tx_stats.restart_queue;
Expand Down Expand Up @@ -2179,7 +2179,7 @@ static inline void i40e_update_enable_itr(struct i40e_vsi *vsi,
}

enable_int:
if (!test_bit(__I40E_DOWN, &vsi->state))
if (!test_bit(__I40E_VSI_DOWN, &vsi->state))
wr32(hw, INTREG(vector - 1), txval);

if (q_vector->itr_countdown)
Expand Down Expand Up @@ -2208,7 +2208,7 @@ int i40e_napi_poll(struct napi_struct *napi, int budget)
int budget_per_ring;
int work_done = 0;

if (test_bit(__I40E_DOWN, &vsi->state)) {
if (test_bit(__I40E_VSI_DOWN, &vsi->state)) {
napi_complete(napi);
return 0;
}
Expand Down
8 changes: 4 additions & 4 deletions drivers/net/ethernet/intel/i40evf/i40e_txrx.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ static bool i40e_clean_tx_irq(struct i40e_vsi *vsi,

if (budget &&
((j / WB_STRIDE) == 0) && (j > 0) &&
!test_bit(__I40E_DOWN, &vsi->state) &&
!test_bit(__I40E_VSI_DOWN, &vsi->state) &&
(I40E_DESC_UNUSED(tx_ring) != tx_ring->count))
tx_ring->arm_wb = true;
}
Expand All @@ -284,7 +284,7 @@ static bool i40e_clean_tx_irq(struct i40e_vsi *vsi,
smp_mb();
if (__netif_subqueue_stopped(tx_ring->netdev,
tx_ring->queue_index) &&
!test_bit(__I40E_DOWN, &vsi->state)) {
!test_bit(__I40E_VSI_DOWN, &vsi->state)) {
netif_wake_subqueue(tx_ring->netdev,
tx_ring->queue_index);
++tx_ring->tx_stats.restart_queue;
Expand Down Expand Up @@ -1508,7 +1508,7 @@ static inline void i40e_update_enable_itr(struct i40e_vsi *vsi,
}

enable_int:
if (!test_bit(__I40E_DOWN, &vsi->state))
if (!test_bit(__I40E_VSI_DOWN, &vsi->state))
wr32(hw, INTREG(vector - 1), txval);

if (q_vector->itr_countdown)
Expand Down Expand Up @@ -1537,7 +1537,7 @@ int i40evf_napi_poll(struct napi_struct *napi, int budget)
int budget_per_ring;
int work_done = 0;

if (test_bit(__I40E_DOWN, &vsi->state)) {
if (test_bit(__I40E_VSI_DOWN, &vsi->state)) {
napi_complete(napi);
return 0;
}
Expand Down
Loading

0 comments on commit d19cb64

Please sign in to comment.