Re: [PATCH v3 01/13] net/ixgbe: remove unused field in Rx queue struct

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

On 5/14/2025 6:32 PM, Bruce Richardson wrote:
> On Mon, May 12, 2025 at 01:54:27PM +0100, Anatoly Burakov wrote:
>> The `rdh` (read head) field in the `ixgbe_rx_queue` struct is not used
>> anywhere in the codebase, and can be removed.
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> 
> With the one comment below fixed.
> 
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> 
>> ---
>>   drivers/net/intel/ixgbe/ixgbe_rxtx.c | 9 ++-------
>>   drivers/net/intel/ixgbe/ixgbe_rxtx.h | 1 -
>>   2 files changed, 2 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/net/intel/ixgbe/ixgbe_rxtx.c b/drivers/net/intel/ixgbe/ixgbe_rxtx.c
>> index 95c80ac1b8..0c07ce3186 100644
>> --- a/drivers/net/intel/ixgbe/ixgbe_rxtx.c
>> +++ b/drivers/net/intel/ixgbe/ixgbe_rxtx.c
>> @@ -3296,17 +3296,12 @@ ixgbe_dev_rx_queue_setup(struct rte_eth_dev *dev,
>>   	/*
>>   	 * Modified to setup VFRDT for Virtual Function
>>   	 */
>> -	if (ixgbe_is_vf(dev)) {
>> +	if (ixgbe_is_vf(dev))
>>   		rxq->rdt_reg_addr =
>>   			IXGBE_PCI_REG_ADDR(hw, IXGBE_VFRDT(queue_idx));
>> -		rxq->rdh_reg_addr =
>> -			IXGBE_PCI_REG_ADDR(hw, IXGBE_VFRDH(queue_idx));
>> -	} else {
>> +	else
>>   		rxq->rdt_reg_addr =
>>   			IXGBE_PCI_REG_ADDR(hw, IXGBE_RDT(rxq->reg_idx));
>> -		rxq->rdh_reg_addr =
>> -			IXGBE_PCI_REG_ADDR(hw, IXGBE_RDH(rxq->reg_idx));
>> -	}
>>   
> 
> Unrelated code change. Please remove from this patch in any subsequent
> versions - it's not worth fixing this unless we are otherwise modifying
> this function.

It was removed because we either keep rdh_reg_addr in common Rx queue 
structure, or we get rid of both the field and all its usages. If we 
don't remove this, we'll have to include rdh_reg_addr in common Rx queue 
structure and since it's not used anywhere except here it would be 
pretty pointless to do so.

> 
>>   	rxq->rx_ring_phys_addr = rz->iova;
>>   	rxq->rx_ring = (union ixgbe_adv_rx_desc *) rz->addr;
>> diff --git a/drivers/net/intel/ixgbe/ixgbe_rxtx.h b/drivers/net/intel/ixgbe/ixgbe_rxtx.h
>> index 641f982b01..20a5c5a0af 100644
>> --- a/drivers/net/intel/ixgbe/ixgbe_rxtx.h
>> +++ b/drivers/net/intel/ixgbe/ixgbe_rxtx.h
>> @@ -85,7 +85,6 @@ struct ixgbe_rx_queue {
>>   	volatile union ixgbe_adv_rx_desc *rx_ring; /**< RX ring virtual address. */
>>   	uint64_t            rx_ring_phys_addr; /**< RX ring DMA address. */
>>   	volatile uint32_t   *rdt_reg_addr; /**< RDT register address. */
>> -	volatile uint32_t   *rdh_reg_addr; /**< RDH register address. */
>>   	struct ixgbe_rx_entry *sw_ring; /**< address of RX software ring. */
>>   	struct ixgbe_scattered_rx_entry *sw_sc_ring; /**< address of scattered Rx software ring. */
>>   	struct rte_mbuf *pkt_first_seg; /**< First segment of current packet. */
>> -- 
>> 2.47.1
>>


-- 
Thanks,
Anatoly