[PATCH v3] net/iavf: fix to consolidate link change event handling

[Loftus, Ciara]

> Subject: [PATCH v3] net/iavf: fix to consolidate link change event handling
> 
> Handled link-change events through a common static function that
> reads the correct advanced & legacy link fields properly and
> updates no-poll/watchdog/LSC state consistently.
> 
> Fixes: 5e03e316c753 ("net/iavf: handle virtchnl event message without
> interrupt")
> Fixes: 48de41ca11f0 ("net/avf: enable link status update")
> Cc: stable at dpdk.org
> 
> Signed-off-by: Anurag Mandal <anurag.mandal at intel.com>

Thanks Anurag.

Acked-by: Ciara Loftus <ciara.loftus at intel.com>

> ---
> V3: Addressed Ciara Loftus's review comments
>  - removed two unnecessary NULL checks
> V2: Addressed Ciara Loftus's review comments
>  - removed unnecessary NULL checks which were overly defensive checks
> 
>  drivers/net/intel/iavf/iavf_vchnl.c | 115 +++++++++++++++-------------
>  1 file changed, 63 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/net/intel/iavf/iavf_vchnl.c
> b/drivers/net/intel/iavf/iavf_vchnl.c
> index 0643a835d5..36b7ee9526 100644
> --- a/drivers/net/intel/iavf/iavf_vchnl.c
> +++ b/drivers/net/intel/iavf/iavf_vchnl.c
> @@ -216,6 +216,67 @@ iavf_convert_link_speed(enum virtchnl_link_speed
> virt_link_speed)
>  	return speed;
>  }
> 
> +/*
> + * iavf_handle_link_change_event: common handler for VIRTCHNL link
> change events
> + *
> + * @dev: pointer to rte_eth_dev for this VF
> + * @vpe: pointer to the virtchnl_pf_event payload received from the PF
> + *
> + * Handle PF link-change event: decode adv/legacy link info, update VF
> + * link state, sync no-poll/watchdog behavior & notify app via LSC event.
> + */
> +static void
> +iavf_handle_link_change_event(struct rte_eth_dev *dev,
> +			      struct virtchnl_pf_event *vpe)
> +{
> +	struct iavf_adapter *adapter =
> +		IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> +	struct iavf_info *vf = &adapter->vf;
> +	bool adv_link_speed;
> +
> +	adv_link_speed = (vf->vf_res != NULL) &&
> +		((vf->vf_res->vf_cap_flags &
> VIRTCHNL_VF_CAP_ADV_LINK_SPEED) != 0);
> +
> +	if (adv_link_speed) {
> +		vf->link_up = vpe->event_data.link_event_adv.link_status;
> +		vf->link_speed = vpe->event_data.link_event_adv.link_speed;
> +	} else {
> +		enum virtchnl_link_speed speed;
> +
> +		vf->link_up = vpe->event_data.link_event.link_status;
> +		speed = vpe->event_data.link_event.link_speed;
> +		vf->link_speed = iavf_convert_link_speed(speed);
> +	}
> +
> +	iavf_dev_link_update(dev, 0);
> +
> +	/*
> +	 * Update watchdog/no_poll state BEFORE notifying the application
> via
> +	 * the LSC event. Otherwise the application's link-up callback could
> +	 * race with stale (link-down) no_poll/watchdog state and either
> +	 * continue to drop traffic or trigger a spurious reset detection.
> +	 *
> +	 * Keeping the watchdog enabled whenever the link cannot be trusted
> +	 * (link is down or a VF reset is in progress); the watchdog drives
> +	 * auto-reset recovery, so it must remain armed in those cases.
> +	 */
> +	if (vf->link_up && !vf->vf_reset)
> +		iavf_dev_watchdog_disable(adapter);
> +	else
> +		iavf_dev_watchdog_enable(adapter);
> +
> +	if (adapter->devargs.no_poll_on_link_down) {
> +		iavf_set_no_poll(adapter, true);
> +		PMD_DRV_LOG(DEBUG, "VF no poll turned %s",
> +			    adapter->no_poll ? "on" : "off");
> +	}
> +
> +	iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_LSC, NULL, 0);
> +
> +	PMD_DRV_LOG(INFO, "Link status update:%s",
> +		vf->link_up ? "up" : "down");
> +}
> +
>  /* Read data in admin queue to get msg from pf driver */
>  static enum iavf_aq_result
>  iavf_read_msg_from_pf(struct iavf_adapter *adapter, uint16_t buf_len,
> @@ -253,34 +314,7 @@ iavf_read_msg_from_pf(struct iavf_adapter
> *adapter, uint16_t buf_len,
>  		result = IAVF_MSG_SYS;
>  		switch (vpe->event) {
>  		case VIRTCHNL_EVENT_LINK_CHANGE:
> -			vf->link_up =
> -				vpe->event_data.link_event.link_status;
> -			if (vf->vf_res != NULL &&
> -			    vf->vf_res->vf_cap_flags &
> VIRTCHNL_VF_CAP_ADV_LINK_SPEED) {
> -				vf->link_speed =
> -				    vpe-
> >event_data.link_event_adv.link_speed;
> -			} else {
> -				enum virtchnl_link_speed speed;
> -				speed = vpe-
> >event_data.link_event.link_speed;
> -				vf->link_speed =
> iavf_convert_link_speed(speed);
> -			}
> -			iavf_dev_link_update(vf->eth_dev, 0);
> -			iavf_dev_event_post(vf->eth_dev,
> RTE_ETH_EVENT_INTR_LSC, NULL, 0);
> -			if (vf->link_up && !vf->vf_reset) {
> -				iavf_dev_watchdog_disable(adapter);
> -			} else {
> -				if (!vf->link_up)
> -					iavf_dev_watchdog_enable(adapter);
> -			}
> -			if (adapter->devargs.no_poll_on_link_down) {
> -				iavf_set_no_poll(adapter, true);
> -				if (adapter->no_poll)
> -					PMD_DRV_LOG(DEBUG, "VF no poll
> turned on");
> -				else
> -					PMD_DRV_LOG(DEBUG, "VF no poll
> turned off");
> -			}
> -			PMD_DRV_LOG(INFO, "Link status update:%s",
> -					vf->link_up ? "up" : "down");
> +			iavf_handle_link_change_event(vf->eth_dev, vpe);
>  			break;
>  		case VIRTCHNL_EVENT_RESET_IMPENDING:
>  			vf->vf_reset = true;
> @@ -525,30 +559,7 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev,
> uint8_t *msg,
>  		break;
>  	case VIRTCHNL_EVENT_LINK_CHANGE:
>  		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_LINK_CHANGE
> event");
> -		vf->link_up = pf_msg->event_data.link_event.link_status;
> -		if (vf->vf_res->vf_cap_flags &
> VIRTCHNL_VF_CAP_ADV_LINK_SPEED) {
> -			vf->link_speed =
> -				pf_msg-
> >event_data.link_event_adv.link_speed;
> -		} else {
> -			enum virtchnl_link_speed speed;
> -			speed = pf_msg->event_data.link_event.link_speed;
> -			vf->link_speed = iavf_convert_link_speed(speed);
> -		}
> -		iavf_dev_link_update(dev, 0);
> -		if (vf->link_up && !vf->vf_reset) {
> -			iavf_dev_watchdog_disable(adapter);
> -		} else {
> -			if (!vf->link_up)
> -				iavf_dev_watchdog_enable(adapter);
> -		}
> -		if (adapter->devargs.no_poll_on_link_down) {
> -			iavf_set_no_poll(adapter, true);
> -			if (adapter->no_poll)
> -				PMD_DRV_LOG(DEBUG, "VF no poll turned
> on");
> -			else
> -				PMD_DRV_LOG(DEBUG, "VF no poll turned
> off");
> -		}
> -		iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_LSC, NULL,
> 0);
> +		iavf_handle_link_change_event(dev, pf_msg);
>  		break;
>  	case VIRTCHNL_EVENT_PF_DRIVER_CLOSE:
>  		PMD_DRV_LOG(DEBUG,
> "VIRTCHNL_EVENT_PF_DRIVER_CLOSE event");
> --
> 2.34.1