From: Vlad Zolotarov <vladz@cloudius-systems•com>
To: Alexander Duyck <alexander.h.duyck@redhat•com>,
Jeff Kirsher <jeffrey.t.kirsher@intel•com>,
davem@davemloft•net
Cc: Emil Tantilov <emil.s.tantilov@intel•com>,
netdev@vger•kernel.org, nhorman@redhat•com, sassmann@redhat•com,
jogreene@redhat•com
Subject: Re: [net-next 05/16] ixgbevf: enable multiple queue support
Date: Wed, 25 Mar 2015 21:49:39 +0200 [thread overview]
Message-ID: <55131153.2090401@cloudius-systems.com> (raw)
In-Reply-To: <5512FEDC.7080902@redhat.com>
On 03/25/15 20:30, Alexander Duyck wrote:
> On 03/25/2015 01:44 AM, Vlad Zolotarov wrote:
>>
>>
>> On 02/05/15 11:35, Jeff Kirsher wrote:
>>> From: Emil Tantilov <emil.s.tantilov@intel•com>
>>>
>>> This patch enables multiple queues and RSS support for the VF.
>>> Maximum of 2 queues are supported due to available vectors.
>>>
>>> Signed-off-by: Emil Tantilov <emil.s.tantilov@intel•com>
>>> Tested-by: Krishneil Singh <Krishneil.k.singh@intel•com>
>>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel•com>
>>> ---
>>> drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 1 +
>>> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 22
>>> +++++++++++++++++++---
>>> 2 files changed, 20 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
>>> b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
>>> index 8c44ab2..65cda34 100644
>>> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
>>> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
>>> @@ -124,6 +124,7 @@ struct ixgbevf_ring {
>>> #define MAX_RX_QUEUES IXGBE_VF_MAX_RX_QUEUES
>>> #define MAX_TX_QUEUES IXGBE_VF_MAX_TX_QUEUES
>>> +#define IXGBEVF_MAX_RSS_QUEUES 2
>>
>> According to the MRQE register description in 82599 and x540 specs 2
>> (the value of IXGBEVF_MAX_RSS_QUEUES) is the minimal allowed number
>> of queues in a single pool thus it's a minimal number of VF queues -
>> not the maximal (see more comments on this below).
>
> This is the maximum since the VF only supports 3 interrupts, one for
> mailbox and 2 for queues, and RSS requires an interrupt per queue
> pair. The other possibility is that DCB has been enabled in which
> case there RSS is reduced to one queue pair per pool with one RSS pool
> for each traffic class.
>
>>
>>> #define IXGBEVF_DEFAULT_TXD 1024
>>> #define IXGBEVF_DEFAULT_RXD 512
>>> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>>> b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>>> index c9b49bf..815808f 100644
>>> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>>> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>>> @@ -1794,7 +1794,8 @@ static int ixgbevf_configure_dcb(struct
>>> ixgbevf_adapter *adapter)
>>> struct ixgbe_hw *hw = &adapter->hw;
>>> unsigned int def_q = 0;
>>> unsigned int num_tcs = 0;
>>> - unsigned int num_rx_queues = 1;
>>> + unsigned int num_rx_queues = adapter->num_rx_queues;
>>> + unsigned int num_tx_queues = adapter->num_tx_queues;
>>> int err;
>>> spin_lock_bh(&adapter->mbx_lock);
>>> @@ -1808,6 +1809,9 @@ static int ixgbevf_configure_dcb(struct
>>> ixgbevf_adapter *adapter)
>>> return err;
>>> if (num_tcs > 1) {
>>> + /* we need only one Tx queue */
>>> + num_tx_queues = 1;
>>> +
>>> /* update default Tx ring register index */
>>> adapter->tx_ring[0]->reg_idx = def_q;
>>> @@ -1816,7 +1820,8 @@ static int ixgbevf_configure_dcb(struct
>>> ixgbevf_adapter *adapter)
>>> }
>>> /* if we have a bad config abort request queue reset */
>>> - if (adapter->num_rx_queues != num_rx_queues) {
>>> + if ((adapter->num_rx_queues != num_rx_queues) ||
>>> + (adapter->num_tx_queues != num_tx_queues)) {
>>> /* force mailbox timeout to prevent further messages */
>>> hw->mbx.timeout = 0;
>>> @@ -2181,8 +2186,19 @@ static void ixgbevf_set_num_queues(struct
>>> ixgbevf_adapter *adapter)
>>> return;
>>> /* we need as many queues as traffic classes */
>>> - if (num_tcs > 1)
>>> + if (num_tcs > 1) {
>>> adapter->num_rx_queues = num_tcs;
>>> + } else {
>>> + u16 rss = min_t(u16, num_online_cpus(),
>>> IXGBEVF_MAX_RSS_QUEUES);
>>
>> Emil,
>> Here u ignore the IXGBE_VF_RX_QUEUES value returned the PF (stored in
>> the hw->mac.max_rx_queues). This will accidentally work if PF is
>> driven by a current ixgbe Linux driver but what would happen if HV is
>> not a Linux box? What if HV decides to configure the PSRTYPE[x].RQPL
>> to 0 and decide that VF should be configured with a single queue? I
>> maybe miss something here but the above line seems bogus. Please,
>> comment.
>
> It isn't ignored. The fact that num_tcs is being tested for checks
> for the one case where the number of RSS queues available would be 1,
> otherwise the hardware will provide at least 2 queues.
> If the RETA is not configured for 2 queues one of them will not be
> active, but that should not be any sort of actual issue and would be
> more of an administrative choice anyway as it would also limit the PF.
This would be an administrative choice ignored by the ixgbevf driver.
Frankly, I don't understand why are we arguing about this for so long?
It's obvious that the safest and the most correct way to write it would
be to limit the num_rx_queues by the value returned by PF. It would also
remove any further questions and it's a one line patch.
However, if u still insist to keep it the way it is then let's drop this
discussion and move on.
>
> - Alex
next prev parent reply other threads:[~2015-03-25 19:49 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-02-05 9:35 [net-next 00/16][pull request] Intel Wired LAN Driver Updates 2015-02-05 Jeff Kirsher
2015-02-05 9:35 ` [net-next 01/16] fm10k: Validate VLAN ID in fm10k_update_xc_addr_pf Jeff Kirsher
2015-02-05 9:35 ` [net-next 02/16] fm10k: Resolve compile warnings with W=1 Jeff Kirsher
2015-02-05 9:35 ` [net-next 03/16] ixgbe: cleanup sparse errors in new ixgbe_x550.c file Jeff Kirsher
2015-02-05 9:48 ` David Laight
2015-02-05 22:40 ` David Miller
2015-02-05 23:19 ` Skidmore, Donald C
2015-02-05 9:35 ` [net-next 04/16] ixgbe: allow multiple queues in SRIOV mode Jeff Kirsher
2015-02-05 9:35 ` [net-next 05/16] ixgbevf: enable multiple queue support Jeff Kirsher
2015-03-25 8:44 ` Vlad Zolotarov
2015-03-25 14:32 ` Tantilov, Emil S
2015-03-25 16:27 ` Vlad Zolotarov
2015-03-25 18:30 ` Alexander Duyck
2015-03-25 19:49 ` Vlad Zolotarov [this message]
2015-03-25 20:16 ` Alexander Duyck
2015-03-25 20:35 ` Vlad Zolotarov
2015-02-05 9:35 ` [net-next 06/16] ixgbevf: add RSS support for X550 Jeff Kirsher
2015-02-05 9:35 ` [net-next 07/16] ixgbe: fix setting port VLAN Jeff Kirsher
2015-02-05 9:35 ` [net-next 08/16] ixgbe: cleanup redundant default method set_rxpba Jeff Kirsher
2015-02-05 9:35 ` [net-next 09/16] ixgbe: Cleanup probe to remove redundant attempt to ID PHY Jeff Kirsher
2015-02-05 9:35 ` [net-next 10/16] ixgbe: add VXLAN offload support for X550 devices Jeff Kirsher
2015-02-05 9:35 ` [net-next 11/16] ixgbevf: set vlan_features in a single write instead of several ORs Jeff Kirsher
2015-02-05 9:35 ` [net-next 12/16] ixgbevf: Fix ordering of shutdown to correctly disable Rx and Tx Jeff Kirsher
2015-02-05 9:35 ` [net-next 13/16] ixgbevf: Add code to check for Tx hang Jeff Kirsher
2015-02-05 9:35 ` [net-next 14/16] ixgbevf: rewrite watchdog task to function similar to igbvf Jeff Kirsher
2015-02-05 9:35 ` [net-next 15/16] ixgbevf: combine all of the tasks into a single service task Jeff Kirsher
2015-02-05 9:35 ` [net-next 16/16] ixgbe: add Tx anti spoofing support Jeff Kirsher
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55131153.2090401@cloudius-systems.com \
--to=vladz@cloudius-systems$(echo .)com \
--cc=alexander.h.duyck@redhat$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=emil.s.tantilov@intel$(echo .)com \
--cc=jeffrey.t.kirsher@intel$(echo .)com \
--cc=jogreene@redhat$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=nhorman@redhat$(echo .)com \
--cc=sassmann@redhat$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox