From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <konstantin.ananyev@intel.com>
Received: from mga11.intel.com (mga11.intel.com [192.55.52.93])
 by dpdk.org (Postfix) with ESMTP id 9C02591FC
 for <dev@dpdk.org>; Fri, 16 Oct 2015 00:42:06 +0200 (CEST)
Received: from orsmga001.jf.intel.com ([10.7.209.18])
 by fmsmga102.fm.intel.com with ESMTP; 15 Oct 2015 15:42:06 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.17,687,1437462000"; d="scan'208";a="794183674"
Received: from irsmsx110.ger.corp.intel.com ([163.33.3.25])
 by orsmga001.jf.intel.com with ESMTP; 15 Oct 2015 15:42:04 -0700
Received: from irsmsx155.ger.corp.intel.com (163.33.192.3) by
 irsmsx110.ger.corp.intel.com (163.33.3.25) with Microsoft SMTP Server (TLS)
 id 14.3.248.2; Thu, 15 Oct 2015 23:42:03 +0100
Received: from irsmsx105.ger.corp.intel.com ([169.254.7.75]) by
 irsmsx155.ger.corp.intel.com ([169.254.14.242]) with mapi id 14.03.0248.002;
 Thu, 15 Oct 2015 23:42:03 +0100
From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: "Lu, Wenzhuo" <wenzhuo.lu@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Thread-Topic: [dpdk-dev] [PATCH 2/4] ixgbe: VF RSS config on x550
Thread-Index: AQHQ+cLCM1jbJDgXcEaq3PS27Z/rL55tP2lQ
Date: Thu, 15 Oct 2015 22:42:02 +0000
Message-ID: <2601191342CEEE43887BDE71AB97725836AB0310@irsmsx105.ger.corp.intel.com>
References: <1443426751-4906-1-git-send-email-wenzhuo.lu@intel.com>
 <1443426751-4906-3-git-send-email-wenzhuo.lu@intel.com>
In-Reply-To: <1443426751-4906-3-git-send-email-wenzhuo.lu@intel.com>
Accept-Language: en-IE, en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-originating-ip: [163.33.239.182]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH 2/4] ixgbe: VF RSS config on x550
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Thu, 15 Oct 2015 22:42:07 -0000



> -----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
>=20
> On x550, there're separate registers provided for VF RSS while on the oth=
er
> 10G NICs, for example, 82599, VF and PF share the same registers.
> This patch lets x550 use the VF specific registers when doing RSS configu=
ration
> on VF. The behavior of other 10G NICs doesn't change.
>=20
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> ---
>  drivers/net/ixgbe/ixgbe_rxtx.c | 111 +++++++++++++++++++++++++++++++++++=
++++--
>  1 file changed, 108 insertions(+), 3 deletions(-)
>=20
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxt=
x.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);
>  }
>=20
> +static void
> +ixgbevf_rss_disable_x550(struct rte_eth_dev *dev)
> +{
> +	struct ixgbe_hw *hw =3D IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private)=
;
> +	uint32_t vfmrqc;
> +
> +	vfmrqc =3D IXGBE_READ_REG(hw, IXGBE_VFMRQC);
> +	vfmrqc &=3D ~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 =3D rss_conf->rss_key;
> +	if (hash_key !=3D NULL) {
> +		/* Fill in RSS hash key */
> +		for (i =3D 0; i < 10; i++) {
> +			rss_key  =3D hash_key[(i * 4)];
> +			rss_key |=3D hash_key[(i * 4) + 1] << 8;
> +			rss_key |=3D hash_key[(i * 4) + 2] << 16;
> +			rss_key |=3D 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 =3D rss_conf->rss_hf;
> +	vfmrqc =3D IXGBE_MRQC_RSSEN; /* Enable RSS */
> +	if (rss_hf & ETH_RSS_IPV4)
> +		vfmrqc |=3D IXGBE_MRQC_RSS_FIELD_IPV4;
> +	if (rss_hf & ETH_RSS_NONFRAG_IPV4_TCP)
> +		vfmrqc |=3D IXGBE_MRQC_RSS_FIELD_IPV4_TCP;
> +	if (rss_hf & ETH_RSS_IPV6)
> +		vfmrqc |=3D IXGBE_MRQC_RSS_FIELD_IPV6;
> +	if (rss_hf & ETH_RSS_IPV6_EX)
> +		vfmrqc |=3D IXGBE_MRQC_RSS_FIELD_IPV6_EX;
> +	if (rss_hf & ETH_RSS_NONFRAG_IPV6_TCP)
> +		vfmrqc |=3D IXGBE_MRQC_RSS_FIELD_IPV6_TCP;
> +	if (rss_hf & ETH_RSS_IPV6_TCP_EX)
> +		vfmrqc |=3D IXGBE_MRQC_RSS_FIELD_IPV6_EX_TCP;
> +	if (rss_hf & ETH_RSS_NONFRAG_IPV4_UDP)
> +		vfmrqc |=3D IXGBE_MRQC_RSS_FIELD_IPV4_UDP;
> +	if (rss_hf & ETH_RSS_NONFRAG_IPV6_UDP)
> +		vfmrqc |=3D IXGBE_MRQC_RSS_FIELD_IPV6_UDP;
> +	if (rss_hf & ETH_RSS_IPV6_UDP_EX)
> +		vfmrqc |=3D 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 regi=
ster 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 reg=
isters 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 =3D IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private)=
;
> +	uint32_t reta =3D 0;
> +	uint16_t i;
> +	uint16_t j;
> +
> +	PMD_INIT_FUNC_TRACE();
> +
> +	if (dev->data->dev_conf.rxmode.mq_mode !=3D 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 =3D 0, j =3D 0; i < ETH_RSS_RETA_SIZE_64; i++, j++) {
> +		if (j =3D=3D dev->data->nb_rx_queues)
> +			j =3D 0;
> +		reta =3D (reta << 8) | j;
> +		if ((i & 3) =3D=3D 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 =3D dev->data->dev_conf.rx_adv_conf.rss_conf;
> +	if ((rss_conf.rss_hf & IXGBE_RSS_OFFLOAD_ALL) =3D=3D 0) {
> +		ixgbevf_rss_disable_x550(dev);
> +		return;
> +	}
> +	if (rss_conf.rss_key =3D=3D NULL)
> +		rss_conf.rss_key =3D 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 gro=
wth.

>  #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 =3D IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private)=
;
>  	uint32_t mrqc;
>=20
> -	ixgbe_rss_configure(dev);
> +	if (hw->mac.type =3D=3D ixgbe_mac_X550_vf ||
> +		hw->mac.type =3D=3D ixgbe_mac_X550EM_x_vf) {
> +		ixgbevf_rss_configure_x550(dev);
> +		return 0;
> +	}
>=20
> -	hw =3D IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	ixgbe_rss_configure(dev);
>=20
>  	/* MRQC: enable VF RSS */
>  	mrqc =3D IXGBE_READ_REG(hw, IXGBE_MRQC);
> --
> 1.9.3