From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: "Cui, LunyuanX" <lunyuanx.cui@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: "Lu, Wenzhuo" <wenzhuo.lu@intel.com>,
"Yang, Qiming" <qiming.yang@intel.com>,
Ananyev <IMCEAINVALID-Ananyev@namprd11.prod.outlook.com>,
"stable@dpdk.org" <stable@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: fixed port can not link up in FreeBSD
Date: Fri, 13 Dec 2019 14:32:33 +0000 [thread overview]
Message-ID: <SN6PR11MB255896BA82493A4F94E54F469A540@SN6PR11MB2558.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20191213023621.38787-1-lunyuanx.cui@intel.com>
Few small nits below.
s/fixed/fix/
> In FreeBSD environment, nic_uio drivers do not support interrupts,
> rte_intr_callback_register() will fail to register interrupts.
> We can not make link status to change from down to up by interrupt
> callback. So we need to wait for the controller to acquire link
> when ports start. Through multiple tests, 5s should be enough.
>
> Fixes: b9bd0f09fa15 ("ethdev: fix link status query")
> Cc: stable@dpdk.org
>
> Signed-off-by: Lunyuan Cui <lunyuanx.cui@intel.com>
> ---
>
> v2 changes:
> * Put waiting into a separate function to keep start() code clean.
> ---
> drivers/net/ixgbe/ixgbe_ethdev.c | 32 ++++++++++++++++++++++++++++++++
> drivers/net/ixgbe/ixgbe_ethdev.h | 3 +++
> 2 files changed, 35 insertions(+)
>
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 2c6fd0f13..fba666186 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -2801,6 +2801,13 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
> "please call hierarchy_commit() "
> "before starting the port");
>
> +#ifdef RTE_EXEC_ENV_FREEBSD
> + /* wait for the controller to acquire link */
> + err = ixgbe_wait_for_link_up(hw);
> + if (err)
> + goto error;
> +#endif
You can hide ifdef inside the function, see below.
> +
> /*
> * Update link status right before return, because it may
> * start link configuration process in a separate thread.
> @@ -4114,6 +4121,31 @@ ixgbe_dev_setup_link_alarm_handler(void *param)
> intr->flags &= ~IXGBE_FLAG_NEED_LINK_CONFIG;
> }
>
> +#ifdef RTE_EXEC_ENV_FREEBSD
> +/*
> + * In freebsd environment, nic_uio drivers do not support interrupts,
> + * rte_intr_callback_register() will fail to register interrupts.
> + * We can not make link status to change from down to up by interrupt
> + * callback. So we need to wait for the controller to acquire link
> + * when ports start.
> + * It returns 0 on link up.
> + */
> +int32_t ixgbe_wait_for_link_up(struct ixgbe_hw *hw)
This function can be static, plus pls try to follow dpdk codying style:
static int
ixgbe_wait_for_link_up(struct ixgbe_hw *hw)
{
....
You can put #ifdef inside the function:
#ifdef RTE_EXEC_ENV_FREEBSD
<do something meaningfull here>
#else
RTE_SET_USED(hw);
#endif
Or even:
#ifdef RTE_EXEC_ENV_FREEBSD
const uint32_t nb_iter = 25;
#else
const uint32_t nb_iter = 0;
#endif
...
for (i = 0; i < nb_iter; i++) {
...
> +{
> + int err, i, link_up = 0;
> + uint32_t speed = 0;
Empty line here would help readability.
> + for (i = 0; i < 25; i++) {
> + err = ixgbe_check_link(hw, &speed, &link_up, 0);
> + if (err)
> + return err;
> + if (link_up)
> + return 0;
> + msec_delay(200);
> + }
> + return 0;
> +}
> +#endif
> +
> /* return 0 means link status changed, -1 means not changed */
> int
> ixgbe_dev_link_update_share(struct rte_eth_dev *dev,
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
> index 76a1b9d18..9a1d8c54c 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.h
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> @@ -700,6 +700,9 @@ int
> ixgbe_dev_link_update_share(struct rte_eth_dev *dev,
> int wait_to_complete, int vf);
>
> +#ifdef RTE_EXEC_ENV_FREEBSD
> +int32_t ixgbe_wait_for_link_up(struct ixgbe_hw *hw);
> +#endif
Pls see above, that function can be static.
> /*
> * misc function prototypes
> */
> --
> 2.17.1
next prev parent reply other threads:[~2019-12-13 14:32 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-12 11:17 [dpdk-dev] [PATCH] " Lunyuan Cui
2019-12-12 11:15 ` Ananyev, Konstantin
2019-12-13 2:36 ` [dpdk-dev] [PATCH v2] " Lunyuan Cui
2019-12-13 14:32 ` Ananyev, Konstantin [this message]
2019-12-16 2:24 ` [dpdk-dev] [PATCH v3] net/ixgbe: fix " Lunyuan Cui
2019-12-18 2:47 ` Ye Xiaolong
2019-12-18 10:21 ` Ananyev, Konstantin
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=SN6PR11MB255896BA82493A4F94E54F469A540@SN6PR11MB2558.namprd11.prod.outlook.com \
--to=konstantin.ananyev@intel.com \
--cc=IMCEAINVALID-Ananyev@namprd11.prod.outlook.com \
--cc=dev@dpdk.org \
--cc=lunyuanx.cui@intel.com \
--cc=qiming.yang@intel.com \
--cc=stable@dpdk.org \
--cc=wenzhuo.lu@intel.com \
/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).