[ovs-dev] [RFC v2] bridge: Allow manual notifications about interfaces' updates.

[Eelco Chaudron]

On 4 Nov 2019, at 14:22, Ilya Maximets wrote:

> Sometimes interface updates could happen in a way ifnotifier is not
> able to catch.  For example some heavy operations (device reset) in
> netdev-dpdk could require re-applying of the bridge configuration.
>
> For this purpose new manual notifier introduced. Its function
> 'if_notifier_manual_report()' could be called directly by the code
> that aware about changes.  This new notifier is thread safe.
>
> Signed-off-by: Ilya Maximets <i.maximets at ovn.org>
> ---
>
> Sending this as RFC that might be useful in context of the following 
> patch:
> https://mail.openvswitch.org/pipermail/ovs-dev/2019-October/364155.html
>
> It will allow us to not introduce heavy dpdk notifier just to update
> one sequence number on RTE_ETH_EVENT_INTR_RESET events. We could also
> use it to report other changes from DPDK, e.g. LSC interrupts.
>
> Version 2:
>  * Main functions moved from bridge.c to the new 
> lib/if-notifier-manual.c
>    to allow using from other lib code.
>
>  lib/automake.mk          |  3 ++-
>  lib/if-notifier-manual.c | 55 
> ++++++++++++++++++++++++++++++++++++++++
>  lib/if-notifier.h        |  7 +++++
>  vswitchd/bridge.c        |  2 ++
>  4 files changed, 66 insertions(+), 1 deletion(-)
>  create mode 100644 lib/if-notifier-manual.c
>
> diff --git a/lib/automake.mk b/lib/automake.mk
> index 17b36b43d..ebf714501 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -111,6 +111,8 @@ lib_libopenvswitch_la_SOURCES = \
>  	lib/hmapx.h \
>  	lib/id-pool.c \
>  	lib/id-pool.h \
> +	lib/if-notifier-manual.c \
> +	lib/if-notifier.h \
>  	lib/ipf.c \
>  	lib/ipf.h \
>  	lib/jhash.c \
> @@ -394,7 +396,6 @@ lib_libopenvswitch_la_SOURCES += \
>  	lib/dpif-netlink-rtnl.c \
>  	lib/dpif-netlink-rtnl.h \
>  	lib/if-notifier.c \
> -	lib/if-notifier.h \
>  	lib/netdev-linux.c \
>  	lib/netdev-linux.h \
>  	lib/netdev-linux-private.h \
> diff --git a/lib/if-notifier-manual.c b/lib/if-notifier-manual.c
> new file mode 100644
> index 000000000..72d6d8b8d
> --- /dev/null
> +++ b/lib/if-notifier-manual.c
> @@ -0,0 +1,55 @@
> +/*
> + * Copyright (c) 2019 Ilya Maximets <i.maximets at ovn.org>.
> + *
> + * 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 "openvswitch/compiler.h"
> +#include "openvswitch/thread.h"
> +#include "openvswitch/util.h"
> +#include "if-notifier.h"
> +
> +/* Implementation of a manual interface notifier.
> + *
> + * Intended for catching interface events that could not be tracked 
> by
> + * OS specific interface notifiers, e.g. iface updates in 
> netdev-dpdk.
> + * For that purpose 'if_notifier_manual_report()' should be called 
> directly
> + * by the code that aware of interface changes.
> + *
> + * Thread-safety
> + * =============
> + * This notifier is thread-safe in terms of calling its functions 
> from
> + * different thread contexts,  however if the callback passed to
> + * 'if_notifier_manual_set_cb' is used by some other code (i.e. by OS
> + * specific notifiers) it must be thread-safe itself.
> + */
> +
> +static struct ovs_mutex manual_notifier_mutex = 
> OVS_MUTEX_INITIALIZER;
> +static if_notify_func *notify OVS_GUARDED_BY(manual_notifier_mutex) = 
> NULL;
> +
> +void if_notifier_manual_set_cb(if_notify_func *cb)
> +{
> +    ovs_mutex_lock(&manual_notifier_mutex);
> +    notify = cb;
> +    ovs_mutex_unlock(&manual_notifier_mutex);
> +}
> +
> +void if_notifier_manual_report(void)
> +{
> +    ovs_mutex_lock(&manual_notifier_mutex);
> +    if (notify) {
> +        notify(NULL);
> +    }
> +    ovs_mutex_unlock(&manual_notifier_mutex);
> +}
> diff --git a/lib/if-notifier.h b/lib/if-notifier.h
> index 259138f70..dae852e9f 100644
> --- a/lib/if-notifier.h
> +++ b/lib/if-notifier.h
> @@ -27,4 +27,11 @@ void if_notifier_destroy(struct if_notifier *);
>  void if_notifier_run(void);
>  void if_notifier_wait(void);
>
> +/* Below functions are reserved for if-notifier-manual , i.e. for 
> manual
> + * notifications from the OVS code.
> + * Must not be implemented by OS specific notifiers. */
> +
> +void if_notifier_manual_set_cb(if_notify_func *);
> +void if_notifier_manual_report(void);
> +
>  #endif  /* if-notifier.h */
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index 9095ebf5d..37131712d 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -529,11 +529,13 @@ bridge_init(const char *remote)
>      ifaces_changed = seq_create();
>      last_ifaces_changed = seq_read(ifaces_changed);
>      ifnotifier = if_notifier_create(if_change_cb, NULL);
> +    if_notifier_manual_set_cb(if_change_cb);
>  }
>
>  void
>  bridge_exit(bool delete_datapath)
>  {
> +    if_notifier_manual_set_cb(NULL);
>      if_notifier_destroy(ifnotifier);
>      seq_destroy(ifaces_changed);
>
> -- 
> 2.17.1

This looks good to me, please make it an official patch ;)

//Eelco