Re: [PATCH net-next 1/2] sfc: default config to 1 channel/core in local NUMA node only

From: Jakub Kicinski <kuba@kernel.org>
To: "Íñigo Huguet" <ihuguet@redhat.com>
Cc: Edward Cree <ecree.xilinx@gmail.com>,
	habetsm.xilinx@gmail.com, "David S. Miller" <davem@davemloft.net>,
	netdev@vger.kernel.org
Subject: Re: [PATCH net-next 1/2] sfc: default config to 1 channel/core in local NUMA node only
Date: Fri, 11 Feb 2022 11:01:00 -0800	[thread overview]
Message-ID: <20220211110100.5580d1ac@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> (raw)
In-Reply-To: <CACT4ouepk83kxTGd6S3gVyFAjofofwQfxsmhe97vGP+twkoW1g@mail.gmail.com>

On Fri, 11 Feb 2022 12:05:19 +0100 Íñigo Huguet wrote:
> Totally. My comment was intended to be more like a question to see why
> we should or shouldn't consider NUMA nodes in
> netif_get_num_default_rss_queues. But now I understand your point
> better.
> 
> However, would it make sense something like this for
> netif_get_num_default_rss_queues, or it would be a bit overkill?
> if the system has more than one NUMA node, allocate one queue per
> physical core in local NUMA node.
> else, allocate physical cores / 2

I don't have a strong opinion on the NUMA question, to be honest.
It gets complicated pretty quickly. If there is one NIC we may or 
may not want to divide - for pure packet forwarding sure, best if
its done on the node with the NIC, but that assumes the other node 
is idle or doing something else? How does it not need networking?

If each node has a separate NIC we should definitely divide. But
it's impossible to know the NIC count at the netdev level..

So my thinking was let's leave NUMA configurations to manual tuning.
If we don't do anything special for NUMA it's less likely someone will
tell us we did the wrong thing there :) But feel free to implement what
you suggested above.

One thing I'm not sure of is if anyone uses the early AMD chiplet CPUs 
in a NUMA-per-chiplet mode? IIRC they had a mode like that. And that'd
potentially be problematic if we wanted to divide by number of nodes.
Maybe not as much if just dividing by 2.

> Another thing: this patch series appears in patchwork with state
> "Changes Requested", but no changes have been requested, actually. Can
> the state be changed so it has more visibility to get reviews?

I think resend would be best.