From: Kevin Traynor <ktraynor@redhat.com>
To: Qi Zhang <qi.z.zhang@intel.com>, jingjing.wu@intel.com
Cc: dev@dpdk.org, stable@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] net/i40e: fix flow control watermark mismatch
Date: Thu, 10 Aug 2017 16:15:32 +0100 [thread overview]
Message-ID: <adbf5925-dba2-fef9-6aa0-4aec613a5c85@redhat.com> (raw)
In-Reply-To: <20170810104807.59436-1-qi.z.zhang@intel.com>
On 08/10/2017 11:48 AM, Qi Zhang wrote:
> Flow control watermark is not read out correctly,
> that may cause an application who not intend to change
> watermark but does change it with a rte_eth_dev_flow_ctrl_set
> call right after rte_eth_dev_flow_ctrl_get.
>
> Fixes: f53577f06925 ("i40e: support flow control")
> Cc: stable@dpdk.org
>
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> ---
> drivers/net/i40e/i40e_ethdev.c | 19 +++++++++++++------
> 1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 71cb7d3..8b008c4 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -86,12 +86,6 @@
> /* Flow control default timer */
> #define I40E_DEFAULT_PAUSE_TIME 0xFFFFU
>
> -/* Flow control default high water */
> -#define I40E_DEFAULT_HIGH_WATER (0x1C40/1024)
> -
> -/* Flow control default low water */
> -#define I40E_DEFAULT_LOW_WATER (0x1A40/1024)
> -
> /* Flow control enable fwd bit */
> #define I40E_PRTMAC_FWD_CTRL 0x00000001
>
> @@ -101,6 +95,12 @@
> /* Kilobytes shift */
> #define I40E_KILOSHIFT 10
>
> +/* Flow control default high water */
> +#define I40E_DEFAULT_HIGH_WATER (0xF2000 >> I40E_KILOSHIFT)
> +
> +/* Flow control default low water */
> +#define I40E_DEFAULT_LOW_WATER (0xF2000 >> I40E_KILOSHIFT)
> +
I'm not sure these are needed anymore. Would seem if you want a correct
value prior to flow_ctl_get() being called, the registers should also be
read during init. Is there a need for the PMD to have the correct value
prior to flow_ctrl_get() ? I didn't see one.
> /* Receive Average Packet Size in Byte*/
> #define I40E_PACKET_AVERAGE_SIZE 128
>
> @@ -3199,6 +3199,13 @@ i40e_flow_ctrl_get(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
> struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
>
> fc_conf->pause_time = pf->fc_conf.pause_time;
> +
> + /* read out from register, in case they are modified by other port */
> + pf->fc_conf.high_water[I40E_MAX_TRAFFIC_CLASS] =
> + I40E_READ_REG(hw, I40E_GLRPB_GHW) >> I40E_KILOSHIFT;
> + pf->fc_conf.low_water[I40E_MAX_TRAFFIC_CLASS] =
> + I40E_READ_REG(hw, I40E_GLRPB_GLW) >> I40E_KILOSHIFT;
> +
> fc_conf->high_water = pf->fc_conf.high_water[I40E_MAX_TRAFFIC_CLASS];
> fc_conf->low_water = pf->fc_conf.low_water[I40E_MAX_TRAFFIC_CLASS];
>
>
next prev parent reply other threads:[~2017-08-10 15:15 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-10 10:48 Qi Zhang
2017-08-10 15:15 ` Kevin Traynor [this message]
2017-08-13 0:05 ` Zhang, Qi Z
2017-08-15 17:40 ` Kevin Traynor
2017-08-18 14:35 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
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=adbf5925-dba2-fef9-6aa0-4aec613a5c85@redhat.com \
--to=ktraynor@redhat.com \
--cc=dev@dpdk.org \
--cc=jingjing.wu@intel.com \
--cc=qi.z.zhang@intel.com \
--cc=stable@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).