Date: Sat, 16 Jul 2005 09:53:54 -0700 From: Luigi Rizzo <rizzo@icir.org> To: Max Laier <max@love2party.net> Cc: freebsd-ipfw@freebsd.org, freebsd-net@freebsd.org, Chris Dionissopoulos <dionch@freemail.gr> Subject: Re: Traffic quota features in IPFW Message-ID: <20050716095353.B86993@xorpc.icir.org> In-Reply-To: <200507161740.38234.max@love2party.net>; from max@love2party.net on Sat, Jul 16, 2005 at 05:40:32PM %2B0200 References: <001c01c58a17$5dbe4a40$0100000a@R3B> <200507161740.38234.max@love2party.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Jul 16, 2005 at 05:40:32PM +0200, Max Laier wrote: > On Saturday 16 July 2005 17:02, Chris Dionissopoulos wrote: > > Hi ppl, ( and sorry for cross posting) > > > > I review Andrey's Elsukov patch for adding "bound" support in ipfw, and i > > decide to push a little forward this feature. > > Sorry to be blunt, but I don't see the point in this feature nor do I think > it's a good idea. All it does is adding overhead to every packet that is > processed by IPFW. You might argue that this overhead is fairly little, but max, you are entitled to dislike the idea, but you should present your arguments correctly and not in a misleading way. There is no extra per-packet overhead in the common case introduced by this particular option (and in practically all new options added to ipfw2) because all it adds is a few entries to the main switch. Re. readability, you surely know very well (and it's widely documented through the ip_fw2.[ch] code) that each IPFW2 opcode is independent of others, so to understand the main function you just need to understand the code outside the switch (which grabs the packets' data), and the individual case you are looking at - which does a 'break, break 2 or break 3' depending on the case (and not having the 'break n' construct in C we are forced to use gotos). Surely the more opcodes you have, the bigger the switch becomes, but i don't see readability suffering too much. In any case it would be trivial to move to a different structure where each opcode handler is called through an indirect function and depending on the return value one does a break, break2 or break 3. I don't have a particular interest in this patch, i think it could be done in a better way (e.g. by using a single opcode for below/above, and a more efficient check-state perhaps) but none of your criticism really applies to the code as it has been submitted. "sorry to be blunt" :) cheers luifgi > if you combine the last ten "neat to have though not really necessary" > features this adds up. Also the code is getting more and more hacked up. > Your feature might be nicely done, but it adds to the main switch-loops > making them more and more unreadable until it all falls over and nobody is > willing to touch the code anymore. I have seen (too) much ipfw code lately > while tieing together lose ends in the IPv6-import and it's already messy > enough. > > I urge you to reconsider if we really need this. If you think we can't live > without it, it'd be nice if you could come up with a clean(er) way to extend > IPFW with additional stuff like this without impact to performance and > maintainability for the common case (without the magic foobar-option of the > day). Thanks. > > BTW: This function can be done with a three line awk-skript without any effect > on performance. Of course you will lose some precision, but I don't see > applications where you have to be *that* percise. > > > You can see the whole picture in there: > > http://www.freebsd.org/cgi/query-pr.cgi?pr=80642 > > and there: > > http://butcher.heavennet.ru/ > > > > In my patch, 3 new options are added: > > 1. "below <VALUE>" (which is the same option as Andrey's "bound" option, I > > just rename it) 2. "above <VALUE>" which is the oposite option of "below". > > Match rules when the counter is above <value> 3. "check-quota" (which is > > the same option as Andrey's "check-bound" , but now applies to both "above" > > and "below" options). > > > > Notes: > > 1. Patch is against releng_6. > > 2. I also include a more compicated example which is (IMHO) a complete > > traffic quota+shaping solution for a small (or not so small) ISP. > > 3. For installation, follow the instructions Adrey publish in his webspace: > > http://butcher.heavennet.ru/ > > 4. Patch doesn't breaks ipfw ABI (today) , because adds new options at the > > end of list. If you apply this patch in a month or so, I cannot guarantee > > success. > > 5. Please test, and send me your feedbacks. > > > > > > I 'll be happy if you find usefull these features and if any developer > > commits this patch in current or releng_6 branch. > > -- > /"\ Best regards, | mlaier@freebsd.org > \ / Max Laier | ICQ #67774661 > X http://pf4freebsd.love2party.net/ | mlaier@EFnet > / \ ASCII Ribbon Campaign | Against HTML Mail and News
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20050716095353.B86993>