[PATCH net 0/2] ipv6: fixes for the multicast routes

[PATCH net 0/2] ipv6: fixes for the multicast routes

[Matteo Croce]

From: Matteo Croce <mcroce@microsoft.com>

Fix two wrong flags in the IPv6 multicast routes created
by the autoconf code.

Matteo Croce (2):
  ipv6: create multicast route with RTPROT_KERNEL
  ipv6: set multicast flag on the multicast route

 net/ipv6/addrconf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

-- 
2.29.2

[PATCH net 1/2] ipv6: create multicast route with RTPROT_KERNEL

[Matteo Croce]

From: Matteo Croce <mcroce@microsoft.com>

The ff00::/8 multicast route is created without specifying the fc_protocol
field, so the default RTPROT_BOOT value is used:

  $ ip -6 -d route
  unicast ::1 dev lo proto kernel scope global metric 256 pref medium
  unicast fe80::/64 dev eth0 proto kernel scope global metric 256 pref medium
  unicast ff00::/8 dev eth0 proto boot scope global metric 256 pref medium

As the documentation says, this value identifies routes installed during
boot, but the route is created when interface is set up.
Change the value to RTPROT_KERNEL which is a better value.

Signed-off-by: Matteo Croce <mcroce@microsoft.com>
---
 net/ipv6/addrconf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index eff2cacd5209..19bf6822911c 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2469,6 +2469,7 @@ static void addrconf_add_mroute(struct net_device *dev)
 		.fc_flags = RTF_UP,
 		.fc_type = RTN_UNICAST,
 		.fc_nlinfo.nl_net = dev_net(dev),
+		.fc_protocol = RTPROT_KERNEL,
 	};
 
 	ipv6_addr_set(&cfg.fc_dst, htonl(0xFF000000), 0, 0, 0);
-- 
2.29.2

[PATCH net 2/2] ipv6: set multicast flag on the multicast route

[Matteo Croce]

From: Matteo Croce <mcroce@microsoft.com>

The multicast route ff00::/8 is created with type RTN_UNICAST:

  $ ip -6 -d route
  unicast ::1 dev lo proto kernel scope global metric 256 pref medium
  unicast fe80::/64 dev eth0 proto kernel scope global metric 256 pref medium
  unicast ff00::/8 dev eth0 proto kernel scope global metric 256 pref medium

Set the type to RTN_MULTICAST which is more appropriate.

Signed-off-by: Matteo Croce <mcroce@microsoft.com>
---
 net/ipv6/addrconf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 19bf6822911c..9edc5bb2d531 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2467,7 +2467,7 @@ static void addrconf_add_mroute(struct net_device *dev)
 		.fc_ifindex = dev->ifindex,
 		.fc_dst_len = 8,
 		.fc_flags = RTF_UP,
-		.fc_type = RTN_UNICAST,
+		.fc_type = RTN_MULTICAST,
 		.fc_nlinfo.nl_net = dev_net(dev),
 		.fc_protocol = RTPROT_KERNEL,
 	};
-- 
2.29.2

Re: [PATCH net 0/2] ipv6: fixes for the multicast routes

[Jakub Kicinski]

On Fri, 15 Jan 2021 19:42:07 +0100 Matteo Croce wrote:
> From: Matteo Croce <mcroce@microsoft.com>
> 
> Fix two wrong flags in the IPv6 multicast routes created
> by the autoconf code.

Any chance for Fixes tags here?

Re: [PATCH net 0/2] ipv6: fixes for the multicast routes

[Matteo Croce]

On Fri, Jan 15, 2021 at 11:50 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 15 Jan 2021 19:42:07 +0100 Matteo Croce wrote:
> > From: Matteo Croce <mcroce@microsoft.com>
> >
> > Fix two wrong flags in the IPv6 multicast routes created
> > by the autoconf code.
>
> Any chance for Fixes tags here?

Right.
For 1/2 I don't know exactly, that code was touched last time in
86872cb57925 ("[IPv6] route: FIB6 configuration using struct
fib6_config"), but it was only refactored. Before 86872cb57925, it was
pushed in the git import commit by Linus: 1da177e4c3f4
("Linux-2.6.12-rc2").
BTW, according the history repo, it entered the tree in the 2.4.0
import, so I'd say it's here since the beginning.

While for 2/2 I'd say:

Fixes: e8478e80e5a7 ("net/ipv6: Save route type in rt6_info")

-- 
per aspera ad upstream

Re: [PATCH net 1/2] ipv6: create multicast route with RTPROT_KERNEL

[David Ahern]

On 1/15/21 11:42 AM, Matteo Croce wrote:
> From: Matteo Croce <mcroce@microsoft.com>
> 
> The ff00::/8 multicast route is created without specifying the fc_protocol
> field, so the default RTPROT_BOOT value is used:
> 
>   $ ip -6 -d route
>   unicast ::1 dev lo proto kernel scope global metric 256 pref medium
>   unicast fe80::/64 dev eth0 proto kernel scope global metric 256 pref medium
>   unicast ff00::/8 dev eth0 proto boot scope global metric 256 pref medium
> 
> As the documentation says, this value identifies routes installed during
> boot, but the route is created when interface is set up.
> Change the value to RTPROT_KERNEL which is a better value.
> 
> Signed-off-by: Matteo Croce <mcroce@microsoft.com>
> ---
>  net/ipv6/addrconf.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index eff2cacd5209..19bf6822911c 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -2469,6 +2469,7 @@ static void addrconf_add_mroute(struct net_device *dev)
>  		.fc_flags = RTF_UP,
>  		.fc_type = RTN_UNICAST,
>  		.fc_nlinfo.nl_net = dev_net(dev),
> +		.fc_protocol = RTPROT_KERNEL,
>  	};
>  
>  	ipv6_addr_set(&cfg.fc_dst, htonl(0xFF000000), 0, 0, 0);
> 


What's the motivation for changing this? ie., what s/w cares that it is
kernel vs boot?

Re: [PATCH net 0/2] ipv6: fixes for the multicast routes

[David Ahern]

On 1/15/21 4:12 PM, Matteo Croce wrote:
> On Fri, Jan 15, 2021 at 11:50 PM Jakub Kicinski <kuba@kernel.org> wrote:
>>
>> On Fri, 15 Jan 2021 19:42:07 +0100 Matteo Croce wrote:
>>> From: Matteo Croce <mcroce@microsoft.com>
>>>
>>> Fix two wrong flags in the IPv6 multicast routes created
>>> by the autoconf code.
>>
>> Any chance for Fixes tags here?
> 
> Right.
> For 1/2 I don't know exactly, that code was touched last time in
> 86872cb57925 ("[IPv6] route: FIB6 configuration using struct
> fib6_config"), but it was only refactored. Before 86872cb57925, it was
> pushed in the git import commit by Linus: 1da177e4c3f4
> ("Linux-2.6.12-rc2").
> BTW, according the history repo, it entered the tree in the 2.4.0
> import, so I'd say it's here since the beginning.
> 
> While for 2/2 I'd say:
> 
> Fixes: e8478e80e5a7 ("net/ipv6: Save route type in rt6_info")
> 

As I recall (memory jogging from commit description) my patch only moved
the setting from ip6_route_info_create default to here.

The change is correct, just thinking it goes back beyond 4.16. If
someone has a system running a 4.14 or earlier kernel it should be easy
to know if this was the default prior.

Re: [PATCH net 1/2] ipv6: create multicast route with RTPROT_KERNEL

[Matteo Croce]

On Sat, Jan 16, 2021 at 5:36 AM David Ahern <dsahern@gmail.com> wrote:
>
> On 1/15/21 11:42 AM, Matteo Croce wrote:
> > From: Matteo Croce <mcroce@microsoft.com>
> >
> > The ff00::/8 multicast route is created without specifying the fc_protocol
> > field, so the default RTPROT_BOOT value is used:
> >
> >   $ ip -6 -d route
> >   unicast ::1 dev lo proto kernel scope global metric 256 pref medium
> >   unicast fe80::/64 dev eth0 proto kernel scope global metric 256 pref medium
> >   unicast ff00::/8 dev eth0 proto boot scope global metric 256 pref medium
> >
> > As the documentation says, this value identifies routes installed during
> > boot, but the route is created when interface is set up.
> > Change the value to RTPROT_KERNEL which is a better value.
> >
> > Signed-off-by: Matteo Croce <mcroce@microsoft.com>
> > ---
> >  net/ipv6/addrconf.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> > index eff2cacd5209..19bf6822911c 100644
> > --- a/net/ipv6/addrconf.c
> > +++ b/net/ipv6/addrconf.c
> > @@ -2469,6 +2469,7 @@ static void addrconf_add_mroute(struct net_device *dev)
> >               .fc_flags = RTF_UP,
> >               .fc_type = RTN_UNICAST,
> >               .fc_nlinfo.nl_net = dev_net(dev),
> > +             .fc_protocol = RTPROT_KERNEL,
> >       };
> >
> >       ipv6_addr_set(&cfg.fc_dst, htonl(0xFF000000), 0, 0, 0);
> >
>
>
> What's the motivation for changing this? ie., what s/w cares that it is
> kernel vs boot?

A practical example: systemd-networkd explicitly ignores routes with
kernel flag[1]. Having a different flag leads the daemon to remove the
route sooner or later.

[1] https://github.com/systemd/systemd/blob/0b81225e5791f660506f7db0ab88078cf296b771/src/network/networkd-routing-policy-rule.c#L675-L677
-- 
per aspera ad upstream

Re: [PATCH net 0/2] ipv6: fixes for the multicast routes

[Matteo Croce]

On Sat, Jan 16, 2021 at 5:41 AM David Ahern <dsahern@gmail.com> wrote:
>
> On 1/15/21 4:12 PM, Matteo Croce wrote:
> > On Fri, Jan 15, 2021 at 11:50 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >>
> >> On Fri, 15 Jan 2021 19:42:07 +0100 Matteo Croce wrote:
> >>> From: Matteo Croce <mcroce@microsoft.com>
> >>>
> >>> Fix two wrong flags in the IPv6 multicast routes created
> >>> by the autoconf code.
> >>
> >> Any chance for Fixes tags here?
> >
> > Right.
> > For 1/2 I don't know exactly, that code was touched last time in
> > 86872cb57925 ("[IPv6] route: FIB6 configuration using struct
> > fib6_config"), but it was only refactored. Before 86872cb57925, it was
> > pushed in the git import commit by Linus: 1da177e4c3f4
> > ("Linux-2.6.12-rc2").
> > BTW, according the history repo, it entered the tree in the 2.4.0
> > import, so I'd say it's here since the beginning.
> >
> > While for 2/2 I'd say:
> >
> > Fixes: e8478e80e5a7 ("net/ipv6: Save route type in rt6_info")
> >
>
> As I recall (memory jogging from commit description) my patch only moved
> the setting from ip6_route_info_create default to here.
>
> The change is correct, just thinking it goes back beyond 4.16. If
> someone has a system running a 4.14 or earlier kernel it should be easy
> to know if this was the default prior.

Indeed, it was the same long before 4.14:

# uname -a
Linux ubuntu 4.4.0-142-generic #168~14.04.1-Ubuntu SMP Sat Jan 19
11:26:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
# ip -6 -d route show table local dev eth0
unicast ff00::/8 proto boot scope global metric 256 pref medium

Regards,
-- 
per aspera ad upstream

Re: [PATCH net 0/2] ipv6: fixes for the multicast routes

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (refs/heads/master):

On Fri, 15 Jan 2021 19:42:07 +0100 you wrote:
> From: Matteo Croce <mcroce@microsoft.com>
> 
> Fix two wrong flags in the IPv6 multicast routes created
> by the autoconf code.
> 
> Matteo Croce (2):
>   ipv6: create multicast route with RTPROT_KERNEL
>   ipv6: set multicast flag on the multicast route
> 
> [...]

Here is the summary with links:
  - [net,1/2] ipv6: create multicast route with RTPROT_KERNEL
    https://git.kernel.org/netdev/net/c/a826b04303a4
  - [net,2/2] ipv6: set multicast flag on the multicast route
    https://git.kernel.org/netdev/net/c/ceed9038b278

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html