ceph: add support for getvxattr op

Problem:
Directory vxattrs like ceph.dir.pin* and ceph.dir.layout* may not be propagated to the client as frequently to keep them updated. This creates vxattr availability problems.

Solution:
Adds new getvxattr op to fetch ceph.dir.pin*, ceph.dir.layout* and ceph.file.layout* vxattrs.
If the entire layout for a dir or a file is being set, then it is expected that the layout be set in standard JSON format. Individual field value retrieval is not wrapped in JSON. The JSON format also applies while setting the vxattr if the entire layout is being set in one go.
As a temporary measure, setting a vxattr can also be done in the old format. The old format will be deprecated in the future.

URL: https://tracker.ceph.com/issues/51062

Signed-off-by Milind Changire <[email protected]>

---
  fs/ceph/inode.c              | 51 ++++++++++++++++++++++++++++++++++++
  fs/ceph/mds_client.c         | 27 ++++++++++++++++++-
  fs/ceph/mds_client.h         | 12 ++++++++-
  fs/ceph/strings.c            |  1 +
  fs/ceph/super.h              |  1 +
  fs/ceph/xattr.c              | 34 ++++++++++++++++++++++++
  include/linux/ceph/ceph_fs.h |  1 +
  7 files changed, 125 insertions(+), 2 deletions(-)

fs/ceph/inode.c +51 -0

index e3322fcb2e8d..b63746a7a0e0 100644
@@ -2291,6 +2291,57 @@ int __ceph_do_getattr(struct inode *inode, struct page *locked_page,
  	return err;
  }
 
+int ceph_do_getvxattr(struct inode *inode, const char *name, void *value,
+		      size_t size)
+{
+	struct ceph_fs_client *fsc = ceph_sb_to_client(inode->i_sb);
+	struct ceph_mds_client *mdsc = fsc->mdsc;
+	struct ceph_mds_request *req;
+	int mode = USE_AUTH_MDS;
+	int err;
+	char *xattr_value;
+	size_t xattr_value_len;
+
+	req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_GETVXATTR, mode);
+	if (IS_ERR(req)) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	req->r_path2 = kstrdup(name, GFP_NOFS);
+	if (!req->r_path2) {
+		err = -ENOMEM;
+		goto put;
+	}
+
+	ihold(inode);
+	req->r_inode = inode;
+	err = ceph_mdsc_do_request(mdsc, NULL, req);
+	if (err < 0)
+		goto put;
+
+	xattr_value = req->r_reply_info.xattr_info.xattr_value;
+	xattr_value_len = req->r_reply_info.xattr_info.xattr_value_len;
+
+	dout("do_getvxattr xattr_value_len:%u, size:%u\n", xattr_value_len, size);
+
+	err = xattr_value_len;
+	if (size == 0)
+		goto put;
+
+	if (xattr_value_len > size) {
+		err = -ERANGE;
+		goto put;
+	}
+
+	memcpy(value, xattr_value, xattr_value_len);
+put:
+	ceph_mdsc_put_request(req);
+out:
+	dout("do_getvxattr result=%d\n", err);
+	return err;
+}
+
 
  /*
   * Check inode permissions.  We verify we have a valid value for

fs/ceph/mds_client.c +26 -1

index c30eefc0ac19..a5eafc71d976 100644
@@ -555,6 +555,29 @@ static int parse_reply_info_create(void **p, void *end,
  	return -EIO;
  }
 
+static int parse_reply_info_getvxattr(void **p, void *end,
+				      struct ceph_mds_reply_info_parsed *info,
+				      u64 features)
+{
+	u8 struct_v, struct_compat;
+	u32 struct_len;
+	u32 value_len;
+
+	ceph_decode_8_safe(p, end, struct_v, bad);
+	ceph_decode_8_safe(p, end, struct_compat, bad);
+	ceph_decode_32_safe(p, end, struct_len, bad);
+	ceph_decode_32_safe(p, end, value_len, bad);
+
+	if (value_len == end - *p) {
+	  info->xattr_info.xattr_value = *p;
+	  info->xattr_info.xattr_value_len = end - *p;
+	  *p = end;
+	  return info->xattr_info.xattr_value_len;
+	}
+bad:
+	return -EIO;
+}
+
  /*
   * parse extra results
   */

@@ -570,6 +593,8 @@ static int parse_reply_info_extra(void **p, void *end,
  		return parse_reply_info_readdir(p, end, info, features);
  	else if (op == CEPH_MDS_OP_CREATE)
  		return parse_reply_info_create(p, end, info, features, s);
+	else if (op == CEPH_MDS_OP_GETVXATTR)
+		return parse_reply_info_getvxattr(p, end, info, features);
  	else
  		return -EIO;
  }

@@ -615,7 +640,7 @@ static int parse_reply_info(struct ceph_mds_session *s, struct ceph_msg *msg,
 
  	if (p != end)
  		goto bad;
-	return 0;
+	return err;
 
  bad:
  	err = -EIO;

fs/ceph/mds_client.h +11 -1

index 97c7f7bfa55f..f2a8e5af3c2e 100644
@@ -29,8 +29,10 @@ enum ceph_feature_type {
  	CEPHFS_FEATURE_MULTI_RECONNECT,
  	CEPHFS_FEATURE_DELEG_INO,
  	CEPHFS_FEATURE_METRIC_COLLECT,
+	CEPHFS_FEATURE_ALTERNATE_NAME,
+	CEPHFS_FEATURE_GETVXATTR,
 
-	CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_METRIC_COLLECT,
+	CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_GETVXATTR,
  };
 
  /*

@@ -45,6 +47,8 @@ enum ceph_feature_type {
  	CEPHFS_FEATURE_MULTI_RECONNECT,		\
  	CEPHFS_FEATURE_DELEG_INO,		\
  	CEPHFS_FEATURE_METRIC_COLLECT,		\
+	CEPHFS_FEATURE_ALTERNATE_NAME,		\
+	CEPHFS_FEATURE_GETVXATTR,		\
  						\
  	CEPHFS_FEATURE_MAX,			\
  }

@@ -100,6 +104,11 @@ struct ceph_mds_reply_dir_entry {
  	loff_t			      offset;
  };
 
+struct ceph_mds_reply_xattr {
+	char *xattr_value;
+	size_t xattr_value_len;
+};
+
  /*
   * parsed info about an mds reply, including information about
   * either: 1) the target inode and/or its parent directory and dentry,

@@ -115,6 +124,7 @@ struct ceph_mds_reply_info_parsed {
  	char                          *dname;
  	u32                           dname_len;
  	struct ceph_mds_reply_lease   *dlease;
+	struct ceph_mds_reply_xattr   xattr_info;
 
  	/* extra */
  	union {

fs/ceph/strings.c +1 -0

index 573bb9556fb5..e36e8948e728 100644
@@ -60,6 +60,7 @@ const char *ceph_mds_op_name(int op)
  	case CEPH_MDS_OP_LOOKUPINO:  return "lookupino";
  	case CEPH_MDS_OP_LOOKUPNAME:  return "lookupname";
  	case CEPH_MDS_OP_GETATTR:  return "getattr";
+	case CEPH_MDS_OP_GETVXATTR:  return "getvxattr";
  	case CEPH_MDS_OP_SETXATTR: return "setxattr";
  	case CEPH_MDS_OP_SETATTR: return "setattr";
  	case CEPH_MDS_OP_RMXATTR: return "rmxattr";

fs/ceph/super.h +1 -0

index ac331aa07cfa..a627fa69668e 100644
@@ -1043,6 +1043,7 @@ static inline bool ceph_inode_is_shutdown(struct inode *inode)
 
  /* xattr.c */
  int __ceph_setxattr(struct inode *, const char *, const void *, size_t, int);
+int ceph_do_getvxattr(struct inode *inode, const char *name, void *value, size_t size);
  ssize_t __ceph_getxattr(struct inode *, const char *, void *, size_t);
  extern ssize_t ceph_listxattr(struct dentry *, char *, size_t);
  extern struct ceph_buffer *__ceph_build_xattrs_blob(struct ceph_inode_info *ci);

fs/ceph/xattr.c +9 -0

index fcf7dfdecf96..dc32876a541a 100644
@@ -918,6 +918,30 @@ static inline int __get_request_mask(struct inode *in) {
  	return mask;
  }
 
+/* check if the entire cluster supports the given feature */
+static inline bool ceph_cluster_has_feature(struct inode *inode, int feature_bit)
+{
+	int64_t i;
+	struct ceph_fs_client *fsc = ceph_inode_to_client(inode);
+	struct ceph_mds_session **sessions = fsc->mdsc->sessions;
+	int64_t num_sessions = atomic_read(&fsc->mdsc->num_sessions);
+
+	if (fsc->mdsc->stopping)

fs/ceph/xattr.c +5 -0

+			return false;
+	}
+	return true;
+}
+
  ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value,
  		      size_t size)
  {

@@ -927,6 +951,16 @@ ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value,
  	int req_mask;
  	ssize_t err;
 

fs/ceph/xattr.c +0 -0

  	/* let's see if a virtual xattr was requested */
  	vxattr = ceph_match_vxattr(inode, name);
  	if (vxattr) {

include/linux/ceph/ceph_fs.h +1 -0

index 7ad6c3d0db7d..66db21ac5f0c 100644
@@ -328,6 +328,7 @@ enum {
  	CEPH_MDS_OP_LOOKUPPARENT = 0x00103,
  	CEPH_MDS_OP_LOOKUPINO  = 0x00104,
  	CEPH_MDS_OP_LOOKUPNAME = 0x00105,
+	CEPH_MDS_OP_GETVXATTR  = 0x00106,
 
  	CEPH_MDS_OP_SETXATTR   = 0x01105,
  	CEPH_MDS_OP_RMXATTR    = 0x01106,

Quoted 61 lines · Milind Changire

Problem:
Directory vxattrs like ceph.dir.pin* and ceph.dir.layout* may not be propagated to the client as frequently to keep them updated. This creates vxattr availability problems.

Solution:
Adds new getvxattr op to fetch ceph.dir.pin*, ceph.dir.layout* and ceph.file.layout* vxattrs.
If the entire layout for a dir or a file is being set, then it is expected that the layout be set in standard JSON format. Individual field value retrieval is not wrapped in JSON. The JSON format also applies while setting the vxattr if the entire layout is being set in one go.
As a temporary measure, setting a vxattr can also be done in the old format. The old format will be deprecated in the future.

URL: https://tracker.ceph.com/issues/51062

Signed-off-by Milind Changire <[email protected]>

---
  fs/ceph/inode.c              | 51 ++++++++++++++++++++++++++++++++++++
  fs/ceph/mds_client.c         | 27 ++++++++++++++++++-
  fs/ceph/mds_client.h         | 12 ++++++++-
  fs/ceph/strings.c            |  1 +
  fs/ceph/super.h              |  1 +
  fs/ceph/xattr.c              | 34 ++++++++++++++++++++++++
  include/linux/ceph/ceph_fs.h |  1 +
  7 files changed, 125 insertions(+), 2 deletions(-)

fs/ceph/inode.c +25 -0

index e3322fcb2e8d..b63746a7a0e0 100644
@@ -2291,6 +2291,57 @@ int __ceph_do_getattr(struct inode *inode, struct page *locked_page,
      return err;
  }
 
+int ceph_do_getvxattr(struct inode *inode, const char *name, void *value,
+                   size_t size)
+{
+     struct ceph_fs_client *fsc = ceph_sb_to_client(inode->i_sb);
+     struct ceph_mds_client *mdsc = fsc->mdsc;
+     struct ceph_mds_request *req;
+     int mode = USE_AUTH_MDS;
+     int err;
+     char *xattr_value;
+     size_t xattr_value_len;
+
+     req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_GETVXATTR, mode);
+     if (IS_ERR(req)) {
+             err = -ENOMEM;
+             goto out;
+     }
+
+     req->r_path2 = kstrdup(name, GFP_NOFS);
+     if (!req->r_path2) {
+             err = -ENOMEM;
+             goto put;
+     }
+
+     ihold(inode);
+     req->r_inode = inode;

fs/ceph/inode.c int __ceph_do_getattr(struct inode *inode, struct page *locked_page, · line 2326

Quoted 8 lines

fs/ceph/inode.c +8 -0

+     err = ceph_mdsc_do_request(mdsc, NULL, req);
+     if (err < 0)
+             goto put;
+
+     xattr_value = req->r_reply_info.xattr_info.xattr_value;
+     xattr_value_len = req->r_reply_info.xattr_info.xattr_value_len;
+
+     dout("do_getvxattr xattr_value_len:%u, size:%u\n", xattr_value_len, size);

Quoted 164 lines

fs/ceph/inode.c +18 -0

+
+     err = xattr_value_len;
+     if (size == 0)
+             goto put;
+
+     if (xattr_value_len > size) {
+             err = -ERANGE;
+             goto put;
+     }
+
+     memcpy(value, xattr_value, xattr_value_len);
+put:
+     ceph_mdsc_put_request(req);
+out:
+     dout("do_getvxattr result=%d\n", err);
+     return err;
+}
+
 
  /*
   * Check inode permissions.  We verify we have a valid value for

fs/ceph/mds_client.c +26 -1

index c30eefc0ac19..a5eafc71d976 100644
@@ -555,6 +555,29 @@ static int parse_reply_info_create(void **p, void *end,
      return -EIO;
  }
 
+static int parse_reply_info_getvxattr(void **p, void *end,
+                                   struct ceph_mds_reply_info_parsed *info,
+                                   u64 features)
+{
+     u8 struct_v, struct_compat;
+     u32 struct_len;
+     u32 value_len;
+
+     ceph_decode_8_safe(p, end, struct_v, bad);
+     ceph_decode_8_safe(p, end, struct_compat, bad);
+     ceph_decode_32_safe(p, end, struct_len, bad);
+     ceph_decode_32_safe(p, end, value_len, bad);
+
+     if (value_len == end - *p) {
+       info->xattr_info.xattr_value = *p;
+       info->xattr_info.xattr_value_len = end - *p;
+       *p = end;
+       return info->xattr_info.xattr_value_len;
+     }
+bad:
+     return -EIO;
+}
+
  /*
   * parse extra results
   */

@@ -570,6 +593,8 @@ static int parse_reply_info_extra(void **p, void *end,
              return parse_reply_info_readdir(p, end, info, features);
      else if (op == CEPH_MDS_OP_CREATE)
              return parse_reply_info_create(p, end, info, features, s);
+     else if (op == CEPH_MDS_OP_GETVXATTR)
+             return parse_reply_info_getvxattr(p, end, info, features);
      else
              return -EIO;
  }

@@ -615,7 +640,7 @@ static int parse_reply_info(struct ceph_mds_session *s, struct ceph_msg *msg,
 
      if (p != end)
              goto bad;
-     return 0;
+     return err;
 
  bad:
      err = -EIO;

fs/ceph/mds_client.h +11 -1

index 97c7f7bfa55f..f2a8e5af3c2e 100644
@@ -29,8 +29,10 @@ enum ceph_feature_type {
      CEPHFS_FEATURE_MULTI_RECONNECT,
      CEPHFS_FEATURE_DELEG_INO,
      CEPHFS_FEATURE_METRIC_COLLECT,
+     CEPHFS_FEATURE_ALTERNATE_NAME,
+     CEPHFS_FEATURE_GETVXATTR,
 
-     CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_METRIC_COLLECT,
+     CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_GETVXATTR,
  };
 
  /*

@@ -45,6 +47,8 @@ enum ceph_feature_type {
      CEPHFS_FEATURE_MULTI_RECONNECT,         \
      CEPHFS_FEATURE_DELEG_INO,               \
      CEPHFS_FEATURE_METRIC_COLLECT,          \
+     CEPHFS_FEATURE_ALTERNATE_NAME,          \
+     CEPHFS_FEATURE_GETVXATTR,               \
                                              \
      CEPHFS_FEATURE_MAX,                     \
  }

@@ -100,6 +104,11 @@ struct ceph_mds_reply_dir_entry {
      loff_t                        offset;
  };
 
+struct ceph_mds_reply_xattr {
+     char *xattr_value;
+     size_t xattr_value_len;
+};
+
  /*
   * parsed info about an mds reply, including information about
   * either: 1) the target inode and/or its parent directory and dentry,

@@ -115,6 +124,7 @@ struct ceph_mds_reply_info_parsed {
      char                          *dname;
      u32                           dname_len;
      struct ceph_mds_reply_lease   *dlease;
+     struct ceph_mds_reply_xattr   xattr_info;
 
      /* extra */
      union {

fs/ceph/strings.c +1 -0

index 573bb9556fb5..e36e8948e728 100644
@@ -60,6 +60,7 @@ const char *ceph_mds_op_name(int op)
      case CEPH_MDS_OP_LOOKUPINO:  return "lookupino";
      case CEPH_MDS_OP_LOOKUPNAME:  return "lookupname";
      case CEPH_MDS_OP_GETATTR:  return "getattr";
+     case CEPH_MDS_OP_GETVXATTR:  return "getvxattr";
      case CEPH_MDS_OP_SETXATTR: return "setxattr";
      case CEPH_MDS_OP_SETATTR: return "setattr";
      case CEPH_MDS_OP_RMXATTR: return "rmxattr";

fs/ceph/super.h +1 -0

index ac331aa07cfa..a627fa69668e 100644
@@ -1043,6 +1043,7 @@ static inline bool ceph_inode_is_shutdown(struct inode *inode)
 
  /* xattr.c */
  int __ceph_setxattr(struct inode *, const char *, const void *, size_t, int);
+int ceph_do_getvxattr(struct inode *inode, const char *name, void *value, size_t size);
  ssize_t __ceph_getxattr(struct inode *, const char *, void *, size_t);
  extern ssize_t ceph_listxattr(struct dentry *, char *, size_t);
  extern struct ceph_buffer *__ceph_build_xattrs_blob(struct ceph_inode_info *ci);

fs/ceph/xattr.c +14 -0

index fcf7dfdecf96..dc32876a541a 100644
@@ -918,6 +918,30 @@ static inline int __get_request_mask(struct inode *in) {
      return mask;
  }
 
+/* check if the entire cluster supports the given feature */
+static inline bool ceph_cluster_has_feature(struct inode *inode, int feature_bit)
+{
+     int64_t i;
+     struct ceph_fs_client *fsc = ceph_inode_to_client(inode);
+     struct ceph_mds_session **sessions = fsc->mdsc->sessions;
+     int64_t num_sessions = atomic_read(&fsc->mdsc->num_sessions);
+
+     if (fsc->mdsc->stopping)
+             return false;
+
+     if (!sessions)
+             return false;
+

fs/ceph/xattr.c static inline int __get_request_mask(struct inode *in) { · line 939

Quoted 5 lines

fs/ceph/xattr.c +5 -0

+     for (i = 0; i < num_sessions; i++) {
+             struct ceph_mds_session *session = sessions[i];
+             if (!session)
+                     return false;
+             if (!test_bit(feature_bit, &session->s_features))

This will be possibly cause crash because, and you should put the whole for loop and 'fsc->mdsc->xxx' above under the 'mdsc->mutex'.

Quoted 14 lines

fs/ceph/xattr.c +5 -0

+                     return false;
+     }
+     return true;
+}
+
  ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value,
                    size_t size)
  {

@@ -927,6 +951,16 @@ ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value,
      int req_mask;
      ssize_t err;
 

fs/ceph/xattr.c ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, · line 963

Quoted 10 lines

fs/ceph/xattr.c +10 -0

+     if (!strncmp(name, XATTR_CEPH_PREFIX, XATTR_CEPH_PREFIX_LEN) &&
+         ceph_cluster_has_feature(inode, CEPHFS_FEATURE_GETVXATTR)) {
+             err = ceph_do_getvxattr(inode, name, value, size);
+             /* if cluster doesn't support xattr, we try to service it
+              * locally
+              */
+             if (err >= 0)
+                     return err;
+     }
+

If the 'Fa' or 'Fr' caps is issued to kclient, could we get this vxattr locally instead of getting it from MDS every time ?

Quoted 18 lines

fs/ceph/xattr.c +0 -0

      /* let's see if a virtual xattr was requested */
      vxattr = ceph_match_vxattr(inode, name);
      if (vxattr) {

include/linux/ceph/ceph_fs.h +1 -0

index 7ad6c3d0db7d..66db21ac5f0c 100644
@@ -328,6 +328,7 @@ enum {
      CEPH_MDS_OP_LOOKUPPARENT = 0x00103,
      CEPH_MDS_OP_LOOKUPINO  = 0x00104,
      CEPH_MDS_OP_LOOKUPNAME = 0x00105,
+     CEPH_MDS_OP_GETVXATTR  = 0x00106,
 
      CEPH_MDS_OP_SETXATTR   = 0x01105,
      CEPH_MDS_OP_RMXATTR    = 0x01106,

Quoted 63 lines · Xiubo Li

Quoted 61 lines · Milind Changire

Problem:
Directory vxattrs like ceph.dir.pin* and ceph.dir.layout* may not be propagated to the client as frequently to keep them updated. This creates vxattr availability problems.

Solution:
Adds new getvxattr op to fetch ceph.dir.pin*, ceph.dir.layout* and ceph.file.layout* vxattrs.
If the entire layout for a dir or a file is being set, then it is expected that the layout be set in standard JSON format. Individual field value retrieval is not wrapped in JSON. The JSON format also applies while setting the vxattr if the entire layout is being set in one go.
As a temporary measure, setting a vxattr can also be done in the old format. The old format will be deprecated in the future.

URL: https://tracker.ceph.com/issues/51062

Signed-off-by Milind Changire <[email protected]>

---
   fs/ceph/inode.c              | 51 ++++++++++++++++++++++++++++++++++++
   fs/ceph/mds_client.c         | 27 ++++++++++++++++++-
   fs/ceph/mds_client.h         | 12 ++++++++-
   fs/ceph/strings.c            |  1 +
   fs/ceph/super.h              |  1 +
   fs/ceph/xattr.c              | 34 ++++++++++++++++++++++++
   include/linux/ceph/ceph_fs.h |  1 +
   7 files changed, 125 insertions(+), 2 deletions(-)

fs/ceph/inode.c +25 -0

index e3322fcb2e8d..b63746a7a0e0 100644
@@ -2291,6 +2291,57 @@ int __ceph_do_getattr(struct inode *inode, struct page *locked_page,
       return err;
   }
 
+int ceph_do_getvxattr(struct inode *inode, const char *name, void *value,
+                   size_t size)
+{
+     struct ceph_fs_client *fsc = ceph_sb_to_client(inode->i_sb);
+     struct ceph_mds_client *mdsc = fsc->mdsc;
+     struct ceph_mds_request *req;
+     int mode = USE_AUTH_MDS;
+     int err;
+     char *xattr_value;
+     size_t xattr_value_len;
+
+     req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_GETVXATTR, mode);
+     if (IS_ERR(req)) {
+             err = -ENOMEM;
+             goto out;
+     }
+
+     req->r_path2 = kstrdup(name, GFP_NOFS);
+     if (!req->r_path2) {
+             err = -ENOMEM;
+             goto put;
+     }
+
+     ihold(inode);
+     req->r_inode = inode;

Quoted 8 lines

fs/ceph/inode.c int __ceph_do_getattr(struct inode *inode, struct page *locked_page, · line 2326

Quoted 8 lines

fs/ceph/inode.c +8 -0

+     err = ceph_mdsc_do_request(mdsc, NULL, req);
+     if (err < 0)
+             goto put;
+
+     xattr_value = req->r_reply_info.xattr_info.xattr_value;
+     xattr_value_len = req->r_reply_info.xattr_info.xattr_value_len;
+
+     dout("do_getvxattr xattr_value_len:%u, size:%u\n", xattr_value_len, size);

Need some help here.
The kernel CI reported the following warnings:
1. for i386 build that size_t is unsigned int
2. for riscv build that size_t is unsigned long int

So the above (dout) statement gets a warning either way if I change the expected arguments to be either %u or %lu.

Quoted 10 lines

Quoted 10 lines

fs/ceph/inode.c +10 -0

+
+     err = xattr_value_len;
+     if (size == 0)
+             goto put;
+
+     if (xattr_value_len > size) {
+             err = -ERANGE;
+             goto put;
+     }
+

Quoted 12 lines

Quoted 12 lines

fs/ceph/inode.c +5 -0

+                     return false;
+     }
+     return true;
+}
+
   ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value,
                     size_t size)
   {

@@ -927,6 +951,16 @@ ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value,
       int req_mask;
       ssize_t err;
 

Quoted 12 lines

fs/ceph/inode.c ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, · line 963

Quoted 10 lines

fs/ceph/inode.c +10 -0

+     if (!strncmp(name, XATTR_CEPH_PREFIX, XATTR_CEPH_PREFIX_LEN) &&
+         ceph_cluster_has_feature(inode, CEPHFS_FEATURE_GETVXATTR)) {
+             err = ceph_do_getvxattr(inode, name, value, size);
+             /* if cluster doesn't support xattr, we try to service it
+              * locally
+              */
+             if (err >= 0)
+                     return err;
+     }
+

If the 'Fa' or 'Fr' caps is issued to kclient, could we get this vxattr locally instead of getting it from MDS every time ?

The new mechanism is meant to supersede the old one.
1. The new layout output format is JSON
2. The new mechanism also recursively resolves the layout to the closest ancestor
3. The new mechanism deals with ceph vxattrs exclusively at the server end (currently getvxattr handles only a limited subset of all the vxattrs)
4. There's no way to fetch ceph.dir.pin* vxattrs locally
(see https://github.com/ceph/ceph/pull/42001 for userspace work)

Quoted 17 lines

Quoted 17 lines

fs/ceph/inode.c +0 -0

       /* let's see if a virtual xattr was requested */
       vxattr = ceph_match_vxattr(inode, name);
       if (vxattr) {

include/linux/ceph/ceph_fs.h +1 -0

index 7ad6c3d0db7d..66db21ac5f0c 100644
@@ -328,6 +328,7 @@ enum {
       CEPH_MDS_OP_LOOKUPPARENT = 0x00103,
       CEPH_MDS_OP_LOOKUPINO  = 0x00104,
       CEPH_MDS_OP_LOOKUPNAME = 0x00105,
+     CEPH_MDS_OP_GETVXATTR  = 0x00106,
 
       CEPH_MDS_OP_SETXATTR   = 0x01105,
       CEPH_MDS_OP_RMXATTR    = 0x01106,

Quoted 64 lines · Xiubo Li

Quoted 61 lines · Milind Changire

Problem:
Directory vxattrs like ceph.dir.pin* and ceph.dir.layout* may not be propagated to the client as frequently to keep them updated. This creates vxattr availability problems.

Solution:
Adds new getvxattr op to fetch ceph.dir.pin*, ceph.dir.layout* and ceph.file.layout* vxattrs.
If the entire layout for a dir or a file is being set, then it is expected that the layout be set in standard JSON format. Individual field value retrieval is not wrapped in JSON. The JSON format also applies while setting the vxattr if the entire layout is being set in one go.
As a temporary measure, setting a vxattr can also be done in the old format. The old format will be deprecated in the future.

URL: https://tracker.ceph.com/issues/51062

Signed-off-by Milind Changire <[email protected]>

---
  fs/ceph/inode.c              | 51 ++++++++++++++++++++++++++++++++++++
  fs/ceph/mds_client.c         | 27 ++++++++++++++++++-
  fs/ceph/mds_client.h         | 12 ++++++++-
  fs/ceph/strings.c            |  1 +
  fs/ceph/super.h              |  1 +
  fs/ceph/xattr.c              | 34 ++++++++++++++++++++++++
  include/linux/ceph/ceph_fs.h |  1 +
  7 files changed, 125 insertions(+), 2 deletions(-)

fs/ceph/inode.c +25 -0

index e3322fcb2e8d..b63746a7a0e0 100644
@@ -2291,6 +2291,57 @@ int __ceph_do_getattr(struct inode *inode, struct page *locked_page,
      return err;
  }
 
+int ceph_do_getvxattr(struct inode *inode, const char *name, void *value,
+                   size_t size)
+{
+     struct ceph_fs_client *fsc = ceph_sb_to_client(inode->i_sb);
+     struct ceph_mds_client *mdsc = fsc->mdsc;
+     struct ceph_mds_request *req;
+     int mode = USE_AUTH_MDS;
+     int err;
+     char *xattr_value;
+     size_t xattr_value_len;
+
+     req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_GETVXATTR, mode);
+     if (IS_ERR(req)) {
+             err = -ENOMEM;
+             goto out;
+     }
+
+     req->r_path2 = kstrdup(name, GFP_NOFS);
+     if (!req->r_path2) {
+             err = -ENOMEM;
+             goto put;
+     }
+
+     ihold(inode);
+     req->r_inode = inode;

Quoted 8 lines

fs/ceph/inode.c int __ceph_do_getattr(struct inode *inode, struct page *locked_page, · line 2326

Quoted 8 lines

fs/ceph/inode.c +8 -0

+     err = ceph_mdsc_do_request(mdsc, NULL, req);
+     if (err < 0)
+             goto put;
+
+     xattr_value = req->r_reply_info.xattr_info.xattr_value;
+     xattr_value_len = req->r_reply_info.xattr_info.xattr_value_len;
+
+     dout("do_getvxattr xattr_value_len:%u, size:%u\n", xattr_value_len, size);

Need some help here.
The kernel CI reported the following warnings:
1. for i386 build that size_t is unsigned int
2. for riscv build that size_t is unsigned long int

So the above (dout) statement gets a warning either way if I change the expected arguments to be either %u or %lu.

Quoted 164 lines

Quoted 164 lines

fs/ceph/inode.c +18 -0

+
+     err = xattr_value_len;
+     if (size == 0)
+             goto put;
+
+     if (xattr_value_len > size) {
+             err = -ERANGE;
+             goto put;
+     }
+
+     memcpy(value, xattr_value, xattr_value_len);
+put:
+     ceph_mdsc_put_request(req);
+out:
+     dout("do_getvxattr result=%d\n", err);
+     return err;
+}
+
 
  /*
   * Check inode permissions.  We verify we have a valid value for

fs/ceph/mds_client.c +26 -1

index c30eefc0ac19..a5eafc71d976 100644
@@ -555,6 +555,29 @@ static int parse_reply_info_create(void **p, void *end,
      return -EIO;
  }
 
+static int parse_reply_info_getvxattr(void **p, void *end,
+                                   struct ceph_mds_reply_info_parsed *info,
+                                   u64 features)
+{
+     u8 struct_v, struct_compat;
+     u32 struct_len;
+     u32 value_len;
+
+     ceph_decode_8_safe(p, end, struct_v, bad);
+     ceph_decode_8_safe(p, end, struct_compat, bad);
+     ceph_decode_32_safe(p, end, struct_len, bad);
+     ceph_decode_32_safe(p, end, value_len, bad);
+
+     if (value_len == end - *p) {
+       info->xattr_info.xattr_value = *p;
+       info->xattr_info.xattr_value_len = end - *p;
+       *p = end;
+       return info->xattr_info.xattr_value_len;
+     }
+bad:
+     return -EIO;
+}
+
  /*
   * parse extra results
   */

@@ -570,6 +593,8 @@ static int parse_reply_info_extra(void **p, void *end,
              return parse_reply_info_readdir(p, end, info, features);
      else if (op == CEPH_MDS_OP_CREATE)
              return parse_reply_info_create(p, end, info, features, s);
+     else if (op == CEPH_MDS_OP_GETVXATTR)
+             return parse_reply_info_getvxattr(p, end, info, features);
      else
              return -EIO;
  }

@@ -615,7 +640,7 @@ static int parse_reply_info(struct ceph_mds_session *s, struct ceph_msg *msg,
 
      if (p != end)
              goto bad;
-     return 0;
+     return err;
 
  bad:
      err = -EIO;

fs/ceph/mds_client.h +11 -1

index 97c7f7bfa55f..f2a8e5af3c2e 100644
@@ -29,8 +29,10 @@ enum ceph_feature_type {
      CEPHFS_FEATURE_MULTI_RECONNECT,
      CEPHFS_FEATURE_DELEG_INO,
      CEPHFS_FEATURE_METRIC_COLLECT,
+     CEPHFS_FEATURE_ALTERNATE_NAME,
+     CEPHFS_FEATURE_GETVXATTR,
 
-     CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_METRIC_COLLECT,
+     CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_GETVXATTR,
  };
 
  /*

@@ -45,6 +47,8 @@ enum ceph_feature_type {
      CEPHFS_FEATURE_MULTI_RECONNECT,         \
      CEPHFS_FEATURE_DELEG_INO,               \
      CEPHFS_FEATURE_METRIC_COLLECT,          \
+     CEPHFS_FEATURE_ALTERNATE_NAME,          \
+     CEPHFS_FEATURE_GETVXATTR,               \
                                              \
      CEPHFS_FEATURE_MAX,                     \
  }

@@ -100,6 +104,11 @@ struct ceph_mds_reply_dir_entry {
      loff_t                        offset;
  };
 
+struct ceph_mds_reply_xattr {
+     char *xattr_value;
+     size_t xattr_value_len;
+};
+
  /*
   * parsed info about an mds reply, including information about
   * either: 1) the target inode and/or its parent directory and dentry,

@@ -115,6 +124,7 @@ struct ceph_mds_reply_info_parsed {
      char                          *dname;
      u32                           dname_len;
      struct ceph_mds_reply_lease   *dlease;
+     struct ceph_mds_reply_xattr   xattr_info;
 
      /* extra */
      union {

fs/ceph/strings.c +1 -0

index 573bb9556fb5..e36e8948e728 100644
@@ -60,6 +60,7 @@ const char *ceph_mds_op_name(int op)
      case CEPH_MDS_OP_LOOKUPINO:  return "lookupino";
      case CEPH_MDS_OP_LOOKUPNAME:  return "lookupname";
      case CEPH_MDS_OP_GETATTR:  return "getattr";
+     case CEPH_MDS_OP_GETVXATTR:  return "getvxattr";
      case CEPH_MDS_OP_SETXATTR: return "setxattr";
      case CEPH_MDS_OP_SETATTR: return "setattr";
      case CEPH_MDS_OP_RMXATTR: return "rmxattr";

fs/ceph/super.h +1 -0

index ac331aa07cfa..a627fa69668e 100644
@@ -1043,6 +1043,7 @@ static inline bool ceph_inode_is_shutdown(struct inode *inode)
 
  /* xattr.c */
  int __ceph_setxattr(struct inode *, const char *, const void *, size_t, int);
+int ceph_do_getvxattr(struct inode *inode, const char *name, void *value, size_t size);
  ssize_t __ceph_getxattr(struct inode *, const char *, void *, size_t);
  extern ssize_t ceph_listxattr(struct dentry *, char *, size_t);
  extern struct ceph_buffer *__ceph_build_xattrs_blob(struct ceph_inode_info *ci);

fs/ceph/xattr.c +14 -0

index fcf7dfdecf96..dc32876a541a 100644
@@ -918,6 +918,30 @@ static inline int __get_request_mask(struct inode *in) {
      return mask;
  }
 
+/* check if the entire cluster supports the given feature */
+static inline bool ceph_cluster_has_feature(struct inode *inode, int feature_bit)
+{
+     int64_t i;
+     struct ceph_fs_client *fsc = ceph_inode_to_client(inode);
+     struct ceph_mds_session **sessions = fsc->mdsc->sessions;
+     int64_t num_sessions = atomic_read(&fsc->mdsc->num_sessions);
+
+     if (fsc->mdsc->stopping)
+             return false;
+
+     if (!sessions)
+             return false;
+

Quoted 8 lines

fs/ceph/xattr.c static inline int __get_request_mask(struct inode *in) { · line 939

Quoted 5 lines

fs/ceph/xattr.c +5 -0

+     for (i = 0; i < num_sessions; i++) {
+             struct ceph_mds_session *session = sessions[i];
+             if (!session)
+                     return false;
+             if (!test_bit(feature_bit, &session->s_features))

This will be possibly cause crash because, and you should put the whole for loop and 'fsc->mdsc->xxx' above under the 'mdsc->mutex'.

Thanks for pointing this out. I had a suspicion that the code needed to be wrapped
within some lock guard.

Quoted 14 lines

Quoted 14 lines

fs/ceph/xattr.c +5 -0

+                     return false;
+     }
+     return true;
+}
+
  ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value,
                    size_t size)
  {

@@ -927,6 +951,16 @@ ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value,
      int req_mask;
      ssize_t err;
 

Quoted 13 lines

fs/ceph/xattr.c ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, · line 963

Quoted 10 lines

fs/ceph/xattr.c +10 -0

+     if (!strncmp(name, XATTR_CEPH_PREFIX, XATTR_CEPH_PREFIX_LEN) &&
+         ceph_cluster_has_feature(inode, CEPHFS_FEATURE_GETVXATTR)) {
+             err = ceph_do_getvxattr(inode, name, value, size);
+             /* if cluster doesn't support xattr, we try to service it
+              * locally
+              */
+             if (err >= 0)
+                     return err;
+     }
+

If the 'Fa' or 'Fr' caps is issued to kclient, could we get this vxattr locally instead of getting it from MDS every time ?

The new mechanism is meant to supersede the old one.
1. The new layout output format is JSON

2. The new mechanism also recursively resolves the layout to the closest ancestor

3. The new mechanism deals with ceph vxattrs exclusively at the server end (currently getvxattr handles only a limited subset of all the vxattrs)
4. There's no way to fetch ceph.dir.pin* vxattrs locally
(see https://github.com/ceph/ceph/pull/42001 for userspace work)

Quoted 19 lines

Quoted 18 lines

fs/ceph/xattr.c +0 -0

      /* let's see if a virtual xattr was requested */
      vxattr = ceph_match_vxattr(inode, name);
      if (vxattr) {

include/linux/ceph/ceph_fs.h +1 -0

index 7ad6c3d0db7d..66db21ac5f0c 100644
@@ -328,6 +328,7 @@ enum {
      CEPH_MDS_OP_LOOKUPPARENT = 0x00103,
      CEPH_MDS_OP_LOOKUPINO  = 0x00104,
      CEPH_MDS_OP_LOOKUPNAME = 0x00105,
+     CEPH_MDS_OP_GETVXATTR  = 0x00106,
 
      CEPH_MDS_OP_SETXATTR   = 0x01105,
      CEPH_MDS_OP_RMXATTR    = 0x01106,