* Re: [PATCHv1 net] xen-netback: stop the guest rx thread after a fatal error
From: David Miller @ 2015-02-03 3:39 UTC (permalink / raw)
To: david.vrabel; +Cc: netdev, xen-devel, ian.campbell, wei.liu2
In-Reply-To: <1422896271-26551-1-git-send-email-david.vrabel@citrix.com>
From: David Vrabel <david.vrabel@citrix.com>
Date: Mon, 2 Feb 2015 16:57:51 +0000
> After commit e9d8b2c2968499c1f96563e6522c56958d5a1d0d (xen-netback:
> disable rogue vif in kthread context), a fatal (protocol) error would
> leave the guest Rx thread spinning, wasting CPU time. Commit
> ecf08d2dbb96d5a4b4bcc53a39e8d29cc8fef02e (xen-netback: reintroduce
> guest Rx stall detection) made this even worse by removing a
> cond_resched() from this path.
>
> Since a fatal error is non-recoverable, just allow the guest Rx thread
> to exit. This requires taking additional refs to the task so the
> thread exiting early is handled safely.
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> Reported-by: Julien Grall <julien.grall@linaro.org>
> Tested-by: Julien Grall <julien.grall@linaro.org>
Applied, thank you.
^ permalink raw reply
* RE: [PATCH net-next 2/8] cxgb4: Added support in debugfs to display tp_la stats
From: Hariprasad S @ 2015-02-03 5:00 UTC (permalink / raw)
To: David Miller
Cc: netdev@vger.kernel.org, Casey Leedom, Anish Bhatt,
Nirranjan Kirubaharan, Praveen Madhavan
In-Reply-To: <20150202.131436.102902508236295371.davem@davemloft.net>
Hi David,
Will send a V2. For now will send only busy_poll.
Thanks,
Hari
________________________________________
From: David Miller [davem@davemloft.net]
Sent: Tuesday, February 03, 2015 2:44 AM
To: Hariprasad S
Cc: netdev@vger.kernel.org; Casey Leedom; Anish Bhatt; Nirranjan Kirubaharan; Praveen Madhavan
Subject: Re: [PATCH net-next 2/8] cxgb4: Added support in debugfs to display tp_la stats
From: Hariprasad Shenai <hariprasad@chelsio.com>
Date: Mon, 2 Feb 2015 20:23:03 +0530
> Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
Ok I've seen enough.
You guys really, truly, abuse debugfs. So I'm putting my foot down now.
Stats like this can be exported through traditional means such as via
ethtool.
^ permalink raw reply
* [PATCH net-next] cxgb4: Add low latency socket busy_poll support
From: Hariprasad Shenai @ 2015-02-03 5:20 UTC (permalink / raw)
To: netdev
Cc: davem, leedom, anish, nirranjan, praveenm, kumaras,
Hariprasad Shenai
cxgb_busy_poll, corresponding to ndo_busy_poll, gets called by the socket
waiting for data.
With busy_poll enabled, improvement is seen in latency numbers as observed by
collecting netperf TCP_RR numbers.
Based on original work by Kumar Sanghvi <kumaras@chelsio.com>
Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 113 +++++++++++++++++++++++
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 16 +++-
drivers/net/ethernet/chelsio/cxgb4/sge.c | 47 +++++++++-
drivers/net/ethernet/chelsio/cxgb4/t4_values.h | 1 +
4 files changed, 174 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
index fb6980a..55019c9 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
@@ -476,6 +476,22 @@ struct sge_rspq { /* state for an SGE response queue */
struct adapter *adap;
struct net_device *netdev; /* associated net device */
rspq_handler_t handler;
+#ifdef CONFIG_NET_RX_BUSY_POLL
+#define CXGB_POLL_STATE_IDLE 0
+#define CXGB_POLL_STATE_NAPI BIT(0) /* NAPI owns this poll */
+#define CXGB_POLL_STATE_POLL BIT(1) /* poll owns this poll */
+#define CXGB_POLL_STATE_NAPI_YIELD BIT(2) /* NAPI yielded this poll */
+#define CXGB_POLL_STATE_POLL_YIELD BIT(3) /* poll yielded this poll */
+#define CXGB_POLL_YIELD (CXGB_POLL_STATE_NAPI_YIELD | \
+ CXGB_POLL_STATE_POLL_YIELD)
+#define CXGB_POLL_LOCKED (CXGB_POLL_STATE_NAPI | \
+ CXGB_POLL_STATE_POLL)
+#define CXGB_POLL_USER_PEND (CXGB_POLL_STATE_POLL | \
+ CXGB_POLL_STATE_POLL_YIELD)
+ unsigned int bpoll_state;
+ spinlock_t bpoll_lock; /* lock for busy poll */
+#endif /* CONFIG_NET_RX_BUSY_POLL */
+
};
struct sge_eth_stats { /* Ethernet queue statistics */
@@ -880,6 +896,102 @@ static inline struct adapter *netdev2adap(const struct net_device *dev)
return netdev2pinfo(dev)->adapter;
}
+#ifdef CONFIG_NET_RX_BUSY_POLL
+static inline void cxgb_busy_poll_init_lock(struct sge_rspq *q)
+{
+ spin_lock_init(&q->bpoll_lock);
+ q->bpoll_state = CXGB_POLL_STATE_IDLE;
+}
+
+static inline bool cxgb_poll_lock_napi(struct sge_rspq *q)
+{
+ bool rc = true;
+
+ spin_lock(&q->bpoll_lock);
+ if (q->bpoll_state & CXGB_POLL_LOCKED) {
+ q->bpoll_state |= CXGB_POLL_STATE_NAPI_YIELD;
+ rc = false;
+ } else {
+ q->bpoll_state = CXGB_POLL_STATE_NAPI;
+ }
+ spin_unlock(&q->bpoll_lock);
+ return rc;
+}
+
+static inline bool cxgb_poll_unlock_napi(struct sge_rspq *q)
+{
+ bool rc = false;
+
+ spin_lock(&q->bpoll_lock);
+ if (q->bpoll_state & CXGB_POLL_STATE_POLL_YIELD)
+ rc = true;
+ q->bpoll_state = CXGB_POLL_STATE_IDLE;
+ spin_unlock(&q->bpoll_lock);
+ return rc;
+}
+
+static inline bool cxgb_poll_lock_poll(struct sge_rspq *q)
+{
+ bool rc = true;
+
+ spin_lock_bh(&q->bpoll_lock);
+ if (q->bpoll_state & CXGB_POLL_LOCKED) {
+ q->bpoll_state |= CXGB_POLL_STATE_POLL_YIELD;
+ rc = false;
+ } else {
+ q->bpoll_state |= CXGB_POLL_STATE_POLL;
+ }
+ spin_unlock_bh(&q->bpoll_lock);
+ return rc;
+}
+
+static inline bool cxgb_poll_unlock_poll(struct sge_rspq *q)
+{
+ bool rc = false;
+
+ spin_lock_bh(&q->bpoll_lock);
+ if (q->bpoll_state & CXGB_POLL_STATE_POLL_YIELD)
+ rc = true;
+ q->bpoll_state = CXGB_POLL_STATE_IDLE;
+ spin_unlock_bh(&q->bpoll_lock);
+ return rc;
+}
+
+static inline bool cxgb_poll_busy_polling(struct sge_rspq *q)
+{
+ return q->bpoll_state & CXGB_POLL_USER_PEND;
+}
+#else
+static inline void cxgb_busy_poll_init_lock(struct sge_rspq *q)
+{
+}
+
+static inline bool cxgb_poll_lock_napi(struct sge_rspq *q)
+{
+ return true;
+}
+
+static inline bool cxgb_poll_unlock_napi(struct sge_rspq *q)
+{
+ return false;
+}
+
+static inline bool cxgb_poll_lock_poll(struct sge_rspq *q)
+{
+ return false;
+}
+
+static inline bool cxgb_poll_unlock_poll(struct sge_rspq *q)
+{
+ return false;
+}
+
+static inline bool cxgb_poll_busy_polling(struct sge_rspq *q)
+{
+ return false;
+}
+#endif /* CONFIG_NET_RX_BUSY_POLL */
+
void t4_os_portmod_changed(const struct adapter *adap, int port_id);
void t4_os_link_changed(struct adapter *adap, int port_id, int link_stat);
@@ -908,6 +1020,7 @@ irqreturn_t t4_sge_intr_msix(int irq, void *cookie);
int t4_sge_init(struct adapter *adap);
void t4_sge_start(struct adapter *adap);
void t4_sge_stop(struct adapter *adap);
+int cxgb_busy_poll(struct napi_struct *napi);
extern int dbfifo_int_thresh;
#define for_each_port(adapter, iter) \
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 5bf490a..041742b 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -923,8 +923,14 @@ static void quiesce_rx(struct adapter *adap)
for (i = 0; i < ARRAY_SIZE(adap->sge.ingr_map); i++) {
struct sge_rspq *q = adap->sge.ingr_map[i];
- if (q && q->handler)
+ if (q && q->handler) {
napi_disable(&q->napi);
+ local_bh_disable();
+ while (!cxgb_poll_lock_napi(q))
+ mdelay(1);
+ local_bh_enable();
+ }
+
}
}
@@ -940,8 +946,10 @@ static void enable_rx(struct adapter *adap)
if (!q)
continue;
- if (q->handler)
+ if (q->handler) {
+ cxgb_busy_poll_init_lock(q);
napi_enable(&q->napi);
+ }
/* 0-increment GTS to start the timer and enable interrupts */
t4_write_reg(adap, MYPF_REG(SGE_PF_GTS_A),
SEINTARM_V(q->intr_params) |
@@ -4563,6 +4571,10 @@ static const struct net_device_ops cxgb4_netdev_ops = {
#ifdef CONFIG_NET_POLL_CONTROLLER
.ndo_poll_controller = cxgb_netpoll,
#endif
+#ifdef CONFIG_NET_RX_BUSY_POLL
+ .ndo_busy_poll = cxgb_busy_poll,
+#endif
+
};
void t4_fatal_err(struct adapter *adap)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/sge.c b/drivers/net/ethernet/chelsio/cxgb4/sge.c
index 6191561..7b1a140 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/sge.c
@@ -43,6 +43,9 @@
#include <linux/export.h>
#include <net/ipv6.h>
#include <net/tcp.h>
+#ifdef CONFIG_NET_RX_BUSY_POLL
+#include <net/busy_poll.h>
+#endif /* CONFIG_NET_RX_BUSY_POLL */
#include "cxgb4.h"
#include "t4_regs.h"
#include "t4_values.h"
@@ -1720,6 +1723,7 @@ static void do_gro(struct sge_eth_rxq *rxq, const struct pkt_gl *gl,
skb->truesize += skb->data_len;
skb->ip_summed = CHECKSUM_UNNECESSARY;
skb_record_rx_queue(skb, rxq->rspq.idx);
+ skb_mark_napi_id(skb, &rxq->rspq.napi);
if (rxq->rspq.netdev->features & NETIF_F_RXHASH)
skb_set_hash(skb, (__force u32)pkt->rsshdr.hash_val,
PKT_HASH_TYPE_L3);
@@ -1763,6 +1767,7 @@ int t4_ethrx_handler(struct sge_rspq *q, const __be64 *rsp,
csum_ok = pkt->csum_calc && !pkt->err_vec &&
(q->netdev->features & NETIF_F_RXCSUM);
if ((pkt->l2info & htonl(RXF_TCP_F)) &&
+ !(cxgb_poll_busy_polling(q)) &&
(q->netdev->features & NETIF_F_GRO) && csum_ok && !pkt->ip_frag) {
do_gro(rxq, si, pkt);
return 0;
@@ -1801,6 +1806,7 @@ int t4_ethrx_handler(struct sge_rspq *q, const __be64 *rsp,
__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), ntohs(pkt->vlan));
rxq->stats.vlan_ex++;
}
+ skb_mark_napi_id(skb, &q->napi);
netif_receive_skb(skb);
return 0;
}
@@ -1963,6 +1969,38 @@ static int process_responses(struct sge_rspq *q, int budget)
return budget - budget_left;
}
+#ifdef CONFIG_NET_RX_BUSY_POLL
+int cxgb_busy_poll(struct napi_struct *napi)
+{
+ struct sge_rspq *q = container_of(napi, struct sge_rspq, napi);
+ unsigned int params, work_done;
+ u32 val;
+
+ if (!cxgb_poll_lock_poll(q))
+ return LL_FLUSH_BUSY;
+
+ work_done = process_responses(q, 4);
+ params = QINTR_TIMER_IDX(TIMERREG_COUNTER0_X) | QINTR_CNT_EN;
+ q->next_intr_params = params;
+ val = CIDXINC_V(work_done) | SEINTARM_V(params);
+
+ /* If we don't have access to the new User GTS (T5+), use the old
+ * doorbell mechanism; otherwise use the new BAR2 mechanism.
+ */
+ if (unlikely(!q->bar2_addr))
+ t4_write_reg(q->adap, MYPF_REG(SGE_PF_GTS_A),
+ val | INGRESSQID_V((u32)q->cntxt_id));
+ else {
+ writel(val | INGRESSQID_V(q->bar2_qid),
+ q->bar2_addr + SGE_UDB_GTS);
+ wmb();
+ }
+
+ cxgb_poll_unlock_poll(q);
+ return work_done;
+}
+#endif /* CONFIG_NET_RX_BUSY_POLL */
+
/**
* napi_rx_handler - the NAPI handler for Rx processing
* @napi: the napi instance
@@ -1978,9 +2016,13 @@ static int napi_rx_handler(struct napi_struct *napi, int budget)
{
unsigned int params;
struct sge_rspq *q = container_of(napi, struct sge_rspq, napi);
- int work_done = process_responses(q, budget);
+ int work_done = 0;
u32 val;
+ if (!cxgb_poll_lock_napi(q))
+ return work_done;
+
+ work_done = process_responses(q, budget);
if (likely(work_done < budget)) {
int timer_index;
@@ -2018,6 +2060,7 @@ static int napi_rx_handler(struct napi_struct *napi, int budget)
q->bar2_addr + SGE_UDB_GTS);
wmb();
}
+ cxgb_poll_unlock_napi(q);
return work_done;
}
@@ -2341,6 +2384,7 @@ int t4_sge_alloc_rxq(struct adapter *adap, struct sge_rspq *iq, bool fwevtq,
goto err;
netif_napi_add(dev, &iq->napi, napi_rx_handler, 64);
+ napi_hash_add(&iq->napi);
iq->cur_desc = iq->desc;
iq->cidx = 0;
iq->gen = 1;
@@ -2598,6 +2642,7 @@ static void free_rspq_fl(struct adapter *adap, struct sge_rspq *rq,
rq->cntxt_id, fl_id, 0xffff);
dma_free_coherent(adap->pdev_dev, (rq->size + 1) * rq->iqe_len,
rq->desc, rq->phys_addr);
+ napi_hash_del(&rq->napi);
netif_napi_del(&rq->napi);
rq->netdev = NULL;
rq->cntxt_id = rq->abs_id = 0;
diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4_values.h b/drivers/net/ethernet/chelsio/cxgb4/t4_values.h
index a404844..997ec87 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/t4_values.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/t4_values.h
@@ -59,6 +59,7 @@
/* GTS register */
#define SGE_TIMERREGS 6
+#define TIMERREG_COUNTER0_X 0
/* T5 and later support a new BAR2-based doorbell mechanism for Egress Queues.
* The User Doorbells are each 128 bytes in length with a Simple Doorbell at
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] net: rocker: Change netdev names to include slot number
From: Scott Feldman @ 2015-02-03 5:46 UTC (permalink / raw)
To: David Ahern; +Cc: Florian Fainelli, Netdev
In-Reply-To: <54CFF5B3.7070509@gmail.com>
On Mon, Feb 2, 2015 at 2:09 PM, David Ahern <dsahern@gmail.com> wrote:
> On 2/2/15 3:03 PM, Florian Fainelli wrote:
>>
>> Why not? virtio and rocker interfaces are backed by different devices
>> drivers which should allow you to use that to name interfaces
>> differently. In the case of rocker, you would probably want to read the
>> phys_port_id sysfs attribute to name them after their parent switch id
>> too.
>
>
> [root@f21 ~]# cat /sys/devices/virtual/net/sw5p0/phys_port_id
> cat: /sys/devices/virtual/net/sw5p0/phys_port_id: Operation not supported
We should implement .ndo_get_phys_port_id in rocker to return a unique
name for the port, which can then be used in udev script to name
interface. When rocker device is instantiated, a unique switch name
is given on qemu cmd line, for example "sw1". Rocker device could
return that string to the driver, or even combine that string with the
port index to have "sw1p1". The second instantiated rocker device
would give up "sw2px" port names.
A real switch with a physical port than can be split, or when multiple
ports are ganged to form one mega port, letting the device pick a
unique name is probably best. For example, splitting a single
physical 40Gb port into 4 10Gb ports would present 4 kernel
interfaces, but we probably want to use the base port in the naming,
so we'd have something like "sw1p1s1" to mean switch 1, front panel
port 1, split port 1. Letting the device name the ports on current
port configuration of the port will keep driver/kernel simple, and let
udev be the final name chooser.
>
> But in general I guess need to find time to figure out systemd-udev files.
>
> David
^ permalink raw reply
* Re: [PATCHv2 net] net: restore lro after device detached from bridge
From: Michal Kubecek @ 2015-02-03 6:54 UTC (permalink / raw)
To: Fan Du; +Cc: Alexander Duyck, Fan Du, bhutchings, davem, netdev
In-Reply-To: <54D0326C.2050704@gmail.com>
On Tue, Feb 03, 2015 at 10:29:00AM +0800, Fan Du wrote:
> 于 2015年02月02日 19:15, Michal Kubecek 写道:
> >On Mon, Feb 02, 2015 at 10:20:12AM +0800, Fan Du wrote:
> >
> >>Note, SRC is defaulted to *ON* in practice for ALL ixgbe NICs, as same
> >>other RSC capable NICs.
> >
> >A very bad idea, IMHO. A lot of bug reports resulted from it.
>
> Why are you saying this an idea?? this a fact for all RSC capable NIC
> drivers. search drivers/net/ethernet/ to find more.
I didn't say it's not turned on by default, I just said I consider this
a bad idea. Why? Because the feature is known to break network
communication in common and frequently used scenarios (essentially
whenever received packets may leave the host). When this happens, you
observe strange networking malfunction and unless you know this may be
the cause (or read release notes very carefully), it's very difficult to
identify the cause. Personally, I handled four bug reports of this type
in last three years.
On the other hand, having it turned off when it could be on is only a
performance problem and the communication works. When you are tuning the
performance, you obviously look at the offloading features and check
which could be turned on.
This disbalance (broken communication on one side and slight performance
difference on the other) is why I believe feature like this should not
be turned on by default. Unfortunately it is and we have to deal with
it; that's why dev_disable_lro() is called in certain situations and why
it is propagated down lo lower devices. Turning it back on without
carefully checking that _none_ of the reasons to have it off exists
would do more harm than good.
Michal Kubecek
^ permalink raw reply
* Re: [PATCHv2 net] net: restore lro after device detached from bridge
From: Fan Du @ 2015-02-03 7:08 UTC (permalink / raw)
To: alexander.h.duyck; +Cc: Fan Du, bhutchings, davem, netdev
In-Reply-To: <54CF52D6.90701@redhat.com>
于 2015年02月02日 18:35, Alexander Duyck 写道:
> On 02/01/2015 06:20 PM, Fan Du wrote:
>> 于 2015年01月31日 04:48, Alexander Duyck 写道:
>>> On 01/30/2015 04:33 AM, Fan Du wrote:
>>>> Either detaching a device from bridge or switching a device
>>>> out of FORWARDING state, the original lro feature should
>>>> possibly be enabled for good reason, e.g. hw feature like
>>>> receive side coalescing could come into play.
>>>>
>>>> BEFORE:
>>>> echo 1 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k ens806f0 | grep large
>>>> large-receive-offload: off
>>>>
>>>> echo 0 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k ens806f0 | grep large
>>>> large-receive-offload: off
>>>>
>>>> AFTER:
>>>> echo 1 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k ens806f0 | grep large
>>>> large-receive-offload: off
>>>>
>>>> echo 0 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k ens806f0 | grep large
>>>> large-receive-offload: on
>>>>
>>>> Signed-off-by: Fan Du <fan.du@intel.com>
>>>> Fixes: 0187bdfb0567 ("net: Disable LRO on devices that are forwarding")
>>>
>>
>>> First off this isn't a "fix". This is going to likely break more than
>>> it fixes. The main reason why LRO is disabled is because it can cause
>>> more harm then it helps. Since GRO is available we should err on the
>>> side of caution since enabling LRO/RSC can have undesirable side effects
>>> in a number of cases.
>>
>> I think you are talking about bad scenarios when net device is attached to a bridge.
>> Then what's the good reason user has to pay extra cpu power for using GRO, instead
>> of using hw capable LRO/RSC when this net device is detached from bridge acting as
>> a standalone NIC?
>>
>> Note, SRC is defaulted to *ON* in practice for ALL ixgbe NICs, as same other RSC capable
>> NICs. Attaching net device to a bridge _once_ should not changed its default configuration,
>> moreover it's a subtle change without any message that user won't noticed at all.
> No, RSC only has benefits for IPv4/TCP large packets. However
> historically there have been issues seen w/ small packet performance
> with RSC enabled.
Only when parallel client exceeds 4, gro trumps lro performance on my testbed for small packets.
The difference comes from the fact that TCP RSS hash flows from clients into different NIC queues
for multiple cpu, while RSC engine inside NIC has limit resource compared with that of cpu used by gro.
NICs: 82599EB
server:ipserf -s -B 192.168.5.1
client:iperf -c 192.168.5.1 -i 1 -M 100 -P x
-P Bandwidth/lro on Bandwidth/lro off
gro off gro on
1 2.31 Gbits/sec 947 Mbits/sec
2 3.09 Gbits/sec 1.97 Gbits/sec
3 3.19 Gbits/sec 2.70 Gbits/sec
4 3.16 Gbits/sec 3.39 Gbits/sec
5 3.23 Gbits/sec 3.33 Gbits/sec
6 3.19 Gbits/sec 3.74 Gbits/sec
7 3.18 Gbits/sec 3.88 Gbits/sec
8 3.17 Gbits/sec 3.24 Gbits/sec
9 3.16 Gbits/sec 3.70 Gbits/sec
10 3.15 Gbits/sec 3.76 Gbits/sec
11 3.10 Gbits/sec 4.03 Gbits/sec
12 3.11 Gbits/sec 3.13 Gbits/sec
13 3.12 Gbits/sec 4.12 Gbits/sec
14 3.07 Gbits/sec 4.04 Gbits/sec
15 3.03 Gbits/sec 3.14 Gbits/sec
16 2.99 Gbits/sec 3.93 Gbits/sec
Some have been addressed, however there are still
> other effects such as increasing latency for receive unless the push
> flag is set in the frame.
>
> I still say this patch is not valid, even with your changes. Your
> performance gain doesn't trump the regressions you would be causing on
> other peoples platforms.
>
> I would suggest figuring out why you are seeing issues with routing or
> bridging being enabled and disabled and possibly cleaning up the issue
> via a script rather than trying to modify the kernel to make it take
> care of it for you.
> - Alex
^ permalink raw reply
* [PATCH v3] gianfar: correct the bad expression while writing bit-pattern
From: Sanjeev Sharma @ 2015-02-03 7:32 UTC (permalink / raw)
To: claudiu.manoil
Cc: davem, matei.pavaluca, netdev, linux-kernel, Sanjeev Sharma,
Sanjeev Sharma
In-Reply-To: <54B3D899.5060204@freescale.com>
This patch correct the bad expression while writing the
bit-pattern from software's buffer to hardware registers.
Signed-off-by: Sanjeev Sharma <Sanjeev_Sharma@mentor.com>
---
Changes in v3:
- corrected the expression by introducing tab->fe[i].prop.
drivers/net/ethernet/freescale/gianfar_ethtool.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/gianfar_ethtool.c b/drivers/net/ethernet/freescale/gianfar_ethtool.c
index 3e1a9c1..fda12fb 100644
--- a/drivers/net/ethernet/freescale/gianfar_ethtool.c
+++ b/drivers/net/ethernet/freescale/gianfar_ethtool.c
@@ -1586,7 +1586,7 @@ static int gfar_write_filer_table(struct gfar_private *priv,
return -EBUSY;
/* Fill regular entries */
- for (; i < MAX_FILER_IDX - 1 && (tab->fe[i].ctrl | tab->fe[i].ctrl);
+ for (; i < MAX_FILER_IDX - 1 && (tab->fe[i].ctrl | tab->fe[i].prop);
i++)
gfar_write_filer(priv, i, tab->fe[i].ctrl, tab->fe[i].prop);
/* Fill the rest with fall-troughs */
--
1.7.11.7
^ permalink raw reply related
* RE: [PATCH] net: rocker: Change netdev names to include slot number
From: Rosen, Rami @ 2015-02-03 7:33 UTC (permalink / raw)
To: David Ahern, Florian Fainelli, Scott Feldman; +Cc: Netdev
In-Reply-To: <54CFF5B3.7070509@gmail.com>
Hi,
>root@f21 ~]# cat /sys/devices/virtual/net/sw5p0/phys_port_id
>cat: /sys/devices/virtual/net/sw5p0/phys_port_id: Operation not supported
Just a side note: commit aecbe01e7410 "net-sysfs: expose physical switch id for particular device" added support for this entry, relying on the fact that the ndo_switch_parent_id_get() is implemented (which seems to be a valid assumption for switch devices) and that the dev_isalive() condition is met.
Could it be that the reason for getting -EOPNOTSUPP was that the ndo_switch_parent_id_get() is not implemented in your switch device or that the condition was not met in your test?
Rami Rosen
Intel Corporation
^ permalink raw reply
* pull request: bluetooth-next 2015-02-03
From: Johan Hedberg @ 2015-02-03 8:21 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-bluetooth
[-- Attachment #1: Type: text/plain, Size: 5731 bytes --]
Hi Dave,
Here's what's likely the last bluetooth-next pull request for 3.20.
Notable changes include:
- xHCI workaround + a new id for the ath3k driver
- Several new ids for the btusb driver
- Support for new Intel Bluetooth controllers
- Minor cleanups to ieee802154 code
- Nested sleep warning fix in socket accept() code path
- Fixes for Out of Band pairing handling
- Support for LE scan restarting for HCI_QUIRK_STRICT_DUPLICATE_FILTER
- Improvements to data we expose through debugfs
- Proper handling of Hardware Error HCI events
Please let me know if there are any issues pulling. Thanks.
Johan
---
The following changes since commit 0c49087462e8587c12ecfeaf1dd46fdc0ddc4532:
Merge tag 'mac80211-next-for-davem-2015-01-19' of git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next (2015-01-19 16:22:19 -0500)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git for-upstream
for you to fetch changes up to 88d9077c27d0c1a7c022d9dc987640beecf23560:
Bluetooth: Fix potential NULL dereference (2015-02-03 09:02:12 +0100)
----------------------------------------------------------------
Adam Lee (1):
Bluetooth: ath3k: workaround the compatibility issue with xHCI controller
Dmitry Tunin (1):
Bluetooth: ath3k: Add support of AR3012 bluetooth 13d3:3423 device
Jakub Pawlowski (4):
Bluetooth: Set HCI_QUIRK_STRICT_DUPLICATE_FILTER for BTUSB_ATH3012
Bluetooth: Set HCI_QUIRK_STRICT_DUPLICATE_FILTER for BTUSB_INTEL
Bluetooth: Add le_scan_restart work for LE scan restarting
Bluetooth: Add restarting to service discovery
Johan Hedberg (12):
Bluetooth: Check for valid bdaddr in add_remote_oob_data
Bluetooth: Remove incorrect check for BDADDR_BREDR address type
Bluetooth: Convert Set SC to use HCI Request
Bluetooth: Enforce zero-valued hash/rand192 for LE OOB
Bluetooth: btusb: Remove redundant call to btusb_free_frags()
Bluetooth: Fix check for SSP when enabling SC
Bluetooth: Fix notifying discovery state upon reset
Bluetooth: Fix notifying discovery state when powering off
Bluetooth: btusb: Fix race when waiting for BTUSB_DOWNLOADING
Bluetooth: btusb: Use wait_on_bit_timeout() for BTUSB_BOOTING
Bluetooth: Remove mgmt_rp_read_local_oob_ext_data struct
Bluetooth: Fix potential NULL dereference
Marcel Holtmann (28):
Bluetooth: Fix dependency for BR/EDR Secure Connections mode on SSP
Bluetooth: Limit BR/EDR switching for LE only with secure connections
Bluetooth: Require SSP enabling before BR/EDR Secure Connections
Bluetooth: btusb: Add support for Dynex/Insignia USB dongles
Bluetooth: btusb: Add firmware loading for Intel Snowfield Peak devices
Bluetooth: Clear P-192 values for OOB when in Secure Connections Only mode
Bluetooth: Use helper function to determine BR/EDR OOB data present
Bluetooth: Check for P-256 OOB values in Secure Connections Only mode
Bluetooth: btusb: Handle out of order firmware loading complete event
Bluetooth: Introduce hci_dev_do_reset helper function
Bluetooth: Perform a power cycle when receiving hardware error event
Bluetooth: btusb: Provide hardware error handler for Intel devices
Bluetooth: Move smp_unregister() into hci_dev_do_close() function
Bluetooth: btusb: Sort USB_DEVICE entries for Marvell by vendor id
Bluetooth: btusb: Ignore unknown Intel devices with generic descriptor
Bluetooth: btusb: Add support for USB based AMP controllers
Bluetooth: btusb: Limit hardware error handling to Intel Snowfield Peak
Bluetooth: Store OOB data present value for each set of remote OOB data
Bluetooth: Fix OOB data present value for BR/EDR Secure Connections
Bluetooth: Fix OOB data present value for SMP pairing
Bluetooth: Allow remote OOB data to only provide P-192 or P-256 values
Bluetooth: Expose Secure Simple Pairing debug mode setting in debugfs
Bluetooth: Track changes from HCI Write Simple Pairing Debug Mode command
Bluetooth: Expose debug keys usage setting via debugfs
Bluetooth: Expose hardware error code as debugfs entry
Bluetooth: Expose remote OOB information as debugfs entry
Bluetooth: Fix OOB data present for BR/EDR Secure Connections Only mode
Bluetooth: Set HCI_QUIRK_STRICT_DUPLICATE_FILTER for BTUSB_INTEL_NEW
Matej Dubovy (1):
Bluetooth: btusb: Add support for Lite-On (04ca) Broadcom based, BCM43142
Mohammad Jamal (2):
ieee802154: cc2520: Replace shift operations by BIT macro
ieee802154: cc2520: Fix space before , coding style issue
Peter Hurley (1):
Bluetooth: Fix nested sleeps
Rick Dunn (1):
Bluetooth: btusb: Add Broadcom patchram support for ASUSTek devices
Szymon Janc (2):
Bluetooth: Fix reporting invalid RSSI for LE devices
Bluetooth: Fix sending Read Remote Extended Features command
drivers/bluetooth/ath3k.c | 10 +
drivers/bluetooth/btusb.c | 671 +++++++++++++++++++++++++++++++++++++-
drivers/net/ieee802154/cc2520.c | 10 +-
include/net/bluetooth/hci_core.h | 11 +-
include/net/bluetooth/mgmt.h | 4 -
net/bluetooth/bnep/core.c | 7 +-
net/bluetooth/hci_core.c | 169 ++++++++--
net/bluetooth/hci_debugfs.c | 79 ++++-
net/bluetooth/hci_event.c | 135 ++++++--
net/bluetooth/l2cap_sock.c | 9 +-
net/bluetooth/mgmt.c | 279 +++++++++++-----
net/bluetooth/rfcomm/sock.c | 9 +-
net/bluetooth/sco.c | 8 +-
net/bluetooth/smp.c | 2 +-
14 files changed, 1214 insertions(+), 189 deletions(-)
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCHv2 net] net: restore lro after device detached from bridge
From: Alexander Duyck @ 2015-02-03 8:37 UTC (permalink / raw)
To: Fan Du; +Cc: Fan Du, bhutchings, davem, netdev
In-Reply-To: <54D0740A.6050701@gmail.com>
On 02/03/2015 08:08 AM, Fan Du wrote:
> 于 2015年02月02日 18:35, Alexander Duyck 写道:
>> On 02/01/2015 06:20 PM, Fan Du wrote:
>>> 于 2015年01月31日 04:48, Alexander Duyck 写道:
>>>> On 01/30/2015 04:33 AM, Fan Du wrote:
>>>>> Either detaching a device from bridge or switching a device
>>>>> out of FORWARDING state, the original lro feature should
>>>>> possibly be enabled for good reason, e.g. hw feature like
>>>>> receive side coalescing could come into play.
>>>>>
>>>>> BEFORE:
>>>>> echo 1 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k
>>>>> ens806f0 | grep large
>>>>> large-receive-offload: off
>>>>>
>>>>> echo 0 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k
>>>>> ens806f0 | grep large
>>>>> large-receive-offload: off
>>>>>
>>>>> AFTER:
>>>>> echo 1 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k
>>>>> ens806f0 | grep large
>>>>> large-receive-offload: off
>>>>>
>>>>> echo 0 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k
>>>>> ens806f0 | grep large
>>>>> large-receive-offload: on
>>>>>
>>>>> Signed-off-by: Fan Du <fan.du@intel.com>
>>>>> Fixes: 0187bdfb0567 ("net: Disable LRO on devices that are
>>>>> forwarding")
>>>>
>>>
>>>> First off this isn't a "fix". This is going to likely break more than
>>>> it fixes. The main reason why LRO is disabled is because it can cause
>>>> more harm then it helps. Since GRO is available we should err on the
>>>> side of caution since enabling LRO/RSC can have undesirable side
>>>> effects
>>>> in a number of cases.
>>>
>>> I think you are talking about bad scenarios when net device is
>>> attached to a bridge.
>>> Then what's the good reason user has to pay extra cpu power for
>>> using GRO, instead
>>> of using hw capable LRO/RSC when this net device is detached from
>>> bridge acting as
>>> a standalone NIC?
>>>
>>> Note, SRC is defaulted to *ON* in practice for ALL ixgbe NICs, as
>>> same other RSC capable
>>> NICs. Attaching net device to a bridge _once_ should not changed its
>>> default configuration,
>>> moreover it's a subtle change without any message that user won't
>>> noticed at all.
>
>> No, RSC only has benefits for IPv4/TCP large packets. However
>> historically there have been issues seen w/ small packet performance
>> with RSC enabled.
>
> Only when parallel client exceeds 4, gro trumps lro performance on my
> testbed for small packets.
> The difference comes from the fact that TCP RSS hash flows from
> clients into different NIC queues
> for multiple cpu, while RSC engine inside NIC has limit resource
> compared with that of cpu used by gro.
>
> NICs: 82599EB
> server:ipserf -s -B 192.168.5.1
> client:iperf -c 192.168.5.1 -i 1 -M 100 -P x
>
> -P Bandwidth/lro on Bandwidth/lro off
> gro off gro on
>
> 1 2.31 Gbits/sec 947 Mbits/sec
> 2 3.09 Gbits/sec 1.97 Gbits/sec
> 3 3.19 Gbits/sec 2.70 Gbits/sec
> 4 3.16 Gbits/sec 3.39 Gbits/sec
> 5 3.23 Gbits/sec 3.33 Gbits/sec
> 6 3.19 Gbits/sec 3.74 Gbits/sec
> 7 3.18 Gbits/sec 3.88 Gbits/sec
> 8 3.17 Gbits/sec 3.24 Gbits/sec
> 9 3.16 Gbits/sec 3.70 Gbits/sec
> 10 3.15 Gbits/sec 3.76 Gbits/sec
> 11 3.10 Gbits/sec 4.03 Gbits/sec
> 12 3.11 Gbits/sec 3.13 Gbits/sec
> 13 3.12 Gbits/sec 4.12 Gbits/sec
> 14 3.07 Gbits/sec 4.04 Gbits/sec
> 15 3.03 Gbits/sec 3.14 Gbits/sec
> 16 2.99 Gbits/sec 3.93 Gbits/sec
>
>
>
>
> Some have been addressed, however there are still
The point I think you are not getting is that bulk throughput
performance does not justify enabling a feature that may impact
stability or possibly harm small packet performance.
There are more reasons than routing and bridging to disable LRO. Those
two reasons though were so bad that we couldn't allow end users to
possibly encounter them so we disabled the feature for them.
There are a number of other cases where LRO might be disabled as in the
possible latency case I reported. As such you should not be enabling
LRO just because only two of the possible issues have now been addressed.
It is best to leave this up to the end-user to re-enable. If you are
seeing the feature disabled as a result of some init script on the
system you may want to look at re-enabling it as a part of some other
init script that you use when disabling routing or bridging.
- Alex
^ permalink raw reply
* 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
* [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
* [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
* 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
* 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: [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
* [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 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
* 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
* [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: 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
* 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
* [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 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
* 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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox