[dpdk-dev,2/4] ixgbe: VF RSS config on x550

  On x550, there're separate registers provided for VF RSS while on the other
10G NICs, for example, 82599, VF and PF share the same registers.
This patch lets x550 use the VF specific registers when doing RSS configuration
on VF. The behavior of other 10G NICs doesn't change.

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 drivers/net/ixgbe/ixgbe_rxtx.c | 111 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 108 insertions(+), 3 deletions(-)
  

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo Lu
> Sent: Monday, September 28, 2015 8:52 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 2/4] ixgbe: VF RSS config on x550
> 
> On x550, there're separate registers provided for VF RSS while on the other
> 10G NICs, for example, 82599, VF and PF share the same registers.
> This patch lets x550 use the VF specific registers when doing RSS configuration
> on VF. The behavior of other 10G NICs doesn't change.
> 
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> ---
>  drivers/net/ixgbe/ixgbe_rxtx.c | 111 +++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 108 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index a746ae7..4a2d24a 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -2838,6 +2838,107 @@ ixgbe_rss_configure(struct rte_eth_dev *dev)
>  	ixgbe_hw_rss_hash_set(hw, &rss_conf);
>  }
> 
> +static void
> +ixgbevf_rss_disable_x550(struct rte_eth_dev *dev)
> +{
> +	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	uint32_t vfmrqc;
> +
> +	vfmrqc = IXGBE_READ_REG(hw, IXGBE_VFMRQC);
> +	vfmrqc &= ~IXGBE_MRQC_RSSEN;
> +	IXGBE_WRITE_REG(hw, IXGBE_VFMRQC, vfmrqc);
> +}
> +
> +static void
> +ixgbevf_hw_rss_hash_set_x550(struct ixgbe_hw *hw,
> +			struct rte_eth_rss_conf *rss_conf)
> +{
> +	uint8_t  *hash_key;
> +	uint32_t vfmrqc;
> +	uint32_t rss_key;
> +	uint64_t rss_hf;
> +	uint16_t i;
> +
> +	hash_key = rss_conf->rss_key;
> +	if (hash_key != NULL) {
> +		/* Fill in RSS hash key */
> +		for (i = 0; i < 10; i++) {
> +			rss_key  = hash_key[(i * 4)];
> +			rss_key |= hash_key[(i * 4) + 1] << 8;
> +			rss_key |= hash_key[(i * 4) + 2] << 16;
> +			rss_key |= hash_key[(i * 4) + 3] << 24;
> +			IXGBE_WRITE_REG_ARRAY(hw, IXGBE_VFRSSRK(0), i, rss_key);
> +		}
> +	}
> +
> +	/* Set configured hashing protocols in VFMRQC register */
> +	rss_hf = rss_conf->rss_hf;
> +	vfmrqc = IXGBE_MRQC_RSSEN; /* Enable RSS */
> +	if (rss_hf & ETH_RSS_IPV4)
> +		vfmrqc |= IXGBE_MRQC_RSS_FIELD_IPV4;
> +	if (rss_hf & ETH_RSS_NONFRAG_IPV4_TCP)
> +		vfmrqc |= IXGBE_MRQC_RSS_FIELD_IPV4_TCP;
> +	if (rss_hf & ETH_RSS_IPV6)
> +		vfmrqc |= IXGBE_MRQC_RSS_FIELD_IPV6;
> +	if (rss_hf & ETH_RSS_IPV6_EX)
> +		vfmrqc |= IXGBE_MRQC_RSS_FIELD_IPV6_EX;
> +	if (rss_hf & ETH_RSS_NONFRAG_IPV6_TCP)
> +		vfmrqc |= IXGBE_MRQC_RSS_FIELD_IPV6_TCP;
> +	if (rss_hf & ETH_RSS_IPV6_TCP_EX)
> +		vfmrqc |= IXGBE_MRQC_RSS_FIELD_IPV6_EX_TCP;
> +	if (rss_hf & ETH_RSS_NONFRAG_IPV4_UDP)
> +		vfmrqc |= IXGBE_MRQC_RSS_FIELD_IPV4_UDP;
> +	if (rss_hf & ETH_RSS_NONFRAG_IPV6_UDP)
> +		vfmrqc |= IXGBE_MRQC_RSS_FIELD_IPV6_UDP;
> +	if (rss_hf & ETH_RSS_IPV6_UDP_EX)
> +		vfmrqc |= IXGBE_MRQC_RSS_FIELD_IPV6_EX_UDP;
> +	IXGBE_WRITE_REG(hw, IXGBE_VFMRQC, vfmrqc);
> +}

This function is identical to ixgbe_hw_rss_hash_set(), except the 2 HW register sets it updates:
IXGBE_VFMRQC vs IXGBE_MRQC and IXGBE_VFRSSRK(0) vs IXGBE_RSSRK(0).
Plus they botha re static functions, so why not addresses of these 2 HW registers as extra parameters
to the ixgbe_hw_rss_hash_set() and use it in all places.
That way you'll avoid quite big code duplication.

> +
> +static void
> +ixgbevf_rss_configure_x550(struct rte_eth_dev *dev)
> +{
> +	struct rte_eth_rss_conf rss_conf;
> +	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	uint32_t reta = 0;
> +	uint16_t i;
> +	uint16_t j;
> +
> +	PMD_INIT_FUNC_TRACE();
> +
> +	if (dev->data->dev_conf.rxmode.mq_mode != ETH_MQ_RX_RSS) {
> +		ixgbevf_rss_disable_x550(dev);
> +		PMD_DRV_LOG(DEBUG, "RSS not configured\n");
> +		return;
> +	}
> +	/*
> +	 * Fill in redirection table
> +	 * The byte-swap is needed because NIC registers are in
> +	 * little-endian order.
> +	 */
> +	for (i = 0, j = 0; i < ETH_RSS_RETA_SIZE_64; i++, j++) {
> +		if (j == dev->data->nb_rx_queues)
> +			j = 0;
> +		reta = (reta << 8) | j;
> +		if ((i & 3) == 3)
> +			IXGBE_WRITE_REG(hw, IXGBE_VFRETA(i >> 2),
> +					rte_bswap32(reta));
> +	}
> +
> +	/*
> +	 * Configure the RSS key and the RSS protocols used to compute
> +	 * the RSS hash of input packets.
> +	 */
> +	rss_conf = dev->data->dev_conf.rx_adv_conf.rss_conf;
> +	if ((rss_conf.rss_hf & IXGBE_RSS_OFFLOAD_ALL) == 0) {
> +		ixgbevf_rss_disable_x550(dev);
> +		return;
> +	}
> +	if (rss_conf.rss_key == NULL)
> +		rss_conf.rss_key = rss_intel_key; /* Default hash key */
> +	ixgbevf_hw_rss_hash_set_x550(hw, &rss_conf);
> +}
> +

Same comment as above: > 90%of that function is just copy & paste of ixgbe_rss_configure().
Pls find a way to unify them and avoid unnecessary code duplication and growth.

>  #define NUM_VFTA_REGISTERS 128
>  #define NIC_RX_BUFFER_SIZE 0x200
>  #define X550_RX_BUFFER_SIZE 0x180
> @@ -3621,12 +3722,16 @@ ixgbe_alloc_rx_queue_mbufs(struct ixgbe_rx_queue *rxq)
>  static int
>  ixgbe_config_vf_rss(struct rte_eth_dev *dev)
>  {
> -	struct ixgbe_hw *hw;
> +	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>  	uint32_t mrqc;
> 
> -	ixgbe_rss_configure(dev);
> +	if (hw->mac.type == ixgbe_mac_X550_vf ||
> +		hw->mac.type == ixgbe_mac_X550EM_x_vf) {
> +		ixgbevf_rss_configure_x550(dev);
> +		return 0;
> +	}
> 
> -	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	ixgbe_rss_configure(dev);
> 
>  	/* MRQC: enable VF RSS */
>  	mrqc = IXGBE_READ_REG(hw, IXGBE_MRQC);
> --
> 1.9.3