> -----Original Message----- > From: Loftus, Ciara <[email protected]> > Sent: 10 June 2026 15:13 > To: Mandal, Anurag <[email protected]>; [email protected] > Cc: Richardson, Bruce <[email protected]>; Medvedkin, Vladimir > <[email protected]>; [email protected] > Subject: RE: [PATCH v2] net/iavf: fix to consolidate link change event > handling > > > 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
Hi Ciara, Thank you for the review. Removed those two unwarranted NULL checks. Sent v3. Please check. Thanks, Anurag
