On Mon, Jul 02, 2018 at 04:52:14PM +0200, Martynas Pumputis wrote:
> This patch enables the clash resolution for NAT (disabled in
> "590b52e10d41") if clashing conntracks match (i.e. both tuples are equal)
> and a protocol allows it.
>
> The clash might happen for a connections-less protocol (e.g. UDP) when
> two threads in parallel writes to the same socket and consequent calls
> to "get_unique_tuple" return the same tuples (incl. reply tuples).
>
> In this case it is safe to perform the resolution, as the losing CT
> describes the same mangling as the winning CT, so no modifications to
> the packet are needed, and the result of rules traversal for the loser's
> packet stays valid.
>
> Signed-off-by: Martynas Pumputis <martynas@weave.works>
> ---
> net/netfilter/nf_conntrack_core.c | 31 +++++++++++++++++++++++--------
> 1 file changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
> index 3465da2a98bd..cc5ef8c9d0da 100644
> --- a/net/netfilter/nf_conntrack_core.c
> +++ b/net/netfilter/nf_conntrack_core.c
> @@ -502,6 +502,18 @@ nf_ct_key_equal(struct nf_conntrack_tuple_hash *h,
> net_eq(net, nf_ct_net(ct));
> }
>
> +static inline bool
> +nf_ct_match(const struct nf_conn *ct1, const struct nf_conn *ct2)
> +{
> + return nf_ct_tuple_equal(&ct1->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
> + &ct2->tuplehash[IP_CT_DIR_ORIGINAL].tuple) &&
> + nf_ct_tuple_equal(&ct1->tuplehash[IP_CT_DIR_REPLY].tuple,
> + &ct2->tuplehash[IP_CT_DIR_REPLY].tuple) &&
> + nf_ct_zone_equal(ct1, nf_ct_zone(ct2), IP_CT_DIR_ORIGINAL) &&
> + nf_ct_zone_equal(ct1, nf_ct_zone(ct2), IP_CT_DIR_REPLY) &&
> + net_eq(nf_ct_net(ct1), nf_ct_net(ct2));
> +}
> +
> /* caller must hold rcu readlock and none of the nf_conntrack_locks */
> static void nf_ct_gc_expired(struct nf_conn *ct)
> {
> @@ -696,18 +708,21 @@ static int nf_ct_resolve_clash(struct net *net, struct sk_buff *skb,
> struct nf_conn *ct = nf_ct_tuplehash_to_ctrack(h);
> const struct nf_conntrack_l4proto *l4proto;
>
> + enum ip_conntrack_info oldinfo;
> + struct nf_conn *loser_ct = nf_ct_get(skb, &oldinfo);
> +
> l4proto = __nf_ct_l4proto_find(nf_ct_l3num(ct), nf_ct_protonum(ct));
> if (l4proto->allow_clash &&
> - ((ct->status & IPS_NAT_DONE_MASK) == 0) &&
> !nf_ct_is_dying(ct) &&
> atomic_inc_not_zero(&ct->ct_general.use)) {
> - enum ip_conntrack_info oldinfo;
> - struct nf_conn *loser_ct = nf_ct_get(skb, &oldinfo);
> -
> - nf_ct_acct_merge(ct, ctinfo, loser_ct);
> - nf_conntrack_put(&loser_ct->ct_general);
> - nf_ct_set(skb, ct, oldinfo);
> - return NF_ACCEPT;
> + if (((ct->status & IPS_NAT_DONE_MASK) == 0) ||
> + nf_ct_match(ct, loser_ct)) {
> + nf_ct_acct_merge(ct, ctinfo, loser_ct);
> + nf_conntrack_put(&loser_ct->ct_general);
> + nf_ct_set(skb, ct, oldinfo);
> + return NF_ACCEPT;
This will not work because the packet losing race will get mangled by
when NAT is performed.
Did you test this? Or this is some sort of RFC?
--
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
