On 3/20/2026 12:33 PM, Simon Horman wrote:
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html --- ice: do not carry link status over to link event data This patch removes code that updated the link_up parameter with the current firmware state after calling ice_update_link_info().diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c index e7308e381e2f..348c86b7328d 100644 --- a/drivers/net/ethernet/intel/ice/ice_main.c +++ b/drivers/net/ethernet/intel/ice/ice_main.c @@ -1127,12 +1127,6 @@ ice_link_event(struct ice_pf *pf, struct ice_port_info *pi, bool link_up, ice_check_link_cfg_err(pf, pi->phy.link_info.link_cfg_err); - /* Check if the link state is up after updating link info, and treat - * this event as an UP event since the link is actually UP now. - */ - if (phy_info->link_info.link_info & ICE_AQ_LINK_UP) - link_up = true; -Could removing this check create a race condition with stale link events?
This was a part of other changes that allows this to happen. By itself, as evidenced here, it does not.
Thanks, Tony
Consider this scenario: firmware generates a link DOWN event, but before the driver processes it, the link comes back UP. When the driver later processes the event and calls ice_update_link_info(), the firmware query returns the current state (UP). Previously, the removed code would check phy_info->link_info.link_info & ICE_AQ_LINK_UP and override link_up to true, reflecting the current reality. Now link_up stays false (the stale event data). Later in ice_link_event(), the comparison checks: if (link_up == old_link && link_speed == old_link_speed) If the link was previously UP (old_link=true), the comparison fails (false != true), and the driver proceeds to process a link DOWN event even though the firmware reports the link is UP. This leads to ice_vsi_link_event() calling netif_carrier_off() and netif_tx_stop_all_queues() inappropriately, marking the network interface DOWN and causing packet loss until the next link event. The removed comment explicitly documented this protection: "Check if the link state is up after updating link info, and treat this event as an UP event since the link is actually UP now." The commit message states "changes that occurred from update link info...will be handled appropriately by itself" but doesn't explain what mechanism handles this. The ice_update_link_info() function only queries firmware state; it doesn't generate events or trigger link state change handling. How does the new code prevent stale link DOWN events from incorrectly bringing down an interface when the link is actually UP?vsi = ice_get_main_vsi(pf); if (!vsi || !vsi->port_info) return -EINVAL;
