Bug 178317 - [ipfw] ipfw options need to specifed in specific order
Summary: [ipfw] ipfw options need to specifed in specific order
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: Unspecified
Hardware: Any Any
: Normal Affects Only Me
Assignee: Tom Jones
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-05-03 11:00 UTC by Jens Kassel
Modified: 2020-09-19 16:51 UTC (History)
1 user (show)

See Also:


Attachments
patch_01.txt (336 bytes, text/plain; charset=US-ASCII)
2013-05-05 12:35 UTC, kirill.diduk
no flags Details
patch_02.txt (419 bytes, text/plain; charset=US-ASCII)
2013-05-05 12:35 UTC, kirill.diduk
no flags Details
patch.txt (419 bytes, text/plain; charset=US-ASCII)
2013-05-06 11:06 UTC, kirill.diduk
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jens Kassel 2013-05-03 11:00:00 UTC
In FreeBSD 7.2 this command was successful

ipfw pipe 3 config bw 1000000kbit/s mask src-ip 0xffffffff queue 92

but with current FreeBSD 8.4-RC2 i must run the command as

ipfw pipe 3 config bw 1000000kbit/s queue 92 mask src-ip 0xffffffff

otherwise I get the error

ipfw: unrecognised option ``92''

Accordning to man page both ways should be correct.

Fix: 

Switch order of arguments.
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2013-05-04 22:23:57 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-ipfw

Over to maintainer(s).
Comment 2 kirill.diduk 2013-05-05 12:35:44 UTC
Hello,

The problem is related to the command line parsing implementation in
the file "sbin/ipfw/dummynet.c" (function "ipfw_config_pipe").

Consider the example:

# ipfw pipe 3 config bw 1000000kbit/s mask src-ip 0xffffffff queue 92

When the "mask" token is encountered, it starts to parse FLOW_MASK
options ('src-ip", etc.), and skips the "queue" option. After that,
"92" is parsed as a standalone option which causes an "unrecognised
option" error.

I suggest a simple solution that fixes this problem (attached as
"patch_01.txt").

--------------------------------------------------------------------------------
--- /usr/src/sbin/ipfw/dummynet.c.orig	2013-04-21 01:39:08.000000000 +0000
+++ /usr/src/sbin/ipfw/dummynet.c	2013-05-05 08:45:58.000000000 +0000
@@ -929,6 +929,7 @@
 			    case TOK_QUEUE:
 				    mask->extra = ~0;
 				    *flags |= DN_HAVE_MASK;
+				    ac++; av--; /* backtrack */
 				    goto end_mask;

 			    case TOK_DSTIP:
--------------------------------------------------------------------------------


Also, there is a more elegant solution (attached as "patch_02.txt"),
but I'm not sure about it :

--------------------------------------------------------------------------------
--- /usr/src/sbin/ipfw/dummynet.c.orig	2013-04-21 01:39:08.000000000 +0000
+++ /usr/src/sbin/ipfw/dummynet.c	2013-05-05 10:03:40.000000000 +0000
@@ -926,11 +926,6 @@
 				    *flags |= DN_HAVE_MASK;
 				    goto end_mask;

-			    case TOK_QUEUE:
-				    mask->extra = ~0;
-				    *flags |= DN_HAVE_MASK;
-				    goto end_mask;
-
 			    case TOK_DSTIP:
 				    mask->addr_type = 4;
 				    p32 = &mask->dst_ip;
--------------------------------------------------------------------------------


Luigi, could you help, please? Can we remove the whole case-branch
"TOK_QUEUE" here?

If no, we can apply the first solution ("patch_01.txt"): it restores
the previous behavior of the "ipfw" command line parsing.


Thanks,
Kirill

P.S. This issue is also observed on other FreeBSD releases (e.g. 9.0 and 9.1).
Comment 3 rizzo 2013-05-05 14:51:10 UTC
On Sun, May 05, 2013 at 02:35:44PM +0300, Kirill Diduk wrote:
> Hello,
> 
> The problem is related to the command line parsing implementation in
> the file "sbin/ipfw/dummynet.c" (function "ipfw_config_pipe").
> 
> Consider the example:
> 
> # ipfw pipe 3 config bw 1000000kbit/s mask src-ip 0xffffffff queue 92
> 
> When the "mask" token is encountered, it starts to parse FLOW_MASK
> options ('src-ip", etc.), and skips the "queue" option. After that,
> "92" is parsed as a standalone option which causes an "unrecognised
> option" error.
> 
> I suggest a simple solution that fixes this problem (attached as
> "patch_01.txt").
> 
> --------------------------------------------------------------------------------
> --- /usr/src/sbin/ipfw/dummynet.c.orig	2013-04-21 01:39:08.000000000 +0000
> +++ /usr/src/sbin/ipfw/dummynet.c	2013-05-05 08:45:58.000000000 +0000
> @@ -929,6 +929,7 @@
>  			    case TOK_QUEUE:
>  				    mask->extra = ~0;
>  				    *flags |= DN_HAVE_MASK;
> +				    ac++; av--; /* backtrack */
>  				    goto end_mask;
> 
>  			    case TOK_DSTIP:
> --------------------------------------------------------------------------------
> 
> 
> Also, there is a more elegant solution (attached as "patch_02.txt"),
> but I'm not sure about it :
> 
> --------------------------------------------------------------------------------
> --- /usr/src/sbin/ipfw/dummynet.c.orig	2013-04-21 01:39:08.000000000 +0000
> +++ /usr/src/sbin/ipfw/dummynet.c	2013-05-05 10:03:40.000000000 +0000
> @@ -926,11 +926,6 @@
>  				    *flags |= DN_HAVE_MASK;
>  				    goto end_mask;
> 
> -			    case TOK_QUEUE:
> -				    mask->extra = ~0;
> -				    *flags |= DN_HAVE_MASK;
> -				    goto end_mask;
> -
>  			    case TOK_DSTIP:
>  				    mask->addr_type = 4;
>  				    p32 = &mask->dst_ip;
> --------------------------------------------------------------------------------
> 
> 
> Luigi, could you help, please? Can we remove the whole case-branch
> "TOK_QUEUE" here?
> 
> If no, we can apply the first solution ("patch_01.txt"): it restores
> the previous behavior of the "ipfw" command line parsing.

i haven't touched this code in a while, so i will let it
to your judgement. I suppose that any non-mask option
(e.g. queue, plr, delay, ... ) should exit the flow-mask
parsing. The second patch seems more appropriate.

cheers
luigi
Comment 4 kirill.diduk 2013-05-06 11:06:11 UTC
OK, here is the final solution (attached as "patch.txt"). I reviewed
it once again.

Now, someone who has write-access to the FreeBSD svn, may have to apply it.

Thanks,
Kirill
Comment 5 Eitan Adler freebsd_committer freebsd_triage 2017-12-31 08:00:44 UTC
For bugs matching the following criteria:

Status: In Progress Changed: (is less than) 2014-06-01

Reset to default assignee and clear in-progress tags.

Mail being skipped