[01/29] net/ena: check if size of buffer is at least 1400B
diff mbox series

Message ID 20200327101823.12646-2-mk@semihalf.com
State Superseded, archived
Delegated to: Andrew Rybchenko
Headers show
Series
  • Update ENA driver to v2.1.0
Related show

Checks

Context Check Description
ci/iol-testing fail Testing issues
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/Intel-compilation fail Compilation issues
ci/checkpatch success coding style OK

Commit Message

Michal Krawczyk March 27, 2020, 10:17 a.m. UTC 
Some of the ENA devices can't handle buffers which are smaller than a
1400B. Because of this limitation, size of the buffer is being checked
and limited during the Rx queue setup.

If it's below the allowed value, PMD won't finish it's configuration
successfully..

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
 drivers/net/ena/ena_ethdev.c | 12 ++++++++++++
 drivers/net/ena/ena_ethdev.h |  1 +
 2 files changed, 13 insertions(+)

Comments

Andrew Rybchenko March 27, 2020, 10:55 a.m. UTC | #1 
On 3/27/20 1:17 PM, Michal Krawczyk wrote:
> Some of the ENA devices can't handle buffers which are smaller than a
> 1400B. Because of this limitation, size of the buffer is being checked
> and limited during the Rx queue setup.
> 
> If it's below the allowed value, PMD won't finish it's configuration
> successfully..
> 
> Signed-off-by: Michal Krawczyk <mk@semihalf.com>
> Reviewed-by: Igor Chauskin <igorch@amazon.com>
> Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
> ---
>  drivers/net/ena/ena_ethdev.c | 12 ++++++++++++
>  drivers/net/ena/ena_ethdev.h |  1 +
>  2 files changed, 13 insertions(+)
> 
> diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
> index 665afee4f0..a8f8784a9f 100644
> --- a/drivers/net/ena/ena_ethdev.c
> +++ b/drivers/net/ena/ena_ethdev.c
> @@ -1282,6 +1282,7 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev,
>  {
>  	struct ena_adapter *adapter = dev->data->dev_private;
>  	struct ena_ring *rxq = NULL;
> +	size_t buffer_size;
>  	int i;
>  
>  	rxq = &adapter->rx_ring[queue_idx];
> @@ -1309,6 +1310,17 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev,
>  		return -EINVAL;
>  	}
>  
> +	/* ENA isn't supporting buffers smaller than 1400 bytes */
> +	buffer_size = mp->elt_size - sizeof(struct rte_mbuf) -
> +		RTE_PKTMBUF_HEADROOM;
> +	if (buffer_size < ENA_RX_BUF_MIN_SIZE) {
> +		PMD_DRV_LOG(ERR,
> +			"Unsupported size of RX buffer: %zu (min size: %d)\n",
> +			buffer_size, ENA_RX_BUF_MIN_SIZE);
> +		return -EINVAL;
> +	}
> +	printf("mempool size: %ld\n", buffer_size);

Is it debug printout left?

> +
>  	rxq->port_id = dev->data->port_id;
>  	rxq->next_to_clean = 0;
>  	rxq->next_to_use = 0;
> diff --git a/drivers/net/ena/ena_ethdev.h b/drivers/net/ena/ena_ethdev.h
> index af5eeea280..c1457defeb 100644
> --- a/drivers/net/ena/ena_ethdev.h
> +++ b/drivers/net/ena/ena_ethdev.h
> @@ -20,6 +20,7 @@
>  #define ENA_MIN_FRAME_LEN	64
>  #define ENA_NAME_MAX_LEN	20
>  #define ENA_PKT_MAX_BUFS	17
> +#define ENA_RX_BUF_MIN_SIZE	1400
>  
>  #define ENA_MIN_MTU		128
>  
>
Stephen Hemminger March 27, 2020, 2:51 p.m. UTC | #2 
On Fri, 27 Mar 2020 11:17:55 +0100
Michal Krawczyk <mk@semihalf.com> wrote:

> +	/* ENA isn't supporting buffers smaller than 1400 bytes */
> +	buffer_size = mp->elt_size - sizeof(struct rte_mbuf) -
> +		RTE_PKTMBUF_HEADROOM;

This should use rte_pktmbuf_data_room_size(mp) - RTE_PKTMBUF_HEADROOM
Michal Krawczyk March 31, 2020, 9:47 a.m. UTC | #3 
pt., 27 mar 2020 o 11:56 Andrew Rybchenko <arybchenko@solarflare.com>
napisał(a):
>
> On 3/27/20 1:17 PM, Michal Krawczyk wrote:
> > Some of the ENA devices can't handle buffers which are smaller than a
> > 1400B. Because of this limitation, size of the buffer is being checked
> > and limited during the Rx queue setup.
> >
> > If it's below the allowed value, PMD won't finish it's configuration
> > successfully..
> >
> > Signed-off-by: Michal Krawczyk <mk@semihalf.com>
> > Reviewed-by: Igor Chauskin <igorch@amazon.com>
> > Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
> > ---
> >  drivers/net/ena/ena_ethdev.c | 12 ++++++++++++
> >  drivers/net/ena/ena_ethdev.h |  1 +
> >  2 files changed, 13 insertions(+)
> >
> > diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
> > index 665afee4f0..a8f8784a9f 100644
> > --- a/drivers/net/ena/ena_ethdev.c
> > +++ b/drivers/net/ena/ena_ethdev.c
> > @@ -1282,6 +1282,7 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev,
> >  {
> >       struct ena_adapter *adapter = dev->data->dev_private;
> >       struct ena_ring *rxq = NULL;
> > +     size_t buffer_size;
> >       int i;
> >
> >       rxq = &adapter->rx_ring[queue_idx];
> > @@ -1309,6 +1310,17 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev,
> >               return -EINVAL;
> >       }
> >
> > +     /* ENA isn't supporting buffers smaller than 1400 bytes */
> > +     buffer_size = mp->elt_size - sizeof(struct rte_mbuf) -
> > +             RTE_PKTMBUF_HEADROOM;
> > +     if (buffer_size < ENA_RX_BUF_MIN_SIZE) {
> > +             PMD_DRV_LOG(ERR,
> > +                     "Unsupported size of RX buffer: %zu (min size: %d)\n",
> > +                     buffer_size, ENA_RX_BUF_MIN_SIZE);
> > +             return -EINVAL;
> > +     }
> > +     printf("mempool size: %ld\n", buffer_size);
>
> Is it debug printout left?
>

Yes, sorry. I'll remove it in v2.

> > +
> >       rxq->port_id = dev->data->port_id;
> >       rxq->next_to_clean = 0;
> >       rxq->next_to_use = 0;
> > diff --git a/drivers/net/ena/ena_ethdev.h b/drivers/net/ena/ena_ethdev.h
> > index af5eeea280..c1457defeb 100644
> > --- a/drivers/net/ena/ena_ethdev.h
> > +++ b/drivers/net/ena/ena_ethdev.h
> > @@ -20,6 +20,7 @@
> >  #define ENA_MIN_FRAME_LEN    64
> >  #define ENA_NAME_MAX_LEN     20
> >  #define ENA_PKT_MAX_BUFS     17
> > +#define ENA_RX_BUF_MIN_SIZE  1400
> >
> >  #define ENA_MIN_MTU          128
> >
> >
>
Michal Krawczyk March 31, 2020, 9:48 a.m. UTC | #4 
pt., 27 mar 2020 o 15:51 Stephen Hemminger
<stephen@networkplumber.org> napisał(a):
>
> On Fri, 27 Mar 2020 11:17:55 +0100
> Michal Krawczyk <mk@semihalf.com> wrote:
>
> > +     /* ENA isn't supporting buffers smaller than 1400 bytes */
> > +     buffer_size = mp->elt_size - sizeof(struct rte_mbuf) -
> > +             RTE_PKTMBUF_HEADROOM;
>
> This should use rte_pktmbuf_data_room_size(mp) - RTE_PKTMBUF_HEADROOM

Thanks - I'll change it in v2.

Patch
diff mbox series 

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 665afee4f0..a8f8784a9f 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -1282,6 +1282,7 @@  static int ena_rx_queue_setup(struct rte_eth_dev *dev,
 {
 	struct ena_adapter *adapter = dev->data->dev_private;
 	struct ena_ring *rxq = NULL;
+	size_t buffer_size;
 	int i;
 
 	rxq = &adapter->rx_ring[queue_idx];
@@ -1309,6 +1310,17 @@  static int ena_rx_queue_setup(struct rte_eth_dev *dev,
 		return -EINVAL;
 	}
 
+	/* ENA isn't supporting buffers smaller than 1400 bytes */
+	buffer_size = mp->elt_size - sizeof(struct rte_mbuf) -
+		RTE_PKTMBUF_HEADROOM;
+	if (buffer_size < ENA_RX_BUF_MIN_SIZE) {
+		PMD_DRV_LOG(ERR,
+			"Unsupported size of RX buffer: %zu (min size: %d)\n",
+			buffer_size, ENA_RX_BUF_MIN_SIZE);
+		return -EINVAL;
+	}
+	printf("mempool size: %ld\n", buffer_size);
+
 	rxq->port_id = dev->data->port_id;
 	rxq->next_to_clean = 0;
 	rxq->next_to_use = 0;
diff --git a/drivers/net/ena/ena_ethdev.h b/drivers/net/ena/ena_ethdev.h
index af5eeea280..c1457defeb 100644
--- a/drivers/net/ena/ena_ethdev.h
+++ b/drivers/net/ena/ena_ethdev.h
@@ -20,6 +20,7 @@ 
 #define ENA_MIN_FRAME_LEN	64
 #define ENA_NAME_MAX_LEN	20
 #define ENA_PKT_MAX_BUFS	17
+#define ENA_RX_BUF_MIN_SIZE	1400
 
 #define ENA_MIN_MTU		128