On 9/18/2026 6:33 AM, Aleksandr Loktionov wrote:
When multiple VFs are brought up simultaneously, a VF can receive a CTS
(Clear To Send) mailbox message instead of the expected RESET | (N)ACK
response, because the watchdog fires a gratuitous ping right as the VF
mailbox transaction is in progress.
Remove the ixgbe_ping_all_vfs() call from both ixgbe_watchdog_link_is_up()
and ixgbe_watchdog_link_is_down(). Both paths contain an identical call
with the same race window during VF initialization: link-state changes
(up or down) can arrive while VFs are mid-initialization, and the
gratuitous ping from either direction collides with the VF mailbox state
machine for the same reason.
Link-state changes are already communicated to VFs through the normal
mailbox protocol; these extra pings are redundant on both paths.
This removes the last two callers of ixgbe_ping_all_vfs(), so remove
the now-unused function and its declaration as well.
Suggested-by: Sebastian Basierski <[email protected]>
Signed-off-by: Aleksandr Loktionov <[email protected]>
Reviewed-by: Simon Horman <[email protected]>
---
v1 -> v2: no code changes, resending to pick up Reviewed-by
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 6 ------
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 14 --------------
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h | 1 -
3 files changed, 21 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index f918564..365acf8 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -8181,9 +8181,6 @@ static void ixgbe_watchdog_link_is_up(struct
ixgbe_adapter *adapter)
/* update the default user priority for VFs */
ixgbe_update_default_up(adapter);
-
- /* ping all the active vfs to let them know link has changed */
- ixgbe_ping_all_vfs(adapter);
From Sashiko:
Does removing this PF-to-VF mailbox write prevent the PF from sending
the CTS message required for the VF to transition its internal link
state from down to up?
The ixgbevf driver appears to rely on this CTS message to correctly
clear mac->get_link_status and transition to up:
drivers/net/ethernet/intel/ixgbevf/vf.c:ixgbevf_check_mac_link_vf() {
...
if (!(in_msg & IXGBE_VT_MSGTYPE_CTS)) {
/* msg is not CTS and is NACK we must have lost CTS status */
if (in_msg & IXGBE_VT_MSGTYPE_FAILURE)
ret_val = -1;
goto out;
}
...
mac->get_link_status = false;
out:
*link_up = !mac->get_link_status;
}
Without this ping, will the VF persistently fail the mailbox read and
force the link state down?
}
/**
@@ -8214,9 +8211,6 @@ static void ixgbe_watchdog_link_is_down(struct
ixgbe_adapter *adapter)
e_info(drv, "NIC Link is Down\n");
netif_carrier_off(netdev);
-
- /* ping all the active vfs to let them know link has changed */
- ixgbe_ping_all_vfs(adapter);
Does removing this ping sever the mechanism that triggers the VF's
mailbox interrupt on physical link drops?
It looks like the ixgbevf driver uses mac->get_link_status to determine
if it should poll the physical link. Once up, it sets this to false and
relies on a mailbox interrupt (ixgbevf_msix_other()) to set it back to 1
on state changes.
Will SR-IOV VFs remain stuck in the up state and permanently report
incorrect link states if they are not notified of link drops via this
mailbox write?
}
static bool ixgbe_ring_tx_pending(struct ixgbe_adapter *adapter)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index 431d77d..da641e9 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -1515,20 +1515,6 @@ static inline void ixgbe_ping_vf(struct ixgbe_adapter
*adapter, int vf)
ixgbe_write_mbx(hw, &ping, 1, vf);
}
-void ixgbe_ping_all_vfs(struct ixgbe_adapter *adapter)
-{
- struct ixgbe_hw *hw = &adapter->hw;
- u32 ping;
- int i;
-
- for (i = 0 ; i < adapter->num_vfs; i++) {
- ping = IXGBE_PF_CONTROL_MSG;
- if (adapter->vfinfo[i].clear_to_send)
- ping |= IXGBE_VT_MSGTYPE_CTS;
- ixgbe_write_mbx(hw, &ping, 1, i);
- }
-}
Is this function actually essential for notifying VFs of asynchronous
link state changes?
Since VFs do not have access to physical PHY interrupts, it seems they
might depend on this PF-initiated ping to learn about state transitions.
/**
* ixgbe_set_all_vfs - update vfs queues
* @adapter: Pointer to adapter struct
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
index bc4cab9..f755bfd 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
@@ -18,7 +18,6 @@ void ixgbe_restore_vf_multicasts(struct ixgbe_adapter
*adapter);
bool ixgbe_check_mdd_event(struct ixgbe_adapter *adapter);
void ixgbe_msg_task(struct ixgbe_adapter *adapter);
int ixgbe_vf_configuration(struct pci_dev *pdev, unsigned int event_mask);
-void ixgbe_ping_all_vfs(struct ixgbe_adapter *adapter);
void ixgbe_set_all_vfs(struct ixgbe_adapter *adapter);
int ixgbe_ndo_set_vf_mac(struct net_device *netdev, int queue, u8 *mac);
int ixgbe_ndo_set_vf_vlan(struct net_device *netdev, int queue, u16 vlan,
