2017-12-07 08:50:07

by Antoine Tenart

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

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


2017-12-07 08:50:05

by Antoine Tenart

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

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 <[email protected]>
---
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

2017-12-07 08:50:37

by Antoine Tenart

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

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

Suggested-by: Yan Markman <[email protected]>
Signed-off-by: Antoine Tenart <[email protected]>
---
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

2017-12-07 08:50:04

by Antoine Tenart

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

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

Signed-off-by: Antoine Tenart <[email protected]>
---
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

2017-12-07 08:51:01

by Antoine Tenart

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

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

Suggested-by: Yan Markman <[email protected]>
Signed-off-by: Antoine Tenart <[email protected]>
---
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

2017-12-07 08:51:41

by Antoine Tenart

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

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 <[email protected]>
---
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

2017-12-07 08:52:15

by Antoine Tenart

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

From: Yan Markman <[email protected]>

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 <[email protected]>
[Antoine: commit message]
Signed-off-by: Antoine Tenart <[email protected]>
---
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

2017-12-07 19:53:33

by David Miller

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

From: Antoine Tenart <[email protected]>
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 <[email protected]>

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.

2017-12-08 08:24:23

by Antoine Tenart

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

Hi David,

On Thu, Dec 07, 2017 at 02:53:29PM -0500, David Miller wrote:
> From: Antoine Tenart <[email protected]>
> 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 <[email protected]>
>
> 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

2017-12-08 08:28:53

by Antoine Tenart

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

On Thu, Dec 07, 2017 at 02:53:29PM -0500, David Miller wrote:
> From: Antoine Tenart <[email protected]>
> 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 <[email protected]>
>
> 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