On 9/27/2022 8:32 AM, Junfeng Guo wrote:
Add Rx/Tx of GQI_QPL queue format and GQI_RDA queue format. Signed-off-by: Xiaoyun Li <xiaoyun...@intel.com> Signed-off-by: Junfeng Guo <junfeng....@intel.com>
<...>
--- a/drivers/net/gve/gve_ethdev.c +++ b/drivers/net/gve/gve_ethdev.c @@ -583,6 +583,11 @@ gve_dev_init(struct rte_eth_dev *eth_dev) if (err) return err; + if (gve_is_gqi(priv)) { + eth_dev->rx_pkt_burst = gve_rx_burst; + eth_dev->tx_pkt_burst = gve_tx_burst; + } +
What do you think to add a log here for 'else' case, to inform user why datapath is not working?
<...>
+uint16_t +gve_rx_burst(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) +{ + volatile struct gve_rx_desc *rxr, *rxd; + struct gve_rx_queue *rxq = rx_queue; + uint16_t rx_id = rxq->rx_tail; + struct rte_mbuf *rxe; + uint16_t nb_rx, len; + uint64_t addr; + + rxr = rxq->rx_desc_ring; + + for (nb_rx = 0; nb_rx < nb_pkts; nb_rx++) { + rxd = &rxr[rx_id]; + if (GVE_SEQNO(rxd->flags_seq) != rxq->expected_seqno) + break; + + if (rxd->flags_seq & GVE_RXF_ERR) + continue; + + len = rte_be_to_cpu_16(rxd->len) - GVE_RX_PAD; + rxe = rxq->sw_ring[rx_id]; + rxe->data_off = RTE_PKTMBUF_HEADROOM; + if (rxq->is_gqi_qpl) { + addr = (uint64_t)(rxq->qpl->mz->addr) + rx_id * PAGE_SIZE + GVE_RX_PAD; + rte_memcpy((void *)((size_t)rxe->buf_addr + rxe->data_off), + (void *)(size_t)addr, len);
Why a 'memcpy' is needed? Can't it DMA to mbuf data buffer?
+ } + rxe->nb_segs = 1; + rxe->next = NULL; + rxe->pkt_len = len; + rxe->data_len = len; + rxe->port = rxq->port_id; + rxe->packet_type = 0; + rxe->ol_flags = 0; +
As far as I can see 'sw_ring[]' filled using 'rte_pktmbuf_alloc_bulk()' API, which should reset mbuf fields to default values, so some of the assignment above can be redundant.
+ if (rxd->flags_seq & GVE_RXF_TCP) + rxe->packet_type |= RTE_PTYPE_L4_TCP; + if (rxd->flags_seq & GVE_RXF_UDP) + rxe->packet_type |= RTE_PTYPE_L4_UDP; + if (rxd->flags_seq & GVE_RXF_IPV4) + rxe->packet_type |= RTE_PTYPE_L3_IPV4; + if (rxd->flags_seq & GVE_RXF_IPV6) + rxe->packet_type |= RTE_PTYPE_L3_IPV6; +
If you are setting packet_type, it is better to implement 'dev_supported_ptypes_get()' dev_ops too, to announce host which packet type parsin supporting. (+ dev_ptypes_set() dev_ops)
And later driver can announce "Packet type parsing" feature in .ini file.