Re: [PATCH v2] solos-pci: Increase headroom on received packets

[David Miller]

From: David Woodhouse 
Date: Wed, 16 Sep 2015 12:35:00 +0100

> A comment in include/linux/skbuff.h says that:
> 
>  * Various parts of the networking layer expect at least 32 bytes of
>  * headroom, you should not reduce this.
> 
> This was demonstrated by a panic when handling fragmented IPv6 packets:
> http://marc.info/?l=linux-netdev=144236093519172=2
> 
> It's not entirely clear if that comment is still valid ― and if it is,
> perhaps netif_rx() ought to be enforcing it with a warning.
> 
> But either way, it is rather stupid from a performance point of view
> for us to be receiving packets into a buffer which doesn't have enough
> room to prepend an Ethernet header ― it means that *every* incoming
> packet is going to be need to be reallocated. So let's fix that.
> 
> Signed-off-by: David Woodhouse 

Applied, thanks David.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] solos-pci: Increase headroom on received packets

[David Woodhouse]

A comment in include/linux/skbuff.h says that:

 * Various parts of the networking layer expect at least 32 bytes of
 * headroom, you should not reduce this.

This was demonstrated by a panic when handling fragmented IPv6 packets:
http://marc.info/?l=linux-netdev=144236093519172=2

It's not entirely clear if that comment is still valid — and if it is,
perhaps netif_rx() ought to be enforcing it with a warning.

But either way, it is rather stupid from a performance point of view
for us to be receiving packets into a buffer which doesn't have enough
room to prepend an Ethernet header — it means that *every* incoming
packet is going to be need to be reallocated. So let's fix that.

Signed-off-by: David Woodhouse 
---
diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
index 74e18b0..3d7fb65 100644
--- a/drivers/atm/solos-pci.c
+++ b/drivers/atm/solos-pci.c
@@ -805,7 +805,12 @@ static void solos_bh(unsigned long card_arg)
continue;
}
 
-   skb = alloc_skb(size + 1, GFP_ATOMIC);
+   /* Use netdev_alloc_skb() because it adds 
NET_SKB_PAD of
+* headroom, and ensures we can route packets 
back out an
+* Ethernet interface (for example) without 
having to
+* reallocate. Adding NET_IP_ALIGN also ensures 
that both
+* PPPoATM and PPPoEoBR2684 packets end up 
aligned. */
+   skb = netdev_alloc_skb_ip_align(NULL, size + 1);
if (!skb) {
if (net_ratelimit())
dev_warn(>dev->dev, 
"Failed to allocate sk_buff for RX\n");
@@ -869,7 +874,10 @@ static void solos_bh(unsigned long card_arg)
/* Allocate RX skbs for any ports which need them */
if (card->using_dma && card->atmdev[port] &&
!card->rx_skb[port]) {
-   struct sk_buff *skb = alloc_skb(RX_DMA_SIZE, 
GFP_ATOMIC);
+   /* Unlike the MMIO case (qv) we can't add NET_IP_ALIGN
+* here; the FPGA can only DMA to addresses which are
+* aligned to 4 bytes. */
+   struct sk_buff *skb = dev_alloc_skb(RX_DMA_SIZE);
if (skb) {
SKB_CB(skb)->dma_addr =
dma_map_single(>dev->dev, 
skb->data,

-- 
David WoodhouseOpen Source Technology Centre
david.woodho...@intel.com  Intel Corporation



smime.p7s
Description: S/MIME cryptographic signature