[ovs-dev] linux: Assume it is local if no API is available.

Message ID 20180607141052.30002-1-fbl@redhat.com
State Accepted
Headers show
Series
  • [ovs-dev] linux: Assume it is local if no API is available.
Related show

Commit Message

Flavio Leitner June 7, 2018, 2:10 p.m.
If the 'openvswitch' kernel module is not loaded, the API is not
available and the userspace will keep retrying. This approach is
not ideal for the netdev datapath type.

This patch disables network netns support if the error code returned
indicates that the API is not available.

Reported-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Flavio Leitner <fbl@redhat.com>
---
 lib/netdev-linux.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Eelco Chaudron June 7, 2018, 2:33 p.m. | #1
On 07/06/18 16:10, Flavio Leitner wrote:
> If the 'openvswitch' kernel module is not loaded, the API is not
> available and the userspace will keep retrying. This approach is
> not ideal for the netdev datapath type.
>
> This patch disables network netns support if the error code returned
> indicates that the API is not available.
>
> Reported-by: Eelco Chaudron <echaudro@redhat.com>
> Signed-off-by: Flavio Leitner <fbl@redhat.com>
> ---
>   lib/netdev-linux.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index d2b79e569..10c1e4386 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -586,7 +586,11 @@ netdev_linux_netnsid_update__(struct netdev_linux *netdev)
>       int error;
>   
>       error = dpif_netlink_vport_get(netdev_get_name(&netdev->up), &reply, &buf);
> -    if (error) {
> +    if (error == ENOENT) {
> +        /* Assume it is local if there is no API */
> +        netnsid_set_local(&netdev->netnsid);
> +        return error;
> +    } else if (error) {
>           netnsid_unset(&netdev->netnsid);
>           return error;
>       }

Acked-by: Eelco Chaudron <echaudro@redhat.com>
Tested-by: Eelco Chaudron <echaudro@redhat.com>
Ben Pfaff June 14, 2018, 11:09 p.m. | #2
On Thu, Jun 07, 2018 at 04:33:50PM +0200, Eelco Chaudron wrote:
> On 07/06/18 16:10, Flavio Leitner wrote:
> >If the 'openvswitch' kernel module is not loaded, the API is not
> >available and the userspace will keep retrying. This approach is
> >not ideal for the netdev datapath type.
> >
> >This patch disables network netns support if the error code returned
> >indicates that the API is not available.
> >
> >Reported-by: Eelco Chaudron <echaudro@redhat.com>
> >Signed-off-by: Flavio Leitner <fbl@redhat.com>
> >---
> >  lib/netdev-linux.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> >diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> >index d2b79e569..10c1e4386 100644
> >--- a/lib/netdev-linux.c
> >+++ b/lib/netdev-linux.c
> >@@ -586,7 +586,11 @@ netdev_linux_netnsid_update__(struct netdev_linux *netdev)
> >      int error;
> >      error = dpif_netlink_vport_get(netdev_get_name(&netdev->up), &reply, &buf);
> >-    if (error) {
> >+    if (error == ENOENT) {
> >+        /* Assume it is local if there is no API */
> >+        netnsid_set_local(&netdev->netnsid);
> >+        return error;
> >+    } else if (error) {
> >          netnsid_unset(&netdev->netnsid);
> >          return error;
> >      }
> 
> Acked-by: Eelco Chaudron <echaudro@redhat.com>
> Tested-by: Eelco Chaudron <echaudro@redhat.com>

Thanks, Flavio (and Eelco).  I applied this to master.  I hope you will
forgive me for fussing with it a bit stylistically and to improve the
comment.  I committed it in the following form:

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index e2bbbb827434..1d2b7ea4cd00 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -587,7 +587,13 @@ netdev_linux_netnsid_update__(struct netdev_linux *netdev)
 
     error = dpif_netlink_vport_get(netdev_get_name(&netdev->up), &reply, &buf);
     if (error) {
-        netnsid_unset(&netdev->netnsid);
+        if (error == ENOENT) {
+            /* Assume it is local if there is no API (e.g. if the openvswitch
+             * kernel module is not loaded). */
+            netnsid_set_local(&netdev->netnsid);
+        } else {
+            netnsid_unset(&netdev->netnsid);
+        }
         return error;
     }
Flavio Leitner June 14, 2018, 11:46 p.m. | #3
On Thu, Jun 14, 2018 at 04:09:24PM -0700, Ben Pfaff wrote:
> On Thu, Jun 07, 2018 at 04:33:50PM +0200, Eelco Chaudron wrote:
> > On 07/06/18 16:10, Flavio Leitner wrote:
> > >If the 'openvswitch' kernel module is not loaded, the API is not
> > >available and the userspace will keep retrying. This approach is
> > >not ideal for the netdev datapath type.
> > >
> > >This patch disables network netns support if the error code returned
> > >indicates that the API is not available.
> > >
> > >Reported-by: Eelco Chaudron <echaudro@redhat.com>
> > >Signed-off-by: Flavio Leitner <fbl@redhat.com>
> > >---
> > >  lib/netdev-linux.c | 6 +++++-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > >diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> > >index d2b79e569..10c1e4386 100644
> > >--- a/lib/netdev-linux.c
> > >+++ b/lib/netdev-linux.c
> > >@@ -586,7 +586,11 @@ netdev_linux_netnsid_update__(struct netdev_linux *netdev)
> > >      int error;
> > >      error = dpif_netlink_vport_get(netdev_get_name(&netdev->up), &reply, &buf);
> > >-    if (error) {
> > >+    if (error == ENOENT) {
> > >+        /* Assume it is local if there is no API */
> > >+        netnsid_set_local(&netdev->netnsid);
> > >+        return error;
> > >+    } else if (error) {
> > >          netnsid_unset(&netdev->netnsid);
> > >          return error;
> > >      }
> > 
> > Acked-by: Eelco Chaudron <echaudro@redhat.com>
> > Tested-by: Eelco Chaudron <echaudro@redhat.com>
> 
> Thanks, Flavio (and Eelco).  I applied this to master.  I hope you will
> forgive me for fussing with it a bit stylistically and to improve the
> comment.  I committed it in the following form:

Looks better to me!
Thanks Ben!
fbl

> 
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index e2bbbb827434..1d2b7ea4cd00 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -587,7 +587,13 @@ netdev_linux_netnsid_update__(struct netdev_linux *netdev)
>  
>      error = dpif_netlink_vport_get(netdev_get_name(&netdev->up), &reply, &buf);
>      if (error) {
> -        netnsid_unset(&netdev->netnsid);
> +        if (error == ENOENT) {
> +            /* Assume it is local if there is no API (e.g. if the openvswitch
> +             * kernel module is not loaded). */
> +            netnsid_set_local(&netdev->netnsid);
> +        } else {
> +            netnsid_unset(&netdev->netnsid);
> +        }
>          return error;
>      }
>

Patch

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index d2b79e569..10c1e4386 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -586,7 +586,11 @@  netdev_linux_netnsid_update__(struct netdev_linux *netdev)
     int error;
 
     error = dpif_netlink_vport_get(netdev_get_name(&netdev->up), &reply, &buf);
-    if (error) {
+    if (error == ENOENT) {
+        /* Assume it is local if there is no API */
+        netnsid_set_local(&netdev->netnsid);
+        return error;
+    } else if (error) {
         netnsid_unset(&netdev->netnsid);
         return error;
     }