All of lore.kernel.org
 help / color / mirror / Atom feed
* [nf-next V2] netfilter: nf_conntrack, add IPS_HW_OFFLOAD status bit
@ 2020-04-21 15:04 Bodong Wang
  2020-04-26 21:53 ` Pablo Neira Ayuso
  0 siblings, 1 reply; 3+ messages in thread
From: Bodong Wang @ 2020-04-21 15:04 UTC (permalink / raw)
  To: netfilter-devel; +Cc: pablo, ozsh, paulb, Bodong Wang

This bit indicates that the conntrack entry is offloaded to hardware
flow table. nf_conntrack entry will be tagged with [HW_OFFLOAD] if
it's offload to hardware.

cat /proc/net/nf_conntrack
	ipv4 2 tcp 6 \
	src=1.1.1.17 dst=1.1.1.16 sport=56394 dport=5001 \
	src=1.1.1.16 dst=1.1.1.17 sport=5001 dport=56394 [HW_OFFLOAD] \
	mark=0 zone=0 use=3

Note that HW_OFFLOAD/OFFLOAD/ASSURED are mutually exclusive.

Changelog:

* V1->V2:
- Remove check of lastused from stats. It was meant for cases such
  as removing driver module while traffic still running. Better to
  handle such cases from garbage collector.

Signed-off-by: Bodong Wang <bodong@mellanox.com>
Reviewed-by: Oz Shlomo <ozsh@mellanox.com>
Reviewed-by: Paul Blakey <paulb@mellanox.com>
---
 include/uapi/linux/netfilter/nf_conntrack_common.h | 8 ++++++--
 net/netfilter/nf_conntrack_standalone.c            | 4 +++-
 net/netfilter/nf_flow_table_offload.c              | 3 +++
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/netfilter/nf_conntrack_common.h b/include/uapi/linux/netfilter/nf_conntrack_common.h
index b6f0bb1dc799..4b3395082d15 100644
--- a/include/uapi/linux/netfilter/nf_conntrack_common.h
+++ b/include/uapi/linux/netfilter/nf_conntrack_common.h
@@ -114,15 +114,19 @@ enum ip_conntrack_status {
 	IPS_OFFLOAD_BIT = 14,
 	IPS_OFFLOAD = (1 << IPS_OFFLOAD_BIT),
 
+	/* Conntrack has been offloaded to hardware. */
+	IPS_HW_OFFLOAD_BIT = 15,
+	IPS_HW_OFFLOAD = (1 << IPS_HW_OFFLOAD_BIT),
+
 	/* Be careful here, modifying these bits can make things messy,
 	 * so don't let users modify them directly.
 	 */
 	IPS_UNCHANGEABLE_MASK = (IPS_NAT_DONE_MASK | IPS_NAT_MASK |
 				 IPS_EXPECTED | IPS_CONFIRMED | IPS_DYING |
 				 IPS_SEQ_ADJUST | IPS_TEMPLATE | IPS_UNTRACKED |
-				 IPS_OFFLOAD),
+				 IPS_OFFLOAD | IPS_HW_OFFLOAD),
 
-	__IPS_MAX_BIT = 15,
+	__IPS_MAX_BIT = 16,
 };
 
 /* Connection tracking event types */
diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
index 9b57330c81f8..5a3e6c43ee68 100644
--- a/net/netfilter/nf_conntrack_standalone.c
+++ b/net/netfilter/nf_conntrack_standalone.c
@@ -348,7 +348,9 @@ static int ct_seq_show(struct seq_file *s, void *v)
 	if (seq_print_acct(s, ct, IP_CT_DIR_REPLY))
 		goto release;
 
-	if (test_bit(IPS_OFFLOAD_BIT, &ct->status))
+	if (test_bit(IPS_HW_OFFLOAD_BIT, &ct->status))
+		seq_puts(s, "[HW_OFFLOAD] ");
+	else if (test_bit(IPS_OFFLOAD_BIT, &ct->status))
 		seq_puts(s, "[OFFLOAD] ");
 	else if (test_bit(IPS_ASSURED_BIT, &ct->status))
 		seq_puts(s, "[ASSURED] ");
diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
index e3b099c14eff..a2abb0feab7f 100644
--- a/net/netfilter/nf_flow_table_offload.c
+++ b/net/netfilter/nf_flow_table_offload.c
@@ -754,12 +754,15 @@ static void flow_offload_work_add(struct flow_offload_work *offload)
 	err = flow_offload_rule_add(offload, flow_rule);
 	if (err < 0)
 		set_bit(NF_FLOW_HW_REFRESH, &offload->flow->flags);
+	else
+		set_bit(IPS_HW_OFFLOAD_BIT, &offload->flow->ct->status);
 
 	nf_flow_offload_destroy(flow_rule);
 }
 
 static void flow_offload_work_del(struct flow_offload_work *offload)
 {
+	clear_bit(IPS_HW_OFFLOAD_BIT, &offload->flow->ct->status);
 	flow_offload_tuple_del(offload, FLOW_OFFLOAD_DIR_ORIGINAL);
 	flow_offload_tuple_del(offload, FLOW_OFFLOAD_DIR_REPLY);
 	set_bit(NF_FLOW_HW_DEAD, &offload->flow->flags);
-- 
2.21.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [nf-next V2] netfilter: nf_conntrack, add IPS_HW_OFFLOAD status bit
  2020-04-21 15:04 [nf-next V2] netfilter: nf_conntrack, add IPS_HW_OFFLOAD status bit Bodong Wang
@ 2020-04-26 21:53 ` Pablo Neira Ayuso
  2020-04-27 21:02   ` Bodong Wang
  0 siblings, 1 reply; 3+ messages in thread
From: Pablo Neira Ayuso @ 2020-04-26 21:53 UTC (permalink / raw)
  To: Bodong Wang; +Cc: netfilter-devel, ozsh, paulb

[-- Attachment #1: Type: text/plain, Size: 716 bytes --]

On Tue, Apr 21, 2020 at 10:04:16AM -0500, Bodong Wang wrote:
> This bit indicates that the conntrack entry is offloaded to hardware
> flow table. nf_conntrack entry will be tagged with [HW_OFFLOAD] if
> it's offload to hardware.
> 
> cat /proc/net/nf_conntrack
> 	ipv4 2 tcp 6 \
> 	src=1.1.1.17 dst=1.1.1.16 sport=56394 dport=5001 \
> 	src=1.1.1.16 dst=1.1.1.17 sport=5001 dport=56394 [HW_OFFLOAD] \
> 	mark=0 zone=0 use=3
> 
> Note that HW_OFFLOAD/OFFLOAD/ASSURED are mutually exclusive.

Applied, thanks.

Could you also test the following userspace patches for
libnetfilter_conntrack and the conntrack-tools to get the netlink
tools in feature parity? If they work fine there, I'll formally submit
them.

Thanks.

[-- Attachment #2: libnetfilter_conntrack.patch --]
[-- Type: text/x-diff, Size: 2042 bytes --]

diff --git a/include/libnetfilter_conntrack/linux_nf_conntrack_common.h b/include/libnetfilter_conntrack/linux_nf_conntrack_common.h
index 32efa357c8db..131ca258a904 100644
--- a/include/libnetfilter_conntrack/linux_nf_conntrack_common.h
+++ b/include/libnetfilter_conntrack/linux_nf_conntrack_common.h
@@ -102,6 +102,15 @@ enum ip_conntrack_status {
 	IPS_UNTRACKED_BIT = 12,
 	IPS_UNTRACKED = (1 << IPS_UNTRACKED_BIT),
 
+#ifdef __KERNEL__
+	/* Re-purposed for in-kernel use:
+	 * Tags a conntrack entry that clashed with an existing entry
+	 * on insert.
+	 */
+	IPS_NAT_CLASH_BIT = IPS_UNTRACKED_BIT,
+	IPS_NAT_CLASH = IPS_UNTRACKED,
+#endif
+
 	/* Conntrack got a helper explicitly attached via CT target. */
 	IPS_HELPER_BIT = 13,
 	IPS_HELPER = (1 << IPS_HELPER_BIT),
@@ -110,14 +119,19 @@ enum ip_conntrack_status {
 	IPS_OFFLOAD_BIT = 14,
 	IPS_OFFLOAD = (1 << IPS_OFFLOAD_BIT),
 
+	/* Conntrack has been offloaded to hardware. */
+	IPS_HW_OFFLOAD_BIT = 15,
+	IPS_HW_OFFLOAD = (1 << IPS_HW_OFFLOAD_BIT),
+
 	/* Be careful here, modifying these bits can make things messy,
 	 * so don't let users modify them directly.
 	 */
 	IPS_UNCHANGEABLE_MASK = (IPS_NAT_DONE_MASK | IPS_NAT_MASK |
 				 IPS_EXPECTED | IPS_CONFIRMED | IPS_DYING |
-				 IPS_SEQ_ADJUST | IPS_TEMPLATE | IPS_OFFLOAD),
+				 IPS_SEQ_ADJUST | IPS_TEMPLATE | IPS_UNTRACKED |
+				 IPS_OFFLOAD | IPS_HW_OFFLOAD),
 
-	__IPS_MAX_BIT = 15,
+	__IPS_MAX_BIT = 16,
 };
 
 /* Connection tracking event types */
diff --git a/src/conntrack/snprintf_default.c b/src/conntrack/snprintf_default.c
index 765ce726ea7d..2f2f91864d39 100644
--- a/src/conntrack/snprintf_default.c
+++ b/src/conntrack/snprintf_default.c
@@ -184,7 +184,9 @@ static int __snprintf_status_assured(char *buf,
 {
 	int size = 0;
 
-	if (ct->status & IPS_OFFLOAD)
+	if (ct->status & IPS_HW_OFFLOAD)
+		size = snprintf(buf, len, "[HW_OFFLOAD] ");
+	else if (ct->status & IPS_OFFLOAD)
 		size = snprintf(buf, len, "[OFFLOAD] ");
 	else if (ct->status & IPS_ASSURED)
 		size = snprintf(buf, len, "[ASSURED] ");

[-- Attachment #3: conntrack-tools.patch --]
[-- Type: text/x-diff, Size: 2252 bytes --]

diff --git a/include/linux/netfilter/nf_conntrack_common.h b/include/linux/netfilter/nf_conntrack_common.h
index 8023e5b6572f..16d20a3b2ff0 100644
--- a/include/linux/netfilter/nf_conntrack_common.h
+++ b/include/linux/netfilter/nf_conntrack_common.h
@@ -97,6 +97,15 @@ enum ip_conntrack_status {
 	IPS_UNTRACKED_BIT = 12,
 	IPS_UNTRACKED = (1 << IPS_UNTRACKED_BIT),
 
+#ifdef __KERNEL__
+	/* Re-purposed for in-kernel use:
+	 * Tags a conntrack entry that clashed with an existing entry
+	 * on insert.
+	 */
+	IPS_NAT_CLASH_BIT = IPS_UNTRACKED_BIT,
+	IPS_NAT_CLASH = IPS_UNTRACKED,
+#endif
+
 	/* Conntrack got a helper explicitly attached via CT target. */
 	IPS_HELPER_BIT = 13,
 	IPS_HELPER = (1 << IPS_HELPER_BIT),
@@ -105,14 +114,19 @@ enum ip_conntrack_status {
 	IPS_OFFLOAD_BIT = 14,
 	IPS_OFFLOAD = (1 << IPS_OFFLOAD_BIT),
 
+	/* Conntrack has been offloaded to hardware. */
+	IPS_HW_OFFLOAD_BIT = 15,
+	IPS_HW_OFFLOAD = (1 << IPS_HW_OFFLOAD_BIT),
+
 	/* Be careful here, modifying these bits can make things messy,
 	 * so don't let users modify them directly.
 	 */
 	IPS_UNCHANGEABLE_MASK = (IPS_NAT_DONE_MASK | IPS_NAT_MASK |
 				 IPS_EXPECTED | IPS_CONFIRMED | IPS_DYING |
-				 IPS_SEQ_ADJUST | IPS_TEMPLATE | IPS_OFFLOAD),
+				 IPS_SEQ_ADJUST | IPS_TEMPLATE | IPS_UNTRACKED |
+				 IPS_OFFLOAD | IPS_HW_OFFLOAD),
 
-	__IPS_MAX_BIT = 15,
+	__IPS_MAX_BIT = 16,
 };
 
 /* Connection tracking event types */
diff --git a/src/conntrack.c b/src/conntrack.c
index f65926b298ad..fb4e5be86ed8 100644
--- a/src/conntrack.c
+++ b/src/conntrack.c
@@ -870,8 +870,8 @@ static struct parse_parameter {
 	size_t  size;
 	unsigned int value[8];
 } parse_array[PARSE_MAX] = {
-	{ {"ASSURED", "SEEN_REPLY", "UNSET", "FIXED_TIMEOUT", "EXPECTED", "OFFLOAD"}, 6,
-	  { IPS_ASSURED, IPS_SEEN_REPLY, 0, IPS_FIXED_TIMEOUT, IPS_EXPECTED, IPS_OFFLOAD} },
+	{ {"ASSURED", "SEEN_REPLY", "UNSET", "FIXED_TIMEOUT", "EXPECTED", "OFFLOAD", "HW_OFFLOAD"}, 7,
+	  { IPS_ASSURED, IPS_SEEN_REPLY, 0, IPS_FIXED_TIMEOUT, IPS_EXPECTED, IPS_OFFLOAD, IPS_HW_OFFLOAD} },
 	{ {"ALL", "NEW", "UPDATES", "DESTROY"}, 4,
 	  { CT_EVENT_F_ALL, CT_EVENT_F_NEW, CT_EVENT_F_UPD, CT_EVENT_F_DEL } },
 	{ {"xml", "extended", "timestamp", "id", "ktimestamp", "labels", "userspace" }, 7,

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [nf-next V2] netfilter: nf_conntrack, add IPS_HW_OFFLOAD status bit
  2020-04-26 21:53 ` Pablo Neira Ayuso
@ 2020-04-27 21:02   ` Bodong Wang
  0 siblings, 0 replies; 3+ messages in thread
From: Bodong Wang @ 2020-04-27 21:02 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel, ozsh, paulb

On 4/26/2020 4:53 PM, Pablo Neira Ayuso wrote:
> On Tue, Apr 21, 2020 at 10:04:16AM -0500, Bodong Wang wrote:
>> This bit indicates that the conntrack entry is offloaded to hardware
>> flow table. nf_conntrack entry will be tagged with [HW_OFFLOAD] if
>> it's offload to hardware.
>>
>> cat /proc/net/nf_conntrack
>> 	ipv4 2 tcp 6 \
>> 	src=1.1.1.17 dst=1.1.1.16 sport=56394 dport=5001 \
>> 	src=1.1.1.16 dst=1.1.1.17 sport=5001 dport=56394 [HW_OFFLOAD] \
>> 	mark=0 zone=0 use=3
>>
>> Note that HW_OFFLOAD/OFFLOAD/ASSURED are mutually exclusive.
> Applied, thanks.
>
> Could you also test the following userspace patches for
> libnetfilter_conntrack and the conntrack-tools to get the netlink
> tools in feature parity? If they work fine there, I'll formally submit
> them.
>
> Thanks.

Hi Pablo,

I tested your patches, they worked well.

Thanks!

# conntrack -L | grep 1.1.1.16
conntrack v1.4.6 (conntrack-tools): 12 flow entries have been shown.
tcp      6 src=1.1.1.17 dst=1.1.1.16 sport=56408 dport=5001 src=1.1.1.16 
dst=1.1.1.17 sport=5001 dport=56408 [HW_OFFLOAD] mark=0 use=2
tcp      6 src=1.1.1.17 dst=1.1.1.16 sport=56404 dport=5001 src=1.1.1.16 
dst=1.1.1.17 sport=5001 dport=56404 [HW_OFFLOAD] mark=0 use=2

# cat /proc/net/nf_conntrack | grep 1.1.1.16
ipv4     2 tcp      6 src=1.1.1.17 dst=1.1.1.16 sport=56408 dport=5001 
src=1.1.1.16 dst=1.1.1.17 sport=5001 dport=56408 [HW_OFFLOAD] mark=0 
zone=0 use=3
ipv4     2 tcp      6 src=1.1.1.17 dst=1.1.1.16 sport=56404 dport=5001 
src=1.1.1.16 dst=1.1.1.17 sport=5001 dport=56404 [HW_OFFLOAD] mark=0 
zone=0 use=3


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-04-27 21:02 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-21 15:04 [nf-next V2] netfilter: nf_conntrack, add IPS_HW_OFFLOAD status bit Bodong Wang
2020-04-26 21:53 ` Pablo Neira Ayuso
2020-04-27 21:02   ` Bodong Wang

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.