From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <jeffrey.b.shaw@intel.com>
Received: from mga01.intel.com (mga01.intel.com [192.55.52.88])
 by dpdk.org (Postfix) with ESMTP id E5EA430D
 for <dev@dpdk.org>; Wed, 28 May 2014 18:54:43 +0200 (CEST)
Received: from fmsmga002.fm.intel.com ([10.253.24.26])
 by fmsmga101.fm.intel.com with ESMTP; 28 May 2014 09:54:49 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="4.98,929,1392192000"; d="scan'208";a="546262696"
Received: from fmsmsx106.amr.corp.intel.com ([10.19.9.37])
 by fmsmga002.fm.intel.com with ESMTP; 28 May 2014 09:54:45 -0700
Received: from FMSMSX109.amr.corp.intel.com (10.18.116.9) by
 FMSMSX106.amr.corp.intel.com (10.19.9.37) with Microsoft SMTP Server (TLS) id
 14.3.123.3; Wed, 28 May 2014 09:54:45 -0700
Received: from fmsmsx103.amr.corp.intel.com ([169.254.3.77]) by
 FMSMSX109.amr.corp.intel.com ([169.254.15.159]) with mapi id 14.03.0123.003;
 Wed, 28 May 2014 09:54:45 -0700
From: "Shaw, Jeffrey B" <jeffrey.b.shaw@intel.com>
To: "Doherty, Declan" <declan.doherty@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Thread-Topic: [dpdk-dev] [PATCH 1/4] Link Bonding Library
Thread-Index: AQHPeoo2OlwUXPNhp0OuNh6/BEBAi5tWMDFQ
Date: Wed, 28 May 2014 16:54:44 +0000
Message-ID: <4032A54B6BB5F04B8C08B6CFF08C59285543C357@FMSMSX103.amr.corp.intel.com>
References: <cover.1401287412.git.declan.doherty@intel.com>
 <4d8e6bc2665fbaac641f0577714d7be9b0415d3c.1401287412.git.declan.doherty@intel.com>
In-Reply-To: <4d8e6bc2665fbaac641f0577714d7be9b0415d3c.1401287412.git.declan.doherty@intel.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-originating-ip: [10.1.200.106]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH 1/4] Link Bonding Library
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: Wed, 28 May 2014 16:54:44 -0000

Hi Declan,
I'm worried about one thing in "bond_ethdev_tx_broadcast()" related to free=
ing of the broadcasted packets.

> +static uint16_t
> +bond_ethdev_tx_broadcast(void *queue, struct rte_mbuf **bufs, uint16_t n=
b_pkts)
> +{
> +	struct bond_dev_private *internals;
> +	struct bond_tx_queue *bd_tx_q;
> +
> +	uint8_t num_of_slaves;
> +	uint8_t slaves[RTE_MAX_ETHPORTS];
> +
> +	uint16_t num_tx_total =3D 0;
> +
> +	int i;
> +
> +	bd_tx_q =3D (struct bond_tx_queue *)queue;
> +	internals =3D bd_tx_q->dev_private;
> +
> +	/* Copy slave list to protect against slave up/down changes during tx
> +	 * bursting */
> +	num_of_slaves =3D internals->active_slave_count;
> +	memcpy(slaves, internals->active_slaves,
> +			sizeof(internals->active_slaves[0]) * num_of_slaves);
> +
> +	if (num_of_slaves < 1)
> +		return 0;
> +
> +
> +	for (i =3D 0; i < num_of_slaves; i++) {
> +		num_tx_total +=3D rte_eth_tx_burst(slaves[i], bd_tx_q->queue_id,
> +				bufs, nb_pkts);
> +	}
> +
> +	return num_tx_total;
> +}
> +

Transmitting the same buffers on all slaves will cause problems when the PM=
D frees the mbufs for previously transmitted packets.
So if you broadcast 1 packet on 4 slaves, each of the 4 slaves will (eventu=
ally) have to free the same mbuf.  Without updating the refcnt, you will en=
d up incorrectly freeing the same mbuf 3 extra times.

Your test application does not catch this case since the max packets that a=
re tested is 320, which is less than the size of the Tx descriptor rings (5=
12).  Try testing with more packets and you should see some latent segmenta=
tion faults.  You should also see this with testpmd, if you try broadcastin=
g enough packets.


Thanks,
Jeff