From: "Zhang, Qi Z" <qi.z.zhang@intel.com>
To: Chas Williams <3chas3@gmail.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
"Xing, Beilei" <beilei.xing@intel.com>,
Chas Williams <chas3@att.com>
Subject: Re: [dpdk-dev] [PATCH] net/i40e: stop lldp before setting local lldp MIB
Date: Thu, 2 Aug 2018 02:16:24 +0000 [thread overview]
Message-ID: <039ED4275CED7440929022BC67E7061153262AC1@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <CAG2-GkmkqWPCdyxYiJ9cGQd_Rd_OMxKekXhkv5F=eK25k3HTgg@mail.gmail.com>
From: Chas Williams [mailto:3chas3@gmail.com]
Sent: Wednesday, August 1, 2018 11:31 PM
To: Zhang, Qi Z <qi.z.zhang@intel.com>
Cc: dev@dpdk.org; Xing, Beilei <beilei.xing@intel.com>; 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 <qi.z.zhang@intel.com<mailto:qi.z.zhang@intel.com>> wrote:
Hi Williams:
> -----Original Message-----
> From: Chas Williams [mailto:3chas3@gmail.com<mailto:3chas3@gmail.com>]
> Sent: Wednesday, August 1, 2018 12:07 PM
> To: dev@dpdk.org<mailto:dev@dpdk.org>
> Cc: Xing, Beilei <beilei.xing@intel.com<mailto:beilei.xing@intel.com>>; Zhang, Qi Z <qi.z.zhang@intel.com<mailto:qi.z.zhang@intel.com>>;
> Charles (Chas) Williams <chas3@att.com<mailto:chas3@att.com>>
> Subject: [PATCH] net/i40e: stop lldp before setting local lldp MIB
>
> From: "Charles (Chas) Williams" <chas3@att.com<mailto:chas3@att.com>>
>
> 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 = -53, aq_err = 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 <chas3@att.com<mailto:chas3@att.com>>
> ---
> 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_EPERM
> * 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 always 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 case sw_dcb = TRUE since we can’t 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’s just keep exist way , there should be some re-work in future, and more test and review is required.
Thanks
Qi
Regards.
Qi
> if ((ret == I40E_SUCCESS) || (ret != I40E_SUCCESS &&
> hw->aq.asq_last_status == I40E_AQ_RC_EPERM)) {
> memset(&hw->local_dcbx_config, 0,
> --
> 2.14.4
next prev parent reply other threads:[~2018-08-02 2:16 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-01 4:07 Chas Williams
2018-08-01 13:59 ` Zhang, Qi Z
2018-08-01 15:31 ` Chas Williams
2018-08-02 2:16 ` Zhang, Qi Z [this message]
2018-08-02 21:19 ` Chas Williams
2018-08-03 2:10 ` Zhang, Qi Z
2018-08-06 19:03 ` Chas Williams
2018-08-06 20:05 ` [dpdk-dev] [PATCH v2] " Chas Williams
2018-09-13 12:30 ` Zhang, Qi Z
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=039ED4275CED7440929022BC67E7061153262AC1@SHSMSX103.ccr.corp.intel.com \
--to=qi.z.zhang@intel.com \
--cc=3chas3@gmail.com \
--cc=beilei.xing@intel.com \
--cc=chas3@att.com \
--cc=dev@dpdk.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).