* [PATCH iproute2 net-next] tc: m_mirred: Fix parsing of 'index' optional argument
@ 2016-10-21 20:01 Shmulik Ladkani
2016-10-26 18:23 ` Stephen Hemminger
0 siblings, 1 reply; 2+ messages in thread
From: Shmulik Ladkani @ 2016-10-21 20:01 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Jamal Hadi Salim, netdev, Shmulik Ladkani
Code in parse_mirred() suggests "index" argument can be placed either
after the "egress" clause, or as the first argument (after "action mirred").
However, parse_egress() fails to correctly parse "index" if it's the
first argument.
For example:
# tc filter add ... action mirred index 5
RTNETLINK answers: Invalid argument
(unnecessary RTNETLINK issued, should have been parse error)
# tc filter add ... action mirred index 5 egress redirect dev eth0
bad action type egress
(should have been parsed successfully)
Fix parse_egress as follows:
- continue parsing after valid "index" is seen
- don't issue the RTNETLINK unless valid "egress" is seen
Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
---
Well, both man and usage suggest "index" comes after the egress clause:
tc ... action mirred DIRECTION ACTION [ index INDEX ] dev DEVICENAME
So an alternative would be banning "index" as 1st argument in parse_mirred.
tc/m_mirred.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/tc/m_mirred.c b/tc/m_mirred.c
index 11f4c9b46f..d3e750b89b 100644
--- a/tc/m_mirred.c
+++ b/tc/m_mirred.c
@@ -91,9 +91,8 @@ parse_egress(struct action_util *a, int *argc_p, char ***argv_p,
}
iok++;
if (!ok) {
- argc--;
- argv++;
- break;
+ NEXT_ARG();
+ continue;
}
} else if (!ok) {
fprintf(stderr, "was expecting egress (%s)\n", *argv);
@@ -134,7 +133,7 @@ parse_egress(struct action_util *a, int *argc_p, char ***argv_p,
NEXT_ARG();
}
- if (!ok && !iok) {
+ if (!ok) {
return -1;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH iproute2 net-next] tc: m_mirred: Fix parsing of 'index' optional argument
2016-10-21 20:01 [PATCH iproute2 net-next] tc: m_mirred: Fix parsing of 'index' optional argument Shmulik Ladkani
@ 2016-10-26 18:23 ` Stephen Hemminger
0 siblings, 0 replies; 2+ messages in thread
From: Stephen Hemminger @ 2016-10-26 18:23 UTC (permalink / raw)
To: Shmulik Ladkani; +Cc: Jamal Hadi Salim, netdev
On Fri, 21 Oct 2016 23:01:15 +0300
Shmulik Ladkani <shmulik.ladkani@gmail.com> wrote:
> Code in parse_mirred() suggests "index" argument can be placed either
> after the "egress" clause, or as the first argument (after "action mirred").
>
> However, parse_egress() fails to correctly parse "index" if it's the
> first argument.
>
> For example:
>
> # tc filter add ... action mirred index 5
> RTNETLINK answers: Invalid argument
> (unnecessary RTNETLINK issued, should have been parse error)
>
> # tc filter add ... action mirred index 5 egress redirect dev eth0
> bad action type egress
> (should have been parsed successfully)
>
> Fix parse_egress as follows:
> - continue parsing after valid "index" is seen
> - don't issue the RTNETLINK unless valid "egress" is seen
>
> Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Looks like a good fix but does not apply cleanly to current net-next branch.
Please cleanup and resubmit.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-10-26 18:22 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-21 20:01 [PATCH iproute2 net-next] tc: m_mirred: Fix parsing of 'index' optional argument Shmulik Ladkani
2016-10-26 18:23 ` Stephen Hemminger
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.