Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH V2 1/6] rtlwifi: Change logging level for key change
From: Kalle Valo @ 2015-02-03 13:01 UTC (permalink / raw)
  To: Larry Finger; +Cc: linux-wireless, netdev
In-Reply-To: <54CBA794.90508@lwfinger.net>

Larry Finger <Larry.Finger@lwfinger.net> writes:

> On 01/26/2015 02:42 PM, Larry Finger wrote:
>> A recent change in key handling included logging of these changes for
>> all debug levels. Such key changes should only be logged when a high
>> level of debugging is enabled.
>>
>> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
>> ---
>>   drivers/net/wireless/rtlwifi/cam.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/rtlwifi/cam.c b/drivers/net/wireless/rtlwifi/cam.c
>> index 3ef870d..6e64792 100644
>> --- a/drivers/net/wireless/rtlwifi/cam.c
>> +++ b/drivers/net/wireless/rtlwifi/cam.c
>> @@ -406,7 +406,7 @@ u8 rtl_cam_get_free_entry(struct ieee80211_hw *hw, struct ieee80211_sta *sta,
>>   		}
>>   	}
>>   	if (found) {
>> -		RT_TRACE(rtlpriv, COMP_SEC, DBG_EMERG,
>> +		RT_TRACE(rtlpriv, COMP_SEC, DBG_DMESG,
>>   			 "key_index=%d,cam_bitmap: 0x%x entry_idx=%d\n",
>>   			  key_index, rtlpriv->sec.cam_bitmap, entry_idx);
>>   		return entry_idx;
>>
>
> Kalle,
>
> Please include this patch even though the rest of this set should be
> dropped.

I tried to do that but it failed:

Applying: rtlwifi: Change logging level for key change
fatal: sha1 information is lacking or useless (drivers/net/wireless/rtlwifi/cam.c).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 rtlwifi: Change logging level for key change

I was not even able to find that code from cam.c. Have I missed a patch?

> Once the wifi-BT communications problem is resolved, new versions of
> those will be presented.

Sounds good.

> I'm sorry that my commit message was not as informative as it could
> have been. I was presented with this material that I did not
> understand. One saving grace is that the RTL8812AE hardware is
> apparently rare - I certainly do not have any samples. I'm not sure of
> the value of a 1x1 implementation of 802.11ac, but that may be another
> example of my ignorance.

No worries. But stuff like this is usually best to send as RFC first and
ask people to analyse it before sending it officially.

-- 
Kalle Valo

^ permalink raw reply

* btcoex subsystem
From: Kalle Valo @ 2015-02-03 12:29 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Larry Finger, linux-wireless, Troy Tan, netdev, linux-bluetooth
In-Reply-To: <B814EF26-CBC9-43E4-A6C9-761A013F85C1@holtmann.org>

Marcel Holtmann <marcel@holtmann.org> writes:

>> I will work on combining the latest BT drivers from Realtek with
>> btusb to see if I can achieve a patch that will both work with the
>> Realtek hardware, and get approval from the reviewers.
>> 
>> What would be an approved method of communicating between two kernel
>> modules? Is there some example in the kernel that I could study?
>
> We need a btcoex subsystem that both WiFi and Bluetooth can register
> to and communicate with.

The need for this seems to periodically come up, I remember needing
something like that back in the Maemo wl1251 days and also ath6kl needed
this. Are there any ideas for this subsystem? How would this btcoex
subsystem work? What kind of information would it provide?

Is this something we should discuss at Ottawa?

-- 
Kalle Valo

^ permalink raw reply

* Re: [PATCHv3, ipsec-next] xfrm: Do not parse 32bits compiled xfrm netlink msg on 64bits host
From: Steffen Klassert @ 2015-02-03 12:24 UTC (permalink / raw)
  To: Nicolas Dichtel; +Cc: Fan Du, herbert, davem, netdev, fengyuleidian0615
In-Reply-To: <54CF3D3A.5040607@6wind.com>

On Mon, Feb 02, 2015 at 10:02:50AM +0100, Nicolas Dichtel wrote:
> Le 02/02/2015 09:44, Steffen Klassert a écrit :
> >On Thu, Jan 29, 2015 at 11:29:51AM +0100, Nicolas Dichtel wrote:
> [snip]
> >>
> >>The point I try to make is that patching userland apps allows to use xfrm on a
> >>32bits userland / 64bits kernel.
> >
> >Ugh, I did not know that this is used that way. Which applications do this?
> >So the situation is worse than I thought. What happens to such applications
> >if we add a compat layer in the kernel? I'd guess they will break, right?
> A compat layer will be perfect. I just wanted to highlight the fact that without
> this patch, it's possible to have a workaround to use netlink-xfrm and after it,
> it will be impossible.

You did not answer my question about the applications that do this.
If it is just possible, but there are no actual users, we should
apply this patch as soon as possible to avoid any abuse of this ABI.

I tend to apply this patch unless you can come up with a real world
application that will break if we do so.

^ permalink raw reply

* Re: Question: should local address be expired when updating PMTU?
From: Steffen Klassert @ 2015-02-03 12:01 UTC (permalink / raw)
  To: shengyong; +Cc: davem, netdev, yangyingliang, hannes
In-Reply-To: <54D0A8DB.4010106@huawei.com>

On Tue, Feb 03, 2015 at 06:54:19PM +0800, shengyong wrote:
> 
> 
> 在 2015/2/3 17:28, Steffen Klassert 写道:
> > On Mon, Feb 02, 2015 at 04:20:24PM +0800, shengyong wrote:
> > 
> > We first need to find out why you receive this Packet Too Big message,
> The packet is sent by a commercial-off-the-shelf testcase, and I can reproduce the
> situation by using scapy and creating a packet as the following:
> 
> 	$ cat packet-too-big.py
> 	#!/usr/bin/python
> 	
> 	from scapy.all import *
> 
> 	# fe80::800:27ff:fe00:0 is linklocal addr of PC
> 	# fe80::a00:27ff:fe1a:e2a0 is linklocal addr of VM
> 	base=IPv6(src='fe80::800:27ff:fe00:0',dst='fe80::a00:27ff:fe1a:e2a0')
> 	pkt_too_big=ICMPv6PacketTooBig(mtu=1024)
> 	ext_base=IPv6(src='fe80::a00:27ff:fe1a:e2a0',dst='fe80::a00:27ff:fe1a:e2a0',plen=24)
> 	ext_nd_na=ICMPv6ND_NA()
> 	
> 	packet=base/pkt_too_big/ext_base/ext_nd_na
> 	send(packet)

So it is not a valid pmtu update, this make life easier.

Can you please test the patch below (compile tested only)?

This should fix your problem, and in combination with the two patches I sent
out last week, it should cure the whole 'expiring of uncached routes' problem.

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 49596535..4ccfb9c 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1156,7 +1156,8 @@ static void ip6_rt_update_pmtu(struct dst_entry *dst, struct sock *sk,
 	struct rt6_info *rt6 = (struct rt6_info *)dst;
 
 	dst_confirm(dst);
-	if (mtu < dst_mtu(dst) && rt6->rt6i_dst.plen == 128) {
+	if (mtu < dst_mtu(dst) && rt6->rt6i_dst.plen == 128 &&
+	    (rt6->rt6i_flags & RTF_CACHE)) {
 		struct net *net = dev_net(dst->dev);
 
 		rt6->rt6i_flags |= RTF_MODIFIED;

^ permalink raw reply related

* Re: [PATCH] ath9k_htc: add adaptive usb flow control to repair soft lockup with monitor mode
From: Kalle Valo @ 2015-02-03 11:52 UTC (permalink / raw)
  To: yuweizheng
  Cc: linux-kernel, ath9k-devel, linux-wireless, ath9k-devel, netdev,
	Yuwei Zheng
In-Reply-To: <1422957585-19736-1-git-send-email-yuweizheng@139.com>

yuweizheng@139.com writes:

> From: Yuwei Zheng <yuweizheng@139.com>
>
> In the environment with heavy wifi traffic, set the ar9271 into monitor mode, will
> trigger a deadloop panic.
>  
> The ath9k_hif_usb_rx_cb function excute on  the interrupt context, and ath9k_rx_tasklet excute
> on the soft irq context. In other words, the ath9k_hif_usb_rx_cb have more chance to excute than
> ath9k_rx_tasklet.  So in the worst condition,  the rx.rxbuf receive list is always full,
> and the do {}while(true) loop will not be break. The kernel get a soft lockup panic. 

The word wrapping is still wrong, please limit the width to 72 chars or
so. But no need to wrap log snippets, they can exceed the limit.

> Signed-off-by: Yuwei Zheng <zhengyuwei@360.cn>
> Signed-off-by: Yuwei Zheng <yuweizheng@139.com>

Why two of these? Please use just one Signed-off-by line. Unless these
are two different persons with the same name :)

-- 
Kalle Valo

^ permalink raw reply

* Re: Throughput regression with `tcp: refine TSO autosizing`
From: Michal Kazior @ 2015-02-03 11:50 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: linux-wireless, Network Development, eyalpe
In-Reply-To: <1422926330.21689.138.camel@edumazet-glaptop2.roam.corp.google.com>

On 3 February 2015 at 02:18, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Mon, 2015-02-02 at 10:52 -0800, Eric Dumazet wrote:
>
>> It seems to break ACK clocking badly (linux stack has a somewhat buggy
>> tcp_tso_should_defer(), which relies on ACK being received smoothly, as
>> no timer is setup to split the TSO packet.)
>
> Following patch might help the TSO split defer logic.
>
> It would avoid setting the TSO defer 'pseudo timer' twice, if/when TCP
> Small Queue logic prevented the xmit at the expiration of first 'timer'.
>
> This patch clears the tso_deferred variable only if we could really
> send something.
>
> Please try it, thanks !
[..patch..]

I've done a second round of tests. I've added the A-MSDU count
parameter I've mentioned in my other email into the mix.

 net - net/master (includes stretch ack patches)
 net-tso - net/master + your TSO defer patch
 net-gro - net/master + my ath10k GRO patch
 net-gro-tso - net/master + duh

Here's the best of amsdu count 1 and 3:

 ; for (i in */output.txt) { echo $i; for (j in (1 3)) { cat $i | awk
'x && /Mbits/ {y=$0}; x && y && !/Mbits/ {print y; x=0; y=""}; /set
amsdu cnt to '$j'/{x=1}' | awk '{ if (x < $(NF-1)) {x=$(NF-1)} }
END{print "A-MSDU limit='$j', " x " Mbits/sec"}' } }
 net-gro-tso/output.txt
 A-MSDU limit=1, 436 Mbits/sec
 A-MSDU limit=3, 284 Mbits/sec
 net-gro/output.txt
 A-MSDU limit=1, 444 Mbits/sec
 A-MSDU limit=3, 283 Mbits/sec
 net-tso/output.txt
 A-MSDU limit=1, 376 Mbits/sec
 A-MSDU limit=3, 251 Mbits/sec
 net/output.txt
 A-MSDU limit=1, 387 Mbits/sec
 A-MSDU limit=3, 260 Mbits/sec

IOW:
 - stretch acks / TSO defer don't seem to help much (when compared to
throughput results from yesterday)
 - GRO helps
 - disabling A-MSDU on sender helps
 - net/master+GRO still doesn't reach the performance from before the
regression (~600mbps w/ GRO)

You can grab logs and dumps here: http://www.filedropper.com/test2tar


Michał

^ permalink raw reply

* [PATCH] csiostor:Use firmware version from cxgb4/t4fw_version.h
From: Praveen Madhavan @ 2015-02-03 11:48 UTC (permalink / raw)
  To: netdev, linux-scsi; +Cc: davem, JBottomley, hch, hariprasad, praveenm

This patch is to use firmware version macros from t4fw_version.h
and also enables 40g T5 adapter.

Signed-off-by: Praveen Madhavan <praveenm@chelsio.com>
---
 drivers/scsi/csiostor/csio_hw.c      | 6 ++++--
 drivers/scsi/csiostor/csio_hw_chip.h | 6 +-----
 drivers/scsi/csiostor/csio_mb.c      | 3 ++-
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/csiostor/csio_hw.c b/drivers/scsi/csiostor/csio_hw.c
index c81b06b..2e66f34 100644
--- a/drivers/scsi/csiostor/csio_hw.c
+++ b/drivers/scsi/csiostor/csio_hw.c
@@ -1372,7 +1372,8 @@ csio_config_device_caps(struct csio_hw *hw)
 	}
 
 	/* Validate device capabilities */
-	if (csio_hw_validate_caps(hw, mbp))
+	rv = csio_hw_validate_caps(hw, mbp);
+	if (rv != 0)
 		goto out;
 
 	/* Don't config device capabilities if already configured */
@@ -1776,7 +1777,8 @@ csio_hw_use_fwconfig(struct csio_hw *hw, int reset, u32 *fw_cfg_param)
 	}
 
 	/* Validate device capabilities */
-	if (csio_hw_validate_caps(hw, mbp))
+	rv = csio_hw_validate_caps(hw, mbp);
+	if (rv != 0)
 		goto bye;
 	/*
 	 * Note that we're operating with parameters
diff --git a/drivers/scsi/csiostor/csio_hw_chip.h b/drivers/scsi/csiostor/csio_hw_chip.h
index a5f624f..b56a11d 100644
--- a/drivers/scsi/csiostor/csio_hw_chip.h
+++ b/drivers/scsi/csiostor/csio_hw_chip.h
@@ -45,11 +45,6 @@
 #define FW_FNAME_T5				"cxgb4/t5fw.bin"
 #define FW_CFG_NAME_T5				"cxgb4/t5-config.txt"
 
-#define T5FW_VERSION_MAJOR 0x01
-#define T5FW_VERSION_MINOR 0x0B
-#define T5FW_VERSION_MICRO 0x1B
-#define T5FW_VERSION_BUILD 0x00
-
 #define CHELSIO_CHIP_CODE(version, revision) (((version) << 4) | (revision))
 #define CHELSIO_CHIP_FPGA          0x100
 #define CHELSIO_CHIP_VERSION(code) (((code) >> 12) & 0xf)
@@ -74,6 +69,7 @@ static inline int csio_is_t5(uint16_t chip)
 	{ PCI_VENDOR_ID_CHELSIO, (devid), PCI_ANY_ID, PCI_ANY_ID, 0, 0, (idx) }
 
 #include "t4fw_api.h"
+#include "t4fw_version.h"
 
 #define FW_VERSION(chip) ( \
 		FW_HDR_FW_VER_MAJOR_G(chip##FW_VERSION_MAJOR) | \
diff --git a/drivers/scsi/csiostor/csio_mb.c b/drivers/scsi/csiostor/csio_mb.c
index 1132c41..9451787 100644
--- a/drivers/scsi/csiostor/csio_mb.c
+++ b/drivers/scsi/csiostor/csio_mb.c
@@ -327,7 +327,8 @@ csio_mb_caps_config(struct csio_hw *hw, struct csio_mb *mbp, uint32_t tmo,
 }
 
 #define CSIO_ADVERT_MASK     (FW_PORT_CAP_SPEED_100M | FW_PORT_CAP_SPEED_1G |\
-			      FW_PORT_CAP_SPEED_10G | FW_PORT_CAP_ANEG)
+			      FW_PORT_CAP_SPEED_10G | FW_PORT_CAP_SPEED_40G |\
+			      FW_PORT_CAP_ANEG)
 
 /*
  * csio_mb_port- FW PORT command helper
-- 
2.0.2


^ permalink raw reply related

* Re: [GIT PULL nf] IPVS Fixes for v3.19
From: Pablo Neira Ayuso @ 2015-02-03 11:47 UTC (permalink / raw)
  To: Simon Horman
  Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
	Julian Anastasov
In-Reply-To: <1422580938-31520-1-git-send-email-horms@verge.net.au>

On Fri, Jan 30, 2015 at 10:22:17AM +0900, Simon Horman wrote:
> Hi Pablo,
> 
> please consider this fix for v3.19.
> 
> It resolves a crash in xfrm reported by Florian Wiessner.
> 
> I believe this problem manifests since 0a5ebb8000c5 ("ipv4: Pass explicit
> daddr arg to ip_send_reply().") which was included in v2.6.39.
> 
> Julian reports that the patch has been tested on net tree (Dec 7), 3.14.25,
> 3.12.33, 3.10.61. 3.4.104 needs a modified fix that resolves rejects. It
> applies with little fuzz on 3.2.64. Please let us know if we should post
> separate 3.2 and 3.4 patches.

Regarding -stable. This seems to apply cleanly to:

3.10.x
3.12.x
3.14.x
3.18.x

Not compiled tested yet, just clean application.

Lower versions need some backporting, I leave this to you to evaluate
if it is worth the effort.

Thanks!

^ permalink raw reply

* Re: Question: should local address be expired when updating PMTU?
From: shengyong @ 2015-02-03 10:54 UTC (permalink / raw)
  To: Steffen Klassert; +Cc: davem, netdev, yangyingliang, hannes
In-Reply-To: <20150203092845.GT13046@secunet.com>



在 2015/2/3 17:28, Steffen Klassert 写道:
> On Mon, Feb 02, 2015 at 04:20:24PM +0800, shengyong wrote:
>> Hi, David Miller
>> Since commit 81aded246 (ipv6: Handle PMTU in ICMP error handlers), the entries
>> in neigh table may get expired. But in the situation:
>>
>>           Host only
>>     PC <------------> Virtual Machine
>>
>> a packet is sent from PC to VM, and the packet looks like:
>> -----------------------------------
>> | IPv6 (src=PC-addr, dst=VM-addr) |
>> |---------------------------------|
>> |     ICMPv6 (Packet Too Big)     |
>> |---------------------------------|
>> | IPv6 (src=VM-addr, dst=VM-addr) |
>> |---------------------------------|
>> | ICMPv6 (Neighbor Advertisement) |
>> -----------------------------------
>>
>> Then the local addr on VM will be updated with an expire value. After the
>> lifetime of the local addr is expired, the VM is unreachable from PC.
>>
>> 	# ip -6 route list table local
>> 	local fe80::1 dev lo  metric 0 *expire 596*
> 
> We first need to find out why you receive this Packet Too Big message,
The packet is sent by a commercial-off-the-shelf testcase, and I can reproduce the
situation by using scapy and creating a packet as the following:

	$ cat packet-too-big.py
	#!/usr/bin/python
	
	from scapy.all import *

	# fe80::800:27ff:fe00:0 is linklocal addr of PC
	# fe80::a00:27ff:fe1a:e2a0 is linklocal addr of VM
	base=IPv6(src='fe80::800:27ff:fe00:0',dst='fe80::a00:27ff:fe1a:e2a0')
	pkt_too_big=ICMPv6PacketTooBig(mtu=1024)
	ext_base=IPv6(src='fe80::a00:27ff:fe1a:e2a0',dst='fe80::a00:27ff:fe1a:e2a0',plen=24)
	ext_nd_na=ICMPv6ND_NA()
	
	packet=base/pkt_too_big/ext_base/ext_nd_na
	send(packet)

> can you capture this packet somehow? 
I captured the packet in wireshark, it is exact the packet created by the script.
> Then we have to see why this loopback
> route gets a pmtu update from that packet.
I tried to print info when the VM receives pkt-too-big packet. The calling stack is
icmpv6_rcv->icmpv6_notify->icmpv6_err->ip6_update_pmtu->ip6_rt_update_pmtu->rt6_update_expires.
In ip6_update_pmtu, ip6_route_output looks up the route table, and returns the dst_entry of the
linklocal addr. Then it is set to expire.
> Is the destination address
> of the Packet Too Big message really fe80::1?
In fact, if the dst of the above `ext_base' is the local addr of the VM, the local addr on VM
will be expired.

thx,
Sheng

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

^ permalink raw reply

* Re: [PATCH] ath9k_htc: add adaptive rate control to repair soft lockup with monitor mode
From: Oleksij Rempel @ 2015-02-03 10:39 UTC (permalink / raw)
  To: yuweizheng, linux-kernel, ath9k-devel, linux-wireless, kvalo,
	ath9k-devel
  Cc: netdev, Yuwei Zheng
In-Reply-To: <1422955270-19499-1-git-send-email-yuweizheng@139.com>

[-- Attachment #1: Type: text/plain, Size: 12884 bytes --]

Looks good,

please just one note. Rename "rate control" to "usb flow control", or
some thing like this. In this context "rate control" has different meaning.

I'll run you patches on my system with different adapters.

Am 03.02.2015 um 10:21 schrieb yuweizheng@139.com:
> From: Yuwei Zheng <yuweizheng@139.com>
> 
> In the environment with heavy wifi traffic, set the ar9271 into monitor mode, will
> trigger a deadloop panic.
> The ath9k_hif_usb_rx_cb function excute on  the interrupt context, and ath9k_rx_tasklet excute
> on the soft irq context. In other words, the ath9k_hif_usb_rx_cb have more chance to excute than
> ath9k_rx_tasklet.  So in the worst condition,  the rx.rxbuf receive list is always full,
> and the do {}while(true) loop will not be break. The kernel get a soft lockup panic.
> [59011.007210] BUG: soft lockup - CPU#0 stuck for 23s!
> [kworker/0:0:30609]
> [59011.030560] BUG: scheduling while atomic: kworker/0:0/30609/0x40010100
> [59013.804486] BUG: scheduling while atomic: kworker/0:0/30609/0x40010100
> [59013.858522] Kernel panic - not syncing: softlockup: hung tasks
> [59014.038891] Exception stack(0xdf4bbc38 to 0xdf4bbc80)
> [59014.046834] bc20:                                                       de57b950 60000113
> [59014.059579] bc40: 00000000 bb32bb32 60000113 de57b948 de57b500 dc7bb440 df4bbcd0 00000000
> [59014.072337] bc60: de57b950 60000113 df4bbcd0 df4bbc80 c04c259d c04c25a0 60000133 ffffffff
> [59014.085233] [<c04c28db>] (__irq_svc+0x3b/0x5c) from [<c04c25a0>] (_raw_spin_unlock_irqrestore+0xc/0x10)
> [59014.100437] [<c04c25a0>] (_raw_spin_unlock_irqrestore+0xc/0x10) from [<bf9c2089>] (ath9k_rx_tasklet+0x290/0x490 [ath9k_htc])
> [59014.118267] [<bf9c2089>] (ath9k_rx_tasklet+0x290/0x490 [ath9k_htc]) from [<c0036d23>] (tasklet_action+0x3b/0x98)
> [59014.134132] [<c0036d23>] (tasklet_action+0x3b/0x98) from [<c0036709>] (__do_softirq+0x99/0x16c)
> [59014.147784] [<c0036709>] (__do_softirq+0x99/0x16c) from [<c00369f7>] (irq_exit+0x5b/0x5c)
> [59014.160653] [<c00369f7>] (irq_exit+0x5b/0x5c) from [<c000cfc3>] (handle_IRQ+0x37/0x78)
> [59014.173124] [<c000cfc3>] (handle_IRQ+0x37/0x78) from [<c00085df>] (omap3_intc_handle_irq+0x5f/0x68)
> [59014.187225] [<c00085df>] (omap3_intc_handle_irq+0x5f/0x68) from [<c04c28db>](__irq_svc+0x3b/0x5c)
> This bug can be see with low performance board, such as uniprocessor beagle bone board.
> Signed-off-by: Yuwei Zheng <zhengyuwei@360.cn>
> Signed-off-by: Yuwei Zheng <yuweizheng@139.com>
> 
> ---
>  drivers/net/wireless/ath/ath9k/hif_usb.c       | 61 +++++++++++++++++++++++---
>  drivers/net/wireless/ath/ath9k/hif_usb.h       |  6 +++
>  drivers/net/wireless/ath/ath9k/htc.h           | 18 ++++++++
>  drivers/net/wireless/ath/ath9k/htc_drv_debug.c | 46 +++++++++++++++++++
>  drivers/net/wireless/ath/ath9k/htc_drv_txrx.c  | 25 +++++++++++
>  5 files changed, 149 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c
> index 8e7153b..9166f10 100644
> --- a/drivers/net/wireless/ath/ath9k/hif_usb.c
> +++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
> @@ -640,6 +640,7 @@ static void ath9k_hif_usb_rx_cb(struct urb *urb)
>  	struct hif_device_usb *hif_dev =
>  		usb_get_intfdata(usb_ifnum_to_if(urb->dev, 0));
>  	int ret;
> +	int delay;
>  
>  	if (!skb)
>  		return;
> @@ -658,7 +659,6 @@ static void ath9k_hif_usb_rx_cb(struct urb *urb)
>  	default:
>  		goto resubmit;
>  	}
> -
>  	if (likely(urb->actual_length != 0)) {
>  		skb_put(skb, urb->actual_length);
>  		ath9k_hif_usb_rx_stream(hif_dev, skb);
> @@ -667,12 +667,18 @@ static void ath9k_hif_usb_rx_cb(struct urb *urb)
>  resubmit:
>  	skb_reset_tail_pointer(skb);
>  	skb_trim(skb, 0);
> -
> -	usb_anchor_urb(urb, &hif_dev->rx_submitted);
> -	ret = usb_submit_urb(urb, GFP_ATOMIC);
> -	if (ret) {
> -		usb_unanchor_urb(urb);
> -		goto free;
> +	if (atomic_read(&hif_dev->rx_urb_submit_delay) > 0) {
> +		usb_anchor_urb(urb, &hif_dev->rx_delayed_submitted);
> +		delay = atomic_read(&hif_dev->rx_urb_submit_delay);
> +		schedule_delayed_work(&hif_dev->rx_submit_delayed_work,
> +				      usecs_to_jiffies(delay));
> +	} else {
> +		usb_anchor_urb(urb, &hif_dev->rx_submitted);
> +		ret = usb_submit_urb(urb, GFP_ATOMIC);
> +		if (ret) {
> +			usb_unanchor_urb(urb);
> +			goto free;
> +		}
>  	}
>  
>  	return;
> @@ -818,9 +824,43 @@ err:
>  	return -ENOMEM;
>  }
>  
> +static void rx_urb_delayed_submit_handler(struct work_struct *work)
> +{
> +	struct hif_device_usb *hif_dev =
> +		container_of(work,
> +			     struct hif_device_usb,
> +			     rx_submit_delayed_work.work);
> +
> +	struct urb *urb = NULL;
> +	struct sk_buff *skb = NULL;
> +	int ret;
> +	int loop_times = 0;
> +
> +	while (true) {
> +		loop_times++;
> +		if (loop_times > (MAX_RX_URB_NUM))
> +			atomic_add(ARC_RX_STEP, &hif_dev->rx_urb_submit_delay);
> +
> +		urb = usb_get_from_anchor(&hif_dev->rx_delayed_submitted);
> +		if (urb) {
> +			skb = (struct sk_buff *)urb->context;
> +			ret = usb_submit_urb(urb, GFP_KERNEL);
> +			if (ret != 0) {
> +				usb_unanchor_urb(urb);
> +				dev_kfree_skb_any(skb);
> +				urb->context = NULL;
> +			}
> +		} else {
> +			break;
> +		}
> +	}
> +}
> +
>  static void ath9k_hif_usb_dealloc_rx_urbs(struct hif_device_usb *hif_dev)
>  {
>  	usb_kill_anchored_urbs(&hif_dev->rx_submitted);
> +	usb_kill_anchored_urbs(&hif_dev->rx_delayed_submitted);
> +	flush_delayed_work(&hif_dev->rx_submit_delayed_work);
>  }
>  
>  static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
> @@ -830,6 +870,8 @@ static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
>  	int i, ret;
>  
>  	init_usb_anchor(&hif_dev->rx_submitted);
> +	init_usb_anchor(&hif_dev->rx_delayed_submitted);
> +
>  	spin_lock_init(&hif_dev->rx_lock);
>  
>  	for (i = 0; i < MAX_RX_URB_NUM; i++) {
> @@ -871,6 +913,11 @@ static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
>  		usb_free_urb(urb);
>  	}
>  
> +	/* add for adaptive rate control*/
> +	atomic_set(&hif_dev->rx_urb_submit_delay, 0);
> +	INIT_DELAYED_WORK(&hif_dev->rx_submit_delayed_work,
> +			  rx_urb_delayed_submit_handler);
> +
>  	return 0;
>  
>  err_submit:
> diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.h b/drivers/net/wireless/ath/ath9k/hif_usb.h
> index 51496e7..2127e50 100644
> --- a/drivers/net/wireless/ath/ath9k/hif_usb.h
> +++ b/drivers/net/wireless/ath/ath9k/hif_usb.h
> @@ -41,6 +41,7 @@
>  #define MAX_RX_URB_NUM  8
>  #define MAX_RX_BUF_SIZE 16384
>  #define MAX_PKT_NUM_IN_TRANSFER 10
> +#define ARC_RX_STEP 100 /* 100us */
>  
>  #define MAX_REG_OUT_URB_NUM  1
>  #define MAX_REG_IN_URB_NUM   64
> @@ -98,9 +99,14 @@ struct hif_device_usb {
>  	struct hif_usb_tx tx;
>  	struct usb_anchor regout_submitted;
>  	struct usb_anchor rx_submitted;
> +	struct usb_anchor rx_delayed_submitted; /* delayed submit anchor */
>  	struct usb_anchor reg_in_submitted;
>  	struct usb_anchor mgmt_submitted;
>  	struct sk_buff *remain_skb;
> +
> +	struct delayed_work rx_submit_delayed_work;
> +	atomic_t  rx_urb_submit_delay; /*us*/
> +
>  	const char *fw_name;
>  	int rx_remain_len;
>  	int rx_pkt_len;
> diff --git a/drivers/net/wireless/ath/ath9k/htc.h b/drivers/net/wireless/ath/ath9k/htc.h
> index 9dde265..3061dfa 100644
> --- a/drivers/net/wireless/ath/ath9k/htc.h
> +++ b/drivers/net/wireless/ath/ath9k/htc.h
> @@ -331,6 +331,13 @@ static inline struct ath9k_htc_tx_ctl *HTC_SKB_CB(struct sk_buff *skb)
>  
>  #define TX_QSTAT_INC(q) (priv->debug.tx_stats.queue_stats[q]++)
>  
> +#define ARCRX_STAT_INC(c) \
> +	(hif_dev->htc_handle->drv_priv->debug.arcrx_stats.c++)
> +#define ARCRX_STAT_ADD(c, a) \
> +	(hif_dev->htc_handle->drv_priv->debug.arcrx_stats.c += a)
> +#define ARCRX_STAT_SET(c, a) \
> +	(hif_dev->htc_handle->drv_priv->debug.arcrx_stats.c = a)
> +
>  void ath9k_htc_err_stat_rx(struct ath9k_htc_priv *priv,
>  			   struct ath_rx_status *rs);
>  
> @@ -352,11 +359,19 @@ struct ath_skbrx_stats {
>  	u32 skb_dropped;
>  };
>  
> +struct ath_arcrx_stats {
> +	u32 arcrx_highwater;
> +	u32 arcrx_lowwater;
> +	u32 arcrx_watermark_triggered;
> +	u32 arcrx_urb_submit_delay;
> +};
> +
>  struct ath9k_debug {
>  	struct dentry *debugfs_phy;
>  	struct ath_tx_stats tx_stats;
>  	struct ath_rx_stats rx_stats;
>  	struct ath_skbrx_stats skbrx_stats;
> +	struct ath_arcrx_stats arcrx_stats;
>  };
>  
>  void ath9k_htc_get_et_strings(struct ieee80211_hw *hw,
> @@ -377,6 +392,9 @@ void ath9k_htc_get_et_stats(struct ieee80211_hw *hw,
>  
>  #define TX_QSTAT_INC(c) do { } while (0)
>  
> +#define ARCRX_STAT_INC(c) do {} while (0)
> +#define ARCRX_STAT_ADD(c, a) do {} while (0)
> +#define ARCRX_STAT_SET(c, a) do {} while (0)
>  static inline void ath9k_htc_err_stat_rx(struct ath9k_htc_priv *priv,
>  					 struct ath_rx_status *rs)
>  {
> diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_debug.c b/drivers/net/wireless/ath/ath9k/htc_drv_debug.c
> index 8cef1ed..ddc8f4f 100644
> --- a/drivers/net/wireless/ath/ath9k/htc_drv_debug.c
> +++ b/drivers/net/wireless/ath/ath9k/htc_drv_debug.c
> @@ -286,6 +286,48 @@ static const struct file_operations fops_skb_rx = {
>  	.llseek = default_llseek,
>  };
>  
> +static ssize_t read_file_arc_rx(struct file *file, char __user *user_buf,
> +				size_t count, loff_t *ppos)
> +{
> +	struct ath9k_htc_priv *priv = file->private_data;
> +	char *buf;
> +	unsigned int len = 0, size = 1500;
> +	ssize_t retval = 0;
> +
> +	buf = kzalloc(size, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	len += scnprintf(buf + len, size - len,
> +			"%20s : %10u\n", "High watermark",
> +			priv->debug.arcrx_stats.arcrx_highwater);
> +	len += scnprintf(buf + len, size - len,
> +			"%20s : %10u\n", "Low watermark",
> +			priv->debug.arcrx_stats.arcrx_lowwater);
> +
> +	len += scnprintf(buf + len, size - len,
> +			"%20s : %10u\n", "WM triggered",
> +			priv->debug.arcrx_stats.arcrx_watermark_triggered);
> +
> +	len += scnprintf(buf + len, size - len,
> +			"%20s : %10u\n", "URB delay",
> +			priv->debug.arcrx_stats.arcrx_urb_submit_delay);
> +	if (len > size)
> +		len = size;
> +
> +	retval = simple_read_from_buffer(user_buf, count, ppos, buf, len);
> +	kfree(buf);
> +
> +	return retval;
> +}
> +
> +static const struct file_operations fops_arc_rx = {
> +	.read = read_file_arc_rx,
> +	.open = simple_open,
> +	.owner = THIS_MODULE,
> +	.llseek = default_llseek,
> +};
> +
>  static ssize_t read_file_slot(struct file *file, char __user *user_buf,
>  			      size_t count, loff_t *ppos)
>  {
> @@ -518,7 +560,11 @@ int ath9k_htc_init_debug(struct ath_hw *ah)
>  	debugfs_create_file("skb_rx", S_IRUSR, priv->debug.debugfs_phy,
>  			    priv, &fops_skb_rx);
>  
> +	debugfs_create_file("arc_rx", S_IRUSR, priv->debug.debugfs_phy,
> +			    priv, &fops_arc_rx);
> +
>  	ath9k_cmn_debug_recv(priv->debug.debugfs_phy, &priv->debug.rx_stats);
> +
>  	ath9k_cmn_debug_phy_err(priv->debug.debugfs_phy, &priv->debug.rx_stats);
>  
>  	debugfs_create_file("slot", S_IRUSR, priv->debug.debugfs_phy,
> diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
> index a0f58e2..9d2dc33 100644
> --- a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
> +++ b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
> @@ -1061,7 +1061,27 @@ void ath9k_rx_tasklet(unsigned long data)
>  	unsigned long flags;
>  	struct ieee80211_hdr *hdr;
>  
> +	/* add for adaptive flow control*/
> +	int looptimes = 0;
> +	int highwatermark = ATH9K_HTC_RXBUF*3/4;
> +	int lowwatermark = ATH9K_HTC_RXBUF/32;
> +	unsigned int delay = 0;
> +
> +	struct htc_target *htc = priv->htc;
> +	struct hif_device_usb *hif_dev = htc->hif_dev;
> +
> +	ARCRX_STAT_SET(arcrx_highwater, highwatermark);
> +	ARCRX_STAT_SET(arcrx_lowwater, lowwatermark);
> +
>  	do {
> +		looptimes++;
> +		if (looptimes > highwatermark) {
> +			delay = looptimes*ARC_RX_STEP;
> +			atomic_set(&hif_dev->rx_urb_submit_delay, delay);
> +			ARCRX_STAT_INC(arcrx_watermark_triggered);
> +			ARCRX_STAT_SET(arcrx_urb_submit_delay, delay);
> +		}
> +
>  		spin_lock_irqsave(&priv->rx.rxbuflock, flags);
>  		list_for_each_entry(tmp_buf, &priv->rx.rxbuf, list) {
>  			if (tmp_buf->in_process) {
> @@ -1072,6 +1092,11 @@ void ath9k_rx_tasklet(unsigned long data)
>  
>  		if (rxbuf == NULL) {
>  			spin_unlock_irqrestore(&priv->rx.rxbuflock, flags);
> +			if (looptimes < lowwatermark) {
> +				atomic_set(&hif_dev->rx_urb_submit_delay, 0);
> +				ARCRX_STAT_SET(arcrx_urb_submit_delay, 0);
> +			}
> +
>  			break;
>  		}
>  
> 


-- 
Regards,
Oleksij


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

^ permalink raw reply

* Re: [PATCH v2 08/17] {macvtap,tun}_get_user(): switch to iov_iter
From: Michael S. Tsirkin @ 2015-02-03 10:10 UTC (permalink / raw)
  To: Al Viro; +Cc: David Miller, netdev, linux-kernel
In-Reply-To: <1416924151-28698-8-git-send-email-viro@ZenIV.linux.org.uk>

On Tue, Nov 25, 2014 at 02:02:22PM +0000, Al Viro wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> allows to switch macvtap and tun from ->aio_write() to ->write_iter()
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  drivers/net/macvtap.c |   44 +++++++++++++++++++++-----------------------
>  drivers/net/tun.c     |   44 ++++++++++++++++++++++++--------------------
>  2 files changed, 45 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index 8f80045..22b4cf2 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
> @@ -640,12 +640,12 @@ static void macvtap_skb_to_vnet_hdr(const struct sk_buff *skb,
>  
>  /* Get packet from user space buffer */
>  static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
> -				const struct iovec *iv, unsigned long total_len,
> -				size_t count, int noblock)
> +				struct iov_iter *from, int noblock)
>  {
>  	int good_linear = SKB_MAX_HEAD(NET_IP_ALIGN);
>  	struct sk_buff *skb;
>  	struct macvlan_dev *vlan;
> +	unsigned long total_len = iov_iter_count(from);
>  	unsigned long len = total_len;
>  	int err;
>  	struct virtio_net_hdr vnet_hdr = { 0 };
> @@ -653,6 +653,7 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
>  	int copylen = 0;
>  	bool zerocopy = false;
>  	size_t linear;
> +	ssize_t n;
>  
>  	if (q->flags & IFF_VNET_HDR) {
>  		vnet_hdr_len = q->vnet_hdr_sz;
> @@ -662,10 +663,11 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
>  			goto err;
>  		len -= vnet_hdr_len;
>  
> -		err = memcpy_fromiovecend((void *)&vnet_hdr, iv, 0,
> -					   sizeof(vnet_hdr));
> -		if (err < 0)
> +		err = -EFAULT;
> +		n = copy_from_iter(&vnet_hdr, sizeof(vnet_hdr), from);
> +		if (n != sizeof(vnet_hdr))
>  			goto err;
> +		iov_iter_advance(from, vnet_hdr_len - sizeof(vnet_hdr));
>  		if ((vnet_hdr.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) &&
>  		     vnet_hdr.csum_start + vnet_hdr.csum_offset + 2 >
>  							vnet_hdr.hdr_len)
> @@ -680,17 +682,16 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
>  	if (unlikely(len < ETH_HLEN))
>  		goto err;
>  
> -	err = -EMSGSIZE;
> -	if (unlikely(count > UIO_MAXIOV))
> -		goto err;
> -
>  	if (m && m->msg_control && sock_flag(&q->sk, SOCK_ZEROCOPY)) {
> +		struct iov_iter i;
> +
>  		copylen = vnet_hdr.hdr_len ? vnet_hdr.hdr_len : GOODCOPY_LEN;
>  		if (copylen > good_linear)
>  			copylen = good_linear;
>  		linear = copylen;
> -		if (iov_pages(iv, vnet_hdr_len + copylen, count)
> -		    <= MAX_SKB_FRAGS)
> +		i = *from;
> +		iov_iter_advance(&i, copylen);
> +		if (iov_iter_npages(&i, INT_MAX) <= MAX_SKB_FRAGS)
>  			zerocopy = true;
>  	}
>  
> @@ -708,10 +709,9 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
>  		goto err;
>  
>  	if (zerocopy)
> -		err = zerocopy_sg_from_iovec(skb, iv, vnet_hdr_len, count);
> +		err = zerocopy_sg_from_iter(skb, from);
>  	else {
> -		err = skb_copy_datagram_from_iovec(skb, 0, iv, vnet_hdr_len,
> -						   len);
> +		err = skb_copy_datagram_from_iter(skb, 0, from, len);
>  		if (!err && m && m->msg_control) {
>  			struct ubuf_info *uarg = m->msg_control;
>  			uarg->callback(uarg, false);
> @@ -764,16 +764,12 @@ err:
>  	return err;
>  }
>  
> -static ssize_t macvtap_aio_write(struct kiocb *iocb, const struct iovec *iv,
> -				 unsigned long count, loff_t pos)
> +static ssize_t macvtap_write_iter(struct kiocb *iocb, struct iov_iter *from)
>  {
>  	struct file *file = iocb->ki_filp;
> -	ssize_t result = -ENOLINK;
>  	struct macvtap_queue *q = file->private_data;
>  
> -	result = macvtap_get_user(q, NULL, iv, iov_length(iv, count), count,
> -				  file->f_flags & O_NONBLOCK);
> -	return result;
> +	return macvtap_get_user(q, NULL, from, file->f_flags & O_NONBLOCK);
>  }
>  
>  /* Put packet to the user space buffer */
> @@ -1081,8 +1077,9 @@ static const struct file_operations macvtap_fops = {
>  	.open		= macvtap_open,
>  	.release	= macvtap_release,
>  	.read		= new_sync_read,
> +	.write		= new_sync_write,
>  	.read_iter	= macvtap_read_iter,
> -	.aio_write	= macvtap_aio_write,
> +	.write_iter	= macvtap_write_iter,
>  	.poll		= macvtap_poll,
>  	.llseek		= no_llseek,
>  	.unlocked_ioctl	= macvtap_ioctl,
> @@ -1095,8 +1092,9 @@ static int macvtap_sendmsg(struct kiocb *iocb, struct socket *sock,
>  			   struct msghdr *m, size_t total_len)
>  {
>  	struct macvtap_queue *q = container_of(sock, struct macvtap_queue, sock);
> -	return macvtap_get_user(q, m, m->msg_iov, total_len, m->msg_iovlen,
> -			    m->msg_flags & MSG_DONTWAIT);
> +	struct iov_iter from;
> +	iov_iter_init(&from, WRITE, m->msg_iov, m->msg_iovlen, total_len);
> +	return macvtap_get_user(q, m, &from, m->msg_flags & MSG_DONTWAIT);
>  }
>  
>  static int macvtap_recvmsg(struct kiocb *iocb, struct socket *sock,
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 405dfdf..4b743c6 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1012,28 +1012,29 @@ static struct sk_buff *tun_alloc_skb(struct tun_file *tfile,
>  
>  /* Get packet from user space buffer */
>  static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> -			    void *msg_control, const struct iovec *iv,
> -			    size_t total_len, size_t count, int noblock)
> +			    void *msg_control, struct iov_iter *from,
> +			    int noblock)
>  {
>  	struct tun_pi pi = { 0, cpu_to_be16(ETH_P_IP) };
>  	struct sk_buff *skb;
> +	size_t total_len = iov_iter_count(from);
>  	size_t len = total_len, align = NET_SKB_PAD, linear;
>  	struct virtio_net_hdr gso = { 0 };
>  	int good_linear;
> -	int offset = 0;
>  	int copylen;
>  	bool zerocopy = false;
>  	int err;
>  	u32 rxhash;
> +	ssize_t n;
>  
>  	if (!(tun->flags & TUN_NO_PI)) {
>  		if (len < sizeof(pi))
>  			return -EINVAL;
>  		len -= sizeof(pi);
>  
> -		if (memcpy_fromiovecend((void *)&pi, iv, 0, sizeof(pi)))
> +		n = copy_from_iter(&pi, sizeof(pi), from);
> +		if (n != sizeof(pi))
>  			return -EFAULT;
> -		offset += sizeof(pi);
>  	}
>  
>  	if (tun->flags & TUN_VNET_HDR) {
> @@ -1041,7 +1042,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>  			return -EINVAL;
>  		len -= tun->vnet_hdr_sz;
>  
> -		if (memcpy_fromiovecend((void *)&gso, iv, offset, sizeof(gso)))
> +		n = copy_from_iter(&gso, sizeof(gso), from);
> +		if (n != sizeof(gso))
>  			return -EFAULT;
>  
>  		if ((gso.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) &&
> @@ -1050,7 +1052,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>  
>  		if (gso.hdr_len > len)
>  			return -EINVAL;
> -		offset += tun->vnet_hdr_sz;
> +		iov_iter_advance(from, tun->vnet_hdr_sz);
>  	}
>  
>  	if ((tun->flags & TUN_TYPE_MASK) == TUN_TAP_DEV) {


Hmm does copy_from_iter actually modify the iovec?
If so, won't this break aio on tun/macvtap, by
reversing the effect of
commit 6f26c9a7555e5bcca3560919db9b852015077dae
    tun: fix tun_chr_aio_write so that aio works
?


Maybe we should change iovec_iter to avoid modifying the
underlying iovec?



> @@ -1063,6 +1065,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>  	good_linear = SKB_MAX_HEAD(align);
>  
>  	if (msg_control) {
> +		struct iov_iter i = *from;
> +
>  		/* There are 256 bytes to be copied in skb, so there is
>  		 * enough room for skb expand head in case it is used.
>  		 * The rest of the buffer is mapped from userspace.
> @@ -1071,7 +1075,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>  		if (copylen > good_linear)
>  			copylen = good_linear;
>  		linear = copylen;
> -		if (iov_pages(iv, offset + copylen, count) <= MAX_SKB_FRAGS)
> +		iov_iter_advance(&i, copylen);
> +		if (iov_iter_npages(&i, INT_MAX) <= MAX_SKB_FRAGS)
>  			zerocopy = true;
>  	}
>  
> @@ -1091,9 +1096,9 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>  	}
>  
>  	if (zerocopy)
> -		err = zerocopy_sg_from_iovec(skb, iv, offset, count);
> +		err = zerocopy_sg_from_iter(skb, from);
>  	else {
> -		err = skb_copy_datagram_from_iovec(skb, 0, iv, offset, len);
> +		err = skb_copy_datagram_from_iter(skb, 0, from, len);
>  		if (!err && msg_control) {
>  			struct ubuf_info *uarg = msg_control;
>  			uarg->callback(uarg, false);
> @@ -1207,8 +1212,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>  	return total_len;
>  }
>  
> -static ssize_t tun_chr_aio_write(struct kiocb *iocb, const struct iovec *iv,
> -			      unsigned long count, loff_t pos)
> +static ssize_t tun_chr_write_iter(struct kiocb *iocb, struct iov_iter *from)
>  {
>  	struct file *file = iocb->ki_filp;
>  	struct tun_struct *tun = tun_get(file);
> @@ -1218,10 +1222,7 @@ static ssize_t tun_chr_aio_write(struct kiocb *iocb, const struct iovec *iv,
>  	if (!tun)
>  		return -EBADFD;
>  
> -	tun_debug(KERN_INFO, tun, "tun_chr_write %ld\n", count);
> -
> -	result = tun_get_user(tun, tfile, NULL, iv, iov_length(iv, count),
> -			      count, file->f_flags & O_NONBLOCK);
> +	result = tun_get_user(tun, tfile, NULL, from, file->f_flags & O_NONBLOCK);
>  
>  	tun_put(tun);
>  	return result;
> @@ -1445,11 +1446,14 @@ static int tun_sendmsg(struct kiocb *iocb, struct socket *sock,
>  	int ret;
>  	struct tun_file *tfile = container_of(sock, struct tun_file, socket);
>  	struct tun_struct *tun = __tun_get(tfile);
> +	struct iov_iter from;
>  
>  	if (!tun)
>  		return -EBADFD;
> -	ret = tun_get_user(tun, tfile, m->msg_control, m->msg_iov, total_len,
> -			   m->msg_iovlen, m->msg_flags & MSG_DONTWAIT);
> +
> +	iov_iter_init(&from, WRITE, m->msg_iov, m->msg_iovlen, total_len);
> +	ret = tun_get_user(tun, tfile, m->msg_control, &from,
> +			   m->msg_flags & MSG_DONTWAIT);
>  	tun_put(tun);
>  	return ret;
>  }
> @@ -2233,9 +2237,9 @@ static const struct file_operations tun_fops = {
>  	.owner	= THIS_MODULE,
>  	.llseek = no_llseek,
>  	.read  = new_sync_read,
> +	.write = new_sync_write,
>  	.read_iter  = tun_chr_read_iter,
> -	.write = do_sync_write,
> -	.aio_write = tun_chr_aio_write,
> +	.write_iter = tun_chr_write_iter,
>  	.poll	= tun_chr_poll,
>  	.unlocked_ioctl	= tun_chr_ioctl,
>  #ifdef CONFIG_COMPAT
> -- 
> 1.7.10.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 17/18] vhost: don't bother copying iovecs in handle_rx(), kill memcpy_toiovecend()
From: Michael S. Tsirkin @ 2015-02-03 10:04 UTC (permalink / raw)
  To: Al Viro; +Cc: David Miller, netdev, kvm
In-Reply-To: <1422863977-17668-17-git-send-email-viro@ZenIV.linux.org.uk>

On Mon, Feb 02, 2015 at 07:59:36AM +0000, Al Viro wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: kvm@vger.kernel.org
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  drivers/vhost/net.c | 79 ++++++++++++++---------------------------------------
>  include/linux/uio.h |  3 --
>  lib/iovec.c         | 26 ------------------
>  3 files changed, 20 insertions(+), 88 deletions(-)
> 
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index d86cc9b..73c0ebf 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -84,10 +84,6 @@ struct vhost_net_ubuf_ref {
>  
>  struct vhost_net_virtqueue {
>  	struct vhost_virtqueue vq;
> -	/* hdr is used to store the virtio header.
> -	 * Since each iovec has >= 1 byte length, we never need more than
> -	 * header length entries to store the header. */
> -	struct iovec hdr[sizeof(struct virtio_net_hdr_mrg_rxbuf)];
>  	size_t vhost_hlen;
>  	size_t sock_hlen;
>  	/* vhost zerocopy support fields below: */
> @@ -235,44 +231,6 @@ static bool vhost_sock_zcopy(struct socket *sock)
>  		sock_flag(sock->sk, SOCK_ZEROCOPY);
>  }
>  
> -/* Pop first len bytes from iovec. Return number of segments used. */
> -static int move_iovec_hdr(struct iovec *from, struct iovec *to,
> -			  size_t len, int iov_count)
> -{
> -	int seg = 0;
> -	size_t size;
> -
> -	while (len && seg < iov_count) {
> -		size = min(from->iov_len, len);
> -		to->iov_base = from->iov_base;
> -		to->iov_len = size;
> -		from->iov_len -= size;
> -		from->iov_base += size;
> -		len -= size;
> -		++from;
> -		++to;
> -		++seg;
> -	}
> -	return seg;
> -}
> -/* Copy iovec entries for len bytes from iovec. */
> -static void copy_iovec_hdr(const struct iovec *from, struct iovec *to,
> -			   size_t len, int iovcount)
> -{
> -	int seg = 0;
> -	size_t size;
> -
> -	while (len && seg < iovcount) {
> -		size = min(from->iov_len, len);
> -		to->iov_base = from->iov_base;
> -		to->iov_len = size;
> -		len -= size;
> -		++from;
> -		++to;
> -		++seg;
> -	}
> -}
> -
>  /* In case of DMA done not in order in lower device driver for some reason.
>   * upend_idx is used to track end of used idx, done_idx is used to track head
>   * of used idx. Once lower device DMA done contiguously, we will signal KVM
> @@ -570,9 +528,9 @@ static void handle_rx(struct vhost_net *net)
>  		.msg_controllen = 0,
>  		.msg_flags = MSG_DONTWAIT,
>  	};
> -	struct virtio_net_hdr_mrg_rxbuf hdr = {
> -		.hdr.flags = 0,
> -		.hdr.gso_type = VIRTIO_NET_HDR_GSO_NONE
> +	struct virtio_net_hdr hdr = {
> +		.flags = 0,
> +		.gso_type = VIRTIO_NET_HDR_GSO_NONE
>  	};
>  	size_t total_len = 0;
>  	int err, mergeable;
> @@ -580,6 +538,7 @@ static void handle_rx(struct vhost_net *net)
>  	size_t vhost_hlen, sock_hlen;
>  	size_t vhost_len, sock_len;
>  	struct socket *sock;
> +	struct iov_iter fixup;
>  
>  	mutex_lock(&vq->mutex);
>  	sock = vq->private_data;
> @@ -624,14 +583,17 @@ static void handle_rx(struct vhost_net *net)
>  			break;
>  		}
>  		/* We don't need to be notified again. */
> -		if (unlikely((vhost_hlen)))
> -			/* Skip header. TODO: support TSO. */
> -			move_iovec_hdr(vq->iov, nvq->hdr, vhost_hlen, in);
> -		else
> -			/* Copy the header for use in VIRTIO_NET_F_MRG_RXBUF:
> -			 * needed because recvmsg can modify msg_iov. */
> -			copy_iovec_hdr(vq->iov, nvq->hdr, sock_hlen, in);
> -		iov_iter_init(&msg.msg_iter, READ, vq->iov, in, sock_len);
> +		iov_iter_init(&msg.msg_iter, READ, vq->iov, in, vhost_len);
> +		fixup = msg.msg_iter;
> +		if (unlikely((vhost_hlen))) {
> +			/* We will supply the header ourselves
> +			 * TODO: support TSO. */
> +			iov_iter_advance(&msg.msg_iter, vhost_hlen);
> +		} else {
> +			/* It'll come from socket; we'll need to patch
> +			 * ->num_buffers over if VIRTIO_NET_F_MRG_RXBUF */
> +			iov_iter_advance(&fixup, sizeof(hdr));
> +		}
>  		err = sock->ops->recvmsg(NULL, sock, &msg,
>  					 sock_len, MSG_DONTWAIT | MSG_TRUNC);
>  		/* Userspace might have consumed the packet meanwhile:


Hmm having second thoughts here.
Will this modify the iov in vq->iov?
If yes, how will recvmsg fill it?




> @@ -643,18 +605,17 @@ static void handle_rx(struct vhost_net *net)
>  			vhost_discard_vq_desc(vq, headcount);
>  			continue;
>  		}
> +		/* Supply virtio_net_hdr if VHOST_NET_F_VIRTIO_NET_HDR */
>  		if (unlikely(vhost_hlen) &&
> -		    memcpy_toiovecend(nvq->hdr, (unsigned char *)&hdr, 0,
> -				      vhost_hlen)) {
> +		    copy_to_iter(&hdr, sizeof(hdr), &fixup) != sizeof(hdr)) {
>  			vq_err(vq, "Unable to write vnet_hdr at addr %p\n",
>  			       vq->iov->iov_base);
>  			break;
>  		}
> -		/* TODO: Should check and handle checksum. */
> +		/* Supply (or replace) ->num_buffers if VIRTIO_NET_F_MRG_RXBUF
> +		 * TODO: Should check and handle checksum. */
>  		if (likely(mergeable) &&
> -		    memcpy_toiovecend(nvq->hdr, (unsigned char *)&headcount,
> -				      offsetof(typeof(hdr), num_buffers),
> -				      sizeof hdr.num_buffers)) {
> +		    copy_to_iter(&headcount, 2, &fixup) != 2) {
>  			vq_err(vq, "Failed num_buffers write");
>  			vhost_discard_vq_desc(vq, headcount);
>  			break;

Here, if recvmsg modified the iov, how does copy_to_iter
see the header?


> diff --git a/include/linux/uio.h b/include/linux/uio.h
> index af3439f..02bd8a9 100644
> --- a/include/linux/uio.h
> +++ b/include/linux/uio.h
> @@ -137,7 +137,4 @@ size_t csum_and_copy_from_iter(void *addr, size_t bytes, __wsum *csum, struct io
>  
>  int memcpy_fromiovecend(unsigned char *kdata, const struct iovec *iov,
>  			int offset, int len);
> -int memcpy_toiovecend(const struct iovec *v, unsigned char *kdata,
> -		      int offset, int len);
> -
>  #endif
> diff --git a/lib/iovec.c b/lib/iovec.c
> index 4a90875..d8f17a9 100644
> --- a/lib/iovec.c
> +++ b/lib/iovec.c
> @@ -3,32 +3,6 @@
>  #include <linux/uio.h>
>  
>  /*
> - *	Copy kernel to iovec. Returns -EFAULT on error.
> - */
> -
> -int memcpy_toiovecend(const struct iovec *iov, unsigned char *kdata,
> -		      int offset, int len)
> -{
> -	int copy;
> -	for (; len > 0; ++iov) {
> -		/* Skip over the finished iovecs */
> -		if (unlikely(offset >= iov->iov_len)) {
> -			offset -= iov->iov_len;
> -			continue;
> -		}
> -		copy = min_t(unsigned int, iov->iov_len - offset, len);
> -		if (copy_to_user(iov->iov_base + offset, kdata, copy))
> -			return -EFAULT;
> -		offset = 0;
> -		kdata += copy;
> -		len -= copy;
> -	}
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL(memcpy_toiovecend);
> -
> -/*
>   *	Copy iovec to kernel. Returns -EFAULT on error.
>   */
>  
> -- 
> 2.1.4

^ permalink raw reply

* [PATCH] ath9k_htc: add adaptive usb flow control to repair soft lockup with monitor mode
From: yuweizheng @ 2015-02-03  9:59 UTC (permalink / raw)
  To: linux-kernel, ath9k-devel, linux-wireless, kvalo, ath9k-devel
  Cc: netdev, Yuwei Zheng, Yuwei Zheng

From: Yuwei Zheng <yuweizheng@139.com>

In the environment with heavy wifi traffic, set the ar9271 into monitor mode, will
trigger a deadloop panic.
 
The ath9k_hif_usb_rx_cb function excute on  the interrupt context, and ath9k_rx_tasklet excute
on the soft irq context. In other words, the ath9k_hif_usb_rx_cb have more chance to excute than
ath9k_rx_tasklet.  So in the worst condition,  the rx.rxbuf receive list is always full,
and the do {}while(true) loop will not be break. The kernel get a soft lockup panic. 
 
[59011.007210] BUG: soft lockup - CPU#0 stuck for 23s!
[kworker/0:0:30609]
[59011.030560] BUG: scheduling while atomic: kworker/0:0/30609/0x40010100
[59013.804486] BUG: scheduling while atomic: kworker/0:0/30609/0x40010100
[59013.858522] Kernel panic - not syncing: softlockup: hung tasks
 
[59014.038891] Exception stack(0xdf4bbc38 to 0xdf4bbc80)
[59014.046834] bc20:                                                       de57b950 60000113
[59014.059579] bc40: 00000000 bb32bb32 60000113 de57b948 de57b500 dc7bb440 df4bbcd0 00000000
[59014.072337] bc60: de57b950 60000113 df4bbcd0 df4bbc80 c04c259d c04c25a0 60000133 ffffffff
[59014.085233] [<c04c28db>] (__irq_svc+0x3b/0x5c) from [<c04c25a0>] (_raw_spin_unlock_irqrestore+0xc/0x10)
[59014.100437] [<c04c25a0>] (_raw_spin_unlock_irqrestore+0xc/0x10) from [<bf9c2089>] (ath9k_rx_tasklet+0x290/0x490 [ath9k_htc])
[59014.118267] [<bf9c2089>] (ath9k_rx_tasklet+0x290/0x490 [ath9k_htc]) from [<c0036d23>] (tasklet_action+0x3b/0x98)
[59014.134132] [<c0036d23>] (tasklet_action+0x3b/0x98) from [<c0036709>] (__do_softirq+0x99/0x16c)
[59014.147784] [<c0036709>] (__do_softirq+0x99/0x16c) from [<c00369f7>] (irq_exit+0x5b/0x5c)
[59014.160653] [<c00369f7>] (irq_exit+0x5b/0x5c) from [<c000cfc3>] (handle_IRQ+0x37/0x78)
[59014.173124] [<c000cfc3>] (handle_IRQ+0x37/0x78) from [<c00085df>] (omap3_intc_handle_irq+0x5f/0x68)
[59014.187225] [<c00085df>] (omap3_intc_handle_irq+0x5f/0x68) from [<c04c28db>](__irq_svc+0x3b/0x5c)
 
This bug can be see with low performance board, such as uniprocessor beagle bone board.
 
 
Signed-off-by: Yuwei Zheng <zhengyuwei@360.cn>
Signed-off-by: Yuwei Zheng <yuweizheng@139.com>

---
 drivers/net/wireless/ath/ath9k/hif_usb.c       | 61 +++++++++++++++++++++++---
 drivers/net/wireless/ath/ath9k/hif_usb.h       |  6 +++
 drivers/net/wireless/ath/ath9k/htc.h           | 18 ++++++++
 drivers/net/wireless/ath/ath9k/htc_drv_debug.c | 46 +++++++++++++++++++
 drivers/net/wireless/ath/ath9k/htc_drv_txrx.c  | 25 +++++++++++
 5 files changed, 149 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c
index 8e7153b..718bbd6 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.c
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
@@ -640,6 +640,7 @@ static void ath9k_hif_usb_rx_cb(struct urb *urb)
 	struct hif_device_usb *hif_dev =
 		usb_get_intfdata(usb_ifnum_to_if(urb->dev, 0));
 	int ret;
+	int delay;
 
 	if (!skb)
 		return;
@@ -658,7 +659,6 @@ static void ath9k_hif_usb_rx_cb(struct urb *urb)
 	default:
 		goto resubmit;
 	}
-
 	if (likely(urb->actual_length != 0)) {
 		skb_put(skb, urb->actual_length);
 		ath9k_hif_usb_rx_stream(hif_dev, skb);
@@ -667,12 +667,18 @@ static void ath9k_hif_usb_rx_cb(struct urb *urb)
 resubmit:
 	skb_reset_tail_pointer(skb);
 	skb_trim(skb, 0);
-
-	usb_anchor_urb(urb, &hif_dev->rx_submitted);
-	ret = usb_submit_urb(urb, GFP_ATOMIC);
-	if (ret) {
-		usb_unanchor_urb(urb);
-		goto free;
+	if (atomic_read(&hif_dev->rx_urb_submit_delay) > 0) {
+		usb_anchor_urb(urb, &hif_dev->rx_delayed_submitted);
+		delay = atomic_read(&hif_dev->rx_urb_submit_delay);
+		schedule_delayed_work(&hif_dev->rx_submit_delayed_work,
+				      usecs_to_jiffies(delay));
+	} else {
+		usb_anchor_urb(urb, &hif_dev->rx_submitted);
+		ret = usb_submit_urb(urb, GFP_ATOMIC);
+		if (ret) {
+			usb_unanchor_urb(urb);
+			goto free;
+		}
 	}
 
 	return;
@@ -818,9 +824,43 @@ err:
 	return -ENOMEM;
 }
 
+static void rx_urb_delayed_submit_handler(struct work_struct *work)
+{
+	struct hif_device_usb *hif_dev =
+		container_of(work,
+			     struct hif_device_usb,
+			     rx_submit_delayed_work.work);
+
+	struct urb *urb = NULL;
+	struct sk_buff *skb = NULL;
+	int ret;
+	int loop_times = 0;
+
+	while (true) {
+		loop_times++;
+		if (loop_times > (MAX_RX_URB_NUM))
+			atomic_add(ARC_RX_STEP, &hif_dev->rx_urb_submit_delay);
+
+		urb = usb_get_from_anchor(&hif_dev->rx_delayed_submitted);
+		if (urb) {
+			skb = (struct sk_buff *)urb->context;
+			ret = usb_submit_urb(urb, GFP_KERNEL);
+			if (ret != 0) {
+				usb_unanchor_urb(urb);
+				dev_kfree_skb_any(skb);
+				urb->context = NULL;
+			}
+		} else {
+			break;
+		}
+	}
+}
+
 static void ath9k_hif_usb_dealloc_rx_urbs(struct hif_device_usb *hif_dev)
 {
 	usb_kill_anchored_urbs(&hif_dev->rx_submitted);
+	usb_kill_anchored_urbs(&hif_dev->rx_delayed_submitted);
+	flush_delayed_work(&hif_dev->rx_submit_delayed_work);
 }
 
 static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
@@ -830,6 +870,8 @@ static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
 	int i, ret;
 
 	init_usb_anchor(&hif_dev->rx_submitted);
+	init_usb_anchor(&hif_dev->rx_delayed_submitted);
+
 	spin_lock_init(&hif_dev->rx_lock);
 
 	for (i = 0; i < MAX_RX_URB_NUM; i++) {
@@ -871,6 +913,11 @@ static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
 		usb_free_urb(urb);
 	}
 
+	/* add for adaptive rate control*/
+	atomic_set(&hif_dev->rx_urb_submit_delay, 0);
+	INIT_DELAYED_WORK(&hif_dev->rx_submit_delayed_work,
+			  rx_urb_delayed_submit_handler);
+
 	return 0;
 
 err_submit:
diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.h b/drivers/net/wireless/ath/ath9k/hif_usb.h
index 51496e7..2127e50 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.h
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.h
@@ -41,6 +41,7 @@
 #define MAX_RX_URB_NUM  8
 #define MAX_RX_BUF_SIZE 16384
 #define MAX_PKT_NUM_IN_TRANSFER 10
+#define ARC_RX_STEP 100 /* 100us */
 
 #define MAX_REG_OUT_URB_NUM  1
 #define MAX_REG_IN_URB_NUM   64
@@ -98,9 +99,14 @@ struct hif_device_usb {
 	struct hif_usb_tx tx;
 	struct usb_anchor regout_submitted;
 	struct usb_anchor rx_submitted;
+	struct usb_anchor rx_delayed_submitted; /* delayed submit anchor */
 	struct usb_anchor reg_in_submitted;
 	struct usb_anchor mgmt_submitted;
 	struct sk_buff *remain_skb;
+
+	struct delayed_work rx_submit_delayed_work;
+	atomic_t  rx_urb_submit_delay; /*us*/
+
 	const char *fw_name;
 	int rx_remain_len;
 	int rx_pkt_len;
diff --git a/drivers/net/wireless/ath/ath9k/htc.h b/drivers/net/wireless/ath/ath9k/htc.h
index 9dde265..3061dfa 100644
--- a/drivers/net/wireless/ath/ath9k/htc.h
+++ b/drivers/net/wireless/ath/ath9k/htc.h
@@ -331,6 +331,13 @@ static inline struct ath9k_htc_tx_ctl *HTC_SKB_CB(struct sk_buff *skb)
 
 #define TX_QSTAT_INC(q) (priv->debug.tx_stats.queue_stats[q]++)
 
+#define ARCRX_STAT_INC(c) \
+	(hif_dev->htc_handle->drv_priv->debug.arcrx_stats.c++)
+#define ARCRX_STAT_ADD(c, a) \
+	(hif_dev->htc_handle->drv_priv->debug.arcrx_stats.c += a)
+#define ARCRX_STAT_SET(c, a) \
+	(hif_dev->htc_handle->drv_priv->debug.arcrx_stats.c = a)
+
 void ath9k_htc_err_stat_rx(struct ath9k_htc_priv *priv,
 			   struct ath_rx_status *rs);
 
@@ -352,11 +359,19 @@ struct ath_skbrx_stats {
 	u32 skb_dropped;
 };
 
+struct ath_arcrx_stats {
+	u32 arcrx_highwater;
+	u32 arcrx_lowwater;
+	u32 arcrx_watermark_triggered;
+	u32 arcrx_urb_submit_delay;
+};
+
 struct ath9k_debug {
 	struct dentry *debugfs_phy;
 	struct ath_tx_stats tx_stats;
 	struct ath_rx_stats rx_stats;
 	struct ath_skbrx_stats skbrx_stats;
+	struct ath_arcrx_stats arcrx_stats;
 };
 
 void ath9k_htc_get_et_strings(struct ieee80211_hw *hw,
@@ -377,6 +392,9 @@ void ath9k_htc_get_et_stats(struct ieee80211_hw *hw,
 
 #define TX_QSTAT_INC(c) do { } while (0)
 
+#define ARCRX_STAT_INC(c) do {} while (0)
+#define ARCRX_STAT_ADD(c, a) do {} while (0)
+#define ARCRX_STAT_SET(c, a) do {} while (0)
 static inline void ath9k_htc_err_stat_rx(struct ath9k_htc_priv *priv,
 					 struct ath_rx_status *rs)
 {
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_debug.c b/drivers/net/wireless/ath/ath9k/htc_drv_debug.c
index 8cef1ed..45cc79d 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_debug.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_debug.c
@@ -286,6 +286,48 @@ static const struct file_operations fops_skb_rx = {
 	.llseek = default_llseek,
 };
 
+static ssize_t read_file_arc_rx(struct file *file, char __user *user_buf,
+				size_t count, loff_t *ppos)
+{
+	struct ath9k_htc_priv *priv = file->private_data;
+	char *buf;
+	unsigned int len = 0, size = 1500;
+	ssize_t retval = 0;
+
+	buf = kzalloc(size, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	len += scnprintf(buf + len, size - len,
+			"%20s : %10u\n", "High watermark",
+			priv->debug.arcrx_stats.arcrx_highwater);
+	len += scnprintf(buf + len, size - len,
+			"%20s : %10u\n", "Low watermark",
+			priv->debug.arcrx_stats.arcrx_lowwater);
+
+	len += scnprintf(buf + len, size - len,
+			"%20s : %10u\n", "WM triggered",
+			priv->debug.arcrx_stats.arcrx_watermark_triggered);
+
+	len += scnprintf(buf + len, size - len,
+			"%20s : %10u\n", "URB delay",
+			priv->debug.arcrx_stats.arcrx_urb_submit_delay);
+	if (len > size)
+		len = size;
+
+	retval = simple_read_from_buffer(user_buf, count, ppos, buf, len);
+	kfree(buf);
+
+	return retval;
+}
+
+static const struct file_operations fops_arc_rx = {
+	.read = read_file_arc_rx,
+	.open = simple_open,
+	.owner = THIS_MODULE,
+	.llseek = default_llseek,
+};
+
 static ssize_t read_file_slot(struct file *file, char __user *user_buf,
 			      size_t count, loff_t *ppos)
 {
@@ -518,7 +560,11 @@ int ath9k_htc_init_debug(struct ath_hw *ah)
 	debugfs_create_file("skb_rx", S_IRUSR, priv->debug.debugfs_phy,
 			    priv, &fops_skb_rx);
 
+	debugfs_create_file("arc_rx", S_IRUSR, priv->debug.debugfs_phy,
+			    priv, &fops_arc_rx);
+
 	ath9k_cmn_debug_recv(priv->debug.debugfs_phy, &priv->debug.rx_stats);
+
 	ath9k_cmn_debug_phy_err(priv->debug.debugfs_phy, &priv->debug.rx_stats);
 
 	debugfs_create_file("slot", S_IRUSR, priv->debug.debugfs_phy,
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
index a0f58e2..9d2dc33 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
@@ -1061,7 +1061,27 @@ void ath9k_rx_tasklet(unsigned long data)
 	unsigned long flags;
 	struct ieee80211_hdr *hdr;
 
+	/* add for adaptive flow control*/
+	int looptimes = 0;
+	int highwatermark = ATH9K_HTC_RXBUF*3/4;
+	int lowwatermark = ATH9K_HTC_RXBUF/32;
+	unsigned int delay = 0;
+
+	struct htc_target *htc = priv->htc;
+	struct hif_device_usb *hif_dev = htc->hif_dev;
+
+	ARCRX_STAT_SET(arcrx_highwater, highwatermark);
+	ARCRX_STAT_SET(arcrx_lowwater, lowwatermark);
+
 	do {
+		looptimes++;
+		if (looptimes > highwatermark) {
+			delay = looptimes*ARC_RX_STEP;
+			atomic_set(&hif_dev->rx_urb_submit_delay, delay);
+			ARCRX_STAT_INC(arcrx_watermark_triggered);
+			ARCRX_STAT_SET(arcrx_urb_submit_delay, delay);
+		}
+
 		spin_lock_irqsave(&priv->rx.rxbuflock, flags);
 		list_for_each_entry(tmp_buf, &priv->rx.rxbuf, list) {
 			if (tmp_buf->in_process) {
@@ -1072,6 +1092,11 @@ void ath9k_rx_tasklet(unsigned long data)
 
 		if (rxbuf == NULL) {
 			spin_unlock_irqrestore(&priv->rx.rxbuflock, flags);
+			if (looptimes < lowwatermark) {
+				atomic_set(&hif_dev->rx_urb_submit_delay, 0);
+				ARCRX_STAT_SET(arcrx_urb_submit_delay, 0);
+			}
+
 			break;
 		}
 
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH net-next 3/4] ethtool: add RX_ALLOC_ORDER to tunable
From: Govindarajulu Varadarajan @ 2015-02-03  9:49 UTC (permalink / raw)
  To: David Miller; +Cc: _govind, netdev, ssujith, benve, edumazet, ben
In-Reply-To: <20150202.192159.66698996958688718.davem@davemloft.net>

On Mon, 2 Feb 2015, David Miller wrote:

> From: Govindarajulu Varadarajan <_govind@gmx.com>
> Date: Sat, 31 Jan 2015 17:58:09 +0530
>
>> Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
>
> This is terrible.
>
> You haven't explained what this means.
>
> And to tell you the truth, from what I can tell this tunable is
> very specific to how you have implemented RX frags in the enic
> driver in this series and won't necessarily translate to how
> other drivers manage RX buffers.
>

Yes I agree that this is too specific to driver. From my knowledge mlx4 also
uses similar frag allocation from large order page. Other that these two
drivers, this facility may not make sense to other drivers.

On systems with huge memory we can go higher than order-3, and if user sees that
order-3 are failing, he should be able to reduce the order.

Since this is very driver specific, are you fine if I move this to device sysfs?


> You need to actually design this facility properly, understand
> what the needs are of other drivers and how this facility
> can be relevant for more drivers than your own.
>

^ permalink raw reply

* Re: Question: should local address be expired when updating PMTU?
From: Steffen Klassert @ 2015-02-03  9:28 UTC (permalink / raw)
  To: shengyong; +Cc: davem, netdev, yangyingling, hannes
In-Reply-To: <54CF3348.40207@huawei.com>

On Mon, Feb 02, 2015 at 04:20:24PM +0800, shengyong wrote:
> Hi, David Miller
> Since commit 81aded246 (ipv6: Handle PMTU in ICMP error handlers), the entries
> in neigh table may get expired. But in the situation:
> 
>           Host only
>     PC <------------> Virtual Machine
> 
> a packet is sent from PC to VM, and the packet looks like:
> -----------------------------------
> | IPv6 (src=PC-addr, dst=VM-addr) |
> |---------------------------------|
> |     ICMPv6 (Packet Too Big)     |
> |---------------------------------|
> | IPv6 (src=VM-addr, dst=VM-addr) |
> |---------------------------------|
> | ICMPv6 (Neighbor Advertisement) |
> -----------------------------------
> 
> Then the local addr on VM will be updated with an expire value. After the
> lifetime of the local addr is expired, the VM is unreachable from PC.
> 
> 	# ip -6 route list table local
> 	local fe80::1 dev lo  metric 0 *expire 596*

We first need to find out why you receive this Packet Too Big message,
can you capture this packet somehow? Then we have to see why this loopback
route gets a pmtu update from that packet. Is the destination address
of the Packet Too Big message really fe80::1?

^ permalink raw reply

* [PATCH] ath9k_htc: add adaptive rate control to repair soft lockup with monitor mode
From: yuweizheng @ 2015-02-03  9:21 UTC (permalink / raw)
  To: linux-kernel, ath9k-devel, linux-wireless, kvalo, ath9k-devel
  Cc: netdev, Yuwei Zheng

From: Yuwei Zheng <yuweizheng@139.com>

In the environment with heavy wifi traffic, set the ar9271 into monitor mode, will
trigger a deadloop panic.
The ath9k_hif_usb_rx_cb function excute on  the interrupt context, and ath9k_rx_tasklet excute
on the soft irq context. In other words, the ath9k_hif_usb_rx_cb have more chance to excute than
ath9k_rx_tasklet.  So in the worst condition,  the rx.rxbuf receive list is always full,
and the do {}while(true) loop will not be break. The kernel get a soft lockup panic.
[59011.007210] BUG: soft lockup - CPU#0 stuck for 23s!
[kworker/0:0:30609]
[59011.030560] BUG: scheduling while atomic: kworker/0:0/30609/0x40010100
[59013.804486] BUG: scheduling while atomic: kworker/0:0/30609/0x40010100
[59013.858522] Kernel panic - not syncing: softlockup: hung tasks
[59014.038891] Exception stack(0xdf4bbc38 to 0xdf4bbc80)
[59014.046834] bc20:                                                       de57b950 60000113
[59014.059579] bc40: 00000000 bb32bb32 60000113 de57b948 de57b500 dc7bb440 df4bbcd0 00000000
[59014.072337] bc60: de57b950 60000113 df4bbcd0 df4bbc80 c04c259d c04c25a0 60000133 ffffffff
[59014.085233] [<c04c28db>] (__irq_svc+0x3b/0x5c) from [<c04c25a0>] (_raw_spin_unlock_irqrestore+0xc/0x10)
[59014.100437] [<c04c25a0>] (_raw_spin_unlock_irqrestore+0xc/0x10) from [<bf9c2089>] (ath9k_rx_tasklet+0x290/0x490 [ath9k_htc])
[59014.118267] [<bf9c2089>] (ath9k_rx_tasklet+0x290/0x490 [ath9k_htc]) from [<c0036d23>] (tasklet_action+0x3b/0x98)
[59014.134132] [<c0036d23>] (tasklet_action+0x3b/0x98) from [<c0036709>] (__do_softirq+0x99/0x16c)
[59014.147784] [<c0036709>] (__do_softirq+0x99/0x16c) from [<c00369f7>] (irq_exit+0x5b/0x5c)
[59014.160653] [<c00369f7>] (irq_exit+0x5b/0x5c) from [<c000cfc3>] (handle_IRQ+0x37/0x78)
[59014.173124] [<c000cfc3>] (handle_IRQ+0x37/0x78) from [<c00085df>] (omap3_intc_handle_irq+0x5f/0x68)
[59014.187225] [<c00085df>] (omap3_intc_handle_irq+0x5f/0x68) from [<c04c28db>](__irq_svc+0x3b/0x5c)
This bug can be see with low performance board, such as uniprocessor beagle bone board.
Signed-off-by: Yuwei Zheng <zhengyuwei@360.cn>
Signed-off-by: Yuwei Zheng <yuweizheng@139.com>

---
 drivers/net/wireless/ath/ath9k/hif_usb.c       | 61 +++++++++++++++++++++++---
 drivers/net/wireless/ath/ath9k/hif_usb.h       |  6 +++
 drivers/net/wireless/ath/ath9k/htc.h           | 18 ++++++++
 drivers/net/wireless/ath/ath9k/htc_drv_debug.c | 46 +++++++++++++++++++
 drivers/net/wireless/ath/ath9k/htc_drv_txrx.c  | 25 +++++++++++
 5 files changed, 149 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c
index 8e7153b..9166f10 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.c
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
@@ -640,6 +640,7 @@ static void ath9k_hif_usb_rx_cb(struct urb *urb)
 	struct hif_device_usb *hif_dev =
 		usb_get_intfdata(usb_ifnum_to_if(urb->dev, 0));
 	int ret;
+	int delay;
 
 	if (!skb)
 		return;
@@ -658,7 +659,6 @@ static void ath9k_hif_usb_rx_cb(struct urb *urb)
 	default:
 		goto resubmit;
 	}
-
 	if (likely(urb->actual_length != 0)) {
 		skb_put(skb, urb->actual_length);
 		ath9k_hif_usb_rx_stream(hif_dev, skb);
@@ -667,12 +667,18 @@ static void ath9k_hif_usb_rx_cb(struct urb *urb)
 resubmit:
 	skb_reset_tail_pointer(skb);
 	skb_trim(skb, 0);
-
-	usb_anchor_urb(urb, &hif_dev->rx_submitted);
-	ret = usb_submit_urb(urb, GFP_ATOMIC);
-	if (ret) {
-		usb_unanchor_urb(urb);
-		goto free;
+	if (atomic_read(&hif_dev->rx_urb_submit_delay) > 0) {
+		usb_anchor_urb(urb, &hif_dev->rx_delayed_submitted);
+		delay = atomic_read(&hif_dev->rx_urb_submit_delay);
+		schedule_delayed_work(&hif_dev->rx_submit_delayed_work,
+				      usecs_to_jiffies(delay));
+	} else {
+		usb_anchor_urb(urb, &hif_dev->rx_submitted);
+		ret = usb_submit_urb(urb, GFP_ATOMIC);
+		if (ret) {
+			usb_unanchor_urb(urb);
+			goto free;
+		}
 	}
 
 	return;
@@ -818,9 +824,43 @@ err:
 	return -ENOMEM;
 }
 
+static void rx_urb_delayed_submit_handler(struct work_struct *work)
+{
+	struct hif_device_usb *hif_dev =
+		container_of(work,
+			     struct hif_device_usb,
+			     rx_submit_delayed_work.work);
+
+	struct urb *urb = NULL;
+	struct sk_buff *skb = NULL;
+	int ret;
+	int loop_times = 0;
+
+	while (true) {
+		loop_times++;
+		if (loop_times > (MAX_RX_URB_NUM))
+			atomic_add(ARC_RX_STEP, &hif_dev->rx_urb_submit_delay);
+
+		urb = usb_get_from_anchor(&hif_dev->rx_delayed_submitted);
+		if (urb) {
+			skb = (struct sk_buff *)urb->context;
+			ret = usb_submit_urb(urb, GFP_KERNEL);
+			if (ret != 0) {
+				usb_unanchor_urb(urb);
+				dev_kfree_skb_any(skb);
+				urb->context = NULL;
+			}
+		} else {
+			break;
+		}
+	}
+}
+
 static void ath9k_hif_usb_dealloc_rx_urbs(struct hif_device_usb *hif_dev)
 {
 	usb_kill_anchored_urbs(&hif_dev->rx_submitted);
+	usb_kill_anchored_urbs(&hif_dev->rx_delayed_submitted);
+	flush_delayed_work(&hif_dev->rx_submit_delayed_work);
 }
 
 static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
@@ -830,6 +870,8 @@ static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
 	int i, ret;
 
 	init_usb_anchor(&hif_dev->rx_submitted);
+	init_usb_anchor(&hif_dev->rx_delayed_submitted);
+
 	spin_lock_init(&hif_dev->rx_lock);
 
 	for (i = 0; i < MAX_RX_URB_NUM; i++) {
@@ -871,6 +913,11 @@ static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
 		usb_free_urb(urb);
 	}
 
+	/* add for adaptive rate control*/
+	atomic_set(&hif_dev->rx_urb_submit_delay, 0);
+	INIT_DELAYED_WORK(&hif_dev->rx_submit_delayed_work,
+			  rx_urb_delayed_submit_handler);
+
 	return 0;
 
 err_submit:
diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.h b/drivers/net/wireless/ath/ath9k/hif_usb.h
index 51496e7..2127e50 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.h
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.h
@@ -41,6 +41,7 @@
 #define MAX_RX_URB_NUM  8
 #define MAX_RX_BUF_SIZE 16384
 #define MAX_PKT_NUM_IN_TRANSFER 10
+#define ARC_RX_STEP 100 /* 100us */
 
 #define MAX_REG_OUT_URB_NUM  1
 #define MAX_REG_IN_URB_NUM   64
@@ -98,9 +99,14 @@ struct hif_device_usb {
 	struct hif_usb_tx tx;
 	struct usb_anchor regout_submitted;
 	struct usb_anchor rx_submitted;
+	struct usb_anchor rx_delayed_submitted; /* delayed submit anchor */
 	struct usb_anchor reg_in_submitted;
 	struct usb_anchor mgmt_submitted;
 	struct sk_buff *remain_skb;
+
+	struct delayed_work rx_submit_delayed_work;
+	atomic_t  rx_urb_submit_delay; /*us*/
+
 	const char *fw_name;
 	int rx_remain_len;
 	int rx_pkt_len;
diff --git a/drivers/net/wireless/ath/ath9k/htc.h b/drivers/net/wireless/ath/ath9k/htc.h
index 9dde265..3061dfa 100644
--- a/drivers/net/wireless/ath/ath9k/htc.h
+++ b/drivers/net/wireless/ath/ath9k/htc.h
@@ -331,6 +331,13 @@ static inline struct ath9k_htc_tx_ctl *HTC_SKB_CB(struct sk_buff *skb)
 
 #define TX_QSTAT_INC(q) (priv->debug.tx_stats.queue_stats[q]++)
 
+#define ARCRX_STAT_INC(c) \
+	(hif_dev->htc_handle->drv_priv->debug.arcrx_stats.c++)
+#define ARCRX_STAT_ADD(c, a) \
+	(hif_dev->htc_handle->drv_priv->debug.arcrx_stats.c += a)
+#define ARCRX_STAT_SET(c, a) \
+	(hif_dev->htc_handle->drv_priv->debug.arcrx_stats.c = a)
+
 void ath9k_htc_err_stat_rx(struct ath9k_htc_priv *priv,
 			   struct ath_rx_status *rs);
 
@@ -352,11 +359,19 @@ struct ath_skbrx_stats {
 	u32 skb_dropped;
 };
 
+struct ath_arcrx_stats {
+	u32 arcrx_highwater;
+	u32 arcrx_lowwater;
+	u32 arcrx_watermark_triggered;
+	u32 arcrx_urb_submit_delay;
+};
+
 struct ath9k_debug {
 	struct dentry *debugfs_phy;
 	struct ath_tx_stats tx_stats;
 	struct ath_rx_stats rx_stats;
 	struct ath_skbrx_stats skbrx_stats;
+	struct ath_arcrx_stats arcrx_stats;
 };
 
 void ath9k_htc_get_et_strings(struct ieee80211_hw *hw,
@@ -377,6 +392,9 @@ void ath9k_htc_get_et_stats(struct ieee80211_hw *hw,
 
 #define TX_QSTAT_INC(c) do { } while (0)
 
+#define ARCRX_STAT_INC(c) do {} while (0)
+#define ARCRX_STAT_ADD(c, a) do {} while (0)
+#define ARCRX_STAT_SET(c, a) do {} while (0)
 static inline void ath9k_htc_err_stat_rx(struct ath9k_htc_priv *priv,
 					 struct ath_rx_status *rs)
 {
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_debug.c b/drivers/net/wireless/ath/ath9k/htc_drv_debug.c
index 8cef1ed..ddc8f4f 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_debug.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_debug.c
@@ -286,6 +286,48 @@ static const struct file_operations fops_skb_rx = {
 	.llseek = default_llseek,
 };
 
+static ssize_t read_file_arc_rx(struct file *file, char __user *user_buf,
+				size_t count, loff_t *ppos)
+{
+	struct ath9k_htc_priv *priv = file->private_data;
+	char *buf;
+	unsigned int len = 0, size = 1500;
+	ssize_t retval = 0;
+
+	buf = kzalloc(size, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	len += scnprintf(buf + len, size - len,
+			"%20s : %10u\n", "High watermark",
+			priv->debug.arcrx_stats.arcrx_highwater);
+	len += scnprintf(buf + len, size - len,
+			"%20s : %10u\n", "Low watermark",
+			priv->debug.arcrx_stats.arcrx_lowwater);
+
+	len += scnprintf(buf + len, size - len,
+			"%20s : %10u\n", "WM triggered",
+			priv->debug.arcrx_stats.arcrx_watermark_triggered);
+
+	len += scnprintf(buf + len, size - len,
+			"%20s : %10u\n", "URB delay",
+			priv->debug.arcrx_stats.arcrx_urb_submit_delay);
+	if (len > size)
+		len = size;
+
+	retval = simple_read_from_buffer(user_buf, count, ppos, buf, len);
+	kfree(buf);
+
+	return retval;
+}
+
+static const struct file_operations fops_arc_rx = {
+	.read = read_file_arc_rx,
+	.open = simple_open,
+	.owner = THIS_MODULE,
+	.llseek = default_llseek,
+};
+
 static ssize_t read_file_slot(struct file *file, char __user *user_buf,
 			      size_t count, loff_t *ppos)
 {
@@ -518,7 +560,11 @@ int ath9k_htc_init_debug(struct ath_hw *ah)
 	debugfs_create_file("skb_rx", S_IRUSR, priv->debug.debugfs_phy,
 			    priv, &fops_skb_rx);
 
+	debugfs_create_file("arc_rx", S_IRUSR, priv->debug.debugfs_phy,
+			    priv, &fops_arc_rx);
+
 	ath9k_cmn_debug_recv(priv->debug.debugfs_phy, &priv->debug.rx_stats);
+
 	ath9k_cmn_debug_phy_err(priv->debug.debugfs_phy, &priv->debug.rx_stats);
 
 	debugfs_create_file("slot", S_IRUSR, priv->debug.debugfs_phy,
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
index a0f58e2..9d2dc33 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
@@ -1061,7 +1061,27 @@ void ath9k_rx_tasklet(unsigned long data)
 	unsigned long flags;
 	struct ieee80211_hdr *hdr;
 
+	/* add for adaptive flow control*/
+	int looptimes = 0;
+	int highwatermark = ATH9K_HTC_RXBUF*3/4;
+	int lowwatermark = ATH9K_HTC_RXBUF/32;
+	unsigned int delay = 0;
+
+	struct htc_target *htc = priv->htc;
+	struct hif_device_usb *hif_dev = htc->hif_dev;
+
+	ARCRX_STAT_SET(arcrx_highwater, highwatermark);
+	ARCRX_STAT_SET(arcrx_lowwater, lowwatermark);
+
 	do {
+		looptimes++;
+		if (looptimes > highwatermark) {
+			delay = looptimes*ARC_RX_STEP;
+			atomic_set(&hif_dev->rx_urb_submit_delay, delay);
+			ARCRX_STAT_INC(arcrx_watermark_triggered);
+			ARCRX_STAT_SET(arcrx_urb_submit_delay, delay);
+		}
+
 		spin_lock_irqsave(&priv->rx.rxbuflock, flags);
 		list_for_each_entry(tmp_buf, &priv->rx.rxbuf, list) {
 			if (tmp_buf->in_process) {
@@ -1072,6 +1092,11 @@ void ath9k_rx_tasklet(unsigned long data)
 
 		if (rxbuf == NULL) {
 			spin_unlock_irqrestore(&priv->rx.rxbuflock, flags);
+			if (looptimes < lowwatermark) {
+				atomic_set(&hif_dev->rx_urb_submit_delay, 0);
+				ARCRX_STAT_SET(arcrx_urb_submit_delay, 0);
+			}
+
 			break;
 		}
 
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH v2 16/18] vhost: don't bother with copying iovec in handle_tx()
From: Michael S. Tsirkin @ 2015-02-03  9:14 UTC (permalink / raw)
  To: Al Viro; +Cc: David Miller, netdev, kvm
In-Reply-To: <1422863977-17668-16-git-send-email-viro@ZenIV.linux.org.uk>

On Mon, Feb 02, 2015 at 07:59:35AM +0000, Al Viro wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> just advance the msg.msg_iter and be done with that.
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: kvm@vger.kernel.org
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Nice.

Acked-by: Michael S. Tsirkin <mst@redhat.com>

But, can you pls copy virtualization@lists.linux-foundation.org ?
I think some guys working on virtio might only hang out there.


> ---
>  drivers/vhost/net.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 6906f76..d86cc9b 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -336,7 +336,7 @@ static void handle_tx(struct vhost_net *net)
>  {
>  	struct vhost_net_virtqueue *nvq = &net->vqs[VHOST_NET_VQ_TX];
>  	struct vhost_virtqueue *vq = &nvq->vq;
> -	unsigned out, in, s;
> +	unsigned out, in;
>  	int head;
>  	struct msghdr msg = {
>  		.msg_name = NULL,
> @@ -395,16 +395,17 @@ static void handle_tx(struct vhost_net *net)
>  			break;
>  		}
>  		/* Skip header. TODO: support TSO. */
> -		s = move_iovec_hdr(vq->iov, nvq->hdr, hdr_size, out);
>  		len = iov_length(vq->iov, out);
>  		iov_iter_init(&msg.msg_iter, WRITE, vq->iov, out, len);
> +		iov_iter_advance(&msg.msg_iter, hdr_size);
>  		/* Sanity check */
> -		if (!len) {
> +		if (!iov_iter_count(&msg.msg_iter)) {
>  			vq_err(vq, "Unexpected header len for TX: "
>  			       "%zd expected %zd\n",
> -			       iov_length(nvq->hdr, s), hdr_size);
> +			       len, hdr_size);
>  			break;
>  		}
> +		len = iov_iter_count(&msg.msg_iter);
>  
>  		zcopy_used = zcopy && len >= VHOST_GOODCOPY_LEN
>  				   && (nvq->upend_idx + 1) % UIO_MAXIOV !=
> -- 
> 2.1.4

^ permalink raw reply

* Re: [PATCH v2 17/18] vhost: don't bother copying iovecs in handle_rx(), kill memcpy_toiovecend()
From: Michael S. Tsirkin @ 2015-02-03  9:13 UTC (permalink / raw)
  To: Al Viro; +Cc: David Miller, netdev, kvm
In-Reply-To: <1422863977-17668-17-git-send-email-viro@ZenIV.linux.org.uk>

On Mon, Feb 02, 2015 at 07:59:36AM +0000, Al Viro wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: kvm@vger.kernel.org
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---

So this made me notice a bug in vhost introduced in 3.19.
I sent a patch for that, this one will have to be
rebased on top. Otherwise:

Acked-by: Michael S. Tsirkin <mst@redhat.com>

But, can you pls copy virtualization@lists.linux-foundation.org ?
I think some guys working on virtio might only hang out there.



>  drivers/vhost/net.c | 79 ++++++++++++++---------------------------------------
>  include/linux/uio.h |  3 --
>  lib/iovec.c         | 26 ------------------
>  3 files changed, 20 insertions(+), 88 deletions(-)
> 
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index d86cc9b..73c0ebf 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -84,10 +84,6 @@ struct vhost_net_ubuf_ref {
>  
>  struct vhost_net_virtqueue {
>  	struct vhost_virtqueue vq;
> -	/* hdr is used to store the virtio header.
> -	 * Since each iovec has >= 1 byte length, we never need more than
> -	 * header length entries to store the header. */
> -	struct iovec hdr[sizeof(struct virtio_net_hdr_mrg_rxbuf)];
>  	size_t vhost_hlen;
>  	size_t sock_hlen;
>  	/* vhost zerocopy support fields below: */
> @@ -235,44 +231,6 @@ static bool vhost_sock_zcopy(struct socket *sock)
>  		sock_flag(sock->sk, SOCK_ZEROCOPY);
>  }
>  
> -/* Pop first len bytes from iovec. Return number of segments used. */
> -static int move_iovec_hdr(struct iovec *from, struct iovec *to,
> -			  size_t len, int iov_count)
> -{
> -	int seg = 0;
> -	size_t size;
> -
> -	while (len && seg < iov_count) {
> -		size = min(from->iov_len, len);
> -		to->iov_base = from->iov_base;
> -		to->iov_len = size;
> -		from->iov_len -= size;
> -		from->iov_base += size;
> -		len -= size;
> -		++from;
> -		++to;
> -		++seg;
> -	}
> -	return seg;
> -}
> -/* Copy iovec entries for len bytes from iovec. */
> -static void copy_iovec_hdr(const struct iovec *from, struct iovec *to,
> -			   size_t len, int iovcount)
> -{
> -	int seg = 0;
> -	size_t size;
> -
> -	while (len && seg < iovcount) {
> -		size = min(from->iov_len, len);
> -		to->iov_base = from->iov_base;
> -		to->iov_len = size;
> -		len -= size;
> -		++from;
> -		++to;
> -		++seg;
> -	}
> -}
> -
>  /* In case of DMA done not in order in lower device driver for some reason.
>   * upend_idx is used to track end of used idx, done_idx is used to track head
>   * of used idx. Once lower device DMA done contiguously, we will signal KVM
> @@ -570,9 +528,9 @@ static void handle_rx(struct vhost_net *net)
>  		.msg_controllen = 0,
>  		.msg_flags = MSG_DONTWAIT,
>  	};
> -	struct virtio_net_hdr_mrg_rxbuf hdr = {
> -		.hdr.flags = 0,
> -		.hdr.gso_type = VIRTIO_NET_HDR_GSO_NONE
> +	struct virtio_net_hdr hdr = {
> +		.flags = 0,
> +		.gso_type = VIRTIO_NET_HDR_GSO_NONE
>  	};
>  	size_t total_len = 0;
>  	int err, mergeable;
> @@ -580,6 +538,7 @@ static void handle_rx(struct vhost_net *net)
>  	size_t vhost_hlen, sock_hlen;
>  	size_t vhost_len, sock_len;
>  	struct socket *sock;
> +	struct iov_iter fixup;
>  
>  	mutex_lock(&vq->mutex);
>  	sock = vq->private_data;
> @@ -624,14 +583,17 @@ static void handle_rx(struct vhost_net *net)
>  			break;
>  		}
>  		/* We don't need to be notified again. */
> -		if (unlikely((vhost_hlen)))
> -			/* Skip header. TODO: support TSO. */
> -			move_iovec_hdr(vq->iov, nvq->hdr, vhost_hlen, in);
> -		else
> -			/* Copy the header for use in VIRTIO_NET_F_MRG_RXBUF:
> -			 * needed because recvmsg can modify msg_iov. */
> -			copy_iovec_hdr(vq->iov, nvq->hdr, sock_hlen, in);
> -		iov_iter_init(&msg.msg_iter, READ, vq->iov, in, sock_len);
> +		iov_iter_init(&msg.msg_iter, READ, vq->iov, in, vhost_len);
> +		fixup = msg.msg_iter;
> +		if (unlikely((vhost_hlen))) {
> +			/* We will supply the header ourselves
> +			 * TODO: support TSO. */
> +			iov_iter_advance(&msg.msg_iter, vhost_hlen);
> +		} else {
> +			/* It'll come from socket; we'll need to patch
> +			 * ->num_buffers over if VIRTIO_NET_F_MRG_RXBUF */
> +			iov_iter_advance(&fixup, sizeof(hdr));
> +		}
>  		err = sock->ops->recvmsg(NULL, sock, &msg,
>  					 sock_len, MSG_DONTWAIT | MSG_TRUNC);
>  		/* Userspace might have consumed the packet meanwhile:
> @@ -643,18 +605,17 @@ static void handle_rx(struct vhost_net *net)
>  			vhost_discard_vq_desc(vq, headcount);
>  			continue;
>  		}
> +		/* Supply virtio_net_hdr if VHOST_NET_F_VIRTIO_NET_HDR */
>  		if (unlikely(vhost_hlen) &&
> -		    memcpy_toiovecend(nvq->hdr, (unsigned char *)&hdr, 0,
> -				      vhost_hlen)) {
> +		    copy_to_iter(&hdr, sizeof(hdr), &fixup) != sizeof(hdr)) {

BTW, all iovecs are pre-validated in vhost core.
I'd like to add __copy_to_iter and __copy_from_iter that are the same
but skip the extra checks, and use that everywhere in vhost (shouln't
matter here specifically, because we don't hit this path).
>From experience, this helps gcc optimize the code resulting
in measureable performance gains.
Comments? Will you be ok with a patch like this?


>  			vq_err(vq, "Unable to write vnet_hdr at addr %p\n",
>  			       vq->iov->iov_base);
>  			break;
>  		}
> -		/* TODO: Should check and handle checksum. */
> +		/* Supply (or replace) ->num_buffers if VIRTIO_NET_F_MRG_RXBUF
> +		 * TODO: Should check and handle checksum. */
>  		if (likely(mergeable) &&
> -		    memcpy_toiovecend(nvq->hdr, (unsigned char *)&headcount,
> -				      offsetof(typeof(hdr), num_buffers),
> -				      sizeof hdr.num_buffers)) {
> +		    copy_to_iter(&headcount, 2, &fixup) != 2) {
>  			vq_err(vq, "Failed num_buffers write");
>  			vhost_discard_vq_desc(vq, headcount);
>  			break;

This made me notice we have a bug: native-endianness integer is copied out to guest.
I sent a patch, hope it'll make it in 3.19.


> diff --git a/include/linux/uio.h b/include/linux/uio.h
> index af3439f..02bd8a9 100644
> --- a/include/linux/uio.h
> +++ b/include/linux/uio.h
> @@ -137,7 +137,4 @@ size_t csum_and_copy_from_iter(void *addr, size_t bytes, __wsum *csum, struct io
>  
>  int memcpy_fromiovecend(unsigned char *kdata, const struct iovec *iov,
>  			int offset, int len);
> -int memcpy_toiovecend(const struct iovec *v, unsigned char *kdata,
> -		      int offset, int len);
> -
>  #endif
> diff --git a/lib/iovec.c b/lib/iovec.c
> index 4a90875..d8f17a9 100644
> --- a/lib/iovec.c
> +++ b/lib/iovec.c
> @@ -3,32 +3,6 @@
>  #include <linux/uio.h>
>  
>  /*
> - *	Copy kernel to iovec. Returns -EFAULT on error.
> - */
> -
> -int memcpy_toiovecend(const struct iovec *iov, unsigned char *kdata,
> -		      int offset, int len)
> -{
> -	int copy;
> -	for (; len > 0; ++iov) {
> -		/* Skip over the finished iovecs */
> -		if (unlikely(offset >= iov->iov_len)) {
> -			offset -= iov->iov_len;
> -			continue;
> -		}
> -		copy = min_t(unsigned int, iov->iov_len - offset, len);
> -		if (copy_to_user(iov->iov_base + offset, kdata, copy))
> -			return -EFAULT;
> -		offset = 0;
> -		kdata += copy;
> -		len -= copy;
> -	}
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL(memcpy_toiovecend);
> -
> -/*
>   *	Copy iovec to kernel. Returns -EFAULT on error.
>   */
>  
> -- 
> 2.1.4

^ permalink raw reply

* [PATCH for-3.19] vhost/net: fix up num_buffers endian-ness
From: Michael S. Tsirkin @ 2015-02-03  9:07 UTC (permalink / raw)
  To: linux-kernel; +Cc: kvm, netdev, virtualization, viro, David Miller

In virtio 1.0 mode, when mergeable buffers are enabled on a big-endian
host, num_buffers wasn't byte-swapped correctly, so large incoming
packets got corrupted.

To fix, fill it in within hdr - this also makes sure it gets
the correct type.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

It seems important to get this into 3.19, otherwise
VIRTIO_1 feature bit is unreliable.

Dave, do you plan another pull request for 3.19?
If yes pls merge this, if not pls let me know and I'll try to
send it to Linus directly.


 drivers/vhost/net.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index d415d69..9484d56 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -650,8 +650,10 @@ static void handle_rx(struct vhost_net *net)
 			break;
 		}
 		/* TODO: Should check and handle checksum. */
+
+		hdr.num_buffers = cpu_to_vhost16(vq, headcount);
 		if (likely(mergeable) &&
-		    memcpy_toiovecend(nvq->hdr, (unsigned char *)&headcount,
+		    memcpy_toiovecend(nvq->hdr, (void *)&hdr.num_buffers,
 				      offsetof(typeof(hdr), num_buffers),
 				      sizeof hdr.num_buffers)) {
 			vq_err(vq, "Failed num_buffers write");
-- 
MST

^ permalink raw reply related

* Re: [PATCH v2 18/18] vhost: vhost_scsi_handle_vq() should just use copy_from_user()
From: Michael S. Tsirkin @ 2015-02-03  9:05 UTC (permalink / raw)
  To: Al Viro; +Cc: David Miller, netdev, Nicholas A. Bellinger, kvm
In-Reply-To: <1422863977-17668-18-git-send-email-viro@ZenIV.linux.org.uk>

On Mon, Feb 02, 2015 at 07:59:37AM +0000, Al Viro wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> it has just verified that it asks no more than the length of the
> first segment of iovec.
> 
> And with that the last user of stuff in lib/iovec.c is gone.
> RIP.
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Nicholas A. Bellinger <nab@linux-iscsi.org>
> Cc: kvm@vger.kernel.org
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

BTW it should really be __copy_from_user since the iovec
is pre-validated by vhost core.

Still Nicholas is rewriting it all anyway, so not worth the
time to optimize it, so

Acked-by: Michael S. Tsirkin <mst@redhat.com>

But, can you pls copy virtualization@lists.linux-foundation.org ?
I think some guys working on virtio might only hang out there.

> ---
>  drivers/vhost/scsi.c |  2 +-
>  include/linux/uio.h  |  2 --
>  lib/Makefile         |  2 +-
>  lib/iovec.c          | 36 ------------------------------------
>  4 files changed, 2 insertions(+), 40 deletions(-)
>  delete mode 100644 lib/iovec.c
> 
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index d695b16..dc78d87 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -1079,7 +1079,7 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
>  			       req_size, vq->iov[0].iov_len);
>  			break;
>  		}
> -		ret = memcpy_fromiovecend(req, &vq->iov[0], 0, req_size);
> +		ret = copy_from_user(req, vq->iov[0].iov_base, req_size);
>  		if (unlikely(ret)) {
>  			vq_err(vq, "Faulted on virtio_scsi_cmd_req\n");
>  			break;
> diff --git a/include/linux/uio.h b/include/linux/uio.h
> index 02bd8a9..3e0cb4e 100644
> --- a/include/linux/uio.h
> +++ b/include/linux/uio.h
> @@ -135,6 +135,4 @@ static inline void iov_iter_reexpand(struct iov_iter *i, size_t count)
>  size_t csum_and_copy_to_iter(void *addr, size_t bytes, __wsum *csum, struct iov_iter *i);
>  size_t csum_and_copy_from_iter(void *addr, size_t bytes, __wsum *csum, struct iov_iter *i);
>  
> -int memcpy_fromiovecend(unsigned char *kdata, const struct iovec *iov,
> -			int offset, int len);
>  #endif
> diff --git a/lib/Makefile b/lib/Makefile
> index 3c3b30b..1071d06 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -24,7 +24,7 @@ obj-y	+= lockref.o
>  
>  obj-y += bcd.o div64.o sort.o parser.o halfmd4.o debug_locks.o random32.o \
>  	 bust_spinlocks.o hexdump.o kasprintf.o bitmap.o scatterlist.o \
> -	 gcd.o lcm.o list_sort.o uuid.o flex_array.o iovec.o clz_ctz.o \
> +	 gcd.o lcm.o list_sort.o uuid.o flex_array.o clz_ctz.o \
>  	 bsearch.o find_last_bit.o find_next_bit.o llist.o memweight.o kfifo.o \
>  	 percpu-refcount.o percpu_ida.o rhashtable.o reciprocal_div.o
>  obj-y += string_helpers.o
> diff --git a/lib/iovec.c b/lib/iovec.c
> deleted file mode 100644
> index d8f17a9..0000000
> --- a/lib/iovec.c
> +++ /dev/null
> @@ -1,36 +0,0 @@
> -#include <linux/uaccess.h>
> -#include <linux/export.h>
> -#include <linux/uio.h>
> -
> -/*
> - *	Copy iovec to kernel. Returns -EFAULT on error.
> - */
> -
> -int memcpy_fromiovecend(unsigned char *kdata, const struct iovec *iov,
> -			int offset, int len)
> -{
> -	/* No data? Done! */
> -	if (len == 0)
> -		return 0;
> -
> -	/* Skip over the finished iovecs */
> -	while (offset >= iov->iov_len) {
> -		offset -= iov->iov_len;
> -		iov++;
> -	}
> -
> -	while (len > 0) {
> -		u8 __user *base = iov->iov_base + offset;
> -		int copy = min_t(unsigned int, len, iov->iov_len - offset);
> -
> -		offset = 0;
> -		if (copy_from_user(kdata, base, copy))
> -			return -EFAULT;
> -		len -= copy;
> -		kdata += copy;
> -		iov++;
> -	}
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL(memcpy_fromiovecend);
> -- 
> 2.1.4

^ permalink raw reply

* Re: [PATCH v2 15/18] vhost: switch vhost get_indirect() to iov_iter, kill memcpy_fromiovec()
From: Michael S. Tsirkin @ 2015-02-03  9:01 UTC (permalink / raw)
  To: Al Viro; +Cc: David Miller, netdev, kvm
In-Reply-To: <1422863977-17668-15-git-send-email-viro@ZenIV.linux.org.uk>

On Mon, Feb 02, 2015 at 07:59:34AM +0000, Al Viro wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: kvm@vger.kernel.org
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

But, can you pls copy virtualization@lists.linux-foundation.org ?
I think some guys working on virtio might only hang out there.

> ---
>  drivers/vhost/vhost.c |  6 ++++--
>  include/linux/uio.h   |  1 -
>  lib/iovec.c           | 25 -------------------------
>  3 files changed, 4 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index cb807d0..2ee2826 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -1125,6 +1125,7 @@ static int get_indirect(struct vhost_virtqueue *vq,
>  	struct vring_desc desc;
>  	unsigned int i = 0, count, found = 0;
>  	u32 len = vhost32_to_cpu(vq, indirect->len);
> +	struct iov_iter from;
>  	int ret;
>  
>  	/* Sanity check */
> @@ -1142,6 +1143,7 @@ static int get_indirect(struct vhost_virtqueue *vq,
>  		vq_err(vq, "Translation failure %d in indirect.\n", ret);
>  		return ret;
>  	}
> +	iov_iter_init(&from, READ, vq->indirect, ret, len);
>  
>  	/* We will use the result as an address to read from, so most
>  	 * architectures only need a compiler barrier here. */
> @@ -1164,8 +1166,8 @@ static int get_indirect(struct vhost_virtqueue *vq,
>  			       i, count);
>  			return -EINVAL;
>  		}
> -		if (unlikely(memcpy_fromiovec((unsigned char *)&desc,
> -					      vq->indirect, sizeof desc))) {
> +		if (unlikely(copy_from_iter(&desc, sizeof(desc), &from) !=
> +			     sizeof(desc))) {
>  			vq_err(vq, "Failed indirect descriptor: idx %d, %zx\n",
>  			       i, (size_t)vhost64_to_cpu(vq, indirect->addr) + i * sizeof desc);
>  			return -EINVAL;
> diff --git a/include/linux/uio.h b/include/linux/uio.h
> index 1c5e453..af3439f 100644
> --- a/include/linux/uio.h
> +++ b/include/linux/uio.h
> @@ -135,7 +135,6 @@ static inline void iov_iter_reexpand(struct iov_iter *i, size_t count)
>  size_t csum_and_copy_to_iter(void *addr, size_t bytes, __wsum *csum, struct iov_iter *i);
>  size_t csum_and_copy_from_iter(void *addr, size_t bytes, __wsum *csum, struct iov_iter *i);
>  
> -int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len);
>  int memcpy_fromiovecend(unsigned char *kdata, const struct iovec *iov,
>  			int offset, int len);
>  int memcpy_toiovecend(const struct iovec *v, unsigned char *kdata,
> diff --git a/lib/iovec.c b/lib/iovec.c
> index 2d99cb4..4a90875 100644
> --- a/lib/iovec.c
> +++ b/lib/iovec.c
> @@ -3,31 +3,6 @@
>  #include <linux/uio.h>
>  
>  /*
> - *	Copy iovec to kernel. Returns -EFAULT on error.
> - *
> - *	Note: this modifies the original iovec.
> - */
> -
> -int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len)
> -{
> -	while (len > 0) {
> -		if (iov->iov_len) {
> -			int copy = min_t(unsigned int, len, iov->iov_len);
> -			if (copy_from_user(kdata, iov->iov_base, copy))
> -				return -EFAULT;
> -			len -= copy;
> -			kdata += copy;
> -			iov->iov_base += copy;
> -			iov->iov_len -= copy;
> -		}
> -		iov++;
> -	}
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL(memcpy_fromiovec);
> -
> -/*
>   *	Copy kernel to iovec. Returns -EFAULT on error.
>   */
>  
> -- 
> 2.1.4

^ permalink raw reply

* Re: Throughput regression with `tcp: refine TSO autosizing`
From: Michal Kazior @ 2015-02-03  9:00 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Ben Greear, linux-wireless, Network Development,
	eyalpe-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb
In-Reply-To: <1422918363.21689.132.camel-XN9IlZ5yJG9HTL0Zs8A6p/gx64E7kk8eUsxypvmhUTTZJqsBc5GL+g@public.gmane.org>

On 3 February 2015 at 00:06, Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Mon, 2015-02-02 at 13:25 -0800, Ben Greear wrote:
>
>> It is a big throughput win to have fewer TCP ack packets on
>> wireless since it is a half-duplex environment.  Is there anything
>> we could improve so that we can have fewer acks and still get
>> good tcp stack behaviour?
>
> First apply TCP stretch ack fixes to the sender. There is no way to get
> good performance if the sender does not handle stretch ack.
>
> d6b1a8a92a14 tcp: fix timing issue in CUBIC slope calculation
> 9cd981dcf174 tcp: fix stretch ACK bugs in CUBIC
> c22bdca94782 tcp: fix stretch ACK bugs in Reno
> 814d488c6126 tcp: fix the timid additive increase on stretch ACKs
> e73ebb0881ea tcp: stretch ACK fixes prep
>
> Then, make sure you do not throttle ACK too long, especially if you hope
> to get Gbit line rate on a 4 ms RTT flow.
>
> GRO does not mean : send one ACK every ms, or after 3ms delay...

I think it's worth pointing out that If you assume 3-frame A-MSDU and
64-frame A-MPDU you get 192 frames (as far as TCP/IP is concerned) per
aggregation window. Assuming effective 600mbps throughput:

 python> 1.0/((((600/8)*1024*1024)/1500)/(3*64))
 0.003663003663003663

This is probably worst case, but still probably worth to keep in mind.

ath10k has a knob to tune A-MSDU aggregation count. The default is "3"
and it's what I've been testing so far.

When I change it to "1" on sender I get 250->400mbps boost in TCP -P5
but see no difference with -P1 (number of flows). Changing it to "1"
on receiver yields no difference. I can try adding this configuration
permutation to my future tests if you're interested.

So that you have an idea - using "1" on sender degrades UDP throughput
(even 690->500mbps in some cases).


Michał
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH 2/2] tlan: msecs_to_jiffies convrsion
From: Nicholas Mc Guire @ 2015-02-03  8:45 UTC (permalink / raw)
  To: Samuel Chessman; +Cc: netdev, linux-kernel, Nicholas Mc Guire

This is only an API consolidation and should make things more readable
it replaces var * HZ / 1000 by msecs_to_jiffies(var).

As there is a discrepancy between the code and the comments this is in
a separate patch.

Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
---

Converting milliseconds to jiffies by val * HZ / 1000 is technically
not wrong but msecs_to_jiffies(val) is the cleaner solution and handles
all corner cases correctly. This basically is a minor API cleanup only
but the comment and the code do not match ! Comments states "do the
longer, just in case" which would be 500 ms - the code though sets it
to HZ/20 == 50ms. This needs a review by someone that knows the driver
details to decide if it should be 50 or 500ms here.

Patch was only compile tested for x86_64_defconfig + CONFIG_TLAN=m

Patch is against 3.0.19-rc7 (localversion = -next-20150203)

 drivers/net/ethernet/ti/tlan.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ti/tlan.c b/drivers/net/ethernet/ti/tlan.c
index 1f722c9..691ec93 100644
--- a/drivers/net/ethernet/ti/tlan.c
+++ b/drivers/net/ethernet/ti/tlan.c
@@ -2561,7 +2561,7 @@ static void tlan_phy_power_up(struct net_device *dev)
 	 * transceiver.  The TLAN docs say both 50 ms and
 	 * 500 ms, so do the longer, just in case.
 	 */
-	tlan_set_timer(dev, (HZ/20), TLAN_TIMER_PHY_RESET);
+	tlan_set_timer(dev, msecs_to_jiffies(500), TLAN_TIMER_PHY_RESET);
 
 }
 
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 1/2] tlan: use msecs_to_jiffies for conversion
From: Nicholas Mc Guire @ 2015-02-03  8:44 UTC (permalink / raw)
  To: Samuel Chessman; +Cc: netdev, linux-kernel, Nicholas Mc Guire

This is only an API consolidation and should make things more readable
it replaces var * HZ / 1000 by msecs_to_jiffies(var).

Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
---

Converting milliseconds to jiffies by val * HZ / 1000 is technically
not wrong but msecs_to_jiffies(val) is the cleaner solution and handles
all corner cases correctly. This is a minor API cleanup only.

Patch was only compile tested for x86_64_defconfig + CONFIG_TLAN=m

Patch is against 3.0.19-rc7 (localversion = -next-20150203)

 drivers/net/ethernet/ti/tlan.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/ti/tlan.c b/drivers/net/ethernet/ti/tlan.c
index f2ff007..1f722c9 100644
--- a/drivers/net/ethernet/ti/tlan.c
+++ b/drivers/net/ethernet/ti/tlan.c
@@ -2540,7 +2540,7 @@ static void tlan_phy_power_down(struct net_device *dev)
 	 * This is abitrary.  It is intended to make sure the
 	 * transceiver settles.
 	 */
-	tlan_set_timer(dev, (HZ/20), TLAN_TIMER_PHY_PUP);
+	tlan_set_timer(dev, msecs_to_jiffies(50), TLAN_TIMER_PHY_PUP);
 
 }
 
@@ -2593,7 +2593,7 @@ static void tlan_phy_reset(struct net_device *dev)
 	 * I don't remember why I wait this long.
 	 * I've changed this to 50ms, as it seems long enough.
 	 */
-	tlan_set_timer(dev, (HZ/20), TLAN_TIMER_PHY_START_LINK);
+	tlan_set_timer(dev, msecs_to_jiffies(50), TLAN_TIMER_PHY_START_LINK);
 
 }
 
@@ -2658,7 +2658,7 @@ static void tlan_phy_start_link(struct net_device *dev)
 		data = TLAN_NET_CFG_1FRAG | TLAN_NET_CFG_1CHAN
 			| TLAN_NET_CFG_PHY_EN;
 		tlan_dio_write16(dev->base_addr, TLAN_NET_CONFIG, data);
-		tlan_set_timer(dev, (40*HZ/1000), TLAN_TIMER_PHY_PDOWN);
+		tlan_set_timer(dev, msecs_to_jiffies(40), TLAN_TIMER_PHY_PDOWN);
 		return;
 	} else if (priv->phy_num == 0) {
 		control = 0;
@@ -2725,7 +2725,7 @@ static void tlan_phy_finish_auto_neg(struct net_device *dev)
 	    (priv->adapter->flags & TLAN_ADAPTER_USE_INTERN_10) &&
 	    (priv->phy_num != 0)) {
 		priv->phy_num = 0;
-		tlan_set_timer(dev, (400*HZ/1000), TLAN_TIMER_PHY_PDOWN);
+		tlan_set_timer(dev, msecs_to_jiffies(400), TLAN_TIMER_PHY_PDOWN);
 		return;
 	}
 
@@ -2744,7 +2744,7 @@ static void tlan_phy_finish_auto_neg(struct net_device *dev)
 
 	/* Wait for 100 ms.  No reason in partiticular.
 	 */
-	tlan_set_timer(dev, (HZ/10), TLAN_TIMER_FINISH_RESET);
+	tlan_set_timer(dev, msecs_to_jiffies(100), TLAN_TIMER_FINISH_RESET);
 
 }
 
@@ -2796,7 +2796,7 @@ static void tlan_phy_monitor(unsigned long data)
 				/* set to external PHY */
 				priv->phy_num = 1;
 				/* restart autonegotiation */
-				tlan_set_timer(dev, 4 * HZ / 10,
+				tlan_set_timer(dev, msecs_to_jiffies(400),
 					       TLAN_TIMER_PHY_PDOWN);
 				return;
 			}
-- 
1.7.10.4

^ permalink raw reply related

* Re: Throughput regression with `tcp: refine TSO autosizing`
From: Michal Kazior @ 2015-02-03  8:44 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: linux-wireless, Network Development, eyalpe
In-Reply-To: <1422903136.21689.114.camel@edumazet-glaptop2.roam.corp.google.com>

On 2 February 2015 at 19:52, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Mon, 2015-02-02 at 11:27 +0100, Michal Kazior wrote:
>
>> While testing I've had my internal GRO patch for ath10k and no stretch
>> ack patches.
>
> Thanks for the data, I took a look at it.
>
> I am afraid this GRO patch might be the problem.

The entire performance drop happens without the GRO patch as well. I
tested with it included because I intended to upstream it later. I'll
run without it in future tests.


[...]
> Could you make again your experiments using upstream kernel (David
> Miller net tree) ?

Sure.


> You also could post the GRO patch so that we can comment on it.

(You probably want to see mac80211 patch as well:
06d181a8fd58031db9c114d920b40d8820380a6e "mac80211: add NAPI support
back")

diff --git a/drivers/net/wireless/ath/ath10k/core.c
b/drivers/net/wireless/ath/ath10k/core.c
index 36a8fcf..367e896 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -1147,6 +1147,12 @@ err:
 }
 EXPORT_SYMBOL(ath10k_core_start);

+static int ath10k_core_napi_dummy_poll(struct napi_struct *napi, int budget)
+{
+       WARN_ON(1);
+       return 0;
+}
+
 int ath10k_wait_for_suspend(struct ath10k *ar, u32 suspend_opt)
 {
        int ret;
@@ -1414,6 +1420,10 @@ struct ath10k *ath10k_core_create(size_t
priv_size, struct device *dev,
        INIT_WORK(&ar->register_work, ath10k_core_register_work);
        INIT_WORK(&ar->restart_work, ath10k_core_restart);

+       init_dummy_netdev(&ar->napi_dev);
+       ieee80211_napi_add(ar->hw, &ar->napi, &ar->napi_dev,
+                          ath10k_core_napi_dummy_poll, 64);
+
        ret = ath10k_debug_create(ar);
        if (ret)
                goto err_free_wq;
@@ -1434,6 +1444,7 @@ void ath10k_core_destroy(struct ath10k *ar)
 {
        flush_workqueue(ar->workqueue);
        destroy_workqueue(ar->workqueue);
+       netif_napi_del(&ar->napi);

        ath10k_debug_destroy(ar);
        ath10k_mac_destroy(ar);
diff --git a/drivers/net/wireless/ath/ath10k/core.h
b/drivers/net/wireless/ath/ath10k/core.h
index 2d9f871..b5a8847 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -623,6 +623,9 @@ struct ath10k {

        struct dfs_pattern_detector *dfs_detector;

+       struct net_device napi_dev;
+       struct napi_struct napi;
+
 #ifdef CONFIG_ATH10K_DEBUGFS
        struct ath10k_debug debug;
 #endif
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c
b/drivers/net/wireless/ath/ath10k/htt_rx.c
index c1da44f..7e58b38 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -2061,5 +2061,7 @@ static void ath10k_htt_txrx_compl_task(unsigned long ptr)
                ath10k_htt_rx_in_ord_ind(ar, skb);
                dev_kfree_skb_any(skb);
        }
+
+       napi_gro_flush(&htt->ar->napi, false);
        spin_unlock_bh(&htt->rx_ring.lock);
 }

So that you can quickly get an understanding how ath10k Rx works:
first tasklet (not visible in the patch) picks up smallish event
buffers from firmware and puts them into ath10k queue for latter
processing by another tasklet (the last hunk). Each such event buffer
is just some metainfo but can "carry" tens of frames (both Rx and Tx
completions). The count is arbitrary and depends on fw/hw combo and
air conditions. The GRO flush is called after all queued small event
buffers are processed (frames delivered up to mac80211 which can in
turn perform aggregation reordering in case some frames were
re-transmitted in the meantime before handing them to net subsystem).


Michał

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox