Re: [PATCH] netfilter: xtables: add cluster match

[Patrick McHardy <kaber@trash.net>]

Pablo Neira Ayuso wrote:
> Patrick McHardy wrote:
>> Pablo Neira Ayuso wrote:
>>> +enum xt_cluster_flags {
>>> +    XT_CLUSTER_F_INV    = (1 << 0)
>>> +};
>>> +
>>> +struct xt_cluster_match_info {
>>> +    u_int32_t        total_nodes;
>>> +    u_int32_t        node_mask;
>>> +    u_int32_t        hash_seed;
>>> +    u_int32_t        flags;
>>> +};
>>
>> This doesn't seem like such a hot idea. I haven't seen the new
>> userspace patch, but assuming you're interested in the flags and
>> not ignoring them in userspace, the user has to specify the hash
>> seed for rule deletions.
> 
> The user has to specify the hash seed to delete the rule if it's 
> non-zero, otherwise it must be specified. The hash seed is optional. I 
> don't quite see the problem.

Its a parameter without a meaning, the user is needlessly bothered
with this.

>> You also have to chose the same seed for all nodes in a cluster.
>> This seems needlessly complicated, I'd suggest to simply use zero.
> 
> One may want to forge traffic to flood a single node? The hash seed 
> avoids this.

No, it only makes it easier to shut off since I have to use the same
source address to be sure I hit the same node. This seems like a valid
argument though.

The fact that you have to specify it for deletion still seems unnecesary
though. You would never have two rules differing only in the seed value
since that would mean the node is part of two clusters. So we might as
well move it to the end and ignore it in userspace. What do you think?
In case you agree, I also think "secret" would be a more fitting name.