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

[Pablo Neira Ayuso <pablo@netfilter.org>]

Patrick McHardy wrote:
> 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.

>From the user view, yes. No matter what value you set as long as it is
the same in all the cluster nodes.

>>> 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?

But the value has to be the same in all the cluster nodes, so how can it
be set to ensure that it is the same value?

> In case you agree, I also think "secret" would be a more fitting name.

I can rename the field to "secret" in the structure or change the
iptables cluster match option to be "--cluster-secret" instead of
"--cluster-hash-seed" if you like.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers