Re: [PATCH v3] iptables: libxt_recent: Add support for --reap option

[Tim Gardner <tim.gardner@canonical.com>]

On 12/02/2011 11:46 AM, Tim Gardner wrote:
> On 12/02/2011 08:30 AM, Jan Engelhardt wrote:
>>
>> On Friday 2011-12-02 02:29, Tim Gardner wrote:
>>> @@ -34,6 +36,8 @@ static const struct xt_option_entry recent_opts[] = {
>>> .excl = F_ANY_OP, .flags = XTOPT_INVERT},
>>> {.name = "seconds", .id = O_SECONDS, .type = XTTYPE_UINT32,
>>> .flags = XTOPT_PUT, XTOPT_POINTER(s, seconds)},
>>> + {.name = "reap", .id = O_REAP, .type = XTTYPE_NONE,
>>> + .also = F_SECONDS },
>>> {.name = "hitcount", .id = O_HITCOUNT, .type = XTTYPE_UINT32,
>>> .flags = XTOPT_PUT, XTOPT_POINTER(s, hit_count)},
>>> {.name = "rttl", .id = O_RTTL, .type = XTTYPE_NONE,
>>
>>> +
>>> + if ((info->check_set& XT_RECENT_REAP)&& !info->seconds)
>>> + xtables_error(PARAMETER_PROBLEM,
>>> + "recent: you must specify `--seconds' with `--reap'");
>>> }
>>
>> Well, I did mean that .also = F_SECONDS makes the extra
>> "info->check_set& XT_RECENT_REAP)&& !info->seconds" test
>> redundant. Or, the error message is wrong, because you are
>> actually testing for seconds==0 rather than "reap was specified
>> without seconds".
>> Is seconds=0 even useful for non-reap cases?
>
> Its not meaningful in that 0 is the default value in the kernel filter
> and implies no timeout.
>
>> If not, we should probably consider using .min=1 on the --seconds
>> parameter, in which case the test is also redundant.
>>
>
> Done. Tested with the following combinations and received the expected
> failures on the first 2:
>
> iptables -A FORWARD -m recent --rcheck --seconds 0 -j DROP
> iptables -A FORWARD -m recent --rcheck --reap -j DROP
> iptables -A FORWARD -m recent --rcheck --seconds 10 --reap -j DROP
>
> rtg

Jan ? Is this v3 patch sufficient ?

rtg
-- 
Tim Gardner tim.gardner@canonical.com