net/ice cast rx_flex_ring to proper type
PATCH 1/1 net/ice cast rx_flex_ring to proper type
its look like, that some parts of fdir code was not updated with new generic ci_rx_flex_desc. This patch fixes it
Also im found out more verbose logs for ice_check_fdir_programming_status in linux driver implementation in function ice_vf_verify_rx_desc:
https://github.com/intel/ethernet-linux-ice/blob/e4a92b1e84e084bc599580d065020986e501b8ad/src/ice_virtchnl_fdir.c#L3513
--- drivers/net/intel/ice/ice_rxtx.c | 46 +++++++++++++------------------- 1 file changed, 19 insertions(+), 27 deletions(-)
drivers/net/intel/ice/ice_rxtx.c +19 -27
@@ -2698,7 +2698,7 @@ ice_fdir_setup_rx_resources(struct ice_pf *pf) }/* Allocate RX hardware ring descriptors. */ - ring_size =sizeof (union ice_32byte_rx_desc ) *ICE_FDIR_NUM_RX_DESC ; + ring_size =sizeof (union ci_rx_flex_desc ) *ICE_FDIR_NUM_RX_DESC ; ring_size =RTE_ALIGN (ring_size,ICE_DMA_MEM_ALIGN ); rz = rte_eth_dma_zone_reserve(dev, "fdir_rx_ring",
@@ -2718,7 +2718,7 @@ ice_fdir_setup_rx_resources(struct ice_pf *pf) rxq->rx_ring_phys_addr = rz->iova;memset (rz->addr,0 ,ICE_FDIR_NUM_RX_DESC * -sizeof (union ice_32byte_rx_desc )); +sizeof (union ci_rx_flex_desc )); rxq->rx_flex_ring = (union ci_rx_flex_desc *)rz->addr;/*
@@ -4464,40 +4464,33 @@ ice_set_default_ptype_table(struct rte_eth_dev *dev)static inline int ice_check_fdir_programming_status (struct ci_rx_queue *rxq) { -volatile union ice_32byte_rx_desc *rxdp; -uint64_t qword1; -uint32_t rx_status; -uint32_t error; -uint32_t id; +volatile union ci_rx_flex_desc *rxdp; +uint32_t stat_err, error, id;int ret = -EAGAIN ; - rxdp = (volatile union ice_32byte_rx_desc *)&rxq->rx_flex_ring[rxq->rx_tail]; - qword1 =rte_le_to_cpu_64 (rxdp->wb.qword1.status_error_len); - rx_status = (qword1 &ICE_RXD_QW1_STATUS_M ) - >>ICE_RXD_QW1_STATUS_S ; + rxdp = &rxq->rx_flex_ring[rxq->rx_tail]; + stat_err =rte_le_to_cpu_16 (rxdp->wb.status_error0); -if (rx_status & (1 <<ICE_RX_DESC_STATUS_DD_S )) { +if (FIELD_GET (ICE_FXD_FLTR_WB_QW1_DD_M , stat_err) ==ICE_FXD_FLTR_WB_QW1_DD_YES ) { ret =0 ; - error = (qword1 &ICE_RX_PROG_STATUS_DESC_WB_QW1_FAIL_M ) >> -ICE_RX_PROG_STATUS_DESC_WB_QW1_FAIL_S ; - id = (qword1 &ICE_RX_PROG_STATUS_DESC_WB_QW1_PROGID_M ) >> -ICE_RX_PROG_STATUS_DESC_WB_QW1_PROGID_S ; -if (error) { -if (id ==ICE_RX_PROG_STATUS_DESC_WB_QW1_PROG_ADD ) -PMD_DRV_LOG (ERR ,"Failed to add FDIR rule." ); -else if (id ==ICE_RX_PROG_STATUS_DESC_WB_QW1_PROG_DEL ) -PMD_DRV_LOG (ERR ,"Failed to remove FDIR rule." ); + id =FIELD_GET (ICE_FXD_FLTR_WB_QW1_PROG_ID_M , stat_err); + error =FIELD_GET (ICE_FXD_FLTR_WB_QW1_FAIL_M , stat_err); +if (error ==ICE_FXD_FLTR_WB_QW1_FAIL_YES ) { +if (id ==ICE_FXD_FLTR_WB_QW1_PROG_ADD ) +PMD_DRV_LOG (ERR ,"Failed to add FDIR rule due to no space in the table" ); +else +PMD_DRV_LOG (ERR ,"Failed to remove FDIR rule, attempt to remove non-existent entry" ); ret = -EINVAL ; goto err; } - error = (qword1 &ICE_RX_PROG_STATUS_DESC_WB_QW1_FAIL_PROF_M ) >> -ICE_RX_PROG_STATUS_DESC_WB_QW1_FAIL_PROF_S ; -if (error) { -PMD_DRV_LOG (ERR ,"Failed to create FDIR profile." ); + error =FIELD_GET (ICE_FXD_FLTR_WB_QW1_FAIL_PROF_M , stat_err); +if (error ==ICE_FXD_FLTR_WB_QW1_FAIL_PROF_YES ) { +PMD_DRV_LOG (ERR ,"Profile matching error" ); ret = -EINVAL ; + goto err; } err: - rxdp->wb.qword1.status_error_len =0 ; + rxdp->wb.status_error0 =0 ; rxq->rx_tail++;if (unlikely (rxq->rx_tail == rxq->nb_rx_desc)) rxq->rx_tail =0 ;
@@ -4506,7 +4499,6 @@ ice_check_fdir_programming_status(struct ci_rx_queue *rxq)else ICE_PCI_REG_WRITE (rxq->qrx_tail, rxq->rx_tail -1 ); } -return ret; }
Signature · 1 line
2.54.0
Quoted 8 lines · Omar Munchaev
its look like, that some parts of fdir code was not updated with new generic
ci_rx_flex_desc. This patch fixes it
Also im found out more verbose logs forice_check_fdir_programming_statusin linux driver implementation in functionice_vf_verify_rx_desc:
https://github.com/intel/ethernet-linux-ice/blob/e4a92b1e84e084bc599580d065020986e501b8ad/src/ice_virtchnl_fdir.c#L3513Signed-off-by Omar Munchaev <[email protected]>---
Hi Omar,
This looks like two commits jammed together into one. I would split up the ci_rx_flex_desc changes from the rest of the fix (the logging changes are technically separate but IMO they can go in with the macro changes). The flex desc changes in particular would probably also require a Fixes: tag and a Cc: stable if the source commit is old enough to be in stable releases.
For the macro changes, DPDK is a BSD-3 clause licensed project, so technically we cannot use kernel code as it's GPL-licensed. Please do not refer to kernel implementation directly.
I do however think it's worth rewriting the Rx status etc code with FIELD_GET macros the way you did as that makes the code a lot clearer, and better detailed logging is certainly appreciated.
This patch would also benefit from better commit message. Suggested rewording:
[patch 1: flex descriptor changes]
net/ice: fix missing type casts for flex descriptors
When ice driver was ported to use the common Rx descriptor format, some type casts were missed. Add them.
---
[patch 2: macro/logging changes]
net/ice: use macros to get field values
Currently, in ice_check_fdir_programming_status, descriptor fields are accessed through direct bit mask manipulation and shifts. The ice driver has macros for this, so rewrite the function to use FIELD_GET macros instead.
Signature · 2 lines
Thanks, Anatoly