| Message ID | 20191114014949.31057-1-uli+renesas@fpond.eu (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 15fb35fa9ff456b81159033eba6397fcee85e671 |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | [v4] ravb: implement MTU change while device is up | expand |
Hi Uli, On Thu, Nov 14, 2019 at 2:50 AM Ulrich Hecht <uli+renesas@fpond.eu> wrote: > Pre-allocates buffers sufficient for the maximum supported MTU (2026) in > order to eliminate the possibility of resource exhaustion when changing the > MTU while the device is up. > > Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu> Thanks for your patch! > --- a/drivers/net/ethernet/renesas/ravb_main.c > +++ b/drivers/net/ethernet/renesas/ravb_main.c > @@ -293,9 +293,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(priv->rx_buf_sz); > + rx_desc->ds_cc = cpu_to_le16(RX_BUF_SZ); So the RAVB hardware will always see the full buffer size. Hence if it receives a frame that is larger than the configured MTU, it will still happily pass it to the driver, and to the network stack, which will reject it, presumably? Note that the driver doesn't seem to configure the Reception Truncation Configuration register, so it will never reject frames up to 4092 bytes (unless the boot loader has changed that). Gr{oetje,eeting}s, Geert
Hello! On 11/14/2019 04:49 AM, Ulrich Hecht wrote: > Pre-allocates buffers sufficient for the maximum supported MTU (2026) in > order to eliminate the possibility of resource exhaustion when changing the > MTU while the device is up. > > Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu> Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > --- > drivers/net/ethernet/renesas/ravb.h | 3 ++- > drivers/net/ethernet/renesas/ravb_main.c | 26 +++++++++++++----------- > 2 files changed, 16 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h > index a9c89d5d8898..9f88b5db4f89 100644 > --- a/drivers/net/ethernet/renesas/ravb.h > +++ b/drivers/net/ethernet/renesas/ravb.h > @@ -955,6 +955,8 @@ enum RAVB_QUEUE { > #define NUM_RX_QUEUE 2 > #define NUM_TX_QUEUE 2 > > +#define RX_BUF_SZ (2048 - ETH_FCS_LEN + sizeof(__sum16)) That's smart but won't scale iff we decide to support e.g. Jumbo frames (the real AVB h/w supports up to 128 KiB frames. [...] MBR, Sergei
From: Ulrich Hecht <uli+renesas@fpond.eu> Date: Thu, 14 Nov 2019 02:49:49 +0100 > Pre-allocates buffers sufficient for the maximum supported MTU (2026) in > order to eliminate the possibility of resource exhaustion when changing the > MTU while the device is up. > > Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu> Applied.
Hello: The following patches were marked "accepted", because they were applied to geert/renesas-devel (refs/heads/master): Patch: clocksource/drivers/sh_mtu2: Do not loop using platform_get_irq_by_name() Submitter: Geert Uytterhoeven <geert+renesas@glider.be> Patchwork: https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=188637 Patch: [v4] ravb: implement MTU change while device is up Submitter: Ulrich Hecht <uli+renesas@fpond.eu> Patchwork: https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=202751 Total patches: 2
diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h index a9c89d5d8898..9f88b5db4f89 100644 --- a/drivers/net/ethernet/renesas/ravb.h +++ b/drivers/net/ethernet/renesas/ravb.h @@ -955,6 +955,8 @@ enum RAVB_QUEUE { #define NUM_RX_QUEUE 2 #define NUM_TX_QUEUE 2 +#define RX_BUF_SZ (2048 - ETH_FCS_LEN + sizeof(__sum16)) + /* TX descriptors per packet */ #define NUM_TX_DESC_GEN2 2 #define NUM_TX_DESC_GEN3 1 @@ -1018,7 +1020,6 @@ 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 5ea14b5fbed8..4b13a184bfc7 100644 --- a/drivers/net/ethernet/renesas/ravb_main.c +++ b/drivers/net/ethernet/renesas/ravb_main.c @@ -230,7 +230,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), - priv->rx_buf_sz, + RX_BUF_SZ, DMA_FROM_DEVICE); } ring_size = sizeof(struct ravb_ex_rx_desc) * @@ -293,9 +293,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(priv->rx_buf_sz); + rx_desc->ds_cc = cpu_to_le16(RX_BUF_SZ); dma_addr = dma_map_single(ndev->dev.parent, priv->rx_skb[q][i]->data, - priv->rx_buf_sz, + RX_BUF_SZ, DMA_FROM_DEVICE); /* We just set the data size to 0 for a failed mapping which * should prevent DMA from happening... @@ -342,9 +342,6 @@ static int ravb_ring_init(struct net_device *ndev, int q) int ring_size; int i; - priv->rx_buf_sz = (ndev->mtu <= 1492 ? PKT_BUF_SZ : ndev->mtu) + - ETH_HLEN + VLAN_HLEN + sizeof(__sum16); - /* Allocate RX and TX skb rings */ priv->rx_skb[q] = kcalloc(priv->num_rx_ring[q], sizeof(*priv->rx_skb[q]), GFP_KERNEL); @@ -354,7 +351,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, priv->rx_buf_sz + RAVB_ALIGN - 1); + skb = netdev_alloc_skb(ndev, RX_BUF_SZ + RAVB_ALIGN - 1); if (!skb) goto error; ravb_set_buffer_align(skb); @@ -584,7 +581,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), - priv->rx_buf_sz, + RX_BUF_SZ, DMA_FROM_DEVICE); get_ts &= (q == RAVB_NC) ? RAVB_RXTSTAMP_TYPE_V2_L2_EVENT : @@ -617,11 +614,11 @@ 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(priv->rx_buf_sz); + desc->ds_cc = cpu_to_le16(RX_BUF_SZ); if (!priv->rx_skb[q][entry]) { skb = netdev_alloc_skb(ndev, - priv->rx_buf_sz + + RX_BUF_SZ + RAVB_ALIGN - 1); if (!skb) break; /* Better luck next round. */ @@ -1801,10 +1798,15 @@ static int ravb_do_ioctl(struct net_device *ndev, struct ifreq *req, int cmd) static int ravb_change_mtu(struct net_device *ndev, int new_mtu) { - if (netif_running(ndev)) - return -EBUSY; + struct ravb_private *priv = netdev_priv(ndev); ndev->mtu = new_mtu; + + if (netif_running(ndev)) { + synchronize_irq(priv->emac_irq); + ravb_emac_init(ndev); + } + netdev_update_features(ndev); return 0;
Pre-allocates buffers sufficient for the maximum supported MTU (2026) in order to eliminate the possibility of resource exhaustion when changing the MTU while the device is up. Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu> --- drivers/net/ethernet/renesas/ravb.h | 3 ++- drivers/net/ethernet/renesas/ravb_main.c | 26 +++++++++++++----------- 2 files changed, 16 insertions(+), 13 deletions(-)