From mboxrd@z Thu Jan  1 00:00:00 1970
From: Chas Williams <3chas3@gmail.com>
Subject: Re: 8023ad bond tx crashed if one port has 2 more tx
	queues
Date: Tue, 19 Mar 2019 11:05:54 -0400
Message-ID: <99fe680a-13e2-3d90-2fb2-4a389a50fd56@gmail.com>
References: <001a01d4de37$ec097f80$c41c7e80$@netitest.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
To: hfli@netitest.com, declan.doherty@intel.com, chas3@att.com, dev@dpdk.org
Return-path: <dev-bounces@dpdk.org>
Received: from mail-qt1-f193.google.com (mail-qt1-f193.google.com
 [209.85.160.193]) by dpdk.org (Postfix) with ESMTP id 015611DBD
 for <dev@dpdk.org>; Tue, 19 Mar 2019 16:05:56 +0100 (CET)
Received: by mail-qt1-f193.google.com with SMTP id v32so22466148qtc.10
 for <dev@dpdk.org>; Tue, 19 Mar 2019 08:05:56 -0700 (PDT)
In-Reply-To: <001a01d4de37$ec097f80$c41c7e80$@netitest.com>
Content-Language: en-US
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On 3/19/19 5:41 AM, hfli@netitest.com wrote:
> Hi Guys,
> 
> I found a bug in dpdk bond code, while one port has 2 more tx queues, 
> 8023ad bond port will be crashed in tx burst.
> 
> Just analyzed the code below, if 2 more CPU cores send packets on a port 
> by different tx queue, the arrays like
> 
> slave_port_ids/ dist_slave_port_ids/ slave_tx_fail_coun/ slave_bufs will 
> be shared by all of cores, it will be crashed in this function.

I am afraid that I don't understand this description. I believe that
these are allocated on the stack and shouldn't be shared between
threads.

There isn't any locking currently in the driver. You need to stop the
data path before making changes to the slaves (with the exception of
link status) attached to the bonding device.

> Is there any better solution for this? For now, I just add lock for 
> rte_eth_tx_burst.
> 
> static uint16_t
> 
> bond_ethdev_tx_burst_8023ad(void *queue, struct rte_mbuf **bufs,
>                     uint16_t nb_bufs)
> 
> {
>           struct bond_tx_queue *bd_tx_q = (struct bond_tx_queue *)queue;
>           struct bond_dev_private *internals = bd_tx_q->dev_private;
>           uint16_t slave_port_ids[RTE_MAX_ETHPORTS];
>           uint16_t slave_count;
>
>           uint16_t dist_slave_port_ids[RTE_MAX_ETHPORTS];
>           uint16_t dist_slave_count;
> 
>           /* 2-D array to sort mbufs for transmission on each slave into */
>           struct rte_mbuf *slave_bufs[RTE_MAX_ETHPORTS][nb_bufs];
> 
>           /* Number of mbufs for transmission on each slave */
>           uint16_t slave_nb_bufs[RTE_MAX_ETHPORTS] = { 0 };
> 
>           /* Mapping array generated by hash function to map mbufs to 
> slaves */
>           uint16_t bufs_slave_port_idxs[RTE_MAX_ETHPORTS] = { 0 };
>           uint16_t slave_tx_count, slave_tx_fail_count[RTE_MAX_ETHPORTS] 
> = { 0 };
>           uint16_t total_tx_count = 0, total_tx_fail_count = 0;
> 
> Thanks and Regards,
> 
> Haifeng
>