[PATCH net-next 0/6] net: mvpp2: various improvements

[PATCH net-next 0/6] net: mvpp2: various improvements

[Antoine Tenart]

Hi all,

These patches are sent as a series to avoid any possible conflict, even
though there're not entirely related. I can send them separately if
needed. The series applies on today's net-next tree.

Patches 1-2 improve the TSO support, with one logic update: if the TSO
related buffers can't be allocate the feature is deactivated but the
driver won't fail to probe.

The other patches are small improvements.

Thanks!
Antoine

Antoine Tenart (5):
  net: mvpp2: only free the TSO header buffers when it was allocated
  net: mvpp2: disable TSO if its buffers cannot be allocated
  net: mvpp2: align values in ethtool get_coalesce
  net: mvpp2: report the tx-usec coalescing information to ethtool
  net: mvpp2: adjust the coalescing parameters

Yan Markman (1):
  net: mvpp2: split the max ring size from the default one

 drivers/net/ethernet/marvell/mvpp2.c | 79 ++++++++++++++++++++++++++----------
 1 file changed, 58 insertions(+), 21 deletions(-)

-- 
2.14.3

[PATCH net-next 1/6] net: mvpp2: only free the TSO header buffers when it was allocated

[Antoine Tenart]

This patch adds a check to only free the TSO header buffer when its
allocation previously succeeded.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index fed2b2f909fc..d67f40ee63b3 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -5802,6 +5802,7 @@ static int mvpp2_txq_init(struct mvpp2_port *port,
 		txq_pcpu->reserved_num = 0;
 		txq_pcpu->txq_put_index = 0;
 		txq_pcpu->txq_get_index = 0;
+		txq_pcpu->tso_headers = NULL;
 
 		txq_pcpu->stop_threshold = txq->size - MVPP2_MAX_SKB_DESCS;
 		txq_pcpu->wake_threshold = txq_pcpu->stop_threshold / 2;
@@ -5829,10 +5830,13 @@ static void mvpp2_txq_deinit(struct mvpp2_port *port,
 		txq_pcpu = per_cpu_ptr(txq->pcpu, cpu);
 		kfree(txq_pcpu->buffs);
 
-		dma_free_coherent(port->dev->dev.parent,
-				  txq_pcpu->size * TSO_HEADER_SIZE,
-				  txq_pcpu->tso_headers,
-				  txq_pcpu->tso_headers_dma);
+		if (txq_pcpu->tso_headers)
+			dma_free_coherent(port->dev->dev.parent,
+					  txq_pcpu->size * TSO_HEADER_SIZE,
+					  txq_pcpu->tso_headers,
+					  txq_pcpu->tso_headers_dma);
+
+		txq_pcpu->tso_headers = NULL;
 	}
 
 	if (txq->descs)
-- 
2.14.3

[PATCH net-next 2/6] net: mvpp2: disable TSO if its buffers cannot be allocated

[Antoine Tenart]

This patch changes the PPv2 behaviour when the TSO specific headers
cannot be allocated. In such cases the driver was previously failing and
the interface wasn't enabled while only the TSO feature is disabled now
and a warning message is shown.

Such memory allocation failures can happen when DMA_CMA is enabled and
CMA_SIZE_MBYTES is too small (ie. its default value).

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index d67f40ee63b3..2f86358614a6 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -5735,6 +5735,8 @@ static int mvpp2_txq_init(struct mvpp2_port *port,
 	u32 val;
 	int cpu, desc, desc_per_txq, tx_port_num;
 	struct mvpp2_txq_pcpu *txq_pcpu;
+	struct net_device *dev = port->dev;
+	bool tso = dev->features & NETIF_F_TSO;
 
 	txq->size = port->tx_ring_size;
 
@@ -5807,13 +5809,41 @@ static int mvpp2_txq_init(struct mvpp2_port *port,
 		txq_pcpu->stop_threshold = txq->size - MVPP2_MAX_SKB_DESCS;
 		txq_pcpu->wake_threshold = txq_pcpu->stop_threshold / 2;
 
+		if (!tso)
+			continue;
+
 		txq_pcpu->tso_headers =
 			dma_alloc_coherent(port->dev->dev.parent,
 					   txq_pcpu->size * TSO_HEADER_SIZE,
 					   &txq_pcpu->tso_headers_dma,
 					   GFP_KERNEL);
 		if (!txq_pcpu->tso_headers)
-			return -ENOMEM;
+			tso = false;
+	}
+
+	/* TSO was enabled but not enough memory is available to allocate the
+	 * TSO specific buffers. Free the successfully allocated buffers, warn
+	 * the user and disable TSO.
+	 */
+	if ((dev->features & NETIF_F_TSO) && !tso)
+		goto cleanup_tso;
+
+	return 0;
+
+cleanup_tso:
+	dev->features &= ~NETIF_F_TSO;
+	netdev_warn(dev, "TSO disabled\n");
+
+	for_each_present_cpu(cpu) {
+		txq_pcpu = per_cpu_ptr(txq->pcpu, cpu);
+
+		if (txq_pcpu->tso_headers)
+			dma_free_coherent(port->dev->dev.parent,
+					  txq_pcpu->size * TSO_HEADER_SIZE,
+					  txq_pcpu->tso_headers,
+					  txq_pcpu->tso_headers_dma);
+
+		txq_pcpu->tso_headers = NULL;
 	}
 
 	return 0;
-- 
2.14.3

[PATCH net-next 3/6] net: mvpp2: split the max ring size from the default one

[Antoine Tenart]

From: Yan Markman <ymarkman@marvell.com>

The Rx/Tx ring sizes can be adjusted thanks to ethtool given specific
network needs. This commit splits the default ring size from its max
value to allow ethtool to vary the parameters in both ways.

Signed-off-by: Yan Markman <ymarkman@marvell.com>
[Antoine: commit message]
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 2f86358614a6..1e4129f71071 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -504,10 +504,12 @@
 #define MVPP2_DEFAULT_RXQ		4
 
 /* Max number of Rx descriptors */
-#define MVPP2_MAX_RXD			128
+#define MVPP2_MAX_RXD_MAX		1024
+#define MVPP2_MAX_RXD_DFLT		128
 
 /* Max number of Tx descriptors */
-#define MVPP2_MAX_TXD			1024
+#define MVPP2_MAX_TXD_MAX		2048
+#define MVPP2_MAX_TXD_DFLT		1024
 
 /* Amount of Tx descriptors that can be reserved at once by CPU */
 #define MVPP2_CPU_DESC_CHUNK		64
@@ -6866,13 +6868,13 @@ static int mvpp2_check_ringparam_valid(struct net_device *dev,
 	if (ring->rx_pending == 0 || ring->tx_pending == 0)
 		return -EINVAL;
 
-	if (ring->rx_pending > MVPP2_MAX_RXD)
-		new_rx_pending = MVPP2_MAX_RXD;
+	if (ring->rx_pending > MVPP2_MAX_RXD_MAX)
+		new_rx_pending = MVPP2_MAX_RXD_MAX;
 	else if (!IS_ALIGNED(ring->rx_pending, 16))
 		new_rx_pending = ALIGN(ring->rx_pending, 16);
 
-	if (ring->tx_pending > MVPP2_MAX_TXD)
-		new_tx_pending = MVPP2_MAX_TXD;
+	if (ring->tx_pending > MVPP2_MAX_TXD_MAX)
+		new_tx_pending = MVPP2_MAX_TXD_MAX;
 	else if (!IS_ALIGNED(ring->tx_pending, 32))
 		new_tx_pending = ALIGN(ring->tx_pending, 32);
 
@@ -7374,8 +7376,8 @@ static void mvpp2_ethtool_get_ringparam(struct net_device *dev,
 {
 	struct mvpp2_port *port = netdev_priv(dev);
 
-	ring->rx_max_pending = MVPP2_MAX_RXD;
-	ring->tx_max_pending = MVPP2_MAX_TXD;
+	ring->rx_max_pending = MVPP2_MAX_RXD_MAX;
+	ring->tx_max_pending = MVPP2_MAX_TXD_MAX;
 	ring->rx_pending = port->rx_ring_size;
 	ring->tx_pending = port->tx_ring_size;
 }
@@ -7822,7 +7824,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 		goto err_free_netdev;
 	}
 
-	dev->tx_queue_len = MVPP2_MAX_TXD;
+	dev->tx_queue_len = MVPP2_MAX_TXD_MAX;
 	dev->watchdog_timeo = 5 * HZ;
 	dev->netdev_ops = &mvpp2_netdev_ops;
 	dev->ethtool_ops = &mvpp2_eth_tool_ops;
@@ -7905,8 +7907,8 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 
 	mvpp2_port_copy_mac_addr(dev, priv, port_node, &mac_from);
 
-	port->tx_ring_size = MVPP2_MAX_TXD;
-	port->rx_ring_size = MVPP2_MAX_RXD;
+	port->tx_ring_size = MVPP2_MAX_TXD_DFLT;
+	port->rx_ring_size = MVPP2_MAX_RXD_DFLT;
 	SET_NETDEV_DEV(dev, &pdev->dev);
 
 	err = mvpp2_port_init(port);
-- 
2.14.3

[PATCH net-next 4/6] net: mvpp2: align values in ethtool get_coalesce

[Antoine Tenart]

Cosmetic patch aligning values in the ethtool get_coalesce function.
This patch do not modify in anyway the driver's behaviour.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 1e4129f71071..65e2e5338f66 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -7354,9 +7354,9 @@ static int mvpp2_ethtool_get_coalesce(struct net_device *dev,
 {
 	struct mvpp2_port *port = netdev_priv(dev);
 
-	c->rx_coalesce_usecs        = port->rxqs[0]->time_coal;
-	c->rx_max_coalesced_frames  = port->rxqs[0]->pkts_coal;
-	c->tx_max_coalesced_frames =  port->txqs[0]->done_pkts_coal;
+	c->rx_coalesce_usecs       = port->rxqs[0]->time_coal;
+	c->rx_max_coalesced_frames = port->rxqs[0]->pkts_coal;
+	c->tx_max_coalesced_frames = port->txqs[0]->done_pkts_coal;
 	return 0;
 }
 
-- 
2.14.3

[PATCH net-next 5/6] net: mvpp2: report the tx-usec coalescing information to ethtool

[Antoine Tenart]

This patch adds the tx-usec value to the informations reported to
ethtool by the get_coalesce function.

Suggested-by: Yan Markman <ymarkman@marvell.com>
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 65e2e5338f66..0eea15c740ad 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -7357,6 +7357,7 @@ static int mvpp2_ethtool_get_coalesce(struct net_device *dev,
 	c->rx_coalesce_usecs       = port->rxqs[0]->time_coal;
 	c->rx_max_coalesced_frames = port->rxqs[0]->pkts_coal;
 	c->tx_max_coalesced_frames = port->txqs[0]->done_pkts_coal;
+	c->tx_coalesce_usecs       = port->tx_time_coal;
 	return 0;
 }
 
-- 
2.14.3

[PATCH net-next 6/6] net: mvpp2: adjust the coalescing parameters

[Antoine Tenart]

This patch adjust the coalescing parameters to the vendor
recommendations for the PPv2 network controller.

Suggested-by: Yan Markman <ymarkman@marvell.com>
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 0eea15c740ad..3d951556c132 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -454,11 +454,11 @@
 /* Various constants */
 
 /* Coalescing */
-#define MVPP2_TXDONE_COAL_PKTS_THRESH	15
+#define MVPP2_TXDONE_COAL_PKTS_THRESH	64
 #define MVPP2_TXDONE_HRTIMER_PERIOD_NS	1000000UL
 #define MVPP2_TXDONE_COAL_USEC		1000
 #define MVPP2_RX_COAL_PKTS		32
-#define MVPP2_RX_COAL_USEC		100
+#define MVPP2_RX_COAL_USEC		64
 
 /* The two bytes Marvell header. Either contains a special value used
  * by Marvell switches when a specific hardware mode is enabled (not
-- 
2.14.3

Re: [PATCH net-next 1/6] net: mvpp2: only free the TSO header buffers when it was allocated

[David Miller]

From: Antoine Tenart <antoine.tenart@free-electrons.com>
Date: Thu,  7 Dec 2017 09:48:58 +0100

> This patch adds a check to only free the TSO header buffer when its
> allocation previously succeeded.
> 
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>

No, please keep this as a failure to bring up.

Even if you emit a log message, it is completely unintuitive to
have netdev features change on the user just because of a memory
allocation failure.

Re: [PATCH net-next 1/6] net: mvpp2: only free the TSO header buffers when it was allocated

[Antoine Tenart]

Hi David,

On Thu, Dec 07, 2017 at 02:53:29PM -0500, David Miller wrote:
> From: Antoine Tenart <antoine.tenart@free-electrons.com>
> Date: Thu,  7 Dec 2017 09:48:58 +0100
> 
> > This patch adds a check to only free the TSO header buffer when its
> > allocation previously succeeded.
> > 
> > Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> 
> No, please keep this as a failure to bring up.
> 
> Even if you emit a log message, it is completely unintuitive to
> have netdev features change on the user just because of a memory
> allocation failure.

OK, makes sense.

One other possibility would be to disable TSO if CMA_SIZE_MBYTES is set
to a too small value (i.e. its default). But I don't think this would be
a good solution either.

The drawback is the default configuration when selecting DMA_CMA won't
work for PPv2.

Anyway, I'll send a v2 without these patches.

Thanks!
Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH net-next 1/6] net: mvpp2: only free the TSO header buffers when it was allocated

[Antoine Tenart]

On Thu, Dec 07, 2017 at 02:53:29PM -0500, David Miller wrote:
> From: Antoine Tenart <antoine.tenart@free-electrons.com>
> Date: Thu,  7 Dec 2017 09:48:58 +0100
> 
> > This patch adds a check to only free the TSO header buffer when its
> > allocation previously succeeded.
> > 
> > Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> 
> No, please keep this as a failure to bring up.
> 
> Even if you emit a log message, it is completely unintuitive to
> have netdev features change on the user just because of a memory
> allocation failure.

One thing I forgot, this patch still is needed for a proper error path
handling. We can't be sure all buffers were allocated correctly when
calling mvpp2_txq_deinit() (e.g. if one of them was the reason of the
fail).

I'll send a v2 without the patch 2/6, but I'll keep this one.

Thanks,
Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com