From: Alexander Lobakin <aleksander.lobakin@intel.com>
To: "Tantilov, Emil S" <emil.s.tantilov@intel.com>
Cc: <intel-wired-lan@lists.osuosl.org>, <netdev@vger.kernel.org>,
<linux-kernel@vger.kernel.org>,
Eric Dumazet <edumazet@google.com>,
"Tony Nguyen" <anthony.l.nguyen@intel.com>,
<nex.sw.ncis.osdt.itp.upstreaming@intel.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
"David S. Miller" <davem@davemloft.net>
Subject: Re: [Intel-wired-lan] [PATCH RFC iwl-next 07/12] idpf: compile singleq code only under default-n CONFIG_IDPF_SINGLEQ
Date: Thu, 16 May 2024 12:40:12 +0200 [thread overview]
Message-ID: <095fc242-ef9f-441d-b376-f2cfd4d5759a@intel.com> (raw)
In-Reply-To: <033c24ce-b0e4-4653-ab8a-2cefbbec0893@intel.com>
From: Tantilov, Emil S <emil.s.tantilov@intel.com>
Date: Mon, 13 May 2024 19:01:30 -0700
> On 5/10/2024 8:26 AM, Alexander Lobakin wrote:
>> Currently, there's no HW supporting idpf in the singleq model. Still,
>> this dead code is supported by the driver and often times add hotpath
> The driver supports the HW in single queue mode (not the other way
> around), so I would like to point out that all current HW on which idpf
> can load supports this model.
But no hardware currently wants to work in this mode.
>
>> branches and redundant cacheline accesses.
>> While it can't currently be removed, add CONFIG_IDPF_SINGLEQ and build
>> the singleq code only when it's enabled manually. This corresponds to
>> -10 Kb of object code size and a good bunch of hotpath checks.
>> idpf_is_queue_model_split() works as a gate and compiles out to `true`
>> when the config option is disabled.
> Compiling singleq out does introduce an issue for the users that would
> end up without a netdev if the driver is loaded on a setup where the
There will be a message in dmesg saying that the support is not compiled in.
> Control Plane is configured for singlq mode. In that scenario the user
Any real world examples where the CP requests singleq mode?
> will have to recompile the driver/kernel in order to load the driver
> successfully.
All distros will have it enabled as usually.
>
>>
>> Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.com>
>> ---
>> drivers/net/ethernet/intel/Kconfig | 13 +---------
>> drivers/net/ethernet/intel/idpf/Kconfig | 26 +++++++++++++++++++
>> drivers/net/ethernet/intel/idpf/Makefile | 3 ++-
>> drivers/net/ethernet/intel/idpf/idpf.h | 3 ++-
>> drivers/net/ethernet/intel/idpf/idpf_txrx.c | 2 +-
>> .../net/ethernet/intel/idpf/idpf_virtchnl.c | 15 ++++++++---
>> 6 files changed, 43 insertions(+), 19 deletions(-)
>> create mode 100644 drivers/net/ethernet/intel/idpf/Kconfig
>>
>> diff --git a/drivers/net/ethernet/intel/Kconfig
>> b/drivers/net/ethernet/intel/Kconfig
>> index e0287fbd501d..0375c7448a57 100644
>> --- a/drivers/net/ethernet/intel/Kconfig
>> +++ b/drivers/net/ethernet/intel/Kconfig
>> @@ -384,17 +384,6 @@ config IGC_LEDS
>> Optional support for controlling the NIC LED's with the netdev
>> LED trigger.
>> -config IDPF
>> - tristate "Intel(R) Infrastructure Data Path Function Support"
>> - depends on PCI_MSI
>> - select DIMLIB
>> - select PAGE_POOL
>> - select PAGE_POOL_STATS
>> - help
>> - This driver supports Intel(R) Infrastructure Data Path Function
>> - devices.
>> -
>> - To compile this driver as a module, choose M here. The module
>> - will be called idpf.
>> +source "drivers/net/ethernet/intel/idpf/Kconfig"
>> endif # NET_VENDOR_INTEL
>> diff --git a/drivers/net/ethernet/intel/idpf/Kconfig
>> b/drivers/net/ethernet/intel/idpf/Kconfig
>> new file mode 100644
>> index 000000000000..bee83a40f218
>> --- /dev/null
>> +++ b/drivers/net/ethernet/intel/idpf/Kconfig
>> @@ -0,0 +1,26 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +# Copyright (C) 2024 Intel Corporation
>> +
>> +config IDPF
>> + tristate "Intel(R) Infrastructure Data Path Function Support"
>> + depends on PCI_MSI
>> + select DIMLIB
>> + select PAGE_POOL
>> + select PAGE_POOL_STATS
>> + help
>> + This driver supports Intel(R) Infrastructure Data Path Function
>> + devices.
>> +
>> + To compile this driver as a module, choose M here. The module
>> + will be called idpf.
>> +
>> +if IDPF
>> +
>> +config IDPF_SINGLEQ
>> + bool "idpf singleq support"
>> + help
>> + This option enables support for legacy single Rx/Tx queues w/no
>> + completion and fill queues. Only enable if you have such hardware
> This description is not accurate - all HW supports single queue. The
> configuration is done by the Control Plane. Furthermore, without access
> to the Control Plane config, there is no way for the user to know what
> mode is enabled.
I can rephrase to "if your hardware is configured to work in singleq mode".
Anyway, during both the internal review and this one, no one still gave
me a real world example where the HW wants singleq mode.
When the idpf made it into the kernel, singleq didn't work, which was
proven by several patches from me which fixed critical issues.
If it's not tested and no HW wants this when splitq is available, why
decrease perf by dead code?
>
> Thanks,
> Emil
Thanks,
Olek
next prev parent reply other threads:[~2024-05-16 10:41 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-10 15:26 [PATCH RFC iwl-next 00/12] idpf: XDP chapter I: convert Rx to libeth Alexander Lobakin
2024-05-10 15:26 ` [PATCH RFC iwl-next 01/12] libeth: add cacheline / struct alignment helpers Alexander Lobakin
2024-05-10 15:26 ` [PATCH RFC iwl-next 02/12] idpf: stop using macros for accessing queue descriptors Alexander Lobakin
2024-05-16 17:45 ` Mina Almasry
2024-05-10 15:26 ` [PATCH RFC iwl-next 03/12] idpf: split &idpf_queue into 4 strictly-typed queue structures Alexander Lobakin
2024-05-10 15:26 ` [PATCH RFC iwl-next 04/12] idpf: avoid bloating &idpf_q_vector with big %NR_CPUS Alexander Lobakin
2024-05-10 15:26 ` [PATCH RFC iwl-next 05/12] idpf: strictly assert cachelines of queue and queue vector structures Alexander Lobakin
2024-05-10 15:26 ` [PATCH RFC iwl-next 06/12] idpf: merge singleq and splitq &net_device_ops Alexander Lobakin
2024-05-10 15:26 ` [PATCH RFC iwl-next 07/12] idpf: compile singleq code only under default-n CONFIG_IDPF_SINGLEQ Alexander Lobakin
2024-05-14 2:01 ` [Intel-wired-lan] " Tantilov, Emil S
2024-05-16 10:40 ` Alexander Lobakin [this message]
2024-05-10 15:26 ` [PATCH RFC iwl-next 08/12] idpf: reuse libeth's definitions of parsed ptype structures Alexander Lobakin
2024-05-10 16:22 ` Mina Almasry
2024-05-27 11:13 ` Alexander Lobakin
2024-05-10 15:26 ` [PATCH RFC iwl-next 09/12] idpf: remove legacy Page Pool Ethtool stats Alexander Lobakin
2024-05-10 15:26 ` [PATCH RFC iwl-next 10/12] libeth: support different types of buffers for Rx Alexander Lobakin
2024-05-10 15:26 ` [PATCH RFC iwl-next 11/12] idpf: convert header split mode to libeth + napi_build_skb() Alexander Lobakin
2024-05-10 15:26 ` [PATCH RFC iwl-next 12/12] idpf: use libeth Rx buffer management for payload buffer Alexander Lobakin
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=095fc242-ef9f-441d-b376-f2cfd4d5759a@intel.com \
--to=aleksander.lobakin@intel.com \
--cc=anthony.l.nguyen@intel.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=emil.s.tantilov@intel.com \
--cc=intel-wired-lan@lists.osuosl.org \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=nex.sw.ncis.osdt.itp.upstreaming@intel.com \
--cc=pabeni@redhat.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 read-only IMAP folder(s) and NNTP newsgroup(s).