Good design to expose debug info from kernel module

[guy keren]

take a look at this:

http://www.linuxfoundation.org/collaborate/workgroups/networking/generic_netlink_howto

(link got broken - place it all on a single line)

--guy

On 03/26/2015 11:36 PM, Elazar Leibovich wrote:
> Hi,
>
> I'm writing a kernel module, and I want to expose some debug
> information about it.
>
> The debug information is often of the form of request-response.
>
> For example:
>
> - Hey module, what's up with data at 0xffffe8ff0040c000?
> - Cached, populated two hours ago.
>
> - Hey module, please invalidate data at 0xffffe8ff0002cb00
> - Sure thing.
>
> - Hey module, please record all accesses to 0xffffe8ff0006bbf0.
> - OK, ask me again for stats-5
> ...
> - Hey module, what's in stats-5?
> - So far, 41 accesses by 22 users.
>
> Now, the question is, what is a good design to expose this information.
>
> I think that the most reasonable way to interact with userspace is
> through a debugfs file.
>
> The user would open the debugfs file in read+write mode, would write a
> request, and accept a response from it.
>
> As I see it, there are two fundamental problems needs to be solved:
>
> - Parsing the request from the client.
> - Writing the response in a recognizeable format.
>
> A simple solution I first came up with, is to use a ad-hoc
> request-response format. In my case, request and response are line
> delimited, request is a hex address, and response is a translated hex
> address.
>
> Here is the relevant snippet.
>
> struct pipe {
>         DECLARE_KFIFO(fifo, T, (1<<4));
>         wait_queue_head_t queue;
>         char buf[100];
>         int buflen;
>         char resp[100];
>         int resp_len;
> };
> static DEFINE_MUTEX(mutex);
> static int open(struct inode *inode, struct file *file)
> {
>      struct pipe *pipe;
>      if (!(file->f_mode & FMODE_READ) || !(file->f_mode & FMODE_READ)) {
>          pr_warn("must open with O_RDWR\n");
>          return -EINVAL;
>      }
>      mutex_lock(&mutex);
>      pipe = kzalloc(sizeof(*pipe), GFP_KERNEL);
>      INIT_KFIFO(pipe->fifo);
>      init_waitqueue_head(&pipe->queue);
>      file->private = pipe;
> }
>
> static int write(struct file *file, const char __user *ubuf, size_t
> count, loff_t *ppos)
> {
>      char *eol;
>      size_t n = min_t(size_t, count, sizeof(pipe->buf));
>      struct pipe *pipe = file->private_data;
>      if (copy_from_user(&pipe->buf[pipe->buflen], ubuf, n)
>          return -EFAULT;
>      eol = memchr(buf, '\n', n);
>      if (eol == NULL)
>          return count;
>      *eol = '\0';
>      // TODO: wait when queue full
>      if (!kfifo_in(&pipe->fifo, processLine(buf), 1)
>          return -EFAULT;
>      wake_up_interruptible(&pipe->queue);
>      memmove(&pipe->buf[0], &pipe->buf[n], pipe->buflen-n);
> }
>
> static int read(struct file *file, const char __user *ubuf, size_t
> count, loff_t *ppos)
> {
>      struct pipe *pipe = file->private_data;
>      T req;
>      wait_event_interruptible(pipe->queue, kfifo_out(&pipe->fifo, &req, 1));
>      process_request(req, &pipe->resp, &pipe->resp_len);
>      if (count < pipe->resp_len)
>          return -EFAULT; // TODO: handle copy to client in parts
>      if (copy_to_user(userbuf, buf, pipe->resp_len))
>          return -EFAULT;
> }
>
> Usage is:
>
> fd = io.FileIO("/debug/mymodule/file", "r+")
> fd.write('req...')
> print fd.read(100)
>
> This is not so robust, for many reasons (look how many bugs are in
> this small and simple snippet), and some parts need to be repeated for
> each input type.
>
> What I've had in mind, in similar fashion to grpc.io, have the user
> write a size prefixed protocol buffer object to the file, and
> similarly read it as a response.
>
> Something like:
>
> fd = io.FileIO("/debug/mymodule/file", "r+")
> fd.write(myReq.SerializeToString())
> len = struct.unpack("<i", fd.read(4))
> Resp.ParseFromString(fd.read(len))
>
> I believe it is not hard to create a kernel compatible protocol buffer
> code generator.
>
> When you have this in place, you have to write a very simple logic to
> add a new functionality to the debugfs file. Handler would essentially
> get pointers to a request struct, and a response struct, and would
> need to fill out the response struct.
>
> Are there similar solutions?
> What problems might my approach cause?
> Is there a better idea for this problem altogether?
>
> Thanks,
>
> _______________________________________________
> Linux-il mailing list
> Linux-il at cs.huji.ac.il
> http://mailman.cs.huji.ac.il/mailman/listinfo/linux-il
>