Re: [PATCH] r8169: Fix rtl8169_rx_interrupt()
From: Sergey Senozhatsky <hidden>
Date: 2010-03-16 14:55:29
Also in:
lkml
Hello,
Eric, I did pktgen testing (3 times).
sudo cat /proc/net/pktgen/eth0
Params: count 1000000000 min_pkt_size: 42 max_pkt_size: 42
frags: 0 delay: 0 clone_skb: 4000000 ifname: eth0
flows: 0 flowlen: 0
queue_map_min: 0 queue_map_max: 0
dst_min: 10.6.22.59 dst_max:
src_min: src_max:
src_mac: 00:1a:92:c9:a0:68 dst_mac: 00:1a:92:c9:a0:68
udp_src_min: 9 udp_src_max: 9 udp_dst_min: 9 udp_dst_max: 9
src_mac_count: 0 dst_mac_count: 0
Flags:
Current:
pkts-sofar: 1000000000 errors: 0
started: 9957912410us stopped: 16682620361us idle: 1231us
seq_num: 1000000001 cur_dst_mac_offset: 0 cur_src_mac_offset: 0
cur_saddr: 0x3b16060a cur_daddr: 0x3b16060a
cur_udp_dst: 9 cur_udp_src: 9
cur_queue_map: 0
flows: 0
Result: OK: 6724707951(c6724706719+d1231) nsec, 1000000000 (42byte,0frags)
148705pps 49Mb/sec (49964880bps) errors: 0
Without any errors from the r8169 side. Can we consider testing successful?
If so, fell free to add Tested-by: Sergey Senozhatsky [off-list ref]
Sergey
On (03/16/10 01:33), Eric Dumazet wrote:quoted hunk ↗ jump to hunk
quoted
Yes, this is wrong. In this context (process context, not softirq), we should use netif_rx() or just drop frames if we are in reset phase.Sergey, Here is a compiled but untested patch (I dont have the hardware), could you please test it ? Thanks [PATCH] r8169: Fix rtl8169_rx_interrupt() In case a reset is performed, rtl8169_rx_interrupt() is called from process context instead of softirq context. Special care must be taken to call appropriate network core services (netif_rx() instead of netif_receive_skb()). VLAN handling also corrected. Reported-by: Sergey Senozhatsky <redacted> Diagnosed-by: Oleg Nesterov [off-list ref] Signed-off-by: Eric Dumazet <redacted> ---diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c index 9d3ebf3..d873639 100644 --- a/drivers/net/r8169.c +++ b/drivers/net/r8169.c@@ -1038,14 +1038,14 @@ static void rtl8169_vlan_rx_register(struct net_device *dev, } static int rtl8169_rx_vlan_skb(struct rtl8169_private *tp, struct RxDesc *desc, - struct sk_buff *skb) + struct sk_buff *skb, int polling) { u32 opts2 = le32_to_cpu(desc->opts2); struct vlan_group *vlgrp = tp->vlgrp; int ret; if (vlgrp && (opts2 & RxVlanTag)) { - vlan_hwaccel_receive_skb(skb, vlgrp, swab16(opts2 & 0xffff)); + __vlan_hwaccel_rx(skb, vlgrp, swab16(opts2 & 0xffff), polling); ret = 0; } else ret = -1;@@ -1062,7 +1062,7 @@ static inline u32 rtl8169_tx_vlan_tag(struct rtl8169_private *tp, } static int rtl8169_rx_vlan_skb(struct rtl8169_private *tp, struct RxDesc *desc, - struct sk_buff *skb) + struct sk_buff *skb, int polling) { return -1; }@@ -4429,12 +4429,20 @@ out: return done; } +/* + * Warning : rtl8169_rx_interrupt() might be called : + * 1) from NAPI (softirq) context + * (polling = 1 : we should call netif_receive_skb()) + * 2) from process context (rtl8169_reset_task()) + * (polling = 0 : we must call netif_rx() instead) + */ static int rtl8169_rx_interrupt(struct net_device *dev, struct rtl8169_private *tp, void __iomem *ioaddr, u32 budget) { unsigned int cur_rx, rx_left; unsigned int delta, count; + int polling = (budget != ~(u32)0) ? 1 : 0; cur_rx = tp->cur_rx; rx_left = NUM_RX_DESC + tp->dirty_rx - cur_rx;@@ -4496,8 +4504,12 @@ static int rtl8169_rx_interrupt(struct net_device *dev, skb_put(skb, pkt_size); skb->protocol = eth_type_trans(skb, dev); - if (rtl8169_rx_vlan_skb(tp, desc, skb) < 0) - netif_receive_skb(skb); + if (rtl8169_rx_vlan_skb(tp, desc, skb, polling) < 0) { + if (likely(polling)) + netif_receive_skb(skb); + else + netif_rx(skb); + } dev->stats.rx_bytes += pkt_size; dev->stats.rx_packets++;
Attachments
- (unnamed) [application/pgp-signature] 316 bytes