From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <3chas3@gmail.com> Received: from mail-qt1-f193.google.com (mail-qt1-f193.google.com [209.85.160.193]) by dpdk.org (Postfix) with ESMTP id 4C2721B8FF; Sat, 9 Feb 2019 14:17:20 +0100 (CET) Received: by mail-qt1-f193.google.com with SMTP id r9so7143938qtt.3; Sat, 09 Feb 2019 05:17:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=NLI1p/AYg8kbsDUhQ9DsVMWqQaeL4pvzqYMH4DnWCMU=; b=DA2DP1Tuvq6gUY/Zum4dUCZYqDn1X0aE5vujj8b7jw69qHHX2n2ZCIEXB/Gn5HAAE/ +ziBPCVF/07u2Wbq6s2ahBjuxZdUOQuN/JsQPqjQl5Af+oSVKLMQs254tdoNScIEMvYc 5DBDOaj3fxaw7q/u0LWZ1pPSskRTyeomYXUPrtm4HsGTZDLFQSmsizsi2+2Q7GYVm/Ga 0860xe99E4apdxS5xckrLB2xfsbBouT9053dXnZqsXP3U8CqWGMOC9SvXQww20za/Dot c2oMh5crOrHE2Y6qC4F9li78cm6ORNkhZhX4EAXvVFiAH6LkQjTIY9Fza0H6Fb6A5lOP 02CQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=NLI1p/AYg8kbsDUhQ9DsVMWqQaeL4pvzqYMH4DnWCMU=; b=AhERL+kb4YICf8V390Y0xUhzpo2LgADPZqdowtADT5uHosaIsip3oBLJ6lxMZ96qpa 4olOUbVHl3MTpCAqShlXHmvy4dCJLKVAB8Um1iYLdwZCG6xDHrSmLaBMQy+nYaWVDVQL uBi1oZzdT3kBYOfp7LBj5uijUM3iSOguVZALKWDqjQgx0kVAQnDdFm2DGFzbGwrwy0LX c2w81J3F3MLJHygw+GSOZ+C28D/CyEOj3KlPnLXfAEHdbA0N+JBqxVy3xHDf0QJpIURm 7tPBtiffoNcXWaBcr46i2gUFJ82KmghKlrKJ1QoZKsYOO1AqlPqcnA7qh3tMZaW/bKae 4Njg== X-Gm-Message-State: AHQUAuZM/JvqH/n2u7+5z6y/D/Vbc46EyliPoSH8FKziATGfo8wfcw2l UntB/iGHvdyQCnDn8MNSOfd8wpze X-Google-Smtp-Source: AHgI3IZLkHl+PGQS4IwdqPnrIJNkgUDkOTNz0ow8JMhmZ/8w611VqCn+uA/onmhY8blQ/MxMrth4Lg== X-Received: by 2002:a0c:d947:: with SMTP id t7mr518630qvj.49.1549718239642; Sat, 09 Feb 2019 05:17:19 -0800 (PST) Received: from [192.168.1.10] (pool-96-255-82-34.washdc.fios.verizon.net. [96.255.82.34]) by smtp.gmail.com with ESMTPSA id t64sm4444905qkf.28.2019.02.09.05.17.18 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 09 Feb 2019 05:17:19 -0800 (PST) To: Hyong Youb Kim , Ferruh Yigit , Declan Doherty , Chas Williams Cc: dev@dpdk.org, stable@dpdk.org References: <20190110102235.1238-1-hyonkim@cisco.com> <20190110102235.1238-3-hyonkim@cisco.com> From: Chas Williams <3chas3@gmail.com> Message-ID: Date: Sat, 9 Feb 2019 08:17:18 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: <20190110102235.1238-3-hyonkim@cisco.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 2/2] net/bonding: avoid the next active slave going out of bound 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: Sat, 09 Feb 2019 13:17:20 -0000 On 1/10/19 5:22 AM, Hyong Youb Kim wrote: > For bonding modes like broadcast that use bond_ethdev_rx_burst(), it > is fairly easy to produce a crash simply by bringing a slave port's > link down. When slave links go down, the driver on one thread reduces > active_slave_count via the LSC callback and deactivate_slave(). At the > same time, bond_ethdev_rx_burst() running on a forwarding thread may > increment active_slave (next active slave) beyond > active_slave_count. Here is a typical sequence of events. > > At time 0: > active_slave_count = 3 > active_slave = 2 > > At time 1: > A slave link goes down. > Thread 0 (main) reduces active_slave_count to 2. > > At time 2: > Thread 1 (forwarding) executes bond_ethdev_rx_burst(). > - Reads active_slave_count = 2. > - Increments active_slave at the end to 3. > > From this point on, everytime bond_ethdev_rx_burst() runs, > active_slave increments by one, eventually going well out of bound of > the active_slaves array and causing a crash. > > Make the rx burst function to first check that active_slave is within > bound. If not, reset it to 0 to avoid out-of-range array access. > > Fixes: e1110e977648 ("net/bonding: fix Rx slave fairness") > Cc: stable@dpdk.org > > Signed-off-by: Hyong Youb Kim Acked-by: Chas Williams > --- > drivers/net/bonding/rte_eth_bond_pmd.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c > index daf2440cd..bc2405e54 100644 > --- a/drivers/net/bonding/rte_eth_bond_pmd.c > +++ b/drivers/net/bonding/rte_eth_bond_pmd.c > @@ -68,6 +68,15 @@ bond_ethdev_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) > internals = bd_rx_q->dev_private; > slave_count = internals->active_slave_count; > active_slave = internals->active_slave; > + /* > + * Reset the active slave index, in case active_slave goes out > + * of bound. It can hapen when slave links go down, and > + * another thread (LSC callback) shrinks the slave count. > + */ > + if (active_slave >= slave_count) { > + internals->active_slave = 0; > + active_slave = 0; > + } > > for (i = 0; i < slave_count && nb_pkts; i++) { > uint16_t num_rx_slave; > @@ -273,6 +282,11 @@ bond_ethdev_rx_burst_8023ad_fast_queue(void *queue, struct rte_mbuf **bufs, > active_slave = internals->active_slave; > memcpy(slaves, internals->active_slaves, > sizeof(internals->active_slaves[0]) * slave_count); > + /* active_slave may go out of bound. See bond_ethdev_rx_burst() */ > + if (active_slave >= slave_count) { > + internals->active_slave = 0; > + active_slave = 0; > + } > > for (i = 0; i < slave_count && nb_pkts; i++) { > uint16_t num_rx_slave; >