Skip to content
Snippets Groups Projects

Control socket activation

Merged Control socket activation
dkg/resolver:control-socket-activation into master
Merged Daniel Kahn Gillmor requested to merge dkg/resolver:control-socket-activation into master

This branch provides reasonable configs for full systemd socket activation for kresd.

Merge request reports

Merged by avatar (Apr 7, 2025 3:39pm UTC)

Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Daniel Kahn Gillmor Added 3 commits:

    Added 3 commits:

    • 8005121d - allow control socket to be specified by systemd supervision
    • f1c1803d - systemd rules for closely-supervised knot-resolver service
    • 3300c9f9 - update documentation to explain systemd socket-activation configuration
  • Daniel Kahn Gillmor Added 3 commits:

    Added 3 commits:

    • 0b5a8277 - allow control socket to be specified by systemd supervision
    • 390fb104 - systemd rules for closely-supervised knot-resolver service
    • b9ebff6a - update documentation to explain systemd socket-activation configuration
  • Daniel Kahn Gillmor Added 2 commits:

    Added 2 commits:

    • ab3dd04f - systemd rules for closely-supervised knot-resolver service
    • efa731c3 - update documentation to explain systemd socket-activation configuration
  • Daniel Kahn Gillmor Added 7 commits:

    Added 7 commits:

    • efa731c3...5426e206 - 4 commits from branch knot:master
    • f431416d - allow control socket to be specified by systemd supervision
    • d176afeb - systemd rules for closely-supervised knot-resolver service
    • c1ab798f - update documentation to explain systemd socket-activation configuration
    Toggle commit list
  • Marek Vavrusa
    Marek Vavrusa @vavrusam started a thread on the diff
daemon/main.c
335 335 uv_pipe_open(&pipe, 0);
336 336 uv_read_start((uv_stream_t*) &pipe, tty_alloc, tty_read);
337 337 } else {
338 (void) mkdir("tty", S_IRWXU|S_IRWXG);
339 sock_file = afmt("tty/%ld", getpid());
340 if (sock_file) {
341 uv_pipe_bind(&pipe, sock_file);
342 uv_listen((uv_stream_t *) &pipe, 16, tty_accept);
338 int pipe_ret = -1;
339 if (control_fd != -1) {
  • Marek Vavrusa
    Marek Vavrusa @vavrusam ·
    Reporter

    While one control socket per process group is something that I'd like to do in the future - what's the benefit from taking it from systemd?

  • Daniel Kahn Gillmor
    Daniel Kahn Gillmor @dkg ·
    Author Guest

    The benefit is that the socket can be placed somewhere that the non-privileged user has no write permissions. I want to start kresd in a fully-unprivileged mode.

  • Marek Vavrusa
    Marek Vavrusa @vavrusam started a thread on the diff
    • daemon/main.c
    457 464
    458 465 #ifdef HAS_SYSTEMD
    459 466 /* Accept passed sockets from systemd supervisor. */
    460 int sd_nsocks = sd_listen_fds(0);
    467 char **socket_names = NULL;
    468 int sd_nsocks = sd_listen_fds_with_names(0, &socket_names);
    461 469 for (int i = 0; i < sd_nsocks; ++i) {
    462 470 int fd = SD_LISTEN_FDS_START + i;
    463 array_push(fd_set, fd);
    471 /* when run under systemd supervision, do not use interactive mode */
    472 g_interactive = false;
    473 if (forks != 1) {
    474 kr_log_error("[system] when run under systemd-style supervision, "
    475 "use single-process only (bad: --fork=%d).\n", forks);
    • Marek Vavrusa
      Marek Vavrusa @vavrusam ·
      Reporter

      Err how does systemd work when you want to start multiple forks? Toy supervisor in scripts can do this perfectly fine because it communicates bound sockets through env variable and all workers are free to use them. systemd passes pipe so I guess it wouldn't be happy if all workers asked for the same socket set, but there should be a way to do that.

      Edited by Marek Vavrusa
    • Daniel Kahn Gillmor
      Daniel Kahn Gillmor @dkg ·
      Author Guest

      I think systemd can bind a set of supervised daemons to the same ports. I'd implement this with a systemd generator, but in practice i think that's likely to be overkill for most implementations.

      I can provide an example set of generator unit files if that's something you want, but i figured it'd be nice to start more simply.

    • Ondřej Surý
      Ondřej Surý @ondrej ·
      Guest

      The idea is that you just use one fork. While the generators would be a nice feature we can provide in future, I think we don't need to cover all use cases. This would be to start kresd in full unprivileged mode using systemd, and if the sysadmin wants anything more complicated he would need to do his own provisioning anyway.

    • Marek Vavrusa
      Marek Vavrusa @vavrusam ·
      Reporter

      Okay, sounds reasonable.

  • Marek Vavrusa
  • Marek Vavrusa
    Marek Vavrusa @vavrusam started a thread on the diff
    • daemon/main.c
    457 464
    458 465 #ifdef HAS_SYSTEMD
    459 466 /* Accept passed sockets from systemd supervisor. */
    460 int sd_nsocks = sd_listen_fds(0);
    467 char **socket_names = NULL;
    468 int sd_nsocks = sd_listen_fds_with_names(0, &socket_names);
    461 469 for (int i = 0; i < sd_nsocks; ++i) {
    462 470 int fd = SD_LISTEN_FDS_START + i;
    463 array_push(fd_set, fd);
    471 /* when run under systemd supervision, do not use interactive mode */
    472 g_interactive = false;
    473 if (forks != 1) {
    474 kr_log_error("[system] when run under systemd-style supervision, "
    475 "use single-process only (bad: --fork=%d).\n", forks);
    476 return EXIT_FAILURE;
    • Marek Vavrusa
      Marek Vavrusa @vavrusam ·
      Reporter

      Leaks socket_names

    • Daniel Kahn Gillmor
      Daniel Kahn Gillmor @dkg ·
      Author Guest

      ---eh? i don't think so -- it free()s it later in the patch, no? ---

      Edited to add: oh, i see what you mean, in the failure case. In this case, the daemon is going to exit anyway, so i hadn't spent the extra cycles to clean up the RAM. but i can do that in a future revision if you prefer.

      Edited by Daniel Kahn Gillmor
  • Marek Vavrusa
    Marek Vavrusa @vavrusam started a thread on the diff
    • daemon/main.c
    463 array_push(fd_set, fd);
    471 /* when run under systemd supervision, do not use interactive mode */
    472 g_interactive = false;
    473 if (forks != 1) {
    474 kr_log_error("[system] when run under systemd-style supervision, "
    475 "use single-process only (bad: --fork=%d).\n", forks);
    476 return EXIT_FAILURE;
    477 }
    478 if (!strcasecmp("control",socket_names[i])) {
    479 control_fd = fd;
    480 } else {
    481 array_push(fd_set, fd);
    482 }
    483 free(socket_names[i]);
    464 484 }
    485 free(socket_names);
  • Daniel Kahn Gillmor Added 9 commits:

    Added 9 commits:

    • c1ab798f...6887a4a2 - 6 commits from branch knot:master
    • 2a95547e - allow control socket to be specified by systemd supervision
    • ed62cc88 - systemd rules for closely-supervised knot-resolver service
    • 023b92a4 - update documentation to explain systemd socket-activation configuration
    Toggle commit list
  • Daniel Kahn Gillmor
    Daniel Kahn Gillmor @dkg ·
    Author Guest

    I've revised this series now, taking into account the concerns and suggestions raised by Marek and Ondřej above.

  • Marek Vavrusa
    Marek Vavrusa @vavrusam ·
    Reporter

    LGTM, thanks a lot for contributing this!

  • Marek Vavrusa mentioned in commit 7dddb6c0

    mentioned in commit 7dddb6c0

  • Marek Vavrusa Status changed to merged

    Status changed to merged

  • Vladimír Čunát
    Vladimír Čunát @vcunat started a thread on commit ed62cc88
    • systemd/knot-resolver.service 0 → 100644
    1 [Unit]
    2 Description=Knot DNS Resolver daemon
    3 ## This is a socket-activated service:
    4 RefuseManualStart=true
    5
    6 [Service]
    7 Type=notify
    8 WorkingDirectory=/run/knot-resolver/cache
    9 ExecStart=/usr/sbin/kresd
    10 User=knot-resolver
    11 Restart=on-failure
    12
    13 [Install]
    14 WantedBy=sockets.target
    Please register or sign in to reply