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 F300D1B398 for ; Mon, 6 Aug 2018 21:03:51 +0200 (CEST) Received: by mail-io0-f196.google.com with SMTP id n18-v6so2407916ioa.9 for ; Mon, 06 Aug 2018 12:03:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=wUty6eQGkAElA0WHMUPoPlG/nGwL1cJshwpxWGSCbhs=; b=YmCx9cOnkdXspOmpENrPbzIa8qvCu5cCsR0Jzq7V0WDPTlhtqtbWlSXhrTLELa5422 7TM/NboUtMSaDlhSPODVtdxAfuJ11y3yafGy1o4nDPuNC5E7e0pkldAFhsYH2/agTrns uW/lS395QuYjVLypC34Enm+5/hZfJ55CEoPjLJedOECP1SpFfXHZ2weAIYkrys/NG4Aw qoi/KbJ43KUQlB6Z9Ga6p2CtX0TN2fK+8/I2XaoBOPb23tbSrCBRm4LaVXD7taa2vc79 fikDQNGtJVbZz5aE1U4eKbY8EXLnY7mBYlkR4zzUSzjS1xjetof/ztN4Xf7ujoSjLPNk WboQ== 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:from:date :message-id:subject:to:cc; bh=wUty6eQGkAElA0WHMUPoPlG/nGwL1cJshwpxWGSCbhs=; b=DvcwWaSiPStZL4e+jIfU9nAEUvSSEwRmaXh4A2/gb5aYuQRYTRlpfbN5PkNVLOMxhx ZdfYlpszw4H9hEwVr5/Kp0V/F9XwPxrcHW2TB8yFgoByvkMQNFXwW/aXKrJU8Li7nj+V C2ZATpATC0W+NP68rDpUKzHPdwZKeHNz69x090B0UthBpcsqhlUFVrSst35cbh0BqAZs lrnBVYBwykIp+N3/xumUlMb5nxs2uzloFVrvWBnw1PwE1cNQcmjI1YKp/oIg6/gAIi+Z llQlKf1O/2eO8dSb0Cf7R+5g5B9amO1fMwK3RvlE+BFRVXqOx2I5JjF/4qoeQGvI4aLX IBhQ== X-Gm-Message-State: AOUpUlFrdZD3l4Qh2le+jNd/pAq6NV60ZGul2p6if/bJ9nS7Qll5NNkt cUdBxdvrUHALXNSoplZtdwxnlnOqOkds9TFZtK3rYw== X-Google-Smtp-Source: AA+uWPxKv5C1r54bS7WMkgndqipeOWmjQ/LS228rXL0Qqlh+Wpxy630kpyCOkJyutFeqtohZul+9nH2+zE0Qx1ooIpY= X-Received: by 2002:a6b:1e52:: with SMTP id e79-v6mr16467064ioe.110.1533582231336; Mon, 06 Aug 2018 12:03:51 -0700 (PDT) MIME-Version: 1.0 References: <20180801040712.13792-1-3chas3@gmail.com> <039ED4275CED7440929022BC67E706115326261E@SHSMSX103.ccr.corp.intel.com> <039ED4275CED7440929022BC67E7061153262AC1@SHSMSX103.ccr.corp.intel.com> <039ED4275CED7440929022BC67E7061153264042@SHSMSX103.ccr.corp.intel.com> In-Reply-To: <039ED4275CED7440929022BC67E7061153264042@SHSMSX103.ccr.corp.intel.com> From: Chas Williams <3chas3@gmail.com> Date: Mon, 6 Aug 2018 15:03:40 -0400 Message-ID: To: "Zhang, Qi Z" Cc: dev@dpdk.org, beilei.xing@intel.com, Chas Williams Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH] net/i40e: stop lldp before setting local lldp MIB 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, 06 Aug 2018 19:03:52 -0000 On Thu, Aug 2, 2018 at 10:10 PM Zhang, Qi Z wrote: > > > > > *From:* Chas Williams [mailto:3chas3@gmail.com] > *Sent:* Friday, August 3, 2018 5:20 AM > *To:* Zhang, Qi Z > *Cc:* dev@dpdk.org; Xing, Beilei ; Chas Williams < > chas3@att.com> > *Subject:* Re: [PATCH] net/i40e: stop lldp before setting local lldp MIB > > > > > > On Wed, Aug 1, 2018 at 10:16 PM Zhang, Qi Z wrote: > > > > > > *From:* Chas Williams [mailto:3chas3@gmail.com] > *Sent:* Wednesday, August 1, 2018 11:31 PM > *To:* Zhang, Qi Z > *Cc:* dev@dpdk.org; Xing, Beilei ; Chas Williams < > chas3@att.com> > *Subject:* Re: [PATCH] net/i40e: stop lldp before setting local lldp MIB > > > > > > On Wed, Aug 1, 2018 at 10:00 AM Zhang, Qi Z wrote: > > Hi Williams: > > > -----Original Message----- > > From: Chas Williams [mailto:3chas3@gmail.com] > > Sent: Wednesday, August 1, 2018 12:07 PM > > To: dev@dpdk.org > > Cc: Xing, Beilei ; Zhang, Qi Z < > qi.z.zhang@intel.com>; > > Charles (Chas) Williams > > Subject: [PATCH] net/i40e: stop lldp before setting local lldp MIB > > > > From: "Charles (Chas) Williams" > > > > From the Intel Ethernet Controller X710/XXV710/XL710 Specifiction > > Update: > > > > Starting from NVM 5.02, if the Set Local LLDP MIB command is > > received while the DCBx specific agent is stopped, the command > > returns an EPERM error. If the command is received while the > > LLDP agent is stopped, it sets the local MIB without exchanging > > LLDP with peer, and returns SUCCESS. > > > > This results in the harmless, but annoying, diagnostic: > > > > default dcb config fails. err =3D -53, aq_err =3D 1. > > > > So, always stop the lldp daemon when we are in software mode before we > > attempt to call i40e_set_dcb_config. > > > > Signed-off-by: Chas Williams > > --- > > drivers/net/i40e/i40e_ethdev.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/net/i40e/i40e_ethdev.c > b/drivers/net/i40e/i40e_ethdev.c > > index a340540ef..03bedf5c1 100644 > > --- a/drivers/net/i40e/i40e_ethdev.c > > +++ b/drivers/net/i40e/i40e_ethdev.c > > @@ -11237,6 +11237,7 @@ i40e_dcb_init_configure(struct rte_eth_dev > > *dev, bool sw_dcb) > > * i40e_init_dcb we expect is failure with I40E_AQ_RC_EPE= RM > > * adminq status. Otherwise, it should return success. > > */ > > + i40e_aq_stop_lldp(hw, TRUE, NULL); > > I40e_aq_stop_lldp is intended to be removed with below two patches. > > commit c6431c891d9e9691e3205fe5c5350071cbaeb852 > commit fcbd40d4327b36725c4de9f33f57809edc359f4a > > > > Yeah about that. The i40e driver seems to go out of its way to program > the flow > > control outside of LLDP see i40e_update_flow_control() > and i40e_flow_ctrl_set(). > > So it's not clear to me what is going on here with DPDK and LLDP. Do you > really > > want LLDP running in this mode? The other branch > in i40e_dcb_init_configure() > > explicitly starts LLDP, so I assume at some point, LLDP wasn't/isn't > running. How did > > it get this way? With respect to item #70, shouldn't you therefore alway= s > start > > LLDP to workaround the bug? > > > > > > LLDP is enabled by firmware as default, from my view it is not necessary > to start LLDP, if We never stop it, and maybe we should not allow the ca= se > sw_dcb =3D TRUE since we can=E2=80=99t stop LLDP. > > > > Item#70 is fixed in later NVM's (6.01 or later). Perhaps a check to see > if the NVM > > is new enough to safely allow LLDP stopping? > > > > Yes, I agree, would you mind to add a firmware check for lldp_start and > lldp_stop in v2? So that fix your concern in the case with a new firmware > > For old firmware case, let=E2=80=99s just keep exist way , there should b= e some > re-work in future, and more test and review is required. > > > > Yes, I can probably manage to add a firmware check. I will need a little > bit to test it. > > > > [Qi] OK, just a reminder, this change will be deferred to v18.11. > > > > One other question. What's up with i40e_update_flow_control()? Should > this be called whenever the link state changes? Right now it only seems = to > be called when the device is started/initialized. > > > > [Qi] I=E2=80=99m not very sure if the flow control mode will be changed d= uring > LSC, Did you see some real issue that related with it? > There's no reason to believe that the other end wouldn't change the status of flow control. It just seems like this might be something you would want to do more often than device init/reset? > > > > > > > Thanks > > Qi > > > > > > > Regards. > Qi > > > > if ((ret =3D=3D I40E_SUCCESS) || (ret !=3D I40E_SUCCESS &= & > > hw->aq.asq_last_status =3D=3D I40E_AQ_RC_EPERM)) { > > memset(&hw->local_dcbx_config, 0, > > -- > > 2.14.4 > >