diff mbox series

[v3] ceph: fail the request if the peer MDS doesn't support getvxattr op

Message ID 20220808070823.707829-1-xiubli@redhat.com (mailing list archive)
State New, archived
Headers show
Series [v3] ceph: fail the request if the peer MDS doesn't support getvxattr op | expand

Commit Message

Xiubo Li Aug. 8, 2022, 7:08 a.m. UTC
From: Xiubo Li <xiubli@redhat.com>

Just fail the request instead sending the request out, or the peer
MDS will crash.

URL: https://tracker.ceph.com/issues/56529
Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
 fs/ceph/inode.c      |  1 +
 fs/ceph/mds_client.c | 11 +++++++++++
 fs/ceph/mds_client.h |  6 +++++-
 3 files changed, 17 insertions(+), 1 deletion(-)

Comments

Luis Henriques Aug. 9, 2022, 10:07 a.m. UTC | #1
On Mon, Aug 08, 2022 at 03:08:23PM +0800, xiubli@redhat.com wrote:
> From: Xiubo Li <xiubli@redhat.com>
> 
> Just fail the request instead sending the request out, or the peer
> MDS will crash.
> 
> URL: https://tracker.ceph.com/issues/56529
> Signed-off-by: Xiubo Li <xiubli@redhat.com>
> ---
>  fs/ceph/inode.c      |  1 +
>  fs/ceph/mds_client.c | 11 +++++++++++
>  fs/ceph/mds_client.h |  6 +++++-
>  3 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index 79ff197c7cc5..cfa0b550eef2 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -2607,6 +2607,7 @@ int ceph_do_getvxattr(struct inode *inode, const char *name, void *value,
>  		goto out;
>  	}
>  
> +	req->r_feature_needed = CEPHFS_FEATURE_OP_GETVXATTR;
>  	req->r_path2 = kstrdup(name, GFP_NOFS);
>  	if (!req->r_path2) {
>  		err = -ENOMEM;
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index 598012ddc401..3e22783109ad 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -2501,6 +2501,7 @@ ceph_mdsc_create_request(struct ceph_mds_client *mdsc, int op, int mode)
>  	INIT_LIST_HEAD(&req->r_unsafe_dir_item);
>  	INIT_LIST_HEAD(&req->r_unsafe_target_item);
>  	req->r_fmode = -1;
> +	req->r_feature_needed = -1;
>  	kref_init(&req->r_kref);
>  	RB_CLEAR_NODE(&req->r_node);
>  	INIT_LIST_HEAD(&req->r_wait);
> @@ -3255,6 +3256,16 @@ static void __do_request(struct ceph_mds_client *mdsc,
>  
>  	dout("do_request mds%d session %p state %s\n", mds, session,
>  	     ceph_session_state_name(session->s_state));
> +
> +	/*
> +	 * The old ceph will crash the MDSs when see unknown OPs
> +	 */
> +	if (req->r_feature_needed > 0 &&
> +	    !test_bit(req->r_feature_needed, &session->s_features)) {
> +		err = -ENODATA;
> +		goto out_session;
> +	}

This patch looks good to me.  The only thing I would have preferred would
be to have ->r_feature_needed defined as an unsigned and initialised to
zero (instead of -1).  But that's me, so feel free to ignore this nit.

Cheers,
--
Luís

> +
>  	if (session->s_state != CEPH_MDS_SESSION_OPEN &&
>  	    session->s_state != CEPH_MDS_SESSION_HUNG) {
>  		/*
> diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
> index e15ee2858fef..728b7d72bf76 100644
> --- a/fs/ceph/mds_client.h
> +++ b/fs/ceph/mds_client.h
> @@ -31,8 +31,9 @@ enum ceph_feature_type {
>  	CEPHFS_FEATURE_METRIC_COLLECT,
>  	CEPHFS_FEATURE_ALTERNATE_NAME,
>  	CEPHFS_FEATURE_NOTIFY_SESSION_STATE,
> +	CEPHFS_FEATURE_OP_GETVXATTR,
>  
> -	CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_NOTIFY_SESSION_STATE,
> +	CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_OP_GETVXATTR,
>  };
>  
>  #define CEPHFS_FEATURES_CLIENT_SUPPORTED {	\
> @@ -45,6 +46,7 @@ enum ceph_feature_type {
>  	CEPHFS_FEATURE_METRIC_COLLECT,		\
>  	CEPHFS_FEATURE_ALTERNATE_NAME,		\
>  	CEPHFS_FEATURE_NOTIFY_SESSION_STATE,	\
> +	CEPHFS_FEATURE_OP_GETVXATTR,		\
>  }
>  
>  /*
> @@ -352,6 +354,8 @@ struct ceph_mds_request {
>  	long long	  r_dir_ordered_cnt;
>  	int		  r_readdir_cache_idx;
>  
> +	int		  r_feature_needed;
> +
>  	struct ceph_cap_reservation r_caps_reservation;
>  };
>  
> -- 
> 2.36.0.rc1
>
Xiubo Li Aug. 9, 2022, 10:16 a.m. UTC | #2
On 8/9/22 6:07 PM, Luís Henriques wrote:
> On Mon, Aug 08, 2022 at 03:08:23PM +0800, xiubli@redhat.com wrote:
>> From: Xiubo Li <xiubli@redhat.com>
>>
>> Just fail the request instead sending the request out, or the peer
>> MDS will crash.
>>
>> URL: https://tracker.ceph.com/issues/56529
>> Signed-off-by: Xiubo Li <xiubli@redhat.com>
>> ---
>>   fs/ceph/inode.c      |  1 +
>>   fs/ceph/mds_client.c | 11 +++++++++++
>>   fs/ceph/mds_client.h |  6 +++++-
>>   3 files changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
>> index 79ff197c7cc5..cfa0b550eef2 100644
>> --- a/fs/ceph/inode.c
>> +++ b/fs/ceph/inode.c
>> @@ -2607,6 +2607,7 @@ int ceph_do_getvxattr(struct inode *inode, const char *name, void *value,
>>   		goto out;
>>   	}
>>   
>> +	req->r_feature_needed = CEPHFS_FEATURE_OP_GETVXATTR;
>>   	req->r_path2 = kstrdup(name, GFP_NOFS);
>>   	if (!req->r_path2) {
>>   		err = -ENOMEM;
>> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
>> index 598012ddc401..3e22783109ad 100644
>> --- a/fs/ceph/mds_client.c
>> +++ b/fs/ceph/mds_client.c
>> @@ -2501,6 +2501,7 @@ ceph_mdsc_create_request(struct ceph_mds_client *mdsc, int op, int mode)
>>   	INIT_LIST_HEAD(&req->r_unsafe_dir_item);
>>   	INIT_LIST_HEAD(&req->r_unsafe_target_item);
>>   	req->r_fmode = -1;
>> +	req->r_feature_needed = -1;
>>   	kref_init(&req->r_kref);
>>   	RB_CLEAR_NODE(&req->r_node);
>>   	INIT_LIST_HEAD(&req->r_wait);
>> @@ -3255,6 +3256,16 @@ static void __do_request(struct ceph_mds_client *mdsc,
>>   
>>   	dout("do_request mds%d session %p state %s\n", mds, session,
>>   	     ceph_session_state_name(session->s_state));
>> +
>> +	/*
>> +	 * The old ceph will crash the MDSs when see unknown OPs
>> +	 */
>> +	if (req->r_feature_needed > 0 &&
>> +	    !test_bit(req->r_feature_needed, &session->s_features)) {
>> +		err = -ENODATA;
>> +		goto out_session;
>> +	}
> This patch looks good to me.  The only thing I would have preferred would
> be to have ->r_feature_needed defined as an unsigned and initialised to
> zero (instead of -1).  But that's me, so feel free to ignore this nit.

I am just following the 'test_bit()':

    9    132  include/asm-generic/bitops/instrumented-non-atomic.h 
<<test_bit>>
              static inline bool test_bit(long nr, const volatile 
unsigned long *addr)
   10    104  include/asm-generic/bitops/non-atomic.h <<test_bit>>
              static inline int test_bit(int nr, const volatile unsigned 
long *addr)

Which is a signed type. If we use a unsigned, won't compiler complain 
about it ?

BRs

-- Xiubo

> Cheers,
> --
> Luís
>
>> +
>>   	if (session->s_state != CEPH_MDS_SESSION_OPEN &&
>>   	    session->s_state != CEPH_MDS_SESSION_HUNG) {
>>   		/*
>> diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
>> index e15ee2858fef..728b7d72bf76 100644
>> --- a/fs/ceph/mds_client.h
>> +++ b/fs/ceph/mds_client.h
>> @@ -31,8 +31,9 @@ enum ceph_feature_type {
>>   	CEPHFS_FEATURE_METRIC_COLLECT,
>>   	CEPHFS_FEATURE_ALTERNATE_NAME,
>>   	CEPHFS_FEATURE_NOTIFY_SESSION_STATE,
>> +	CEPHFS_FEATURE_OP_GETVXATTR,
>>   
>> -	CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_NOTIFY_SESSION_STATE,
>> +	CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_OP_GETVXATTR,
>>   };
>>   
>>   #define CEPHFS_FEATURES_CLIENT_SUPPORTED {	\
>> @@ -45,6 +46,7 @@ enum ceph_feature_type {
>>   	CEPHFS_FEATURE_METRIC_COLLECT,		\
>>   	CEPHFS_FEATURE_ALTERNATE_NAME,		\
>>   	CEPHFS_FEATURE_NOTIFY_SESSION_STATE,	\
>> +	CEPHFS_FEATURE_OP_GETVXATTR,		\
>>   }
>>   
>>   /*
>> @@ -352,6 +354,8 @@ struct ceph_mds_request {
>>   	long long	  r_dir_ordered_cnt;
>>   	int		  r_readdir_cache_idx;
>>   
>> +	int		  r_feature_needed;
>> +
>>   	struct ceph_cap_reservation r_caps_reservation;
>>   };
>>   
>> -- 
>> 2.36.0.rc1
>>
Luis Henriques Aug. 9, 2022, 12:53 p.m. UTC | #3
On Tue, Aug 09, 2022 at 06:16:36PM +0800, Xiubo Li wrote:
> 
> On 8/9/22 6:07 PM, Luís Henriques wrote:
> > On Mon, Aug 08, 2022 at 03:08:23PM +0800, xiubli@redhat.com wrote:
> > > From: Xiubo Li <xiubli@redhat.com>
> > > 
> > > Just fail the request instead sending the request out, or the peer
> > > MDS will crash.
> > > 
> > > URL: https://tracker.ceph.com/issues/56529
> > > Signed-off-by: Xiubo Li <xiubli@redhat.com>
> > > ---
> > >   fs/ceph/inode.c      |  1 +
> > >   fs/ceph/mds_client.c | 11 +++++++++++
> > >   fs/ceph/mds_client.h |  6 +++++-
> > >   3 files changed, 17 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> > > index 79ff197c7cc5..cfa0b550eef2 100644
> > > --- a/fs/ceph/inode.c
> > > +++ b/fs/ceph/inode.c
> > > @@ -2607,6 +2607,7 @@ int ceph_do_getvxattr(struct inode *inode, const char *name, void *value,
> > >   		goto out;
> > >   	}
> > > +	req->r_feature_needed = CEPHFS_FEATURE_OP_GETVXATTR;
> > >   	req->r_path2 = kstrdup(name, GFP_NOFS);
> > >   	if (!req->r_path2) {
> > >   		err = -ENOMEM;
> > > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> > > index 598012ddc401..3e22783109ad 100644
> > > --- a/fs/ceph/mds_client.c
> > > +++ b/fs/ceph/mds_client.c
> > > @@ -2501,6 +2501,7 @@ ceph_mdsc_create_request(struct ceph_mds_client *mdsc, int op, int mode)
> > >   	INIT_LIST_HEAD(&req->r_unsafe_dir_item);
> > >   	INIT_LIST_HEAD(&req->r_unsafe_target_item);
> > >   	req->r_fmode = -1;
> > > +	req->r_feature_needed = -1;
> > >   	kref_init(&req->r_kref);
> > >   	RB_CLEAR_NODE(&req->r_node);
> > >   	INIT_LIST_HEAD(&req->r_wait);
> > > @@ -3255,6 +3256,16 @@ static void __do_request(struct ceph_mds_client *mdsc,
> > >   	dout("do_request mds%d session %p state %s\n", mds, session,
> > >   	     ceph_session_state_name(session->s_state));
> > > +
> > > +	/*
> > > +	 * The old ceph will crash the MDSs when see unknown OPs
> > > +	 */
> > > +	if (req->r_feature_needed > 0 &&
> > > +	    !test_bit(req->r_feature_needed, &session->s_features)) {
> > > +		err = -ENODATA;
> > > +		goto out_session;
> > > +	}
> > This patch looks good to me.  The only thing I would have preferred would
> > be to have ->r_feature_needed defined as an unsigned and initialised to
> > zero (instead of -1).  But that's me, so feel free to ignore this nit.
> 
> I am just following the 'test_bit()':
> 
>    9    132  include/asm-generic/bitops/instrumented-non-atomic.h
> <<test_bit>>
>              static inline bool test_bit(long nr, const volatile unsigned
> long *addr)
>   10    104  include/asm-generic/bitops/non-atomic.h <<test_bit>>
>              static inline int test_bit(int nr, const volatile unsigned long
> *addr)
> 
> Which is a signed type. If we use a unsigned, won't compiler complain about
> it ?
> 

Oh, yeah you're right.  And now that I think about it, I think I've been
there already in the past.  Sorry for the noise.

Cheers,
--
Luís

> BRs
> 
> -- Xiubo
> 
> > Cheers,
> > --
> > Luís
> > 
> > > +
> > >   	if (session->s_state != CEPH_MDS_SESSION_OPEN &&
> > >   	    session->s_state != CEPH_MDS_SESSION_HUNG) {
> > >   		/*
> > > diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
> > > index e15ee2858fef..728b7d72bf76 100644
> > > --- a/fs/ceph/mds_client.h
> > > +++ b/fs/ceph/mds_client.h
> > > @@ -31,8 +31,9 @@ enum ceph_feature_type {
> > >   	CEPHFS_FEATURE_METRIC_COLLECT,
> > >   	CEPHFS_FEATURE_ALTERNATE_NAME,
> > >   	CEPHFS_FEATURE_NOTIFY_SESSION_STATE,
> > > +	CEPHFS_FEATURE_OP_GETVXATTR,
> > > -	CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_NOTIFY_SESSION_STATE,
> > > +	CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_OP_GETVXATTR,
> > >   };
> > >   #define CEPHFS_FEATURES_CLIENT_SUPPORTED {	\
> > > @@ -45,6 +46,7 @@ enum ceph_feature_type {
> > >   	CEPHFS_FEATURE_METRIC_COLLECT,		\
> > >   	CEPHFS_FEATURE_ALTERNATE_NAME,		\
> > >   	CEPHFS_FEATURE_NOTIFY_SESSION_STATE,	\
> > > +	CEPHFS_FEATURE_OP_GETVXATTR,		\
> > >   }
> > >   /*
> > > @@ -352,6 +354,8 @@ struct ceph_mds_request {
> > >   	long long	  r_dir_ordered_cnt;
> > >   	int		  r_readdir_cache_idx;
> > > +	int		  r_feature_needed;
> > > +
> > >   	struct ceph_cap_reservation r_caps_reservation;
> > >   };
> > > -- 
> > > 2.36.0.rc1
> > > 
>
Xiubo Li Aug. 9, 2022, 12:55 p.m. UTC | #4
On 8/9/22 8:53 PM, Luís Henriques wrote:
> On Tue, Aug 09, 2022 at 06:16:36PM +0800, Xiubo Li wrote:
>> On 8/9/22 6:07 PM, Luís Henriques wrote:
>>> On Mon, Aug 08, 2022 at 03:08:23PM +0800, xiubli@redhat.com wrote:
>>>> From: Xiubo Li <xiubli@redhat.com>
>>>>
>>>> Just fail the request instead sending the request out, or the peer
>>>> MDS will crash.
>>>>
>>>> URL: https://tracker.ceph.com/issues/56529
>>>> Signed-off-by: Xiubo Li <xiubli@redhat.com>
>>>> ---
>>>>    fs/ceph/inode.c      |  1 +
>>>>    fs/ceph/mds_client.c | 11 +++++++++++
>>>>    fs/ceph/mds_client.h |  6 +++++-
>>>>    3 files changed, 17 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
>>>> index 79ff197c7cc5..cfa0b550eef2 100644
>>>> --- a/fs/ceph/inode.c
>>>> +++ b/fs/ceph/inode.c
>>>> @@ -2607,6 +2607,7 @@ int ceph_do_getvxattr(struct inode *inode, const char *name, void *value,
>>>>    		goto out;
>>>>    	}
>>>> +	req->r_feature_needed = CEPHFS_FEATURE_OP_GETVXATTR;
>>>>    	req->r_path2 = kstrdup(name, GFP_NOFS);
>>>>    	if (!req->r_path2) {
>>>>    		err = -ENOMEM;
>>>> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
>>>> index 598012ddc401..3e22783109ad 100644
>>>> --- a/fs/ceph/mds_client.c
>>>> +++ b/fs/ceph/mds_client.c
>>>> @@ -2501,6 +2501,7 @@ ceph_mdsc_create_request(struct ceph_mds_client *mdsc, int op, int mode)
>>>>    	INIT_LIST_HEAD(&req->r_unsafe_dir_item);
>>>>    	INIT_LIST_HEAD(&req->r_unsafe_target_item);
>>>>    	req->r_fmode = -1;
>>>> +	req->r_feature_needed = -1;
>>>>    	kref_init(&req->r_kref);
>>>>    	RB_CLEAR_NODE(&req->r_node);
>>>>    	INIT_LIST_HEAD(&req->r_wait);
>>>> @@ -3255,6 +3256,16 @@ static void __do_request(struct ceph_mds_client *mdsc,
>>>>    	dout("do_request mds%d session %p state %s\n", mds, session,
>>>>    	     ceph_session_state_name(session->s_state));
>>>> +
>>>> +	/*
>>>> +	 * The old ceph will crash the MDSs when see unknown OPs
>>>> +	 */
>>>> +	if (req->r_feature_needed > 0 &&
>>>> +	    !test_bit(req->r_feature_needed, &session->s_features)) {
>>>> +		err = -ENODATA;
>>>> +		goto out_session;
>>>> +	}
>>> This patch looks good to me.  The only thing I would have preferred would
>>> be to have ->r_feature_needed defined as an unsigned and initialised to
>>> zero (instead of -1).  But that's me, so feel free to ignore this nit.
>> I am just following the 'test_bit()':
>>
>>     9    132  include/asm-generic/bitops/instrumented-non-atomic.h
>> <<test_bit>>
>>               static inline bool test_bit(long nr, const volatile unsigned
>> long *addr)
>>    10    104  include/asm-generic/bitops/non-atomic.h <<test_bit>>
>>               static inline int test_bit(int nr, const volatile unsigned long
>> *addr)
>>
>> Which is a signed type. If we use a unsigned, won't compiler complain about
>> it ?
>>
> Oh, yeah you're right.  And now that I think about it, I think I've been
> there already in the past.  Sorry for the noise.

No worry about that.

Thanks very much Luis for your reviewing of this !

-- Xiubo


> Cheers,
> --
> Luís
>
>> BRs
>>
>> -- Xiubo
>>
>>> Cheers,
>>> --
>>> Luís
>>>
>>>> +
>>>>    	if (session->s_state != CEPH_MDS_SESSION_OPEN &&
>>>>    	    session->s_state != CEPH_MDS_SESSION_HUNG) {
>>>>    		/*
>>>> diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
>>>> index e15ee2858fef..728b7d72bf76 100644
>>>> --- a/fs/ceph/mds_client.h
>>>> +++ b/fs/ceph/mds_client.h
>>>> @@ -31,8 +31,9 @@ enum ceph_feature_type {
>>>>    	CEPHFS_FEATURE_METRIC_COLLECT,
>>>>    	CEPHFS_FEATURE_ALTERNATE_NAME,
>>>>    	CEPHFS_FEATURE_NOTIFY_SESSION_STATE,
>>>> +	CEPHFS_FEATURE_OP_GETVXATTR,
>>>> -	CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_NOTIFY_SESSION_STATE,
>>>> +	CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_OP_GETVXATTR,
>>>>    };
>>>>    #define CEPHFS_FEATURES_CLIENT_SUPPORTED {	\
>>>> @@ -45,6 +46,7 @@ enum ceph_feature_type {
>>>>    	CEPHFS_FEATURE_METRIC_COLLECT,		\
>>>>    	CEPHFS_FEATURE_ALTERNATE_NAME,		\
>>>>    	CEPHFS_FEATURE_NOTIFY_SESSION_STATE,	\
>>>> +	CEPHFS_FEATURE_OP_GETVXATTR,		\
>>>>    }
>>>>    /*
>>>> @@ -352,6 +354,8 @@ struct ceph_mds_request {
>>>>    	long long	  r_dir_ordered_cnt;
>>>>    	int		  r_readdir_cache_idx;
>>>> +	int		  r_feature_needed;
>>>> +
>>>>    	struct ceph_cap_reservation r_caps_reservation;
>>>>    };
>>>> -- 
>>>> 2.36.0.rc1
>>>>
diff mbox series

Patch

diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index 79ff197c7cc5..cfa0b550eef2 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -2607,6 +2607,7 @@  int ceph_do_getvxattr(struct inode *inode, const char *name, void *value,
 		goto out;
 	}
 
+	req->r_feature_needed = CEPHFS_FEATURE_OP_GETVXATTR;
 	req->r_path2 = kstrdup(name, GFP_NOFS);
 	if (!req->r_path2) {
 		err = -ENOMEM;
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 598012ddc401..3e22783109ad 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -2501,6 +2501,7 @@  ceph_mdsc_create_request(struct ceph_mds_client *mdsc, int op, int mode)
 	INIT_LIST_HEAD(&req->r_unsafe_dir_item);
 	INIT_LIST_HEAD(&req->r_unsafe_target_item);
 	req->r_fmode = -1;
+	req->r_feature_needed = -1;
 	kref_init(&req->r_kref);
 	RB_CLEAR_NODE(&req->r_node);
 	INIT_LIST_HEAD(&req->r_wait);
@@ -3255,6 +3256,16 @@  static void __do_request(struct ceph_mds_client *mdsc,
 
 	dout("do_request mds%d session %p state %s\n", mds, session,
 	     ceph_session_state_name(session->s_state));
+
+	/*
+	 * The old ceph will crash the MDSs when see unknown OPs
+	 */
+	if (req->r_feature_needed > 0 &&
+	    !test_bit(req->r_feature_needed, &session->s_features)) {
+		err = -ENODATA;
+		goto out_session;
+	}
+
 	if (session->s_state != CEPH_MDS_SESSION_OPEN &&
 	    session->s_state != CEPH_MDS_SESSION_HUNG) {
 		/*
diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
index e15ee2858fef..728b7d72bf76 100644
--- a/fs/ceph/mds_client.h
+++ b/fs/ceph/mds_client.h
@@ -31,8 +31,9 @@  enum ceph_feature_type {
 	CEPHFS_FEATURE_METRIC_COLLECT,
 	CEPHFS_FEATURE_ALTERNATE_NAME,
 	CEPHFS_FEATURE_NOTIFY_SESSION_STATE,
+	CEPHFS_FEATURE_OP_GETVXATTR,
 
-	CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_NOTIFY_SESSION_STATE,
+	CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_OP_GETVXATTR,
 };
 
 #define CEPHFS_FEATURES_CLIENT_SUPPORTED {	\
@@ -45,6 +46,7 @@  enum ceph_feature_type {
 	CEPHFS_FEATURE_METRIC_COLLECT,		\
 	CEPHFS_FEATURE_ALTERNATE_NAME,		\
 	CEPHFS_FEATURE_NOTIFY_SESSION_STATE,	\
+	CEPHFS_FEATURE_OP_GETVXATTR,		\
 }
 
 /*
@@ -352,6 +354,8 @@  struct ceph_mds_request {
 	long long	  r_dir_ordered_cnt;
 	int		  r_readdir_cache_idx;
 
+	int		  r_feature_needed;
+
 	struct ceph_cap_reservation r_caps_reservation;
 };