Netdev List
 help / color / mirror / Atom feed
* Re: [Bugme-new] [Bug 16603] New: send of data > 4 GB fails on 64 bit systems
From: David Miller @ 2010-09-28  2:41 UTC (permalink / raw)
  To: akpm; +Cc: netdev, bugzilla-daemon, bugme-daemon, bono
In-Reply-To: <20100927161540.776f748e.akpm@linux-foundation.org>

From: Andrew Morton <akpm@linux-foundation.org>
Date: Mon, 27 Sep 2010 16:15:40 -0700

> Yes, I think seglen should be size_t.  Do you know if making that
> change fixes the bug?

Oh boy, what a rats nest.

Just scanning generically I see that net/core/iovec.c:verify_iovec()
has similar issues, it should probably use "long" instead of "int"
because it's trying to prevent the return value being interpreted
as an error code.

So if you fix that and make it return "long" this leads to another
set of problems, even if you fix that TCP bit sys_sendmsg() holds
the total length in an 'int' too.  So more stuff to fix.

I'll try to do the whole audit, but no promises ;)

^ permalink raw reply

* pull-request: bluetooth-2.6 2010-09-27
From: Gustavo F. Padovan @ 2010-09-28  2:30 UTC (permalink / raw)
  To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
  Cc: linville-2XuSBdqkA4R54TAoqtyWWQ, marcel-kz+m5ild9QBg9hUCZPvPmw,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA

Hi David,

These are the Bluetooth fixes for 2.6.36, you are used to see Marcel doing this
job, but he has been busy lately so I'm taking the job of the pull request this
time. Since we are a bit late on the pull request I'm skipping the
wireless-2.6 step, tell me if you (including John here) disagree.

In this patch set we have two fixes for regressions in L2CAP due to ERTM code
we added in L2CAP for 2.6.36, a bugfix in the L2CAP Streaming Mode that was
making the kernel crash. And a fix for a deadlock issue between the sk_sndbuf
and the backlog queue in ERTM. The rest are also needed bug fixes.

Please tell me any problem you have pulling this.

PS: Considering that these patches do not go immediately to net-next how do I
run the Bluetooth pull request to net-next? Currently my -next tree is rebased
on net-next but before the bluetooth-next patches I put the bluetooth patches
I have in my bluetooth-2.6 tree, i.e., when making the pull request I have to be
sure that the -next patches will be on top of the patches I'm submitting now
for 2.6.36. Any help on that is welcome. :)

Regards,


---

The following changes since commit b30a3f6257ed2105259b404d419b4964e363928c:

  Linux 2.6.36-rc5 (2010-09-20 16:56:53 -0700)

are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/padovan/bluetooth-2.6.git master

Andrei Emeltchenko (1):
      Bluetooth: fix MTU L2CAP configuration parameter

Gustavo F. Padovan (4):
      Bluetooth: Simplify L2CAP Streaming mode sending
      Bluetooth: Fix inconsistent lock state with RFCOMM
      Revert "Bluetooth: Don't accept ConfigReq if we aren't in the BT_CONFIG state"
      Bluetooth: Fix deadlock in the ERTM logic

Mat Martineau (1):
      Bluetooth: Only enable L2CAP FCS for ERTM or streaming

 include/net/bluetooth/bluetooth.h |   11 +++++++
 net/bluetooth/l2cap.c             |   57 +++++++++++++++---------------------
 net/bluetooth/rfcomm/sock.c       |    4 ++
 3 files changed, 39 insertions(+), 33 deletions(-)

-- 
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

^ permalink raw reply

* RE: [PATCH v11 00/17] Provide a zero-copy method on KVM virtio-net.
From: Xin, Xiaohui @ 2010-09-28  1:25 UTC (permalink / raw)
  To: Xin, Xiaohui, Michael S. Tsirkin
  Cc: netdev@vger.kernel.org, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org, mingo@elte.hu, davem@davemloft.net,
	herbert@gondor.hengli.com.au, jdike@linux.intel.com
In-Reply-To: <F2E9EB7348B8264F86B6AB8151CE2D792EC64F74FA@shsmsx502.ccr.corp.intel.com>

>-----Original Message-----
>From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-owner@vger.kernel.org] On
>Behalf Of Xin, Xiaohui
>Sent: Monday, September 27, 2010 8:45 AM
>To: Michael S. Tsirkin
>Cc: netdev@vger.kernel.org; kvm@vger.kernel.org; linux-kernel@vger.kernel.org;
>mingo@elte.hu; davem@davemloft.net; herbert@gondor.hengli.com.au;
>jdike@linux.intel.com
>Subject: RE: [PATCH v11 00/17] Provide a zero-copy method on KVM virtio-net.
>
>>From: Michael S. Tsirkin [mailto:mst@redhat.com]
>>Sent: Monday, September 27, 2010 1:02 AM
>>To: Xin, Xiaohui
>>Cc: netdev@vger.kernel.org; kvm@vger.kernel.org; linux-kernel@vger.kernel.org;
>>mingo@elte.hu; davem@davemloft.net; herbert@gondor.hengli.com.au;
>>jdike@linux.intel.com
>>Subject: Re: [PATCH v11 00/17] Provide a zero-copy method on KVM virtio-net.
>>
>>On Sat, Sep 25, 2010 at 12:27:18PM +0800, xiaohui.xin@intel.com wrote:
>>> We provide an zero-copy method which driver side may get external
>>> buffers to DMA. Here external means driver don't use kernel space
>>> to allocate skb buffers. Currently the external buffer can be from
>>> guest virtio-net driver.
>>>
>>> The idea is simple, just to pin the guest VM user space and then
>>> let host NIC driver has the chance to directly DMA to it.
>>> The patches are based on vhost-net backend driver. We add a device
>>> which provides proto_ops as sendmsg/recvmsg to vhost-net to
>>> send/recv directly to/from the NIC driver. KVM guest who use the
>>> vhost-net backend may bind any ethX interface in the host side to
>>> get copyless data transfer thru guest virtio-net frontend.
>>> patch 01-10:  	net core and kernel changes.
>>> patch 11-13:  	new device as interface to mantpulate external buffers.
>>> patch 14: 	for vhost-net.
>>> patch 15:	An example on modifying NIC driver to using napi_gro_frags().
>>> patch 16:	An example how to get guest buffers based on driver
>>> 		who using napi_gro_frags().
>>> patch 17:	It's a patch to address comments from Michael S. Thirkin
>>> 		to add 2 new ioctls in mp device.
>>> 		We split it out here to make easier reiewer.
>>
>>
>>
>>I commented on how to avoid mm semaphore on data path separately, and
>>since you didn't have time to review that yet, I won't repeat that here.
>>
>
>I think I did avoid that in data path to use mm semaphore. I stored the value
>in mp structures and check with that.
>
Hi Michael,
Did you think I have addressed your comments about to avoid mm semaphore on
data path or not? I stored the value in mp structures and don't use mmap_semphore.
Or you still have some concerns with it?

>>At this point what are the plans on macvtap integration?
>>You indicated this is the interface you intend to use longterm.
>
>I'm now working on that.
>
I'm now trying to export some functions from mp device, and let macvtap to use them
for rx zero-copy.
>
>Thanks
>Xiaohui
>--
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* Re: IGMP v3 reponse
From: David Stevens @ 2010-09-28  0:41 UTC (permalink / raw)
  To: Bob Arendt
  Cc: Christoph Lameter, Jason Gunthorpe,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	netdev-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <4CA12DF3.6050608-x0S3BwdUo6DQT0dZR+AlfA@public.gmane.org>

Bob Arendt <rda-x0S3BwdUo6DQT0dZR+AlfA@public.gmane.org> wrote on 09/27/2010 04:51:15 PM:

> Per rfc 2236, the v2 client *can't* drop the IGMPv3 query.  From para 
2.5:
>    2.5.  Other fields
>     Note that IGMP messages may be longer than 8 octets, especially
>     future backwards-compatible versions of IGMP.  As long as the Type 
is
>     one that is recognized, an IGMPv2 implementation MUST ignore 
anything
>     past the first 8 octets while processing the packet.  However, the
>     IGMP checksum is always computed over the whole IP payload, not just
>     over the first 8 octets.
> 
> The IGMPv3 query *is* a valid v2 query with extra crap at the end (it's
> backward compatible).  Per rfc 2236 p2.5, the v3 query has to be 
regarded
> as a valid v2 query if you're correctly implementing IGMPv2.  Some 
switch
> vendors (Cisco for one) only generate v3 queries when operating as an
> IGMP snooping switch, since a proper v2 client will respond with IGMPv2
> packets and it handles this properly.  However, we're seeing some 
switches
> get confused when a client initial joins with v2, then responds to a 
query
> with v3.  It ends up creating 2 entries, and only one is cleared by the
> leave message.  This is also an issue when the primary (querier) switch
> only generates v3 queries, and some intermediate switches only support 
v2.
> We set all the Linux clients to v2 .. but they respond the v2 query with
> v3 protocols, which could be missed by the intermediate switch.
> 
> I believe the intent of the bug 18212 patch is correct.

Bob,
        That would've been a nice quote for the other discussion; I didn't
do the v2 implementation for Linux, but it appears I broke this. I'll
take another look (and thanks for pointing that out!).

                                                        +-DLS

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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: IGMP v3 reponse
From: Bob Arendt @ 2010-09-27 23:51 UTC (permalink / raw)
  To: David Stevens
  Cc: Christoph Lameter, Jason Gunthorpe, linux-rdma@vger.kernel.org,
	netdev@vger.kernel.org, netdev-owner@vger.kernel.org
In-Reply-To: <OF9D99FA29.B3F53176-ON882577AB.00780023-882577AB.0078FAAE@us.ibm.com>

On 09/27/10 15:01, David Stevens wrote:
> Bob Arendt<rda@rincon.com>  wrote on 09/27/2010 01:54:36 PM:
>> And it turns out that  force_igmp_version=2 doesn't fully work.
>> If the host sees a IGMPv3 query, it still responds with a v3 Join
>> despite the flag.  Bug report and candidate patch here:
>> https://bugzilla.kernel.org/show_bug.cgi?id=18212
>
>          This is a special case. The "correct" alternative is to drop
> the query and not send any report at all. Sending an answer in the
> originating protocol doesn't hurt anything here, because MC routers
> are required to use the earlier version too; there should be no such
> thing as an "IGMPv3-only querier" as in that report. IGMPv3 compliance
> *requires* falling back to IGMPv2 if there is a v2 query by another
> router.
>          By answering instead of dropping, it allows fuller filter
> information from a manual query to be returned even if the network
> is using v2 MC routers, but dropping and ignoring the query as
> required by RFC does not fix the bug&  patch submitter's problem.
> Which is why I also NACKed that patch.
>
>                                                          +-DLS
>
>
Per rfc 2236, the v2 client *can't* drop the IGMPv3 query.  From para 2.5:
   2.5.  Other fields
    Note that IGMP messages may be longer than 8 octets, especially
    future backwards-compatible versions of IGMP.  As long as the Type is
    one that is recognized, an IGMPv2 implementation MUST ignore anything
    past the first 8 octets while processing the packet.  However, the
    IGMP checksum is always computed over the whole IP payload, not just
    over the first 8 octets.

The IGMPv3 query *is* a valid v2 query with extra crap at the end (it's
backward compatible).  Per rfc 2236 p2.5, the v3 query has to be regarded
as a valid v2 query if you're correctly implementing IGMPv2.  Some switch
vendors (Cisco for one) only generate v3 queries when operating as an
IGMP snooping switch, since a proper v2 client will respond with IGMPv2
packets and it handles this properly.  However, we're seeing some switches
get confused when a client initial joins with v2, then responds to a query
with v3.  It ends up creating 2 entries, and only one is cleared by the
leave message.  This is also an issue when the primary (querier) switch
only generates v3 queries, and some intermediate switches only support v2.
We set all the Linux clients to v2 .. but they respond the v2 query with
v3 protocols, which could be missed by the intermediate switch.

I believe the intent of the bug 18212 patch is correct.

-Bob Arendt

^ permalink raw reply

* Re: TCP: orphans broken by RFC 2525 #2.17
From: Herbert Xu @ 2010-09-27 23:28 UTC (permalink / raw)
  To: Willy Tarreau; +Cc: davem, netdev
In-Reply-To: <20100927192112.GT12373@1wt.eu>

Willy Tarreau <w@1wt.eu> wrote:
> 
> Two quick facts :
>  - HTTP allows the client to send whatever it wants whenever it wants
>    and allows the server to close after whatever response it wants.
>    Thus the server cannot predict that the client will talk.

No it does not.  Only buggy HTTP clients do that.  Also, have
you ever observed any buggy HTTP client that sends more than
one CRLF? If not then you only have to deal with the case of
a single extra CRLF.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [Bugme-new] [Bug 16603] New: send of data > 4 GB fails on 64 bit systems
From: Andrew Morton @ 2010-09-27 23:15 UTC (permalink / raw)
  To: netdev; +Cc: bugzilla-daemon, bugme-daemon, bono
In-Reply-To: <bug-16603-10286@https.bugzilla.kernel.org/>


(switched to email.  Please respond via emailed reply-to-all, not via the
bugzilla web interface).

On Mon, 16 Aug 2010 10:01:09 GMT
bugzilla-daemon@bugzilla.kernel.org wrote:

> https://bugzilla.kernel.org/show_bug.cgi?id=16603
> 
>            Summary: send of data > 4 GB fails on 64 bit systems
>            Product: Networking
>            Version: 2.5
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: IPV4
>         AssignedTo: shemminger@linux-foundation.org
>         ReportedBy: bono@onlinehome.de
>         Regression: No
> 
> 
> Sending of data using linux function send fails if size is too large. glibc
> function is:
> 
> ssize_t send(int sockfd, const void *buf, size_t len, int flags);
> 
> Type of len is size_t, argument is stored in a kernel structure msgheader which
> contains an iovec. This iovec contains a size_t (64bit) length field, but in
> the linux kernel in function tcp_sendmsg the following lines
> 
>        while (--iovlen >= 0) {
>                 int seglen = iov->iov_len;
>                 unsigned char __user *from = iov->iov_base;
> 
> 
> convert the len to int (32 bit).
> 
> Thus sending of 5 GB of data results in 1 GB sent (no problem), but sending of
> 4 GB results in 0 bytes sent.
> 
> Workaround in userspace is easy (e.g. instead of len use len < 0x8000000 ? len
> : 0x7fffffff) but the kernel should handle this correctly.
> 

whoops.

Yes, I think seglen should be size_t.  Do you know if making that
change fixes the bug?



^ permalink raw reply

* Re: [PATCH] net/9p: Mount only matching virtio channels
From: David Miller @ 2010-09-27 22:54 UTC (permalink / raw)
  To: sven.eckelmann; +Cc: ericvh, netdev, linux-kernel
In-Reply-To: <1285514893-11233-1-git-send-email-sven.eckelmann@gmx.de>

From: Sven Eckelmann <sven.eckelmann@gmx.de>
Date: Sun, 26 Sep 2010 17:28:13 +0200

> p9_virtio_create will only compare the the channel's tag characters
> against the device name till the end of the channel's tag but not till
> the end of the device name. This means that if a user defines channels
> with the tags foo and foobar then he would mount foo when he requested
> foonot and may mount foo when he requested foobar.
> 
> Thus it is necessary to check both string lengths against each other in
> case of a successful partial string match.
> 
> Signed-off-by: Sven Eckelmann <sven.eckelmann@gmx.de>

I'll apply this, but this code would be so much simpler and this bug
would have never happened if these tags were NULL terminated when they
were pulled in from virtio.

^ permalink raw reply

* Re: [PATCH] de2104x: fix ethtool
From: David Miller @ 2010-09-27 22:40 UTC (permalink / raw)
  To: jgarzik; +Cc: linux, netdev, linux-kernel
In-Reply-To: <4CA11546.8@pobox.com>

From: Jeff Garzik <jgarzik@pobox.com>
Date: Mon, 27 Sep 2010 18:05:58 -0400

> On 09/27/2010 05:41 PM, Ondrej Zary wrote:
>> When the interface is up, using ethtool breaks it because:
>> a) link is put down but media_timer interval is not shortened to
>> NO_LINK
>> b) rxtx is stopped but not restarted
>>
>> Also manual 10baseT-HD (and probably FD too - untested) mode does not
>> work -
>> the link is forced up, packets are transmitted but nothing is
>> received.
>> Changing CSR14 value to match documentation (not disabling link check)
>> fixes this.
>>
>> Signed-off-by: Ondrej Zary<linux@rainbow-software.org>
> 
> Acked-by: Jeff Garzik <jgarzik@redhat.com>
> 
> Good catch on the missing de_start_rxtx()

Applied, thanks everyone.

^ permalink raw reply

* Re: [stable 2.6.32] instant crash (jump to NULL) with virtio-net, tap, bridge and veth
From: Michael Tokarev @ 2010-09-27 22:18 UTC (permalink / raw)
  To: Kernel Mailing List, netdev, kvm-devel
In-Reply-To: <4C753BC0.6010803@msgid.tls.msk.ru>

Replying to my own, quite old (more than a month old)
email, and top-posting as well.

I had a chance finally to try another theory with this
problem -- the suspect this time was stack overflow.
And indeed it looks like the case.  I can disable
the bridge hooks in /proc/sys/net/bridge/, and the
system works just fine (in the backtraces we can
see ip_rcv_finish() and ip_rcv() calls, which are
in the NF_HOOK macro).

So, by disabling all nf hooks the problem goes away.
After enabling them again the kernel crashes again
as before.

Since this is our production host, I wont do more
tests in a near future, leaving the nf hooks disabled.

Thanks for listening!

/mjt

25.08.2010 19:50, Michael Tokarev wrote:
> Hello.
> 
> I'm seeing instant host kernel crash triggered by _any_
> network activity to/from a kvm guest that's using virtio-net.
> 
> My setup is maybe a bit unusual, but here we go.
> 
> I've a host machine that has one bridge configured,
> and is running a few kvm virtual machines and a few
> linux containers (LXC).  All the guests/containers
> are "connected" to that single bridge - guests using
> tap devices, lxc containers using veth devices. Host
> eth0 is connected to the same bridge as well.
> 
> The problem happens with virtio-net drivers used in
> guest (this is windowsXP virtual machine with latest
> netkvm driver from alt.fedoraproject.org), when I
> connect to that guest from an LXC container.  I.e,
> when packet goes lxc => veth => bridge => tun =>
> kvm => virtio in guest (or back).
> 
> When I connect to the same guest from _host_, it all
> works as expected.  When I change (virtual) NIC in
> guest to e1000 or older (from 2009) virtio-net driver,
> it works.  When I connect from lxc container to a
> linux guest with latest virtio-net drivers, it all
> works as expected too.  So only one combination so
> far that triggers the issue.
> 
> This is all with 2.6.32 kernel.  Initially it was
> 2.6.32.15, but 2.6.32.20 behaves the same way too.
> All 64bit.
> 
> Also it does NOT happen with 2.6.35.3, the current
> latest released kernel.
> 
> Here's one of captured OOPSes (i did it several
> times, but they were incomplete):
> 
> console [netcon0] enabled
> netconsole: network logging started
> BUG: unable to handle kernel NULL pointer dereference at (null)
> IP: [<(null)>] (null)
> PGD 177bf2067 PUD 177ae5067 PMD 0
> Oops: 0010 [#1] SMP
> last sysfs file: /sys/devices/virtual/block/md8/md/mismatch_cnt
> CPU 0
> Modules linked in: netconsole configfs squashfs kvm_amd kvm veth autofs4 bridge quota_v2 quota_tree ext4 jbd2 crc16 raid0 raid456 async_pq async_xor xor async_memcpy async_raid6_recov raid6_pq async_tx loop sr_mod cdrom tun powernow_k8 processor thermal_sys 8021q garp stp llc asus_atk0110 hwmon atl1 mii ext3 jbd mbcache raid1 md_mod pata_atiixp ehci_hcd ohci_hcd usbcore nls_base ahci libata sd_mod scsi_mod
> Pid: 2345, comm: kvm Not tainted 2.6.32-amd64 #2.6.32.20 System Product Name
> RIP: 0010:[<0000000000000000>]  [<(null)>] (null)
> RSP: 0018:ffff880028203e70  EFLAGS: 00010293
> RAX: ffff880179480ec0 RBX: ffff8801a07770c0 RCX: 0000000000000000
> RDX: 0000000000000000 RSI: ffff8801a07770c0 RDI: ffff8801a07770c0
> RBP: ffff880124b89030 R08: ffffffff8125fab0 R09: ffff880028203e40
> R10: 0000000000000000 R11: 0000000000000000 R12: ffff880028210888
> R13: ffff880028210880 R14: 000000010000e60f R15: 0000000000000040
> FS:  00007fe2da5e5700(0000) GS:ffff880028200000(0000) knlGS:00000000f74a59d0
> CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> CR2: 0000000000000000 CR3: 0000000177a8a000 CR4: 00000000000006f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> Process kvm64 (pid: 2345, threadinfo ffff880177be2000, task ffff880177a7c0c0)
> Stack:
>  ffffffff8125fbd5 0000000000000040 ffffffff8126013c 0000000080000000
> <0> ffff8800282108b8 0000000000000002 ffff880028210888 ffff880028210880
> <0> ffffffff81236276 ffff880028203f48 ffff8800282108b8 0000000000000000
> Call Trace:
>  <IRQ>
>  [<ffffffff8125fbd5>] ? ip_rcv_finish+0x125/0x430
>  [<ffffffff8126013c>] ? ip_rcv+0x25c/0x350
>  [<ffffffff81236276>] ? process_backlog+0x76/0xd0
>  [<ffffffff81236a18>] ? net_rx_action+0xf8/0x1f0
>  [<ffffffff81059120>] ? __do_softirq+0xb0/0x1d0
>  [<ffffffff8100c56c>] ? call_softirq+0x1c/0x30
>  <EOI>
>  [<ffffffff8100e595>] ? do_softirq+0x65/0xa0
>  [<ffffffff81236b2e>] ? netif_rx_ni+0x1e/0x30
>  [<ffffffffa014e97a>] ? tun_chr_aio_write+0x35a/0x510 [tun]
>  [<ffffffffa014e620>] ? tun_chr_aio_write+0x0/0x510 [tun]
>  [<ffffffff810ffea4>] ? do_sync_readv_writev+0xd4/0x110
>  [<ffffffff8106e890>] ? autoremove_wake_function+0x0/0x30
>  [<ffffffff81071709>] ? enqueue_hrtimer+0x79/0xc0
>  [<ffffffff810ffd08>] ? rw_copy_check_uvector+0x88/0x110
>  [<ffffffff811005bc>] ? do_readv_writev+0xdc/0x220
>  [<ffffffff8106dafc>] ? sys_timer_settime+0x13c/0x2e0
>  [<ffffffff8110084e>] ? sys_writev+0x4e/0x90
>  [<ffffffff8100b482>] ? system_call_fastpath+0x16/0x1b
> Code:  Bad RIP value.
> RIP  [<(null)>] (null)
>  RSP <ffff880028203e70>
> CR2: 0000000000000000
> ---[ end trace 1dcd3c52bde0fa25 ]---
> Kernel panic - not syncing: Fatal exception in interrupt
> Pid: 2345, comm: kvm Tainted: G      D    2.6.32-amd64 #2.6.32.20
> Call Trace:
>  <IRQ>  [<ffffffff812c22de>] ? panic+0x7a/0x134
>  [<ffffffff812c23d8>] ? printk+0x40/0x48
>  [<ffffffff8100faa3>] ? oops_end+0xa3/0xb0
>  [<ffffffff8103138a>] ? no_context+0xfa/0x260
>  [<ffffffff812c52a5>] ? page_fault+0x25/0x30
>  [<ffffffff8125fab0>] ? ip_rcv_finish+0x0/0x430
>  [<ffffffff8125fbd5>] ? ip_rcv_finish+0x125/0x430
>  [<ffffffff8126013c>] ? ip_rcv+0x25c/0x350
>  [<ffffffff81236276>] ? process_backlog+0x76/0xd0
>  [<ffffffff81236a18>] ? net_rx_action+0xf8/0x1f0
>  [<ffffffff81059120>] ? __do_softirq+0xb0/0x1d0
>  [<ffffffff8100c56c>] ? call_softirq+0x1c/0x30
>  <EOI>  [<ffffffff8100e595>] ? do_softirq+0x65/0xa0
>  [<ffffffff81236b2e>] ? netif_rx_ni+0x1e/0x30
>  [<ffffffffa014e97a>] ? tun_chr_aio_write+0x35a/0x510 [tun]
>  [<ffffffffa014e620>] ? tun_chr_aio_write+0x0/0x510 [tun]
>  [<ffffffff810ffea4>] ? do_sync_readv_writev+0xd4/0x110
>  [<ffffffff8106e890>] ? autoremove_wake_function+0x0/0x30
>  [<ffffffff81071709>] ? enqueue_hrtimer+0x79/0xc0
>  [<ffffffff810ffd08>] ? rw_copy_check_uvector+0x88/0x110
>  [<ffffffff811005bc>] ? do_readv_writev+0xdc/0x220
>  [<ffffffff8106dafc>] ? sys_timer_settime+0x13c/0x2e0
>  [<ffffffff8110084e>] ? sys_writev+0x4e/0x90
>  [<ffffffff8100b482>] ? system_call_fastpath+0x16/0x1b
> Rebooting in 60 seconds..
> 
> 
> Another:
> 
> BUG: unable to handle kernel NULL pointer dereference at (null)
> IP: [<(null)>] (null)
> PGD 10c804067 PUD 212d0e067 PMD 0
> Oops: 0010 [#1] SMP
> last sysfs file: /sys/devices/virtual/vc/vcsa2/dev
> CPU 0
> Modules linked in: netconsole configfs squashfs kvm_amd kvm veth autofs4 bridge quota_v2 quota_tree ext4 jbd2 crc16 raid0 raid456 async_pq async_xor xor async_memcpy async_raid6_recov raid6_pq async_tx loop sr_mod cdrom tun powernow_k8 processor thermal_sys 8021q garp stp llc asus_atk0110 hwmon atl1 mii ext3 jbd mbcache raid1 md_mod pata_atiixp ehci_hcd ohci_hcd usbcore nls_base [<ffffffff8100bff3>] ? apic_timer_interrupt+0x13/0x20
>  [<ffffffff8100fced>] ? oops_end+0x9d/0xb0
>  [<ffffffff810320b7>] ? no_context+0xf7/0x260
>  [<ffffffff81032375>] ? __bad_area_nosemaphore+0x155/0x230
>  [<ffffffffa0273ea0>] ? br_nf_pre_routing_finish+0x0/0x350 [bridge]
>  [<ffffffffa0274759>] ? br_nf_pre_routing+0x569/0x880 [bridge]
>  [<ffffffff812cc945>] ? page_fault+0x25/0x30
>  [<ffffffff812650a0>] ? ip_rcv+0x0/0x350
>  [<ffffffff81264c60>] ? ip_rcv_finish+0x0/0x440
>  [<ffffffff81264e19>] ? ip_rcv_finish+0x1b9/0x440
>  [<ffffffff81265354>] ? ip_rcv+0x2b4/0x350
>  [<ffffffff8123ba85>] ? process_backlog+0x75/0xc0
>  [<ffffffff8123c246>] ? net_rx_action+0x106/0x220
>  [<ffffffff8105abcb>] ? __do_softirq+0xfb/0x1d0
>  [<ffffffff8100c62c>] ? call_softirq+0x1c/0x30
>  <EOI>  [<ffffffff8100e765>] ? do_softirq+0x65/0xa0
>  [<ffffffff8123c379>] ? netif_rx_ni+0x19/0x20
>  [<ffffffffa0151b0b>] ? tun_chr_aio_write+0x3fb/0x550 [tun]
>  [<ffffffffa0151710>] ? tun_chr_aio_write+0x0/0x550 [tun]
>  [<ffffffff811031fb>] ? do_sync_readv_writev+0xcb/0x110
>  [<ffffffff81065941>] ? __dequeue_signal+0xe1/0x210
>  [<ffffffff810706b0>] ? autoremove_wake_function+0x0/0x30
>  [<ffffffff81012bc2>] ? read_tsc+0x12/0x40
>  [<ffffffff81024608>] ? lapic_next_event+0x18/0x20
>  [<ffffffff8107d156>] ? tick_dev_program_event+0x36/0xb0
>  [<ffffffff81103036>] ? rw_copy_check_uvector+0x86/0x130
>  [<ffffffff81103912>] ? do_readv_writev+0xe2/0x230
>  [<ffffffff8106f883>] ? sys_timer_settime+0x153/0x350
>  [<ffffffff81103bb3>] ? sys_writev+0x53/0xa0
>  [<ffffffff8100b542>] ? system_call_fastpath+0x16/0x1b
> Rebooting in 60 seconds..
> 
> I looked at the changes in tun, virtio-net, bridge code and
> veth between 2.6.32 and 2.6.35, but I see nothing relevant
> in there (but I'm not an expert in that area anyway). The
> changes mentions a few crashes, but all were related to
> device registration/deregistration or module unload, not
> to normal send/receive path.
> 
> It will be really nice to fix this for long-stable 2.6.32
> series... ;)
> 
> Thanks!
> 
> /mjt
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* Re: [PATCH] de2104x: fix ethtool
From: Jeff Garzik @ 2010-09-27 22:05 UTC (permalink / raw)
  To: Ondrej Zary; +Cc: netdev, Kernel development list
In-Reply-To: <201009272341.50668.linux@rainbow-software.org>

On 09/27/2010 05:41 PM, Ondrej Zary wrote:
> When the interface is up, using ethtool breaks it because:
> a) link is put down but media_timer interval is not shortened to NO_LINK
> b) rxtx is stopped but not restarted
>
> Also manual 10baseT-HD (and probably FD too - untested) mode does not work -
> the link is forced up, packets are transmitted but nothing is received.
> Changing CSR14 value to match documentation (not disabling link check) fixes this.
>
> Signed-off-by: Ondrej Zary<linux@rainbow-software.org>

Acked-by: Jeff Garzik <jgarzik@redhat.com>

Good catch on the missing de_start_rxtx()

^ permalink raw reply

* Re: [GIT PULL net-2.6] vhost-net: last minute fix
From: David Miller @ 2010-09-27 22:04 UTC (permalink / raw)
  To: mst; +Cc: kvm, virtualization, netdev, linux-kernel
In-Reply-To: <20100927133411.GA4033@redhat.com>

From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Mon, 27 Sep 2010 15:34:13 +0200

> The following tree includes a last minute bugfix for vhost-net.
> It is on top of net-2.6.  Please merge it for 2.6.36.

Pulled, thanks Michael.

^ permalink raw reply

* Re: [PATCH] tproxy: check for transparent flag in ip_route_newports
From: David Miller @ 2010-09-27 22:03 UTC (permalink / raw)
  To: uweber; +Cc: netdev
In-Reply-To: <20100927133100.GB8916@babylon>

From: Ulrich Weber <uweber@astaro.com>
Date: Mon, 27 Sep 2010 15:31:00 +0200

> as done in ip_route_connect()
> 
> Signed-off-by: Ulrich Weber <uweber@astaro.com>

Applied, thanks for fixing this.

^ permalink raw reply

* Re:
From: David Stevens @ 2010-09-27 22:01 UTC (permalink / raw)
  To: Bob Arendt
  Cc: Christoph Lameter, Jason Gunthorpe, linux-rdma@vger.kernel.org,
	netdev@vger.kernel.org, netdev-owner@vger.kernel.org
In-Reply-To: <4CA1048C.8020508@rincon.com>

Bob Arendt <rda@rincon.com> wrote on 09/27/2010 01:54:36 PM:
 
> On 09/27/10 13:23, Christoph Lameter wrote:
> > On Mon, 27 Sep 2010, David Stevens wrote:
> >
> >>          Because a querier can set the robustness value and
> >> query interval to anything you want. In the original report,
> >> he's not running a querier. The fact that it's a new group
> >> doesn't matter -- these are per-interface.
> >
> > The per interface settings are used to force an IGMP version 
overriding
> > any information by the queriers. You would not want to enable that 
because
> > it disables support for other IGMP versions. Without the override
> > different version of IGMP can be handled per MC group.
> >
> If a network vlan has IGMPv3 capability, then it should be able
> to support both v2 and v3 Joins (clients).  But if the vlan is
> IGMPv2 only, then an initial Join from a Linux client might go out
> as v3 (if it hasn't seen a query yet) and be ignored.  I believe
> this is the case that force_igmp_version really addresses.

        Not really. It's for the case where there is no querier at all,
but a snooping switch that only supports IGMPv2. After any query has
put an interface in IGMPv2 mode (or IGMPv1), the initial report for
all joins will use the earlier protocol. It isn't per-group, it's
per interface, and you cannot mix versions of IGMP on the same network.

> 
> And it turns out that  force_igmp_version=2 doesn't fully work.
> If the host sees a IGMPv3 query, it still responds with a v3 Join
> despite the flag.  Bug report and candidate patch here:
> https://bugzilla.kernel.org/show_bug.cgi?id=18212

        This is a special case. The "correct" alternative is to drop
the query and not send any report at all. Sending an answer in the
originating protocol doesn't hurt anything here, because MC routers
are required to use the earlier version too; there should be no such
thing as an "IGMPv3-only querier" as in that report. IGMPv3 compliance
*requires* falling back to IGMPv2 if there is a v2 query by another
router. 
        By answering instead of dropping, it allows fuller filter
information from a manual query to be returned even if the network
is using v2 MC routers, but dropping and ignoring the query as
required by RFC does not fix the bug & patch submitter's problem.
Which is why I also NACKed that patch.

                                                        +-DLS



^ permalink raw reply

* Re: [PATCH] fix TSO FACK loss marking in tcp_mark_head_lost
From: David Miller @ 2010-09-27 21:56 UTC (permalink / raw)
  To: ilpo.jarvinen; +Cc: ycheng, netdev
In-Reply-To: <alpine.DEB.2.00.1009272155030.15680@melkinpaasi.cs.helsinki.fi>

From: "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Mon, 27 Sep 2010 22:20:57 +0300 (EEST)

> I disagree now with myself btw, it's not just "a policy decision" as we 
> do not retransmit some segments at all before RTO in some scenarios.

Ok, thanks for explaining all of this.

I've applied Yuchung's patch to net-2.6, thanks everyone!

^ permalink raw reply

* Re:
From: David Stevens @ 2010-09-27 21:50 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Jason Gunthorpe, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	netdev-owner-u79uwXL29TY76Z2rM5mHXA, Bob Arendt
In-Reply-To: <alpine.DEB.2.00.1009271521510.14117-sBS69tsa9Uj/9pzu0YdTqQ@public.gmane.org>

Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org> wrote on 09/27/2010 01:23:00 PM:

> From: Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>
> To: David Stevens/Beaverton/IBM@IBMUS
> Cc: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>, linux-
> rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, netdev-
> owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Bob Arendt <rda-x0S3BwdUo6DQT0dZR+AlfA@public.gmane.org>
> Date: 09/27/2010 01:23 PM
> Subject: Re:
> 
> On Mon, 27 Sep 2010, David Stevens wrote:
> 
> >         Because a querier can set the robustness value and
> > query interval to anything you want. In the original report,
> > he's not running a querier. The fact that it's a new group
> > doesn't matter -- these are per-interface.
> 
> The per interface settings are used to force an IGMP version overriding
> any information by the queriers.

        No. I'm not talking about the force_igmp_tunable here, I'm talking
about the per-interface robustness and interval settings which come from
the querier (whatever version you are using).

> You would not want to enable that because
> it disables support for other IGMP versions. Without the override
> different version of IGMP can be handled per MC group.

        No. IGMPv3 includes backward compatibility for both IGMPv2 and
IGMPv1. If queries for an earlier version are present, that is the
IGMP version all use, and the features of the later version are not
available to anyone.

                                                                +-DLS


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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 v3] xmit_compl_seq: information to reclaim vmsplice buffers
From: Tom Herbert @ 2010-09-27 21:49 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Michael S. Tsirkin, netdev, davem, sridharr
In-Reply-To: <1285614757.2512.6.camel@edumazet-laptop>

> Alternative would be to copy data on retransmits, for tcp sockets using
> SOCK_XMIT_COMPL_SEQ. (ie not using skb_clone but skb_copy())
>

That's coming awfully close to COW! ;-)

>
>
>
>

^ permalink raw reply

* Re: igmp: Allow mininum interval specification for igmp timers.
From: David Stevens @ 2010-09-27 21:45 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: David Miller, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	netdev-owner-u79uwXL29TY76Z2rM5mHXA, rda-x0S3BwdUo6DQT0dZR+AlfA
In-Reply-To: <alpine.DEB.2.00.1009271503420.14117-sBS69tsa9Uj/9pzu0YdTqQ@public.gmane.org>

Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org> wrote on 09/27/2010 01:20:54 PM:

> 
> >         As I said before, I think per protocol, back-to-back is both
> > allowed and not a problem, even if both subsequent randomized reports
> > come out to 0 time. But if we wanted to enforce a minimum interval
> > of, say, X, then I think the better way to do that is to set the
> > timer to X + rand(Interval-X) and not a table of fixed intervals
> 
> The second patch sets the intervals to X .. X + Rand (interval) and not 
to
> a table of fixed intervals as you state here. I have pointed this out
> before.

        Sorry if I've misunderstood something you're proposing, but what
you describe above would be certainly technically incorrect. There are
really no circumstances for sending a report greater than <Interval>
that is protocol-compliant. You can enforce a minimum greater than 0,
which is a departure from both RFCs, though IGMPv2 uses wishy-washy
language. The intent for both was to explicitly allow 0, IMO.

> 
> > as in the original patch. For v2, X=1 or 2 sec and Interval=10
> > might work well, but for v3, the entire interval is 1 sec and I
> > think I saw that the set-up time for the fabric may be on the
> > order of 1 sec.
> 
> Again there is no knowledge about V2 or V3 without a query and this is
> during the period when no querier is known yet. You stated elsewhere 
that
> I can assume V3 by default? So 1 sec?

        Yes, without a querier or the tunable to force it to IGMPv2,
the default is IGMPv3. It appears there is a bug where IGMPv3 is also
using a 10sec interval (haven't verified that), but a 1 sec interval
as required makes your situation worse, not better. It makes it even
more likely that all the initial reports will occur before your set-up
is done.
 
> There can be any number of reasons that a short outage could prevent the
> packets from going through. A buffer overrun (that you mentioned
> elsewhere) usually causes lots of packets to be lost. Buffer overrun
> scenarios usually mean that all igmp queries are lost.

        You're arguing against protocol compliance. I didn't define
the protocol, I only implemented it. And your view is through the
IB lens, but I don't believe this is an actual problem in any way
for typical networks. If you wrote a standards-track RFC that modifies
IGMP for NBMA networks that require a delay or different parameters
there, I'd have no objection to implementing that. Unilaterally
changing linux's behavior on all network types without cause for
departing from RFC on the most common types is another matter.


> There is no solution on the IB layer since there is no notification when
> the fabric reconfiguration necessary for an multicast group is complete.

        Certainly that's not true; without notification, you can queue for
first use of a new hardware multicast address and send the queue after an
appropriate delay (1 sec? If that covers your set-up time). If you had
positive acknowledgement from the IB network, you'd know exactly when to
do it, but there's no need to change anything for non-IB networks here.

> The querier is of not use since (for the gazillionth of times) this is 
an
> unsolicited IGMP report. If there is a querier then the unsolicited igmp
> reports would not be used but the timeout indicated by the querier would
> be used.

        A querier affects unsolicited reports because it sets both the
query interval and the robustness value. If you want to send 10 reports,
you can cause that by having a querier that sets it to that many. The
initial join would then send 10 reports and the query interval can also
be as low as you like.
        But the linux code is not just for your particular problem or
particular configuration. You can solve your problem by adding a querier,
but I know you're trying to do it without. The mail I was responding to
referred also to the case of a querier present, which is actually the
"normal" case for using full IGMP is. I'm saying that for the non-querier
case, making those per-interface configurable is reasonable because
they *are* querier-changeable, but you can also use a querier to change
it _for_the_unsolicited_reports_, as well as making the querier interval
small enough that you don't have to care at all whether any or all of
the unsolicited reports are lost.

                                                                +-DLS

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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-2.6 16/17] tg3: Use netif_set_real_num_{rx,tx}_queues()
From: Matt Carlson @ 2010-09-27 21:44 UTC (permalink / raw)
  To: Matt Carlson
  Cc: Ben Hutchings, David Miller, netdev@vger.kernel.org,
	linux-net-drivers@solarflare.com, Michael Chan
In-Reply-To: <20100927214107.GA32336@mcarlson.broadcom.com>

On Mon, Sep 27, 2010 at 02:41:07PM -0700, Matt Carlson wrote:
> On Mon, Sep 27, 2010 at 11:32:37AM -0700, Ben Hutchings wrote:
> > Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> > ---
> > This *always* sets real_num_tx_queues to 1, so this could be improved.
> > For now, do a simple conversion.
> > 
> > Ben.
> > 
> >  drivers/net/tg3.c |    8 ++++++--
> >  1 files changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
> > index fdb438d..ca41140 100644
> > --- a/drivers/net/tg3.c
> > +++ b/drivers/net/tg3.c
> > @@ -8821,7 +8821,11 @@ static bool tg3_enable_msix(struct tg3 *tp)
> >  	for (i = 0; i < tp->irq_max; i++)
> >  		tp->napi[i].irq_vec = msix_ent[i].vector;
> >  
> > -	tp->dev->real_num_tx_queues = 1;
> > +	netif_set_real_num_tx_queues(tp->dev, 1);
> > +	if (netif_set_real_num_rx_queues(tp->dev, tp->irq_cnt)) {
> 
> This should be tp->irq_cnt - 1, not tp->irq_cnt.  The first MSI-X vector
> only handles link interrupts and device errors.

I need to correct myself.  If the irq_cnt > 1, then it needs to be
tp->irq_cnt - 1.  If the irq_cnt == 1, then it should be 1.

> > +		pci_disable_msix(tp->pdev);
> > +		return false;
> > +	}
> >  	if (tp->irq_cnt > 1)
> >  		tp->tg3_flags3 |= TG3_FLG3_ENABLE_RSS;
> >
> > @@ -8856,7 +8860,7 @@ defcfg:
> >  	if (!(tp->tg3_flags2 & TG3_FLG2_USING_MSIX)) {
> >  		tp->irq_cnt = 1;
> >  		tp->napi[0].irq_vec = tp->pdev->irq;
> > -		tp->dev->real_num_tx_queues = 1;
> > +		netif_set_real_num_tx_queues(tp->dev, 1);
> >  	}
> >  }
> >  
> > -- 
> > 1.7.2.1
> > 
> > 
> > 
> > -- 
> > Ben Hutchings, Senior Software Engineer, Solarflare Communications
> > Not speaking for my employer; that's the marketing department's job.
> > They asked us to note that Solarflare product names are trademarked.
> > 
> > 
> 


^ permalink raw reply

* [PATCH] de2104x: fix ethtool
From: Ondrej Zary @ 2010-09-27 21:41 UTC (permalink / raw)
  To: jgarzik; +Cc: netdev, Kernel development list

When the interface is up, using ethtool breaks it because:
a) link is put down but media_timer interval is not shortened to NO_LINK
b) rxtx is stopped but not restarted

Also manual 10baseT-HD (and probably FD too - untested) mode does not work -
the link is forced up, packets are transmitted but nothing is received.
Changing CSR14 value to match documentation (not disabling link check) fixes this.

Signed-off-by: Ondrej Zary <linux@rainbow-software.org>

--- linux-2.6.36-rc3-/drivers/net/tulip/de2104x.c	2010-09-26 21:58:42.000000000 +0200
+++ linux-2.6.36-rc3/drivers/net/tulip/de2104x.c	2010-09-27 23:36:51.000000000 +0200
@@ -364,9 +364,9 @@ static u16 t21040_csr15[] = { 0, 0, 0x00
 
 /* 21041 transceiver register settings: TP AUTO, BNC, AUI, TP, TP FD*/
 static u16 t21041_csr13[] = { 0xEF01, 0xEF09, 0xEF09, 0xEF01, 0xEF09, };
-static u16 t21041_csr14[] = { 0xFFFF, 0xF7FD, 0xF7FD, 0x6F3F, 0x6F3D, };
+static u16 t21041_csr14[] = { 0xFFFF, 0xF7FD, 0xF7FD, 0x7F3F, 0x7F3D, };
 /* If on-chip autonegotiation is broken, use half-duplex (FF3F) instead */
-static u16 t21041_csr14_brk[] = { 0xFF3F, 0xF7FD, 0xF7FD, 0x6F3F, 0x6F3D, };
+static u16 t21041_csr14_brk[] = { 0xFF3F, 0xF7FD, 0xF7FD, 0x7F3F, 0x7F3D, };
 static u16 t21041_csr15[] = { 0x0008, 0x0006, 0x000E, 0x0008, 0x0008, };
 
 
@@ -1596,12 +1596,15 @@ static int __de_set_settings(struct de_p
 		return 0; /* nothing to change */
 
 	de_link_down(de);
+	mod_timer(&de->media_timer, jiffies + DE_TIMER_NO_LINK);
 	de_stop_rxtx(de);
 
 	de->media_type = new_media;
 	de->media_lock = media_lock;
 	de->media_advertise = ecmd->advertising;
 	de_set_media(de);
+	if (netif_running(de->dev))
+		de_start_rxtx(de);
 
 	return 0;
 }


-- 
Ondrej Zary

^ permalink raw reply

* Re: [PATCH net-next-2.6 16/17] tg3: Use netif_set_real_num_{rx,tx}_queues()
From: Matt Carlson @ 2010-09-27 21:41 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: David Miller, netdev@vger.kernel.org,
	linux-net-drivers@solarflare.com, Matthew Carlson, Michael Chan
In-Reply-To: <1285612357.2263.319.camel@achroite.uk.solarflarecom.com>

On Mon, Sep 27, 2010 at 11:32:37AM -0700, Ben Hutchings wrote:
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
> This *always* sets real_num_tx_queues to 1, so this could be improved.
> For now, do a simple conversion.
> 
> Ben.
> 
>  drivers/net/tg3.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
> index fdb438d..ca41140 100644
> --- a/drivers/net/tg3.c
> +++ b/drivers/net/tg3.c
> @@ -8821,7 +8821,11 @@ static bool tg3_enable_msix(struct tg3 *tp)
>  	for (i = 0; i < tp->irq_max; i++)
>  		tp->napi[i].irq_vec = msix_ent[i].vector;
>  
> -	tp->dev->real_num_tx_queues = 1;
> +	netif_set_real_num_tx_queues(tp->dev, 1);
> +	if (netif_set_real_num_rx_queues(tp->dev, tp->irq_cnt)) {

This should be tp->irq_cnt - 1, not tp->irq_cnt.  The first MSI-X vector
only handles link interrupts and device errors.

> +		pci_disable_msix(tp->pdev);
> +		return false;
> +	}
>  	if (tp->irq_cnt > 1)
>  		tp->tg3_flags3 |= TG3_FLG3_ENABLE_RSS;
>
> @@ -8856,7 +8860,7 @@ defcfg:
>  	if (!(tp->tg3_flags2 & TG3_FLG2_USING_MSIX)) {
>  		tp->irq_cnt = 1;
>  		tp->napi[0].irq_vec = tp->pdev->irq;
> -		tp->dev->real_num_tx_queues = 1;
> +		netif_set_real_num_tx_queues(tp->dev, 1);
>  	}
>  }
>  
> -- 
> 1.7.2.1
> 
> 
> 
> -- 
> Ben Hutchings, Senior Software Engineer, Solarflare Communications
> Not speaking for my employer; that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
> 
> 


^ permalink raw reply

* Re: [PATCH v11 17/17]add two new ioctls for mp device.
From: Ben Hutchings @ 2010-09-27 21:36 UTC (permalink / raw)
  To: xiaohui.xin; +Cc: netdev, kvm, linux-kernel, mingo, davem, herbert, jdike
In-Reply-To: <b74412a16bc32fe2550461f25156e5b2563c5a2c.1285385607.git.xiaohui.xin@intel.com>

On Sat, 2010-09-25 at 12:27 +0800, xiaohui.xin@intel.com wrote:
> From: Xin Xiaohui <xiaohui.xin@intel.com>
> 
> The patch add two ioctls for mp device.
> One is for userspace to query how much memory locked to make mp device
> run smoothly. Another one is for userspace to set how much meory locked
> it really wants.
[...]
> diff --git a/include/linux/mpassthru.h b/include/linux/mpassthru.h
> index ba8f320..083e9f7 100644
> --- a/include/linux/mpassthru.h
> +++ b/include/linux/mpassthru.h
> @@ -7,6 +7,8 @@
>  /* ioctl defines */
>  #define MPASSTHRU_BINDDEV      _IOW('M', 213, int)
>  #define MPASSTHRU_UNBINDDEV    _IO('M', 214)
> +#define MPASSTHRU_SET_MEM_LOCKED	_IOW('M', 215, unsigned long)
> +#define MPASSTHRU_GET_MEM_LOCKED_NEED	_IOR('M', 216, unsigned long)
[...]

These will need compat handling.  You can avoid that by defining them to
use a parameter type of u32 or u64.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply

* Re: BUG - qdev - partial loss of network connectivity
From: Leszek Urbanski @ 2010-09-27 21:32 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: netdev, linux-nfs, qemu-devel, virtualization
In-Reply-To: <20100926154324.GD21843@redhat.com>

<20100926154324.GD21843@redhat.com>; from Michael S. Tsirkin on Sun, Sep 26, 2010 at 17:43:24 +0200

> > > >It's vanilla 2.6.32.22, but I also reproduced this on Debian's 2.6.32-23
> > > >(based on 2.6.32.21).
> > > >
> > > >If offload is the only difference, I'll play with different offload
> > > >options and check which one causes it.
> > > >   
> > > 
> > > It's not technically the only difference but it's the most likely 
> > > culprit IMHO.
> > 
> > udp fragmentation offload is definitely the culprit.
> 
> I see. Most likely guest bug - won't be the first bug around UFO.
> If so pls copy netdev linux-nfs and virtualization.
> Do you see anything in dmesg? Can try 2.6.36-rc5?

(for reference: first post is at:
http://lists.nongnu.org/archive/html/qemu-devel/2010-09/msg01685.html )

I can't reproduce it on 2.6.36-rc5. Do you have an idea which patch may have
fixed it, or should I dissect?

2.6.32.x - there's nothing interesting in dmesg, apart from traces related
to tasks in D state waiting on the NFS mounts:

[   84.396127] nfs: server 10.0.0.1 not responding, still trying
[  240.568162] INFO: task cp:1838 blocked for more than 120 seconds.
[  240.569715] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[  240.571486] cp            D 0000000000000002     0  1838   1831 0x00000000
[  240.573340]  ffff88011fa5b880 0000000000000082 0000000000000000 ffff88011e45bb44
[  240.575508]  ffff88011e45bcc8 ffffffff8102cdac 000000000000f9e0 ffff88011e45bfd8
[  240.578827]  0000000000015780 0000000000015780 ffff88011c7ce2e0 ffff88011c7ce5d8
[  240.580502] Call Trace:
[  240.581132]  [<ffffffff8102cdac>] ? pvclock_clocksource_read+0x3a/0x8b
[  240.582427]  [<ffffffff8102cdac>] ? pvclock_clocksource_read+0x3a/0x8b
[  240.583869]  [<ffffffff810b3bdd>] ? sync_page+0x0/0x46
[  240.585034]  [<ffffffff810b3bdd>] ? sync_page+0x0/0x46
[  240.586087]  [<ffffffff812f9939>] ? io_schedule+0x73/0xb7
[  240.587287]  [<ffffffff810b3c1e>] ? sync_page+0x41/0x46
[  240.588202]  [<ffffffff812f9e46>] ? __wait_on_bit+0x41/0x70
[  240.589314]  [<ffffffff810b3da2>] ? wait_on_page_bit+0x6b/0x71
[  240.590630]  [<ffffffff81064a1c>] ? wake_bit_function+0x0/0x23
[  240.591906]  [<ffffffff810bb9ea>] ? pagevec_lookup_tag+0x1a/0x21
[  240.592954]  [<ffffffff810b4577>] ? wait_on_page_writeback_range+0x69/0x11b
[  240.594403]  [<ffffffff810b536e>] ? filemap_write_and_wait+0x26/0x32
[  240.595563]  [<ffffffffa02c0d35>] ? nfs_setattr+0xb9/0x117 [nfs]
[  240.596670]  [<ffffffff810b3a0b>] ? find_get_page+0x1a/0x77
[  240.598012]  [<ffffffff810b3bb9>] ? lock_page+0x9/0x1f
[  240.598878]  [<ffffffff810b41ee>] ? filemap_fault+0xb9/0x2f6
[  240.599839]  [<ffffffff810ca3c2>] ? __do_fault+0x38c/0x3c3
[  240.601003]  [<ffffffff810ee1ce>] ? do_sync_write+0xce/0x113
[  240.602082]  [<ffffffff81051e75>] ? current_fs_time+0x1e/0x24
[  240.602968]  [<ffffffff811009b7>] ? notify_change+0x180/0x2c5
[  240.604245]  [<ffffffff8110b7b5>] ? utimes_common+0x12d/0x14d
[  240.605355]  [<ffffffff8110b856>] ? do_utimes+0x81/0xca
[  240.606558]  [<ffffffff8110b9ab>] ? sys_utimensat+0x5b/0x6a
[  240.607817]  [<ffffffff81010b42>] ? system_call_fastpath+0x16/0x1b
[  240.609124] INFO: task find:1866 blocked for more than 120 seconds.
[  240.610409] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[  240.612066] find          D 0000000000000000     0  1866   1863 0x00000000
[  240.613490]  ffffffff8145d1f0 0000000000000086 0000000000000000 ffff88011e2d2350
[  240.615188]  00000022b63d07c7 ffff88011c55e000 000000000000f9e0 ffff8800c78a5fd8
[  240.616576]  0000000000015780 0000000000015780 ffff88011e2d2350 ffff88011e2d2648
[  240.618297] Call Trace:
[  240.618777]  [<ffffffff810e5369>] ? virt_to_head_page+0x9/0x2a
[  240.619906]  [<ffffffff812fa07a>] ? __mutex_lock_common+0x122/0x192
[  240.621324]  [<ffffffff812fa1a2>] ? mutex_lock+0x1a/0x31
[  240.622543]  [<ffffffff81102c11>] ? mntput_no_expire+0x23/0xee
[  240.623860]  [<ffffffffa02c0b03>] ? nfs_getattr+0x3b/0xda [nfs]
[  240.625219]  [<ffffffff810f1839>] ? vfs_fstatat+0x43/0x57
[  240.626290]  [<ffffffff810f185e>] ? sys_newfstatat+0x11/0x30
[  240.627594]  [<ffffffff81102c11>] ? mntput_no_expire+0x23/0xee
[  240.628768]  [<ffffffff8101195b>] ? device_not_available+0x1b/0x20
[  240.629644]  [<ffffffff81010b42>] ? system_call_fastpath+0x16/0x1b


-- 
Leszek "Tygrys" Urbanski, SCSA, SCNA
 "Unix-to-Unix Copy Program;" said PDP-1. "You will never find a more
  wretched hive of bugs and flamers. We must be cautious." -- DECWARS
     http://cygnus.moo.pl/ -- Cygnus High Altitude Balloon

^ permalink raw reply

* Re: [PATCH v11 13/17] Add mp(mediate passthru) device.
From: Ben Hutchings @ 2010-09-27 21:23 UTC (permalink / raw)
  To: xiaohui.xin; +Cc: netdev, kvm, linux-kernel, mingo, davem, herbert, jdike
In-Reply-To: <2c954a831e2d9a93437aaf06386cd3a5af6b73b9.1285385607.git.xiaohui.xin@intel.com>

On Sat, 2010-09-25 at 12:27 +0800, xiaohui.xin@intel.com wrote:
> From: Xin Xiaohui <xiaohui.xin@intel.com>
> 
> The patch add mp(mediate passthru) device, which now
> based on vhost-net backend driver and provides proto_ops
> to send/receive guest buffers data from/to guest vitio-net
> driver.
> 
> Signed-off-by: Xin Xiaohui <xiaohui.xin@intel.com>
> Signed-off-by: Zhao Yu <yzhao81new@gmail.com>
> Reviewed-by: Jeff Dike <jdike@linux.intel.com>
> ---
>  drivers/vhost/mpassthru.c | 1407 +++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 1407 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/vhost/mpassthru.c
> 
> diff --git a/drivers/vhost/mpassthru.c b/drivers/vhost/mpassthru.c
> new file mode 100644
> index 0000000..d86d94c
> --- /dev/null
> +++ b/drivers/vhost/mpassthru.c
[...]
> +/* #define MPASSTHRU_DEBUG 1 */
> +
> +#ifdef MPASSTHRU_DEBUG
> +static int debug;
> +
> +#define DBG  if (mp->debug) printk
> +#define DBG1 if (debug == 2) printk

This is unsafe; consider this usage:

	if (foo)
		DBG("bar\n");
	else
		baz();

You should use the standard pr_debug() or dev_dbg() instead.

[...]
> +struct page_ctor {
> +       struct list_head        readq;
> +       int                     wq_len;
> +       int                     rq_len;
> +       spinlock_t              read_lock;

Only one queue?!

I would have appreciated some introductory comments on these structures.
I still don't have any sort of clear picture of how this is all supposed
to work.

[...]
> +/* The main function to allocate external buffers */
> +static struct skb_ext_page *page_ctor(struct mpassthru_port *port,
> +		struct sk_buff *skb, int npages)
> +{
> +	int i;
> +	unsigned long flags;
> +	struct page_ctor *ctor;
> +	struct page_info *info = NULL;
> +
> +	ctor = container_of(port, struct page_ctor, port);
> +
> +	spin_lock_irqsave(&ctor->read_lock, flags);
> +	if (!list_empty(&ctor->readq)) {
> +		info = list_first_entry(&ctor->readq, struct page_info, list);
> +		list_del(&info->list);
> +		ctor->rq_len--;
> +	}
> +	spin_unlock_irqrestore(&ctor->read_lock, flags);
> +	if (!info)
> +		return NULL;
> +
> +	for (i = 0; i < info->pnum; i++)
> +		get_page(info->pages[i]);
> +	info->skb = skb;
> +	return &info->ext_page;
> +}

Why isn't the npages parameter used?

[...]
> +static void relinquish_resource(struct page_ctor *ctor)
> +{
> +	if (!(ctor->dev->flags & IFF_UP) &&
> +			!(ctor->wq_len + ctor->rq_len))
> +		printk(KERN_INFO "relinquish_resource\n");
> +}

Looks like something's missing here.

> +static void mp_ki_dtor(struct kiocb *iocb)
> +{
> +	struct page_info *info = (struct page_info *)(iocb->private);
> +	int i;
> +
> +	if (info->flags == INFO_READ) {
> +		for (i = 0; i < info->pnum; i++) {
> +			if (info->pages[i]) {
> +				set_page_dirty_lock(info->pages[i]);
> +				put_page(info->pages[i]);
> +			}
> +		}
> +		mp_hash_delete(info->ctor, info);
> +		if (info->skb) {
> +			info->skb->destructor = NULL;
> +			kfree_skb(info->skb);
> +		}
> +		info->ctor->rq_len--;

Doesn't rq_len represent the number of buffers queued between the guest
and the driver?  It is already decremented in page_ctor() so it seems
like it gets decremented twice for each buffer.  Also don't you need to
hold the read_lock when updating rq_len?

> +	} else
> +		info->ctor->wq_len--;

Maybe you should define rq_len and wq_len both as atomic_t.

[...]
> +static void __mp_detach(struct mp_struct *mp)
> +{
> +	mp->mfile = NULL;
> +
> +	mp_dev_change_flags(mp->dev, mp->dev->flags & ~IFF_UP);
> +	page_ctor_detach(mp);
> +	mp_dev_change_flags(mp->dev, mp->dev->flags | IFF_UP);

This is racy; you should hold rtnl_lock over all these changes.

[...]
> +typedef u32 key_mp_t;
> +static inline key_mp_t mp_hash(struct page *page, int buckets)
> +{
> +	key_mp_t k;
> +
> +	k = ((((unsigned long)page << 32UL) >> 32UL) / 0x38) % buckets ;
> +	return k;
> +}

This is never going to work on a 32-bit machine, and what is the purpose
of the magic number 0x38?

Try using hash_ptr() from <linux/hash.h>.

> +static struct page_info *mp_hash_delete(struct page_ctor *ctor,
> +					struct page_info *info)
> +{
> +	key_mp_t key = mp_hash(info->pages[0], HASH_BUCKETS);
> +	struct page_info *tmp = NULL;
> +	int i;
> +
> +	tmp = ctor->hash_table[key];
> +	while (tmp) {
> +		if (tmp == info) {
> +			if (!tmp->prev) {
> +				ctor->hash_table[key] = tmp->next;
> +				if (tmp->next)
> +					tmp->next->prev = NULL;
> +			} else {
> +				tmp->prev->next = tmp->next;
> +				if (tmp->next)
> +					tmp->next->prev = tmp->prev;
> +			}
> +			return tmp;
> +		}
> +		tmp = tmp->next;
> +	}
> +	return tmp;
> +}
> +
> +static struct page_info *mp_hash_lookup(struct page_ctor *ctor,
> +					struct page *page)
> +{
> +	key_mp_t key = mp_hash(page, HASH_BUCKETS);
> +	struct page_info *tmp = NULL;
> +
> +	int i;
> +	tmp = ctor->hash_table[key];
> +	while (tmp) {
> +		for (i = 0; i < tmp->pnum; i++) {
> +			if (tmp->pages[i] == page)
> +				return tmp;
> +		}
> +		tmp = tmp->next;
> +	}
> +	return tmp;
> +}

How are thse serialised?

> +/* The main function to transform the guest user space address
> + * to host kernel address via get_user_pages(). Thus the hardware
> + * can do DMA directly to the external buffer address.
> + */
> +static struct page_info *alloc_page_info(struct page_ctor *ctor,
> +		struct kiocb *iocb, struct iovec *iov,
> +		int count, struct frag *frags,
> +		int npages, int total)
> +{
> +	int rc;
> +	int i, j, n = 0;
> +	int len;
> +	unsigned long base, lock_limit;
> +	struct page_info *info = NULL;
> +
> +	lock_limit = current->signal->rlim[RLIMIT_MEMLOCK].rlim_cur;
> +	lock_limit >>= PAGE_SHIFT;
> +
> +	if (ctor->lock_pages + count > lock_limit && npages) {
> +		printk(KERN_INFO "exceed the locked memory rlimit.");
> +		return NULL;
> +	}

What if the process is locking pages with mlock() as well?  Doesn't this
allow it to lock twice as many pages as it should be able to?

> +	info = kmem_cache_alloc(ext_page_info_cache, GFP_KERNEL);
> +	
> +	if (!info)
> +		return NULL;
> +	info->skb = NULL;
> +	info->next = info->prev = NULL;
> +
> +	for (i = j = 0; i < count; i++) {
> +		base = (unsigned long)iov[i].iov_base;
> +		len = iov[i].iov_len;
> +
> +		if (!len)
> +			continue;
> +		n = ((base & ~PAGE_MASK) + len + ~PAGE_MASK) >> PAGE_SHIFT;
> +
> +		rc = get_user_pages_fast(base, n, npages ? 1 : 0,
> +				&info->pages[j]);
> +		if (rc != n)
> +			goto failed;
> +
> +		while (n--) {
> +			frags[j].offset = base & ~PAGE_MASK;
> +			frags[j].size = min_t(int, len,
> +					PAGE_SIZE - frags[j].offset);
> +			len -= frags[j].size;
> +			base += frags[j].size;
> +			j++;
> +		}
> +	}
> +
> +#ifdef CONFIG_HIGHMEM
> +	if (npages && !(dev->features & NETIF_F_HIGHDMA)) {
> +		for (i = 0; i < j; i++) {
> +			if (PageHighMem(info->pages[i]))
> +				goto failed;
> +		}
> +	}
> +#endif

Shouldn't you try to allocate lowmem pages explicitly, rather than
failing at this point?

[...]
> +static int mp_recvmsg(struct kiocb *iocb, struct socket *sock,
> +		struct msghdr *m, size_t total_len,
> +		int flags)
> +{
> +	struct mp_struct *mp = container_of(sock->sk, struct mp_sock, sk)->mp;
> +	struct page_ctor *ctor;
> +	struct iovec *iov = m->msg_iov;
> +	int count = m->msg_iovlen;
> +	int npages, payload;
> +	struct page_info *info;
> +	struct frag frags[MAX_SKB_FRAGS];
> +	unsigned long base;
> +	int i, len;
> +	unsigned long flag;
> +
> +	if (!(flags & MSG_DONTWAIT))
> +		return -EINVAL;
> +
> +	ctor = mp->ctor;
> +	if (!ctor)
> +		return -EINVAL;
> +
> +	/* Error detections in case invalid external buffer */
> +	if (count > 2 && iov[1].iov_len < ctor->port.hdr_len &&
> +			mp->dev->features & NETIF_F_SG) {
> +		return -EINVAL;
> +	}
> +
> +	npages = ctor->port.npages;
> +	payload = ctor->port.data_len;
> +
> +	/* If KVM guest virtio-net FE driver use SG feature */
> +	if (count > 2) {
> +		for (i = 2; i < count; i++) {
> +			base = (unsigned long)iov[i].iov_base & ~PAGE_MASK;
> +			len = iov[i].iov_len;
> +			if (npages == 1)
> +				len = min_t(int, len, PAGE_SIZE - base);
> +			else if (base)
> +				break;
> +			payload -= len;
> +			if (payload <= 0)
> +				goto proceed;
> +			if (npages == 1 || (len & ~PAGE_MASK))
> +				break;
> +		}
> +	}
> +
> +	if ((((unsigned long)iov[1].iov_base & ~PAGE_MASK)
> +				- NET_SKB_PAD - NET_IP_ALIGN) >= 0)
> +		goto proceed;
> +
> +	return -EINVAL;
> +
> +proceed:
> +	/* skip the virtnet head */
> +	if (count > 1) {
> +		iov++;
> +		count--;
> +	}
> +
> +	if (!ctor->lock_pages || !ctor->rq_len) {
> +		set_memlock_rlimit(ctor, RLIMIT_MEMLOCK,
> +				iocb->ki_user_data * 4096 * 2,
> +				iocb->ki_user_data * 4096 * 2);
> +	}
> +
> +	/* Translate address to kernel */
> +	info = alloc_page_info(ctor, iocb, iov, count, frags, npages, 0);
> +	if (!info)
> +		return -ENOMEM;

I'm not convinced that the checks above this ensure that there will be
<= MAX_SKB_FRAGS fragments.

[...]
> +static int mp_chr_open(struct inode *inode, struct file * file)
> +{
> +	struct mp_file *mfile;
> +	cycle_kernel_lock();

Seriously?

[...]
> +static ssize_t mp_chr_aio_write(struct kiocb *iocb, const struct iovec *iov,
> +				unsigned long count, loff_t pos)
> +{
> +	struct file *file = iocb->ki_filp;
> +	struct mp_struct *mp = mp_get(file->private_data);
> +	struct sock *sk = mp->socket.sk;
> +	struct sk_buff *skb;
> +	int len, err;
> +	ssize_t result = 0;
> +
> +	if (!mp)
> +		return -EBADFD;
> +
> +	/* currently, async is not supported.
> +	 * but we may support real async aio from user application,
> +	 * maybe qemu virtio-net backend.
> +	 */
> +	if (!is_sync_kiocb(iocb))
> +		return -EFAULT;
> +
> +	len = iov_length(iov, count);
> +
> +	if (unlikely(len) < ETH_HLEN)
> +		return -EINVAL;

The first close-paren is in the wrong place.

> +	skb = sock_alloc_send_skb(sk, len + NET_IP_ALIGN,
> +				  file->f_flags & O_NONBLOCK, &err);
> +
> +	if (!skb)
> +		return -EFAULT;

Why EFAULT?

> +	skb_reserve(skb, NET_IP_ALIGN);
> +	skb_put(skb, len);
> +
> +	if (skb_copy_datagram_from_iovec(skb, 0, iov, 0, len)) {
> +		kfree_skb(skb);
> +		return -EAGAIN;
> +	}
> +
> +	skb->protocol = eth_type_trans(skb, mp->dev);

Why are you calling eth_type_trans() on transmit?

[...]
> +static int mp_device_event(struct notifier_block *unused,
> +		unsigned long event, void *ptr)
> +{
> +	struct net_device *dev = ptr;
> +	struct mpassthru_port *port;
> +	struct mp_struct *mp = NULL;
> +	struct socket *sock = NULL;
> +	struct sock *sk;
> +
> +	port = dev->mp_port;
> +	if (port == NULL)
> +		return NOTIFY_DONE;
> +
> +	switch (event) {
> +	case NETDEV_UNREGISTER:
> +		sock = dev->mp_port->sock;
> +		mp = container_of(sock->sk, struct mp_sock, sk)->mp;
> +		do_unbind(mp->mfile);
[...]

This can deadlock - netdev notifiers are called under the RTNL lock and
do_unbind() acquires the mp_mutex, whereas in other places they are
acquired in the opposite order.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re:
From: Bob Arendt @ 2010-09-27 20:54 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: David Stevens, Jason Gunthorpe,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	netdev-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <alpine.DEB.2.00.1009271521510.14117-sBS69tsa9Uj/9pzu0YdTqQ@public.gmane.org>

On 09/27/10 13:23, Christoph Lameter wrote:
> On Mon, 27 Sep 2010, David Stevens wrote:
>
>>          Because a querier can set the robustness value and
>> query interval to anything you want. In the original report,
>> he's not running a querier. The fact that it's a new group
>> doesn't matter -- these are per-interface.
>
> The per interface settings are used to force an IGMP version overriding
> any information by the queriers. You would not want to enable that because
> it disables support for other IGMP versions. Without the override
> different version of IGMP can be handled per MC group.
>
If a network vlan has IGMPv3 capability, then it should be able
to support both v2 and v3 Joins (clients).  But if the vlan is
IGMPv2 only, then an initial Join from a Linux client might go out
as v3 (if it hasn't seen a query yet) and be ignored.  I believe
this is the case that force_igmp_version really addresses.

And it turns out that  force_igmp_version=2 doesn't fully work.
If the host sees a IGMPv3 query, it still responds with a v3 Join
despite the flag.  Bug report and candidate patch here:
https://bugzilla.kernel.org/show_bug.cgi?id=18212
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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


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