Re: [PATCH v2 net-next 7/7] net: ethernet: ti: cpsw: add XDP support

[Saeed Mahameed]

On Fri, 2019-05-31 at 20:03 +0300, Ivan Khoronzhuk wrote:
> On Fri, May 31, 2019 at 06:32:41PM +0200, Jesper Dangaard Brouer
> wrote:
> > On Fri, 31 May 2019 19:25:24 +0300 Ivan Khoronzhuk <
> > ivan.khoronzhuk@xxxxxxxxxx> wrote:
> > 
> > > On Fri, May 31, 2019 at 05:46:43PM +0200, Jesper Dangaard Brouer
> > > wrote:
> > > > From below code snippets, it looks like you only allocated 1
> > > > page_pool
> > > > and sharing it with several RX-queues, as I don't have the full
> > > > context
> > > > and don't know this driver, I might be wrong?
> > > > 
> > > > To be clear, a page_pool object is needed per RX-queue, as it
> > > > is
> > > > accessing a small RX page cache (which protected by
> > > > NAPI/softirq).
> > > 
> > > There is one RX interrupt and one RX NAPI for all rx channels.
> > 
> > So, what are you saying?
> > 
> > You _are_ sharing the page_pool between several RX-channels, but it
> > is
> > safe because this hardware only have one RX interrupt + NAPI
> > instance??
> 
> I can miss smth but in case of cpsw technically it means:
> 1) RX interrupts are disabled while NAPI is scheduled,
>    not for particular CPU or channel, but at all, for whole cpsw
> module.
> 2) RX channels are handled one by one by priority.

Hi Ivan, I got a silly question.. 

What is the reason behind having multiple RX rings and one CPU/NAPI
handling all of them ? priority ? how do you priorities ?

> 3) After all of them handled and no more in budget - interrupts are
> enabled.
> 4) If page is returned to the pool, and it's within NAPI, no races as
> it's
>    returned protected by softirq. If it's returned not in softirq
> it's protected 
>    by producer lock of the ring.
> 
> Probably it's not good example for others how it should be used, not
> a big
> problem to move it to separate pools.., even don't remember why I
> decided to
> use shared pool, there was some more reasons... need search in
> history.
> 
> > -- 
> > Best regards,
> >  Jesper Dangaard Brouer
> >  MSc.CS, Principal Kernel Engineer at Red Hat
> >  LinkedIn: http://www.linkedin.com/in/brouer