RE: [EXT] Re: [PATCH net-next v2 6/8] octeon_ep_vf: add Tx/Rx processing and interrupt support

[Shinas Rasheed]

Thanks Simon for the comments. Will update in v3

> -----Original Message-----
> From: Simon Horman <horms@xxxxxxxxxx>
> Sent: Friday, January 5, 2024 4:35 PM
> To: Shinas Rasheed <srasheed@xxxxxxxxxxx>
> Cc: Jakub Kicinski <kuba@xxxxxxxxxx>; netdev@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; Haseeb Gani <hgani@xxxxxxxxxxx>; Vimlesh Kumar
> <vimleshk@xxxxxxxxxxx>; Sathesh B Edara <sedara@xxxxxxxxxxx>;
> egallen@xxxxxxxxxx; mschmidt@xxxxxxxxxx; pabeni@xxxxxxxxxx;
> wizhao@xxxxxxxxxx; kheib@xxxxxxxxxx; konguyen@xxxxxxxxxx;
> Veerasenareddy Burru <vburru@xxxxxxxxxxx>; Satananda Burla
> <sburla@xxxxxxxxxxx>; David S. Miller <davem@xxxxxxxxxxxxx>; Eric
> Dumazet <edumazet@xxxxxxxxxx>
> Subject: Re: [EXT] Re: [PATCH net-next v2 6/8] octeon_ep_vf: add Tx/Rx
> processing and interrupt support
> 
> 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.