misc/178317: IPFW options need to specifed in specific order

[Luigi Rizzo]

The following reply was made to PR kern/178317; it has been noted by GNATS.

From: Luigi Rizzo <rizzo at iet.unipi.it>
To: Kirill Diduk <kirill.diduk at gmail.com>
Cc: bug-followup at FreeBSD.org, jens.kassel at aptilo.com, luigi at FreeBSD.org
Subject: Re: misc/178317: IPFW options need to specifed in specific order
Date: Sun, 5 May 2013 15:51:10 +0200

 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