[PATCH iproute2] utils: bump max args number to 256 for batch files

Subsystems: library code, the rest

STALE1884d

5 messages, 2 authors, 2021-06-11 · open the first message on its own page

[PATCH iproute2] utils: bump max args number to 256 for batch files

From: Guillaume Nault <hidden>
Date: 2021-06-01 17:09:40

Large tc filters can have many arguments. For example the following
filter matches the first 7 MPLS LSEs, pops all of them, then updates
the Ethernet header and redirects the resulting packet to eth1.

filter add dev eth0 ingress handle 44 priority 100 \
  protocol mpls_uc flower mpls                     \
    lse depth 1 label 1040076 tc 4 bos 0 ttl 175   \
    lse depth 2 label 89648 tc 2 bos 0 ttl 9       \
    lse depth 3 label 63417 tc 5 bos 0 ttl 185     \
    lse depth 4 label 593135 tc 5 bos 0 ttl 67     \
    lse depth 5 label 857021 tc 0 bos 0 ttl 181    \
    lse depth 6 label 239239 tc 1 bos 0 ttl 254    \
    lse depth 7 label 30 tc 7 bos 1 ttl 237        \
  action mpls pop protocol mpls_uc pipe            \
  action mpls pop protocol mpls_uc pipe            \
  action mpls pop protocol mpls_uc pipe            \
  action mpls pop protocol mpls_uc pipe            \
  action mpls pop protocol mpls_uc pipe            \
  action mpls pop protocol mpls_uc pipe            \
  action mpls pop protocol ipv6 pipe               \
  action vlan pop_eth pipe                         \
  action vlan push_eth                             \
    dst_mac 00:00:5e:00:53:7e                      \
    src_mac 00:00:5e:00:53:03 pipe                 \
  action mirred egress redirect dev eth1

This filter has 149 arguments, so it can't be used with tc -batch
which is limited to a 100.

Let's bump the limit to the next power of 2. That should leave a lot of
room for big batch commands.

Signed-off-by: Guillaume Nault <redacted>
---

Note: I have no production use case for MPLS stacks with 7 LSEs at the
      moment, but 7 is the maximum depth the flow dissector can handle,
      and having the possibility to express such rules with tc -batch
      would help testing the kernel API (writing scripts that generate
      filters, without worrying about the 100 parameters limit).

 lib/utils.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/utils.c b/lib/utils.c
index 93ae0c55..d5496e45 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -1714,10 +1714,10 @@ int do_batch(const char *name, bool force,
 
 	cmdlineno = 0;
 	while (getcmdline(&line, &len, stdin) != -1) {
-		char *largv[100];
+		char *largv[256];
 		int largc;
 
-		largc = makeargs(line, largv, 100);
+		largc = makeargs(line, largv, 256);
 		if (!largc)
 			continue;	/* blank line */
 
-- 
2.21.3

Re: [PATCH iproute2] utils: bump max args number to 256 for batch files

From: Stephen Hemminger <stephen@networkplumber.org>
Date: 2021-06-10 00:00:01

On Tue, 1 Jun 2021 19:09:31 +0200
Guillaume Nault [off-list ref] wrote:
Large tc filters can have many arguments. For example the following
filter matches the first 7 MPLS LSEs, pops all of them, then updates
the Ethernet header and redirects the resulting packet to eth1.

filter add dev eth0 ingress handle 44 priority 100 \
  protocol mpls_uc flower mpls                     \
    lse depth 1 label 1040076 tc 4 bos 0 ttl 175   \
    lse depth 2 label 89648 tc 2 bos 0 ttl 9       \
    lse depth 3 label 63417 tc 5 bos 0 ttl 185     \
    lse depth 4 label 593135 tc 5 bos 0 ttl 67     \
    lse depth 5 label 857021 tc 0 bos 0 ttl 181    \
    lse depth 6 label 239239 tc 1 bos 0 ttl 254    \
    lse depth 7 label 30 tc 7 bos 1 ttl 237        \
  action mpls pop protocol mpls_uc pipe            \
  action mpls pop protocol mpls_uc pipe            \
  action mpls pop protocol mpls_uc pipe            \
  action mpls pop protocol mpls_uc pipe            \
  action mpls pop protocol mpls_uc pipe            \
  action mpls pop protocol mpls_uc pipe            \
  action mpls pop protocol ipv6 pipe               \
  action vlan pop_eth pipe                         \
  action vlan push_eth                             \
    dst_mac 00:00:5e:00:53:7e                      \
    src_mac 00:00:5e:00:53:03 pipe                 \
  action mirred egress redirect dev eth1

This filter has 149 arguments, so it can't be used with tc -batch
which is limited to a 100.

Let's bump the limit to the next power of 2. That should leave a lot of
room for big batch commands.

Signed-off-by: Guillaume Nault <redacted>
Good idea, but we should probably go further up to 512.
Also, rather than keeping magic constant. Why not add value to
utils.h.

I considered using sysconf(_SC_ARG_MAX) gut that is huge on modern
machines (2M). And we don't need to allocate for all possible args.
diff --git a/include/utils.h b/include/utils.h
index 187444d52b41..6c4c403fe6c2 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -50,6 +50,9 @@ void incomplete_command(void) __attribute__((noreturn));
 #define NEXT_ARG_FWD() do { argv++; argc--; } while(0)
 #define PREV_ARG() do { argv--; argc++; } while(0)
 
+/* upper limit for batch mode */
+#define MAX_ARGS 512
+
 #define TIME_UNITS_PER_SEC     1000000
 #define NSEC_PER_USEC 1000
 #define NSEC_PER_MSEC 1000000
diff --git a/lib/utils.c b/lib/utils.c
index 93ae0c55063a..0559923beced 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -1714,10 +1714,10 @@ int do_batch(const char *name, bool force,
 
        cmdlineno = 0;
        while (getcmdline(&line, &len, stdin) != -1) {
-               char *largv[100];
+               char *largv[MAX_ARGS];
                int largc;
 
-               largc = makeargs(line, largv, 100);
+               largc = makeargs(line, largv, MAX_ARGS);
                if (!largc)
                        continue;       /* blank line */
 

Re: [PATCH iproute2] utils: bump max args number to 256 for batch files

From: Guillaume Nault <hidden>
Date: 2021-06-10 07:59:05

On Wed, Jun 09, 2021 at 04:59:49PM -0700, Stephen Hemminger wrote:
On Tue, 1 Jun 2021 19:09:31 +0200
Guillaume Nault [off-list ref] wrote:
quoted
Large tc filters can have many arguments. For example the following
filter matches the first 7 MPLS LSEs, pops all of them, then updates
the Ethernet header and redirects the resulting packet to eth1.

filter add dev eth0 ingress handle 44 priority 100 \
  protocol mpls_uc flower mpls                     \
    lse depth 1 label 1040076 tc 4 bos 0 ttl 175   \
    lse depth 2 label 89648 tc 2 bos 0 ttl 9       \
    lse depth 3 label 63417 tc 5 bos 0 ttl 185     \
    lse depth 4 label 593135 tc 5 bos 0 ttl 67     \
    lse depth 5 label 857021 tc 0 bos 0 ttl 181    \
    lse depth 6 label 239239 tc 1 bos 0 ttl 254    \
    lse depth 7 label 30 tc 7 bos 1 ttl 237        \
  action mpls pop protocol mpls_uc pipe            \
  action mpls pop protocol mpls_uc pipe            \
  action mpls pop protocol mpls_uc pipe            \
  action mpls pop protocol mpls_uc pipe            \
  action mpls pop protocol mpls_uc pipe            \
  action mpls pop protocol mpls_uc pipe            \
  action mpls pop protocol ipv6 pipe               \
  action vlan pop_eth pipe                         \
  action vlan push_eth                             \
    dst_mac 00:00:5e:00:53:7e                      \
    src_mac 00:00:5e:00:53:03 pipe                 \
  action mirred egress redirect dev eth1

This filter has 149 arguments, so it can't be used with tc -batch
which is limited to a 100.

Let's bump the limit to the next power of 2. That should leave a lot of
room for big batch commands.

Signed-off-by: Guillaume Nault <redacted>
Good idea, but we should probably go further up to 512.
Also, rather than keeping magic constant. Why not add value to
utils.h.
Yes, right.
I considered using sysconf(_SC_ARG_MAX) gut that is huge on modern
machines (2M). And we don't need to allocate for all possible args.
Yes, 2M is probably overkill (and too much to allocate on the stack).
quoted hunk
diff --git a/include/utils.h b/include/utils.h
index 187444d52b41..6c4c403fe6c2 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -50,6 +50,9 @@ void incomplete_command(void) __attribute__((noreturn));
 #define NEXT_ARG_FWD() do { argv++; argc--; } while(0)
 #define PREV_ARG() do { argv--; argc++; } while(0)
 
+/* upper limit for batch mode */
+#define MAX_ARGS 512
+
 #define TIME_UNITS_PER_SEC     1000000
 #define NSEC_PER_USEC 1000
 #define NSEC_PER_MSEC 1000000
diff --git a/lib/utils.c b/lib/utils.c
index 93ae0c55063a..0559923beced 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -1714,10 +1714,10 @@ int do_batch(const char *name, bool force,
 
        cmdlineno = 0;
        while (getcmdline(&line, &len, stdin) != -1) {
-               char *largv[100];
+               char *largv[MAX_ARGS];
                int largc;
 
-               largc = makeargs(line, largv, 100);
+               largc = makeargs(line, largv, MAX_ARGS);
                if (!largc)
                        continue;       /* blank line */
 
Is this a patch you're going to apply, or should I repost it formally?

Re: [PATCH iproute2] utils: bump max args number to 256 for batch files

From: Stephen Hemminger <stephen@networkplumber.org>
Date: 2021-06-10 23:19:07

On Thu, 10 Jun 2021 09:58:57 +0200
Guillaume Nault [off-list ref] wrote:
On Wed, Jun 09, 2021 at 04:59:49PM -0700, Stephen Hemminger wrote:
quoted
On Tue, 1 Jun 2021 19:09:31 +0200
Guillaume Nault [off-list ref] wrote:
  
quoted
Large tc filters can have many arguments. For example the following
filter matches the first 7 MPLS LSEs, pops all of them, then updates
the Ethernet header and redirects the resulting packet to eth1.

filter add dev eth0 ingress handle 44 priority 100 \
  protocol mpls_uc flower mpls                     \
    lse depth 1 label 1040076 tc 4 bos 0 ttl 175   \
    lse depth 2 label 89648 tc 2 bos 0 ttl 9       \
    lse depth 3 label 63417 tc 5 bos 0 ttl 185     \
    lse depth 4 label 593135 tc 5 bos 0 ttl 67     \
    lse depth 5 label 857021 tc 0 bos 0 ttl 181    \
    lse depth 6 label 239239 tc 1 bos 0 ttl 254    \
    lse depth 7 label 30 tc 7 bos 1 ttl 237        \
  action mpls pop protocol mpls_uc pipe            \
  action mpls pop protocol mpls_uc pipe            \
  action mpls pop protocol mpls_uc pipe            \
  action mpls pop protocol mpls_uc pipe            \
  action mpls pop protocol mpls_uc pipe            \
  action mpls pop protocol mpls_uc pipe            \
  action mpls pop protocol ipv6 pipe               \
  action vlan pop_eth pipe                         \
  action vlan push_eth                             \
    dst_mac 00:00:5e:00:53:7e                      \
    src_mac 00:00:5e:00:53:03 pipe                 \
  action mirred egress redirect dev eth1

This filter has 149 arguments, so it can't be used with tc -batch
which is limited to a 100.

Let's bump the limit to the next power of 2. That should leave a lot of
room for big batch commands.

Signed-off-by: Guillaume Nault <redacted>  
Good idea, but we should probably go further up to 512.
Also, rather than keeping magic constant. Why not add value to
utils.h.  
Yes, right.
quoted
I considered using sysconf(_SC_ARG_MAX) gut that is huge on modern
machines (2M). And we don't need to allocate for all possible args.  
Yes, 2M is probably overkill (and too much to allocate on the stack).
quoted
diff --git a/include/utils.h b/include/utils.h
index 187444d52b41..6c4c403fe6c2 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -50,6 +50,9 @@ void incomplete_command(void) __attribute__((noreturn));
 #define NEXT_ARG_FWD() do { argv++; argc--; } while(0)
 #define PREV_ARG() do { argv--; argc++; } while(0)
 
+/* upper limit for batch mode */
+#define MAX_ARGS 512
+
 #define TIME_UNITS_PER_SEC     1000000
 #define NSEC_PER_USEC 1000
 #define NSEC_PER_MSEC 1000000
diff --git a/lib/utils.c b/lib/utils.c
index 93ae0c55063a..0559923beced 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -1714,10 +1714,10 @@ int do_batch(const char *name, bool force,
 
        cmdlineno = 0;
        while (getcmdline(&line, &len, stdin) != -1) {
-               char *largv[100];
+               char *largv[MAX_ARGS];
                int largc;
 
-               largc = makeargs(line, largv, 100);
+               largc = makeargs(line, largv, MAX_ARGS);
                if (!largc)
                        continue;       /* blank line */
 
  
Is this a patch you're going to apply, or should I repost it formally?
Either way, you get credit

Re: [PATCH iproute2] utils: bump max args number to 256 for batch files

From: Guillaume Nault <hidden>
Date: 2021-06-11 09:51:38

On Thu, Jun 10, 2021 at 04:17:42PM -0700, Stephen Hemminger wrote:
On Thu, 10 Jun 2021 09:58:57 +0200
Guillaume Nault [off-list ref] wrote:
quoted
quoted
diff --git a/include/utils.h b/include/utils.h
index 187444d52b41..6c4c403fe6c2 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -50,6 +50,9 @@ void incomplete_command(void) __attribute__((noreturn));
 #define NEXT_ARG_FWD() do { argv++; argc--; } while(0)
 #define PREV_ARG() do { argv--; argc++; } while(0)
 
+/* upper limit for batch mode */
+#define MAX_ARGS 512
+
 #define TIME_UNITS_PER_SEC     1000000
 #define NSEC_PER_USEC 1000
 #define NSEC_PER_MSEC 1000000
diff --git a/lib/utils.c b/lib/utils.c
index 93ae0c55063a..0559923beced 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -1714,10 +1714,10 @@ int do_batch(const char *name, bool force,
 
        cmdlineno = 0;
        while (getcmdline(&line, &len, stdin) != -1) {
-               char *largv[100];
+               char *largv[MAX_ARGS];
                int largc;
 
-               largc = makeargs(line, largv, 100);
+               largc = makeargs(line, largv, MAX_ARGS);
                if (!largc)
                        continue;       /* blank line */
 
  
Is this a patch you're going to apply, or should I repost it formally?
Either way, you get credit
I've sent v2. Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help