On 05.11.2019 20:20, Ben Pfaff wrote:
On Mon, Nov 04, 2019 at 02:22:34PM +0100, 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.maxim...@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.
I *think* I understand what's going on here. It's that the proposed
dpdk-notifier
(https://mail.openvswitch.org/pipermail/ovs-dev/2019-October/364154.html)
takes more locks, etc., so it's going to be slow. This one doesn't so
it's cheaper. This one, however, will only work if the OVS code that
makes the device changes calls into it, whereas the other one gets
notified by DPDK itself.
Both implementations doesn't receive notifications from DPDK itself.
dpdk-notifier is just a more complex variant of the same thing with
dynamic allocations and list of notifiers. In practice, the only
way to trigger it is to call dpdk_notifierr_report_link() from the
OVS code.
The part that really receives DPDK events is 'dpdk_eth_event_callback()'
from https://mail.openvswitch.org/pipermail/ovs-dev/2019-October/364155.html
There is a possibility to make everything super right (not sure).
This should look like this:
* Allow more than one type of notifiers. i.e. introduce
ifnotifier-provider.h, make a common interface in ifnotifier.{c,h}.
Move code of the existing notifiers to ifnotifier-{linux,bsd}.{c.h}
and make them both implement ifnotifier with type='system'.
* Introduce dpdk_notifier (analogue of rtnetlink_notifier) that will
subscribe itself to DPDK events with rte_eth_dev_callback_register()
and receive actual DPDK events via callback.
* Introduce ifnotifier-dpdk.{c,h} that will register itself as ifnotifier
with type='dpdk'. Subscribe ifnotifier-dpdk to dpdk_notifier with
dpdk_notifier_create().
* bridge.c should subscribe itself to all registered types of ifnotifiers
to increase 'ifaces_changed' sequence number.
* netdev-dpdk.c should subscribe to dpdk_notifier to receive DPDK
events and perform required actions on devices.
* Something else to resolve issues with DPDK intialization.
(we can't register dpdk callbacks before dpdk initialization that
could happen way after bridge initialization)
I'm not sure if it's really possible to properly implement, but it looks
like an over-engineering anyway.
BTW, I'm not sure if any of our solutions are good enough.
I think that this one could avoid introducing a mutex by using an atomic
pointer, but I don't know whether that's worthwhile.
The case is that we need to be sure that 'ifaces_changed' sequence number
will not be destroyed while (before, actually) we're using it.
Main thread Notification thread
create seq
set cb <-- func
notification appeared
func = read cb
func()
set cb <-- NULL
destroy seq
seq_change() // change of destroyed seq will lead to crash.
So, the mutex ensures that callback is still registered while we're
executing it.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
