I have been writing on a new match "superlimit" with uses the existing limit match as base. To be short this new match limits based on pair of source/mask and dest/mask. Anyhow, i found a couple of strange things in the existing limit:
1. The lock is ONE global lock. Used for all instances of limit. Why is it so? Just a "bug"? It should be in the ipt_ratelimti stuct and have one lock for each instance of limit. 2. The burst argument is as far as i can see and understand the algorithm a multiplyer for the normal --limit argument. Look at this example: --limit 40/s --limit-burst 80 The above gives you now a limit of 40 packets per second when you have reached 40*80 packets per second. I think we want it to limit it down to 40 pps when we reach 80 pps. Not use it as a multiplyer. Both the help and the HOWTO seams to handle the burst as an absolut figure rather than a multiplyer. >From the Help: --limit-burst number number to match in a burst, default 5 >From the HOWTO: Say we say match one packet per second with a five packet burst, ... Also locking at the code we have: r->credit = user2credits(r->avg * r->burst); /* Credits full. */ r->credit_cap = user2credits(r->avg * r->burst); /* Credits full. */ Meaning that we take the avg calculated credits value (recalculated alot to fit the algorithm) and just multiply it by the unmodified (not changed since read from commandprompt) burst value. Am i wrong here? Shouldn't we calculate the burst here in same way as avg and change it to: r->credit = user2credits(r->burst); /* Credits full. */ r->credit_cap = user2credits(r->burst); /* Credits full. */ Meaning that we start at the absolut burst value. One other thing. IF its really intended to be a multiplyer it lacks feature only being able to multiply integers (1,2,3...). Second IF its intended to be a multiplyer the HOWTO and help should be change to state this more clearly, e.g. don't use the example of 1 packet/sec since --burst 5 will be the same if its a multiplyer or an absolut value. I might have interpered the algorithm completly wrong. Please correct me and explain how it works if so. 3. At the end of ipt_limit_checkentry we can find: /* For SMP, we only want to use one set of counters. */ r->master = r; Now, i dont really understand how a match works. But is the match copied to one for each CPU? Why is that? Too keep the memory faster? About every match that needs to modify its data need to have the one and same data structure. The ones that don't need to modify can use the same memory. It wont write in the memory, those not destroying the cache for the other cpus. How can I in my new match make sure that i only get one. I'm going to use some memory and don't want to waste memory by initing copies that i don't use. 4. This is more of features that i'm about to add: - An inverter so u can match inverted: iptables -A INPUT -m limit --limit ! 40/s -j DRP Watch the !. It will now send all packets NOT matching to DROP. Saving you one rule afterwards of needing to DROP. - A match for bytes/second as well. Ranging from 12.5MegaByte/sec to 1 byte per 32 seconds. You can know limit the traffic flow, not just packets per second. It uses the same algorithm but a much different way of calculating the value used in the algorithm to prevent under/overflows. These two addments are easy to add. However, changing the semantics of burst (as i think is a flaw above) will break many scripts around the world. Maybe I should add these new featues in a new match "newlimit"? Thank you for your time with these wonderings. -- /Gozem A.K.A. Joakim Axelsson
