* Re: [PATCH v3 2/7] USB: serial: comments on suspend failure
From: Johan Hovold @ 2013-03-15 9:02 UTC (permalink / raw)
To: Ming Lei
Cc: David S. Miller, Greg Kroah-Hartman, Jiri Kosina, Alan Stern,
Oliver Neukum, netdev, linux-usb, linux-input, Johan Hovold
In-Reply-To: <1363320539-23012-3-git-send-email-ming.lei@canonical.com>
On Fri, Mar 15, 2013 at 12:08:54PM +0800, Ming Lei wrote:
> If suspend callback fails in system sleep context, usb core will
> ignore the failure and let system sleep go ahead further, so
> this patch comments on the case and requires that serial->type->suspend()
> MUST return 0 in system sleep context.
>
> Cc: Johan Hovold <jhovold@gmail.com>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
> drivers/usb/serial/usb-serial.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
> index a19ed74..16eb02b 100644
> --- a/drivers/usb/serial/usb-serial.c
> +++ b/drivers/usb/serial/usb-serial.c
> @@ -1140,6 +1140,11 @@ int usb_serial_suspend(struct usb_interface *intf, pm_message_t message)
>
> serial->suspending = 1;
>
> + /*
> + * serial->type->suspend() MUST return 0 in system sleep context,
> + * otherwise, the resume callback has to recover device from
> + * previous suspend failure.
> + */
> if (serial->type->suspend) {
> r = serial->type->suspend(serial, message);
> if (r < 0) {
Acked-by: Johan Hovold <jhovold@gmail.com>
Thanks,
Johan
^ permalink raw reply
* Re: Trying to implement secondary loopback
From: Eric W. Biederman @ 2013-03-15 8:59 UTC (permalink / raw)
To: Thomas Martitz
Cc: richard -rw- weinberger, netdev@vger.kernel.org,
davem@davemloft.net, edumazet@google.com,
herbert@gondor.apana.org.au
In-Reply-To: <5142CE23.9070804@hhi.fraunhofer.de>
Thomas Martitz <thomas.martitz@hhi.fraunhofer.de> writes:
> Am 13.03.2013 22:21, schrieb Eric W. Biederman:
>> "Martitz, Thomas" <thomas.martitz@hhi.fraunhofer.de> writes:
>>
>>>> Von: Eric W. Biederman [ebiederm@xmission.com]
>>>>
>>>> The easy solution is to setup two network namespaces and two nics.
>>>> And transmit data from one network namespace to another, through your
>>>> nics.
>>
>>>
>>> That sounds a lot more involed than using loopback to send data back
>>> to the same process. And I guess just calling netif_rx() in the xmit()
>>> function wouldn't be sufficient either? Also I wouldn't know how to do
>>> that as I'm not yet very familiar with network namespaces (but that
>>> can clearly get fixed).
>>
>> It may sound involved but it is just a few lines of code to set up.
>>
>> ip netns add ns1
>> ip netns add ns2
>> ip link set nic1 netns ns1
>> ip link set nic2 netns ns2
>>
>> int main(int argc, char **argv)
>> {
>> int netfd1, netfd2;
>> int sk1, sk2;
>> netfd1 = open(/var/run/netns/nic1);
>> netfd2 = open(/var/run/netns/nic2);
>>
>> setns(netfd1, 0);
>> sk1 = socket(...);
>>
>> setns(netfd2, 0);
>> sk2 = socket(...);
>>
>> /* test test test */
>> }
>>
>> And what is partidcularly interesting is that all of this works with
>> your drivers normal code paths without any kernel hacks.
>>
>> Eric
>>
>
>
> I have trouble understanding this approach. Does it mean that I have
> to expose two interfaces from my kernel module. I.e. call
> register_netdev() twice?
The suggestion was to have one driver, driving two identical nics
plugged into the same machine with a cable between them. That creates no
special cases on your side for testing.
Shrug. It is your driver, and your hardware, feel free to
develop however you want.
> Or use a "struct pernet_operations" to create
> a netdev on a per-namespace basis.
> If I do either of the above I end up with two "struct netdev" and two
> MAC addresses, which I don't really want. Also I would need to
> transfer from one netdev to the other in the driver's ndo_start_xmit()
> method instead of simply calling netif_rx().
>
> Perhaps I'm misunderstanding, I'm s
>
> During my development, looping back the data in ndo_start_xmit() is
> only a first step. The next step is to pass the data via PCIe and
> getting the same data back later. This is why I want to keep the
> loopback as simple/straightforward as possible.
I guess the question is why do you want to loopback the data?
For testing your NIC driver? Or are you trying some strange scheme
where you try and offload processing between sockets on the same machine?
Eric
^ permalink raw reply
* [PATCH net-next] Documentation: fix neigh/default/gc_thresh1 default value.
From: roy.qing.li @ 2013-03-15 8:49 UTC (permalink / raw)
To: netdev
From: Li RongQing <roy.qing.li@gmail.com>
The default value is 128, not 256
#grep gc_thresh1 net/ -rI
net/decnet/dn_neigh.c: .gc_thresh1 = 128,
net/ipv6/ndisc.c: .gc_thresh1 = 128,
net/ipv4/arp.c: .gc_thresh1 = 128,
Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
---
Documentation/networking/ip-sysctl.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index 1cae6c3..18a24c4 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -29,7 +29,7 @@ route/max_size - INTEGER
neigh/default/gc_thresh1 - INTEGER
Minimum number of entries to keep. Garbage collector will not
purge entries if there are fewer than this number.
- Default: 256
+ Default: 128
neigh/default/gc_thresh3 - INTEGER
Maximum number of neighbor entries allowed. Increase this
--
1.7.10.4
^ permalink raw reply related
* [PATCH net-next] driver/qlogic: replace ip_fast_csum with csum_replace2
From: roy.qing.li @ 2013-03-15 8:50 UTC (permalink / raw)
To: netdev
From: Li RongQing <roy.qing.li@gmail.com>
replace ip_fast_csum with csum_replace2 to save cpu cycles
Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
---
drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c | 4 ++--
drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c | 7 +++----
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
index 4782dcf..7692dfd 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
@@ -27,6 +27,7 @@
#include <linux/delay.h>
#include <linux/slab.h>
#include <linux/if_vlan.h>
+#include <net/checksum.h>
#include "netxen_nic.h"
#include "netxen_nic_hw.h"
@@ -1641,9 +1642,8 @@ netxen_process_lro(struct netxen_adapter *adapter,
th = (struct tcphdr *)((skb->data + vhdr_len) + (iph->ihl << 2));
length = (iph->ihl << 2) + (th->doff << 2) + lro_length;
+ csum_replace2(&iph->check, iph->tot_len, htons(length));
iph->tot_len = htons(length);
- iph->check = 0;
- iph->check = ip_fast_csum((unsigned char *)iph, iph->ihl);
th->psh = push;
th->seq = htonl(seq_number);
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
index 0e63006..891f12d 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
@@ -9,6 +9,7 @@
#include <linux/if_vlan.h>
#include <net/ip.h>
#include <linux/ipv6.h>
+#include <net/checksum.h>
#include "qlcnic.h"
@@ -1132,9 +1133,8 @@ qlcnic_process_lro(struct qlcnic_adapter *adapter,
iph = (struct iphdr *)skb->data;
th = (struct tcphdr *)(skb->data + (iph->ihl << 2));
length = (iph->ihl << 2) + (th->doff << 2) + lro_length;
+ csum_replace2(&iph->check, iph->tot_len, htons(length));
iph->tot_len = htons(length);
- iph->check = 0;
- iph->check = ip_fast_csum((unsigned char *)iph, iph->ihl);
}
th->psh = push;
@@ -1595,9 +1595,8 @@ qlcnic_83xx_process_lro(struct qlcnic_adapter *adapter,
iph = (struct iphdr *)skb->data;
th = (struct tcphdr *)(skb->data + (iph->ihl << 2));
length = (iph->ihl << 2) + (th->doff << 2) + lro_length;
+ csum_replace2(&iph->check, iph->tot_len, htons(length));
iph->tot_len = htons(length);
- iph->check = 0;
- iph->check = ip_fast_csum((unsigned char *)iph, iph->ihl);
}
th->psh = push;
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] rtnetlink: Mask the rta_type when range checking
From: Thomas Graf @ 2013-03-15 8:51 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: Stephen Hemminger, netdev, davem
In-Reply-To: <51426BB6.5030204@redhat.com>
On 03/14/13 at 08:30pm, Vlad Yasevich wrote:
> Doing a quick check on all the callers for rtnl_register and their
> handlers the following do not use nla_parse:
> 1) dn_fib_rtm_newroute/delroute - Don't seem to care about attribute
> types.
> 2) dn_cache_getroute() - suspect use. relies on rta_buf populated by
> rtnetlink_rcv_msg
>
> 3) inet_rtm_newroute/delroute - rtm_to_fib_config() uses a custom loop
> with nla_type(), so safe.
>
> That's all that a quick look finds. Out of all of them, looks like
> on dn_cache_getroute() would be broken.
So checking values in rta_max[] which lists the maximum attribute
allowed for each message family range, all are limited to low
values so the NLA_F_NESTED bit is guaranteed to have been unused
up to now.
The risk that remains is that we would start accepting an attribute
which we previously didn't but we have that risk with every new
attribute that is added.
Acked-by: Thomas Graf <tgraf@suug.ch>
^ permalink raw reply
* Re: igb_poll - device driver failed to check map error
From: Christoph Paasch @ 2013-03-15 7:52 UTC (permalink / raw)
To: Alexander Duyck
Cc: Jeff Kirsher, Jesse Brandeburg, Bruce Allan, Alex Duyck,
Eric Dumazet, netdev
In-Reply-To: <514284EA.3050305@gmail.com>
On Thursday 14 March 2013 19:18:18 Alexander Duyck wrote:
> On 03/12/2013 02:31 AM, Christoph Paasch wrote:
> > Hello,
> >
> > I'm seeing a warning while booting my machine when DMA_API_DEBUG is set:
> >
> > [ 36.402824] ------------[ cut here ]------------
> > [ 36.458070] WARNING: at
> > /home/cpaasch/builder/net-next/lib/dma-debug.c:934
> > check_unmap+0x648/0x702()
> > [ 36.567377] Hardware name: ProLiant DL165 G7
> > [ 36.618452] igb 0000:04:00.0: DMA-API: device driver failed to check
> > map
> > error[device address=0x0000000233d9b232] [size=154 bytes] [mapped as
> > single] [ 36.776640] Modules linked in:
> > [ 36.815446] Pid: 0, comm: swapper/7 Not tainted 3.9.0-rc1-mptcp+ #101
> > [ 36.892515] Call Trace:
> > [ 36.921745] <IRQ> [<ffffffff8102ad7f>] warn_slowpath_common+0x80/0x9a
> > [ 37.001023] [<ffffffff8102ae2d>] warn_slowpath_fmt+0x41/0x43
> > [ 37.069771] [<ffffffff811db17f>] check_unmap+0x648/0x702
> > [ 37.134363] [<ffffffff811db3e9>] debug_dma_unmap_page+0x50/0x52
> > [ 37.206234] [<ffffffff8136676a>] igb_poll+0x144/0xf7c
> > [ 37.267706] [<ffffffff8104dd19>] ? sched_clock_cpu+0x46/0xd1
> > [ 37.336456] [<ffffffff814458ce>] net_rx_action+0xa7/0x1d0
> > [ 37.402085] [<ffffffff81030b65>] __do_softirq+0xb4/0x16f
> > [ 37.466673] [<ffffffff81030c90>] irq_exit+0x40/0x87
> > [ 37.526067] [<ffffffff81002db1>] do_IRQ+0x98/0xaf
> > [ 37.583378] [<ffffffff815210aa>] common_interrupt+0x6a/0x6a
> > [ 37.651086] <EOI> [<ffffffff8105d4be>] ?
> > __tick_nohz_idle_enter+0x116/0x31f
> > [ 37.736595] [<ffffffff81008a04>] ? default_idle+0x24/0x39
> > [ 37.802224] [<ffffffff81008c62>] cpu_idle+0x68/0xa4
> > [ 37.861616] [<ffffffff81519f78>] start_secondary+0x1a9/0x1ad
> > [ 37.930364] ---[ end trace 01b5bb0fd75a464c ]---
> >
> >
> > It happens shortly after mounting the NFS-root filesystem.
> >
> > I tried to understand what is going on, but I am now at my wit's end.
> >
> > By adding some print-statements, here is what I found out (not sure if
> > this is anyhow helpful):
> >
> > The difference between tx_buffer->time_stamp and the current 'jiffies' is
> > up to 2000 jiffies (HZ==1000) at the first time the above warning happens
> > (this seems too much for me). From then on, I see my print 3-4 times
> > appear but without such a big difference between the timestamps
> > (difference around 1 and 2 jiffies).
> >
> > Some other stuff, I printed:
> > tx_buffer->skb: ffff880235054c80
> > tx_buffer->bytecount: 154
> > tx_buffer->gso_segs: 1
> > tx_buffer->protocol: 8
> > tx_buffer->tx_flags 0x20
> >
> >
> > One last thing:
> > Am I right that after each call to dma_map_single/page a call to
> > dma_mapping_error is needed? If that's the case, I have some patches that
> > add this statement at missing places in the e1000, e1000e and ixgb
> > driver. But these patches do not fix my above problem.
> >
> >
> > Thanks for your help,
> > Christoph
>
> Christoph,
>
> One thing that might be useful would be to reproduce this with a
> standard 3.9-rc kernel instead of one using the multipath TCP patches.
> This will help us to verify that the issue is reproducible with a stock
> kernel and is not related to any ongoing work you may have only in your
> tree.
Hello,
this is on a clean net-next kernel without any MPTCP-code.
I bisected it down to 787314c35fbb (Merge tag 'iommu-updates-v3.8' of
git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu), which simply
introduces the debug_dma_mapping_error-checks.
Am I right with the missing calls to dma_mapping_error in e1000, e1000e and
ixgb?
Cheers,
Christoph
--
IP Networking Lab --- http://inl.info.ucl.ac.be
MultiPath TCP in the Linux Kernel --- http://multipath-tcp.org
UCLouvain
--
^ permalink raw reply
* [PATCH net-next 2/2] net: reset transport header if it was not set before transmission
From: Jason Wang @ 2013-03-15 7:41 UTC (permalink / raw)
To: davem, netdev, linux-kernel; +Cc: mst, Jason Wang, Eric Dumazet
In-Reply-To: <1363333305-54398-1-git-send-email-jasowang@redhat.com>
Some drivers depends on transport_header to do packet transmission, but it was
unset in some cases (one example is macvtap driver which build skbs from
userspace and generate CHECKSUM_NONE packets). The driver may crash in those
cases since the transport_header was not valid. The problem becomes more obvious
since commit fda55eca5a33f33ffcd4192c6b2d75179714a52c (net: introduce
skb_transport_header_was_set()) since it initializes transport_header to ~0U.
So before passing the skb to driver, this patch reset the transport_header if it
was not set to avoid such crash such as:
hp-z800-04.qe.lab.eng.nay.redhat.com login: BUG: unable to handle kernel paging
request at ffff8805166f760c
IP: [<ffffffffa035a5d0>] ixgbe_xmit_frame_ring+0x220/0x5e0 [ixgbe]
PGD 1ece067 PUD 0
Oops: 0000 [#1] SMP
Modules linked in: vhost_net tun nfsv3 nfs_acl nfsv4 auth_rpcgss nfs fscache
lockd autofs4 sunrpc openvswitch ipv6 iTCO_wdt iTCO_vendor_support hp_wmi
sparse_keymap rfkill acpi_cpufreq freq_table mperf coretemp kvm_intel kvm
crc32c_intel ghash_clmulni_intel microcode serio_raw pcspkr sg lpc_ich mfd_core
tg3 snd_hda_codec_realtek snd_hda_intel snd_hda_codec snd_hwdep snd_seq
snd_seq_device snd_pcm snd_timer snd soundcore snd_page_alloc i7core_edac
edac_core ixgbe dca ptp pps_core mdio ext4(F) mbcache(F) jbd2(F) sd_mod(F)
crc_t10dif(F) sr_mod(F) cdrom(F) firewire_ohci(F) firewire_core(F) crc_itu_t(F)
aesni_intel(F) ablk_helper(F) cryptd(F) lrw(F) aes_x86_64(F) xts(F) gf128mul(F)
floppy(F) mptsas(F) mptscsih(F) mptbase(F) scsi_transport_sas(F) ahci(F)
libahci(F) nouveau(F) ttm(F) drm_kms_helper(F) drm(F) i2c_algo_bit(F)
i2c_core(F) mxm_wmi(F) video(F) wmi(F) dm_mirror(F) dm_region_hash(F) dm_log(F)
dm_mod(F) [last unloaded: tun]
CPU 6
Pid: 17337, comm: vhost-17317 Tainted: GF 3.9.0-rc1+ #7
Hewlett-Packard HP Z800 Workstation/0AECh
RIP: 0010:[<ffffffffa035a5d0>] [<ffffffffa035a5d0>]
ixgbe_xmit_frame_ring+0x220/0x5e0 [ixgbe]
RSP: 0018:ffff880222cddb18 EFLAGS: 00010286
RAX: 00000000ffffffff RBX: ffff880416b4b000 RCX: ffff8805166f75ff
RDX: 0000000000000008 RSI: ffff8804166f760e RDI: 0000000000000007
RBP: ffff880222cddb68 R08: 0000000000000008 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: ffffc90009dce120
R13: ffff880416b4b300 R14: 0000000000000000 R15: ffff8804118f0800
FS: 0000000000000000(0000) GS:ffff88042fc40000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: ffff8805166f760c CR3: 000000041e98c000 CR4: 00000000000027e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process vhost-17317 (pid: 17337, threadinfo ffff880222cdc000, task
ffff8802211d4040)
Stack:
00000000ffffffff 0000000000000180 ffff880222cddbb7 0000000000000180
ffff880222cddb48 ffff88040d5dd1c0 ffff8804118f0000 0000000000000036
ffff8804118f0000 ffff8804165d7a9c ffff880222cddb88 ffffffffa035a9d3
Call Trace:
[<ffffffffa035a9d3>] ixgbe_xmit_frame+0x43/0x90 [ixgbe]
[<ffffffff8149d54a>] dev_hard_start_xmit+0x12a/0x570
[<ffffffff814bd8da>] sch_direct_xmit+0xfa/0x1d0
[<ffffffff8149db28>] dev_queue_xmit+0x198/0x4c0
[<ffffffff813d23fa>] macvlan_start_xmit+0x6a/0x170
[<ffffffff813d3974>] macvtap_get_user+0x404/0x4e0
[<ffffffff813d3a7b>] macvtap_sendmsg+0x2b/0x30
[<ffffffffa06d9efa>] handle_tx+0x34a/0x680 [vhost_net]
[<ffffffffa06da265>] handle_tx_kick+0x15/0x20 [vhost_net]
[<ffffffffa06d7dfc>] vhost_worker+0x10c/0x1c0 [vhost_net]
[<ffffffffa06d7cf0>] ? vhost_attach_cgroups_work+0x30/0x30 [vhost_net]
[<ffffffffa06d7cf0>] ? vhost_attach_cgroups_work+0x30/0x30 [vhost_net]
[<ffffffff8107b77e>] kthread+0xce/0xe0
[<ffffffff8107b6b0>] ? kthread_freezable_should_stop+0x70/0x70
[<ffffffff815749ec>] ret_from_fork+0x7c/0xb0
[<ffffffff8107b6b0>] ? kthread_freezable_should_stop+0x70/0x70
Code: 34 31 0f 84 d3 01 00 00 66 83 fa 08 0f 85 b9 00 00 00 80 7e 09 06 0f 85 af
00 00 00 8b 80 cc 00 00 00 48 01 c1 0f 84 a0 00 00 00 <0f> b6 41 0d a8 01 0f 85
94 00 00 00 a8 02 75 0a 41 3a 7d 5c 0f
RIP [<ffffffffa035a5d0>] ixgbe_xmit_frame_ring+0x220/0x5e0 [ixgbe]
RSP <ffff880222cddb18>
CR2: ffff8805166f760c
Cc: Eric Dumazet <edumazet@google.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
net/core/dev.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 480114d..db315a1 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2525,6 +2525,9 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
}
}
+ if (!skb_transport_header_was_set(skb))
+ skb_reset_transport_header(skb);
+
if (!list_empty(&ptype_all))
dev_queue_xmit_nit(skb, dev);
--
1.7.1
^ permalink raw reply related
* [PATCH net-next 1/2] net_sched: don't do precise pkt_len computation for untrusted packets
From: Jason Wang @ 2013-03-15 7:41 UTC (permalink / raw)
To: davem, netdev, linux-kernel; +Cc: mst, Jason Wang, Eric Dumazet
Commit 1def9238d4aa2 (net_sched: more precise pkt_len computation) tries to do
precise packet len computation for GSO packets, but it does not check whether
the packets were from untrusted source. This is wrong since: we haven't done
header check before so both gso_segs and headers may not be correct. So this
patch just bypass the precise pkt_len computation for packet from untrusted
source (SKB_GSO_DODGY).
Cc: Eric Dumazet <edumazet@google.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
net/core/dev.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 90cee5b..480114d 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2586,7 +2586,7 @@ static void qdisc_pkt_len_init(struct sk_buff *skb)
/* To get more precise estimation of bytes sent on wire,
* we add to pkt_len the headers size of all segments
*/
- if (shinfo->gso_size) {
+ if (shinfo->gso_size && !(shinfo->gso_type & SKB_GSO_DODGY)) {
unsigned int hdr_len;
/* mac layer + network layer */
--
1.7.1
^ permalink raw reply related
* Re: Trying to implement secondary loopback
From: Thomas Martitz @ 2013-03-15 7:30 UTC (permalink / raw)
To: Eric W. Biederman
Cc: richard -rw- weinberger, netdev@vger.kernel.org,
davem@davemloft.net, edumazet@google.com,
herbert@gondor.apana.org.au
In-Reply-To: <878v5r9es7.fsf@xmission.com>
Am 13.03.2013 22:21, schrieb Eric W. Biederman:
> "Martitz, Thomas" <thomas.martitz@hhi.fraunhofer.de> writes:
>
>>> Von: Eric W. Biederman [ebiederm@xmission.com]
>>>
>>> The easy solution is to setup two network namespaces and two nics.
>>> And transmit data from one network namespace to another, through your
>>> nics.
>
>>
>> That sounds a lot more involed than using loopback to send data back
>> to the same process. And I guess just calling netif_rx() in the xmit()
>> function wouldn't be sufficient either? Also I wouldn't know how to do
>> that as I'm not yet very familiar with network namespaces (but that
>> can clearly get fixed).
>
> It may sound involved but it is just a few lines of code to set up.
>
> ip netns add ns1
> ip netns add ns2
> ip link set nic1 netns ns1
> ip link set nic2 netns ns2
>
> int main(int argc, char **argv)
> {
> int netfd1, netfd2;
> int sk1, sk2;
> netfd1 = open(/var/run/netns/nic1);
> netfd2 = open(/var/run/netns/nic2);
>
> setns(netfd1, 0);
> sk1 = socket(...);
>
> setns(netfd2, 0);
> sk2 = socket(...);
>
> /* test test test */
> }
>
> And what is partidcularly interesting is that all of this works with
> your drivers normal code paths without any kernel hacks.
>
> Eric
>
I have trouble understanding this approach. Does it mean that I have to
expose two interfaces from my kernel module. I.e. call register_netdev()
twice? Or use a "struct pernet_operations" to create a netdev on a
per-namespace basis.
If I do either of the above I end up with two "struct netdev" and two
MAC addresses, which I don't really want. Also I would need to transfer
from one netdev to the other in the driver's ndo_start_xmit() method
instead of simply calling netif_rx().
Perhaps I'm misunderstanding, I'm s
During my development, looping back the data in ndo_start_xmit() is only
a first step. The next step is to pass the data via PCIe and getting the
same data back later. This is why I want to keep the loopback as
simple/straightforward as possible.
Thanks for your assistance.
Best regards.
-----
visit us at
OFC 2013 / March 19-21 / Anaheim Convention Center, CA, USA / booth 11807
NABSHOW 2013 / April 8-11 / Las Vegas Convention Center, Nevada, USA / booth C7843
www.hhi.fraunhofer.de/events
^ permalink raw reply
* Re: [PATCH net,stable-3.8] net: cdc_ncm, cdc_mbim: allow user to prefer NCM for backwards compatibility
From: Bjørn Mork @ 2013-03-15 7:02 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, linux-usb, Greg Suarez, Alexey Orishko
In-Reply-To: <1363324945.3937.144.camel@deadeye.wl.decadent.org.uk>
Ben Hutchings <ben@decadent.org.uk> writes:
> On Thu, 2013-03-14 at 12:05 +0100, Bjørn Mork wrote:
>> commit bd329e1 ("net: cdc_ncm: do not bind to NCM compatible MBIM devices")
>> introduced a new policy, preferring MBIM for dual NCM/MBIM functions if
>> the cdc_mbim driver was enabled. This caused a regression for users
>> wanting to use NCM.
>>
>> Devices implementing NCM backwards compatibility according to section
>> 3.2 of the MBIM v1.0 specification allow either NCM or MBIM on a single
>> USB function, using different altsettings. The cdc_ncm and cdc_mbim
>> drivers will both probe such functions, and must agree on a common
>> policy for selecting either MBIM or NCM. Until now, this policy has
>> been set at build time based on CONFIG_USB_NET_CDC_MBIM.
>>
>> Use a module parameter to set the system policy at runtime, allowing the
>> user to prefer NCM on systems with the cdc_mbim driver.
>>
>> Cc: Greg Suarez <gsuarez@smithmicro.com>
>> Cc: Alexey Orishko <alexey.orishko@stericsson.com>
>> Reported-by: Geir Haatveit <nospam@haatveit.nu>
>> Reported-by: Tommi Kyntola <kynde@ts.ray.fi>
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=54791
>> Signed-off-by: Bjørn Mork <bjorn@mork.no>
>> ---
>>
>> We now have two users independently reporting this as a 3.8 regression,
>> so something needs to be done. I am not sure if adding a new module
>> parameter is acceptable for stable, but this problem is definitely a
>> regression and no other solutions came up in response to my RFC.
>>
>> The only real alternative I see for stable, is disabling MBIM support
>> on any dual NCM/MBIM function. Which of course will be a regression
>> for any user wanting MBIM, making it unacceptable.
> [...]
>
> It definitely makes sense for this to be a run-time parameter. And the
> default seems correct for custom kernels.
>
> For a distribution kernel - at least for Debian, where we can't assume
> kernel and userland are always updated together - I think the
> compile-time default should be false, and the userland package
> (presumably ModemManager?) can install a modprobe.conf file to override
> that once it can handle MBIM. We handled KMS transitions in a similar
> way. I don't know that it's worth having a Kconfig option for that,
> though.
We could consider always defaulting to NCM. That would remove the
ugliest parts of the code as well. Should I send a new version with
such a change?
Bundling a modprobe.conf file to enable MBIM with userland tools
supporting it is a great idea, and sounds feasible for custom built
systems as well.
(BTW, if there is any doubt about it, I feel really bad about my recent
request to enable MBIM in Debian, where I wrote "enabling the driver
will not prevent any existing solution from working.". At the time, I
had not seen any of these dual NCM/MBIM functions and I just didn't
think about the problems such devices would run into. Sorry about
that. I guess I owe you one more...)
Bjørn
^ permalink raw reply
* Re: [PATCH] isdn: hisax: netjet requires VIRT_TO_BUS
From: Geert Uytterhoeven @ 2013-03-15 6:26 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linux-kernel, linux-arm-kernel, Karsten Keil, netdev
In-Reply-To: <1363298204-8014-4-git-send-email-arnd@arndb.de>
On Thu, Mar 14, 2013 at 10:56 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> Disabling CONFIG_VIRT_TO_BUS on ARM showed that the hisax netjet
> driver depends on this deprecated functionality but is not
> marked so in Kconfig.
>
> Rather than adding ARM to the already long list of architectures
> that this driver is broken on, this patch adds 'depends on
> VIRT_TO_BUS' and removes the dependency on !SPARC, which is
> also implied by that.
IIRC, the real "arch" dependency for this driver is !BIG_ENDIAN, but
unfortunately
we don't have a generic Kconfig symbol for that.
Perhaps we may want to introduce that?
Of course we prefer to make drivers work on all endianness instead...
> --- a/drivers/isdn/hisax/Kconfig
> +++ b/drivers/isdn/hisax/Kconfig
> @@ -237,7 +237,8 @@ config HISAX_MIC
>
> config HISAX_NETJET
> bool "NETjet card"
> - depends on PCI && (BROKEN || !(SPARC || PPC || PARISC || M68K || (MIPS && !CPU_LITTLE_ENDIAN) || FRV || (XTENSA && !CPU_LITTLE_ENDIAN)))
> + depends on PCI && (BROKEN || !(PPC || PARISC || M68K || (MIPS && !CPU_LITTLE_ENDIAN) || FRV || (XTENSA && !CPU_LITTLE_ENDIAN)))
> + depends on VIRT_TO_BUS
> help
> This enables HiSax support for the NetJet from Traverse
> Technologies.
> @@ -248,7 +249,8 @@ config HISAX_NETJET
>
> config HISAX_NETJET_U
> bool "NETspider U card"
> - depends on PCI && (BROKEN || !(SPARC || PPC || PARISC || M68K || (MIPS && !CPU_LITTLE_ENDIAN) || FRV || (XTENSA && !CPU_LITTLE_ENDIAN)))
> + depends on PCI && (BROKEN || !(PPC || PARISC || M68K || (MIPS && !CPU_LITTLE_ENDIAN) || FRV || (XTENSA && !CPU_LITTLE_ENDIAN)))
> + depends on VIRT_TO_BUS
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [RFC 0/3] arm: mxs: sanitize enet_out clock handling
From: Trent Piepho @ 2013-03-15 5:30 UTC (permalink / raw)
To: netdev
In-Reply-To: <1359470773-14290-1-git-send-email-w.sang@pengutronix.de>
Wolfram Sang <w.sang <at> pengutronix.de> writes:
> Handling enet_out on MX28 is cumbersome at the moment. Most boards need it
> enabled and for that, they have to add code to mach-mxs.c (see sps1 as an
> example). Since this is board specific, we better encode it in the devicetree,
> that is the reason it was made for.
>
> My proposal will overwrite the generic "clock" and "clock-names" properties
> from imx28.dtsi in the board file. The original one has 2 entries, and boards
> needing enet_out will overload it with three entries. The network driver will
> enable the clock if it was specified. The old code enabling the clock will be
> backward compatible but print a WARN if the legacy mode needs to be used.
I was annoyed by this problem when adding a new imx28 board. Ethernet stopped
working as soon as I changed the board id in the dts from imx28evk to a new one.
Looks like a good solution, but one suggestion. Since as you say most boards
need enet_out would it be better to specify enet_out in the dtsi file and then
override it in the dts file of the boards that do not have it?
^ permalink raw reply
* Re: [PATCH net,stable-3.8] net: cdc_ncm, cdc_mbim: allow user to prefer NCM for backwards compatibility
From: Ben Hutchings @ 2013-03-15 5:22 UTC (permalink / raw)
To: Bjørn Mork
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA,
Greg Suarez, Alexey Orishko
In-Reply-To: <1363259113-6909-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2453 bytes --]
On Thu, 2013-03-14 at 12:05 +0100, Bjørn Mork wrote:
> commit bd329e1 ("net: cdc_ncm: do not bind to NCM compatible MBIM devices")
> introduced a new policy, preferring MBIM for dual NCM/MBIM functions if
> the cdc_mbim driver was enabled. This caused a regression for users
> wanting to use NCM.
>
> Devices implementing NCM backwards compatibility according to section
> 3.2 of the MBIM v1.0 specification allow either NCM or MBIM on a single
> USB function, using different altsettings. The cdc_ncm and cdc_mbim
> drivers will both probe such functions, and must agree on a common
> policy for selecting either MBIM or NCM. Until now, this policy has
> been set at build time based on CONFIG_USB_NET_CDC_MBIM.
>
> Use a module parameter to set the system policy at runtime, allowing the
> user to prefer NCM on systems with the cdc_mbim driver.
>
> Cc: Greg Suarez <gsuarez-AKjrjAf1O7qe8kRwQpwjMg@public.gmane.org>
> Cc: Alexey Orishko <alexey.orishko-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org>
> Reported-by: Geir Haatveit <nospam-br728iSPf5UsCylrc8G9yg@public.gmane.org>
> Reported-by: Tommi Kyntola <kynde-oSY6wnT0nbj1KXRcyAk9cg@public.gmane.org>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=54791
> Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
> ---
>
> We now have two users independently reporting this as a 3.8 regression,
> so something needs to be done. I am not sure if adding a new module
> parameter is acceptable for stable, but this problem is definitely a
> regression and no other solutions came up in response to my RFC.
>
> The only real alternative I see for stable, is disabling MBIM support
> on any dual NCM/MBIM function. Which of course will be a regression
> for any user wanting MBIM, making it unacceptable.
[...]
It definitely makes sense for this to be a run-time parameter. And the
default seems correct for custom kernels.
For a distribution kernel - at least for Debian, where we can't assume
kernel and userland are always updated together - I think the
compile-time default should be false, and the userland package
(presumably ModemManager?) can install a modprobe.conf file to override
that once it can handle MBIM. We handled KMS transitions in a similar
way. I don't know that it's worth having a Kconfig option for that,
though.
Ben.
--
Ben Hutchings
Humans are not rational beings; they are rationalising beings.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* Re: [PATCH] net: fec: restart the FEC when PHY speed changes
From: Frank Li @ 2013-03-15 4:37 UTC (permalink / raw)
To: Lucas Stach; +Cc: netdev, Frank Li, David S. Miller
In-Reply-To: <1363273921-5035-1-git-send-email-l.stach@pengutronix.de>
Acked
2013/3/14 Lucas Stach <l.stach@pengutronix.de>:
> Proviously we would only restart the FEC when PHY link or duplex state
> changed. PHY does not always bring down the link for speed changes, in
> which case we would not detect any change and keep FEC running.
>
> Switching link speed without restarting the FEC results in the FEC being
> stuck in an indefinite state, generating error conditions for every
> packet.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> drivers/net/ethernet/freescale/fec.c | 26 +++++++++++++++-----------
> drivers/net/ethernet/freescale/fec.h | 1 +
> 2 files changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
> index 069a155..61d2e62 100644
> --- a/drivers/net/ethernet/freescale/fec.c
> +++ b/drivers/net/ethernet/freescale/fec.c
> @@ -934,24 +934,28 @@ static void fec_enet_adjust_link(struct net_device *ndev)
> goto spin_unlock;
> }
>
> - /* Duplex link change */
> if (phy_dev->link) {
> - if (fep->full_duplex != phy_dev->duplex) {
> - fec_restart(ndev, phy_dev->duplex);
> - /* prevent unnecessary second fec_restart() below */
> + if (!fep->link) {
> fep->link = phy_dev->link;
> status_change = 1;
> }
> - }
>
> - /* Link on or off change */
> - if (phy_dev->link != fep->link) {
> - fep->link = phy_dev->link;
> - if (phy_dev->link)
> + if (fep->full_duplex != phy_dev->duplex)
> + status_change = 1;
> +
> + if (phy_dev->speed != fep->speed) {
> + fep->speed = phy_dev->speed;
> + status_change = 1;
> + }
> +
> + /* if any of the above changed restart the FEC */
> + if (status_change)
> fec_restart(ndev, phy_dev->duplex);
> - else
> + } else {
> + if (fep->link) {
> fec_stop(ndev);
> - status_change = 1;
> + status_change = 1;
> + }
> }
>
> spin_unlock:
> diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
> index f539007..eb43729 100644
> --- a/drivers/net/ethernet/freescale/fec.h
> +++ b/drivers/net/ethernet/freescale/fec.h
> @@ -240,6 +240,7 @@ struct fec_enet_private {
> phy_interface_t phy_interface;
> int link;
> int full_duplex;
> + int speed;
> struct completion mdio_done;
> int irq[FEC_IRQ_NUM];
> int bufdesc_ex;
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v3 4/7] usbnet: cdc_mbim: comments on suspend failure
From: Ming Lei @ 2013-03-15 4:08 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman, Jiri Kosina
Cc: Alan Stern, Oliver Neukum, netdev, linux-usb, linux-input,
Ming Lei
In-Reply-To: <1363320539-23012-1-git-send-email-ming.lei@canonical.com>
If suspend callback fails in system sleep context, usb core will
ignore the failure and let system sleep go ahead further, so
this patch comments on the case and requires that both
usbnet_suspend() and subdriver->suspend() MUST return 0 in
system sleep context.
Acked-by: David S. Miller <davem@davemloft.net>
Acked-by: Bjørn Mork <bjorn@mork.no>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/net/usb/cdc_mbim.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index 248d2dc..406a34d 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -332,6 +332,11 @@ static int cdc_mbim_suspend(struct usb_interface *intf, pm_message_t message)
goto error;
}
+ /*
+ * Both usbnet_suspend() and subdriver->suspend() MUST return 0
+ * in system sleep context, otherwise, the resume callback has
+ * to recover device from previous suspend failure.
+ */
ret = usbnet_suspend(intf, message);
if (ret < 0)
goto error;
--
1.7.9.5
^ permalink raw reply related
* [PATCH v3 3/7] USBHID: don't recover device if suspend fails in system sleep
From: Ming Lei @ 2013-03-15 4:08 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman, Jiri Kosina
Cc: Alan Stern, Oliver Neukum, netdev, linux-usb, linux-input,
Ming Lei
In-Reply-To: <1363320539-23012-1-git-send-email-ming.lei@canonical.com>
If suspend callback fails in system sleep context, usb core will
ignore the failure and let the system sleep go ahead further, so this
patch doesn't recover device under this situation, otherwise
may cause resume() confused.
Acked-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/hid/usbhid/hid-core.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index effcd3d..9941828 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1520,7 +1520,7 @@ static int hid_suspend(struct usb_interface *intf, pm_message_t message)
{
struct hid_device *hid = usb_get_intfdata(intf);
struct usbhid_device *usbhid = hid->driver_data;
- int status;
+ int status = 0;
bool driver_suspended = false;
if (PMSG_IS_AUTO(message)) {
@@ -1547,19 +1547,15 @@ static int hid_suspend(struct usb_interface *intf, pm_message_t message)
}
} else {
- if (hid->driver && hid->driver->suspend) {
+ /* TODO: resume() might need to handle suspend failure */
+ if (hid->driver && hid->driver->suspend)
status = hid->driver->suspend(hid, message);
- if (status < 0)
- return status;
- }
driver_suspended = true;
spin_lock_irq(&usbhid->lock);
set_bit(HID_SUSPENDED, &usbhid->iofl);
spin_unlock_irq(&usbhid->lock);
- if (usbhid_wait_io(hid) < 0) {
+ if (usbhid_wait_io(hid) < 0)
status = -EIO;
- goto failed;
- }
}
hid_cancel_delayed_stuff(usbhid);
@@ -1571,7 +1567,7 @@ static int hid_suspend(struct usb_interface *intf, pm_message_t message)
goto failed;
}
dev_dbg(&intf->dev, "suspend\n");
- return 0;
+ return status;
failed:
hid_resume_common(hid, driver_suspended);
--
1.7.9.5
^ permalink raw reply related
* [PATCH v3 7/7] usbnet: smsc75xx: don't recover device if suspend fails in system sleep
From: Ming Lei @ 2013-03-15 4:08 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman, Jiri Kosina
Cc: Alan Stern, Oliver Neukum, netdev, linux-usb, linux-input,
Ming Lei, Steve Glendinning
In-Reply-To: <1363320539-23012-1-git-send-email-ming.lei@canonical.com>
If suspend callback fails in system sleep context, usb core will
ignore the failure and let system sleep go ahead further, so
this patch doesn't recover device under this situation.
Also add comments on this case.
Acked-by: David S. Miller <davem@davemloft.net>
Cc: Steve Glendinning <steve.glendinning@shawell.net>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/net/usb/smsc75xx.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index 9abe517..21b607a 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -2011,7 +2011,11 @@ static int smsc75xx_suspend(struct usb_interface *intf, pm_message_t message)
ret = smsc75xx_enter_suspend0(dev);
done:
- if (ret)
+ /*
+ * TODO: resume() might need to handle the suspend failure
+ * in system sleep
+ */
+ if (ret && PMSG_IS_AUTO(message))
usbnet_resume(intf);
return ret;
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH v3 6/7] usbnet: smsc95xx: don't recover device if suspend fails in system sleep
From: Ming Lei @ 2013-03-15 4:08 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman, Jiri Kosina
Cc: Alan Stern, Oliver Neukum, netdev, linux-usb, linux-input,
Ming Lei, Steve Glendinning
In-Reply-To: <1363320539-23012-1-git-send-email-ming.lei@canonical.com>
If suspend callback fails in system sleep context, usb core will
ignore the failure and let system sleep go ahead further, so
this patch doesn't recover device under this situation.
Also add comments on the case.
Acked-by: David S. Miller <davem@davemloft.net>
Cc: Steve Glendinning <steve.glendinning@shawell.net>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/net/usb/smsc95xx.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index e6d2dea..3f38ba8 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1660,7 +1660,11 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
ret = smsc95xx_enter_suspend0(dev);
done:
- if (ret)
+ /*
+ * TODO: resume() might need to handle the suspend failure
+ * in system sleep
+ */
+ if (ret && PMSG_IS_AUTO(message))
usbnet_resume(intf);
return ret;
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH v3 5/7] usbnet: qmi_wwan: comments on suspend failure
From: Ming Lei @ 2013-03-15 4:08 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman, Jiri Kosina
Cc: Alan Stern, Oliver Neukum, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA, Ming Lei
In-Reply-To: <1363320539-23012-1-git-send-email-ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
If suspend callback fails in system sleep context, usb core will
ignore the failure and let system sleep go ahead further, so
this patch comments on the case and requires that both
usbnet_suspend() and subdriver->suspend() MUST return 0 in
system sleep context.
Acked-by: David S. Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Acked-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
drivers/net/usb/qmi_wwan.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 968d5d5..bdceb74 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -271,6 +271,11 @@ static int qmi_wwan_suspend(struct usb_interface *intf, pm_message_t message)
struct qmi_wwan_state *info = (void *)&dev->data;
int ret;
+ /*
+ * Both usbnet_suspend() and subdriver->suspend() MUST return 0
+ * in system sleep context, otherwise, the resume callback has
+ * to recover device from previous suspend failure.
+ */
ret = usbnet_suspend(intf, message);
if (ret < 0)
goto err;
--
1.7.9.5
--
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 related
* [PATCH v3 2/7] USB: serial: comments on suspend failure
From: Ming Lei @ 2013-03-15 4:08 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman, Jiri Kosina
Cc: Alan Stern, Oliver Neukum, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA, Ming Lei, Johan Hovold
In-Reply-To: <1363320539-23012-1-git-send-email-ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
If suspend callback fails in system sleep context, usb core will
ignore the failure and let system sleep go ahead further, so
this patch comments on the case and requires that serial->type->suspend()
MUST return 0 in system sleep context.
Cc: Johan Hovold <jhovold-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
drivers/usb/serial/usb-serial.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index a19ed74..16eb02b 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -1140,6 +1140,11 @@ int usb_serial_suspend(struct usb_interface *intf, pm_message_t message)
serial->suspending = 1;
+ /*
+ * serial->type->suspend() MUST return 0 in system sleep context,
+ * otherwise, the resume callback has to recover device from
+ * previous suspend failure.
+ */
if (serial->type->suspend) {
r = serial->type->suspend(serial, message);
if (r < 0) {
--
1.7.9.5
--
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 related
* [PATCH v3 1/7] USB: adds comment on suspend callback
From: Ming Lei @ 2013-03-15 4:08 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman, Jiri Kosina
Cc: Alan Stern, Oliver Neukum, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA, Ming Lei
In-Reply-To: <1363320539-23012-1-git-send-email-ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
This patch adds comments on interface driver suspend callback
to emphasize that the failure return value is ignored by
USB core in system sleep context, so do not try to recover
device for this case and let resume/reset_resume callback
handle the suspend failure if needed.
Also kerneldoc for usb_suspend_both() is updated with the
fact.
Acked-by: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>
Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
drivers/usb/core/driver.c | 11 ++++++++---
include/linux/usb.h | 7 ++++++-
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index d938b2b..eb1d00a 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -1196,9 +1196,14 @@ done:
*
* This is the central routine for suspending USB devices. It calls the
* suspend methods for all the interface drivers in @udev and then calls
- * the suspend method for @udev itself. If an error occurs at any stage,
- * all the interfaces which were suspended are resumed so that they remain
- * in the same state as the device.
+ * the suspend method for @udev itself. When the routine is called in
+ * autosuspend, if an error occurs at any stage, all the interfaces
+ * which were suspended are resumed so that they remain in the same
+ * state as the device, but when called from system sleep, all error
+ * from suspend methods of interfaces and the non-root-hub device itself
+ * are simply ignored, so all suspended interfaces are only resumed
+ * to the device's state when @udev is root-hub and its suspend method
+ * returns failure.
*
* Autosuspend requests originating from a child device or an interface
* driver may be made without the protection of @udev's device lock, but
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 4d22d0f..f82ce57 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -978,7 +978,12 @@ struct usbdrv_wrap {
* the "usbfs" filesystem. This lets devices provide ways to
* expose information to user space regardless of where they
* do (or don't) show up otherwise in the filesystem.
- * @suspend: Called when the device is going to be suspended by the system.
+ * @suspend: Called when the device is going to be suspended by the
+ * system either from system sleep or runtime suspend context. The
+ * return value will be ignored in system sleep context, so do NOT
+ * try to continue using the device if suspend fails in this case.
+ * Instead, let the resume or reset-resume routine recover from
+ * the failure.
* @resume: Called when the device is being resumed by the system.
* @reset_resume: Called when the suspended device has been reset instead
* of being resumed.
--
1.7.9.5
--
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 related
* [PATCH v3 0/7] USB: don't recover device if suspend fails in system sleep
From: Ming Lei @ 2013-03-15 4:08 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman, Jiri Kosina
Cc: Alan Stern, Oliver Neukum, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA
Hi,
This patch adds comments on interface driver suspend callback
to emphasize that the failure return value is ignored by
USB core in system sleep context, so do not try to recover
device for this case, otherwise the recovery things may confuse
resume().
Also fixes USB HID and several usbnet drivers which may recover
device in suspend failure path of system sleep.
v3:
- only 2/7 changed, don't change code but add comments to
restrict subdriver->suspend(), suggested by Johan Hovold.
v2:
- improve comments on suspend callback as suggested by Alan
- update kerneldoc for usb_suspend_both as suggested by Alan
- remove previous check of PMSG_IS_AUTO(message) in cdc_mbim/
qmi_wwan and add comments on suspend failure case, since Bjørn
doesn't like the check.
- add comments on smsc95xx/smsc75xx
v1:
- fix compile failure
- add comments about handling suspend failure in resume()
drivers/hid/usbhid/hid-core.c | 14 +++++---------
drivers/net/usb/cdc_mbim.c | 5 +++++
drivers/net/usb/qmi_wwan.c | 5 +++++
drivers/net/usb/smsc75xx.c | 6 +++++-
drivers/net/usb/smsc95xx.c | 6 +++++-
drivers/usb/core/driver.c | 11 ++++++++---
drivers/usb/serial/usb-serial.c | 5 +++++
include/linux/usb.h | 7 ++++++-
8 files changed, 44 insertions(+), 15 deletions(-)
Thanks
--
Ming Lei
--
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: [PATCH net-next 1/6] GRE: Use RTNL for atomic protocol registration.
From: Pravin Shelar @ 2013-03-15 3:37 UTC (permalink / raw)
To: Michał Mirosław; +Cc: davem, stephen, netdev, jesse
In-Reply-To: <CAHXqBFJzomhYjQvF+3+aNRjpZzByu-U=s5rGVnxwyL+6vemPTw@mail.gmail.com>
On Wed, Mar 13, 2013 at 4:22 PM, Michał Mirosław <mirqus@gmail.com> wrote:
> 2013/3/13 Pravin B Shelar <pshelar@nicira.com>:
>> Currently gre demultiplexer uses its own spin-lock. We should
>> rather RTNL lock.
> [...]
>> + rtnl_lock();
>> err = gre_add_protocol(&gre_pptp_protocol, GREPROTO_PPTP);
>> + rtnl_unlock();
>
> Why not keep lock/unlock inside gre_add/del_protocol()?
>
As Stephen suggested I am planning on dropping this patch.
^ permalink raw reply
* [PATCH 1/1 v2] Fix dst_neigh_lookup/dst_neigh_lookup_skb return value handling bug
From: Zhouyi Zhou @ 2013-03-15 3:21 UTC (permalink / raw)
To: netdev; +Cc: yizhouzhou, Zhouyi Zhou
When neighbour table is full, dst_neigh_lookup/dst_neigh_lookup_skb will return
-ENOBUFS which is absolutely non zero, while all the code in kernel which use
above functions assume failure only on zero return which will cause panic. (for
example: : https://bugzilla.kernel.org/show_bug.cgi?id=54731).
This patch corrects above error with smallest changes to kernel source code and
also correct two return value check missing bugs in drivers/infiniband/hw/cxgb4/cm.c
Tested on my x86_64 SMP machine
Reported-by: Zhouyi Zhou <zhouzhouyi@gmail.com>
Tested-by: Zhouyi Zhou <zhouzhouyi@gmail.com>
Signed-off-by: Zhouyi Zhou <zhouzhouyi@gmail.com>
---
drivers/infiniband/hw/cxgb4/cm.c | 12 ++++++++++++
include/net/dst.h | 6 ++++--
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index 565bfb1..6b95851 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -1575,6 +1575,12 @@ static int c4iw_reconnect(struct c4iw_ep *ep)
neigh = dst_neigh_lookup(ep->dst,
&ep->com.cm_id->remote_addr.sin_addr.s_addr);
+ if (!neigh) {
+ pr_err("%s - cannot alloc neigh.\n", __func__);
+ err = -ENOMEM;
+ goto fail4;
+ }
+
/* get a l2t entry */
if (neigh->dev->flags & IFF_LOOPBACK) {
PDBG("%s LOOPBACK\n", __func__);
@@ -3053,6 +3059,12 @@ static int rx_pkt(struct c4iw_dev *dev, struct sk_buff *skb)
dst = &rt->dst;
neigh = dst_neigh_lookup_skb(dst, skb);
+ if (!neigh) {
+ pr_err("%s - failed to allocate neigh!\n",
+ __func__);
+ goto free_dst;
+ }
+
if (neigh->dev->flags & IFF_LOOPBACK) {
pdev = ip_dev_find(&init_net, iph->daddr);
e = cxgb4_l2t_get(dev->rdev.lldi.l2t, neigh,
diff --git a/include/net/dst.h b/include/net/dst.h
index 853cda1..c083b5a 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -413,13 +413,15 @@ static inline int dst_neigh_output(struct dst_entry *dst, struct neighbour *n,
static inline struct neighbour *dst_neigh_lookup(const struct dst_entry *dst, const void *daddr)
{
- return dst->ops->neigh_lookup(dst, NULL, daddr);
+ struct neighbour *n = dst->ops->neigh_lookup(dst, NULL, daddr);
+ return IS_ERR(n) ? NULL : n;
}
static inline struct neighbour *dst_neigh_lookup_skb(const struct dst_entry *dst,
struct sk_buff *skb)
{
- return dst->ops->neigh_lookup(dst, skb, NULL);
+ struct neighbour *n = dst->ops->neigh_lookup(dst, skb, NULL);
+ return IS_ERR(n) ? NULL : n;
}
static inline void dst_link_failure(struct sk_buff *skb)
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH v2 2/7] USB: serial: handle suspend failure path correctly
From: Ming Lei @ 2013-03-15 3:04 UTC (permalink / raw)
To: Johan Hovold
Cc: David S. Miller, Greg Kroah-Hartman, Jiri Kosina, Alan Stern,
Oliver Neukum, netdev, linux-usb, linux-input
In-Reply-To: <20130314111010.GA15776@localhost>
On Thu, Mar 14, 2013 at 7:10 PM, Johan Hovold <jhovold@gmail.com> wrote:
> On Fri, Mar 08, 2013 at 12:15:19AM +0800, Ming Lei wrote:
>> This patch kills traffic even though type->suspend returns
>> failure inside usb_serial_suspend from system sleep context
>> because USB core ignores the failiure and lets system sleep
>> go ahread, so the serial URB traffic need to be killed
>> in this case.
>>
>> Cc: Johan Hovold <jhovold@gmail.com>
>> Signed-off-by: Ming Lei <ming.lei@canonical.com>
>> ---
>> drivers/usb/serial/usb-serial.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
>> index a19ed74..9d0b9c8 100644
>> --- a/drivers/usb/serial/usb-serial.c
>> +++ b/drivers/usb/serial/usb-serial.c
>> @@ -1142,10 +1142,11 @@ int usb_serial_suspend(struct usb_interface *intf, pm_message_t message)
>>
>> if (serial->type->suspend) {
>> r = serial->type->suspend(serial, message);
>> - if (r < 0) {
>> + if (r < 0 && PMSG_IS_AUTO(message)) {
>> serial->suspending = 0;
>> goto err_out;
>> }
>> + /* TODO: resume() might need to handle suspend failure */
>> }
>>
>> for (i = 0; i < serial->num_ports; ++i) {
>
> Sorry for the late reply.
>
> The usb-serial subdriver suspend callbacks do not and must not return
> non-zero if !PMSG_IS_AUTO(message) so adding code to handle that case
> merely obfuscates this fact.
>
> I'd rather see this documented with a comment just as Bjørn suggested
> for cdc_mbim and qmi_wwan.
No problem, will do it in v3.
Thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox