As the patch is in the "Changes Requested" state and to avoid any
misunderstanding, do you want me to re-submit the patch with the minor
changes applied or are you going to do it yourself? Thanks.
On 10 July 2018 at 13:56, Martynas Pumputis <martynas@weave.works> wrote:
> On 10 July 2018 at 13:37, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
>> 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.
>>
>> Applied, thanks.
>>
>> Will make a minor comestic change before applying, see below.
>
> Thanks!
>
>>
>>> 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);
>>> +
>>
>> Will remove this empty line break.
>>
>>> 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;
>>> + }
>>> + nf_ct_put(ct);
>>> }
>>> NF_CT_STAT_INC(net, drop);
>>> return NF_DROP;
>>> --
>>> 2.18.0
>>>
--
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
