[dpdk-dev] [PATCH 1/4] Link Bonding Library

[Shaw, Jeffrey B]

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

> +static uint16_t
> +bond_ethdev_tx_broadcast(void *queue, struct rte_mbuf **bufs, uint16_t nb_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 = 0;
> +
> +	int i;
> +
> +	bd_tx_q = (struct bond_tx_queue *)queue;
> +	internals = bd_tx_q->dev_private;
> +
> +	/* Copy slave list to protect against slave up/down changes during tx
> +	 * bursting */
> +	num_of_slaves = 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 = 0; i < num_of_slaves; i++) {
> +		num_tx_total += 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 PMD frees the mbufs for previously transmitted packets.
So if you broadcast 1 packet on 4 slaves, each of the 4 slaves will (eventually) have to free the same mbuf.  Without updating the refcnt, you will end up incorrectly freeing the same mbuf 3 extra times.

Your test application does not catch this case since the max packets that are tested is 320, which is less than the size of the Tx descriptor rings (512).  Try testing with more packets and you should see some latent segmentation faults.  You should also see this with testpmd, if you try broadcasting enough packets.


Thanks,
Jeff