All of lore.kernel.org
 help / color / mirror / Atom feed
* [dm-devel] [PATCH v2 00/47] multipathd: uxlsnr overhaul
@ 2021-11-18 22:57 mwilck
  2021-11-18 22:57 ` [dm-devel] [PATCH v2 01/48] libmultipath: add timespeccmp() utility function mwilck
                   ` (47 more replies)
  0 siblings, 48 replies; 76+ messages in thread
From: mwilck @ 2021-11-18 22:57 UTC (permalink / raw)
  To: Christophe Varoqui, Benjamin Marzinski
  Cc: lixiaokeng, Chongyun Wu, dm-devel, Martin Wilck

From: Martin Wilck <mwilck@suse.com>

Hello Christophe, hello Ben,

The current multipathd unix listener code has various deficiencies.

 - client disconnects aren't handled correctly,
 - the uxsock_timeout is applied for receiving, handling, and
   responding to the client requests separately, rather than for
   the entire operation,
 - timeouts are logged, but not acted upon, causing the timeout
   to be noticed in the client rather than in the server.
 - clients may see a timeout while "reconfigure" is running,
 - unpriviledged (non-root) client connections don't work
   correctly
 - most importantly, the code busy-loops, polls, or waits in
   various places in called subroutines, which is a no-go in a
   piece of code designed as an event handler and may lead
   to spurious timeouts and delayed reaction e.g. to signals
   or client requests.

This patch set approaches all these issues. Fixing the last one,
in particular, requires a major refactoring of the uxlsnr code.
Overall, the reliability and latency of client request handling
and signal handling by multipathd should be noticeably improved
by this patch set.

The biggest problem (waiting for the vecs lock in a client handler)
can only be fixed by moving this wait into the handlers ppoll()
loop (another possible fix would have been to handle all clients
in separate threads, but that would have required even more
complexity). The patch set achieves this by adding an eventfd-based
notification mechanism to the vecs lock, which can be passed to
ppoll() to wake up when the lock is freed.

Furthermore, client requests can't be handled in a single poll
iteration any more. Therefore the client connection becomes stateful,
and is handled by a state machine using the states RECEIVE, PARSE,
WAIT FOR LOCK, WORK, and SEND.

The refactoring is done step by step for ease (hopefully) of
review. 1/35-4/35 add utility code that will be used by the uxlsnr
refactoring. 5/35-7/35 are some independent patches that
aren't directly related to uxlnsr, but fix issues that I observed
while working on this set.

8/35-13/35 are minor fixups in the client handling code. This code is
strongly related to the uxlsnr, thus I thought I'd rather fix it
before making the other changes. In 25/35, the cli-handlers are
converted to use the strbuf API everywhere instead of separate "reply"
and "len" arguments. 15/35-18/35 are minor fixes for the
uxlsnr. 19/35-34/35 are the actual refactoring patches for the uxlsnr
code. First I move some code around unchanged, then I add the
state machine (handle_client()) and move the code into it piece
by piece. 35/35 adds a fix for the client side (multipathd -k).

CC'ing Lixiaokeng and Chongyun Wu, as they have test cases that use
the client code heavily AFAIR. Testing by 3rd parties would be
very welcome.

---

Changes wrt v1 (Ben Marzinski):
  03: this is a major library version change.
  07: make set_config_state() static
  12: further simplify add_handler, make it static, and use assert
        to check for multiply-defined handlers
  14: dropped in favor of Ben's "reconfigure all" set, numbering changes
        from here on
  29 (was 30): don't use fallthrough; call state machine in a loop instead.
     fix signedness of return codes. Fix double messages.
  30 (was 31): The lock handling in this patch was broken. It could happen that
     the uxlsnr was cancelled without releasing the lock. Fixed by
     simplification. 
  35 (new): Use recv() for getting the command length, as suggested by Ben.

Moreover, I'm reposting Ben's rebased series on top of mine.

Comments welcome, regards,
Martin

Benjamin Marzinski (12):
  multipathd: move delayed_reconfig out of struct config
  multipathd: remove reconfigure from header file.
  multipathd: pass in the type of reconfigure
  multipathd: add "reconfigure all" command.
  multipathd: remove missing paths on startup
  libmultipath: skip unneeded steps to get path name
  libmultipath: don't use fallback wwid in update_pathvec_from_dm
  libmultipath: always set INIT_REMOVED in set_path_removed
  multipathd: fully initialize paths added by update_pathvec_from_dm
  multipathd: retrigger uevent for partial paths
  multipathd: remove INIT_PARTIAL paths that aren't in a multipath
    device
  multipathd: Remove dependency on systemd-udev-settle.service

Martin Wilck (36):
  libmultipath: add timespeccmp() utility function
  libmultipath: add trylock() helper
  libmultipath: add optional wakeup functionality to lock.c
  libmultipath: print: add __snprint_config()
  libmultipath: improve cleanup of uevent queues on exit
  multipathd: fix systemd notification when stopping while reloading
  multipathd: improve delayed reconfigure
  multipathd: cli.h: formatting improvements
  multipathd: cli_del_map: fix reply for delayed action
  multipathd: add prototype for cli_handler functions
  multipathd: make all cli_handlers static
  multipathd: add and set cli_handlers in a single step
  multipathd: cli.c: use ESRCH for "command not found"
  multipathd: uxlsnr: avoid stalled clients during reconfigure
  multipathd: uxlsnr: handle client HUP
  multipathd: uxlsnr: use symbolic values for pollfd indices
  multipathd: uxlsnr: avoid using fd -1 in ppoll()
  multipathd: uxlsnr: data structure for stateful client connection
  multipathd: move uxsock_trigger() to uxlsnr.c
  multipathd: move parse_cmd() to uxlsnr.c
  multipathd: uxlsnr: remove check_timeout()
  multipathd: uxlsnr: move client handling to separate function
  multipathd: uxlsnr: use main poll loop for receiving
  multipathd: use strbuf in cli_handler functions
  multipathd: uxlsnr: check root on connection startup
  multipathd: uxlsnr: pass struct client to uxsock_trigger() and
    parse_cmd()
  multipathd: uxlsnr: move handler execution to separate function
  multipathd: uxlsnr: use parser to determine non-root commands
  multipathd: uxlsnr: merge uxsock_trigger() into state machine
  multipathd: uxlsnr: add idle notification
  multipathd: uxlsnr: add timeout handling
  multipathd: uxlsnr: use poll loop for sending, too
  multipathd: uxlsnr: drop client_lock
  multipathd: uxclt: allow client mode for non-root, too
  multipathd: uxlsnr: use recv() for command length
  libmultipath: add path wildcard "%I" for init state

 libmpathpersist/libmpathpersist.version |  12 +-
 libmultipath/config.h                   |   1 -
 libmultipath/configure.c                |   4 +-
 libmultipath/devmapper.c                |   2 +
 libmultipath/discovery.c                |   7 +-
 libmultipath/discovery.h                |   2 +
 libmultipath/libmultipath.version       |  10 +-
 libmultipath/lock.c                     |  12 +-
 libmultipath/lock.h                     |  11 +-
 libmultipath/print.c                    |  55 ++-
 libmultipath/print.h                    |   2 +
 libmultipath/structs.h                  |   9 +
 libmultipath/structs_vec.c              |  41 +-
 libmultipath/structs_vec.h              |   2 +-
 libmultipath/time-util.c                |  12 +
 libmultipath/time-util.h                |   1 +
 libmultipath/uevent.c                   |  49 +-
 multipath/main.c                        |   2 +-
 multipathd/cli.c                        | 181 ++-----
 multipathd/cli.h                        | 102 ++--
 multipathd/cli_handlers.c               | 596 ++++++++++++------------
 multipathd/cli_handlers.h               |  61 +--
 multipathd/main.c                       | 357 +++++++-------
 multipathd/main.h                       |   4 +-
 multipathd/multipathd.8                 |  10 +-
 multipathd/multipathd.service           |   3 +-
 multipathd/uxlsnr.c                     | 542 +++++++++++++++------
 multipathd/uxlsnr.h                     |   4 +-
 28 files changed, 1193 insertions(+), 901 deletions(-)

-- 
2.33.1


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


^ permalink raw reply	[flat|nested] 76+ messages in thread

end of thread, other threads:[~2021-12-16 15:34 UTC | newest]

Thread overview: 76+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-18 22:57 [dm-devel] [PATCH v2 00/47] multipathd: uxlsnr overhaul mwilck
2021-11-18 22:57 ` [dm-devel] [PATCH v2 01/48] libmultipath: add timespeccmp() utility function mwilck
2021-11-18 22:57 ` [dm-devel] [PATCH v2 02/48] libmultipath: add trylock() helper mwilck
2021-11-18 22:57 ` [dm-devel] [PATCH v2 03/48] libmultipath: add optional wakeup functionality to lock.c mwilck
2021-11-24 20:41   ` Benjamin Marzinski
2021-11-24 21:20     ` Martin Wilck
2021-11-29 19:27       ` Benjamin Marzinski
2021-11-29 20:52         ` Martin Wilck
2021-11-30 16:52           ` Benjamin Marzinski
2021-11-30 20:28             ` Martin Wilck
2021-12-01 12:06               ` Martin Wilck
2021-12-14 23:25                 ` Benjamin Marzinski
2021-12-15  7:33                   ` Martin Wilck
2021-12-15 21:01                     ` Martin Wilck
2021-12-16 15:34                       ` Benjamin Marzinski
2021-11-18 22:57 ` [dm-devel] [PATCH v2 04/48] libmultipath: print: add __snprint_config() mwilck
2021-11-18 22:57 ` [dm-devel] [PATCH v2 05/48] libmultipath: improve cleanup of uevent queues on exit mwilck
2021-11-18 22:57 ` [dm-devel] [PATCH v2 06/48] multipathd: fix systemd notification when stopping while reloading mwilck
2021-11-18 22:57 ` [dm-devel] [PATCH v2 07/48] multipathd: improve delayed reconfigure mwilck
2021-11-24 21:18   ` Benjamin Marzinski
2021-11-18 22:58 ` [dm-devel] [PATCH v2 08/48] multipathd: cli.h: formatting improvements mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 09/48] multipathd: cli_del_map: fix reply for delayed action mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 10/48] multipathd: add prototype for cli_handler functions mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 11/48] multipathd: make all cli_handlers static mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 12/48] multipathd: add and set cli_handlers in a single step mwilck
2021-11-24 21:41   ` Benjamin Marzinski
2021-11-18 22:58 ` [dm-devel] [PATCH v2 13/48] multipathd: cli.c: use ESRCH for "command not found" mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 14/48] multipathd: uxlsnr: avoid stalled clients during reconfigure mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 15/48] multipathd: uxlsnr: handle client HUP mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 16/48] multipathd: uxlsnr: use symbolic values for pollfd indices mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 17/48] multipathd: uxlsnr: avoid using fd -1 in ppoll() mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 18/48] multipathd: uxlsnr: data structure for stateful client connection mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 19/48] multipathd: move uxsock_trigger() to uxlsnr.c mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 20/48] multipathd: move parse_cmd() " mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 21/48] multipathd: uxlsnr: remove check_timeout() mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 22/48] multipathd: uxlsnr: move client handling to separate function mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 23/48] multipathd: uxlsnr: use main poll loop for receiving mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 24/48] multipathd: use strbuf in cli_handler functions mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 25/48] multipathd: uxlsnr: check root on connection startup mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 26/48] multipathd: uxlsnr: pass struct client to uxsock_trigger() and parse_cmd() mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 27/48] multipathd: uxlsnr: move handler execution to separate function mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 28/48] multipathd: uxlsnr: use parser to determine non-root commands mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 29/48] multipathd: uxlsnr: merge uxsock_trigger() into state machine mwilck
2021-11-24 23:48   ` Benjamin Marzinski
2021-11-25  0:38   ` Benjamin Marzinski
2021-11-26 14:34     ` Martin Wilck
2021-11-29 16:19       ` Benjamin Marzinski
2021-11-29 17:00         ` Martin Wilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 30/48] multipathd: uxlsnr: add idle notification mwilck
2021-11-25  1:08   ` Benjamin Marzinski
2021-11-26 14:23     ` Martin Wilck
2021-11-29 16:26       ` Benjamin Marzinski
2021-11-18 22:58 ` [dm-devel] [PATCH v2 31/48] multipathd: uxlsnr: add timeout handling mwilck
2021-11-25  1:23   ` Benjamin Marzinski
2021-11-18 22:58 ` [dm-devel] [PATCH v2 32/48] multipathd: uxlsnr: use poll loop for sending, too mwilck
2021-11-25  1:43   ` Benjamin Marzinski
2021-11-26 14:06     ` Martin Wilck
2021-11-29 19:01     ` Martin Wilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 33/48] multipathd: uxlsnr: drop client_lock mwilck
2021-11-25  1:45   ` Benjamin Marzinski
2021-11-18 22:58 ` [dm-devel] [PATCH v2 34/48] multipathd: uxclt: allow client mode for non-root, too mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 35/48] multipathd: uxlsnr: use recv() for command length mwilck
2021-11-25  1:54   ` Benjamin Marzinski
2021-11-18 22:58 ` [dm-devel] [PATCH v2 36/48] multipathd: move delayed_reconfig out of struct config mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 37/48] multipathd: remove reconfigure from header file mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 38/48] multipathd: pass in the type of reconfigure mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 39/48] multipathd: add "reconfigure all" command mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 40/48] multipathd: remove missing paths on startup mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 41/48] libmultipath: skip unneeded steps to get path name mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 42/48] libmultipath: don't use fallback wwid in update_pathvec_from_dm mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 43/48] libmultipath: always set INIT_REMOVED in set_path_removed mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 44/48] multipathd: fully initialize paths added by update_pathvec_from_dm mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 45/48] multipathd: retrigger uevent for partial paths mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 46/48] multipathd: remove INIT_PARTIAL paths that aren't in a multipath device mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 47/48] multipathd: Remove dependency on systemd-udev-settle.service mwilck
2021-11-18 22:58 ` [dm-devel] [PATCH v2 48/48] libmultipath: add path wildcard "%I" for init state mwilck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.