[#3870] bfin_mac: RX from DMA to skb payload bug if cache policy is WB
Submitted By: Bryan Wu
2008-01-28 21:16:35 Close Date
Closed Fixed In Release:
Found In Release:
N/A Silicon Revision:
Is this bug repeatable?:
Uboot version or rev.:
Toolchain version or rev.:
App binary format:
Summary: bfin_mac: RX from DMA to skb payload bug if cache policy is WB
Reported by: Alexey Demin <firstname.lastname@example.org> in the forum
I guess there is a bug in the receiving path of bfin_mac driver when cache policy is WriteBack.
DMA Rx use skb payload area to receive ethernet packets.
When skb has been destroyed with kfree_skb, the kernel can reuse this payload area
for another packet.
If this payload has been changed(for example, ip forwarding with ttl decrement)
the changes can be stored in cache.
When dma transfers data of new packet to the payload area(skb->data) and
the cache of deleted packet with the same payload area flushes its changes simultaneously, the data in payload of new packet can be replaced with data of old packet.
So we must invalidate cache of payload before dma will be able
to send new data to this payload.
Here is the diffs with checking for bug and its fixing.
Checking bug presence:
uncomment #define CHECK_CACHE_BUG
make sure CONFIG_BKFIN_WB cache policy is ON.
ping -i 0 or ping -f from any host to this box.
uncomment #define FIX_CACHE_BUG"
With best regards, Lex.
--- bfin_mac.c&revision=4101 2008-01-21 20:17:20.000000000 +0300
+++ bfin_mac.c&revision=4101_fixed 2008-01-25 14:54:21.000000000 +0300
@@ -636,6 +636,17 @@
+#if defined(FIX_CACHE_BUG) && defined(CONFIG_BLKFIN_WT)
+#error There is nothing to fix when write through (WT) cache policy is ON
+#include <linux/ip.h> /* struct iphdr */
static void bf537mac_rx(struct net_device *dev)
struct sk_buff *skb, *new_skb;
@@ -653,6 +664,14 @@
/* reserve 2 bytes for RXDWA padding */
+ /*Important!!! We must do it to cancel flushing of changes from cache
+ * back to payload filling with new data from DMA
+ blackfin_dcache_invalidate_range((unsigned long)new_skb->head, (unsigned long)new_skb->end);
current_rx_ptr->skb = new_skb;
current_rx_ptr->desc_a.start_addr = (unsigned long)new_skb->data - 2;
@@ -669,6 +688,28 @@
skb->ip_summed = CHECKSUM_COMPLETE;
+ if (skb->protocol == htons(ETH_P_IP))
+ struct iphdr* iph = (struct iphdr*) skb->data;
+ printk(KERN_DEBUG "bfin_mac rx_pkt: skb=x%p(x%p-x%p), id=%d, pkt_len=%d, ttl=%d\n",
+ skb,skb->head, skb->end,
+ if (iph->ttl == 55)
+ printk("ooooops, new packet has been corrupted with old data!!!!\n");
+ iph->ttl = 55;
+ iph->check = 0;
+ iph->check = ip_fast_csum((unsigned char *)iph, iph->ihl);
lp->stats.rx_bytes += len;
--- Bryan Wu 2008-01-29 06:37:00
it was fixed in 08R1 and trunk.
in the driver:
1. old_skb->data filled with DMA TTL=64, then you modified it to TTL=55.
Noticed this TTL=55 is in data cache before write to memory.
2. old_skb send to upper level, finally it was free by kfree_skb. Here in data
cache should be different from real memory if not write to memory.
3. bf537mac_rx() call dev_alloc_skb() to get a new_skb. As you point out, this
new_skb->data might be the same as old_skb->data.
4. Soon, DMA will fill this new_skb->data with new data TTL=64 (in real
memory), while in data cache it is TTL=55.
5. Maybe soon, data cache was written to the memory and overwrite the data,
6. So we got this bug.
With you patch, the new_skb->data cache was invalidated, so there is no
further data cache writing operation to memory any more. DMA data will not be
Thanks, I will apply this to our svn and 08R1 release, then send it to upstream
One suggestion is maybe this cache invalidate operation should be added to
alloc_skb(), other architecture can share this protection.
--- Bryan Wu 2008-01-31 02:37:57
It fixed. so close it.
File Name File Type File Size Posted By
No Files Were Found