diff mbox series

[V1,net-next,02/11] net: ena: ethtool: add extra properties retrieval via get_priv_flags

Message ID 20190529095004.13341-3-sameehj@amazon.com
State Changes Requested
Delegated to: David Miller
Headers show
Series Extending the ena driver to support new features and enhance performance | expand

Commit Message

Jubran, Samih May 29, 2019, 9:49 a.m. UTC 
From: Arthur Kiyanovski <akiyano@amazon.com>

This commit adds a mechanism for exposing different driver
properties via ethtool's priv_flags.

In this commit we:

Add commands, structs and defines necessary for handling
extra properties

Add functions for:
Allocation/destruction of a buffer for extra properties strings.
Retreival of extra properties strings and flags from the network device.

Handle the allocation of a buffer for extra properties strings.

* Initialize buffer with extra properties strings from the
  network device at driver startup.

Use ethtool's get_priv_flags to expose extra properties of
the ENA device

Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com>
Signed-off-by: Sameeh Jubran <sameehj@amazon.com>
---
 .../net/ethernet/amazon/ena/ena_admin_defs.h  | 16 ++++
 drivers/net/ethernet/amazon/ena/ena_com.c     | 56 ++++++++++++++
 drivers/net/ethernet/amazon/ena/ena_com.h     | 32 ++++++++
 drivers/net/ethernet/amazon/ena/ena_ethtool.c | 77 ++++++++++++++++---
 drivers/net/ethernet/amazon/ena/ena_netdev.c  | 14 ++++
 drivers/net/ethernet/amazon/ena/ena_netdev.h  |  2 +
 6 files changed, 186 insertions(+), 11 deletions(-)

Comments

Jakub Kicinski May 29, 2019, 10:09 p.m. UTC | #1 
On Wed, 29 May 2019 12:49:55 +0300, sameehj@amazon.com wrote:
> From: Arthur Kiyanovski <akiyano@amazon.com>
> 
> This commit adds a mechanism for exposing different driver
> properties via ethtool's priv_flags.
> 
> In this commit we:
> 
> Add commands, structs and defines necessary for handling
> extra properties
> 
> Add functions for:
> Allocation/destruction of a buffer for extra properties strings.
> Retreival of extra properties strings and flags from the network device.
> 
> Handle the allocation of a buffer for extra properties strings.
> 
> * Initialize buffer with extra properties strings from the
>   network device at driver startup.
> 
> Use ethtool's get_priv_flags to expose extra properties of
> the ENA device
> 
> Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com>
> Signed-off-by: Sameeh Jubran <sameehj@amazon.com>

This commit DMAs in the string set blindly from the FW and exposes it
to user space, without any interpretation by the driver, correct?
Making the driver a mere proxy for the FW.  I think it should be clearly
mentioned in the commit message, to make sure we know what what we are
accepting here.  I'm always a little uncomfortable with such changes :)
(I'm not actually sure there is a precedent for this).
Kiyanovski, Arthur May 30, 2019, 12:21 p.m. UTC | #2 
> -----Original Message-----
> From: Jakub Kicinski <jakub.kicinski@netronome.com>
> Sent: Thursday, May 30, 2019 1:10 AM
> To: Jubran, Samih <sameehj@amazon.com>
> Cc: davem@davemloft.net; netdev@vger.kernel.org; Kiyanovski, Arthur
> <akiyano@amazon.com>; Woodhouse, David <dwmw@amazon.co.uk>;
> Machulsky, Zorik <zorik@amazon.com>; Matushevsky, Alexander
> <matua@amazon.com>; Bshara, Saeed <saeedb@amazon.com>; Wilson, Matt
> <msw@amazon.com>; Liguori, Anthony <aliguori@amazon.com>; Bshara,
> Nafea <nafea@amazon.com>; Tzalik, Guy <gtzalik@amazon.com>; Belgazal,
> Netanel <netanel@amazon.com>; Saidi, Ali <alisaidi@amazon.com>;
> Herrenschmidt, Benjamin <benh@amazon.com>
> Subject: Re: [PATCH V1 net-next 02/11] net: ena: ethtool: add extra properties
> retrieval via get_priv_flags
> 
> On Wed, 29 May 2019 12:49:55 +0300, sameehj@amazon.com wrote:
> > From: Arthur Kiyanovski <akiyano@amazon.com>
> >
> > This commit adds a mechanism for exposing different driver properties
> > via ethtool's priv_flags.
> >
> > In this commit we:
> >
> > Add commands, structs and defines necessary for handling extra
> > properties
> >
> > Add functions for:
> > Allocation/destruction of a buffer for extra properties strings.
> > Retreival of extra properties strings and flags from the network device.
> >
> > Handle the allocation of a buffer for extra properties strings.
> >
> > * Initialize buffer with extra properties strings from the
> >   network device at driver startup.
> >
> > Use ethtool's get_priv_flags to expose extra properties of the ENA
> > device
> >
> > Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com>
> > Signed-off-by: Sameeh Jubran <sameehj@amazon.com>
> 
> This commit DMAs in the string set blindly from the FW and exposes it to user
> space, without any interpretation by the driver, correct?
> Making the driver a mere proxy for the FW.  I think it should be clearly
> mentioned in the commit message, to make sure we know what what we are
> accepting here.  I'm always a little uncomfortable with such changes :) (I'm not
> actually sure there is a precedent for this).

Ack. We will update the commit message in the next version of the patch with more information.
David Miller May 30, 2019, 7:41 p.m. UTC | #3 
From: <sameehj@amazon.com>
Date: Wed, 29 May 2019 12:49:55 +0300

> @@ -560,6 +564,14 @@ struct ena_admin_set_feature_mtu_desc {
>  	u32 mtu;
>  };
>  
> +struct ena_admin_get_extra_properties_strings_desc {
> +	u32 count;
> +};
> +
> +struct ena_admin_get_extra_properties_flags_desc {
> +	u32 flags;
> +};

These single entry structures are a big overkill.  If anything just do one
which is like "ena_value_desc" and has that "u32 val;"
Michal Kubecek May 31, 2019, 8:19 a.m. UTC | #4 
On Wed, May 29, 2019 at 12:49:55PM +0300, sameehj@amazon.com wrote:
> From: Arthur Kiyanovski <akiyano@amazon.com>
> 
> This commit adds a mechanism for exposing different driver
> properties via ethtool's priv_flags.
> 
> In this commit we:
> 
> Add commands, structs and defines necessary for handling
> extra properties
> 
> Add functions for:
> Allocation/destruction of a buffer for extra properties strings.
> Retreival of extra properties strings and flags from the network device.
> 
> Handle the allocation of a buffer for extra properties strings.
> 
> * Initialize buffer with extra properties strings from the
>   network device at driver startup.
> 
> Use ethtool's get_priv_flags to expose extra properties of
> the ENA device
> 
> Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com>
> Signed-off-by: Sameeh Jubran <sameehj@amazon.com>
> ---
...
> diff --git a/drivers/net/ethernet/amazon/ena/ena_com.c b/drivers/net/ethernet/amazon/ena/ena_com.c
> index bd0d785b2..935e8fa8d 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_com.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_com.c
> @@ -1877,6 +1877,62 @@ int ena_com_get_link_params(struct ena_com_dev *ena_dev,
>  	return ena_com_get_feature(ena_dev, resp, ENA_ADMIN_LINK_CONFIG);
>  }
>  
> +int ena_com_extra_properties_strings_init(struct ena_com_dev *ena_dev)
> +{
> +	struct ena_admin_get_feat_resp resp;
> +	struct ena_extra_properties_strings *extra_properties_strings =
> +			&ena_dev->extra_properties_strings;
> +	u32 rc;
> +
> +	extra_properties_strings->size = ENA_ADMIN_EXTRA_PROPERTIES_COUNT *
> +		ENA_ADMIN_EXTRA_PROPERTIES_STRING_LEN;
> +
> +	extra_properties_strings->virt_addr =
> +		dma_alloc_coherent(ena_dev->dmadev,
> +				   extra_properties_strings->size,
> +				   &extra_properties_strings->dma_addr,
> +				   GFP_KERNEL);

Do you need to fetch the private flag names on each ETHTOOL_GSTRING
request? I suppose they could change e.g. on a firmware update but then
even the count could change which you do not seem to handle. Is there
a reason not to fetch the names once on init rather then accessing the
device memory each time?

My point is that ethtool_ops::get_strings() does not return a value
which indicates that it's supposed to be a trivial operation which
cannot fail, usually a simple copy within kernel memory.

> +	if (unlikely(!extra_properties_strings->virt_addr)) {
> +		pr_err("Failed to allocate extra properties strings\n");
> +		return 0;
> +	}
> +
> +	rc = ena_com_get_feature_ex(ena_dev, &resp,
> +				    ENA_ADMIN_EXTRA_PROPERTIES_STRINGS,
> +				    extra_properties_strings->dma_addr,
> +				    extra_properties_strings->size);
> +	if (rc) {
> +		pr_debug("Failed to get extra properties strings\n");
> +		goto err;
> +	}
> +
> +	return resp.u.extra_properties_strings.count;
> +err:
> +	ena_com_delete_extra_properties_strings(ena_dev);
> +	return 0;
> +}
...
> diff --git a/drivers/net/ethernet/amazon/ena/ena_ethtool.c b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
> index fe596bc30..65bc5a2b2 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_ethtool.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
...
> +static void get_private_flags_strings(struct ena_adapter *adapter, u8 *data)
> +{
> +	struct ena_com_dev *ena_dev = adapter->ena_dev;
> +	u8 *strings = ena_dev->extra_properties_strings.virt_addr;
> +	int i;
> +
> +	if (unlikely(!strings)) {
> +		adapter->ena_extra_properties_count = 0;
> +		netif_err(adapter, drv, adapter->netdev,
> +			  "Failed to allocate extra properties strings\n");
> +		return;
> +	}

This is a bit confusing, IMHO. I'm aware we shouldn't really get here as
with strings null, count would be zero and ethtool_get_strings()
wouldn't call the ->get_strings() callback. But if we ever do, it makes
little sense to complain about failed allocation (which happened once on
init) each time userspace makes ETHTOOL_GSTRINGS request for private
flags.

> +
> +	for (i = 0; i < adapter->ena_extra_properties_count; i++) {
> +		snprintf(data, ETH_GSTRING_LEN, "%s",
> +			 strings + ENA_ADMIN_EXTRA_PROPERTIES_STRING_LEN * i);

snprintf() is a bit overkill here, what you are doing is rather
strlcpy() or strscpy(). Or maybe strncpy() as strings returned by
->get_strings() do not have to be null terminated.

> +		data += ETH_GSTRING_LEN;
> +	}
> +}

Michal Kubecek
Machulsky, Zorik May 31, 2019, 9:57 p.m. UTC | #5 
Hi Michal,
Thanks for your comments.

On 5/31/19, 1:20 AM, "Michal Kubecek" <mkubecek@suse.cz> wrote:

    On Wed, May 29, 2019 at 12:49:55PM +0300, sameehj@amazon.com wrote:
    > From: Arthur Kiyanovski <akiyano@amazon.com>
    > 
    > This commit adds a mechanism for exposing different driver
    > properties via ethtool's priv_flags.
    > 
    > In this commit we:
    > 
    > Add commands, structs and defines necessary for handling
    > extra properties
    > 
    > Add functions for:
    > Allocation/destruction of a buffer for extra properties strings.
    > Retreival of extra properties strings and flags from the network device.
    > 
    > Handle the allocation of a buffer for extra properties strings.
    > 
    > * Initialize buffer with extra properties strings from the
    >   network device at driver startup.
    > 
    > Use ethtool's get_priv_flags to expose extra properties of
    > the ENA device
    > 
    > Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com>
    > Signed-off-by: Sameeh Jubran <sameehj@amazon.com>
    > ---
    ...
    > diff --git a/drivers/net/ethernet/amazon/ena/ena_com.c b/drivers/net/ethernet/amazon/ena/ena_com.c
    > index bd0d785b2..935e8fa8d 100644
    > --- a/drivers/net/ethernet/amazon/ena/ena_com.c
    > +++ b/drivers/net/ethernet/amazon/ena/ena_com.c
    > @@ -1877,6 +1877,62 @@ int ena_com_get_link_params(struct ena_com_dev *ena_dev,
    >  	return ena_com_get_feature(ena_dev, resp, ENA_ADMIN_LINK_CONFIG);
    >  }
    >  
    > +int ena_com_extra_properties_strings_init(struct ena_com_dev *ena_dev)
    > +{
    > +	struct ena_admin_get_feat_resp resp;
    > +	struct ena_extra_properties_strings *extra_properties_strings =
    > +			&ena_dev->extra_properties_strings;
    > +	u32 rc;
    > +
    > +	extra_properties_strings->size = ENA_ADMIN_EXTRA_PROPERTIES_COUNT *
    > +		ENA_ADMIN_EXTRA_PROPERTIES_STRING_LEN;
    > +
    > +	extra_properties_strings->virt_addr =
    > +		dma_alloc_coherent(ena_dev->dmadev,
    > +				   extra_properties_strings->size,
    > +				   &extra_properties_strings->dma_addr,
    > +				   GFP_KERNEL);
    
    Do you need to fetch the private flag names on each ETHTOOL_GSTRING
    request? I suppose they could change e.g. on a firmware update but then
    even the count could change which you do not seem to handle. Is there
    a reason not to fetch the names once on init rather then accessing the
    device memory each time?
    
    My point is that ethtool_ops::get_strings() does not return a value
    which indicates that it's supposed to be a trivial operation which
    cannot fail, usually a simple copy within kernel memory.

ena_com_extra_properties_strings_init() is called in probe() only, and not for every ETHTOOL_GSTRING
request. For the latter we use ena_get_strings(). And just to make sure we are on the same page, extra_properties_strings->virt_addr 
points to the host memory and not to the device memory. I believe this should answer your concern.
    
    > +	if (unlikely(!extra_properties_strings->virt_addr)) {
    > +		pr_err("Failed to allocate extra properties strings\n");
    > +		return 0;
    > +	}
    > +
    > +	rc = ena_com_get_feature_ex(ena_dev, &resp,
    > +				    ENA_ADMIN_EXTRA_PROPERTIES_STRINGS,
    > +				    extra_properties_strings->dma_addr,
    > +				    extra_properties_strings->size);
    > +	if (rc) {
    > +		pr_debug("Failed to get extra properties strings\n");
    > +		goto err;
    > +	}
    > +
    > +	return resp.u.extra_properties_strings.count;
    > +err:
    > +	ena_com_delete_extra_properties_strings(ena_dev);
    > +	return 0;
    > +}
    ...
    > diff --git a/drivers/net/ethernet/amazon/ena/ena_ethtool.c b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
    > index fe596bc30..65bc5a2b2 100644
    > --- a/drivers/net/ethernet/amazon/ena/ena_ethtool.c
    > +++ b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
    ...
    > +static void get_private_flags_strings(struct ena_adapter *adapter, u8 *data)
    > +{
    > +	struct ena_com_dev *ena_dev = adapter->ena_dev;
    > +	u8 *strings = ena_dev->extra_properties_strings.virt_addr;
    > +	int i;
    > +
    > +	if (unlikely(!strings)) {
    > +		adapter->ena_extra_properties_count = 0;
    > +		netif_err(adapter, drv, adapter->netdev,
    > +			  "Failed to allocate extra properties strings\n");
    > +		return;
    > +	}
    
    This is a bit confusing, IMHO. I'm aware we shouldn't really get here as
    with strings null, count would be zero and ethtool_get_strings()
    wouldn't call the ->get_strings() callback. But if we ever do, it makes
    little sense to complain about failed allocation (which happened once on
    init) each time userspace makes ETHTOOL_GSTRINGS request for private
    flags.

I believe we still want to check validity of the strings pointer to keep the driver resilient, however I agree that 
the logged message is confusing. Let us rework this commit  
    
    > +
    > +	for (i = 0; i < adapter->ena_extra_properties_count; i++) {
    > +		snprintf(data, ETH_GSTRING_LEN, "%s",
    > +			 strings + ENA_ADMIN_EXTRA_PROPERTIES_STRING_LEN * i);
    
    snprintf() is a bit overkill here, what you are doing is rather
    strlcpy() or strscpy(). Or maybe strncpy() as strings returned by
    ->get_strings() do not have to be null terminated.
    
    > +		data += ETH_GSTRING_LEN;
    > +	}
    > +}
    
    Michal Kubecek
Michal Kubecek May 31, 2019, 10:38 p.m. UTC | #6 
On Fri, May 31, 2019 at 09:57:51PM +0000, Machulsky, Zorik wrote:
>     >  
>     > +int ena_com_extra_properties_strings_init(struct ena_com_dev *ena_dev)
>     > +{
>     > +	struct ena_admin_get_feat_resp resp;
>     > +	struct ena_extra_properties_strings *extra_properties_strings =
>     > +			&ena_dev->extra_properties_strings;
>     > +	u32 rc;
>     > +
>     > +	extra_properties_strings->size = ENA_ADMIN_EXTRA_PROPERTIES_COUNT *
>     > +		ENA_ADMIN_EXTRA_PROPERTIES_STRING_LEN;
>     > +
>     > +	extra_properties_strings->virt_addr =
>     > +		dma_alloc_coherent(ena_dev->dmadev,
>     > +				   extra_properties_strings->size,
>     > +				   &extra_properties_strings->dma_addr,
>     > +				   GFP_KERNEL);
>     
>     Do you need to fetch the private flag names on each ETHTOOL_GSTRING
>     request? I suppose they could change e.g. on a firmware update but then
>     even the count could change which you do not seem to handle. Is there
>     a reason not to fetch the names once on init rather then accessing the
>     device memory each time?
>     
>     My point is that ethtool_ops::get_strings() does not return a value
>     which indicates that it's supposed to be a trivial operation which
>     cannot fail, usually a simple copy within kernel memory.
> 
> ena_com_extra_properties_strings_init() is called in probe() only, and not for every ETHTOOL_GSTRING
> request. For the latter we use ena_get_strings(). And just to make sure we are on the same page, extra_properties_strings->virt_addr 
> points to the host memory and not to the device memory. I believe this should answer your concern.

That's what I misunderstood. Sorry for the noise then.

>     > +static void get_private_flags_strings(struct ena_adapter *adapter, u8 *data)
>     > +{
>     > +	struct ena_com_dev *ena_dev = adapter->ena_dev;
>     > +	u8 *strings = ena_dev->extra_properties_strings.virt_addr;
>     > +	int i;
>     > +
>     > +	if (unlikely(!strings)) {
>     > +		adapter->ena_extra_properties_count = 0;
>     > +		netif_err(adapter, drv, adapter->netdev,
>     > +			  "Failed to allocate extra properties strings\n");
>     > +		return;
>     > +	}
>     
>     This is a bit confusing, IMHO. I'm aware we shouldn't really get here as
>     with strings null, count would be zero and ethtool_get_strings()
>     wouldn't call the ->get_strings() callback. But if we ever do, it makes
>     little sense to complain about failed allocation (which happened once on
>     init) each time userspace makes ETHTOOL_GSTRINGS request for private
>     flags.
> 
> I believe we still want to check validity of the strings pointer to keep the driver resilient, however I agree that 
> the logged message is confusing. Let us rework this commit  

Yes, I didn't question the check, only the error message.

Michal
Jubran, Samih June 3, 2019, 2:29 p.m. UTC | #7 
> -----Original Message-----
> From: David Miller <davem@davemloft.net>
> Sent: Thursday, May 30, 2019 10:42 PM
> To: Jubran, Samih <sameehj@amazon.com>
> Cc: netdev@vger.kernel.org; Kiyanovski, Arthur <akiyano@amazon.com>;
> Woodhouse, David <dwmw@amazon.co.uk>; Machulsky, Zorik
> <zorik@amazon.com>; Matushevsky, Alexander <matua@amazon.com>;
> Bshara, Saeed <saeedb@amazon.com>; Wilson, Matt <msw@amazon.com>;
> Liguori, Anthony <aliguori@amazon.com>; Bshara, Nafea
> <nafea@amazon.com>; Tzalik, Guy <gtzalik@amazon.com>; Belgazal,
> Netanel <netanel@amazon.com>; Saidi, Ali <alisaidi@amazon.com>;
> Herrenschmidt, Benjamin <benh@amazon.com>
> Subject: Re: [PATCH V1 net-next 02/11] net: ena: ethtool: add extra
> properties retrieval via get_priv_flags
> 
> From: <sameehj@amazon.com>
> Date: Wed, 29 May 2019 12:49:55 +0300
> 
> > @@ -560,6 +564,14 @@ struct ena_admin_set_feature_mtu_desc {
> >  	u32 mtu;
> >  };
> >
> > +struct ena_admin_get_extra_properties_strings_desc {
> > +	u32 count;
> > +};
> > +
> > +struct ena_admin_get_extra_properties_flags_desc {
> > +	u32 flags;
> > +};
> 
> These single entry structures are a big overkill.  If anything just do one which
> is like "ena_value_desc" and has that "u32 val;"

We think that it's better to leave it as it is, since it's more readable when the types and fields have meaningful names,  and it also leaves place for extending it in the future.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/amazon/ena/ena_admin_defs.h b/drivers/net/ethernet/amazon/ena/ena_admin_defs.h
index 82cff1e12..414bae989 100644
--- a/drivers/net/ethernet/amazon/ena/ena_admin_defs.h
+++ b/drivers/net/ethernet/amazon/ena/ena_admin_defs.h
@@ -32,6 +32,8 @@ 
 #ifndef _ENA_ADMIN_H_
 #define _ENA_ADMIN_H_
 
+#define ENA_ADMIN_EXTRA_PROPERTIES_STRING_LEN 32
+#define ENA_ADMIN_EXTRA_PROPERTIES_COUNT     32
 
 enum ena_admin_aq_opcode {
 	ENA_ADMIN_CREATE_SQ                         = 1,
@@ -60,6 +62,8 @@  enum ena_admin_aq_feature_id {
 	ENA_ADMIN_MAX_QUEUES_NUM                    = 2,
 	ENA_ADMIN_HW_HINTS                          = 3,
 	ENA_ADMIN_LLQ                               = 4,
+	ENA_ADMIN_EXTRA_PROPERTIES_STRINGS          = 5,
+	ENA_ADMIN_EXTRA_PROPERTIES_FLAGS            = 6,
 	ENA_ADMIN_RSS_HASH_FUNCTION                 = 10,
 	ENA_ADMIN_STATELESS_OFFLOAD_CONFIG          = 11,
 	ENA_ADMIN_RSS_REDIRECTION_TABLE_CONFIG      = 12,
@@ -560,6 +564,14 @@  struct ena_admin_set_feature_mtu_desc {
 	u32 mtu;
 };
 
+struct ena_admin_get_extra_properties_strings_desc {
+	u32 count;
+};
+
+struct ena_admin_get_extra_properties_flags_desc {
+	u32 flags;
+};
+
 struct ena_admin_set_feature_host_attr_desc {
 	/* host OS info base address in OS memory. host info is 4KB of
 	 * physically contiguous
@@ -864,6 +876,10 @@  struct ena_admin_get_feat_resp {
 		struct ena_admin_feature_intr_moder_desc intr_moderation;
 
 		struct ena_admin_ena_hw_hints hw_hints;
+
+		struct ena_admin_get_extra_properties_strings_desc extra_properties_strings;
+
+		struct ena_admin_get_extra_properties_flags_desc extra_properties_flags;
 	} u;
 };
 
diff --git a/drivers/net/ethernet/amazon/ena/ena_com.c b/drivers/net/ethernet/amazon/ena/ena_com.c
index bd0d785b2..935e8fa8d 100644
--- a/drivers/net/ethernet/amazon/ena/ena_com.c
+++ b/drivers/net/ethernet/amazon/ena/ena_com.c
@@ -1877,6 +1877,62 @@  int ena_com_get_link_params(struct ena_com_dev *ena_dev,
 	return ena_com_get_feature(ena_dev, resp, ENA_ADMIN_LINK_CONFIG);
 }
 
+int ena_com_extra_properties_strings_init(struct ena_com_dev *ena_dev)
+{
+	struct ena_admin_get_feat_resp resp;
+	struct ena_extra_properties_strings *extra_properties_strings =
+			&ena_dev->extra_properties_strings;
+	u32 rc;
+
+	extra_properties_strings->size = ENA_ADMIN_EXTRA_PROPERTIES_COUNT *
+		ENA_ADMIN_EXTRA_PROPERTIES_STRING_LEN;
+
+	extra_properties_strings->virt_addr =
+		dma_alloc_coherent(ena_dev->dmadev,
+				   extra_properties_strings->size,
+				   &extra_properties_strings->dma_addr,
+				   GFP_KERNEL);
+	if (unlikely(!extra_properties_strings->virt_addr)) {
+		pr_err("Failed to allocate extra properties strings\n");
+		return 0;
+	}
+
+	rc = ena_com_get_feature_ex(ena_dev, &resp,
+				    ENA_ADMIN_EXTRA_PROPERTIES_STRINGS,
+				    extra_properties_strings->dma_addr,
+				    extra_properties_strings->size);
+	if (rc) {
+		pr_debug("Failed to get extra properties strings\n");
+		goto err;
+	}
+
+	return resp.u.extra_properties_strings.count;
+err:
+	ena_com_delete_extra_properties_strings(ena_dev);
+	return 0;
+}
+
+void ena_com_delete_extra_properties_strings(struct ena_com_dev *ena_dev)
+{
+	struct ena_extra_properties_strings *extra_properties_strings =
+				&ena_dev->extra_properties_strings;
+
+	if (extra_properties_strings->virt_addr) {
+		dma_free_coherent(ena_dev->dmadev,
+				  extra_properties_strings->size,
+				  extra_properties_strings->virt_addr,
+				  extra_properties_strings->dma_addr);
+		extra_properties_strings->virt_addr = NULL;
+	}
+}
+
+int ena_com_get_extra_properties_flags(struct ena_com_dev *ena_dev,
+				       struct ena_admin_get_feat_resp *resp)
+{
+	return ena_com_get_feature(ena_dev, resp,
+				   ENA_ADMIN_EXTRA_PROPERTIES_FLAGS);
+}
+
 int ena_com_get_dev_attr_feat(struct ena_com_dev *ena_dev,
 			      struct ena_com_dev_get_features_ctx *get_feat_ctx)
 {
diff --git a/drivers/net/ethernet/amazon/ena/ena_com.h b/drivers/net/ethernet/amazon/ena/ena_com.h
index f0cb043af..ce36444c3 100644
--- a/drivers/net/ethernet/amazon/ena/ena_com.h
+++ b/drivers/net/ethernet/amazon/ena/ena_com.h
@@ -347,6 +347,12 @@  struct ena_host_attribute {
 	dma_addr_t host_info_dma_addr;
 };
 
+struct ena_extra_properties_strings {
+	u8 *virt_addr;
+	dma_addr_t dma_addr;
+	u32 size;
+};
+
 /* Each ena_dev is a PCI function. */
 struct ena_com_dev {
 	struct ena_com_admin_queue admin_queue;
@@ -375,6 +381,7 @@  struct ena_com_dev {
 	struct ena_intr_moder_entry *intr_moder_tbl;
 
 	struct ena_com_llq_info llq_info;
+	struct ena_extra_properties_strings extra_properties_strings;
 };
 
 struct ena_com_dev_get_features_ctx {
@@ -596,6 +603,31 @@  int ena_com_validate_version(struct ena_com_dev *ena_dev);
 int ena_com_get_link_params(struct ena_com_dev *ena_dev,
 			    struct ena_admin_get_feat_resp *resp);
 
+/* ena_com_extra_properties_strings_init - Initialize the extra properties strings buffer.
+ * @ena_dev: ENA communication layer struct
+ *
+ * Initialize the extra properties strings buffer.
+ */
+int ena_com_extra_properties_strings_init(struct ena_com_dev *ena_dev);
+
+/* ena_com_delete_extra_properties_strings - Free the extra properties strings buffer.
+ * @ena_dev: ENA communication layer struct
+ *
+ * Free the allocated extra properties strings buffer.
+ */
+void ena_com_delete_extra_properties_strings(struct ena_com_dev *ena_dev);
+
+/* ena_com_get_extra_properties_flags - Retrieve extra properties flags.
+ * @ena_dev: ENA communication layer struct
+ * @resp: Extra properties flags.
+ *
+ * Retrieve the extra properties flags.
+ *
+ * @return - 0 on Success negative value otherwise.
+ */
+int ena_com_get_extra_properties_flags(struct ena_com_dev *ena_dev,
+				       struct ena_admin_get_feat_resp *resp);
+
 /* ena_com_get_dma_width - Retrieve physical dma address width the device
  * supports.
  * @ena_dev: ENA communication layer struct
diff --git a/drivers/net/ethernet/amazon/ena/ena_ethtool.c b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
index fe596bc30..65bc5a2b2 100644
--- a/drivers/net/ethernet/amazon/ena/ena_ethtool.c
+++ b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
@@ -197,15 +197,24 @@  static void ena_get_ethtool_stats(struct net_device *netdev,
 	ena_dev_admin_queue_stats(adapter, &data);
 }
 
+static int get_stats_sset_count(struct ena_adapter *adapter)
+{
+	return  adapter->num_queues * (ENA_STATS_ARRAY_TX + ENA_STATS_ARRAY_RX)
+		+ ENA_STATS_ARRAY_GLOBAL + ENA_STATS_ARRAY_ENA_COM;
+}
+
 int ena_get_sset_count(struct net_device *netdev, int sset)
 {
 	struct ena_adapter *adapter = netdev_priv(netdev);
 
-	if (sset != ETH_SS_STATS)
+	switch (sset) {
+	case ETH_SS_STATS:
+		return get_stats_sset_count(adapter);
+	case ETH_SS_PRIV_FLAGS:
+		return adapter->ena_extra_properties_count;
+	default:
 		return -EOPNOTSUPP;
-
-	return  adapter->num_queues * (ENA_STATS_ARRAY_TX + ENA_STATS_ARRAY_RX)
-		+ ENA_STATS_ARRAY_GLOBAL + ENA_STATS_ARRAY_ENA_COM;
+	}
 }
 
 static void ena_queue_strings(struct ena_adapter *adapter, u8 **data)
@@ -247,26 +256,56 @@  static void ena_com_dev_strings(u8 **data)
 	}
 }
 
-static void ena_get_strings(struct net_device *netdev, u32 sset, u8 *data)
+static void get_stats_strings(struct ena_adapter *adapter, u8 *data)
 {
-	struct ena_adapter *adapter = netdev_priv(netdev);
 	const struct ena_stats *ena_stats;
 	int i;
 
-	if (sset != ETH_SS_STATS)
-		return;
-
 	for (i = 0; i < ENA_STATS_ARRAY_GLOBAL; i++) {
 		ena_stats = &ena_stats_global_strings[i];
-
 		memcpy(data, ena_stats->name, ETH_GSTRING_LEN);
 		data += ETH_GSTRING_LEN;
 	}
-
 	ena_queue_strings(adapter, &data);
 	ena_com_dev_strings(&data);
 }
 
+static void get_private_flags_strings(struct ena_adapter *adapter, u8 *data)
+{
+	struct ena_com_dev *ena_dev = adapter->ena_dev;
+	u8 *strings = ena_dev->extra_properties_strings.virt_addr;
+	int i;
+
+	if (unlikely(!strings)) {
+		adapter->ena_extra_properties_count = 0;
+		netif_err(adapter, drv, adapter->netdev,
+			  "Failed to allocate extra properties strings\n");
+		return;
+	}
+
+	for (i = 0; i < adapter->ena_extra_properties_count; i++) {
+		snprintf(data, ETH_GSTRING_LEN, "%s",
+			 strings + ENA_ADMIN_EXTRA_PROPERTIES_STRING_LEN * i);
+		data += ETH_GSTRING_LEN;
+	}
+}
+
+static void ena_get_strings(struct net_device *netdev, u32 sset, u8 *data)
+{
+	struct ena_adapter *adapter = netdev_priv(netdev);
+
+	switch (sset) {
+	case ETH_SS_STATS:
+		get_stats_strings(adapter, data);
+		break;
+	case ETH_SS_PRIV_FLAGS:
+		get_private_flags_strings(adapter, data);
+		break;
+	default:
+		break;
+	}
+}
+
 static int ena_get_link_ksettings(struct net_device *netdev,
 				  struct ethtool_link_ksettings *link_ksettings)
 {
@@ -441,6 +480,7 @@  static void ena_get_drvinfo(struct net_device *dev,
 	strlcpy(info->version, DRV_MODULE_VERSION, sizeof(info->version));
 	strlcpy(info->bus_info, pci_name(adapter->pdev),
 		sizeof(info->bus_info));
+	info->n_priv_flags = adapter->ena_extra_properties_count;
 }
 
 static void ena_get_ringparam(struct net_device *netdev,
@@ -798,6 +838,20 @@  static int ena_set_tunable(struct net_device *netdev,
 	return ret;
 }
 
+static u32 ena_get_priv_flags(struct net_device *netdev)
+{
+	struct ena_adapter *adapter = netdev_priv(netdev);
+	struct ena_com_dev *ena_dev = adapter->ena_dev;
+	struct ena_admin_get_feat_resp get_resp;
+	u32 rc;
+
+	rc = ena_com_get_extra_properties_flags(ena_dev, &get_resp);
+	if (!rc)
+		return get_resp.u.extra_properties_flags.flags;
+
+	return 0;
+}
+
 static const struct ethtool_ops ena_ethtool_ops = {
 	.get_link_ksettings	= ena_get_link_ksettings,
 	.get_drvinfo		= ena_get_drvinfo,
@@ -819,6 +873,7 @@  static const struct ethtool_ops ena_ethtool_ops = {
 	.get_channels		= ena_get_channels,
 	.get_tunable		= ena_get_tunable,
 	.set_tunable		= ena_set_tunable,
+	.get_priv_flags		= ena_get_priv_flags,
 };
 
 void ena_set_ethtool_ops(struct net_device *netdev)
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index d2b82f917..33fab4f41 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -2369,6 +2369,14 @@  err:
 	ena_com_delete_debug_area(adapter->ena_dev);
 }
 
+static void ena_extra_properties_strings_destroy(struct net_device *netdev)
+{
+	struct ena_adapter *adapter = netdev_priv(netdev);
+
+	ena_com_delete_extra_properties_strings(adapter->ena_dev);
+	adapter->ena_extra_properties_count = 0;
+}
+
 static void ena_get_stats64(struct net_device *netdev,
 			    struct rtnl_link_stats64 *stats)
 {
@@ -3424,6 +3432,9 @@  static int ena_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	ena_config_debug_area(adapter);
 
+	adapter->ena_extra_properties_count =
+		ena_com_extra_properties_strings_init(ena_dev);
+
 	memcpy(adapter->netdev->perm_addr, adapter->mac_addr, netdev->addr_len);
 
 	netif_carrier_off(netdev);
@@ -3463,6 +3474,7 @@  static int ena_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	return 0;
 
 err_rss:
+	ena_extra_properties_strings_destroy(netdev);
 	ena_com_delete_debug_area(ena_dev);
 	ena_com_rss_destroy(ena_dev);
 err_free_msix:
@@ -3529,6 +3541,8 @@  static void ena_remove(struct pci_dev *pdev)
 
 	ena_com_delete_host_info(ena_dev);
 
+	ena_extra_properties_strings_destroy(netdev);
+
 	ena_release_bars(ena_dev, pdev);
 
 	pci_disable_device(pdev);
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.h b/drivers/net/ethernet/amazon/ena/ena_netdev.h
index 63870072c..0681e18b0 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.h
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.h
@@ -364,6 +364,8 @@  struct ena_adapter {
 	u32 last_monitored_tx_qid;
 
 	enum ena_regs_reset_reason_types reset_reason;
+
+	u8 ena_extra_properties_count;
 };
 
 void ena_set_ethtool_ops(struct net_device *netdev);