netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Wei Fang <wei.fang@nxp.com>
Cc: claudiu.manoil@nxp.com, vladimir.oltean@nxp.com,
	xiaoning.wang@nxp.com, andrew+netdev@lunn.ch,
	davem@davemloft.net, edumazet@google.com, pabeni@redhat.com,
	christophe.leroy@csgroup.eu, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org, imx@lists.linux.dev
Subject: Re: [PATCH v2 net-next 07/13] net: enetc: add RSS support for i.MX95 ENETC PF
Date: Wed, 15 Jan 2025 14:00:42 -0800	[thread overview]
Message-ID: <20250115140042.63b99c4f@kernel.org> (raw)
In-Reply-To: <20250113082245.2332775-8-wei.fang@nxp.com>

On Mon, 13 Jan 2025 16:22:39 +0800 Wei Fang wrote:
> Add Receive side scaling (RSS) support for i.MX95 ENETC PF to improve the
> network performance and balance the CPU loading. In addition, since both
> ENETC v1 and ENETC v4 only support the toeplitz algorithm, so a check for
> hfunc was added.

This and previous commits are a bi hard to follow. You plumb some
stuff thru in the previous commit. In this one you reshuffle things,
again. Try to separate code movement / restructuring in one commit. 
And new additions more clearly in the next.

> +static void enetc4_set_rss_key(struct enetc_hw *hw, const u8 *key)
> +{
> +	int i;
> +
> +	for (i = 0; i < ENETC_RSSHASH_KEY_SIZE / 4; i++)
> +		enetc_port_wr(hw, ENETC4_PRSSKR(i), ((u32 *)key)[i]);
> +}
> +
> +static void enetc4_get_rss_key(struct enetc_hw *hw, u8 *key)
> +{
> +	int i;
> +
> +	for (i = 0; i < ENETC_RSSHASH_KEY_SIZE / 4; i++)
> +		((u32 *)key)[i] = enetc_port_rd(hw, ENETC4_PRSSKR(i));
> +}

Isn't the only difference between the chips the register offset?
Why create full ops for something this trivial?

> +static int enetc4_get_rxnfc(struct net_device *ndev, struct ethtool_rxnfc *rxnfc,
> +			    u32 *rule_locs)
> +{
> +	struct enetc_ndev_priv *priv = netdev_priv(ndev);
> +
> +	switch (rxnfc->cmd) {
> +	case ETHTOOL_GRXRINGS:
> +		rxnfc->data = priv->num_rx_rings;
> +		break;
> +	case ETHTOOL_GRXFH:
> +		return enetc_get_rsshash(rxnfc);
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	return 0;
> +}

Why add a new function instead of returning EOPNOTSUPP for new chips 
in the existing one?

> @@ -712,6 +730,12 @@ static int enetc_set_rxfh(struct net_device *ndev,
>  	struct enetc_hw *hw = &si->hw;
>  	int err = 0;
>  
> +	if (rxfh->hfunc != ETH_RSS_HASH_NO_CHANGE &&
> +	    rxfh->hfunc != ETH_RSS_HASH_TOP) {
> +		netdev_err(ndev, "Only toeplitz hash function is supported\n");
> +		return -EOPNOTSUPP;

Should be a separate commit.
-- 
pw-bot: cr

  reply	other threads:[~2025-01-15 22:00 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-13  8:22 [PATCH v2 net-next 00/13] Add more feautues for ENETC v4 - round 2 Wei Fang
2025-01-13  8:22 ` [PATCH v2 net-next 01/13] net: enetc: add initial netc-lib driver to support NTMP Wei Fang
2025-01-13  8:22 ` [PATCH v2 net-next 02/13] net: enetc: add command BD ring support for i.MX95 ENETC Wei Fang
2025-01-13  8:22 ` [PATCH v2 net-next 03/13] net: enetc: move generic MAC filterng interfaces to enetc-core Wei Fang
2025-01-13  8:22 ` [PATCH v2 net-next 04/13] net: enetc: add MAC filter for i.MX95 ENETC PF Wei Fang
2025-01-13  8:22 ` [PATCH v2 net-next 05/13] net: enetc: add debugfs interface to dump MAC filter Wei Fang
2025-01-13  8:22 ` [PATCH v2 net-next 06/13] net: enetc: make enetc_set_rxfh() and enetc_get_rxfh() reusable Wei Fang
2025-01-13  8:22 ` [PATCH v2 net-next 07/13] net: enetc: add RSS support for i.MX95 ENETC PF Wei Fang
2025-01-15 22:00   ` Jakub Kicinski [this message]
2025-01-16  2:24     ` Wei Fang
2025-01-16  2:41       ` Jakub Kicinski
2025-01-16  4:03         ` Wei Fang
2025-01-13  8:22 ` [PATCH v2 net-next 08/13] net: enetc: enable RSS feature by default Wei Fang
2025-01-13  8:22 ` [PATCH v2 net-next 09/13] net: enetc: move generic VLAN filter interfaces to enetc-core Wei Fang
2025-01-13  8:22 ` [PATCH v2 net-next 10/13] net: enetc: move generic VLAN hash filter functions to enetc_pf_common.c Wei Fang
2025-01-13  8:22 ` [PATCH v2 net-next 11/13] net: enetc: add VLAN filtering support for i.MX95 ENETC PF Wei Fang
2025-01-13  8:22 ` [PATCH v2 net-next 12/13] net: enetc: add loopback " Wei Fang
2025-01-13  8:22 ` [PATCH v2 net-next 13/13] MAINTAINERS: add new file ntmp.h to ENETC driver Wei Fang

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=20250115140042.63b99c4f@kernel.org \
    --to=kuba@kernel.org \
    --cc=andrew+netdev@lunn.ch \
    --cc=christophe.leroy@csgroup.eu \
    --cc=claudiu.manoil@nxp.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=imx@lists.linux.dev \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=vladimir.oltean@nxp.com \
    --cc=wei.fang@nxp.com \
    --cc=xiaoning.wang@nxp.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).