* Re: [PATCH] tcp: fix possible socket refcount problem for ipv6
From: Eric Dumazet @ 2012-09-05 20:57 UTC (permalink / raw)
To: Julian Anastasov; +Cc: David Miller, netdev
In-Reply-To: <1346878398-4782-1-git-send-email-ja@ssi.bg>
On Wed, 2012-09-05 at 23:53 +0300, Julian Anastasov wrote:
> commit 144d56e91044181ec0ef67aeca91e9a8b5718348
> ("tcp: fix possible socket refcount problem") is missing
> the IPv6 part. As tcp_release_cb is shared by both protocols
> we should hold sock reference for the TCP_MTU_REDUCED_DEFERRED
> bit.
>
> Signed-off-by: Julian Anastasov <ja@ssi.bg>
> ---
> net/ipv6/tcp_ipv6.c | 5 +++--
> 1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 09078b9..f3bfb8b 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -403,8 +403,9 @@ static void tcp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
> tp->mtu_info = ntohl(info);
> if (!sock_owned_by_user(sk))
> tcp_v6_mtu_reduced(sk);
> - else
> - set_bit(TCP_MTU_REDUCED_DEFERRED, &tp->tsq_flags);
> + else if (!test_and_set_bit(TCP_MTU_REDUCED_DEFERRED,
> + &tp->tsq_flags))
> + sock_hold(sk);
> goto out;
> }
>
Thanks !
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [REVIEW][PATCH 10/15] userns: Convert debugfs to use kuid/kgid where appropriate.
From: Greg Kroah-Hartman @ 2012-09-05 21:09 UTC (permalink / raw)
To: Eric W. Biederman
Cc: linux-kernel, netdev, linux-fsdevel, Serge E. Hallyn,
David Miller
In-Reply-To: <871uiufrj6.fsf@xmission.com>
On Sat, Aug 25, 2012 at 05:03:41PM -0700, Eric W. Biederman wrote:
>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply
* RE: Intel 82574L hang when sending short ethernet packets at 100BaseT
From: Dave, Tushar N @ 2012-09-05 21:12 UTC (permalink / raw)
To: Kelvie Wong
Cc: netdev@vger.kernel.org, e1000-devel@lists.sourceforge.net,
Kelvie Wong
In-Reply-To: <CAK2bC5oMnxerNYsgoD7WFJ6p84=BqaZLFdt+GdO91tF0gmyg2w@mail.gmail.com>
>-----Original Message-----
>From: kelvie@gmail.com [mailto:kelvie@gmail.com] On Behalf Of Kelvie Wong
>Sent: Wednesday, August 29, 2012 1:46 PM
>To: Dave, Tushar N
>Cc: netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net; Kelvie Wong
>Subject: Re: Intel 82574L hang when sending short ethernet packets at
>100BaseT
>
>On Wed, Aug 29, 2012 at 1:29 PM, Dave, Tushar N <tushar.n.dave@intel.com>
>wrote:
>> I'm already aware of this issue on PCI/PCI-X parts and e1000 SF driver
>does have this WA implemented. The patch for upstream e1000 driver is
>submitted too which will be accepted soon.
>> I will check with team and see if we need similar WA for PCIe part(s)!
>
>Ah, I see. Thanks for the clarification.
Thanks for your patience.
It is confirmed that minimum 17 bytes of packet size is a HW requirement for all of our PCI/PCI-X/PCIe parts.
Next release of all our drivers (e1000, e1000e , igb and ixgbe) will have this fixed.
-Tushar
^ permalink raw reply
* Re: [PATCH] tcp: fix possible socket refcount problem for ipv6
From: David Miller @ 2012-09-05 21:16 UTC (permalink / raw)
To: eric.dumazet; +Cc: ja, netdev
In-Reply-To: <1346878620.13121.161.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 05 Sep 2012 22:57:00 +0200
> On Wed, 2012-09-05 at 23:53 +0300, Julian Anastasov wrote:
>> commit 144d56e91044181ec0ef67aeca91e9a8b5718348
>> ("tcp: fix possible socket refcount problem") is missing
>> the IPv6 part. As tcp_release_cb is shared by both protocols
>> we should hold sock reference for the TCP_MTU_REDUCED_DEFERRED
>> bit.
>>
>> Signed-off-by: Julian Anastasov <ja@ssi.bg>
...
> Acked-by: Eric Dumazet <edumazet@google.com>
Applied, thanks everyone.
^ permalink raw reply
* Re: kernel BUG at kernel/timer.c:748!
From: Jerry Chu @ 2012-09-05 21:18 UTC (permalink / raw)
To: Lin Ming; +Cc: Dave Jones, netdev
In-Reply-To: <CAF1ivSauxzNhrm9c==_xFpuh9Lo3KrUNLNRb_62fLZxMfTuU1w@mail.gmail.com>
On Wed, Sep 5, 2012 at 9:04 AM, Lin Ming <mlin@ss.pku.edu.cn> wrote:
> On Wed, Sep 5, 2012 at 12:35 PM, Dave Jones <davej@redhat.com> wrote:
>> Just hit this bug on 3.6-rc4.
>>
>> The BUG is..
>>
>> BUG_ON(!timer->function);
>
> TCP keepalive timer is setup when the socket is created.
>
> __sock_create
> inet_create
> tcp_v4_init_sock
> tcp_init_sock
> tcp_init_xmit_timers
> inet_csk_init_xmit_timers
>
> timer->function should not be NULL when set keepalive option.
And tcp_init_xmit_timers() is called on the passive open side as well, v4
as well as v6. I don't see any code explicitly set timer->function back to NULL
(unless through set_timer(..., NULL,...). This may be a corrupted sock (already
released?)
Jerry
>
> Strange...have bug somewhere.
>
> Lin Ming
>
>>
>>
>> Not much to go on... Any thoughts on what I could add to get
>> more debug info on which protocol etc this was ?
>>
>> Dave
>>
>>
>> kernel BUG at kernel/timer.c:748!
>> invalid opcode: 0000 [#1] SMP
>> Modules linked in: tun fuse ipt_ULOG binfmt_misc nfnetlink nfc caif_socket caif phonet can llc2 pppoe pppox ppp_generic slhc irda crc_ccitt rds af_key decnet rose x25 atm netrom appletalk ipx p8023 psnap p8022 llc ax25 nfsv3 nfs_acl nfs fscache lockd sunrpc bluetooth rfkill ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 ip6table_filter ip6_tables nf_conntrack_ipv4 nf_defrag_ipv4 xt_state nf_conntrack kvm_intel kvm crc32c_intel ghash_clmulni_intel microcode pcspkr i2c_i801 e1000e uinput i915 video i2c_algo_bit drm_kms_helper drm i2c_core
>> CPU 3
>> Pid: 12330, comm: trinity-child3 Not tainted 3.6.0-rc4+ #36
>> RIP: 0010:[<ffffffff810813f5>] [<ffffffff810813f5>] mod_timer+0x2c5/0x2f0
>> RSP: 0018:ffff88000dfd7e08 EFLAGS: 00010246
>> RAX: 000000000000001a RBX: ffff880122d62948 RCX: 000000000000001a
>> RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff88000dfd7e10
>> RBP: ffff88000dfd7e48 R08: 0000000000000000 R09: 0000000000000000
>> R10: 0000000001517000 R11: 0000000000000246 R12: 000000016c000000
>> R13: 000000016c12bcb1 R14: ffff8801236cee00 R15: 00000000ffffff01
>> FS: 00007fa96745f740(0000) GS:ffff880148200000(0000) knlGS:0000000000000000
>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> CR2: 00000000100ff000 CR3: 0000000099344000 CR4: 00000000001407e0
>> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
>> Process trinity-child3 (pid: 12330, threadinfo ffff88000dfd6000, task ffff880090890000)
>> Stack:
>> ffffffff8154cb6d 0000000007b5edf7 ffff88000dfd7e28 ffff880122d62520
>> 0000000000000009 0000000000000004 ffff8801236cee00 00000000ffffff01
>> ffff88000dfd7e68 ffffffff8154c79c ffffffff81550e6c ffff880122d62520
>> Call Trace:
>> [<ffffffff8154cb6d>] ? lock_sock_nested+0x8d/0xa0
>> [<ffffffff8154c79c>] sk_reset_timer+0x1c/0x30
>> [<ffffffff81550e6c>] ? sock_setsockopt+0x8c/0x960
>> [<ffffffff815a84a0>] inet_csk_reset_keepalive_timer+0x20/0x30
>> [<ffffffff815c018d>] tcp_set_keepalive+0x3d/0x50
>> [<ffffffff81551703>] sock_setsockopt+0x923/0x960
>> [<ffffffff810ddf76>] ? trace_hardirqs_on_caller+0x16/0x1e0
>> [<ffffffff811db0ac>] ? fget_light+0x24c/0x520
>> [<ffffffff8154af86>] sys_setsockopt+0xc6/0xe0
>> [<ffffffff816a50ed>] system_call_fastpath+0x1a/0x1f
>> Code: 00 74 43 9c 58 0f 1f 44 00 00 f6 c4 02 0f 84 14 ff ff ff eb 93 48 c7 c7 20 48 c3 81 e8 f5 70 05 00 85 c0 0f 85 fe fe ff ff eb b7 <0f> 0b 48 8b 75 08 48 89 df e8 3d f6 ff ff e9 b2 fd ff ff 4d 89
>> RIP [<ffffffff810813f5>] mod_timer+0x2c5/0x2f0
>> RSP <ffff88000dfd7e08>
>> ---[ end trace 7e7b5910138e49a3 ]---
>>
>> --
>> 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 04/10] net/macb: Fix a race in macb_start_xmit()
From: David Miller @ 2012-09-05 21:30 UTC (permalink / raw)
To: nicolas.ferre
Cc: netdev, patrice.vilchez, linux-kernel, havard, jamie, plagnioj,
linux-arm-kernel
In-Reply-To: <40f02ee50a29aaec6c949432a1bcf09f4b027181.1346775479.git.nicolas.ferre@atmel.com>
From: Nicolas Ferre <nicolas.ferre@atmel.com>
Date: Wed, 5 Sep 2012 10:19:11 +0200
> From: Havard Skinnemoen <havard@skinnemoen.net>
>
> Fix a race in macb_start_xmit() where we unconditionally set the TSTART bit.
> If an underrun just happened (we do this with interrupts disabled, so it might
> not have been handled yet), the controller starts transmitting from the first
> entry in the ring, which is usually wrong.
> Restart the controller after error handling.
>
> Signed-off-by: Havard Skinnemoen <havard@skinnemoen.net>
> [nicolas.ferre@atmel.com: split patch in topics]
> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Accumulating special case code and checks into the hot path of TX packet
processing is extremely unwise.
Instead, when you handle the TX error conditions and reset the chip you
should first ensure that there are no flows of control in the transmit
function of your driver by using the appropriate locking et al. facilities.
For example, you can quiesce the transmit path by handling the chip error
interrupt as follows:
1) Disable chip interrupt generation.
2) Schedule a workqueue so you can process the reset outside of hard
interrupt context.
3) In the workqueue function, disable NAPI and perform a
netif_tx_disable() to guarentee there are no threads of
execution trying to queue up packets for TX into the driver.
4) Perform your chip reset and whatever else is necessary.
5) Re-enable NAPI and TX.
Then you don't need any special checks in your xmit method at all.
^ permalink raw reply
* Re: [PATCH 08/10] net/macb: macb_get_drvinfo: add GEM/MACB suffix to differentiate revision
From: David Miller @ 2012-09-05 21:31 UTC (permalink / raw)
To: nicolas.ferre
Cc: netdev, linux-arm-kernel, havard, plagnioj, jamie, linux-kernel,
patrice.vilchez
In-Reply-To: <fc701a2ed18198b7a671c55f1e65725f1709509c.1346775479.git.nicolas.ferre@atmel.com>
From: Nicolas Ferre <nicolas.ferre@atmel.com>
Date: Wed, 5 Sep 2012 11:00:53 +0200
> strcpy(info->driver, bp->pdev->dev.driver->name);
> + if (macb_is_gem(bp))
> + strcat(info->driver, " GEM");
> + else
> + strcat(info->driver, " MACB");
This is a driver string, which means the software driver, and has
absolutely nothing to do with the exact type of the underlying
physical hardware.
Therefore the these suffixes should be left out of the driver string.
^ permalink raw reply
* Re: [PATCH 09/10] net/macb: ethtool interface: add register dump feature
From: David Miller @ 2012-09-05 21:32 UTC (permalink / raw)
To: nicolas.ferre
Cc: netdev, linux-arm-kernel, havard, plagnioj, jamie, linux-kernel,
patrice.vilchez
In-Reply-To: <fcd1c2443a30f23ffcd8de28328c5a382e2a2459.1346775479.git.nicolas.ferre@atmel.com>
From: Nicolas Ferre <nicolas.ferre@atmel.com>
Date: Wed, 5 Sep 2012 11:00:54 +0200
> @@ -10,6 +10,9 @@
> #ifndef _MACB_H
> #define _MACB_H
>
> +
> +#define MACB_GREGS_LEN 32
Please don't add such extraneous empty lines. One empty line between
constructs is more than enough, and anything more is visually awkward.
Thanks.
^ permalink raw reply
* Re: [PATCH] net: add unknown state to sysfs NIC duplex export
From: David Miller @ 2012-09-05 21:40 UTC (permalink / raw)
To: naleksan; +Cc: netdev
In-Reply-To: <1346854288-6380-1-git-send-email-naleksan@redhat.com>
From: Nikolay Aleksandrov <naleksan@redhat.com>
Date: Wed, 5 Sep 2012 16:11:28 +0200
> Currently when the NIC duplex state is DUPLEX_UNKNOWN it is exported as
> full through sysfs, this patch adds support for DUPLEX_UNKNOWN. It is
> handled the same way as in ethtool.
>
> Signed-off-by: Nikolay Aleksandrov <naleksan@redhat.com>
Applied with two minor corrections:
> + char *duplex;
I made this "const char *"
> + switch(cmd.duplex) {
I added the missing space between 'switch' and '(cmd.duplex)'.
Thanks.
^ permalink raw reply
* Re: Commit "ipconfig wait for carrier" makes boot hang for 2 mins if no carrier
From: Joakim Tjernlund @ 2012-09-05 21:41 UTC (permalink / raw)
To: David Miller; +Cc: micha, netdev
In-Reply-To: <20120905.130158.897788930198410991.davem@davemloft.net>
David Miller <davem@davemloft.net> wrote on 2012/09/05 19:01:58:
>
> From: Joakim Tjernlund <joakim.tjernlund@transmode.se>
> Date: Wed, 5 Sep 2012 10:47:56 +0200
>
> > Please adjust the 2 min wait to nfsroot= only and keep the old way for ip=
>
> Sorry, this is not happening, I fully support the change in the tree
> as-is and will not modify it in the way you request.
>
> You'll have to accomodate this situation in some other local way.
I have fixed it locally, but I would like to know why you support the current way.
It adds a incredibly long delay for systems that doesn't need a carrier, they
just want an IP address defined for a particular I/F. I cannot see why
this change would be beneficial for all.
Jocke
^ permalink raw reply
* Re: [PATCH V2 0/2] Remove duplicate register definitions in Chelsio cxgb4
From: David Miller @ 2012-09-05 21:43 UTC (permalink / raw)
To: vipul-ut6Up61K2wZBDgjK7y7TUQ
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
roland-BHEL68pLQRGGvPXPguhicg, divy-ut6Up61K2wZBDgjK7y7TUQ,
dm-ut6Up61K2wZBDgjK7y7TUQ, swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW,
kumaras-ut6Up61K2wZBDgjK7y7TUQ, santosh-ut6Up61K2wZBDgjK7y7TUQ,
sivasu-ut6Up61K2wZBDgjK7y7TUQ
In-Reply-To: <1346846515-25997-1-git-send-email-vipul-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
From: Vipul Pandya <vipul-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
Date: Wed, 5 Sep 2012 17:31:53 +0530
> This patch series has minor changes in cxgb4 driver. It removes the duplicate
> definitions of the registers in cxgb4 driver. It also has a minor update in
> RDMA/cxgb4 driver due to change in cxgb4 driver.
>
> This patch series is built against Stephen Rothwell's linux-next tree. We
> request to merge patch series thorough single tree to avoid build failures.
Applied to net-next, thanks.
--
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] ibmveth: Fix alignment of rx queue bug
From: David Miller @ 2012-09-05 21:45 UTC (permalink / raw)
To: santil; +Cc: netdev
In-Reply-To: <50469FC1.2000202@linux.vnet.ibm.com>
From: Santiago Leon <santil@linux.vnet.ibm.com>
Date: Tue, 04 Sep 2012 19:41:37 -0500
> This patch fixes a bug found by Nish Aravamudan
> (https://lkml.org/lkml/2012/5/15/220) where the driver is not following
> the spec (it is not aligning the rx buffer on a 16-byte boundary) and the
> hypervisor aborts the registration, making the device unusable.
>
> The fix follows BenH's recommendation (https://lkml.org/lkml/2012/7/20/461)
> to replace the kmalloc+map for a single call to dma_alloc_coherent()
> because that function always aligns to a 16-byte boundary.
>
> The stable trees will run into this bug whenever the rx buffer kmalloc call
> returns something not aligned on a 16-byte boundary.
>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Santiago Leon <santil@linux.vnet.ibm.com>
Applied, thanks.
^ permalink raw reply
* Re: linux-next: build failure after merge of the final tree (net-next tree related)
From: David Miller @ 2012-09-05 21:46 UTC (permalink / raw)
To: sfr; +Cc: netdev, linux-next, linux-kernel, kaber
In-Reply-To: <20120905153932.eebf343364800678aecc98f2@canb.auug.org.au>
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Wed, 5 Sep 2012 15:39:32 +1000
> (I always thought IPv6 NAT was a bad idea ;-))
ROFL :)
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Wed, 5 Sep 2012 15:34:58 +1000
> Subject: [PATCH] netfilter: ipv6: using csum_ipv6_magic requires
> net/ip6_checksum.h
>
> Fixes this build error:
>
> net/ipv6/netfilter/nf_nat_l3proto_ipv6.c: In function 'nf_nat_ipv6_csum_recalc':
> net/ipv6/netfilter/nf_nat_l3proto_ipv6.c:144:4: error: implicit declaration of function 'csum_ipv6_magic' [-Werror=implicit-function-declaration]
>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
Applied, thanks Stephen.
^ permalink raw reply
* Re: [PATCH] bnx2x: use list_move_tail instead of list_del/list_add_tail
From: David Miller @ 2012-09-05 21:50 UTC (permalink / raw)
To: weiyj.lk; +Cc: eilong, yongjun_wei, netdev
In-Reply-To: <CAPgLHd-P-x3ZqrUMirMBbx74vz1XXNLb-nb3UWa43nVmmwNMCg@mail.gmail.com>
From: Wei Yongjun <weiyj.lk@gmail.com>
Date: Wed, 5 Sep 2012 15:06:55 +0800
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>
> Using list_move_tail() instead of list_del() + list_add_tail().
>
> spatch with a semantic match is used to found this problem.
> (http://coccinelle.lip6.fr/)
>
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Applied.
^ permalink raw reply
* Re: [RFC PATCH v3] ipv6: fix handling of blackhole and prohibit routes
From: David Miller @ 2012-09-05 21:50 UTC (permalink / raw)
To: nicolas.dichtel; +Cc: netdev
In-Reply-To: <1346847162-3558-1-git-send-email-nicolas.dichtel@6wind.com>
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Wed, 5 Sep 2012 08:12:42 -0400
> When adding a blackhole or a prohibit route, they were handling like classic
> routes. Moreover, it was only possible to add this kind of routes by specifying
> an interface.
>
> Bug already reported here:
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=498498
>
> Before the patch:
> $ ip route add blackhole 2001::1/128
> RTNETLINK answers: No such device
> $ ip route add blackhole 2001::1/128 dev eth0
> $ ip -6 route | grep 2001
> 2001::1 dev eth0 metric 1024
>
> After:
> $ ip route add blackhole 2001::1/128
> $ ip -6 route | grep 2001
> blackhole 2001::1 dev lo metric 1024 error -22
>
> v2: wrong patch
> v3: add a field fc_type in struct fib6_config to store RTN_* type
>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
I like this a lot more than your original patch, applied to net-next,
thanks.
^ permalink raw reply
* Re: [PATCH net-next] net: qdisc busylock needs lockdep annotations
From: David Miller @ 2012-09-05 21:51 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1346842976.13121.143.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 05 Sep 2012 13:02:56 +0200
> From: Eric Dumazet <edumazet@google.com>
>
> It seems we need to provide ability for stacked devices
> to use specific lock_class_key for sch->busylock
>
> We could instead default l2tpeth tx_queue_len to 0 (no qdisc), but
> a user might use a qdisc anyway.
>
> (So same fixes are probably needed on non LLTX stacked drivers)
>
> Noticed while stressing L2TPV3 setup :
...
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied, thanks Eric.
^ permalink raw reply
* Re: [net-next PATCH] be2net: use PCIe AER capability
From: David Miller @ 2012-09-05 21:51 UTC (permalink / raw)
To: sathya.perla; +Cc: netdev
In-Reply-To: <2fad6337-6291-43f5-af76-e8ba8a9a5296@exht1.ad.emulex.com>
From: Sathya Perla <sathya.perla@emulex.com>
Date: Wed, 5 Sep 2012 17:26:48 +0530
> This patch allows code to handle the PCIe AER capability.
> The PCI callbacks for error handling/reset/recovery already exist in be2net
> and have been tested with EEH/ppc.
> This patch has been tested using the aer-inject tool.
>
> Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
Applied.
^ permalink raw reply
* Re: [PATCH] usbnet: drop unneeded check for NULL
From: Ben Hutchings @ 2012-09-05 22:14 UTC (permalink / raw)
To: David Miller; +Cc: oneukum, richardcochran, netdev, Alexander Duyck
In-Reply-To: <20120905.125016.2259674613767082102.davem@davemloft.net>
On Wed, 2012-09-05 at 12:50 -0400, David Miller wrote:
> From: Oliver Neukum <oneukum@suse.de>
> Date: Wed, 05 Sep 2012 08:24:25 +0200
>
> > On Wednesday 05 September 2012 06:47:12 Richard Cochran wrote:
> >> and so I think the problem that the test addresses is still present,
> >> or am I missing something?
> >
> > No,
> >
> > you are right. Thank you.
> >
> > Dave, for now, please don't apply this patch. In the long run, this crap
> > in cdc-ncm needs to go. I am starting rewriting this driver right now.
>
> I already applied it several days ago, someone send me a revert with a
> verbose commit message explaining the situation.
cdc-ncm is aggregating skbs into jumbo USB packets and then, because the
netdev is not signalled when the qdisc stops transmitting, flushing them
after one scheduler tick. Flushing is done by calling
usbnet_start_xmit() with a null skb pointer and then substituting a real
skb in the tx_fixup callback.
Perhaps the skb_tx_timestamp() call should be moved below the
'if (info->tx_fixup)' block, at which point skb is definitely non-null.
It doesn't look like cdc-ncm will provide useful timestamps either way.
cdc-ncm's aggregation could be improved by either (1) implementing some
type of GSO with appropriate gso_max_size and gso_max_segs limits (2)
adding an explicit transmit flushing operation, similar to that
Alexander Duyck proposed.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
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: sctp_close/sk_free: kernel BUG at arch/x86/mm/physaddr.c:18!
From: Fengguang Wu @ 2012-09-05 22:28 UTC (permalink / raw)
To: Eric Dumazet
Cc: Marc Kleine-Budde, H.K. Jerry Chu, Eric W. Biederman, networking,
linux-can
In-Reply-To: <1346864220.13121.157.camel@edumazet-glaptop>
On Wed, Sep 05, 2012 at 06:57:00PM +0200, Eric Dumazet wrote:
> On Wed, 2012-09-05 at 17:40 +0200, Eric Dumazet wrote:
>
> > Could you test the following patch please ?
It works - no single error for 1000 boots!
btw, the first bad commit has been bisected to
commit 8336886f786fdacbc19b719c1f7ea91eb70706d4
Author: Jerry Chu <hkchu@google.com>
Date: Fri Aug 31 12:29:12 2012 +0000
tcp: TCP Fast Open Server - support TFO listeners
> > (Not sure why sctp doesnt memset/bzero its whole socket by the way...)
> >
> > Thanks
>
> Here is a more complete patch, as there are three potential problems,
> not only one :
Great! I'll start tests for it.
Thanks,
Fengguang
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 4f70ef0..845372b 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -149,11 +149,8 @@ void inet_sock_destruct(struct sock *sk)
> pr_err("Attempt to release alive inet socket %p\n", sk);
> return;
> }
> - if (sk->sk_type == SOCK_STREAM) {
> - struct fastopen_queue *fastopenq =
> - inet_csk(sk)->icsk_accept_queue.fastopenq;
> - kfree(fastopenq);
> - }
> + if (sk->sk_protocol == IPPROTO_TCP)
> + kfree(inet_csk(sk)->icsk_accept_queue.fastopenq);
>
> WARN_ON(atomic_read(&sk->sk_rmem_alloc));
> WARN_ON(atomic_read(&sk->sk_wmem_alloc));
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index 8464b79..f0c5b9c 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -314,7 +314,7 @@ struct sock *inet_csk_accept(struct sock *sk, int flags, int *err)
> newsk = req->sk;
>
> sk_acceptq_removed(sk);
> - if (sk->sk_type == SOCK_STREAM && queue->fastopenq != NULL) {
> + if (sk->sk_protocol == IPPROTO_TCP && queue->fastopenq != NULL) {
> spin_lock_bh(&queue->fastopenq->lock);
> if (tcp_rsk(req)->listener) {
> /* We are still waiting for the final ACK from 3WHS
> @@ -775,7 +775,7 @@ void inet_csk_listen_stop(struct sock *sk)
>
> percpu_counter_inc(sk->sk_prot->orphan_count);
>
> - if (sk->sk_type == SOCK_STREAM && tcp_rsk(req)->listener) {
> + if (sk->sk_protocol == IPPROTO_TCP && tcp_rsk(req)->listener) {
> BUG_ON(tcp_sk(child)->fastopen_rsk != req);
> BUG_ON(sk != tcp_rsk(req)->listener);
>
>
^ permalink raw reply
* Re: [PATCH] usbnet: drop unneeded check for NULL
From: Oliver Neukum @ 2012-09-05 22:34 UTC (permalink / raw)
To: Ben Hutchings; +Cc: David Miller, richardcochran, netdev, Alexander Duyck
In-Reply-To: <1346883275.5325.28.camel@bwh-desktop.uk.solarflarecom.com>
On Wednesday 05 September 2012 23:14:35 Ben Hutchings wrote:
> cdc-ncm is aggregating skbs into jumbo USB packets and then, because the
> netdev is not signalled when the qdisc stops transmitting, flushing them
> after one scheduler tick. Flushing is done by calling
> usbnet_start_xmit() with a null skb pointer and then substituting a real
> skb in the tx_fixup callback.
>
> Perhaps the skb_tx_timestamp() call should be moved below the
> 'if (info->tx_fixup)' block, at which point skb is definitely non-null.
> It doesn't look like cdc-ncm will provide useful timestamps either way.
>
> cdc-ncm's aggregation could be improved by either (1) implementing some
> type of GSO with appropriate gso_max_size and gso_max_segs limits (2)
> adding an explicit transmit flushing operation, similar to that
> Alexander Duyck proposed.
Actually, I thought about changing it. This is the current version. What do you think?
It lacks a bit of logic in the completion handler still.
Regards
Oliver
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 4cd582a..56ef743 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -135,9 +135,6 @@ struct cdc_ncm_ctx {
u16 connected;
};
-static void cdc_ncm_txpath_bh(unsigned long param);
-static void cdc_ncm_tx_timeout_start(struct cdc_ncm_ctx *ctx);
-static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *hr_timer);
static struct usb_driver cdc_ncm_driver;
static void
@@ -464,10 +461,6 @@ static int cdc_ncm_bind(struct usbnet *dev, struct usb_interface *intf)
if (ctx == NULL)
return -ENODEV;
- hrtimer_init(&ctx->tx_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
- ctx->tx_timer.function = &cdc_ncm_tx_timer_cb;
- ctx->bh.data = (unsigned long)ctx;
- ctx->bh.func = cdc_ncm_txpath_bh;
atomic_set(&ctx->stop, 0);
spin_lock_init(&ctx->mtx);
ctx->netdev = dev->net;
@@ -650,7 +643,7 @@ static void cdc_ncm_zero_fill(u8 *ptr, u32 first, u32 end, u32 max)
memset(ptr + first, 0, end - first);
}
-static struct sk_buff *
+static int
cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
{
struct sk_buff *skb_out;
@@ -659,12 +652,7 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
u32 last_offset;
u16 n = 0, index;
u8 ready2send = 0;
-
- /* if there is a remaining skb, it gets priority */
- if (skb != NULL)
- swap(skb, ctx->tx_rem_skb);
- else
- ready2send = 1;
+ u8 error = 0;
/*
* +----------------+
@@ -690,7 +678,7 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
dev_kfree_skb_any(skb);
ctx->netdev->stats.tx_dropped++;
}
- goto exit_no_skb;
+ return -EBUSY;
}
/* make room for NTH and NDP */
@@ -719,28 +707,15 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
/* compute maximum buffer size */
rem = ctx->tx_max - offset;
- if (skb == NULL) {
- skb = ctx->tx_rem_skb;
- ctx->tx_rem_skb = NULL;
-
- /* check for end of skb */
- if (skb == NULL)
- break;
- }
-
if (skb->len > rem) {
if (n == 0) {
/* won't fit, MTU problem? */
dev_kfree_skb_any(skb);
skb = NULL;
ctx->netdev->stats.tx_dropped++;
+ error = 1;
} else {
- /* no room for skb - store for later */
- if (ctx->tx_rem_skb != NULL) {
- dev_kfree_skb_any(ctx->tx_rem_skb);
- ctx->netdev->stats.tx_dropped++;
- }
- ctx->tx_rem_skb = skb;
+
skb = NULL;
ready2send = 1;
}
@@ -768,13 +743,6 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
skb = NULL;
}
- /* free up any dangling skb */
- if (skb != NULL) {
- dev_kfree_skb_any(skb);
- skb = NULL;
- ctx->netdev->stats.tx_dropped++;
- }
-
ctx->tx_curr_frame_num = n;
if (n == 0) {
@@ -791,9 +759,7 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
ctx->tx_curr_skb = skb_out;
ctx->tx_curr_offset = offset;
ctx->tx_curr_last_offset = last_offset;
- /* set the pending count */
- if (n < CDC_NCM_RESTART_TIMER_DATAGRAM_CNT)
- ctx->tx_timer_pending = CDC_NCM_TIMER_PENDING_CNT;
+
goto exit_no_skb;
} else {
@@ -874,71 +840,37 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
/* return skb */
ctx->tx_curr_skb = NULL;
ctx->netdev->stats.tx_packets += ctx->tx_curr_frame_num;
- return skb_out;
-exit_no_skb:
- /* Start timer, if there is a remaining skb */
- if (ctx->tx_curr_skb != NULL)
- cdc_ncm_tx_timeout_start(ctx);
- return NULL;
-}
-
-static void cdc_ncm_tx_timeout_start(struct cdc_ncm_ctx *ctx)
-{
- /* start timer, if not already started */
- if (!(hrtimer_active(&ctx->tx_timer) || atomic_read(&ctx->stop)))
- hrtimer_start(&ctx->tx_timer,
- ktime_set(0, CDC_NCM_TIMER_INTERVAL),
- HRTIMER_MODE_REL);
-}
+ if (error)
+ return -EBUSY;
+ if (ready2send)
+ return -EBUSY;
+ else
+ return 0;
-static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *timer)
-{
- struct cdc_ncm_ctx *ctx =
- container_of(timer, struct cdc_ncm_ctx, tx_timer);
+exit_no_skb:
- if (!atomic_read(&ctx->stop))
- tasklet_schedule(&ctx->bh);
- return HRTIMER_NORESTART;
+ return -EAGAIN;
}
-static void cdc_ncm_txpath_bh(unsigned long param)
+static int cdc_ncm_tx_bundle(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
{
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)param;
-
- spin_lock_bh(&ctx->mtx);
- if (ctx->tx_timer_pending != 0) {
- ctx->tx_timer_pending--;
- cdc_ncm_tx_timeout_start(ctx);
- spin_unlock_bh(&ctx->mtx);
- } else if (ctx->netdev != NULL) {
- spin_unlock_bh(&ctx->mtx);
- netif_tx_lock_bh(ctx->netdev);
- usbnet_start_xmit(NULL, ctx->netdev);
- netif_tx_unlock_bh(ctx->netdev);
- }
+ int err;
+ struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+
+ err = cdc_ncm_fill_tx_frame(ctx, skb);
+ return err;
}
static struct sk_buff *
cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
{
- struct sk_buff *skb_out;
struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
- /*
- * The Ethernet API we are using does not support transmitting
- * multiple Ethernet frames in a single call. This driver will
- * accumulate multiple Ethernet frames and send out a larger
- * USB frame when the USB buffer is full or when a single jiffies
- * timeout happens.
- */
if (ctx == NULL)
goto error;
- spin_lock_bh(&ctx->mtx);
- skb_out = cdc_ncm_fill_tx_frame(ctx, skb);
- spin_unlock_bh(&ctx->mtx);
- return skb_out;
+ return ctx->tx_curr_skb;
error:
if (skb != NULL)
@@ -1197,6 +1129,7 @@ static const struct driver_info cdc_ncm_info = {
.manage_power = cdc_ncm_manage_power,
.status = cdc_ncm_status,
.rx_fixup = cdc_ncm_rx_fixup,
+ .tx_bundle = cdc_ncm_tx_bundle,
.tx_fixup = cdc_ncm_tx_fixup,
};
@@ -1211,6 +1144,7 @@ static const struct driver_info wwan_info = {
.manage_power = cdc_ncm_manage_power,
.status = cdc_ncm_status,
.rx_fixup = cdc_ncm_rx_fixup,
+ .tx_bundle = cdc_ncm_tx_bundle,
.tx_fixup = cdc_ncm_tx_fixup,
};
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 8531c1c..d9a595e 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1024,6 +1024,7 @@ static void tx_complete (struct urb *urb)
struct skb_data *entry = (struct skb_data *) skb->cb;
struct usbnet *dev = entry->dev;
+ atomic_dec(&dev->tx_in_flight);
if (urb->status == 0) {
if (!(dev->driver_info->flags & FLAG_MULTI_PACKET))
dev->net->stats.tx_packets++;
@@ -1089,23 +1090,50 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
struct urb *urb = NULL;
struct skb_data *entry;
struct driver_info *info = dev->driver_info;
+ struct sk_buff *skb_old = NULL;
unsigned long flags;
int retval;
+ int transmit_now = 1;
+ int bundle_again = 0;
if (skb)
skb_tx_timestamp(skb);
+ /*
+ * first we allow drivers to bundle packets together
+ * maintainance of the buffer is the responsibility
+ * of the lower layer
+ */
+rebundle:
+ if (info->tx_bundle) {
+ bundle_again = 0;
+ retval = info->tx_bundle(dev, skb, GFP_ATOMIC);
+
+ switch (retval) {
+ case 0: /* the package has been bundled */
+ if (atomic_read(&dev->tx_in_flight) < 2)
+ transmit_now = 1;
+ else
+ transmit_now = 0;
+ break;
+ case -EAGAIN:
+ transmit_now = 1;
+ bundle_again = 1;
+ skb_old = skb;
+ break;
+ case -EBUSY:
+ transmit_now = 1;
+ break;
+ }
+ }
// some devices want funky USB-level framing, for
// win32 driver (usually) and/or hardware quirks
- if (info->tx_fixup) {
+ if (transmit_now && info->tx_fixup) {
skb = info->tx_fixup (dev, skb, GFP_ATOMIC);
if (!skb) {
if (netif_msg_tx_err(dev)) {
netif_dbg(dev, tx_err, dev->net, "can't tx_fixup skb\n");
goto drop;
- } else {
- /* cdc_ncm collected packet; waits for more */
- goto not_drop;
}
}
}
@@ -1164,14 +1192,17 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
}
#endif
+ atomic_inc(&dev->tx_in_flight);
switch ((retval = usb_submit_urb (urb, GFP_ATOMIC))) {
case -EPIPE:
netif_stop_queue (net);
usbnet_defer_kevent (dev, EVENT_TX_HALT);
+ atomic_dec(&dev->tx_in_flight);
usb_autopm_put_interface_async(dev->intf);
break;
default:
usb_autopm_put_interface_async(dev->intf);
+ atomic_dec(&dev->tx_in_flight);
netif_dbg(dev, tx_err, dev->net,
"tx: submit urb err %d\n", retval);
break;
@@ -1187,7 +1218,6 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
netif_dbg(dev, tx_err, dev->net, "drop, code %d\n", retval);
drop:
dev->net->stats.tx_dropped++;
-not_drop:
if (skb)
dev_kfree_skb_any (skb);
usb_free_urb (urb);
@@ -1197,6 +1227,10 @@ not_drop:
#ifdef CONFIG_PM
deferred:
#endif
+ if (bundle_again) {
+ skb = skb_old;
+ goto rebundle;
+ }
return NETDEV_TX_OK;
}
EXPORT_SYMBOL_GPL(usbnet_start_xmit);
@@ -1393,6 +1427,7 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
dev->delay.data = (unsigned long) dev;
init_timer (&dev->delay);
mutex_init (&dev->phy_mutex);
+ atomic_set(&dev->tx_in_flight, 0);
dev->net = net;
strcpy (net->name, "usb%d");
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index f87cf62..bb2f622 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -33,6 +33,7 @@ struct usbnet {
wait_queue_head_t *wait;
struct mutex phy_mutex;
unsigned char suspend_count;
+ atomic_t tx_in_flight;
/* i/o info: pipes etc */
unsigned in, out;
@@ -133,6 +134,12 @@ struct driver_info {
/* fixup rx packet (strip framing) */
int (*rx_fixup)(struct usbnet *dev, struct sk_buff *skb);
+ /* bundle individual package for transmission as
+ * larger package. This cannot sleep
+ */
+ int (*tx_bundle)(struct usbnet *dev,
+ struct sk_buff *skb, gfp_t flags);
+
/* fixup tx packet (add framing) */
struct sk_buff *(*tx_fixup)(struct usbnet *dev,
struct sk_buff *skb, gfp_t flags);
^ permalink raw reply related
* Re: [PATCH] usbnet: drop unneeded check for NULL
From: Ben Hutchings @ 2012-09-05 22:53 UTC (permalink / raw)
To: Oliver Neukum; +Cc: David Miller, richardcochran, netdev, Alexander Duyck
In-Reply-To: <2059099.RnKXUv0tR1@linux-lqwf.site>
On Thu, 2012-09-06 at 00:34 +0200, Oliver Neukum wrote:
> On Wednesday 05 September 2012 23:14:35 Ben Hutchings wrote:
> > cdc-ncm is aggregating skbs into jumbo USB packets and then, because the
> > netdev is not signalled when the qdisc stops transmitting, flushing them
> > after one scheduler tick. Flushing is done by calling
> > usbnet_start_xmit() with a null skb pointer and then substituting a real
> > skb in the tx_fixup callback.
> >
> > Perhaps the skb_tx_timestamp() call should be moved below the
> > 'if (info->tx_fixup)' block, at which point skb is definitely non-null.
> > It doesn't look like cdc-ncm will provide useful timestamps either way.
> >
> > cdc-ncm's aggregation could be improved by either (1) implementing some
> > type of GSO with appropriate gso_max_size and gso_max_segs limits (2)
> > adding an explicit transmit flushing operation, similar to that
> > Alexander Duyck proposed.
>
> Actually, I thought about changing it. This is the current version. What do you think?
> It lacks a bit of logic in the completion handler still.
[...]
I'm not familiar with the USB net framework (or USB in general), so I
don't know whether this would work. But it looks like you're trying to
use the TX completions to trigger flushing of the aggregated USB packet,
and flushing immediately if there are no packets currently outstanding.
That seems like a reasonable approach.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
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: [V2 PATCH 2/9] csiostor: Chelsio FCoE offload driver submission (sources part 2).
From: Stephen Hemminger @ 2012-09-05 22:59 UTC (permalink / raw)
To: Naresh Kumar Inna
Cc: JBottomley@parallels.com, linux-scsi@vger.kernel.org,
Dimitrios Michailidis, Casey Leedom, netdev@vger.kernel.org,
Chethan Seshadri
In-Reply-To: <50478F52.208@chelsio.com>
On Wed, 5 Sep 2012 23:13:46 +0530
Naresh Kumar Inna <naresh@chelsio.com> wrote:
> On 9/5/2012 9:59 PM, Stephen Hemminger wrote:
> > On Wed, 5 Sep 2012 18:03:55 +0530
> > Naresh Kumar Inna <naresh@chelsio.com> wrote:
> >
> >> This patch contains code for driver initialization, driver resource
> >> allocation and the Work Request module functionality. Driver initialization
> >> includes module entry/exit points, registration with PCI, FC transport and
> >> SCSI mid layer subsystems. The Work Request module provides services for
> >> allocation of DMA queues, posting Work Requests on them and processing
> >> completions.
> >>
> >> Signed-off-by: Naresh Kumar Inna <naresh@chelsio.com>
> >
> > Although the comments say you are using proc fs, there is no
> > code here related to that.
>
> I will remove that comment.
>
> >
> > Any use of debugfs must be conditional the DEBUG_FS kernel configuration
> > parameter. Your code probably will break if DEBUG_FS is not
> > enabled. For a possible alternative see how a sub-config parameter
> > was added in sky2 driver.
> >
>
> It appears that debugfs_create_dir() returns an error if DEBUG_FS is not
> enabled. Considering the driver handles this error and continues
> initialization, do you still think I should guard this code within DEBUG_FS?
>
> Thanks.
That works, just make sure and test it.
^ permalink raw reply
* Re: sctp_close/sk_free: kernel BUG at arch/x86/mm/physaddr.c:18!
From: Jerry Chu @ 2012-09-05 23:07 UTC (permalink / raw)
To: Fengguang Wu
Cc: Eric Dumazet, Marc Kleine-Budde, Eric W. Biederman, networking,
linux-can
In-Reply-To: <20120905222850.GA11230@localhost>
On Wed, Sep 5, 2012 at 3:28 PM, Fengguang Wu <fengguang.wu@intel.com> wrote:
> On Wed, Sep 05, 2012 at 06:57:00PM +0200, Eric Dumazet wrote:
>> On Wed, 2012-09-05 at 17:40 +0200, Eric Dumazet wrote:
>>
>> > Could you test the following patch please ?
>
> It works - no single error for 1000 boots!
Sorry for introducing the bug, one of the casualties dealing with code
that is shared
outside of TCP. I did spend some effort adding special checks but I was wrong in
assuming inet_create() will zero all the field including fastopenq
inside icsk_accept_queue
inside struct inet_connection_sock - although this is true for TCP and
DCCP, SCTP doesn't
have inet_connection_sock hence inet_csk(sk) is bogus.
Kudo to Eric for fixing it quickly before I got to it.
Jerry
>
> btw, the first bad commit has been bisected to
>
> commit 8336886f786fdacbc19b719c1f7ea91eb70706d4
> Author: Jerry Chu <hkchu@google.com>
> Date: Fri Aug 31 12:29:12 2012 +0000
>
> tcp: TCP Fast Open Server - support TFO listeners
>
>> > (Not sure why sctp doesnt memset/bzero its whole socket by the way...)
>> >
>> > Thanks
>>
>> Here is a more complete patch, as there are three potential problems,
>> not only one :
>
> Great! I'll start tests for it.
>
> Thanks,
> Fengguang
>
>> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
>> index 4f70ef0..845372b 100644
>> --- a/net/ipv4/af_inet.c
>> +++ b/net/ipv4/af_inet.c
>> @@ -149,11 +149,8 @@ void inet_sock_destruct(struct sock *sk)
>> pr_err("Attempt to release alive inet socket %p\n", sk);
>> return;
>> }
>> - if (sk->sk_type == SOCK_STREAM) {
>> - struct fastopen_queue *fastopenq =
>> - inet_csk(sk)->icsk_accept_queue.fastopenq;
>> - kfree(fastopenq);
>> - }
>> + if (sk->sk_protocol == IPPROTO_TCP)
>> + kfree(inet_csk(sk)->icsk_accept_queue.fastopenq);
>>
>> WARN_ON(atomic_read(&sk->sk_rmem_alloc));
>> WARN_ON(atomic_read(&sk->sk_wmem_alloc));
>> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
>> index 8464b79..f0c5b9c 100644
>> --- a/net/ipv4/inet_connection_sock.c
>> +++ b/net/ipv4/inet_connection_sock.c
>> @@ -314,7 +314,7 @@ struct sock *inet_csk_accept(struct sock *sk, int flags, int *err)
>> newsk = req->sk;
>>
>> sk_acceptq_removed(sk);
>> - if (sk->sk_type == SOCK_STREAM && queue->fastopenq != NULL) {
>> + if (sk->sk_protocol == IPPROTO_TCP && queue->fastopenq != NULL) {
>> spin_lock_bh(&queue->fastopenq->lock);
>> if (tcp_rsk(req)->listener) {
>> /* We are still waiting for the final ACK from 3WHS
>> @@ -775,7 +775,7 @@ void inet_csk_listen_stop(struct sock *sk)
>>
>> percpu_counter_inc(sk->sk_prot->orphan_count);
>>
>> - if (sk->sk_type == SOCK_STREAM && tcp_rsk(req)->listener) {
>> + if (sk->sk_protocol == IPPROTO_TCP && tcp_rsk(req)->listener) {
>> BUG_ON(tcp_sk(child)->fastopen_rsk != req);
>> BUG_ON(sk != tcp_rsk(req)->listener);
>>
>>
> --
> 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: [V2 PATCH 6/9] csiostor: Chelsio FCoE offload driver submission (headers part 1).
From: Ben Hutchings @ 2012-09-05 23:07 UTC (permalink / raw)
To: Naresh Kumar Inna
Cc: Stephen Hemminger, JBottomley@parallels.com,
linux-scsi@vger.kernel.org, Dimitrios Michailidis, Casey Leedom,
netdev@vger.kernel.org, Chethan Seshadri
In-Reply-To: <50478B36.6000103@chelsio.com>
On Wed, 2012-09-05 at 22:56 +0530, Naresh Kumar Inna wrote:
> On 9/5/2012 10:01 PM, Stephen Hemminger wrote:
> > On Wed, 5 Sep 2012 18:03:59 +0530
> > Naresh Kumar Inna <naresh@chelsio.com> wrote:
> >
> >> +#define CSIO_ROUNDUP(__v, __r) (((__v) + (__r) - 1) / (__r))
> >
> > This is similar to existing round_up() in kernel.h could you use that?
> >
> I will replace it with round_up() if it serves the same purpose. Thanks.
Stephen is probably thinking of DIV_ROUND_UP(). round_up() does
something different.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
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 08/10] net/macb: macb_get_drvinfo: add GEM/MACB suffix to differentiate revision
From: Ben Hutchings @ 2012-09-05 23:27 UTC (permalink / raw)
To: Nicolas Ferre
Cc: netdev, linux-arm-kernel, davem, havard, plagnioj, jamie,
linux-kernel, patrice.vilchez
In-Reply-To: <fc701a2ed18198b7a671c55f1e65725f1709509c.1346775479.git.nicolas.ferre@atmel.com>
On Wed, 2012-09-05 at 11:00 +0200, Nicolas Ferre wrote:
> Add an indication about which revision of the hardware we are running in
> info->driver string.
>
> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> ---
> drivers/net/ethernet/cadence/macb.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index bd331fd..c7c39f1 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -1313,6 +1313,10 @@ static void macb_get_drvinfo(struct net_device *dev,
> struct macb *bp = netdev_priv(dev);
>
> strcpy(info->driver, bp->pdev->dev.driver->name);
> + if (macb_is_gem(bp))
> + strcat(info->driver, " GEM");
> + else
> + strcat(info->driver, " MACB");
> strcpy(info->version, "$Revision: 1.14 $");
Related to hardware revisions (which don't belong here, as David said),
I rather doubt this CVS ID is very useful as a driver version.
If the driver doesn't have a meaningful version (aside from the kernel
version) then you can remove this function and let the ethtool core fill
in the other two fields automatically.
Ben.
> strcpy(info->bus_info, dev_name(&bp->pdev->dev));
> }
--
Ben Hutchings, Staff Engineer, Solarflare
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
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