From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 5B992133F for ; Tue, 21 Mar 2017 14:53:14 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=intel.com; i=@intel.com; q=dns/txt; s=intel; t=1490104394; x=1521640394; h=subject:to:references:from:cc:message-id:date: mime-version:in-reply-to:content-transfer-encoding; bh=khnXfJFkIp2hk8XqkRMQyzZIQhPBXY70simTHCYj3Xg=; b=LCW5kyvNvUsO6hrmIBbQs/y/XWR7iRrA1b7lC9LlBtH237cXi9VmOoF3 SZekz8BJBAkgSlHzNYMga1S8BTdZVg==; Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 21 Mar 2017 06:53:13 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,198,1486454400"; d="scan'208";a="79404316" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.122]) ([10.237.220.122]) by fmsmga006.fm.intel.com with ESMTP; 21 Mar 2017 06:53:11 -0700 To: Keith Wiles , dev@dpdk.org References: <20170307223918.33906-1-keith.wiles@intel.com> From: Ferruh Yigit Cc: Declan Doherty Message-ID: Date: Tue, 21 Mar 2017 13:53:11 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <20170307223918.33906-1-keith.wiles@intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v3] net/bonding: reduce slave starvation on rx poll 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: Tue, 21 Mar 2017 13:53:14 -0000 On 3/7/2017 10:39 PM, Keith Wiles wrote: > When polling the bonded ports for RX packets the old driver would > always start with the first slave in the list. If the requested > number of packets is filled on the first port in a two port config > then the second port could be starved or have larger number of > missed packet errors. > > The code attempts to start with a different slave each time RX poll > is done to help eliminate starvation of slave ports. The effect of > the previous code was much lower performance for two slaves in the > bond then just the one slave. > > The performance drop was detected when the application can not poll > the rings of rx packets fast enough and the packets per second for > two or more ports was at the threshold thoughput of the application. > At this threshold the slaves would see very little or no drops in > the case of one slave. Then enable the second slave you would see > a large drop rate on the two slave bond and reduction in thoughput. > > Signed-off-by: Keith Wiles > --- > v3 - remove more checkpatch errors > v2 - remove checkpatch errors > > drivers/net/bonding/rte_eth_bond_pmd.c | 21 +++++++++++++++------ > drivers/net/bonding/rte_eth_bond_private.h | 3 ++- > 2 files changed, 17 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c > index f3ac9e273..18c7b2e85 100644 > --- a/drivers/net/bonding/rte_eth_bond_pmd.c > +++ b/drivers/net/bonding/rte_eth_bond_pmd.c > @@ -1,7 +1,7 @@ > /*- > * BSD LICENSE > * > - * Copyright(c) 2010-2015 Intel Corporation. All rights reserved. > + * Copyright(c) 2010-2017 Intel Corporation. All rights reserved. > * All rights reserved. > * > * Redistribution and use in source and binary forms, with or without > @@ -145,7 +145,7 @@ bond_ethdev_rx_burst_8023ad(void *queue, struct rte_mbuf **bufs, > const uint16_t ether_type_slow_be = rte_be_to_cpu_16(ETHER_TYPE_SLOW); > uint16_t num_rx_total = 0; /* Total number of received packets */ > uint8_t slaves[RTE_MAX_ETHPORTS]; > - uint8_t slave_count; > + uint8_t slave_count, idx; > > uint8_t collecting; /* current slave collecting status */ > const uint8_t promisc = internals->promiscuous_en; > @@ -159,12 +159,18 @@ bond_ethdev_rx_burst_8023ad(void *queue, struct rte_mbuf **bufs, > memcpy(slaves, internals->active_slaves, > sizeof(internals->active_slaves[0]) * slave_count); > > + idx = internals->active_slave; > + if (idx >= slave_count) { > + internals->active_slave = 0; > + idx = 0; > + } > for (i = 0; i < slave_count && num_rx_total < nb_pkts; i++) { > j = num_rx_total; > - collecting = ACTOR_STATE(&mode_8023ad_ports[slaves[i]], COLLECTING); > + collecting = ACTOR_STATE(&mode_8023ad_ports[slaves[idx]], > + COLLECTING); > > /* Read packets from this slave */ > - num_rx_total += rte_eth_rx_burst(slaves[i], bd_rx_q->queue_id, > + num_rx_total += rte_eth_rx_burst(slaves[idx], bd_rx_q->queue_id, > &bufs[num_rx_total], nb_pkts - num_rx_total); > > for (k = j; k < 2 && k < num_rx_total; k++) > @@ -187,8 +193,8 @@ bond_ethdev_rx_burst_8023ad(void *queue, struct rte_mbuf **bufs, > !is_same_ether_addr(&bond_mac, &hdr->d_addr)))) { > > if (hdr->ether_type == ether_type_slow_be) { > - bond_mode_8023ad_handle_slow_pkt(internals, slaves[i], > - bufs[j]); > + bond_mode_8023ad_handle_slow_pkt( > + internals, slaves[idx], bufs[j]); > } else > rte_pktmbuf_free(bufs[j]); > > @@ -201,8 +207,11 @@ bond_ethdev_rx_burst_8023ad(void *queue, struct rte_mbuf **bufs, > } else > j++; > } > + if (unlikely(++idx == slave_count)) > + idx = 0; > } > > + internals->active_slave = idx + 1; idx already increased above, won't using "idx + 1" here cause skipping some slaves in some conditions depending num_rx_total? Like, slave_count = 2 internals->active_slave = 0; idx = 0; receive from slave[0] idx = 1; num_rx_total >= nb_pkts internals->active_slave = 2; idx = 2; internals->active_slave = 0; idx = 0; receive from slave[0] idx = 1; num_rx_total >= nb_pkts internals->active_slave = 2; .... Always read from slave[0] as long as it fills num_rx_total threshold, same as previous. OR slave_count = 3 internals->active_slave = 0; idx = 0; receive from slave[0] idx = 1; receive from slave[1] idx = 2; num_rx_total >= nb_pkts internals->active_slave = 3; idx = 3; internals->active_slave = 0; idx = 0; receive from slave[0] idx = 1; receive from slave[1] idx = 2; num_rx_total >= nb_pkts internals->active_slave = 3; .... Not read from slave[2] as long as slave[0 & 1] fills num_rx_total threshold. > return num_rx_total; > } > > diff --git a/drivers/net/bonding/rte_eth_bond_private.h b/drivers/net/bonding/rte_eth_bond_private.h > index 5a411e22b..c8db09005 100644 > --- a/drivers/net/bonding/rte_eth_bond_private.h > +++ b/drivers/net/bonding/rte_eth_bond_private.h > @@ -1,7 +1,7 @@ > /*- > * BSD LICENSE > * > - * Copyright(c) 2010-2015 Intel Corporation. All rights reserved. > + * Copyright(c) 2010-2017 Intel Corporation. All rights reserved. > * All rights reserved. > * > * Redistribution and use in source and binary forms, with or without > @@ -144,6 +144,7 @@ struct bond_dev_private { > uint16_t nb_rx_queues; /**< Total number of rx queues */ > uint16_t nb_tx_queues; /**< Total number of tx queues*/ > > + uint8_t active_slave; /**< Next active_slave to poll */ > uint8_t active_slave_count; /**< Number of active slaves */ > uint8_t active_slaves[RTE_MAX_ETHPORTS]; /**< Active slave list */ > >