[ovs-dev] [PATCH 3/3] bridge: reconfigure when system interfaces change
Ben Pfaff
blp at nicira.com
Mon Jul 27 22:59:18 UTC 2015
I guess we need an if-notifier-windows.c also.
On Mon, Jul 27, 2015 at 03:55:52PM -0700, Gurucharan Shetty wrote:
> I wonder what happens for Windows here.
>
> On Mon, Jul 27, 2015 at 3:51 PM, Ben Pfaff <blp at nicira.com> wrote:
> > 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 <cascardo at 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(¬ifier->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(¬ifier->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(¬ifier->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 at openvswitch.org
> >> http://openvswitch.org/mailman/listinfo/dev
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
More information about the dev
mailing list