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

[Bruce Richardson <bruce.richardson@intel.com>]

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