From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qg0-f52.google.com (mail-qg0-f52.google.com [209.85.192.52]) by dpdk.org (Postfix) with ESMTP id BBB529236 for ; Fri, 4 Dec 2015 20:18:37 +0100 (CET) Received: by qgec40 with SMTP id c40so96158729qge.2 for ; Fri, 04 Dec 2015 11:18:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=F+QX8EYdoxmNqSnUvRiTwA4T/i562Z8+TWg/6AZsvSo=; b=mOXZUiUTPKUnlFPsgtVjdEihslxtHb4YfhSq734EjACx3icf5W/g6XnVleopAaZOMP AV6GvQ66tzgHA26kGbsquAiKujA8XGYZMPweeVzqSnTQhHMrNHCYrJFyjkiAtnt7XLOA nuknOHisiW2KI7Dkm+E5njF78gEwrlwlCoCDc1v5zzNCRn1sOsptCxyz+A7k6CRcIouv Rh+I91bI/KaYF9n3C9Op9/2htgkQMjHniPkus1G6qBkm8oqjok6MBWBuI3L0DsTLrAyt s19ZAy7Ygaqugw1PoTjVxk2/pqe/c0nz316BHnCUJX+PFjNsppktEetuBPQ+9wNMUnA2 Ax3Q== X-Received: by 10.140.151.145 with SMTP id 139mr21809242qhx.95.1449256717242; Fri, 04 Dec 2015 11:18:37 -0800 (PST) Received: from gmail.com (pool-108-31-208-15.washdc.fios.verizon.net. [108.31.208.15]) by smtp.gmail.com with ESMTPSA id h188sm6040447qhh.23.2015.12.04.11.18.35 (version=TLS1 cipher=AES128-SHA bits=128/128); Fri, 04 Dec 2015 11:18:36 -0800 (PST) Received: by gmail.com (sSMTP sendmail emulation); Fri, 4 Dec 2015 14:18:34 -0500 Date: Fri, 4 Dec 2015 14:18:34 -0500 From: Eric Kinzie To: Andriy Berestovskyy Message-ID: <20151204191831.GA20647@roosta.home> References: <1449249260-15165-1-git-send-email-stephen@networkplumber.org> <1449249260-15165-7-git-send-email-stephen@networkplumber.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Cc: dev@dpdk.org, Eric Kinzie Subject: Re: [dpdk-dev] [PATCH 6/8] bond: handle slaves with fewer queues than bonding device X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 04 Dec 2015 19:18:38 -0000 On Fri Dec 04 19:36:09 +0100 2015, Andriy Berestovskyy wrote: > Hi guys, > I'm not quite sure if we can support less TX queues on a slave that easy: > > > queue_id = bond_slave_txqid(internals, i, bd_tx_q->queue_id); > > num_tx_slave = rte_eth_tx_burst(slaves[i], queue_id, > > slave_bufs[i], slave_nb_pkts[i]); > > It seems that two different lcores might end up writing to the same > slave queue at the same time, isn't it? > > Regards, > Andriy Andriy, I think you're probably right about this. Perhaps it should instead refuse to add or refuse to activate a slave with too few tx queues. Could probably fix this with another layer of buffering so that an lcore with a valid tx queue could pick up the mbufs later, but this doesn't seem very appealing. Eric > On Fri, Dec 4, 2015 at 6:14 PM, Stephen Hemminger > wrote: > > From: Eric Kinzie > > > > In the event that the bonding device has a greater number of tx and/or rx > > queues than the slave being added, track the queue limits of the slave. > > On receive, ignore queue identifiers beyond what the slave interface > > can support. During transmit, pick a different queue id to use if the > > intended queue is not available on the slave. > > > > Signed-off-by: Eric Kinzie > > Signed-off-by: Stephen Hemminger > > --- > > drivers/net/bonding/rte_eth_bond_api.c | 6 +- > > drivers/net/bonding/rte_eth_bond_pmd.c | 141 +++++++++++++++++++++++++---- > > drivers/net/bonding/rte_eth_bond_private.h | 5 +- > > 3 files changed, 129 insertions(+), 23 deletions(-) > > > > diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c > > index 630a461..64058ff 100644 > > --- a/drivers/net/bonding/rte_eth_bond_api.c > > +++ b/drivers/net/bonding/rte_eth_bond_api.c > > @@ -340,11 +340,11 @@ __eth_bond_slave_add_lock_free(uint8_t bonded_port_id, uint8_t slave_port_id) > > > > slave_eth_dev = &rte_eth_devices[slave_port_id]; > > > > - /* Add slave details to bonded device */ > > - slave_add(internals, slave_eth_dev); > > - > > rte_eth_dev_info_get(slave_port_id, &dev_info); > > > > + /* Add slave details to bonded device */ > > + slave_add(internals, slave_eth_dev, &dev_info); > > + > > /* We need to store slaves reta_size to be able to synchronize RETA for all > > * slave devices even if its sizes are different. > > */ > > diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c > > index 77582dd..868e66b 100644 > > --- a/drivers/net/bonding/rte_eth_bond_pmd.c > > +++ b/drivers/net/bonding/rte_eth_bond_pmd.c > > @@ -76,6 +76,47 @@ get_vlan_offset(struct ether_hdr *eth_hdr, uint16_t *proto) > > return vlan_offset; > > } > > > > +static uint8_t > > +bond_active_slaves_by_rxqid(struct bond_dev_private *internals, int queue_id, > > + uint8_t slaves[]) > > +{ > > + struct bond_slave_details *slave_details; > > + uint8_t num_of_slaves; > > + uint8_t i = 0; > > + > > + 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 || internals->kvlist) > > + return num_of_slaves; > > + > > + /* remove slaves that don't have a queue numbered "queue_id" */ > > + while (i < num_of_slaves) { > > + slave_details = &internals->slaves[i]; > > + if (unlikely(queue_id >= slave_details->nb_rx_queues)) { > > + slaves[i] = slaves[num_of_slaves-1]; > > + num_of_slaves--; > > + } else > > + i++; > > + } > > + > > + return num_of_slaves; > > +} > > + > > +static int > > +bond_slave_txqid(struct bond_dev_private *internals, uint8_t slave_id, > > + int queue_id) > > +{ > > + struct bond_slave_details *slave_details; > > + > > + if (internals->kvlist) > > + return queue_id; > > + > > + slave_details = &internals->slaves[slave_id]; > > + return queue_id % slave_details->nb_tx_queues; > > +} > > + > > static uint16_t > > bond_ethdev_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) > > { > > @@ -83,6 +124,8 @@ bond_ethdev_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) > > > > uint16_t num_rx_slave = 0; > > uint16_t num_rx_total = 0; > > + uint8_t slaves[RTE_MAX_ETHPORTS]; > > + uint8_t num_of_slaves; > > > > int i; > > > > @@ -91,11 +134,13 @@ bond_ethdev_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) > > > > internals = bd_rx_q->dev_private; > > > > + num_of_slaves = bond_active_slaves_by_rxqid(internals, bd_rx_q->queue_id, > > + slaves); > > > > - for (i = 0; i < internals->active_slave_count && nb_pkts; i++) { > > + for (i = 0; i < num_of_slaves && nb_pkts; i++) { > > /* Offset of pointer to *bufs increases as packets are received > > * from other slaves */ > > - num_rx_slave = rte_eth_rx_burst(internals->active_slaves[i], > > + num_rx_slave = rte_eth_rx_burst(slaves[i], > > bd_rx_q->queue_id, bufs + num_rx_total, nb_pkts); > > if (num_rx_slave) { > > num_rx_total += num_rx_slave; > > @@ -117,8 +162,13 @@ bond_ethdev_rx_burst_active_backup(void *queue, struct rte_mbuf **bufs, > > > > internals = bd_rx_q->dev_private; > > > > - return rte_eth_rx_burst(internals->current_primary_port, > > - bd_rx_q->queue_id, bufs, nb_pkts); > > + uint8_t active_slave = internals->current_primary_port; > > + struct rte_eth_dev *dev = &rte_eth_devices[active_slave]; > > + > > + if (bd_rx_q->queue_id >= dev->data->nb_rx_queues) > > + return 0; > > + > > + return rte_eth_rx_burst(active_slave, bd_rx_q->queue_id, bufs, nb_pkts); > > } > > > > static uint16_t > > @@ -144,9 +194,9 @@ bond_ethdev_rx_burst_8023ad(void *queue, struct rte_mbuf **bufs, > > rte_eth_macaddr_get(internals->port_id, &bond_mac); > > /* Copy slave list to protect against slave up/down changes during tx > > * bursting */ > > - slave_count = internals->active_slave_count; > > - memcpy(slaves, internals->active_slaves, > > - sizeof(internals->active_slaves[0]) * slave_count); > > + > > + slave_count = bond_active_slaves_by_rxqid(internals, bd_rx_q->queue_id, > > + slaves); > > > > for (i = 0; i < slave_count && num_rx_total < nb_pkts; i++) { > > j = num_rx_total; > > @@ -401,6 +451,7 @@ bond_ethdev_tx_burst_round_robin(void *queue, struct rte_mbuf **bufs, > > > > static int slave_idx = 0; > > int i, cslave_idx = 0, tx_fail_total = 0; > > + int queue_id; > > > > bd_tx_q = (struct bond_tx_queue *)queue; > > internals = bd_tx_q->dev_private; > > @@ -427,7 +478,9 @@ bond_ethdev_tx_burst_round_robin(void *queue, struct rte_mbuf **bufs, > > /* Send packet burst on each slave device */ > > for (i = 0; i < num_of_slaves; i++) { > > if (slave_nb_pkts[i] > 0) { > > - num_tx_slave = rte_eth_tx_burst(slaves[i], bd_tx_q->queue_id, > > + queue_id = bond_slave_txqid(internals, i, > > + bd_tx_q->queue_id); > > + num_tx_slave = rte_eth_tx_burst(slaves[i], queue_id, > > slave_bufs[i], slave_nb_pkts[i]); > > > > /* if tx burst fails move packets to end of bufs */ > > @@ -453,14 +506,27 @@ bond_ethdev_tx_burst_active_backup(void *queue, > > { > > struct bond_dev_private *internals; > > struct bond_tx_queue *bd_tx_q; > > + int queue_id; > > + int i; > > + uint8_t num_of_slaves; > > + uint8_t slaves[RTE_MAX_ETHPORTS]; > > > > bd_tx_q = (struct bond_tx_queue *)queue; > > internals = bd_tx_q->dev_private; > > > > - if (internals->active_slave_count < 1) > > + 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; > > > > - return rte_eth_tx_burst(internals->current_primary_port, bd_tx_q->queue_id, > > + for (i = 0; i < num_of_slaves; i++) > > + if (slaves[i] == internals->current_primary_port) > > + break; > > + > > + queue_id = bond_slave_txqid(internals, i, bd_tx_q->queue_id); > > + return rte_eth_tx_burst(internals->current_primary_port, queue_id, > > bufs, nb_pkts); > > } > > > > @@ -696,6 +762,7 @@ bond_ethdev_tx_burst_tlb(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) > > struct ether_hdr *ether_hdr; > > struct ether_addr primary_slave_addr; > > struct ether_addr active_slave_addr; > > + int queue_id; > > > > if (num_of_slaves < 1) > > return num_tx_total; > > @@ -725,7 +792,8 @@ bond_ethdev_tx_burst_tlb(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) > > #endif > > } > > > > - num_tx_total += rte_eth_tx_burst(slaves[i], bd_tx_q->queue_id, > > + queue_id = bond_slave_txqid(internals, i, bd_tx_q->queue_id); > > + num_tx_total += rte_eth_tx_burst(slaves[i], queue_id, > > bufs + num_tx_total, nb_pkts - num_tx_total); > > > > if (num_tx_total == nb_pkts) > > @@ -903,6 +971,7 @@ bond_ethdev_tx_burst_balance(void *queue, struct rte_mbuf **bufs, > > uint16_t num_tx_total = 0, num_tx_slave = 0, tx_fail_total = 0; > > > > int i, op_slave_id; > > + int queue_id; > > > > struct rte_mbuf *slave_bufs[RTE_MAX_ETHPORTS][nb_pkts]; > > uint16_t slave_nb_pkts[RTE_MAX_ETHPORTS] = { 0 }; > > @@ -931,7 +1000,9 @@ bond_ethdev_tx_burst_balance(void *queue, struct rte_mbuf **bufs, > > /* Send packet burst on each slave device */ > > for (i = 0; i < num_of_slaves; i++) { > > if (slave_nb_pkts[i] > 0) { > > - num_tx_slave = rte_eth_tx_burst(slaves[i], bd_tx_q->queue_id, > > + queue_id = bond_slave_txqid(internals, i, > > + bd_tx_q->queue_id); > > + num_tx_slave = rte_eth_tx_burst(slaves[i], queue_id, > > slave_bufs[i], slave_nb_pkts[i]); > > > > /* if tx burst fails move packets to end of bufs */ > > @@ -977,6 +1048,8 @@ bond_ethdev_tx_burst_8023ad(void *queue, struct rte_mbuf **bufs, > > /* Slow packets placed in each slave */ > > uint8_t slave_slow_nb_pkts[RTE_MAX_ETHPORTS] = { 0 }; > > > > + int queue_id; > > + > > bd_tx_q = (struct bond_tx_queue *)queue; > > internals = bd_tx_q->dev_private; > > > > @@ -1022,7 +1095,8 @@ bond_ethdev_tx_burst_8023ad(void *queue, struct rte_mbuf **bufs, > > if (slave_nb_pkts[i] == 0) > > continue; > > > > - num_tx_slave = rte_eth_tx_burst(slaves[i], bd_tx_q->queue_id, > > + queue_id = bond_slave_txqid(internals, i, bd_tx_q->queue_id); > > + num_tx_slave = rte_eth_tx_burst(slaves[i], queue_id, > > slave_bufs[i], slave_nb_pkts[i]); > > > > /* If tx burst fails drop slow packets */ > > @@ -1057,6 +1131,7 @@ bond_ethdev_tx_burst_broadcast(void *queue, struct rte_mbuf **bufs, > > > > int slave_tx_total[RTE_MAX_ETHPORTS]; > > int i, most_successful_tx_slave = -1; > > + int queue_id; > > > > bd_tx_q = (struct bond_tx_queue *)queue; > > internals = bd_tx_q->dev_private; > > @@ -1076,7 +1151,8 @@ bond_ethdev_tx_burst_broadcast(void *queue, struct rte_mbuf **bufs, > > > > /* Transmit burst on each active slave */ > > for (i = 0; i < num_of_slaves; i++) { > > - slave_tx_total[i] = rte_eth_tx_burst(slaves[i], bd_tx_q->queue_id, > > + queue_id = bond_slave_txqid(internals, i, bd_tx_q->queue_id); > > + slave_tx_total[i] = rte_eth_tx_burst(slaves[i], queue_id, > > bufs, nb_pkts); > > > > if (unlikely(slave_tx_total[i] < nb_pkts)) > > @@ -1298,9 +1374,22 @@ int > > slave_configure(struct rte_eth_dev *bonded_eth_dev, > > struct rte_eth_dev *slave_eth_dev) > > { > > + struct bond_dev_private *internals; > > struct bond_rx_queue *bd_rx_q; > > struct bond_tx_queue *bd_tx_q; > > + int slave_id; > > + > > + internals = bonded_eth_dev->data->dev_private; > > > > + for (slave_id = 0; slave_id < internals->slave_count; slave_id++) > > + if (internals->slaves[slave_id].port_id == > > + slave_eth_dev->data->port_id) > > + break; > > + > > + RTE_VERIFY(slave_id != internals->slave_count); > > + > > + uint16_t nb_rx_queues = internals->slaves[slave_id].nb_rx_queues; > > + uint16_t nb_tx_queues = internals->slaves[slave_id].nb_tx_queues; > > int errval; > > uint16_t q_id; > > > > @@ -1331,8 +1420,7 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev, > > > > /* 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, > > + nb_rx_queues, nb_tx_queues, > > &(slave_eth_dev->data->dev_conf)); > > if (errval != 0) { > > RTE_BOND_LOG(ERR, "Cannot configure slave device: port %u , err (%d)", > > @@ -1343,7 +1431,7 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev, > > /* Setup Rx Queues */ > > /* Use existing queues, if any */ > > for (q_id = slave_eth_dev->data->nb_rx_queues; > > - q_id < bonded_eth_dev->data->nb_rx_queues; q_id++) { > > + q_id < nb_rx_queues ; q_id++) { > > bd_rx_q = (struct bond_rx_queue *)bonded_eth_dev->data->rx_queues[q_id]; > > > > errval = rte_eth_rx_queue_setup(slave_eth_dev->data->port_id, q_id, > > @@ -1361,7 +1449,7 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev, > > /* Setup Tx Queues */ > > /* Use existing queues, if any */ > > for (q_id = slave_eth_dev->data->nb_tx_queues; > > - q_id < bonded_eth_dev->data->nb_tx_queues; q_id++) { > > + q_id < nb_tx_queues ; q_id++) { > > bd_tx_q = (struct bond_tx_queue *)bonded_eth_dev->data->tx_queues[q_id]; > > > > errval = rte_eth_tx_queue_setup(slave_eth_dev->data->port_id, q_id, > > @@ -1440,7 +1528,8 @@ bond_ethdev_slave_link_status_change_monitor(void *cb_arg); > > > > void > > slave_add(struct bond_dev_private *internals, > > - struct rte_eth_dev *slave_eth_dev) > > + struct rte_eth_dev *slave_eth_dev, > > + const struct rte_eth_dev_info *slave_dev_info) > > { > > struct bond_slave_details *slave_details = > > &internals->slaves[internals->slave_count]; > > @@ -1448,6 +1537,20 @@ slave_add(struct bond_dev_private *internals, > > slave_details->port_id = slave_eth_dev->data->port_id; > > slave_details->last_link_status = 0; > > > > + uint16_t bond_nb_rx_queues = > > + rte_eth_devices[internals->port_id].data->nb_rx_queues; > > + uint16_t bond_nb_tx_queues = > > + rte_eth_devices[internals->port_id].data->nb_tx_queues; > > + > > + slave_details->nb_rx_queues = > > + bond_nb_rx_queues > slave_dev_info->max_rx_queues > > + ? slave_dev_info->max_rx_queues > > + : bond_nb_rx_queues; > > + slave_details->nb_tx_queues = > > + bond_nb_tx_queues > slave_dev_info->max_tx_queues > > + ? slave_dev_info->max_tx_queues > > + : bond_nb_tx_queues; > > + > > /* If slave device doesn't support interrupts then we need to enabled > > * polling to monitor link status */ > > if (!(slave_eth_dev->data->dev_flags & RTE_PCI_DRV_INTR_LSC)) { > > diff --git a/drivers/net/bonding/rte_eth_bond_private.h b/drivers/net/bonding/rte_eth_bond_private.h > > index 6c47a29..02f6de1 100644 > > --- a/drivers/net/bonding/rte_eth_bond_private.h > > +++ b/drivers/net/bonding/rte_eth_bond_private.h > > @@ -101,6 +101,8 @@ struct bond_slave_details { > > uint8_t link_status_poll_enabled; > > uint8_t link_status_wait_to_complete; > > uint8_t last_link_status; > > + uint16_t nb_rx_queues; > > + uint16_t nb_tx_queues; > > /**< Port Id of slave eth_dev */ > > struct ether_addr persisted_mac_addr; > > > > @@ -240,7 +242,8 @@ slave_remove(struct bond_dev_private *internals, > > > > void > > slave_add(struct bond_dev_private *internals, > > - struct rte_eth_dev *slave_eth_dev); > > + struct rte_eth_dev *slave_eth_dev, > > + const struct rte_eth_dev_info *slave_dev_info); > > > > uint16_t > > xmit_l2_hash(const struct rte_mbuf *buf, uint8_t slave_count); > > -- > > 2.1.4 > > > > > > -- > Andriy Berestovskyy