[PATCH net-next v1 0/5]: ixgbevf: Allow querying VFs RSS indirection table and key

[PATCH net-next v1 0/5]: ixgbevf: Allow querying VFs RSS indirection table and key

[Vlad Zolotarov]

Add the ethtool ops to VF driver to allow querying the RSS indirection table
and RSS Random Key.
 
 - PF driver: Add new VF-PF channel commands.
 - VF driver: Utilize these new commands and add the corresponding
              ethtool callbacks.

New in v1 (compared to RFC):
   - Use "if-else" statement instead of a "switch-case" for a single option case.
     More specifically: in cases where the newly added API version is the only one
     allowed. We may consider using a "switch-case" back again when the list of
     allowed API versions in these specific places grows up.

Vlad Zolotarov (5):
  ixgbe: Add a RETA query command to VF-PF channel API
  ixgbevf: Add a RETA query code
  ixgbe: Add GET_RSS_KEY command to VF-PF channel commands set
  ixgbevf: Add RSS Key query code
  ixgbevf: Add the appropriate ethtool ops to query RSS indirection
    table and key

 drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h      |   8 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c    |  52 ++++++++++
 drivers/net/ethernet/intel/ixgbevf/ethtool.c      |  37 +++++++
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |   4 +-
 drivers/net/ethernet/intel/ixgbevf/mbx.h          |   8 ++
 drivers/net/ethernet/intel/ixgbevf/vf.c           | 117 ++++++++++++++++++++++
 drivers/net/ethernet/intel/ixgbevf/vf.h           |   2 +
 7 files changed, 227 insertions(+), 1 deletion(-)

-- 
2.1.0

[PATCH net-next v1 1/5] ixgbe: Add a RETA query command to VF-PF channel API

[Vlad Zolotarov]

82599 VFs and PF share the same RSS redirection table (RETA). Therefore we
just return it for all VFs.

RETA table is an array of 32 registers and the maximum number of registers
that may be delivered in a single VF-PF channel command is 15. Therefore
we will deliver the whole table in 3 steps: 12, 12 and 8 registers in each
step correspondingly. Thus this patch does the following:

  - Adds a new API version (to specify a new commands set).
  - Adds the IXGBE_VF_GET_RETA_[0,1,2] commands to the VF-PF commands set.

Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
---
New in v1 (compared to RFC):
   - Use "if-else" statement instead of a "switch-case" for a single option case
     (in ixgbe_get_vf_reta()).
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h   |  6 +++++
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 31 ++++++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
index a5cb755..c1123d9 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
@@ -73,6 +73,7 @@ enum ixgbe_pfvf_api_rev {
 	ixgbe_mbox_api_10,	/* API version 1.0, linux/freebsd VF driver */
 	ixgbe_mbox_api_20,	/* API version 2.0, solaris Phase1 VF driver */
 	ixgbe_mbox_api_11,	/* API version 1.1, linux/freebsd VF driver */
+	ixgbe_mbox_api_12,	/* API version 1.2, linux/freebsd VF driver */
 	/* This value should always be last */
 	ixgbe_mbox_api_unknown,	/* indicates that API version is not known */
 };
@@ -91,6 +92,11 @@ enum ixgbe_pfvf_api_rev {
 /* mailbox API, version 1.1 VF requests */
 #define IXGBE_VF_GET_QUEUES	0x09 /* get queue configuration */
 
+/* mailbox API, version 1.2 VF requests */
+#define IXGBE_VF_GET_RETA_0	0x0a /* get RETA[0..11]  */
+#define IXGBE_VF_GET_RETA_1	0x0b /* get RETA[12..23] */
+#define IXGBE_VF_GET_RETA_2	0x0c /* get RETA[24..31] */
+
 /* GET_QUEUES return data indices within the mailbox */
 #define IXGBE_VF_TX_QUEUES	1	/* number of Tx queues supported */
 #define IXGBE_VF_RX_QUEUES	2	/* number of Rx queues supported */
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index c76ba90..b1625c8 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -427,6 +427,7 @@ static s32 ixgbe_set_vf_lpe(struct ixgbe_adapter *adapter, u32 *msgbuf, u32 vf)
 #endif /* CONFIG_FCOE */
 		switch (adapter->vfinfo[vf].vf_api) {
 		case ixgbe_mbox_api_11:
+		case ixgbe_mbox_api_12:
 			/*
 			 * Version 1.1 supports jumbo frames on VFs if PF has
 			 * jumbo frames enabled which means legacy VFs are
@@ -894,6 +895,7 @@ static int ixgbe_negotiate_vf_api(struct ixgbe_adapter *adapter,
 	switch (api) {
 	case ixgbe_mbox_api_10:
 	case ixgbe_mbox_api_11:
+	case ixgbe_mbox_api_12:
 		adapter->vfinfo[vf].vf_api = api;
 		return 0;
 	default:
@@ -917,6 +919,7 @@ static int ixgbe_get_vf_queues(struct ixgbe_adapter *adapter,
 	switch (adapter->vfinfo[vf].vf_api) {
 	case ixgbe_mbox_api_20:
 	case ixgbe_mbox_api_11:
+	case ixgbe_mbox_api_12:
 		break;
 	default:
 		return -1;
@@ -944,6 +947,25 @@ static int ixgbe_get_vf_queues(struct ixgbe_adapter *adapter,
 	return 0;
 }
 
+static int ixgbe_get_vf_reta(struct ixgbe_adapter *adapter,
+			     u32 *msgbuf, u32 vf, int reta_offset_dw,
+			     size_t dwords)
+{
+	struct ixgbe_hw *hw = &adapter->hw;
+	int i;
+	u32 *reta = &msgbuf[1];
+
+	/* verify the PF is supporting the correct API */
+	if (adapter->vfinfo[vf].vf_api != ixgbe_mbox_api_12)
+		return -EPERM;
+
+	/* Read the appropriate portion of RETA */
+	for (i = 0; i < dwords; i++)
+		reta[i] = IXGBE_READ_REG(hw, IXGBE_RETA(i + reta_offset_dw));
+
+	return 0;
+}
+
 static int ixgbe_rcv_msg_from_vf(struct ixgbe_adapter *adapter, u32 vf)
 {
 	u32 mbx_size = IXGBE_VFMAILBOX_SIZE;
@@ -1000,6 +1022,15 @@ static int ixgbe_rcv_msg_from_vf(struct ixgbe_adapter *adapter, u32 vf)
 	case IXGBE_VF_GET_QUEUES:
 		retval = ixgbe_get_vf_queues(adapter, msgbuf, vf);
 		break;
+	case IXGBE_VF_GET_RETA_0:
+		retval = ixgbe_get_vf_reta(adapter, msgbuf, vf, 0, 12);
+		break;
+	case IXGBE_VF_GET_RETA_1:
+		retval = ixgbe_get_vf_reta(adapter, msgbuf, vf, 12, 12);
+		break;
+	case IXGBE_VF_GET_RETA_2:
+		retval = ixgbe_get_vf_reta(adapter, msgbuf, vf, 24, 8);
+		break;
 	default:
 		e_err(drv, "Unhandled Msg %8.8x\n", msgbuf[0]);
 		retval = IXGBE_ERR_MBX;
-- 
2.1.0

[PATCH net-next v1 2/5] ixgbevf: Add a RETA query code

[Vlad Zolotarov]

   - Added a new API version support.
   - Added the query implementation in the ixgbevf.

Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
---
New in v1 (compared to RFC):
   - Use "if-else" statement instead of a "switch-case" for a single option case
     (in ixgbevf_get_reta()).
---
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |  4 +-
 drivers/net/ethernet/intel/ixgbevf/mbx.h          |  6 ++
 drivers/net/ethernet/intel/ixgbevf/vf.c           | 73 +++++++++++++++++++++++
 drivers/net/ethernet/intel/ixgbevf/vf.h           |  1 +
 4 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 62a0d8e..ba6ab61 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -1880,7 +1880,8 @@ static void ixgbevf_init_last_counter_stats(struct ixgbevf_adapter *adapter)
 static void ixgbevf_negotiate_api(struct ixgbevf_adapter *adapter)
 {
 	struct ixgbe_hw *hw = &adapter->hw;
-	int api[] = { ixgbe_mbox_api_11,
+	int api[] = { ixgbe_mbox_api_12,
+		      ixgbe_mbox_api_11,
 		      ixgbe_mbox_api_10,
 		      ixgbe_mbox_api_unknown };
 	int err = 0, idx = 0;
@@ -3525,6 +3526,7 @@ static int ixgbevf_change_mtu(struct net_device *netdev, int new_mtu)
 
 	switch (adapter->hw.api_version) {
 	case ixgbe_mbox_api_11:
+	case ixgbe_mbox_api_12:
 		max_possible_frame = IXGBE_MAX_JUMBO_FRAME_SIZE;
 		break;
 	default:
diff --git a/drivers/net/ethernet/intel/ixgbevf/mbx.h b/drivers/net/ethernet/intel/ixgbevf/mbx.h
index 0bc3005..3e148a8 100644
--- a/drivers/net/ethernet/intel/ixgbevf/mbx.h
+++ b/drivers/net/ethernet/intel/ixgbevf/mbx.h
@@ -86,6 +86,7 @@ enum ixgbe_pfvf_api_rev {
 	ixgbe_mbox_api_10,	/* API version 1.0, linux/freebsd VF driver */
 	ixgbe_mbox_api_20,	/* API version 2.0, solaris Phase1 VF driver */
 	ixgbe_mbox_api_11,	/* API version 1.1, linux/freebsd VF driver */
+	ixgbe_mbox_api_12,	/* API version 1.2, linux/freebsd VF driver */
 	/* This value should always be last */
 	ixgbe_mbox_api_unknown,	/* indicates that API version is not known */
 };
@@ -104,6 +105,11 @@ enum ixgbe_pfvf_api_rev {
 /* mailbox API, version 1.1 VF requests */
 #define IXGBE_VF_GET_QUEUE	0x09 /* get queue configuration */
 
+/* mailbox API, version 1.2 VF requests */
+#define IXGBE_VF_GET_RETA_0	0x0a /* get RETA[0..11]  */
+#define IXGBE_VF_GET_RETA_1	0x0b /* get RETA[12..23] */
+#define IXGBE_VF_GET_RETA_2	0x0c /* get RETA[24..31] */
+
 /* GET_QUEUES return data indices within the mailbox */
 #define IXGBE_VF_TX_QUEUES	1	/* number of Tx queues supported */
 #define IXGBE_VF_RX_QUEUES	2	/* number of Rx queues supported */
diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c b/drivers/net/ethernet/intel/ixgbevf/vf.c
index cdb53be..8b98cdf 100644
--- a/drivers/net/ethernet/intel/ixgbevf/vf.c
+++ b/drivers/net/ethernet/intel/ixgbevf/vf.c
@@ -258,6 +258,78 @@ static s32 ixgbevf_set_uc_addr_vf(struct ixgbe_hw *hw, u32 index, u8 *addr)
 	return ret_val;
 }
 
+static inline int _ixgbevf_get_reta(struct ixgbe_hw *hw, u32 *msgbuf,
+				    u32 *reta, u32 op, int reta_offset_dw,
+				    size_t dwords)
+{
+	int err;
+
+	msgbuf[0] = op;
+	err = hw->mbx.ops.write_posted(hw, msgbuf, 1);
+
+	if (err)
+		return err;
+
+	err = hw->mbx.ops.read_posted(hw, msgbuf, 1 + dwords);
+
+	if (err)
+		return err;
+
+	msgbuf[0] &= ~IXGBE_VT_MSGTYPE_CTS;
+
+	/* If we didn't get an ACK there must have been
+	 * some sort of mailbox error so we should treat it
+	 * as such.
+	 */
+	if (msgbuf[0] != (op | IXGBE_VT_MSGTYPE_ACK))
+		return IXGBE_ERR_MBX;
+
+	memcpy(reta + reta_offset_dw, msgbuf + 1, 4 * dwords);
+
+	return 0;
+}
+
+/**
+ * ixgbevf_get_reta - get the RSS redirection table (RETA) contents.
+ * @hw: pointer to the HW structure
+ * @reta: buffer to fill with RETA contents.
+ *
+ * The "reta" buffer should be big enough to contain 32 registers.
+ *
+ * Returns: 0 on success.
+ *          if API doesn't support this operation - (-EPERM).
+ */
+int ixgbevf_get_reta(struct ixgbe_hw *hw, u32 *reta)
+{
+	int err;
+	u32 msgbuf[IXGBE_VFMAILBOX_SIZE];
+
+	/* Return an error if API doesn't RETA querying. */
+	if (hw->api_version != ixgbe_mbox_api_12)
+		return -EPERM;
+
+	/* Fetch RETA from the PF. We do it in 3 steps due to mailbox size
+	 * limitation - we can bring up to 15 dwords every time while RETA
+	 * consists of 32 dwords. Therefore we'll bring 12, 12 and 8 dwords in
+	 * each step correspondingly.
+	 */
+
+	/* RETA[0..11] */
+	err = _ixgbevf_get_reta(hw, msgbuf, reta, IXGBE_VF_GET_RETA_0, 0, 12);
+	if (err)
+		return err;
+
+	/* RETA[12..23] */
+	err = _ixgbevf_get_reta(hw, msgbuf, reta, IXGBE_VF_GET_RETA_1, 12, 12);
+	if (err)
+		return err;
+
+	/* RETA[24..31] */
+	err = _ixgbevf_get_reta(hw, msgbuf, reta, IXGBE_VF_GET_RETA_2, 24, 8);
+
+	return err;
+}
+
 /**
  *  ixgbevf_set_rar_vf - set device MAC address
  *  @hw: pointer to hardware structure
@@ -545,6 +617,7 @@ int ixgbevf_get_queues(struct ixgbe_hw *hw, unsigned int *num_tcs,
 	/* do nothing if API doesn't support ixgbevf_get_queues */
 	switch (hw->api_version) {
 	case ixgbe_mbox_api_11:
+	case ixgbe_mbox_api_12:
 		break;
 	default:
 		return 0;
diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.h b/drivers/net/ethernet/intel/ixgbevf/vf.h
index 5b17242..73c1b33 100644
--- a/drivers/net/ethernet/intel/ixgbevf/vf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/vf.h
@@ -208,5 +208,6 @@ void ixgbevf_rlpml_set_vf(struct ixgbe_hw *hw, u16 max_size);
 int ixgbevf_negotiate_api_version(struct ixgbe_hw *hw, int api);
 int ixgbevf_get_queues(struct ixgbe_hw *hw, unsigned int *num_tcs,
 		       unsigned int *default_tc);
+int ixgbevf_get_reta(struct ixgbe_hw *hw, u32 *reta);
 #endif /* __IXGBE_VF_H__ */
 
-- 
2.1.0

Re: [PATCH net-next v1 2/5] ixgbevf: Add a RETA query code

[Alexander Duyck]

I suspect this code is badly broken as it doesn't take several things
into account.

First the PF redirection table can have values outside of the range
supported by the VF.  This is allowed as the VF can set how many bits of
the redirection table it actually wants to use.  This is controlled via
the PSRTYPE register.  So for example the PF can be running with 4
queues, and the VF can run either in single queue or as just a pair of
queues.

Second you could compress this data much more tightly by taking
advantage of the bit widths allowed.  So for everything x540 and older
they only use a 4 bit value per entry.  That means you could
theoretically stuff 8 entries per u32 instead of just 4.  Though I am
not sure if that even really matters since we only care about the last
bit anyway since we should only support RSS for up to 2 queues on the
VFs (IRQ limitation).  You might be able to just get away with a 128b
vector containing a single bit per RSS entry since that is all the VFs
normally will use.

The third issue I see is that this set of patches seem to completely
ignore the X550.  The X550 has a much larger RETA on the PF, and I
believe it does something different for the VF in terms of RSS though I
don't recall exactly what the differences are.

- Alex

On 12/31/2014 01:51 AM, Vlad Zolotarov wrote:
>    - Added a new API version support.
>    - Added the query implementation in the ixgbevf.
>
> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
> ---
> New in v1 (compared to RFC):
>    - Use "if-else" statement instead of a "switch-case" for a single option case
>      (in ixgbevf_get_reta()).
> ---
>  drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |  4 +-
>  drivers/net/ethernet/intel/ixgbevf/mbx.h          |  6 ++
>  drivers/net/ethernet/intel/ixgbevf/vf.c           | 73 +++++++++++++++++++++++
>  drivers/net/ethernet/intel/ixgbevf/vf.h           |  1 +
>  4 files changed, 83 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> index 62a0d8e..ba6ab61 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> @@ -1880,7 +1880,8 @@ static void ixgbevf_init_last_counter_stats(struct ixgbevf_adapter *adapter)
>  static void ixgbevf_negotiate_api(struct ixgbevf_adapter *adapter)
>  {
>  	struct ixgbe_hw *hw = &adapter->hw;
> -	int api[] = { ixgbe_mbox_api_11,
> +	int api[] = { ixgbe_mbox_api_12,
> +		      ixgbe_mbox_api_11,
>  		      ixgbe_mbox_api_10,
>  		      ixgbe_mbox_api_unknown };
>  	int err = 0, idx = 0;
> @@ -3525,6 +3526,7 @@ static int ixgbevf_change_mtu(struct net_device *netdev, int new_mtu)
>  
>  	switch (adapter->hw.api_version) {
>  	case ixgbe_mbox_api_11:
> +	case ixgbe_mbox_api_12:
>  		max_possible_frame = IXGBE_MAX_JUMBO_FRAME_SIZE;
>  		break;
>  	default:
> diff --git a/drivers/net/ethernet/intel/ixgbevf/mbx.h b/drivers/net/ethernet/intel/ixgbevf/mbx.h
> index 0bc3005..3e148a8 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/mbx.h
> +++ b/drivers/net/ethernet/intel/ixgbevf/mbx.h
> @@ -86,6 +86,7 @@ enum ixgbe_pfvf_api_rev {
>  	ixgbe_mbox_api_10,	/* API version 1.0, linux/freebsd VF driver */
>  	ixgbe_mbox_api_20,	/* API version 2.0, solaris Phase1 VF driver */
>  	ixgbe_mbox_api_11,	/* API version 1.1, linux/freebsd VF driver */
> +	ixgbe_mbox_api_12,	/* API version 1.2, linux/freebsd VF driver */
>  	/* This value should always be last */
>  	ixgbe_mbox_api_unknown,	/* indicates that API version is not known */
>  };
> @@ -104,6 +105,11 @@ enum ixgbe_pfvf_api_rev {
>  /* mailbox API, version 1.1 VF requests */
>  #define IXGBE_VF_GET_QUEUE	0x09 /* get queue configuration */
>  
> +/* mailbox API, version 1.2 VF requests */
> +#define IXGBE_VF_GET_RETA_0	0x0a /* get RETA[0..11]  */
> +#define IXGBE_VF_GET_RETA_1	0x0b /* get RETA[12..23] */
> +#define IXGBE_VF_GET_RETA_2	0x0c /* get RETA[24..31] */
> +
>  /* GET_QUEUES return data indices within the mailbox */
>  #define IXGBE_VF_TX_QUEUES	1	/* number of Tx queues supported */
>  #define IXGBE_VF_RX_QUEUES	2	/* number of Rx queues supported */
> diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c b/drivers/net/ethernet/intel/ixgbevf/vf.c
> index cdb53be..8b98cdf 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/vf.c
> +++ b/drivers/net/ethernet/intel/ixgbevf/vf.c
> @@ -258,6 +258,78 @@ static s32 ixgbevf_set_uc_addr_vf(struct ixgbe_hw *hw, u32 index, u8 *addr)
>  	return ret_val;
>  }
>  
> +static inline int _ixgbevf_get_reta(struct ixgbe_hw *hw, u32 *msgbuf,
> +				    u32 *reta, u32 op, int reta_offset_dw,
> +				    size_t dwords)
> +{
> +	int err;
> +
> +	msgbuf[0] = op;
> +	err = hw->mbx.ops.write_posted(hw, msgbuf, 1);
> +
> +	if (err)
> +		return err;
> +
> +	err = hw->mbx.ops.read_posted(hw, msgbuf, 1 + dwords);
> +
> +	if (err)
> +		return err;
> +
> +	msgbuf[0] &= ~IXGBE_VT_MSGTYPE_CTS;
> +
> +	/* If we didn't get an ACK there must have been
> +	 * some sort of mailbox error so we should treat it
> +	 * as such.
> +	 */
> +	if (msgbuf[0] != (op | IXGBE_VT_MSGTYPE_ACK))
> +		return IXGBE_ERR_MBX;
> +
> +	memcpy(reta + reta_offset_dw, msgbuf + 1, 4 * dwords);
> +
> +	return 0;
> +}
> +
> +/**
> + * ixgbevf_get_reta - get the RSS redirection table (RETA) contents.
> + * @hw: pointer to the HW structure
> + * @reta: buffer to fill with RETA contents.
> + *
> + * The "reta" buffer should be big enough to contain 32 registers.
> + *
> + * Returns: 0 on success.
> + *          if API doesn't support this operation - (-EPERM).
> + */
> +int ixgbevf_get_reta(struct ixgbe_hw *hw, u32 *reta)
> +{
> +	int err;
> +	u32 msgbuf[IXGBE_VFMAILBOX_SIZE];
> +
> +	/* Return an error if API doesn't RETA querying. */
> +	if (hw->api_version != ixgbe_mbox_api_12)
> +		return -EPERM;
> +
> +	/* Fetch RETA from the PF. We do it in 3 steps due to mailbox size
> +	 * limitation - we can bring up to 15 dwords every time while RETA
> +	 * consists of 32 dwords. Therefore we'll bring 12, 12 and 8 dwords in
> +	 * each step correspondingly.
> +	 */
> +
> +	/* RETA[0..11] */
> +	err = _ixgbevf_get_reta(hw, msgbuf, reta, IXGBE_VF_GET_RETA_0, 0, 12);
> +	if (err)
> +		return err;
> +
> +	/* RETA[12..23] */
> +	err = _ixgbevf_get_reta(hw, msgbuf, reta, IXGBE_VF_GET_RETA_1, 12, 12);
> +	if (err)
> +		return err;
> +
> +	/* RETA[24..31] */
> +	err = _ixgbevf_get_reta(hw, msgbuf, reta, IXGBE_VF_GET_RETA_2, 24, 8);
> +
> +	return err;
> +}
> +
>  /**
>   *  ixgbevf_set_rar_vf - set device MAC address
>   *  @hw: pointer to hardware structure
> @@ -545,6 +617,7 @@ int ixgbevf_get_queues(struct ixgbe_hw *hw, unsigned int *num_tcs,
>  	/* do nothing if API doesn't support ixgbevf_get_queues */
>  	switch (hw->api_version) {
>  	case ixgbe_mbox_api_11:
> +	case ixgbe_mbox_api_12:
>  		break;
>  	default:
>  		return 0;
> diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.h b/drivers/net/ethernet/intel/ixgbevf/vf.h
> index 5b17242..73c1b33 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/vf.h
> +++ b/drivers/net/ethernet/intel/ixgbevf/vf.h
> @@ -208,5 +208,6 @@ void ixgbevf_rlpml_set_vf(struct ixgbe_hw *hw, u16 max_size);
>  int ixgbevf_negotiate_api_version(struct ixgbe_hw *hw, int api);
>  int ixgbevf_get_queues(struct ixgbe_hw *hw, unsigned int *num_tcs,
>  		       unsigned int *default_tc);
> +int ixgbevf_get_reta(struct ixgbe_hw *hw, u32 *reta);
>  #endif /* __IXGBE_VF_H__ */
>  

Re: [PATCH net-next v1 2/5] ixgbevf: Add a RETA query code

[Vlad Zolotarov]

On 12/31/14 20:00, Alexander Duyck wrote:
> I suspect this code is badly broken as it doesn't take several things
> into account.
>
> First the PF redirection table can have values outside of the range
> supported by the VF.  This is allowed as the VF can set how many bits of
> the redirection table it actually wants to use.  This is controlled via
> the PSRTYPE register.  So for example the PF can be running with 4
> queues, and the VF can run either in single queue or as just a pair of
> queues.
>
> Second you could compress this data much more tightly by taking
> advantage of the bit widths allowed.  So for everything x540 and older
> they only use a 4 bit value per entry.  That means you could
> theoretically stuff 8 entries per u32 instead of just 4.

Compression is nice but I think ethtool expects it in a certain format: 
one entry per byte. And since this patch is targeting the ethtool the 
output format should be as ethtool expects it to be and this is what 
this patch does. However I agree that masking the appropriate bits 
according to PSRTYPE is required. Good catch!

> Though I am
> not sure if that even really matters since we only care about the last
> bit anyway since we should only support RSS for up to 2 queues on the
> VFs (IRQ limitation).  You might be able to just get away with a 128b
> vector containing a single bit per RSS entry since that is all the VFs
> normally will use.

As I mentioned above we shouldn't compress it as u suggested (or in any 
other way) - this would make me uncompress it back in PATCH5 of this 
series... ;)

>
> The third issue I see is that this set of patches seem to completely
> ignore the X550.  The X550 has a much larger RETA on the PF, and I
> believe it does something different for the VF in terms of RSS though I
> don't recall exactly what the differences are.

That's right - the whole patch is written based on the 82599 spec. Now 
when u mentioned it I see that ixgbe/ixgbevf driver supports 3 device 
variants: 82599, x540 and x550. I'll have to revisit their specs and 
update the series correspondingly. Thanks for catching...

vlad

>
> - Alex
>
> On 12/31/2014 01:51 AM, Vlad Zolotarov wrote:
>>     - Added a new API version support.
>>     - Added the query implementation in the ixgbevf.
>>
>> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
>> ---
>> New in v1 (compared to RFC):
>>     - Use "if-else" statement instead of a "switch-case" for a single option case
>>       (in ixgbevf_get_reta()).
>> ---
>>   drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |  4 +-
>>   drivers/net/ethernet/intel/ixgbevf/mbx.h          |  6 ++
>>   drivers/net/ethernet/intel/ixgbevf/vf.c           | 73 +++++++++++++++++++++++
>>   drivers/net/ethernet/intel/ixgbevf/vf.h           |  1 +
>>   4 files changed, 83 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>> index 62a0d8e..ba6ab61 100644
>> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>> @@ -1880,7 +1880,8 @@ static void ixgbevf_init_last_counter_stats(struct ixgbevf_adapter *adapter)
>>   static void ixgbevf_negotiate_api(struct ixgbevf_adapter *adapter)
>>   {
>>   	struct ixgbe_hw *hw = &adapter->hw;
>> -	int api[] = { ixgbe_mbox_api_11,
>> +	int api[] = { ixgbe_mbox_api_12,
>> +		      ixgbe_mbox_api_11,
>>   		      ixgbe_mbox_api_10,
>>   		      ixgbe_mbox_api_unknown };
>>   	int err = 0, idx = 0;
>> @@ -3525,6 +3526,7 @@ static int ixgbevf_change_mtu(struct net_device *netdev, int new_mtu)
>>   
>>   	switch (adapter->hw.api_version) {
>>   	case ixgbe_mbox_api_11:
>> +	case ixgbe_mbox_api_12:
>>   		max_possible_frame = IXGBE_MAX_JUMBO_FRAME_SIZE;
>>   		break;
>>   	default:
>> diff --git a/drivers/net/ethernet/intel/ixgbevf/mbx.h b/drivers/net/ethernet/intel/ixgbevf/mbx.h
>> index 0bc3005..3e148a8 100644
>> --- a/drivers/net/ethernet/intel/ixgbevf/mbx.h
>> +++ b/drivers/net/ethernet/intel/ixgbevf/mbx.h
>> @@ -86,6 +86,7 @@ enum ixgbe_pfvf_api_rev {
>>   	ixgbe_mbox_api_10,	/* API version 1.0, linux/freebsd VF driver */
>>   	ixgbe_mbox_api_20,	/* API version 2.0, solaris Phase1 VF driver */
>>   	ixgbe_mbox_api_11,	/* API version 1.1, linux/freebsd VF driver */
>> +	ixgbe_mbox_api_12,	/* API version 1.2, linux/freebsd VF driver */
>>   	/* This value should always be last */
>>   	ixgbe_mbox_api_unknown,	/* indicates that API version is not known */
>>   };
>> @@ -104,6 +105,11 @@ enum ixgbe_pfvf_api_rev {
>>   /* mailbox API, version 1.1 VF requests */
>>   #define IXGBE_VF_GET_QUEUE	0x09 /* get queue configuration */
>>   
>> +/* mailbox API, version 1.2 VF requests */
>> +#define IXGBE_VF_GET_RETA_0	0x0a /* get RETA[0..11]  */
>> +#define IXGBE_VF_GET_RETA_1	0x0b /* get RETA[12..23] */
>> +#define IXGBE_VF_GET_RETA_2	0x0c /* get RETA[24..31] */
>> +
>>   /* GET_QUEUES return data indices within the mailbox */
>>   #define IXGBE_VF_TX_QUEUES	1	/* number of Tx queues supported */
>>   #define IXGBE_VF_RX_QUEUES	2	/* number of Rx queues supported */
>> diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c b/drivers/net/ethernet/intel/ixgbevf/vf.c
>> index cdb53be..8b98cdf 100644
>> --- a/drivers/net/ethernet/intel/ixgbevf/vf.c
>> +++ b/drivers/net/ethernet/intel/ixgbevf/vf.c
>> @@ -258,6 +258,78 @@ static s32 ixgbevf_set_uc_addr_vf(struct ixgbe_hw *hw, u32 index, u8 *addr)
>>   	return ret_val;
>>   }
>>   
>> +static inline int _ixgbevf_get_reta(struct ixgbe_hw *hw, u32 *msgbuf,
>> +				    u32 *reta, u32 op, int reta_offset_dw,
>> +				    size_t dwords)
>> +{
>> +	int err;
>> +
>> +	msgbuf[0] = op;
>> +	err = hw->mbx.ops.write_posted(hw, msgbuf, 1);
>> +
>> +	if (err)
>> +		return err;
>> +
>> +	err = hw->mbx.ops.read_posted(hw, msgbuf, 1 + dwords);
>> +
>> +	if (err)
>> +		return err;
>> +
>> +	msgbuf[0] &= ~IXGBE_VT_MSGTYPE_CTS;
>> +
>> +	/* If we didn't get an ACK there must have been
>> +	 * some sort of mailbox error so we should treat it
>> +	 * as such.
>> +	 */
>> +	if (msgbuf[0] != (op | IXGBE_VT_MSGTYPE_ACK))
>> +		return IXGBE_ERR_MBX;
>> +
>> +	memcpy(reta + reta_offset_dw, msgbuf + 1, 4 * dwords);
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * ixgbevf_get_reta - get the RSS redirection table (RETA) contents.
>> + * @hw: pointer to the HW structure
>> + * @reta: buffer to fill with RETA contents.
>> + *
>> + * The "reta" buffer should be big enough to contain 32 registers.
>> + *
>> + * Returns: 0 on success.
>> + *          if API doesn't support this operation - (-EPERM).
>> + */
>> +int ixgbevf_get_reta(struct ixgbe_hw *hw, u32 *reta)
>> +{
>> +	int err;
>> +	u32 msgbuf[IXGBE_VFMAILBOX_SIZE];
>> +
>> +	/* Return an error if API doesn't RETA querying. */
>> +	if (hw->api_version != ixgbe_mbox_api_12)
>> +		return -EPERM;
>> +
>> +	/* Fetch RETA from the PF. We do it in 3 steps due to mailbox size
>> +	 * limitation - we can bring up to 15 dwords every time while RETA
>> +	 * consists of 32 dwords. Therefore we'll bring 12, 12 and 8 dwords in
>> +	 * each step correspondingly.
>> +	 */
>> +
>> +	/* RETA[0..11] */
>> +	err = _ixgbevf_get_reta(hw, msgbuf, reta, IXGBE_VF_GET_RETA_0, 0, 12);
>> +	if (err)
>> +		return err;
>> +
>> +	/* RETA[12..23] */
>> +	err = _ixgbevf_get_reta(hw, msgbuf, reta, IXGBE_VF_GET_RETA_1, 12, 12);
>> +	if (err)
>> +		return err;
>> +
>> +	/* RETA[24..31] */
>> +	err = _ixgbevf_get_reta(hw, msgbuf, reta, IXGBE_VF_GET_RETA_2, 24, 8);
>> +
>> +	return err;
>> +}
>> +
>>   /**
>>    *  ixgbevf_set_rar_vf - set device MAC address
>>    *  @hw: pointer to hardware structure
>> @@ -545,6 +617,7 @@ int ixgbevf_get_queues(struct ixgbe_hw *hw, unsigned int *num_tcs,
>>   	/* do nothing if API doesn't support ixgbevf_get_queues */
>>   	switch (hw->api_version) {
>>   	case ixgbe_mbox_api_11:
>> +	case ixgbe_mbox_api_12:
>>   		break;
>>   	default:
>>   		return 0;
>> diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.h b/drivers/net/ethernet/intel/ixgbevf/vf.h
>> index 5b17242..73c1b33 100644
>> --- a/drivers/net/ethernet/intel/ixgbevf/vf.h
>> +++ b/drivers/net/ethernet/intel/ixgbevf/vf.h
>> @@ -208,5 +208,6 @@ void ixgbevf_rlpml_set_vf(struct ixgbe_hw *hw, u16 max_size);
>>   int ixgbevf_negotiate_api_version(struct ixgbe_hw *hw, int api);
>>   int ixgbevf_get_queues(struct ixgbe_hw *hw, unsigned int *num_tcs,
>>   		       unsigned int *default_tc);
>> +int ixgbevf_get_reta(struct ixgbe_hw *hw, u32 *reta);
>>   #endif /* __IXGBE_VF_H__ */
>>   

Re: [PATCH net-next v1 2/5] ixgbevf: Add a RETA query code

[Alexander Duyck]

On 12/31/2014 10:33 AM, Vlad Zolotarov wrote:
>
> On 12/31/14 20:00, Alexander Duyck wrote:
>> I suspect this code is badly broken as it doesn't take several things
>> into account.
>>
>> First the PF redirection table can have values outside of the range
>> supported by the VF.  This is allowed as the VF can set how many bits of
>> the redirection table it actually wants to use.  This is controlled via
>> the PSRTYPE register.  So for example the PF can be running with 4
>> queues, and the VF can run either in single queue or as just a pair of
>> queues.
>>
>> Second you could compress this data much more tightly by taking
>> advantage of the bit widths allowed.  So for everything x540 and older
>> they only use a 4 bit value per entry.  That means you could
>> theoretically stuff 8 entries per u32 instead of just 4.
>
> Compression is nice but I think ethtool expects it in a certain
> format: one entry per byte. And since this patch is targeting the
> ethtool the output format should be as ethtool expects it to be and
> this is what this patch does. However I agree that masking the
> appropriate bits according to PSRTYPE is required. Good catch!

The idea of compression comes into play when you consider there is
significant latency trying to get messages across the mailbox.  By
reducing the number of messages needed to get the redirection table you
should be able to significantly reduce the amount of time needed to
fetch it.  The job of compressing/expanding the values is actually
pretty straight forward when you consider all that should be needed is a
simple loop to perform some shift, and, and or operations.

- Alex

Re: [PATCH net-next v1 2/5] ixgbevf: Add a RETA query code

[Vlad Zolotarov]

On 01/01/15 20:09, Alexander Duyck wrote:
> On 12/31/2014 10:33 AM, Vlad Zolotarov wrote:
>> On 12/31/14 20:00, Alexander Duyck wrote:
>>> I suspect this code is badly broken as it doesn't take several things
>>> into account.
>>>
>>> First the PF redirection table can have values outside of the range
>>> supported by the VF.  This is allowed as the VF can set how many bits of
>>> the redirection table it actually wants to use.  This is controlled via
>>> the PSRTYPE register.  So for example the PF can be running with 4
>>> queues, and the VF can run either in single queue or as just a pair of
>>> queues.
>>>
>>> Second you could compress this data much more tightly by taking
>>> advantage of the bit widths allowed.  So for everything x540 and older
>>> they only use a 4 bit value per entry.  That means you could
>>> theoretically stuff 8 entries per u32 instead of just 4.
>> Compression is nice but I think ethtool expects it in a certain
>> format: one entry per byte. And since this patch is targeting the
>> ethtool the output format should be as ethtool expects it to be and
>> this is what this patch does. However I agree that masking the
>> appropriate bits according to PSRTYPE is required. Good catch!
> The idea of compression comes into play when you consider there is
> significant latency trying to get messages across the mailbox.  By
> reducing the number of messages needed to get the redirection table you
> should be able to significantly reduce the amount of time needed to
> fetch it.  The job of compressing/expanding the values is actually
> pretty straight forward when you consider all that should be needed is a
> simple loop to perform some shift, and, and or operations.

We are talking about a super slow path here. So, regardless how slow it 
is it should be done only once between boots. Therefore IMHO any code 
complication is not justified here.

>
> - Alex

[PATCH net-next v1 3/5] ixgbe: Add GET_RSS_KEY command to VF-PF channel commands set

[Vlad Zolotarov]

82599 VFs and PF share the same RSS Key. Therefore we will return
the same RSS key for all VFs.

Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
---
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   |  2 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 21 +++++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
index c1123d9..52e775b 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
@@ -97,6 +97,8 @@ enum ixgbe_pfvf_api_rev {
 #define IXGBE_VF_GET_RETA_1	0x0b /* get RETA[12..23] */
 #define IXGBE_VF_GET_RETA_2	0x0c /* get RETA[24..31] */
 
+#define IXGBE_VF_GET_RSS_KEY	0x0d /* get RSS key */
+
 /* GET_QUEUES return data indices within the mailbox */
 #define IXGBE_VF_TX_QUEUES	1	/* number of Tx queues supported */
 #define IXGBE_VF_RX_QUEUES	2	/* number of Rx queues supported */
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index b1625c8..d79415e 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -966,6 +966,24 @@ static int ixgbe_get_vf_reta(struct ixgbe_adapter *adapter,
 	return 0;
 }
 
+static int ixgbe_get_vf_rss_key(struct ixgbe_adapter *adapter,
+				u32 *msgbuf, u32 vf)
+{
+	struct ixgbe_hw *hw = &adapter->hw;
+	int i;
+	u32 *rss_key = &msgbuf[1];
+
+	/* verify the PF is supporting the correct API */
+	if (adapter->vfinfo[vf].vf_api != ixgbe_mbox_api_12)
+		return -EPERM;
+
+	/* Read the RSS KEY */
+	for (i = 0; i < 10; i++)
+		rss_key[i] = IXGBE_READ_REG(hw, IXGBE_RSSRK(i));
+
+	return 0;
+}
+
 static int ixgbe_rcv_msg_from_vf(struct ixgbe_adapter *adapter, u32 vf)
 {
 	u32 mbx_size = IXGBE_VFMAILBOX_SIZE;
@@ -1031,6 +1049,9 @@ static int ixgbe_rcv_msg_from_vf(struct ixgbe_adapter *adapter, u32 vf)
 	case IXGBE_VF_GET_RETA_2:
 		retval = ixgbe_get_vf_reta(adapter, msgbuf, vf, 24, 8);
 		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;
-- 
2.1.0

[PATCH net-next v1 4/5] ixgbevf: Add RSS Key query code

[Vlad Zolotarov]

Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
---
New in v1 (compared to RFC):
   - Use "if-else" statement instead of a "switch-case" for a single option case
     (in ixgbevf_get_rss_key()).

---
 drivers/net/ethernet/intel/ixgbevf/mbx.h |  2 ++
 drivers/net/ethernet/intel/ixgbevf/vf.c  | 44 ++++++++++++++++++++++++++++++++
 drivers/net/ethernet/intel/ixgbevf/vf.h  |  1 +
 3 files changed, 47 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbevf/mbx.h b/drivers/net/ethernet/intel/ixgbevf/mbx.h
index 3e148a8..9674ac8 100644
--- a/drivers/net/ethernet/intel/ixgbevf/mbx.h
+++ b/drivers/net/ethernet/intel/ixgbevf/mbx.h
@@ -110,6 +110,8 @@ enum ixgbe_pfvf_api_rev {
 #define IXGBE_VF_GET_RETA_1	0x0b /* get RETA[12..23] */
 #define IXGBE_VF_GET_RETA_2	0x0c /* get RETA[24..31] */
 
+#define IXGBE_VF_GET_RSS_KEY	0x0d /* get RSS hash key */
+
 /* GET_QUEUES return data indices within the mailbox */
 #define IXGBE_VF_TX_QUEUES	1	/* number of Tx queues supported */
 #define IXGBE_VF_RX_QUEUES	2	/* number of Rx queues supported */
diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c b/drivers/net/ethernet/intel/ixgbevf/vf.c
index 8b98cdf..dbac264 100644
--- a/drivers/net/ethernet/intel/ixgbevf/vf.c
+++ b/drivers/net/ethernet/intel/ixgbevf/vf.c
@@ -290,6 +290,50 @@ static inline int _ixgbevf_get_reta(struct ixgbe_hw *hw, u32 *msgbuf,
 }
 
 /**
+ * ixgbevf_get_rss_key - get the RSS Random Key
+ * @hw: pointer to the HW structure
+ * @reta: buffer to fill with RETA contents.
+ *
+ * The "rss_key" buffer should be big enough to contain 10 registers.
+ *
+ * Returns: 0 on success.
+ *          if API doesn't support this operation - (-EPERM).
+ */
+int ixgbevf_get_rss_key(struct ixgbe_hw *hw, u8 *rss_key)
+{
+	int err;
+	u32 msgbuf[IXGBE_VFMAILBOX_SIZE];
+
+	/* Return and error if API doesn't support RSS Random Key retrieval */
+	if (hw->api_version != ixgbe_mbox_api_12)
+		return -EPERM;
+
+	msgbuf[0] = IXGBE_VF_GET_RSS_KEY;
+	err = hw->mbx.ops.write_posted(hw, msgbuf, 1);
+
+	if (err)
+		return err;
+
+	err = hw->mbx.ops.read_posted(hw, msgbuf, 11);
+
+	if (err)
+		return err;
+
+	msgbuf[0] &= ~IXGBE_VT_MSGTYPE_CTS;
+
+	/* If we didn't get an ACK there must have been
+	 * some sort of mailbox error so we should treat it
+	 * as such.
+	 */
+	if (msgbuf[0] != (IXGBE_VF_GET_RSS_KEY | IXGBE_VT_MSGTYPE_ACK))
+		return IXGBE_ERR_MBX;
+
+	memcpy(rss_key, msgbuf + 1, 40);
+
+	return 0;
+}
+
+/**
  * ixgbevf_get_reta - get the RSS redirection table (RETA) contents.
  * @hw: pointer to the HW structure
  * @reta: buffer to fill with RETA contents.
diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.h b/drivers/net/ethernet/intel/ixgbevf/vf.h
index 73c1b33..54f53f2b8 100644
--- a/drivers/net/ethernet/intel/ixgbevf/vf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/vf.h
@@ -209,5 +209,6 @@ int ixgbevf_negotiate_api_version(struct ixgbe_hw *hw, int api);
 int ixgbevf_get_queues(struct ixgbe_hw *hw, unsigned int *num_tcs,
 		       unsigned int *default_tc);
 int ixgbevf_get_reta(struct ixgbe_hw *hw, u32 *reta);
+int ixgbevf_get_rss_key(struct ixgbe_hw *hw, u8 *rss_key);
 #endif /* __IXGBE_VF_H__ */
 
-- 
2.1.0

Re: [PATCH net-next v1 4/5] ixgbevf: Add RSS Key query code

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 818 bytes --]

On Wed, 2014-12-31 at 11:51 +0200, Vlad Zolotarov wrote:
> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
> ---
> New in v1 (compared to RFC):
>    - Use "if-else" statement instead of a "switch-case" for a single
> option case
>      (in ixgbevf_get_rss_key()).
> 
> ---
>  drivers/net/ethernet/intel/ixgbevf/mbx.h |  2 ++
>  drivers/net/ethernet/intel/ixgbevf/vf.c  | 44
> ++++++++++++++++++++++++++++++++
>  drivers/net/ethernet/intel/ixgbevf/vf.h  |  1 +
>  3 files changed, 47 insertions(+)

Just caught this now, sorry but your patch description is sparse (i.e.
non-existent).  I know that the title of the patch pretty much tells
what you are doing, but it would be nice to have a bit more detail as to
why (like your first patch).

Same goes for patch 5 of the series as well.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH net-next v1 4/5] ixgbevf: Add RSS Key query code

[Vlad Zolotarov]

On 12/31/14 18:28, Jeff Kirsher wrote:
> On Wed, 2014-12-31 at 11:51 +0200, Vlad Zolotarov wrote:
>> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
>> ---
>> New in v1 (compared to RFC):
>>     - Use "if-else" statement instead of a "switch-case" for a single
>> option case
>>       (in ixgbevf_get_rss_key()).
>>
>> ---
>>   drivers/net/ethernet/intel/ixgbevf/mbx.h |  2 ++
>>   drivers/net/ethernet/intel/ixgbevf/vf.c  | 44
>> ++++++++++++++++++++++++++++++++
>>   drivers/net/ethernet/intel/ixgbevf/vf.h  |  1 +
>>   3 files changed, 47 insertions(+)
> Just caught this now, sorry but your patch description is sparse (i.e.
> non-existent).  I know that the title of the patch pretty much tells
> what you are doing, but it would be nice to have a bit more detail as to
> why (like your first patch).
>
> Same goes for patch 5 of the series as well.

No problema. ;)
Will send the v2.

Re: [PATCH net-next v1 4/5] ixgbevf: Add RSS Key query code

[Vlad Zolotarov]

On 12/31/14 18:28, Jeff Kirsher wrote:
> On Wed, 2014-12-31 at 11:51 +0200, Vlad Zolotarov wrote:
>> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
>> ---
>> New in v1 (compared to RFC):
>>     - Use "if-else" statement instead of a "switch-case" for a single
>> option case
>>       (in ixgbevf_get_rss_key()).
>>
>> ---
>>   drivers/net/ethernet/intel/ixgbevf/mbx.h |  2 ++
>>   drivers/net/ethernet/intel/ixgbevf/vf.c  | 44
>> ++++++++++++++++++++++++++++++++
>>   drivers/net/ethernet/intel/ixgbevf/vf.h  |  1 +
>>   3 files changed, 47 insertions(+)
> Just caught this now, sorry but your patch description is sparse (i.e.
> non-existent).  I know that the title of the patch pretty much tells
> what you are doing, but it would be nice to have a bit more detail as to
> why (like your first patch).

I've sent a v2 where I tried to address your last remarks. Pls., note 
that the whole series "why" motivation is located in the PATCH0 and in 
the final PATCH5: adding a new ethtool options for 82599 VF devices.

thanks,
vlad

>
> Same goes for patch 5 of the series as well.

Re: [PATCH net-next v1 4/5] ixgbevf: Add RSS Key query code

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 1451 bytes --]

On Wed, 2014-12-31 at 20:03 +0200, Vlad Zolotarov wrote:
> On 12/31/14 18:28, Jeff Kirsher wrote:
> > On Wed, 2014-12-31 at 11:51 +0200, Vlad Zolotarov wrote:
> >> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
> >> ---
> >> New in v1 (compared to RFC):
> >>     - Use "if-else" statement instead of a "switch-case" for a
> single
> >> option case
> >>       (in ixgbevf_get_rss_key()).
> >>
> >> ---
> >>   drivers/net/ethernet/intel/ixgbevf/mbx.h |  2 ++
> >>   drivers/net/ethernet/intel/ixgbevf/vf.c  | 44
> >> ++++++++++++++++++++++++++++++++
> >>   drivers/net/ethernet/intel/ixgbevf/vf.h  |  1 +
> >>   3 files changed, 47 insertions(+)
> > Just caught this now, sorry but your patch description is sparse
> (i.e.
> > non-existent).  I know that the title of the patch pretty much tells
> > what you are doing, but it would be nice to have a bit more detail
> as to
> > why (like your first patch).
> 
> I've sent a v2 where I tried to address your last remarks. Pls., note 
> that the whole series "why" motivation is located in the PATCH0 and
> in 
> the final PATCH5: adding a new ethtool options for 82599 VF devices.

Thanks v2 appears fine with regards to code/patch format goes.  Alex
Duyck appears to have provided feedback on the series, which is a
concern for me.  I have dropped v1 (since v2 is out), but I am going to
hold off on pulling in v2 until Alex's concerns have been addressed.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH net-next v1 4/5] ixgbevf: Add RSS Key query code

[Vlad Zolotarov]

On 12/31/14 20:32, Jeff Kirsher wrote:
> On Wed, 2014-12-31 at 20:03 +0200, Vlad Zolotarov wrote:
>> On 12/31/14 18:28, Jeff Kirsher wrote:
>>> On Wed, 2014-12-31 at 11:51 +0200, Vlad Zolotarov wrote:
>>>> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
>>>> ---
>>>> New in v1 (compared to RFC):
>>>>      - Use "if-else" statement instead of a "switch-case" for a
>> single
>>>> option case
>>>>        (in ixgbevf_get_rss_key()).
>>>>
>>>> ---
>>>>    drivers/net/ethernet/intel/ixgbevf/mbx.h |  2 ++
>>>>    drivers/net/ethernet/intel/ixgbevf/vf.c  | 44
>>>> ++++++++++++++++++++++++++++++++
>>>>    drivers/net/ethernet/intel/ixgbevf/vf.h  |  1 +
>>>>    3 files changed, 47 insertions(+)
>>> Just caught this now, sorry but your patch description is sparse
>> (i.e.
>>> non-existent).  I know that the title of the patch pretty much tells
>>> what you are doing, but it would be nice to have a bit more detail
>> as to
>>> why (like your first patch).
>> I've sent a v2 where I tried to address your last remarks. Pls., note
>> that the whole series "why" motivation is located in the PATCH0 and
>> in
>> the final PATCH5: adding a new ethtool options for 82599 VF devices.
> Thanks v2 appears fine with regards to code/patch format goes.  Alex
> Duyck appears to have provided feedback on the series, which is a
> concern for me.  I have dropped v1 (since v2 is out), but I am going to
> hold off on pulling in v2 until Alex's concerns have been addressed.

Sure. He's caught a few real issues there. The main is that I had only 
82599 in mind and there are also x540 and x550 to consider. It may take 
a day or two... ;)
Again, thanks for looking at this guys.

[PATCH net-next v1 5/5] ixgbevf: Add the appropriate ethtool ops to query RSS indirection table and key

[Vlad Zolotarov]

Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
---
 drivers/net/ethernet/intel/ixgbevf/ethtool.c | 37 ++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ethtool.c b/drivers/net/ethernet/intel/ixgbevf/ethtool.c
index cc0e5b7..255bbc8 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ethtool.c
@@ -792,6 +792,40 @@ static int ixgbevf_set_coalesce(struct net_device *netdev,
 	return 0;
 }
 
+static u32 ixgbevf_get_rxfh_indir_size(struct net_device *netdev)
+{
+	return 128;
+}
+
+static u32 ixgbevf_get_rxfh_key_size(struct net_device *netdev)
+{
+	return 40;
+}
+
+static int ixgbevf_get_rxfh(struct net_device *netdev, u32 *indir, u8 *key,
+			    u8 *hfunc)
+{
+	struct ixgbevf_adapter *adapter = netdev_priv(netdev);
+	int err;
+
+	if (hfunc)
+		*hfunc = ETH_RSS_HASH_TOP;
+
+	if (indir) {
+		err = ixgbevf_get_reta(&adapter->hw, indir);
+		if (err)
+			return err;
+	}
+
+	if (key) {
+		err = ixgbevf_get_rss_key(&adapter->hw, key);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
 static const struct ethtool_ops ixgbevf_ethtool_ops = {
 	.get_settings           = ixgbevf_get_settings,
 	.get_drvinfo            = ixgbevf_get_drvinfo,
@@ -809,6 +843,9 @@ static const struct ethtool_ops ixgbevf_ethtool_ops = {
 	.get_ethtool_stats      = ixgbevf_get_ethtool_stats,
 	.get_coalesce           = ixgbevf_get_coalesce,
 	.set_coalesce           = ixgbevf_set_coalesce,
+	.get_rxfh_indir_size    = ixgbevf_get_rxfh_indir_size,
+	.get_rxfh_key_size      = ixgbevf_get_rxfh_key_size,
+	.get_rxfh		= ixgbevf_get_rxfh,
 };
 
 void ixgbevf_set_ethtool_ops(struct net_device *netdev)
-- 
2.1.0

Re: [PATCH net-next v1 0/5]: ixgbevf: Allow querying VFs RSS indirection table and key

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 739 bytes --]

On Wed, 2014-12-31 at 11:51 +0200, Vlad Zolotarov wrote:
> Add the ethtool ops to VF driver to allow querying the RSS indirection
> table
> and RSS Random Key.
>  
>  - PF driver: Add new VF-PF channel commands.
>  - VF driver: Utilize these new commands and add the corresponding
>               ethtool callbacks.
> 
> New in v1 (compared to RFC):
>    - Use "if-else" statement instead of a "switch-case" for a single
> option case.
>      More specifically: in cases where the newly added API version is
> the only one
>      allowed. We may consider using a "switch-case" back again when
> the list of
>      allowed API versions in these specific places grows up.

Thanks Vlad, I will add your series to my queue.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH net-next v1 0/5]: ixgbevf: Allow querying VFs RSS indirection table and key

[Vlad Zolotarov]

On 12/31/14 18:01, Jeff Kirsher wrote:
> On Wed, 2014-12-31 at 11:51 +0200, Vlad Zolotarov wrote:
>> Add the ethtool ops to VF driver to allow querying the RSS indirection
>> table
>> and RSS Random Key.
>>   
>>   - PF driver: Add new VF-PF channel commands.
>>   - VF driver: Utilize these new commands and add the corresponding
>>                ethtool callbacks.
>>
>> New in v1 (compared to RFC):
>>     - Use "if-else" statement instead of a "switch-case" for a single
>> option case.
>>       More specifically: in cases where the newly added API version is
>> the only one
>>       allowed. We may consider using a "switch-case" back again when
>> the list of
>>       allowed API versions in these specific places grows up.
> Thanks Vlad, I will add your series to my queue.

Great! Thanks a lot Jeff!

Happy New Year to everybody!