From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <3chas3@gmail.com> Received: from mail-io0-f196.google.com (mail-io0-f196.google.com [209.85.223.196]) by dpdk.org (Postfix) with ESMTP id D65C2D018 for ; Mon, 16 Apr 2018 18:44:30 +0200 (CEST) Received: by mail-io0-f196.google.com with SMTP id s14so2096784ioc.0 for ; Mon, 16 Apr 2018 09:44:30 -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=9aSaFYF/d+HljEXYaxsc7hdsEs52glwX5KwEtfIVGUc=; b=kwDU5BnvyvKPzae+prOWmLRzVNd0JX402B8WHCBPh6eukI5PmTfhpKtsQxc3X+i+vn k+81Mdpdz1OfjF6sNq1BGIdcpBiOMpUQgrtTIej+gN0bjaDbenys+qS+xrhYEHyT83am pa5rBk+BEdJlEaORd8rlrE5Z8Axg/at5ZL6yF8kmWSLkYQZRGXPVTo5BDLMqPyO9UHl5 dmFc1ZXlN9kRMsxR0sVOO0D8gpjqek00zj7EGRPhgNGoOvsxQmP6mPw9WstSiFHBLZS3 BZAMkTMU4lUgbhNVouJM2WendX+9pyrbWDqHbqmt+fbC4XkNNC/wnfSkobLSyP+EaiR/ EHTQ== 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=9aSaFYF/d+HljEXYaxsc7hdsEs52glwX5KwEtfIVGUc=; b=NPju44EKUFrHDodCFaKybBAfFoiUwJVLybYTrJYBNnlrbI29K5L5ZcW+01/HPfMhFE vmEo6qtanb0ARVE6baEZc4L4qYJqJGGr441DWOeE6brNFnG5zSQqT0f+hYUvgbGlcCzK HVLK5FQg9hxbofbSx9n1TdxinWLB+OPil7S5DFC/6ZWnc/jzy9pHSrGInWhbU+1Yx8Nh SJC6LYnrGnOGxHdtqLXq1IHSPfEdwbdxCqpxJTM8Ez9mQq9OcuWBQZcYOFDyqQbNPXqe cD6EgMnV70k3qXcgnUDbYOt+ntuJ+fRm4ZZKmepntHXcj3OtUObqtPO+w+czzZRH29sq YkBQ== X-Gm-Message-State: ALQs6tCZz0P0jnO+lmjYdMg63t5Xjnv8lPtZkXi/+5+7rsNVMR4KT/2G ej0als233fy0boomnj5ARKszhSpVkiS2JqCeWpI= X-Google-Smtp-Source: AIpwx4+xbyqnjmPlfXmZzanVUqOStpVOSzUIMiYtLhOqGMPdZ1d8Psr6+YWkrJ0TyWmG8aC+LeCud+yQa17CP4zNYO4= X-Received: by 10.107.146.86 with SMTP id u83mr6677558iod.110.1523897070203; Mon, 16 Apr 2018 09:44:30 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.143.22 with HTTP; Mon, 16 Apr 2018 09:44:29 -0700 (PDT) In-Reply-To: References: <20180213225430.15556-1-3chas3@gmail.com> From: Chas Williams <3chas3@gmail.com> Date: Mon, 16 Apr 2018 12:44:29 -0400 Message-ID: To: Matan Azrad Cc: "dev@dpdk.org" , "declan.doherty@intel.com" , Chas Williams Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH] net/bonding: fix link properties with autoneg 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: Mon, 16 Apr 2018 16:44:31 -0000 On Mon, Apr 16, 2018 at 4:06 AM, Matan Azrad wrote: > Hi Chas > > From: Chas Williams, Wednesday, February 14, 2018 12:55 AM >> If a link is carrier down and using autonegotiation, then the PMD may not >> have detected a speed yet. In this case the best we can do is ignore the link >> speed and duplex since they aren't valid. > > Ok for this. > >> To be completely correct, there >> should be additional checks to prevent a slave that negotiates a different >> speed from being activated. > > Looks like every changing in the link properties should cause LSC interrupt. > In the bonding LCS interrupt you could handle and to deactivate the device. > Also you should deal with the case of the first slave, what is happen if the first slave has invalid link properties? > How can you know that the speed\duplex_mode is invalid? > Are we sure LACP mode can run with auto negotiation? Yes, I am pretty sure bonding doesn't get this right when the interfaces aren't link up. While what bonding is doing is likely wrong, it doesn't mean that the behavior of the PMDs are correct in leaving the link_status unset until the first LSC interrupt. I plan to get around to looking at this bonding problem in a little bit. Luckily it seems that we always tend to get matched links and even if bonding is advertising the wrong aggregate speed and duplex we are find for now. It wouldn't pass close inspection by a protocol analyzer though. >> >> Signed-off-by: Chas Williams >> --- >> drivers/net/bonding/rte_eth_bond_pmd.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c >> b/drivers/net/bonding/rte_eth_bond_pmd.c >> index 92ad688..5559879 100644 >> --- a/drivers/net/bonding/rte_eth_bond_pmd.c >> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c >> @@ -1545,9 +1545,10 @@ link_properties_valid(struct rte_eth_dev >> *ethdev, >> if (bond_ctx->mode == BONDING_MODE_8023AD) { >> struct rte_eth_link *bond_link = &bond_ctx- >> >mode4.slave_link; >> >> - if (bond_link->link_duplex != slave_link->link_duplex || >> - bond_link->link_autoneg != slave_link->link_autoneg >> || >> - bond_link->link_speed != slave_link->link_speed) >> + if (bond_link->link_autoneg != slave_link->link_autoneg || >> + (bond_link->link_autoneg != ETH_LINK_AUTONEG && >> + (bond_link->link_duplex != slave_link->link_duplex || >> + bond_link->link_speed != slave_link->link_speed))) >> return -1; >> } >> >> -- >> 2.9.5 >