diff mbox

[dpdk-dev,6/7] net/qede: fix maximum VF count to 0

Message ID 1480733039-13046-6-git-send-email-harish.patil@qlogic.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
checkpatch/checkpatch success coding style OK

Commit Message

Harish Patil Dec. 3, 2016, 2:43 a.m. UTC 
  Set max_vfs to 0 since it is relevant only to SR-IOV PF
which is not supported yet.

Fixes: 2ea6f76a ("qede: add core driver")

Signed-off-by: Harish Patil <harish.patil@qlogic.com>
---
 drivers/net/qede/qede_ethdev.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Comments

Ferruh Yigit Dec. 8, 2016, 4:45 p.m. UTC | #1 
On 12/3/2016 2:43 AM, Harish Patil wrote:
> Set max_vfs to 0 since it is relevant only to SR-IOV PF
> which is not supported yet.
> 
> Fixes: 2ea6f76a ("qede: add core driver")
> 
> Signed-off-by: Harish Patil <harish.patil@qlogic.com>

Can you please update patch title to indicate what has been fixed
instead of what has been done in the patch.


btw, while checking feature list, I have seen qede_vf supports SR-IOV,
is that correct?

> ---
>  drivers/net/qede/qede_ethdev.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
> index ee8fb43..10abb8b 100644
> --- a/drivers/net/qede/qede_ethdev.c
> +++ b/drivers/net/qede/qede_ethdev.c
> @@ -976,10 +976,7 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev)
>  	dev_info->max_rx_queues = (uint16_t)QEDE_MAX_RSS_CNT(qdev);
>  	dev_info->max_tx_queues = dev_info->max_rx_queues;
>  	dev_info->max_mac_addrs = qdev->dev_info.num_mac_addrs;
> -	if (IS_VF(edev))
> -		dev_info->max_vfs = 0;
> -	else
> -		dev_info->max_vfs = (uint16_t)NUM_OF_VFS(&qdev->edev);
> +	dev_info->max_vfs = 0;
>  	dev_info->reta_size = ECORE_RSS_IND_TABLE_SIZE;
>  	dev_info->hash_key_size = ECORE_RSS_KEY_SIZE * sizeof(uint32_t);
>  	dev_info->flow_type_rss_offloads = (uint64_t)QEDE_RSS_OFFLOAD_ALL;
>
Harish Patil Dec. 12, 2016, 5:47 p.m. UTC | #2 
>On 12/3/2016 2:43 AM, Harish Patil wrote:

>> Set max_vfs to 0 since it is relevant only to SR-IOV PF

>> which is not supported yet.

>> 

>> Fixes: 2ea6f76a ("qede: add core driver")

>> 

>> Signed-off-by: Harish Patil <harish.patil@qlogic.com>

>

>Can you please update patch title to indicate what has been fixed

>instead of what has been done in the patch.


Can you please clarify? The change in the patch is to set max_vfs=0.
So that’s why the title - "fix maximum VF count to 0"

>

>

>btw, while checking feature list, I have seen qede_vf supports SR-IOV,

>is that correct?


Yes. The supported combination for SR-IOV is VF driver (qede PMD) with PF
driver (qede linux driver).
We don’t have support for SR-IOV PF driver yet. When SR-IOV PF driver is
supported then max_vfs shall return the actual max VFs, but for now it
should return max_vfs=0. Hope it is clear.

>

>> ---

>>  drivers/net/qede/qede_ethdev.c | 5 +----

>>  1 file changed, 1 insertion(+), 4 deletions(-)

>> 

>> diff --git a/drivers/net/qede/qede_ethdev.c

>>b/drivers/net/qede/qede_ethdev.c

>> index ee8fb43..10abb8b 100644

>> --- a/drivers/net/qede/qede_ethdev.c

>> +++ b/drivers/net/qede/qede_ethdev.c

>> @@ -976,10 +976,7 @@ static int qede_dev_configure(struct rte_eth_dev

>>*eth_dev)

>>  	dev_info->max_rx_queues = (uint16_t)QEDE_MAX_RSS_CNT(qdev);

>>  	dev_info->max_tx_queues = dev_info->max_rx_queues;

>>  	dev_info->max_mac_addrs = qdev->dev_info.num_mac_addrs;

>> -	if (IS_VF(edev))

>> -		dev_info->max_vfs = 0;

>> -	else

>> -		dev_info->max_vfs = (uint16_t)NUM_OF_VFS(&qdev->edev);

>> +	dev_info->max_vfs = 0;

>>  	dev_info->reta_size = ECORE_RSS_IND_TABLE_SIZE;

>>  	dev_info->hash_key_size = ECORE_RSS_KEY_SIZE * sizeof(uint32_t);

>>  	dev_info->flow_type_rss_offloads = (uint64_t)QEDE_RSS_OFFLOAD_ALL;

>> 

>

>
Ferruh Yigit Dec. 12, 2016, 6:13 p.m. UTC | #3 
On 12/12/2016 5:47 PM, Harish Patil wrote:
> 
>> On 12/3/2016 2:43 AM, Harish Patil wrote:
>>> Set max_vfs to 0 since it is relevant only to SR-IOV PF
>>> which is not supported yet.
>>>
>>> Fixes: 2ea6f76a ("qede: add core driver")
>>>
>>> Signed-off-by: Harish Patil <harish.patil@qlogic.com>
>>
>> Can you please update patch title to indicate what has been fixed
>> instead of what has been done in the patch.
> 
> Can you please clarify? The change in the patch is to set max_vfs=0.
> So that’s why the title - "fix maximum VF count to 0"

Let me try, if I can :)

I can see what patch does, and I got the reasoning behind.

Previously driver was reporting as it was supporting SR-IOV with DPDK
PF, right? But it was wrong and you are fixing it.

How you fix is by setting max_vfs=0, but that is the technical detail of
the fix.

What you are trying to fix is not setting a variable to a specific
value, what you are trying to fix is SR-IOV support.

I hope I can make it more clear.

> 
>>
>>
>> btw, while checking feature list, I have seen qede_vf supports SR-IOV,
>> is that correct?
> 
> Yes. The supported combination for SR-IOV is VF driver (qede PMD) with PF
> driver (qede linux driver).

So you are using SR-IOV feature set in VF driver, as meaning VF driver
support exists. I don't know what SR-IOV feature mean for VF drivers.
Some other VF drivers not has this feature flag set.

CC'ed Thomas for help, if this is the intention of the feature flag, it
is OK.

> We don’t have support for SR-IOV PF driver yet. When SR-IOV PF driver is
> supported then max_vfs shall return the actual max VFs, but for now it
> should return max_vfs=0. Hope it is clear.
> 
>>
>>> ---
>>>  drivers/net/qede/qede_ethdev.c | 5 +----
>>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/net/qede/qede_ethdev.c
>>> b/drivers/net/qede/qede_ethdev.c
>>> index ee8fb43..10abb8b 100644
>>> --- a/drivers/net/qede/qede_ethdev.c
>>> +++ b/drivers/net/qede/qede_ethdev.c
>>> @@ -976,10 +976,7 @@ static int qede_dev_configure(struct rte_eth_dev
>>> *eth_dev)
>>>  	dev_info->max_rx_queues = (uint16_t)QEDE_MAX_RSS_CNT(qdev);
>>>  	dev_info->max_tx_queues = dev_info->max_rx_queues;
>>>  	dev_info->max_mac_addrs = qdev->dev_info.num_mac_addrs;
>>> -	if (IS_VF(edev))
>>> -		dev_info->max_vfs = 0;
>>> -	else
>>> -		dev_info->max_vfs = (uint16_t)NUM_OF_VFS(&qdev->edev);
>>> +	dev_info->max_vfs = 0;
>>>  	dev_info->reta_size = ECORE_RSS_IND_TABLE_SIZE;
>>>  	dev_info->hash_key_size = ECORE_RSS_KEY_SIZE * sizeof(uint32_t);
>>>  	dev_info->flow_type_rss_offloads = (uint64_t)QEDE_RSS_OFFLOAD_ALL;
>>>
>>
>>
> 
>
Thomas Monjalon Dec. 19, 2016, 1:38 p.m. UTC | #4 
2016-12-12 18:13, Ferruh Yigit:
> On 12/12/2016 5:47 PM, Harish Patil wrote:
> >> btw, while checking feature list, I have seen qede_vf supports SR-IOV,
> >> is that correct?
> > 
> > Yes. The supported combination for SR-IOV is VF driver (qede PMD) with PF
> > driver (qede linux driver).
> 
> So you are using SR-IOV feature set in VF driver, as meaning VF driver
> support exists. I don't know what SR-IOV feature mean for VF drivers.
> Some other VF drivers not has this feature flag set.
> 
> CC'ed Thomas for help, if this is the intention of the feature flag, it
> is OK.

Good question.
I wonder where better describe the meaning of this row in the features table.
Maybe it would be clearer by splitting in 2 rows:
- SR-IOV VF
- SR-IOV PF
Harish Patil Dec. 20, 2016, 11:15 p.m. UTC | #5 
>

>2016-12-12 18:13, Ferruh Yigit:

>> On 12/12/2016 5:47 PM, Harish Patil wrote:

>> >> btw, while checking feature list, I have seen qede_vf supports

>>SR-IOV,

>> >> is that correct?

>> > 

>> > Yes. The supported combination for SR-IOV is VF driver (qede PMD)

>>with PF

>> > driver (qede linux driver).

>> 

>> So you are using SR-IOV feature set in VF driver, as meaning VF driver

>> support exists. I don't know what SR-IOV feature mean for VF drivers.

>> Some other VF drivers not has this feature flag set.

>> 

>> CC'ed Thomas for help, if this is the intention of the feature flag, it

>> is OK.

>

>Good question.

>I wonder where better describe the meaning of this row in the features

>table.

>Maybe it would be clearer by splitting in 2 rows:

>- SR-IOV VF

>- SR-IOV PF

>

Thanks, that would make it clearer.
Ferruh, for now I shall reword the patch title.
diff mbox

Patch

diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index ee8fb43..10abb8b 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -976,10 +976,7 @@  static int qede_dev_configure(struct rte_eth_dev *eth_dev)
 	dev_info->max_rx_queues = (uint16_t)QEDE_MAX_RSS_CNT(qdev);
 	dev_info->max_tx_queues = dev_info->max_rx_queues;
 	dev_info->max_mac_addrs = qdev->dev_info.num_mac_addrs;
-	if (IS_VF(edev))
-		dev_info->max_vfs = 0;
-	else
-		dev_info->max_vfs = (uint16_t)NUM_OF_VFS(&qdev->edev);
+	dev_info->max_vfs = 0;
 	dev_info->reta_size = ECORE_RSS_IND_TABLE_SIZE;
 	dev_info->hash_key_size = ECORE_RSS_KEY_SIZE * sizeof(uint32_t);
 	dev_info->flow_type_rss_offloads = (uint64_t)QEDE_RSS_OFFLOAD_ALL;