[dpdk-dev,3/5] testpmd: Change rxfreet default to 32

  To improve performance by using bulk alloc or vectored RX routines, we
need to set rx free threshold (rxfreet) value to 32, so make this the
testpmd default.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 app/test-pmd/testpmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

On Wed, Sep 17, 2014 at 11:01:40AM +0100, Bruce Richardson wrote:
> To improve performance by using bulk alloc or vectored RX routines, we
> need to set rx free threshold (rxfreet) value to 32, so make this the
> testpmd default.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  app/test-pmd/testpmd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 9f6cdc4..5751607 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -225,7 +225,7 @@ struct rte_eth_thresh tx_thresh = {
>  /*
>   * Configurable value of RX free threshold.
>   */
> -uint16_t rx_free_thresh = 0; /* Immediately free RX descriptors by default. */
> +uint16_t rx_free_thresh = 32; /* Refill RX descriptors once every 32 packets */
>  

Why 32?  Was that an experimentally determined value?  Does it hold true for all
PMD's?
  

> -----Original Message-----
> From: Neil Horman [mailto:nhorman@tuxdriver.com]
> Sent: Wednesday, September 17, 2014 4:30 PM
> To: Richardson, Bruce
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 3/5] testpmd: Change rxfreet default to 32
> 
> On Wed, Sep 17, 2014 at 11:01:40AM +0100, Bruce Richardson wrote:
> > To improve performance by using bulk alloc or vectored RX routines, we
> > need to set rx free threshold (rxfreet) value to 32, so make this the
> > testpmd default.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >  app/test-pmd/testpmd.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> > index 9f6cdc4..5751607 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -225,7 +225,7 @@ struct rte_eth_thresh tx_thresh = {
> >  /*
> >   * Configurable value of RX free threshold.
> >   */
> > -uint16_t rx_free_thresh = 0; /* Immediately free RX descriptors by default. */
> > +uint16_t rx_free_thresh = 32; /* Refill RX descriptors once every 32 packets
> */
> >
> 
> Why 32?  Was that an experimentally determined value?  Does it hold true for all
> PMD's?

This is primarily for the ixgbe PMD, which is right now the most highly tuned driver, but it works fine for all other ones too, as far as I'm aware. Basically, this is the minimum setting needed to enable either the bulk alloc or vector RX routines inside the ixgbe driver, so it's best made the default for that reason. Please see " check_rx_burst_bulk_alloc_preconditions()" in ixgbe_rxtx.c, and RX function assignment logic in "ixgbe_dev_rx_queue_setup()" in the same file. 

/Bruce
  

2014-09-18 15:53, Richardson, Bruce:
> > > --- a/app/test-pmd/testpmd.c
> > > +++ b/app/test-pmd/testpmd.c
> > > @@ -225,7 +225,7 @@ struct rte_eth_thresh tx_thresh = {
> > >  /*
> > >   * Configurable value of RX free threshold.
> > >   */
> > > -uint16_t rx_free_thresh = 0; /* Immediately free RX descriptors by default. */
> > > +uint16_t rx_free_thresh = 32; /* Refill RX descriptors once every 32 packets
> > */
> > >
> > 
> > Why 32?  Was that an experimentally determined value?
> > Does it hold true for all PMD's?
> 
> This is primarily for the ixgbe PMD, which is right now the most
> highly tuned driver, but it works fine for all other ones too,
> as far as I'm aware.

Yes, you are changing this value for all PMDs but you're targetting
only one.
These thresholds are dependent of the PMD implementation. There's
something wrong here.

> Basically, this is the minimum setting needed to enable either the
> bulk alloc or vector RX routines inside the ixgbe driver, so it's
> best made the default for that reason. Please see
> "check_rx_burst_bulk_alloc_preconditions()" in ixgbe_rxtx.c, and
> RX function assignment logic in "ixgbe_dev_rx_queue_setup()" in
> the same file.

Since this parameter is so important, it could be a default value somewhere.

I think we should split generic tuning parameters and tuning parameters
related to driver implementation or specific hardware.
Then we should provide some good default values for each of them.
At last, if needed, applications should be able to easily tune the
pmd-specific parameters.

Thoughts?
  

On Thu, Sep 18, 2014 at 03:53:33PM +0000, Richardson, Bruce wrote:
> > -----Original Message-----
> > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > Sent: Wednesday, September 17, 2014 4:30 PM
> > To: Richardson, Bruce
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH 3/5] testpmd: Change rxfreet default to 32
> > 
> > On Wed, Sep 17, 2014 at 11:01:40AM +0100, Bruce Richardson wrote:
> > > To improve performance by using bulk alloc or vectored RX routines, we
> > > need to set rx free threshold (rxfreet) value to 32, so make this the
> > > testpmd default.
> > >
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > ---
> > >  app/test-pmd/testpmd.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> > > index 9f6cdc4..5751607 100644
> > > --- a/app/test-pmd/testpmd.c
> > > +++ b/app/test-pmd/testpmd.c
> > > @@ -225,7 +225,7 @@ struct rte_eth_thresh tx_thresh = {
> > >  /*
> > >   * Configurable value of RX free threshold.
> > >   */
> > > -uint16_t rx_free_thresh = 0; /* Immediately free RX descriptors by default. */
> > > +uint16_t rx_free_thresh = 32; /* Refill RX descriptors once every 32 packets
> > */
> > >
> > 
> > Why 32?  Was that an experimentally determined value?  Does it hold true for all
> > PMD's?
> 
> This is primarily for the ixgbe PMD, which is right now the most highly tuned driver, but it works fine for all other ones too, as far as I'm aware. Basically, this is the minimum setting needed to enable either the bulk alloc or vector RX routines inside the ixgbe driver, so it's best made the default for that reason. Please see " check_rx_burst_bulk_alloc_preconditions()" in ixgbe_rxtx.c, and RX function assignment logic in "ixgbe_dev_rx_queue_setup()" in the same file. 
> 

Maybe codify that information as a macro, or with some documentation right above
the setting, so people not using ixgbe don't have to wonder where that value
came from? :)

Neil

> /Bruce
> 
>
  

On Thu, Sep 18, 2014 at 07:13:52PM +0200, Thomas Monjalon wrote:
> 2014-09-18 15:53, Richardson, Bruce:
> > > > --- a/app/test-pmd/testpmd.c
> > > > +++ b/app/test-pmd/testpmd.c
> > > > @@ -225,7 +225,7 @@ struct rte_eth_thresh tx_thresh = {
> > > >  /*
> > > >   * Configurable value of RX free threshold.
> > > >   */
> > > > -uint16_t rx_free_thresh = 0; /* Immediately free RX descriptors by default. */
> > > > +uint16_t rx_free_thresh = 32; /* Refill RX descriptors once every 32 packets
> > > */
> > > >
> > > 
> > > Why 32?  Was that an experimentally determined value?
> > > Does it hold true for all PMD's?
> > 
> > This is primarily for the ixgbe PMD, which is right now the most
> > highly tuned driver, but it works fine for all other ones too,
> > as far as I'm aware.
> 
> Yes, you are changing this value for all PMDs but you're targetting
> only one.
> These thresholds are dependent of the PMD implementation. There's
> something wrong here.
> 
I agree. Its fine to do this, but it does seem like the sample application
should document why it does this and make note of the fact that other PMDs may
have a separate optimal value.

> > Basically, this is the minimum setting needed to enable either the
> > bulk alloc or vector RX routines inside the ixgbe driver, so it's
> > best made the default for that reason. Please see
> > "check_rx_burst_bulk_alloc_preconditions()" in ixgbe_rxtx.c, and
> > RX function assignment logic in "ixgbe_dev_rx_queue_setup()" in
> > the same file.
> 
> Since this parameter is so important, it could be a default value somewhere.
> 
> I think we should split generic tuning parameters and tuning parameters
> related to driver implementation or specific hardware.
> Then we should provide some good default values for each of them.
> At last, if needed, applications should be able to easily tune the
> pmd-specific parameters.
> 
I like this idea.  I've not got an idea of how much work it is to do so, but in
principle it makes sense.

Perhaps for the immediate need, since rte_eth_rx_queue_setup allows the config
struct to get passed directly to PMDs, we can create a reserved value
RTE_ETH_RX_FREE_THRESH_OPTIMAL, that instructs the pmd to select whatever
threshold is optimal for its own hardware?

Neil

> Thoughts?
> 
> -- 
> Thomas
>
  

> -----Original Message-----
> From: Neil Horman [mailto:nhorman@tuxdriver.com]
> Sent: Thursday, September 18, 2014 7:09 PM
> To: Thomas Monjalon
> Cc: Richardson, Bruce; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 3/5] testpmd: Change rxfreet default to 32
> 
> On Thu, Sep 18, 2014 at 07:13:52PM +0200, Thomas Monjalon wrote:
> > 2014-09-18 15:53, Richardson, Bruce:
> > > > > --- a/app/test-pmd/testpmd.c
> > > > > +++ b/app/test-pmd/testpmd.c
> > > > > @@ -225,7 +225,7 @@ struct rte_eth_thresh tx_thresh = {
> > > > >  /*
> > > > >   * Configurable value of RX free threshold.
> > > > >   */
> > > > > -uint16_t rx_free_thresh = 0; /* Immediately free RX descriptors by
> default. */
> > > > > +uint16_t rx_free_thresh = 32; /* Refill RX descriptors once every 32
> packets
> > > > */
> > > > >
> > > >
> > > > Why 32?  Was that an experimentally determined value?
> > > > Does it hold true for all PMD's?
> > >
> > > This is primarily for the ixgbe PMD, which is right now the most
> > > highly tuned driver, but it works fine for all other ones too,
> > > as far as I'm aware.
> >
> > Yes, you are changing this value for all PMDs but you're targetting
> > only one.
> > These thresholds are dependent of the PMD implementation. There's
> > something wrong here.
> >
> I agree. Its fine to do this, but it does seem like the sample application
> should document why it does this and make note of the fact that other PMDs
> may
> have a separate optimal value.
> 
> > > Basically, this is the minimum setting needed to enable either the
> > > bulk alloc or vector RX routines inside the ixgbe driver, so it's
> > > best made the default for that reason. Please see
> > > "check_rx_burst_bulk_alloc_preconditions()" in ixgbe_rxtx.c, and
> > > RX function assignment logic in "ixgbe_dev_rx_queue_setup()" in
> > > the same file.
> >
> > Since this parameter is so important, it could be a default value somewhere.
> >
> > I think we should split generic tuning parameters and tuning parameters
> > related to driver implementation or specific hardware.
> > Then we should provide some good default values for each of them.
> > At last, if needed, applications should be able to easily tune the
> > pmd-specific parameters.
> >
> I like this idea.  I've not got an idea of how much work it is to do so, but in
> principle it makes sense.
> 
> Perhaps for the immediate need, since rte_eth_rx_queue_setup allows the
> config
> struct to get passed directly to PMDs, we can create a reserved value
> RTE_ETH_RX_FREE_THRESH_OPTIMAL, that instructs the pmd to select
> whatever
> threshold is optimal for its own hardware?
> 
> Neil
> 
Actually, looking at the code, I would suggest a couple of options, some of which may be used together.
	1) we make NULL a valid value for the rxconf structure parameter to rte_eth_rx_queue_setup. There is little information in it that should really need to be passed in by applications to the drivers, and that would allow the drivers to be completely free to select the best options for their own operation. 
	2) As a companion to that (or as an alternative), we could also allow each driver to provide its own functions for rte_eth_get_rxconf_default, and rte_eth_get_txconf_default, to be used by applications that want to use known-good values for thresholds but also want to tweak one of the other values e.g. for rx, set the drop_en bit, and for tx set the txqflags to disable offloads.
	3) Lastly, we could also consider removing the threshold and other not-generally-used values from the rxconf and txconf structures and make those removed fields completely driver-set values. Optionally, we could provide an alternate API to tune them, but I don't really see this being useful in most cases, and I'd probably omit it unless someone can prove a need for such APIs.

Regards,
/Bruce
  

On Fri, Sep 19, 2014 at 09:18:26AM +0000, Richardson, Bruce wrote:
> > -----Original Message-----
> > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > Sent: Thursday, September 18, 2014 7:09 PM
> > To: Thomas Monjalon
> > Cc: Richardson, Bruce; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH 3/5] testpmd: Change rxfreet default to 32
> > 
> > On Thu, Sep 18, 2014 at 07:13:52PM +0200, Thomas Monjalon wrote:
> > > 2014-09-18 15:53, Richardson, Bruce:
> > > > > > --- a/app/test-pmd/testpmd.c
> > > > > > +++ b/app/test-pmd/testpmd.c
> > > > > > @@ -225,7 +225,7 @@ struct rte_eth_thresh tx_thresh = {
> > > > > >  /*
> > > > > >   * Configurable value of RX free threshold.
> > > > > >   */
> > > > > > -uint16_t rx_free_thresh = 0; /* Immediately free RX descriptors by
> > default. */
> > > > > > +uint16_t rx_free_thresh = 32; /* Refill RX descriptors once every 32
> > packets
> > > > > */
> > > > > >
> > > > >
> > > > > Why 32?  Was that an experimentally determined value?
> > > > > Does it hold true for all PMD's?
> > > >
> > > > This is primarily for the ixgbe PMD, which is right now the most
> > > > highly tuned driver, but it works fine for all other ones too,
> > > > as far as I'm aware.
> > >
> > > Yes, you are changing this value for all PMDs but you're targetting
> > > only one.
> > > These thresholds are dependent of the PMD implementation. There's
> > > something wrong here.
> > >
> > I agree. Its fine to do this, but it does seem like the sample application
> > should document why it does this and make note of the fact that other PMDs
> > may
> > have a separate optimal value.
> > 
> > > > Basically, this is the minimum setting needed to enable either the
> > > > bulk alloc or vector RX routines inside the ixgbe driver, so it's
> > > > best made the default for that reason. Please see
> > > > "check_rx_burst_bulk_alloc_preconditions()" in ixgbe_rxtx.c, and
> > > > RX function assignment logic in "ixgbe_dev_rx_queue_setup()" in
> > > > the same file.
> > >
> > > Since this parameter is so important, it could be a default value somewhere.
> > >
> > > I think we should split generic tuning parameters and tuning parameters
> > > related to driver implementation or specific hardware.
> > > Then we should provide some good default values for each of them.
> > > At last, if needed, applications should be able to easily tune the
> > > pmd-specific parameters.
> > >
> > I like this idea.  I've not got an idea of how much work it is to do so, but in
> > principle it makes sense.
> > 
> > Perhaps for the immediate need, since rte_eth_rx_queue_setup allows the
> > config
> > struct to get passed directly to PMDs, we can create a reserved value
> > RTE_ETH_RX_FREE_THRESH_OPTIMAL, that instructs the pmd to select
> > whatever
> > threshold is optimal for its own hardware?
> > 
> > Neil
> > 
> Actually, looking at the code, I would suggest a couple of options, some of which may be used together.
> 	1) we make NULL a valid value for the rxconf structure parameter to rte_eth_rx_queue_setup. There is little information in it that should really need to be passed in by applications to the drivers, and that would allow the drivers to be completely free to select the best options for their own operation. 
> 	2) As a companion to that (or as an alternative), we could also allow each driver to provide its own functions for rte_eth_get_rxconf_default, and rte_eth_get_txconf_default, to be used by applications that want to use known-good values for thresholds but also want to tweak one of the other values e.g. for rx, set the drop_en bit, and for tx set the txqflags to disable offloads.
> 	3) Lastly, we could also consider removing the threshold and other not-generally-used values from the rxconf and txconf structures and make those removed fields completely driver-set values. Optionally, we could provide an alternate API to tune them, but I don't really see this being useful in most cases, and I'd probably omit it unless someone can prove a need for such APIs.
> 
These all sound fairly reasonable to me.
Neil

> Regards,
> /Bruce
>
  

> -----Original Message-----
> From: Neil Horman [mailto:nhorman@tuxdriver.com]
> Sent: Friday, September 19, 2014 11:25 AM
> To: Richardson, Bruce
> Cc: Thomas Monjalon; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 3/5] testpmd: Change rxfreet default to 32
> 
> On Fri, Sep 19, 2014 at 09:18:26AM +0000, Richardson, Bruce wrote:
> > > -----Original Message-----
> > > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > > Sent: Thursday, September 18, 2014 7:09 PM
> > > To: Thomas Monjalon
> > > Cc: Richardson, Bruce; dev@dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH 3/5] testpmd: Change rxfreet default to 32
> > >
> > > On Thu, Sep 18, 2014 at 07:13:52PM +0200, Thomas Monjalon wrote:
> > > > 2014-09-18 15:53, Richardson, Bruce:
> > > > > > > --- a/app/test-pmd/testpmd.c
> > > > > > > +++ b/app/test-pmd/testpmd.c
> > > > > > > @@ -225,7 +225,7 @@ struct rte_eth_thresh tx_thresh = {
> > > > > > >  /*
> > > > > > >   * Configurable value of RX free threshold.
> > > > > > >   */
> > > > > > > -uint16_t rx_free_thresh = 0; /* Immediately free RX descriptors by
> > > default. */
> > > > > > > +uint16_t rx_free_thresh = 32; /* Refill RX descriptors once every 32
> > > packets
> > > > > > */
> > > > > > >
> > > > > >
> > > > > > Why 32?  Was that an experimentally determined value?
> > > > > > Does it hold true for all PMD's?
> > > > >
> > > > > This is primarily for the ixgbe PMD, which is right now the most
> > > > > highly tuned driver, but it works fine for all other ones too,
> > > > > as far as I'm aware.
> > > >
> > > > Yes, you are changing this value for all PMDs but you're targetting
> > > > only one.
> > > > These thresholds are dependent of the PMD implementation. There's
> > > > something wrong here.
> > > >
> > > I agree. Its fine to do this, but it does seem like the sample application
> > > should document why it does this and make note of the fact that other PMDs
> > > may
> > > have a separate optimal value.
> > >
> > > > > Basically, this is the minimum setting needed to enable either the
> > > > > bulk alloc or vector RX routines inside the ixgbe driver, so it's
> > > > > best made the default for that reason. Please see
> > > > > "check_rx_burst_bulk_alloc_preconditions()" in ixgbe_rxtx.c, and
> > > > > RX function assignment logic in "ixgbe_dev_rx_queue_setup()" in
> > > > > the same file.
> > > >
> > > > Since this parameter is so important, it could be a default value
> somewhere.
> > > >
> > > > I think we should split generic tuning parameters and tuning parameters
> > > > related to driver implementation or specific hardware.
> > > > Then we should provide some good default values for each of them.
> > > > At last, if needed, applications should be able to easily tune the
> > > > pmd-specific parameters.
> > > >
> > > I like this idea.  I've not got an idea of how much work it is to do so, but in
> > > principle it makes sense.
> > >
> > > Perhaps for the immediate need, since rte_eth_rx_queue_setup allows the
> > > config
> > > struct to get passed directly to PMDs, we can create a reserved value
> > > RTE_ETH_RX_FREE_THRESH_OPTIMAL, that instructs the pmd to select
> > > whatever
> > > threshold is optimal for its own hardware?
> > >
> > > Neil
> > >
> > Actually, looking at the code, I would suggest a couple of options, some of
> which may be used together.
> > 	1) we make NULL a valid value for the rxconf structure parameter to
> rte_eth_rx_queue_setup. There is little information in it that should really need
> to be passed in by applications to the drivers, and that would allow the drivers to
> be completely free to select the best options for their own operation.
> > 	2) As a companion to that (or as an alternative), we could also allow
> each driver to provide its own functions for rte_eth_get_rxconf_default, and
> rte_eth_get_txconf_default, to be used by applications that want to use known-
> good values for thresholds but also want to tweak one of the other values e.g.
> for rx, set the drop_en bit, and for tx set the txqflags to disable offloads.
> > 	3) Lastly, we could also consider removing the threshold and other not-
> generally-used values from the rxconf and txconf structures and make those
> removed fields completely driver-set values. Optionally, we could provide an
> alternate API to tune them, but I don't really see this being useful in most cases,
> and I'd probably omit it unless someone can prove a need for such APIs.
> >
> These all sound fairly reasonable to me.
> Neil

Further thinking seems to me like 1 doesn't really go very far, so it falls between 2 and 3. Any preference between them?

/Bruce
  

On Fri, Sep 19, 2014 at 10:28:31AM +0000, Richardson, Bruce wrote:
> 
> 
> > -----Original Message-----
> > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > Sent: Friday, September 19, 2014 11:25 AM
> > To: Richardson, Bruce
> > Cc: Thomas Monjalon; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH 3/5] testpmd: Change rxfreet default to 32
> > 
> > On Fri, Sep 19, 2014 at 09:18:26AM +0000, Richardson, Bruce wrote:
> > > > -----Original Message-----
> > > > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > > > Sent: Thursday, September 18, 2014 7:09 PM
> > > > To: Thomas Monjalon
> > > > Cc: Richardson, Bruce; dev@dpdk.org
> > > > Subject: Re: [dpdk-dev] [PATCH 3/5] testpmd: Change rxfreet default to 32
> > > >
> > > > On Thu, Sep 18, 2014 at 07:13:52PM +0200, Thomas Monjalon wrote:
> > > > > 2014-09-18 15:53, Richardson, Bruce:
> > > > > > > > --- a/app/test-pmd/testpmd.c
> > > > > > > > +++ b/app/test-pmd/testpmd.c
> > > > > > > > @@ -225,7 +225,7 @@ struct rte_eth_thresh tx_thresh = {
> > > > > > > >  /*
> > > > > > > >   * Configurable value of RX free threshold.
> > > > > > > >   */
> > > > > > > > -uint16_t rx_free_thresh = 0; /* Immediately free RX descriptors by
> > > > default. */
> > > > > > > > +uint16_t rx_free_thresh = 32; /* Refill RX descriptors once every 32
> > > > packets
> > > > > > > */
> > > > > > > >
> > > > > > >
> > > > > > > Why 32?  Was that an experimentally determined value?
> > > > > > > Does it hold true for all PMD's?
> > > > > >
> > > > > > This is primarily for the ixgbe PMD, which is right now the most
> > > > > > highly tuned driver, but it works fine for all other ones too,
> > > > > > as far as I'm aware.
> > > > >
> > > > > Yes, you are changing this value for all PMDs but you're targetting
> > > > > only one.
> > > > > These thresholds are dependent of the PMD implementation. There's
> > > > > something wrong here.
> > > > >
> > > > I agree. Its fine to do this, but it does seem like the sample application
> > > > should document why it does this and make note of the fact that other PMDs
> > > > may
> > > > have a separate optimal value.
> > > >
> > > > > > Basically, this is the minimum setting needed to enable either the
> > > > > > bulk alloc or vector RX routines inside the ixgbe driver, so it's
> > > > > > best made the default for that reason. Please see
> > > > > > "check_rx_burst_bulk_alloc_preconditions()" in ixgbe_rxtx.c, and
> > > > > > RX function assignment logic in "ixgbe_dev_rx_queue_setup()" in
> > > > > > the same file.
> > > > >
> > > > > Since this parameter is so important, it could be a default value
> > somewhere.
> > > > >
> > > > > I think we should split generic tuning parameters and tuning parameters
> > > > > related to driver implementation or specific hardware.
> > > > > Then we should provide some good default values for each of them.
> > > > > At last, if needed, applications should be able to easily tune the
> > > > > pmd-specific parameters.
> > > > >
> > > > I like this idea.  I've not got an idea of how much work it is to do so, but in
> > > > principle it makes sense.
> > > >
> > > > Perhaps for the immediate need, since rte_eth_rx_queue_setup allows the
> > > > config
> > > > struct to get passed directly to PMDs, we can create a reserved value
> > > > RTE_ETH_RX_FREE_THRESH_OPTIMAL, that instructs the pmd to select
> > > > whatever
> > > > threshold is optimal for its own hardware?
> > > >
> > > > Neil
> > > >
> > > Actually, looking at the code, I would suggest a couple of options, some of
> > which may be used together.
> > > 	1) we make NULL a valid value for the rxconf structure parameter to
> > rte_eth_rx_queue_setup. There is little information in it that should really need
> > to be passed in by applications to the drivers, and that would allow the drivers to
> > be completely free to select the best options for their own operation.
> > > 	2) As a companion to that (or as an alternative), we could also allow
> > each driver to provide its own functions for rte_eth_get_rxconf_default, and
> > rte_eth_get_txconf_default, to be used by applications that want to use known-
> > good values for thresholds but also want to tweak one of the other values e.g.
> > for rx, set the drop_en bit, and for tx set the txqflags to disable offloads.
> > > 	3) Lastly, we could also consider removing the threshold and other not-
> > generally-used values from the rxconf and txconf structures and make those
> > removed fields completely driver-set values. Optionally, we could provide an
> > alternate API to tune them, but I don't really see this being useful in most cases,
> > and I'd probably omit it unless someone can prove a need for such APIs.
> > >
> > These all sound fairly reasonable to me.
> > Neil
> 
> Further thinking seems to me like 1 doesn't really go very far, so it falls between 2 and 3. Any preference between them?
> 
Not to answer a question with a question, but, because I'm not really sure, how
much does an application really need to know or set in regards to hardware queue
parameters.  I ask because I'm inclined to just go with option 3 (since it
prevents expansion of the application visible API), but I'm not sure if theres
important functionality you loose in doing so.

Neil

> /Bruce
>