diff mbox

[net-next,v9,5/7] ixgbe: Add GET_RSS_KEY command to VF-PF channel commands set

Message ID 1427645497-8339-6-git-send-email-vladz@cloudius-systems.com
State Awaiting Upstream, archived
Delegated to: David Miller
Headers show

Commit Message

Vlad Zolotarov March 29, 2015, 4:11 p.m. UTC
For 82599 and x540 VFs and PF share the same RSS Key. Therefore we will return
the same RSS key for all VFs.

Support for other devices will be added later.

Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
---
New in v9:
   - Reduce the support to 82599 and x540 devices only.
   - Get rid of registers access in GET_VF_RSS_KEY flow:
      - Get the RSS HASH Key value from the PF's adapter->rss_key[].

New in v5:
   - Use a newly added netdev op to allow/prevent the RSS Hash Key querying on a per-VF
     basis.

New in v3:
   - Added a support for x550 devices.

New in v1 (compared to RFC):
   - Use "if-else" statement instead of a "switch-case" for a single option case
     (in ixgbe_get_vf_rss_key()).
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h   |  1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+)

Comments

Alexander Duyck March 29, 2015, 10:04 p.m. UTC | #1
On 03/29/2015 09:11 AM, Vlad Zolotarov wrote:
> For 82599 and x540 VFs and PF share the same RSS Key. Therefore we will return
> the same RSS key for all VFs.
> 
> Support for other devices will be added later.
> 
> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
> ---
> New in v9:
>    - Reduce the support to 82599 and x540 devices only.
>    - Get rid of registers access in GET_VF_RSS_KEY flow:
>       - Get the RSS HASH Key value from the PF's adapter->rss_key[].
> 
> New in v5:
>    - Use a newly added netdev op to allow/prevent the RSS Hash Key querying on a per-VF
>      basis.
> 
> New in v3:
>    - Added a support for x550 devices.
> 
> New in v1 (compared to RFC):
>    - Use "if-else" statement instead of a "switch-case" for a single option case
>      (in ixgbe_get_vf_rss_key()).
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h   |  1 +
>  drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 25 +++++++++++++++++++++++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
> index 3522f53..b1e4703 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
> @@ -100,6 +100,7 @@ enum ixgbe_pfvf_api_rev {
>  
>  /* mailbox API, version 1.2 VF requests */
>  #define IXGBE_VF_GET_RETA	0x0a	/* VF request for RETA */
> +#define IXGBE_VF_GET_RSS_KEY	0x0b	/* get RSS key */
>  
>  /* length of permanent address message returned from PF */
>  #define IXGBE_VF_PERMADDR_MSG_LEN 4
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> index 8424e7f..4d87458 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> @@ -996,6 +996,28 @@ static int ixgbe_get_vf_reta(struct ixgbe_adapter *adapter, u32 *msgbuf, u32 vf)
>  	return 0;
>  }
>  
> +static int ixgbe_get_vf_rss_key(struct ixgbe_adapter *adapter,
> +				u32 *msgbuf, u32 vf)
> +{
> +	struct ixgbe_hw *hw = &adapter->hw;
> +	u32 *rss_key = &msgbuf[1];
> +
> +	/* verify the PF is supporting the correct API */
> +	if (!adapter->vfinfo[vf].rss_query_enabled ||
> +	    (adapter->vfinfo[vf].vf_api != ixgbe_mbox_api_12))
> +		return -EPERM;
> +
> +	/* Read the RSS KEY.
> +	 * We only support 82599 and x540 devices at the moment.
> +	 */
> +	if (hw->mac.type >= ixgbe_mac_X550)
> +		return -1;
> +

The return should be not supported, not -1 since that is too ambiguous.

> +	memcpy(rss_key, adapter->rss_key, sizeof(adapter->rss_key));
> +
> +	return 0;
> +}
> +
>  static int ixgbe_rcv_msg_from_vf(struct ixgbe_adapter *adapter, u32 vf)
>  {
>  	u32 mbx_size = IXGBE_VFMAILBOX_SIZE;
> @@ -1055,6 +1077,9 @@ static int ixgbe_rcv_msg_from_vf(struct ixgbe_adapter *adapter, u32 vf)
>  	case IXGBE_VF_GET_RETA:
>  		retval = ixgbe_get_vf_reta(adapter, msgbuf, vf);
>  		break;
> +	case IXGBE_VF_GET_RSS_KEY:
> +		retval = ixgbe_get_vf_rss_key(adapter, msgbuf, vf);
> +		break;
>  	default:
>  		e_err(drv, "Unhandled Msg %8.8x\n", msgbuf[0]);
>  		retval = IXGBE_ERR_MBX;
> 

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vlad Zolotarov March 30, 2015, 9:57 a.m. UTC | #2
On 03/30/15 01:04, Alexander Duyck wrote:
> On 03/29/2015 09:11 AM, Vlad Zolotarov wrote:
>> For 82599 and x540 VFs and PF share the same RSS Key. Therefore we will return
>> the same RSS key for all VFs.
>>
>> Support for other devices will be added later.
>>
>> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
>> ---
>> New in v9:
>>     - Reduce the support to 82599 and x540 devices only.
>>     - Get rid of registers access in GET_VF_RSS_KEY flow:
>>        - Get the RSS HASH Key value from the PF's adapter->rss_key[].
>>
>> New in v5:
>>     - Use a newly added netdev op to allow/prevent the RSS Hash Key querying on a per-VF
>>       basis.
>>
>> New in v3:
>>     - Added a support for x550 devices.
>>
>> New in v1 (compared to RFC):
>>     - Use "if-else" statement instead of a "switch-case" for a single option case
>>       (in ixgbe_get_vf_rss_key()).
>> ---
>>   drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h   |  1 +
>>   drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 25 +++++++++++++++++++++++++
>>   2 files changed, 26 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
>> index 3522f53..b1e4703 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
>> @@ -100,6 +100,7 @@ enum ixgbe_pfvf_api_rev {
>>   
>>   /* mailbox API, version 1.2 VF requests */
>>   #define IXGBE_VF_GET_RETA	0x0a	/* VF request for RETA */
>> +#define IXGBE_VF_GET_RSS_KEY	0x0b	/* get RSS key */
>>   
>>   /* length of permanent address message returned from PF */
>>   #define IXGBE_VF_PERMADDR_MSG_LEN 4
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>> index 8424e7f..4d87458 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>> @@ -996,6 +996,28 @@ static int ixgbe_get_vf_reta(struct ixgbe_adapter *adapter, u32 *msgbuf, u32 vf)
>>   	return 0;
>>   }
>>   
>> +static int ixgbe_get_vf_rss_key(struct ixgbe_adapter *adapter,
>> +				u32 *msgbuf, u32 vf)
>> +{
>> +	struct ixgbe_hw *hw = &adapter->hw;
>> +	u32 *rss_key = &msgbuf[1];
>> +
>> +	/* verify the PF is supporting the correct API */
>> +	if (!adapter->vfinfo[vf].rss_query_enabled ||
>> +	    (adapter->vfinfo[vf].vf_api != ixgbe_mbox_api_12))
>> +		return -EPERM;
>> +
>> +	/* Read the RSS KEY.
>> +	 * We only support 82599 and x540 devices at the moment.
>> +	 */
>> +	if (hw->mac.type >= ixgbe_mac_X550)
>> +		return -1;
>> +
> The return should be not supported, not -1 since that is too ambiguous.
Right and I think the (adapter->vfinfo[vf].vf_api != ixgbe_mbox_api_12) 
should also return -EOPNOTSUPP.
>
>> +	memcpy(rss_key, adapter->rss_key, sizeof(adapter->rss_key));
>> +
>> +	return 0;
>> +}
>> +
>>   static int ixgbe_rcv_msg_from_vf(struct ixgbe_adapter *adapter, u32 vf)
>>   {
>>   	u32 mbx_size = IXGBE_VFMAILBOX_SIZE;
>> @@ -1055,6 +1077,9 @@ static int ixgbe_rcv_msg_from_vf(struct ixgbe_adapter *adapter, u32 vf)
>>   	case IXGBE_VF_GET_RETA:
>>   		retval = ixgbe_get_vf_reta(adapter, msgbuf, vf);
>>   		break;
>> +	case IXGBE_VF_GET_RSS_KEY:
>> +		retval = ixgbe_get_vf_rss_key(adapter, msgbuf, vf);
>> +		break;
>>   	default:
>>   		e_err(drv, "Unhandled Msg %8.8x\n", msgbuf[0]);
>>   		retval = IXGBE_ERR_MBX;
>>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vlad Zolotarov March 30, 2015, 10:08 a.m. UTC | #3
On 03/30/15 01:04, Alexander Duyck wrote:
> On 03/29/2015 09:11 AM, Vlad Zolotarov wrote:
>> For 82599 and x540 VFs and PF share the same RSS Key. Therefore we will return
>> the same RSS key for all VFs.
>>
>> Support for other devices will be added later.
>>
>> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
>> ---
>> New in v9:
>>     - Reduce the support to 82599 and x540 devices only.
>>     - Get rid of registers access in GET_VF_RSS_KEY flow:
>>        - Get the RSS HASH Key value from the PF's adapter->rss_key[].
>>
>> New in v5:
>>     - Use a newly added netdev op to allow/prevent the RSS Hash Key querying on a per-VF
>>       basis.
>>
>> New in v3:
>>     - Added a support for x550 devices.
>>
>> New in v1 (compared to RFC):
>>     - Use "if-else" statement instead of a "switch-case" for a single option case
>>       (in ixgbe_get_vf_rss_key()).
>> ---
>>   drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h   |  1 +
>>   drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 25 +++++++++++++++++++++++++
>>   2 files changed, 26 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
>> index 3522f53..b1e4703 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
>> @@ -100,6 +100,7 @@ enum ixgbe_pfvf_api_rev {
>>   
>>   /* mailbox API, version 1.2 VF requests */
>>   #define IXGBE_VF_GET_RETA	0x0a	/* VF request for RETA */
>> +#define IXGBE_VF_GET_RSS_KEY	0x0b	/* get RSS key */
>>   
>>   /* length of permanent address message returned from PF */
>>   #define IXGBE_VF_PERMADDR_MSG_LEN 4
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>> index 8424e7f..4d87458 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>> @@ -996,6 +996,28 @@ static int ixgbe_get_vf_reta(struct ixgbe_adapter *adapter, u32 *msgbuf, u32 vf)
>>   	return 0;
>>   }
>>   
>> +static int ixgbe_get_vf_rss_key(struct ixgbe_adapter *adapter,
>> +				u32 *msgbuf, u32 vf)
>> +{
>> +	struct ixgbe_hw *hw = &adapter->hw;
>> +	u32 *rss_key = &msgbuf[1];
>> +
>> +	/* verify the PF is supporting the correct API */
>> +	if (!adapter->vfinfo[vf].rss_query_enabled ||
>> +	    (adapter->vfinfo[vf].vf_api != ixgbe_mbox_api_12))
>> +		return -EPERM;
>> +
>> +	/* Read the RSS KEY.
>> +	 * We only support 82599 and x540 devices at the moment.
>> +	 */
>> +	if (hw->mac.type >= ixgbe_mac_X550)
>> +		return -1;
>> +
> The return should be not supported, not -1 since that is too ambiguous.

On the other hand - why this condition here at all? It will never return 
TRUE anyway since the operation will be blocked by 
adapter->vfinfo[vf].rss_query_enabled
check before since ixgbe_ndo_set_vf_rss_query_en() will prevent the 
rss_query_enabling for the devices newer than x540. So IMHO the right 
thing to do here should be to remove the last "if" at all and split the 
previous one in two so that we will return -EOPNOTSUPP for 
(adapter->vfinfo[vf].vf_api != ixgbe_mbox_api_12) condition.

>
>> +	memcpy(rss_key, adapter->rss_key, sizeof(adapter->rss_key));
>> +
>> +	return 0;
>> +}
>> +
>>   static int ixgbe_rcv_msg_from_vf(struct ixgbe_adapter *adapter, u32 vf)
>>   {
>>   	u32 mbx_size = IXGBE_VFMAILBOX_SIZE;
>> @@ -1055,6 +1077,9 @@ static int ixgbe_rcv_msg_from_vf(struct ixgbe_adapter *adapter, u32 vf)
>>   	case IXGBE_VF_GET_RETA:
>>   		retval = ixgbe_get_vf_reta(adapter, msgbuf, vf);
>>   		break;
>> +	case IXGBE_VF_GET_RSS_KEY:
>> +		retval = ixgbe_get_vf_rss_key(adapter, msgbuf, vf);
>> +		break;
>>   	default:
>>   		e_err(drv, "Unhandled Msg %8.8x\n", msgbuf[0]);
>>   		retval = IXGBE_ERR_MBX;
>>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vlad Zolotarov March 30, 2015, 10:41 a.m. UTC | #4
On 03/30/15 01:04, Alexander Duyck wrote:
> On 03/29/2015 09:11 AM, Vlad Zolotarov wrote:
>> For 82599 and x540 VFs and PF share the same RSS Key. Therefore we will return
>> the same RSS key for all VFs.
>>
>> Support for other devices will be added later.
>>
>> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
>> ---
>> New in v9:
>>     - Reduce the support to 82599 and x540 devices only.
>>     - Get rid of registers access in GET_VF_RSS_KEY flow:
>>        - Get the RSS HASH Key value from the PF's adapter->rss_key[].
>>
>> New in v5:
>>     - Use a newly added netdev op to allow/prevent the RSS Hash Key querying on a per-VF
>>       basis.
>>
>> New in v3:
>>     - Added a support for x550 devices.
>>
>> New in v1 (compared to RFC):
>>     - Use "if-else" statement instead of a "switch-case" for a single option case
>>       (in ixgbe_get_vf_rss_key()).
>> ---
>>   drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h   |  1 +
>>   drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 25 +++++++++++++++++++++++++
>>   2 files changed, 26 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
>> index 3522f53..b1e4703 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
>> @@ -100,6 +100,7 @@ enum ixgbe_pfvf_api_rev {
>>   
>>   /* mailbox API, version 1.2 VF requests */
>>   #define IXGBE_VF_GET_RETA	0x0a	/* VF request for RETA */
>> +#define IXGBE_VF_GET_RSS_KEY	0x0b	/* get RSS key */
>>   
>>   /* length of permanent address message returned from PF */
>>   #define IXGBE_VF_PERMADDR_MSG_LEN 4
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>> index 8424e7f..4d87458 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>> @@ -996,6 +996,28 @@ static int ixgbe_get_vf_reta(struct ixgbe_adapter *adapter, u32 *msgbuf, u32 vf)
>>   	return 0;
>>   }
>>   
>> +static int ixgbe_get_vf_rss_key(struct ixgbe_adapter *adapter,
>> +				u32 *msgbuf, u32 vf)
>> +{
>> +	struct ixgbe_hw *hw = &adapter->hw;
>> +	u32 *rss_key = &msgbuf[1];
>> +
>> +	/* verify the PF is supporting the correct API */
>> +	if (!adapter->vfinfo[vf].rss_query_enabled ||
>> +	    (adapter->vfinfo[vf].vf_api != ixgbe_mbox_api_12))
>> +		return -EPERM;
>> +
>> +	/* Read the RSS KEY.
>> +	 * We only support 82599 and x540 devices at the moment.
>> +	 */
>> +	if (hw->mac.type >= ixgbe_mac_X550)
>> +		return -1;
>> +
> The return should be not supported, not -1 since that is too ambiguous.

By the way, this (-1) didn't come from nowhere - I looked at other 
functions in this file (e.g. ixgbe_get_vf_queues() and 
ixgbe_negotiate_vf_api()) and they are returning
this "ambiguous" (-1) just fine in the situations where -EOPNOTSUPP and 
-EINVAL (correspondingly) would be much more appropriate.
So, I went and checked who was the guy who added this terrible 
"ambiguity" and, surprise-surprise!!!, it was mister Alexander Duyck! :D 
In both (!!!) cases! Bang-bang!!! :D Of course it was "two years ago" 
Alex but still... ;)

Anyway, since we fight the ambiguity now I think u, guys, would like to 
send a cleanup follow-up patches that would adjust your "ambiguous code" 
with the new "super-clear styling" we are introducing in this patches... ;)

>
>> +	memcpy(rss_key, adapter->rss_key, sizeof(adapter->rss_key));
>> +
>> +	return 0;
>> +}
>> +
>>   static int ixgbe_rcv_msg_from_vf(struct ixgbe_adapter *adapter, u32 vf)
>>   {
>>   	u32 mbx_size = IXGBE_VFMAILBOX_SIZE;
>> @@ -1055,6 +1077,9 @@ static int ixgbe_rcv_msg_from_vf(struct ixgbe_adapter *adapter, u32 vf)
>>   	case IXGBE_VF_GET_RETA:
>>   		retval = ixgbe_get_vf_reta(adapter, msgbuf, vf);
>>   		break;
>> +	case IXGBE_VF_GET_RSS_KEY:
>> +		retval = ixgbe_get_vf_rss_key(adapter, msgbuf, vf);
>> +		break;
>>   	default:
>>   		e_err(drv, "Unhandled Msg %8.8x\n", msgbuf[0]);
>>   		retval = IXGBE_ERR_MBX;
>>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Duyck March 30, 2015, 3:04 p.m. UTC | #5
On 03/30/2015 03:41 AM, Vlad Zolotarov wrote:
>
>
> On 03/30/15 01:04, Alexander Duyck wrote:
>> On 03/29/2015 09:11 AM, Vlad Zolotarov wrote:
>>> For 82599 and x540 VFs and PF share the same RSS Key. Therefore we 
>>> will return
>>> the same RSS key for all VFs.
>>>
>>> Support for other devices will be added later.
>>>
>>> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
>>> ---
>>> New in v9:
>>>     - Reduce the support to 82599 and x540 devices only.
>>>     - Get rid of registers access in GET_VF_RSS_KEY flow:
>>>        - Get the RSS HASH Key value from the PF's adapter->rss_key[].
>>>
>>> New in v5:
>>>     - Use a newly added netdev op to allow/prevent the RSS Hash Key 
>>> querying on a per-VF
>>>       basis.
>>>
>>> New in v3:
>>>     - Added a support for x550 devices.
>>>
>>> New in v1 (compared to RFC):
>>>     - Use "if-else" statement instead of a "switch-case" for a 
>>> single option case
>>>       (in ixgbe_get_vf_rss_key()).
>>> ---
>>>   drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h   |  1 +
>>>   drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 25 
>>> +++++++++++++++++++++++++
>>>   2 files changed, 26 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h 
>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
>>> index 3522f53..b1e4703 100644
>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
>>> @@ -100,6 +100,7 @@ enum ixgbe_pfvf_api_rev {
>>>     /* mailbox API, version 1.2 VF requests */
>>>   #define IXGBE_VF_GET_RETA    0x0a    /* VF request for RETA */
>>> +#define IXGBE_VF_GET_RSS_KEY    0x0b    /* get RSS key */
>>>     /* length of permanent address message returned from PF */
>>>   #define IXGBE_VF_PERMADDR_MSG_LEN 4
>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c 
>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>>> index 8424e7f..4d87458 100644
>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>>> @@ -996,6 +996,28 @@ static int ixgbe_get_vf_reta(struct 
>>> ixgbe_adapter *adapter, u32 *msgbuf, u32 vf)
>>>       return 0;
>>>   }
>>>   +static int ixgbe_get_vf_rss_key(struct ixgbe_adapter *adapter,
>>> +                u32 *msgbuf, u32 vf)
>>> +{
>>> +    struct ixgbe_hw *hw = &adapter->hw;
>>> +    u32 *rss_key = &msgbuf[1];
>>> +
>>> +    /* verify the PF is supporting the correct API */
>>> +    if (!adapter->vfinfo[vf].rss_query_enabled ||
>>> +        (adapter->vfinfo[vf].vf_api != ixgbe_mbox_api_12))
>>> +        return -EPERM;
>>> +
>>> +    /* Read the RSS KEY.
>>> +     * We only support 82599 and x540 devices at the moment.
>>> +     */
>>> +    if (hw->mac.type >= ixgbe_mac_X550)
>>> +        return -1;
>>> +
>> The return should be not supported, not -1 since that is too ambiguous.
>
> By the way, this (-1) didn't come from nowhere - I looked at other 
> functions in this file (e.g. ixgbe_get_vf_queues() and 
> ixgbe_negotiate_vf_api()) and they are returning
> this "ambiguous" (-1) just fine in the situations where -EOPNOTSUPP 
> and -EINVAL (correspondingly) would be much more appropriate.
> So, I went and checked who was the guy who added this terrible 
> "ambiguity" and, surprise-surprise!!!, it was mister Alexander Duyck! 
> :D In both (!!!) cases! Bang-bang!!! :D Of course it was "two years 
> ago" Alex but still... ;)

Yeah, it was a bad habit I used to have.  That is why I am able to spot 
this kind of thing so easily now.

> Anyway, since we fight the ambiguity now I think u, guys, would like 
> to send a cleanup follow-up patches that would adjust your "ambiguous 
> code" with the new "super-clear styling" we are introducing in this 
> patches... ;)

Well, I'm not part of Intel anymore and have more then enough on my 
plate at the moment.  I'm sure when someone over there gets a few spare 
cycles, or can get some kernel janitors on it they will clean up the 
error return values.

If nothing else it is useful to have the error returns make sense as 
more often then not they make their way all the way back to the use in 
the form of the error message so it is best to return something like a 
not supported, than not permitted.

- Alex
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
index 3522f53..b1e4703 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
@@ -100,6 +100,7 @@  enum ixgbe_pfvf_api_rev {
 
 /* mailbox API, version 1.2 VF requests */
 #define IXGBE_VF_GET_RETA	0x0a	/* VF request for RETA */
+#define IXGBE_VF_GET_RSS_KEY	0x0b	/* get RSS key */
 
 /* length of permanent address message returned from PF */
 #define IXGBE_VF_PERMADDR_MSG_LEN 4
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index 8424e7f..4d87458 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -996,6 +996,28 @@  static int ixgbe_get_vf_reta(struct ixgbe_adapter *adapter, u32 *msgbuf, u32 vf)
 	return 0;
 }
 
+static int ixgbe_get_vf_rss_key(struct ixgbe_adapter *adapter,
+				u32 *msgbuf, u32 vf)
+{
+	struct ixgbe_hw *hw = &adapter->hw;
+	u32 *rss_key = &msgbuf[1];
+
+	/* verify the PF is supporting the correct API */
+	if (!adapter->vfinfo[vf].rss_query_enabled ||
+	    (adapter->vfinfo[vf].vf_api != ixgbe_mbox_api_12))
+		return -EPERM;
+
+	/* Read the RSS KEY.
+	 * We only support 82599 and x540 devices at the moment.
+	 */
+	if (hw->mac.type >= ixgbe_mac_X550)
+		return -1;
+
+	memcpy(rss_key, adapter->rss_key, sizeof(adapter->rss_key));
+
+	return 0;
+}
+
 static int ixgbe_rcv_msg_from_vf(struct ixgbe_adapter *adapter, u32 vf)
 {
 	u32 mbx_size = IXGBE_VFMAILBOX_SIZE;
@@ -1055,6 +1077,9 @@  static int ixgbe_rcv_msg_from_vf(struct ixgbe_adapter *adapter, u32 vf)
 	case IXGBE_VF_GET_RETA:
 		retval = ixgbe_get_vf_reta(adapter, msgbuf, vf);
 		break;
+	case IXGBE_VF_GET_RSS_KEY:
+		retval = ixgbe_get_vf_rss_key(adapter, msgbuf, vf);
+		break;
 	default:
 		e_err(drv, "Unhandled Msg %8.8x\n", msgbuf[0]);
 		retval = IXGBE_ERR_MBX;