[dpdk-dev] [PATCH 4/4] net/bonding: fix configuration of LACP slaves

[Kulasek, TomaszX]

> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Robert Sanford
> Sent: Monday, August 1, 2016 22:43
> To: dev at dpdk.org
> Cc: Doherty, Declan <declan.doherty at intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch at intel.com>; olivier.matz at 6wind.com
> Subject: [dpdk-dev] [PATCH 4/4] net/bonding: fix configuration of LACP
> slaves
> 
> 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.
> 
> 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.
> 
> 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().
> 
> 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 we
> will pass to rte_eth_dev_configure().
> 
> Signed-off-by: Robert Sanford <rsanford at akamai.com>
> ---
>  drivers/net/bonding/rte_eth_bond_pmd.c |   28 +++++++--------------------
> -
>  1 files changed, 7 insertions(+), 21 deletions(-)
> 
> 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;
> 
> @@ -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);
> 
> -	/* 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 = 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
> -				!= 0) {
> -			slave_eth_dev->data-
> >dev_conf.rx_adv_conf.rss_conf.rss_key_len =
> -					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 =
> -					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 = NULL;
> -		}
> +	/* Build slave rte_eth_conf, starting from bonded's conf */
> +	slave_eth_conf = bonded_eth_dev->data->dev_conf;
> 
> -		slave_eth_dev->data->dev_conf.rx_adv_conf.rss_conf.rss_hf =
> -				bonded_eth_dev->data-
> >dev_conf.rx_adv_conf.rss_conf.rss_hf;
> -		slave_eth_dev->data->dev_conf.rxmode.mq_mode =
> -				bonded_eth_dev->data->dev_conf.rxmode.mq_mode;
> -	}
> +	/* Enable interrupts on slave device if supported */
> +	slave_eth_conf.intr_conf.lsc =
> +		!!(slave_eth_dev->data->dev_flags & RTE_ETH_DEV_INTR_LSC);
> 
>  	/* Configure device */
>  	errval = 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 != 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 at intel.com>