diff mbox

[ovs-dev,2/3] netdev-linux: make tap devices persistent.

Message ID AM5PR0701MB2961F4F8937F74798CE611BBA4C80@AM5PR0701MB2961.eurprd07.prod.outlook.com
State Not Applicable
Headers show

Commit Message

Vishal Deep Ajmera June 7, 2017, 9:39 a.m. UTC 
Hi Flavio,

If the tap-device is persistent but the 'netdev' datapath is not yet started then will it create any issues in the system ? What happens if we start sending packets on this interface whereas data-path is not present ?

Regards,
Vishal

-----Original Message-----
From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-bounces@openvswitch.org] On Behalf Of Flavio Leitner
Sent: Tuesday, May 30, 2017 1:10 AM
To: dev@openvswitch.org
Cc: Flavio Leitner <fbl@redhat.com>
Subject: [ovs-dev] [PATCH 2/3] netdev-linux: make tap devices persistent.

When using data path type "netdev", bridge port is a tun device and when OVS restarts, that device and its network configuration is lost.

This patch enables the tap device to persist instead.

Signed-off-by: Flavio Leitner <fbl@redhat.com>
---
 lib/netdev-linux.c | 8 ++++++++
 1 file changed, 8 insertions(+)

--
2.9.4

Comments

Flavio Leitner June 7, 2017, 2:57 p.m. UTC | #1 
On Wed, Jun 07, 2017 at 09:39:16AM +0000, Vishal Deep Ajmera wrote:
> Hi Flavio,
> 
> If the tap-device is persistent but the 'netdev' datapath is not yet
> started then will it create any issues in the system ? What happens
> if we start sending packets on this interface whereas data-path is
> not present ?

The link goes down and packets are dropped.
fbl

> 
> Regards,
> Vishal
> 
> -----Original Message-----
> From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-bounces@openvswitch.org] On Behalf Of Flavio Leitner
> Sent: Tuesday, May 30, 2017 1:10 AM
> To: dev@openvswitch.org
> Cc: Flavio Leitner <fbl@redhat.com>
> Subject: [ovs-dev] [PATCH 2/3] netdev-linux: make tap devices persistent.
> 
> When using data path type "netdev", bridge port is a tun device and when OVS restarts, that device and its network configuration is lost.
> 
> This patch enables the tap device to persist instead.
> 
> Signed-off-by: Flavio Leitner <fbl@redhat.com>
> ---
>  lib/netdev-linux.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index 3ad3d45..d181e4f 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -866,6 +866,13 @@ netdev_linux_construct_tap(struct netdev *netdev_)
>          goto error_close;
>      }
>  
> +    if (ioctl(netdev->tap_fd, TUNSETPERSIST, 1)) {
> +        VLOG_WARN("%s: creating tap device failed (persist): %s", name,
> +                  ovs_strerror(errno));
> +        error = errno;
> +        goto error_close;
> +    }
> +
>      return 0;
>  
>  error_close:
> @@ -885,6 +892,7 @@ netdev_linux_destruct(struct netdev *netdev_)
>      if (netdev_get_class(netdev_) == &netdev_tap_class
>          && netdev->tap_fd >= 0)
>      {
> +        ioctl(netdev->tap_fd, TUNSETPERSIST, 0);
>          close(netdev->tap_fd);
>      }
>  
> --
> 2.9.4
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Vishal Deep Ajmera June 8, 2017, 11:24 a.m. UTC | #2 
Hi Flavio,

I am facing some issue with ovs-master on Ubuntu 14.04 system. Here are the steps I followed and the error message I get. Can it be possibly related to your patch for making tap device persistent ? Let me know if I am missing some steps here.

1. Create netdev bridge
$ ovs-vsctl add-br br0 -- set Bridge br0 datapath_type=netdev
$ ovs-vsctl show
4c516433-c305-4894-96ad-f44af0b01b63
    Bridge "br0"
        Port "br0"
            Interface "br0"
                type: internal
    ovs_version: "2.7.90"

2. Bring up br0 interface
$ ifconfig br0 up

3. Restart the openvswitch
$ service openvswitch-switch restart

4. Show ovs db
$ ovs-vsctl show
4c516433-c305-4894-96ad-f44af0b01b63
    Bridge "br0"
        Port "br0"
            Interface "br0"
                type: internal
                error: "could not open network device br0 (File exists)"
    ovs_version: "2.7.90"

Regards,
Vishal

-----Original Message-----
From: Flavio Leitner [mailto:fbl@redhat.com] 
Sent: Wednesday, June 07, 2017 8:28 PM
To: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com>
Cc: dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH 2/3] netdev-linux: make tap devices persistent.

On Wed, Jun 07, 2017 at 09:39:16AM +0000, Vishal Deep Ajmera wrote:
> Hi Flavio,
> 
> If the tap-device is persistent but the 'netdev' datapath is not yet 
> started then will it create any issues in the system ? What happens if 
> we start sending packets on this interface whereas data-path is not 
> present ?

The link goes down and packets are dropped.
fbl

> 
> Regards,
> Vishal
> 
> -----Original Message-----
> From: ovs-dev-bounces@openvswitch.org 
> [mailto:ovs-dev-bounces@openvswitch.org] On Behalf Of Flavio Leitner
> Sent: Tuesday, May 30, 2017 1:10 AM
> To: dev@openvswitch.org
> Cc: Flavio Leitner <fbl@redhat.com>
> Subject: [ovs-dev] [PATCH 2/3] netdev-linux: make tap devices persistent.
> 
> When using data path type "netdev", bridge port is a tun device and when OVS restarts, that device and its network configuration is lost.
> 
> This patch enables the tap device to persist instead.
> 
> Signed-off-by: Flavio Leitner <fbl@redhat.com>
> ---
>  lib/netdev-linux.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index 
> 3ad3d45..d181e4f 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -866,6 +866,13 @@ netdev_linux_construct_tap(struct netdev *netdev_)
>          goto error_close;
>      }
>  
> +    if (ioctl(netdev->tap_fd, TUNSETPERSIST, 1)) {
> +        VLOG_WARN("%s: creating tap device failed (persist): %s", name,
> +                  ovs_strerror(errno));
> +        error = errno;
> +        goto error_close;
> +    }
> +
>      return 0;
>  
>  error_close:
> @@ -885,6 +892,7 @@ netdev_linux_destruct(struct netdev *netdev_)
>      if (netdev_get_class(netdev_) == &netdev_tap_class
>          && netdev->tap_fd >= 0)
>      {
> +        ioctl(netdev->tap_fd, TUNSETPERSIST, 0);
>          close(netdev->tap_fd);
>      }
>  
> --
> 2.9.4
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

--
Flavio
Flavio Leitner June 8, 2017, 5:22 p.m. UTC | #3 
On Thu, Jun 08, 2017 at 11:24:58AM +0000, Vishal Deep Ajmera wrote:
> Hi Flavio,
> 
> I am facing some issue with ovs-master on Ubuntu 14.04 system. Here are the steps I followed and the error message I get. Can it be possibly related to your patch for making tap device persistent ? Let me know if I am missing some steps here.
> 
> 1. Create netdev bridge
> $ ovs-vsctl add-br br0 -- set Bridge br0 datapath_type=netdev
> $ ovs-vsctl show
> 4c516433-c305-4894-96ad-f44af0b01b63
>     Bridge "br0"
>         Port "br0"
>             Interface "br0"
>                 type: internal
>     ovs_version: "2.7.90"
> 
> 2. Bring up br0 interface
> $ ifconfig br0 up
> 
> 3. Restart the openvswitch
> $ service openvswitch-switch restart
> 
> 4. Show ovs db
> $ ovs-vsctl show
> 4c516433-c305-4894-96ad-f44af0b01b63
>     Bridge "br0"
>         Port "br0"
>             Interface "br0"
>                 type: internal
>                 error: "could not open network device br0 (File exists)"
>     ovs_version: "2.7.90"


The same thing Eric reported to me when a test unit had failed.
Oddly I can't reproduce (yet), but reviewing the code seems that a
rtnetlink to add a route can open the device but not add to the DP, 
then when it tries to that error would be reported.

I will look more into it and see if I can fix it.
Thanks for the report.
Flavio



> 
> Regards,
> Vishal
> 
> -----Original Message-----
> From: Flavio Leitner [mailto:fbl@redhat.com] 
> Sent: Wednesday, June 07, 2017 8:28 PM
> To: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com>
> Cc: dev@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH 2/3] netdev-linux: make tap devices persistent.
> 
> On Wed, Jun 07, 2017 at 09:39:16AM +0000, Vishal Deep Ajmera wrote:
> > Hi Flavio,
> > 
> > If the tap-device is persistent but the 'netdev' datapath is not yet 
> > started then will it create any issues in the system ? What happens if 
> > we start sending packets on this interface whereas data-path is not 
> > present ?
> 
> The link goes down and packets are dropped.
> fbl
> 
> > 
> > Regards,
> > Vishal
> > 
> > -----Original Message-----
> > From: ovs-dev-bounces@openvswitch.org 
> > [mailto:ovs-dev-bounces@openvswitch.org] On Behalf Of Flavio Leitner
> > Sent: Tuesday, May 30, 2017 1:10 AM
> > To: dev@openvswitch.org
> > Cc: Flavio Leitner <fbl@redhat.com>
> > Subject: [ovs-dev] [PATCH 2/3] netdev-linux: make tap devices persistent.
> > 
> > When using data path type "netdev", bridge port is a tun device and when OVS restarts, that device and its network configuration is lost.
> > 
> > This patch enables the tap device to persist instead.
> > 
> > Signed-off-by: Flavio Leitner <fbl@redhat.com>
> > ---
> >  lib/netdev-linux.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index 
> > 3ad3d45..d181e4f 100644
> > --- a/lib/netdev-linux.c
> > +++ b/lib/netdev-linux.c
> > @@ -866,6 +866,13 @@ netdev_linux_construct_tap(struct netdev *netdev_)
> >          goto error_close;
> >      }
> >  
> > +    if (ioctl(netdev->tap_fd, TUNSETPERSIST, 1)) {
> > +        VLOG_WARN("%s: creating tap device failed (persist): %s", name,
> > +                  ovs_strerror(errno));
> > +        error = errno;
> > +        goto error_close;
> > +    }
> > +
> >      return 0;
> >  
> >  error_close:
> > @@ -885,6 +892,7 @@ netdev_linux_destruct(struct netdev *netdev_)
> >      if (netdev_get_class(netdev_) == &netdev_tap_class
> >          && netdev->tap_fd >= 0)
> >      {
> > +        ioctl(netdev->tap_fd, TUNSETPERSIST, 0);
> >          close(netdev->tap_fd);
> >      }
> >  
> > --
> > 2.9.4
> > 
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
> --
> Flavio
Flavio Leitner June 19, 2017, 7:01 p.m. UTC | #4 
Hi Vishal,


On Thu, Jun 08, 2017 at 02:22:38PM -0300, Flavio Leitner wrote:
> On Thu, Jun 08, 2017 at 11:24:58AM +0000, Vishal Deep Ajmera wrote:
> > Hi Flavio,
> > 
> > I am facing some issue with ovs-master on Ubuntu 14.04 system. Here are the steps I followed and the error message I get. Can it be possibly related to your patch for making tap device persistent ? Let me know if I am missing some steps here.
> > 
> > 1. Create netdev bridge
> > $ ovs-vsctl add-br br0 -- set Bridge br0 datapath_type=netdev
> > $ ovs-vsctl show
> > 4c516433-c305-4894-96ad-f44af0b01b63
> >     Bridge "br0"
> >         Port "br0"
> >             Interface "br0"
> >                 type: internal
> >     ovs_version: "2.7.90"
> > 
> > 2. Bring up br0 interface
> > $ ifconfig br0 up
> > 
> > 3. Restart the openvswitch
> > $ service openvswitch-switch restart
> > 
> > 4. Show ovs db
> > $ ovs-vsctl show
> > 4c516433-c305-4894-96ad-f44af0b01b63
> >     Bridge "br0"
> >         Port "br0"
> >             Interface "br0"
> >                 type: internal
> >                 error: "could not open network device br0 (File exists)"
> >     ovs_version: "2.7.90"
> 
> 
> The same thing Eric reported to me when a test unit had failed.
> Oddly I can't reproduce (yet), but reviewing the code seems that a
> rtnetlink to add a route can open the device but not add to the DP, 
> then when it tries to that error would be reported.
> 
> I will look more into it and see if I can fix it.

I think I have reproduced the issue here and it only happens if you
have an IP address on a port. Could you confirm that?

If so, OVS will receive rtnetlink msgs from each device with routes.
That creates netdev objects before they are added to the datapath.
Then when vswitchd is configuring the datapath, netdev_open() returns
EEXIST error preventing the device to be added.

It looks like related to this commit:
https://github.com/openvswitch/ovs/commit/67ac844b55d4c5f6bbfa01773c82b3d6d8b62131

Could you revert it and see if the issue goes away?

Thanks,
Vishal Deep Ajmera June 20, 2017, 6:35 a.m. UTC | #5 
Hi Flavio,

I did not configure IP address to the interface, simply brought up the interface br0 using ifconfig and see this issue consistently.

$ ifconfig br0 up

Regards,
Vishal

-----Original Message-----
From: Flavio Leitner [mailto:fbl@sysclose.org] 
Sent: Tuesday, June 20, 2017 12:32 AM
To: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com>
Cc: dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH 2/3] netdev-linux: make tap devices persistent.

Hi Vishal,


On Thu, Jun 08, 2017 at 02:22:38PM -0300, Flavio Leitner wrote:
> On Thu, Jun 08, 2017 at 11:24:58AM +0000, Vishal Deep Ajmera wrote:
> > Hi Flavio,
> > 
> > I am facing some issue with ovs-master on Ubuntu 14.04 system. Here are the steps I followed and the error message I get. Can it be possibly related to your patch for making tap device persistent ? Let me know if I am missing some steps here.
> > 
> > 1. Create netdev bridge
> > $ ovs-vsctl add-br br0 -- set Bridge br0 datapath_type=netdev $ 
> > ovs-vsctl show
> > 4c516433-c305-4894-96ad-f44af0b01b63
> >     Bridge "br0"
> >         Port "br0"
> >             Interface "br0"
> >                 type: internal
> >     ovs_version: "2.7.90"
> > 
> > 2. Bring up br0 interface
> > $ ifconfig br0 up
> > 
> > 3. Restart the openvswitch
> > $ service openvswitch-switch restart
> > 
> > 4. Show ovs db
> > $ ovs-vsctl show
> > 4c516433-c305-4894-96ad-f44af0b01b63
> >     Bridge "br0"
> >         Port "br0"
> >             Interface "br0"
> >                 type: internal
> >                 error: "could not open network device br0 (File exists)"
> >     ovs_version: "2.7.90"
> 
> 
> The same thing Eric reported to me when a test unit had failed.
> Oddly I can't reproduce (yet), but reviewing the code seems that a 
> rtnetlink to add a route can open the device but not add to the DP, 
> then when it tries to that error would be reported.
> 
> I will look more into it and see if I can fix it.

I think I have reproduced the issue here and it only happens if you have an IP address on a port. Could you confirm that?

If so, OVS will receive rtnetlink msgs from each device with routes.
That creates netdev objects before they are added to the datapath.
Then when vswitchd is configuring the datapath, netdev_open() returns EEXIST error preventing the device to be added.

It looks like related to this commit:
https://github.com/openvswitch/ovs/commit/67ac844b55d4c5f6bbfa01773c82b3d6d8b62131

Could you revert it and see if the issue goes away?

Thanks,
--
Flavio
Flavio Leitner June 20, 2017, 12:04 p.m. UTC | #6 
On Tue, Jun 20, 2017 at 06:35:11AM +0000, Vishal Deep Ajmera wrote:
> Hi Flavio,
> 
> I did not configure IP address to the interface, simply brought up the interface br0 using ifconfig and see this issue consistently.
> 
> $ ifconfig br0 up

Well, that includes IPv6 automatically, so could you try reverting that patch
and see if the issue goes away?



> 
> Regards,
> Vishal
> 
> -----Original Message-----
> From: Flavio Leitner [mailto:fbl@sysclose.org] 
> Sent: Tuesday, June 20, 2017 12:32 AM
> To: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com>
> Cc: dev@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH 2/3] netdev-linux: make tap devices persistent.
> 
> Hi Vishal,
> 
> 
> On Thu, Jun 08, 2017 at 02:22:38PM -0300, Flavio Leitner wrote:
> > On Thu, Jun 08, 2017 at 11:24:58AM +0000, Vishal Deep Ajmera wrote:
> > > Hi Flavio,
> > > 
> > > I am facing some issue with ovs-master on Ubuntu 14.04 system. Here are the steps I followed and the error message I get. Can it be possibly related to your patch for making tap device persistent ? Let me know if I am missing some steps here.
> > > 
> > > 1. Create netdev bridge
> > > $ ovs-vsctl add-br br0 -- set Bridge br0 datapath_type=netdev $ 
> > > ovs-vsctl show
> > > 4c516433-c305-4894-96ad-f44af0b01b63
> > >     Bridge "br0"
> > >         Port "br0"
> > >             Interface "br0"
> > >                 type: internal
> > >     ovs_version: "2.7.90"
> > > 
> > > 2. Bring up br0 interface
> > > $ ifconfig br0 up
> > > 
> > > 3. Restart the openvswitch
> > > $ service openvswitch-switch restart
> > > 
> > > 4. Show ovs db
> > > $ ovs-vsctl show
> > > 4c516433-c305-4894-96ad-f44af0b01b63
> > >     Bridge "br0"
> > >         Port "br0"
> > >             Interface "br0"
> > >                 type: internal
> > >                 error: "could not open network device br0 (File exists)"
> > >     ovs_version: "2.7.90"
> > 
> > 
> > The same thing Eric reported to me when a test unit had failed.
> > Oddly I can't reproduce (yet), but reviewing the code seems that a 
> > rtnetlink to add a route can open the device but not add to the DP, 
> > then when it tries to that error would be reported.
> > 
> > I will look more into it and see if I can fix it.
> 
> I think I have reproduced the issue here and it only happens if you have an IP address on a port. Could you confirm that?
> 
> If so, OVS will receive rtnetlink msgs from each device with routes.
> That creates netdev objects before they are added to the datapath.
> Then when vswitchd is configuring the datapath, netdev_open() returns EEXIST error preventing the device to be added.
> 
> It looks like related to this commit:
> https://github.com/openvswitch/ovs/commit/67ac844b55d4c5f6bbfa01773c82b3d6d8b62131
> 
> Could you revert it and see if the issue goes away?
> 
> Thanks,
> --
> Flavio
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff mbox

Patch

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index 3ad3d45..d181e4f 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -866,6 +866,13 @@  netdev_linux_construct_tap(struct netdev *netdev_)
         goto error_close;
     }
 
+    if (ioctl(netdev->tap_fd, TUNSETPERSIST, 1)) {
+        VLOG_WARN("%s: creating tap device failed (persist): %s", name,
+                  ovs_strerror(errno));
+        error = errno;
+        goto error_close;
+    }
+
     return 0;
 
 error_close:
@@ -885,6 +892,7 @@  netdev_linux_destruct(struct netdev *netdev_)
     if (netdev_get_class(netdev_) == &netdev_tap_class
         && netdev->tap_fd >= 0)
     {
+        ioctl(netdev->tap_fd, TUNSETPERSIST, 0);
         close(netdev->tap_fd);
     }