[SRU][Bionic][Cosmic][PATCH 0/1] geneve: correctly handle ipv6.disable module parameter

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[SRU][Bionic][Cosmic][PATCH 0/1] geneve: correctly handle ipv6.disable module parameter

Nivedita Singhvi
BugLink: https://bugs.launchpad.net/bugs/1794232

SRU Justification

Impact: Cannot create geneve tunnels if ipv6 is disabled dynamically.

Fix:
Fixed by upstream commit in v5.0:
Commit: cf1c9ccba7308e48a68fa77f476287d9d614e4c7
"geneve: correctly handle ipv6.disable module parameter"
     
Hence available in Disco and later; required in X,B,C
Cherry picked and tested successfully for  B, C, with
X being handled slightly differently, separately.
 
Testcase:
1. Boot with "ipv6.disable=1"
2. Then try and create a geneve tunnel using:
   # ovs-vsctl add-br br1
   # ovs-vsctl add-port br1 geneve1 -- set interface geneve1
    type=geneve options:remote_ip=192.168.x.z // ip of the other host

Regression Potential: Low, only geneve tunnels when ipv6 dynamically
disabled, current status is it doesn't work at all.

Other Info:
* Mainline commit msg includes reference to a fix for
  non-metadata tunnels (infrastructure is not yet in
  our tree prior to Disco), hence not being included
  at this time under this case.

  At this time, all geneve tunnels created as above
  are metadata-enabled.

* Patch for X being sent separately


This patch to be applied only to B, C.
 

Jiri Benc (1):
  geneve: correctly handle ipv6.disable module parameter

 drivers/net/geneve.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Bionic][Cosmic][PATCH 1/1] geneve: correctly handle ipv6.disable module parameter

Nivedita Singhvi
From: Jiri Benc <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1794232

When IPv6 is compiled but disabled at runtime, geneve_sock_add returns
-EAFNOSUPPORT. For metadata based tunnels, this causes failure of the whole
operation of bringing up the tunnel.

Ignore failure of IPv6 socket creation for metadata based tunnels caused by
IPv6 not being available.

This is the same fix as what commit d074bf960044 ("vxlan: correctly handle
ipv6.disable module parameter") is doing for vxlan.

Note there's also commit c0a47e44c098 ("geneve: should not call rt6_lookup()
when ipv6 was disabled") which fixes a similar issue but for regular
tunnels, while this patch is needed for metadata based tunnels.

Signed-off-by: Jiri Benc <[hidden email]>
Signed-off-by: David S. Miller <[hidden email]>
(cherry picked from commit cf1c9ccba7308e48a68fa77f476287d9d614e4c7)
Signed-off-by: Nivedita Singhvi <[hidden email]>
---
 drivers/net/geneve.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 8bb5bd72342a..62e62cab6739 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -632,15 +632,20 @@ static int geneve_sock_add(struct geneve_dev *geneve, bool ipv6)
 static int geneve_open(struct net_device *dev)
 {
  struct geneve_dev *geneve = netdev_priv(dev);
- bool ipv6 = !!(geneve->info.mode & IP_TUNNEL_INFO_IPV6);
  bool metadata = geneve->collect_md;
+ bool ipv4, ipv6;
  int ret = 0;
 
+ ipv6 = geneve->info.mode & IP_TUNNEL_INFO_IPV6 || metadata;
+ ipv4 = !ipv6 || metadata;
 #if IS_ENABLED(CONFIG_IPV6)
- if (ipv6 || metadata)
+ if (ipv6) {
  ret = geneve_sock_add(geneve, true);
+ if (ret < 0 && ret != -EAFNOSUPPORT)
+ ipv4 = false;
+ }
 #endif
- if (!ret && (!ipv6 || metadata))
+ if (ipv4)
  ret = geneve_sock_add(geneve, false);
  if (ret < 0)
  geneve_sock_release(geneve);
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU][Bionic][Cosmic][PATCH 1/1] geneve: correctly handle ipv6.disable module parameter

Kleber Souza
On 5/7/19 7:00 PM, Nivedita Singhvi wrote:

> From: Jiri Benc <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1794232
>
> When IPv6 is compiled but disabled at runtime, geneve_sock_add returns
> -EAFNOSUPPORT. For metadata based tunnels, this causes failure of the whole
> operation of bringing up the tunnel.
>
> Ignore failure of IPv6 socket creation for metadata based tunnels caused by
> IPv6 not being available.
>
> This is the same fix as what commit d074bf960044 ("vxlan: correctly handle
> ipv6.disable module parameter") is doing for vxlan.
>
> Note there's also commit c0a47e44c098 ("geneve: should not call rt6_lookup()
> when ipv6 was disabled") which fixes a similar issue but for regular
> tunnels, while this patch is needed for metadata based tunnels.
>
> Signed-off-by: Jiri Benc <[hidden email]>
> Signed-off-by: David S. Miller <[hidden email]>
> (cherry picked from commit cf1c9ccba7308e48a68fa77f476287d9d614e4c7)
> Signed-off-by: Nivedita Singhvi <[hidden email]>

Hi Nivedita,

Thanks for re-sending it.

Clean cherry-pick, testcase well documented and low regression
risk.

Acked-by: Kleber Sacilotto de Souza <[hidden email]>

> ---
>  drivers/net/geneve.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index 8bb5bd72342a..62e62cab6739 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -632,15 +632,20 @@ static int geneve_sock_add(struct geneve_dev *geneve, bool ipv6)
>  static int geneve_open(struct net_device *dev)
>  {
>   struct geneve_dev *geneve = netdev_priv(dev);
> - bool ipv6 = !!(geneve->info.mode & IP_TUNNEL_INFO_IPV6);
>   bool metadata = geneve->collect_md;
> + bool ipv4, ipv6;
>   int ret = 0;
>  
> + ipv6 = geneve->info.mode & IP_TUNNEL_INFO_IPV6 || metadata;
> + ipv4 = !ipv6 || metadata;
>  #if IS_ENABLED(CONFIG_IPV6)
> - if (ipv6 || metadata)
> + if (ipv6) {
>   ret = geneve_sock_add(geneve, true);
> + if (ret < 0 && ret != -EAFNOSUPPORT)
> + ipv4 = false;
> + }
>  #endif
> - if (!ret && (!ipv6 || metadata))
> + if (ipv4)
>   ret = geneve_sock_add(geneve, false);
>   if (ret < 0)
>   geneve_sock_release(geneve);
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU][Bionic][Cosmic][PATCH 1/1] geneve: correctly handle ipv6.disable module parameter

Connor Kuehl
In reply to this post by Nivedita Singhvi
On 5/7/19 10:00 AM, Nivedita Singhvi wrote:

> From: Jiri Benc <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1794232
>
> When IPv6 is compiled but disabled at runtime, geneve_sock_add returns
> -EAFNOSUPPORT. For metadata based tunnels, this causes failure of the whole
> operation of bringing up the tunnel.
>
> Ignore failure of IPv6 socket creation for metadata based tunnels caused by
> IPv6 not being available.
>
> This is the same fix as what commit d074bf960044 ("vxlan: correctly handle
> ipv6.disable module parameter") is doing for vxlan.
>
> Note there's also commit c0a47e44c098 ("geneve: should not call rt6_lookup()
> when ipv6 was disabled") which fixes a similar issue but for regular
> tunnels, while this patch is needed for metadata based tunnels.
>
> Signed-off-by: Jiri Benc <[hidden email]>
> Signed-off-by: David S. Miller <[hidden email]>
> (cherry picked from commit cf1c9ccba7308e48a68fa77f476287d9d614e4c7)
> Signed-off-by: Nivedita Singhvi <[hidden email]>
> ---
Agreed with Kleber, great documentation.

Acked-by: Connor Kuehl <[hidden email]>

>  drivers/net/geneve.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index 8bb5bd72342a..62e62cab6739 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -632,15 +632,20 @@ static int geneve_sock_add(struct geneve_dev *geneve, bool ipv6)
>  static int geneve_open(struct net_device *dev)
>  {
>   struct geneve_dev *geneve = netdev_priv(dev);
> - bool ipv6 = !!(geneve->info.mode & IP_TUNNEL_INFO_IPV6);
>   bool metadata = geneve->collect_md;
> + bool ipv4, ipv6;
>   int ret = 0;
>  
> + ipv6 = geneve->info.mode & IP_TUNNEL_INFO_IPV6 || metadata;
> + ipv4 = !ipv6 || metadata;
>  #if IS_ENABLED(CONFIG_IPV6)
> - if (ipv6 || metadata)
> + if (ipv6) {
>   ret = geneve_sock_add(geneve, true);
> + if (ret < 0 && ret != -EAFNOSUPPORT)
> + ipv4 = false;
> + }
>  #endif
> - if (!ret && (!ipv6 || metadata))
> + if (ipv4)
>   ret = geneve_sock_add(geneve, false);
>   if (ret < 0)
>   geneve_sock_release(geneve);
>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

pEpkey.asc (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

APPLIED: [SRU][Bionic][Cosmic][PATCH 0/1] geneve: correctly handle ipv6.disable module parameter

Khaled Elmously
In reply to this post by Nivedita Singhvi
On 2019-05-07 22:30:19 , Nivedita Singhvi wrote:

> BugLink: https://bugs.launchpad.net/bugs/1794232
>
> SRU Justification
>
> Impact: Cannot create geneve tunnels if ipv6 is disabled dynamically.
>
> Fix:
> Fixed by upstream commit in v5.0:
> Commit: cf1c9ccba7308e48a68fa77f476287d9d614e4c7
> "geneve: correctly handle ipv6.disable module parameter"
>      
> Hence available in Disco and later; required in X,B,C
> Cherry picked and tested successfully for  B, C, with
> X being handled slightly differently, separately.
>  
> Testcase:
> 1. Boot with "ipv6.disable=1"
> 2. Then try and create a geneve tunnel using:
>    # ovs-vsctl add-br br1
>    # ovs-vsctl add-port br1 geneve1 -- set interface geneve1
>     type=geneve options:remote_ip=192.168.x.z // ip of the other host
>
> Regression Potential: Low, only geneve tunnels when ipv6 dynamically
> disabled, current status is it doesn't work at all.
>
> Other Info:
> * Mainline commit msg includes reference to a fix for
>   non-metadata tunnels (infrastructure is not yet in
>   our tree prior to Disco), hence not being included
>   at this time under this case.
>
>   At this time, all geneve tunnels created as above
>   are metadata-enabled.
>
> * Patch for X being sent separately
>
>
> This patch to be applied only to B, C.
>  
>
> Jiri Benc (1):
>   geneve: correctly handle ipv6.disable module parameter
>
>  drivers/net/geneve.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> --
> 2.17.1
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team