Re: [systemd-devel] [PATCH] bus-proxy: cloning smack label
On 2014-12-10 22:37, Lennart Poettering wrote: On Tue, 09.12.14 18:26, Lennart Poettering (lenn...@poettering.net) wrote: Przemyslaw, +++ b/units/u...@.service.m4.in @@ -0,0 +1,23 @@ +# This file is part of systemd. +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. + +[Unit] +Description=User Manager for UID %i +After=systemd-user-sessions.service + +[Service] +User=%i +PAMName=systemd-user +Type=notify +ExecStart=-@rootlibexecdir@/systemd --user +Slice=user-%i.slice +KillMode=mixed +Delegate=yes +m4_ifdef(`HAVE_SMACK', +Capabilities=cap_mac_admin=i +SecureBits=keep-caps +) I have reverted the last bit above again, since it broke bootups in nspawn machines. I figure the CAP_MAC_ADMIN capability is missing from the bounding set in an nspawn, and that breaks the caps logic here. We should find another solution for this. I wanted to get 218 out of the door, hence I reverted this bit for now, but we really should fine a longer term solution for this. I build systemd with SMACK on, but turned off in the kernel. Any suggestions what we can do here? ConditionSecurity=smack instead of HAVE_SMACK would work, but it would also require separate unit for non-smack case, which is crap. No easy solutions come to my mind right now, unfortunately... Cheers, -- Karol Lewandowski, Samsung RD Institute Poland ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [HEADSUP] Next systemd Hackfest takes place at FOSDEM 2015, January 30th, Brussels!
On 2014-12-03 19:28, Lennart Poettering wrote: Heya, Harald organized a room for our next Hackfest now, on January 30th, 2015, one day before FOSDEM 2015 in Brussels, Belgium. For details please see: https://plus.google.com/events/c56kbn26s6g01n6m4tj2nmdgnfc If you intend to show up it would be nice to sign up on the Google Event, so that we have an idea how many people expect. Thanks! Have you started collecting list of topics to discuss somewhere? We would be super-happy add few items to agenda (from top of my head - possible usb gadget/functionfs service type handling, need for extensive smack rework, etc.) See you in Brussels! Hope so. :) Thanks! -- Karol Lewandowski, Samsung RD Institute Poland ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 4/4] gdbus: Add basic kdbus tests
On 12/20/2013 04:23 PM, Yin Kangkai wrote: Hi! Sorry for late response, I've been out of office the last week(s). On 2013-11-21, 12:33 +0100, Karol Lewandowski wrote: +TESTS += + +* Build test binaries: + + cd gio/tests + make + +* Set variable to use custom library and to use kdbus as session bus: + + export LD_LIBRARY_PATH=absolute_path_to_glib_libs_with_kdbus_patch + export DBUS_SESSION_BUS_ADDRESS=kdbus:path=/dev/kdbus/0-kdbus/bus + +* Run test binary server in terminal 1: + + ./gdbus-example-kdbus-server + +* Run test binary client in terminal 2: + + ./gdbus-example-kdbus-client + +* Sample client app output: + + elapsed time : 0.265072 s + elapsed time : 0.264353 s + elapsed time : 0.305062 s + elapsed time : 0.343710 s + elapsed time : 0.451501 s + elapsed time : 1.109851 s + elapsed time : 8.278217 s Will it be more interesting to show some real benchmarking numbers? ;) - results with vanilla GIO; - results with GIO with kdbus transport, talking to kdbus dbus-daemon; - results with GIO with kdbus transport, talking through systemd-bus-proxyd to systemd-bus-driverd; I agree this is very much needed. Currently, due to major changes in kdbus, we are redoing some of the gio-kdbus stuff again. Lukasz Skalski will share result in following days. Cheers, Karol ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [RFC] [PATCH 0/3] journal: Add deferred log processing to reduce synchonous IO overhead
On 12/14/2013 05:12 PM, Zbigniew Jędrzejewski-Szmek wrote: On Fri, Dec 13, 2013 at 10:16:16PM +0100, Karol Lewandowski wrote: One of the problems I see, though, is that no matter how deep I make the queue (`max_dgram_qlen') I still see process sleeping on send() way earlier that configured queue depth would suggest. Are you sure that the sysctl is set early enough, before the listening socket is created? I'm doing most of my testing in nspawn-able container, so it's ok here. I will take special precautions to make sure it's set early enough on our handheld, though. Thanks for the hint! Karol ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [RFC] [PATCH 0/3] journal: Add deferred log processing to reduce synchonous IO overhead
On 12/14/2013 04:47 AM, Lennart Poettering wrote: On Fri, 13.12.13 22:16, Karol Lewandowski (lmc...@gmail.com) wrote: On Fri, Dec 13, 2013 at 03:45:36PM +0100, Lennart Poettering wrote: On Fri, 13.12.13 12:46, Karol Lewandowski (k.lewando...@samsung.com) wrote: One of the problems I see, though, is that no matter how deep I make the queue (`max_dgram_qlen') I still see process sleeping on send() way earlier that configured queue depth would suggest. It would be interesting to find out why this happens. I mean, there are three parameters here I could think of that matter: the qlen, SO_SNDBUF on the sender, and SO_RCVBUF on the receiver (though the latter two might actually change the same value on AF_UNIX? or maybe one of the latter two is a NOP on AF_UNIX?). If any of them reaches the limit then the sender will necessarily have to block. (SO_SNDBUF and SO_RCVBUF can also be controlled via /proc/sys/net/core/rmem* and ../wmem*... For testing purposes it might be easier to play around with these and set them to ludicrously high values...) That's it. While journal code tries to set buffer size via SO_SNDBUF/SO_RCVBUF options to 8MB, kernel limits these to wmem_max/rmem_max. On machines I've tested respective values are quite small - around 150-200kB each. Increasing these did reduce context switches considerably - preliminary tests show that I can now queue thousands of messages (~5k) without problems. I will test this thoroughly in next few days. I do wonder what is the rationale behind such low limits... Thanks a lot! Karol ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [RFC] [PATCH 0/3] journal: Add deferred log processing to reduce synchonous IO overhead
On 12/12/2013 07:05 PM, Lennart Poettering wrote: On Thu, 12.12.13 17:42, Karol Lewandowski (lmc...@gmail.com) wrote: Hmm, but this wouldn't be any different from old syslog, right? I mean, old syslog also uses sendv() and recv() on AF_UNIX SOCK_DGRAM sockets... That's true, however, we aren't migrating from syslog, but from Android Logger. Android provides /dev/log_{main,radio,...} device nodes which are simple in-kernel circular buffers apps can write to at any time. This was invented when /dev/kmsg wasn't writable and serves more-or-less same purpose. From 2.6.29 it's in mailine kernel under drivers/staging/android/logger.c But if the the socket layer is really that much slower than android logging, then that's kinda sad, and the kernel socket stuff should probably be fixed? It's not that slower when we take throughput as only parameter: packet | | size [B] | Adnroid [kB] | DGRAM [kB] -+--+ 64 |110640| 84827 128 |212602|166866 256 |423880|328440 512 |815094|644499 1024 | 1543326| 1246914 2048 | 2751674| 2359654 4096 | 2821260| 4145395 However, if take context switches into account picture starts to look differently: | Android | DGRAM -+---+-- context switches | 1818 | 580443 (involuntary + voluntary) | | system time | 149.97| 118.62 [seconds] user time| 4.03| 7.45 [secodns] (Columns contain cumulative data: client + server) Attached raw results and test program. This is why I decided to try to make logging completely async and see if/what changes. Well, if AF_UNIX is slow, then we really should try to fix that in the kernel instead of bypassing it in userspace... Whether we rely on the socket buffer of the AF_UNIX socket or put together our own buffer in /dev/shm shouldn't be much of a difference if they both have the same size and can take the same number of entries... I do agree. Truth is I have looked at linux/net/ code once but I didn't grok it well. I guess it's the time to take a second look. ;) Here's another option: extend journald to use kdbus as additional transport. This is something we want to do anyway since the kdbus transport will attach the metadata we need without race to each packet. Given that kdbus ultimately is just a way to write into an mmaped tmpfs that some other process owns this should not be much worse than the android logger in performance. I doubt it would help as other side would still be woken up on _every_ message, right? Cheers, Karol Client: /usr/bin/time --verbose ./logger-client Average stats: packet size = Bytes/s 64 =113295768 Bytes/s = 110640 kB/s 128 =217704976 Bytes/s = 212602 kB/s 256 =434053344 Bytes/s = 423880 kB/s 512 =834656704 Bytes/s = 815094 kB/s 1024 = 1580365824 Bytes/s = 1543326 kB/s 2048 = 2817714432 Bytes/s = 2751674 kB/s 4096 = 2888970749 Bytes/s = 2821260 kB/s Command being timed: ./logger-client User time (seconds): 2.20 System time (seconds): 74.80 Percent of CPU this job got: 100% Elapsed (wall clock) time (h:mm:ss or m:ss): 1:17.00 Average shared text size (kbytes): 0 Average unshared data size (kbytes): 0 Average stack size (kbytes): 0 Average total size (kbytes): 0 Maximum resident set size (kbytes): 624 Average resident set size (kbytes): 0 Major (requiring I/O) page faults: 0 Minor (reclaiming a frame) page faults: 204 Voluntary context switches: 1064 Involuntary context switches: 368 Swaps: 0 File system inputs: 0 File system outputs: 0 Socket messages sent: 0 Socket messages received: 0 Signals delivered: 0 Page size (bytes): 4096 Exit status: 0 Server: /usr/bin/time --verbose ./logger-server Command being timed: ./logger-server User time (seconds): 1.83 System time (seconds): 75.17 Percent of CPU this job got: 30% Elapsed (wall clock) time (h:mm:ss or m:ss): 4:08.72 Average shared text size (kbytes): 0 Average unshared data size (kbytes): 0 Average stack size (kbytes): 0 Average total size (kbytes): 0 Maximum resident set size (kbytes): 480 Average resident set size (kbytes): 0 Major (requiring I/O) page faults: 0 Minor (reclaiming a frame) page faults: 167 Voluntary context switches: 242 Involuntary context switches: 144 Swaps: 0 File system inputs: 0 File system outputs: 0 Socket messages sent: 0
Re: [systemd-devel] [RFC] [PATCH 0/3] journal: Add deferred log processing to reduce synchonous IO overhead
On Fri, Dec 13, 2013 at 03:45:36PM +0100, Lennart Poettering wrote: On Fri, 13.12.13 12:46, Karol Lewandowski (k.lewando...@samsung.com) wrote: Well, are you suggesting that the AF_UNIX/SOCK_DGRAM code actually hands off the timeslice to the other side as soon as it queued something in? If by other side you mean receiving one, then no - qnx seems to do that, but it isn't the case here. What I'm trying to say here is that kernel puts the process doing send(2) into sleep when (a) queue fills up and (b) fd is blocking one (otherwise we just get EAGAIN). That's expected, I presume. One of the problems I see, though, is that no matter how deep I make the queue (`max_dgram_qlen') I still see process sleeping on send() way earlier that configured queue depth would suggest. THat would be news to me. The kernel does context switches when the timeslice is over not earlier afaik, and you'll get that aynway... If you take a look at Results.sock_dgram from my previous mail you will find this for sock-client: Voluntary context switches: 577696 Involuntary context switches: 633 Please correct me if I'm wrong but I think that involuntary context switch happens when process' timeslice is over, where voluntary is caused by syscall, probably doing some form of IO (read/send/select/etc.) If I make sock-server sleep(5) just before poll and strace the client I can clearly see that it blocks on send(). After receiving side picks packet up, then the clients is able to sent another one. This what I think is happening, please let me know if this sounds like utter nonsense to you. :) Here's another option: extend journald to use kdbus as additional transport. This is something we want to do anyway since the kdbus transport will attach the metadata we need without race to each packet. Given that kdbus ultimately is just a way to write into an mmaped tmpfs that some other process owns this should not be much worse than the android logger in performance. I doubt it would help as other side would still be woken up on _every_ message, right? Well, it gets woken up if its waiting for that. But the kernel will only give CPU time to it when the senders timeslice is over... I'm either doing something terribly wrong or timeslice can end due to the call to send(2) and friends. There's very little difference to mmap... I mean, you need to tell the other side that it should look in the buffer, how do you want to do that otherwise? I don't. I just want the buffer to be huge enough to not cause client to block, effectively waiting for receiving side pick stuff up. Cheers, Karol ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 2/3] journal: Try to pass log via /dev/shm to avoid waking up journal
Enabled by default for test purposes. --- src/journal/journal-send.c | 39 +++ 1 file changed, 39 insertions(+) diff --git a/src/journal/journal-send.c b/src/journal/journal-send.c index 281e154..9c4acc2 100644 --- a/src/journal/journal-send.c +++ b/src/journal/journal-send.c @@ -196,6 +196,41 @@ finish: return r; } +static int write_snippet(const struct iovec *iov, int n) { + +static const char dir[] = /dev/shm; +char path[PATH_MAX]; /* FIXME */ +struct timespec ts; +struct stat st; +int fd; +int r; + +if (stat(dir, st) 0 || !S_ISDIR(st.st_mode)) +return -1; + +do { +clock_gettime(CLOCK_REALTIME, ts); +snprintf(path, PATH_MAX, %s/journal-snippet.%lu.%lu, dir, ts.tv_sec, ts.tv_nsec); /* FIXME */ + +fd = open(path, O_WRONLY | O_CREAT | O_EXCL, S_IRUSR | S_IWUSR); + +} while (fd 0 errno == EEXIST); + +if (fd 0) +return -errno; + +r = writev(fd, iov, n); +if (r 0) +r = -errno; +else +r = 1; + +close_nointr_nofail(fd); + +return r; +} + + _public_ int sd_journal_sendv(const struct iovec *iov, int n) { PROTECT_ERRNO; int fd, buffer_fd; @@ -289,6 +324,10 @@ _public_ int sd_journal_sendv(const struct iovec *iov, int n) { IOVEC_SET_STRING(w[j++], \n); } +/* Try to drop journald snippet for delayed processing */ +if (write_snippet(w, j) 0) +return 0; + fd = journal_fd(); if (_unlikely_(fd 0)) return fd; -- 1.8.4.rc3 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 3/3] journal-feeder: trivial test program to flood journald
This is not a systemd patch but simple test program I have used for measurements. --- journal-feeder.c | 64 1 file changed, 64 insertions(+) create mode 100644 journal-feeder.c diff --git a/journal-feeder.c b/journal-feeder.c new file mode 100644 index 000..df7a7e4 --- /dev/null +++ b/journal-feeder.c @@ -0,0 +1,64 @@ +/* journal-feeder M N: send N-sized messages M-times a second + * + * Compile: $CC -o journal-feeder journal-feeder.c $(pkg-config --cflags --libs libsystemd-journal) + */ +#include stdio.h +#include stdlib.h +#include string.h +#include assert.h +#include errno.h +#include stdint.h + +#include sys/types.h +#include time.h +#include systemd/sd-journal.h + +#define NELEMS(arr) (sizeof(arr)/sizeof(arr[0])) + +#define SEC_TO_MSEC(n) (1000*(n)) +#define SEC_TO_USEC(n) (1000*(SEC_TO_MSEC(n))) +#define SEC_TO_NSEC(n) (1000*(SEC_TO_USEC(n))) + + +int main(int argc, char *argv[]) +{ +int m_per_sec = 0; +int sz = 0; +int n; +char message[4096] = Oh my, this should make some sense to reader on the other side...; +struct timespec tv; + + +if (argc != 3) { +fprintf(stderr, usage: %s MSG_PER_SEC MSG_SIZE\n, argv[0]); +exit(1); +} +m_per_sec = atoi(argv[1]); +sz = atoi(argv[2]); + +assert(m_per_sec); +assert(sz 0 sz NELEMS(message)); + +n = strlen(message); +if (sz n) +memset(message + n, 'X', sz - n); +message[sz] = 0; + +tv.tv_sec = 0; +tv.tv_nsec = SEC_TO_NSEC(1) / m_per_sec; + +if (tv.tv_nsec == SEC_TO_NSEC(1)) +tv.tv_nsec -= 1; + +printf(will wait %d ns\n, tv.tv_nsec); + +for (;;) { +sd_journal_send(MESSAGE=%s, message, PRIORITY=%i, LOG_INFO, NULL); + +while (nanosleep(tv, NULL) == -1 errno == -EAGAIN) +; + +} + +return 0; +} -- 1.8.4.rc3 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [RFC] [PATCH 0/3] journal: Add deferred log processing to reduce synchonous IO overhead
Folks, We are trying to apply journald as default (and only) logging system for our ARM-based handhelds. There is one major problem, though - performance. We have found that journald can consume considerable amount of CPU time under moderate load (10-20% of CPU per, for say 100-300msg/s). We have profiled journald and there are few places that could benefit from _some_ optimizations but I have come to conclusion it isn't journald code that is real bottleneck - it's, or seems to be, synchronous nature of send()/recv(). In default configuration each send() can cause context-switch which is especially costly on ARM. This is why I decided to try to make logging completely async and see if/what changes. Following patchset (based on top of f20c84c15f3) implements pickup logic in journald [patch 1] and uncoditionally enables it in client api [patch 2]. In short - instead of sending DGRAMs I do drop files to /dev/shm which are then picked up by journald at specified intervals. (Precisely, I have (ab)used mechanism used to pass fd via DGRAM when message is too big.) Patch 3 contains standalone test program, which floods journald with specified number of messages per second. On my development (x86) machine I made following test with good old time(1): # send 2000 messages (each of size 100 bytes) to journal per second # for ~5mins ./journal-feeder 2000 100 sleep $((60*5 - 10)) kill $! Unpatched systemd: == /usr/bin/time --verbose ./systemd-journald sleep $((60*5)) kill $(pidof systemd-journald) User time (seconds): 16.87 System time (seconds): 39.38 Percent of CPU this job got: 18% Elapsed (wall clock) time (h:mm:ss or m:ss): 5:00.06 Major (requiring I/O) page faults: 30 Minor (reclaiming a frame) page faults: 1061740 Voluntary context switches: 496757 Involuntary context switches: 497 File system inputs: 0 File system outputs: 50592 Deferred pickup every 5 secs (logs passed via /dev/shm/FILE rather than DGRAMs): /usr/bin/time --verbose ./systemd-journald sleep $((60*5)) kill $(pidof systemd-journald) User time (seconds): 8.27 System time (seconds): 8.40 Percent of CPU this job got: 5% Elapsed (wall clock) time (h:mm:ss or m:ss): 5:00.36 Major (requiring I/O) page faults: 180 Minor (reclaiming a frame) page faults: 38976 Voluntary context switches: 157 Involuntary context switches: 1933 File system inputs: 8 File system outputs: 307312 Deferred pickup every 1 sec (logs passed via /dev/shm/FILE rather than DGRAMs): === /usr/bin/time --verbose ./systemd-journald sleep $((60*5)) kill $(pidof systemd-journald) User time (seconds): 9.54 System time (seconds): 9.56 Percent of CPU this job got: 6% Elapsed (wall clock) time (h:mm:ss or m:ss): 5:00.15 Major (requiring I/O) page faults: 180 Minor (reclaiming a frame) page faults: 39034 Voluntary context switches: 382 Involuntary context switches: 2359 File system inputs: 0 File system outputs: 307768 This is quite naive test as a far more changes when we pass fd instead of DGRAM (ucred is NULL, which means we won't gather information from proc at all). However, we verified that cost of this is rather minor and not that important in this test. Of course, I'm not proposing to include it as is (uncoditionally enable it for all messages) but I would just like to hear your opinion about this idea. If I read these numbers correctly there seems to be considerable benefit in terms of CPU time. Something like that wouldn't be useful for all messages, but I think that gazzilions of debug messages that our apps produce could be send in non-synchronous and low-overhead manner. RFC. Thanks in advance! Karol Lewandowski (3): journald: Add deferred log processing logic journal: Try to pass log via /dev/shm to avoid waking up journal journal-feeder: trivial test program to flood journald journal-feeder.c | 64 +++ src/journal/journal-send.c | 39 ++ src/journal/journald-gperf.gperf | 2 + src/journal/journald-server.c| 107 +++ src/journal/journald-server.h| 5 ++ src/journal/journald.conf| 2 + 6 files changed, 219 insertions(+) create mode 100644 journal-feeder.c -- 1.8.4.rc3 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [RFC] [PATCH 0/3] journal: Add deferred log processing to reduce synchonous IO overhead
On Thu, Dec 12, 2013 at 03:51:05PM +0100, Lennart Poettering wrote: On Thu, 12.12.13 14:12, Karol Lewandowski (k.lewando...@samsung.com) wrote: (OP here from private email.) Folks, We are trying to apply journald as default (and only) logging system for our ARM-based handhelds. There is one major problem, though - performance. We have found that journald can consume considerable amount of CPU time under moderate load (10-20% of CPU per, for say 100-300msg/s). We have profiled journald and there are few places that could benefit from _some_ optimizations but I have come to conclusion it isn't journald code that is real bottleneck - it's, or seems to be, synchronous nature of send()/recv(). In default configuration each send() can cause context-switch which is especially costly on ARM. Hmm, but this wouldn't be any different from old syslog, right? I mean, old syslog also uses sendv() and recv() on AF_UNIX SOCK_DGRAM sockets... That's true, however, we aren't migrating from syslog, but from Android Logger. Android provides /dev/log_{main,radio,...} device nodes which are simple in-kernel circular buffers apps can write to at any time. This was invented when /dev/kmsg wasn't writable and serves more-or-less same purpose. From 2.6.29 it's in mailine kernel under drivers/staging/android/logger.c Note that on Linux the number of datagrams you can queue in an AF_UNIX/SOCK_DGRAM socket is very low (15). THis is controlled via /proc/sys/net/unix/max_dgram_qlen. When this limit is hit the client needs to wait for the server to finish processing. Unfortunately there's only that system-global way to increase the qlen. For a long time it has been on our wishlist to make this a per-socket tunable with a new sockopt() that behaves similar to SO_SNDBUF and friends... We have played this tunable and it didn't bring considerable/measurable improvements, unfortunately. This is why I decided to try to make logging completely async and see if/what changes. Well, if AF_UNIX is slow, then we really should try to fix that in the kernel instead of bypassing it in userspace... Whether we rely on the socket buffer of the AF_UNIX socket or put together our own buffer in /dev/shm shouldn't be much of a difference if they both have the same size and can take the same number of entries... I do agree. Truth is I have looked at linux/net/ code once but I didn't grok it well. I guess it's the time to take a second look. ;) Cheers, Karol ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 0/7] kdbus related libsystemd-bus patches
On 11/15/2013 07:32 PM, Daniel Mack wrote: Hi! What's missing and still under development is a service that provides org.freedesktop.DBus on kdbus, and which translates the native interface to calls in libsystemd-bus. This almost works now, but there are some missing pieces in libsystemd-bus which need to be done first. Is your development version of org.freedesktop.DBus daeamon available somewhere? We would like to take a look if we can make our kdbus-glib bindings[1] work with it (and if not - make it work together). [1] https://www.mail-archive.com/systemd-devel@lists.freedesktop.org/msg14385.html Cheers Karol ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 0/4] [RFC v1] gdbus: Preliminary kdbus-support patches
On 11/21/2013 08:28 PM, Colin Walters wrote: On Thu, 2013-11-21 at 16:35 +0100, Karol Lewandowski wrote: Truth is that gio guys already merged tizen's glib-kdbus modifications into their own devel branch without us even knowing, not to mention proposing it. If you're referring to https://git.gnome.org/browse/glib/log/?h=tizen/kdbus-dev I think Ryan just wanted to put the patches in a place where he could (more easily) see them and work on them. What I would like to avoid is us and Ryan (and other guys) working on the same piece of code, effectively forking glib multiple times for kdbus. We are quite open about our work - it just took us a while to bring changes to the state where these are more-or-less ready for public consumption. :) The devel branch of glib is master, and anything landing there needs to go through peer review. wip/ and other branches are more freeform. Thanks for this explanation, I wasn't aware of all these details. Cheers, Karol ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 0/4] [RFC v1] gdbus: Preliminary kdbus-support patches
On 11/22/2013 01:32 PM, Matthias Clasen wrote: On Fri, Nov 22, 2013 at 7:03 AM, Karol Lewandowski k.lewando...@samsung.com mailto:k.lewando...@samsung.com wrote: On 11/21/2013 08:28 PM, Colin Walters wrote: On Thu, 2013-11-21 at 16:35 +0100, Karol Lewandowski wrote: Truth is that gio guys already merged tizen's glib-kdbus modifications into their own devel branch without us even knowing, not to mention proposing it. If you're referring to https://git.gnome.org/browse/glib/log/?h=tizen/kdbus-dev I think Ryan just wanted to put the patches in a place where he could (more easily) see them and work on them. What I would like to avoid is us and Ryan (and other guys) working on the same piece of code, effectively forking glib multiple times for kdbus. We are quite open about our work - it just took us a while to bring changes to the state where these are more-or-less ready for public consumption. :) I pointed Ryan at the branch when I learned about it, and he just imported it to have a first look at it. I don't think there was any intention to 'work' on this code right away. Sorry if this startled you. No problem here - we are quite happy that upstream is interested in this. And thanks for pushing out this cleaned up version, I appreciate it! Thanks for bringing this into the light and, consequently - motivating us to clean things up. Cheers, Karol ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 4/4] gdbus: Add basic kdbus tests
--- configure.ac | 1 + gio/tests/Makefile.am | 13 +++ gio/tests/kdbus-test/README | 108 gio/tests/kdbus-test/gdbus-example-kdbus-client.c | 51 ++ gio/tests/kdbus-test/gdbus-example-kdbus-server.c | 117 ++ 5 files changed, 290 insertions(+) create mode 100644 gio/tests/kdbus-test/README create mode 100644 gio/tests/kdbus-test/gdbus-example-kdbus-client.c create mode 100644 gio/tests/kdbus-test/gdbus-example-kdbus-server.c diff --git a/configure.ac b/configure.ac index 413c9f2..c3349c4 100644 --- a/configure.ac +++ b/configure.ac @@ -264,6 +264,7 @@ AS_IF([test x$enable_gc_friendly = xyes], [ ], [ AC_MSG_RESULT([no]) ]) AC_MSG_CHECKING([kdbus transport]) +AM_CONDITIONAL(KDBUS_TRANSPORT, [test x$enable_kdbus_transport = xyes]) AS_IF([test x$enable_kdbus_transport = xyes], [ AC_DEFINE(KDBUS_TRANSPORT, 1, [Define if kdbus transport is enabled]) AC_MSG_RESULT([yes]) diff --git a/gio/tests/Makefile.am b/gio/tests/Makefile.am index bec70d7..8d692bb 100644 --- a/gio/tests/Makefile.am +++ b/gio/tests/Makefile.am @@ -470,6 +470,19 @@ gdbus_serialization_SOURCES = \ gdbus-tests.c endif +if KDBUS_TRANSPORT +test_programs += gdbus-example-kdbus-server +test_programs += gdbus-example-kdbus-client + +gdbus_example_kdbus_server_CFLAGS = $(AM_CFLAGS) $(DBUS1_CFLAGS) +gdbus_example_kdbus_server_LDADD = $(LDADD) $(DBUS1_LIBS) +gdbus_example_kdbus_server_SOURCES = kdbus-test/gdbus-example-kdbus-server.c + +gdbus_example_kdbus_client_CFLAGS = $(AM_CFLAGS) $(DBUS1_CFLAGS) +gdbus_example_kdbus_client_LDADD = $(LDADD) $(DBUS1_LIBS) +gdbus_example_kdbus_client_SOURCES = kdbus-test/gdbus-example-kdbus-client.c +endif + # - # The resources test is a bit more complicated... diff --git a/gio/tests/kdbus-test/README b/gio/tests/kdbus-test/README new file mode 100644 index 000..6f44f0b --- /dev/null +++ b/gio/tests/kdbus-test/README @@ -0,0 +1,108 @@ +Description +=== + +In order to perform tests of modified libgio library (with kdbus +transport) we have created two programs: gdbus-example-kdbus-server +and gdbus-example-kdbus-client. The first of them acts as a server, +registers itself on the bus under well-known name and waits for calls +to its objects and methods. The second one (client) makes calls and +records periods of time between moment of preparing of a call to the +moment of receiving and parsing an answer. The measurement is made by +performing one thousand of calls and computing a sum of duration of +every call (for different sizes of message payload: 4B, 8B, 4kB, 8kB, +16kB, 64kB and 512kB). The client program returns total time of +performed calls after successful execution. + + +Requirements + + +* kdbus kernel module, +* modified version of glib (with kdbus transport), +* modified version of dbus-daemon (with kdbus transport), + + +KDBUS += + +Simple install procedure: + + % git clone https://github.com/gregkh/kdbus.git# download kdbus source + % cd kdbus # change to kdbus directory + % make # build kdbus module + % insmod kdbus.ko # load kdbus kernel module + + +GLIB (modified version) +=== + +Simple install procedure: + + + % cd glib # change to glib directory + % ./configure --enable-kdbus-transport# run the `configure' script with kdbus support + % make# build GLIB + + +DBUS-DAEMON (modified version) +== + +Modified version of dbus-daemon (from tizen.org) is required to run +kdbus transport in libgio. + +Solution, which we propose, includes modifications both in client +libraries (libgio, libdbus) and dbus daemon. Daemon is registered on +kdbus bus as a client under org.freedesktop.DBus name. Only +messages addressed to this interface are handled by daemon. Messages +such as RequestName or AddMatch are translated by daemon to respective +ioctl() calls, which send them through kdbus on behalf of original +client process. The rest of messages would are going straight from +origin to destination through kdbus. + +Simple install procedure: + +* download modified version of dbus from tizen.org repository + (kdbus-dev branch): + + git clone git://review.tizen.org/platform/upstream/dbus.git -b kdbus-dev + +* change to dbus directory and build dbus-daemon with kdbus support: + + ./configure --enable-kdbus-transport + make make install + +* start of custom dbus-daemon supporting kdbus transport: + + ./dbus-daemon --nofork --address=kdbus: --config-file=/etc/dbus-1/session.conf + + +TESTS += + +* Build test binaries: + + cd gio/tests + make + +* Set variable to use
[systemd-devel] [PATCH 1/4] gdbus: Import kdbus interface header
kdbus.h taken from git://github.com/gregkh/kdbus, commit cc5b49c0d. --- gio/kdbus.h | 436 1 file changed, 436 insertions(+) create mode 100644 gio/kdbus.h diff --git a/gio/kdbus.h b/gio/kdbus.h new file mode 100644 index 000..06c2c24 --- /dev/null +++ b/gio/kdbus.h @@ -0,0 +1,436 @@ +/* + * Copyright (C) 2013 Kay Sievers + * Copyright (C) 2013 Greg Kroah-Hartman gre...@linuxfoundation.org + * Copyright (C) 2013 Linux Foundation + * Copyright (C) 2013 Lennart Poettering + * Copyright (C) 2013 Daniel Mack dan...@zonque.org + * + * kdbus is free software; you can redistribute it and/or modify it under + * the terms of the GNU Lesser General Public License as published by the + * Free Software Foundation; either version 2.1 of the License, or (at + * your option) any later version. + */ + +#ifndef _KDBUS_H_ +#define _KDBUS_H_ + +#ifndef __KERNEL__ +#include sys/ioctl.h +#include sys/types.h +#include linux/types.h +#endif + +#define KDBUS_IOC_MAGIC0x95 +#define KDBUS_SRC_ID_KERNEL(0) +#define KDBUS_DST_ID_WELL_KNOWN_NAME (0) +#define KDBUS_MATCH_SRC_ID_ANY (~0ULL) +#define KDBUS_DST_ID_BROADCAST (~0ULL) + +/* Common first elements in a structure which are used to iterate over + * a list of elements. */ +#define KDBUS_PART_HEADER \ + struct {\ + __u64 size; \ + __u64 type; \ + } + +/* Message sent from kernel to userspace, when the owner or starter of + * a well-known name changes */ +struct kdbus_manager_msg_name_change { + __u64 old_id; + __u64 new_id; + __u64 flags;/* 0 or (possibly?) KDBUS_NAME_IN_QUEUE */ + char name[0]; +}; + +struct kdbus_manager_msg_id_change { + __u64 id; + __u64 flags;/* The kernel flags field from KDBUS_HELLO */ +}; + +struct kdbus_creds { + __u64 uid; + __u64 gid; + __u64 pid; + __u64 tid; + + /* The starttime of the process PID. This is useful to detect + PID overruns from the client side. i.e. if you use the PID to + look something up in /proc/$PID/ you can afterwards check the + starttime field of it to ensure you didn't run into a PID + ovretun. */ + __u64 starttime; +}; + +struct kdbus_audit { + __u64 sessionid; + __u64 loginuid; +}; + +struct kdbus_timestamp { + __u64 monotonic_ns; + __u64 realtime_ns; +}; + +struct kdbus_vec { + __u64 size; + union { + __u64 address; + __u64 offset; + }; +}; + +struct kdbus_memfd { + __u64 size; + int fd; + __u32 __pad; +}; + +/* Message Item Types */ +enum { + _KDBUS_MSG_NULL, + + /* Filled in by userspace */ + KDBUS_MSG_PAYLOAD_VEC, /* .data_vec, reference to memory area */ + KDBUS_MSG_PAYLOAD_OFF, /* .data_vec, reference to memory area */ + KDBUS_MSG_PAYLOAD_MEMFD,/* file descriptor of a special data file */ + KDBUS_MSG_FDS, /* .data_fds of file descriptors */ + KDBUS_MSG_BLOOM,/* for broadcasts, carries bloom filter blob in .data */ + KDBUS_MSG_DST_NAME, /* destination's well-known name, in .str */ + KDBUS_MSG_PRIORITY, /* queue priority for message */ + + /* Filled in by kernelspace */ + KDBUS_MSG_SRC_NAMES = 0x400,/* NUL separated string list with well-known names of source */ + KDBUS_MSG_TIMESTAMP,/* .timestamp */ + KDBUS_MSG_SRC_CREDS,/* .creds */ + KDBUS_MSG_SRC_PID_COMM, /* optional, in .str */ + KDBUS_MSG_SRC_TID_COMM, /* optional, in .str */ + KDBUS_MSG_SRC_EXE, /* optional, in .str */ + KDBUS_MSG_SRC_CMDLINE, /* optional, in .str (a chain of NUL str) */ + KDBUS_MSG_SRC_CGROUP, /* optional, in .str */ + KDBUS_MSG_SRC_CAPS, /* caps data blob, in .data */ + KDBUS_MSG_SRC_SECLABEL, /* NUL terminated string, in .str */ + KDBUS_MSG_SRC_AUDIT,/* .audit */ + + /* Special messages from kernel, consisting of one and only one of these data blocks */ + KDBUS_MSG_NAME_ADD = 0x800,/* .name_change */ + KDBUS_MSG_NAME_REMOVE, /* .name_change */ + KDBUS_MSG_NAME_CHANGE, /* .name_change */ + KDBUS_MSG_ID_ADD, /* .id_change */ + KDBUS_MSG_ID_REMOVE,/* .id_change */ + KDBUS_MSG_REPLY_TIMEOUT,/* empty, but .reply_cookie in .kdbus_msg is filled in */ + KDBUS_MSG_REPLY_DEAD, /* dito */ +}; + +/** + * struct kdbus_item - chain of data blocks + * + * size: overall data record size + * type:
[systemd-devel] [PATCH 0/4] [RFC v1] gdbus: Preliminary kdbus-support patches
[ Cced systemd-devel@ and dev@tizen mailing lists in case someone there would be interested too. ] Folks, We have recently started experimenting with possibilities of adding kdbus-support to glib's gio. Following patchset is result of our work. Please note this is cleanup of modifications available from Tizen repositories (please see notes for details), rebased on top of glib's master branch - 6f7d8f6294 (gbacktrace: Print out gdb exec errors correctly). What we would like to accomplish by this RFC is to gather feedback if our approach for glib modifications is sound for you (we do not know glib code that well). In short: - This patchset adds ability for glib programs to connect to kdbus busses via DBUS_SESSION_BUS_ADDRESS=(kernel|kdbus):/dev/kdbus/0-kdbus/bus - Library modifications are not all, it's required to have service handling org.freedesktop.DBus requests in userspace. Currently we use modified[3] dbus-daemon to for that purpose. - Basic functionality works - sending/receiving messages, signals, name registration, etc. - Last patch contains basic tests accompanied with README, please take a look there too. We will be happy to hear any and all of your comments. Thanks! Notes: == This code originates from Tizen[1], and was imported[2] by Ryan Lortie into glib's tizen/kdbus-dev branch. [1] https://review.tizen.org/gerrit/gitweb?p=platform%2Fupstream%2Fglib.git;a=summary git://review.tizen.org/platform/upstream/glib kdbus-dev [2] https://git.gnome.org/browse/glib/log/?h=tizen/kdbus-dev We are the same people that did that work, precisely Lukasz Skalski and Michal Eljasiewicz wrote all the code, I just gave it finishing touches. We think this patchset could replace glibs tizen/kdbus-dev iff you think that it's worth to keep kdbus support code in main repo at all. ;) (At this point in time, that is!) Moreover, we would be more than happy to work directly on that branch, if you find it feasible. kdbus-enabled dbus-daemon is available here: [3] git://review.tizen.org/platform/upstream/dbus kdbus-dev We are aware that in future it will be probably systemd role to provide it (I've seen that Daniel Mack is already working on it). Karol Lewandowski (4): gdbus: Import kdbus interface header gdbus: Add preliminary implementation of kdbus support gdbus: Integrate kdbus into GDBus core gdbus: Add basic kdbus tests configure.ac | 11 + gio/Makefile.am |4 + gio/gdbusaddress.c| 80 +- gio/gdbusconnection.c | 20 +- gio/gdbusprivate.c| 211 +++- gio/gdbusprivate.h|8 +- gio/giotypes.h| 33 + gio/gkdbus.c | 1112 + gio/gkdbus.h | 113 +++ gio/gkdbusconnection.c| 196 gio/gkdbusconnection.h| 91 ++ gio/kdbus.h | 436 gio/tests/Makefile.am | 13 + gio/tests/kdbus-test/README | 108 ++ gio/tests/kdbus-test/gdbus-example-kdbus-client.c | 51 + gio/tests/kdbus-test/gdbus-example-kdbus-server.c | 117 +++ 16 files changed, 2572 insertions(+), 32 deletions(-) create mode 100644 gio/gkdbus.c create mode 100644 gio/gkdbus.h create mode 100644 gio/gkdbusconnection.c create mode 100644 gio/gkdbusconnection.h create mode 100644 gio/kdbus.h create mode 100644 gio/tests/kdbus-test/README create mode 100644 gio/tests/kdbus-test/gdbus-example-kdbus-client.c create mode 100644 gio/tests/kdbus-test/gdbus-example-kdbus-server.c -- 1.8.4.rc3 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 3/4] gdbus: Integrate kdbus into GDBus core
This commit hooks kdbus into GDBus and provides --enable-kdbus-transport configure switch. --- configure.ac | 10 +++ gio/Makefile.am | 4 + gio/gdbusaddress.c| 80 +++ gio/gdbusconnection.c | 20 - gio/gdbusprivate.c| 211 ++ gio/gdbusprivate.h| 8 +- gio/giotypes.h| 33 7 files changed, 334 insertions(+), 32 deletions(-) diff --git a/configure.ac b/configure.ac index deacdc1..413c9f2 100644 --- a/configure.ac +++ b/configure.ac @@ -242,6 +242,10 @@ AC_ARG_ENABLE(gc_friendly, [AS_HELP_STRING([--enable-gc-friendly], [turn on garbage collector friendliness [default=no]])],, [enable_gc_friendly=no]) +AC_ARG_ENABLE(kdbus_transport, + [AS_HELP_STRING([--enable-kdbus-transport], + [enable kdbus transport [default=no]])],, + [enable_kdbus_transport=no]) AC_ARG_ENABLE(mem_pools, [AS_HELP_STRING([--disable-mem-pools], [disable all glib memory pools])],, @@ -259,6 +263,12 @@ AS_IF([test x$enable_gc_friendly = xyes], [ AC_MSG_RESULT([yes]) ], [ AC_MSG_RESULT([no]) ]) +AC_MSG_CHECKING([kdbus transport]) +AS_IF([test x$enable_kdbus_transport = xyes], [ + AC_DEFINE(KDBUS_TRANSPORT, 1, [Define if kdbus transport is enabled]) + AC_MSG_RESULT([yes]) +], [ AC_MSG_RESULT([no]) ]) + AC_MSG_CHECKING([whether to disable memory pools]) AS_IF([test x$disable_mem_pools = xno], [ AC_MSG_RESULT([no]) diff --git a/gio/Makefile.am b/gio/Makefile.am index 5b6cda1..c060ef2 100644 --- a/gio/Makefile.am +++ b/gio/Makefile.am @@ -394,6 +394,8 @@ libgio_2_0_la_SOURCES = \ giostream.c \ gioprivate.h\ giowin32-priv.h \ + gkdbus.c\ + gkdbusconnection.c \ gloadableicon.c \ gmount.c\ gmemoryinputstream.c\ @@ -569,6 +571,8 @@ gio_headers = \ giomodule.h \ gioscheduler.h \ giostream.h \ + gkdbus.h\ + gkdbusconnection.h \ gloadableicon.h \ gmount.h\ gmemoryinputstream.h\ diff --git a/gio/gdbusaddress.c b/gio/gdbusaddress.c index b5143cf..bed0bbe 100644 --- a/gio/gdbusaddress.c +++ b/gio/gdbusaddress.c @@ -1,6 +1,7 @@ /* GDBus - GLib D-Bus Library * * Copyright (C) 2008-2010 Red Hat, Inc. + * Copyright (C) 2013 Samsung Electronics * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -17,7 +18,9 @@ * Free Software Foundation, Inc., 59 Temple Place, Suite 330, * Boston, MA 02111-1307, USA. * - * Author: David Zeuthen dav...@redhat.com + * Author: David Zeuthendav...@redhat.com + * Author: Lukasz Skalski l.skal...@partner.samsung.com + * Author: Michal Eljasiewicz m.eljasie...@samsung.com */ #include config.h @@ -44,6 +47,7 @@ #ifdef G_OS_UNIX #include gio/gunixsocketaddress.h +#include gio/gkdbusconnection.h #endif #ifdef G_OS_WIN32 @@ -360,6 +364,18 @@ is_valid_tcp (const gchar *address_entry, return ret; } +static int +g_dbus_is_supported_address_kdbus (const gchar *transport_name) +{ + int supported = 0; + +#if defined (G_OS_UNIX) (KDBUS_TRANSPORT) + supported = (g_strcmp0 (transport_name, kdbus) == 0) || (g_strcmp0 (transport_name, kernel) == 0); +#endif + + return supported; +} + /** * g_dbus_is_supported_address: * @string: A string. @@ -401,7 +417,8 @@ g_dbus_is_supported_address (const gchar *string, goto out; supported = FALSE; - if (g_strcmp0 (transport_name, unix) == 0) + if ((g_strcmp0 (transport_name, unix) == 0) + || g_dbus_is_supported_address_kdbus (transport_name)) supported = is_valid_unix (a[n], key_value_pairs, error); else if (g_strcmp0 (transport_name, tcp) == 0) supported = is_valid_tcp (a[n], key_value_pairs, error); @@ -553,7 +570,8 @@ g_dbus_address_connect (const gchar *address_entry, { } #ifdef G_OS_UNIX - else if (g_strcmp0 (transport_name, unix) == 0) + if ((g_strcmp0 (transport_name, unix) == 0) + || g_dbus_is_supported_address_kdbus (transport_name)) { const gchar *path; const gchar *abstract; @@ -664,21 +682,49 @@ g_dbus_address_connect (const gchar *address_entry, if (connectable != NULL) { - GSocketClient *client; - GSocketConnection *connection; - - g_assert (ret == NULL); - client = g_socket_client_new (); - connection = g_socket_client_connect (client, -connectable, -cancellable, -error); -
Re: [systemd-devel] [PATCH 1/2] RFC: journald: Allow to cache the cg_get_root_path
On 07/16/2013 03:26 AM, Lennart Poettering wrote: On Mon, 08.07.13 18:39, Karol Lewandowski (k.lewando...@samsung.com) wrote: According to my analysis /proc access is costly and it would be best to cache the result for later use. Difficulty comes from trying to keep cache up to date, though. We can't really cache this. This stuff is already racy, as by the time we read the attributes the process might already be gone. Agreed, this is why I have been naively thinking that caching wouldn't be that bad... However, sending all the required information in message itself is clearly the best solution to this problem. I think the best way to deal with the performance issue here is the stuff discussed here: https://bugzilla.redhat.com/show_bug.cgi?id=963620 i.e. just have the kernel augment our messages with the data we need, unconditionally. That way we fix both the race issue, and the performance issue, since the data is just there and we can make use of it without any further work. Thanks a lot for this link! Having this issue sorted out will allow me to take closer look into eliminating 64-bit divisions in message receive path. ARM doesn't support divide operation so every div[1] is being done purely in software. [1] Every div/% by non-constant value and every div/% by 64-bit constant results in call to (slow) __aeabi_div*. Thanks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [RFC PATCH] condition: add ConditionFileContains=
On Fri, Jul 12, 2013 at 09:19:58PM +0200, Lennart Poettering wrote: On Fri, 12.07.13 20:42, Karol Lewandowski (k.lewando...@samsung.com) wrote: ... - software raid (md) status - /proc/mdstat Not sure what this is really doing... /etc/init.d/hdparm seems to be bailing out if md-raid is not fully operational and /etc/init.d/halt doesn't pass -h to final halt(8) if md is in active state. Every such case could be handled by generic built-in grep instead of dozen of flags like ConditionCPUFeature=, ConditionMDStatus=, ... I am pretty sure we cover most of these cases with some other way too. I mean, I am generally willing to add this, but if there's no strict need for it, I'd avoid it. Let's avoid it for now as our use case can be solved by udev rule (as Kay suggested). I'll take closer into Tizen boot sequence next monday to see if there are other cases where feature like this might come useful (hopefully I won't find any). Thanks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [RFC PATCH] condition: add ConditionFileContains=
On 07/12/2013 03:40 PM, Kay Sievers wrote: On Fri, Jul 12, 2013 at 3:31 PM, Lennart Poettering lenn...@poettering.net wrote: ConditionFileContains=/sys/module/sn/parameters/enabled:1 To make this clear: I am not keen on adding this. I can see the usefulness, and the thing is still simple enough so that it would be OK to add this, but if you guys don't actually need that I'd prefer to keep our codebase smaller... We can workaround this one way or another - be it shell script, udev rule or, in some cases, generator. Patch that started this discussion is yet another option. I have looked into /etc/init.d on my Debian system and found following examples of 'grepping' files used to decide if given service should be started or not: - kernel option specified - /proc/cmdline - filesystem available - /proc/filesystems - file contains non commented out lines - /etc/exports - cpu features available - /proc/cpuinfo - software raid (md) status - /proc/mdstat - ... Every such case could be handled by generic built-in grep instead of dozen of flags like ConditionCPUFeature=, ConditionMDStatus=, ... Having said that I fully understand that the line has to be drawn somewhere. [1] Such condition should probably support globs so we would end up with something like ConditionFileContentsGlob= Just a note: The above example should be triggered by a udev rule anyway. Agreed. Thanks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [RFC PATCH] condition: add ConditionFileContains=
Add ability to test if given file contains specified value. File and expected value are given as one argument separated by colon (:), i.e. ConditionFileContains=/sys/module/sn/parameters/enabled:1 --- As above example suggests we use it to conditionally start service based on kernel module parameter value. This can be (ab)used for other/regular files too. RFC Thanks! --- man/systemd.unit.xml |8 src/core/condition.c | 34 + src/core/condition.h |1 + src/core/load-fragment-gperf.gperf.m4 |1 + 4 files changed, 44 insertions(+) diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml index 4f0bd64..d26bd23 100644 --- a/man/systemd.unit.xml +++ b/man/systemd.unit.xml @@ -841,6 +841,7 @@ termvarnameConditionDirectoryNotEmpty=/varname/term termvarnameConditionFileNotEmpty=/varname/term termvarnameConditionFileIsExecutable=/varname/term + termvarnameConditionFileContains=/varname/term termvarnameConditionKernelCommandLine=/varname/term termvarnameConditionVirtualization=/varname/term termvarnameConditionSecurity=/varname/term @@ -932,6 +933,13 @@ exists, is a regular file and marked executable./para +paravarnameConditionFileContains=/varname +may be used to check whether a given +file contains specified value. The +argument consists of file path to be +tested, separator (:), and the +expected value./para + paraSimilar, varnameConditionKernelCommandLine=/varname may be used to check whether a diff --git a/src/core/condition.c b/src/core/condition.c index 427aa08..7c62d11 100644 --- a/src/core/condition.c +++ b/src/core/condition.c @@ -124,6 +124,36 @@ static bool test_kernel_command_line(const char *parameter) { return found; } +static bool test_file_contains(const char *parameter) { +int r; +char *p; +_cleanup_free_ char *path = NULL; +_cleanup_free_ char *line = NULL; + +assert(parameter); + +path = strdup(parameter); +if (!path) { +log_oom(); +return false; +} + +p = strchr(path, ':'); +if (!p) +return false; + +*(p++) = '\0'; + +r = read_one_line_file(path, line); + +if (r 0) { +log_warning(Failed to read %s, ignoring: %s, path, strerror(-r)); +return false; +} + +return !!streq(line, p); +} + static bool test_virtualization(const char *parameter) { int b; Virtualization v; @@ -307,6 +337,9 @@ bool condition_test(Condition *c) { return (S_ISREG(st.st_mode) (st.st_mode 0111)) == !c-negate; } +case CONDITION_FILE_CONTAINS: +return test_file_contains(c-parameter) == !c-negate; + case CONDITION_KERNEL_COMMAND_LINE: return test_kernel_command_line(c-parameter) == !c-negate; @@ -392,6 +425,7 @@ static const char* const condition_type_table[_CONDITION_TYPE_MAX] = { [CONDITION_DIRECTORY_NOT_EMPTY] = ConditionDirectoryNotEmpty, [CONDITION_FILE_NOT_EMPTY] = ConditionFileNotEmpty, [CONDITION_FILE_IS_EXECUTABLE] = ConditionFileIsExecutable, +[CONDITION_FILE_CONTAINS] = ConditionFileContains, [CONDITION_KERNEL_COMMAND_LINE] = ConditionKernelCommandLine, [CONDITION_VIRTUALIZATION] = ConditionVirtualization, [CONDITION_SECURITY] = ConditionSecurity, diff --git a/src/core/condition.h b/src/core/condition.h index 50ed955..2bf26bd 100644 --- a/src/core/condition.h +++ b/src/core/condition.h @@ -35,6 +35,7 @@ typedef enum ConditionType { CONDITION_DIRECTORY_NOT_EMPTY, CONDITION_FILE_NOT_EMPTY, CONDITION_FILE_IS_EXECUTABLE, +CONDITION_FILE_CONTAINS, CONDITION_KERNEL_COMMAND_LINE, CONDITION_VIRTUALIZATION, CONDITION_SECURITY, diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4 index 2325d6a..310e1c5 100644 --- a/src/core/load-fragment-gperf.gperf.m4 +++ b/src/core/load-fragment-gperf.gperf.m4 @@ -134,6 +134,7 @@ Unit.ConditionPathIsReadWrite, config_parse_unit_condition_path, CONDITION_P Unit.ConditionDirectoryNotEmpty, config_parse_unit_condition_path, CONDITION_DIRECTORY_NOT_EMPTY, 0 Unit.ConditionFileNotEmpty,
Re: [systemd-devel] [PATCH 1/2] RFC: journald: Allow to cache the cg_get_root_path
On 06/27/2013 06:30 PM, Holger Hans Peter Freyther wrote: From: Holger Hans Peter Freyther hol...@moiji-mobile.com Allow to cache the cg_get_root_path and introduce a new method cg_pid_get_path_shifted_with_root that can use the cached version instead of allocating a new string. My 2c, I have been thinking about something similar albeit a bit more generic. According to my analysis /proc access is costly and it would be best to cache the result for later use. Difficulty comes from trying to keep cache up to date, though. I've just started looking into connector's cn_proc which _seem_ to offer all the required functionality (fork, exec, exit notifications). I have to finish my prototype to verify if it's worth complications it brings. Thanks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] tmpfiles, man: Add xattr support to tmpfiles
2013/6/17 Lennart Poettering lenn...@poettering.net: On Mon, 17.06.13 16:27, Maciej Wereski (m.were...@partner.samsung.com) wrote: This patch makes it possible to set extended attributes on files created by tmpfiles. This can be especially used to set SMACK security labels on volatile files and directories. To keep backwards compatibility Argument field is used. If word starts with xattr=, then it is cut out from Argument and parsed. There may be many xattrs. Full format is: xattr=name=value If value contains spaces, then it must be surrounded by quotation marks. User can also put quotation mark in value by escaping it with backslash. I think adding this certainly makes sense, but I am not sure I like the syntax. Maybe it would be simpler to add an extra char for this (a or so?). That way creating a dir and applying an xattr would require two lines instead of one, but the stuff isn't atomic anyway. Admittedly adding a new a isn't particularly nice either, but I have no better idea than that... FWIW, I would like note that we might see similar problem if... when somebody will try to extend tmpfiles with ACLs. That would result in another type and another line. Maybe Type argument could be extended to contain one primary type and 0 to n optional subtypes where n-th subtype would parse n-th semicolon-terminated argument, i.e. fa /tmp/foobar ---- /dev/null ; security.SMACK=foo fxa /tmp/foobar ---- /dev/null ; security.SMACK=foo ; lmctl=rwx x - xattrs, a - acls I do agree this is a bit abusive... Cheers [ Lennart, sorry for two copies - first one didn't contain ML. ] ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] Make it possible to disable smack separately from xattr support
Additionally, compile out rule loading if feature is disabled. --- configure.ac | 26 ++ src/core/smack-setup.c | 10 ++ src/core/socket.c |4 ++-- 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 65186a4..14a90c5 100644 --- a/configure.ac +++ b/configure.ac @@ -445,6 +445,31 @@ AC_SUBST(XATTR_LIBS) AM_CONDITIONAL([HAVE_XATTR], [test x$have_xattr != xno]) # -- +AC_ARG_ENABLE([smack], AS_HELP_STRING([--disable-smack],[Disable optional SMACK support]), +[case ${enableval} in +yes) have_smack=yes ;; +no) have_smack=no ;; +*) AC_MSG_ERROR(bad value ${enableval} for --disable-smack) ;; +esac], +[have_smack=auto]) + +if test x${have_xattr} = xno; then +if test x${have_smack} = xyes; then +AC_MSG_ERROR(SMACK requires xattr support) +else +have_smack=no +fi +else +if test x${have_smack} = xauto; then +have_smack=yes +fi +fi + +if test x${have_smack} = xyes ; then +AC_DEFINE(HAVE_SMACK, 1, [Define if SMACK is available]) +fi + +# -- AC_ARG_ENABLE([gcrypt], AS_HELP_STRING([--disable-gcrypt],[Disable optional GCRYPT support]), [case ${enableval} in @@ -915,6 +940,7 @@ AC_MSG_RESULT([ AUDIT: ${have_audit} IMA: ${have_ima} SELinux: ${have_selinux} +SMACK: ${have_smack} XZ: ${have_xz} ACL: ${have_acl} XATTR: ${have_xattr} diff --git a/src/core/smack-setup.c b/src/core/smack-setup.c index 73eeb04..d67a84a 100644 --- a/src/core/smack-setup.c +++ b/src/core/smack-setup.c @@ -42,6 +42,8 @@ #define SMACK_CONFIG /etc/smack/accesses.d/ #define CIPSO_CONFIG /etc/smack/cipso/ +#ifdef HAVE_SMACK + static int write_rules(const char* dstpath, const char* srcdir) { _cleanup_fclose_ FILE *dst = NULL; _cleanup_closedir_ DIR *dir = NULL; @@ -111,8 +113,12 @@ static int write_rules(const char* dstpath, const char* srcdir) { return r; } +#endif int smack_setup(void) { + +#ifdef HAVE_SMACK + int r; r = write_rules(/sys/fs/smackfs/load2, SMACK_CONFIG); @@ -148,4 +154,8 @@ int smack_setup(void) { strerror(abs(r))); return 0; } + +#endif + +return 0; } diff --git a/src/core/socket.c b/src/core/socket.c index 1b08f0a..37ca228 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -788,7 +788,7 @@ static void socket_apply_socket_options(Socket *s, int fd) { if (setsockopt(fd, SOL_TCP, TCP_CONGESTION, s-tcp_congestion, strlen(s-tcp_congestion)+1) 0) log_warning_unit(UNIT(s)-id, TCP_CONGESTION failed: %m); -#ifdef HAVE_XATTR +#ifdef HAVE_SMACK if (s-smack_ip_in) if (fsetxattr(fd, security.SMACK64IPIN, s-smack_ip_in, strlen(s-smack_ip_in), 0) 0) log_error_unit(UNIT(s)-id, @@ -810,7 +810,7 @@ static void socket_apply_fifo_options(Socket *s, int fd) { log_warning_unit(UNIT(s)-id, F_SETPIPE_SZ: %m); -#ifdef HAVE_XATTR +#ifdef HAVE_SMACK if (s-smack) if (fsetxattr(fd, security.SMACK64, s-smack, strlen(s-smack), 0) 0) log_error_unit(UNIT(s)-id, -- 1.7.10.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] condition, man: Add support for ConditionSecurity=smack
Signed-off-by: Karol Lewandowski k.lewando...@samsung.com diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml index 49103da..256c813 100644 --- a/man/systemd.unit.xml +++ b/man/systemd.unit.xml @@ -984,8 +984,9 @@ may be used to check whether the given security module is enabled on the system. Currently the only recognized -values are varnameselinux/varname -and varnameapparmor/varname. +values are varnameselinux/varname, +varnameapparmor/varname and +varnamesmack/varname. The test may be negated by prepending an exclamation mark./para diff --git a/src/core/condition.c b/src/core/condition.c index 4aa5530..16cae6d 100644 --- a/src/core/condition.c +++ b/src/core/condition.c @@ -164,6 +164,8 @@ static bool test_security(const char *parameter) { #endif if (streq(parameter, apparmor)) return access(/sys/kernel/security/apparmor/, F_OK) == 0; + if (streq(parameter, smack)) + return access(/sys/fs/smackfs, F_OK) == 0; return false; } -- 1.7.10.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] condition, man: Add support for ConditionSecurity=smack
On 05/07/2013 01:32 PM, Lennart Poettering wrote: On Tue, 07.05.13 13:21, Karol Lewandowski (k.lewando...@samsung.com) wrote: Heya, Hmm, does that directory always exist? Or only if AppArmor is actually runtime enabled? /sys/fs/smackfs is only registered when smack lsm is actually enabled: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/security/smack/smackfs.c?id=e93072374112db9dc86635934ee761249be28370#n2179 I.e. this check should ideally only return true if SMACK is not only built into the kernel, but actually really enabled during runtime. That's what the SELinux check does and what the most useful semantics are. Ok, I see that libselinux will consider selinux to be disabled also when policy is not loaded: http://userspace.selinuxproject.org/trac/browser/libselinux/src/enabled.c#L12 I guess we could do something similar (inspect /proc/self/attr/current) but honestly, I don't think it's really needed. Rafał, could you correct me if I'm wrong? Cheers Signed-off-by: Karol Lewandowski k.lewando...@samsung.com diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml index 49103da..256c813 100644 --- a/man/systemd.unit.xml +++ b/man/systemd.unit.xml @@ -984,8 +984,9 @@ may be used to check whether the given security module is enabled on the system. Currently the only recognized -values are varnameselinux/varname -and varnameapparmor/varname. +values are varnameselinux/varname, +varnameapparmor/varname and +varnamesmack/varname. The test may be negated by prepending an exclamation mark./para diff --git a/src/core/condition.c b/src/core/condition.c index 4aa5530..16cae6d 100644 --- a/src/core/condition.c +++ b/src/core/condition.c @@ -164,6 +164,8 @@ static bool test_security(const char *parameter) { #endif if (streq(parameter, apparmor)) return access(/sys/kernel/security/apparmor/, F_OK) == 0; +if (streq(parameter, smack)) +return access(/sys/fs/smackfs, F_OK) == 0; return false; } Lennart ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel