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 DAE771B6DD for ; Wed, 6 Jun 2018 16:28:14 +0200 (CEST) Received: by mail-it0-f68.google.com with SMTP id v83-v6so8287689itc.3 for ; Wed, 06 Jun 2018 07:28:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=QZaIvuc6egplYZaKBSTfzQgrlwem2WOghjxAg+I413o=; b=gJv/+TrGVB0TbPMMcOQf03zEfkTBWqdODjk74pUIWagkXACpW6GGC9XohFz6/vt90V 6KpvFJuFgjdXLhgSC+Cg/JAIjw1Yu/EBlw+fg+DI0BsiYjCR71gCrVqQfoXlGqyWJPBN 6VBxSTu0XOQwpADlMzMD6Wyj34viJQ1AeJXrN/mPame5mRhcsm96H5MoP0sXJKi77VnW oexYP5u6PjwuIm8ksC/Tb7zYG+Hegj+8HUhYLKFvs4xPPn+1I/KDMsW7tqbyWcNg3pGG gui9hJ5/+JoX73euroj1hV/FcO9BtT+c3cwkKa3QYkOoew0lhICJPFJzYQJqg5liSiie hEYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=QZaIvuc6egplYZaKBSTfzQgrlwem2WOghjxAg+I413o=; b=XbUjaj/b+FsUhvjl1gCb8kl65sg2O/TQj3APei0MUS6s9+dorGopW6TfpbkhAqxBZD 3PtDunJG4vWGO37EDfp4caiBqj1uiQplw2r0mKgwmeTMTYLRga3f45zVHSRrZTyjb47o QlnMgus3si8fnBb8At5wVR68bGImFrAFbi24iJ3B3gHYx6WMORvcxaCiqYwE8zM1K5+G B0a1MqqOuiowCHJmUWRPOF2eCK/CP6cmFU9pzEliAZHR6F5+Ozr+hsGE7XSgBB8jcXJ2 EbQ3mllgxu25siekoxummQ4glW19ui1e2a5bwZVO7+TyrFRcSQffTBBvre3zS8lY1Ji8 GAYA== X-Gm-Message-State: APt69E3RCtLYYypZ6t/ggl9ZlUxW4RKJBSKIQbaiacBsnJ55jlQbgO/C XXDw9+O8hmEczKpXik064BVdm52/mFC6mwlVbsw= X-Google-Smtp-Source: ADUXVKKiF/Eq/BHgV5Q8hvusS61BJC5nj0nbr9ctBFJxgljLRZmzkaIwE0f5CmVoZWHbeHQy914GJKjFbQX/R1+vNdg= X-Received: by 2002:a24:8b81:: with SMTP id g123-v6mr2877435ite.67.1528295294253; Wed, 06 Jun 2018 07:28:14 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:e90e:0:0:0:0:0 with HTTP; Wed, 6 Jun 2018 07:28:12 -0700 (PDT) In-Reply-To: References: <20180606122627.18418-1-3chas3@gmail.com> From: Chas Williams <3chas3@gmail.com> Date: Wed, 6 Jun 2018 10:28:12 -0400 Message-ID: To: Matan Azrad Cc: "dev@dpdk.org" , "declan.doherty@intel.com" , "Charles (Chas) Williams" Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH] net/bonding: don't clear active slave count 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: Wed, 06 Jun 2018 14:28:15 -0000 On Wed, Jun 6, 2018 at 9:57 AM, Matan Azrad wrote: > Hi Chas > > From: Chas Williams > > From: "Charles (Chas) Williams" > > > > When the bond PMD is stopped, the active slave count is reset. > > For 802.3ad mode this potentially leaks memory and clears state since a > second > > sequential activate_slave() will occur when the bond PMD is restarted > and the > > LSC callback is triggered while the active slave count is 0. To fix > this, don't clear > > the active slave count when stopping. Only deactivate_slave() should be > used to > > clear the slaves. > > > > Looks like it is a fix, so need fix title, CC stable and fixes line, no? > Yes, I forgot to add those. Let's call it an RFC for now. It's not completely clear to me what is the right thing to do here. The activate state of slaves is really dependent on the link status of the slaves. Does a PMD have a link status outside of a start/stop sequence? Or, since we clear the last_status on stop, the start will just get all the activate/deactivate states corrected. Since we might stop/start for a short interval, it seems like we might not want to break what 802.3ad has negotiated? > > > Signed-off-by: Chas Williams > > --- > > drivers/net/bonding/rte_eth_bond_pmd.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c > > b/drivers/net/bonding/rte_eth_bond_pmd.c > > index 02d94b1b1..4ae577078 100644 > > --- a/drivers/net/bonding/rte_eth_bond_pmd.c > > +++ b/drivers/net/bonding/rte_eth_bond_pmd.c > > @@ -2173,7 +2173,6 @@ bond_ethdev_stop(struct rte_eth_dev *eth_dev) > > tlb_last_obytets[internals->active_slaves[i]] = 0; > > } > > > > - internals->active_slave_count = 0; > > But why not to call deactivate_slave() for all the active slaves? > > > internals->link_status_polling_enabled = 0; > > for (i = 0; i < internals->slave_count; i++) > > internals->slaves[i].last_link_status = 0; > > -- > > 2.14.3 > > >