* [PATCH net 0/6][pull request] Intel Wired LAN Driver Updates 2024-11-04 (ice, idpf, i40e, e1000e)
@ 2024-11-04 22:36 Tony Nguyen
2024-11-04 22:36 ` [PATCH net 1/6] ice: Fix use after free during unload with ports in bridge Tony Nguyen
` (6 more replies)
0 siblings, 7 replies; 9+ messages in thread
From: Tony Nguyen @ 2024-11-04 22:36 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet, andrew+netdev, netdev; +Cc: Tony Nguyen
For ice:
Marcin adjusts ordering of calls in ice_eswitch_detach() to resolve a
use after free issue.
Mateusz corrects variable type for Flow Director queue to fix issues
related to drop actions.
For idpf:
Pavan resolves issues related to reset on idpf; avoiding use of freed
vport and correctly unrolling the mailbox task.
For i40e:
Aleksandr fixes a race condition involving addition and deletion of VF
MAC filters.
For e1000e:
Vitaly reverts workaround for Meteor Lake causing regressions in power
management flows.
The following are changes since commit 5ccdcdf186aec6b9111845fd37e1757e9b413e2f:
net: xilinx: axienet: Enqueue Tx packets in dql before dmaengine starts
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/net-queue 100GbE
Aleksandr Loktionov (1):
i40e: fix race condition by adding filter's intermediate sync state
Marcin Szycik (1):
ice: Fix use after free during unload with ports in bridge
Mateusz Polchlopek (1):
ice: change q_index variable type to s16 to store -1 value
Pavan Kumar Linga (2):
idpf: avoid vport access in idpf_get_link_ksettings
idpf: fix idpf_vc_core_init error path
Vitaly Lifshits (1):
e1000e: Remove Meteor Lake SMBUS workarounds
drivers/net/ethernet/intel/e1000e/ich8lan.c | 17 ++++-------------
drivers/net/ethernet/intel/i40e/i40e.h | 1 +
drivers/net/ethernet/intel/i40e/i40e_debugfs.c | 1 +
drivers/net/ethernet/intel/i40e/i40e_main.c | 12 ++++++++++--
drivers/net/ethernet/intel/ice/ice_eswitch.c | 3 ++-
.../net/ethernet/intel/ice/ice_ethtool_fdir.c | 3 ++-
drivers/net/ethernet/intel/ice/ice_fdir.h | 4 +++-
drivers/net/ethernet/intel/idpf/idpf.h | 4 ++--
drivers/net/ethernet/intel/idpf/idpf_ethtool.c | 11 +++--------
drivers/net/ethernet/intel/idpf/idpf_lib.c | 5 +++--
drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 3 +--
11 files changed, 32 insertions(+), 32 deletions(-)
--
2.42.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net 1/6] ice: Fix use after free during unload with ports in bridge
2024-11-04 22:36 [PATCH net 0/6][pull request] Intel Wired LAN Driver Updates 2024-11-04 (ice, idpf, i40e, e1000e) Tony Nguyen
@ 2024-11-04 22:36 ` Tony Nguyen
2024-11-04 22:36 ` [PATCH net 2/6] ice: change q_index variable type to s16 to store -1 value Tony Nguyen
` (5 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Tony Nguyen @ 2024-11-04 22:36 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet, andrew+netdev, netdev
Cc: Marcin Szycik, anthony.l.nguyen, michal.swiatkowski, Paul Menzel,
Sujai Buvaneswaran
From: Marcin Szycik <marcin.szycik@linux.intel.com>
Unloading the ice driver while switchdev port representors are added to
a bridge can lead to kernel panic. Reproducer:
modprobe ice
devlink dev eswitch set $PF1_PCI mode switchdev
ip link add $BR type bridge
ip link set $BR up
echo 2 > /sys/class/net/$PF1/device/sriov_numvfs
sleep 2
ip link set $PF1 master $BR
ip link set $VF1_PR master $BR
ip link set $VF2_PR master $BR
ip link set $PF1 up
ip link set $VF1_PR up
ip link set $VF2_PR up
ip link set $VF1 up
rmmod irdma ice
When unloading the driver, ice_eswitch_detach() is eventually called as
part of VF freeing. First, it removes a port representor from xarray,
then unregister_netdev() is called (via repr->ops.rem()), finally
representor is deallocated. The problem comes from the bridge doing its
own deinit at the same time. unregister_netdev() triggers a notifier
chain, resulting in ice_eswitch_br_port_deinit() being called. It should
set repr->br_port = NULL, but this does not happen since repr has
already been removed from xarray and is not found. Regardless, it
finishes up deallocating br_port. At this point, repr is still not freed
and an fdb event can happen, in which ice_eswitch_br_fdb_event_work()
takes repr->br_port and tries to use it, which causes a panic (use after
free).
Note that this only happens with 2 or more port representors added to
the bridge, since with only one representor port, the bridge deinit is
slightly different (ice_eswitch_br_port_deinit() is called via
ice_eswitch_br_ports_flush(), not ice_eswitch_br_port_unlink()).
Trace:
Oops: general protection fault, probably for non-canonical address 0xf129010fd1a93284: 0000 [#1] PREEMPT SMP KASAN NOPTI
KASAN: maybe wild-memory-access in range [0x8948287e8d499420-0x8948287e8d499427]
(...)
Workqueue: ice_bridge_wq ice_eswitch_br_fdb_event_work [ice]
RIP: 0010:__rht_bucket_nested+0xb4/0x180
(...)
Call Trace:
(...)
ice_eswitch_br_fdb_find+0x3fa/0x550 [ice]
? __pfx_ice_eswitch_br_fdb_find+0x10/0x10 [ice]
ice_eswitch_br_fdb_event_work+0x2de/0x1e60 [ice]
? __schedule+0xf60/0x5210
? mutex_lock+0x91/0xe0
? __pfx_ice_eswitch_br_fdb_event_work+0x10/0x10 [ice]
? ice_eswitch_br_update_work+0x1f4/0x310 [ice]
(...)
A workaround is available: brctl setageing $BR 0, which stops the bridge
from adding fdb entries altogether.
Change the order of operations in ice_eswitch_detach(): move the call to
unregister_netdev() before removing repr from xarray. This way
repr->br_port will be correctly set to NULL in
ice_eswitch_br_port_deinit(), preventing a panic.
Fixes: fff292b47ac1 ("ice: add VF representors one by one")
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Signed-off-by: Marcin Szycik <marcin.szycik@linux.intel.com>
Tested-by: Sujai Buvaneswaran <sujai.buvaneswaran@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
drivers/net/ethernet/intel/ice/ice_eswitch.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_eswitch.c b/drivers/net/ethernet/intel/ice/ice_eswitch.c
index c0b3e70a7ea3..fb527434b58b 100644
--- a/drivers/net/ethernet/intel/ice/ice_eswitch.c
+++ b/drivers/net/ethernet/intel/ice/ice_eswitch.c
@@ -552,13 +552,14 @@ int ice_eswitch_attach_sf(struct ice_pf *pf, struct ice_dynamic_port *sf)
static void ice_eswitch_detach(struct ice_pf *pf, struct ice_repr *repr)
{
ice_eswitch_stop_reprs(pf);
+ repr->ops.rem(repr);
+
xa_erase(&pf->eswitch.reprs, repr->id);
if (xa_empty(&pf->eswitch.reprs))
ice_eswitch_disable_switchdev(pf);
ice_eswitch_release_repr(pf, repr);
- repr->ops.rem(repr);
ice_repr_destroy(repr);
if (xa_empty(&pf->eswitch.reprs)) {
--
2.42.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net 2/6] ice: change q_index variable type to s16 to store -1 value
2024-11-04 22:36 [PATCH net 0/6][pull request] Intel Wired LAN Driver Updates 2024-11-04 (ice, idpf, i40e, e1000e) Tony Nguyen
2024-11-04 22:36 ` [PATCH net 1/6] ice: Fix use after free during unload with ports in bridge Tony Nguyen
@ 2024-11-04 22:36 ` Tony Nguyen
2024-11-05 5:26 ` Hariprasad Kelam
2024-11-04 22:36 ` [PATCH net 3/6] idpf: avoid vport access in idpf_get_link_ksettings Tony Nguyen
` (4 subsequent siblings)
6 siblings, 1 reply; 9+ messages in thread
From: Tony Nguyen @ 2024-11-04 22:36 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet, andrew+netdev, netdev
Cc: Mateusz Polchlopek, anthony.l.nguyen, Przemek Kitszel,
Simon Horman, Pucha Himasekhar Reddy
From: Mateusz Polchlopek <mateusz.polchlopek@intel.com>
Fix Flow Director not allowing to re-map traffic to 0th queue when action
is configured to drop (and vice versa).
The current implementation of ethtool callback in the ice driver forbids
change Flow Director action from 0 to -1 and from -1 to 0 with an error,
e.g:
# ethtool -U eth2 flow-type tcp4 src-ip 1.1.1.1 loc 1 action 0
# ethtool -U eth2 flow-type tcp4 src-ip 1.1.1.1 loc 1 action -1
rmgr: Cannot insert RX class rule: Invalid argument
We set the value of `u16 q_index = 0` at the beginning of the function
ice_set_fdir_input_set(). In case of "drop traffic" action (which is
equal to -1 in ethtool) we store the 0 value. Later, when want to change
traffic rule to redirect to queue with index 0 it returns an error
caused by duplicate found.
Fix this behaviour by change of the type of field `q_index` from u16 to s16
in `struct ice_fdir_fltr`. This allows to store -1 in the field in case
of "drop traffic" action. What is more, change the variable type in the
function ice_set_fdir_input_set() and assign at the beginning the new
`#define ICE_FDIR_NO_QUEUE_IDX` which is -1. Later, if the action is set
to another value (point specific queue index) the variable value is
overwritten in the function.
Fixes: cac2a27cd9ab ("ice: Support IPv4 Flow Director filters")
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Signed-off-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c | 3 ++-
drivers/net/ethernet/intel/ice/ice_fdir.h | 4 +++-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c b/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c
index 5412eff8ef23..ee9862ddfe15 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c
@@ -1830,11 +1830,12 @@ static int
ice_set_fdir_input_set(struct ice_vsi *vsi, struct ethtool_rx_flow_spec *fsp,
struct ice_fdir_fltr *input)
{
- u16 dest_vsi, q_index = 0;
+ s16 q_index = ICE_FDIR_NO_QUEUE_IDX;
u16 orig_q_index = 0;
struct ice_pf *pf;
struct ice_hw *hw;
int flow_type;
+ u16 dest_vsi;
u8 dest_ctl;
if (!vsi || !fsp || !input)
diff --git a/drivers/net/ethernet/intel/ice/ice_fdir.h b/drivers/net/ethernet/intel/ice/ice_fdir.h
index ab5b118daa2d..820023c0271f 100644
--- a/drivers/net/ethernet/intel/ice/ice_fdir.h
+++ b/drivers/net/ethernet/intel/ice/ice_fdir.h
@@ -53,6 +53,8 @@
*/
#define ICE_FDIR_IPV4_PKT_FLAG_MF 0x20
+#define ICE_FDIR_NO_QUEUE_IDX -1
+
enum ice_fltr_prgm_desc_dest {
ICE_FLTR_PRGM_DESC_DEST_DROP_PKT,
ICE_FLTR_PRGM_DESC_DEST_DIRECT_PKT_QINDEX,
@@ -186,7 +188,7 @@ struct ice_fdir_fltr {
u16 flex_fltr;
/* filter control */
- u16 q_index;
+ s16 q_index;
u16 orig_q_index;
u16 dest_vsi;
u8 dest_ctl;
--
2.42.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net 3/6] idpf: avoid vport access in idpf_get_link_ksettings
2024-11-04 22:36 [PATCH net 0/6][pull request] Intel Wired LAN Driver Updates 2024-11-04 (ice, idpf, i40e, e1000e) Tony Nguyen
2024-11-04 22:36 ` [PATCH net 1/6] ice: Fix use after free during unload with ports in bridge Tony Nguyen
2024-11-04 22:36 ` [PATCH net 2/6] ice: change q_index variable type to s16 to store -1 value Tony Nguyen
@ 2024-11-04 22:36 ` Tony Nguyen
2024-11-04 22:36 ` [PATCH net 4/6] idpf: fix idpf_vc_core_init error path Tony Nguyen
` (3 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Tony Nguyen @ 2024-11-04 22:36 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet, andrew+netdev, netdev
Cc: Pavan Kumar Linga, anthony.l.nguyen, stable, horms, Tarun K Singh,
Krishneil Singh
From: Pavan Kumar Linga <pavan.kumar.linga@intel.com>
When the device control plane is removed or the platform
running device control plane is rebooted, a reset is detected
on the driver. On driver reset, it releases the resources and
waits for the reset to complete. If the reset fails, it takes
the error path and releases the vport lock. At this time if the
monitoring tools tries to access link settings, it call traces
for accessing released vport pointer.
To avoid it, move link_speed_mbps to netdev_priv structure
which removes the dependency on vport pointer and the vport lock
in idpf_get_link_ksettings. Also use netif_carrier_ok()
to check the link status and adjust the offsetof to use link_up
instead of link_speed_mbps.
Fixes: 02cbfba1add5 ("idpf: add ethtool callbacks")
Cc: stable@vger.kernel.org # 6.7+
Reviewed-by: Tarun K Singh <tarun.k.singh@intel.com>
Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com>
Tested-by: Krishneil Singh <krishneil.k.singh@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
drivers/net/ethernet/intel/idpf/idpf.h | 4 ++--
drivers/net/ethernet/intel/idpf/idpf_ethtool.c | 11 +++--------
drivers/net/ethernet/intel/idpf/idpf_lib.c | 4 ++--
drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 2 +-
4 files changed, 8 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/intel/idpf/idpf.h b/drivers/net/ethernet/intel/idpf/idpf.h
index 2c31ad87587a..66544faab710 100644
--- a/drivers/net/ethernet/intel/idpf/idpf.h
+++ b/drivers/net/ethernet/intel/idpf/idpf.h
@@ -141,6 +141,7 @@ enum idpf_vport_state {
* @adapter: Adapter back pointer
* @vport: Vport back pointer
* @vport_id: Vport identifier
+ * @link_speed_mbps: Link speed in mbps
* @vport_idx: Relative vport index
* @state: See enum idpf_vport_state
* @netstats: Packet and byte stats
@@ -150,6 +151,7 @@ struct idpf_netdev_priv {
struct idpf_adapter *adapter;
struct idpf_vport *vport;
u32 vport_id;
+ u32 link_speed_mbps;
u16 vport_idx;
enum idpf_vport_state state;
struct rtnl_link_stats64 netstats;
@@ -287,7 +289,6 @@ struct idpf_port_stats {
* @tx_itr_profile: TX profiles for Dynamic Interrupt Moderation
* @port_stats: per port csum, header split, and other offload stats
* @link_up: True if link is up
- * @link_speed_mbps: Link speed in mbps
* @sw_marker_wq: workqueue for marker packets
*/
struct idpf_vport {
@@ -331,7 +332,6 @@ struct idpf_vport {
struct idpf_port_stats port_stats;
bool link_up;
- u32 link_speed_mbps;
wait_queue_head_t sw_marker_wq;
};
diff --git a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
index 3806ddd3ce4a..59b1a1a09996 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
@@ -1296,24 +1296,19 @@ static void idpf_set_msglevel(struct net_device *netdev, u32 data)
static int idpf_get_link_ksettings(struct net_device *netdev,
struct ethtool_link_ksettings *cmd)
{
- struct idpf_vport *vport;
-
- idpf_vport_ctrl_lock(netdev);
- vport = idpf_netdev_to_vport(netdev);
+ struct idpf_netdev_priv *np = netdev_priv(netdev);
ethtool_link_ksettings_zero_link_mode(cmd, supported);
cmd->base.autoneg = AUTONEG_DISABLE;
cmd->base.port = PORT_NONE;
- if (vport->link_up) {
+ if (netif_carrier_ok(netdev)) {
cmd->base.duplex = DUPLEX_FULL;
- cmd->base.speed = vport->link_speed_mbps;
+ cmd->base.speed = np->link_speed_mbps;
} else {
cmd->base.duplex = DUPLEX_UNKNOWN;
cmd->base.speed = SPEED_UNKNOWN;
}
- idpf_vport_ctrl_unlock(netdev);
-
return 0;
}
diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c
index 4f20343e49a9..c3848e10e7db 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
@@ -1860,7 +1860,7 @@ int idpf_initiate_soft_reset(struct idpf_vport *vport,
* mess with. Nothing below should use those variables from new_vport
* and should instead always refer to them in vport if they need to.
*/
- memcpy(new_vport, vport, offsetof(struct idpf_vport, link_speed_mbps));
+ memcpy(new_vport, vport, offsetof(struct idpf_vport, link_up));
/* Adjust resource parameters prior to reallocating resources */
switch (reset_cause) {
@@ -1906,7 +1906,7 @@ int idpf_initiate_soft_reset(struct idpf_vport *vport,
/* Same comment as above regarding avoiding copying the wait_queues and
* mutexes applies here. We do not want to mess with those if possible.
*/
- memcpy(vport, new_vport, offsetof(struct idpf_vport, link_speed_mbps));
+ memcpy(vport, new_vport, offsetof(struct idpf_vport, link_up));
if (reset_cause == IDPF_SR_Q_CHANGE)
idpf_vport_alloc_vec_indexes(vport);
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
index 15c00a01f1c0..ce217e274506 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
@@ -141,7 +141,7 @@ static void idpf_handle_event_link(struct idpf_adapter *adapter,
}
np = netdev_priv(vport->netdev);
- vport->link_speed_mbps = le32_to_cpu(v2e->link_speed);
+ np->link_speed_mbps = le32_to_cpu(v2e->link_speed);
if (vport->link_up == v2e->link_status)
return;
--
2.42.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net 4/6] idpf: fix idpf_vc_core_init error path
2024-11-04 22:36 [PATCH net 0/6][pull request] Intel Wired LAN Driver Updates 2024-11-04 (ice, idpf, i40e, e1000e) Tony Nguyen
` (2 preceding siblings ...)
2024-11-04 22:36 ` [PATCH net 3/6] idpf: avoid vport access in idpf_get_link_ksettings Tony Nguyen
@ 2024-11-04 22:36 ` Tony Nguyen
2024-11-04 22:36 ` [PATCH net 5/6] i40e: fix race condition by adding filter's intermediate sync state Tony Nguyen
` (2 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Tony Nguyen @ 2024-11-04 22:36 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet, andrew+netdev, netdev
Cc: Pavan Kumar Linga, anthony.l.nguyen, stable, horms, Tarun K Singh,
Krishneil Singh
From: Pavan Kumar Linga <pavan.kumar.linga@intel.com>
In an event where the platform running the device control plane
is rebooted, reset is detected on the driver. It releases
all the resources and waits for the reset to complete. Once the
reset is done, it tries to build the resources back. At this
time if the device control plane is not yet started, then
the driver timeouts on the virtchnl message and retries to
establish the mailbox again.
In the retry flow, mailbox is deinitialized but the mailbox
workqueue is still alive and polling for the mailbox message.
This results in accessing the released control queue leading to
null-ptr-deref. Fix it by unrolling the work queue cancellation
and mailbox deinitialization in the reverse order which they got
initialized.
Fixes: 4930fbf419a7 ("idpf: add core init and interrupt request")
Fixes: 34c21fa894a1 ("idpf: implement virtchnl transaction manager")
Cc: stable@vger.kernel.org # 6.9+
Reviewed-by: Tarun K Singh <tarun.k.singh@intel.com>
Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com>
Tested-by: Krishneil Singh <krishneil.k.singh@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
drivers/net/ethernet/intel/idpf/idpf_lib.c | 1 +
drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 1 -
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c
index c3848e10e7db..b4fbb99bfad2 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
@@ -1786,6 +1786,7 @@ static int idpf_init_hard_reset(struct idpf_adapter *adapter)
*/
err = idpf_vc_core_init(adapter);
if (err) {
+ cancel_delayed_work_sync(&adapter->mbx_task);
idpf_deinit_dflt_mbx(adapter);
goto unlock_mutex;
}
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
index ce217e274506..d46c95f91b0d 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
@@ -3063,7 +3063,6 @@ int idpf_vc_core_init(struct idpf_adapter *adapter)
adapter->state = __IDPF_VER_CHECK;
if (adapter->vcxn_mngr)
idpf_vc_xn_shutdown(adapter->vcxn_mngr);
- idpf_deinit_dflt_mbx(adapter);
set_bit(IDPF_HR_DRV_LOAD, adapter->flags);
queue_delayed_work(adapter->vc_event_wq, &adapter->vc_event_task,
msecs_to_jiffies(task_delay));
--
2.42.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net 5/6] i40e: fix race condition by adding filter's intermediate sync state
2024-11-04 22:36 [PATCH net 0/6][pull request] Intel Wired LAN Driver Updates 2024-11-04 (ice, idpf, i40e, e1000e) Tony Nguyen
` (3 preceding siblings ...)
2024-11-04 22:36 ` [PATCH net 4/6] idpf: fix idpf_vc_core_init error path Tony Nguyen
@ 2024-11-04 22:36 ` Tony Nguyen
2024-11-04 22:36 ` [PATCH net 6/6] e1000e: Remove Meteor Lake SMBUS workarounds Tony Nguyen
2024-11-06 2:20 ` [PATCH net 0/6][pull request] Intel Wired LAN Driver Updates 2024-11-04 (ice, idpf, i40e, e1000e) patchwork-bot+netdevbpf
6 siblings, 0 replies; 9+ messages in thread
From: Tony Nguyen @ 2024-11-04 22:36 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet, andrew+netdev, netdev
Cc: Aleksandr Loktionov, anthony.l.nguyen, Pucha Himasekhar Reddy,
Michal Schmidt
From: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Fix a race condition in the i40e driver that leads to MAC/VLAN filters
becoming corrupted and leaking. Address the issue that occurs under
heavy load when multiple threads are concurrently modifying MAC/VLAN
filters by setting mac and port VLAN.
1. Thread T0 allocates a filter in i40e_add_filter() within
i40e_ndo_set_vf_port_vlan().
2. Thread T1 concurrently frees the filter in __i40e_del_filter() within
i40e_ndo_set_vf_mac().
3. Subsequently, i40e_service_task() calls i40e_sync_vsi_filters(), which
refers to the already freed filter memory, causing corruption.
Reproduction steps:
1. Spawn multiple VFs.
2. Apply a concurrent heavy load by running parallel operations to change
MAC addresses on the VFs and change port VLANs on the host.
3. Observe errors in dmesg:
"Error I40E_AQ_RC_ENOSPC adding RX filters on VF XX,
please set promiscuous on manually for VF XX".
Exact code for stable reproduction Intel can't open-source now.
The fix involves implementing a new intermediate filter state,
I40E_FILTER_NEW_SYNC, for the time when a filter is on a tmp_add_list.
These filters cannot be deleted from the hash list directly but
must be removed using the full process.
Fixes: 278e7d0b9d68 ("i40e: store MAC/VLAN filters in a hash with the MAC Address as key")
Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
Reviewed-by: Michal Schmidt <mschmidt@redhat.com>
Tested-by: Michal Schmidt <mschmidt@redhat.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e.h | 1 +
drivers/net/ethernet/intel/i40e/i40e_debugfs.c | 1 +
drivers/net/ethernet/intel/i40e/i40e_main.c | 12 ++++++++++--
3 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index 2089a0e172bf..d4255c2706fa 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -755,6 +755,7 @@ enum i40e_filter_state {
I40E_FILTER_ACTIVE, /* Added to switch by FW */
I40E_FILTER_FAILED, /* Rejected by FW */
I40E_FILTER_REMOVE, /* To be removed */
+ I40E_FILTER_NEW_SYNC, /* New, not sent yet, is in i40e_sync_vsi_filters() */
/* There is no 'removed' state; the filter struct is freed */
};
struct i40e_mac_filter {
diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
index abf624d770e6..208c2f0857b6 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
@@ -89,6 +89,7 @@ static char *i40e_filter_state_string[] = {
"ACTIVE",
"FAILED",
"REMOVE",
+ "NEW_SYNC",
};
/**
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 25295ae370b2..55fb362eb508 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -1255,6 +1255,7 @@ int i40e_count_filters(struct i40e_vsi *vsi)
hash_for_each_safe(vsi->mac_filter_hash, bkt, h, f, hlist) {
if (f->state == I40E_FILTER_NEW ||
+ f->state == I40E_FILTER_NEW_SYNC ||
f->state == I40E_FILTER_ACTIVE)
++cnt;
}
@@ -1441,6 +1442,8 @@ static int i40e_correct_mac_vlan_filters(struct i40e_vsi *vsi,
new->f = add_head;
new->state = add_head->state;
+ if (add_head->state == I40E_FILTER_NEW)
+ add_head->state = I40E_FILTER_NEW_SYNC;
/* Add the new filter to the tmp list */
hlist_add_head(&new->hlist, tmp_add_list);
@@ -1550,6 +1553,8 @@ static int i40e_correct_vf_mac_vlan_filters(struct i40e_vsi *vsi,
return -ENOMEM;
new_mac->f = add_head;
new_mac->state = add_head->state;
+ if (add_head->state == I40E_FILTER_NEW)
+ add_head->state = I40E_FILTER_NEW_SYNC;
/* Add the new filter to the tmp list */
hlist_add_head(&new_mac->hlist, tmp_add_list);
@@ -2437,7 +2442,8 @@ static int
i40e_aqc_broadcast_filter(struct i40e_vsi *vsi, const char *vsi_name,
struct i40e_mac_filter *f)
{
- bool enable = f->state == I40E_FILTER_NEW;
+ bool enable = f->state == I40E_FILTER_NEW ||
+ f->state == I40E_FILTER_NEW_SYNC;
struct i40e_hw *hw = &vsi->back->hw;
int aq_ret;
@@ -2611,6 +2617,7 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
/* Add it to the hash list */
hlist_add_head(&new->hlist, &tmp_add_list);
+ f->state = I40E_FILTER_NEW_SYNC;
}
/* Count the number of active (current and new) VLAN
@@ -2762,7 +2769,8 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
spin_lock_bh(&vsi->mac_filter_hash_lock);
hlist_for_each_entry_safe(new, h, &tmp_add_list, hlist) {
/* Only update the state if we're still NEW */
- if (new->f->state == I40E_FILTER_NEW)
+ if (new->f->state == I40E_FILTER_NEW ||
+ new->f->state == I40E_FILTER_NEW_SYNC)
new->f->state = new->state;
hlist_del(&new->hlist);
netdev_hw_addr_refcnt(new->f, vsi->netdev, -1);
--
2.42.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net 6/6] e1000e: Remove Meteor Lake SMBUS workarounds
2024-11-04 22:36 [PATCH net 0/6][pull request] Intel Wired LAN Driver Updates 2024-11-04 (ice, idpf, i40e, e1000e) Tony Nguyen
` (4 preceding siblings ...)
2024-11-04 22:36 ` [PATCH net 5/6] i40e: fix race condition by adding filter's intermediate sync state Tony Nguyen
@ 2024-11-04 22:36 ` Tony Nguyen
2024-11-06 2:20 ` [PATCH net 0/6][pull request] Intel Wired LAN Driver Updates 2024-11-04 (ice, idpf, i40e, e1000e) patchwork-bot+netdevbpf
6 siblings, 0 replies; 9+ messages in thread
From: Tony Nguyen @ 2024-11-04 22:36 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet, andrew+netdev, netdev
Cc: Vitaly Lifshits, anthony.l.nguyen, dima.ruinskiy, horms,
Avigail Dahan
From: Vitaly Lifshits <vitaly.lifshits@intel.com>
This is a partial revert to commit 76a0a3f9cc2f ("e1000e: fix force smbus
during suspend flow"). That commit fixed a sporadic PHY access issue but
introduced a regression in runtime suspend flows.
The original issue on Meteor Lake systems was rare in terms of the
reproduction rate and the number of the systems affected.
After the integration of commit 0a6ad4d9e169 ("e1000e: avoid failing the
system during pm_suspend"), PHY access loss can no longer cause a
system-level suspend failure. As it only occurs when the LAN cable is
disconnected, and is recovered during system resume flow. Therefore, its
functional impact is low, and the priority is given to stabilizing
runtime suspend.
Fixes: 76a0a3f9cc2f ("e1000e: fix force smbus during suspend flow")
Signed-off-by: Vitaly Lifshits <vitaly.lifshits@intel.com>
Tested-by: Avigail Dahan <avigailx.dahan@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
drivers/net/ethernet/intel/e1000e/ich8lan.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index ce227b56cf72..2f9655cf5dd9 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -1205,12 +1205,10 @@ s32 e1000_enable_ulp_lpt_lp(struct e1000_hw *hw, bool to_sx)
if (ret_val)
goto out;
- if (hw->mac.type != e1000_pch_mtp) {
- ret_val = e1000e_force_smbus(hw);
- if (ret_val) {
- e_dbg("Failed to force SMBUS: %d\n", ret_val);
- goto release;
- }
+ ret_val = e1000e_force_smbus(hw);
+ if (ret_val) {
+ e_dbg("Failed to force SMBUS: %d\n", ret_val);
+ goto release;
}
/* Si workaround for ULP entry flow on i127/rev6 h/w. Enable
@@ -1273,13 +1271,6 @@ s32 e1000_enable_ulp_lpt_lp(struct e1000_hw *hw, bool to_sx)
}
release:
- if (hw->mac.type == e1000_pch_mtp) {
- ret_val = e1000e_force_smbus(hw);
- if (ret_val)
- e_dbg("Failed to force SMBUS over MTL system: %d\n",
- ret_val);
- }
-
hw->phy.ops.release(hw);
out:
if (ret_val)
--
2.42.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net 2/6] ice: change q_index variable type to s16 to store -1 value
2024-11-04 22:36 ` [PATCH net 2/6] ice: change q_index variable type to s16 to store -1 value Tony Nguyen
@ 2024-11-05 5:26 ` Hariprasad Kelam
0 siblings, 0 replies; 9+ messages in thread
From: Hariprasad Kelam @ 2024-11-05 5:26 UTC (permalink / raw)
To: Tony Nguyen
Cc: davem, kuba, pabeni, edumazet, andrew+netdev, netdev,
Mateusz Polchlopek, Przemek Kitszel, Simon Horman,
Pucha Himasekhar Reddy
On 2024-11-05 at 04:06:30, Tony Nguyen (anthony.l.nguyen@intel.com) wrote:
> From: Mateusz Polchlopek <mateusz.polchlopek@intel.com>
>
> Fix Flow Director not allowing to re-map traffic to 0th queue when action
> is configured to drop (and vice versa).
>
> The current implementation of ethtool callback in the ice driver forbids
> change Flow Director action from 0 to -1 and from -1 to 0 with an error,
> e.g:
>
> # ethtool -U eth2 flow-type tcp4 src-ip 1.1.1.1 loc 1 action 0
> # ethtool -U eth2 flow-type tcp4 src-ip 1.1.1.1 loc 1 action -1
> rmgr: Cannot insert RX class rule: Invalid argument
>
> We set the value of `u16 q_index = 0` at the beginning of the function
> ice_set_fdir_input_set(). In case of "drop traffic" action (which is
> equal to -1 in ethtool) we store the 0 value. Later, when want to change
> traffic rule to redirect to queue with index 0 it returns an error
> caused by duplicate found.
>
> Fix this behaviour by change of the type of field `q_index` from u16 to s16
> in `struct ice_fdir_fltr`. This allows to store -1 in the field in case
> of "drop traffic" action. What is more, change the variable type in the
> function ice_set_fdir_input_set() and assign at the beginning the new
> `#define ICE_FDIR_NO_QUEUE_IDX` which is -1. Later, if the action is set
> to another value (point specific queue index) the variable value is
> overwritten in the function.
>
> Fixes: cac2a27cd9ab ("ice: Support IPv4 Flow Director filters")
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Signed-off-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com>
> Reviewed-by: Simon Horman <horms@kernel.org>
> Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> ---
Reviewed-by: Hariprasad Kelam <hkelam@marvell.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net 0/6][pull request] Intel Wired LAN Driver Updates 2024-11-04 (ice, idpf, i40e, e1000e)
2024-11-04 22:36 [PATCH net 0/6][pull request] Intel Wired LAN Driver Updates 2024-11-04 (ice, idpf, i40e, e1000e) Tony Nguyen
` (5 preceding siblings ...)
2024-11-04 22:36 ` [PATCH net 6/6] e1000e: Remove Meteor Lake SMBUS workarounds Tony Nguyen
@ 2024-11-06 2:20 ` patchwork-bot+netdevbpf
6 siblings, 0 replies; 9+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-11-06 2:20 UTC (permalink / raw)
To: Tony Nguyen; +Cc: davem, kuba, pabeni, edumazet, andrew+netdev, netdev
Hello:
This series was applied to netdev/net.git (main)
by Tony Nguyen <anthony.l.nguyen@intel.com>:
On Mon, 4 Nov 2024 14:36:28 -0800 you wrote:
> For ice:
>
> Marcin adjusts ordering of calls in ice_eswitch_detach() to resolve a
> use after free issue.
>
> Mateusz corrects variable type for Flow Director queue to fix issues
> related to drop actions.
>
> [...]
Here is the summary with links:
- [net,1/6] ice: Fix use after free during unload with ports in bridge
https://git.kernel.org/netdev/net/c/e9942bfe4931
- [net,2/6] ice: change q_index variable type to s16 to store -1 value
https://git.kernel.org/netdev/net/c/64502dac974a
- [net,3/6] idpf: avoid vport access in idpf_get_link_ksettings
https://git.kernel.org/netdev/net/c/81d2fb4c7c18
- [net,4/6] idpf: fix idpf_vc_core_init error path
https://git.kernel.org/netdev/net/c/9b58031ff96b
- [net,5/6] i40e: fix race condition by adding filter's intermediate sync state
https://git.kernel.org/netdev/net/c/f30490e9695e
- [net,6/6] e1000e: Remove Meteor Lake SMBUS workarounds
https://git.kernel.org/netdev/net/c/b8473723272e
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-11-06 2:20 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-04 22:36 [PATCH net 0/6][pull request] Intel Wired LAN Driver Updates 2024-11-04 (ice, idpf, i40e, e1000e) Tony Nguyen
2024-11-04 22:36 ` [PATCH net 1/6] ice: Fix use after free during unload with ports in bridge Tony Nguyen
2024-11-04 22:36 ` [PATCH net 2/6] ice: change q_index variable type to s16 to store -1 value Tony Nguyen
2024-11-05 5:26 ` Hariprasad Kelam
2024-11-04 22:36 ` [PATCH net 3/6] idpf: avoid vport access in idpf_get_link_ksettings Tony Nguyen
2024-11-04 22:36 ` [PATCH net 4/6] idpf: fix idpf_vc_core_init error path Tony Nguyen
2024-11-04 22:36 ` [PATCH net 5/6] i40e: fix race condition by adding filter's intermediate sync state Tony Nguyen
2024-11-04 22:36 ` [PATCH net 6/6] e1000e: Remove Meteor Lake SMBUS workarounds Tony Nguyen
2024-11-06 2:20 ` [PATCH net 0/6][pull request] Intel Wired LAN Driver Updates 2024-11-04 (ice, idpf, i40e, e1000e) patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox