> Subject: [PATCH v2] net/iavf: fix to consolidate link change event handling
>
[snip]
> +
> /* 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,
> @@ -249,38 +310,15 @@ iavf_read_msg_from_pf(struct iavf_adapter
> *adapter, uint16_t buf_len,
> if (opcode == VIRTCHNL_OP_EVENT) {
> struct virtchnl_pf_event *vpe =
> (struct virtchnl_pf_event *)event.msg_buf;
> + if (vpe == NULL) {
> + PMD_DRV_LOG(ERR, "Invalid PF event message");
> + return IAVF_MSG_ERR;
> + }
This check can be removed.
iavf_read_msg_from_pf is called from iavf_wait_for_msg which performs a
NULL check on the same location (args->out_buffer) before passing it to
iavf_read_msg_from_pf
>
> 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;
> @@ -505,6 +543,12 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev,
> uint8_t *msg,
> PMD_DRV_LOG(DEBUG, "Error event");
> return;
> }
> +
> + if (pf_msg == NULL) {
> + PMD_DRV_LOG(ERR, "Invalid PF event message");
> + return;
> + }
This too can be removed.
pf_msg resolves to vf->aq_resp which is a fixed buffer allocated at
driver init time. It cannot be NULL here.
With those two changes I think the patch will be good to go.
> +
> switch (pf_msg->event) {
> case VIRTCHNL_EVENT_RESET_IMPENDING:
> PMD_DRV_LOG(DEBUG,
> "VIRTCHNL_EVENT_RESET_IMPENDING event");
> @@ -518,30 +562,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
