On Fri, Jan 19, 2018 at 10:51:59AM +0000, Stokes, Ian wrote:
> > Keepalive feature is aimed at achieving Fastpath Service Assurance in OVS-
> > DPDK deployments. It adds support for monitoring the packet processing
> > threads by dispatching heartbeats at regular intervals.
> > 
> 
> Hi All,
> 
> This feature has been kicking around for quite a while now. Previously there 
> was concern regarding whether OVS was the correct place for such a feature as 
> well as a lack of documentation detailing it's use.
> 
> I think Bhanu has resolved the documentation issue in this patchset by 
> providing fairly detailed steps for use and setup.
> 
> That leaves whether OVS is the correct place for the feature. At this point I 
> think we need to ACK or NACK and provide our reasons for doing so.
> 
> @Aaron & Flavio, you've been involved in reviewing this to date and have 
> flagged some concerns which Bhanu has responded to.
> 
> Is there anything from your side that is unclear or blocking the feature at 
> this point?

Well, I am very late to review the series and it has changed
significantly between the patchset versions. My concern is that it is
not clear yet how it needs to be done. Also that it may provide a
status that we can't actually trust and then we will end up with an
external monitoring system as well.

So, consider myself on the fence. I have no real reason to NACK but
I also don't have enough confidence to ACK at this point.

fbl

> 
> Thanks
> Ian
> 
> > keepalive feature can be enabled through below OVSDB settings.
> > 
> >     enable-keepalive=true
> >       - Keepalive feature is disabled by default and should be enabled
> >         at startup before ovs-vswitchd daemon is started.
> > 
> >     keepalive-interval="5000"
> >       - Timer interval in milliseconds for monitoring the packet
> >         processing cores.
> > 
> > v4 -> v5
> >   * Add 3 more patches to the series
> >      - xnanosleep()
> >      - Documentation
> >      - Update to NEWS
> >   * Remove all references to core_id and instead implemented thread based
> > tracking.
> >   * Addressed most of the comments in v4.
> > 
> > v3 -> v4
> >   * Split the functionality in to 2 parts. This patch series only updates
> >     PMD status to OVSDB. The incremental patch series to handle false
> > positives,
> >     negatives and more checking and stats.
> >   * Remove code from netdev layer and dependency on rte_keepalive lib.
> >   * Merged few patches and simplified the patch series.
> >   * Timestamp in human readable form.
> > 
> > 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 (10):
> >   process: Extend get_process_info() for additional fields.
> >   Keepalive: Add initial keepalive support.
> >   util: Add high resolution sleep support.
> >   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 status and statistics.
> >   Documentation: Update DPDK doc with Keepalive feature.
> >   NEWS: Add keepalive support information in NEWS.
> > 
> >  Documentation/howto/dpdk.rst | 113 +++++++++
> >  NEWS                         |   2 +
> >  lib/automake.mk              |   2 +
> >  lib/dpif-netdev.c            |  91 +++++++
> >  lib/keepalive.c              | 556
> > +++++++++++++++++++++++++++++++++++++++++++
> >  lib/keepalive.h              | 111 +++++++++
> >  lib/ovs-thread.c             |   6 +
> >  lib/ovs-thread.h             |   1 +
> >  lib/process.c                |  43 ++--
> >  lib/process.h                |   2 +
> >  lib/timeval.c                |   2 +-
> >  lib/timeval.h                |   1 +
> >  lib/util.c                   |  41 ++++
> >  lib/util.h                   |   2 +
> >  vswitchd/bridge.c            |  29 +++
> >  vswitchd/vswitch.ovsschema   |   8 +-
> >  vswitchd/vswitch.xml         |  49 ++++
> >  17 files changed, 1036 insertions(+), 23 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

-- 
Flavio

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to