* [PATCH iproute2] utils: bump max args number to 256 for batch files
@ 2021-06-01 17:09 Guillaume Nault
2021-06-09 23:59 ` Stephen Hemminger
0 siblings, 1 reply; 5+ messages in thread
From: Guillaume Nault @ 2021-06-01 17:09 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
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 <gnault@redhat.com>
---
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH iproute2] utils: bump max args number to 256 for batch files
2021-06-01 17:09 [PATCH iproute2] utils: bump max args number to 256 for batch files Guillaume Nault
@ 2021-06-09 23:59 ` Stephen Hemminger
2021-06-10 7:58 ` Guillaume Nault
0 siblings, 1 reply; 5+ messages in thread
From: Stephen Hemminger @ 2021-06-09 23:59 UTC (permalink / raw)
To: Guillaume Nault; +Cc: netdev
On Tue, 1 Jun 2021 19:09:31 +0200
Guillaume Nault <gnault@redhat.com> 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 <gnault@redhat.com>
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 */
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH iproute2] utils: bump max args number to 256 for batch files
2021-06-09 23:59 ` Stephen Hemminger
@ 2021-06-10 7:58 ` Guillaume Nault
2021-06-10 23:17 ` Stephen Hemminger
0 siblings, 1 reply; 5+ messages in thread
From: Guillaume Nault @ 2021-06-10 7:58 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
On Wed, Jun 09, 2021 at 04:59:49PM -0700, Stephen Hemminger wrote:
> On Tue, 1 Jun 2021 19:09:31 +0200
> Guillaume Nault <gnault@redhat.com> 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 <gnault@redhat.com>
>
> 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).
> 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?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH iproute2] utils: bump max args number to 256 for batch files
2021-06-10 7:58 ` Guillaume Nault
@ 2021-06-10 23:17 ` Stephen Hemminger
2021-06-11 9:51 ` Guillaume Nault
0 siblings, 1 reply; 5+ messages in thread
From: Stephen Hemminger @ 2021-06-10 23:17 UTC (permalink / raw)
To: Guillaume Nault; +Cc: netdev
On Thu, 10 Jun 2021 09:58:57 +0200
Guillaume Nault <gnault@redhat.com> wrote:
> On Wed, Jun 09, 2021 at 04:59:49PM -0700, Stephen Hemminger wrote:
> > On Tue, 1 Jun 2021 19:09:31 +0200
> > Guillaume Nault <gnault@redhat.com> 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 <gnault@redhat.com>
> >
> > 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).
>
> > 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH iproute2] utils: bump max args number to 256 for batch files
2021-06-10 23:17 ` Stephen Hemminger
@ 2021-06-11 9:51 ` Guillaume Nault
0 siblings, 0 replies; 5+ messages in thread
From: Guillaume Nault @ 2021-06-11 9:51 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
On Thu, Jun 10, 2021 at 04:17:42PM -0700, Stephen Hemminger wrote:
> On Thu, 10 Jun 2021 09:58:57 +0200
> Guillaume Nault <gnault@redhat.com> wrote:
> > > 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.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-06-11 9:51 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-01 17:09 [PATCH iproute2] utils: bump max args number to 256 for batch files Guillaume Nault
2021-06-09 23:59 ` Stephen Hemminger
2021-06-10 7:58 ` Guillaume Nault
2021-06-10 23:17 ` Stephen Hemminger
2021-06-11 9:51 ` Guillaume Nault
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.