[PATCH 0/3] net/tap: queue limit patches

[PATCH 0/3] net/tap: queue limit patches

[Stephen Hemminger]

Some patches related to recent increase in possible queues.

Stephen Hemminger (3):
  net/tap: handle increase in mp_max_fds
  net/tap: add static assert to make sure max queues less than fd limit
  net/tap: increase the maximum allowable queues

 drivers/net/tap/meson.build   | 2 +-
 drivers/net/tap/rte_eth_tap.c | 8 ++++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

-- 
2.45.2

[PATCH 1/3] net/tap: handle increase in mp_max_fds

[Stephen Hemminger]

Now that max MP fds has increased to 253 it is possible that
the number of queues the TAP device can handle is less than that.
Therefore the code to handle MP message should only allow the
number of queues it can handle.

Coverity issue: 445386
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/tap/rte_eth_tap.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 5ad3bbadd1..c486c6f073 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -2391,9 +2391,10 @@ tap_mp_sync_queues(const struct rte_mp_msg *request, const void *peer)
 	reply_param->q_count = 0;
 
 	RTE_ASSERT(dev->data->nb_rx_queues == dev->data->nb_tx_queues);
-	if (dev->data->nb_rx_queues > RTE_MP_MAX_FD_NUM) {
+
+	if (dev->data->nb_rx_queues > RTE_PMD_TAP_MAX_QUEUES) {
 		TAP_LOG(ERR, "Number of rx/tx queues %u exceeds max number of fds %u",
-			dev->data->nb_rx_queues, RTE_MP_MAX_FD_NUM);
+			dev->data->nb_rx_queues, RTE_PMD_TAP_MAX_QUEUES);
 		return -1;
 	}
 
-- 
2.45.2

[PATCH 2/3] net/tap: add static assert to make sure max queues less than fd limit

[Stephen Hemminger]

Since tap needs to pass fd's between primary and secondary process.
The number of tap queues must be less than the limit.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/tap/rte_eth_tap.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index c486c6f073..a7aa034a55 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -20,6 +20,7 @@
 #include <rte_errno.h>
 #include <rte_cycles.h>
 
+#include <assert.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/socket.h>
@@ -71,6 +72,8 @@
 #define TAP_MP_KEY "tap_mp_sync_queues"
 #define TAP_MP_REQ_START_RXTX "tap_mp_req_start_rxtx"
 
+static_assert(RTE_PMD_TAP_MAX_QUEUES <= RTE_MP_MAX_FD_NUM, "TAP max queues exceeds MP fd limit");
+
 #define TAP_IOV_DEFAULT_MAX 1024
 
 #define TAP_RX_OFFLOAD (RTE_ETH_RX_OFFLOAD_SCATTER |	\
-- 
2.45.2

[PATCH 3/3] net/tap: increase the maximum allowable queues

[Stephen Hemminger]

The default of 16 is too low for larger systems, and the limit
is set at compile time.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/tap/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/tap/meson.build b/drivers/net/tap/meson.build
index 5e5a3ad3c6..fa4e6cbec9 100644
--- a/drivers/net/tap/meson.build
+++ b/drivers/net/tap/meson.build
@@ -14,7 +14,7 @@ sources = files(
 
 deps = ['bus_vdev', 'gso', 'hash']
 
-max_queues = '-DTAP_MAX_QUEUES=16'
+max_queues = '-DTAP_MAX_QUEUES=64'
 cflags += max_queues
 
 require_iova_in_mbuf = false
-- 
2.45.2

Re: [PATCH 3/3] net/tap: increase the maximum allowable queues

[Stephen Hemminger]

On Fri, 11 Oct 2024 10:29:25 -0700
Stephen Hemminger <stephen@networkplumber.org> wrote:

> The default of 16 is too low for larger systems, and the limit
> is set at compile time.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  drivers/net/tap/meson.build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/tap/meson.build b/drivers/net/tap/meson.build
> index 5e5a3ad3c6..fa4e6cbec9 100644
> --- a/drivers/net/tap/meson.build
> +++ b/drivers/net/tap/meson.build
> @@ -14,7 +14,7 @@ sources = files(
>  
>  deps = ['bus_vdev', 'gso', 'hash']
>  
> -max_queues = '-DTAP_MAX_QUEUES=16'
> +max_queues = '-DTAP_MAX_QUEUES=64'
>  cflags += max_queues
>  
>  require_iova_in_mbuf = false

Why does this build fail on Loongix? It works on x86 and Arm.
Is it something about being merged to wrong main branch?


Loongnix-Server 8.3
    Kernel: 4.19.190+
    Compiler: gcc 8.3


Ninja build logs:
-------------------------------BEGIN LOGS----------------------------
FAILED: drivers/libtmp_rte_net_tap.a.p/net_tap_rte_eth_tap.c.o
cc -Idrivers/libtmp_rte_net_tap.a.p -Idrivers -I../drivers -Idrivers/net/tap -I../drivers/net/tap -Idrivers/bus/vdev -I../drivers/bus/vdev -I. -I.. -Iconfig -I../config -Ilib/eal/include -I../lib/eal/include -Ilib/eal/linux/include -I../lib/eal/linux/include -Ilib/eal/loongarch/include -I../lib/eal/loongarch/include -Ilib/eal/common -I../lib/eal/common -Ilib/eal -I../lib/eal -Ilib/kvargs -I../lib/kvargs -Ilib/log -I../lib/log -Ilib/metrics -I../lib/metrics -Ilib/telemetry -I../lib/telemetry -Ilib/gso -I../lib/gso -Ilib/ethdev -I../lib/ethdev -Ilib/net -I../lib/net -Ilib/mbuf -I../lib/mbuf -Ilib/mempool -I../lib/mempool -Ilib/ring -I../lib/ring -Ilib/meter -I../lib/meter -Ilib/hash -I../lib/hash -Ilib/rcu -I../lib/rcu -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -std=c11 -O3 -include rte_config.h -Wcast-qual -Wdeprecated -Wformat -Wformat-nonliteral -Wformat-security -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wold-style-definit
 ion -Wpointer-arith -Wsign-compare -Wstrict-prototypes -Wundef -Wwrite-strings -Wno-packed-not-aligned -Wno-missing-field-initializers -D_GNU_SOURCE -fPIC -march=loongarch64 -DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API -Wno-format-truncation -DTAP_MAX_QUEUES=64 -DHAVE_TCA_FLOWER -DRTE_LOG_DEFAULT_LOGTYPE=pmd.net.tap -MD -MQ drivers/libtmp_rte_net_tap.a.p/net_tap_rte_eth_tap.c.o -MF drivers/libtmp_rte_net_tap.a.p/net_tap_rte_eth_tap.c.o.d -o drivers/libtmp_rte_net_tap.a.p/net_tap_rte_eth_tap.c.o -c ../drivers/net/tap/rte_eth_tap.c
In file included from ../drivers/net/tap/rte_eth_tap.c:23:
./drivers/net/tap/rte_eth_tap.c:75:1: error: static assertion failed: "TAP max queues exceeds MP fd limit"
static_assert(RTE_PMD_TAP_MAX_QUEUES <= RTE_MP_MAX_FD_NUM, "TAP max queues exceeds MP fd limit");

Re: [PATCH 0/3] net/tap: queue limit patches

[Ferruh Yigit]

On 10/11/2024 6:29 PM, Stephen Hemminger wrote:
> Some patches related to recent increase in possible queues.
> 
> Stephen Hemminger (3):
>   net/tap: handle increase in mp_max_fds
>   net/tap: add static assert to make sure max queues less than fd limit
>   net/tap: increase the maximum allowable queues
> 

For series,
Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>

Series applied to dpdk-next-net/main, thanks.

[PATCH v2 0/3] net/tap: queue limit patches

[Stephen Hemminger]

Some patches related to recent queue limit changes.

v2 - up the limit to maximum Linux can support
     dont use static_assert here
     get rid of unreachable checks in configure

Stephen Hemminger (3):
  net/tap: handle increase in mp_max_fds
  net/tap: increase the maximum allowable queues
  net/tap: remove unnecessary checks in configure

 drivers/net/tap/meson.build   |  3 ++-
 drivers/net/tap/rte_eth_tap.c | 21 +++------------------
 2 files changed, 5 insertions(+), 19 deletions(-)

-- 
2.45.2

[PATCH v2 1/3] net/tap: handle increase in mp_max_fds

[Stephen Hemminger]

Now that max MP fds has increased to 253 it is possible that
the number of queues the TAP device can handle is less than that.
Therefore the code to handle MP message should only allow the
number of queues it can handle.

Coverity issue: 445386
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/tap/rte_eth_tap.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 5ad3bbadd1..c486c6f073 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -2391,9 +2391,10 @@ tap_mp_sync_queues(const struct rte_mp_msg *request, const void *peer)
 	reply_param->q_count = 0;
 
 	RTE_ASSERT(dev->data->nb_rx_queues == dev->data->nb_tx_queues);
-	if (dev->data->nb_rx_queues > RTE_MP_MAX_FD_NUM) {
+
+	if (dev->data->nb_rx_queues > RTE_PMD_TAP_MAX_QUEUES) {
 		TAP_LOG(ERR, "Number of rx/tx queues %u exceeds max number of fds %u",
-			dev->data->nb_rx_queues, RTE_MP_MAX_FD_NUM);
+			dev->data->nb_rx_queues, RTE_PMD_TAP_MAX_QUEUES);
 		return -1;
 	}
 
-- 
2.45.2

[PATCH v2 2/3] net/tap: increase the maximum allowable queues

[Stephen Hemminger]

The default of 16 is too low for larger systems.
Upper bound is the maximum number of fd's that can be passed to
secondary process (253) and the maximum number of queue's allowed
by Linux kernel (256).  Both these manifest constants, are not
exposed in visible kernel API.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/tap/meson.build | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/tap/meson.build b/drivers/net/tap/meson.build
index 5e5a3ad3c6..4853efb236 100644
--- a/drivers/net/tap/meson.build
+++ b/drivers/net/tap/meson.build
@@ -14,7 +14,8 @@ sources = files(
 
 deps = ['bus_vdev', 'gso', 'hash']
 
-max_queues = '-DTAP_MAX_QUEUES=16'
+# TAP device is limited by RTE_MP_MAX_FD_NUM
+max_queues = '-DTAP_MAX_QUEUES=253'
 cflags += max_queues
 
 require_iova_in_mbuf = false
-- 
2.45.2

[PATCH v2 3/3] net/tap: remove unnecessary checks in configure

[Stephen Hemminger]

The ethdev layer already validates that the number of requested
queues is less than the reported max queues.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/tap/rte_eth_tap.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index c486c6f073..46afc9e2cb 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -948,22 +948,6 @@ tap_dev_configure(struct rte_eth_dev *dev)
 {
 	struct pmd_internals *pmd = dev->data->dev_private;
 
-	if (dev->data->nb_rx_queues > RTE_PMD_TAP_MAX_QUEUES) {
-		TAP_LOG(ERR,
-			"%s: number of rx queues %d exceeds max num of queues %d",
-			dev->device->name,
-			dev->data->nb_rx_queues,
-			RTE_PMD_TAP_MAX_QUEUES);
-		return -1;
-	}
-	if (dev->data->nb_tx_queues > RTE_PMD_TAP_MAX_QUEUES) {
-		TAP_LOG(ERR,
-			"%s: number of tx queues %d exceeds max num of queues %d",
-			dev->device->name,
-			dev->data->nb_tx_queues,
-			RTE_PMD_TAP_MAX_QUEUES);
-		return -1;
-	}
 	if (dev->data->nb_rx_queues != dev->data->nb_tx_queues) {
 		TAP_LOG(ERR,
 			"%s: number of rx queues %d must be equal to number of tx queues %d",
-- 
2.45.2

Re: [PATCH 0/3] net/tap: queue limit patches

[Stephen Hemminger]

On Sat, 12 Oct 2024 02:01:12 +0100
Ferruh Yigit <ferruh.yigit@amd.com> wrote:

> On 10/11/2024 6:29 PM, Stephen Hemminger wrote:
> > Some patches related to recent increase in possible queues.
> > 
> > Stephen Hemminger (3):
> >   net/tap: handle increase in mp_max_fds
> >   net/tap: add static assert to make sure max queues less than fd limit
> >   net/tap: increase the maximum allowable queues
> >   
> 
> For series,
> Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>
> 
> Series applied to dpdk-next-net/main, thanks.
> 


I sent a revised one, mostly because of the failure in loongarch build.
But also after further investigation of what is upper limit in Linux
and doing tests at max queues.

Re: [PATCH v2 0/3] net/tap: queue limit patches

[Ferruh Yigit]

On 10/12/2024 3:17 AM, Stephen Hemminger wrote:
> Some patches related to recent queue limit changes.
> 
> v2 - up the limit to maximum Linux can support
>      dont use static_assert here
>      get rid of unreachable checks in configure
> 
> Stephen Hemminger (3):
>   net/tap: handle increase in mp_max_fds
>   net/tap: increase the maximum allowable queues
>   net/tap: remove unnecessary checks in configure
> 

Hi Stephen,

Previous version already merged, but I am checking the difference in v2.

Patch 1/3 is identical.
Patch 2/3, I prefer the v1 version, that increases the queue limit to
64, instead of 253.

v1 patch 3/3 static assert seems gone, it seems because of the loongarch
build, but can we please root cause why it failed, and can the failure
be a test environment issue?

v2 patch 3/3 looks good, it can be merged separately.

Re: [PATCH v2 0/3] net/tap: queue limit patches

[Stephen Hemminger]

On Sun, 13 Oct 2024 00:53:54 +0100
Ferruh Yigit <ferruh.yigit@amd.com> wrote:

> On 10/12/2024 3:17 AM, Stephen Hemminger wrote:
> > Some patches related to recent queue limit changes.
> > 
> > v2 - up the limit to maximum Linux can support
> >      dont use static_assert here
> >      get rid of unreachable checks in configure
> > 
> > Stephen Hemminger (3):
> >   net/tap: handle increase in mp_max_fds
> >   net/tap: increase the maximum allowable queues
> >   net/tap: remove unnecessary checks in configure
> >   
> 
> Hi Stephen,
> 
> Previous version already merged, but I am checking the difference in v2.
> 
> Patch 1/3 is identical.
> Patch 2/3, I prefer the v1 version, that increases the queue limit to
> 64, instead of 253.

Ok, we can bump it later if anyone wants to run on 128 core cpu

> 
> v1 patch 3/3 static assert seems gone, it seems because of the loongarch
> build, but can we please root cause why it failed, and can the failure
> be a test environment issue?

Right, not sure what was wrong there, and not easy to setup a loongarch
build (even with qemu) to repro.


> v2 patch 3/3 looks good, it can be merged separately.

Ok, will add it to later follow up set. The tap device driver needs
lots more cleanups.

Re: [PATCH v2 0/3] net/tap: queue limit patches

[Ferruh Yigit]

On 10/13/2024 1:53 AM, Stephen Hemminger wrote:
> On Sun, 13 Oct 2024 00:53:54 +0100
> Ferruh Yigit <ferruh.yigit@amd.com> wrote:
> 
>> On 10/12/2024 3:17 AM, Stephen Hemminger wrote:
>>> Some patches related to recent queue limit changes.
>>>
>>> v2 - up the limit to maximum Linux can support
>>>      dont use static_assert here
>>>      get rid of unreachable checks in configure
>>>
>>> Stephen Hemminger (3):
>>>   net/tap: handle increase in mp_max_fds
>>>   net/tap: increase the maximum allowable queues
>>>   net/tap: remove unnecessary checks in configure
>>>   
>>
>> Hi Stephen,
>>
>> Previous version already merged, but I am checking the difference in v2.
>>
>> Patch 1/3 is identical.
>> Patch 2/3, I prefer the v1 version, that increases the queue limit to
>> 64, instead of 253.
> 
> Ok, we can bump it later if anyone wants to run on 128 core cpu
> 

ack

>>
>> v1 patch 3/3 static assert seems gone, it seems because of the loongarch
>> build, but can we please root cause why it failed, and can the failure
>> be a test environment issue?
> 
> Right, not sure what was wrong there, and not easy to setup a loongarch
> build (even with qemu) to repro.
> 

Indeed I was hoping to get some support from loongarch CI maintainers,
instead of we setup an environment to test.

As the patch is already merged in next-net, they should able to test &
debug it.

> 
>> v2 patch 3/3 looks good, it can be merged separately.
> 
> Ok, will add it to later follow up set. The tap device driver needs
> lots more cleanups.
> 

I can merge the patch directly from this set, planning to check it soon.

Re: [PATCH v2 3/3] net/tap: remove unnecessary checks in configure

[Ferruh Yigit]

On 10/12/2024 3:17 AM, Stephen Hemminger wrote:
> The ethdev layer already validates that the number of requested
> queues is less than the reported max queues.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>

Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>

Applied to dpdk-next-net/main, thanks.