diff mbox series

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

Message ID afcd4013458bdb8b970e7ed57d6a1ede0ee654b8.1747054471.git.anatoly.burakov@intel.com (mailing list archive)
State Superseded
Delegated to: Bruce Richardson
Headers
Series [v3,01/13] net/ixgbe: remove unused field in Rx queue struct |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Burakov, Anatoly May 12, 2025, 12:54 p.m. UTC 
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>
---
 drivers/net/intel/ixgbe/ixgbe_rxtx.c | 9 ++-------
 drivers/net/intel/ixgbe/ixgbe_rxtx.h | 1 -
 2 files changed, 2 insertions(+), 8 deletions(-)

Comments

Bruce Richardson May 12, 2025, 12:58 p.m. UTC | #1 
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>
> ---
>  drivers/net/intel/ixgbe/ixgbe_rxtx.c | 9 ++-------
>  drivers/net/intel/ixgbe/ixgbe_rxtx.h | 1 -
>  2 files changed, 2 insertions(+), 8 deletions(-)
> 
Hi Anatoly,

for any future versions, can you please include a cover letter, with a
summary indicating what patches have changed any why. I hope to start
reviewing this patchset shortly.

Thanks,
/Bruce
Bruce Richardson May 14, 2025, 4:32 p.m. UTC | #2 
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.

>  	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
>
Burakov, Anatoly May 15, 2025, 11:15 a.m. UTC | #3 
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
>>
Bruce Richardson May 15, 2025, 12:58 p.m. UTC | #4 
On Thu, May 15, 2025 at 01:15:43PM +0200, Burakov, Anatoly wrote:
> 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.
> 
My mistake. Yes, this is a proper change belonging here, sorry.

/Bruce
diff mbox series

Patch

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));
-	}
 
 	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. */