Hi Bhanuprakash,
Thanks for working on this.
I have a general concern about implementation of this functionality:
*What is the profit from using rte_keepalive library ?*
Pros:
* No need to implement 'rte_keepalive_dispatch_pings()' (40 LOC)
and struct rte_keepalive (30 LOC, can be significantly decreased
by removing not needed elements) ---> ~70 LOC.
Cons:
* DPDK dependency:
* Implementation of PMD threads management (KA) inside netdev code
(netdev-dpdk) looks very strange.
* Many DPDK references in generic code (like dpdk_is_enabled).
* Feature isn't available for the common threads (main?) wihtout DPDK.
* Many stubs and placeholders for cases without dpdk.
* No ability for unit testing.
So, does it worth to use rte_keepalive? To make functionality fully
generic we only need to implement 'rte_keepalive_dispatch_pings()'
and few helpers. As soon as this function does nothing dpdk-specific
it's a really simple task which will allow to greatly clean up the
code. The feature is too big to use external library for 70 LOCs of
really simple code. (Clean up should save much more).
Am I missed something?
Any thoughts?
Best regards, Ilya Maximets.
> Keepalive feature is aimed at achieving Fastpath Service Assurance
> in OVS-DPDK deployments. It adds support for monitoring the packet
> processing cores(PMD thread cores) by dispatching heartbeats at regular
> intervals. Incase of heartbeat misses additional health checks are
> enabled on the PMD thread to detect the failure and the same shall be
> reported to higher level fault management systems/frameworks.
>
> The implementation uses OVSDB for reporting the health of the PMD threads.
> Any external monitoring application can read the status from OVSDB at
> regular intervals (or) subscribe to the updates in OVSDB so that they get
> notified when the changes happen on OVSDB.
>
> keepalive info struct is created and initialized for storing the
> status of the PMD threads. This is initialized by main thread(vswitchd)
> as part of init process and will be periodically updated by 'keepalive'
> thread. keepalive feature can be enabled through below OVSDB settings.
>
> enable-keepalive=true
> - Keepalive feature is disabled by default.
>
> keepalive-interval="5000"
> - Timer interval in milliseconds for monitoring the packet
> processing cores.
>
> When KA is enabled, 'ovs-keepalive' thread shall be spawned that wakes
> up at regular intervals to update the timestamp and status of pmd cores
> in keepalive info struct. This information shall be read by vswitchd thread
> and write the status in to 'keepalive' column of Open_vSwitch table in OVSDB.
>
> An external monitoring framework like collectd with ovs events support
> can read (or) subscribe to the datapath status changes in ovsdb. When the
> state
> is updated, the collectd shall be notified and will eventually relay the
> status
> to ceilometer service running in the controller. Below is the high level
> overview of deployment model.
>
> Compute Node Controller Compute Node
>
> Collectd <----------> Ceilometer <--------> Collectd
>
> OvS DPDK OvS DPDK
>
> +-----+
> | VM |
> +--+--+
> \---+---/
> |
> +--+---+ +------------+----------+ +------+-------+
> | OVS |-----> | ovsevents plugin | --> | collectd |
> +--+---+ +------------+----------+ +------+-------+
>
> +------+-----+ +---------------+------------+ |
> | Ceilometer | <-- | collectd ceilometer plugin | <---
> +------+-----+ +---------------+------------+
>
> github: The patches can be found here:
> https://github.com/bbodired/ovs (Last master commit e7cd8c363)
>
> Performance impact:
> No noticeable performance or latency impact is observed with
> KA feature enabled.
>
> -------------------------------------------------
> v2 -> v3
> * Rebase.
> * Verified with dpdk-stable-17.05.1 release.
> * Fixed build issues with MSVC and cross checked with appveyor.
>
> v1 -> v2
> * Rebase
> * Drop 01/20 Patch "Consolidate process related APIs" of V1 as it
> is already applied as separate patch.
>
> RFCv3 -> v1
> * Made changes to fix failures in some unit test cases.
> * some more code cleanup w.r.t process related APIs.
>
> RFCv2 -> RFCv3
> * Remove POSIX shared memory block implementation (suggested by Aaron).
> * Rework the logic to register and track threads instead of cores. This way
> in the future any thread can be registered to KA framework. For now only
> PMD
> threads are tracked (suggested by Aaron).
> * Refactor few APIs and further clean up the code.
>
> RFCv1 -> RFCv2
> * Merged the xml and schema commits to later commit where the actual
> implementation is done(suggested by Ben).
> * Fix ovs-appctl keepalive/* hang issue when KA disabled.
> * Fixed memory leaks with appctl commands for keepalive/pmd-health-show,
> pmd-xstats-show.
> * Refactor code and fixed APIs dealing with PMD health monitoring.
>
>
> Bhanuprakash Bodireddy (19):
> dpdk: Add helper functions for DPDK datapath keepalive.
> process: Avoid warnings compiling process.c
> process: Add helper function to retrieve process status.
> Keepalive: Add initial keepalive support.
> keepalive: Add more helper functions to KA framework.
> dpif-netdev: Add helper function to store datapath tids.
> dpif-netdev: Register packet processing cores to KA framework.
> dpif-netdev: Enable heartbeats for DPDK datapath.
> keepalive: Retrieve PMD status periodically.
> bridge: Update keepalive status in OVSDB
> keepalive: Add support to query keepalive statistics.
> keepalive: Add support to query keepalive status.
> dpif-netdev: Add additional datapath health checks.
> keepalive: Check the link status as part of PMD health checks.
> keepalive: Check the packet statistics as part of PMD health checks.
> keepalive: Check the PMD cycle stats as part of PMD health checks.
> netdev-dpdk: Enable PMD health checks on heartbeat failure.
> keepalive: Display extended Keepalive status.
> Documentation: Update DPDK doc with Keepalive feature.
>
> Documentation/howto/dpdk.rst | 90 +++++
> lib/automake.mk | 2 +
> lib/dpdk-stub.c | 36 ++
> lib/dpdk.c | 68 +++-
> lib/dpdk.h | 13 +
> lib/dpif-netdev.c | 203 +++++++++-
> lib/dpif-netdev.h | 7 +
> lib/keepalive.c | 922
> +++++++++++++++++++++++++++++++++++++++++++
> lib/keepalive.h | 149 +++++++
> lib/netdev-dpdk.c | 119 +++++-
> lib/netdev-dpdk.h | 5 +
> lib/process.c | 84 +++-
> lib/process.h | 13 +
> lib/util.c | 10 +
> lib/util.h | 1 +
> vswitchd/bridge.c | 34 ++
> vswitchd/vswitch.ovsschema | 8 +-
> vswitchd/vswitch.xml | 49 +++
> 18 files changed, 1779 insertions(+), 34 deletions(-)
> create mode 100644 lib/keepalive.c
> create mode 100644 lib/keepalive.h
>
> --
> 2.4.11
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
