* Re: [PATCH net-next 2/3] tipc: message reassembly using fragment chain
From: David Miller @ 2013-10-29 3:49 UTC (permalink / raw)
To: ying.xue
Cc: maloy, jon.maloy, paul.gortmaker, erik.hugne, tipc-discussion,
netdev
In-Reply-To: <526F15E8.9020009@windriver.com>
From: Ying Xue <ying.xue@windriver.com>
Date: Tue, 29 Oct 2013 09:56:56 +0800
> Therefore, the best method is to divide tipc_recv_msg() into several
> smaller functions to simplify the current implementation. But it's not
> an easy job. Actually I ever tried to do it, but I gave up lastly
> because I did not find one perfect solution about how to divide it.
But you're going to have to find a way, this indent level is out of
control.
^ permalink raw reply
* Re: [PATCH net-next] virtio_net: migrate mergeable rx buffers to page frag allocators
From: David Miller @ 2013-10-29 3:57 UTC (permalink / raw)
To: eric.dumazet
Cc: ffusco, mwdalton, mst, netdev, dborkman, virtualization, edumazet
In-Reply-To: <1383002389.4344.7.camel@edumazet-glaptop.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 28 Oct 2013 16:19:49 -0700
> On Mon, 2013-10-28 at 15:44 -0700, Michael Dalton wrote:
>> The virtio_net driver's mergeable receive buffer allocator
>> uses 4KB packet buffers. For MTU-sized traffic, SKB truesize
>> is > 4KB but only ~1500 bytes of the buffer is used to store
>> packet data, reducing the effective TCP window size
>> substantially. This patch addresses the performance concerns
>> with mergeable receive buffers by allocating MTU-sized packet
>> buffers using page frag allocators. If more than MAX_SKB_FRAGS
>> buffers are needed, the SKB frag_list is used.
>>
>> Signed-off-by: Michael Dalton <mwdalton@google.com>
>> ---
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
>
> Daniel & Francesco, this should address the performance problem you
> tried to address with ("tcp: rcvbuf autotuning improvements")
>
> ( http://www.spinics.net/lists/netdev/msg252642.html )
Applied, thanks everyone.
^ permalink raw reply
* Re: [BUG] 3.12.0-rcX IPv6 panic
From: Bob Tracy @ 2013-10-29 4:00 UTC (permalink / raw)
To: linux-kernel, netdev
In-Reply-To: <20131021234041.GA12350@gherkin.frus.com>
On Mon, Oct 21, 2013 at 06:40:41PM -0500, Bob Tracy wrote:
> On Mon, Oct 21, 2013 at 05:52:52PM +0200, Hannes Frederic Sowa wrote:
> > On Mon, Oct 21, 2013 at 08:18:46AM -0500, Bob Tracy wrote:
> > > Actually, a regression: the 3.11 kernel is rock-solid stable on my
> > > Alpha.
> > >
> > > Beginning with 3.12.0-rc1, I can reliably trigger a kernel panic by
> > > executing the gogo6.net "gw6c" IPv6 client program. If the networking
> > > layer is active, an "Oops" will eventually (within a day) occur regardless
> > > of whether I attempt to run "gw6c". 3.12.0-rcX is stable as long as I
> > > leave networking completely disabled. The error has persisted up through
> > > -rc6. Apologies for not mentioning this earlier, but the state of my
> > > PWS-433au has been questionable, and I wanted to make sure I had a
> > > legitimate bug sighting.
> > >
> > > I'll have to transcribe the panic backtrace by hand: nothing makes it
> > > into any of the system logs :-(. I *can* recall that every backtrace
> > > I've seen thus far has included one of the skb_copy() variants near the
> > > top of the list (first or second function).
> >
> > Try to capture the panic via serial console. Otherwise a picture
> > would give us a first hint. Please watch out for lines like
> > skb_(over|under)_panic.
>
> I'll get a screen capture of some kind for you within the next day or
> so.
>
> > gw6c is a tunnel client? Can you post ip -6 tunnel ls?
>
> Assuming you meant "show [NAME]" (no "ls" option for the tunnel object),
> that yields the following with "gw6c" running on a 3.11.0 kernel:
>
> smirkin:~# ip -6 tunnel show sit1
> sit1: any/ipv6 remote 4056:5874:: local 4500:0:0:4000:4029:: encaplimit 0 hoplimit 0 tclass 0x00 flowlabel 0x00000 (flowinfo 0x00000000)
>
> I'm running the gw6c client in gateway mode: the Alpha is my IPv6
> gateway/firewall.
Update: no significant change for 3.12.0-rc7. Can still reliably panic
the system by running "gw6c" to set up the IPv6 tunnel. Here's a hand-
transcribed backtrace: I hope it's sufficient... I would have included
the stack/register dump, but about half of it had scrolled off the top
of the screen.
skb_copy_and_csum_bits+0x88/0x380
icmp_glue_bits+0x48/0xce0
__ip_append_data+0x8f4/0xb40
ip_append_data+0xb0/0x130
icmp_glue_bits+0x0/0xe0
icmp_glue_bits+0x0/0xe0 (yes: repeated once)
icmp_push_reply+0x6c/0x190
icmp_send+0x3fc/0x4c0
ip_local_deliver_finish+0x20c/0x2e0
ip_rcv_finish+0x1d8/0x3d0
nf_ct_attach+0x32/0x40
ip_rcv_finish_0x148/0x3d0
__netif_receive_skb_core+0x27c/0x890
process_backlog+0xb8/0x1a0
net_rx_action+0xc8/0x210
__do_softirq+0x1a0/0x230
do_softirq+0x5c/0x70
irq_exit+0x68/0x80
handle_irq+0x90/0xf0
miata_srm_device_interrupt+0x30/0x50
do_entInt+0x1cc/0x1f0
__do_fault+0x3e0/0x5e0
ret_from_sys_call+0x0/0x10
entMM+0x9c/0xc0
do_page_fault+0x0/0x500
do_page_fault+0x48/0x500
entMM+0x9c/0xc0
filp_close+0x6c/0xe0
filp_close+0x98/0xe0
filp_close+0x6c/0xe0
filp_close+0x98/0xe0
__close_fd+0xb8/0xe0
Code: 44640003 e4600010 203ffff2 a77de680 b67e0010 47f10410 <b0340000> 47ff0411
--Bob
^ permalink raw reply
* Re: [PATCH net-next] WD80x3: fix printk() calls to netdev_*() calls
From: David Miller @ 2013-10-29 4:00 UTC (permalink / raw)
To: quantumdude836; +Cc: netdev
In-Reply-To: <1382738165-27852-1-git-send-email-quantumdude836@gmail.com>
From: Drew McGowen <quantumdude836@gmail.com>
Date: Fri, 25 Oct 2013 21:56:05 +0000
> - printk("%s: WD80x3 at %#3x, %pM",
> - dev->name, ioaddr, dev->dev_addr);
> + netdev_info(dev, "WD80x3 at %#3x, %pM",
> + ioaddr, dev->dev_addr);
This is incorrect indentation.
On a multi-line function call, the arguments one the second and subsequent
lines must start at exactly the first column after the openning parenthesis
on the first line.
Everyone who makes this mistake is trying to be lazy and use only TAB
characters to indent their code, don't do this. You must use the
appropriate number of TAB _and_ SPACE characters necessary to achieve
the correct indentation.
^ permalink raw reply
* Re: [PATCH net-next] 3c515: Fix warning when building
From: David Miller @ 2013-10-29 4:03 UTC (permalink / raw)
To: ricec2; +Cc: netdev
In-Reply-To: <1382738386-29888-1-git-send-email-ricec2@rpi.edu>
From: Colin Rice <ricec2@rpi.edu>
Date: Fri, 25 Oct 2013 21:59:46 +0000
> - outl((int) (skb->data), ioaddr + Wn7_MasterAddr);
> + outl((size_t) (skb->data), ioaddr + Wn7_MasterAddr);
outl() takes an "int" not a "size_t". You're avoiding the warning,
but on 64-bit, for example, you're silently allowing the compiler
to accept chopping off the top 32-bits of the 64-bit pointer address
off.
The warning is valid, this code won't work in certain environments,
and killing the warning is just papering over the problem such that
it will never get addressed properly.
I'm not applying patches like this, sorry.
^ permalink raw reply
* Re: [PATCH net-next] tcp: gso: fix truesize tracking
From: David Miller @ 2013-10-29 4:05 UTC (permalink / raw)
To: eric.dumazet; +Cc: ast, edumazet, stephen, netdev
In-Reply-To: <1382747177.4032.21.camel@edumazet-glaptop.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 25 Oct 2013 17:26:17 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> commit 6ff50cd55545 ("tcp: gso: do not generate out of order packets")
> had an heuristic that can trigger a warning in skb_try_coalesce(),
> because skb->truesize of the gso segments were exactly set to mss.
>
> This breaks the requirement that
>
> skb->truesize >= skb->len + truesizeof(struct sk_buff);
>
> It can trivially be reproduced by :
>
> ifconfig lo mtu 1500
> ethtool -K lo tso off
> netperf
>
> As the skbs are looped into the TCP networking stack, skb_try_coalesce()
> warns us of these skb under-estimating their truesize.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Alexei Starovoitov <ast@plumgrid.com>
I decided to apply this to 'net' and queue it up for -stable,
thanks Eric!
^ permalink raw reply
* Re: [PATCH net-next] ipv4: introduce new IP_MTU_DISCOVER mode IP_PMTUDISC_INTERFACE
From: David Miller @ 2013-10-29 4:08 UTC (permalink / raw)
To: hannes; +Cc: netdev, fweimer
In-Reply-To: <20131026201158.GI15744@order.stressinduktion.org>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Sat, 26 Oct 2013 22:11:58 +0200
> Sockets marked with IP_PMTUDISC_INTERFACE won't do path mtu discovery,
> their sockets won't accept and install new path mtu information and they
> will always use the interface mtu for outgoing packets. It is guaranteed
> that the packet is not fragmented locally. But we won't set the DF-Flag
> on the outgoing frames.
>
> Florian Weimer had the idea to use this flag to ensure DNS servers are
> never generating outgoing fragments. They may well be fragmented on the
> path, but the server never stores or usees path mtu values, which could
> well be forged in an attack.
>
> (The root of the problem with path MTU discovery is that there is
> no reliable way to authenticate ICMP Fragmentation Needed But DF Set
> messages because they are sent from intermediate routers with their
> source addresses, and the IMCP payload will not always contain sufficient
> information to identify a flow.)
I do not like this reasoning. You have several more acceptable paths to take
to resolve this problem:
1) "I don't trust path MTU information at all"
Just turn it off globally, end of story. It has the same effect as your
new per-application mode.
2) "I don't trust path MTU information unless the full socket ID is available
in the ICMP packets quoted headers"
Then simply implement a policy as such and submit it to me.
^ permalink raw reply
* Re: [PATCH net 0/2] bnx2x: Bug fixes patch series
From: David Miller @ 2013-10-29 4:13 UTC (permalink / raw)
To: yuvalmin; +Cc: netdev, dmitry, ariele, eilong
In-Reply-To: <1382872008-1073-1-git-send-email-yuvalmin@broadcom.com>
From: "Yuval Mintz" <yuvalmin@broadcom.com>
Date: Sun, 27 Oct 2013 13:06:47 +0200
> This series contains 2 unrelated fixes:
>
> 1. FW asserts during unload might be encountered if specific memory
> allocations fail during load.
>
> 2. SR-IOV related, fixes a crash that will occour if the driver were to
> be rmmoded and then re-probed while there are both assigned and unassigned
> VFs originating from the same PF.
>
> Please consider applying these patches to `net'.
Both applied, thanks.
^ permalink raw reply
* Re: [PATCH net] cxgb3: Fix length calculation in write_ofld_wr() on 32-bit architectures
From: David Miller @ 2013-10-29 4:14 UTC (permalink / raw)
To: ben; +Cc: divy, netdev
In-Reply-To: <1382907759.2994.36.camel@deadeye.wl.decadent.org.uk>
From: Ben Hutchings <ben@decadent.org.uk>
Date: Sun, 27 Oct 2013 21:02:39 +0000
> The length calculation here is now invalid on 32-bit architectures,
> since sk_buff::tail is a pointer and sk_buff::transport_header is
> an integer offset:
>
> drivers/net/ethernet/chelsio/cxgb3/sge.c: In function 'write_ofld_wr':
> drivers/net/ethernet/chelsio/cxgb3/sge.c:1603:9: warning: passing argument 4 of 'make_sgl' makes integer from pointer without a cast [enabled by default]
> adap->pdev);
> ^
> drivers/net/ethernet/chelsio/cxgb3/sge.c:964:28: note: expected 'unsigned int' but argument is of type 'sk_buff_data_t'
> static inline unsigned int make_sgl(const struct sk_buff *skb,
> ^
>
> Use the appropriate skb accessor functions.
>
> Compile-tested only.
>
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> Fixes: 1a37e412a022 ('net: Use 16bits for *_headers fields of struct skbuff')
> ---
> This is needed for 3.11-stable.
Applied and queued up for -stable, thanks Ben.
^ permalink raw reply
* Re: [PATCH net-next v2] ipv4: fix DO and PROBE pmtu mode regarding local fragmentation with UFO/CORK
From: David Miller @ 2013-10-29 4:16 UTC (permalink / raw)
To: hannes; +Cc: netdev
In-Reply-To: <20131027162911.GA4714@order.stressinduktion.org>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Sun, 27 Oct 2013 17:29:11 +0100
> UFO as well as UDP_CORK do not respect IP_PMTUDISC_DO and
> IP_PMTUDISC_PROBE well enough.
>
> UFO enabled packet delivery just appends all frags to the cork and hands
> it over to the network card. So we just deliver non-DF udp fragments
> (DF-flag may get overwritten by hardware or virtual UFO enabled
> interface).
>
> UDP_CORK does enqueue the data until the cork is disengaged. At this
> point it sets the correct IP_DF and local_df flags and hands it over to
> ip_fragment which in this case will generate an icmp error which gets
> appended to the error socket queue. This is not reflected in the syscall
> error (of course, if UFO is enabled this also won't happen).
>
> Improve this by checking the pmtudisc flags before appending data to the
> socket and if we still can fit all data in one packet when IP_PMTUDISC_DO
> or IP_PMTUDISC_PROBE is set, only then proceed.
>
> We use (mtu-fragheaderlen) to check for the maximum length because we
> ensure not to generate a fragment and non-fragmented data does not need
> to have its length aligned on 64 bit boundaries. Also the passed in
> ip_options are already aligned correctly.
>
> Maybe, we can relax some other checks around ip_fragment. This needs
> more research.
>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
> v2:
> Switch from maxfraglen to mtu for length check as outlined in the commit
> message.
Looks good, applied.
^ permalink raw reply
* Re: [PATCH v2 1/2] net/benet: Remove interface type
From: David Miller @ 2013-10-29 4:17 UTC (permalink / raw)
To: shangw; +Cc: netdev, Sathya.Perla
In-Reply-To: <1382926367-14968-1-git-send-email-shangw@linux.vnet.ibm.com>
These two patches don't apply cleanly to net-next and that's where I
intend to apply them.
Please respin and resubmit, thanks.
^ permalink raw reply
* Re: [PATCH v2 1/3] vxlan: silence one build warning
From: David Miller @ 2013-10-29 4:19 UTC (permalink / raw)
To: zwu.kernel; +Cc: netdev, linux-kernel, stephen, wuzhy
In-Reply-To: <1382940110-10737-1-git-send-email-zwu.kernel@gmail.com>
From: Zhi Yong Wu <zwu.kernel@gmail.com>
Date: Mon, 28 Oct 2013 14:01:48 +0800
> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>
> drivers/net/vxlan.c: In function ‘vxlan_sock_add’:
> drivers/net/vxlan.c:2298:11: warning: ‘sock’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> drivers/net/vxlan.c:2275:17: note: ‘sock’ was declared here
> LD drivers/net/built-in.o
>
> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH v2 2/3] net, datagram: fix the incorrect comment in zerocopy_sg_from_iovec()
From: David Miller @ 2013-10-29 4:19 UTC (permalink / raw)
To: zwu.kernel; +Cc: netdev, linux-kernel, stephen, wuzhy
In-Reply-To: <1382940110-10737-2-git-send-email-zwu.kernel@gmail.com>
From: Zhi Yong Wu <zwu.kernel@gmail.com>
Date: Mon, 28 Oct 2013 14:01:49 +0800
> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>
> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH v2 3/3] net, iovec: fix the incorrect comment in memcpy_fromiovecend()
From: David Miller @ 2013-10-29 4:19 UTC (permalink / raw)
To: zwu.kernel; +Cc: netdev, linux-kernel, stephen, wuzhy
In-Reply-To: <1382940110-10737-3-git-send-email-zwu.kernel@gmail.com>
From: Zhi Yong Wu <zwu.kernel@gmail.com>
Date: Mon, 28 Oct 2013 14:01:50 +0800
> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>
> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
Applied to net-next
^ permalink raw reply
* Re: [PATCH] net, mc: fix the incorrect comments in two mc-related functions
From: David Miller @ 2013-10-29 4:19 UTC (permalink / raw)
To: zwu.kernel; +Cc: netdev, linux-kernel, wuzhy
In-Reply-To: <1382948150-14349-1-git-send-email-zwu.kernel@gmail.com>
From: Zhi Yong Wu <zwu.kernel@gmail.com>
Date: Mon, 28 Oct 2013 16:15:50 +0800
> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>
> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH 0/5] r8152 bug fixes
From: David Miller @ 2013-10-29 4:23 UTC (permalink / raw)
To: hayeswang; +Cc: netdev, nic_swsd, linux-kernel, linux-usb
In-Reply-To: <1382961494-2272-1-git-send-email-hayeswang@realtek.com>
From: Hayes Wang <hayeswang@realtek.com>
Date: Mon, 28 Oct 2013 19:58:09 +0800
> These could fix some driver issues.
>
> Hayes Wang (5):
> net/usb/r8152: fix tx/rx memory overflow
> net/usb/r8152: make sure the tx checksum setting is correct
> net/usb/r8152: modify the tx flow
> net/usb/r8152: fix incorrect type in assignment
> net/usb/r8152: code adjust
These are not bug fixes, some of them are just cleanups.
It is inappropriate to mix real bug fixes and non-bug fixes into
a patch series.
You must send purely the bug fixes for 'net' tree, and then later
the code cleanups and other changes targetting the 'net-next' tree.
Also, from patch #5:
====================
-Something else
====================
That is completely unacceptable. You must say what changes you are
making, the above says nothing to me nor the person reading your
commit messages in the future.
In general, your commit messages are poorly written in that they
contain not enough information for the person trying to understand
your patches at all.
^ permalink raw reply
* Re: [PATCH net V3] xen-netback: use jiffies_64 value to calculate credit timeout
From: David Miller @ 2013-10-29 4:25 UTC (permalink / raw)
To: wei.liu2
Cc: xen-devel, netdev, david.vrabel, jbeulich, annie.li, ian.campbell,
jianhai.luan
In-Reply-To: <1382962077-13406-1-git-send-email-wei.liu2@citrix.com>
From: Wei Liu <wei.liu2@citrix.com>
Date: Mon, 28 Oct 2013 12:07:57 +0000
> time_after_eq() only works if the delta is < MAX_ULONG/2.
>
> For a 32bit Dom0, if netfront sends packets at a very low rate, the time
> between subsequent calls to tx_credit_exceeded() may exceed MAX_ULONG/2
> and the test for timer_after_eq() will be incorrect. Credit will not be
> replenished and the guest may become unable to send packets (e.g., if
> prior to the long gap, all credit was exhausted).
>
> Use jiffies_64 variant to mitigate this problem for 32bit Dom0.
>
> Suggested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Reviewed-by: David Vrabel <david.vrabel@citrix.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net-next 2/3] tipc: message reassembly using fragment chain
From: Ying Xue @ 2013-10-29 5:36 UTC (permalink / raw)
To: David Miller, maloy, jon.maloy
Cc: paul.gortmaker, erik.hugne, tipc-discussion, netdev
In-Reply-To: <20131028.234953.1884858499661587979.davem@davemloft.net>
On 10/29/2013 11:49 AM, David Miller wrote:
> From: Ying Xue <ying.xue@windriver.com>
> Date: Tue, 29 Oct 2013 09:56:56 +0800
>
>> Therefore, the best method is to divide tipc_recv_msg() into several
>> smaller functions to simplify the current implementation. But it's not
>> an easy job. Actually I ever tried to do it, but I gave up lastly
>> because I did not find one perfect solution about how to divide it.
>
> But you're going to have to find a way, this indent level is out of
> control.
>
>
By simply adjusting the order of branch blocks, this is what I can figure out the simplest
method to resolve the issue now :)
Please review below change. If it's fine, I will send one formal patch.
(Below change still follows original code logic)
diff --git a/net/tipc/link.c b/net/tipc/link.c
index 12c72d2..6c88dbc 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -1592,97 +1592,94 @@ void tipc_recv_msg(struct sk_buff *head, struct tipc_bearer *b_ptr)
/* Now (finally!) process the incoming message */
protocol_check:
- if (likely(link_working_working(l_ptr))) {
- if (likely(seq_no == mod(l_ptr->next_in_no))) {
- l_ptr->next_in_no++;
- if (unlikely(l_ptr->oldest_deferred_in))
- head = link_insert_deferred_queue(l_ptr,
- head);
-deliver:
- if (likely(msg_isdata(msg))) {
- tipc_node_unlock(n_ptr);
- tipc_port_recv_msg(buf);
- continue;
- }
- switch (msg_user(msg)) {
- int ret;
- case MSG_BUNDLER:
- l_ptr->stats.recv_bundles++;
- l_ptr->stats.recv_bundled +=
- msg_msgcnt(msg);
- tipc_node_unlock(n_ptr);
- tipc_link_recv_bundle(buf);
- continue;
- case NAME_DISTRIBUTOR:
- n_ptr->bclink.recv_permitted = true;
- tipc_node_unlock(n_ptr);
- tipc_named_recv(buf);
- continue;
- case BCAST_PROTOCOL:
- tipc_link_recv_sync(n_ptr, buf);
- tipc_node_unlock(n_ptr);
- continue;
- case CONN_MANAGER:
- tipc_node_unlock(n_ptr);
- tipc_port_recv_proto_msg(buf);
- continue;
- case MSG_FRAGMENTER:
- l_ptr->stats.recv_fragments++;
- ret = tipc_link_recv_fragment(
- &l_ptr->defragm_buf,
- &buf, &msg);
- if (ret == 1) {
- l_ptr->stats.recv_fragmented++;
- goto deliver;
- }
- if (ret == -1)
- l_ptr->next_in_no--;
- break;
- case CHANGEOVER_PROTOCOL:
- type = msg_type(msg);
- if (link_recv_changeover_msg(&l_ptr,
- &buf)) {
- msg = buf_msg(buf);
- seq_no = msg_seqno(msg);
- if (type == ORIGINAL_MSG)
- goto deliver;
- goto protocol_check;
- }
- break;
- default:
- kfree_skb(buf);
- buf = NULL;
- break;
- }
+ if (unlikely(!link_working_working(l_ptr))) {
+ if (msg_user(msg) == LINK_PROTOCOL) {
+ link_recv_proto_msg(l_ptr, buf);
+ head = link_insert_deferred_queue(l_ptr, head);
+ tipc_node_unlock(n_ptr);
+ continue;
+ }
+
+ /* Traffic message. Conditionally activate link */
+ link_state_event(l_ptr, TRAFFIC_MSG_EVT);
+
+ if (link_working_working(l_ptr)) {
+ /* Re-insert buffer in front of queue */
+ buf->next = head;
+ head = buf;
tipc_node_unlock(n_ptr);
- tipc_net_route_msg(buf);
continue;
}
+ tipc_node_unlock(n_ptr);
+ goto cont;
+ }
+
+ /* Link is now in state WORKING_WORKING */
+ if (unlikely(seq_no != mod(l_ptr->next_in_no))) {
link_handle_out_of_seq_msg(l_ptr, buf);
head = link_insert_deferred_queue(l_ptr, head);
tipc_node_unlock(n_ptr);
continue;
}
-
- /* Link is not in state WORKING_WORKING */
- if (msg_user(msg) == LINK_PROTOCOL) {
- link_recv_proto_msg(l_ptr, buf);
+ l_ptr->next_in_no++;
+ if (unlikely(l_ptr->oldest_deferred_in))
head = link_insert_deferred_queue(l_ptr, head);
+deliver:
+ if (likely(msg_isdata(msg))) {
tipc_node_unlock(n_ptr);
+ tipc_port_recv_msg(buf);
continue;
}
-
- /* Traffic message. Conditionally activate link */
- link_state_event(l_ptr, TRAFFIC_MSG_EVT);
-
- if (link_working_working(l_ptr)) {
- /* Re-insert buffer in front of queue */
- buf->next = head;
- head = buf;
+ switch (msg_user(msg)) {
+ int ret;
+ case MSG_BUNDLER:
+ l_ptr->stats.recv_bundles++;
+ l_ptr->stats.recv_bundled += msg_msgcnt(msg);
+ tipc_node_unlock(n_ptr);
+ tipc_link_recv_bundle(buf);
+ continue;
+ case NAME_DISTRIBUTOR:
+ n_ptr->bclink.recv_permitted = true;
+ tipc_node_unlock(n_ptr);
+ tipc_named_recv(buf);
+ continue;
+ case BCAST_PROTOCOL:
+ tipc_link_recv_sync(n_ptr, buf);
+ tipc_node_unlock(n_ptr);
+ continue;
+ case CONN_MANAGER:
tipc_node_unlock(n_ptr);
+ tipc_port_recv_proto_msg(buf);
continue;
+ case MSG_FRAGMENTER:
+ l_ptr->stats.recv_fragments++;
+ ret = tipc_link_recv_fragment(&l_ptr->defragm_buf,
+ &buf, &msg);
+ if (ret == 1) {
+ l_ptr->stats.recv_fragmented++;
+ goto deliver;
+ }
+ if (ret == -1)
+ l_ptr->next_in_no--;
+ break;
+ case CHANGEOVER_PROTOCOL:
+ type = msg_type(msg);
+ if (link_recv_changeover_msg(&l_ptr, &buf)) {
+ msg = buf_msg(buf);
+ seq_no = msg_seqno(msg);
+ if (type == ORIGINAL_MSG)
+ goto deliver;
+ goto protocol_check;
+ }
+ break;
+ default:
+ kfree_skb(buf);
+ buf = NULL;
+ break;
}
tipc_node_unlock(n_ptr);
+ tipc_net_route_msg(buf);
+ continue;
cont:
kfree_skb(buf);
}
Regards,
Ying
^ permalink raw reply related
* Re: [PATCH 00/19] Enable various Renesas drivers on all ARM platforms
From: Simon Horman @ 2013-10-29 6:04 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-fbdev, Wolfram Sang, Linus Walleij, Guennadi Liakhovetski,
Thierry Reding, linux-mtd, linux-i2c, Vinod Koul, Joerg Roedel,
linux-sh, Magnus Damm, Eduardo Valentin, Tomi Valkeinen,
linux-serial, linux-input, Zhang Rui, Chris Ball,
Jean-Christophe Plagniol-Villard, linux-media, linux-pwm,
Samuel Ortiz, linux-pm, Ian Molton, Mark Brown, linux-arm-kernel,
Ser
In-Reply-To: <1383004027-25036-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com>
On Tue, Oct 29, 2013 at 12:46:48AM +0100, Laurent Pinchart wrote:
> Hello,
>
> This patch series, based on v3.12-rc7, prepares various Renesas drivers
> for migration to multiplatform kernels by enabling their compilation or
> otherwise fixing them on all ARM platforms. The patches are pretty
> straightforward and are described in their commit message.
>
> I'd like to get all these patches merged in v3.14. As they will need to go
> through their respective subsystems' trees, I would appreciate if all
> maintainers involved could notify me when they merge patches from this series
> in their tree to help me tracking the merge status. I don't plan to send pull
> requests individually for these patches, and I will repost patches
> individually if changes are requested during review.
>
> If you believe the issue should be solved in a different way (for instance by
> removing the architecture dependency completely) please reply to the cover
> letter to let other maintainers chime in.
I think this is a step in a good direction.
However, I think it would be even better if the architecture dependency was
removed completely.
>
> Cc: Chris Ball <cjb@laptop.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: dmaengine@vger.kernel.org
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Eduardo Valentin <eduardo.valentin@ti.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> Cc: Ian Molton <ian@mnementh.co.uk>
> Cc: iommu@lists.linux-foundation.org
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: linux-fbdev@vger.kernel.org
> Cc: linux-i2c@vger.kernel.org
> Cc: linux-input@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-media@vger.kernel.org
> Cc: linux-mmc@vger.kernel.org
> Cc: linux-mtd@lists.infradead.org
> Cc: linux-pm@vger.kernel.org
> Cc: linux-pwm@vger.kernel.org
> Cc: linux-serial@vger.kernel.org
> Cc: linux-spi@vger.kernel.org
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
> Cc: netdev@vger.kernel.org
> Cc: Samuel Ortiz <samuel@sortiz.org>
> Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Vinod Koul <vinod.koul@intel.com>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Cc: Zhang Rui <rui.zhang@intel.com>
>
> Laurent Pinchart (19):
> serial: sh-sci: Enable the driver on all ARM platforms
> DMA: shdma: Enable the driver on all ARM platforms
> i2c: sh_mobile: Enable the driver on all ARM platforms
> input: sh_keysc: Enable the driver on all ARM platforms
> iommu: shmobile: Enable the driver on all ARM platforms
> i2c: rcar: Enable the driver on all ARM platforms
> v4l: sh_vou: Enable the driver on all ARM platforms
> mmc: sdhi: Enable the driver on all ARM platforms
> mmc: sh_mmcif: Enable the driver on all ARM platforms
> mtd: sh_flctl: Enable the driver on all ARM platforms
> net: sh_eth: Set receive alignment correctly on all ARM platforms
> irda: sh_irda: Enable the driver on all ARM platforms
> pinctrl: sh-pfc: Enable the driver on all ARM platforms
> pwm: pwm-renesas-tpu: Enable the driver on all ARM platforms
> sh: intc: Enable the driver on all ARM platforms
> spi: sh_msiof: Enable the driver on all ARM platforms
> spi: sh_hspi: Enable the driver on all ARM platforms
> thermal: rcar-thermal: Enable the driver on all ARM platforms
> fbdev: sh-mobile-lcdcfb: Enable the driver on all ARM platforms
>
> drivers/dma/sh/Kconfig | 2 +-
> drivers/dma/sh/shdmac.c | 6 +++---
> drivers/i2c/busses/Kconfig | 4 ++--
> drivers/input/keyboard/Kconfig | 2 +-
> drivers/iommu/Kconfig | 2 +-
> drivers/media/platform/Kconfig | 2 +-
> drivers/mmc/host/Kconfig | 4 ++--
> drivers/mmc/host/tmio_mmc_dma.c | 2 +-
> drivers/mtd/nand/Kconfig | 2 +-
> drivers/net/ethernet/renesas/sh_eth.c | 2 +-
> drivers/net/ethernet/renesas/sh_eth.h | 2 +-
> drivers/net/irda/Kconfig | 2 +-
> drivers/pinctrl/Makefile | 2 +-
> drivers/pinctrl/sh-pfc/Kconfig | 2 +-
> drivers/pwm/Kconfig | 2 +-
> drivers/sh/intc/Kconfig | 2 +-
> drivers/spi/Kconfig | 4 ++--
> drivers/thermal/Kconfig | 2 +-
> drivers/tty/serial/Kconfig | 2 +-
> drivers/video/Kconfig | 6 +++---
> 20 files changed, 27 insertions(+), 27 deletions(-)
>
> --
> Regards,
>
> Laurent Pinchart
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* Re: [PATCH net-next] virtio_net: migrate mergeable rx buffers to page frag allocators
From: Jason Wang @ 2013-10-29 6:27 UTC (permalink / raw)
To: Michael Dalton, David S. Miller
Cc: Michael S. Tsirkin, netdev, Daniel Borkmann, virtualization,
Eric Dumazet
In-Reply-To: <1383000258-1458-1-git-send-email-mwdalton@google.com>
On 10/29/2013 06:44 AM, Michael Dalton wrote:
> The virtio_net driver's mergeable receive buffer allocator
> uses 4KB packet buffers. For MTU-sized traffic, SKB truesize
> is > 4KB but only ~1500 bytes of the buffer is used to store
> packet data, reducing the effective TCP window size
> substantially. This patch addresses the performance concerns
> with mergeable receive buffers by allocating MTU-sized packet
> buffers using page frag allocators. If more than MAX_SKB_FRAGS
> buffers are needed, the SKB frag_list is used.
>
> Signed-off-by: Michael Dalton <mwdalton@google.com>
Do you have any perf numberf of this patch? Have a glance of the patch,
looks like it will hurt the performance of large GSO packet. Always
allocating 1500 bytes mean a virtqueue can only hold about 4 full size
gso packets, and frag list will introduce extra overheads on skb
allocation. I just test it on my desktop, performance of full size gso
packet drops about 10%.
Mergeable buffer is a good balance between performance and the space it
occupies. If you just want a ~1500 bytes packet to be received, you can
just disable the mergeable buffer and just use small packet.
Alternatively, a simpler way is just use build_skb() and head frag to
build the skb directly on the page for medium size packets (small
packets were still copied) like following patch (may need more work
since vnet header is too short for NET_SKB_PAD). This can reduce the
truesize while keep the performance for large GSO packet.
---
drivers/net/virtio_net.c | 48
++++++++++++++++++++++++++++++---------------
1 files changed, 32 insertions(+), 16 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 3c23fdc..4c7ad89 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -239,16 +239,12 @@ static struct sk_buff *page_to_skb(struct
receive_queue *rq,
struct skb_vnet_hdr *hdr;
unsigned int copy, hdr_len, offset;
char *p;
+ int skb_size = SKB_DATA_ALIGN(len) +
+ SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+ bool frag;
p = page_address(page);
- /* copy small packet so we can reuse these pages for small data */
- skb = netdev_alloc_skb_ip_align(vi->dev, GOOD_COPY_LEN);
- if (unlikely(!skb))
- return NULL;
-
- hdr = skb_vnet_hdr(skb);
-
if (vi->mergeable_rx_bufs) {
hdr_len = sizeof hdr->mhdr;
offset = hdr_len;
@@ -257,18 +253,38 @@ static struct sk_buff *page_to_skb(struct
receive_queue *rq,
offset = sizeof(struct padded_vnet_hdr);
}
- memcpy(hdr, p, hdr_len);
-
len -= hdr_len;
p += offset;
- copy = len;
- if (copy > skb_tailroom(skb))
- copy = skb_tailroom(skb);
- memcpy(skb_put(skb, copy), p, copy);
+ frag = (len > GOOD_COPY_LEN) && (skb_size <= PAGE_SIZE) &&
+ vi->mergeable_rx_bufs;
+ if (frag) {
+ skb = build_skb(page_address(page), skb_size);
+ if (unlikely(!skb))
+ return NULL;
+
+ skb_reserve(skb, offset);
+ skb_put(skb, len);
+ len = 0;
+ } else {
+ /* copy small packet so we can reuse these pages for small data
+ */
+ skb = netdev_alloc_skb_ip_align(vi->dev, GOOD_COPY_LEN);
+ if (unlikely(!skb))
+ return NULL;
+
+ copy = len;
+ if (copy > skb_tailroom(skb))
+ copy = skb_tailroom(skb);
+ memcpy(skb_put(skb, copy), p, copy);
+
+ len -= copy;
+ offset += copy;
+ }
+
+ hdr = skb_vnet_hdr(skb);
- len -= copy;
- offset += copy;
+ memcpy(hdr, page_address(page), hdr_len);
/*
* Verify that we can indeed put this data into a skb.
@@ -288,7 +304,7 @@ static struct sk_buff *page_to_skb(struct
receive_queue *rq,
offset = 0;
}
- if (page)
+ if (page && !frag)
give_pages(rq, page);
return skb;
--
1.7.1
^ permalink raw reply related
* Re: [RFC TRY#2][PATCH] bgmac: pass received packet to the netif instead of copying it
From: Nathan Hintz @ 2013-10-29 6:50 UTC (permalink / raw)
To: OpenWrt Development List; +Cc: netdev, David S. Miller
In-Reply-To: <1382982142-16876-1-git-send-email-zajec5@gmail.com>
On Mon, 28 Oct 2013 18:42:22 +0100
Rafał Miłecki <zajec5@gmail.com> wrote:
Hi:
A few questions/comments inline...
Nathan
> Copying whole packets with skb_copy_from_linear_data_offset is a pretty
> bad idea. CPU was spending time in __copy_user_common and network
> performance was lower. With the new solution iperf-measured speed
> increased from 116Mb/s to 134Mb/s.
>
> Another way to improve performance could be switching to build_skb. It
> is cache-specific, so will require testing of various devices.
>
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> ---
> drivers/net/ethernet/broadcom/bgmac.c | 71 ++++++++++++++++++++-------------
> 1 file changed, 44 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
> index 6b7541f..fde9a11 100644
> --- a/drivers/net/ethernet/broadcom/bgmac.c
> +++ b/drivers/net/ethernet/broadcom/bgmac.c
> @@ -307,7 +307,6 @@ static int bgmac_dma_rx_read(struct bgmac *bgmac, struct bgmac_dma_ring *ring,
> struct device *dma_dev = bgmac->core->dma_dev;
> struct bgmac_slot_info *slot = &ring->slots[ring->start];
> struct sk_buff *skb = slot->skb;
> - struct sk_buff *new_skb;
> struct bgmac_rx_header *rx;
> u16 len, flags;
>
> @@ -320,38 +319,56 @@ static int bgmac_dma_rx_read(struct bgmac *bgmac, struct bgmac_dma_ring *ring,
> len = le16_to_cpu(rx->len);
> flags = le16_to_cpu(rx->flags);
>
> - /* Check for poison and drop or pass the packet */
> - if (len == 0xdead && flags == 0xbeef) {
> - bgmac_err(bgmac, "Found poisoned packet at slot %d, DMA issue!\n",
> - ring->start);
> - } else {
> + do {
"old_skb" duplicates "skb" stored above, can that one be used (renamed) instead of creating a new one here?
> + struct sk_buff *old_skb = slot->skb;
> + dma_addr_t old_dma_addr = slot->dma_addr;
> + int err;
> +
> + /* Check for poison and drop or pass the packet */
> + if (len == 0xdead && flags == 0xbeef) {
> + bgmac_err(bgmac, "Found poisoned packet at slot %d, DMA issue!\n",
> + ring->start);
Nothing in the buffer has been changed by the cpu yet, so is this sync necessary?
> + dma_sync_single_for_device(dma_dev,
> + slot->dma_addr,
> + BGMAC_RX_BUF_SIZE,
> + DMA_FROM_DEVICE);
> + break;
> + }
> +
> /* Omit CRC. */
> len -= ETH_FCS_LEN;
>
> - new_skb = netdev_alloc_skb_ip_align(bgmac->net_dev, len);
> - if (new_skb) {
> - skb_put(new_skb, len);
> - skb_copy_from_linear_data_offset(skb, BGMAC_RX_FRAME_OFFSET,
> - new_skb->data,
> - len);
> - skb_checksum_none_assert(skb);
> - new_skb->protocol =
> - eth_type_trans(new_skb, bgmac->net_dev);
> - netif_receive_skb(new_skb);
> - handled++;
> - } else {
> - bgmac->net_dev->stats.rx_dropped++;
> - bgmac_err(bgmac, "Allocation of skb for copying packet failed!\n");
> + /* Prepare new skb as replacement */
> + err = bgmac_dma_rx_skb_for_slot(bgmac, slot);
> + if (err) {
I've sent a separate patch against "bgmac_dma_rx_skb_for_slot" to not corrupt the slot at all if an
error occurs (skb alloc or dma mapping), and free the skb that was allocated if a dma mapping error
occurs. Assuming that patch is accepted, then the following two lines would not be needed.
With "bgmac_dma_rx_skb_for_slot" as it currently exists, this would leak an skb for a dma mapping
error (this was pre-existing to the changes in this patch).
> + slot->skb = old_skb;
> + slot->dma_addr = old_dma_addr;
> +
> + /* Poison the old skb */
> + rx->len = cpu_to_le16(0xdead);
> + rx->flags = cpu_to_le16(0xbeef);
> +
> + dma_sync_single_for_device(dma_dev,
> + slot->dma_addr,
> + BGMAC_RX_BUF_SIZE,
> + DMA_FROM_DEVICE);
> + break;
> }
> + bgmac_dma_rx_setup_desc(bgmac, ring, ring->start);
>
> - /* Poison the old skb */
> - rx->len = cpu_to_le16(0xdead);
> - rx->flags = cpu_to_le16(0xbeef);
> - }
> + /* Unmap old skb, we'll pass it to the netfif */
> + dma_unmap_single(dma_dev, old_dma_addr,
> + BGMAC_RX_BUF_SIZE,
> + DMA_FROM_DEVICE);
> +
> + skb_put(skb, BGMAC_RX_FRAME_OFFSET + len);
> + skb_pull(skb, BGMAC_RX_FRAME_OFFSET);
>
> - /* Make it back accessible to the hardware */
> - dma_sync_single_for_device(dma_dev, slot->dma_addr,
> - BGMAC_RX_BUF_SIZE, DMA_FROM_DEVICE);
> + skb_checksum_none_assert(skb);
> + skb->protocol = eth_type_trans(skb, bgmac->net_dev);
> + netif_receive_skb(skb);
> + handled++;
> + } while (0);
>
> if (++ring->start >= BGMAC_RX_RING_SLOTS)
> ring->start = 0;
--
Nathan
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
^ permalink raw reply
* [PATCH] bgmac: don't update slot on skb alloc/dma mapping error
From: Nathan Hintz @ 2013-10-29 6:44 UTC (permalink / raw)
To: netdev; +Cc: zajec5
Don't update the slot in "bgmac_dma_rx_skb_for_slot" unless both the
skb alloc and dma mapping are successful; and free the newly allocated
skb if a dma mapping error occurs. This will prevent an skb leak upon
returning when an error occurs.
Signed-off-by: Nathan Hintz <nlhintz@hotmail.com>
---
drivers/net/ethernet/broadcom/bgmac.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 7eca5a1..11e5d8d 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -252,25 +252,33 @@ static int bgmac_dma_rx_skb_for_slot(struct bgmac *bgmac,
struct bgmac_slot_info *slot)
{
struct device *dma_dev = bgmac->core->dma_dev;
+ struct sk_buff *skb;
+ dma_addr_t dma_addr;
struct bgmac_rx_header *rx;
/* Alloc skb */
- slot->skb = netdev_alloc_skb(bgmac->net_dev, BGMAC_RX_BUF_SIZE);
- if (!slot->skb)
+ skb = netdev_alloc_skb(bgmac->net_dev, BGMAC_RX_BUF_SIZE);
+ if (!skb)
return -ENOMEM;
/* Poison - if everything goes fine, hardware will overwrite it */
- rx = (struct bgmac_rx_header *)slot->skb->data;
+ rx = (struct bgmac_rx_header *)skb->data;
rx->len = cpu_to_le16(0xdead);
rx->flags = cpu_to_le16(0xbeef);
/* Map skb for the DMA */
- slot->dma_addr = dma_map_single(dma_dev, slot->skb->data,
- BGMAC_RX_BUF_SIZE, DMA_FROM_DEVICE);
- if (dma_mapping_error(dma_dev, slot->dma_addr)) {
+ dma_addr = dma_map_single(dma_dev, skb->data,
+ BGMAC_RX_BUF_SIZE, DMA_FROM_DEVICE);
+ if (dma_mapping_error(dma_dev, dma_addr)) {
bgmac_err(bgmac, "DMA mapping error\n");
+ dev_kfree_skb(skb);
return -ENOMEM;
}
+
+ /* Update the slot */
+ slot->skb = skb;
+ slot->dma_addr = dma_addr;
+
if (slot->dma_addr & 0xC0000000)
bgmac_warn(bgmac, "DMA address using 0xC0000000 bit(s), it may need translation trick\n");
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] bgmac: don't update slot on skb alloc/dma mapping error
From: Rafał Miłecki @ 2013-10-29 6:52 UTC (permalink / raw)
To: Nathan Hintz; +Cc: Network Development
In-Reply-To: <BLU0-SMTP119F7A71B34291D1CC2486BAC090@phx.gbl>
2013/10/29 Nathan Hintz <nlhintz@hotmail.com>:
> Don't update the slot in "bgmac_dma_rx_skb_for_slot" unless both the
> skb alloc and dma mapping are successful; and free the newly allocated
> skb if a dma mapping error occurs. This will prevent an skb leak upon
> returning when an error occurs.
In case of bgmac_dma_rx_skb_for_slot failure we're giving up anyway
(and freeing everything), but with your patch code is simpler to
understand, so I'm OK with that.
Acked-by: Rafał Miłecki <zajec5@gmail.com>
--
Rafał
^ permalink raw reply
* Re: [OpenWrt-Devel] [RFC TRY#2][PATCH] bgmac: pass received packet to the netif instead of copying it
From: Rafał Miłecki @ 2013-10-29 6:59 UTC (permalink / raw)
To: Nathan Hintz
Cc: OpenWrt Development List, Network Development, David S. Miller
In-Reply-To: <BLU0-SMTP122B5D616AD665A29B9DDEFAC090@phx.gbl>
2013/10/29 Nathan Hintz <nlhintz@hotmail.com>:
> On Mon, 28 Oct 2013 18:42:22 +0100
> Rafał Miłecki <zajec5@gmail.com> wrote:
>
> Hi:
>
> A few questions/comments inline...
>
> Nathan
>
>> Copying whole packets with skb_copy_from_linear_data_offset is a pretty
>> bad idea. CPU was spending time in __copy_user_common and network
>> performance was lower. With the new solution iperf-measured speed
>> increased from 116Mb/s to 134Mb/s.
>>
>> Another way to improve performance could be switching to build_skb. It
>> is cache-specific, so will require testing of various devices.
>>
>> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
>> ---
>> drivers/net/ethernet/broadcom/bgmac.c | 71 ++++++++++++++++++++-------------
>> 1 file changed, 44 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
>> index 6b7541f..fde9a11 100644
>> --- a/drivers/net/ethernet/broadcom/bgmac.c
>> +++ b/drivers/net/ethernet/broadcom/bgmac.c
>> @@ -307,7 +307,6 @@ static int bgmac_dma_rx_read(struct bgmac *bgmac, struct bgmac_dma_ring *ring,
>> struct device *dma_dev = bgmac->core->dma_dev;
>> struct bgmac_slot_info *slot = &ring->slots[ring->start];
>> struct sk_buff *skb = slot->skb;
>> - struct sk_buff *new_skb;
>> struct bgmac_rx_header *rx;
>> u16 len, flags;
>>
>> @@ -320,38 +319,56 @@ static int bgmac_dma_rx_read(struct bgmac *bgmac, struct bgmac_dma_ring *ring,
>> len = le16_to_cpu(rx->len);
>> flags = le16_to_cpu(rx->flags);
>>
>> - /* Check for poison and drop or pass the packet */
>> - if (len == 0xdead && flags == 0xbeef) {
>> - bgmac_err(bgmac, "Found poisoned packet at slot %d, DMA issue!\n",
>> - ring->start);
>> - } else {
>> + do {
>
> "old_skb" duplicates "skb" stored above, can that one be used (renamed) instead of creating a new one here?
I've focused on clean code too much. That won't be needed anyway when
I rebase my patch on top of yours.
>> + struct sk_buff *old_skb = slot->skb;
>> + dma_addr_t old_dma_addr = slot->dma_addr;
>> + int err;
>> +
>> + /* Check for poison and drop or pass the packet */
>> + if (len == 0xdead && flags == 0xbeef) {
>> + bgmac_err(bgmac, "Found poisoned packet at slot %d, DMA issue!\n",
>> + ring->start);
>> + dma_sync_single_for_device(dma_dev,
>> + slot->dma_addr,
>> + BGMAC_RX_BUF_SIZE,
>> + DMA_FROM_DEVICE);
>
> Nothing in the buffer has been changed by the cpu yet, so is this sync necessary?
I've moved your comment a line below, so it comments the code *above*.
I'm using LDD3 to understand DMA and it contains this explanation:
> void dma_sync_single_for_cpu(struct device *dev, dma_handle_t bus_addr,
> size_t size, enum dma_data_direction direction);
>
> This function should be called before the processor accesses a streaming DMA buffer. Once the call has been made, the CPU “owns” the DMA buffer and can work with it as needed. Before the device accesses the buffer, however, ownership should be transferred back to it with:
>
> void dma_sync_single_for_device(struct device *dev, dma_handle_t bus_addr,
> size_t size, enum dma_data_direction direction);
So even if I didn't change anything in the buffer, I believe we still
need to "sync" it back to make it accessible to the hardware.
> I've sent a separate patch against "bgmac_dma_rx_skb_for_slot" to not corrupt the slot at all if an
> error occurs (skb alloc or dma mapping), and free the skb that was allocated if a dma mapping error
> occurs. Assuming that patch is accepted, then the following two lines would not be needed.
> With "bgmac_dma_rx_skb_for_slot" as it currently exists, this would leak an skb for a dma mapping
> error (this was pre-existing to the changes in this patch).
Thanks, I'll rebase my patch.
^ permalink raw reply
* Re: [PATCH 03/16] wl1251: add sysfs interface for bluetooth coexistence mode configuration
From: Luca Coelho @ 2013-10-29 7:09 UTC (permalink / raw)
To: Ben Hutchings
Cc: Pali Rohár, John W. Linville, Johannes Berg, David S. Miller,
linux-wireless, netdev, linux-kernel, freemangordon,
aaro.koskinen, pavel, sre, joni.lapilainen, David Gnedt
In-Reply-To: <1383003587.3779.49.camel@bwh-desktop.uk.level5networks.com>
On Mon, 2013-10-28 at 23:39 +0000, Ben Hutchings wrote:
> On Sat, 2013-10-26 at 22:34 +0200, Pali Rohár wrote:
> > From: David Gnedt <david.gnedt@davizone.at>
> >
> > Port the bt_coex_mode sysfs interface from wl1251 driver version included
> > in the Maemo Fremantle kernel to allow bt-coexistence mode configuration.
> > This enables userspace applications to set one of the modes
> > WL1251_BT_COEX_OFF, WL1251_BT_COEX_ENABLE and WL1251_BT_COEX_MONOAUDIO.
> > The default mode is WL1251_BT_COEX_OFF.
> > It should be noted that this driver always enabled bt-coexistence before
> > and enabled bt-coexistence directly affects the receiving performance,
> > rendering it unusable in some low-signal situations. Especially monitor
> > mode is affected very badly with bt-coexistence enabled.
> [...]
>
> This should be implemented consistently with other drivers:
>
> drivers/net/wireless/ath/ath9k/htc_drv_init.c:module_param_named(btcoex_enable, ath9k_htc_btcoex_enable, int, 0444);
> drivers/net/wireless/ath/ath9k/init.c:module_param_named(btcoex_enable, ath9k_btcoex_enable, int, 0444);
> drivers/net/wireless/b43/main.c:module_param_named(btcoex, modparam_btcoex, int, 0444);
> drivers/net/wireless/ipw2x00/ipw2200.c:module_param(bt_coexist, int, 0444);
> drivers/net/wireless/iwlegacy/common.c:module_param(bt_coex_active, bool, S_IRUGO);
> drivers/net/wireless/iwlwifi/iwl-drv.c:module_param_named(bt_coex_active, iwlwifi_mod_params.bt_coex_active,
> drivers/net/wireless/ti/wlcore/sysfs.c:static DEVICE_ATTR(bt_coex_state, S_IRUGO | S_IWUSR,
>
> Oh, hmm, I see a problem here.
With so many drivers doing the same thing, isn't it about time to add
this to nl80211?
--
Luca.
^ 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