[dpdk-dev,6/7] net/qede: fix maximum VF count to 0
Checks
Commit Message
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
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;
>
>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;
>>
>
>
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;
>>>
>>
>>
>
>
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
>
>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.
@@ -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;