diff options
author | Kirill Smelkov <kirr@mns.spb.ru> | 2013-05-03 04:22:04 +0000 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2013-05-03 16:10:34 -0400 |
commit | 88dccf5b344163645d694d74cffc5a7e64a385bb (patch) | |
tree | ddd5a21ecda0db1b312eef75397dd60e150d32a4 /drivers | |
parent | 8da3056c04bfc5f69f840ab038a38389e2de8189 (diff) |
sky2: Fix crash on receiving VLAN frames
After recent 86a9bad3 (net: vlan: add protocol argument to packet
tagging functions) my sky2 started to crash on receive of tagged
frames, with backtrace similar to
#CRASH!!!
vlan_do_receive
__netif_receive_skb_core
__netif_receive_skb
netif_receive_skb
sky2_poll
...
__net_rx_action
__do_softirq
The problem turned out to be:
1) sky2 copies small packets from ring on RX, and in its
receive_copy() skb header is copied manually field, by field, and
only for some fields;
2) 86a9bad3 added skb->vlan_proto, which vlan_untag() or
__vlan_hwaccel_put_tag() set, and which is later used in
vlan_do_receive().
That patch updated copy_skb_header() for newly introduced
skb->vlan_proto, but overlooked the need to also copy it in sky2's
receive_copy().
Because of 2, we have the following scenario:
- frame is received and tagged in a ring, by sky2_rx_tag(). Both
skb->vlan_proto and skb->vlan_tci are set;
- later skb is decided to be copied, but skb->vlan_proto is
forgotten and becomes 0.
- in the beginning of vlan_do_receive() we call
__be16 vlan_proto = skb->vlan_proto;
vlan_dev = vlan_find_dev(skb->dev, vlan_proto, vlan_id);
which eventually invokes
vlan_proto_idx(vlan_proto)
and that routine BUGs for everything except ETH_P_8021Q and
ETH_P_8021AD.
Oops.
Fix it.
P.S.
Stephen, I wonder, why copy_skb_header() is not used in
sky2.c::receive_copy() ? Problems, where receive_copy was updated field
by field showed several times already, e.g.
3f42941b (sky2: propogate rx hash when packet is copied)
e072b3fa (sky2: fix receive length error in mixed non-VLAN/VLAN traffic)
Cc: Patrick McHardy <kaber@trash.net>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: Mirko Lindner <mlindner@marvell.com>
Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'drivers')
-rw-r--r-- | drivers/net/ethernet/marvell/sky2.c | 2 |
1 files changed, 2 insertions, 0 deletions
diff --git a/drivers/net/ethernet/marvell/sky2.c b/drivers/net/ethernet/marvell/sky2.c index 256ae789c143..d175bbd3ffd3 100644 --- a/drivers/net/ethernet/marvell/sky2.c +++ b/drivers/net/ethernet/marvell/sky2.c @@ -2496,10 +2496,12 @@ static struct sk_buff *receive_copy(struct sky2_port *sky2, skb->ip_summed = re->skb->ip_summed; skb->csum = re->skb->csum; skb->rxhash = re->skb->rxhash; + skb->vlan_proto = re->skb->vlan_proto; skb->vlan_tci = re->skb->vlan_tci; pci_dma_sync_single_for_device(sky2->hw->pdev, re->data_addr, length, PCI_DMA_FROMDEVICE); + re->skb->vlan_proto = 0; re->skb->vlan_tci = 0; re->skb->rxhash = 0; re->skb->ip_summed = CHECKSUM_NONE; |