One general comment: Looks like there are exact same code comparing with legacy rx, such as logic to update tail, multi-segments loop. It will be good if all the common code can be wrapped and use for multi recv functions
[...] +/* Get the number of used descriptors of a rx queue for flexible RXD */ +uint32_t iavf_dev_rxq_count_flex_rxd(struct rte_eth_dev *dev, uint16_t +queue_id) { #define IAVF_RXQ_SCAN_INTERVAL 4 + volatile union iavf_rx_flex_desc *rxdp; + struct iavf_rx_queue *rxq; + uint16_t desc = 0; + + rxq = dev->data->rx_queues[queue_id]; + rxdp = (volatile union iavf_rx_flex_desc *)&rxq->rx_ring[rxq->rx_tail]; + while ((desc < rxq->nb_rx_desc) && + rte_le_to_cpu_16(rxdp->wb.status_error0) & + (1 << IAVF_RX_FLEX_DESC_STATUS0_DD_S)) { + /* Check the DD bit of a rx descriptor of each 4 in a group, + * to avoid checking too frequently and downgrading performance + * too much. + */ + desc += IAVF_RXQ_SCAN_INTERVAL; + rxdp += IAVF_RXQ_SCAN_INTERVAL; + if (rxq->rx_tail + desc >= rxq->nb_rx_desc) + rxdp = (volatile union iavf_rx_flex_desc *) + &(rxq->rx_ring[rxq->rx_tail + + desc - rxq->nb_rx_desc]); + } + + return desc; +} No much difference between iavf_dev_rxq_count. Why do we need a new one? DD bit is located in the same place, right? Can we merge those two functions to one? + /* Get the number of used descriptors of a rx queue */ uint32_t iavf_dev_rxq_count(struct rte_eth_dev *dev, uint16_t queue_id) @@ -1795,6 +2264,10 @@ iavf_dev_rxq_count(struct rte_eth_dev *dev, uint16_t queue_id) rxq = dev->data->rx_queues[queue_id]; rxdp = &rxq->rx_ring[rxq->rx_tail]; + + if (rxq->rxdid == IAVF_RXDID_COMMS_OVS_1) + return iavf_dev_rxq_count_flex_rxd(dev, queue_id); + while ((desc < rxq->nb_rx_desc) && ((rte_le_to_cpu_64(rxdp->wb.qword1.status_error_len) & IAVF_RXD_QW1_STATUS_MASK) >> IAVF_RXD_QW1_STATUS_SHIFT) & @@ -1813,6 +2286,31 @@ iavf_dev_rxq_count(struct rte_eth_dev *dev, uint16_t queue_id) return desc; } +int +iavf_dev_rx_desc_status_flex_rxd(void *rx_queue, uint16_t offset) { + volatile union iavf_rx_flex_desc *rxdp; + struct iavf_rx_queue *rxq = rx_queue; + uint32_t desc; + + if (unlikely(offset >= rxq->nb_rx_desc)) + return -EINVAL; + + if (offset >= rxq->nb_rx_desc - rxq->nb_rx_hold) + return RTE_ETH_RX_DESC_UNAVAIL; + + desc = rxq->rx_tail + offset; + if (desc >= rxq->nb_rx_desc) + desc -= rxq->nb_rx_desc; + + rxdp = (volatile union iavf_rx_flex_desc *)&rxq->rx_ring[desc]; + if (rte_le_to_cpu_16(rxdp->wb.status_error0) & + (1 << IAVF_RX_FLEX_DESC_STATUS0_DD_S)) + return RTE_ETH_RX_DESC_DONE; + + return RTE_ETH_RX_DESC_AVAIL; +} + Similar comments as above. [......] { @@ -569,6 +596,20 @@ iavf_configure_queues(struct iavf_adapter *adapter) vc_qp->rxq.ring_len = rxq[i]->nb_rx_desc; vc_qp->rxq.dma_ring_addr = rxq[i]->rx_ring_phys_addr; vc_qp->rxq.databuffer_size = rxq[i]->rx_buf_len; + + if (vf->vf_res->vf_cap_flags & + VIRTCHNL_VF_OFFLOAD_RX_FLEX_DESC && + vf->supported_rxdid & BIT(IAVF_RXDID_COMMS_OVS_1)) { + vc_qp->rxq.rxdid = IAVF_RXDID_COMMS_OVS_1; + rxq[i]->rxdid = IAVF_RXDID_COMMS_OVS_1; Because this function is used to construct virtchnl message for configure queues. The rxdid in rxq[i] should be set before this function is called. How about to move this line assignment to iavf_dev_rx_queue_setup? + PMD_DRV_LOG(NOTICE, "request RXDID == %d in " + "Queue[%d]", vc_qp->rxq.rxdid, i); + } else { + vc_qp->rxq.rxdid = IAVF_RXDID_LEGACY_1; + rxq[i]->rxdid = IAVF_RXDID_LEGACY_1; Same as above. + PMD_DRV_LOG(NOTICE, "request RXDID == %d in " + "Queue[%d]", vc_qp->rxq.rxdid, i); + } } } -- 2.17.1