| Message ID | 20211020013005.GA4864@ICIPI.localdomain (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Sysctl addr_gen_mode does not control tunnel link-local addr | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
Hi, On 20/10/2021 03:30, Stephen Suryaputra wrote: > Hi, > > I noticed that tunnels, especially sit before commit e5dd729460ca8 > ("ip/ip6_gre: use the same logic as SIT interfaces when computing v6LL > address"), generates link-local addr regardless of addr_gen_mode > setting. Is this a bug, or is there a specific reason? > > In my system, the link-local addr are generated by a userspace process. > So, we set net.ipv6.conf.<dev>.addr_gen_mode to 1 (IN6_ADDR_GEN_MODE_NONE). > > The commit e5dd729460ca8 doesn't change the behavior as it is renaming > sit_add_v4_addrs() to add_v4_addrs() as make it generic for GRE/IP6GRE > cases. I'm not sure what the current behavior is for GRE, i.e. whether > addr_gen_mode can control the generation of link-local addr. > > If this is a bug or oversight, I think this diff should fix it and I can > put a formal patch. When I implemented this behavior for SIT interfaces I basically ported what is currently happening for GRE. Therefore, any change you apply to SIT, should also be reflected in the GRE code path. As of now, the SIT/GRE addrgen logic does not consider the mode setting at all. But I am not sure it'd be easy to support all the values. For sure, supporting IN6_ADDR_GEN_MODE_NONE is a low hanging fruit and should be implemented. > > diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c > index c6a90b7bbb70..da7e83699eef 100644 > --- a/net/ipv6/addrconf.c > +++ b/net/ipv6/addrconf.c > @@ -3392,6 +3392,8 @@ static void addrconf_sit_config(struct net_device > *dev) > return; > } > > + if (idev->cnf.addr_gen_mode == IN6_ADDR_GEN_MODE_NONE) > + return; Please add an empty line here. > add_v4_addrs(idev); > > if (dev->flags&IFF_POINTOPOINT) > > The commit mention about addressing violation of RFC4291 on GRE and the diff > above doesn't cause it to change as the default addr_gen_mode is 0 > (IN6_ADDR_GEN_MODE_EUI64). Other than my nitpick above, I'd suggest applying the same change to the GRE code path, as the behavior should be the same. Moreover, do you think that addrconf_add_mroute() should not be executed in this case? Otherwise you could simply add your new code into add_v4_addrs(). Regards,
On Wed, Oct 20, 2021 at 02:59:01PM +0200, Antonio Quartulli wrote: > For sure, supporting IN6_ADDR_GEN_MODE_NONE is a low hanging fruit and > should be implemented. > Thanks for reaffirming this! > Other than my nitpick above, I'd suggest applying the same change to the > GRE code path, as the behavior should be the same. > Moreover, do you think that addrconf_add_mroute() should not be executed > in this case? Otherwise you could simply add your new code into > add_v4_addrs(). Initially I was thinking that why there is a need to call addrconf_add_mroute() given the link-local addr hasn't been set. On a second look, yes, the conditional is better be in add_v4_addrs(). It will take care of GRE as well. I'll put a formal patch. Thanks!
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index c6a90b7bbb70..da7e83699eef 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -3392,6 +3392,8 @@ static void addrconf_sit_config(struct net_device *dev) return; } + if (idev->cnf.addr_gen_mode == IN6_ADDR_GEN_MODE_NONE) + return; add_v4_addrs(idev);
Hi, I noticed that tunnels, especially sit before commit e5dd729460ca8 ("ip/ip6_gre: use the same logic as SIT interfaces when computing v6LL address"), generates link-local addr regardless of addr_gen_mode setting. Is this a bug, or is there a specific reason? In my system, the link-local addr are generated by a userspace process. So, we set net.ipv6.conf.<dev>.addr_gen_mode to 1 (IN6_ADDR_GEN_MODE_NONE). The commit e5dd729460ca8 doesn't change the behavior as it is renaming sit_add_v4_addrs() to add_v4_addrs() as make it generic for GRE/IP6GRE cases. I'm not sure what the current behavior is for GRE, i.e. whether addr_gen_mode can control the generation of link-local addr. If this is a bug or oversight, I think this diff should fix it and I can put a formal patch. if (dev->flags&IFF_POINTOPOINT) The commit mention about addressing violation of RFC4291 on GRE and the diff above doesn't cause it to change as the default addr_gen_mode is 0 (IN6_ADDR_GEN_MODE_EUI64). Thanks, Stephen.