netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexander Duyck <alexander.duyck@gmail.com>
To: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>,
	Netdev <netdev@vger.kernel.org>
Subject: Re: [PATCH 2/2] ixgbe: Set rss key in order.
Date: Tue, 23 Jan 2018 08:24:57 -0800	[thread overview]
Message-ID: <CAKgT0Uf-=nAO-2RcVX1zOsEkA6_g=0noVtopeyNPA3GRMmiF7Q@mail.gmail.com> (raw)
In-Reply-To: <1516721041-82282-2-git-send-email-xiangxia.m.yue@gmail.com>

On Tue, Jan 23, 2018 at 7:24 AM, Tonghao Zhang <xiangxia.m.yue@gmail.com> wrote:
> If we use ethtool to set rss key, for example,
> we except 0x6d is K[0], and 0x5a is K[1]. But
> the ixgbe driver set the 0xda is K[0].
>
> ethtool -X eth0 hkey 6d:5a:56:da:25:5b:0e:c2:41:67:25:3d
>         :43:a3:8f:b0:d0:ca:2b:cb:ae:7b:30:b4:77:cb:2d:a3
>         :80:30:f2:0c:6a:42:b7:3b:be:ac:01:fa
>
> The key is the original Microsoft's key.
>
> 7.1.2.8.1 RSS Hash Function: (82599 datasheet)
> Given an array K with k bytes, our nomenclature assumes
> that the array is laid out as follows:
>
> K[0] K[1] K[2] ... K[k-1]
>
> K[0] is the left-most byte, and the MSB of K[0] is the
> left-most bit. K[k-1] is the right-most byte, and the
> LSB of K[k-1] is the right-most bit.
>
> 8.2.3.7.14 RSS Random Key Register:
> The K[0] is the 7:0 bit of RSSRK (RSS Random Key Register).
>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 95aba97..0ca2e0e 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -3777,10 +3777,18 @@ u32 ixgbe_rss_indir_tbl_entries(struct ixgbe_adapter *adapter)
>  void ixgbe_store_key(struct ixgbe_adapter *adapter)
>  {
>         struct ixgbe_hw *hw = &adapter->hw;
> -       int i;
> +       u8 *rss_key = (u8 *)adapter->rss_key;
> +       u32 key, i;
>
> -       for (i = 0; i < 10; i++)
> -               IXGBE_WRITE_REG(hw, IXGBE_RSSRK(i), adapter->rss_key[i]);
> +       /* Fill out hash function seeds */
> +       for (i = 0; i < 10; i++) {
> +               key  = rss_key[(i * 4)];
> +               key |= rss_key[(i * 4) + 1] << 8;
> +               key |= rss_key[(i * 4) + 2] << 16;
> +               key |= rss_key[(i * 4) + 3] << 24;
> +
> +               IXGBE_WRITE_REG(hw, IXGBE_RSSRK(i), key);
> +       }
>  }
>
>  /**

I would argue the issue here isn't in ixgbe_store_key. The problem
appears to be in ixgbe_set_rxfh and ixgbe_get_rxfh. It seems the key
provided should be populated as if it were a __be32 array, not a u32
array. So doing a memcpy from the u8 to u32 isn't correct in this
case. Really what should probably be happening is the population of
rss_key should be done using a for loop with be32_to_cpu calls, and
when we display it we should be using cpu_to_be32 to translate it
back.

An alternative would be to convert the rss_key pointer in the adapter
struct to be a be32 array pointer in the first place, and then change
the key population code to use be32_to_cpup in order to do the correct
byte swapping before writing the the value to the register.

Also as pointed out by Jeff please submit this to the intel-wired-lan
mailing list next time.

Thanks.

- Alex

  reply	other threads:[~2018-01-23 16:24 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-23 15:24 [PATCH 1/2] ixgbe: Avoid to write the RETA table when unnecessary Tonghao Zhang
2018-01-23 15:24 ` [PATCH 2/2] ixgbe: Set rss key in order Tonghao Zhang
2018-01-23 16:24   ` Alexander Duyck [this message]
2018-01-23 15:28 ` [PATCH 1/2] ixgbe: Avoid to write the RETA table when unnecessary Jeff Kirsher

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='CAKgT0Uf-=nAO-2RcVX1zOsEkA6_g=0noVtopeyNPA3GRMmiF7Q@mail.gmail.com' \
    --to=alexander.duyck@gmail.com \
    --cc=jeffrey.t.kirsher@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=xiangxia.m.yue@gmail.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).