[PATCH nf] netfilter: Update ip6_route_me_harder to consider L3 domain

[PATCH nf] netfilter: Update ip6_route_me_harder to consider L3 domain

[Martin Willi]

The commit referenced below fixed packet re-routing if Netfilter mangles
a routing key property of a packet and the packet is routed in a VRF L3
domain. The fix, however, addressed IPv4 re-routing, only.

This commit applies the same behavior for IPv6. While at it, untangle
the nested ternary operator to make the code more readable.

Fixes: 6d8b49c3a3a3 ("netfilter: Update ip_route_me_harder to consider L3 domain")
Signed-off-by: Martin Willi <martin@strongswan.org>
---
 net/ipv6/netfilter.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/netfilter.c b/net/ipv6/netfilter.c
index 1da332450d98..8ce60ab89015 100644
--- a/net/ipv6/netfilter.c
+++ b/net/ipv6/netfilter.c
@@ -24,14 +24,13 @@ int ip6_route_me_harder(struct net *net, struct sock *sk_partial, struct sk_buff
 {
 	const struct ipv6hdr *iph = ipv6_hdr(skb);
 	struct sock *sk = sk_to_full_sk(sk_partial);
+	struct net_device *dev = skb_dst(skb)->dev;
 	struct flow_keys flkeys;
 	unsigned int hh_len;
 	struct dst_entry *dst;
 	int strict = (ipv6_addr_type(&iph->daddr) &
 		      (IPV6_ADDR_MULTICAST | IPV6_ADDR_LINKLOCAL));
 	struct flowi6 fl6 = {
-		.flowi6_oif = sk && sk->sk_bound_dev_if ? sk->sk_bound_dev_if :
-			strict ? skb_dst(skb)->dev->ifindex : 0,
 		.flowi6_mark = skb->mark,
 		.flowi6_uid = sock_net_uid(net, sk),
 		.daddr = iph->daddr,
@@ -39,6 +38,13 @@ int ip6_route_me_harder(struct net *net, struct sock *sk_partial, struct sk_buff
 	};
 	int err;
 
+	if (sk && sk->sk_bound_dev_if)
+		fl6.flowi6_oif = sk->sk_bound_dev_if;
+	else if (strict)
+		fl6.flowi6_oif = dev->ifindex;
+	else
+		fl6.flowi6_oif = l3mdev_master_ifindex(dev);
+
 	fib6_rules_early_flow_dissect(net, skb, &fl6, &flkeys);
 	dst = ip6_route_output(net, sk, &fl6);
 	err = dst->error;
-- 
2.25.1

Re: [PATCH nf] netfilter: Update ip6_route_me_harder to consider L3 domain

[David Ahern]

On 4/12/22 1:46 AM, Martin Willi wrote:
> @@ -39,6 +38,13 @@ int ip6_route_me_harder(struct net *net, struct sock *sk_partial, struct sk_buff
>  	};
>  	int err;
>  
> +	if (sk && sk->sk_bound_dev_if)
> +		fl6.flowi6_oif = sk->sk_bound_dev_if;
> +	else if (strict)
> +		fl6.flowi6_oif = dev->ifindex;
> +	else
> +		fl6.flowi6_oif = l3mdev_master_ifindex(dev);

For top of tree, this is now fl6.flowi6_l3mdev and dev is only needed
here so make this:
	fl6.flowi6_l3mdev = l3mdev_master_ifindex(skb_dst(skb)->dev);

> +
>  	fib6_rules_early_flow_dissect(net, skb, &fl6, &flkeys);
>  	dst = ip6_route_output(net, sk, &fl6);
>  	err = dst->error;

Re: [PATCH nf] netfilter: Update ip6_route_me_harder to consider L3 domain

[Martin Willi]

Hi David,

> > @@ -39,6 +38,13 @@ int ip6_route_me_harder(struct net *net, struct
> > sock *sk_partial, struct sk_buff
> >  	};
> >  	int err;
> >  
> > +	if (sk && sk->sk_bound_dev_if)
> > +		fl6.flowi6_oif = sk->sk_bound_dev_if;
> > +	else if (strict)
> > +		fl6.flowi6_oif = dev->ifindex;
> > +	else
> > +		fl6.flowi6_oif = l3mdev_master_ifindex(dev);
> 
> For top of tree, this is now fl6.flowi6_l3mdev

Ah, I see, missed that.

Given that IPv4 should be converted to flowi4_l3mdev as well (?), what
about:

 * Keep the IPv6 patch in this form, as this allows stable to pick it
   up as-is
 * I'll add a follow-up patch, which converts both to flowi[46]_l3mdev

This would avoid some noise for a separate stable patch, but let me
know what you prefer.

>  and dev is only needed here so make this:
> 	fl6.flowi6_l3mdev = l3mdev_master_ifindex(skb_dst(skb)->dev);

Actually it is used in that "strict" branch, this is why I've added
"dev" as a local variable. I guess that is still needed
with flowi6_l3mdev?

Thanks,
Martin

Re: [PATCH nf] netfilter: Update ip6_route_me_harder to consider L3 domain

[David Ahern]

On 4/13/22 3:05 AM, Martin Willi wrote:
> Hi David,
> 
>>> @@ -39,6 +38,13 @@ int ip6_route_me_harder(struct net *net, struct
>>> sock *sk_partial, struct sk_buff
>>>  	};
>>>  	int err;
>>>  
>>> +	if (sk && sk->sk_bound_dev_if)
>>> +		fl6.flowi6_oif = sk->sk_bound_dev_if;
>>> +	else if (strict)
>>> +		fl6.flowi6_oif = dev->ifindex;
>>> +	else
>>> +		fl6.flowi6_oif = l3mdev_master_ifindex(dev);
>>
>> For top of tree, this is now fl6.flowi6_l3mdev
> 
> Ah, I see, missed that.
> 
> Given that IPv4 should be converted to flowi4_l3mdev as well (?), what
> about:
> 
>  * Keep the IPv6 patch in this form, as this allows stable to pick it
>    up as-is
>  * I'll add a follow-up patch, which converts both to flowi[46]_l3mdev

sure, backport to stable will be easier.

> 
> This would avoid some noise for a separate stable patch, but let me
> know what you prefer.
> 
>>  and dev is only needed here so make this:
>> 	fl6.flowi6_l3mdev = l3mdev_master_ifindex(skb_dst(skb)->dev);
> 
> Actually it is used in that "strict" branch, this is why I've added
> "dev" as a local variable. I guess that is still needed
> with flowi6_l3mdev?

ah, missed the strict branch use.