Re: [dpdk-dev] [PATCH] bonding: fix wrong slaves capacity check

[Radu Nicolau <radu.nicolau@intel.com>]

On 7/27/2017 1:00 PM, Tomasz Kulasek wrote:
> For fortville NIC bond_ethdev_8023ad_flow_verify fails when action queue
> index indicates unavailable queue before slaves configuration.
>
> This fix verifies flow settings for queue 0, which is always available,
> and checks if slaves max queue number capacity meets requirements.
>
> Fixes: 112891cd27e5 ("net/bonding: add dedicated HW queues for LACP control")
>
> Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
Reviewed-by:  Radu Nicolau <radu.nicolau@intel.com>
> ---
>   drivers/net/bonding/rte_eth_bond_pmd.c | 26 +++++++++++++++++++-------
>   1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
> index 8f9a860..ab5ebe0 100644
> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
> @@ -175,12 +175,13 @@ const struct rte_flow_attr flow_attr_8023ad = {
>   int
>   bond_ethdev_8023ad_flow_verify(struct rte_eth_dev *bond_dev,
>   		uint8_t slave_port) {
> +	struct rte_eth_dev_info slave_info;
>   	struct rte_flow_error error;
>   	struct bond_dev_private *internals = (struct bond_dev_private *)
>   			(bond_dev->data->dev_private);
>   
> -	struct rte_flow_action_queue lacp_queue_conf = {
> -		.index = internals->mode4.dedicated_queues.rx_qid,
> +	const struct rte_flow_action_queue lacp_queue_conf = {
> +		.index = 0,
>   	};
>   
>   	const struct rte_flow_action actions[] = {
> @@ -195,8 +196,22 @@ bond_ethdev_8023ad_flow_verify(struct rte_eth_dev *bond_dev,
>   
>   	int ret = rte_flow_validate(slave_port, &flow_attr_8023ad,
>   			flow_item_8023ad, actions, &error);
> -	if (ret < 0)
> +	if (ret < 0) {
> +		RTE_BOND_LOG(ERR, "bond_ethdev_8023ad_flow_verify: %s "
> +				"(slave_port=%d queue_id=%d)",
> +				error.message, slave_port,
> +				internals->mode4.dedicated_queues.rx_qid);
> +		return -1;
> +	}
> +
> +	rte_eth_dev_info_get(slave_port, &slave_info);
> +	if ((slave_info.max_rx_queues < bond_dev->data->nb_rx_queues) ||
> +			(slave_info.max_tx_queues < bond_dev->data->nb_tx_queues)) {
> +		RTE_BOND_LOG(ERR, "bond_ethdev_8023ad_flow_verify: Slave %d"
> +				" capabilities doesn't allow to allocate "
> +				"additional queues", slave_port);
>   		return -1;
> +	}
>   
>   	return 0;
>   }
> @@ -206,7 +221,7 @@ bond_8023ad_slow_pkt_hw_filter_supported(uint8_t port_id) {
>   	struct rte_eth_dev *bond_dev = &rte_eth_devices[port_id];
>   	struct bond_dev_private *internals = (struct bond_dev_private *)
>   			(bond_dev->data->dev_private);
> -	struct rte_eth_dev_info bond_info, slave_info;
> +	struct rte_eth_dev_info bond_info;
>   	uint8_t idx;
>   
>   	/* Verify if all slaves in bonding supports flow director and */
> @@ -217,9 +232,6 @@ bond_8023ad_slow_pkt_hw_filter_supported(uint8_t port_id) {
>   		internals->mode4.dedicated_queues.tx_qid = bond_info.nb_tx_queues;
>   
>   		for (idx = 0; idx < internals->slave_count; idx++) {
> -			rte_eth_dev_info_get(internals->slaves[idx].port_id,
> -					&slave_info);
> -
>   			if (bond_ethdev_8023ad_flow_verify(bond_dev,
>   					internals->slaves[idx].port_id) != 0)
>   				return -1;