Question regarding {G,S}CHANNELS API

Question regarding {G,S}CHANNELS API

[Jakub Kicinski]

Hi!

I'm trying to understand how number of "separate" rx/tx vs combined
channels should be configured.  I'd like to express asymmetric but
mostly combined queue configuration (i.e. min(rx, tx) is combined the
rest is separate).  Since default number of RX queues is just 8 it is
tempting to allocate 8 RX queues but num_online_cpus() TX queues.  Does
this configuration make sense?  What should ethtool -l report?

I had a look through the drivers and it seems that most of them fall
nicely into the all combined and all separate categories. Two
exceptions I found are bnxt_en (recent Michael's changes) and tg3.
bnxt_en can switch between combined and separate mode.  tg3 is more
interesting, it uses separate rx/tx parameters but combines first
min(rx, tx) of the queues as I would like to.

The problem is if I hijack the "separate" rx/tx queues parameters like
tg3 does there is no way for the user to express that (s)he truly wants
them separate.  Also it seems that tg3 goes against the ethtool manual
which states:
>  rx N   Changes the number of channels with only receive queues.
>  tx N   Changes the number of channels with only transmit queues.
Which indicates that if user wants 8 rx, 12 tx and combining (s)he
should do:
  ethtool -L ethX combined 8 tx 4

This, however, makes the max_* info from SCHANNLES quite imprecise
since device which has 16 IRQs and 16 queues in each directions would
probably report:
max_rx:       16
max_tx:       16
max_combined: 16
But setting 'ethtool -L ethX combined 16 tx 8' would obviously
fail even though parameters are within 1..max limits.

Any comments/advice would be much appreciated.

Re: Question regarding {G,S}CHANNELS API

[Prashant Sreedharan]

On Thu, 2016-01-07 at 15:43 +0000, Jakub Kicinski wrote:
> Hi!
> 
> I'm trying to understand how number of "separate" rx/tx vs combined
> channels should be configured.  I'd like to express asymmetric but
> mostly combined queue configuration (i.e. min(rx, tx) is combined the
> rest is separate).  Since default number of RX queues is just 8 it is
> tempting to allocate 8 RX queues but num_online_cpus() TX queues.  Does
> this configuration make sense?  What should ethtool -l report?
> 
> I had a look through the drivers and it seems that most of them fall
> nicely into the all combined and all separate categories. Two
> exceptions I found are bnxt_en (recent Michael's changes) and tg3.
> bnxt_en can switch between combined and separate mode.  tg3 is more
> interesting, it uses separate rx/tx parameters but combines first
> min(rx, tx) of the queues as I would like to.

It does not combine it takes the max(rx, tx)
u32 irq_cnt = max(tp->rxq_cnt, tp->txq_cnt);

> 
> The problem is if I hijack the "separate" rx/tx queues parameters like
> tg3 does there is no way for the user to express that (s)he truly wants
> them separate.  Also it seems that tg3 goes against the ethtool manual
> which states:
> >  rx N   Changes the number of channels with only receive queues.
> >  tx N   Changes the number of channels with only transmit queues.
> Which indicates that if user wants 8 rx, 12 tx and combining (s)he
> should do:
>   ethtool -L ethX combined 8 tx 4

there is no combined option only separate tx/rx option is supported by
tg3 driver. And at max only 4 RX, 4 TX rings are supported by the tg3
family of chips in non-iov mode.

Each status block (which is similar to completion ring) supports 1 RX or
1 TX or both. So at max there are 4 status blocks plus 1 additional
status block for async events. And each status block is assigned a irq.

Re: Question regarding {G,S}CHANNELS API

[Jakub Kicinski]

On Thu, 7 Jan 2016 13:24:24 -0800, Prashant Sreedharan wrote:
> On Thu, 2016-01-07 at 15:43 +0000, Jakub Kicinski wrote:
> > Hi!
> >
> > I'm trying to understand how number of "separate" rx/tx vs combined
> > channels should be configured.  I'd like to express asymmetric but
> > mostly combined queue configuration (i.e. min(rx, tx) is combined the
> > rest is separate).  Since default number of RX queues is just 8 it is
> > tempting to allocate 8 RX queues but num_online_cpus() TX queues.  Does
> > this configuration make sense?  What should ethtool -l report?
> >
> > I had a look through the drivers and it seems that most of them fall
> > nicely into the all combined and all separate categories. Two
> > exceptions I found are bnxt_en (recent Michael's changes) and tg3.
> > bnxt_en can switch between combined and separate mode.  tg3 is more
> > interesting, it uses separate rx/tx parameters but combines first
> > min(rx, tx) of the queues as I would like to.
>
> It does not combine it takes the max(rx, tx)
> u32 irq_cnt = max(tp->rxq_cnt, tp->txq_cnt);
>
> >
> > The problem is if I hijack the "separate" rx/tx queues parameters like
> > tg3 does there is no way for the user to express that (s)he truly wants
> > them separate.  Also it seems that tg3 goes against the ethtool manual
> > which states:
> > >  rx N   Changes the number of channels with only receive queues.
> > >  tx N   Changes the number of channels with only transmit queues.
> > Which indicates that if user wants 8 rx, 12 tx and combining (s)he
> > should do:
> >   ethtool -L ethX combined 8 tx 4
>
> there is no combined option only separate tx/rx option is supported by
> tg3 driver. And at max only 4 RX, 4 TX rings are supported by the tg3
> family of chips in non-iov mode.
>
> Each status block (which is similar to completion ring) supports 1 RX or
> 1 TX or both. So at max there are 4 status blocks plus 1 additional
> status block for async events. And each status block is assigned a irq.

Thanks for response Prashant.

I think what you describe is exactly what is meant by combined rx/tx
rings.  Single status block used for both RX and TX. tg3 can allocate
up to max_rx RX queues and up to max_tx TX queues, the numbers are
independent but then first min(rx, tx) will get combined and use the
same block and IRQ, right?

The point of my email was that it's a bit cumbersome to express such
configuration with the current ethtool API.  Therefore I'm asking if
it's preferred to use:
  ethtool ethX -L rx N tx M
in this case, or:
  ethtool ethX -L combined N tx/rx abs(M-N)
If the second then I think I'll post a patch to extend ethtool's help
since it may be non-obvious to users.

Re: Question regarding {G,S}CHANNELS API

[Prashant Sreedharan]

On Fri, 2016-01-08 at 11:41 +0000, Jakub Kicinski wrote:

> The point of my email was that it's a bit cumbersome to express such
> configuration with the current ethtool API.  Therefore I'm asking if
> it's preferred to use:
>   ethtool ethX -L rx N tx M
For tg3 driver this is the preferred way

> in this case, or:
>   ethtool ethX -L combined N tx/rx abs(M-N)
tg3 will ignore the combined option and only consider separate tx / rx
values here. Combined will always be displayed as 0 to the user.

We understand your concern, and agree that the internal implementation
does not assign separate irq's to each tx, rx rings when tx/rx option is
used by user. With limited number of status blocks (max 4) and a HW bug
due to which not more than 1 TX ring is recommended to be enabled, it
was implemented in this manner.