* [PATCH nf-next] netfilter: flowtable: fix nft_flow_route use saddr for reverse route
@ 2022-05-25 16:23 wenxu
2022-05-25 18:44 ` Pablo Neira Ayuso
0 siblings, 1 reply; 5+ messages in thread
From: wenxu @ 2022-05-25 16:23 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, wenxu
From: wenxu <wenxu@chinatelecom.cn>
The nf_flow_tabel get route through ip_route_output_key which
the saddr should be local ones. For the forward case it always
can't get the other_dst and can't offload any flows
Fixes: 3412e1641828 (netfilter: flowtable: nft_flow_route use more data for reverse route)
Signed-off-by: wenxu <wenxu@chinatelecom.cn>
---
net/netfilter/nft_flow_offload.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
index 40d18aa..742a494 100644
--- a/net/netfilter/nft_flow_offload.c
+++ b/net/netfilter/nft_flow_offload.c
@@ -230,7 +230,6 @@ static int nft_flow_route(const struct nft_pktinfo *pkt,
switch (nft_pf(pkt)) {
case NFPROTO_IPV4:
fl.u.ip4.daddr = ct->tuplehash[dir].tuple.src.u3.ip;
- fl.u.ip4.saddr = ct->tuplehash[dir].tuple.dst.u3.ip;
fl.u.ip4.flowi4_oif = nft_in(pkt)->ifindex;
fl.u.ip4.flowi4_iif = this_dst->dev->ifindex;
fl.u.ip4.flowi4_tos = RT_TOS(ip_hdr(pkt->skb)->tos);
@@ -238,7 +237,6 @@ static int nft_flow_route(const struct nft_pktinfo *pkt,
break;
case NFPROTO_IPV6:
fl.u.ip6.daddr = ct->tuplehash[dir].tuple.src.u3.in6;
- fl.u.ip6.saddr = ct->tuplehash[dir].tuple.dst.u3.in6;
fl.u.ip6.flowi6_oif = nft_in(pkt)->ifindex;
fl.u.ip6.flowi6_iif = this_dst->dev->ifindex;
fl.u.ip6.flowlabel = ip6_flowinfo(ipv6_hdr(pkt->skb));
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH nf-next] netfilter: flowtable: fix nft_flow_route use saddr for reverse route
2022-05-25 16:23 [PATCH nf-next] netfilter: flowtable: fix nft_flow_route use saddr for reverse route wenxu
@ 2022-05-25 18:44 ` Pablo Neira Ayuso
2022-05-25 20:09 ` Sven Auhagen
2022-05-26 1:22 ` wenxu
0 siblings, 2 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2022-05-25 18:44 UTC (permalink / raw)
To: wenxu; +Cc: netfilter-devel, sven.auhagen
On Wed, May 25, 2022 at 12:23:57PM -0400, wenxu@chinatelecom.cn wrote:
> From: wenxu <wenxu@chinatelecom.cn>
>
> The nf_flow_tabel get route through ip_route_output_key which
> the saddr should be local ones. For the forward case it always
> can't get the other_dst and can't offload any flows
>
> Fixes: 3412e1641828 (netfilter: flowtable: nft_flow_route use more data for reverse route)
> Signed-off-by: wenxu <wenxu@chinatelecom.cn>
> ---
> net/netfilter/nft_flow_offload.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
> index 40d18aa..742a494 100644
> --- a/net/netfilter/nft_flow_offload.c
> +++ b/net/netfilter/nft_flow_offload.c
> @@ -230,7 +230,6 @@ static int nft_flow_route(const struct nft_pktinfo *pkt,
> switch (nft_pf(pkt)) {
> case NFPROTO_IPV4:
> fl.u.ip4.daddr = ct->tuplehash[dir].tuple.src.u3.ip;
> - fl.u.ip4.saddr = ct->tuplehash[dir].tuple.dst.u3.ip;
I think this should be instead:
fl.u.ip4.saddr = ct->tuplehash[!dir].tuple.src.u3.ip;
to accordingly deal with snat and dnat.
> fl.u.ip4.flowi4_oif = nft_in(pkt)->ifindex;
> fl.u.ip4.flowi4_iif = this_dst->dev->ifindex;
> fl.u.ip4.flowi4_tos = RT_TOS(ip_hdr(pkt->skb)->tos);
> @@ -238,7 +237,6 @@ static int nft_flow_route(const struct nft_pktinfo *pkt,
> break;
> case NFPROTO_IPV6:
> fl.u.ip6.daddr = ct->tuplehash[dir].tuple.src.u3.in6;
> - fl.u.ip6.saddr = ct->tuplehash[dir].tuple.dst.u3.in6;
fl.u.ip6.saddr = ct->tuplehash[!dir].tuple.src.u3.in6;
> fl.u.ip6.flowi6_oif = nft_in(pkt)->ifindex;
> fl.u.ip6.flowi6_iif = this_dst->dev->ifindex;
> fl.u.ip6.flowlabel = ip6_flowinfo(ipv6_hdr(pkt->skb));
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH nf-next] netfilter: flowtable: fix nft_flow_route use saddr for reverse route
2022-05-25 18:44 ` Pablo Neira Ayuso
@ 2022-05-25 20:09 ` Sven Auhagen
2022-05-26 1:22 ` wenxu
2022-05-26 1:22 ` wenxu
1 sibling, 1 reply; 5+ messages in thread
From: Sven Auhagen @ 2022-05-25 20:09 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: wenxu, netfilter-devel
On Wed, May 25, 2022 at 08:44:56PM +0200, Pablo Neira Ayuso wrote:
> On Wed, May 25, 2022 at 12:23:57PM -0400, wenxu@chinatelecom.cn wrote:
> > From: wenxu <wenxu@chinatelecom.cn>
> >
> > The nf_flow_tabel get route through ip_route_output_key which
> > the saddr should be local ones. For the forward case it always
> > can't get the other_dst and can't offload any flows
> >
> > Fixes: 3412e1641828 (netfilter: flowtable: nft_flow_route use more data for reverse route)
> > Signed-off-by: wenxu <wenxu@chinatelecom.cn>
> > ---
> > net/netfilter/nft_flow_offload.c | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
> > index 40d18aa..742a494 100644
> > --- a/net/netfilter/nft_flow_offload.c
> > +++ b/net/netfilter/nft_flow_offload.c
> > @@ -230,7 +230,6 @@ static int nft_flow_route(const struct nft_pktinfo *pkt,
> > switch (nft_pf(pkt)) {
> > case NFPROTO_IPV4:
> > fl.u.ip4.daddr = ct->tuplehash[dir].tuple.src.u3.ip;
> > - fl.u.ip4.saddr = ct->tuplehash[dir].tuple.dst.u3.ip;
>
> I think this should be instead:
>
> fl.u.ip4.saddr = ct->tuplehash[!dir].tuple.src.u3.ip;
>
> to accordingly deal with snat and dnat.
Hi,
I think what is actually missing here to cover all cases is:
fl.u.ip4.flowi4_flags = FLOWI_FLAG_ANYSRC;
and
fl.u.ip6.flowi6_flags = FLOWI_FLAG_ANYSRC;
this is used in other places in the kernel to fix this problem.
Do you want me to send a fix for that?
Best
Sven
>
> > fl.u.ip4.flowi4_oif = nft_in(pkt)->ifindex;
> > fl.u.ip4.flowi4_iif = this_dst->dev->ifindex;
> > fl.u.ip4.flowi4_tos = RT_TOS(ip_hdr(pkt->skb)->tos);
> > @@ -238,7 +237,6 @@ static int nft_flow_route(const struct nft_pktinfo *pkt,
> > break;
> > case NFPROTO_IPV6:
> > fl.u.ip6.daddr = ct->tuplehash[dir].tuple.src.u3.in6;
> > - fl.u.ip6.saddr = ct->tuplehash[dir].tuple.dst.u3.in6;
>
> fl.u.ip6.saddr = ct->tuplehash[!dir].tuple.src.u3.in6;
>
> > fl.u.ip6.flowi6_oif = nft_in(pkt)->ifindex;
> > fl.u.ip6.flowi6_iif = this_dst->dev->ifindex;
> > fl.u.ip6.flowlabel = ip6_flowinfo(ipv6_hdr(pkt->skb));
> > --
> > 1.8.3.1
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH nf-next] netfilter: flowtable: fix nft_flow_route use saddr for reverse route
2022-05-25 20:09 ` Sven Auhagen
@ 2022-05-26 1:22 ` wenxu
0 siblings, 0 replies; 5+ messages in thread
From: wenxu @ 2022-05-26 1:22 UTC (permalink / raw)
To: Sven Auhagen, Pablo Neira Ayuso; +Cc: netfilter-devel
在 2022/5/26 4:09, Sven Auhagen 写道:
> On Wed, May 25, 2022 at 08:44:56PM +0200, Pablo Neira Ayuso wrote:
>> On Wed, May 25, 2022 at 12:23:57PM -0400, wenxu@chinatelecom.cn wrote:
>>> From: wenxu <wenxu@chinatelecom.cn>
>>>
>>> The nf_flow_tabel get route through ip_route_output_key which
>>> the saddr should be local ones. For the forward case it always
>>> can't get the other_dst and can't offload any flows
>>>
>>> Fixes: 3412e1641828 (netfilter: flowtable: nft_flow_route use more data for reverse route)
>>> Signed-off-by: wenxu <wenxu@chinatelecom.cn>
>>> ---
>>> net/netfilter/nft_flow_offload.c | 2 --
>>> 1 file changed, 2 deletions(-)
>>>
>>> diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
>>> index 40d18aa..742a494 100644
>>> --- a/net/netfilter/nft_flow_offload.c
>>> +++ b/net/netfilter/nft_flow_offload.c
>>> @@ -230,7 +230,6 @@ static int nft_flow_route(const struct nft_pktinfo *pkt,
>>> switch (nft_pf(pkt)) {
>>> case NFPROTO_IPV4:
>>> fl.u.ip4.daddr = ct->tuplehash[dir].tuple.src.u3.ip;
>>> - fl.u.ip4.saddr = ct->tuplehash[dir].tuple.dst.u3.ip;
>> I think this should be instead:
>>
>> fl.u.ip4.saddr = ct->tuplehash[!dir].tuple.src.u3.ip;
>>
>> to accordingly deal with snat and dnat.
> Hi,
>
> I think what is actually missing here to cover all cases is:
>
> fl.u.ip4.flowi4_flags = FLOWI_FLAG_ANYSRC;
>
> and
>
> fl.u.ip6.flowi6_flags = FLOWI_FLAG_ANYSRC;
Thank， This is much better. I will test and send a fix.
>
> this is used in other places in the kernel to fix this problem.
>
> Do you want me to send a fix for that?
>
> Best
> Sven
>
>>> fl.u.ip4.flowi4_oif = nft_in(pkt)->ifindex;
>>> fl.u.ip4.flowi4_iif = this_dst->dev->ifindex;
>>> fl.u.ip4.flowi4_tos = RT_TOS(ip_hdr(pkt->skb)->tos);
>>> @@ -238,7 +237,6 @@ static int nft_flow_route(const struct nft_pktinfo *pkt,
>>> break;
>>> case NFPROTO_IPV6:
>>> fl.u.ip6.daddr = ct->tuplehash[dir].tuple.src.u3.in6;
>>> - fl.u.ip6.saddr = ct->tuplehash[dir].tuple.dst.u3.in6;
>> fl.u.ip6.saddr = ct->tuplehash[!dir].tuple.src.u3.in6;
>>
>>> fl.u.ip6.flowi6_oif = nft_in(pkt)->ifindex;
>>> fl.u.ip6.flowi6_iif = this_dst->dev->ifindex;
>>> fl.u.ip6.flowlabel = ip6_flowinfo(ipv6_hdr(pkt->skb));
>>> --
>>> 1.8.3.1
>>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH nf-next] netfilter: flowtable: fix nft_flow_route use saddr for reverse route
2022-05-25 18:44 ` Pablo Neira Ayuso
2022-05-25 20:09 ` Sven Auhagen
@ 2022-05-26 1:22 ` wenxu
1 sibling, 0 replies; 5+ messages in thread
From: wenxu @ 2022-05-26 1:22 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, sven.auhagen
在 2022/5/26 2:44, Pablo Neira Ayuso 写道:
> On Wed, May 25, 2022 at 12:23:57PM -0400, wenxu@chinatelecom.cn wrote:
>> From: wenxu <wenxu@chinatelecom.cn>
>>
>> The nf_flow_tabel get route through ip_route_output_key which
>> the saddr should be local ones. For the forward case it always
>> can't get the other_dst and can't offload any flows
>>
>> Fixes: 3412e1641828 (netfilter: flowtable: nft_flow_route use more data for reverse route)
>> Signed-off-by: wenxu <wenxu@chinatelecom.cn>
>> ---
>> net/netfilter/nft_flow_offload.c | 2 --
>> 1 file changed, 2 deletions(-)
>>
>> diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
>> index 40d18aa..742a494 100644
>> --- a/net/netfilter/nft_flow_offload.c
>> +++ b/net/netfilter/nft_flow_offload.c
>> @@ -230,7 +230,6 @@ static int nft_flow_route(const struct nft_pktinfo *pkt,
>> switch (nft_pf(pkt)) {
>> case NFPROTO_IPV4:
>> fl.u.ip4.daddr = ct->tuplehash[dir].tuple.src.u3.ip;
>> - fl.u.ip4.saddr = ct->tuplehash[dir].tuple.dst.u3.ip;
> I think this should be instead:
>
> fl.u.ip4.saddr = ct->tuplehash[!dir].tuple.src.u3.ip;
>
> to accordingly deal with snat and dnat.
Yes, This is another problem. I will also send a fix.
>> fl.u.ip4.flowi4_oif = nft_in(pkt)->ifindex;
>> fl.u.ip4.flowi4_iif = this_dst->dev->ifindex;
>> fl.u.ip4.flowi4_tos = RT_TOS(ip_hdr(pkt->skb)->tos);
>> @@ -238,7 +237,6 @@ static int nft_flow_route(const struct nft_pktinfo *pkt,
>> break;
>> case NFPROTO_IPV6:
>> fl.u.ip6.daddr = ct->tuplehash[dir].tuple.src.u3.in6;
>> - fl.u.ip6.saddr = ct->tuplehash[dir].tuple.dst.u3.in6;
> fl.u.ip6.saddr = ct->tuplehash[!dir].tuple.src.u3.in6;
>
>> fl.u.ip6.flowi6_oif = nft_in(pkt)->ifindex;
>> fl.u.ip6.flowi6_iif = this_dst->dev->ifindex;
>> fl.u.ip6.flowlabel = ip6_flowinfo(ipv6_hdr(pkt->skb));
>> --
>> 1.8.3.1
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-05-26 1:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-05-25 16:23 [PATCH nf-next] netfilter: flowtable: fix nft_flow_route use saddr for reverse route wenxu
2022-05-25 18:44 ` Pablo Neira Ayuso
2022-05-25 20:09 ` Sven Auhagen
2022-05-26 1:22 ` wenxu
2022-05-26 1:22 ` wenxu
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.