All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] ip/ip6_gre: Fix GRE tunnels not generating IPv6 link local addresses
@ 2022-12-19  1:06 Thomas Winter
  2022-12-19  1:06 ` [PATCH 1/2] ip/ip6_gre: Fix changing addr gen mode not generating IPv6 link local address Thomas Winter
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Thomas Winter @ 2022-12-19  1:06 UTC (permalink / raw)
  To: davem, yoshfuji, dsahern, kuba, a, netdev, linux-kernel; +Cc: Thomas Winter

For our point-to-point GRE tunnels, they have IN6_ADDR_GEN_MODE_NONE
when they are created then we set IN6_ADDR_GEN_MODE_EUI64 when they
come up to generate the IPv6 link local address for the interface.
Recently we found that they were no longer generating IPv6 addresses.

Also, non-point-to-point tunnels were not generating any IPv6 link
local address and instead generating an IPv6 compat address,
breaking IPv6 communication on the tunnel.

These failures were caused by commit e5dd729460ca and this patch set
aims to resolve these issues.

Thomas Winter (2):
  ip/ip6_gre: Fix changing addr gen mode not generating IPv6 link local
    address
  ip/ip6_gre: Fix non-point-to-point tunnel not generating IPv6 link
    local address

 net/ipv6/addrconf.c | 57 ++++++++++++++++++++++++---------------------
 1 file changed, 31 insertions(+), 26 deletions(-)

-- 
2.37.3


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] ip/ip6_gre: Fix changing addr gen mode not generating IPv6 link local address
  2022-12-19  1:06 [PATCH 0/2] ip/ip6_gre: Fix GRE tunnels not generating IPv6 link local addresses Thomas Winter
@ 2022-12-19  1:06 ` Thomas Winter
  2022-12-20 13:16   ` Paolo Abeni
  2022-12-19  1:06 ` [PATCH 2/2] ip/ip6_gre: Fix non-point-to-point tunnel " Thomas Winter
  2022-12-20  2:25 ` [PATCH 0/2] ip/ip6_gre: Fix GRE tunnels not generating IPv6 link local addresses Chris Packham
  2 siblings, 1 reply; 12+ messages in thread
From: Thomas Winter @ 2022-12-19  1:06 UTC (permalink / raw)
  To: davem, yoshfuji, dsahern, kuba, a, netdev, linux-kernel; +Cc: Thomas Winter

Commit e5dd729460ca changed the code path so that GRE tunnels
generate an IPv6 address based on the tunnel source address.
It also changed the code path so GRE tunnels don't call addrconf_addr_gen
in addrconf_dev_config which is called by addrconf_sysctl_addr_gen_mode
when the IN6_ADDR_GEN_MODE is changed.

This patch aims to fix this issue by moving the code in addrconf_notify
which calls the addr gen for GRE and SIT into a separate function
and calling it in the places that expect the IPv6 address to be
generated.

The previous addrconf_dev_config is renamed to addrconf_eth_config
since it only expected eth type interfaces and follows the
addrconf_gre/sit_config format.

Fixes: e5dd729460ca ("ip/ip6_gre: use the same logic as SIT interfaces when computing v6LL address")
Signed-off-by: Thomas Winter <Thomas.Winter@alliedtelesis.co.nz>
---
 net/ipv6/addrconf.c | 47 +++++++++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 6dcf034835ec..e9d7ec03316d 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3355,7 +3355,7 @@ static void addrconf_addr_gen(struct inet6_dev *idev, bool prefix_route)
 	}
 }
 
-static void addrconf_dev_config(struct net_device *dev)
+static void addrconf_eth_config(struct net_device *dev)
 {
 	struct inet6_dev *idev;
 
@@ -3447,6 +3447,30 @@ static void addrconf_gre_config(struct net_device *dev)
 }
 #endif
 
+static void addrconf_dev_config(struct net_device *dev)
+{
+	switch (dev->type) {
+#if IS_ENABLED(CONFIG_IPV6_SIT)
+	case ARPHRD_SIT:
+		addrconf_sit_config(dev);
+		break;
+#endif
+#if IS_ENABLED(CONFIG_NET_IPGRE) || IS_ENABLED(CONFIG_IPV6_GRE)
+	case ARPHRD_IP6GRE:
+	case ARPHRD_IPGRE:
+		addrconf_gre_config(dev);
+		break;
+#endif
+	case ARPHRD_LOOPBACK:
+		init_loopback(dev);
+		break;
+
+	default:
+		addrconf_eth_config(dev);
+		break;
+	}
+}
+
 static int fixup_permanent_addr(struct net *net,
 				struct inet6_dev *idev,
 				struct inet6_ifaddr *ifp)
@@ -3611,26 +3635,7 @@ static int addrconf_notify(struct notifier_block *this, unsigned long event,
 			run_pending = 1;
 		}
 
-		switch (dev->type) {
-#if IS_ENABLED(CONFIG_IPV6_SIT)
-		case ARPHRD_SIT:
-			addrconf_sit_config(dev);
-			break;
-#endif
-#if IS_ENABLED(CONFIG_NET_IPGRE) || IS_ENABLED(CONFIG_IPV6_GRE)
-		case ARPHRD_IP6GRE:
-		case ARPHRD_IPGRE:
-			addrconf_gre_config(dev);
-			break;
-#endif
-		case ARPHRD_LOOPBACK:
-			init_loopback(dev);
-			break;
-
-		default:
-			addrconf_dev_config(dev);
-			break;
-		}
+		addrconf_dev_config(dev);
 
 		if (!IS_ERR_OR_NULL(idev)) {
 			if (run_pending)
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] ip/ip6_gre: Fix non-point-to-point tunnel not generating IPv6 link local address
  2022-12-19  1:06 [PATCH 0/2] ip/ip6_gre: Fix GRE tunnels not generating IPv6 link local addresses Thomas Winter
  2022-12-19  1:06 ` [PATCH 1/2] ip/ip6_gre: Fix changing addr gen mode not generating IPv6 link local address Thomas Winter
@ 2022-12-19  1:06 ` Thomas Winter
  2022-12-20  2:25 ` [PATCH 0/2] ip/ip6_gre: Fix GRE tunnels not generating IPv6 link local addresses Chris Packham
  2 siblings, 0 replies; 12+ messages in thread
From: Thomas Winter @ 2022-12-19  1:06 UTC (permalink / raw)
  To: davem, yoshfuji, dsahern, kuba, a, netdev, linux-kernel; +Cc: Thomas Winter

Previously, addrconf_gre_config always would call addrconf_addr_gen
and generate a EUI64 link local address for the tunnel.
Then commit e5dd729460ca changed the code path so that add_v4_addrs
is called but this only generates a compat IPv6 address for
non-point-to-point tunnels.

I assume the compat address is specifically for SIT tunnels so
have kept that only for SIT - GRE tunnels now always generate link
local addresses.

Fixes: e5dd729460ca ("ip/ip6_gre: use the same logic as SIT interfaces when computing v6LL address")
Signed-off-by: Thomas Winter <Thomas.Winter@alliedtelesis.co.nz>
---
 net/ipv6/addrconf.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index e9d7ec03316d..a51fb106fa4a 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3129,17 +3129,17 @@ static void add_v4_addrs(struct inet6_dev *idev)
 		offset = sizeof(struct in6_addr) - 4;
 	memcpy(&addr.s6_addr32[3], idev->dev->dev_addr + offset, 4);
 
-	if (idev->dev->flags&IFF_POINTOPOINT) {
+	if (!(idev->dev->flags & IFF_POINTOPOINT) && idev->dev->type == ARPHRD_SIT) {
+		scope = IPV6_ADDR_COMPATv4;
+		plen = 96;
+		pflags |= RTF_NONEXTHOP;
+	} else {
 		if (idev->cnf.addr_gen_mode == IN6_ADDR_GEN_MODE_NONE)
 			return;
 
 		addr.s6_addr32[0] = htonl(0xfe800000);
 		scope = IFA_LINK;
 		plen = 64;
-	} else {
-		scope = IPV6_ADDR_COMPATv4;
-		plen = 96;
-		pflags |= RTF_NONEXTHOP;
 	}
 
 	if (addr.s6_addr32[3]) {
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] ip/ip6_gre: Fix GRE tunnels not generating IPv6 link local addresses
  2022-12-19  1:06 [PATCH 0/2] ip/ip6_gre: Fix GRE tunnels not generating IPv6 link local addresses Thomas Winter
  2022-12-19  1:06 ` [PATCH 1/2] ip/ip6_gre: Fix changing addr gen mode not generating IPv6 link local address Thomas Winter
  2022-12-19  1:06 ` [PATCH 2/2] ip/ip6_gre: Fix non-point-to-point tunnel " Thomas Winter
@ 2022-12-20  2:25 ` Chris Packham
  2022-12-20  2:32   ` Chris Packham
  2 siblings, 1 reply; 12+ messages in thread
From: Chris Packham @ 2022-12-20  2:25 UTC (permalink / raw)
  To: Thomas Winter, davem, yoshfuji, dsahern, kuba, a, netdev, linux-kernel

Hi Thomas,

On 19/12/22 14:06, Thomas Winter wrote:
> For our point-to-point GRE tunnels, they have IN6_ADDR_GEN_MODE_NONE
> when they are created then we set IN6_ADDR_GEN_MODE_EUI64 when they
> come up to generate the IPv6 link local address for the interface.
> Recently we found that they were no longer generating IPv6 addresses.
>
> Also, non-point-to-point tunnels were not generating any IPv6 link
> local address and instead generating an IPv6 compat address,
> breaking IPv6 communication on the tunnel.
>
> These failures were caused by commit e5dd729460ca and this patch set
> aims to resolve these issues.

This appears to be a v2 of 
https://lore.kernel.org/all/20221218215718.1491444-1-Thomas.Winter@alliedtelesis.co.nz/#t 
but you haven't said so in the subject nor have you included a changelog 
in the patches or in the cover letter.

Also for networking patches you should include either "net" or 
"net-next" in the subject prefix. As this appears to be a bugfix "net" 
is appropriate.

>
> Thomas Winter (2):
>    ip/ip6_gre: Fix changing addr gen mode not generating IPv6 link local
>      address
>    ip/ip6_gre: Fix non-point-to-point tunnel not generating IPv6 link
>      local address
>
>   net/ipv6/addrconf.c | 57 ++++++++++++++++++++++++---------------------
>   1 file changed, 31 insertions(+), 26 deletions(-)
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] ip/ip6_gre: Fix GRE tunnels not generating IPv6 link local addresses
  2022-12-20  2:25 ` [PATCH 0/2] ip/ip6_gre: Fix GRE tunnels not generating IPv6 link local addresses Chris Packham
@ 2022-12-20  2:32   ` Chris Packham
  2022-12-20  3:06     ` Thomas Winter
  0 siblings, 1 reply; 12+ messages in thread
From: Chris Packham @ 2022-12-20  2:32 UTC (permalink / raw)
  To: Thomas Winter, davem, yoshfuji, dsahern, kuba, a, netdev, linux-kernel


On 20/12/22 15:25, Chris Packham wrote:
> Hi Thomas,
>
> On 19/12/22 14:06, Thomas Winter wrote:
>> For our point-to-point GRE tunnels, they have IN6_ADDR_GEN_MODE_NONE
>> when they are created then we set IN6_ADDR_GEN_MODE_EUI64 when they
>> come up to generate the IPv6 link local address for the interface.
>> Recently we found that they were no longer generating IPv6 addresses.
>>
>> Also, non-point-to-point tunnels were not generating any IPv6 link
>> local address and instead generating an IPv6 compat address,
>> breaking IPv6 communication on the tunnel.
>>
>> These failures were caused by commit e5dd729460ca and this patch set
>> aims to resolve these issues.
>
> This appears to be a v2 of 
> https://lore.kernel.org/all/20221218215718.1491444-1-Thomas.Winter@alliedtelesis.co.nz/#t 
> but you haven't said so in the subject nor have you included a 
> changelog in the patches or in the cover letter.
>
> Also for networking patches you should include either "net" or 
> "net-next" in the subject prefix. As this appears to be a bugfix "net" 
> is appropriate.
>
Took me a bit of looking but most of this stuff is covered by 
https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html#netdev-faq
>>
>> Thomas Winter (2):
>>    ip/ip6_gre: Fix changing addr gen mode not generating IPv6 link local
>>      address
>>    ip/ip6_gre: Fix non-point-to-point tunnel not generating IPv6 link
>>      local address
>>
>>   net/ipv6/addrconf.c | 57 ++++++++++++++++++++++++---------------------
>>   1 file changed, 31 insertions(+), 26 deletions(-)
>>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] ip/ip6_gre: Fix GRE tunnels not generating IPv6 link local addresses
  2022-12-20  2:32   ` Chris Packham
@ 2022-12-20  3:06     ` Thomas Winter
  2022-12-20  3:07       ` Chris Packham
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Winter @ 2022-12-20  3:06 UTC (permalink / raw)
  To: davem, dsahern, linux-kernel, yoshfuji, kuba, a, Chris Packham, netdev

On Tue, 2022-12-20 at 15:32 +1300, Chris Packham wrote:
> On 20/12/22 15:25, Chris Packham wrote:
> > Hi Thomas,
> > 
> > On 19/12/22 14:06, Thomas Winter wrote:
> > > For our point-to-point GRE tunnels, they have
> > > IN6_ADDR_GEN_MODE_NONE
> > > when they are created then we set IN6_ADDR_GEN_MODE_EUI64 when
> > > they
> > > come up to generate the IPv6 link local address for the
> > > interface.
> > > Recently we found that they were no longer generating IPv6
> > > addresses.
> > > 
> > > Also, non-point-to-point tunnels were not generating any IPv6
> > > link
> > > local address and instead generating an IPv6 compat address,
> > > breaking IPv6 communication on the tunnel.
> > > 
> > > These failures were caused by commit e5dd729460ca and this patch
> > > set
> > > aims to resolve these issues.
> > 
> > This appears to be a v2 of 
> > https://lore.kernel.org/all/20221218215718.1491444-1-Thomas.Winter@alliedtelesis.co.nz/#t 
> > but you haven't said so in the subject nor have you included a 
> > changelog in the patches or in the cover letter.
> > 
> > Also for networking patches you should include either "net" or 
> > "net-next" in the subject prefix. As this appears to be a bugfix
> > "net" 
> > is appropriate.
> > 
> Took me a bit of looking but most of this stuff is covered by 
> https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html#netdev-faq

Thanks, the git command I used did not put in the v2 that I expected
and I didn't check the output properly. I will send a new patch set as
v3.

> > > Thomas Winter (2):
> > >    ip/ip6_gre: Fix changing addr gen mode not generating IPv6
> > > link local
> > >      address
> > >    ip/ip6_gre: Fix non-point-to-point tunnel not generating IPv6
> > > link
> > >      local address
> > > 
> > >   net/ipv6/addrconf.c | 57 ++++++++++++++++++++++++------------
> > > ---------
> > >   1 file changed, 31 insertions(+), 26 deletions(-)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] ip/ip6_gre: Fix GRE tunnels not generating IPv6 link local addresses
  2022-12-20  3:06     ` Thomas Winter
@ 2022-12-20  3:07       ` Chris Packham
  0 siblings, 0 replies; 12+ messages in thread
From: Chris Packham @ 2022-12-20  3:07 UTC (permalink / raw)
  To: Thomas Winter, davem, dsahern, linux-kernel, yoshfuji, kuba, a, netdev


On 20/12/22 16:06, Thomas Winter wrote:
> On Tue, 2022-12-20 at 15:32 +1300, Chris Packham wrote:
>> On 20/12/22 15:25, Chris Packham wrote:
>>> Hi Thomas,
>>>
>>> On 19/12/22 14:06, Thomas Winter wrote:
>>>> For our point-to-point GRE tunnels, they have
>>>> IN6_ADDR_GEN_MODE_NONE
>>>> when they are created then we set IN6_ADDR_GEN_MODE_EUI64 when
>>>> they
>>>> come up to generate the IPv6 link local address for the
>>>> interface.
>>>> Recently we found that they were no longer generating IPv6
>>>> addresses.
>>>>
>>>> Also, non-point-to-point tunnels were not generating any IPv6
>>>> link
>>>> local address and instead generating an IPv6 compat address,
>>>> breaking IPv6 communication on the tunnel.
>>>>
>>>> These failures were caused by commit e5dd729460ca and this patch
>>>> set
>>>> aims to resolve these issues.
>>> This appears to be a v2 of
>>> https://lore.kernel.org/all/20221218215718.1491444-1-Thomas.Winter@alliedtelesis.co.nz/#t
>>> but you haven't said so in the subject nor have you included a
>>> changelog in the patches or in the cover letter.
>>>
>>> Also for networking patches you should include either "net" or
>>> "net-next" in the subject prefix. As this appears to be a bugfix
>>> "net"
>>> is appropriate.
>>>
>> Took me a bit of looking but most of this stuff is covered by
>> https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html#netdev-faq
> Thanks, the git command I used did not put in the v2 that I expected
> and I didn't check the output properly. I will send a new patch set as
> v3.
I don't think there's any particular need to rush. I wouldn't expect 
much of a response over the holiday period.
>>>> Thomas Winter (2):
>>>>     ip/ip6_gre: Fix changing addr gen mode not generating IPv6
>>>> link local
>>>>       address
>>>>     ip/ip6_gre: Fix non-point-to-point tunnel not generating IPv6
>>>> link
>>>>       local address
>>>>
>>>>    net/ipv6/addrconf.c | 57 ++++++++++++++++++++++++------------
>>>> ---------
>>>>    1 file changed, 31 insertions(+), 26 deletions(-)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] ip/ip6_gre: Fix changing addr gen mode not generating IPv6 link local address
  2022-12-19  1:06 ` [PATCH 1/2] ip/ip6_gre: Fix changing addr gen mode not generating IPv6 link local address Thomas Winter
@ 2022-12-20 13:16   ` Paolo Abeni
  2023-01-03 22:28     ` Thomas Winter
  0 siblings, 1 reply; 12+ messages in thread
From: Paolo Abeni @ 2022-12-20 13:16 UTC (permalink / raw)
  To: Thomas Winter, davem, yoshfuji, dsahern, kuba, a, netdev, linux-kernel

On Mon, 2022-12-19 at 14:06 +1300, Thomas Winter wrote:
> Commit e5dd729460ca changed the code path so that GRE tunnels
> generate an IPv6 address based on the tunnel source address.
> It also changed the code path so GRE tunnels don't call addrconf_addr_gen
> in addrconf_dev_config which is called by addrconf_sysctl_addr_gen_mode
> when the IN6_ADDR_GEN_MODE is changed.
> 
> This patch aims to fix this issue by moving the code in addrconf_notify
> which calls the addr gen for GRE and SIT into a separate function
> and calling it in the places that expect the IPv6 address to be
> generated.
> 
> The previous addrconf_dev_config is renamed to addrconf_eth_config
> since it only expected eth type interfaces and follows the
> addrconf_gre/sit_config format.
> 
> Fixes: e5dd729460ca ("ip/ip6_gre: use the same logic as SIT interfaces when computing v6LL address")
> Signed-off-by: Thomas Winter <Thomas.Winter@alliedtelesis.co.nz>
> ---
>  net/ipv6/addrconf.c | 47 +++++++++++++++++++++++++--------------------
>  1 file changed, 26 insertions(+), 21 deletions(-)
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 6dcf034835ec..e9d7ec03316d 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -3355,7 +3355,7 @@ static void addrconf_addr_gen(struct inet6_dev *idev, bool prefix_route)
>  	}
>  }
>  
> -static void addrconf_dev_config(struct net_device *dev)
> +static void addrconf_eth_config(struct net_device *dev)

You are creating a new function with the name of an existing one, while
renaming the latter. IMHO this makes the patch hard to review as there
are some existing call side for the old name, which we likelly want to
explicitly see here.

>  {
>  	struct inet6_dev *idev;
>  
> @@ -3447,6 +3447,30 @@ static void addrconf_gre_config(struct net_device *dev)
>  }
>  #endif
>  
> +static void addrconf_dev_config(struct net_device *dev)
> +{
> +	switch (dev->type) {
> +#if IS_ENABLED(CONFIG_IPV6_SIT)
> +	case ARPHRD_SIT:
> +		addrconf_sit_config(dev);
> +		break;
> +#endif
> +#if IS_ENABLED(CONFIG_NET_IPGRE) || IS_ENABLED(CONFIG_IPV6_GRE)
> +	case ARPHRD_IP6GRE:
> +	case ARPHRD_IPGRE:
> +		addrconf_gre_config(dev);
> +		break;
> +#endif
> +	case ARPHRD_LOOPBACK:
> +		init_loopback(dev);
> +		break;

If I read correctly, this change will cause unneeded attempt to re-add
the loopback address on the loopback device when the lo.addr_gen_mode
sysfs entry is touched. I think such side effect should be avoided.

Thanks,

Paolo


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] ip/ip6_gre: Fix changing addr gen mode not generating IPv6 link local address
  2022-12-20 13:16   ` Paolo Abeni
@ 2023-01-03 22:28     ` Thomas Winter
  0 siblings, 0 replies; 12+ messages in thread
From: Thomas Winter @ 2023-01-03 22:28 UTC (permalink / raw)
  To: davem, dsahern, linux-kernel, yoshfuji, a, netdev, kuba, pabeni

On Tue, 2022-12-20 at 14:16 +0100, Paolo Abeni wrote:
> On Mon, 2022-12-19 at 14:06 +1300, Thomas Winter wrote:
> > Commit e5dd729460ca changed the code path so that GRE tunnels
> > generate an IPv6 address based on the tunnel source address.
> > It also changed the code path so GRE tunnels don't call
> > addrconf_addr_gen
> > in addrconf_dev_config which is called by
> > addrconf_sysctl_addr_gen_mode
> > when the IN6_ADDR_GEN_MODE is changed.
> > 
> > This patch aims to fix this issue by moving the code in
> > addrconf_notify
> > which calls the addr gen for GRE and SIT into a separate function
> > and calling it in the places that expect the IPv6 address to be
> > generated.
> > 
> > The previous addrconf_dev_config is renamed to addrconf_eth_config
> > since it only expected eth type interfaces and follows the
> > addrconf_gre/sit_config format.
> > 
> > Fixes: e5dd729460ca ("ip/ip6_gre: use the same logic as SIT
> > interfaces when computing v6LL address")
> > Signed-off-by: Thomas Winter <Thomas.Winter@alliedtelesis.co.nz>
> > ---
> >  net/ipv6/addrconf.c | 47 +++++++++++++++++++++++++--------------
> > ------
> >  1 file changed, 26 insertions(+), 21 deletions(-)
> > 
> > diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> > index 6dcf034835ec..e9d7ec03316d 100644
> > --- a/net/ipv6/addrconf.c
> > +++ b/net/ipv6/addrconf.c
> > @@ -3355,7 +3355,7 @@ static void addrconf_addr_gen(struct
> > inet6_dev *idev, bool prefix_route)
> >  	}
> >  }
> >  
> > -static void addrconf_dev_config(struct net_device *dev)
> > +static void addrconf_eth_config(struct net_device *dev)
> 
> You are creating a new function with the name of an existing one,
> while
> renaming the latter. IMHO this makes the patch hard to review as
> there
> are some existing call side for the old name, which we likelly want
> to
> explicitly see here.
> 

Perhaps these functions can be named something like
"addrconfig_init_auto_addrs".

> >  {
> >  	struct inet6_dev *idev;
> >  
> > @@ -3447,6 +3447,30 @@ static void addrconf_gre_config(struct
> > net_device *dev)
> >  }
> >  #endif
> >  
> > +static void addrconf_dev_config(struct net_device *dev)
> > +{
> > +	switch (dev->type) {
> > +#if IS_ENABLED(CONFIG_IPV6_SIT)
> > +	case ARPHRD_SIT:
> > +		addrconf_sit_config(dev);
> > +		break;
> > +#endif
> > +#if IS_ENABLED(CONFIG_NET_IPGRE) || IS_ENABLED(CONFIG_IPV6_GRE)
> > +	case ARPHRD_IP6GRE:
> > +	case ARPHRD_IPGRE:
> > +		addrconf_gre_config(dev);
> > +		break;
> > +#endif
> > +	case ARPHRD_LOOPBACK:
> > +		init_loopback(dev);
> > +		break;
> 
> If I read correctly, this change will cause unneeded attempt to re-
> add
> the loopback address on the loopback device when the lo.addr_gen_mode
> sysfs entry is touched. I think such side effect should be avoided.

My understanding is that these addresses should always exist. It
doesn't look like any problem will happen if the loopback address
already exists and this called.

> 
> Thanks,
> 
> Paolo
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] ip/ip6_gre: Fix changing addr gen mode not generating IPv6 link local address
  2023-01-24  3:21 Thomas Winter
@ 2023-01-24  3:21 ` Thomas Winter
  0 siblings, 0 replies; 12+ messages in thread
From: Thomas Winter @ 2023-01-24  3:21 UTC (permalink / raw)
  To: davem, yoshfuji, dsahern, pabeni, edumazet, kuba, a, netdev,
	linux-kernel
  Cc: Thomas Winter

Commit e5dd729460ca changed the code path so that GRE tunnels
generate an IPv6 address based on the tunnel source address.
It also changed the code path so GRE tunnels don't call addrconf_addr_gen
in addrconf_dev_config which is called by addrconf_sysctl_addr_gen_mode
when the IN6_ADDR_GEN_MODE is changed.

This patch aims to fix this issue by moving the code in addrconf_notify
which calls the addr gen for GRE and SIT into a separate function
and calling it in the places that expect the IPv6 address to be
generated.

The previous addrconf_dev_config is renamed to addrconf_eth_config
since it only expected eth type interfaces and follows the
addrconf_gre/sit_config format.

Fixes: e5dd729460ca ("ip/ip6_gre: use the same logic as SIT interfaces when computing v6LL address")
Signed-off-by: Thomas Winter <Thomas.Winter@alliedtelesis.co.nz>
---
 net/ipv6/addrconf.c | 51 +++++++++++++++++++++++++--------------------
 1 file changed, 28 insertions(+), 23 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index f7a84a4acffc..0065b38fc85b 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3355,7 +3355,7 @@ static void addrconf_addr_gen(struct inet6_dev *idev, bool prefix_route)
 	}
 }
 
-static void addrconf_dev_config(struct net_device *dev)
+static void addrconf_eth_config(struct net_device *dev)
 {
 	struct inet6_dev *idev;
 
@@ -3447,6 +3447,30 @@ static void addrconf_gre_config(struct net_device *dev)
 }
 #endif
 
+static void addrconfig_init_auto_addrs(struct net_device *dev)
+{
+	switch (dev->type) {
+#if IS_ENABLED(CONFIG_IPV6_SIT)
+	case ARPHRD_SIT:
+		addrconf_sit_config(dev);
+		break;
+#endif
+#if IS_ENABLED(CONFIG_NET_IPGRE) || IS_ENABLED(CONFIG_IPV6_GRE)
+	case ARPHRD_IP6GRE:
+	case ARPHRD_IPGRE:
+		addrconf_gre_config(dev);
+		break;
+#endif
+	case ARPHRD_LOOPBACK:
+		init_loopback(dev);
+		break;
+
+	default:
+		addrconf_eth_config(dev);
+		break;
+	}
+}
+
 static int fixup_permanent_addr(struct net *net,
 				struct inet6_dev *idev,
 				struct inet6_ifaddr *ifp)
@@ -3615,26 +3639,7 @@ static int addrconf_notify(struct notifier_block *this, unsigned long event,
 			run_pending = 1;
 		}
 
-		switch (dev->type) {
-#if IS_ENABLED(CONFIG_IPV6_SIT)
-		case ARPHRD_SIT:
-			addrconf_sit_config(dev);
-			break;
-#endif
-#if IS_ENABLED(CONFIG_NET_IPGRE) || IS_ENABLED(CONFIG_IPV6_GRE)
-		case ARPHRD_IP6GRE:
-		case ARPHRD_IPGRE:
-			addrconf_gre_config(dev);
-			break;
-#endif
-		case ARPHRD_LOOPBACK:
-			init_loopback(dev);
-			break;
-
-		default:
-			addrconf_dev_config(dev);
-			break;
-		}
+		addrconfig_init_auto_addrs(dev);
 
 		if (!IS_ERR_OR_NULL(idev)) {
 			if (run_pending)
@@ -6397,7 +6402,7 @@ static int addrconf_sysctl_addr_gen_mode(struct ctl_table *ctl, int write,
 
 			if (idev->cnf.addr_gen_mode != new_val) {
 				idev->cnf.addr_gen_mode = new_val;
-				addrconf_dev_config(idev->dev);
+				addrconfig_init_auto_addrs(idev->dev);
 			}
 		} else if (&net->ipv6.devconf_all->addr_gen_mode == ctl->data) {
 			struct net_device *dev;
@@ -6408,7 +6413,7 @@ static int addrconf_sysctl_addr_gen_mode(struct ctl_table *ctl, int write,
 				if (idev &&
 				    idev->cnf.addr_gen_mode != new_val) {
 					idev->cnf.addr_gen_mode = new_val;
-					addrconf_dev_config(idev->dev);
+					addrconfig_init_auto_addrs(idev->dev);
 				}
 			}
 		}
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] ip/ip6_gre: Fix changing addr gen mode not generating IPv6 link local address
  2022-12-18 21:57 ` [PATCH 1/2] ip/ip6_gre: Fix changing addr gen mode not generating IPv6 link local address Thomas Winter
@ 2022-12-18 22:16   ` Stephen Hemminger
  0 siblings, 0 replies; 12+ messages in thread
From: Stephen Hemminger @ 2022-12-18 22:16 UTC (permalink / raw)
  To: Thomas Winter; +Cc: davem, yoshfuji, dsahern, kuba, a, netdev, linux-kernel

On Mon, 19 Dec 2022 10:57:17 +1300
Thomas Winter <Thomas.Winter@alliedtelesis.co.nz> wrote:

> +	switch (dev->type) {
> +	#if IS_ENABLED(CONFIG_IPV6_SIT)
> +	case ARPHRD_SIT:
> +		addrconf_sit_config(dev);
> +		break;
> +	#endif
> +	#if IS_ENABLED(CONFIG_NET_IPGRE) || IS_ENABLED(CONFIG_IPV6_GRE)
> +	case ARPHRD_IP6GRE:
> +	case ARPHRD_IPGRE:
> +		addrconf_gre_config(dev);
> +		break;
> +	#endif

Linux kernel style is to start any conditional compilation in first column

I.e.

#if IS_ENABLED(CONFIG_IPV6_SIT)

Not
	#if IS_ENABLED(CONFIG_IPV6_SIT)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] ip/ip6_gre: Fix changing addr gen mode not generating IPv6 link local address
  2022-12-18 21:57 [PATCH 0/2] ip/ip6_gre: Fix GRE tunnels not generating IPv6 link local addresses Thomas Winter
@ 2022-12-18 21:57 ` Thomas Winter
  2022-12-18 22:16   ` Stephen Hemminger
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Winter @ 2022-12-18 21:57 UTC (permalink / raw)
  To: davem, yoshfuji, dsahern, kuba, a, netdev, linux-kernel; +Cc: Thomas Winter

Commit e5dd729460ca changed the code path so that GRE tunnels
generate an IPv6 address based on the tunnel source address.
It also changed the code path so GRE tunnels don't call addrconf_addr_gen
in addrconf_dev_config which is called by addrconf_sysctl_addr_gen_mode
when the IN6_ADDR_GEN_MODE is changed.

This patch aims to fix this issue by moving the code in addrconf_notify
which calls the addr gen for GRE and SIT into a separate function
and calling it in the places that expect the IPv6 address to be
generated.

The previous addrconf_dev_config is renamed to addrconf_eth_config
since it only expected eth type interfaces and follows the
addrconf_gre/sit_config format.

Fixes: e5dd729460ca ("ip/ip6_gre: use the same logic as SIT interfaces when computing v6LL address")
Signed-off-by: Thomas Winter <Thomas.Winter@alliedtelesis.co.nz>
---
 net/ipv6/addrconf.c | 47 +++++++++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 6dcf034835ec..9c0d085fb9af 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3355,7 +3355,7 @@ static void addrconf_addr_gen(struct inet6_dev *idev, bool prefix_route)
 	}
 }
 
-static void addrconf_dev_config(struct net_device *dev)
+static void addrconf_eth_config(struct net_device *dev)
 {
 	struct inet6_dev *idev;
 
@@ -3447,6 +3447,30 @@ static void addrconf_gre_config(struct net_device *dev)
 }
 #endif
 
+static void addrconf_dev_config(struct net_device *dev)
+{
+	switch (dev->type) {
+	#if IS_ENABLED(CONFIG_IPV6_SIT)
+	case ARPHRD_SIT:
+		addrconf_sit_config(dev);
+		break;
+	#endif
+	#if IS_ENABLED(CONFIG_NET_IPGRE) || IS_ENABLED(CONFIG_IPV6_GRE)
+	case ARPHRD_IP6GRE:
+	case ARPHRD_IPGRE:
+		addrconf_gre_config(dev);
+		break;
+	#endif
+	case ARPHRD_LOOPBACK:
+		init_loopback(dev);
+		break;
+
+	default:
+		addrconf_eth_config(dev);
+		break;
+	}
+}
+
 static int fixup_permanent_addr(struct net *net,
 				struct inet6_dev *idev,
 				struct inet6_ifaddr *ifp)
@@ -3611,26 +3635,7 @@ static int addrconf_notify(struct notifier_block *this, unsigned long event,
 			run_pending = 1;
 		}
 
-		switch (dev->type) {
-#if IS_ENABLED(CONFIG_IPV6_SIT)
-		case ARPHRD_SIT:
-			addrconf_sit_config(dev);
-			break;
-#endif
-#if IS_ENABLED(CONFIG_NET_IPGRE) || IS_ENABLED(CONFIG_IPV6_GRE)
-		case ARPHRD_IP6GRE:
-		case ARPHRD_IPGRE:
-			addrconf_gre_config(dev);
-			break;
-#endif
-		case ARPHRD_LOOPBACK:
-			init_loopback(dev);
-			break;
-
-		default:
-			addrconf_dev_config(dev);
-			break;
-		}
+		addrconf_dev_config(dev);
 
 		if (!IS_ERR_OR_NULL(idev)) {
 			if (run_pending)
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-01-24  3:21 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-19  1:06 [PATCH 0/2] ip/ip6_gre: Fix GRE tunnels not generating IPv6 link local addresses Thomas Winter
2022-12-19  1:06 ` [PATCH 1/2] ip/ip6_gre: Fix changing addr gen mode not generating IPv6 link local address Thomas Winter
2022-12-20 13:16   ` Paolo Abeni
2023-01-03 22:28     ` Thomas Winter
2022-12-19  1:06 ` [PATCH 2/2] ip/ip6_gre: Fix non-point-to-point tunnel " Thomas Winter
2022-12-20  2:25 ` [PATCH 0/2] ip/ip6_gre: Fix GRE tunnels not generating IPv6 link local addresses Chris Packham
2022-12-20  2:32   ` Chris Packham
2022-12-20  3:06     ` Thomas Winter
2022-12-20  3:07       ` Chris Packham
  -- strict thread matches above, loose matches on Subject: below --
2023-01-24  3:21 Thomas Winter
2023-01-24  3:21 ` [PATCH 1/2] ip/ip6_gre: Fix changing addr gen mode not generating IPv6 link local address Thomas Winter
2022-12-18 21:57 [PATCH 0/2] ip/ip6_gre: Fix GRE tunnels not generating IPv6 link local addresses Thomas Winter
2022-12-18 21:57 ` [PATCH 1/2] ip/ip6_gre: Fix changing addr gen mode not generating IPv6 link local address Thomas Winter
2022-12-18 22:16   ` Stephen Hemminger

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.