diff mbox series

[nf,v2,3/3] netfilter: nf_flow_table_offload: fix the nat port mangle.

Message ID 1576572767-19779-4-git-send-email-wenxu@ucloud.cn
State Accepted
Delegated to: Pablo Neira
Headers show
Series netfilter: nf_flow_table_offload: something fixes | expand

Commit Message

wenxu Dec. 17, 2019, 8:52 a.m. UTC 
From: wenxu <wenxu@ucloud.cn>

For dnat:
The original dir maybe modify the dst port to src port of reply dir
The reply dir maybe modify the src port to dst port of origin dir

For snat:
The original dir maybe modify the src port to dst port of reply dir
The reply dir maybe modify the dst port to src port of reply dir

Fixes: c29f74e0df7a ("netfilter: nf_flow_table: hardware offload support")
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
v2: no change

 net/netfilter/nf_flow_table_offload.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

Comments

Pablo Neira Ayuso Dec. 19, 2019, 11:35 p.m. UTC | #1 
On Tue, Dec 17, 2019 at 04:52:47PM +0800, wenxu@ucloud.cn wrote:
> From: wenxu <wenxu@ucloud.cn>
> 
> For dnat:
> The original dir maybe modify the dst port to src port of reply dir
> The reply dir maybe modify the src port to dst port of origin dir
> 
> For snat:
> The original dir maybe modify the src port to dst port of reply dir
> The reply dir maybe modify the dst port to src port of reply dir

Good catch.

Probably this description is better, and good for the record:

                SNAT         after mangling
    original   A -> B   =>    _FW_ -> B
     reply     B -> FW  =>       B -> _A_

                DNAT         after mangling
    original   A -> FW  =>       A -> _B_
     reply     B -> A   =>     _FW_-> A

This patch is also fixing incorrect 7acd9378dc652 BTW.

Thanks.
diff mbox series

Patch

diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
index acaa1ef..95f6dc4 100644
--- a/net/netfilter/nf_flow_table_offload.c
+++ b/net/netfilter/nf_flow_table_offload.c
@@ -349,22 +349,26 @@  static void flow_offload_port_snat(struct net *net,
 				   struct nf_flow_rule *flow_rule)
 {
 	struct flow_action_entry *entry = flow_action_entry_next(flow_rule);
-	u32 mask = ~htonl(0xffff0000), port;
+	u32 mask, port;
 	u32 offset;
 
 	switch (dir) {
 	case FLOW_OFFLOAD_DIR_ORIGINAL:
 		port = ntohs(flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.dst_port);
 		offset = 0; /* offsetof(struct tcphdr, source); */
+		port = htonl(port << 16);
+		mask = ~htonl(0xffff0000);
 		break;
 	case FLOW_OFFLOAD_DIR_REPLY:
 		port = ntohs(flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.src_port);
 		offset = 0; /* offsetof(struct tcphdr, dest); */
+		port = htonl(port);
+		mask = ~htonl(0xffff);
 		break;
 	default:
 		return;
 	}
-	port = htonl(port << 16);
+
 	flow_offload_mangle(entry, flow_offload_l4proto(flow), offset,
 			    &port, &mask);
 }
@@ -375,22 +379,26 @@  static void flow_offload_port_dnat(struct net *net,
 				   struct nf_flow_rule *flow_rule)
 {
 	struct flow_action_entry *entry = flow_action_entry_next(flow_rule);
-	u32 mask = ~htonl(0xffff), port;
+	u32 mask, port;
 	u32 offset;
 
 	switch (dir) {
 	case FLOW_OFFLOAD_DIR_ORIGINAL:
-		port = ntohs(flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.dst_port);
-		offset = 0; /* offsetof(struct tcphdr, source); */
+		port = ntohs(flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.src_port);
+		offset = 0; /* offsetof(struct tcphdr, dest); */
+		port = htonl(port);
+		mask = ~htonl(0xffff);
 		break;
 	case FLOW_OFFLOAD_DIR_REPLY:
-		port = ntohs(flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.src_port);
-		offset = 0; /* offsetof(struct tcphdr, dest); */
+		port = ntohs(flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.dst_port);
+		offset = 0; /* offsetof(struct tcphdr, source); */
+		port = htonl(port << 16);
+		mask = ~htonl(0xffff0000);
 		break;
 	default:
 		return;
 	}
-	port = htonl(port);
+
 	flow_offload_mangle(entry, flow_offload_l4proto(flow), offset,
 			    &port, &mask);
 }