* RE: [PATCH] qla3xxx: Ensure req_q_phy_addr writes to the register
From: Jitendra Kalsaria @ 2012-10-18 17:10 UTC (permalink / raw)
To: Joe Jin
Cc: Ron Mercer, Dept-Eng Linux Driver, netdev, linux-kernel,
Greg Marsden
In-Reply-To: <507F66DF.4080606@oracle.com>
>>> -----Original Message-----
>>> From: Joe Jin [mailto:joe.jin@oracle.com]
>>> Sent: Tuesday, October 16, 2012 11:32 PM
>>> To: Ron Mercer; Jitendra Kalsaria; Dept-Eng Linux Driver
>>> Cc: netdev; linux-kernel; Greg Marsden
>>> Subject: [PATCH] qla3xxx: Ensure req_q_phy_addr writes to the register
>>>
>>> Make sure req_q_phy_addr write to the register.
>>>
>>> Signed-off-by: Joe Jin <joe.jin@oracle.com>
>>> Cc: Ron Mercer <ron.mercer@qlogic.com>
>>> Cc: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
>>> ---
>>> drivers/net/ethernet/qlogic/qla3xxx.c | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/qlogic/qla3xxx.c b/drivers/net/ethernet/qlogic/qla3xxx.c
>>> index df09b1c..78b4cba 100644
>>> --- a/drivers/net/ethernet/qlogic/qla3xxx.c
>>> +++ b/drivers/net/ethernet/qlogic/qla3xxx.c
>>> @@ -2525,6 +2525,12 @@ static int ql_alloc_net_req_rsp_queues(struct ql3_adapter *qdev)
>>> qdev->req_q_size =
>>> (u32) (NUM_REQ_Q_ENTRIES * sizeof(struct ob_mac_iocb_req));
>>>
>>> + /*
>>> + * The barrier is required to ensure that req_q_phy_addr writes to
>>> + * the memory.
>>> + */
>>> + wmb();
>>> +
>>> qdev->req_q_virt_addr =
>>> pci_alloc_consistent(qdev->pdev,
>>> (size_t) qdev->req_q_size,
>>
>> Your changes only take care of request queue but not response queue which also need barrier.
>
>Jiten,
>
>Thanks for review!
>The barrier to make sure writel() call for req_q_phy_addr and rsp_q_phy_addr in
>ql_adapter_initialize(), so I think call once wmb() is enough but I need to update
>the comment, any idea?
Yes and something like "ensure request/response queue addr writes to the register"
^ permalink raw reply
* Re: listen(2) backlog changes in or around Linux 3.1?
From: Eric Dumazet @ 2012-10-18 16:54 UTC (permalink / raw)
To: enh; +Cc: netdev
In-Reply-To: <CAJgzZoq9+KZpQ1tVQJUK++VVk4JfY9u8timaJ5q6wSYoF+_tog@mail.gmail.com>
On Tue, 2012-10-16 at 16:31 -0700, enh wrote:
> boiling things down to a short C++ program, i see that i can reproduce
> the behavior even on 2.6 kernels. if i run this, i see 4 connections
> immediately (3 + 1, as i'd expect)... but then about 10s later i see
> another 2. and every few seconds after that, i see another 2. i've let
> this run until i have hundreds of connect(2) calls that have returned,
> despite my small listen(2) backlog and the fact that i'm not
> accept(2)ing.
>
> so i guess the only thing that's changed with newer kernels is timing
> (hell, since i only see newer kernels on newer hardware, it might just
> be a hardware thing).
>
> and clearly i don't understand what the listen(2) backlog means any more.
Hi Elliott
I would say there is a bug (or several !!), and this needs a fix.
I am investigating.
Thanks
^ permalink raw reply
* Re: listen(2) backlog changes in or around Linux 3.1?
From: Venkat Venkatsubra @ 2012-10-18 16:53 UTC (permalink / raw)
To: enh; +Cc: netdev
In-Reply-To: <5080279F.80008@oracle.com>
Correction. I don't see the client side receiving any abort/termination
notification.
They all remain on ESTABLISHED state on the client side.
In tcpdump I don't see a FIN or RST coming from the server for the
aborted connections.
Venkat
On 10/18/2012 11:00 AM, Venkat Venkatsubra wrote:
> Hi Elliott,
>
> I see the same behavior with your test program.
> The connect() keeps succeeding even though accept() is not performed.
> It pauses after 4 connections for a while and then periodically keeps
> adding few (2 I think).
>
> But the server side end points are terminated too. You will see only
> the first 2 sessions on the server side.
> If you modify your test program to say read or poll the sockets you
> should get a termination notification on them I think .
>
> The behavior overall looks fine in my opinion. But it could be a
> change of behavior for your test program.
>
> Venkat
>
> On 10/16/2012 6:31 PM, enh wrote:
>> boiling things down to a short C++ program, i see that i can reproduce
>> the behavior even on 2.6 kernels. if i run this, i see 4 connections
>> immediately (3 + 1, as i'd expect)... but then about 10s later i see
>> another 2. and every few seconds after that, i see another 2. i've let
>> this run until i have hundreds of connect(2) calls that have returned,
>> despite my small listen(2) backlog and the fact that i'm not
>> accept(2)ing.
>>
>> so i guess the only thing that's changed with newer kernels is timing
>> (hell, since i only see newer kernels on newer hardware, it might just
>> be a hardware thing).
>>
>> and clearly i don't understand what the listen(2) backlog means any
>> more.
>>
>> #include<netinet/ip.h>
>> #include<netinet/tcp.h>
>> #include<sys/types.h>
>> #include<sys/socket.h>
>> #include<iostream>
>> #include<stdlib.h>
>> #include<string.h>
>> #include<errno.h>
>>
>> void dump_ti(int fd) {
>> tcp_info ti;
>> socklen_t tcp_info_length = sizeof(tcp_info);
>> int rc = getsockopt(fd, SOL_IP, TCP_INFO,&ti,&tcp_info_length);
>> if (rc == -1) {
>> std::cout<< "getsockopt rc "<< rc<< ": "<< strerror(errno)<<
>> "\n";
>> return;
>> }
>>
>> std::cout<< "ti.tcpi_unacked="<< ti.tcpi_unacked<< "\n";
>> std::cout<< "ti.tcpi_sacked="<< ti.tcpi_sacked<< "\n";
>> }
>>
>> void connect_to(sockaddr_in& sa) {
>> int s = socket(AF_INET, SOCK_STREAM, 0);
>> if (s == -1) {
>> abort();
>> }
>>
>> int rc = connect(s, (sockaddr*)&sa, sizeof(sockaddr_in));
>> std::cout<< "connect = "<< rc<< "\n";
>> }
>>
>> int main() {
>> int ss = socket(AF_INET, SOCK_STREAM, 0);
>> std::cout<< "socket fd "<< ss<< "\n";
>>
>> sockaddr_in sa;
>> memset(&sa, 0, sizeof(sa));
>> sa.sin_family = AF_INET;
>> sa.sin_addr.s_addr = htonl(INADDR_ANY);
>> sa.sin_port = htons(9877);
>> int rc = bind(ss, (sockaddr*)&sa, sizeof(sa));
>> std::cout<< "bind rc "<< rc<< ": "<< strerror(errno)<< "\n";
>> std::cout<< "bind port "<< sa.sin_port<< "\n";
>>
>> rc = listen(ss, 1);
>> std::cout<< "listen rc "<< rc<< ": "<< strerror(errno)<< "\n";
>> dump_ti(ss);
>>
>> while (true) {
>> connect_to(sa);
>> dump_ti(ss);
>> }
>>
>> return 0;
>> }
>>
>>
>> On Mon, Oct 15, 2012 at 10:26 AM, enh<enh@google.com> wrote:
>>> On Mon, Oct 15, 2012 at 10:12 AM, Venkat Venkatsubra
>>> <venkat.x.venkatsubra@oracle.com> wrote:
>>>> On 10/12/2012 6:40 PM, enh wrote:
>>>>> i used to use the following hack to unit test connect timeouts: i'd
>>>>> call listen(2) on a socket and then deliberately connect (backlog
>>>>> + 3)
>>>>> sockets without accept(2)ing any of the connections. (why 3? because
>>>>> Stevens told me so, and experiment backed him up. see figure 4.10 in
>>>>> his UNIX Network Programming.)
>>>>>
>>>>> with "old" kernels, 2.6.35-ish to 3.0-ish, this worked great. my next
>>>>> connect(2) to the same loopback port would hang indefinitely. i could
>>>>> even unblock the connect by calling accept(2) in another thread. this
>>>>> was awesome for testing.
>>>>>
>>>>> in 3.1 on ARM, 3.2 on x86 (Ubuntu desktop), and 3.4 on ARM, this no
>>>>> longer works. it doesn't seem to be as simple as "the constant is no
>>>>> longer 3". my tests are now flaky. sometimes they work like they used
>>>>> to, and sometimes an extra connect(2) will succeed. (or, if i'm in
>>>>> non-blocking mode, my poll(2) will return with the non-blocking
>>>>> socket
>>>>> that's trying to connect now ready.)
>>>>>
>>>>> i'm guessing if this changed in 3.1 and is still changed in 3.4,
>>>>> whatever's changed wasn't an accident. but i haven't been able to
>>>>> find
>>>>> the right search terms to RTFM. i also finally got around to grepping
>>>>> the kernel for the "+ 3", but wasn't able to find that. (so i'd be
>>>>> interested to know where the old behavior came from too.)
>>>>>
>>>>> my least worst workaround at the moment is to use one of RFC5737's
>>>>> test networks, but that requires that the device have a network
>>>>> connection, otherwise my connect(2)s fail immediately with
>>>>> ENETUNREACH, which is no use to me. also, unlike my old trick, i've
>>>>> got no way to suddenly "unblock" a slow connect(2) (this is useful
>>>>> for
>>>>> unit testing the code that does the poll(2) part of the usual
>>>>> connect-with-timeout implementation).
>>>>> https://android-review.googlesource.com/#/c/44563/
>>>>>
>>>>> hopefully someone here can shed some light on this? ideally someone
>>>>> will have a workaround as good as my old trick. i realize i was
>>>>> relying on undocumented behavior, and i'm happy to have to check
>>>>> /proc/version and behave appropriately, but i'd really like a way to
>>>>> keep my unit tests!
>>>>>
>>>>> thanks,
>>>>> elliott
>>>>> --
>>>>> 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
>>>> Hi Elliott,
>>>>
>>>> In BSD I think the backlog used to be reset to 3/2 times that
>>>> passed by the
>>>> user. So, 2 becomes 3.
>>>> Probably the 1/2 times increase was to accommodate the ones in
>>>> partial/incomplete queue.
>>>> In Linux is it possible you were getting the same behavior before
>>>> the below
>>>> commit ?
>>>> Since the check used to be "backlog+1" a 2 will behave as 3 ?
>>> i don't think so, because with<= 3.0 kernels i used to have a backlog
>>> of 1 and be able to make _4_ connections before my next connect would
>>> hang. but this> to>= change is at least something for me to
>>> investigate...
>>>
>>>> commit 8488df894d05d6fa41c2bd298c335f944bb0e401
>>>> Author: Wei Dong<weid@np.css.fujitsu.com>
>>>> Date: Fri Mar 2 12:37:26 2007 -0800
>>>>
>>>> [NET]: Fix bugs in "Whether sock accept queue is full" checking
>>>>
>>>> when I use linux TCP socket, and find there is a bug in
>>>> function
>>>> sk_acceptq_is_full().
>>>>
>>>> When a new SYN comes, TCP module first checks its
>>>> validation. If
>>>> valid,
>>>> send SYN,ACK to the client and add the sock to the syn hash
>>>> table. Next
>>>> time if received the valid ACK for SYN,ACK from the client.
>>>> server will
>>>> accept this connection and increase the sk->sk_ack_backlog --
>>>> which is
>>>> done in function tcp_check_req().We check wether acceptq is
>>>> full in
>>>> function tcp_v4_syn_recv_sock().
>>>>
>>>> Consider an example:
>>>>
>>>> After listen(sockfd, 1) system call, sk->sk_max_ack_backlog
>>>> is set to
>>>> 1. As we know, sk->sk_ack_backlog is initialized to 0.
>>>> Assuming accept()
>>>> system call is not invoked now.
>>>>
>>>> 1. 1st connection comes. invoke sk_acceptq_is_full().
>>>> sk->sk_ack_backlog=0 sk->sk_max_ack_backlog=1, function
>>>> return 0 accept
>>>> this connection.
>>>> Increase the sk->sk_ack_backlog
>>>> 2. 2nd connection comes. invoke sk_acceptq_is_full().
>>>> sk->sk_ack_backlog=1 sk->sk_max_ack_backlog=1, function
>>>> return 0 accept
>>>> this connection.
>>>> Increase the sk->sk_ack_backlog
>>>> 3. 3rd connection comes. invoke sk_acceptq_is_full().
>>>> sk->sk_ack_backlog=2 sk->sk_max_ack_backlog=1, function
>>>> return 1.
>>>> Refuse this connection.
>>>>
>>>> I think it has bugs. after listen system call.
>>>> sk->sk_max_ack_backlog=1
>>>> but now it can accept 2 connections.
>>>>
>>>> Signed-off-by: Wei Dong<weid@np.css.fujitsu.com>
>>>> Signed-off-by: David S. Miller<davem@davemloft.net>
>>>>
>>>> Venkat
>> --
>> 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
>
> --
> 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
* Re: [PATCH 1/5] ARM: OMAP2+: gpmc: Fix kernel BUG for DT boot mode
From: Jon Hunter @ 2012-10-18 16:42 UTC (permalink / raw)
To: Hiremath, Vaibhav
Cc: Richard Cochran, netdev@vger.kernel.org, Mohammed, Afzal,
Russell King, Arnd Bergmann, Tony Lindgren, David Miller,
linux-arm-kernel@lists.infradead.org, linux-omap@vger.kernel.org
In-Reply-To: <79CD15C6BA57404B839C016229A409A83EB43B17@DBDE01.ent.ti.com>
On 10/18/2012 11:16 AM, Hiremath, Vaibhav wrote:
> On Wed, Oct 17, 2012 at 01:17:56, Hunter, Jon wrote:
>>
>> On 10/15/2012 02:16 PM, Richard Cochran wrote:
>>> From: hvaibhav@ti.com <hvaibhav@ti.com>
>>>
>>> With recent changes in omap gpmc driver code, in case of DT
>>> boot mode, where bootloader does not configure gpmc cs space
>>> will result into kernel BUG() inside gpmc_mem_init() function,
>>> as gpmc cs0 gpmc_config7[0].csvalid bit is set to '1' and
>>> gpmc_config7[0].baseaddress is set to '0' on reset.
>>
>> I am not sure I completely follow the logic here.
>>
>> Won't this problem occur if the bootloader does not configure the gpmc
>> cs space AND we are not using DT?
>>
>
> That's what exactly the above comment describes.
Hmm ... you said "in the case of DT", but I am saying even "in the case
WITHOUT DT" this can happen. So I think the subject is mis-leading.
>> Is the csvalid bit set because we are booting from the internal ROM?
>>
>
> As per TRM, the reset value of the CS0_valis bit is set to 0. I have pasted
> TRM statement below -
>
> "Chip-select enable (reset value is 1 for CS[0] and 0 for CS[1-5])."
The above two sentences don't see to agree ...
> And same applies to OMAP3 family of devices.
For which boot-modes? All or just the gpmc boot-modes?
My omap3430 beagle has been booting with DT fine for some time and I
have not encountered this problem even on the latest kernel with the
gpmc driver present.
>> I guess I don't see why csvalid bit being set and a base-address of 0x0
>> should not be allowed. That should be a valid combination.
>>
>
> Yes, agreed.
>
>> One problem I see with gpmc_mem_init() is that it assumes that
>> BOOT_ROM_SPACE is 1MB for all devices starting at 0x0 apart from the
>> apollon board. For newer devices such as OMAP4, there is only 48KB of
>> internal ROM and only mapped to 0x0 when booting from internal ROM. So
>> this needs to be fixed.
>>
>> How much internal ROM does the AM335x have and where is it mapped?
>>
>
> AM33xx memory map is something like,
>
> Boot ROM 0x4000_0000 0x4001_FFFF 128KB
> 0x4002_0000 0x4002_BFFF 48KB 32-bit Ex/R(1) - Public
> Reserved 0x4002_C000 0x400F_FFFF 848KB Reserved
> Reserved 0x4010_0000 0x401F_FFFF 1MB Reserved
> Reserved 0x4020_0000 0x402E_FFFF 960KB Reserved
> Reserved 0x402f_0000 0x4020_03FF 64KB Reserved
> SRAM internal 0x402F_0400 0x402F_FFFF 32-bit Ex/R/W(1)
Does the boot ROM get mapped to 0x0, when booting from ROM?
>>> This use-case is applicable for any board/EVM which doesn't have
>>> any peripheral connected to gpmc cs0, for example BeagleXM and
>>> BeagleBone, so DT boot mode fails.
>>>
>>> This patch adds of_have_populated_dt() check before creating
>>> device, so that for DT boot mode, gpmc probe will not be called
>>> which is expected behavior, as gpmc is not supported yet from DT.
>>
>> Yes, but we do actually still allow some platform devices to be probed
>> (such as dmtimers) when booting with DT that don't support DT yet. So
>> this change prevents us from using the gpmc on boards when booting with DT.
>>
>
> The idea here was,
>
> In order to use GPMC in meaningful way, where some peripheral is connected
> to the GPMC, you must create platform_device for the probe to happen
> properly. Now all the devices I know so far, we have gpmc_smsc911x_init(),
> omap_nand_flash_init(), etc...
> These api's are getting called only through machine_desc.init_xxx callbacks,
> And in case of DT, we have centralized machine_desc definition for all
> platforms (board-generic.c). So even though you want to use GPMC for DT boot
> mode, you can not make use of peripheral without changing board-files to
> change to create platform_device.
>
> Does it make sense?
Sure, if you are using one of the generic machine configurations for DT.
However, while this migration happens people may create their own custom
machine configurations for DT for testing things like smsc911x.
>> I am not convinced that this is addressing the underlying problem with
>> gpmc_mem_init().
>>
>
> The patch you submitted is cleanup patch and is required irrespective of
> this patch. I believe this patch is just makes sure that, if you are booting
> from DT and you do not have meaningful DT node for GPMC and peripheral
> interfaced to it, no point in probing it.
>
> Does it make any sense???
Yes, but do you also see the bug that is hiding in gpmc_mem_init()?
My point is to highlight this and not hide it, so that we can fix it
now. Otherwise if we wait until we enable the gpmc driver with DT and
this could hinder the DT migration later.
Jon
^ permalink raw reply
* Re: [PATCH v1] net/core: support runtime PM on net_device
From: Daniel Lezcano @ 2012-10-18 16:34 UTC (permalink / raw)
To: Ming Lei
Cc: Bjørn Mork, David S. Miller, Rafael J. Wysocki,
Oliver Neukum, Alan Stern, netdev, linux-pm
In-Reply-To: <CACVXFVMmSFHYU-sNHRM7g_8U6mjbdbE+bNPxM-F8iAB=4WudRA@mail.gmail.com>
On 10/18/2012 02:55 PM, Ming Lei wrote:
> On Thu, Oct 18, 2012 at 7:40 PM, Bjørn Mork <bjorn@mork.no> wrote:
>>
>> Because bugs are supposed to be fixed and not hidden?
>
> OK, now let's discuss solution for the problem which should be
> clear and be solved.
>
> The 1st one is to do it by this patch, or rule out some ioctl cmd
> which needn't wakeup device.
>
> The second one is to fix ioctl one by one in each driver.
>
> Any other solution?
>
>>
>> Note that I am not claiming this is a bug. That is still an open
>> question as far as I can see.
Hi Ming,
IIUC, the pm_runtime is related to the device drivers, so at the first
glance, we should see invocation of the runtime's functions in drivers/*
and arch/*. Adding these calls in the net core, which makes sense at a
certain point, is a bit weird for me.
From my POV, if the drivers have been modified to support the pm_runtime
and this new functionality brought a regression with the ioctl, that
should be fixed in the drivers and not in the core code.
What happens with your patch if we use ethtool on a virtual device like
veth, macvlan, bridge, ... ?
Shouldn't handle the mac address, mtu, ... changes with the rtnetlink also ?
Thanks
-- Daniel
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* RE: [PATCH 1/5] ARM: OMAP2+: gpmc: Fix kernel BUG for DT boot mode
From: Hiremath, Vaibhav @ 2012-10-18 16:16 UTC (permalink / raw)
To: Hunter, Jon, Richard Cochran
Cc: netdev@vger.kernel.org, Mohammed, Afzal, Russell King,
Arnd Bergmann, Tony Lindgren, David Miller,
linux-arm-kernel@lists.infradead.org, linux-omap@vger.kernel.org
In-Reply-To: <507DB9EC.9060407@ti.com>
On Wed, Oct 17, 2012 at 01:17:56, Hunter, Jon wrote:
>
> On 10/15/2012 02:16 PM, Richard Cochran wrote:
> > From: hvaibhav@ti.com <hvaibhav@ti.com>
> >
> > With recent changes in omap gpmc driver code, in case of DT
> > boot mode, where bootloader does not configure gpmc cs space
> > will result into kernel BUG() inside gpmc_mem_init() function,
> > as gpmc cs0 gpmc_config7[0].csvalid bit is set to '1' and
> > gpmc_config7[0].baseaddress is set to '0' on reset.
>
> I am not sure I completely follow the logic here.
>
> Won't this problem occur if the bootloader does not configure the gpmc
> cs space AND we are not using DT?
>
That's what exactly the above comment describes.
> Is the csvalid bit set because we are booting from the internal ROM?
>
As per TRM, the reset value of the CS0_valis bit is set to 0. I have pasted
TRM statement below -
"Chip-select enable (reset value is 1 for CS[0] and 0 for CS[1-5])."
And same applies to OMAP3 family of devices.
> I guess I don't see why csvalid bit being set and a base-address of 0x0
> should not be allowed. That should be a valid combination.
>
Yes, agreed.
> One problem I see with gpmc_mem_init() is that it assumes that
> BOOT_ROM_SPACE is 1MB for all devices starting at 0x0 apart from the
> apollon board. For newer devices such as OMAP4, there is only 48KB of
> internal ROM and only mapped to 0x0 when booting from internal ROM. So
> this needs to be fixed.
>
> How much internal ROM does the AM335x have and where is it mapped?
>
AM33xx memory map is something like,
Boot ROM 0x4000_0000 0x4001_FFFF 128KB
0x4002_0000 0x4002_BFFF 48KB 32-bit Ex/R(1) - Public
Reserved 0x4002_C000 0x400F_FFFF 848KB Reserved
Reserved 0x4010_0000 0x401F_FFFF 1MB Reserved
Reserved 0x4020_0000 0x402E_FFFF 960KB Reserved
Reserved 0x402f_0000 0x4020_03FF 64KB Reserved
SRAM internal 0x402F_0400 0x402F_FFFF 32-bit Ex/R/W(1)
> > This use-case is applicable for any board/EVM which doesn't have
> > any peripheral connected to gpmc cs0, for example BeagleXM and
> > BeagleBone, so DT boot mode fails.
> >
> > This patch adds of_have_populated_dt() check before creating
> > device, so that for DT boot mode, gpmc probe will not be called
> > which is expected behavior, as gpmc is not supported yet from DT.
>
> Yes, but we do actually still allow some platform devices to be probed
> (such as dmtimers) when booting with DT that don't support DT yet. So
> this change prevents us from using the gpmc on boards when booting with DT.
>
The idea here was,
In order to use GPMC in meaningful way, where some peripheral is connected
to the GPMC, you must create platform_device for the probe to happen
properly. Now all the devices I know so far, we have gpmc_smsc911x_init(),
omap_nand_flash_init(), etc...
These api's are getting called only through machine_desc.init_xxx callbacks,
And in case of DT, we have centralized machine_desc definition for all
platforms (board-generic.c). So even though you want to use GPMC for DT boot
mode, you can not make use of peripheral without changing board-files to
change to create platform_device.
Does it make sense?
> I am not convinced that this is addressing the underlying problem with
> gpmc_mem_init().
>
The patch you submitted is cleanup patch and is required irrespective of
this patch. I believe this patch is just makes sure that, if you are booting
from DT and you do not have meaningful DT node for GPMC and peripheral
interfaced to it, no point in probing it.
Does it make any sense???
On other hand, Your patch is anyway required, as that I would consider as
cleanup of existing code (in error handling).
Thanks,
Vaibhav
> Cheers
> Jon
>
^ permalink raw reply
* RE: [PATCH 3/5] net: davinci_mdio: Fix type mistake in calling runtime-pm api
From: Hiremath, Vaibhav @ 2012-10-18 16:13 UTC (permalink / raw)
To: Richard Cochran, netdev@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org, Arnd Bergmann, David Miller,
Russell King, N, Mugunthan V
In-Reply-To: <62191bdcb85ab48f5ea16fe69c2f92aa9e9a0a8b.1350327325.git.richardcochran@gmail.com>
On Tue, Oct 16, 2012 at 00:46:33, Richard Cochran wrote:
> From: Vaibhav Hiremath <hvaibhav@ti.com>
>
> By mistake (most likely a copy-paste), instead of pm_runtime_get_sync()
> api, driver is calling pm_runtime_put_sync() api in resume callback
> function. The bug was introduced by commit id (ae2c07aaf74:
> davinci_mdio: runtime PM support).
>
> Now, the reason why it didn't impact functionality is, the patch has
> been tested on AM335x-EVM and BeagleBone platform while submitting;
> and in case of AM335x the MDIO driver doesn't control the module
> enable/disable part, which is handled by CPSW driver.
>
> Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
> Cc: Mugunthan V N <mugunthanvnm@ti.com>
Thanks for submitting the patches, I couldn't able to do it for almost 2
weeks now. I really appreciate it.
Thanks,
Vaibhav
> ---
> drivers/net/ethernet/ti/davinci_mdio.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
> index 51a96db..ae74280 100644
> --- a/drivers/net/ethernet/ti/davinci_mdio.c
> +++ b/drivers/net/ethernet/ti/davinci_mdio.c
> @@ -465,7 +465,7 @@ static int davinci_mdio_resume(struct device *dev)
> u32 ctrl;
>
> spin_lock(&data->lock);
> - pm_runtime_put_sync(data->dev);
> + pm_runtime_get_sync(data->dev);
>
> /* restart the scan state machine */
> ctrl = __raw_readl(&data->regs->control);
> --
> 1.7.2.5
>
>
^ permalink raw reply
* RE: [PATCH 4/5] net: cpsw: Add parent<->child relation support between cpsw and mdio
From: Hiremath, Vaibhav @ 2012-10-18 16:13 UTC (permalink / raw)
To: Richard Cochran, netdev@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org, Arnd Bergmann, David Miller,
Russell King, N, Mugunthan V
In-Reply-To: <9a4368fa1264f573072544a34169a3f0f8797989.1350327325.git.richardcochran@gmail.com>
On Tue, Oct 16, 2012 at 00:46:34, Richard Cochran wrote:
> From: Vaibhav Hiremath <hvaibhav@ti.com>
>
> CPGMAC SubSystem consist of various sub-modules, like, mdio, cpdma,
> cpsw, etc... These sub-modules are also used in some of Davinci family
> of devices. Now based on requirement, use-case and available technology
> nodes the integration of these sub-modules varies across devices.
>
> So coming back to Linux net driver, currently separate and independent
> platform devices & drivers for CPSW and MDIO is implemented. In case of
> Davinci they both has separate control, from resources perspective,
> like clock.
>
> In case of AM33XX, the resources are shared and only one register
> bit-field is provided to control module/clock enable/disable, makes it
> difficult to handle common resource.
>
> So the solution here implemented in this patch is,
>
> Create parent<->child relationship between both the drivers, making
> CPSW as a parent and MDIO as its child and enumerate all the child nodes
> under cpsw module.
> Both the drivers will function exactly the way it was operating before,
> including runtime-pm functionality. No change is required in MDIO driver
> (for that matter to any child driver).
>
> As this is only supported during DT boot, the parent<->child relationship
> is created and populated in DT execution flow. The only required change
> is inside DTS file, making MDIO as a child to CPSW node.
>
> Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
> Cc: Mugunthan V N <mugunthanvnm@ti.com>
> ---
> drivers/net/ethernet/ti/cpsw.c | 16 ++++++++++++++--
> 1 files changed, 14 insertions(+), 2 deletions(-)
>
Thanks for stepping ahead and submitting patches from my tree.
Thanks,
Vaibhav
> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> index df55e24..fb1a692 100644
> --- a/drivers/net/ethernet/ti/cpsw.c
> +++ b/drivers/net/ethernet/ti/cpsw.c
> @@ -827,7 +827,7 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
> }
> data->mac_control = prop;
>
> - for_each_child_of_node(node, slave_node) {
> + for_each_node_by_name(slave_node, "slave") {
> struct cpsw_slave_data *slave_data = data->slave_data + i;
> const char *phy_id = NULL;
> const void *mac_addr = NULL;
> @@ -862,6 +862,14 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
> i++;
> }
>
> + /*
> + * Populate all the child nodes here...
> + */
> + ret = of_platform_populate(node, NULL, NULL, &pdev->dev);
> + /* We do not want to force this, as in some cases may not have child */
> + if (ret)
> + pr_warn("Doesn't have any child node\n");
> +
> return 0;
>
> error_ret:
> @@ -895,6 +903,11 @@ static int __devinit cpsw_probe(struct platform_device *pdev)
> priv->msg_enable = netif_msg_init(debug_level, CPSW_DEBUG);
> priv->rx_packet_max = max(rx_packet_max, 128);
>
> + /*
> + * This may be required here for child devices.
> + */
> + pm_runtime_enable(&pdev->dev);
> +
> if (cpsw_probe_dt(&priv->data, pdev)) {
> pr_err("cpsw: platform data missing\n");
> ret = -ENODEV;
> @@ -921,7 +934,6 @@ static int __devinit cpsw_probe(struct platform_device *pdev)
> for (i = 0; i < data->slaves; i++)
> priv->slaves[i].slave_num = i;
>
> - pm_runtime_enable(&pdev->dev);
> priv->clk = clk_get(&pdev->dev, "fck");
> if (IS_ERR(priv->clk)) {
> dev_err(&pdev->dev, "fck is not found\n");
> --
> 1.7.2.5
>
>
^ permalink raw reply
* Re: listen(2) backlog changes in or around Linux 3.1?
From: Venkat Venkatsubra @ 2012-10-18 16:00 UTC (permalink / raw)
To: enh; +Cc: netdev
In-Reply-To: <CAJgzZoq9+KZpQ1tVQJUK++VVk4JfY9u8timaJ5q6wSYoF+_tog@mail.gmail.com>
Hi Elliott,
I see the same behavior with your test program.
The connect() keeps succeeding even though accept() is not performed.
It pauses after 4 connections for a while and then periodically keeps
adding few (2 I think).
But the server side end points are terminated too. You will see only the
first 2 sessions on the server side.
If you modify your test program to say read or poll the sockets you
should get a termination notification on them I think .
The behavior overall looks fine in my opinion. But it could be a change
of behavior for your test program.
Venkat
On 10/16/2012 6:31 PM, enh wrote:
> boiling things down to a short C++ program, i see that i can reproduce
> the behavior even on 2.6 kernels. if i run this, i see 4 connections
> immediately (3 + 1, as i'd expect)... but then about 10s later i see
> another 2. and every few seconds after that, i see another 2. i've let
> this run until i have hundreds of connect(2) calls that have returned,
> despite my small listen(2) backlog and the fact that i'm not
> accept(2)ing.
>
> so i guess the only thing that's changed with newer kernels is timing
> (hell, since i only see newer kernels on newer hardware, it might just
> be a hardware thing).
>
> and clearly i don't understand what the listen(2) backlog means any more.
>
> #include<netinet/ip.h>
> #include<netinet/tcp.h>
> #include<sys/types.h>
> #include<sys/socket.h>
> #include<iostream>
> #include<stdlib.h>
> #include<string.h>
> #include<errno.h>
>
> void dump_ti(int fd) {
> tcp_info ti;
> socklen_t tcp_info_length = sizeof(tcp_info);
> int rc = getsockopt(fd, SOL_IP, TCP_INFO,&ti,&tcp_info_length);
> if (rc == -1) {
> std::cout<< "getsockopt rc "<< rc<< ": "<< strerror(errno)<< "\n";
> return;
> }
>
> std::cout<< "ti.tcpi_unacked="<< ti.tcpi_unacked<< "\n";
> std::cout<< "ti.tcpi_sacked="<< ti.tcpi_sacked<< "\n";
> }
>
> void connect_to(sockaddr_in& sa) {
> int s = socket(AF_INET, SOCK_STREAM, 0);
> if (s == -1) {
> abort();
> }
>
> int rc = connect(s, (sockaddr*)&sa, sizeof(sockaddr_in));
> std::cout<< "connect = "<< rc<< "\n";
> }
>
> int main() {
> int ss = socket(AF_INET, SOCK_STREAM, 0);
> std::cout<< "socket fd "<< ss<< "\n";
>
> sockaddr_in sa;
> memset(&sa, 0, sizeof(sa));
> sa.sin_family = AF_INET;
> sa.sin_addr.s_addr = htonl(INADDR_ANY);
> sa.sin_port = htons(9877);
> int rc = bind(ss, (sockaddr*)&sa, sizeof(sa));
> std::cout<< "bind rc "<< rc<< ": "<< strerror(errno)<< "\n";
> std::cout<< "bind port "<< sa.sin_port<< "\n";
>
> rc = listen(ss, 1);
> std::cout<< "listen rc "<< rc<< ": "<< strerror(errno)<< "\n";
> dump_ti(ss);
>
> while (true) {
> connect_to(sa);
> dump_ti(ss);
> }
>
> return 0;
> }
>
>
> On Mon, Oct 15, 2012 at 10:26 AM, enh<enh@google.com> wrote:
>> On Mon, Oct 15, 2012 at 10:12 AM, Venkat Venkatsubra
>> <venkat.x.venkatsubra@oracle.com> wrote:
>>> On 10/12/2012 6:40 PM, enh wrote:
>>>> i used to use the following hack to unit test connect timeouts: i'd
>>>> call listen(2) on a socket and then deliberately connect (backlog + 3)
>>>> sockets without accept(2)ing any of the connections. (why 3? because
>>>> Stevens told me so, and experiment backed him up. see figure 4.10 in
>>>> his UNIX Network Programming.)
>>>>
>>>> with "old" kernels, 2.6.35-ish to 3.0-ish, this worked great. my next
>>>> connect(2) to the same loopback port would hang indefinitely. i could
>>>> even unblock the connect by calling accept(2) in another thread. this
>>>> was awesome for testing.
>>>>
>>>> in 3.1 on ARM, 3.2 on x86 (Ubuntu desktop), and 3.4 on ARM, this no
>>>> longer works. it doesn't seem to be as simple as "the constant is no
>>>> longer 3". my tests are now flaky. sometimes they work like they used
>>>> to, and sometimes an extra connect(2) will succeed. (or, if i'm in
>>>> non-blocking mode, my poll(2) will return with the non-blocking socket
>>>> that's trying to connect now ready.)
>>>>
>>>> i'm guessing if this changed in 3.1 and is still changed in 3.4,
>>>> whatever's changed wasn't an accident. but i haven't been able to find
>>>> the right search terms to RTFM. i also finally got around to grepping
>>>> the kernel for the "+ 3", but wasn't able to find that. (so i'd be
>>>> interested to know where the old behavior came from too.)
>>>>
>>>> my least worst workaround at the moment is to use one of RFC5737's
>>>> test networks, but that requires that the device have a network
>>>> connection, otherwise my connect(2)s fail immediately with
>>>> ENETUNREACH, which is no use to me. also, unlike my old trick, i've
>>>> got no way to suddenly "unblock" a slow connect(2) (this is useful for
>>>> unit testing the code that does the poll(2) part of the usual
>>>> connect-with-timeout implementation).
>>>> https://android-review.googlesource.com/#/c/44563/
>>>>
>>>> hopefully someone here can shed some light on this? ideally someone
>>>> will have a workaround as good as my old trick. i realize i was
>>>> relying on undocumented behavior, and i'm happy to have to check
>>>> /proc/version and behave appropriately, but i'd really like a way to
>>>> keep my unit tests!
>>>>
>>>> thanks,
>>>> elliott
>>>> --
>>>> 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
>>> Hi Elliott,
>>>
>>> In BSD I think the backlog used to be reset to 3/2 times that passed by the
>>> user. So, 2 becomes 3.
>>> Probably the 1/2 times increase was to accommodate the ones in
>>> partial/incomplete queue.
>>> In Linux is it possible you were getting the same behavior before the below
>>> commit ?
>>> Since the check used to be "backlog+1" a 2 will behave as 3 ?
>> i don't think so, because with<= 3.0 kernels i used to have a backlog
>> of 1 and be able to make _4_ connections before my next connect would
>> hang. but this> to>= change is at least something for me to
>> investigate...
>>
>>> commit 8488df894d05d6fa41c2bd298c335f944bb0e401
>>> Author: Wei Dong<weid@np.css.fujitsu.com>
>>> Date: Fri Mar 2 12:37:26 2007 -0800
>>>
>>> [NET]: Fix bugs in "Whether sock accept queue is full" checking
>>>
>>> when I use linux TCP socket, and find there is a bug in function
>>> sk_acceptq_is_full().
>>>
>>> When a new SYN comes, TCP module first checks its validation. If
>>> valid,
>>> send SYN,ACK to the client and add the sock to the syn hash table. Next
>>> time if received the valid ACK for SYN,ACK from the client. server will
>>> accept this connection and increase the sk->sk_ack_backlog -- which is
>>> done in function tcp_check_req().We check wether acceptq is full in
>>> function tcp_v4_syn_recv_sock().
>>>
>>> Consider an example:
>>>
>>> After listen(sockfd, 1) system call, sk->sk_max_ack_backlog is set to
>>> 1. As we know, sk->sk_ack_backlog is initialized to 0. Assuming accept()
>>> system call is not invoked now.
>>>
>>> 1. 1st connection comes. invoke sk_acceptq_is_full().
>>> sk->sk_ack_backlog=0 sk->sk_max_ack_backlog=1, function return 0 accept
>>> this connection.
>>> Increase the sk->sk_ack_backlog
>>> 2. 2nd connection comes. invoke sk_acceptq_is_full().
>>> sk->sk_ack_backlog=1 sk->sk_max_ack_backlog=1, function return 0 accept
>>> this connection.
>>> Increase the sk->sk_ack_backlog
>>> 3. 3rd connection comes. invoke sk_acceptq_is_full().
>>> sk->sk_ack_backlog=2 sk->sk_max_ack_backlog=1, function return 1.
>>> Refuse this connection.
>>>
>>> I think it has bugs. after listen system call. sk->sk_max_ack_backlog=1
>>> but now it can accept 2 connections.
>>>
>>> Signed-off-by: Wei Dong<weid@np.css.fujitsu.com>
>>> Signed-off-by: David S. Miller<davem@davemloft.net>
>>>
>>> Venkat
> --
> 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 net] net: qmi_wwan: adding more ZTE devices
From: Bjørn Mork @ 2012-10-18 15:11 UTC (permalink / raw)
To: netdev; +Cc: linux-usb, Bjørn Mork
Analyzed a few Windows driver description files, supporting
this long list of devices:
%ztewwan.DeviceDesc0002% = ztewwan.ndi, USB\VID_19D2&PID_0002&MI_01
%ztewwan.DeviceDesc0012% = ztewwan.ndi, USB\VID_19D2&PID_0012&MI_01
%ztewwan.DeviceDesc0017% = ztewwan.ndi, USB\VID_19D2&PID_0017&MI_03
%ztewwan.DeviceDesc0021% = ztewwan.ndi, USB\VID_19D2&PID_0021&MI_04
%ztewwan.DeviceDesc0025% = ztewwan.ndi, USB\VID_19D2&PID_0025&MI_01
%ztewwan.DeviceDesc0031% = ztewwan.ndi, USB\VID_19D2&PID_0031&MI_04
%ztewwan.DeviceDesc0042% = ztewwan.ndi, USB\VID_19D2&PID_0042&MI_04
%ztewwan.DeviceDesc0049% = ztewwan.ndi, USB\VID_19D2&PID_0049&MI_05
%ztewwan.DeviceDesc0052% = ztewwan.ndi, USB\VID_19D2&PID_0052&MI_04
%ztewwan.DeviceDesc0055% = ztewwan.ndi, USB\VID_19D2&PID_0055&MI_01
%ztewwan.DeviceDesc0058% = ztewwan.ndi, USB\VID_19D2&PID_0058&MI_04
%ztewwan.DeviceDesc0063% = ztewwan.ndi, USB\VID_19D2&PID_0063&MI_04
%ztewwan.DeviceDesc2002% = ztewwan.ndi, USB\VID_19D2&PID_2002&MI_04
%ztewwan.DeviceDesc0104% = ztewwan.ndi, USB\VID_19D2&PID_0104&MI_04
%ztewwan.DeviceDesc0113% = ztewwan.ndi, USB\VID_19D2&PID_0113&MI_05
%ztewwan.DeviceDesc0118% = ztewwan.ndi, USB\VID_19D2&PID_0118&MI_05
%ztewwan.DeviceDesc0121% = ztewwan.ndi, USB\VID_19D2&PID_0121&MI_05
%ztewwan.DeviceDesc0123% = ztewwan.ndi, USB\VID_19D2&PID_0123&MI_04
%ztewwan.DeviceDesc0124% = ztewwan.ndi, USB\VID_19D2&PID_0124&MI_05
%ztewwan.DeviceDesc0125% = ztewwan.ndi, USB\VID_19D2&PID_0125&MI_06
%ztewwan.DeviceDesc0126% = ztewwan.ndi, USB\VID_19D2&PID_0126&MI_05
%ztewwan.DeviceDesc1008% = ztewwan.ndi, USB\VID_19D2&PID_1008&MI_04
%ztewwan.DeviceDesc1010% = ztewwan.ndi, USB\VID_19D2&PID_1010&MI_04
%ztewwan.DeviceDesc1012% = ztewwan.ndi, USB\VID_19D2&PID_1012&MI_04
%ztewwan.DeviceDesc1402% = ztewwan.ndi, USB\VID_19D2&PID_1402&MI_02
%ztewwan.DeviceDesc0157% = ztewwan.ndi, USB\VID_19D2&PID_0157&MI_05
%ztewwan.DeviceDesc0158% = ztewwan.ndi, USB\VID_19D2&PID_0158&MI_03
%ztewwan.DeviceDesc1401% = ztewwan.ndi, USB\VID_19D2&PID_1401&MI_02
%ztewwan.DeviceDesc0130% = ztewwan.ndi, USB\VID_19D2&PID_0130&MI_01
%ztewwan.DeviceDesc0133% = ztewwan.ndi, USB\VID_19D2&PID_0133&MI_03
%ztewwan.DeviceDesc0176% = ztewwan.ndi, USB\VID_19D2&PID_0176&MI_03
%ztewwan.DeviceDesc0178% = ztewwan.ndi, USB\VID_19D2&PID_0178&MI_03
%ztewwan.DeviceDesc0168% = ztewwan.ndi, USB\VID_19D2&PID_0168&MI_04
;EuFi890
%ztewwan.DeviceDesc0191% = ztewwan.ndi, USB\VID_19D2&PID_0191&MI_04
;AL621
%ztewwan.DeviceDesc0167% = ztewwan.ndi, USB\VID_19D2&PID_0167&MI_04
;MF821
%ztewwan.DeviceDesc0199% = ztewwan.ndi, USB\VID_19D2&PID_0199&MI_01
%ztewwan.DeviceDesc0200% = ztewwan.ndi, USB\VID_19D2&PID_0200&MI_01
%ztewwan.DeviceDesc0257% = ztewwan.ndi, USB\VID_19D2&PID_0257&MI_03
;MF821V
%ztewwan.DeviceDesc1018% = ztewwan.ndi, USB\VID_19D2&PID_1018&MI_03
;MF91
%ztewwan.DeviceDesc1426% = ztewwan.ndi, USB\VID_19D2&PID_1426&MI_02
;0141
%ztewwan.DeviceDesc1247% = ztewwan.ndi, USB\VID_19D2&PID_1247&MI_04
%ztewwan.DeviceDesc1425% = ztewwan.ndi, USB\VID_19D2&PID_1425&MI_02
%ztewwan.DeviceDesc1424% = ztewwan.ndi, USB\VID_19D2&PID_1424&MI_02
%ztewwan.DeviceDesc1252% = ztewwan.ndi, USB\VID_19D2&PID_1252&MI_04
%ztewwan.DeviceDesc1254% = ztewwan.ndi, USB\VID_19D2&PID_1254&MI_04
%ztewwan.DeviceDesc1255A% = ztewwan.ndi, USB\VID_19D2&PID_1255&MI_03
%ztewwan.DeviceDesc1255B% = ztewwan.ndi, USB\VID_19D2&PID_1255&MI_04
%ztewwan.DeviceDesc1256% = ztewwan.ndi, USB\VID_19D2&PID_1256&MI_04
%ztewwan.DeviceDesc1245% = ztewwanCombB.ndi, USB\VID_19D2&PID_1245&MI_04
%ztewwan.DeviceDesc1021% = ztewwan.ndi, USB\VID_19D2&PID_1021&MI_02
Adding the ones we were missing.
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
drivers/net/usb/qmi_wwan.c | 41 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 6883c37..9d23ba2 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -371,16 +371,57 @@ static const struct usb_device_id products[] = {
},
/* 3. Combined interface devices matching on interface number */
+ {QMI_FIXED_INTF(0x19d2, 0x0002, 1)},
+ {QMI_FIXED_INTF(0x19d2, 0x0012, 1)},
+ {QMI_FIXED_INTF(0x19d2, 0x0017, 3)},
+ {QMI_FIXED_INTF(0x19d2, 0x0021, 4)},
+ {QMI_FIXED_INTF(0x19d2, 0x0025, 1)},
+ {QMI_FIXED_INTF(0x19d2, 0x0031, 4)},
+ {QMI_FIXED_INTF(0x19d2, 0x0042, 4)},
+ {QMI_FIXED_INTF(0x19d2, 0x0049, 5)},
+ {QMI_FIXED_INTF(0x19d2, 0x0052, 4)},
{QMI_FIXED_INTF(0x19d2, 0x0055, 1)}, /* ZTE (Vodafone) K3520-Z */
+ {QMI_FIXED_INTF(0x19d2, 0x0058, 4)},
{QMI_FIXED_INTF(0x19d2, 0x0063, 4)}, /* ZTE (Vodafone) K3565-Z */
{QMI_FIXED_INTF(0x19d2, 0x0104, 4)}, /* ZTE (Vodafone) K4505-Z */
+ {QMI_FIXED_INTF(0x19d2, 0x0113, 5)},
+ {QMI_FIXED_INTF(0x19d2, 0x0118, 5)},
+ {QMI_FIXED_INTF(0x19d2, 0x0121, 5)},
+ {QMI_FIXED_INTF(0x19d2, 0x0123, 4)},
+ {QMI_FIXED_INTF(0x19d2, 0x0124, 5)},
+ {QMI_FIXED_INTF(0x19d2, 0x0125, 6)},
+ {QMI_FIXED_INTF(0x19d2, 0x0126, 5)},
+ {QMI_FIXED_INTF(0x19d2, 0x0130, 1)},
+ {QMI_FIXED_INTF(0x19d2, 0x0133, 3)},
+ {QMI_FIXED_INTF(0x19d2, 0x0141, 5)},
{QMI_FIXED_INTF(0x19d2, 0x0157, 5)}, /* ZTE MF683 */
+ {QMI_FIXED_INTF(0x19d2, 0x0158, 3)},
{QMI_FIXED_INTF(0x19d2, 0x0167, 4)}, /* ZTE MF820D */
+ {QMI_FIXED_INTF(0x19d2, 0x0168, 4)},
+ {QMI_FIXED_INTF(0x19d2, 0x0176, 3)},
+ {QMI_FIXED_INTF(0x19d2, 0x0178, 3)},
+ {QMI_FIXED_INTF(0x19d2, 0x0191, 4)}, /* ZTE EuFi890 */
+ {QMI_FIXED_INTF(0x19d2, 0x0199, 1)}, /* ZTE MF820S */
+ {QMI_FIXED_INTF(0x19d2, 0x0200, 1)},
+ {QMI_FIXED_INTF(0x19d2, 0x0257, 3)}, /* ZTE MF821 */
{QMI_FIXED_INTF(0x19d2, 0x0326, 4)}, /* ZTE MF821D */
{QMI_FIXED_INTF(0x19d2, 0x1008, 4)}, /* ZTE (Vodafone) K3570-Z */
{QMI_FIXED_INTF(0x19d2, 0x1010, 4)}, /* ZTE (Vodafone) K3571-Z */
+ {QMI_FIXED_INTF(0x19d2, 0x1012, 4)},
{QMI_FIXED_INTF(0x19d2, 0x1018, 3)}, /* ZTE (Vodafone) K5006-Z */
+ {QMI_FIXED_INTF(0x19d2, 0x1021, 2)},
+ {QMI_FIXED_INTF(0x19d2, 0x1245, 4)},
+ {QMI_FIXED_INTF(0x19d2, 0x1247, 4)},
+ {QMI_FIXED_INTF(0x19d2, 0x1252, 4)},
+ {QMI_FIXED_INTF(0x19d2, 0x1254, 4)},
+ {QMI_FIXED_INTF(0x19d2, 0x1255, 3)},
+ {QMI_FIXED_INTF(0x19d2, 0x1255, 4)},
+ {QMI_FIXED_INTF(0x19d2, 0x1256, 4)},
+ {QMI_FIXED_INTF(0x19d2, 0x1401, 2)},
{QMI_FIXED_INTF(0x19d2, 0x1402, 2)}, /* ZTE MF60 */
+ {QMI_FIXED_INTF(0x19d2, 0x1424, 2)},
+ {QMI_FIXED_INTF(0x19d2, 0x1425, 2)},
+ {QMI_FIXED_INTF(0x19d2, 0x1426, 2)}, /* ZTE MF91 */
{QMI_FIXED_INTF(0x19d2, 0x2002, 4)}, /* ZTE (Vodafone) K3765-Z */
{QMI_FIXED_INTF(0x0f3d, 0x68a2, 8)}, /* Sierra Wireless MC7700 */
{QMI_FIXED_INTF(0x114f, 0x68a2, 8)}, /* Sierra Wireless MC7750 */
--
1.7.10.4
^ permalink raw reply related
* [PATCH] netlink: use kfree_rcu() in netlink_release()
From: Eric Dumazet @ 2012-10-18 13:21 UTC (permalink / raw)
To: David Miller
Cc: netdev, Stéphane Marchesin, Jonathan Kliegman, Sam Leffler
From: Eric Dumazet <edumazet@google.com>
On some suspend/resume operations involving wimax device, we have
noticed some intermittent memory corruptions in netlink code.
Stéphane Marchesin tracked this corruption in netlink_update_listeners()
and suggested a patch.
It appears netlink_release() should use kfree_rcu() instead of kfree()
for the listeners structure as it may be used by other cpus using RCU
protection.
netlink_release() must set to NULL the listeners pointer when
it is about to be freed.
Also have to protect netlink_update_listeners() and
netlink_has_listeners() if listeners is NULL.
Add a nl_deref_protected() lockdep helper to properly document which
locks protects us.
Reported-by: Jonathan Kliegman <kliegs@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Stéphane Marchesin <marcheu@google.com>
Cc: Sam Leffler <sleffler@google.com>
---
net/netlink/af_netlink.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 01e944a..4da797f 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -138,6 +138,8 @@ static int netlink_dump(struct sock *sk);
static DEFINE_RWLOCK(nl_table_lock);
static atomic_t nl_table_users = ATOMIC_INIT(0);
+#define nl_deref_protected(X) rcu_dereference_protected(X, lockdep_is_held(&nl_table_lock));
+
static ATOMIC_NOTIFIER_HEAD(netlink_chain);
static inline u32 netlink_group_mask(u32 group)
@@ -345,6 +347,11 @@ netlink_update_listeners(struct sock *sk)
struct hlist_node *node;
unsigned long mask;
unsigned int i;
+ struct listeners *listeners;
+
+ listeners = nl_deref_protected(tbl->listeners);
+ if (!listeners)
+ return;
for (i = 0; i < NLGRPLONGS(tbl->groups); i++) {
mask = 0;
@@ -352,7 +359,7 @@ netlink_update_listeners(struct sock *sk)
if (i < NLGRPLONGS(nlk_sk(sk)->ngroups))
mask |= nlk_sk(sk)->groups[i];
}
- tbl->listeners->masks[i] = mask;
+ listeners->masks[i] = mask;
}
/* this function is only called with the netlink table "grabbed", which
* makes sure updates are visible before bind or setsockopt return. */
@@ -536,7 +543,11 @@ static int netlink_release(struct socket *sock)
if (netlink_is_kernel(sk)) {
BUG_ON(nl_table[sk->sk_protocol].registered == 0);
if (--nl_table[sk->sk_protocol].registered == 0) {
- kfree(nl_table[sk->sk_protocol].listeners);
+ struct listeners *old;
+
+ old = nl_deref_protected(nl_table[sk->sk_protocol].listeners);
+ RCU_INIT_POINTER(nl_table[sk->sk_protocol].listeners, NULL);
+ kfree_rcu(old, rcu);
nl_table[sk->sk_protocol].module = NULL;
nl_table[sk->sk_protocol].bind = NULL;
nl_table[sk->sk_protocol].flags = 0;
@@ -982,7 +993,7 @@ int netlink_has_listeners(struct sock *sk, unsigned int group)
rcu_read_lock();
listeners = rcu_dereference(nl_table[sk->sk_protocol].listeners);
- if (group - 1 < nl_table[sk->sk_protocol].groups)
+ if (listeners && group - 1 < nl_table[sk->sk_protocol].groups)
res = test_bit(group - 1, listeners->masks);
rcu_read_unlock();
@@ -1625,7 +1636,7 @@ int __netlink_change_ngroups(struct sock *sk, unsigned int groups)
new = kzalloc(sizeof(*new) + NLGRPSZ(groups), GFP_ATOMIC);
if (!new)
return -ENOMEM;
- old = rcu_dereference_protected(tbl->listeners, 1);
+ old = nl_deref_protected(tbl->listeners);
memcpy(new->masks, old->masks, NLGRPSZ(tbl->groups));
rcu_assign_pointer(tbl->listeners, new);
^ permalink raw reply related
* Re: Bug? TCP shutdown behaviour when deleting local IP addresses
From: Benny Amorsen @ 2012-10-18 13:09 UTC (permalink / raw)
To: Mikael Abrahamsson
Cc: Chris Friesen, netdev, David Miller, Alexey Kuznetsov,
James Morris, Patrick McHardy, Hideaki YOSHIFUJI
In-Reply-To: <alpine.DEB.2.00.1210181458560.21297@uplift.swm.pp.se>
Mikael Abrahamsson <swmike@swm.pp.se> writes:
> Would you mind if it was user configurable?
No, I would not mind. But I am the wrong person to ask that question, I
do not have to deal with supporting configuration options.
> Would you mind if the TCP retransmit timers were severely lowered upon
> reconnection so your TCP sessions started working quicker? Or you
> don't see this problem at all?
I would not mind that at all. I do not particularly recall seeing the
problem, but that sounds like a great idea.
/Benny
^ permalink raw reply
* Re: Bug? TCP shutdown behaviour when deleting local IP addresses
From: Mikael Abrahamsson @ 2012-10-18 13:00 UTC (permalink / raw)
To: Benny Amorsen
Cc: Chris Friesen, netdev, David Miller, Alexey Kuznetsov,
James Morris, Patrick McHardy, Hideaki YOSHIFUJI
In-Reply-To: <m3vce83w1y.fsf@ursa.amorsen.dk>
On Thu, 18 Oct 2012, Benny Amorsen wrote:
> Chris Friesen <chris.friesen@genband.com> writes:
>
>> 7) client waits for response from server, and waits forever or until
>> keepalive expires
>
> Please don't fix that. I sometimes move my laptop from one ethernet
> socket to another. During the move I lose my DHCP-assigned IP address.
Would you mind if it was user configurable?
> I do not want to lose my SSH sessions too.
Would you mind if the TCP retransmit timers were severely lowered upon
reconnection so your TCP sessions started working quicker? Or you don't
see this problem at all?
--
Mikael Abrahamsson email: swmike@swm.pp.se
^ permalink raw reply
* Re: [PATCH v1] net/core: support runtime PM on net_device
From: Ming Lei @ 2012-10-18 12:55 UTC (permalink / raw)
To: Bjørn Mork
Cc: David S. Miller, Rafael J. Wysocki, Oliver Neukum, Alan Stern,
netdev, linux-pm
In-Reply-To: <87obk080us.fsf@nemi.mork.no>
On Thu, Oct 18, 2012 at 7:40 PM, Bjørn Mork <bjorn@mork.no> wrote:
>
> Because bugs are supposed to be fixed and not hidden?
OK, now let's discuss solution for the problem which should be
clear and be solved.
The 1st one is to do it by this patch, or rule out some ioctl cmd
which needn't wakeup device.
The second one is to fix ioctl one by one in each driver.
Any other solution?
>
> Note that I am not claiming this is a bug. That is still an open
> question as far as I can see.
Thanks,
--
Ming Lei
^ permalink raw reply
* RE: sock_getsockopt() not exported
From: Eric Dumazet @ 2012-10-18 12:27 UTC (permalink / raw)
To: David Laight; +Cc: netdev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B7053@saturn3.aculab.com>
On Thu, 2012-10-18 at 13:03 +0100, David Laight wrote:
> > On Thu, 2012-10-18 at 10:11 +0100, David Laight wrote:
> > > I've noticed that net/core/sock.c contains an
> > > EXPORT_SYMBOL(sock_setsockopt)
> > > but is missing the corresponding
> > > EXPORT_SYMBOL(sock_getsockopt)
> > >
> > > In-kernel users of sockets probably manage without
> > > needing to read SOL_SOCKET options.
> > > (They do need to set SO_REUSADDR and SO_KEEPALIVE.)
> >
> > sock_setsockopt() is exported because sunrpc needs it, and sunrpc can be
> > a module
> >
> > sock_getsockopt() is not exported because no module needs it yet.
> >
> > The day one user needs it, we'll add the EXPORT_SYMBOL()
>
> The problem is that it might be needed by an 'out of tree' driver.
> We already have the function:
We dont care of out of tree drivers.
Thats really simple.
Submit this driver, and add the EXPORT_SYMBOL() you need at that time.
^ permalink raw reply
* Re: Re: [PATCH v3] net-tcp: TCP/IP stack bypass for loopback connections
From: Bruce Curtis @ 2012-10-18 12:23 UTC (permalink / raw)
To: Weiping Pan; +Cc: open list:NETWORKING [GENERAL]
In-Reply-To: <507FD7A5.9080503@gmail.com>
Hi Weiping,
Also wasn't able to repro on a real machine, will try a VM, but has to
do with the "making friends" code-paths which have been refactored
since last patch (due to Eric's comments above about an unreferenced
skb->friend sock pointer), I'll be sending out a new patch soon.
Thanks,
Bruce
On Thu, Oct 18, 2012 at 2:19 PM, Weiping Pan <panweiping3@gmail.com> wrote:
> Sorry, forget to cc the list.
>
>
> 2012/9/18 Bruce "Brutus" Curtis<brutus@google.com>:
>>
>> From: "Bruce \"Brutus\" Curtis"<brutus@google.com>
>>
>> TCP/IP loopback socket pair stack bypass, based on an idea by, and
>> rough upstream patch from, David Miller<davem@davemloft.net> called
>> "friends", the data structure modifcations and connection scheme are
>> reused with extensive data-path changes.
>
>
> Hi, Bruce,
>
> I found that there is a bug in the tcp friends patch,
> when I kill netperf randomly, panic occurs in tcp_close().
>
> BUG: unable to handle kernel NULL pointer dereference at 0000000d
> IP: [<c0835a9b>] tcp_close+0x7b/0x3a0
> *pde = 00000000
> Oops: 0000 [#1] SMP
> Modules linked in: fuse 8021q garp stp llc ip6t_REJECT
> nf_conntrack_ipv6 nf_defrag_ipv6 nf_conntrack_ipv4 nf_defrag_ipv4
> ip6table_filter xt_state nf_conntrack ip6_tables ppdev parport_pc
> pcspkr i2c_piix4 i2c_core parport microcode e1000 uinput
> Pid: 16627, comm: netperf Not tainted 3.6.0+ #25 innotek GmbH VirtualBox
> EIP: 0060:[<c0835a9b>] EFLAGS: 00010202 CPU: 1
> EIP is at tcp_close+0x7b/0x3a0
> EAX: f6f41240 EBX: c2a46f40 ECX: 00000000 EDX: 00000001
> ESI: 00000000 EDI: c2a46f88 EBP: c2a1fd8c ESP: c2a1fd78
> DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
> CR0: 8005003b CR2: 0000000d CR3: 00c07000 CR4: 000006d0
> DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
> DR6: ffff0ff0 DR7: 00000400
> Process netperf (pid: 16627, ti=c2a1e000 task=f44cd780 task.ti=c2a1e000)
> Stack:
> c0b4d880 00000000 c2a46f40 ed821080 f4de7f00 c2a1fd9c c0857cff ed821080
> 00000000 c2a1fdb0 c07e3db0 00000000 f6f07540 00000008 c2a1fdbc c07e4127
> f4de7f00 c2a1fdec c05378e8 00000001 00000000 00000000 f54bf010 ed82109c
> Call Trace:
> [<c0857cff>] inet_release+0x5f/0x70
> [<c07e3db0>] sock_release+0x20/0x80
> [<c07e4127>] sock_close+0x17/0x30
> [<c05378e8>] __fput+0x98/0x1f0
> [<c0537a4d>] ____fput+0xd/0x10
> [<c04580f1>] task_work_run+0x91/0xb0
> [<c0441157>] do_exit+0x177/0x7f0
> [<c0422c97>] ? smp_reschedule_interrupt+0x27/0x30
> [<c0441a67>] do_group_exit+0x37/0xa0
> [<c044e989>] get_signal_to_deliver+0x1c9/0x5b0
> [<c0471393>] ? update_curr+0x213/0x380
> [<c0402bca>] do_signal+0x2a/0x980
> [<c04026c7>] ? __switch_to+0xc7/0x340
> [<c08ea8c9>] ? __schedule+0x379/0x780
> [<c08ec3f8>] ? apic_timer_interrupt+0x34/0x3c
> [<c04ad0ae>] ? __audit_syscall_exit+0x36e/0x3a0
> [<c04ad0ae>] ? __audit_syscall_exit+0x36e/0x3a0
> [<c04036e5>] do_notify_resume+0x75/0xa0
> [<c08ec1c1>] work_notifysig+0x30/0x37
> Code: 85 c0 74 3e 83 6b 50 01 8b 08 8b 50 04 c7 00 00 00 00 00 c7 40
> 04 00 00 00 00 89 51 04 89 0a 8b 88 9c 00 00 00 8b 50 34 2b 50 30<0f>
> b6 49 0d 83 e1 01 29 ca 01 d6 e8 c5 64 fb ff 8b 43 48 39 f8
> EIP: [<c0835a9b>] tcp_close+0x7b/0x3a0 SS:ESP 0068:c2a1fd78
> CR2: 000000000000000d
> ---[ end trace 9f6d5c8fc973265c ]---
>
>
> How to reproduce it ?
> 1 run netserver in a loop
> 2 run netperf with different modes in a loop
> 3 kill netperf randomly
>
> And I found it is easy to see the panic on VirtualBox, but I did not
> see it on the real machine.
>
> Any hints ?
>
> thanks
> Weiping Pan
>
>
> --
> 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
* Re: [PATCH v7 00/10] IPC: checkpoint/restore in userspace enhancements
From: Eric W. Biederman @ 2012-10-18 12:11 UTC (permalink / raw)
To: Stanislav Kinsbursky
Cc: akpm, catalin.marinas, will.deacon, dhowells, manfred, hughd,
jmorris, mtk.manpages, kosaki.motohiro, paulmck, sds, devel,
a.p.zijlstra, cmetcalf, linux-driver, ron.mercer, viro, eparis,
tglx, jitendra.kalsaria, netdev, linux-kernel,
linux-security-module, casey, Michael Kerrisk (man-pages)
In-Reply-To: <20121018101543.16036.12221.stgit@localhost.localdomain>
Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>
You ignored all of my feedback that the first 7 messages of your
patchset are unnecessary. In particular you did not attempt to focus
your patchset on those operations that are most important.
Upon examination it appears also that the 8th and 9th patches of the
patchset are also unnecessary. And the 10th patch is just a test of the
previous patches, making the 10th patch unneceesary without the rest.
In net this entire patchset is unnecessary and a waste of your reviewers
time.
Once you have IPC checkpoint and restore working there may be a point to
come back and optimize things. Please don't come back with any System V
Interprocess Communication patches until you can report how much time
is saved by each and every change.
Eric
^ permalink raw reply
* RE: sock_getsockopt() not exported
From: David Laight @ 2012-10-18 12:03 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1350552612.26103.1281.camel@edumazet-glaptop>
> On Thu, 2012-10-18 at 10:11 +0100, David Laight wrote:
> > I've noticed that net/core/sock.c contains an
> > EXPORT_SYMBOL(sock_setsockopt)
> > but is missing the corresponding
> > EXPORT_SYMBOL(sock_getsockopt)
> >
> > In-kernel users of sockets probably manage without
> > needing to read SOL_SOCKET options.
> > (They do need to set SO_REUSADDR and SO_KEEPALIVE.)
>
> sock_setsockopt() is exported because sunrpc needs it, and sunrpc can be
> a module
>
> sock_getsockopt() is not exported because no module needs it yet.
>
> The day one user needs it, we'll add the EXPORT_SYMBOL()
The problem is that it might be needed by an 'out of tree' driver.
We already have the function:
static int
do_getsockopt(struct socket *sock, int level, int name, void *val, int len)
{
mm_segment_t prevfs = get_fs();
int rval;
set_fs(KERNEL_DS);
if (level == SOL_SOCKET) {
/* There is no EXPORT_SYMBOL(sock_getsockopt) in net/core/sock.c */
rval = -EINVAL; // sock_getsockopt(sock, level, name, val, &len);
} else {
rval = sock->ops->getsockopt(sock, level, name, val, &len);
}
set_fs(prevfs);
return rval;
}
We need to get options (for SCTP) fortunately they aren't
SOL_SOCKET ones.
David
^ permalink raw reply
* Re: [PATCH v1] net/core: support runtime PM on net_device
From: Bjørn Mork @ 2012-10-18 11:40 UTC (permalink / raw)
To: Ming Lei
Cc: David S. Miller, Rafael J. Wysocki, Oliver Neukum, Alan Stern,
netdev, linux-pm
In-Reply-To: <CACVXFVPu1m3CJaFavW9eaU3Z850q+x1N_Yo7tri1UTFpgfYVXQ@mail.gmail.com>
Ming Lei <ming.lei@canonical.com> writes:
> On Thu, Oct 18, 2012 at 4:29 PM, Bjørn Mork <bjorn@mork.no> wrote:
>> Ming Lei <ming.lei@canonical.com> writes:
>>
>>> In ioctl path on net_device, the physical deivce is often
>>> touched, but the physical device may have been put into runtime
>>> suspend state already, so cause some utilitis(ifconfig, ethtool,
>>> ...) to return failure in this situation.
>>
>> I have to as the stupid questions again, sorry...
>>
>> Just wondering, isn't that really a driver problem? The driver will
>
> It is or not, :-)
>
>> know whether or not hardware access is required, and should wake up the
>
> The netcore knows that first, doesn't it？
Really? Does netcore know which ioctls the driver can handle without
waking the device? You can of course do an educated guess, but I really
hate guesswork if there is a real answer somewhere else...
>> device if necessary. Unless I misunderstand something here, this seems
>> like papering over driver bugs?
>
> Suppose it is driver bug, and basically most network drivers don't consider
> that, and we can fix that in netcore generally, so why bother all drivers to do
> that?
Because bugs are supposed to be fixed and not hidden?
Note that I am not claiming this is a bug. That is still an open
question as far as I can see.
Bjørn
^ permalink raw reply
* Re: [PATCH v1] net/core: support runtime PM on net_device
From: Ming Lei @ 2012-10-18 11:01 UTC (permalink / raw)
To: Bjørn Mork
Cc: David S. Miller, Rafael J. Wysocki, Oliver Neukum, Alan Stern,
netdev, linux-pm
In-Reply-To: <8762689o9u.fsf@nemi.mork.no>
On Thu, Oct 18, 2012 at 4:29 PM, Bjørn Mork <bjorn@mork.no> wrote:
> Ming Lei <ming.lei@canonical.com> writes:
>
>> In ioctl path on net_device, the physical deivce is often
>> touched, but the physical device may have been put into runtime
>> suspend state already, so cause some utilitis(ifconfig, ethtool,
>> ...) to return failure in this situation.
>
> I have to as the stupid questions again, sorry...
>
> Just wondering, isn't that really a driver problem? The driver will
It is or not, :-)
> know whether or not hardware access is required, and should wake up the
The netcore knows that first, doesn't it？
> device if necessary. Unless I misunderstand something here, this seems
> like papering over driver bugs?
Suppose it is driver bug, and basically most network drivers don't consider
that, and we can fix that in netcore generally, so why bother all drivers to do
that?
Thanks,
--
Ming Lei
^ permalink raw reply
* Re: Bug? TCP shutdown behaviour when deleting local IP addresses
From: Benny Amorsen @ 2012-10-18 10:37 UTC (permalink / raw)
To: Chris Friesen
Cc: netdev, David Miller, Alexey Kuznetsov, James Morris,
Patrick McHardy, Hideaki YOSHIFUJI
In-Reply-To: <507F38D4.102@genband.com>
Chris Friesen <chris.friesen@genband.com> writes:
> 7) client waits for response from server, and waits forever or until
> keepalive expires
Please don't fix that. I sometimes move my laptop from one ethernet
socket to another. During the move I lose my DHCP-assigned IP address.
I do not want to lose my SSH sessions too.
/Benny
^ permalink raw reply
* Re: [PATCH v7 00/10] IPC: checkpoint/restore in userspace enhancements
From: Michael Kerrisk (man-pages) @ 2012-10-18 10:41 UTC (permalink / raw)
To: Stanislav Kinsbursky
Cc: akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
dhowells-H+wXaHxf7aLQT0dZR+AlfA, manfred-nhLOkwUX5cPe2c5cEj3t2g,
hughd-hpIqsD4AKlfQT0dZR+AlfA, jmorris-gx6/JNMH7DfYtjvyW6yDsg,
kosaki.motohiro-+CUm20s59erQFUHtdCDX3A,
paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
sds-+05T5uksL2qpZYMLLGbcSA, devel-GEFAQzZX7r8dnm+yROfE0A,
a.p.zijlstra-/NLkJaSkS4VmR6Xm/wNWPw,
cmetcalf-kv+TWInifGbQT0dZR+AlfA,
linux-driver-h88ZbnxC6KDQT0dZR+AlfA,
ron.mercer-h88ZbnxC6KDQT0dZR+AlfA,
viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn,
eparis-FjpueFixGhCM4zKIHC2jIg, tglx-hfZtesqFncYOwBW4kG4KsQ,
jitendra.kalsaria-h88ZbnxC6KDQT0dZR+AlfA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, casey-iSGtlc1asvQWG2LlvL+J4A,
Linux API
In-Reply-To: <20121018101543.16036.12221.stgit-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
Stanislaw,
This patch series is an API change. Could you please make sure that
all future revisions CC linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, as per
Documentation/SubmitChecklist.
Cheers,
Michael
On Thu, Oct 18, 2012 at 12:22 PM, Stanislav Kinsbursky
<skinsbursky-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> wrote:
> v7:
> 1) Added comments in places, when behiviour is not obvious.
> 2) Compilation fixed for compat layer
> 3) test updated to use existent header files (instead of hard-coding new
> defines in case of absence).
> 4) comment fixed in qlge driver
>
> v6:
> 1) rebased on 3.7-rc1
>
> v5:
> 1) Several define-dependent compile bugs fixed
> 2) IPC message copy test updated
> 3) A couple of minor fixes.
> 4) Qlogic driver update: rename of its internal SEM_SET define into SEM_INIT
> (compile error).
>
> v4:
> 1) If MSG_COPY flag is specified, then "mtype" is not a type, but message
> number to copy.
> 2) MSG_SET_COPY logic for sys_msgctl() was removed.
>
> v3:
> 1) Copy messages to user-space under spinlock was replaced by allocation of
> dummy message before queue lock and then copy of desired message to the dummy
> one instead of unlinking it from queue list.
> I.e. the message queue copy logic was changed: messages can be retrived one by
> one (instead of receiving of the whole list at once).
>
> This patch set is aimed to provide additional functionality for all IPC
> objects,
> which is required for migration of these objects by user-space
> checkpoint/restore utils (CRIU).
>
> The main problem here was impossibility to set up object id. This patch set
> solves the problem in two steps:
> 1) Makes it possible to create new object (shared memory, semaphores set or
> messages queue) with ID, equal to passed key.
> 2) Makes it possible to change existent object key.
>
> Another problem was to peek messages from queues without deleting them.
> This was achived by introducing of new MSG_COPY flag for sys_msgrcv(). If
> MSG_COPY flag is set, then msgtyp is interpreted as message number.
>
> The following series implements...
>
> ---
>
> Stanislav Kinsbursky (10):
> ipc: remove forced assignment of selected message
> ipc: "use key as id" functionality for resource get system call introduced
> ipc: segment key change helper introduced
> ipc: add new SHM_SET command for sys_shmctl() call
> ipc: add new MSG_SET command for sys_msgctl() call
> qlge driver: rename internal SEM_SET macro to SEM_INIT
> ipc: add new SEM_SET command for sys_semctl() call
> IPC: message queue receive cleanup
> IPC: message queue copy feature introduced
> test: IPC message queue copy feture test
>
>
> drivers/net/ethernet/qlogic/qlge/qlge.h | 4
> drivers/net/ethernet/qlogic/qlge/qlge_main.c | 16 +-
> include/linux/msg.h | 5 -
> include/uapi/linux/ipc.h | 1
> include/uapi/linux/msg.h | 2
> include/uapi/linux/sem.h | 1
> include/uapi/linux/shm.h | 1
> ipc/compat.c | 54 +++---
> ipc/msg.c | 117 ++++++++++---
> ipc/msgutil.c | 38 ++++
> ipc/sem.c | 15 +-
> ipc/shm.c | 18 ++
> ipc/util.c | 67 +++++++-
> ipc/util.h | 6 +
> security/selinux/hooks.c | 3
> security/smack/smack_lsm.c | 3
> tools/testing/selftests/ipc/Makefile | 25 +++
> tools/testing/selftests/ipc/msgque.c | 231 ++++++++++++++++++++++++++
> 18 files changed, 526 insertions(+), 81 deletions(-)
> create mode 100644 tools/testing/selftests/ipc/Makefile
> create mode 100644 tools/testing/selftests/ipc/msgque.c
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface"; http://man7.org/tlpi/
^ permalink raw reply
* Re: [PATCH v7 09/10] IPC: message queue copy feature introduced
From: Michael Kerrisk (man-pages) @ 2012-10-18 10:39 UTC (permalink / raw)
To: Stanislav Kinsbursky
Cc: akpm, catalin.marinas, will.deacon, dhowells, manfred, hughd,
jmorris, kosaki.motohiro, paulmck, sds, devel, a.p.zijlstra,
cmetcalf, linux-driver, ron.mercer, viro, eparis, tglx,
jitendra.kalsaria, netdev, linux-kernel, linux-security-module,
ebiederm, casey
In-Reply-To: <20121018102322.16036.86866.stgit@localhost.localdomain>
On Thu, Oct 18, 2012 at 12:23 PM, Stanislav Kinsbursky
<skinsbursky@parallels.com> wrote:
> This patch is required for checkpoint/restore in userspace.
> IOW, c/r requires some way to get all pending IPC messages without deleting
> them from the queue (checkpoint can fail and in this case tasks will be resumed,
> so queue have to be valid).
> To achive this, new operation flag MSG_COPY for sys_msgrcv() system call was
> introduced. If this flag was specified, then mtype is interpreted as number of
> the message to copy.
> If MSG_COPY is set, then kernel will allocate dummy message with passed size,
> and then use new copy_msg() helper function to copy desired message (instead of
> unlinking it from the queue).
>
> Notes:
> 1) Return -ENOSYS if MSG_COPY is specified, but CONFIG_CHECKPOINT_RESTORE is
> not set.
Stanislav,
A naive question, because I have not followed C/R closely. How do you
deal with the case that other processes may be reading from the queue?
(Or is that disabled during checkpointing?)
Thanks,
Michael
^ permalink raw reply
* [PATCH v7 10/10] test: IPC message queue copy feture test
From: Stanislav Kinsbursky @ 2012-10-18 10:23 UTC (permalink / raw)
To: akpm
Cc: catalin.marinas, will.deacon, dhowells, manfred, hughd, jmorris,
mtk.manpages, kosaki.motohiro, paulmck, sds, devel, a.p.zijlstra,
cmetcalf, linux-driver, ron.mercer, viro, eparis, tglx,
jitendra.kalsaria, netdev, linux-kernel, linux-security-module,
ebiederm, casey
In-Reply-To: <20121018101543.16036.12221.stgit@localhost.localdomain>
This test can be used to check wheither kernel supports IPC message queue copy
and restore features (required by CRIU project).
Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
---
tools/testing/selftests/ipc/Makefile | 25 ++++
tools/testing/selftests/ipc/msgque.c | 231 ++++++++++++++++++++++++++++++++++
2 files changed, 256 insertions(+), 0 deletions(-)
create mode 100644 tools/testing/selftests/ipc/Makefile
create mode 100644 tools/testing/selftests/ipc/msgque.c
diff --git a/tools/testing/selftests/ipc/Makefile b/tools/testing/selftests/ipc/Makefile
new file mode 100644
index 0000000..5386fd7
--- /dev/null
+++ b/tools/testing/selftests/ipc/Makefile
@@ -0,0 +1,25 @@
+uname_M := $(shell uname -m 2>/dev/null || echo not)
+ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/i386/)
+ifeq ($(ARCH),i386)
+ ARCH := X86
+ CFLAGS := -DCONFIG_X86_32 -D__i386__
+endif
+ifeq ($(ARCH),x86_64)
+ ARCH := X86
+ CFLAGS := -DCONFIG_X86_64 -D__x86_64__
+endif
+
+CFLAGS += -I../../../../usr/include/
+
+all:
+ifeq ($(ARCH),X86)
+ gcc $(CFLAGS) msgque.c -o msgque_test
+else
+ echo "Not an x86 target, can't build msgque selftest"
+endif
+
+run_tests: all
+ ./msgque_test
+
+clean:
+ rm -fr ./msgque_test
diff --git a/tools/testing/selftests/ipc/msgque.c b/tools/testing/selftests/ipc/msgque.c
new file mode 100644
index 0000000..e2d6a64
--- /dev/null
+++ b/tools/testing/selftests/ipc/msgque.c
@@ -0,0 +1,231 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <errno.h>
+#include <linux/msg.h>
+
+#define MAX_MSG_SIZE 32
+
+struct msg1 {
+ int msize;
+ long mtype;
+ char mtext[MAX_MSG_SIZE];
+};
+
+#define TEST_STRING "Test sysv5 msg"
+#define MSG_TYPE 1
+
+#define ANOTHER_TEST_STRING "Yet another test sysv5 msg"
+#define ANOTHER_MSG_TYPE 26538
+
+struct msgque_data {
+ int msq_id;
+ int qbytes;
+ int kern_id;
+ int qnum;
+ int mode;
+ struct msg1 *messages;
+};
+
+int restore_queue(struct msgque_data *msgque)
+{
+ struct msqid_ds ds;
+ int id, i;
+
+ id = msgget(msgque->msq_id,
+ msgque->mode | IPC_CREAT | IPC_EXCL | IPC_PRESET);
+ if (id == -1) {
+ printf("Failed to create queue\n");
+ return -errno;
+ }
+
+ if (id != msgque->msq_id) {
+ printf("Failed to preset id (%d instead of %d)\n",
+ id, msgque->msq_id);
+ return -EFAULT;
+ }
+
+ if (msgctl(id, MSG_STAT, &ds) < 0) {
+ printf("Failed to stat queue\n");
+ return -errno;
+ }
+
+ ds.msg_perm.key = msgque->msq_id;
+ ds.msg_qbytes = msgque->qbytes;
+ if (msgctl(id, MSG_SET, &ds) < 0) {
+ printf("Failed to update message key\n");
+ return -errno;
+ }
+
+ for (i = 0; i < msgque->qnum; i++) {
+ if (msgsnd(msgque->msq_id, &msgque->messages[i].mtype, msgque->messages[i].msize, IPC_NOWAIT) != 0) {
+ printf("msgsnd failed (%m)\n");
+ return -errno;
+ };
+ }
+ return 0;
+}
+
+int check_and_destroy_queue(struct msgque_data *msgque)
+{
+ struct msg1 message;
+ int cnt = 0, ret;
+
+ while (1) {
+ ret = msgrcv(msgque->msq_id, &message.mtype, MAX_MSG_SIZE, 0, IPC_NOWAIT);
+ if (ret < 0) {
+ if (errno == ENOMSG)
+ break;
+ printf("Failed to read IPC message: %m\n");
+ ret = -errno;
+ goto err;
+ }
+ if (ret != msgque->messages[cnt].msize) {
+ printf("Wrong message size: %d (expected %d)\n", ret, msgque->messages[cnt].msize);
+ ret = -EINVAL;
+ goto err;
+ }
+ if (message.mtype != msgque->messages[cnt].mtype) {
+ printf("Wrong message type\n");
+ ret = -EINVAL;
+ goto err;
+ }
+ if (memcmp(message.mtext, msgque->messages[cnt].mtext, ret)) {
+ printf("Wrong message content\n");
+ ret = -EINVAL;
+ goto err;
+ }
+ cnt++;
+ }
+
+ if (cnt != msgque->qnum) {
+ printf("Wrong message number\n");
+ ret = -EINVAL;
+ goto err;
+ }
+
+ ret = 0;
+err:
+ if (msgctl(msgque->msq_id, IPC_RMID, 0)) {
+ printf("Failed to destroy queue: %d\n", -errno);
+ return -errno;
+ }
+ return ret;
+}
+
+int dump_queue(struct msgque_data *msgque)
+{
+ struct msqid_ds ds;
+ int i, ret;
+
+ for (msgque->kern_id = 0; msgque->kern_id < 256; msgque->kern_id++) {
+ ret = msgctl(msgque->kern_id, MSG_STAT, &ds);
+ if (ret < 0) {
+ if (errno == -EINVAL)
+ continue;
+ printf("Failed to get stats for IPC queue with id %d\n", msgque->kern_id);
+ return -errno;
+ }
+
+ if (ret == msgque->msq_id)
+ break;
+ }
+
+ msgque->messages = malloc(sizeof(struct msg1) * ds.msg_qnum);
+ if (msgque->messages == NULL) {
+ printf("Failed to get stats for IPC queue\n");
+ return -ENOMEM;
+ }
+
+ msgque->qnum = ds.msg_qnum;
+ msgque->mode = ds.msg_perm.mode;
+ msgque->qbytes = ds.msg_qbytes;
+
+ for (i = 0; i < msgque->qnum; i++) {
+ ret = msgrcv(msgque->msq_id, &msgque->messages[i].mtype, MAX_MSG_SIZE, i, IPC_NOWAIT | MSG_COPY);
+ if (ret < 0) {
+ printf("Failed to copy IPC message: %m (%d)\n", errno);
+ return -errno;
+ }
+ msgque->messages[i].msize = ret;
+ }
+ return 0;
+}
+
+int fill_msgque(struct msgque_data *msgque)
+{
+ struct msg1 msgbuf;
+
+ msgbuf.mtype = MSG_TYPE;
+ memcpy(msgbuf.mtext, TEST_STRING, sizeof(TEST_STRING));
+ if (msgsnd(msgque->msq_id, &msgbuf.mtype, sizeof(TEST_STRING), IPC_NOWAIT) != 0) {
+ printf("First message send failed (%m)\n");
+ return -errno;
+ };
+
+ msgbuf.mtype = ANOTHER_MSG_TYPE;
+ memcpy(msgbuf.mtext, ANOTHER_TEST_STRING, sizeof(ANOTHER_TEST_STRING));
+ if (msgsnd(msgque->msq_id, &msgbuf.mtype, sizeof(ANOTHER_TEST_STRING), IPC_NOWAIT) != 0) {
+ printf("Second message send failed (%m)\n");
+ return -errno;
+ };
+ return 0;
+}
+
+int main (int argc, char **argv)
+{
+ key_t key;
+ int msg, pid, err;
+ struct msgque_data msgque;
+
+ key = ftok(argv[0], 822155650);
+ if (key == -1) {
+ printf("Can't make key\n");
+ return -errno;
+ }
+
+ msgque.msq_id = msgget(key, IPC_CREAT | IPC_EXCL | 0666);
+ if (msgque.msq_id == -1) {
+ printf("Can't create queue\n");
+ goto err_out;
+ }
+
+ err = fill_msgque(&msgque);
+ if (err) {
+ printf("Failed to fill queue\n");
+ goto err_destroy;
+ }
+
+ err = dump_queue(&msgque);
+ if (err) {
+ printf("Failed to dump queue\n");
+ goto err_destroy;
+ }
+
+ err = check_and_destroy_queue(&msgque);
+ if (err) {
+ printf("Failed to check and destroy queue\n");
+ goto err_out;
+ }
+
+ err = restore_queue(&msgque);
+ if (err) {
+ printf("Failed to restore queue\n");
+ goto err_destroy;
+ }
+
+ err = check_and_destroy_queue(&msgque);
+ if (err) {
+ printf("Failed to test queue\n");
+ goto err_out;
+ }
+ return 0;
+
+err_destroy:
+ if (msgctl(msgque.msq_id, IPC_RMID, 0)) {
+ printf("Failed to destroy queue: %d\n", -errno);
+ return -errno;
+ }
+err_out:
+ return err;
+}
^ permalink raw reply related
* [PATCH v7 09/10] IPC: message queue copy feature introduced
From: Stanislav Kinsbursky @ 2012-10-18 10:23 UTC (permalink / raw)
To: akpm
Cc: catalin.marinas, will.deacon, dhowells, manfred, hughd, jmorris,
mtk.manpages, kosaki.motohiro, paulmck, sds, devel, a.p.zijlstra,
cmetcalf, linux-driver, ron.mercer, viro, eparis, tglx,
jitendra.kalsaria, netdev, linux-kernel, linux-security-module,
ebiederm, casey
In-Reply-To: <20121018101543.16036.12221.stgit@localhost.localdomain>
This patch is required for checkpoint/restore in userspace.
IOW, c/r requires some way to get all pending IPC messages without deleting
them from the queue (checkpoint can fail and in this case tasks will be resumed,
so queue have to be valid).
To achive this, new operation flag MSG_COPY for sys_msgrcv() system call was
introduced. If this flag was specified, then mtype is interpreted as number of
the message to copy.
If MSG_COPY is set, then kernel will allocate dummy message with passed size,
and then use new copy_msg() helper function to copy desired message (instead of
unlinking it from the queue).
Notes:
1) Return -ENOSYS if MSG_COPY is specified, but CONFIG_CHECKPOINT_RESTORE is
not set.
Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
---
include/uapi/linux/msg.h | 1 +
ipc/msg.c | 50 ++++++++++++++++++++++++++++++++++++++++++++--
ipc/msgutil.c | 38 +++++++++++++++++++++++++++++++++++
ipc/util.h | 1 +
4 files changed, 88 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/msg.h b/include/uapi/linux/msg.h
index 76999c9..c1af84a 100644
--- a/include/uapi/linux/msg.h
+++ b/include/uapi/linux/msg.h
@@ -11,6 +11,7 @@
/* msgrcv options */
#define MSG_NOERROR 010000 /* no error if message is too big */
#define MSG_EXCEPT 020000 /* recv any msg except of specified type.*/
+#define MSG_COPY 040000 /* copy (not remove) all queue messages */
/* Obsolete, used only for backwards compatibility and libc5 compiles */
struct msqid_ds {
diff --git a/ipc/msg.c b/ipc/msg.c
index 028ab87..a908529 100644
--- a/ipc/msg.c
+++ b/ipc/msg.c
@@ -789,19 +789,48 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp,
struct msg_msg *msg;
int mode;
struct ipc_namespace *ns;
+#ifdef CONFIG_CHECKPOINT_RESTORE
+ struct msg_msg *copy = NULL;
+ unsigned long copy_number = 0;
+#endif
if (msqid < 0 || (long) bufsz < 0)
return -EINVAL;
+ if (msgflg & MSG_COPY) {
+#ifdef CONFIG_CHECKPOINT_RESTORE
+
+ if (msgflg & MSG_COPY) {
+ copy_number = msgtyp;
+ msgtyp = 0;
+ }
+
+ /*
+ * Create dummy message to copy real message to.
+ */
+ copy = load_msg(buf, bufsz);
+ if (IS_ERR(copy))
+ return PTR_ERR(copy);
+ copy->m_ts = bufsz;
+#else
+ return -ENOSYS;
+#endif
+ }
mode = convert_mode(&msgtyp, msgflg);
ns = current->nsproxy->ipc_ns;
msq = msg_lock_check(ns, msqid);
- if (IS_ERR(msq))
+ if (IS_ERR(msq)) {
+#ifdef CONFIG_CHECKPOINT_RESTORE
+ if (msgflg & MSG_COPY)
+ free_msg(copy);
+#endif
return PTR_ERR(msq);
+ }
for (;;) {
struct msg_receiver msr_d;
struct list_head *tmp;
+ long msg_counter = 0;
msg = ERR_PTR(-EACCES);
if (ipcperms(ns, &msq->q_perm, S_IRUGO))
@@ -821,8 +850,16 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp,
if (mode == SEARCH_LESSEQUAL &&
walk_msg->m_type != 1) {
msgtyp = walk_msg->m_type - 1;
+#ifdef CONFIG_CHECKPOINT_RESTORE
+ } else if (msgflg & MSG_COPY) {
+ if (copy_number == msg_counter) {
+ msg = copy_msg(walk_msg, copy);
+ break;
+ }
+#endif
} else
break;
+ msg_counter++;
}
tmp = tmp->next;
}
@@ -835,6 +872,10 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp,
msg = ERR_PTR(-E2BIG);
goto out_unlock;
}
+#ifdef CONFIG_CHECKPOINT_RESTORE
+ if (msgflg & MSG_COPY)
+ goto out_unlock;
+#endif
list_del(&msg->m_list);
msq->q_qnum--;
msq->q_rtime = get_seconds();
@@ -918,8 +959,13 @@ out_unlock:
break;
}
}
- if (IS_ERR(msg))
+ if (IS_ERR(msg)) {
+#ifdef CONFIG_CHECKPOINT_RESTORE
+ if (msgflg & MSG_COPY)
+ free_msg(copy);
+#endif
return PTR_ERR(msg);
+ }
bufsz = msg_handler(buf, msg, bufsz);
free_msg(msg);
diff --git a/ipc/msgutil.c b/ipc/msgutil.c
index 26143d3..b281f5c 100644
--- a/ipc/msgutil.c
+++ b/ipc/msgutil.c
@@ -100,7 +100,45 @@ out_err:
free_msg(msg);
return ERR_PTR(err);
}
+#ifdef CONFIG_CHECKPOINT_RESTORE
+struct msg_msg *copy_msg(struct msg_msg *src, struct msg_msg *dst)
+{
+ struct msg_msgseg *dst_pseg, *src_pseg;
+ int len = src->m_ts;
+ int alen;
+
+ BUG_ON(dst == NULL);
+ if (src->m_ts > dst->m_ts)
+ return ERR_PTR(-EINVAL);
+
+ alen = len;
+ if (alen > DATALEN_MSG)
+ alen = DATALEN_MSG;
+
+ dst->next = NULL;
+ dst->security = NULL;
+ memcpy(dst + 1, src + 1, alen);
+
+ len -= alen;
+ dst_pseg = dst->next;
+ src_pseg = src->next;
+ while (len > 0) {
+ alen = len;
+ if (alen > DATALEN_SEG)
+ alen = DATALEN_SEG;
+ memcpy(dst_pseg + 1, src_pseg + 1, alen);
+ dst_pseg = dst_pseg->next;
+ len -= alen;
+ src_pseg = src_pseg->next;
+ }
+
+ dst->m_type = src->m_type;
+ dst->m_ts = src->m_ts;
+
+ return dst;
+}
+#endif
int store_msg(void __user *dest, struct msg_msg *msg, int len)
{
int alen;
diff --git a/ipc/util.h b/ipc/util.h
index 271bded..027f507 100644
--- a/ipc/util.h
+++ b/ipc/util.h
@@ -142,6 +142,7 @@ int ipc_parse_version (int *cmd);
extern void free_msg(struct msg_msg *msg);
extern struct msg_msg *load_msg(const void __user *src, int len);
+extern struct msg_msg *copy_msg(struct msg_msg *src, struct msg_msg *dst);
extern int store_msg(void __user *dest, struct msg_msg *msg, int len);
extern void recompute_msgmni(struct ipc_namespace *);
^ permalink raw reply related
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