Skip to content

Commit

Permalink
Merge branch '40GbE' of git://git.kernel.org/pub/scm/linux/kernel/git…
Browse files Browse the repository at this point in the history 
…/tnguy/net-queue

Tony Nguyen says:

====================
iavf: fix racing in VLANs

Ahmed Zaki says:

This patchset mainly fixes a racing issue in the iavf where the number of
VLANs in the vlan_filter_list might be more than the PF limit. To fix that,
we get rid of the cvlans and svlans bitmaps and keep all the required info
in the list.

The second patch adds two new states that are needed so that we keep the
VLAN info while the interface goes DOWN:
    -- DISABLE    (notify PF, but keep the filter in the list)
    -- INACTIVE   (dev is DOWN, filter is removed from PF)

Finally, the current code keeps each state in a separate bit field, which
is error prone. The first patch refactors that by replacing all bits with
a single enum. The changes are minimal where each bit change is replaced
with the new state value.

* '40GbE' of git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/net-queue:
  iavf: remove active_cvlans and active_svlans bitmaps
  iavf: refactor VLAN filter states
====================

Link: https://lore.kernel.org/r/20230407210730.3046149-1-anthony.l.nguyen@intel.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
Jakub Kicinski committed Apr 12, 2023
2 parents 160c131 + 9c85b7f commit adacf21
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 66 deletions.
20 changes: 12 additions & 8 deletions drivers/net/ethernet/intel/iavf/iavf.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,6 @@ enum iavf_vsi_state_t {
struct iavf_vsi {
struct iavf_adapter *back;
struct net_device *netdev;
unsigned long active_cvlans[BITS_TO_LONGS(VLAN_N_VID)];
unsigned long active_svlans[BITS_TO_LONGS(VLAN_N_VID)];
u16 seid;
u16 id;
DECLARE_BITMAP(state, __IAVF_VSI_STATE_SIZE__);
Expand Down Expand Up @@ -158,15 +156,20 @@ struct iavf_vlan {
u16 tpid;
};

enum iavf_vlan_state_t {
IAVF_VLAN_INVALID,
IAVF_VLAN_ADD, /* filter needs to be added */
IAVF_VLAN_IS_NEW, /* filter is new, wait for PF answer */
IAVF_VLAN_ACTIVE, /* filter is accepted by PF */
IAVF_VLAN_DISABLE, /* filter needs to be deleted by PF, then marked INACTIVE */
IAVF_VLAN_INACTIVE, /* filter is inactive, we are in IFF_DOWN */
IAVF_VLAN_REMOVE, /* filter needs to be removed from list */
};

struct iavf_vlan_filter {
struct list_head list;
struct iavf_vlan vlan;
struct {
u8 is_new_vlan:1; /* filter is new, wait for PF answer */
u8 remove:1; /* filter needs to be removed */
u8 add:1; /* filter needs to be added */
u8 padding:5;
};
enum iavf_vlan_state_t state;
};

#define IAVF_MAX_TRAFFIC_CLASS 4
Expand Down Expand Up @@ -258,6 +261,7 @@ struct iavf_adapter {
wait_queue_head_t vc_waitqueue;
struct iavf_q_vector *q_vectors;
struct list_head vlan_filter_list;
int num_vlan_filters;
struct list_head mac_filter_list;
struct mutex crit_lock;
struct mutex client_lock;
Expand Down
44 changes: 18 additions & 26 deletions drivers/net/ethernet/intel/iavf/iavf_main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -791,7 +791,8 @@ iavf_vlan_filter *iavf_add_vlan(struct iavf_adapter *adapter,
f->vlan = vlan;

list_add_tail(&f->list, &adapter->vlan_filter_list);
f->add = true;
f->state = IAVF_VLAN_ADD;
adapter->num_vlan_filters++;
adapter->aq_required |= IAVF_FLAG_AQ_ADD_VLAN_FILTER;
}

Expand All @@ -813,7 +814,7 @@ static void iavf_del_vlan(struct iavf_adapter *adapter, struct iavf_vlan vlan)

f = iavf_find_vlan(adapter, vlan);
if (f) {
f->remove = true;
f->state = IAVF_VLAN_REMOVE;
adapter->aq_required |= IAVF_FLAG_AQ_DEL_VLAN_FILTER;
}

Expand All @@ -828,14 +829,18 @@ static void iavf_del_vlan(struct iavf_adapter *adapter, struct iavf_vlan vlan)
**/
static void iavf_restore_filters(struct iavf_adapter *adapter)
{
u16 vid;
struct iavf_vlan_filter *f;

/* re-add all VLAN filters */
for_each_set_bit(vid, adapter->vsi.active_cvlans, VLAN_N_VID)
iavf_add_vlan(adapter, IAVF_VLAN(vid, ETH_P_8021Q));
spin_lock_bh(&adapter->mac_vlan_list_lock);

for_each_set_bit(vid, adapter->vsi.active_svlans, VLAN_N_VID)
iavf_add_vlan(adapter, IAVF_VLAN(vid, ETH_P_8021AD));
list_for_each_entry(f, &adapter->vlan_filter_list, list) {
if (f->state == IAVF_VLAN_INACTIVE)
f->state = IAVF_VLAN_ADD;
}

spin_unlock_bh(&adapter->mac_vlan_list_lock);
adapter->aq_required |= IAVF_FLAG_AQ_ADD_VLAN_FILTER;
}

/**
Expand All @@ -844,8 +849,7 @@ static void iavf_restore_filters(struct iavf_adapter *adapter)
*/
u16 iavf_get_num_vlans_added(struct iavf_adapter *adapter)
{
return bitmap_weight(adapter->vsi.active_cvlans, VLAN_N_VID) +
bitmap_weight(adapter->vsi.active_svlans, VLAN_N_VID);
return adapter->num_vlan_filters;
}

/**
Expand Down Expand Up @@ -928,11 +932,6 @@ static int iavf_vlan_rx_kill_vid(struct net_device *netdev,
return 0;

iavf_del_vlan(adapter, IAVF_VLAN(vid, be16_to_cpu(proto)));
if (proto == cpu_to_be16(ETH_P_8021Q))
clear_bit(vid, adapter->vsi.active_cvlans);
else
clear_bit(vid, adapter->vsi.active_svlans);

return 0;
}

Expand Down Expand Up @@ -1293,16 +1292,11 @@ static void iavf_clear_mac_vlan_filters(struct iavf_adapter *adapter)
}
}

/* remove all VLAN filters */
/* disable all VLAN filters */
list_for_each_entry_safe(vlf, vlftmp, &adapter->vlan_filter_list,
list) {
if (vlf->add) {
list_del(&vlf->list);
kfree(vlf);
} else {
vlf->remove = true;
}
}
list)
vlf->state = IAVF_VLAN_DISABLE;

spin_unlock_bh(&adapter->mac_vlan_list_lock);
}

Expand Down Expand Up @@ -2914,6 +2908,7 @@ static void iavf_disable_vf(struct iavf_adapter *adapter)
list_del(&fv->list);
kfree(fv);
}
adapter->num_vlan_filters = 0;

spin_unlock_bh(&adapter->mac_vlan_list_lock);

Expand Down Expand Up @@ -3131,9 +3126,6 @@ static void iavf_reset_task(struct work_struct *work)
adapter->aq_required |= IAVF_FLAG_AQ_ADD_CLOUD_FILTER;
iavf_misc_irq_enable(adapter);

bitmap_clear(adapter->vsi.active_cvlans, 0, VLAN_N_VID);
bitmap_clear(adapter->vsi.active_svlans, 0, VLAN_N_VID);

mod_delayed_work(adapter->wq, &adapter->watchdog_task, 2);

/* We were running when the reset started, so we need to restore some
Expand Down
68 changes: 36 additions & 32 deletions drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -642,16 +642,10 @@ static void iavf_vlan_add_reject(struct iavf_adapter *adapter)

spin_lock_bh(&adapter->mac_vlan_list_lock);
list_for_each_entry_safe(f, ftmp, &adapter->vlan_filter_list, list) {
if (f->is_new_vlan) {
if (f->vlan.tpid == ETH_P_8021Q)
clear_bit(f->vlan.vid,
adapter->vsi.active_cvlans);
else
clear_bit(f->vlan.vid,
adapter->vsi.active_svlans);

if (f->state == IAVF_VLAN_IS_NEW) {
list_del(&f->list);
kfree(f);
adapter->num_vlan_filters--;
}
}
spin_unlock_bh(&adapter->mac_vlan_list_lock);
Expand Down Expand Up @@ -679,7 +673,7 @@ void iavf_add_vlans(struct iavf_adapter *adapter)
spin_lock_bh(&adapter->mac_vlan_list_lock);

list_for_each_entry(f, &adapter->vlan_filter_list, list) {
if (f->add)
if (f->state == IAVF_VLAN_ADD)
count++;
}
if (!count || !VLAN_FILTERING_ALLOWED(adapter)) {
Expand Down Expand Up @@ -710,11 +704,10 @@ void iavf_add_vlans(struct iavf_adapter *adapter)
vvfl->vsi_id = adapter->vsi_res->vsi_id;
vvfl->num_elements = count;
list_for_each_entry(f, &adapter->vlan_filter_list, list) {
if (f->add) {
if (f->state == IAVF_VLAN_ADD) {
vvfl->vlan_id[i] = f->vlan.vid;
i++;
f->add = false;
f->is_new_vlan = true;
f->state = IAVF_VLAN_IS_NEW;
if (i == count)
break;
}
Expand Down Expand Up @@ -760,7 +753,7 @@ void iavf_add_vlans(struct iavf_adapter *adapter)
vvfl_v2->vport_id = adapter->vsi_res->vsi_id;
vvfl_v2->num_elements = count;
list_for_each_entry(f, &adapter->vlan_filter_list, list) {
if (f->add) {
if (f->state == IAVF_VLAN_ADD) {
struct virtchnl_vlan_supported_caps *filtering_support =
&adapter->vlan_v2_caps.filtering.filtering_support;
struct virtchnl_vlan *vlan;
Expand All @@ -778,8 +771,7 @@ void iavf_add_vlans(struct iavf_adapter *adapter)
vlan->tpid = f->vlan.tpid;

i++;
f->add = false;
f->is_new_vlan = true;
f->state = IAVF_VLAN_IS_NEW;
}
}

Expand Down Expand Up @@ -822,10 +814,16 @@ void iavf_del_vlans(struct iavf_adapter *adapter)
* filters marked for removal to enable bailing out before
* sending a virtchnl message
*/
if (f->remove && !VLAN_FILTERING_ALLOWED(adapter)) {
if (f->state == IAVF_VLAN_REMOVE &&
!VLAN_FILTERING_ALLOWED(adapter)) {
list_del(&f->list);
kfree(f);
} else if (f->remove) {
adapter->num_vlan_filters--;
} else if (f->state == IAVF_VLAN_DISABLE &&
!VLAN_FILTERING_ALLOWED(adapter)) {
f->state = IAVF_VLAN_INACTIVE;
} else if (f->state == IAVF_VLAN_REMOVE ||
f->state == IAVF_VLAN_DISABLE) {
count++;
}
}
Expand Down Expand Up @@ -857,11 +855,18 @@ void iavf_del_vlans(struct iavf_adapter *adapter)
vvfl->vsi_id = adapter->vsi_res->vsi_id;
vvfl->num_elements = count;
list_for_each_entry_safe(f, ftmp, &adapter->vlan_filter_list, list) {
if (f->remove) {
if (f->state == IAVF_VLAN_DISABLE) {
vvfl->vlan_id[i] = f->vlan.vid;
f->state = IAVF_VLAN_INACTIVE;
i++;
if (i == count)
break;
} else if (f->state == IAVF_VLAN_REMOVE) {
vvfl->vlan_id[i] = f->vlan.vid;
list_del(&f->list);
kfree(f);
adapter->num_vlan_filters--;
i++;
if (i == count)
break;
}
Expand Down Expand Up @@ -901,7 +906,8 @@ void iavf_del_vlans(struct iavf_adapter *adapter)
vvfl_v2->vport_id = adapter->vsi_res->vsi_id;
vvfl_v2->num_elements = count;
list_for_each_entry_safe(f, ftmp, &adapter->vlan_filter_list, list) {
if (f->remove) {
if (f->state == IAVF_VLAN_DISABLE ||
f->state == IAVF_VLAN_REMOVE) {
struct virtchnl_vlan_supported_caps *filtering_support =
&adapter->vlan_v2_caps.filtering.filtering_support;
struct virtchnl_vlan *vlan;
Expand All @@ -915,8 +921,13 @@ void iavf_del_vlans(struct iavf_adapter *adapter)
vlan->tci = f->vlan.vid;
vlan->tpid = f->vlan.tpid;

list_del(&f->list);
kfree(f);
if (f->state == IAVF_VLAN_DISABLE) {
f->state = IAVF_VLAN_INACTIVE;
} else {
list_del(&f->list);
kfree(f);
adapter->num_vlan_filters--;
}
i++;
if (i == count)
break;
Expand Down Expand Up @@ -2192,7 +2203,7 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter,
list_for_each_entry(vlf,
&adapter->vlan_filter_list,
list)
vlf->add = true;
vlf->state = IAVF_VLAN_ADD;

adapter->aq_required |=
IAVF_FLAG_AQ_ADD_VLAN_FILTER;
Expand Down Expand Up @@ -2260,7 +2271,7 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter,
list_for_each_entry(vlf,
&adapter->vlan_filter_list,
list)
vlf->add = true;
vlf->state = IAVF_VLAN_ADD;

aq_required |= IAVF_FLAG_AQ_ADD_VLAN_FILTER;
}
Expand Down Expand Up @@ -2444,15 +2455,8 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter,

spin_lock_bh(&adapter->mac_vlan_list_lock);
list_for_each_entry(f, &adapter->vlan_filter_list, list) {
if (f->is_new_vlan) {
f->is_new_vlan = false;
if (f->vlan.tpid == ETH_P_8021Q)
set_bit(f->vlan.vid,
adapter->vsi.active_cvlans);
else
set_bit(f->vlan.vid,
adapter->vsi.active_svlans);
}
if (f->state == IAVF_VLAN_IS_NEW)
f->state = IAVF_VLAN_ACTIVE;
}
spin_unlock_bh(&adapter->mac_vlan_list_lock);
}
Expand Down

0 comments on commit adacf21

Please sign in to comment.