From 0ccb998bf46d8dc9799b60ea0090fb08c37e96ad Mon Sep 17 00:00:00 2001 From: Jon Cooper Date: Fri, 17 Feb 2017 15:49:13 +0000 Subject: [PATCH 1/3] sfc: fix filter_id misinterpretation in edge case On EF10, hardware filter IDs are 13 bits, but in some places we store 32-bit "full filter IDs" in which higher order bits encode the filter match-priority. This could cause a filter to have a full filter ID of 0xffff, which is also the value EFX_EF10_FILTER_ID_INVALID which we use in 16-bit "short" filter IDs (without match-priority bits). This would occur if the hardware filter ID was 0x1fff and the match-priority was 7. Unfortunately, some code that checks for EFX_EF10_FILTER_ID_INVALID can be called on full filter IDs, and will WARN_ON if this ever happens. So, since we have plenty of spare bits in the full filter ID, this patch shifts the priority bits left one bit when constructing the full filter IDs, ensuring that the 0x2000 bit of a full filter ID will always be 0 and thus no full filter ID can ever equal EFX_EF10_FILTER_ID_INVALID. This patch also replaces open-coded full<->short filter ID conversions with calls to functions, thus keeping the definition of the full filter ID format in one place. Signed-off-by: Edward Cree Signed-off-by: David S. Miller --- drivers/net/ethernet/sfc/ef10.c | 48 ++++++++++++++++++++------------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c index b29a819f721df..7da96998488af 100644 --- a/drivers/net/ethernet/sfc/ef10.c +++ b/drivers/net/ethernet/sfc/ef10.c @@ -134,6 +134,22 @@ static void efx_ef10_filter_del_vlan_internal(struct efx_nic *efx, static void efx_ef10_filter_del_vlan(struct efx_nic *efx, u16 vid); static int efx_ef10_set_udp_tnl_ports(struct efx_nic *efx, bool unloading); +static u32 efx_ef10_filter_get_unsafe_id(u32 filter_id) +{ + WARN_ON_ONCE(filter_id == EFX_EF10_FILTER_ID_INVALID); + return filter_id & (HUNT_FILTER_TBL_ROWS - 1); +} + +static unsigned int efx_ef10_filter_get_unsafe_pri(u32 filter_id) +{ + return filter_id / (HUNT_FILTER_TBL_ROWS * 2); +} + +static u32 efx_ef10_make_filter_id(unsigned int pri, u16 idx) +{ + return pri * HUNT_FILTER_TBL_ROWS * 2 + idx; +} + static int efx_ef10_get_warm_boot_count(struct efx_nic *efx) { efx_dword_t reg; @@ -4194,7 +4210,7 @@ static s32 efx_ef10_filter_insert(struct efx_nic *efx, /* If successful, return the inserted filter ID */ if (rc == 0) - rc = match_pri * HUNT_FILTER_TBL_ROWS + ins_index; + rc = efx_ef10_make_filter_id(match_pri, ins_index); wake_up_all(&table->waitq); out_unlock: @@ -4217,7 +4233,7 @@ static int efx_ef10_filter_remove_internal(struct efx_nic *efx, unsigned int priority_mask, u32 filter_id, bool by_index) { - unsigned int filter_idx = filter_id % HUNT_FILTER_TBL_ROWS; + unsigned int filter_idx = efx_ef10_filter_get_unsafe_id(filter_id); struct efx_ef10_filter_table *table = efx->filter_state; MCDI_DECLARE_BUF(inbuf, MC_CMD_FILTER_OP_IN_HANDLE_OFST + @@ -4244,7 +4260,7 @@ static int efx_ef10_filter_remove_internal(struct efx_nic *efx, if (!spec || (!by_index && efx_ef10_filter_pri(table, spec) != - filter_id / HUNT_FILTER_TBL_ROWS)) { + efx_ef10_filter_get_unsafe_pri(filter_id))) { rc = -ENOENT; goto out_unlock; } @@ -4319,11 +4335,6 @@ static int efx_ef10_filter_remove_safe(struct efx_nic *efx, filter_id, false); } -static u32 efx_ef10_filter_get_unsafe_id(struct efx_nic *efx, u32 filter_id) -{ - return filter_id % HUNT_FILTER_TBL_ROWS; -} - static void efx_ef10_filter_remove_unsafe(struct efx_nic *efx, enum efx_filter_priority priority, u32 filter_id) @@ -4337,7 +4348,7 @@ static int efx_ef10_filter_get_safe(struct efx_nic *efx, enum efx_filter_priority priority, u32 filter_id, struct efx_filter_spec *spec) { - unsigned int filter_idx = filter_id % HUNT_FILTER_TBL_ROWS; + unsigned int filter_idx = efx_ef10_filter_get_unsafe_id(filter_id); struct efx_ef10_filter_table *table = efx->filter_state; const struct efx_filter_spec *saved_spec; int rc; @@ -4346,7 +4357,7 @@ static int efx_ef10_filter_get_safe(struct efx_nic *efx, saved_spec = efx_ef10_filter_entry_spec(table, filter_idx); if (saved_spec && saved_spec->priority == priority && efx_ef10_filter_pri(table, saved_spec) == - filter_id / HUNT_FILTER_TBL_ROWS) { + efx_ef10_filter_get_unsafe_pri(filter_id)) { *spec = *saved_spec; rc = 0; } else { @@ -4398,7 +4409,7 @@ static u32 efx_ef10_filter_get_rx_id_limit(struct efx_nic *efx) { struct efx_ef10_filter_table *table = efx->filter_state; - return table->rx_match_count * HUNT_FILTER_TBL_ROWS; + return table->rx_match_count * HUNT_FILTER_TBL_ROWS * 2; } static s32 efx_ef10_filter_get_rx_ids(struct efx_nic *efx, @@ -4418,8 +4429,9 @@ static s32 efx_ef10_filter_get_rx_ids(struct efx_nic *efx, count = -EMSGSIZE; break; } - buf[count++] = (efx_ef10_filter_pri(table, spec) * - HUNT_FILTER_TBL_ROWS + + buf[count++] = + efx_ef10_make_filter_id( + efx_ef10_filter_pri(table, spec), filter_idx); } } @@ -4971,7 +4983,7 @@ static void efx_ef10_filter_mark_one_old(struct efx_nic *efx, uint16_t *id) unsigned int filter_idx; if (*id != EFX_EF10_FILTER_ID_INVALID) { - filter_idx = efx_ef10_filter_get_unsafe_id(efx, *id); + filter_idx = efx_ef10_filter_get_unsafe_id(*id); if (!table->entry[filter_idx].spec) netif_dbg(efx, drv, efx->net_dev, "marked null spec old %04x:%04x\n", *id, @@ -5106,7 +5118,7 @@ static int efx_ef10_filter_insert_addr_list(struct efx_nic *efx, rc = EFX_EF10_FILTER_ID_INVALID; } } - ids[i] = efx_ef10_filter_get_unsafe_id(efx, rc); + ids[i] = efx_ef10_filter_get_unsafe_id(rc); } if (multicast && rollback) { @@ -5130,7 +5142,7 @@ static int efx_ef10_filter_insert_addr_list(struct efx_nic *efx, return rc; } else { vlan->default_filters[EFX_EF10_BCAST] = - efx_ef10_filter_get_unsafe_id(efx, rc); + efx_ef10_filter_get_unsafe_id(rc); } } @@ -5225,7 +5237,7 @@ static int efx_ef10_filter_insert_def(struct efx_nic *efx, id = &vlan->default_filters[map[encap_type]]; EFX_WARN_ON_PARANOID(*id != EFX_EF10_FILTER_ID_INVALID); - *id = efx_ef10_filter_get_unsafe_id(efx, rc); + *id = efx_ef10_filter_get_unsafe_id(rc); if (!nic_data->workaround_26807 && !encap_type) { /* Also need an Ethernet broadcast filter */ efx_filter_init_rx(&spec, EFX_FILTER_PRI_AUTO, @@ -5250,7 +5262,7 @@ static int efx_ef10_filter_insert_def(struct efx_nic *efx, vlan->default_filters[EFX_EF10_BCAST] != EFX_EF10_FILTER_ID_INVALID); vlan->default_filters[EFX_EF10_BCAST] = - efx_ef10_filter_get_unsafe_id(efx, rc); + efx_ef10_filter_get_unsafe_id(rc); } } rc = 0; From 105eac6c35a168b8b6d8e594830a1da5585b260d Mon Sep 17 00:00:00 2001 From: Bert Kenward Date: Fri, 17 Feb 2017 15:50:12 +0000 Subject: [PATCH 2/3] sfc: forget filters from sw table if hw replies ENOENT on removing them If the hw doesn't think they exist, we should defer to its authority. Signed-off-by: Edward Cree Signed-off-by: David S. Miller --- drivers/net/ethernet/sfc/ef10.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c index 7da96998488af..ec976ffb273f7 100644 --- a/drivers/net/ethernet/sfc/ef10.c +++ b/drivers/net/ethernet/sfc/ef10.c @@ -4309,13 +4309,18 @@ static int efx_ef10_filter_remove_internal(struct efx_nic *efx, MC_CMD_FILTER_OP_IN_OP_UNSUBSCRIBE); MCDI_SET_QWORD(inbuf, FILTER_OP_IN_HANDLE, table->entry[filter_idx].handle); - rc = efx_mcdi_rpc(efx, MC_CMD_FILTER_OP, - inbuf, sizeof(inbuf), NULL, 0, NULL); + rc = efx_mcdi_rpc_quiet(efx, MC_CMD_FILTER_OP, + inbuf, sizeof(inbuf), NULL, 0, NULL); spin_lock_bh(&efx->filter_lock); - if (rc == 0) { + if ((rc == 0) || (rc == -ENOENT)) { + /* Filter removed OK or didn't actually exist */ kfree(spec); efx_ef10_filter_set_entry(table, filter_idx, NULL, 0); + } else { + efx_mcdi_display_error(efx, MC_CMD_FILTER_OP, + MC_CMD_FILTER_OP_IN_LEN, + NULL, 0, rc); } } From 9c568fd8844ec3986eb19b0b5d97536243d10d46 Mon Sep 17 00:00:00 2001 From: Peter Dunning Date: Fri, 17 Feb 2017 15:50:43 +0000 Subject: [PATCH 3/3] sfc: do not device_attach if a reset is pending efx_start_all can return without initialising queues as a reset is pending. This means that when netif_device_attach is called, the kernel can start sending traffic without having an initialised TX queue to send to. This patch avoids this by not calling netif_device_attach if there is a pending reset. Fixes: e283546c0465 ("sfc:On MCDI timeout, issue an FLR (and mark MCDI to fail-fast)") Signed-off-by: Edward Cree Signed-off-by: David S. Miller --- drivers/net/ethernet/sfc/ef10.c | 8 ++++---- drivers/net/ethernet/sfc/ef10_sriov.c | 4 ++-- drivers/net/ethernet/sfc/efx.c | 10 ++++++---- drivers/net/ethernet/sfc/efx.h | 6 ++++++ drivers/net/ethernet/sfc/selftest.c | 2 +- 5 files changed, 19 insertions(+), 11 deletions(-) diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c index ec976ffb273f7..92e1c6d8b2937 100644 --- a/drivers/net/ethernet/sfc/ef10.c +++ b/drivers/net/ethernet/sfc/ef10.c @@ -5389,7 +5389,7 @@ static int efx_ef10_vport_set_mac_address(struct efx_nic *efx) if (rc2) goto reset_nic; - netif_device_attach(efx->net_dev); + efx_device_attach_if_not_resetting(efx); return rc; @@ -5675,7 +5675,7 @@ static int efx_ef10_set_mac_address(struct efx_nic *efx) if (was_enabled) efx_net_open(efx->net_dev); - netif_device_attach(efx->net_dev); + efx_device_attach_if_not_resetting(efx); #ifdef CONFIG_SFC_SRIOV if (efx->pci_dev->is_virtfn && efx->pci_dev->physfn) { @@ -6127,7 +6127,7 @@ static int efx_ef10_set_udp_tnl_ports(struct efx_nic *efx, bool unloading) if (!(nic_data->datapath_caps & (1 << MC_CMD_GET_CAPABILITIES_OUT_VXLAN_NVGRE_LBN))) { - netif_device_attach(efx->net_dev); + efx_device_attach_if_not_resetting(efx); return 0; } @@ -6199,7 +6199,7 @@ static int efx_ef10_set_udp_tnl_ports(struct efx_nic *efx, bool unloading) * trigger a re-attach. Since there won't be an MC reset, we * have to do the attach ourselves. */ - netif_device_attach(efx->net_dev); + efx_device_attach_if_not_resetting(efx); } return rc; diff --git a/drivers/net/ethernet/sfc/ef10_sriov.c b/drivers/net/ethernet/sfc/ef10_sriov.c index ed4b142834613..b7e4345c990d5 100644 --- a/drivers/net/ethernet/sfc/ef10_sriov.c +++ b/drivers/net/ethernet/sfc/ef10_sriov.c @@ -549,7 +549,7 @@ int efx_ef10_sriov_set_vf_mac(struct efx_nic *efx, int vf_i, u8 *mac) vf->efx->type->filter_table_probe(vf->efx); up_write(&vf->efx->filter_sem); efx_net_open(vf->efx->net_dev); - netif_device_attach(vf->efx->net_dev); + efx_device_attach_if_not_resetting(vf->efx); } return 0; @@ -667,7 +667,7 @@ int efx_ef10_sriov_set_vf_vlan(struct efx_nic *efx, int vf_i, u16 vlan, if (rc2) goto reset_nic; - netif_device_attach(vf->efx->net_dev); + efx_device_attach_if_not_resetting(vf->efx); } return rc; diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c index 8c4c273643dc3..334bcc6df6b2b 100644 --- a/drivers/net/ethernet/sfc/efx.c +++ b/drivers/net/ethernet/sfc/efx.c @@ -876,7 +876,7 @@ efx_realloc_channels(struct efx_nic *efx, u32 rxq_entries, u32 txq_entries) efx_schedule_reset(efx, RESET_TYPE_DISABLE); } else { efx_start_all(efx); - netif_device_attach(efx->net_dev); + efx_device_attach_if_not_resetting(efx); } return rc; @@ -2182,6 +2182,8 @@ int efx_net_open(struct net_device *net_dev) efx_link_status_changed(efx); efx_start_all(efx); + if (efx->state == STATE_DISABLED || efx->reset_pending) + netif_device_detach(efx->net_dev); efx_selftest_async_start(efx); return 0; } @@ -2248,7 +2250,7 @@ static int efx_change_mtu(struct net_device *net_dev, int new_mtu) mutex_unlock(&efx->mac_lock); efx_start_all(efx); - netif_device_attach(efx->net_dev); + efx_device_attach_if_not_resetting(efx); return 0; } @@ -2744,7 +2746,7 @@ int efx_reset(struct efx_nic *efx, enum reset_type method) efx->state = STATE_DISABLED; } else { netif_dbg(efx, drv, efx->net_dev, "reset complete\n"); - netif_device_attach(efx->net_dev); + efx_device_attach_if_not_resetting(efx); } return rc; } @@ -3417,7 +3419,7 @@ static int efx_pm_thaw(struct device *dev) efx_start_all(efx); - netif_device_attach(efx->net_dev); + efx_device_attach_if_not_resetting(efx); efx->state = STATE_READY; diff --git a/drivers/net/ethernet/sfc/efx.h b/drivers/net/ethernet/sfc/efx.h index 342ae16e1f2dd..ee14662415c5d 100644 --- a/drivers/net/ethernet/sfc/efx.h +++ b/drivers/net/ethernet/sfc/efx.h @@ -276,6 +276,12 @@ static inline void efx_device_detach_sync(struct efx_nic *efx) netif_tx_unlock_bh(dev); } +static inline void efx_device_attach_if_not_resetting(struct efx_nic *efx) +{ + if ((efx->state != STATE_DISABLED) && !efx->reset_pending) + netif_device_attach(efx->net_dev); +} + static inline bool efx_rwsem_assert_write_locked(struct rw_semaphore *sem) { if (WARN_ON(down_read_trylock(sem))) { diff --git a/drivers/net/ethernet/sfc/selftest.c b/drivers/net/ethernet/sfc/selftest.c index cd38b44ae23af..dab286a337a6b 100644 --- a/drivers/net/ethernet/sfc/selftest.c +++ b/drivers/net/ethernet/sfc/selftest.c @@ -768,7 +768,7 @@ int efx_selftest(struct efx_nic *efx, struct efx_self_tests *tests, __efx_reconfigure_port(efx); mutex_unlock(&efx->mac_lock); - netif_device_attach(efx->net_dev); + efx_device_attach_if_not_resetting(efx); return rc_test; }