Josh Hunt <johunt@xxxxxxxxxx> wrote:
> Below is a first pass attempt at fixing a problem we've come across when
> trying to do an iptables-restore where the hashlimit name stays the same, but
> one of the hashlimit parameters changes but does not take affect.
>
> For ex, if you have an existing hashlimit rule, do an iptables-save, change the
> rate for that rule, and then do an iptables-restore the new rate will not be
> enforced.
>
> This appears to be due to a problem where hashlimit only checks for existing
> hashes by name and family and does not consider any of the other config
> parameters.
>
> I've attempted to fix this by having it check for all hashlimit config params,
> this way it doesn't accidentally match just on name. This brought up an issue
> of having to make hashlimit aware of how many references there are to its
> proc entry.
>
> I'm not submitting this for inclusion yet, but for feedback. Mainly on the approach
> and if there's possibly a better way of resolving this problem. My handling of
> the proc "problem" is pretty messy right now and possibly incomplete, but the
> patch below allows the case I described above to pass now. I hope to clean up
> the proc handling in a v2.
>
> Any feedback is greatly appreciated.
[ ignoring proc issue you pointed out ]
> from:
> -A INPUT -s 10.18.40.44/32 -i eth1 -p tcp -m hashlimit --hashlimit-upto 10/sec --hashlimit-burst 10 --hashlimit-name test -j ACCEPT
>
> to:
> -A INPUT -s 10.18.40.44/32 -i eth1 -p tcp -m hashlimit --hashlimit-upto 1000/sec --hashlimit-burst 10 --hashlimit-name test -j ACCEPT
>
> Currently when we do do this the new parameters are not enforced.
Note that:
-A INPUT -m hashlimit --hashlimit-upto 10/sec --hashlimit-burst 10 --hashlimit-name test
-A INPUT -m hashlimit --hashlimit-upto 1/sec --hashlimit-burst 10 --hashlimit-name test
doesn't work as expected either (rule #2 uses config options of #1).
I think is behaviour is so unexpected that I would consider this a bug...
Wrt. to the patch, aside from proc issue I only see style/indent nits, f.e.
> + hlist_for_each_entry(hinfo, &hashlimit_net->htables, node) {
> + if (!strcmp(name, hinfo->name) &&
> + family == hinfo->family) {
'family' should align with the 'if (', not the body.
Other than that, it looks good to me.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
