Date: Fri, 6 Jun 2014 00:10:26 +0800 From: "bycn82" <bycn82@gmail.com> To: "'Luigi Rizzo'" <rizzo@iet.unipi.it> Cc: "'Alexander V. Chernikov'" <melifaro@ipfw.ru>, 'FreeBSD Net' <net@FreeBSD.org> Subject: RE: [CFT]: ipfw named tables / different tabletypes Message-ID: <000401cf80d8$ad1bb840$075328c0$@gmail.com> In-Reply-To: <20140605155402.GA81905@onelab2.iet.unipi.it> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?000401cf80d8$ad1bb840$075328c0$>