* Re: [PATCH net-next 2/4] net: allow > 0 order atomic page alloc in skb_page_frag_refill
From: Eric Dumazet @ 2013-11-12 22:42 UTC (permalink / raw)
To: Michael Dalton
Cc: Michael S. Tsirkin, netdev, Daniel Borkmann, virtualization,
Eric Dumazet, David S. Miller
In-Reply-To: <1384294885-6444-2-git-send-email-mwdalton@google.com>
On Tue, 2013-11-12 at 14:21 -0800, Michael Dalton wrote:
> skb_page_frag_refill currently permits only order-0 page allocs
> unless GFP_WAIT is used. Change skb_page_frag_refill to attempt
> higher-order page allocations whether or not GFP_WAIT is used. If
> memory cannot be allocated, the allocator will fall back to
> successively smaller page allocs (down to order-0 page allocs).
>
> This change brings skb_page_frag_refill in line with the existing
> page allocation strategy employed by netdev_alloc_frag, which attempts
> higher-order page allocations whether or not GFP_WAIT is set, falling
> back to successively lower-order page allocations on failure. Part
> of migration of virtio-net to per-receive queue page frag allocators.
>
> Signed-off-by: Michael Dalton <mwdalton@google.com>
> ---
> net/core/sock.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [PATCH net-next 1/4] virtio-net: mergeable buffer size should include virtio-net header
From: Eric Dumazet @ 2013-11-12 22:41 UTC (permalink / raw)
To: Michael Dalton
Cc: Michael S. Tsirkin, netdev, Daniel Borkmann, virtualization,
Eric Dumazet, David S. Miller
In-Reply-To: <1384294885-6444-1-git-send-email-mwdalton@google.com>
On Tue, 2013-11-12 at 14:21 -0800, Michael Dalton wrote:
> Commit 2613af0ed18a ("virtio_net: migrate mergeable rx buffers to page
> frag allocators") changed the mergeable receive buffer size from PAGE_SIZE
> to MTU-size. However, the merge buffer size does not take into account the
> size of the virtio-net header. Consequently, packets that are MTU-size
> will take two buffers intead of one (to store the virtio-net header),
> substantially decreasing the throughput of MTU-size traffic due to TCP
> window / SKB truesize effects.
>
> This commit changes the mergeable buffer size to include the virtio-net
> header. The buffer size is cacheline-aligned because skb_page_frag_refill
> will not automatically align the requested size.
>
> Benchmarks taken from an average of 5 netperf 30-second TCP_STREAM runs
> between two QEMU VMs on a single physical machine. Each VM has two VCPUs and
> vhost enabled. All VMs and vhost threads run in a single 4 CPU cgroup
> cpuset, using cgroups to ensure that other processes in the system will not
> be scheduled on the benchmark CPUs. Transmit offloads and mergeable receive
> buffers are enabled, but guest_tso4 / guest_csum are explicitly disabled to
> force MTU-sized packets on the receiver.
>
> next-net trunk before 2613af0ed18a (PAGE_SIZE buf): 3861.08Gb/s
> net-next trunk (MTU 1500- packet uses two buf due to size bug): 4076.62Gb/s
> net-next trunk (MTU 1480- packet fits in one buf): 6301.34Gb/s
> net-next trunk w/ size fix (MTU 1500 - packet fits in one buf): 6445.44Gb/s
>
> Suggested-by: Eric Northup <digitaleric@google.com>
> Signed-off-by: Michael Dalton <mwdalton@google.com>
> ---
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* [PATCH v2 net] core/dev: do not ignore dmac in dev_forward_skb()
From: Alexei Starovoitov @ 2013-11-12 22:39 UTC (permalink / raw)
To: David S. Miller
Cc: Eric Dumazet, netdev, Isaku Yamahata, Maciej Zenczykowski,
Nicolas Dichtel
commit 06a23fe31ca3
("core/dev: set pkt_type after eth_type_trans() in dev_forward_skb()")
and refactoring 64261f230a91
("dev: move skb_scrub_packet() after eth_type_trans()")
are forcing pkt_type to be PACKET_HOST when skb traverses veth.
which means that ip forwarding will kick in inside netns
even if skb->eth->h_dest != dev->dev_addr
Fix order of eth_type_trans() and skb_scrub_packet() in dev_forward_skb()
and in ip_tunnel_rcv()
Fixes: 06a23fe31ca3 ("core/dev: set pkt_type after eth_type_trans() in dev_forward_skb()")
CC: Isaku Yamahata <yamahatanetdev@gmail.com>
CC: Maciej Zenczykowski <zenczykowski@gmail.com>
CC: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
v2: fixing order in ip_tunnel_rcv() as suggested by Nicolas
Isaku's test still works fine.
not touching ipv6 since I don't have testbed for it
net/core/dev.c | 6 +-----
net/ipv4/ip_tunnel.c | 4 ++--
2 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 3430b1e..2afc521 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1691,13 +1691,9 @@ int dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
kfree_skb(skb);
return NET_RX_DROP;
}
- skb->protocol = eth_type_trans(skb, dev);
- /* eth_type_trans() can set pkt_type.
- * call skb_scrub_packet() after it to clear pkt_type _after_ calling
- * eth_type_trans().
- */
skb_scrub_packet(skb, true);
+ skb->protocol = eth_type_trans(skb, dev);
return netif_rx(skb);
}
diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index 63a6d6d..254f11c 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -454,6 +454,8 @@ int ip_tunnel_rcv(struct ip_tunnel *tunnel, struct sk_buff *skb,
tstats->rx_bytes += skb->len;
u64_stats_update_end(&tstats->syncp);
+ skb_scrub_packet(skb, !net_eq(tunnel->net, dev_net(tunnel->dev)));
+
if (tunnel->dev->type == ARPHRD_ETHER) {
skb->protocol = eth_type_trans(skb, tunnel->dev);
skb_postpull_rcsum(skb, eth_hdr(skb), ETH_HLEN);
@@ -461,8 +463,6 @@ int ip_tunnel_rcv(struct ip_tunnel *tunnel, struct sk_buff *skb,
skb->dev = tunnel->dev;
}
- skb_scrub_packet(skb, !net_eq(tunnel->net, dev_net(tunnel->dev)));
-
gro_cells_receive(&tunnel->gro_cells, skb);
return 0;
--
1.7.9.5
^ permalink raw reply related
* [PATCH iproute2] htb: support 64bit rates
From: Eric Dumazet @ 2013-11-12 22:34 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
From: Eric Dumazet <edumazet@google.com>
Starting from linux-3.13, we can break the 32bit limitation of
rates on HTB qdisc/classes.
Prior limit was 34.359.738.360 bits per second.
lpq83:~# tc -s qdisc show dev lo ; tc -s class show dev lo
qdisc htb 1: root refcnt 2 r2q 2000 default 1 direct_packets_stat 0 direct_qlen 6000
Sent 6591936144493 bytes 149549182 pkt (dropped 0, overlimits 213757419 requeues 0)
rate 39464Mbit 114938pps backlog 0b 15p requeues 0
class htb 1:1 root prio 0 rate 50000Mbit ceil 50000Mbit burst 200000b cburst 0b
Sent 6591942184547 bytes 149549310 pkt (dropped 0, overlimits 0 requeues 0)
rate 39464Mbit 114938pps backlog 0b 15p requeues 0
lended: 149549310 borrowed: 0 giants: 0
tokens: 336 ctokens: -164
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
tc/q_htb.c | 56 ++++++++++++++++++++++++++++++++++++-------------
tc/tc_core.c | 6 ++---
tc/tc_core.h | 4 +--
tc/tc_util.c | 27 ++++++++++++++++++++++-
tc/tc_util.h | 1
5 files changed, 74 insertions(+), 20 deletions(-)
diff --git a/tc/q_htb.c b/tc/q_htb.c
index e108857..1d8c56f 100644
--- a/tc/q_htb.c
+++ b/tc/q_htb.c
@@ -113,6 +113,7 @@ static int htb_parse_class_opt(struct qdisc_util *qu, int argc, char **argv, str
unsigned int direct_qlen = ~0U;
unsigned int linklayer = LINKLAYER_ETHERNET; /* Assume ethernet */
struct rtattr *tail;
+ __u64 ceil64 = 0, rate64 = 0;
memset(&opt, 0, sizeof(opt)); mtu = 1600; /* eth packet len */
@@ -173,22 +174,22 @@ static int htb_parse_class_opt(struct qdisc_util *qu, int argc, char **argv, str
ok++;
} else if (strcmp(*argv, "ceil") == 0) {
NEXT_ARG();
- if (opt.ceil.rate) {
+ if (ceil64) {
fprintf(stderr, "Double \"ceil\" spec\n");
return -1;
}
- if (get_rate(&opt.ceil.rate, *argv)) {
+ if (get_rate64(&ceil64, *argv)) {
explain1("ceil");
return -1;
}
ok++;
} else if (strcmp(*argv, "rate") == 0) {
NEXT_ARG();
- if (opt.rate.rate) {
+ if (rate64) {
fprintf(stderr, "Double \"rate\" spec\n");
return -1;
}
- if (get_rate(&opt.rate.rate, *argv)) {
+ if (get_rate64(&rate64, *argv)) {
explain1("rate");
return -1;
}
@@ -207,17 +208,23 @@ static int htb_parse_class_opt(struct qdisc_util *qu, int argc, char **argv, str
/* if (!ok)
return 0;*/
- if (opt.rate.rate == 0) {
+ if (!rate64) {
fprintf(stderr, "\"rate\" is required.\n");
return -1;
}
/* if ceil params are missing, use the same as rate */
- if (!opt.ceil.rate) opt.ceil = opt.rate;
+ if (!ceil64)
+ ceil64 = rate64;
+
+ opt.rate.rate = (rate64 >= (1ULL << 32)) ? ~0U : rate64;
+ opt.ceil.rate = (ceil64 >= (1ULL << 32)) ? ~0U : ceil64;
/* compute minimal allowed burst from rate; mtu is added here to make
sute that buffer is larger than mtu and to have some safeguard space */
- if (!buffer) buffer = opt.rate.rate / get_hz() + mtu;
- if (!cbuffer) cbuffer = opt.ceil.rate / get_hz() + mtu;
+ if (!buffer)
+ buffer = rate64 / get_hz() + mtu;
+ if (!cbuffer)
+ cbuffer = ceil64 / get_hz() + mtu;
opt.ceil.overhead = overhead;
opt.rate.overhead = overhead;
@@ -229,19 +236,26 @@ static int htb_parse_class_opt(struct qdisc_util *qu, int argc, char **argv, str
fprintf(stderr, "htb: failed to calculate rate table.\n");
return -1;
}
- opt.buffer = tc_calc_xmittime(opt.rate.rate, buffer);
+ opt.buffer = tc_calc_xmittime(rate64, buffer);
if (tc_calc_rtable(&opt.ceil, ctab, ccell_log, mtu, linklayer) < 0) {
fprintf(stderr, "htb: failed to calculate ceil rate table.\n");
return -1;
}
- opt.cbuffer = tc_calc_xmittime(opt.ceil.rate, cbuffer);
+ opt.cbuffer = tc_calc_xmittime(ceil64, cbuffer);
tail = NLMSG_TAIL(n);
if (direct_qlen != ~0U)
addattr_l(n, 1024, TCA_HTB_DIRECT_QLEN,
&direct_qlen, sizeof(direct_qlen));
addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+
+ if (rate64 >= (1ULL << 32))
+ addattr_l(n, 1124, TCA_HTB_RATE64, &rate64, sizeof(rate64));
+
+ if (ceil64 >= (1ULL << 32))
+ addattr_l(n, 1224, TCA_HTB_CEIL64, &ceil64, sizeof(ceil64));
+
addattr_l(n, 2024, TCA_HTB_PARMS, &opt, sizeof(opt));
addattr_l(n, 3024, TCA_HTB_RTAB, rtab, 1024);
addattr_l(n, 4024, TCA_HTB_CTAB, ctab, 1024);
@@ -256,6 +270,7 @@ static int htb_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
struct tc_htb_glob *gopt;
double buffer,cbuffer;
unsigned int linklayer;
+ __u64 rate64, ceil64;
SPRINT_BUF(b1);
SPRINT_BUF(b2);
SPRINT_BUF(b3);
@@ -275,12 +290,25 @@ static int htb_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
if (show_details)
fprintf(f, "quantum %d ", (int)hopt->quantum);
}
- fprintf(f, "rate %s ", sprint_rate(hopt->rate.rate, b1));
+
+ rate64 = hopt->rate.rate;
+ if (tb[TCA_HTB_RATE64] &&
+ RTA_PAYLOAD(tb[TCA_HTB_RATE64]) >= sizeof(rate64)) {
+ rate64 = rta_getattr_u64(tb[TCA_HTB_RATE64]);
+ }
+
+ ceil64 = hopt->ceil.rate;
+ if (tb[TCA_HTB_CEIL64] &&
+ RTA_PAYLOAD(tb[TCA_HTB_CEIL64]) >= sizeof(ceil64))
+ ceil64 = rta_getattr_u64(tb[TCA_HTB_CEIL64]);
+
+ fprintf(f, "rate %s ", sprint_rate(rate64, b1));
if (hopt->rate.overhead)
fprintf(f, "overhead %u ", hopt->rate.overhead);
- buffer = tc_calc_xmitsize(hopt->rate.rate, hopt->buffer);
- fprintf(f, "ceil %s ", sprint_rate(hopt->ceil.rate, b1));
- cbuffer = tc_calc_xmitsize(hopt->ceil.rate, hopt->cbuffer);
+ buffer = tc_calc_xmitsize(rate64, hopt->buffer);
+
+ fprintf(f, "ceil %s ", sprint_rate(ceil64, b1));
+ cbuffer = tc_calc_xmitsize(ceil64, hopt->cbuffer);
linklayer = (hopt->rate.linklayer & TC_LINKLAYER_MASK);
if (linklayer > TC_LINKLAYER_ETHERNET || show_details)
fprintf(f, "linklayer %s ", sprint_linklayer(linklayer, b4));
diff --git a/tc/tc_core.c b/tc/tc_core.c
index a524337..46eaefb 100644
--- a/tc/tc_core.c
+++ b/tc/tc_core.c
@@ -56,12 +56,12 @@ unsigned tc_core_ktime2time(unsigned ktime)
return ktime / clock_factor;
}
-unsigned tc_calc_xmittime(unsigned rate, unsigned size)
+unsigned tc_calc_xmittime(__u64 rate, unsigned size)
{
- return tc_core_time2tick(TIME_UNITS_PER_SEC*((double)size/rate));
+ return tc_core_time2tick(TIME_UNITS_PER_SEC*((double)size/(double)rate));
}
-unsigned tc_calc_xmitsize(unsigned rate, unsigned ticks)
+unsigned tc_calc_xmitsize(__u64 rate, unsigned ticks)
{
return ((double)rate*tc_core_tick2time(ticks))/TIME_UNITS_PER_SEC;
}
diff --git a/tc/tc_core.h b/tc/tc_core.h
index 5a693ba..8a63b79 100644
--- a/tc/tc_core.h
+++ b/tc/tc_core.h
@@ -18,8 +18,8 @@ unsigned tc_core_time2tick(unsigned time);
unsigned tc_core_tick2time(unsigned tick);
unsigned tc_core_time2ktime(unsigned time);
unsigned tc_core_ktime2time(unsigned ktime);
-unsigned tc_calc_xmittime(unsigned rate, unsigned size);
-unsigned tc_calc_xmitsize(unsigned rate, unsigned ticks);
+unsigned tc_calc_xmittime(__u64 rate, unsigned size);
+unsigned tc_calc_xmitsize(__u64 rate, unsigned ticks);
int tc_calc_rtable(struct tc_ratespec *r, __u32 *rtab,
int cell_log, unsigned mtu, enum link_layer link_layer);
int tc_calc_size_table(struct tc_sizespec *s, __u16 **stab);
diff --git a/tc/tc_util.c b/tc/tc_util.c
index be3ed07..808c768 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -171,6 +171,31 @@ int get_rate(unsigned *rate, const char *str)
return 0;
}
+int get_rate64(__u64 *rate, const char *str)
+{
+ char *p;
+ double bps = strtod(str, &p);
+ const struct rate_suffix *s;
+
+ if (p == str)
+ return -1;
+
+ for (s = suffixes; s->name; ++s) {
+ if (strcasecmp(s->name, p) == 0) {
+ bps *= s->scale;
+ p += strlen(p);
+ break;
+ }
+ }
+
+ if (*p)
+ return -1; /* unknown suffix */
+
+ bps /= 8; /* -> bytes per second */
+ *rate = bps;
+ return 0;
+}
+
void print_rate(char *buf, int len, __u64 rate)
{
double tmp = (double)rate*8;
diff --git a/tc/tc_util.h b/tc/tc_util.h
index 7c3709f..d418367 100644
--- a/tc/tc_util.h
+++ b/tc/tc_util.h
@@ -58,6 +58,7 @@ extern struct filter_util *get_filter_kind(const char *str);
extern int get_qdisc_handle(__u32 *h, const char *str);
extern int get_rate(unsigned *rate, const char *str);
+extern int get_rate64(__u64 *rate, const char *str);
extern int get_size(unsigned *size, const char *str);
extern int get_size_and_cell(unsigned *size, int *cell_log, char *str);
extern int get_time(unsigned *time, const char *str);
^ permalink raw reply related
* Re: shutdown(3) and bluetooth.
From: Marcel Holtmann @ 2013-11-12 22:32 UTC (permalink / raw)
To: Dave Jones
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-bluetooth-u79uwXL29TY76Z2rM5mHXA@public.gmane.org development
In-Reply-To: <20131112221038.GA6689-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Hi Dave,
>>> Is shutdown() allowed to block indefinitely ? The man page doesn't say either way,
>>> and I've noticed that my fuzz tester occasionally hangs for days spinning in bt_sock_wait_state()
>>>
>>> Is there something I should be doing to guarantee that this operation
>>> will either time out, or return instantly ?
>>>
>>> In this specific case, I doubt anything is on the "sender" end of the socket, so
>>> it's going to be waiting forever for a state change that won't arrive.
>>
>> can you give us some extra information here. What kind of Bluetooth socket is this actually. From the top of my head, I have no idea why we would even wait forever. Normally when all low-level links are gone, the socket will shut down anyway.
>
> Here's the info I found in the logs, it looks like this was the only bluetooth socket.
>
> fd[195] = domain:31 (PF_BLUETOOTH) type:0x5 protocol:2
> Setsockopt(1 d 2134000 8) on fd 195
this is a bit confusing. Protocol 2 is actually SCO, but the stack trace shows RFCOMM.
> it doesn't look like any further operations were done on this fd during the fuzzers runtime.
>
> Quick way to reproduce:
>
> ./trinity -P PF_BLUETOOTH -l off -c setsockopt
>
> let it run a few seconds, and then ctrl-c. The main process will never exit.
>
> 5814 pts/6 Ss 0:00 | \_ bash
> 5876 pts/6 S+ 0:00 | | \_ ./trinity -P PF_BLUETOOTH -l off -c setsockopt
> 5877 pts/6 Z+ 0:00 | | \_ [trinity] <defunct>
> 5878 pts/6 S+ 0:01 | | \_ [trinity-main]
>
> $ sudo cat /proc/5878/stack
> [<ffffffffa04397a2>] bt_sock_wait_state+0xc2/0x190 [bluetooth]
> [<ffffffffa0847a75>] rfcomm_sock_shutdown+0x85/0xb0 [rfcomm]
> [<ffffffffa0847ad9>] rfcomm_sock_release+0x39/0xb0 [rfcomm]
> [<ffffffff81532fcf>] sock_release+0x1f/0x80
> [<ffffffff81533042>] sock_close+0x12/0x20
> [<ffffffff811a9ac1>] __fput+0xe1/0x230
> [<ffffffff811a9c5e>] ____fput+0xe/0x10
> [<ffffffff8108534c>] task_work_run+0xbc/0xe0
> [<ffffffff8106944c>] do_exit+0x2bc/0xa20
> [<ffffffff81069c2f>] do_group_exit+0x3f/0xa0
> [<ffffffff81069ca4>] SyS_exit_group+0x14/0x20
> [<ffffffff81656b27>] tracesys+0xdd/0xe2
> [<ffffffffffffffff>] 0xffffffffffffffff
What kernel did you run this against? It is a shot in the dark, but can you try linux-next quickly. There was a socket related fix for the socket options where we confused RFCOMM vs L2CAP struct sock.
Regards
Marcel
^ permalink raw reply
* [PATCH net-next 4/4] virtio-net: auto-tune mergeable rx buffer size for improved performance
From: Michael Dalton @ 2013-11-12 22:21 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Eric Dumazet, Rusty Russell, Michael S. Tsirkin,
Daniel Borkmann, Jason Wang, Eric Northup, virtualization,
Michael Dalton
In-Reply-To: <1384294885-6444-1-git-send-email-mwdalton@google.com>
Commit 2613af0ed18a ("virtio_net: migrate mergeable rx buffers to page frag
allocators") changed the mergeable receive buffer size from PAGE_SIZE to
MTU-size, introducing a single-stream regression for benchmarks with large
average packet size. There is no single optimal buffer size for all workloads.
For workloads with packet size <= MTU bytes, MTU + virtio-net header-sized
buffers are preferred as larger buffers reduce the TCP window due to SKB
truesize. However, single-stream workloads with large average packet sizes
have higher throughput if larger (e.g., PAGE_SIZE) buffers are used.
This commit auto-tunes the mergeable receiver buffer packet size by choosing
the packet buffer size based on an EWMA of the recent packet sizes for the
receive queue. Packet buffer sizes range from MTU_SIZE + virtio-net header
len to PAGE_SIZE. This improves throughput for large packet workloads, as
any workload with average packet size >= PAGE_SIZE will use PAGE_SIZE
buffers.
These optimizations interact positively with recent commit
ba275241030c ("virtio-net: coalesce rx frags when possible during rx"),
which coalesces adjacent RX SKB fragments in virtio_net. The coalescing
optimizations benefit buffers of any size.
Benchmarks taken from an average of 5 netperf 30-second TCP_STREAM runs
between two QEMU VMs on a single physical machine. Each VM has two VCPUs
with all offloads & vhost enabled. All VMs and vhost threads run in a
single 4 CPU cgroup cpuset, using cgroups to ensure that other processes
in the system will not be scheduled on the benchmark CPUs. Trunk includes
SKB rx frag coalescing.
net-next trunk w/ virtio_net before 2613af0ed18a (PAGE_SIZE bufs): 14642.85Gb/s
net-next trunk (MTU-size bufs): 13170.01Gb/s
net-next trunk + auto-tune: 14555.94Gb/s
Signed-off-by: Michael Dalton <mwdalton@google.com>
---
drivers/net/virtio_net.c | 73 +++++++++++++++++++++++++++++++++++-------------
1 file changed, 53 insertions(+), 20 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 0c93054..b1086e0 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -27,6 +27,7 @@
#include <linux/if_vlan.h>
#include <linux/slab.h>
#include <linux/cpu.h>
+#include <linux/average.h>
static int napi_weight = NAPI_POLL_WEIGHT;
module_param(napi_weight, int, 0444);
@@ -37,10 +38,8 @@ module_param(gso, bool, 0444);
/* FIXME: MTU in config. */
#define GOOD_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
-#define MERGE_BUFFER_LEN (ALIGN(GOOD_PACKET_LEN + \
- sizeof(struct virtio_net_hdr_mrg_rxbuf), \
- L1_CACHE_BYTES))
#define GOOD_COPY_LEN 128
+#define RECEIVE_AVG_WEIGHT 64
#define VIRTNET_DRIVER_VERSION "1.0.0"
@@ -79,6 +78,9 @@ struct receive_queue {
/* Chain pages by the private ptr. */
struct page *pages;
+ /* Average packet length for mergeable receive buffers. */
+ struct ewma mrg_avg_pkt_len;
+
/* Page frag for GFP_ATOMIC packet buffer allocation. */
struct page_frag atomic_frag;
@@ -302,14 +304,17 @@ static struct sk_buff *page_to_skb(struct receive_queue *rq,
return skb;
}
-static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb)
+static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb,
+ struct page *head_page)
{
struct skb_vnet_hdr *hdr = skb_vnet_hdr(head_skb);
struct sk_buff *curr_skb = head_skb;
+ struct page *page = head_page;
char *buf;
- struct page *page;
- int num_buf, len, offset, truesize;
+ int num_buf, len, offset;
+ u32 est_buffer_len;
+ len = head_skb->len;
num_buf = hdr->mhdr.num_buffers;
while (--num_buf) {
int num_skb_frags = skb_shinfo(curr_skb)->nr_frags;
@@ -320,7 +325,6 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb)
head_skb->dev->stats.rx_length_errors++;
return -EINVAL;
}
- truesize = max_t(int, len, MERGE_BUFFER_LEN);
if (unlikely(num_skb_frags == MAX_SKB_FRAGS)) {
struct sk_buff *nskb = alloc_skb(0, GFP_ATOMIC);
if (unlikely(!nskb)) {
@@ -338,20 +342,38 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb)
if (curr_skb != head_skb) {
head_skb->data_len += len;
head_skb->len += len;
- head_skb->truesize += truesize;
+ head_skb->truesize += len;
}
page = virt_to_head_page(buf);
offset = buf - (char *)page_address(page);
if (skb_can_coalesce(curr_skb, num_skb_frags, page, offset)) {
put_page(page);
skb_coalesce_rx_frag(curr_skb, num_skb_frags - 1,
- len, truesize);
+ len, len);
} else {
skb_add_rx_frag(curr_skb, num_skb_frags, page,
- offset, len, truesize);
+ offset, len, len);
}
--rq->num;
}
+ /* All frags before the last frag are fully used -- for those frags,
+ * truesize = len. Use the size of the most recent buffer allocation
+ * from the last frag's page to estimate the truesize of the last frag.
+ * EWMA with a weight of 64 makes the size adjustments quite small in
+ * the frags allocated on one page (even a order-3 one), and truesize
+ * doesn't need to be 100% accurate.
+ */
+ if (page) {
+ est_buffer_len = page_private(page);
+ if (est_buffer_len > len) {
+ u32 truesize_delta = est_buffer_len - len;
+
+ curr_skb->truesize += truesize_delta;
+ if (curr_skb != head_skb)
+ head_skb->truesize += truesize_delta;
+ }
+ }
+ ewma_add(&rq->mrg_avg_pkt_len, head_skb->len);
return 0;
}
@@ -382,16 +404,21 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
skb_trim(skb, len);
} else if (vi->mergeable_rx_bufs) {
struct page *page = virt_to_head_page(buf);
- int truesize = max_t(int, len, MERGE_BUFFER_LEN);
+ /* Use an initial truesize of 'len' bytes for page_to_skb --
+ * receive_mergeable will fixup the truesize of the last page
+ * frag if the packet is non-linear (> GOOD_COPY_LEN bytes).
+ */
skb = page_to_skb(rq, page,
(char *)buf - (char *)page_address(page),
- len, truesize);
+ len, len);
if (unlikely(!skb)) {
dev->stats.rx_dropped++;
put_page(page);
return;
}
- if (receive_mergeable(rq, skb)) {
+ if (!skb_is_nonlinear(skb))
+ page = NULL;
+ if (receive_mergeable(rq, skb, page)) {
dev_kfree_skb(skb);
return;
}
@@ -540,24 +567,29 @@ static int add_recvbuf_big(struct receive_queue *rq, gfp_t gfp)
static int add_recvbuf_mergeable(struct receive_queue *rq, gfp_t gfp)
{
struct virtnet_info *vi = rq->vq->vdev->priv;
+ const size_t hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
struct page_frag *alloc_frag;
char *buf;
- int err, len, hole;
+ int err, hole;
+ u32 buflen;
+ buflen = hdr_len + clamp_t(u32, ewma_read(&rq->mrg_avg_pkt_len),
+ GOOD_PACKET_LEN, PAGE_SIZE - hdr_len);
+ buflen = ALIGN(buflen, L1_CACHE_BYTES);
alloc_frag = (gfp & __GFP_WAIT) ? &vi->sleep_frag : &rq->atomic_frag;
- if (unlikely(!skb_page_frag_refill(MERGE_BUFFER_LEN, alloc_frag, gfp)))
+ if (unlikely(!skb_page_frag_refill(buflen, alloc_frag, gfp)))
return -ENOMEM;
buf = (char *)page_address(alloc_frag->page) + alloc_frag->offset;
get_page(alloc_frag->page);
- len = MERGE_BUFFER_LEN;
- alloc_frag->offset += len;
+ alloc_frag->offset += buflen;
+ set_page_private(alloc_frag->page, buflen);
hole = alloc_frag->size - alloc_frag->offset;
- if (hole < MERGE_BUFFER_LEN) {
- len += hole;
+ if (hole < buflen) {
+ buflen += hole;
alloc_frag->offset += hole;
}
- sg_init_one(rq->sg, buf, len);
+ sg_init_one(rq->sg, buf, buflen);
err = virtqueue_add_inbuf(rq->vq, rq->sg, 1, buf, gfp);
if (err < 0)
put_page(virt_to_head_page(buf));
@@ -1475,6 +1507,7 @@ static int virtnet_alloc_queues(struct virtnet_info *vi)
napi_weight);
sg_init_table(vi->rq[i].sg, ARRAY_SIZE(vi->rq[i].sg));
+ ewma_init(&vi->rq[i].mrg_avg_pkt_len, 1, RECEIVE_AVG_WEIGHT);
sg_init_table(vi->sq[i].sg, ARRAY_SIZE(vi->sq[i].sg));
}
--
1.8.4.1
^ permalink raw reply related
* [PATCH net-next 3/4] virtio-net: use per-receive queue page frag alloc for mergeable bufs
From: Michael Dalton @ 2013-11-12 22:21 UTC (permalink / raw)
To: David S. Miller
Cc: Michael Dalton, Michael S. Tsirkin, netdev, Daniel Borkmann,
virtualization, Eric Dumazet
In-Reply-To: <1384294885-6444-1-git-send-email-mwdalton@google.com>
The virtio-net driver currently uses netdev_alloc_frag() for GFP_ATOMIC
mergeable rx buffer allocations. This commit migrates virtio-net to use
per-receive queue page frags for GFP_ATOMIC allocation. This change unifies
mergeable rx buffer memory allocation, which now will use skb_refill_frag()
for both atomic and GFP-WAIT buffer allocations.
To address fragmentation concerns, if after buffer allocation there
is too little space left in the page frag to allocate a subsequent
buffer, the remaining space is added to the current allocated buffer
so that the remaining space can be used to store packet data.
Signed-off-by: Michael Dalton <mwdalton@google.com>
---
drivers/net/virtio_net.c | 70 +++++++++++++++++++++++++++---------------------
1 file changed, 39 insertions(+), 31 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 69fb225..0c93054 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -79,6 +79,9 @@ struct receive_queue {
/* Chain pages by the private ptr. */
struct page *pages;
+ /* Page frag for GFP_ATOMIC packet buffer allocation. */
+ struct page_frag atomic_frag;
+
/* RX: fragments + linear part + virtio header */
struct scatterlist sg[MAX_SKB_FRAGS + 2];
@@ -128,9 +131,9 @@ struct virtnet_info {
struct mutex config_lock;
/* Page_frag for GFP_KERNEL packet buffer allocation when we run
- * low on memory.
+ * low on memory. May sleep.
*/
- struct page_frag alloc_frag;
+ struct page_frag sleep_frag;
/* Does the affinity hint is set for virtqueues? */
bool affinity_hint_set;
@@ -305,7 +308,7 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb)
struct sk_buff *curr_skb = head_skb;
char *buf;
struct page *page;
- int num_buf, len, offset;
+ int num_buf, len, offset, truesize;
num_buf = hdr->mhdr.num_buffers;
while (--num_buf) {
@@ -317,11 +320,7 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb)
head_skb->dev->stats.rx_length_errors++;
return -EINVAL;
}
- if (unlikely(len > MERGE_BUFFER_LEN)) {
- pr_debug("%s: rx error: merge buffer too long\n",
- head_skb->dev->name);
- len = MERGE_BUFFER_LEN;
- }
+ truesize = max_t(int, len, MERGE_BUFFER_LEN);
if (unlikely(num_skb_frags == MAX_SKB_FRAGS)) {
struct sk_buff *nskb = alloc_skb(0, GFP_ATOMIC);
if (unlikely(!nskb)) {
@@ -339,17 +338,17 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb)
if (curr_skb != head_skb) {
head_skb->data_len += len;
head_skb->len += len;
- head_skb->truesize += MERGE_BUFFER_LEN;
+ head_skb->truesize += truesize;
}
page = virt_to_head_page(buf);
offset = buf - (char *)page_address(page);
if (skb_can_coalesce(curr_skb, num_skb_frags, page, offset)) {
put_page(page);
skb_coalesce_rx_frag(curr_skb, num_skb_frags - 1,
- len, MERGE_BUFFER_LEN);
+ len, truesize);
} else {
skb_add_rx_frag(curr_skb, num_skb_frags, page,
- offset, len, MERGE_BUFFER_LEN);
+ offset, len, truesize);
}
--rq->num;
}
@@ -383,9 +382,10 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
skb_trim(skb, len);
} else if (vi->mergeable_rx_bufs) {
struct page *page = virt_to_head_page(buf);
+ int truesize = max_t(int, len, MERGE_BUFFER_LEN);
skb = page_to_skb(rq, page,
(char *)buf - (char *)page_address(page),
- len, MERGE_BUFFER_LEN);
+ len, truesize);
if (unlikely(!skb)) {
dev->stats.rx_dropped++;
put_page(page);
@@ -540,24 +540,24 @@ static int add_recvbuf_big(struct receive_queue *rq, gfp_t gfp)
static int add_recvbuf_mergeable(struct receive_queue *rq, gfp_t gfp)
{
struct virtnet_info *vi = rq->vq->vdev->priv;
- char *buf = NULL;
- int err;
+ struct page_frag *alloc_frag;
+ char *buf;
+ int err, len, hole;
- if (gfp & __GFP_WAIT) {
- if (skb_page_frag_refill(MERGE_BUFFER_LEN, &vi->alloc_frag,
- gfp)) {
- buf = (char *)page_address(vi->alloc_frag.page) +
- vi->alloc_frag.offset;
- get_page(vi->alloc_frag.page);
- vi->alloc_frag.offset += MERGE_BUFFER_LEN;
- }
- } else {
- buf = netdev_alloc_frag(MERGE_BUFFER_LEN);
- }
- if (!buf)
+ alloc_frag = (gfp & __GFP_WAIT) ? &vi->sleep_frag : &rq->atomic_frag;
+ if (unlikely(!skb_page_frag_refill(MERGE_BUFFER_LEN, alloc_frag, gfp)))
return -ENOMEM;
+ buf = (char *)page_address(alloc_frag->page) + alloc_frag->offset;
+ get_page(alloc_frag->page);
+ len = MERGE_BUFFER_LEN;
+ alloc_frag->offset += len;
+ hole = alloc_frag->size - alloc_frag->offset;
+ if (hole < MERGE_BUFFER_LEN) {
+ len += hole;
+ alloc_frag->offset += hole;
+ }
- sg_init_one(rq->sg, buf, MERGE_BUFFER_LEN);
+ sg_init_one(rq->sg, buf, len);
err = virtqueue_add_inbuf(rq->vq, rq->sg, 1, buf, gfp);
if (err < 0)
put_page(virt_to_head_page(buf));
@@ -1335,6 +1335,16 @@ static void free_receive_bufs(struct virtnet_info *vi)
}
}
+static void free_receive_page_frags(struct virtnet_info *vi)
+{
+ int i;
+ for (i = 0; i < vi->max_queue_pairs; i++)
+ if (vi->rq[i].atomic_frag.page)
+ put_page(vi->rq[i].atomic_frag.page);
+ if (vi->sleep_frag.page)
+ put_page(vi->sleep_frag.page);
+}
+
static void free_unused_bufs(struct virtnet_info *vi)
{
void *buf;
@@ -1655,8 +1665,7 @@ free_recv_bufs:
free_vqs:
cancel_delayed_work_sync(&vi->refill);
virtnet_del_vqs(vi);
- if (vi->alloc_frag.page)
- put_page(vi->alloc_frag.page);
+ free_receive_page_frags(vi);
free_stats:
free_percpu(vi->stats);
free:
@@ -1690,8 +1699,7 @@ static void virtnet_remove(struct virtio_device *vdev)
unregister_netdev(vi->dev);
remove_vq_common(vi);
- if (vi->alloc_frag.page)
- put_page(vi->alloc_frag.page);
+ free_receive_page_frags(vi);
flush_work(&vi->config_work);
--
1.8.4.1
^ permalink raw reply related
* [PATCH net-next 2/4] net: allow > 0 order atomic page alloc in skb_page_frag_refill
From: Michael Dalton @ 2013-11-12 22:21 UTC (permalink / raw)
To: David S. Miller
Cc: Michael Dalton, Michael S. Tsirkin, netdev, Daniel Borkmann,
virtualization, Eric Dumazet
In-Reply-To: <1384294885-6444-1-git-send-email-mwdalton@google.com>
skb_page_frag_refill currently permits only order-0 page allocs
unless GFP_WAIT is used. Change skb_page_frag_refill to attempt
higher-order page allocations whether or not GFP_WAIT is used. If
memory cannot be allocated, the allocator will fall back to
successively smaller page allocs (down to order-0 page allocs).
This change brings skb_page_frag_refill in line with the existing
page allocation strategy employed by netdev_alloc_frag, which attempts
higher-order page allocations whether or not GFP_WAIT is set, falling
back to successively lower-order page allocations on failure. Part
of migration of virtio-net to per-receive queue page frag allocators.
Signed-off-by: Michael Dalton <mwdalton@google.com>
---
net/core/sock.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/core/sock.c b/net/core/sock.c
index ab20ed9..7383d23 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1865,9 +1865,7 @@ bool skb_page_frag_refill(unsigned int sz, struct page_frag *pfrag, gfp_t prio)
put_page(pfrag->page);
}
- /* We restrict high order allocations to users that can afford to wait */
- order = (prio & __GFP_WAIT) ? SKB_FRAG_PAGE_ORDER : 0;
-
+ order = SKB_FRAG_PAGE_ORDER;
do {
gfp_t gfp = prio;
--
1.8.4.1
^ permalink raw reply related
* [PATCH net-next 1/4] virtio-net: mergeable buffer size should include virtio-net header
From: Michael Dalton @ 2013-11-12 22:21 UTC (permalink / raw)
To: David S. Miller
Cc: Michael Dalton, Michael S. Tsirkin, netdev, Daniel Borkmann,
virtualization, Eric Dumazet
Commit 2613af0ed18a ("virtio_net: migrate mergeable rx buffers to page
frag allocators") changed the mergeable receive buffer size from PAGE_SIZE
to MTU-size. However, the merge buffer size does not take into account the
size of the virtio-net header. Consequently, packets that are MTU-size
will take two buffers intead of one (to store the virtio-net header),
substantially decreasing the throughput of MTU-size traffic due to TCP
window / SKB truesize effects.
This commit changes the mergeable buffer size to include the virtio-net
header. The buffer size is cacheline-aligned because skb_page_frag_refill
will not automatically align the requested size.
Benchmarks taken from an average of 5 netperf 30-second TCP_STREAM runs
between two QEMU VMs on a single physical machine. Each VM has two VCPUs and
vhost enabled. All VMs and vhost threads run in a single 4 CPU cgroup
cpuset, using cgroups to ensure that other processes in the system will not
be scheduled on the benchmark CPUs. Transmit offloads and mergeable receive
buffers are enabled, but guest_tso4 / guest_csum are explicitly disabled to
force MTU-sized packets on the receiver.
next-net trunk before 2613af0ed18a (PAGE_SIZE buf): 3861.08Gb/s
net-next trunk (MTU 1500- packet uses two buf due to size bug): 4076.62Gb/s
net-next trunk (MTU 1480- packet fits in one buf): 6301.34Gb/s
net-next trunk w/ size fix (MTU 1500 - packet fits in one buf): 6445.44Gb/s
Suggested-by: Eric Northup <digitaleric@google.com>
Signed-off-by: Michael Dalton <mwdalton@google.com>
---
drivers/net/virtio_net.c | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 01f4eb5..69fb225 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -36,7 +36,10 @@ module_param(csum, bool, 0444);
module_param(gso, bool, 0444);
/* FIXME: MTU in config. */
-#define MAX_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
+#define GOOD_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
+#define MERGE_BUFFER_LEN (ALIGN(GOOD_PACKET_LEN + \
+ sizeof(struct virtio_net_hdr_mrg_rxbuf), \
+ L1_CACHE_BYTES))
#define GOOD_COPY_LEN 128
#define VIRTNET_DRIVER_VERSION "1.0.0"
@@ -314,10 +317,10 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb)
head_skb->dev->stats.rx_length_errors++;
return -EINVAL;
}
- if (unlikely(len > MAX_PACKET_LEN)) {
+ if (unlikely(len > MERGE_BUFFER_LEN)) {
pr_debug("%s: rx error: merge buffer too long\n",
head_skb->dev->name);
- len = MAX_PACKET_LEN;
+ len = MERGE_BUFFER_LEN;
}
if (unlikely(num_skb_frags == MAX_SKB_FRAGS)) {
struct sk_buff *nskb = alloc_skb(0, GFP_ATOMIC);
@@ -336,18 +339,17 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb)
if (curr_skb != head_skb) {
head_skb->data_len += len;
head_skb->len += len;
- head_skb->truesize += MAX_PACKET_LEN;
+ head_skb->truesize += MERGE_BUFFER_LEN;
}
page = virt_to_head_page(buf);
offset = buf - (char *)page_address(page);
if (skb_can_coalesce(curr_skb, num_skb_frags, page, offset)) {
put_page(page);
skb_coalesce_rx_frag(curr_skb, num_skb_frags - 1,
- len, MAX_PACKET_LEN);
+ len, MERGE_BUFFER_LEN);
} else {
skb_add_rx_frag(curr_skb, num_skb_frags, page,
- offset, len,
- MAX_PACKET_LEN);
+ offset, len, MERGE_BUFFER_LEN);
}
--rq->num;
}
@@ -383,7 +385,7 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
struct page *page = virt_to_head_page(buf);
skb = page_to_skb(rq, page,
(char *)buf - (char *)page_address(page),
- len, MAX_PACKET_LEN);
+ len, MERGE_BUFFER_LEN);
if (unlikely(!skb)) {
dev->stats.rx_dropped++;
put_page(page);
@@ -471,11 +473,11 @@ static int add_recvbuf_small(struct receive_queue *rq, gfp_t gfp)
struct skb_vnet_hdr *hdr;
int err;
- skb = __netdev_alloc_skb_ip_align(vi->dev, MAX_PACKET_LEN, gfp);
+ skb = __netdev_alloc_skb_ip_align(vi->dev, GOOD_PACKET_LEN, gfp);
if (unlikely(!skb))
return -ENOMEM;
- skb_put(skb, MAX_PACKET_LEN);
+ skb_put(skb, GOOD_PACKET_LEN);
hdr = skb_vnet_hdr(skb);
sg_set_buf(rq->sg, &hdr->hdr, sizeof hdr->hdr);
@@ -542,20 +544,20 @@ static int add_recvbuf_mergeable(struct receive_queue *rq, gfp_t gfp)
int err;
if (gfp & __GFP_WAIT) {
- if (skb_page_frag_refill(MAX_PACKET_LEN, &vi->alloc_frag,
+ if (skb_page_frag_refill(MERGE_BUFFER_LEN, &vi->alloc_frag,
gfp)) {
buf = (char *)page_address(vi->alloc_frag.page) +
vi->alloc_frag.offset;
get_page(vi->alloc_frag.page);
- vi->alloc_frag.offset += MAX_PACKET_LEN;
+ vi->alloc_frag.offset += MERGE_BUFFER_LEN;
}
} else {
- buf = netdev_alloc_frag(MAX_PACKET_LEN);
+ buf = netdev_alloc_frag(MERGE_BUFFER_LEN);
}
if (!buf)
return -ENOMEM;
- sg_init_one(rq->sg, buf, MAX_PACKET_LEN);
+ sg_init_one(rq->sg, buf, MERGE_BUFFER_LEN);
err = virtqueue_add_inbuf(rq->vq, rq->sg, 1, buf, gfp);
if (err < 0)
put_page(virt_to_head_page(buf));
--
1.8.4.1
^ permalink raw reply related
* Re: [PATCH] can: add Renesas R-Car CAN driver
From: Wolfgang Grandegger @ 2013-11-12 22:17 UTC (permalink / raw)
To: Sergei Shtylyov, netdev, mkl, linux-can; +Cc: linux-sh, vksavl
In-Reply-To: <5282A17C.1030309@cogentembedded.com>
Hi Sergei,
On 11/12/2013 10:45 PM, Sergei Shtylyov wrote:
> Hello.
>
> On 11/09/2013 04:02 AM, Sergei Shtylyov wrote:
>
>>>>> 2. ... with short-circuited CAN high and low and doing some time later
>>>>> a manual recovery with "ip link set can0 type can restart"
>
>>>> Now we have auto recovery only. Manual recovery was tested with the
>>>> first driver version and worked.
>
>>> What do you mean with "auto recovery"? Auto recovery by the hardware or
>>> via "restart-ms <ms>"? How do you choose between "manual" and "auto"
>>> recovery?
>
>> This exact test was done with hardware auto-recovery only. No
>> "restart-ms"
>> was programmed.
>
>>>> Terminal 1:
>
>>>> root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
>>>> root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
>>>> root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
>>>> root@10.0.0.104:/opt/can-utils#
>
>>>> Terminal 2:
>
>>>> root@10.0.0.104:/opt/can-utils# ./candump -td -e any,0:0,#FFFFFFFF
>>>> (000.000000) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
>>>> controller-problem{}
>>>> protocol-violation{{tx-dominant-bit-error}{}}
>>>> bus-error
>>>> (000.021147) can0 20000144 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
>>>> controller-problem{}
>>>> bus-off
>>>> restarted-after-bus-off
>
>>> Why does it get "restarted" directly after the bus-off?
>
>> Because we have hardware auto-recovery enabled.
>
>>>> (011.738522) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
>>>> controller-problem{}
>
>>> What controller problem? data[1] is not set for some reasom.
>
>> Not comments. Looking into it.
>
> Sorry, this has been fixed a while ago. Now the log looks like:
>
> root@10.0.0.104:/opt/can-utils# ./candump -td -e any,0:0,#FFFFFFFF
> (000.000000) can0 2000008C [8] 00 28 08 00 00 00 88 00 ERRORFRAME
> controller-problem{tx-error-warning,tx-error-passive}
Only the highest level should be reported.
> protocol-violation{{tx-dominant-bit-error}{}}
> bus-error
> error-counter-tx-rx{{136}{0}}
> (000.007578) can0 20000040 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
> bus-off
> (000.091847) can0 20000100 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
> restarted-after-bus-off
> (056.136722) can0 2000008C [8] 00 28 08 00 00 00 88 00 ERRORFRAME
> controller-problem{tx-error-warning,tx-error-passive}
Ditto.
Wolfgang.
^ permalink raw reply
* Re: [PATCH] genetlink: fix usage of NLM_F_EXCL or NLM_F_REPLACE
From: David Miller @ 2013-11-12 22:12 UTC (permalink / raw)
To: pablo; +Cc: netdev
In-Reply-To: <20130801020049.GA4067@localhost>
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Thu, 1 Aug 2013 04:00:49 +0200
> @@ -571,7 +571,8 @@ static int genl_family_rcv_msg(struct genl_family *family,
> !capable(CAP_NET_ADMIN))
> return -EPERM;
>
> - if ((nlh->nlmsg_flags & NLM_F_DUMP) == NLM_F_DUMP) {
> + if ((ops->flags & GENL_CMD_CAP_DUMP) &&
> + nlh->nlmsg_flags & NLM_F_DUMP) {
> struct netlink_dump_control c = {
> .dump = ops->dumpit,
> .done = ops->done,
Sorry for taking so long to get back to you on this, it looks perfect!
I did some auditing of other uses, and briefly crypto does the same:
crypto/crypto_user.c:
if ((type == (CRYPTO_MSG_GETALG - CRYPTO_MSG_BASE) &&
(nlh->nlmsg_flags & NLM_F_DUMP))) {
"If this is a GET command, test dump flag"
Same thing for net/xfrm/xfrm_user.c:
if ((type == (XFRM_MSG_GETSA - XFRM_MSG_BASE) ||
type == (XFRM_MSG_GETPOLICY - XFRM_MSG_BASE)) &&
(nlh->nlmsg_flags & NLM_F_DUMP)) {
Similarly all of the netfilter stuff performs this NLM_F_DUMP bit
test in contexts where we are processing some GET command.
All the "diag" modules are implicitly processing GET commands.
Pablo, could you please retest and resubmit this patch? I will
apply it and push to -stable as well.
Thanks!
^ permalink raw reply
* Re: shutdown(3) and bluetooth.
From: Dave Jones @ 2013-11-12 22:10 UTC (permalink / raw)
To: Marcel Holtmann
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-bluetooth-u79uwXL29TY76Z2rM5mHXA@public.gmane.org development
In-Reply-To: <DF4C2B40-BD87-4E88-911D-E3E5F488CAE4-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
On Wed, Nov 13, 2013 at 06:56:23AM +0900, Marcel Holtmann wrote:
> Hi Dave,
>
> > Is shutdown() allowed to block indefinitely ? The man page doesn't say either way,
> > and I've noticed that my fuzz tester occasionally hangs for days spinning in bt_sock_wait_state()
> >
> > Is there something I should be doing to guarantee that this operation
> > will either time out, or return instantly ?
> >
> > In this specific case, I doubt anything is on the "sender" end of the socket, so
> > it's going to be waiting forever for a state change that won't arrive.
>
> can you give us some extra information here. What kind of Bluetooth socket is this actually. From the top of my head, I have no idea why we would even wait forever. Normally when all low-level links are gone, the socket will shut down anyway.
Here's the info I found in the logs, it looks like this was the only bluetooth socket.
fd[195] = domain:31 (PF_BLUETOOTH) type:0x5 protocol:2
Setsockopt(1 d 2134000 8) on fd 195
it doesn't look like any further operations were done on this fd during the fuzzers runtime.
Quick way to reproduce:
./trinity -P PF_BLUETOOTH -l off -c setsockopt
let it run a few seconds, and then ctrl-c. The main process will never exit.
5814 pts/6 Ss 0:00 | \_ bash
5876 pts/6 S+ 0:00 | | \_ ./trinity -P PF_BLUETOOTH -l off -c setsockopt
5877 pts/6 Z+ 0:00 | | \_ [trinity] <defunct>
5878 pts/6 S+ 0:01 | | \_ [trinity-main]
$ sudo cat /proc/5878/stack
[<ffffffffa04397a2>] bt_sock_wait_state+0xc2/0x190 [bluetooth]
[<ffffffffa0847a75>] rfcomm_sock_shutdown+0x85/0xb0 [rfcomm]
[<ffffffffa0847ad9>] rfcomm_sock_release+0x39/0xb0 [rfcomm]
[<ffffffff81532fcf>] sock_release+0x1f/0x80
[<ffffffff81533042>] sock_close+0x12/0x20
[<ffffffff811a9ac1>] __fput+0xe1/0x230
[<ffffffff811a9c5e>] ____fput+0xe/0x10
[<ffffffff8108534c>] task_work_run+0xbc/0xe0
[<ffffffff8106944c>] do_exit+0x2bc/0xa20
[<ffffffff81069c2f>] do_group_exit+0x3f/0xa0
[<ffffffff81069ca4>] SyS_exit_group+0x14/0x20
[<ffffffff81656b27>] tracesys+0xdd/0xe2
[<ffffffffffffffff>] 0xffffffffffffffff
Dave
^ permalink raw reply
* Re: [PATCH] net/7990: Make lance_private.name const
From: Sergei Shtylyov @ 2013-11-12 22:04 UTC (permalink / raw)
To: Geert Uytterhoeven, David S. Miller; +Cc: netdev, linux-m68k
In-Reply-To: <1384283649-10482-1-git-send-email-geert@linux-m68k.org>
Hello.
On 11/12/2013 10:14 PM, Geert Uytterhoeven wrote:
> This allows to drop a few casts.
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> drivers/net/ethernet/amd/7990.h | 2 +-
> drivers/net/ethernet/amd/hplance.c | 2 +-
> drivers/net/ethernet/amd/mvme147.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
> diff --git a/drivers/net/ethernet/amd/7990.h b/drivers/net/ethernet/amd/7990.h
> index 0a5837b96421..60c60926afda 100644
> --- a/drivers/net/ethernet/amd/7990.h
> +++ b/drivers/net/ethernet/amd/7990.h
> @@ -100,7 +100,7 @@ struct lance_init_block {
> */
> struct lance_private
> {
> - char *name;
> + const char *name;
Indent with tab, not spaces, please.
> diff --git a/drivers/net/ethernet/amd/hplance.c b/drivers/net/ethernet/amd/hplance.c
> index 0c61fd50d882..a9046cf3a564 100644
> --- a/drivers/net/ethernet/amd/hplance.c
> +++ b/drivers/net/ethernet/amd/hplance.c
> @@ -149,7 +149,7 @@ static void hplance_init(struct net_device *dev, struct dio_dev *d)
> }
>
> lp = netdev_priv(dev);
> - lp->lance.name = (char*)d->name; /* discards const, shut up gcc */
> + lp->lance.name = d->name;
Same here.
WBR, Sergei
^ permalink raw reply
* Re: shutdown(3) and bluetooth.
From: Marcel Holtmann @ 2013-11-12 21:56 UTC (permalink / raw)
To: Dave Jones
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-bluetooth-u79uwXL29TY76Z2rM5mHXA@public.gmane.org development
In-Reply-To: <20131112211125.GA2912-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Hi Dave,
> Is shutdown() allowed to block indefinitely ? The man page doesn't say either way,
> and I've noticed that my fuzz tester occasionally hangs for days spinning in bt_sock_wait_state()
>
> Is there something I should be doing to guarantee that this operation
> will either time out, or return instantly ?
>
> In this specific case, I doubt anything is on the "sender" end of the socket, so
> it's going to be waiting forever for a state change that won't arrive.
can you give us some extra information here. What kind of Bluetooth socket is this actually. From the top of my head, I have no idea why we would even wait forever. Normally when all low-level links are gone, the socket will shut down anyway.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH] can: add Renesas R-Car CAN driver
From: Sergei Shtylyov @ 2013-11-12 21:45 UTC (permalink / raw)
To: Wolfgang Grandegger, netdev, mkl, linux-can; +Cc: linux-sh, vksavl
In-Reply-To: <527D89A3.1070403@cogentembedded.com>
Hello.
On 11/09/2013 04:02 AM, Sergei Shtylyov wrote:
>>>> 2. ... with short-circuited CAN high and low and doing some time later
>>>> a manual recovery with "ip link set can0 type can restart"
>>> Now we have auto recovery only. Manual recovery was tested with the
>>> first driver version and worked.
>> What do you mean with "auto recovery"? Auto recovery by the hardware or
>> via "restart-ms <ms>"? How do you choose between "manual" and "auto"
>> recovery?
> This exact test was done with hardware auto-recovery only. No "restart-ms"
> was programmed.
>>> Terminal 1:
>>> root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
>>> root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
>>> root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
>>> root@10.0.0.104:/opt/can-utils#
>>> Terminal 2:
>>> root@10.0.0.104:/opt/can-utils# ./candump -td -e any,0:0,#FFFFFFFF
>>> (000.000000) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
>>> controller-problem{}
>>> protocol-violation{{tx-dominant-bit-error}{}}
>>> bus-error
>>> (000.021147) can0 20000144 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
>>> controller-problem{}
>>> bus-off
>>> restarted-after-bus-off
>> Why does it get "restarted" directly after the bus-off?
> Because we have hardware auto-recovery enabled.
>>> (011.738522) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
>>> controller-problem{}
>> What controller problem? data[1] is not set for some reasom.
> Not comments. Looking into it.
Sorry, this has been fixed a while ago. Now the log looks like:
root@10.0.0.104:/opt/can-utils# ./candump -td -e any,0:0,#FFFFFFFF
(000.000000) can0 2000008C [8] 00 28 08 00 00 00 88 00 ERRORFRAME
controller-problem{tx-error-warning,tx-error-passive}
protocol-violation{{tx-dominant-bit-error}{}}
bus-error
error-counter-tx-rx{{136}{0}}
(000.007578) can0 20000040 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
bus-off
(000.091847) can0 20000100 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
restarted-after-bus-off
(056.136722) can0 2000008C [8] 00 28 08 00 00 00 88 00 ERRORFRAME
controller-problem{tx-error-warning,tx-error-passive}
protocol-violation{{tx-dominant-bit-error}{}}
bus-error
error-counter-tx-rx{{136}{0}}
>>> dmesg:
>>> rcar_can rcar_can.0 can0: Error warning interrupt
>>> rcar_can rcar_can.0 can0: Error passive interrupt
>>> rcar_can rcar_can.0 can0: Bus error interrupt:
>>> rcar_can rcar_can.0 can0: Bit Error (dominant)
>>> rcar_can rcar_can.0 can0: Error warning interrupt
>>> rcar_can rcar_can.0 can0: Error passive interrupt
>> Why are they reported again. You are already in error passive.
> Don't know. :-/
This also has been dealt with. Here's an example:
rcar_can rcar_can.0 can0: Bus error interrupt:
rcar_can rcar_can.0 can0: ACK Error
rcar_can rcar_can.0 can0: Error warning interrupt
rcar_can rcar_can.0 can0: Bus error interrupt:
rcar_can rcar_can.0 can0: ACK Error
rcar_can rcar_can.0 can0: Error passive interrupt
WBR, Sergei
^ permalink raw reply
* Re: VLAN filtering/VLAN aware bridge problems
From: Stefan Priebe @ 2013-11-12 21:31 UTC (permalink / raw)
To: vyasevic; +Cc: David Miller, Linux Netdev List
In-Reply-To: <522F2887.1080300@redhat.com>
sorry for the very late response.
Am 10.09.2013 16:11, schrieb Vlad Yasevich:
> On 08/30/2013 11:01 AM, Stefan Priebe - Profihost AG wrote:
>> Yes
>>
>
> Can you apply this patch and see if this fixes your problem.
> http://patchwork.ozlabs.org/patch/273841/
>
> In my attempts to reproduce your problem I didn't configuring filtering
> on the upper bridge, but that is what could have been causing
> your problem. I'll attempt it and let you know.
Even with the complete patchset which was merged upstream it doesn't
work ;-(
What's wrong there and / or how can i debug?
Stefan
> -vlad
>
>
>> Stefan
>>
>> This mail was sent with my iPhone.
>>
>> Am 30.08.2013 um 16:13 schrieb Vlad Yasevich <vyasevic@redhat.com>:
>>
>>> On 08/30/2013 03:24 AM, Stefan Priebe - Profihost AG wrote:
>>>> Am 29.08.2013 22:45, schrieb Vlad Yasevich:
>>>>> On 08/29/2013 08:50 AM, Stefan Priebe - Profihost AG wrote:
>>>>
>>>>>> The packets never reach the TAP device.
>>>>>>
>>>>>> Here is an output of ip a l (vlan 3021):
>>>>>
>>>>> Can you provide output of brctl show?
>>>>
>>>> Sure:
>>>> # brctl show
>>>> bridge name bridge id STP enabled interfaces
>>>> vmbr0 8000.00259084dea8 no bond0
>>>> tap320i0
>>>> vmbr1 8000.00259084deaa no bond1
>>>> vmbr1v3021 8000.00259084deaa no
>>>> tap320i1
>>>> vmbr1.3021
>>>
>>> so let me see if I can understand this configuration.
>>>
>>> vmbr1v3021 (bridge)
>>> / \
>>> tap320i1 vmbr1.3021 (vlan)
>>> \
>>> vmbr1 (bridge)
>>> \
>>> bond1
>>> \
>>> eth X
>>>
>>>
>>> Is that right? Is this the setup that has the problem you are
>>> describing?
>>>
>>> Thanks
>>> -vlad
>>>
>>>>> On the off chance that you are actually trying to configure vlan
>>>>> filtering, can you give this patch a try (net-2.6 tree):
>>>>>
>>>>> Author: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
>>>>> Date: Tue Aug 20 17:10:18 2013 +0900
>>>>>
>>>>> bridge: Use the correct bit length for bitmap functions in the
>>>>> VLAN
>>>>> code
>>>>>
>>>>> I don't think it made it to stable yet.
>>>>
>>>> I addd that patch and now the vlan stuff works at least on the host
>>>> node. But my tap devices still don't work.
>>>>
>>>> I also tried to attach the tap device on top of a vlan attached to
>>>> bond1
>>>> but then gvrp does not work anymore. The kernel announces gvrp once and
>>>> then does not answer the query packets from the switch.
>>>>
>>>> Stefan
>>>
>
^ permalink raw reply
* Re: shutdown(3) and bluetooth.
From: David Miller @ 2013-11-12 21:13 UTC (permalink / raw)
To: davej-H+wXaHxf7aLQT0dZR+AlfA
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20131112211125.GA2912-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
From: Dave Jones <davej-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Date: Tue, 12 Nov 2013 16:11:25 -0500
> Is shutdown() allowed to block indefinitely ? The man page doesn't say either way,
> and I've noticed that my fuzz tester occasionally hangs for days spinning in bt_sock_wait_state()
>
> Is there something I should be doing to guarantee that this operation
> will either time out, or return instantly ?
>
> In this specific case, I doubt anything is on the "sender" end of the socket, so
> it's going to be waiting forever for a state change that won't arrive.
Adding bluetooth and wireless lists. Dave, please consult MAINTAINERS when
asking questions like this, thanks!
^ permalink raw reply
* Re: [PATCH] net/7990: Make lance_private.name const
From: David Miller @ 2013-11-12 21:12 UTC (permalink / raw)
To: sergei.shtylyov; +Cc: geert, netdev, linux-m68k
In-Reply-To: <5282A5E1.9090501@cogentembedded.com>
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Wed, 13 Nov 2013 01:04:17 +0300
>> @@ -100,7 +100,7 @@ struct lance_init_block {
>> */
>> struct lance_private
>> {
>> - char *name;
>> + const char *name;
>
> Indent with tab, not spaces, please.
This whole file is %99 space indentation, you really can't blame
Geert for this.
^ permalink raw reply
* shutdown(3) and bluetooth.
From: Dave Jones @ 2013-11-12 21:11 UTC (permalink / raw)
To: netdev
Is shutdown() allowed to block indefinitely ? The man page doesn't say either way,
and I've noticed that my fuzz tester occasionally hangs for days spinning in bt_sock_wait_state()
Is there something I should be doing to guarantee that this operation
will either time out, or return instantly ?
In this specific case, I doubt anything is on the "sender" end of the socket, so
it's going to be waiting forever for a state change that won't arrive.
Dave
^ permalink raw reply
* XMAS LOAN
From: 11122121 @ 2013-11-12 21:00 UTC (permalink / raw)
To: Recipients
Dear Sir/Madam
Are you in debts or Do you need loan to start up a business or do you need christmas loan? We give out all types of loans from $3,000.00 to $100,000,000.00 at 3%, Just address your email to Mr. Peterson West (petersonloanfundingcompany1@gmail.com) with the following information:
1: Names:...
2: Country....
3: Loan Amount:....
4: Loan Duration....
5: Phone Number:....
6: Monthly Income:......
7: State:.......
Note:All reply should be forwarded only to petersonloanfundingcompany1@gmail.com for immediate attention.
Contact us now and get an urgent loan within two (2) days!!!
Regards,
Mr.Peterson West
^ permalink raw reply
* Re: [Fwd: Re: [PATCH v2 2/2] x86: add prefetching to do_csum]
From: Neil Horman @ 2013-11-12 20:59 UTC (permalink / raw)
To: Joe Perches
Cc: netdev, Dave Jones, linux-kernel, sebastien.dugue,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Eric Dumazet
In-Reply-To: <1384288681.3665.22.camel@joe-AO722>
On Tue, Nov 12, 2013 at 12:38:01PM -0800, Joe Perches wrote:
> On Tue, 2013-11-12 at 14:50 -0500, Neil Horman wrote:
> > On Tue, Nov 12, 2013 at 09:33:35AM -0800, Joe Perches wrote:
> > > On Tue, 2013-11-12 at 12:12 -0500, Neil Horman wrote:
> []
> > > > So, the numbers are correct now that I returned my hardware to its previous
> > > > interrupt affinity state, but the trend seems to be the same (namely that there
> > > > isn't a clear one). We seem to find peak performance around a readahead of 2
> > > > cachelines, but its very small (about 3%), and its inconsistent (larger set
> > > > sizes fall to either side of that stride). So I don't see it as a clear win. I
> > > > still think we should probably scrap the readahead for now, just take the perf
> > > > bits, and revisit this when we can use the vector instructions or the
> > > > independent carry chain instructions to improve this more consistently.
> > > >
> > > > Thoughts
> > >
> > > Perhaps a single prefetch, not of the first addr but of
> > > the addr after PREFETCH_STRIDE would work best but only
> > > if length is > PREFETCH_STRIDE.
> > >
> > > I'd try:
> > >
> > > if (len > PREFETCH_STRIDE)
> > > prefetch(buf + PREFETCH_STRIDE);
> > > while (count64) {
> > > etc...
> > > }
> > >
> > > I still don't know how much that impacts very short lengths.
> > > Can you please add a 20 byte length to your tests?
> > Sure, I modified the code so that we only prefetched 2 cache lines ahead, but
> > only if the overall length of the input buffer is more than 2 cache lines.
> > Below are the results (all counts are the average of 1000000 iterations of the
> > csum operation, as previous tests were, I just omitted that column).
> >
> > len set cycles/byte cycles/byte improvement
> > no prefetch prefetch
> > ===========================================================
> > 20B 64MB 45.014989 44.402432 1.3%
> > 20B 128MB 44.900317 46.146447 -2.7%
> > 20B 256MB 45.303223 48.193623 -6.3%
> > 20B 512MB 45.615301 44.486872 2.2%
> []
> > I'm still left thinking we should just abandon the prefetch at this point and
> > keep the perf code until we have new instructions to help us with this further,
> > unless you see something I dont.
>
> I tend to agree but perhaps the 3% performance
> increase with a prefetch for longer lengths is
> actually significant and desirable.
>
Maybe, but I worry that its not going to be consistent. At least not with the
cost of the extra comparison and jump.
> It doesn't seem you've done the test I suggested
> where prefetch is done only for
> "len > PREFETCH_STRIDE".
>
No, thats exactly what I did, I did this:
#define PREFETCH_STRIDE (cache_line_size() * 2)
...
if (len > PREFETCH_STRIDE)
prefecth(buf + PREFETCH_STRIDE)
while (count64) {
...
> Is it ever useful to do a prefetch of the
> address/data being accessed by the next
> instruction?
>
Doubtful, you need to prefetch the data far enough in advance that its loaded by
the time you need to reference it. Otherwise you wind up stalling the data
pipeline while the load completes. So unless you have really fast memory, the
prefetch is effectively a no-op for the next access. But the next cacheline
ahead is good, as it prevents the stall there. Any more than that though (from
this testing), seems to again be a no-op as modern hardware automatically issues
the prefetch because it notices the linear data access pattern.
> Anyway, thanks for doing all the work.
>
No worries, glad to do it. Thanks for the review
Ingo, what do you think, shall I submit the perf bits as a separate thread, or
do you not want those any more?
Regards
Neil
> Joe
>
>
^ permalink raw reply
* Re: linux-next: manual merge of the random tree with the net-next tree
From: Stephen Rothwell @ 2013-11-12 20:46 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Theodore Ts'o, linux-next, linux-kernel, Hannes Frederic Sowa,
David Miller, netdev
In-Reply-To: <5281ED01.5020204@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1079 bytes --]
Hi Daniel,
On Tue, 12 Nov 2013 09:55:29 +0100 Daniel Borkmann <dborkman@redhat.com> wrote:
>
> On 11/12/2013 05:55 AM, Stephen Rothwell wrote:
> >
> > Today's linux-next merge of the random tree got a conflict in
> > drivers/char/random.c between commit 0244ad004a54 ("random32: add
> > prandom_reseed_late() and call when nonblocking pool becomes
> > initialized") from the net-next tree and commit 301f0595c0e7 ("random:
> > printk notifications for urandom pool initialization") from the random
> > tree.
> >
> > I fixed it up (probably not properly - see below) and can carry the fix
> > as necessary (no action is required).
>
> As per Hannes' suggestion, the result should look like (see cover
> letter in [1]):
>
> if (r->entropy_total > 128) {
> r->initialized = 1;
> r->entropy_total = 0;
> if (r == &nonblocking_pool) {
> prandom_reseed_late();
> pr_notice("random: %s pool is initialized\n",
> r->name);
> }
> }
I will use that from today. Thanks.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [Fwd: Re: [PATCH v2 2/2] x86: add prefetching to do_csum]
From: Joe Perches @ 2013-11-12 20:38 UTC (permalink / raw)
To: Neil Horman
Cc: netdev, Dave Jones, linux-kernel, sebastien.dugue,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Eric Dumazet
In-Reply-To: <20131112195005.GD19780@hmsreliant.think-freely.org>
On Tue, 2013-11-12 at 14:50 -0500, Neil Horman wrote:
> On Tue, Nov 12, 2013 at 09:33:35AM -0800, Joe Perches wrote:
> > On Tue, 2013-11-12 at 12:12 -0500, Neil Horman wrote:
[]
> > > So, the numbers are correct now that I returned my hardware to its previous
> > > interrupt affinity state, but the trend seems to be the same (namely that there
> > > isn't a clear one). We seem to find peak performance around a readahead of 2
> > > cachelines, but its very small (about 3%), and its inconsistent (larger set
> > > sizes fall to either side of that stride). So I don't see it as a clear win. I
> > > still think we should probably scrap the readahead for now, just take the perf
> > > bits, and revisit this when we can use the vector instructions or the
> > > independent carry chain instructions to improve this more consistently.
> > >
> > > Thoughts
> >
> > Perhaps a single prefetch, not of the first addr but of
> > the addr after PREFETCH_STRIDE would work best but only
> > if length is > PREFETCH_STRIDE.
> >
> > I'd try:
> >
> > if (len > PREFETCH_STRIDE)
> > prefetch(buf + PREFETCH_STRIDE);
> > while (count64) {
> > etc...
> > }
> >
> > I still don't know how much that impacts very short lengths.
> > Can you please add a 20 byte length to your tests?
> Sure, I modified the code so that we only prefetched 2 cache lines ahead, but
> only if the overall length of the input buffer is more than 2 cache lines.
> Below are the results (all counts are the average of 1000000 iterations of the
> csum operation, as previous tests were, I just omitted that column).
>
> len set cycles/byte cycles/byte improvement
> no prefetch prefetch
> ===========================================================
> 20B 64MB 45.014989 44.402432 1.3%
> 20B 128MB 44.900317 46.146447 -2.7%
> 20B 256MB 45.303223 48.193623 -6.3%
> 20B 512MB 45.615301 44.486872 2.2%
[]
> I'm still left thinking we should just abandon the prefetch at this point and
> keep the perf code until we have new instructions to help us with this further,
> unless you see something I dont.
I tend to agree but perhaps the 3% performance
increase with a prefetch for longer lengths is
actually significant and desirable.
It doesn't seem you've done the test I suggested
where prefetch is done only for
"len > PREFETCH_STRIDE".
Is it ever useful to do a prefetch of the
address/data being accessed by the next
instruction?
Anyway, thanks for doing all the work.
Joe
^ permalink raw reply
* Re: [RFC] Revert "sierra_net: keep status interrupt URB active"
From: Bjørn Mork @ 2013-11-12 20:29 UTC (permalink / raw)
To: Dan Williams
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA,
John Henderson
In-Reply-To: <1384277825.14773.1.camel-wKZy7rqYPVb5EHUCmHmTqw@public.gmane.org>
Dan Williams <dcbw-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> Actually, is "[PATCH] usbnet: fix status interrupt urb handling" the
>> real fix for this problem?
>>
>> John, any chance you could revert my RFC patch and try Felix's patch
>in
>> that mail?
>
>It fixes the problem for me without requiring the revert or my RFC
>patch....
Yes, this looks like a reasonable explanation. Good timing.
Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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: [Fwd: Re: [PATCH v2 2/2] x86: add prefetching to do_csum]
From: Neil Horman @ 2013-11-12 19:50 UTC (permalink / raw)
To: Joe Perches
Cc: netdev, Dave Jones, linux-kernel, sebastien.dugue,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Eric Dumazet
In-Reply-To: <1384277615.3665.10.camel@joe-AO722>
On Tue, Nov 12, 2013 at 09:33:35AM -0800, Joe Perches wrote:
> On Tue, 2013-11-12 at 12:12 -0500, Neil Horman wrote:
> > On Mon, Nov 11, 2013 at 05:42:22PM -0800, Joe Perches wrote:
> > > Hi again Neil.
> > >
> > > Forwarding on to netdev with a concern as to how often
> > > do_csum is used via csum_partial for very short headers
> > > and what impact any prefetch would have there.
> > >
> > > Also, what changed in your test environment?
> > >
> > > Why are the new values 5+% higher cycles/byte than the
> > > previous values?
> > >
> > > And here is the new table reformatted:
> > >
> > > len set iterations Readahead cachelines vs cycles/byte
> > > 1 2 3 4 6 10 20
> > > 1500B 64MB 1000000 1.4342 1.4300 1.4350 1.4350 1.4396 1.4315 1.4555
> > > 1500B 128MB 1000000 1.4312 1.4346 1.4271 1.4284 1.4376 1.4318 1.4431
> > > 1500B 256MB 1000000 1.4309 1.4254 1.4316 1.4308 1.4418 1.4304 1.4367
> > > 1500B 512MB 1000000 1.4534 1.4516 1.4523 1.4563 1.4554 1.4644 1.4590
> > > 9000B 64MB 1000000 0.8921 0.8924 0.8932 0.8949 0.8952 0.8939 0.8985
> > > 9000B 128MB 1000000 0.8841 0.8856 0.8845 0.8854 0.8861 0.8879 0.8861
> > > 9000B 256MB 1000000 0.8806 0.8821 0.8813 0.8833 0.8814 0.8827 0.8895
> > > 9000B 512MB 1000000 0.8838 0.8852 0.8841 0.8865 0.8846 0.8901 0.8865
> > > 64KB 64MB 1000000 0.8132 0.8136 0.8132 0.8150 0.8147 0.8149 0.8147
> > > 64KB 128MB 1000000 0.8013 0.8014 0.8013 0.8020 0.8041 0.8015 0.8033
> > > 64KB 256MB 1000000 0.7956 0.7959 0.7956 0.7976 0.7981 0.7967 0.7973
> > > 64KB 512MB 1000000 0.7934 0.7932 0.7937 0.7951 0.7954 0.7943 0.7948
> > >
> >
> >
> > There we go, thats better:
> > len set iterations Readahead cachelines vs cycles/byte
> > 1 2 3 4 5 10 20
> > 1500B 64MB 1000000 1.3638 1.3288 1.3464 1.3505 1.3586 1.3527 1.3408
> > 1500B 128MB 1000000 1.3394 1.3357 1.3625 1.3456 1.3536 1.3400 1.3410
> > 1500B 256MB 1000000 1.3773 1.3362 1.3419 1.3548 1.3543 1.3442 1.4163
> > 1500B 512MB 1000000 1.3442 1.3390 1.3434 1.3505 1.3767 1.3513 1.3820
> > 9000B 64MB 1000000 0.8505 0.8492 0.8521 0.8593 0.8566 0.8577 0.8547
> > 9000B 128MB 1000000 0.8507 0.8507 0.8523 0.8627 0.8593 0.8670 0.8570
> > 9000B 256MB 1000000 0.8516 0.8515 0.8568 0.8546 0.8549 0.8609 0.8596
> > 9000B 512MB 1000000 0.8517 0.8526 0.8552 0.8675 0.8547 0.8526 0.8621
> > 64KB 64MB 1000000 0.7679 0.7689 0.7688 0.7716 0.7714 0.7722 0.7716
> > 64KB 128MB 1000000 0.7683 0.7687 0.7710 0.7690 0.7717 0.7694 0.7703
> > 64KB 256MB 1000000 0.7680 0.7703 0.7688 0.7689 0.7726 0.7717 0.7713
> > 64KB 512MB 1000000 0.7692 0.7690 0.7701 0.7705 0.7698 0.7693 0.7735
> >
> >
> > So, the numbers are correct now that I returned my hardware to its previous
> > interrupt affinity state, but the trend seems to be the same (namely that there
> > isn't a clear one). We seem to find peak performance around a readahead of 2
> > cachelines, but its very small (about 3%), and its inconsistent (larger set
> > sizes fall to either side of that stride). So I don't see it as a clear win. I
> > still think we should probably scrap the readahead for now, just take the perf
> > bits, and revisit this when we can use the vector instructions or the
> > independent carry chain instructions to improve this more consistently.
> >
> > Thoughts
>
> Perhaps a single prefetch, not of the first addr but of
> the addr after PREFETCH_STRIDE would work best but only
> if length is > PREFETCH_STRIDE.
>
> I'd try:
>
> if (len > PREFETCH_STRIDE)
> prefetch(buf + PREFETCH_STRIDE);
> while (count64) {
> etc...
> }
>
> I still don't know how much that impacts very short lengths.
>
> Can you please add a 20 byte length to your tests?
>
>
Sure, I modified the code so that we only prefetched 2 cache lines ahead, but
only if the overall length of the input buffer is more than 2 cache lines.
Below are the results (all counts are the average of 1000000 iterations of the
csum operation, as previous tests were, I just omitted that column).
len set cycles/byte cycles/byte improvement
no prefetch prefetch
===========================================================
20B 64MB 45.014989 44.402432 1.3%
20B 128MB 44.900317 46.146447 -2.7%
20B 256MB 45.303223 48.193623 -6.3%
20B 512MB 45.615301 44.486872 2.2%
1500B 64MB 1.364365 1.332285 1.9%
1500B 128MB 1.373945 1.335907 1.4%
1500B 256MB 1.356971 1.339084 1.2%
1500B 512MB 1.351091 1.341431 0.7%
9000B 64MB 0.850966 0.851077 -0.1%
9000B 128MB 0.851013 0.850366 0.1%
9000B 256MB 0.854212 0.851033 0.3%
9000B 512MB 0.857346 0.851744 0.7%
64KB 64MB 0.768224 0.768450 ~0%
64KB 128MB 0.768706 0.768884 ~0%
64KB 256MB 0.768459 0.768445 ~0%
64KB 512MB 0.768808 0.769404 -0.1%
The 20 byte results seem to have a few outliers. I'm guessing the improvement
came from good fortune in that the random selection happened to hit on the same
range of numbers a few times over, so we hit already cached data. I would
expect them to run more slowly (as the 2 and 3 rows illustrate), since 20B is
less than the 128 bytes in 2 cachelines on my test system, and so all were doing
is adding in an additional comparison and jump per iteration. Our sweet spot is
the 1500 byte range, giving us a small performance boost, but that quickly gets
lost in the noise as the buffer size grows beyond that.
I'm still left thinking we should just abandon the prefetch at this point and
keep the perf code until we have new instructions to help us with this further,
unless you see something I dont.
Neil
^ 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