Re: Addressing architectural differences between FUSE driver and fs - Re: virtio-fs tests between host(x86) and dpu(arm64)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 3 Jun 2024 at 17:28, Stefan Hajnoczi <stefanha@xxxxxxxxxx> wrote:
>
> On Mon, Jun 03, 2024 at 04:56:14PM +0200, Miklos Szeredi wrote:
> > On Mon, Jun 3, 2024 at 3:44 PM Stefan Hajnoczi <stefanha@xxxxxxxxxx> wrote:
> > >
> > > On Mon, Jun 03, 2024 at 11:06:19AM +0200, Miklos Szeredi wrote:
> > > > On Mon, 3 Jun 2024 at 10:53, Peter-Jan Gootzen <pgootzen@xxxxxxxxxx> wrote:
> > > >
> > > > > We also considered this idea, it would kind of be like locking FUSE into
> > > > > being x86. However I think this is not backwards compatible. Currently
> > > > > an ARM64 client and ARM64 server work just fine. But making such a
> > > > > change would break if the client has the new driver version and the
> > > > > server is not updated to know that it should interpret x86 specifically.
> > > >
> > > > This would need to be negotiated, of course.
> > > >
> > > > But it's certainly simpler to just indicate the client arch in the
> > > > INIT request.   Let's go with that for now.
> > >
> > > In the long term it would be cleanest to choose a single canonical
> > > format instead of requiring drivers and devices to implement many
> > > arch-specific formats. I liked the single canonical format idea you
> > > suggested.
> > >
> > > My only concern is whether there are more commands/fields in FUSE that
> > > operate in an arch-specific way (e.g. ioctl)? If there really are parts
> > > that need to be arch-specific, then it might be necessary to negotiate
> > > an architecture after all.
> >
> > How about something like this:
> >
> >  - by default fall back to no translation for backward compatibility
> >  - server may request matching by sending its own arch identifier in
> > fuse_init_in
> >  - client sends back its arch identifier in fuse_init_out
> >  - client also sends back a flag indicating whether it will transform
> > to canonical or not
> >
> > This means the client does not have to implement translation for every
> > architecture, only ones which are frequently used as guest.  The
> > server may opt to implement its own translation if it's lacking in the
> > client, or it can just fail.
>
> From the client perspective:
>
> 1. Do not negotiate arch in fuse_init_out - hopefully client and server
>    know what they are doing :). This is the current behavior.
> 2. Reply to fuse_init_in with server's arch in fuse_init_out - client
>    translates according to server's arch.
> 3. Reply to fuse_init_in with canonical flag set in fuse_init_out -
>    client and server use canonical format.
>
> From the server perspective:
>
> 1. Client does not negotiate arch - the current behavior (good luck!).
> 2. Arch received in fuse_init_out from client - must be equal to
>    server's arch since there is no way for the server to reject the
>    arch.
> 3. Canonical flag received in fuse_init_out from client - client and
>    server use canonical format.

Yeah, something like that (I got the direction of the negotiation wrong).

See below patch.

I'm thinking that fuse_init_out need not even have the server arch,
since the client will only be translating to the canonical arch.  The
client sends its own arch in fuse_init_in.arch_id and advertises with
FUSE_CANON_ARCH set in fuse_init_in.flags whether it supports
transforming to canonical.  If the server wants canonicalization, then
it responds with FUSE_CANON_ARCH set in fuse_init_out.flags.

This works for legacy server that doesn't interpret the new flag and
field, and also for legacy client that doesn't set either (zero
arch_id means: unknown architecture).

arch_id could be a hash of the arch name, so that the fuse header file
doesn't need to be updated whenever a new architecture is added.

Thanks,
Miklos

diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index d08b99d60f6f..c63d8bab2d37 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -421,6 +421,7 @@ struct fuse_file_lock {
  * FUSE_NO_EXPORT_SUPPORT: explicitly disable export support
  * FUSE_HAS_RESEND: kernel supports resending pending requests, and
the high bit
  * FUSE_NO_EXPORT_SUPPORT: explicitly disable export support
  * FUSE_HAS_RESEND: kernel supports resending pending requests, and
the high bit
  *                 of the request ID indicates resend requests
+ * FUSE_CANON_ARCH: translate arch specific constants to canonical
  */
 #define FUSE_ASYNC_READ                (1 << 0)
 #define FUSE_POSIX_LOCKS       (1 << 1)
@@ -463,6 +464,7 @@ struct fuse_file_lock {
 #define FUSE_PASSTHROUGH       (1ULL << 37)
 #define FUSE_NO_EXPORT_SUPPORT (1ULL << 38)
 #define FUSE_HAS_RESEND                (1ULL << 39)
+#define FUSE_CANON_ARCH                (1ULL << 40)

 /* Obsolete alias for FUSE_DIRECT_IO_ALLOW_MMAP */
 #define FUSE_DIRECT_IO_RELAX   FUSE_DIRECT_IO_ALLOW_MMAP
@@ -874,7 +876,8 @@ struct fuse_init_in {
        uint32_t        max_readahead;
        uint32_t        flags;
        uint32_t        flags2;
-       uint32_t        unused[11];
+       uint32_t        arch_id;
+       uint32_t        unused[10];
 };

 #define FUSE_COMPAT_INIT_OUT_SIZE 8





[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux