* Re: [PATCH] usbnet: drop unneeded check for NULL
From: David Miller @ 2012-09-05 16:50 UTC (permalink / raw)
To: oneukum; +Cc: richardcochran, netdev
In-Reply-To: <2037108.ZemStJh9zr@linux-lqwf.site>
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.
^ permalink raw reply
* Re: [PATCH net-next] netfilter: x_tables: xt_init() should run earlier
From: Pablo Neira Ayuso @ 2012-09-05 16:53 UTC (permalink / raw)
To: Eric Dumazet
Cc: Cong Wang, Patrick McHardy, netfilter-devel,
Linux Kernel Network Developers
In-Reply-To: <1346863073.13121.155.camel@edumazet-glaptop>
Hi Eric,
On Wed, Sep 05, 2012 at 06:37:53PM +0200, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Cong Wang reported a NULL dereference in xt_register_target()
>
> It turns out xt_nat_init() was called before xt_init(), so xt array
> was not yet setup.
>
> xt_init() should be marked core_initcall() to solve this problem.
>
> Reported-by: Cong Wang <xiyou.wangcong@gmail.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
> net/netfilter/x_tables.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
> index 8d987c3..afcea11 100644
> --- a/net/netfilter/x_tables.c
> +++ b/net/netfilter/x_tables.c
> @@ -1390,6 +1390,6 @@ static void __exit xt_fini(void)
> kfree(xt);
> }
>
> -module_init(xt_init);
> +core_initcall(xt_init);
> module_exit(xt_fini);
It seems we've clashed fixing this, sorry. Can you still see any
problem with my patch?
Thanks for looking into this.
^ permalink raw reply
* Re: sctp_close/sk_free: kernel BUG at arch/x86/mm/physaddr.c:18!
From: Eric Dumazet @ 2012-09-05 16:57 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: Fengguang Wu, H.K. Jerry Chu, Eric W. Biederman, networking,
linux-can
In-Reply-To: <1346859645.13121.146.camel@edumazet-glaptop>
On Wed, 2012-09-05 at 17:40 +0200, Eric Dumazet wrote:
> Could you test the following patch please ?
>
> (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 :
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 related
* Re: [PATCH net-next] netfilter: x_tables: xt_init() should run earlier
From: Eric Dumazet @ 2012-09-05 16:59 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Cong Wang, Patrick McHardy, netfilter-devel,
Linux Kernel Network Developers
In-Reply-To: <20120905165321.GA22243@1984>
On Wed, 2012-09-05 at 18:53 +0200, Pablo Neira Ayuso wrote:
> Hi Eric,
>
> It seems we've clashed fixing this, sorry. Can you still see any
> problem with my patch?
>
> Thanks for looking into this.
No problem !
It seems link order is the way to go, so your patch is good too !
Thanks
^ permalink raw reply
* Re: [PATCH] decnet: fix shutdown parameter checking
From: David Miller @ 2012-09-05 17:00 UTC (permalink / raw)
To: swhiteho; +Cc: xi.wang, netdev, linux-kernel
In-Reply-To: <1346834276.2799.4.camel@menhir>
From: Steven Whitehouse <swhiteho@redhat.com>
Date: Wed, 05 Sep 2012 09:37:56 +0100
> However, I'm not overly worried and we'll soon know if it will cause any
> problems or not,
You can be sure that if we get real bug reports due to this change, I will
revert it back to how it was before.
^ permalink raw reply
* Re: Commit "ipconfig wait for carrier" makes boot hang for 2 mins if no carrier
From: David Miller @ 2012-09-05 17:01 UTC (permalink / raw)
To: joakim.tjernlund; +Cc: micha, netdev
In-Reply-To: <OF70CDBF7F.EFE301C2-ONC1257A70.002F0D3F-C1257A70.003055BF@transmode.se>
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.
^ permalink raw reply
* Re: [PATCH net-next] ipv6: Export nd_tbl to allow modules to support IPv6
From: David Miller @ 2012-09-05 17:06 UTC (permalink / raw)
To: tgraf; +Cc: netdev
In-Reply-To: <fd8b9a9ad300edfc378065ef7887c3debeeb5e89.1346843399.git.tgraf@suug.ch>
From: Thomas Graf <tgraf@suug.ch>
Date: Wed, 5 Sep 2012 13:14:08 +0200
> It does not make sense to export these functions if we don't
> export the table itself as well.
Yes it does make perfect sense. It's exported for the sake of the
_implementation_ of a neighbour table in a kernel module.
I do not want to add more users with direct access to the neighbour
tables, because it is therefore impossible to go and add the inline
refcount'less lookups et al. to those external users.
So if one of our goals is to move towards a situation where all neigh
accesses are refcount'less, having those external users makes that
nearly impossible.
Instead, I'd rather see patches that mark arp_tbl as being exported
only for internal usage inside of the tree, so that we can reach that
goal.
I'm not applying this, sorry.
^ permalink raw reply
* Re: kernel BUG at kernel/timer.c:748!
From: Dave Jones @ 2012-09-05 17:08 UTC (permalink / raw)
To: Yuchung Cheng; +Cc: Lin Ming, netdev
In-Reply-To: <CAK6E8=dWnd1LYJEnvmiG+CN7x7jBt5OL5uv=4L=2Cx3=NmW3tg@mail.gmail.com>
On Wed, Sep 05, 2012 at 09:37:12AM -0700, Yuchung Cheng wrote:
> 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.
> >
> > Strange...have bug somewhere.
>
> is this a passively opened socket or actively opened one?
I have no idea. That fuzzer ran for around 10 hours before it triggered
this. In that time, millions of syscalls were done.
Even though I had logs of every syscall, the volume of data to sift
through to try and map back to the socket that caused this is firmly
in 'needle in haystick' territory.
Dave
^ permalink raw reply
* Re: when the MTU interface is modified, the promiscuous mode is reset in gianfar driver
From: David Miller @ 2012-09-05 17:09 UTC (permalink / raw)
To: claudiu.manoil; +Cc: chikazawa.akifu, netdev, linux-kernel
In-Reply-To: <504757AC.90007@freescale.com>
From: Claudiu Manoil <claudiu.manoil@freescale.com>
Date: Wed, 5 Sep 2012 16:46:20 +0300
> I don't see how the promisc mode would be reset by changing the
> mtu. Maybe more details on the issue reproduction would help (or
> maybe a usecase involving multicast settings?)
Agreed, I can't see how this problem is even possible either.
^ permalink raw reply
* Re: [PATCH] netfilter: take care of timewait sockets
From: Pablo Neira Ayuso @ 2012-09-05 17:10 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Patrick McHardy, netfilter-devel, netdev
In-Reply-To: <1346780943.13121.58.camel@edumazet-glaptop>
On Tue, Sep 04, 2012 at 07:49:03PM +0200, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Sami Farin reported crashes in xt_LOG because it assumes skb->sk is a
> full blown socket.
>
> But with TCP early demux, we can have skb->sk pointing to a timewait
> socket.
TCP early demux is there since 3.6-rc.
I'll add that to the changelog if you don't mind, to help tracking
things for -stable.
^ permalink raw reply
* Re: [net-next.git 4/7] stmmac: add Rx watchdog optimization to mitigate the DMA irqs
From: David Miller @ 2012-09-05 17:16 UTC (permalink / raw)
To: bhutchings; +Cc: peppe.cavallaro, netdev
In-Reply-To: <1346861675.5325.13.camel@bwh-desktop.uk.solarflarecom.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 5 Sep 2012 17:14:35 +0100
> On Wed, 2012-09-05 at 17:03 +0200, Giuseppe CAVALLARO wrote:
>> GMAC devices newer than databook 3.50 has an embedded timer
>> that can be used for mitigating the number of interrupts.
>> So this patch adds this optimizations.
>> Old MAC will continue to use NAPI.
> [...]
>
> Interrupt moderation is *not* a substitute for NAPI; you should continue
> using NAPI as well.
Absolutely correct, these two facilities are _complementary_ not _exlusive_.
In fact, the most optimal driver will use NAPI with relatively short HW
mitigation settings.
^ permalink raw reply
* Re: [V2 PATCH 1/9] csiostor: Chelsio FCoE offload driver submission (sources part 1).
From: Naresh Kumar Inna @ 2012-09-05 17:18 UTC (permalink / raw)
To: Stephen Hemminger
Cc: JBottomley@parallels.com, linux-scsi@vger.kernel.org,
Dimitrios Michailidis, Casey Leedom, netdev@vger.kernel.org,
Chethan Seshadri
In-Reply-To: <20120905092326.0a54226f@nehalam.linuxnetplumber.net>
On 9/5/2012 9:53 PM, Stephen Hemminger wrote:
> On Wed, 5 Sep 2012 18:03:54 +0530
> Naresh Kumar Inna <naresh@chelsio.com> wrote:
>
>> +
>> +/* FCoE Adapter types & its description */
>> +static struct csio_adap_desc csio_fcoe_adapters[] = {
>
> Tables like this should be const.
>
Thanks for reviewing. I will change it to const.
^ permalink raw reply
* Re: [V2 PATCH 6/9] csiostor: Chelsio FCoE offload driver submission (headers part 1).
From: Naresh Kumar Inna @ 2012-09-05 17:26 UTC (permalink / raw)
To: Stephen Hemminger
Cc: JBottomley@parallels.com, linux-scsi@vger.kernel.org,
Dimitrios Michailidis, Casey Leedom, netdev@vger.kernel.org,
Chethan Seshadri
In-Reply-To: <20120905093133.5f97fcbb@nehalam.linuxnetplumber.net>
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.
^ permalink raw reply
* Re: [V2 PATCH 2/9] csiostor: Chelsio FCoE offload driver submission (sources part 2).
From: Naresh Kumar Inna @ 2012-09-05 17:43 UTC (permalink / raw)
To: Stephen Hemminger
Cc: JBottomley@parallels.com, linux-scsi@vger.kernel.org,
Dimitrios Michailidis, Casey Leedom, netdev@vger.kernel.org,
Chethan Seshadri
In-Reply-To: <20120905092957.7d24187e@nehalam.linuxnetplumber.net>
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.
^ permalink raw reply
* Re: [V2 PATCH 6/9] csiostor: Chelsio FCoE offload driver submission (headers part 1).
From: Naresh Kumar Inna @ 2012-09-05 17:44 UTC (permalink / raw)
To: Stephen Hemminger
Cc: JBottomley@parallels.com, linux-scsi@vger.kernel.org,
Dimitrios Michailidis, Casey Leedom, netdev@vger.kernel.org,
Chethan Seshadri
In-Reply-To: <20120905093305.2418eac8@nehalam.linuxnetplumber.net>
On 9/5/2012 10:03 PM, Stephen Hemminger wrote:
> On Wed, 5 Sep 2012 18:03:59 +0530
> Naresh Kumar Inna <naresh@chelsio.com> wrote:
>
>> +
>> +#define CSIO_ASSERT(cond) \
>> +do { \
>> + if (unlikely(!((cond)))) \
>> + BUG(); \
>> +} while (0)
>> +
>
> Why is this not just BUG_ON(!(cond)) ?
>
I will replace the 2 lines in the macro with BUG_ON() as you have
suggested. Thanks.
^ permalink raw reply
* Re: [PATCH] netfilter: take care of timewait sockets
From: Eric Dumazet @ 2012-09-05 17:52 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Patrick McHardy, netfilter-devel, netdev
In-Reply-To: <20120905171016.GA23211@1984>
On Wed, 2012-09-05 at 19:10 +0200, Pablo Neira Ayuso wrote:
> On Tue, Sep 04, 2012 at 07:49:03PM +0200, Eric Dumazet wrote:
> > From: Eric Dumazet <edumazet@google.com>
> >
> > Sami Farin reported crashes in xt_LOG because it assumes skb->sk is a
> > full blown socket.
> >
> > But with TCP early demux, we can have skb->sk pointing to a timewait
> > socket.
>
> TCP early demux is there since 3.6-rc.
>
> I'll add that to the changelog if you don't mind, to help tracking
> things for -stable.
Sure, its not a stable candidate.
^ permalink raw reply
* Re: [net-next 0/4][pull request] Intel Wired LAN Driver Updates
From: David Miller @ 2012-09-05 19:10 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, sassmann
In-Reply-To: <1346826350-27733-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue, 4 Sep 2012 23:25:46 -0700
> This series contains updates to igb and ixgbe.
>
> The following are changes since commit 600e177920df936d03b807780ca92c662af98990:
> net: Providing protocol type via system.sockprotoname xattr of /proc/PID/fd entries
> and are available in the git repository at:
> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master
Pulled, thanks Jeff.
^ permalink raw reply
* Re: pull-request: can 2012-09-04
From: David Miller @ 2012-09-05 19:12 UTC (permalink / raw)
To: mkl; +Cc: netdev, linux-can
In-Reply-To: <1346792142-17609-1-git-send-email-mkl@pengutronix.de>
From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Tue, 4 Sep 2012 22:55:41 +0200
> this patch is for the v3.6 release cycle. Benoît Locher fixed a repeated frame
> bug in the mcp251x driver. He implemented the workaround suggested by the
> errata sheet.
...
> git://gitorious.org/linux-can/linux-can.git fixes-for-3.6
Pulled, thanks Marc.
^ permalink raw reply
* Re: [PATCH v3 1/3] tcp: add generic netlink support for tcp_metrics
From: David Miller @ 2012-09-05 19:17 UTC (permalink / raw)
To: ja; +Cc: netdev, shemminger
In-Reply-To: <1346792597-2427-2-git-send-email-ja@ssi.bg>
From: Julian Anastasov <ja@ssi.bg>
Date: Wed, 5 Sep 2012 00:03:15 +0300
> Add support for genl "tcp_metrics". No locking
> is changed, only that now we can unlink and delete
> entries after grace period. We implement get/del for
> single entry and dump to support show/flush filtering
> in user space. Del without address attribute causes
> flush for all addresses, sadly under genl_mutex.
>
> v2:
> - remove rcu_assign_pointer as suggested by Eric Dumazet,
> it is not needed because there are no other writes under lock
> - move the flushing code in tcp_metrics_flush_all
>
> v3:
> - remove synchronize_rcu on flush as suggested by Eric Dumazet
>
> Signed-off-by: Julian Anastasov <ja@ssi.bg>
Looks great, applied to net-next, thanks Julian.
^ permalink raw reply
* [PATCH] Revert "usbnet: drop unneeded check for NULL"
From: Oliver Neukum @ 2012-09-05 19:22 UTC (permalink / raw)
To: davem, netdev; +Cc: Oliver Neukum, Oliver Neukum
This reverts commit 5d65878d7031b6c39054b282faceff406bb2fda9.
The upper layers call usbnet_start_xmit() with a valid skb.
However cdc_ncm abuses this method by calling it with NULL
to trigger IO for the aggregated private skb holding erlier
packets. Until cdc_ncm is fixed, the check for NULL must
be reintroduced.
Signed-off-by: Oliver Neukum <oneukum@suse.de>
---
drivers/net/usb/usbnet.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 5234d20..8531c1c 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1092,7 +1092,8 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
unsigned long flags;
int retval;
- skb_tx_timestamp(skb);
+ if (skb)
+ skb_tx_timestamp(skb);
// some devices want funky USB-level framing, for
// win32 driver (usually) and/or hardware quirks
--
1.7.7
^ permalink raw reply related
* changing usbnet's API to better deal with cdc-ncm
From: Oliver Neukum @ 2012-09-05 20:12 UTC (permalink / raw)
To: alexey.orishko, bjorn, netdev, linux-usb
Hi,
looking at cdc-ncm it seeems to me that cdc-ncm is forced to play
very dirty games because usbnet doesn't have a notion about aggregating
packets for a single transfer.
It seems to me that this can be fixed introducing a method for bundling,
which tells usbnet how packets have been aggregated. To have performance
usbnet strives to always keep at least two transfers in flight.
The code isn't complete and I need to get a device for testing, but to get
your opinion, I ask you to comment on what I have now.
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] Revert "usbnet: drop unneeded check for NULL"
From: David Miller @ 2012-09-05 20:20 UTC (permalink / raw)
To: oliver; +Cc: netdev, oneukum
In-Reply-To: <1346872952-21192-1-git-send-email-oliver@neukum.org>
From: Oliver Neukum <oliver@neukum.org>
Date: Wed, 5 Sep 2012 21:22:32 +0200
> This reverts commit 5d65878d7031b6c39054b282faceff406bb2fda9.
> The upper layers call usbnet_start_xmit() with a valid skb.
> However cdc_ncm abuses this method by calling it with NULL
> to trigger IO for the aggregated private skb holding erlier
> packets. Until cdc_ncm is fixed, the check for NULL must
> be reintroduced.
>
> Signed-off-by: Oliver Neukum <oneukum@suse.de>
Applied, thanks Oliver.
^ permalink raw reply
* Re: kernel BUG at kernel/timer.c:748!
From: Julian Anastasov @ 2012-09-05 20:48 UTC (permalink / raw)
To: Dave Jones; +Cc: netdev
In-Reply-To: <20120905043523.GA12988@redhat.com>
Hello,
On Wed, 5 Sep 2012, Dave Jones wrote:
> Just hit this bug on 3.6-rc4.
>
> The BUG is..
>
> BUG_ON(!timer->function);
>
>
> 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 ]---
Can this help? In case you see ICMPV6_PKT_TOOBIG...
[PATCH] tcp: fix possible socket refcount problem for ipv6
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;
}
--
1.7.3.4
^ permalink raw reply related
* [PATCH] tcp: fix possible socket refcount problem for ipv6
From: Julian Anastasov @ 2012-09-05 20:53 UTC (permalink / raw)
To: David Miller; +Cc: netdev
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;
}
--
1.7.3.4
^ permalink raw reply related
* 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
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