* [PATCH v2] ravb: add support for changing MTU
@ 2018-02-16 16:10 Niklas Söderlund
2018-02-16 19:42 ` Florian Fainelli
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Niklas Söderlund @ 2018-02-16 16:10 UTC (permalink / raw)
To: Sergei Shtylyov, David S . Miller, netdev
Cc: linux-renesas-soc, Niklas Söderlund
Allow for changing the MTU within the limit of the maximum size of a
descriptor (2048 bytes). Add the callback to change MTU from user-space
and take the configurable MTU into account when configuring the
hardware.
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
drivers/net/ethernet/renesas/ravb.h | 1 +
drivers/net/ethernet/renesas/ravb_main.c | 34 +++++++++++++++++++++++++-------
2 files changed, 28 insertions(+), 7 deletions(-)
* Changes since v1
- Fix spelling error.
- s/le16_to_cpu(rx_desc->ds_cc)/priv->rx_buf_sz/.
- Drop ETH_FCS_LEN + 16 from rx_buf_sz calculation as pointed out by
Sergei.
diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 96a27b00c90e212a..b81f4faf7b10114d 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -1018,6 +1018,7 @@ struct ravb_private {
u32 dirty_rx[NUM_RX_QUEUE]; /* Producer ring indices */
u32 cur_tx[NUM_TX_QUEUE];
u32 dirty_tx[NUM_TX_QUEUE];
+ u32 rx_buf_sz; /* Based on MTU+slack. */
struct napi_struct napi[NUM_RX_QUEUE];
struct work_struct work;
/* MII transceiver section. */
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index c87f57ca44371586..34e841306e04a3d3 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -238,7 +238,7 @@ static void ravb_ring_free(struct net_device *ndev, int q)
le32_to_cpu(desc->dptr)))
dma_unmap_single(ndev->dev.parent,
le32_to_cpu(desc->dptr),
- PKT_BUF_SZ,
+ priv->rx_buf_sz,
DMA_FROM_DEVICE);
}
ring_size = sizeof(struct ravb_ex_rx_desc) *
@@ -300,9 +300,9 @@ static void ravb_ring_format(struct net_device *ndev, int q)
for (i = 0; i < priv->num_rx_ring[q]; i++) {
/* RX descriptor */
rx_desc = &priv->rx_ring[q][i];
- rx_desc->ds_cc = cpu_to_le16(PKT_BUF_SZ);
+ rx_desc->ds_cc = cpu_to_le16(priv->rx_buf_sz);
dma_addr = dma_map_single(ndev->dev.parent, priv->rx_skb[q][i]->data,
- PKT_BUF_SZ,
+ priv->rx_buf_sz,
DMA_FROM_DEVICE);
/* We just set the data size to 0 for a failed mapping which
* should prevent DMA from happening...
@@ -346,6 +346,10 @@ static int ravb_ring_init(struct net_device *ndev, int q)
int ring_size;
int i;
+ /* +16 gets room from the status from the card. */
+ priv->rx_buf_sz = (ndev->mtu <= 1492 ? PKT_BUF_SZ : ndev->mtu) +
+ ETH_HLEN + VLAN_HLEN;
+
/* Allocate RX and TX skb rings */
priv->rx_skb[q] = kcalloc(priv->num_rx_ring[q],
sizeof(*priv->rx_skb[q]), GFP_KERNEL);
@@ -355,7 +359,7 @@ static int ravb_ring_init(struct net_device *ndev, int q)
goto error;
for (i = 0; i < priv->num_rx_ring[q]; i++) {
- skb = netdev_alloc_skb(ndev, PKT_BUF_SZ + RAVB_ALIGN - 1);
+ skb = netdev_alloc_skb(ndev, priv->rx_buf_sz + RAVB_ALIGN - 1);
if (!skb)
goto error;
ravb_set_buffer_align(skb);
@@ -586,7 +590,7 @@ static bool ravb_rx(struct net_device *ndev, int *quota, int q)
skb = priv->rx_skb[q][entry];
priv->rx_skb[q][entry] = NULL;
dma_unmap_single(ndev->dev.parent, le32_to_cpu(desc->dptr),
- PKT_BUF_SZ,
+ priv->rx_buf_sz,
DMA_FROM_DEVICE);
get_ts &= (q == RAVB_NC) ?
RAVB_RXTSTAMP_TYPE_V2_L2_EVENT :
@@ -619,11 +623,12 @@ static bool ravb_rx(struct net_device *ndev, int *quota, int q)
for (; priv->cur_rx[q] - priv->dirty_rx[q] > 0; priv->dirty_rx[q]++) {
entry = priv->dirty_rx[q] % priv->num_rx_ring[q];
desc = &priv->rx_ring[q][entry];
- desc->ds_cc = cpu_to_le16(PKT_BUF_SZ);
+ desc->ds_cc = cpu_to_le16(priv->rx_buf_sz);
if (!priv->rx_skb[q][entry]) {
skb = netdev_alloc_skb(ndev,
- PKT_BUF_SZ + RAVB_ALIGN - 1);
+ priv->rx_buf_sz +
+ RAVB_ALIGN - 1);
if (!skb)
break; /* Better luck next round. */
ravb_set_buffer_align(skb);
@@ -1854,6 +1859,17 @@ static int ravb_do_ioctl(struct net_device *ndev, struct ifreq *req, int cmd)
return phy_mii_ioctl(phydev, req, cmd);
}
+static int ravb_change_mtu(struct net_device *ndev, int new_mtu)
+{
+ if (netif_running(ndev))
+ return -EBUSY;
+
+ ndev->mtu = new_mtu;
+ netdev_update_features(ndev);
+
+ return 0;
+}
+
static void ravb_set_rx_csum(struct net_device *ndev, bool enable)
{
struct ravb_private *priv = netdev_priv(ndev);
@@ -1895,6 +1911,7 @@ static const struct net_device_ops ravb_netdev_ops = {
.ndo_set_rx_mode = ravb_set_rx_mode,
.ndo_tx_timeout = ravb_tx_timeout,
.ndo_do_ioctl = ravb_do_ioctl,
+ .ndo_change_mtu = ravb_change_mtu,
.ndo_validate_addr = eth_validate_addr,
.ndo_set_mac_address = eth_mac_addr,
.ndo_set_features = ravb_set_features,
@@ -2117,6 +2134,9 @@ static int ravb_probe(struct platform_device *pdev)
goto out_release;
}
+ ndev->max_mtu = 2048 - (ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN);
+ ndev->min_mtu = ETH_MIN_MTU;
+
/* Set function */
ndev->netdev_ops = &ravb_netdev_ops;
ndev->ethtool_ops = &ravb_ethtool_ops;
--
2.16.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] ravb: add support for changing MTU
2018-02-16 16:10 [PATCH v2] ravb: add support for changing MTU Niklas Söderlund
@ 2018-02-16 19:42 ` Florian Fainelli
2018-02-16 19:43 ` Sergei Shtylyov
2018-02-16 21:35 ` David Miller
2018-02-17 9:08 ` Sergei Shtylyov
2 siblings, 1 reply; 6+ messages in thread
From: Florian Fainelli @ 2018-02-16 19:42 UTC (permalink / raw)
To: Niklas Söderlund, Sergei Shtylyov, David S . Miller, netdev
Cc: linux-renesas-soc
On 02/16/2018 08:10 AM, Niklas Söderlund wrote:
> Allow for changing the MTU within the limit of the maximum size of a
> descriptor (2048 bytes). Add the callback to change MTU from user-space
> and take the configurable MTU into account when configuring the
> hardware.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>
> +static int ravb_change_mtu(struct net_device *ndev, int new_mtu)
> +{
> + if (netif_running(ndev))
> + return -EBUSY;
> +
> + ndev->mtu = new_mtu;
> + netdev_update_features(ndev);
Don't you somehow need to quiesce the RX DMA and make sure you that you
re-allocate all RX buffers within priv->rx_skb[q][entry] such that they
will be able to accept a larger buffer size?
If we put the ravb interface under high RX load and we change the MTU on
the fly, can we crash the kernel?
--
Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] ravb: add support for changing MTU
2018-02-16 19:42 ` Florian Fainelli
@ 2018-02-16 19:43 ` Sergei Shtylyov
2018-02-16 19:45 ` Florian Fainelli
0 siblings, 1 reply; 6+ messages in thread
From: Sergei Shtylyov @ 2018-02-16 19:43 UTC (permalink / raw)
To: Florian Fainelli, Niklas Söderlund, David S . Miller, netdev
Cc: linux-renesas-soc
Hello!
On 02/16/2018 10:42 PM, Florian Fainelli wrote:
>> Allow for changing the MTU within the limit of the maximum size of a
>> descriptor (2048 bytes). Add the callback to change MTU from user-space
>> and take the configurable MTU into account when configuring the
>> hardware.
>>
>> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
>> ---
>
>>
>> +static int ravb_change_mtu(struct net_device *ndev, int new_mtu)
>> +{
>> + if (netif_running(ndev))
>> + return -EBUSY;
>> +
>> + ndev->mtu = new_mtu;
>> + netdev_update_features(ndev);
>
> Don't you somehow need to quiesce the RX DMA and make sure you that you
> re-allocate all RX buffers within priv->rx_skb[q][entry] such that they
> will be able to accept a larger buffer size?
>
> If we put the ravb interface under high RX load and we change the MTU on
> the fly, can we crash the kernel?
I thought the netif_running() check guards against that. Does it not?
MBR, Sergei
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] ravb: add support for changing MTU
2018-02-16 19:43 ` Sergei Shtylyov
@ 2018-02-16 19:45 ` Florian Fainelli
0 siblings, 0 replies; 6+ messages in thread
From: Florian Fainelli @ 2018-02-16 19:45 UTC (permalink / raw)
To: Sergei Shtylyov, Niklas Söderlund, David S . Miller, netdev
Cc: linux-renesas-soc
On 02/16/2018 11:43 AM, Sergei Shtylyov wrote:
> Hello!
>
> On 02/16/2018 10:42 PM, Florian Fainelli wrote:
>
>>> Allow for changing the MTU within the limit of the maximum size of a
>>> descriptor (2048 bytes). Add the callback to change MTU from user-space
>>> and take the configurable MTU into account when configuring the
>>> hardware.
>>>
>>> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
>>> ---
>>
>>>
>>> +static int ravb_change_mtu(struct net_device *ndev, int new_mtu)
>>> +{
>>> + if (netif_running(ndev))
>>> + return -EBUSY;
>>> +
>>> + ndev->mtu = new_mtu;
>>> + netdev_update_features(ndev);
>>
>> Don't you somehow need to quiesce the RX DMA and make sure you that you
>> re-allocate all RX buffers within priv->rx_skb[q][entry] such that they
>> will be able to accept a larger buffer size?
>>
>> If we put the ravb interface under high RX load and we change the MTU on
>> the fly, can we crash the kernel?
>
> I thought the netif_running() check guards against that. Does it not?
It does, completely missed that, thanks!
--
Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] ravb: add support for changing MTU
2018-02-16 16:10 [PATCH v2] ravb: add support for changing MTU Niklas Söderlund
2018-02-16 19:42 ` Florian Fainelli
@ 2018-02-16 21:35 ` David Miller
2018-02-17 9:08 ` Sergei Shtylyov
2 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2018-02-16 21:35 UTC (permalink / raw)
To: niklas.soderlund+renesas; +Cc: sergei.shtylyov, netdev, linux-renesas-soc
From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Date: Fri, 16 Feb 2018 17:10:08 +0100
> Allow for changing the MTU within the limit of the maximum size of a
> descriptor (2048 bytes). Add the callback to change MTU from user-space
> and take the configurable MTU into account when configuring the
> hardware.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Applied.
However, most drivers support MTU change while the interface is up and
frankly that is what is most useful for users.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] ravb: add support for changing MTU
2018-02-16 16:10 [PATCH v2] ravb: add support for changing MTU Niklas Söderlund
2018-02-16 19:42 ` Florian Fainelli
2018-02-16 21:35 ` David Miller
@ 2018-02-17 9:08 ` Sergei Shtylyov
2 siblings, 0 replies; 6+ messages in thread
From: Sergei Shtylyov @ 2018-02-17 9:08 UTC (permalink / raw)
To: Niklas Söderlund, David S . Miller, netdev; +Cc: linux-renesas-soc
Hello!
On 2/16/2018 7:10 PM, Niklas Söderlund wrote:
> Allow for changing the MTU within the limit of the maximum size of a
> descriptor (2048 bytes). Add the callback to change MTU from user-space
> and take the configurable MTU into account when configuring the
> hardware.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> drivers/net/ethernet/renesas/ravb.h | 1 +
> drivers/net/ethernet/renesas/ravb_main.c | 34 +++++++++++++++++++++++++-------
> 2 files changed, 28 insertions(+), 7 deletions(-)
>
> * Changes since v1
> - Fix spelling error.
> - s/le16_to_cpu(rx_desc->ds_cc)/priv->rx_buf_sz/.
> - Drop ETH_FCS_LEN + 16 from rx_buf_sz calculation as pointed out by
> Sergei.
>
> diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
> index 96a27b00c90e212a..b81f4faf7b10114d 100644
> --- a/drivers/net/ethernet/renesas/ravb.h
> +++ b/drivers/net/ethernet/renesas/ravb.h
> @@ -1018,6 +1018,7 @@ struct ravb_private {
> u32 dirty_rx[NUM_RX_QUEUE]; /* Producer ring indices */
> u32 cur_tx[NUM_TX_QUEUE];
> u32 dirty_tx[NUM_TX_QUEUE];
> + u32 rx_buf_sz; /* Based on MTU+slack. */
> struct napi_struct napi[NUM_RX_QUEUE];
> struct work_struct work;
> /* MII transceiver section. */
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index c87f57ca44371586..34e841306e04a3d3 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -238,7 +238,7 @@ static void ravb_ring_free(struct net_device *ndev, int q)
> le32_to_cpu(desc->dptr)))
> dma_unmap_single(ndev->dev.parent,
> le32_to_cpu(desc->dptr),
> - PKT_BUF_SZ,
> + priv->rx_buf_sz,
> DMA_FROM_DEVICE);
> }
> ring_size = sizeof(struct ravb_ex_rx_desc) *
> @@ -300,9 +300,9 @@ static void ravb_ring_format(struct net_device *ndev, int q)
> for (i = 0; i < priv->num_rx_ring[q]; i++) {
> /* RX descriptor */
> rx_desc = &priv->rx_ring[q][i];
> - rx_desc->ds_cc = cpu_to_le16(PKT_BUF_SZ);
> + rx_desc->ds_cc = cpu_to_le16(priv->rx_buf_sz);
> dma_addr = dma_map_single(ndev->dev.parent, priv->rx_skb[q][i]->data,
> - PKT_BUF_SZ,
> + priv->rx_buf_sz,
> DMA_FROM_DEVICE);
> /* We just set the data size to 0 for a failed mapping which
> * should prevent DMA from happening...
> @@ -346,6 +346,10 @@ static int ravb_ring_init(struct net_device *ndev, int q)
> int ring_size;
> int i;
>
> + /* +16 gets room from the status from the card. */
You removed the addition of 16 but left the comment intact. :-)
Will have to fix it up in a follow-up patch now...
> + priv->rx_buf_sz = (ndev->mtu <= 1492 ? PKT_BUF_SZ : ndev->mtu) +
Not sure why PKT_BUF_SZ is 1538, sigh...
> + ETH_HLEN + VLAN_HLEN;
> +
> /* Allocate RX and TX skb rings */
> priv->rx_skb[q] = kcalloc(priv->num_rx_ring[q],
> sizeof(*priv->rx_skb[q]), GFP_KERNEL);
[...]
MBR, Sergei
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-02-17 9:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-16 16:10 [PATCH v2] ravb: add support for changing MTU Niklas Söderlund
2018-02-16 19:42 ` Florian Fainelli
2018-02-16 19:43 ` Sergei Shtylyov
2018-02-16 19:45 ` Florian Fainelli
2018-02-16 21:35 ` David Miller
2018-02-17 9:08 ` Sergei Shtylyov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).