From owner-freebsd-net@FreeBSD.ORG Thu Jun 5 16:10:34 2014 Return-Path: Delivered-To: net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 9324BB12 for ; Thu, 5 Jun 2014 16:10:34 +0000 (UTC) Received: from mail-pb0-x22b.google.com (mail-pb0-x22b.google.com [IPv6:2607:f8b0:400e:c01::22b]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 5DCAC2E95 for ; Thu, 5 Jun 2014 16:10:34 +0000 (UTC) Received: by mail-pb0-f43.google.com with SMTP id up15so1325505pbc.2 for ; Thu, 05 Jun 2014 09:10:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:references:in-reply-to:subject:date:message-id :mime-version:content-type:thread-index:content-language; bh=7ITMaFxCFSHOW+DtI7rfJdv+cxcn463NKo5NyYfvgyg=; b=LVwHL2CKIJ1Lxd2TZINR34jg1E6kF9KHttd4mF8Gim12A64VTGnctxnSZ8VW+2SE9l 52ZPdy033RKo8bNjE2T7qeeNxIwrMw6vj8Q5SmZMJscpdkarIY2alEuQeQWlmDrgF6tV mmr4GoyEwPiUjDm+DUAuqUwiKiwuFS9FO2r6F6bLcN4Y09cKacbyhAdeHYt6WZD2HY9G SoSj1qdJ3DIL7tMwuwP/aC05csn0T8teZtoMt9H056c608KoAOeriiF/iB2uXdIrZN/6 udYeuPWzcZT/n4TGN/wEqSWjvITARyweJbWmaJaKFN/sEApRpxxXIJA/pGFBLYYExAp6 nQPA== X-Received: by 10.68.216.101 with SMTP id op5mr79067389pbc.148.1401984633893; Thu, 05 Jun 2014 09:10:33 -0700 (PDT) Received: from billwin7 (amx-tls2.starhub.net.sg. [203.116.164.12]) by mx.google.com with ESMTPSA id ak1sm24493269pbc.58.2014.06.05.09.10.29 for (version=TLSv1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 05 Jun 2014 09:10:32 -0700 (PDT) From: "bycn82" To: "'Luigi Rizzo'" References: <20140521204826.GA67124@onelab2.iet.unipi.it> <537E1029.70007@FreeBSD.org> <20140522154740.GA76448@onelab2.iet.unipi.it> <537E2153.1040005@FreeBSD.org> <20140522163812.GA77634@onelab2.iet.unipi.it> <538B2FE5.6070407@FreeBSD.org> <539044E4.1020904@ipfw.ru> <000c01cf80be$41194370$c34bca50$@gmail.com> <20140605134256.GA81234@onelab2.iet.unipi.it> <000001cf80cd$5dc1d9b0$19458d10$@gmail.com> <20140605155402.GA81905@onelab2.iet.unipi.it> In-Reply-To: <20140605155402.GA81905@onelab2.iet.unipi.it> Subject: RE: [CFT]: ipfw named tables / different tabletypes Date: Fri, 6 Jun 2014 00:10:26 +0800 Message-ID: <000401cf80d8$ad1bb840$075328c0$@gmail.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_NextPart_000_0005_01CF811B.BB40A5F0" X-Mailer: Microsoft Outlook 14.0 Thread-Index: AQIybStz9zMiwtQqal1c8BJmK0TJagKNzWvvAlvxzMYB26ii/AFrQU/SAWbxMPACXuZNZAL6quccAc5lezAC2ZyC3wK96E6hmeob/RA= Content-Language: en-us Cc: "'Alexander V. Chernikov'" , 'FreeBSD Net' X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 05 Jun 2014 16:10:34 -0000 This is a multipart message in MIME format. ------=_NextPart_000_0005_01CF811B.BB40A5F0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Sorry for waste you time to explain it again, I will read the code = first. And the latest patch of `PPS` should be OK, I checked the logic = carefully this time. I sent it out last weekend. logic as below, PPS actually will be fulfilled using `PPT`,(N packets = per M ticks). + case O_PPT: + case O_PPS:{ + ipfw_insn_pps *pps =3D (ipfw_insn_pps *)cmd; + if(pps->limit=3D=3D0){ + int limit,duration_in_ticks; + if(1000/hz > pps->duration){ + duration_in_ticks=3D1; + }else{ + duration_in_ticks=3D(pps->duration * hz +500)/1000; + }=09 + limit=3D(cmd->arg1 *1000 * duration_in_ticks + hz * = pps->duration/2)/(hz * pps->duration); + pps->limit=3Dlimit; + pps->duration_in_ticks=3Dduration_in_ticks; + } + if(pps->start_time+pps->duration_in_ticks>=3D ticks){ + if(pps->count < pps->limit){ + retval =3D IP_FW_PASS; + }else{ + retval =3D IP_FW_DENY; + } + pps->count++; + }else{ + pps->start_time=3Dticks; + pps->count=3D1; + retval =3D IP_FW_PASS; + } + l =3D 0; =09 + done =3D 1; + break;=09 + } =20 > -----Original Message----- > From: 'Luigi Rizzo' [mailto:rizzo@iet.unipi.it] > Sent: 05 June, 2014 23:54 > To: bycn82 > Cc: 'Alexander V. Chernikov'; 'FreeBSD Net' > Subject: Re: [CFT]: ipfw named tables / different tabletypes >=20 > On Thu, Jun 05, 2014 at 10:49:27PM +0800, bycn82 wrote: > > Hi Luigi, > > Yes, use string instead of integer for the ID of table, but the same > method cannot apply to the feature `set type of table`. And in the > kernel, compare string will cause more than compare an integer. In my > opinion, actually they are just alias name for the object. Users = already > accept that every object has an integer ID. >=20 > Bill, > I appreciate your enthusiasm on contributing to the project, but = before > starting to code, you (and everyone else, including myself) should = spend > some time at the drawing board, question your assumptions, and = consider > possible implementations. >=20 > Also the fact that previous implementations had bugs or restrictions > does not imply that we should repeat the same mistakes now. >=20 > Specifically: >=20 > - comparing strings in the kernel is perfectly fine, we do it all the > time (sysctl names, filenames, interfaces and whatnot). > What would be terribly wrong is having to do, on every packet, > an expensive string or number lookup (note- it's the entire lookup > process, not a string comparison) to locate the table. > However, this is not the case. > The way to implement named objects (tables etc.) is to accept = strings > as object names, and when the rule is added the string is checked > and converted to a pointers or an integer which permits direct > access to the table. > This is as fast as it can be at runtime, which is all what matters. >=20 > - at the moment tables and pipes have a single ID, which happens to be = a > sequence of digits for mostly historical reasons (you can read it > as laziness of the original authors; i can take the blame for = pipes). >=20 > If we extend the namespace to include strings we improve the user's > experience and remain compatible with the existing user interface. >=20 > Instead, if we require users to create a mapping before using > alphanumeric names, not only we add a step that was not there = before, > but also create two names for the same object which makes it harder > to reason about the rulesets, and there is also the issue of how > to handle references to non-existing tables (which are trivially > supported now or with the approach i suggested, and would instead > require rescanning the ruleset whenever a name-number association > is added or deleted. > Another troubles with that approach is that you opening a race > window between the creation of the name-number mapping and its use, > and this is also something you don't want in a security-related = tool. >=20 > We have had a similar discussion about your 'pps' extension for ipfw: > useful feature, but your various implementations have issues (even the > final one), and we have only so much time for reviewing patches; = please > do not burn it. >=20 > cheers > luigi ------=_NextPart_000_0005_01CF811B.BB40A5F0 Content-Type: application/octet-stream; name="pps_and_ppt.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="pps_and_ppt.patch" Index: sbin/ipfw/ipfw.8=0A= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=0A= --- sbin/ipfw/ipfw.8 (revision 266886)=0A= +++ sbin/ipfw/ipfw.8 (working copy)=0A= @@ -602,6 +602,22 @@=0A= Note: logging is done after all other packet matching conditions=0A= have been successfully verified, and before performing the final=0A= action (accept, deny, etc.) on the packet.=0A= +.It Cm pps Ar limit duration=0A= +Rule with the =0A= +.Cm pps=0A= +keyword will allow the first=0A= +.Ar limit=0A= +packets in recent =0A= +.Ar duration =0A= +milliseconds.=0A= +.It Cm ppt Ar limit duration=0A= +Rule with the =0A= +.Cm ppt=0A= +keyword will allow the first=0A= +.Ar limit=0A= +packets in recent =0A= +.Ar duration =0A= +ticks.=0A= .It Cm tag Ar number=0A= When a packet matches a rule with the=0A= .Cm tag=0A= Index: sbin/ipfw/ipfw2.c=0A= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=0A= --- sbin/ipfw/ipfw2.c (revision 266886)=0A= +++ sbin/ipfw/ipfw2.c (working copy)=0A= @@ -244,6 +244,8 @@=0A= { "allow", TOK_ACCEPT },=0A= { "permit", TOK_ACCEPT },=0A= { "count", TOK_COUNT },=0A= + { "pps", TOK_PPS },=0A= + { "ppt", TOK_PPT },=0A= { "pipe", TOK_PIPE },=0A= { "queue", TOK_QUEUE },=0A= { "divert", TOK_DIVERT },=0A= @@ -1232,6 +1234,20 @@=0A= PRINT_UINT_ARG("skipto ", cmd->arg1);=0A= break;=0A= =0A= + case O_PPS:=0A= + {=0A= + ipfw_insn_pps *pps=3D(ipfw_insn_pps *)cmd;=0A= + printf("pps %d %d",cmd->arg1,pps->duration);=0A= + break; =0A= + }=0A= + =0A= + case O_PPT:=0A= + {=0A= + ipfw_insn_pps *pps=3D(ipfw_insn_pps *)cmd;=0A= + printf("ppt %d %d",pps->limit,pps->duration_in_ticks);=0A= + break; =0A= + }=0A= +=0A= case O_PIPE:=0A= PRINT_UINT_ARG("pipe ", cmd->arg1);=0A= break;=0A= @@ -2985,7 +3001,43 @@=0A= case TOK_COUNT:=0A= action->opcode =3D O_COUNT;=0A= break;=0A= + =0A= + case TOK_PPS:=0A= + action->opcode =3D O_PPS;=0A= + ipfw_insn_pps *pps =3D (ipfw_insn_pps *)action;=0A= + action->len =3D F_INSN_SIZE(ipfw_insn_pps);=0A= + if (isdigit(**av)) {=0A= + action->arg1 =3D strtoul(*av, NULL, 10);=0A= + av++;=0A= + }else{=0A= + errx(EX_USAGE, "illegal argument pps `limit` %s", *av);=0A= + }=0A= + if (isdigit(**av)) {=0A= + pps->duration=3D strtoul(*av, NULL, 10);=0A= + av++; =0A= + }else{=0A= + errx(EX_USAGE,"illegal arugment pps `duration` %s", *av);=0A= + }=0A= + break; =0A= =0A= + case TOK_PPT:=0A= + action->opcode =3D O_PPT;=0A= + ipfw_insn_pps *ppt =3D (ipfw_insn_pps *)action;=0A= + action->len =3D F_INSN_SIZE(ipfw_insn_pps);=0A= + if (isdigit(**av)) {=0A= + ppt->limit =3D strtoul(*av, NULL, 10);=0A= + av++;=0A= + }else{=0A= + errx(EX_USAGE, "illegal argument ppt `limit` %s", *av);=0A= + }=0A= + if (isdigit(**av)) {=0A= + ppt->duration_in_ticks=3D strtoul(*av, NULL, 10);=0A= + av++; =0A= + }else{=0A= + errx(EX_USAGE,"illegal arugment ppt `ticks` %s", *av);=0A= + }=0A= + break; =0A= +=0A= case TOK_NAT:=0A= action->opcode =3D O_NAT;=0A= action->len =3D F_INSN_SIZE(ipfw_insn_nat);=0A= Index: sbin/ipfw/ipfw2.h=0A= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=0A= --- sbin/ipfw/ipfw2.h (revision 266886)=0A= +++ sbin/ipfw/ipfw2.h (working copy)=0A= @@ -92,6 +92,8 @@=0A= TOK_NGTEE,=0A= TOK_FORWARD,=0A= TOK_SKIPTO,=0A= + TOK_PPS,=0A= + TOK_PPT,=0A= TOK_DENY,=0A= TOK_REJECT,=0A= TOK_RESET,=0A= Index: sys/netinet/ip_fw.h=0A= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=0A= --- sys/netinet/ip_fw.h (revision 266886)=0A= +++ sys/netinet/ip_fw.h (working copy)=0A= @@ -165,6 +165,8 @@=0A= O_REJECT, /* arg1=3Dicmp arg (same as deny) */=0A= O_COUNT, /* none */=0A= O_SKIPTO, /* arg1=3Dnext rule number */=0A= + O_PPS, /* arg1=3Dlimit, pps->duration */=0A= + O_PPT, /* pps->limit, pps->duration_in_ticks */=0A= O_PIPE, /* arg1=3Dpipe number */=0A= O_QUEUE, /* arg1=3Dqueue number */=0A= O_DIVERT, /* arg1=3Dport number */=0A= @@ -378,6 +380,18 @@=0A= } ipfw_insn_log;=0A= =0A= /*=0A= + * This is used for PPS=0A= + */=0A= +typedef struct _ipfw_insn_pps{=0A= + ipfw_insn o;=0A= + uint32_t start_time;=0A= + uint32_t count;=0A= + uint32_t duration; =0A= + uint32_t limit;=0A= + uint32_t duration_in_ticks;=0A= +} ipfw_insn_pps;=0A= +=0A= +/*=0A= * Data structures required by both ipfw(8) and ipfw(4) but not part of = the=0A= * management API are protected by IPFW_INTERNAL.=0A= */=0A= Index: sys/netpfil/ipfw/ip_fw2.c=0A= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=0A= --- sys/netpfil/ipfw/ip_fw2.c (revision 266886)=0A= +++ sys/netpfil/ipfw/ip_fw2.c (working copy)=0A= @@ -2188,6 +2188,37 @@=0A= skip_or =3D 0;=0A= continue;=0A= break; /* not reached */=0A= + =0A= + case O_PPT:=0A= + case O_PPS:{=0A= + ipfw_insn_pps *pps =3D (ipfw_insn_pps *)cmd;=0A= + if(pps->limit=3D=3D0){=0A= + int limit,duration_in_ticks;=0A= + if(1000/hz > pps->duration){=0A= + duration_in_ticks=3D1;=0A= + }else{=0A= + duration_in_ticks=3D(pps->duration * hz +500)/1000;=0A= + } =0A= + limit=3D(cmd->arg1 *1000 * duration_in_ticks + hz * = pps->duration/2)/(hz * pps->duration);=0A= + pps->limit=3Dlimit;=0A= + pps->duration_in_ticks=3Dduration_in_ticks;=0A= + }=0A= + if(pps->start_time+pps->duration_in_ticks>=3D ticks){=0A= + if(pps->count < pps->limit){=0A= + retval =3D IP_FW_PASS;=0A= + }else{=0A= + retval =3D IP_FW_DENY;=0A= + }=0A= + pps->count++;=0A= + }else{=0A= + pps->start_time=3Dticks;=0A= + pps->count=3D1;=0A= + retval =3D IP_FW_PASS;=0A= + }=0A= + l =3D 0; =0A= + done =3D 1;=0A= + break; =0A= + }=0A= =0A= case O_CALLRETURN: {=0A= /*=0A= Index: sys/netpfil/ipfw/ip_fw_sockopt.c=0A= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=0A= --- sys/netpfil/ipfw/ip_fw_sockopt.c (revision 266886)=0A= +++ sys/netpfil/ipfw/ip_fw_sockopt.c (working copy)=0A= @@ -703,6 +703,13 @@=0A= goto bad_size;=0A= break;=0A= =0A= + case O_PPS:=0A= + case O_PPT:=0A= + have_action=3D1;=0A= + if (cmdlen !=3D F_INSN_SIZE(ipfw_insn_pps))=0A= + goto bad_size;=0A= + break;=0A= +=0A= case O_PIPE:=0A= case O_QUEUE:=0A= if (cmdlen !=3D F_INSN_SIZE(ipfw_insn))=0A= ------=_NextPart_000_0005_01CF811B.BB40A5F0--