From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <tomaszx.kulasek@intel.com>
Received: from mga06.intel.com (mga06.intel.com [134.134.136.31])
 by dpdk.org (Postfix) with ESMTP id 27365B62
 for <dev@dpdk.org>; Mon,  7 Nov 2016 17:03:46 +0100 (CET)
Received: from orsmga001.jf.intel.com ([10.7.209.18])
 by orsmga104.jf.intel.com with ESMTP; 07 Nov 2016 08:03:45 -0800
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.31,606,1473145200"; d="scan'208";a="1056369502"
Received: from irsmsx101.ger.corp.intel.com ([163.33.3.153])
 by orsmga001.jf.intel.com with ESMTP; 07 Nov 2016 08:03:44 -0800
Received: from irsmsx102.ger.corp.intel.com ([169.254.2.226]) by
 IRSMSX101.ger.corp.intel.com ([163.33.3.153]) with mapi id 14.03.0248.002;
 Mon, 7 Nov 2016 16:03:43 +0000
From: "Kulasek, TomaszX" <tomaszx.kulasek@intel.com>
To: Robert Sanford <rsanford2@gmail.com>, "dev@dpdk.org" <dev@dpdk.org>
CC: "Doherty, Declan" <declan.doherty@intel.com>, "De Lara Guarch, Pablo"
 <pablo.de.lara.guarch@intel.com>, "olivier.matz@6wind.com"
 <olivier.matz@6wind.com>
Thread-Topic: [dpdk-dev] [PATCH 4/4] net/bonding: fix configuration of LACP
 slaves
Thread-Index: AQHR7DW1YCPVuB29LkebDKCVOudus6DOSGeQ
Date: Mon, 7 Nov 2016 16:03:43 +0000
Message-ID: <3042915272161B4EB253DA4D77EB373A14F46B27@IRSMSX102.ger.corp.intel.com>
References: <1470084176-79932-1-git-send-email-rsanford@akamai.com>
 <1470084176-79932-5-git-send-email-rsanford@akamai.com>
In-Reply-To: <1470084176-79932-5-git-send-email-rsanford@akamai.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-originating-ip: [163.33.239.181]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH 4/4] net/bonding: fix configuration of LACP
 slaves
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: Mon, 07 Nov 2016 16:03:46 -0000



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Robert Sanford
> Sent: Monday, August 1, 2016 22:43
> To: dev@dpdk.org
> Cc: Doherty, Declan <declan.doherty@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; olivier.matz@6wind.com
> Subject: [dpdk-dev] [PATCH 4/4] net/bonding: fix configuration of LACP
> slaves
>=20
> Problem: When adding a slave or starting a bond device, the bond device
> configures slave devices via function slave_configure().
> However, settings configured in the bond device's rte_eth_conf are not
> propagated to the slaves. For example, VLAN and CRC stripping are not
> working as expected.
>=20
> The problem is that we pass the wrong argument when we invoke
> rte_eth_dev_configure(). We pass the slave's currently configured
> rte_eth_conf (as a source arg!), when we should pass a copy of the
> (master) bond device's rte_eth_conf.
>=20
> Solution: Make a local copy of the bond device's rte_eth_conf, adjust the
> LSC flag based on the slave, and then pass that rte_eth_conf to
> rte_eth_dev_configure().
>=20
> Also, remove code that directly pokes RSS data into the slave's
> rte_eth_conf, as that is also contained in the proper rte_eth_conf that w=
e
> will pass to rte_eth_dev_configure().
>=20
> Signed-off-by: Robert Sanford <rsanford@akamai.com>
> ---
>  drivers/net/bonding/rte_eth_bond_pmd.c |   28 +++++++-------------------=
-
> -
>  1 files changed, 7 insertions(+), 21 deletions(-)
>=20
> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c
> b/drivers/net/bonding/rte_eth_bond_pmd.c
> index b20a272..486582f 100644
> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
> @@ -1302,6 +1302,7 @@ int
>  slave_configure(struct rte_eth_dev *bonded_eth_dev,
>  		struct rte_eth_dev *slave_eth_dev)
>  {
> +	struct rte_eth_conf slave_eth_conf;
>  	struct bond_rx_queue *bd_rx_q;
>  	struct bond_tx_queue *bd_tx_q;
>=20
> @@ -1313,33 +1314,18 @@ slave_configure(struct rte_eth_dev
> *bonded_eth_dev,
>  	/* Stop slave */
>  	rte_eth_dev_stop(slave_eth_dev->data->port_id);
>=20
> -	/* Enable interrupts on slave device if supported */
> -	if (slave_eth_dev->data->dev_flags & RTE_ETH_DEV_INTR_LSC)
> -		slave_eth_dev->data->dev_conf.intr_conf.lsc =3D 1;
> -
> -	/* If RSS is enabled for bonding, try to enable it for slaves  */
> -	if (bonded_eth_dev->data->dev_conf.rxmode.mq_mode &
> ETH_MQ_RX_RSS_FLAG) {
> -		if (bonded_eth_dev->data-
> >dev_conf.rx_adv_conf.rss_conf.rss_key_len
> -				!=3D 0) {
> -			slave_eth_dev->data-
> >dev_conf.rx_adv_conf.rss_conf.rss_key_len =3D
> -					bonded_eth_dev->data-
> >dev_conf.rx_adv_conf.rss_conf.rss_key_len;
> -			slave_eth_dev->data-
> >dev_conf.rx_adv_conf.rss_conf.rss_key =3D
> -					bonded_eth_dev->data-
> >dev_conf.rx_adv_conf.rss_conf.rss_key;
> -		} else {
> -			slave_eth_dev->data-
> >dev_conf.rx_adv_conf.rss_conf.rss_key =3D NULL;
> -		}
> +	/* Build slave rte_eth_conf, starting from bonded's conf */
> +	slave_eth_conf =3D bonded_eth_dev->data->dev_conf;
>=20
> -		slave_eth_dev->data->dev_conf.rx_adv_conf.rss_conf.rss_hf =3D
> -				bonded_eth_dev->data-
> >dev_conf.rx_adv_conf.rss_conf.rss_hf;
> -		slave_eth_dev->data->dev_conf.rxmode.mq_mode =3D
> -				bonded_eth_dev->data->dev_conf.rxmode.mq_mode;
> -	}
> +	/* Enable interrupts on slave device if supported */
> +	slave_eth_conf.intr_conf.lsc =3D
> +		!!(slave_eth_dev->data->dev_flags & RTE_ETH_DEV_INTR_LSC);
>=20
>  	/* Configure device */
>  	errval =3D rte_eth_dev_configure(slave_eth_dev->data->port_id,
>  			bonded_eth_dev->data->nb_rx_queues,
>  			bonded_eth_dev->data->nb_tx_queues,
> -			&(slave_eth_dev->data->dev_conf));
> +			&slave_eth_conf);
>  	if (errval !=3D 0) {
>  		RTE_BOND_LOG(ERR, "Cannot configure slave device: port %u , err
> (%d)",
>  				slave_eth_dev->data->port_id, errval);
> --
> 1.7.1

Reviewed-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>