Re: divert-to with port range
On Tue, Sep 17, 2013 at 03:42:28PM -0400, Dave Anderson wrote: This appears to be intended to divide connections equally among five ports, but (given that the probability applies only to the packets which actually reach the rule) doesn't it actually divide them as 20%, 16%, 12.8%, 10.24%, 40.96%? To get an (approximately) equal distribution I think you'd need to use probabilities 20%, 25%, 33%, 50%. Requirement was just to simplify existing config as is, but this is interesting point. If using a port range were to implicitly divide connections equally among those ports, this problem would go away. But that's not what your patch does. Ok, will take a look and see what can be done. Thanks.
divert-to with port range
Hi I've been asked, by net admin, to implement pf.conf simplification for divert-to rule. Reason is that divert-to is written to support only one port per line and because of that there are situations where admins must write lot of lines only because different ports. After looking at pfctl/parse.y I've found that patch (for 5.3) would be trivial and wouldn't break anything, ie. works for one port and port range at the same time. Please let me know if there is interest for this and ofc if something needs to be fixed. Here is an example. Now: pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 127.0.0.1 port 42240 modulate state probability 20% pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 127.0.0.1 port 42241 modulate state probability 20% pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 127.0.0.1 port 42242 modulate state probability 20% pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 127.0.0.1 port 42243 modulate state probability 20% pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 127.0.0.1 port 42244 modulate state After patching: pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 127.0.0.1 port 42240:42243 modulate state probability 20% pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 127.0.0.1 port 42244 modulate state Patch: Index: parse.y === RCS file: /cvs/src/sbin/pfctl/parse.y,v retrieving revision 1.621 diff -u -r1.621 parse.y --- parse.y 16 Jan 2013 01:49:20 - 1.621 +++ parse.y 17 Sep 2013 15:45:20 - @@ -261,7 +261,7 @@ u_int8_t set_prio[2]; struct { struct node_host*addr; - u_int16_t port; + u_int16_t port, port_top; }divert, divert_packet; struct redirspec nat; struct redirspec rdr; @@ -475,7 +475,7 @@ %type v.i sourcetrack flush unaryop statelock %type v.b action %type v.b flags flag blockspec prio -%type v.range portplain portstar portrange +%type v.range portstar portrange %type v.hashkey hashkey %type v.proto proto proto_list proto_item %type v.number protoval @@ -2078,6 +2078,28 @@ r.divert.addr = $8.divert.addr-addr.v.a.addr; } + if ($8.divert.port_top + $8.divert.port_top r.divert.port) { + yyerror(invalid divert port range: + %u:%u, ntohs(r.divert.port), + ntohs($8.divert.port_top)); + YYERROR; + } + +#define NHS_LT(x, y) (ntohs(x) ntohs(y)) +#define NHS_INC(x) x = htons(ntohs(x) + 1) + while(NHS_LT(r.divert.port, + $8.divert.port_top)) { + expand_rule(r, 1, $4, $8.nat, $8.rdr, + $8.rroute, $6, $7.src_os, + $7.src.host, $7.src.port, + $7.dst.host, $7.dst.port, + $8.uid, $8.gid, $8.rcv, + $8.icmpspec, ); + NHS_INC(r.divert.port); + } +#undef NHS_INC +#undef NHS_LT } r.divert_packet.port = $8.divert_packet.port; @@ -2197,7 +2219,7 @@ } filter_opts.rtableid = $2; } - | DIVERTTO STRING PORT portplain { + | DIVERTTO STRING PORT portrange { if ((filter_opts.divert.addr = host($2)) == NULL) { yyerror(could not parse divert address: %s, $2); @@ -2210,6 +2232,7 @@ yyerror(invalid divert port: %u, ntohs($4.a)); YYERROR; } + filter_opts.divert.port_top = $4.b; } | DIVERTREPLY { filter_opts.divert.port = 1;/* some random value */ @@ -3073,15 +3096,6 @@ $$-op = $2; $$-next = NULL; $$-tail = $$; - } - ; - -portplain : numberstring
Re: divert-to with port range
On Tue, 17 Sep 2013, Ivan Popovski wrote: Hi I've been asked, by net admin, to implement pf.conf simplification for divert-to rule. Reason is that divert-to is written to support only one port per line and because of that there are situations where admins must write lot of lines only because different ports. After looking at pfctl/parse.y I've found that patch (for 5.3) would be trivial and wouldn't break anything, ie. works for one port and port range at the same time. Please let me know if there is interest for this and ofc if something needs to be fixed. Here is an example. Now: pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 127.0.0.1 port 42240 modulate state probability 20% pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 127.0.0.1 port 42241 modulate state probability 20% pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 127.0.0.1 port 42242 modulate state probability 20% pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 127.0.0.1 port 42243 modulate state probability 20% pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 127.0.0.1 port 42244 modulate state This appears to be intended to divide connections equally among five ports, but (given that the probability applies only to the packets which actually reach the rule) doesn't it actually divide them as 20%, 16%, 12.8%, 10.24%, 40.96%? To get an (approximately) equal distribution I think you'd need to use probabilities 20%, 25%, 33%, 50%. If using a port range were to implicitly divide connections equally among those ports, this problem would go away. But that's not what your patch does. Dave After patching: pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 127.0.0.1 port 42240:42243 modulate state probability 20% pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 127.0.0.1 port 42244 modulate state Patch: Index: parse.y === RCS file: /cvs/src/sbin/pfctl/parse.y,v retrieving revision 1.621 diff -u -r1.621 parse.y --- parse.y16 Jan 2013 01:49:20 - 1.621 +++ parse.y17 Sep 2013 15:45:20 - @@ -261,7 +261,7 @@ u_int8_t set_prio[2]; struct { struct node_host*addr; - u_int16_t port; + u_int16_t port, port_top; }divert, divert_packet; struct redirspec nat; struct redirspec rdr; @@ -475,7 +475,7 @@ %type v.i sourcetrack flush unaryop statelock %type v.b action %type v.b flags flag blockspec prio -%type v.range portplain portstar portrange +%type v.range portstar portrange %type v.hashkey hashkey %type v.proto proto proto_list proto_item %type v.number protoval @@ -2078,6 +2078,28 @@ r.divert.addr = $8.divert.addr-addr.v.a.addr; } + if ($8.divert.port_top + $8.divert.port_top r.divert.port) { + yyerror(invalid divert port range: + %u:%u, ntohs(r.divert.port), + ntohs($8.divert.port_top)); + YYERROR; + } + +#define NHS_LT(x, y) (ntohs(x) ntohs(y)) +#define NHS_INC(x) x = htons(ntohs(x) + 1) + while(NHS_LT(r.divert.port, + $8.divert.port_top)) { + expand_rule(r, 1, $4, $8.nat, $8.rdr, + $8.rroute, $6, $7.src_os, + $7.src.host, $7.src.port, + $7.dst.host, $7.dst.port, + $8.uid, $8.gid, $8.rcv, + $8.icmpspec, ); + NHS_INC(r.divert.port); + } +#undef NHS_INC +#undef NHS_LT } r.divert_packet.port = $8.divert_packet.port; @@ -2197,7 +2219,7 @@ } filter_opts.rtableid = $2; } - | DIVERTTO STRING PORT portplain { + | DIVERTTO STRING PORT portrange { if ((filter_opts.divert.addr = host($2)) == NULL) { yyerror(could not parse divert address: %s, $2); @@ -2210,6 +2232,7 @@ yyerror(invalid divert port: %u, ntohs($4
Re: divert-to with port range
The patch is extending the rules, so i dont see how it could behave differently The original set of percentage is still strange so you have a point. Unless they expect this behavior (they still end with the good 100% rules) isn't it possible to round robin this ? with relayd or something else ? On Tue, Sep 17, 2013 at 3:42 PM, Dave Anderson d...@daveanderson.comwrote: On Tue, 17 Sep 2013, Ivan Popovski wrote: Hi I've been asked, by net admin, to implement pf.conf simplification for divert-to rule. Reason is that divert-to is written to support only one port per line and because of that there are situations where admins must write lot of lines only because different ports. After looking at pfctl/parse.y I've found that patch (for 5.3) would be trivial and wouldn't break anything, ie. works for one port and port range at the same time. Please let me know if there is interest for this and ofc if something needs to be fixed. Here is an example. Now: pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 127.0.0.1 port 42240 modulate state probability 20% pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 127.0.0.1 port 42241 modulate state probability 20% pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 127.0.0.1 port 42242 modulate state probability 20% pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 127.0.0.1 port 42243 modulate state probability 20% pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 127.0.0.1 port 42244 modulate state This appears to be intended to divide connections equally among five ports, but (given that the probability applies only to the packets which actually reach the rule) doesn't it actually divide them as 20%, 16%, 12.8%, 10.24%, 40.96%? To get an (approximately) equal distribution I think you'd need to use probabilities 20%, 25%, 33%, 50%. If using a port range were to implicitly divide connections equally among those ports, this problem would go away. But that's not what your patch does. Dave After patching: pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 127.0.0.1 port 42240:42243 modulate state probability 20% pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 127.0.0.1 port 42244 modulate state Patch: Index: parse.y === RCS file: /cvs/src/sbin/pfctl/parse.y,v retrieving revision 1.621 diff -u -r1.621 parse.y --- parse.y16 Jan 2013 01:49:20 - 1.621 +++ parse.y17 Sep 2013 15:45:20 - @@ -261,7 +261,7 @@ u_int8_t set_prio[2]; struct { struct node_host*addr; - u_int16_t port; + u_int16_t port, port_top; }divert, divert_packet; struct redirspec nat; struct redirspec rdr; @@ -475,7 +475,7 @@ %type v.i sourcetrack flush unaryop statelock %type v.b action %type v.b flags flag blockspec prio -%type v.range portplain portstar portrange +%type v.range portstar portrange %type v.hashkey hashkey %type v.proto proto proto_list proto_item %type v.number protoval @@ -2078,6 +2078,28 @@ r.divert.addr = $8.divert.addr-addr.v.a.addr; } + if ($8.divert.port_top + $8.divert.port_top r.divert.port) { + yyerror(invalid divert port range: + %u:%u, ntohs(r.divert.port), + ntohs($8.divert.port_top)); + YYERROR; + } + +#define NHS_LT(x, y) (ntohs(x) ntohs(y)) +#define NHS_INC(x) x = htons(ntohs(x) + 1) + while(NHS_LT(r.divert.port, + $8.divert.port_top)) { + expand_rule(r, 1, $4, $8.nat, $8.rdr, + $8.rroute, $6, $7.src_os, + $7.src.host, $7.src.port, + $7.dst.host, $7.dst.port, + $8.uid, $8.gid, $8.rcv, + $8.icmpspec, ); + NHS_INC(r.divert.port); + } +#undef NHS_INC +#undef NHS_LT } r.divert_packet.port = $8.divert_packet.port; @@ -2197,7 +2219,7