On Mon, Jul 27, 2015 at 02:24:20PM -0300, Thadeu Lima de Souza Cascardo wrote:
> Whenever system interfaces are removed, added or change state, reconfigure
> bridge. This allows late interfaces to be added to the datapath when they are
> added to the system after ovs-vswitchd is started.
> 
> Signed-off-by: Thadeu Lima de Souza Cascardo <casca...@redhat.com>

I'd always envisioned this kind of feature as needing to keep track of
the interfaces that OVS tried to configure but were missing and only
reconfiguring if one of those showed up.  The approach in this commit is
simpler.  I like that as a first step.  Then, we can optimize it later,
only if optimization turns out to be necessary,

Here are some style fixes to fold in for v2.

diff --git a/lib/if-notifier-bsd.c b/lib/if-notifier-bsd.c
index 06874b0..aa750aa 100644
--- a/lib/if-notifier-bsd.c
+++ b/lib/if-notifier-bsd.c
@@ -25,7 +25,8 @@ struct if_notifier {
     void *aux;
 };
 
-static void if_notifier_cb(const struct rtbsd_change *change, void *aux)
+static void
+if_notifier_cb(const struct rtbsd_change *change, void *aux)
 {
     struct if_notifier *notifier;
     notifier = aux;
@@ -49,7 +50,8 @@ if_notifier_create(if_notify_func *cb, void *aux)
     return notifier;
 }
 
-void if_notifier_destroy(struct if_notifier *notifier)
+void
+if_notifier_destroy(struct if_notifier *notifier)
 {
     if (notifier) {
         rtbsd_notifier_unregister(&notifier->notifier);
@@ -57,12 +59,14 @@ void if_notifier_destroy(struct if_notifier *notifier)
     }
 }
 
-void if_notifier_run(void)
+void
+if_notifier_run(void)
 {
     rtbsd_notifier_run();
 }
 
-void if_notifier_wait(void)
+void
+if_notifier_wait(void)
 {
     rtbsd_notifier_wait();
 }
diff --git a/lib/if-notifier.h b/lib/if-notifier.h
index 9610d2d..fc1330b 100644
--- a/lib/if-notifier.h
+++ b/lib/if-notifier.h
@@ -19,11 +19,9 @@
 
 struct if_notifier;
 
-typedef
-void if_notify_func(void *aux);
+typedef void if_notify_func(void *aux);
 
-struct if_notifier *
-if_notifier_create(if_notify_func *, void *aux);
+struct if_notifier *if_notifier_create(if_notify_func *, void *aux);
 void if_notifier_destroy(struct if_notifier *);
 
 void if_notifier_run(void);


> ---
>  lib/automake.mk       |  3 +++
>  lib/if-notifier-bsd.c | 68 
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/if-notifier.c     | 64 ++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/if-notifier.h     | 32 ++++++++++++++++++++++++
>  vswitchd/bridge.c     | 24 +++++++++++++++++-
>  5 files changed, 190 insertions(+), 1 deletion(-)
>  create mode 100644 lib/if-notifier-bsd.c
>  create mode 100644 lib/if-notifier.c
>  create mode 100644 lib/if-notifier.h
> 
> diff --git a/lib/automake.mk b/lib/automake.mk
> index 018e62c..4946d90 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -338,6 +338,8 @@ if LINUX
>  lib_libopenvswitch_la_SOURCES += \
>       lib/dpif-netlink.c \
>       lib/dpif-netlink.h \
> +     lib/if-notifier.c \
> +     lib/if-notifier.h \
>       lib/netdev-linux.c \
>       lib/netdev-linux.h \
>       lib/netlink-notifier.c \
> @@ -384,6 +386,7 @@ endif
>  
>  if HAVE_IF_DL
>  lib_libopenvswitch_la_SOURCES += \
> +     lib/if-notifier-bsd.c \
>       lib/netdev-bsd.c \
>       lib/rtbsd.c \
>       lib/rtbsd.h \
> diff --git a/lib/if-notifier-bsd.c b/lib/if-notifier-bsd.c
> new file mode 100644
> index 0000000..06874b0
> --- /dev/null
> +++ b/lib/if-notifier-bsd.c
> @@ -0,0 +1,68 @@
> +/*
> + * Copyright (c) 2014 Red Hat, Inc.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <config.h>
> +#include "if-notifier.h"
> +#include "rtbsd.h"
> +#include "util.h"
> +
> +struct if_notifier {
> +    struct rtbsd_notifier notifier;
> +    if_notify_func *cb;
> +    void *aux;
> +};
> +
> +static void if_notifier_cb(const struct rtbsd_change *change, void *aux)
> +{
> +    struct if_notifier *notifier;
> +    notifier = aux;
> +    notifier->cb(notifier->aux);
> +}
> +
> +struct if_notifier *
> +if_notifier_create(if_notify_func *cb, void *aux)
> +{
> +    struct if_notifier *notifier;
> +    int ret;
> +    notifier = xzalloc(sizeof *notifier);
> +    notifier->cb = cb;
> +    notifier->aux = aux;
> +    ret = rtbsd_notifier_register(&notifier->notifier, if_notifier_cb,
> +                                  notifier);
> +    if (ret) {
> +        free(notifier);
> +        return NULL;
> +    }
> +    return notifier;
> +}
> +
> +void if_notifier_destroy(struct if_notifier *notifier)
> +{
> +    if (notifier) {
> +        rtbsd_notifier_unregister(&notifier->notifier);
> +        free(notifier);
> +    }
> +}
> +
> +void if_notifier_run(void)
> +{
> +    rtbsd_notifier_run();
> +}
> +
> +void if_notifier_wait(void)
> +{
> +    rtbsd_notifier_wait();
> +}
> diff --git a/lib/if-notifier.c b/lib/if-notifier.c
> new file mode 100644
> index 0000000..1bf77c5
> --- /dev/null
> +++ b/lib/if-notifier.c
> @@ -0,0 +1,64 @@
> +/*
> + * Copyright (c) 2014 Red Hat, Inc.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <config.h>
> +#include "if-notifier.h"
> +#include "rtnetlink-link.h"
> +#include "util.h"
> +
> +struct if_notifier {
> +    struct nln_notifier *notifier;
> +    if_notify_func *cb;
> +    void *aux;
> +};
> +
> +static
> +void if_notifier_cb(const struct rtnetlink_link_change *change OVS_UNUSED,
> +                           void *aux)
> +{
> +    struct if_notifier *notifier;
> +    notifier = aux;
> +    notifier->cb(notifier->aux);
> +}
> +
> +struct if_notifier *
> +if_notifier_create(if_notify_func *cb, void *aux)
> +{
> +    struct if_notifier *notifier;
> +    notifier = xmalloc(sizeof *notifier);
> +    notifier->cb = cb;
> +    notifier->aux = aux;
> +    notifier->notifier = rtnetlink_link_notifier_create(if_notifier_cb, 
> notifier);
> +    return notifier;
> +}
> +
> +void if_notifier_destroy(struct if_notifier *notifier)
> +{
> +    if (notifier) {
> +        rtnetlink_link_notifier_destroy(notifier->notifier);
> +        free(notifier);
> +    }
> +}
> +
> +void if_notifier_run(void)
> +{
> +    rtnetlink_link_run();
> +}
> +
> +void if_notifier_wait(void)
> +{
> +    rtnetlink_link_wait();
> +}
> diff --git a/lib/if-notifier.h b/lib/if-notifier.h
> new file mode 100644
> index 0000000..9610d2d
> --- /dev/null
> +++ b/lib/if-notifier.h
> @@ -0,0 +1,32 @@
> +/*
> + * Copyright (c) 2014 Red Hat, Inc.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#ifndef IF_NOTIFIER_H
> +#define IF_NOTIFIER_H 1
> +
> +struct if_notifier;
> +
> +typedef
> +void if_notify_func(void *aux);
> +
> +struct if_notifier *
> +if_notifier_create(if_notify_func *, void *aux);
> +void if_notifier_destroy(struct if_notifier *);
> +
> +void if_notifier_run(void);
> +void if_notifier_wait(void);
> +
> +#endif
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index 39d4cb3..625105b 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -48,6 +48,7 @@
>  #include "ofproto/ofproto.h"
>  #include "ovs-numa.h"
>  #include "poll-loop.h"
> +#include "if-notifier.h"
>  #include "seq.h"
>  #include "sha1.h"
>  #include "shash.h"
> @@ -217,6 +218,12 @@ static long long int stats_timer = LLONG_MIN;
>  #define AA_REFRESH_INTERVAL (1000) /* In milliseconds. */
>  static long long int aa_refresh_timer = LLONG_MIN;
>  
> +/* Whenever system interfaces are added, removed or change state, the bridge
> + * will be reconfigured.
> + */
> +static struct if_notifier *ifnotifier;
> +static bool ifaces_changed = false;
> +
>  static void add_del_bridges(const struct ovsrec_open_vswitch *);
>  static void bridge_run__(void);
>  static void bridge_create(const struct ovsrec_bridge *);
> @@ -375,6 +382,12 @@ bridge_init_ofproto(const struct ovsrec_open_vswitch 
> *cfg)
>      shash_destroy_free_data(&iface_hints);
>      initialized = true;
>  }
> +
> +static void
> +if_change_cb(void *aux OVS_UNUSED)
> +{
> +    ifaces_changed = true;
> +}
>  
>  /* Public functions. */
>  
> @@ -478,6 +491,7 @@ bridge_init(const char *remote)
>      stp_init();
>      lldp_init();
>      rstp_init();
> +    ifnotifier = if_notifier_create(if_change_cb, NULL);
>  }
>  
>  void
> @@ -485,6 +499,7 @@ bridge_exit(void)
>  {
>      struct bridge *br, *next_br;
>  
> +    if_notifier_destroy(ifnotifier);
>      HMAP_FOR_EACH_SAFE (br, next_br, node, &all_bridges) {
>          bridge_destroy(br);
>      }
> @@ -2877,6 +2892,8 @@ bridge_run(void)
>  
>      ovsdb_idl_run(idl);
>  
> +    if_notifier_run();
> +
>      if (ovsdb_idl_is_lock_contended(idl)) {
>          static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
>          struct bridge *br, *next_br;
> @@ -2943,9 +2960,12 @@ bridge_run(void)
>          }
>      }
>  
> -    if (ovsdb_idl_get_seqno(idl) != idl_seqno || vlan_splinters_changed) {
> +    if (ovsdb_idl_get_seqno(idl) != idl_seqno || vlan_splinters_changed
> +        || ifaces_changed) {
>          struct ovsdb_idl_txn *txn;
>  
> +        ifaces_changed = false;
> +
>          idl_seqno = ovsdb_idl_get_seqno(idl);
>          txn = ovsdb_idl_txn_create(idl);
>          bridge_reconfigure(cfg ? cfg : &null_cfg);
> @@ -3002,6 +3022,8 @@ bridge_wait(void)
>          ovsdb_idl_txn_wait(daemonize_txn);
>      }
>  
> +    if_notifier_wait();
> +
>      sset_init(&types);
>      ofproto_enumerate_types(&types);
>      SSET_FOR_EACH (type, &types) {
> -- 
> 2.4.2
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to