* [PATCH nf v2 1/3] netfilter: nf_flow_table_offload: fix dst_neigh lookup
2019-12-17 8:52 [PATCH nf v2 0/3] netfilter: nf_flow_table_offload: something fixes wenxu
@ 2019-12-17 8:52 ` wenxu
2019-12-19 22:18 ` Pablo Neira Ayuso
2019-12-17 8:52 ` [PATCH nf v2 2/3] netfilter: nf_flow_table_offload: check the status of dst_neigh wenxu
2019-12-17 8:52 ` [PATCH nf v2 3/3] netfilter: nf_flow_table_offload: fix the nat port mangle wenxu
2 siblings, 1 reply; 12+ messages in thread
From: wenxu @ 2019-12-17 8:52 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel
From: wenxu <wenxu@ucloud.cn>
Get the dst_neigh through dst_ip, The dst_ip should get
through peer tuple.src_v4 fix for dnat case.
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 | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
index de7a0d1..91dd6eb 100644
--- a/net/netfilter/nf_flow_table_offload.c
+++ b/net/netfilter/nf_flow_table_offload.c
@@ -166,14 +166,16 @@ static int flow_offload_eth_dst(struct net *net,
enum flow_offload_tuple_dir dir,
struct nf_flow_rule *flow_rule)
{
- const struct flow_offload_tuple *tuple = &flow->tuplehash[dir].tuple;
struct flow_action_entry *entry0 = flow_action_entry_next(flow_rule);
struct flow_action_entry *entry1 = flow_action_entry_next(flow_rule);
+ const void *daddr = &flow->tuplehash[!dir].tuple.src_v4;
+ const struct dst_entry *dst_cache;
struct neighbour *n;
u32 mask, val;
u16 val16;
- n = dst_neigh_lookup(tuple->dst_cache, &tuple->dst_v4);
+ dst_cache = flow->tuplehash[dir].tuple.dst_cache;
+ n = dst_neigh_lookup(dst_cache, daddr);
if (!n)
return -ENOENT;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH nf v2 1/3] netfilter: nf_flow_table_offload: fix dst_neigh lookup
2019-12-17 8:52 ` [PATCH nf v2 1/3] netfilter: nf_flow_table_offload: fix dst_neigh lookup wenxu
@ 2019-12-19 22:18 ` Pablo Neira Ayuso
2019-12-20 1:18 ` Pablo Neira Ayuso
2019-12-20 3:53 ` wenxu
0 siblings, 2 replies; 12+ messages in thread
From: Pablo Neira Ayuso @ 2019-12-19 22:18 UTC (permalink / raw)
To: wenxu; +Cc: netfilter-devel
[-- Attachment #1: Type: text/plain, Size: 609 bytes --]
On Tue, Dec 17, 2019 at 04:52:45PM +0800, wenxu@ucloud.cn wrote:
> From: wenxu <wenxu@ucloud.cn>
>
> Get the dst_neigh through dst_ip, The dst_ip should get
> through peer tuple.src_v4 fix for dnat case.
Please, revamp patch description:
netfilter: nf_flow_table_offload: fix incorrect ethernet dst address
Proposed description:
original: A -> FW
reply: B -> A
Traffic going in original direction uses address B as
destination. Traffic going in reply direction uses address A
as destination.
I'd suggest a more simplified patch, attached.
[-- Attachment #2: x.patch --]
[-- Type: text/x-diff, Size: 859 bytes --]
diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
index 506aaaf8151d..8680fc56cd7c 100644
--- a/net/netfilter/nf_flow_table_offload.c
+++ b/net/netfilter/nf_flow_table_offload.c
@@ -156,14 +156,14 @@ static int flow_offload_eth_dst(struct net *net,
enum flow_offload_tuple_dir dir,
struct nf_flow_rule *flow_rule)
{
- const struct flow_offload_tuple *tuple = &flow->tuplehash[dir].tuple;
+ const struct flow_offload_tuple *tuple = &flow->tuplehash[!dir].tuple;
struct flow_action_entry *entry0 = flow_action_entry_next(flow_rule);
struct flow_action_entry *entry1 = flow_action_entry_next(flow_rule);
struct neighbour *n;
u32 mask, val;
u16 val16;
- n = dst_neigh_lookup(tuple->dst_cache, &tuple->dst_v4);
+ n = dst_neigh_lookup(tuple->dst_cache, &tuple->src_v4);
if (!n)
return -ENOENT;
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH nf v2 1/3] netfilter: nf_flow_table_offload: fix dst_neigh lookup
2019-12-19 22:18 ` Pablo Neira Ayuso
@ 2019-12-20 1:18 ` Pablo Neira Ayuso
2019-12-20 3:53 ` wenxu
1 sibling, 0 replies; 12+ messages in thread
From: Pablo Neira Ayuso @ 2019-12-20 1:18 UTC (permalink / raw)
To: wenxu; +Cc: netfilter-devel
On Thu, Dec 19, 2019 at 11:18:16PM +0100, Pablo Neira Ayuso wrote:
> On Tue, Dec 17, 2019 at 04:52:45PM +0800, wenxu@ucloud.cn wrote:
> > From: wenxu <wenxu@ucloud.cn>
> >
> > Get the dst_neigh through dst_ip, The dst_ip should get
> > through peer tuple.src_v4 fix for dnat case.
>
> Please, revamp patch description:
>
> netfilter: nf_flow_table_offload: fix incorrect ethernet dst address
>
> Proposed description:
>
> original: A -> FW
> reply: B -> A
This part above is not correct, actually FW should be B instead.
> Traffic going in original direction uses address B as
> destination. Traffic going in reply direction uses address A
> as destination.
This should be instead:
Ethernet destination for original traffic takes the source ethernet address
in the reply direction. For reply traffic, this takes the source
ethernet address of the original direction.
Hope this helps to clarify.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH nf v2 1/3] netfilter: nf_flow_table_offload: fix dst_neigh lookup
2019-12-19 22:18 ` Pablo Neira Ayuso
2019-12-20 1:18 ` Pablo Neira Ayuso
@ 2019-12-20 3:53 ` wenxu
2019-12-20 9:13 ` Pablo Neira Ayuso
1 sibling, 1 reply; 12+ messages in thread
From: wenxu @ 2019-12-20 3:53 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
Maybe the patch your suggestion is not correct?
On 12/20/2019 6:18 AM, Pablo Neira Ayuso wrote:
> diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
> index 506aaaf8151d..8680fc56cd7c 100644
> --- a/net/netfilter/nf_flow_table_offload.c
> +++ b/net/netfilter/nf_flow_table_offload.c
> @@ -156,14 +156,14 @@ static int flow_offload_eth_dst(struct net *net,
> enum flow_offload_tuple_dir dir,
> struct nf_flow_rule *flow_rule)
> {
> - const struct flow_offload_tuple *tuple = &flow->tuplehash[dir].tuple;
> + const struct flow_offload_tuple *tuple = &flow->tuplehash[!dir].tuple;
> struct flow_action_entry *entry0 = flow_action_entry_next(flow_rule);
> struct flow_action_entry *entry1 = flow_action_entry_next(flow_rule);
> struct neighbour *n;
> u32 mask, val;
> u16 val16;
>
> - n = dst_neigh_lookup(tuple->dst_cache, &tuple->dst_v4);
> + n = dst_neigh_lookup(tuple->dst_cache, &tuple->src_v4);
The dst_cache should be flow->tuplehash[dir].tuple.dst_cache but not peer dir's;
> if (!n)
> return -ENOENT;
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH nf v2 1/3] netfilter: nf_flow_table_offload: fix dst_neigh lookup
2019-12-20 3:53 ` wenxu
@ 2019-12-20 9:13 ` Pablo Neira Ayuso
2019-12-20 9:19 ` wenxu
0 siblings, 1 reply; 12+ messages in thread
From: Pablo Neira Ayuso @ 2019-12-20 9:13 UTC (permalink / raw)
To: wenxu; +Cc: netfilter-devel
On Fri, Dec 20, 2019 at 11:53:38AM +0800, wenxu wrote:
> Maybe the patch your suggestion is not correct?
>
> On 12/20/2019 6:18 AM, Pablo Neira Ayuso wrote:
> > diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
> > index 506aaaf8151d..8680fc56cd7c 100644
> > --- a/net/netfilter/nf_flow_table_offload.c
> > +++ b/net/netfilter/nf_flow_table_offload.c
> > @@ -156,14 +156,14 @@ static int flow_offload_eth_dst(struct net *net,
>
> > enum flow_offload_tuple_dir dir,
> > struct nf_flow_rule *flow_rule)
> > {
> > - const struct flow_offload_tuple *tuple = &flow->tuplehash[dir].tuple;
> > + const struct flow_offload_tuple *tuple = &flow->tuplehash[!dir].tuple;
> > struct flow_action_entry *entry0 = flow_action_entry_next(flow_rule);
> > struct flow_action_entry *entry1 = flow_action_entry_next(flow_rule);
> > struct neighbour *n;
> > u32 mask, val;
> > u16 val16;
> >
> > - n = dst_neigh_lookup(tuple->dst_cache, &tuple->dst_v4);
> > + n = dst_neigh_lookup(tuple->dst_cache, &tuple->src_v4);
> The dst_cache should be flow->tuplehash[dir].tuple.dst_cache but not peer dir's;
Hm, I think this is like your patch, but without the two extra new lines
and new variable definitions.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH nf v2 1/3] netfilter: nf_flow_table_offload: fix dst_neigh lookup
2019-12-20 9:13 ` Pablo Neira Ayuso
@ 2019-12-20 9:19 ` wenxu
2019-12-20 10:31 ` Pablo Neira Ayuso
0 siblings, 1 reply; 12+ messages in thread
From: wenxu @ 2019-12-20 9:19 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
On 12/20/2019 5:13 PM, Pablo Neira Ayuso wrote:
> On Fri, Dec 20, 2019 at 11:53:38AM +0800, wenxu wrote:
>> Maybe the patch your suggestion is not correct?
>>
>> On 12/20/2019 6:18 AM, Pablo Neira Ayuso wrote:
>>> diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
>>> index 506aaaf8151d..8680fc56cd7c 100644
>>> --- a/net/netfilter/nf_flow_table_offload.c
>>> +++ b/net/netfilter/nf_flow_table_offload.c
>>> @@ -156,14 +156,14 @@ static int flow_offload_eth_dst(struct net *net,
>>> enum flow_offload_tuple_dir dir,
>>> struct nf_flow_rule *flow_rule)
>>> {
>>> - const struct flow_offload_tuple *tuple = &flow->tuplehash[dir].tuple;
>>> + const struct flow_offload_tuple *tuple = &flow->tuplehash[!dir].tuple;
>>> struct flow_action_entry *entry0 = flow_action_entry_next(flow_rule);
>>> struct flow_action_entry *entry1 = flow_action_entry_next(flow_rule);
>>> struct neighbour *n;
>>> u32 mask, val;
>>> u16 val16;
>>>
>>> - n = dst_neigh_lookup(tuple->dst_cache, &tuple->dst_v4);
>>> + n = dst_neigh_lookup(tuple->dst_cache, &tuple->src_v4);
>> The dst_cache should be flow->tuplehash[dir].tuple.dst_cache but not peer dir's;
> Hm, I think this is like your patch, but without the two extra new lines
> and new variable definitions.
There is a little bit different. The dst_cache should get from flow->tuplehash[dir].tuple.dst_cache
but not flow->tuplehash[!dir].tuple.dst_cache
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH nf v2 1/3] netfilter: nf_flow_table_offload: fix dst_neigh lookup
2019-12-20 9:19 ` wenxu
@ 2019-12-20 10:31 ` Pablo Neira Ayuso
2019-12-20 14:15 ` wenxu
0 siblings, 1 reply; 12+ messages in thread
From: Pablo Neira Ayuso @ 2019-12-20 10:31 UTC (permalink / raw)
To: wenxu; +Cc: netfilter-devel
On Fri, Dec 20, 2019 at 05:19:12PM +0800, wenxu wrote:
>
> On 12/20/2019 5:13 PM, Pablo Neira Ayuso wrote:
> > On Fri, Dec 20, 2019 at 11:53:38AM +0800, wenxu wrote:
> >> Maybe the patch your suggestion is not correct?
> >>
> >> On 12/20/2019 6:18 AM, Pablo Neira Ayuso wrote:
> >>> diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
> >>> index 506aaaf8151d..8680fc56cd7c 100644
> >>> --- a/net/netfilter/nf_flow_table_offload.c
> >>> +++ b/net/netfilter/nf_flow_table_offload.c
> >>> @@ -156,14 +156,14 @@ static int flow_offload_eth_dst(struct net *net,
> >>> enum flow_offload_tuple_dir dir,
> >>> struct nf_flow_rule *flow_rule)
> >>> {
> >>> - const struct flow_offload_tuple *tuple = &flow->tuplehash[dir].tuple;
> >>> + const struct flow_offload_tuple *tuple = &flow->tuplehash[!dir].tuple;
> >>> struct flow_action_entry *entry0 = flow_action_entry_next(flow_rule);
> >>> struct flow_action_entry *entry1 = flow_action_entry_next(flow_rule);
> >>> struct neighbour *n;
> >>> u32 mask, val;
> >>> u16 val16;
> >>>
> >>> - n = dst_neigh_lookup(tuple->dst_cache, &tuple->dst_v4);
> >>> + n = dst_neigh_lookup(tuple->dst_cache, &tuple->src_v4);
> >> The dst_cache should be flow->tuplehash[dir].tuple.dst_cache but not peer dir's;
> > Hm, I think this is like your patch, but without the two extra new lines
> > and new variable definitions.
>
> There is a little bit different. The dst_cache should get from flow->tuplehash[dir].tuple.dst_cache
>
> but not flow->tuplehash[!dir].tuple.dst_cache
SNAT mangling ether dst
original A -> B C -> B [!dir].src
reply B -> C B -> A [!dir].src
DNAT mangling
original A -> C A -> B [!dir].src
reply B -> A C -> A [!dir].src
SNAT+DNAT mangling
original A -> C2 C1 -> B [!dir].src
reply B -> C1 C2 -> A [!dir].src
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH nf v2 1/3] netfilter: nf_flow_table_offload: fix dst_neigh lookup
2019-12-20 10:31 ` Pablo Neira Ayuso
@ 2019-12-20 14:15 ` wenxu
0 siblings, 0 replies; 12+ messages in thread
From: wenxu @ 2019-12-20 14:15 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
在 2019/12/20 18:31, Pablo Neira Ayuso 写道:
> On Fri, Dec 20, 2019 at 05:19:12PM +0800, wenxu wrote:
>> On 12/20/2019 5:13 PM, Pablo Neira Ayuso wrote:
>>> On Fri, Dec 20, 2019 at 11:53:38AM +0800, wenxu wrote:
>>>> Maybe the patch your suggestion is not correct?
>>>>
>>>> On 12/20/2019 6:18 AM, Pablo Neira Ayuso wrote:
>>>>> diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
>>>>> index 506aaaf8151d..8680fc56cd7c 100644
>>>>> --- a/net/netfilter/nf_flow_table_offload.c
>>>>> +++ b/net/netfilter/nf_flow_table_offload.c
>>>>> @@ -156,14 +156,14 @@ static int flow_offload_eth_dst(struct net *net,
>>>>> enum flow_offload_tuple_dir dir,
>>>>> struct nf_flow_rule *flow_rule)
>>>>> {
>>>>> - const struct flow_offload_tuple *tuple = &flow->tuplehash[dir].tuple;
>>>>> + const struct flow_offload_tuple *tuple = &flow->tuplehash[!dir].tuple;
>>>>> struct flow_action_entry *entry0 = flow_action_entry_next(flow_rule);
>>>>> struct flow_action_entry *entry1 = flow_action_entry_next(flow_rule);
>>>>> struct neighbour *n;
>>>>> u32 mask, val;
>>>>> u16 val16;
>>>>>
>>>>> - n = dst_neigh_lookup(tuple->dst_cache, &tuple->dst_v4);
>>>>> + n = dst_neigh_lookup(tuple->dst_cache, &tuple->src_v4);
>>>> The dst_cache should be flow->tuplehash[dir].tuple.dst_cache but not peer dir's;
>>> Hm, I think this is like your patch, but without the two extra new lines
>>> and new variable definitions.
>> There is a little bit different. The dst_cache should get from flow->tuplehash[dir].tuple.dst_cache
>>
>> but not flow->tuplehash[!dir].tuple.dst_cache
> SNAT mangling ether dst
> original A -> B C -> B [!dir].src
> reply B -> C B -> A [!dir].src
>
> DNAT mangling
> original A -> C A -> B [!dir].src
> reply B -> A C -> A [!dir].src
>
> SNAT+DNAT mangling
> original A -> C2 C1 -> B [!dir].src
> reply B -> C1 C2 -> A [!dir].src
yes, The daddr should always set as [!dir].src .
The problem I point out is the first parameters of dst_neigh_lookup, the
dst_entry used to find the gateway or daddr neighbor through dst_entry->dev.
This dst_entry should alway set as [dir].dst_entry.
For example :
DNAT mangling
original A -> C A -> B [!dir].src
reply B -> A C -> A [!dir].src
for the original dir: device receive from D1 and forward to D2,
So the mangle addr B([!dir].src) should do neigh lookup on D2([dir].dst_entry->dev)
BR
wenxu
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH nf v2 2/3] netfilter: nf_flow_table_offload: check the status of dst_neigh
2019-12-17 8:52 [PATCH nf v2 0/3] netfilter: nf_flow_table_offload: something fixes wenxu
2019-12-17 8:52 ` [PATCH nf v2 1/3] netfilter: nf_flow_table_offload: fix dst_neigh lookup wenxu
@ 2019-12-17 8:52 ` wenxu
2019-12-17 8:52 ` [PATCH nf v2 3/3] netfilter: nf_flow_table_offload: fix the nat port mangle wenxu
2 siblings, 0 replies; 12+ messages in thread
From: wenxu @ 2019-12-17 8:52 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel
From: wenxu <wenxu@ucloud.cn>
It is better to get the dst_neigh with neigh->lock and check the
nud_state is VALID
Fixes: c29f74e0df7a ("netfilter: nf_flow_table: hardware offload support")
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
v2: neigh_release for non NUD_VALID neighbor
net/netfilter/nf_flow_table_offload.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
index 91dd6eb..acaa1ef 100644
--- a/net/netfilter/nf_flow_table_offload.c
+++ b/net/netfilter/nf_flow_table_offload.c
@@ -170,8 +170,10 @@ static int flow_offload_eth_dst(struct net *net,
struct flow_action_entry *entry1 = flow_action_entry_next(flow_rule);
const void *daddr = &flow->tuplehash[!dir].tuple.src_v4;
const struct dst_entry *dst_cache;
+ unsigned char ha[ETH_ALEN];
struct neighbour *n;
u32 mask, val;
+ u8 nud_state;
u16 val16;
dst_cache = flow->tuplehash[dir].tuple.dst_cache;
@@ -179,13 +181,23 @@ static int flow_offload_eth_dst(struct net *net,
if (!n)
return -ENOENT;
+ read_lock_bh(&n->lock);
+ nud_state = n->nud_state;
+ ether_addr_copy(ha, n->ha);
+ read_unlock_bh(&n->lock);
+
+ if (!(nud_state & NUD_VALID)) {
+ neigh_release(n);
+ return -ENOENT;
+ }
+
mask = ~0xffffffff;
- memcpy(&val, n->ha, 4);
+ memcpy(&val, ha, 4);
flow_offload_mangle(entry0, FLOW_ACT_MANGLE_HDR_TYPE_ETH, 0,
&val, &mask);
mask = ~0x0000ffff;
- memcpy(&val16, n->ha + 4, 2);
+ memcpy(&val16, ha + 4, 2);
val = val16;
flow_offload_mangle(entry1, FLOW_ACT_MANGLE_HDR_TYPE_ETH, 4,
&val, &mask);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH nf v2 3/3] netfilter: nf_flow_table_offload: fix the nat port mangle.
2019-12-17 8:52 [PATCH nf v2 0/3] netfilter: nf_flow_table_offload: something fixes wenxu
2019-12-17 8:52 ` [PATCH nf v2 1/3] netfilter: nf_flow_table_offload: fix dst_neigh lookup wenxu
2019-12-17 8:52 ` [PATCH nf v2 2/3] netfilter: nf_flow_table_offload: check the status of dst_neigh wenxu
@ 2019-12-17 8:52 ` wenxu
2019-12-19 23:35 ` Pablo Neira Ayuso
2 siblings, 1 reply; 12+ messages in thread
From: wenxu @ 2019-12-17 8:52 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel
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(-)
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);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH nf v2 3/3] netfilter: nf_flow_table_offload: fix the nat port mangle.
2019-12-17 8:52 ` [PATCH nf v2 3/3] netfilter: nf_flow_table_offload: fix the nat port mangle wenxu
@ 2019-12-19 23:35 ` Pablo Neira Ayuso
0 siblings, 0 replies; 12+ messages in thread
From: Pablo Neira Ayuso @ 2019-12-19 23:35 UTC (permalink / raw)
To: wenxu; +Cc: netfilter-devel
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.
^ permalink raw reply [flat|nested] 12+ messages in thread