* [PATCH 0/5] netfilter: NAT sequence number adjustment fixes/improvements
@ 2013-07-28 20:54 Patrick McHardy
2013-07-28 20:54 ` [PATCH 1/5] netfilter: nf_conntrack: remove net_ratelimit() for LOG_INVALID() Patrick McHardy
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: Patrick McHardy @ 2013-07-28 20:54 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel
the following patches from my SYNPROXY tree contain some fixes and improvements
for netfilter sequence number adjustment handling and two unrelated minor
patches:
- Remove net_ratelimit() for LOG_INVALID: ratelimiting explicitly enabled
packet logging is inconsistent with other netfilter logging behaviour and
makes debugging harder
- Constify nf_ct_attach() source skb argument
- Fix locking in nf_nat_seq_adjust(): we need to take nf_nat_seqofs lock
to protect against concurrent changes to the sequence adjustment data
- Increase sequence number offset size to 32 bits. When many adjustments
happen in a single connection, the offsets can overflow and break the
connection.
- Use per-conntrack locks to protect sequence number adjustment data.
Should increase scalability and additionally saves one lock/unlock operation
per TCP packet.
Please apply. thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/5] netfilter: nf_conntrack: remove net_ratelimit() for LOG_INVALID()
2013-07-28 20:54 [PATCH 0/5] netfilter: NAT sequence number adjustment fixes/improvements Patrick McHardy
@ 2013-07-28 20:54 ` Patrick McHardy
2013-07-31 16:53 ` Pablo Neira Ayuso
2013-07-28 20:54 ` [PATCH 2/5] netfilter: nf_conntrack: constify sk_buff argument to nf_ct_attach() Patrick McHardy
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Patrick McHardy @ 2013-07-28 20:54 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel
Logging of invalid packets has to be explicitly enabled. Rate-limiting these
messages is inconsistent with other netfilter logging features and makes
debugging harder.
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
include/net/netfilter/nf_conntrack_l4proto.h | 7 -------
1 file changed, 7 deletions(-)
diff --git a/include/net/netfilter/nf_conntrack_l4proto.h b/include/net/netfilter/nf_conntrack_l4proto.h
index 914d8d9..b411d7b 100644
--- a/include/net/netfilter/nf_conntrack_l4proto.h
+++ b/include/net/netfilter/nf_conntrack_l4proto.h
@@ -148,17 +148,10 @@ extern int nf_ct_port_nlattr_tuple_size(void);
extern const struct nla_policy nf_ct_port_nla_policy[];
#ifdef CONFIG_SYSCTL
-#ifdef DEBUG_INVALID_PACKETS
#define LOG_INVALID(net, proto) \
((net)->ct.sysctl_log_invalid == (proto) || \
(net)->ct.sysctl_log_invalid == IPPROTO_RAW)
#else
-#define LOG_INVALID(net, proto) \
- (((net)->ct.sysctl_log_invalid == (proto) || \
- (net)->ct.sysctl_log_invalid == IPPROTO_RAW) \
- && net_ratelimit())
-#endif
-#else
static inline int LOG_INVALID(struct net *net, int proto) { return 0; }
#endif /* CONFIG_SYSCTL */
--
1.8.1.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/5] netfilter: nf_conntrack: constify sk_buff argument to nf_ct_attach()
2013-07-28 20:54 [PATCH 0/5] netfilter: NAT sequence number adjustment fixes/improvements Patrick McHardy
2013-07-28 20:54 ` [PATCH 1/5] netfilter: nf_conntrack: remove net_ratelimit() for LOG_INVALID() Patrick McHardy
@ 2013-07-28 20:54 ` Patrick McHardy
2013-07-31 16:53 ` Pablo Neira Ayuso
2013-07-28 20:54 ` [PATCH 3/5] netfilter: nf_nat: fix locking in nf_nat_seq_adjust() Patrick McHardy
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Patrick McHardy @ 2013-07-28 20:54 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
include/linux/netfilter.h | 4 ++--
net/netfilter/core.c | 7 ++++---
net/netfilter/nf_conntrack_core.c | 2 +-
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index de70f7b..f4bbf2c 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -314,8 +314,8 @@ nf_nat_decode_session(struct sk_buff *skb, struct flowi *fl, u_int8_t family)
#endif /*CONFIG_NETFILTER*/
#if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
-extern void (*ip_ct_attach)(struct sk_buff *, struct sk_buff *) __rcu;
-extern void nf_ct_attach(struct sk_buff *, struct sk_buff *);
+extern void (*ip_ct_attach)(struct sk_buff *, const struct sk_buff *) __rcu;
+extern void nf_ct_attach(struct sk_buff *, const struct sk_buff *);
extern void (*nf_ct_destroy)(struct nf_conntrack *) __rcu;
struct nf_conn;
diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index 2217363..593b16e 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -234,12 +234,13 @@ EXPORT_SYMBOL(skb_make_writable);
/* This does not belong here, but locally generated errors need it if connection
tracking in use: without this, connection may not be in hash table, and hence
manufactured ICMP or RST packets will not be associated with it. */
-void (*ip_ct_attach)(struct sk_buff *, struct sk_buff *) __rcu __read_mostly;
+void (*ip_ct_attach)(struct sk_buff *, const struct sk_buff *)
+ __rcu __read_mostly;
EXPORT_SYMBOL(ip_ct_attach);
-void nf_ct_attach(struct sk_buff *new, struct sk_buff *skb)
+void nf_ct_attach(struct sk_buff *new, const struct sk_buff *skb)
{
- void (*attach)(struct sk_buff *, struct sk_buff *);
+ void (*attach)(struct sk_buff *, const struct sk_buff *);
if (skb->nfct) {
rcu_read_lock();
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 0283bae..d32afaf 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1192,7 +1192,7 @@ EXPORT_SYMBOL_GPL(nf_ct_port_nlattr_tuple_size);
#endif
/* Used by ipt_REJECT and ip6t_REJECT. */
-static void nf_conntrack_attach(struct sk_buff *nskb, struct sk_buff *skb)
+static void nf_conntrack_attach(struct sk_buff *nskb, const struct sk_buff *skb)
{
struct nf_conn *ct;
enum ip_conntrack_info ctinfo;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/5] netfilter: nf_nat: fix locking in nf_nat_seq_adjust()
2013-07-28 20:54 [PATCH 0/5] netfilter: NAT sequence number adjustment fixes/improvements Patrick McHardy
2013-07-28 20:54 ` [PATCH 1/5] netfilter: nf_conntrack: remove net_ratelimit() for LOG_INVALID() Patrick McHardy
2013-07-28 20:54 ` [PATCH 2/5] netfilter: nf_conntrack: constify sk_buff argument to nf_ct_attach() Patrick McHardy
@ 2013-07-28 20:54 ` Patrick McHardy
2013-07-31 16:55 ` Pablo Neira Ayuso
2013-07-28 20:54 ` [PATCH 4/5] netfilter: nf_nat: change sequence number adjustments to 32 bits Patrick McHardy
2013-07-28 20:54 ` [PATCH 5/5] netfilter: nf_nat: use per-conntrack locking for sequence number adjustments Patrick McHardy
4 siblings, 1 reply; 13+ messages in thread
From: Patrick McHardy @ 2013-07-28 20:54 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel
nf_nat_seq_adjust() needs to grab nf_nat_setofs_lock to protect against
concurrent changes to the sequence adjustment data.
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
net/netfilter/nf_nat_helper.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/nf_nat_helper.c b/net/netfilter/nf_nat_helper.c
index 85e20a9..a7262ed 100644
--- a/net/netfilter/nf_nat_helper.c
+++ b/net/netfilter/nf_nat_helper.c
@@ -373,6 +373,7 @@ nf_nat_seq_adjust(struct sk_buff *skb,
s16 seqoff, ackoff;
struct nf_conn_nat *nat = nfct_nat(ct);
struct nf_nat_seq *this_way, *other_way;
+ int res;
dir = CTINFO2DIR(ctinfo);
@@ -383,6 +384,7 @@ nf_nat_seq_adjust(struct sk_buff *skb,
return 0;
tcph = (void *)skb->data + protoff;
+ spin_lock_bh(&nf_nat_seqofs_lock);
if (after(ntohl(tcph->seq), this_way->correction_pos))
seqoff = this_way->offset_after;
else
@@ -407,7 +409,10 @@ nf_nat_seq_adjust(struct sk_buff *skb,
tcph->seq = newseq;
tcph->ack_seq = newack;
- return nf_nat_sack_adjust(skb, protoff, tcph, ct, ctinfo);
+ res = nf_nat_sack_adjust(skb, protoff, tcph, ct, ctinfo);
+ spin_unlock_bh(&nf_nat_seqofs_lock);
+
+ return res;
}
/* Setup NAT on this expected conntrack so it follows master. */
--
1.8.1.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/5] netfilter: nf_nat: change sequence number adjustments to 32 bits
2013-07-28 20:54 [PATCH 0/5] netfilter: NAT sequence number adjustment fixes/improvements Patrick McHardy
` (2 preceding siblings ...)
2013-07-28 20:54 ` [PATCH 3/5] netfilter: nf_nat: fix locking in nf_nat_seq_adjust() Patrick McHardy
@ 2013-07-28 20:54 ` Patrick McHardy
2013-07-31 17:58 ` Pablo Neira Ayuso
2013-07-28 20:54 ` [PATCH 5/5] netfilter: nf_nat: use per-conntrack locking for sequence number adjustments Patrick McHardy
4 siblings, 1 reply; 13+ messages in thread
From: Patrick McHardy @ 2013-07-28 20:54 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel
Using 16 bits is too small, when many adjustments happen the offsets might
overflow and break the connection.
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
include/linux/netfilter.h | 2 +-
include/net/netfilter/nf_conntrack.h | 2 +-
include/net/netfilter/nf_nat.h | 2 +-
include/net/netfilter/nf_nat_helper.h | 6 +++---
net/netfilter/nf_conntrack_core.c | 2 +-
net/netfilter/nf_conntrack_proto_tcp.c | 4 ++--
net/netfilter/nf_nat_helper.c | 8 ++++----
7 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index f4bbf2c..655d5d1 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -330,7 +330,7 @@ extern struct nfq_ct_hook __rcu *nfq_ct_hook;
struct nfq_ct_nat_hook {
void (*seq_adjust)(struct sk_buff *skb, struct nf_conn *ct,
- u32 ctinfo, int off);
+ u32 ctinfo, s32 off);
};
extern struct nfq_ct_nat_hook __rcu *nfq_ct_nat_hook;
#else
diff --git a/include/net/netfilter/nf_conntrack.h b/include/net/netfilter/nf_conntrack.h
index 644d9c2..21c374c 100644
--- a/include/net/netfilter/nf_conntrack.h
+++ b/include/net/netfilter/nf_conntrack.h
@@ -235,7 +235,7 @@ static inline bool nf_ct_kill(struct nf_conn *ct)
}
/* These are for NAT. Icky. */
-extern s16 (*nf_ct_nat_offset)(const struct nf_conn *ct,
+extern s32 (*nf_ct_nat_offset)(const struct nf_conn *ct,
enum ip_conntrack_dir dir,
u32 seq);
diff --git a/include/net/netfilter/nf_nat.h b/include/net/netfilter/nf_nat.h
index ad14a79..e244141 100644
--- a/include/net/netfilter/nf_nat.h
+++ b/include/net/netfilter/nf_nat.h
@@ -19,7 +19,7 @@ struct nf_nat_seq {
u_int32_t correction_pos;
/* sequence number offset before and after last modification */
- int16_t offset_before, offset_after;
+ int32_t offset_before, offset_after;
};
#include <linux/list.h>
diff --git a/include/net/netfilter/nf_nat_helper.h b/include/net/netfilter/nf_nat_helper.h
index b4d6bfc..194c347 100644
--- a/include/net/netfilter/nf_nat_helper.h
+++ b/include/net/netfilter/nf_nat_helper.h
@@ -41,7 +41,7 @@ extern int nf_nat_mangle_udp_packet(struct sk_buff *skb,
extern void nf_nat_set_seq_adjust(struct nf_conn *ct,
enum ip_conntrack_info ctinfo,
- __be32 seq, s16 off);
+ __be32 seq, s32 off);
extern int nf_nat_seq_adjust(struct sk_buff *skb,
struct nf_conn *ct,
enum ip_conntrack_info ctinfo,
@@ -56,11 +56,11 @@ extern int (*nf_nat_seq_adjust_hook)(struct sk_buff *skb,
extern void nf_nat_follow_master(struct nf_conn *ct,
struct nf_conntrack_expect *this);
-extern s16 nf_nat_get_offset(const struct nf_conn *ct,
+extern s32 nf_nat_get_offset(const struct nf_conn *ct,
enum ip_conntrack_dir dir,
u32 seq);
extern void nf_nat_tcp_seq_adjust(struct sk_buff *skb, struct nf_conn *ct,
- u32 dir, int off);
+ u32 dir, s32 off);
#endif
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index d32afaf..26c5b37 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1692,7 +1692,7 @@ err_stat:
return ret;
}
-s16 (*nf_ct_nat_offset)(const struct nf_conn *ct,
+s32 (*nf_ct_nat_offset)(const struct nf_conn *ct,
enum ip_conntrack_dir dir,
u32 seq);
EXPORT_SYMBOL_GPL(nf_ct_nat_offset);
diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
index 7dcc376..8f308d8 100644
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -496,7 +496,7 @@ static void tcp_sack(const struct sk_buff *skb, unsigned int dataoff,
}
#ifdef CONFIG_NF_NAT_NEEDED
-static inline s16 nat_offset(const struct nf_conn *ct,
+static inline s32 nat_offset(const struct nf_conn *ct,
enum ip_conntrack_dir dir,
u32 seq)
{
@@ -525,7 +525,7 @@ static bool tcp_in_window(const struct nf_conn *ct,
struct ip_ct_tcp_state *receiver = &state->seen[!dir];
const struct nf_conntrack_tuple *tuple = &ct->tuplehash[dir].tuple;
__u32 seq, ack, sack, end, win, swin;
- s16 receiver_offset;
+ s32 receiver_offset;
bool res;
/*
diff --git a/net/netfilter/nf_nat_helper.c b/net/netfilter/nf_nat_helper.c
index a7262ed..ff4a589 100644
--- a/net/netfilter/nf_nat_helper.c
+++ b/net/netfilter/nf_nat_helper.c
@@ -68,13 +68,13 @@ adjust_tcp_sequence(u32 seq,
}
/* Get the offset value, for conntrack */
-s16 nf_nat_get_offset(const struct nf_conn *ct,
+s32 nf_nat_get_offset(const struct nf_conn *ct,
enum ip_conntrack_dir dir,
u32 seq)
{
struct nf_conn_nat *nat = nfct_nat(ct);
struct nf_nat_seq *this_way;
- s16 offset;
+ s32 offset;
if (!nat)
return 0;
@@ -143,7 +143,7 @@ static int enlarge_skb(struct sk_buff *skb, unsigned int extra)
}
void nf_nat_set_seq_adjust(struct nf_conn *ct, enum ip_conntrack_info ctinfo,
- __be32 seq, s16 off)
+ __be32 seq, s32 off)
{
if (!off)
return;
@@ -370,7 +370,7 @@ nf_nat_seq_adjust(struct sk_buff *skb,
struct tcphdr *tcph;
int dir;
__be32 newseq, newack;
- s16 seqoff, ackoff;
+ s32 seqoff, ackoff;
struct nf_conn_nat *nat = nfct_nat(ct);
struct nf_nat_seq *this_way, *other_way;
int res;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 5/5] netfilter: nf_nat: use per-conntrack locking for sequence number adjustments
2013-07-28 20:54 [PATCH 0/5] netfilter: NAT sequence number adjustment fixes/improvements Patrick McHardy
` (3 preceding siblings ...)
2013-07-28 20:54 ` [PATCH 4/5] netfilter: nf_nat: change sequence number adjustments to 32 bits Patrick McHardy
@ 2013-07-28 20:54 ` Patrick McHardy
2013-07-31 17:59 ` Pablo Neira Ayuso
4 siblings, 1 reply; 13+ messages in thread
From: Patrick McHardy @ 2013-07-28 20:54 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel
Get rid of the global lock and use per-conntrack locks for protecting the
sequencen number adjustment data. Additionally saves one lock/unlock
operation for every TCP packet.
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
net/netfilter/nf_nat_helper.c | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/net/netfilter/nf_nat_helper.c b/net/netfilter/nf_nat_helper.c
index ff4a589..46b9baa 100644
--- a/net/netfilter/nf_nat_helper.c
+++ b/net/netfilter/nf_nat_helper.c
@@ -30,8 +30,6 @@
pr_debug("offset_before=%d, offset_after=%d, correction_pos=%u\n", \
x->offset_before, x->offset_after, x->correction_pos);
-static DEFINE_SPINLOCK(nf_nat_seqofs_lock);
-
/* Setup TCP sequence correction given this change at this sequence */
static inline void
adjust_tcp_sequence(u32 seq,
@@ -49,7 +47,7 @@ adjust_tcp_sequence(u32 seq,
pr_debug("adjust_tcp_sequence: Seq_offset before: ");
DUMP_OFFSET(this_way);
- spin_lock_bh(&nf_nat_seqofs_lock);
+ spin_lock_bh(&ct->lock);
/* SYN adjust. If it's uninitialized, or this is after last
* correction, record it: we don't handle more than one
@@ -61,31 +59,26 @@ adjust_tcp_sequence(u32 seq,
this_way->offset_before = this_way->offset_after;
this_way->offset_after += sizediff;
}
- spin_unlock_bh(&nf_nat_seqofs_lock);
+ spin_unlock_bh(&ct->lock);
pr_debug("adjust_tcp_sequence: Seq_offset after: ");
DUMP_OFFSET(this_way);
}
-/* Get the offset value, for conntrack */
+/* Get the offset value, for conntrack. Caller must have the conntrack locked */
s32 nf_nat_get_offset(const struct nf_conn *ct,
enum ip_conntrack_dir dir,
u32 seq)
{
struct nf_conn_nat *nat = nfct_nat(ct);
struct nf_nat_seq *this_way;
- s32 offset;
if (!nat)
return 0;
this_way = &nat->seq[dir];
- spin_lock_bh(&nf_nat_seqofs_lock);
- offset = after(seq, this_way->correction_pos)
+ return after(seq, this_way->correction_pos)
? this_way->offset_after : this_way->offset_before;
- spin_unlock_bh(&nf_nat_seqofs_lock);
-
- return offset;
}
/* Frobs data inside this packet, which is linear. */
@@ -384,7 +377,7 @@ nf_nat_seq_adjust(struct sk_buff *skb,
return 0;
tcph = (void *)skb->data + protoff;
- spin_lock_bh(&nf_nat_seqofs_lock);
+ spin_lock_bh(&ct->lock);
if (after(ntohl(tcph->seq), this_way->correction_pos))
seqoff = this_way->offset_after;
else
@@ -410,7 +403,7 @@ nf_nat_seq_adjust(struct sk_buff *skb,
tcph->ack_seq = newack;
res = nf_nat_sack_adjust(skb, protoff, tcph, ct, ctinfo);
- spin_unlock_bh(&nf_nat_seqofs_lock);
+ spin_unlock_bh(&ct->lock);
return res;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/5] netfilter: nf_conntrack: remove net_ratelimit() for LOG_INVALID()
2013-07-28 20:54 ` [PATCH 1/5] netfilter: nf_conntrack: remove net_ratelimit() for LOG_INVALID() Patrick McHardy
@ 2013-07-31 16:53 ` Pablo Neira Ayuso
0 siblings, 0 replies; 13+ messages in thread
From: Pablo Neira Ayuso @ 2013-07-31 16:53 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netfilter-devel
On Sun, Jul 28, 2013 at 10:54:07PM +0200, Patrick McHardy wrote:
> Logging of invalid packets has to be explicitly enabled. Rate-limiting these
> messages is inconsistent with other netfilter logging features and makes
> debugging harder.
Applied, thanks Patrick!
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/5] netfilter: nf_conntrack: constify sk_buff argument to nf_ct_attach()
2013-07-28 20:54 ` [PATCH 2/5] netfilter: nf_conntrack: constify sk_buff argument to nf_ct_attach() Patrick McHardy
@ 2013-07-31 16:53 ` Pablo Neira Ayuso
0 siblings, 0 replies; 13+ messages in thread
From: Pablo Neira Ayuso @ 2013-07-31 16:53 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netfilter-devel
Also applied, thanks
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/5] netfilter: nf_nat: fix locking in nf_nat_seq_adjust()
2013-07-28 20:54 ` [PATCH 3/5] netfilter: nf_nat: fix locking in nf_nat_seq_adjust() Patrick McHardy
@ 2013-07-31 16:55 ` Pablo Neira Ayuso
2013-07-31 17:51 ` Patrick McHardy
0 siblings, 1 reply; 13+ messages in thread
From: Pablo Neira Ayuso @ 2013-07-31 16:55 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netfilter-devel
On Sun, Jul 28, 2013 at 10:54:09PM +0200, Patrick McHardy wrote:
> nf_nat_seq_adjust() needs to grab nf_nat_setofs_lock to protect against
> concurrent changes to the sequence adjustment data.
Applied, thanks.
I have enqueued this to nf so it hits 3.11. I have to wait to take 4/5
and 5/5 for a while until this patch hits Linus tree.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/5] netfilter: nf_nat: fix locking in nf_nat_seq_adjust()
2013-07-31 16:55 ` Pablo Neira Ayuso
@ 2013-07-31 17:51 ` Patrick McHardy
2013-07-31 17:57 ` Pablo Neira Ayuso
0 siblings, 1 reply; 13+ messages in thread
From: Patrick McHardy @ 2013-07-31 17:51 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
On Wed, Jul 31, 2013 at 06:55:23PM +0200, Pablo Neira Ayuso wrote:
> On Sun, Jul 28, 2013 at 10:54:09PM +0200, Patrick McHardy wrote:
> > nf_nat_seq_adjust() needs to grab nf_nat_setofs_lock to protect against
> > concurrent changes to the sequence adjustment data.
>
> Applied, thanks.
>
> I have enqueued this to nf so it hits 3.11. I have to wait to take 4/5
> and 5/5 for a while until this patch hits Linus tree.
We've had this problem since the beginning of netfilter, so I think
this can go to -next. So far I haven't seen any bugreports related to
this, but its clearly wrong.
A follow up patch will move the entire sequence number adjustment data
to an extend, so the size increase will only affect the quite rare case
of connections using a helper.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/5] netfilter: nf_nat: fix locking in nf_nat_seq_adjust()
2013-07-31 17:51 ` Patrick McHardy
@ 2013-07-31 17:57 ` Pablo Neira Ayuso
0 siblings, 0 replies; 13+ messages in thread
From: Pablo Neira Ayuso @ 2013-07-31 17:57 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netfilter-devel
On Wed, Jul 31, 2013 at 07:51:18PM +0200, Patrick McHardy wrote:
> On Wed, Jul 31, 2013 at 06:55:23PM +0200, Pablo Neira Ayuso wrote:
> > On Sun, Jul 28, 2013 at 10:54:09PM +0200, Patrick McHardy wrote:
> > > nf_nat_seq_adjust() needs to grab nf_nat_setofs_lock to protect against
> > > concurrent changes to the sequence adjustment data.
> >
> > Applied, thanks.
> >
> > I have enqueued this to nf so it hits 3.11. I have to wait to take 4/5
> > and 5/5 for a while until this patch hits Linus tree.
>
> We've had this problem since the beginning of netfilter, so I think
> this can go to -next. So far I haven't seen any bugreports related to
> this, but its clearly wrong.
Fair enough, will enqueue this to -next then.
> A follow up patch will move the entire sequence number adjustment data
> to an extend, so the size increase will only affect the quite rare case
> of connections using a helper.
Great to know, thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/5] netfilter: nf_nat: change sequence number adjustments to 32 bits
2013-07-28 20:54 ` [PATCH 4/5] netfilter: nf_nat: change sequence number adjustments to 32 bits Patrick McHardy
@ 2013-07-31 17:58 ` Pablo Neira Ayuso
0 siblings, 0 replies; 13+ messages in thread
From: Pablo Neira Ayuso @ 2013-07-31 17:58 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netfilter-devel
On Sun, Jul 28, 2013 at 10:54:10PM +0200, Patrick McHardy wrote:
> Using 16 bits is too small, when many adjustments happen the offsets might
> overflow and break the connection.
Applied, thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 5/5] netfilter: nf_nat: use per-conntrack locking for sequence number adjustments
2013-07-28 20:54 ` [PATCH 5/5] netfilter: nf_nat: use per-conntrack locking for sequence number adjustments Patrick McHardy
@ 2013-07-31 17:59 ` Pablo Neira Ayuso
0 siblings, 0 replies; 13+ messages in thread
From: Pablo Neira Ayuso @ 2013-07-31 17:59 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netfilter-devel
On Sun, Jul 28, 2013 at 10:54:11PM +0200, Patrick McHardy wrote:
> Get rid of the global lock and use per-conntrack locks for protecting the
> sequencen number adjustment data. Additionally saves one lock/unlock
> operation for every TCP packet.
also applied, thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2013-07-31 17:59 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-28 20:54 [PATCH 0/5] netfilter: NAT sequence number adjustment fixes/improvements Patrick McHardy
2013-07-28 20:54 ` [PATCH 1/5] netfilter: nf_conntrack: remove net_ratelimit() for LOG_INVALID() Patrick McHardy
2013-07-31 16:53 ` Pablo Neira Ayuso
2013-07-28 20:54 ` [PATCH 2/5] netfilter: nf_conntrack: constify sk_buff argument to nf_ct_attach() Patrick McHardy
2013-07-31 16:53 ` Pablo Neira Ayuso
2013-07-28 20:54 ` [PATCH 3/5] netfilter: nf_nat: fix locking in nf_nat_seq_adjust() Patrick McHardy
2013-07-31 16:55 ` Pablo Neira Ayuso
2013-07-31 17:51 ` Patrick McHardy
2013-07-31 17:57 ` Pablo Neira Ayuso
2013-07-28 20:54 ` [PATCH 4/5] netfilter: nf_nat: change sequence number adjustments to 32 bits Patrick McHardy
2013-07-31 17:58 ` Pablo Neira Ayuso
2013-07-28 20:54 ` [PATCH 5/5] netfilter: nf_nat: use per-conntrack locking for sequence number adjustments Patrick McHardy
2013-07-31 17:59 ` Pablo Neira Ayuso
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.