All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vivek Goyal <vgoyal@redhat.com>
To: Max Reitz <mreitz@redhat.com>
Cc: virtio-fs@redhat.com, qemu-devel@nongnu.org,
	Stefan Hajnoczi <stefanha@redhat.com>,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>
Subject: Re: [PATCH v2 0/9] virtiofsd: Allow using file handles instead of O_PATH FDs
Date: Fri, 11 Jun 2021 15:19:25 -0400	[thread overview]
Message-ID: <20210611191925.GA767764@redhat.com> (raw)
In-Reply-To: <20210609155551.44437-1-mreitz@redhat.com>

On Wed, Jun 09, 2021 at 05:55:42PM +0200, Max Reitz wrote:
> Hi,
> 
> v1 cover letter for an overview:
> https://listman.redhat.com/archives/virtio-fs/2021-June/msg00033.html

Hi Max,

What's the impact of these patches on performance? Just trying to 
get some idea what to expect. Performance remains more or less
same or we expect a hit.

Thanks
Vivek

> 
> In v2, I (tried to) fix the bug Dave found, which is that
> get_file_handle() indiscriminately opened the given dirfd/name
> combination to get an O_RDONLY fd without checking whether we’re
> actually allowed to open dirfd/name; namely, we don’t allow ourselves to
> open files that aren’t regular files or directories.
> 
> So that openat(.., O_RDONLY) is changed to an openat(..., O_PATH), and
> then check the file type with the statx() we’re doing anyway.  If the
> file is OK to open, we reopen it O_RDONLY with the help of
> /proc/self/fd, like we always do.
> 
> (This only affects patch 8.)
> 
> 
> git-backport-diff against v1:
> 
> Key:
> [----] : patches are identical
> [####] : number of functional differences between upstream/downstream patch
> [down] : patch is downstream-only
> The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
> 
> 001/9:[----] [--] 'virtiofsd: Add TempFd structure'
> 002/9:[----] [--] 'virtiofsd: Use lo_inode_open() instead of openat()'
> 003/9:[----] [--] 'virtiofsd: Add lo_inode_fd() helper'
> 004/9:[----] [--] 'virtiofsd: Let lo_fd() return a TempFd'
> 005/9:[----] [--] 'virtiofsd: Let lo_inode_open() return a TempFd'
> 006/9:[----] [--] 'virtiofsd: Add lo_inode.fhandle'
> 007/9:[----] [--] 'virtiofsd: Add inodes_by_handle hash table'
> 008/9:[0045] [FC] 'virtiofsd: Optionally fill lo_inode.fhandle'
> 009/9:[----] [--] 'virtiofsd: Add lazy lo_do_find()'
> 
> 
> Max Reitz (9):
>   virtiofsd: Add TempFd structure
>   virtiofsd: Use lo_inode_open() instead of openat()
>   virtiofsd: Add lo_inode_fd() helper
>   virtiofsd: Let lo_fd() return a TempFd
>   virtiofsd: Let lo_inode_open() return a TempFd
>   virtiofsd: Add lo_inode.fhandle
>   virtiofsd: Add inodes_by_handle hash table
>   virtiofsd: Optionally fill lo_inode.fhandle
>   virtiofsd: Add lazy lo_do_find()
> 
>  tools/virtiofsd/helper.c              |   3 +
>  tools/virtiofsd/passthrough_ll.c      | 836 +++++++++++++++++++++-----
>  tools/virtiofsd/passthrough_seccomp.c |   2 +
>  3 files changed, 694 insertions(+), 147 deletions(-)
> 
> -- 
> 2.31.1
> 
> 



WARNING: multiple messages have this Message-ID (diff)
From: Vivek Goyal <vgoyal@redhat.com>
To: Max Reitz <mreitz@redhat.com>
Cc: virtio-fs@redhat.com, qemu-devel@nongnu.org
Subject: Re: [Virtio-fs] [PATCH v2 0/9] virtiofsd: Allow using file handles instead of O_PATH FDs
Date: Fri, 11 Jun 2021 15:19:25 -0400	[thread overview]
Message-ID: <20210611191925.GA767764@redhat.com> (raw)
In-Reply-To: <20210609155551.44437-1-mreitz@redhat.com>

On Wed, Jun 09, 2021 at 05:55:42PM +0200, Max Reitz wrote:
> Hi,
> 
> v1 cover letter for an overview:
> https://listman.redhat.com/archives/virtio-fs/2021-June/msg00033.html

Hi Max,

What's the impact of these patches on performance? Just trying to 
get some idea what to expect. Performance remains more or less
same or we expect a hit.

Thanks
Vivek

> 
> In v2, I (tried to) fix the bug Dave found, which is that
> get_file_handle() indiscriminately opened the given dirfd/name
> combination to get an O_RDONLY fd without checking whether we’re
> actually allowed to open dirfd/name; namely, we don’t allow ourselves to
> open files that aren’t regular files or directories.
> 
> So that openat(.., O_RDONLY) is changed to an openat(..., O_PATH), and
> then check the file type with the statx() we’re doing anyway.  If the
> file is OK to open, we reopen it O_RDONLY with the help of
> /proc/self/fd, like we always do.
> 
> (This only affects patch 8.)
> 
> 
> git-backport-diff against v1:
> 
> Key:
> [----] : patches are identical
> [####] : number of functional differences between upstream/downstream patch
> [down] : patch is downstream-only
> The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
> 
> 001/9:[----] [--] 'virtiofsd: Add TempFd structure'
> 002/9:[----] [--] 'virtiofsd: Use lo_inode_open() instead of openat()'
> 003/9:[----] [--] 'virtiofsd: Add lo_inode_fd() helper'
> 004/9:[----] [--] 'virtiofsd: Let lo_fd() return a TempFd'
> 005/9:[----] [--] 'virtiofsd: Let lo_inode_open() return a TempFd'
> 006/9:[----] [--] 'virtiofsd: Add lo_inode.fhandle'
> 007/9:[----] [--] 'virtiofsd: Add inodes_by_handle hash table'
> 008/9:[0045] [FC] 'virtiofsd: Optionally fill lo_inode.fhandle'
> 009/9:[----] [--] 'virtiofsd: Add lazy lo_do_find()'
> 
> 
> Max Reitz (9):
>   virtiofsd: Add TempFd structure
>   virtiofsd: Use lo_inode_open() instead of openat()
>   virtiofsd: Add lo_inode_fd() helper
>   virtiofsd: Let lo_fd() return a TempFd
>   virtiofsd: Let lo_inode_open() return a TempFd
>   virtiofsd: Add lo_inode.fhandle
>   virtiofsd: Add inodes_by_handle hash table
>   virtiofsd: Optionally fill lo_inode.fhandle
>   virtiofsd: Add lazy lo_do_find()
> 
>  tools/virtiofsd/helper.c              |   3 +
>  tools/virtiofsd/passthrough_ll.c      | 836 +++++++++++++++++++++-----
>  tools/virtiofsd/passthrough_seccomp.c |   2 +
>  3 files changed, 694 insertions(+), 147 deletions(-)
> 
> -- 
> 2.31.1
> 
> 


  parent reply	other threads:[~2021-06-11 19:21 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-09 15:55 [PATCH v2 0/9] virtiofsd: Allow using file handles instead of O_PATH FDs Max Reitz
2021-06-09 15:55 ` [Virtio-fs] " Max Reitz
2021-06-09 15:55 ` [PATCH v2 1/9] virtiofsd: Add TempFd structure Max Reitz
2021-06-09 15:55   ` [Virtio-fs] " Max Reitz
2021-06-09 15:55 ` [PATCH v2 2/9] virtiofsd: Use lo_inode_open() instead of openat() Max Reitz
2021-06-09 15:55   ` [Virtio-fs] " Max Reitz
2021-06-09 15:55 ` [PATCH v2 3/9] virtiofsd: Add lo_inode_fd() helper Max Reitz
2021-06-09 15:55   ` [Virtio-fs] " Max Reitz
2021-06-09 15:55 ` [PATCH v2 4/9] virtiofsd: Let lo_fd() return a TempFd Max Reitz
2021-06-09 15:55   ` [Virtio-fs] " Max Reitz
2021-06-09 15:55 ` [PATCH v2 5/9] virtiofsd: Let lo_inode_open() " Max Reitz
2021-06-09 15:55   ` [Virtio-fs] " Max Reitz
2021-06-09 15:55 ` [PATCH v2 6/9] virtiofsd: Add lo_inode.fhandle Max Reitz
2021-06-09 15:55   ` [Virtio-fs] " Max Reitz
2021-06-09 15:55 ` [PATCH v2 7/9] virtiofsd: Add inodes_by_handle hash table Max Reitz
2021-06-09 15:55   ` [Virtio-fs] " Max Reitz
2021-06-11 20:04   ` Vivek Goyal
2021-06-16 13:38     ` Max Reitz
2021-06-17 21:21       ` Vivek Goyal
2021-06-18  8:28         ` Max Reitz
2021-06-18 18:29           ` Vivek Goyal
2021-06-21  9:02             ` Max Reitz
2021-06-21 15:51               ` Vivek Goyal
2021-06-21 17:07                 ` Max Reitz
2021-06-21 21:27                   ` Vivek Goyal
2021-07-13 15:07               ` Max Reitz
2021-07-20 14:50                 ` Vivek Goyal
2021-07-21  8:29                   ` Max Reitz
2021-06-18  8:30   ` Max Reitz
2021-06-18  8:30     ` [Virtio-fs] " Max Reitz
2021-06-09 15:55 ` [PATCH v2 8/9] virtiofsd: Optionally fill lo_inode.fhandle Max Reitz
2021-06-09 15:55   ` [Virtio-fs] " Max Reitz
2021-06-09 15:55 ` [PATCH v2 9/9] virtiofsd: Add lazy lo_do_find() Max Reitz
2021-06-09 15:55   ` [Virtio-fs] " Max Reitz
2021-06-11 19:19 ` Vivek Goyal [this message]
2021-06-11 19:19   ` [Virtio-fs] [PATCH v2 0/9] virtiofsd: Allow using file handles instead of O_PATH FDs Vivek Goyal
2021-06-16 13:41   ` Max Reitz
2021-06-16 13:41     ` [Virtio-fs] " Max Reitz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210611191925.GA767764@redhat.com \
    --to=vgoyal@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=virtio-fs@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.