Pablo Neira Ayuso wrote:
Patrick McHardy wrote:
Pablo Neira Ayuso wrote:
+ if (ct->master)
+ hash = xt_cluster_hash(ct->master, info);
+ else
+ hash = xt_cluster_hash(ct, info);
This makes a lot of sense for helpers like SIP, where the expectation
can arrive from a different source address. I'm just wondering how
this works when not using reliable synchronization - in that case, other
nodes might not be aware of the expectation and also accept the packet.
I don't have a suggestion besides making sure expectations are
synchronized, just thought I'd point it out.
Indeed.
This sort of problem is interesting, just in case that you have some
spare time to think about other synchronization-related problems
(otherwise you can skip the following below :)). Conntrackd does not
synchronize expectations (at least, it's not in my plans yet), it
synchronizes conntrack entries, and that includes the relationship
between master and related conntracks. Thus, after the failover, the new
primary node knows that the master connection has a helper (so it can
create new expectations) and already existing established-related
connections are linked to the master conntracks.
Still I see two possible problematic situations with these approach:
* If expectations are not propagated, this means than a FTP-data
connections that is about to start would not success if that connection
happens during a failover as the expectation information is lost.
* If the state information is lost for whatever reason (like not using
conntrackd at all or losing the state information due to netlink
unreliability), then the former expected connection would be handled
like a normal connection by one cluster node. For example, this would
break if destination nat is used in the case of FTP (and similarly for
other helpers I think).
For the first problem, I can say that conntrackd can be tuned to reduce
the chances of this to happen (at the cost of investing more resources
in the synchronization). Moreover, connections that are about to start
may retry in short and no data was exchanged indeed.
Good point.
For the second problem, this is actually the sort of problems that I
want to avoid making netlink reliable by dropping packets. By reducing
the chances to lose state information for whatever reason.
Yes, although the netlink delivery only covers part of it. It might
be the path where most events are lost though.
+static bool xt_cluster_mt_checkentry(const struct xt_mtchk_param *par)
+{
+ struct xt_cluster_match_info *info = par->matchinfo;
+
+ if (info->node_mask > (1 << info->total_nodes)) {
+ printk(KERN_ERR "xt_cluster: the id of this node cannot be "
+ "higher than the total number of nodes\n");
This looks like an off-by-one (warning: still at first coffee :)).
It may also not be equal to the mask I'd expect. I can change it
to >= when applying if you agree.
You're right! Please change it.
I noticed another problem during compilation:
net/netfilter/xt_cluster.c: In function 'xt_cluster_mt':
net/netfilter/xt_cluster.c:124: warning: passing argument 2 of
'constant_test_bit' from incompatible pointer type
net/netfilter/xt_cluster.c:124: warning: passing argument 2 of
'variable_test_bit' from incompatible pointer type
The problem is that is uses a u32 for the mask, but the bitops are
only defined for unsigned longs. Which is a bit unfortunate since
they're not well suited for ABI structures. I'd suggest to simply
open-code the bit tests.
--
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
