From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <3chas3@gmail.com> Received: from mail-it0-f68.google.com (mail-it0-f68.google.com [209.85.214.68]) by dpdk.org (Postfix) with ESMTP id 93BA25F51; Thu, 20 Sep 2018 14:47:20 +0200 (CEST) Received: by mail-it0-f68.google.com with SMTP id x79-v6so1544993ita.1; Thu, 20 Sep 2018 05:47:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:sender:from:date:message-id :subject:to:cc; bh=xUAsRvECs2roCUeZEhgndHbakF0qRJIrgvVogKSxEhM=; b=RFWkKrr3VY+OD8JMdFypEycq0sz+mdPcvOm5RovrkYivWEkZ4HRSvaMdgnGyQMyYhE ncM7srU7H71YTbGE/oAqpLe0zkXtyzp568N/VgD0rK4yAG+QNdiN7vwqhd+bphnyyc8N 9hOcCbxQ6NuLSAP+o9kbySx80P2yQgbinqAFldMjVttwxpa/Au5rBXD8FxFr4N3cVZS3 Re06xK/vsr7FrEtTZDzyacQOJ5rU+p26HvY44ZmQBTWCM0zp/ZpXTk51QSued+nxzX2y YxhO8u+XM5vMeH+jxMRsX7OgplggnjdnMq5WuAv9Rxbb7v0wWxKs3IVb+WMIhbCrmnwz 0LyQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:sender:from :date:message-id:subject:to:cc; bh=xUAsRvECs2roCUeZEhgndHbakF0qRJIrgvVogKSxEhM=; b=E7JgNb/uGgZd0MDJV8sj/6rDMwbnExRgWM4TTdxVBW//tx6rUwsNeyVJI6zWriVyzR wjgawSawTDW2yIPvxvlZFZu+2xiACmiNWtXA7fMp60R2eodpFQ2h0GiwD9L8AeIaewDd nUPP+CFwQhYNx8IDZ3BhvmXE1XzM8oRrGwtl+H1XLwa7tN4PLyRBWmGTQZ8c/o4oVgFU DqTBPyt7DxB6ffQzYmwbwIU/iWjx+ZOrLkXLMMedZzooM+AB7nE2+QcFG7+vewjqfCEt OZwufQCxwx9SlsDv+czWCU9UuPI0K5WX1/2W3rGkuzstBfWTwPEyFp92N4YmIhYVPeC8 Fjyw== X-Gm-Message-State: APzg51BdbcHnVxvI+vvei/G24DhW4gPxuEYPAjOCuVvUfbbWjFjtUVYZ c99IqkHVcVWPzrhAs5o627xJUW0Ga0iFtHajanY= X-Google-Smtp-Source: ANB0VdZ0NC8VJdCYksBmvXC1XDBxSqcUHg0SE+p9+bKQVa83E1tln3Kpd/xTItPmLuzlTGjprrRDBG2c4m1sC6Dhcxo= X-Received: by 2002:a24:cfd7:: with SMTP id y206-v6mr1802372itf.112.1537447639796; Thu, 20 Sep 2018 05:47:19 -0700 (PDT) MIME-Version: 1.0 References: <20180919154825.5183-1-3chas3@gmail.com> In-Reply-To: Sender: chasmosaurus@gmail.com X-Google-Sender-Delegation: chasmosaurus@gmail.com From: Chas Williams <3chas3@gmail.com> Date: Thu, 20 Sep 2018 08:47:08 -0400 X-Google-Sender-Auth: 6gIhkoQGbCj1J4keIU0c3aLyCjs Message-ID: To: Matan Azrad Cc: dev@dpdk.org, Declan Doherty , Eric Kinzie , Chas Williams , stable@dpdk.org Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH] net/bonding: ensure fairness among slaves X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 20 Sep 2018 12:47:20 -0000 On Thu, Sep 20, 2018 at 2:28 AM Matan Azrad wrote: > > Hi Chas > Please see small comments. > > > From: Chas Williams > > Some PMDs, especially ones with vector receives, require a minimum > > number of receive buffers in order to receive any packets. If the first slave > > read leaves less than this number available, a read from the next slave may > > return 0 implying that the slave doesn't have any packets which results in > > skipping over that slave as the next active slave. > > It is true not only in case of 0. > It makes sense that the first polling slave gets the majority part of the burst while the others just get smaller part > I suggest to rephrase to the general issue . It doesn't happen for the 802.3ad burst routines in general. If you run out of buffers then you don't advance to the next slave and that slave picks up where you left off during the next rx burst. If some slave is attempting to do this, it will consume all the buffers and you will be at the next slave for the next rx and all is well. There are just some odd corner cases, where you read just slow (or fast?) enough that the first slave leaves just a few buffers. But reading the next slave results in a 0 (because of the vector RX), and you don't loop back around to the first slave. So next time around you start back at the troublesome slave. The fix for the other RX burst routines is just completeness. > > > > > To fix this, implement round robin for the slaves during receive that is only > > advanced to the next slave at the end of each receive burst. > > This should also provide some additional fairness in processing in > > bond_ethdev_rx_burst as well. > > > > Fixes: 2efb58cbab6e ("bond: new link bonding library") > > If it is a fix, why not to use a fix title? > Maybe > net/bonding: fix the slaves Rx fairness I can use the word fix I suppose. > > > Cc: stable@dpdk.org > > > > Signed-off-by: Chas Williams > Besides that: > Acked-by: Matan Azrad > > > --- > > drivers/net/bonding/rte_eth_bond_pmd.c | 50 > > ++++++++++++++++++++++------------ > > 1 file changed, 32 insertions(+), 18 deletions(-) > > > > diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c > > b/drivers/net/bonding/rte_eth_bond_pmd.c > > index b84f32263..f25faa75c 100644 > > --- a/drivers/net/bonding/rte_eth_bond_pmd.c > > +++ b/drivers/net/bonding/rte_eth_bond_pmd.c > > @@ -58,28 +58,33 @@ bond_ethdev_rx_burst(void *queue, struct > > rte_mbuf **bufs, uint16_t nb_pkts) { > > struct bond_dev_private *internals; > > > > - uint16_t num_rx_slave = 0; > > uint16_t num_rx_total = 0; > > - > > + uint16_t slave_count; > > + uint16_t active_slave; > > int i; > > > > /* Cast to structure, containing bonded device's port id and queue id > > */ > > struct bond_rx_queue *bd_rx_q = (struct bond_rx_queue *)queue; > > - > > internals = bd_rx_q->dev_private; > > + slave_count = internals->active_slave_count; > > + active_slave = internals->active_slave; > > > > + for (i = 0; i < slave_count && nb_pkts; i++) { > > + uint16_t num_rx_slave; > > > > - for (i = 0; i < internals->active_slave_count && 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( > > + internals->active_slaves[active_slave], > > bd_rx_q->queue_id, bufs + num_rx_total, > > nb_pkts); > > - if (num_rx_slave) { > > - num_rx_total += num_rx_slave; > > - nb_pkts -= num_rx_slave; > > - } > > + num_rx_total += num_rx_slave; > > + nb_pkts -= num_rx_slave; > > + if (++active_slave == slave_count) > > + active_slave = 0; > > } > > > > + if (++internals->active_slave == slave_count) > > + internals->active_slave = 0; > > return num_rx_total; > > } > > > > @@ -258,25 +263,32 @@ bond_ethdev_rx_burst_8023ad_fast_queue(void > > *queue, struct rte_mbuf **bufs, > > uint16_t num_rx_total = 0; /* Total number of received packets > > */ > > uint16_t slaves[RTE_MAX_ETHPORTS]; > > uint16_t slave_count; > > - > > - uint16_t i, idx; > > + uint16_t active_slave; > > + uint16_t i; > > > > /* Copy slave list to protect against slave up/down changes during tx > > * bursting */ > > slave_count = internals->active_slave_count; > > + active_slave = internals->active_slave; > > memcpy(slaves, internals->active_slaves, > > sizeof(internals->active_slaves[0]) * slave_count); > > > > - for (i = 0, idx = internals->active_slave; > > - i < slave_count && num_rx_total < nb_pkts; i++, > > idx++) { > > - idx = idx % slave_count; > > + for (i = 0; i < slave_count && nb_pkts; i++) { > > + uint16_t num_rx_slave; > > > > /* Read packets from this slave */ > > - num_rx_total += rte_eth_rx_burst(slaves[idx], bd_rx_q- > > >queue_id, > > - &bufs[num_rx_total], nb_pkts - > > num_rx_total); > > + num_rx_slave = rte_eth_rx_burst(slaves[active_slave], > > + bd_rx_q->queue_id, > > + bufs + num_rx_total, > > nb_pkts); > > + num_rx_total += num_rx_slave; > > + nb_pkts -= num_rx_slave; > > + > > + if (++active_slave == slave_count) > > + active_slave = 0; > > } > > > > - internals->active_slave = idx; > > + if (++internals->active_slave == slave_count) > > + internals->active_slave = 0; > > > > return num_rx_total; > > } > > @@ -459,7 +471,9 @@ bond_ethdev_rx_burst_8023ad(void *queue, struct > > rte_mbuf **bufs, > > idx = 0; > > } > > > > - internals->active_slave = idx; > > + if (++internals->active_slave == slave_count) > > + internals->active_slave = 0; > > + > > return num_rx_total; > > } > > > > -- > > 2.14.4 >