[ovs-dev] debian: Add another note on interfaces in "auto" section.

On 7 August 2017 at 10:14, Ben Pfaff <blp@ovn.org> wrote:

> On Mon, Aug 07, 2017 at 10:10:40AM -0700, Guru Shetty wrote:
> > On 7 August 2017 at 09:50, Ben Pfaff <blp@ovn.org> wrote:
> >
> > > On Sun, Aug 06, 2017 at 10:56:30PM -0700, Gurucharan Shetty wrote:
> > > > We had one note about the issues with adding OVS interfaces
> > > > in the "auto" section.  This commit adds another note about
> > > > how adding OVS bridges in the "auto" section can cause
> > > > race conditions with systemd.
> > > >
> > > > Signed-off-by: Gurucharan Shetty <guru@ovn.org>
> > > > ---
> > > >  debian/openvswitch-switch.README.Debian | 8 ++++++++
> > > >  1 file changed, 8 insertions(+)
> > > >
> > > > diff --git a/debian/openvswitch-switch.README.Debian
> > > b/debian/openvswitch-switch.README.Debian
> > > > index 5f8f823..d236824 100644
> > > > --- a/debian/openvswitch-switch.README.Debian
> > > > +++ b/debian/openvswitch-switch.README.Debian
> > > > @@ -231,3 +231,11 @@ in the 'auto' section, openvswitch-switch will
> > > forcefully be started when
> > > >  ifupdown kicks in. In a case like this, the admin needs to make sure
> > > that /usr
> > > >  has already been mounted and that a remote $syslog (if used) is
> ready to
> > > >  receive openvswitch logs.
> > > > +
> > > > +* With systemd, adding openvswitch bridges in the 'auto' section of
> the
> > > > +'interfaces' file can cause race conditions.  Debian systems have
> added
> > > a
> > > > +systemd ifup@.service file.  This file will call ifdown and ifup on
> > > interfaces
> > > > +in "auto" section automatically when they disappear and appear
> > > respectively.
> > > > +This will cause race conditions if you delete and add the same
> bridges
> > > using
> > > > +tools like "ovs-vsctl" or "ovs-dpctl".  This is also a problem when
> you
> > > > +upgrade your openvswitch kernel module using commands like
> > > 'force-reload-kmod'.
> > >
> > > I'm not really sure what the "auto" section is.  Do you just mean that
> > > they should not add lines like "auto br0" where br0 is an Open vSwitch
> > > bridge?  (I do see that there is already some wording about an "auto"
> > > section.)
> > >
> > Yes. Adding lines like "auto br0" is a license for systemd to handle them
> > "automatically".
>
> OK, can we reword this (including the existing text) a little bit, to be
> more precise?  I think that referring to this feature as a section is
> going to cause confusion for others, too.
>

How about the following diff to the existing text (and then repeat the same
in this commit)?:


 But, if the admin wants to go down this route and adds openvswitch bridges





>
> Thanks,
>
> Ben.
>

On Mon, Aug 07, 2017 at 10:27:26AM -0700, Guru Shetty wrote:
> On 7 August 2017 at 10:14, Ben Pfaff <blp@ovn.org> wrote:
> 
> > On Mon, Aug 07, 2017 at 10:10:40AM -0700, Guru Shetty wrote:
> > > On 7 August 2017 at 09:50, Ben Pfaff <blp@ovn.org> wrote:
> > >
> > > > On Sun, Aug 06, 2017 at 10:56:30PM -0700, Gurucharan Shetty wrote:
> > > > > We had one note about the issues with adding OVS interfaces
> > > > > in the "auto" section.  This commit adds another note about
> > > > > how adding OVS bridges in the "auto" section can cause
> > > > > race conditions with systemd.
> > > > >
> > > > > Signed-off-by: Gurucharan Shetty <guru@ovn.org>
> > > > > ---
> > > > >  debian/openvswitch-switch.README.Debian | 8 ++++++++
> > > > >  1 file changed, 8 insertions(+)
> > > > >
> > > > > diff --git a/debian/openvswitch-switch.README.Debian
> > > > b/debian/openvswitch-switch.README.Debian
> > > > > index 5f8f823..d236824 100644
> > > > > --- a/debian/openvswitch-switch.README.Debian
> > > > > +++ b/debian/openvswitch-switch.README.Debian
> > > > > @@ -231,3 +231,11 @@ in the 'auto' section, openvswitch-switch will
> > > > forcefully be started when
> > > > >  ifupdown kicks in. In a case like this, the admin needs to make sure
> > > > that /usr
> > > > >  has already been mounted and that a remote $syslog (if used) is
> > ready to
> > > > >  receive openvswitch logs.
> > > > > +
> > > > > +* With systemd, adding openvswitch bridges in the 'auto' section of
> > the
> > > > > +'interfaces' file can cause race conditions.  Debian systems have
> > added
> > > > a
> > > > > +systemd ifup@.service file.  This file will call ifdown and ifup on
> > > > interfaces
> > > > > +in "auto" section automatically when they disappear and appear
> > > > respectively.
> > > > > +This will cause race conditions if you delete and add the same
> > bridges
> > > > using
> > > > > +tools like "ovs-vsctl" or "ovs-dpctl".  This is also a problem when
> > you
> > > > > +upgrade your openvswitch kernel module using commands like
> > > > 'force-reload-kmod'.
> > > >
> > > > I'm not really sure what the "auto" section is.  Do you just mean that
> > > > they should not add lines like "auto br0" where br0 is an Open vSwitch
> > > > bridge?  (I do see that there is already some wording about an "auto"
> > > > section.)
> > > >
> > > Yes. Adding lines like "auto br0" is a license for systemd to handle them
> > > "automatically".
> >
> > OK, can we reword this (including the existing text) a little bit, to be
> > more precise?  I think that referring to this feature as a section is
> > going to cause confusion for others, too.
> >
> 
> How about the following diff to the existing text (and then repeat the same
> in this commit)?:
> 
> diff --git a/debian/openvswitch-switch.README.Debian
> b/debian/openvswitch-switch.README.
> index 5f8f823..200266e 100644
> --- a/debian/openvswitch-switch.README.Debian
> +++ b/debian/openvswitch-switch.README.Debian
> @@ -223,7 +223,8 @@ script that depends on openvswitch but starts before
> it, needs to be
>  to depend on openvswitch-switch too.
> 
>  * Ideally, an admin should not add openvswitch bridges in the 'auto'
> -section of the 'interfaces' file.  This is because, when ifupdown starts
> +section of the 'interfaces' file (i.e., if "br0" is a OVS bridge, you
> should
> +not have a line "auto br0"). This is because, when ifupdown starts
>  working on bridges listed in 'auto', openvswitch has not yet started.
> 
>  But, if the admin wants to go down this route and adds openvswitch bridges

Acked-by: Ben Pfaff <blp@ovn.org>

>
>
> Acked-by: Ben Pfaff <blp@ovn.org>
>

Thank you! I applied this to master.