Received-SPF: pass (google.com: domain of linux-kernel+bounces-17778-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Fri, 5 Jan 2024 11:05:08 +0000
From: Simon Horman <horms@kernel.org>
To: Shinas Rasheed <srasheed@marvell.com>
Cc: Jakub Kicinski <kuba@kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Haseeb Gani <hgani@marvell.com>,
	Vimlesh Kumar <vimleshk@marvell.com>,
	Sathesh B Edara <sedara@marvell.com>,
	"egallen@redhat.com" <egallen@redhat.com>,
	"mschmidt@redhat.com" <mschmidt@redhat.com>,
	"pabeni@redhat.com" <pabeni@redhat.com>,
	"wizhao@redhat.com" <wizhao@redhat.com>,
	"kheib@redhat.com" <kheib@redhat.com>,
	"konguyen@redhat.com" <konguyen@redhat.com>,
	Veerasenareddy Burru <vburru@marvell.com>,
	Satananda Burla <sburla@marvell.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>
Subject: Re: [EXT] Re: [PATCH net-next v2 6/8] octeon_ep_vf: add Tx/Rx
 processing and interrupt support
Message-ID: <20240105110508.GU31813@kernel.org>
References: <20231223134000.2906144-1-srasheed@marvell.com>
 <20231223134000.2906144-7-srasheed@marvell.com>
 <20240104130016.47bc1071@kernel.org>
 <PH0PR18MB473450986CAA852218F225F0C7662@PH0PR18MB4734.namprd18.prod.outlook.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <PH0PR18MB473450986CAA852218F225F0C7662@PH0PR18MB4734.namprd18.prod.outlook.com>

On Fri, Jan 05, 2024 at 07:26:14AM +0000, Shinas Rasheed wrote:
> Hi Jakub,
> 
> Thanks for the review!
> 
> > > +	rx_done = octep_vf_oq_process_rx(ioq_vector->oq, budget);
> > 
> > You should not call Rx processing if budget is 0 at all, please see
> > NAPI docs. Looks like the function may try to refill Rx buffers with
> > budget of 0.
> > 
> If budget is zero, octep_vf_oq_process_rx just wouldn't try to query hw for packets. Also since by then, the refill count should have been less than refill threshold, if not it only flushes free buffers back to the ring, (and tell hw that there are more free buffers available which have been processed from maybe previous calls - but this seems unlikely and should have been flushed at that time). 
> 
> > > @@ -114,8 +158,8 @@ static int octep_vf_setup_oq(struct octep_vf_device
> > *oct, int q_no)
> > >  		goto desc_dma_alloc_err;
> > >  	}
> > >
> > > -	oq->buff_info = (struct octep_vf_rx_buffer *)
> > > -			vzalloc(oq->max_count *
> > OCTEP_VF_OQ_RECVBUF_SIZE);
> > > +	oq->buff_info = vzalloc(oq->max_count *
> > OCTEP_VF_OQ_RECVBUF_SIZE);
> > > +
> > 
> > bad fixup squash?
> > 
> Sorry, I didn't understand. Can you explain?

I think he means that this change seems to make a minor update
to code introduced in a previous patch of this series, and thus should
be squashed into that previous patch.