* Re: [PATCH v3 03/10] net: Add netdev interfaces recording send/compl
From: Tom Herbert @ 2011-11-24 0:32 UTC (permalink / raw)
To: David Miller; +Cc: shemminger, netdev
In-Reply-To: <20111123.190623.2026426589805797580.davem@davemloft.net>
>> I was referring to the transmit routine, there for netdev_sent_queue()
>> is always called with one packet.
>
> Optimism for the future? :-)
>
Yep, I am looking forward to the day that someone figures out how to
batch transmits in a meaningful way!
> But yeah if nobody batches right now, no reason to support this in the
> interfaces, we can add it later.
Okay.
>
^ permalink raw reply
* WARNING: at net/core/dev.c:1904 skb_gso_segment+0x146/0x298()
From: Paweł Staszewski @ 2011-11-24 0:30 UTC (permalink / raw)
To: Linux Network Development list
Hello
After upgrade from 2.6.38.2 to 3.1.2 i have this im dmesg:
[ 600.266497] WARNING: at net/core/dev.c:1904 skb_gso_segment+0x146/0x298()
[ 600.266500] Hardware name: X8DTU-6+
[ 600.266503] 802.1Q VLAN Support: caps=(0x20115833, 0x0) len=2816
data_len=2776 ip_summed=1
[ 600.266506] Modules linked in: macvlan
[ 600.266511] Pid: 0, comm: kworker/0:1 Not tainted 3.1.2 #1
[ 600.266513] Call Trace:
[ 600.266515] <IRQ> [<ffffffff8103449c>] warn_slowpath_common+0x80/0x98
[ 600.266527] [<ffffffff81034548>] warn_slowpath_fmt+0x41/0x43
[ 600.266530] [<ffffffff813c838f>] skb_gso_segment+0x146/0x298
[ 600.266535] [<ffffffff8103994e>] ? local_bh_enable+0xd/0xf
[ 600.266540] [<ffffffff813cc646>] dev_hard_start_xmit+0x35a/0x57d
[ 600.266544] [<ffffffff8103994e>] ? local_bh_enable+0xd/0xf
[ 600.266548] [<ffffffff813cccb2>] dev_queue_xmit+0x449/0x4ef
[ 600.266554] [<ffffffff813ffc4d>] ip_finish_output2+0x1c4/0x201
[ 600.266560] [<ffffffff813ffd1c>] ip_finish_output+0x92/0x97
[ 600.266562] [<ffffffff813ffe7c>] T.1037+0x4f/0x56
[ 600.266565] [<ffffffff81400005>] ip_output+0x58/0x5b
[ 600.266567] [<ffffffff813fc4f0>] ip_forward_finish+0x44/0x48
[ 600.266569] [<ffffffff813fc7f4>] ip_forward+0x300/0x36c
[ 600.266572] [<ffffffff813fb144>] ip_rcv_finish+0x2a4/0x2ce
[ 600.266575] [<ffffffff813faea0>] ? inet_del_protocol+0x37/0x37
[ 600.266577] [<ffffffff813fb431>] T.935+0x4c/0x53
[ 600.266579] [<ffffffff813fb6bc>] ip_rcv+0x237/0x263
[ 600.266582] [<ffffffff813cb76b>] __netif_receive_skb+0x41d/0x44f
[ 600.266584] [<ffffffff813cb891>] process_backlog+0xf4/0x1d3
[ 600.266587] [<ffffffff813cbfee>] net_rx_action+0x74/0x1cb
[ 600.266589] [<ffffffff81039a74>] __do_softirq+0xc8/0x1a4
[ 600.266591] [<ffffffff81039b31>] ? __do_softirq+0x185/0x1a4
[ 600.266595] [<ffffffff814a8bec>] call_softirq+0x1c/0x30
[ 600.266599] [<ffffffff8100385d>] do_softirq+0x41/0x7e
[ 600.266601] [<ffffffff8103987b>] irq_exit+0x44/0x74
[ 600.266603] [<ffffffff81003182>] do_IRQ+0x98/0xaf
[ 600.266606] [<ffffffff814a0f2e>] common_interrupt+0x6e/0x6e
[ 600.266608] <EOI> [<ffffffff8100887e>] ? mwait_idle+0x7e/0xa4
[ 600.266613] [<ffffffff81008836>] ? mwait_idle+0x36/0xa4
[ 600.266615] [<ffffffff81001da7>] cpu_idle+0x5f/0x91
[ 600.266620] [<ffffffff81acca55>] start_secondary+0x192/0x196
[ 600.266622] ---[ end trace 15512840060b2da9 ]---
Network controller: Intel Corporation 82598EB 10-Gigabit AT CX4 Network
Connection (rev 01)
ethtool -i eth4
driver: ixgbe
version: 3.4.8-k
firmware-version: 1.12-2
bus-info: 0000:04:00.0
ethtool -k eth4
Offload parameters for eth4:
rx-checksumming: on
tx-checksumming: on
scatter-gather: on
tcp-segmentation-offload: on
udp-fragmentation-offload: off
generic-segmentation-offload: on
generic-receive-offload: on
large-receive-offload: off
ntuple-filters: off
receive-hashing: on
Thanks
Pawel
^ permalink raw reply
* Re: [PATCH 5/5] ipv4: Don't use the cached pmtu informations for input routes
From: David Miller @ 2011-11-24 0:17 UTC (permalink / raw)
To: herbert; +Cc: steffen.klassert, netdev
In-Reply-To: <20111124001035.GA14996@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.hengli.com.au>
Date: Thu, 24 Nov 2011 08:10:35 +0800
> Is there a particular scenario where this breaks? I could only think
> of policy routing cases but that's not unique to forwarding as it
> also affects us as a host.
Steffen has a specific situation.
But regardless I think the current behavior is terrible.
It means there is a propagation delay of PMTU increasing which is on the
order of the number of hops between the sender and the PMTU increased link.
So if the PMTU timeout is X, the propagation time is X * N_HOPS.
^ permalink raw reply
* Re: [PATCH 5/5] ipv4: Don't use the cached pmtu informations for input routes
From: Herbert Xu @ 2011-11-24 0:10 UTC (permalink / raw)
To: David Miller; +Cc: steffen.klassert, netdev
In-Reply-To: <20111123.164941.1950228526500366081.davem@davemloft.net>
On Wed, Nov 23, 2011 at 04:49:41PM -0500, David Miller wrote:
>
> The issue is that "we", the forwarding entity, end up with a cached on
> the input route and report this cached PMTU to the sender.
>
> This takes a while to time out.
I understand. However, this could be seen as an optimisation
as if somehow we as a router discovered the smaller MTU this
would save everybody behind us from having to disocver it by
probing until our discovery expires.
Is there a particular scenario where this breaks? I could only think
of policy routing cases but that's not unique to forwarding as it
also affects us as a host.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] qlge: fix size of external list for TX address descriptors
From: David Miller @ 2011-11-24 0:10 UTC (permalink / raw)
To: cascardo
Cc: netdev, linux-kernel, linux-driver, ron.mercer, jitendra.kalsaria,
anirban.chakraborty
In-Reply-To: <1322089842-11694-1-git-send-email-cascardo@linux.vnet.ibm.com>
From: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
Date: Wed, 23 Nov 2011 21:10:42 -0200
> When transmiting a fragmented skb, qlge fills a descriptor with the
> fragment addresses, after DMA-mapping them. If there are more than eight
> fragments, it will use the eighth descriptor as a pointer to an external
> list. After mapping this external list, called OAL to a structure
> containing more descriptors, it fills it with the extra fragments.
>
> However, considering that systems with pages larger than 8KiB would have
> less than 8 fragments, which was true before commit a715dea3c8e, it
> defined a macro for the OAL size as 0 in those cases.
>
> Now, if a skb with more than 8 fragments (counting skb->data as one
> fragment), this would start overwriting the list of addresses already
> mapped and would make the driver fail to properly unmap the right
> addresses on architectures with pages larger than 8KiB.
>
> Besides that, the list of mappings was one size too small, since it must
> have a mapping for the maxinum number of skb fragments plus one for
> skb->data and another for the OAL. So, even on architectures with page
> sizes 4KiB and 8KiB, a skb with the maximum number of fragments would
> make the driver overwrite its counter for the number of mappings, which,
> again, would make it fail to unmap the mapped DMA addresses.
>
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
Applied, thanks for the detailed commit message.
^ permalink raw reply
* Re: [net] bnx2x: Fix 5461x LED
From: David Miller @ 2011-11-24 0:08 UTC (permalink / raw)
To: yanivr; +Cc: netdev, eilong
In-Reply-To: <1322056448-22290-1-git-send-email-yanivr@broadcom.com>
From: "Yaniv Rosner" <yanivr@broadcom.com>
Date: Wed, 23 Nov 2011 15:54:08 +0200
> Fix port identify test on 5461x PHY by driving LEDs through MDIO.
>
> Signed-off-by: Yaniv Rosner <yanivr@broadcom.com>
> Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
Applied.
^ permalink raw reply
* Re: |PATCH net-next] net: treewide use of RCU_INIT_POINTER
From: David Miller @ 2011-11-24 0:08 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1322068172.17693.61.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 23 Nov 2011 18:09:32 +0100
> rcu_assign_pointer(ptr, NULL) can be safely replaced by
> RCU_INIT_POINTER(ptr, NULL)
>
> (old rcu_assign_pointer() macro was testing the NULL value and could
> omit the smp_wmb(), but this had to be removed because of compiler
> warnings)
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH v3 03/10] net: Add netdev interfaces recording send/compl
From: David Miller @ 2011-11-24 0:06 UTC (permalink / raw)
To: shemminger; +Cc: therbert, netdev
In-Reply-To: <20111123160320.76143e3b@nehalam.linuxnetplumber.net>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 23 Nov 2011 16:03:20 -0800
> On Wed, 23 Nov 2011 18:58:12 -0500 (EST)
> David Miller <davem@davemloft.net> wrote:
>
>> From: Stephen Hemminger <shemminger@vyatta.com>
>> Date: Wed, 23 Nov 2011 09:46:14 -0800
>>
>> > Since all the drivers that you show do this for one packet at a time,
>> > why bother with a packets arguement to netdev_sent_queue()?
>>
>> They batch the completion, that's why f.e. e1000e's changes keep track of
>> pkts_compl and bytes_compl, and then pass that in at the end of the TX
>> completion processing so that we only invoke these interfaces once per
>> run instead of once per packet.
>
> I was referring to the transmit routine, there for netdev_sent_queue()
> is always called with one packet.
Optimism for the future? :-)
But yeah if nobody batches right now, no reason to support this in the
interfaces, we can add it later.
^ permalink raw reply
* Re: [PATCH v2] ipv4 : igmp : optimize timer modify logic in igmp_mod_timer()
From: David Miller @ 2011-11-24 0:04 UTC (permalink / raw)
To: mypopydev; +Cc: eric.dumazet, netdev
In-Reply-To: <1322088885.5402.17.camel@barry.pixelworks.com>
From: Jun Zhao <mypopydev@gmail.com>
Date: Thu, 24 Nov 2011 06:54:45 +0800
> On Wed, 2011-11-23 at 17:28 -0500, David Miller wrote:
>> From: Jun Zhao <mypopydev@gmail.com>
>> Date: Thu, 24 Nov 2011 00:38:42 +0800
>>
>> > When timer is pending and expires less-than-or-equal-to new delay,
>> > we need not used del_timer()/add_timer().
>> >
>> > Signed-off-by: Jun Zhao <mypopydev@gmail.com>
>>
>> You did not answer Eric's question, why are you optimizing this
>> less-used code path?
>
> 1). Oh, in the RFC 3376 $5.2, Page 23:
Then your commit message is terrible.
Your commit message, one the one hand, talks about optimizing the code.
Your explanation here talks about RFC conformance.
Your inconsistencies, and how you ignore important questions posed to
you like Eric's (until I point it out to you) makes your work
incredibly irritating to review and process.
Your patch submissions need to be more well formed and your commit
messages need to explain exactly what your goals are with your change
and how those goals are being met by the patch you are proposing.
When we read "optimize timer modify logic" how the heck are we
supposed to know what this change is actually doing? Why should we
think that we actually need your change? How am we supposed to figure
out that you are fixing an RFC conformance issue?
I'm sorry, this patch submission is junk. Don't send us junk.
^ permalink raw reply
* Re: [PATCH v3 03/10] net: Add netdev interfaces recording send/compl
From: Stephen Hemminger @ 2011-11-24 0:03 UTC (permalink / raw)
To: David Miller; +Cc: therbert, netdev
In-Reply-To: <20111123.185812.1721878183759040757.davem@davemloft.net>
On Wed, 23 Nov 2011 18:58:12 -0500 (EST)
David Miller <davem@davemloft.net> wrote:
> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Wed, 23 Nov 2011 09:46:14 -0800
>
> > Since all the drivers that you show do this for one packet at a time,
> > why bother with a packets arguement to netdev_sent_queue()?
>
> They batch the completion, that's why f.e. e1000e's changes keep track of
> pkts_compl and bytes_compl, and then pass that in at the end of the TX
> completion processing so that we only invoke these interfaces once per
> run instead of once per packet.
I was referring to the transmit routine, there for netdev_sent_queue()
is always called with one packet.
Many drivers batch packets in completion, and call netdev_completed_queue()
with multiple packets (and bytes).
^ permalink raw reply
* Re: [PATCH v3 0/10] bql: Byte Queue Limits
From: David Miller @ 2011-11-23 23:59 UTC (permalink / raw)
To: therbert; +Cc: netdev
In-Reply-To: <alpine.DEB.2.00.1111222134590.9126@pokey.mtv.corp.google.com>
From: Tom Herbert <therbert@google.com>
Date: Tue, 22 Nov 2011 21:52:12 -0800 (PST)
> This patch series implements byte queue limits (bql) for NIC TX queues.
No fundamental objections from me.
Please address the feedback (in particular the dql data structure layout,
32-bit counters, and getting rid of the config options).
^ permalink raw reply
* Re: [PATCH v3 03/10] net: Add netdev interfaces recording send/compl
From: David Miller @ 2011-11-23 23:58 UTC (permalink / raw)
To: shemminger; +Cc: therbert, netdev
In-Reply-To: <20111123094614.36560215@nehalam.linuxnetplumber.net>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 23 Nov 2011 09:46:14 -0800
> Since all the drivers that you show do this for one packet at a time,
> why bother with a packets arguement to netdev_sent_queue()?
They batch the completion, that's why f.e. e1000e's changes keep track of
pkts_compl and bytes_compl, and then pass that in at the end of the TX
completion processing so that we only invoke these interfaces once per
run instead of once per packet.
^ permalink raw reply
* Re: [PATCH v3 01/10] dql: Dynamic queue limits
From: David Miller @ 2011-11-23 23:53 UTC (permalink / raw)
To: shemminger; +Cc: therbert, netdev
In-Reply-To: <20111123083712.69a2ac6e@nehalam.linuxnetplumber.net>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 23 Nov 2011 08:37:12 -0800
> 1. Since linux/dql.h is only kernel components (not API), it should
> be net/dql.h
It's a library routine under lib/, meant to be available to anyone not
just networking components even though networking is the only current
consumer.
His placement of the header file is therefore correct as-is.
^ permalink raw reply
* Re: linux-next: build failure after merge of the final tree (net-next tree related)
From: David Miller @ 2011-11-23 23:44 UTC (permalink / raw)
To: akpm; +Cc: nhorman, sfr, netdev, linux-next, linux-kernel
In-Reply-To: <20111123150213.ed7257cb.akpm@linux-foundation.org>
From: Andrew Morton <akpm@linux-foundation.org>
Date: Wed, 23 Nov 2011 15:02:13 -0800
> Needs a thorough redo. Please take a look at the other cgroup
> subsystem headers. The basic pattern is
>
> #ifdef CONFIG_FOO_CGROUP
> <definitions and declarations>
> #else
> <any needed stubs go here>
> #endif
>
> And that's it.
I told Neil to put the stub in net/core/dev.c since that's the one and
only call site.
^ permalink raw reply
* Re: slub: use irqsafe_cpu_cmpxchg for put_cpu_partial
From: David Rientjes @ 2011-11-23 23:15 UTC (permalink / raw)
To: Christoph Lameter
Cc: Pekka Enberg, Christian Kujau, Benjamin Herrenschmidt,
Eric Dumazet, Markus Trippelsdorf, Alex,Shi, linux-kernel,
linux-mm, Matt Mackall, netdev, Tejun Heo
In-Reply-To: <alpine.DEB.2.00.1111230907330.16139@router.home>
On Wed, 23 Nov 2011, Christoph Lameter wrote:
> Subject: slub: use irqsafe_cpu_cmpxchg for put_cpu_partial
>
> The cmpxchg must be irq safe. The fallback for this_cpu_cmpxchg only
> disables preemption which results in per cpu partial page operation
> potentially failing on non x86 platforms.
>
> Signed-off-by: Christoph Lameter <cl@linux.com>
Acked-by: David Rientjes <rientjes@google.com>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* [PATCH] qlge: fix size of external list for TX address descriptors
From: Thadeu Lima de Souza Cascardo @ 2011-11-23 23:10 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, linux-driver, Ron Mercer, Jitendra Kalsaria,
Anirban Chakraborty, Thadeu Lima de Souza Cascardo
When transmiting a fragmented skb, qlge fills a descriptor with the
fragment addresses, after DMA-mapping them. If there are more than eight
fragments, it will use the eighth descriptor as a pointer to an external
list. After mapping this external list, called OAL to a structure
containing more descriptors, it fills it with the extra fragments.
However, considering that systems with pages larger than 8KiB would have
less than 8 fragments, which was true before commit a715dea3c8e, it
defined a macro for the OAL size as 0 in those cases.
Now, if a skb with more than 8 fragments (counting skb->data as one
fragment), this would start overwriting the list of addresses already
mapped and would make the driver fail to properly unmap the right
addresses on architectures with pages larger than 8KiB.
Besides that, the list of mappings was one size too small, since it must
have a mapping for the maxinum number of skb fragments plus one for
skb->data and another for the OAL. So, even on architectures with page
sizes 4KiB and 8KiB, a skb with the maximum number of fragments would
make the driver overwrite its counter for the number of mappings, which,
again, would make it fail to unmap the mapped DMA addresses.
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
---
drivers/net/ethernet/qlogic/qlge/qlge.h | 8 +++-----
1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qlge/qlge.h b/drivers/net/ethernet/qlogic/qlge/qlge.h
index 8731f79..b8478aa 100644
--- a/drivers/net/ethernet/qlogic/qlge/qlge.h
+++ b/drivers/net/ethernet/qlogic/qlge/qlge.h
@@ -58,10 +58,8 @@
#define TX_DESC_PER_IOCB 8
-/* The maximum number of frags we handle is based
- * on PAGE_SIZE...
- */
-#if (PAGE_SHIFT == 12) || (PAGE_SHIFT == 13) /* 4k & 8k pages */
+
+#if ((MAX_SKB_FRAGS - TX_DESC_PER_IOCB) + 2) > 0
#define TX_DESC_PER_OAL ((MAX_SKB_FRAGS - TX_DESC_PER_IOCB) + 2)
#else /* all other page sizes */
#define TX_DESC_PER_OAL 0
@@ -1353,7 +1351,7 @@ struct tx_ring_desc {
struct ob_mac_iocb_req *queue_entry;
u32 index;
struct oal oal;
- struct map_list map[MAX_SKB_FRAGS + 1];
+ struct map_list map[MAX_SKB_FRAGS + 2];
int map_cnt;
struct tx_ring_desc *next;
};
--
1.7.4.4
^ permalink raw reply related
* Re: linux-next: build failure after merge of the final tree (net-next tree related)
From: Andrew Morton @ 2011-11-23 23:02 UTC (permalink / raw)
To: Neil Horman
Cc: Stephen Rothwell, David Miller, netdev, linux-next, linux-kernel
In-Reply-To: <20111123120937.GB27968@hmsreliant.think-freely.org>
On Wed, 23 Nov 2011 07:09:37 -0500
Neil Horman <nhorman@tuxdriver.com> wrote:
> On Wed, Nov 23, 2011 at 03:00:04PM +1100, Stephen Rothwell wrote:
> > Hi all,
> >
> > After merging the final tree, today's linux-next build (powerpc allnoconfig)
> > failed like this:
> >
> > In file included from include/linux/netdevice.h:53:0,
> > from include/linux/icmpv6.h:173,
> > from include/linux/ipv6.h:220,
> > from include/net/ipv6.h:16,
> > from include/linux/sunrpc/clnt.h:26,
> > from include/linux/nfs_fs.h:50,
> > from init/do_mounts.c:20:
> > include/net/netprio_cgroup.h:23:29: error: field 'css' has incomplete type
>
> ...
>
> FYI, I've got a more appropriate fix building right now getting posted for the
> net-next tree. I'll cc you on it.
This header looks to be pretty screwed up:
- Extraneous newline after "struct cgroup_netprio_state"
- Weird special-casing of the CONFIG_CGROUPS &&
!CONFIG_NETPRIO_CGROUP case looks suspicious.
- Unnecessary use of IS_ENABLED - jsut use #ifdef CONFIG_NETPRIO_CGROUP
- Adds code stubs specifically for the CONFIG_CGROUPS &&
!CONFIG_NETPRIO_CGROUP case. Doesn't add any for the !CONFIG_CGROUPS
case. Seems wrong.
- Uses empty macros for the sock_update_netprioidx() and
skb_update_prio() which can cause build errors. Should be changed to
plain old typechecked inline C functions. If that causes build
errors then something else is screwed up.
- Adds a stub for skb_update_prio() but there's no non-stub version
of it.
also
- doesn't compile
Needs a thorough redo. Please take a look at the other cgroup
subsystem headers. The basic pattern is
#ifdef CONFIG_FOO_CGROUP
<definitions and declarations>
#else
<any needed stubs go here>
#endif
And that's it.
^ permalink raw reply
* Re: RAW netfilter - "advanced netfilter setting" or not?
From: Linus Torvalds @ 2011-11-23 22:58 UTC (permalink / raw)
To: Jan Engelhardt
Cc: David Miller, Pablo Neira Ayuso, Patrick McHardy, netfilter-devel,
netdev
In-Reply-To: <alpine.LNX.2.01.1111232325500.27025@frira.zrqbmnf.qr>
On Wed, Nov 23, 2011 at 2:32 PM, Jan Engelhardt <jengelh@medozas.de> wrote:
>
> Right, but how would you decide what will be enabled/disabled by
> default? It seems unlikely you will be adding a patch (like Dave's)
> everytime a default distro installation throws certain errors once
> you run your own kernel configs.
I do think that "major distributions do this by default" should simply
be the point for deciding it. Nothing more, nothing less.
Linus
^ permalink raw reply
* Re: [PATCH v2] ipv4 : igmp : optimize timer modify logic in igmp_mod_timer()
From: Jun Zhao @ 2011-11-23 22:54 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, netdev
In-Reply-To: <20111123.172814.1626722266908002386.davem@davemloft.net>
On Wed, 2011-11-23 at 17:28 -0500, David Miller wrote:
> From: Jun Zhao <mypopydev@gmail.com>
> Date: Thu, 24 Nov 2011 00:38:42 +0800
>
> > When timer is pending and expires less-than-or-equal-to new delay,
> > we need not used del_timer()/add_timer().
> >
> > Signed-off-by: Jun Zhao <mypopydev@gmail.com>
>
> You did not answer Eric's question, why are you optimizing this
> less-used code path?
1). Oh, in the RFC 3376 $5.2, Page 23:
"
4. If there already is a pending response to a previous Query
scheduled for this group, and either the new Query is a Group-
Specific Query or the recorded source-list associated with the
group is empty, then the group source-list is cleared and a single
response is scheduled using the group timer. The new response is
scheduled to be sent at the earliest of the remaining time for the
pending report and the selected delay.
5. If the received Query is a Group-and-Source-Specific Query and
there is a pending response for this group with a non-empty
source-list, then the group source list is augmented to contain
the list of sources in the new Query and a single response is
scheduled using the group timer. The new response is scheduled to
be sent at the earliest of the remaining time for the pending
report and the selected delay.
"
I think this patch more conform to the RFC.
2). Maybe this is a less-used code path, but we can do better than
current.
3). In the current implementation, "im->tm_running = 1;" is redundant
^ permalink raw reply
* Re: [iproute2 PATCH 2/2] tc: Use correct variable type for get_distribution() result
From: Stephen Hemminger @ 2011-11-23 22:48 UTC (permalink / raw)
To: Thomas Jarosch; +Cc: netdev
In-Reply-To: <4ECD6267.3090406@intra2net.com>
On Wed, 23 Nov 2011 22:15:19 +0100
Thomas Jarosch <thomas.jarosch@intra2net.com> wrote:
> get_distribution() returns an int.
>
> cppcheck reported:
> [tc/q_netem.c:243]: (style) Checking if unsigned variable 'dist_size' is less than zero.
>
Both applied, thanks.
^ permalink raw reply
* Re: [PATCH] netns: fix proxy ARP entries listing on a netns
From: David Miller @ 2011-11-23 22:36 UTC (permalink / raw)
To: jorge; +Cc: netdev
In-Reply-To: <1321904774-3030-1-git-send-email-jorge@dti2.net>
From: "Jorge Boncompte [DTI2]" <jorge@dti2.net>
Date: Mon, 21 Nov 2011 20:46:14 +0100
> @@ -2397,6 +2397,7 @@ static struct pneigh_entry *pneigh_get_next(struct seq_file *seq,
> struct net *net = seq_file_net(seq);
> struct neigh_table *tbl = state->tbl;
>
> +restart:
> pn = pn->next;
> while (!pn) {
We don't need to use goto to fix this bug, instead change this assignment
to something like:
do {
pn = pn->next;
} while (pn && !net_eq(pneigh_net(pn), net));
and this way it will mirror the while loop inside of the "while (!pn)"
code block.
^ permalink raw reply
* Re: RAW netfilter - "advanced netfilter setting" or not?
From: Jan Engelhardt @ 2011-11-23 22:32 UTC (permalink / raw)
To: Linus Torvalds
Cc: David Miller, Pablo Neira Ayuso, Patrick McHardy, netfilter-devel,
netdev
In-Reply-To: <CA+55aFyQGf4sFNOVsv4krddn3gxQ=roqVHpC98-Ynx8iBqpRaQ@mail.gmail.com>
On Wednesday 2011-11-23 23:02, Linus Torvalds wrote:
>On Wed, Nov 23, 2011 at 1:27 PM, Jan Engelhardt <jengelh@medozas.de> wrote:
>>
>> In my opinion, NETFILTER_ADVANCED should be changed to only control
>> the visibility of all suboptions, i.e. I suggest that "default m if
>> NETFILTER_ADVANCED=n" be done for all non-deprecated modules.
>> (Similar to how CONFIG_EXPERT works.)
>
>No thank you. That makes the whole option pointless. [...]
>The whole point of NETFILTER_ADVANCED is for people like me who
>actually want a fairly *minimal* kernel config, and probably one that
>has no modules.
Right, but how would you decide what will be enabled/disabled by
default? It seems unlikely you will be adding a patch (like Dave's)
everytime a default distro installation throws certain errors once
you run your own kernel configs.
^ permalink raw reply
* Re: [PATCH v3] ipv4 : igmp : fix error handle in ip_mc_add_src()
From: David Miller @ 2011-11-23 22:32 UTC (permalink / raw)
To: mypopydev; +Cc: eric.dumazet, dlstevens, netdev
In-Reply-To: <1322018343-7552-1-git-send-email-mypopydev@gmail.com>
From: Jun Zhao <mypopydev@gmail.com>
Date: Wed, 23 Nov 2011 11:19:03 +0800
> from: Jun Zhao <mypopydev@gmail.com>
>
> When add sources to interface failure, need to roll back the sfcount[MODE]
> to before state. We need to match it corresponding.
>
> Acked-by: David L Stevens <dlstevens@us.ibm.com>
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Jun Zhao <mypopydev@gmail.com>
Applied, thanks.
^ permalink raw reply
* Re: Missing TCP SYN on loopback, retransmits after 1s
From: David Miller @ 2011-11-23 22:29 UTC (permalink / raw)
To: eric.dumazet; +Cc: jlyo, netdev
In-Reply-To: <1322059124.17693.24.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 23 Nov 2011 15:38:44 +0100
> [PATCH] ipv6: tcp: fix tcp_v6_conn_request()
>
> Since linux 2.6.26 (commit c6aefafb7ec6 : Add IPv6 support to TCP SYN
> cookies), we can drop a SYN packet reusing a TIME_WAIT socket.
>
> (As a matter of fact we fail to send the SYNACK answer)
>
> As the client resends its SYN packet after a one second timeout, we
> accept it, because first packet removed the TIME_WAIT socket before
> being dropped.
>
> This probably explains why nobody ever noticed or complained.
>
> Reported-by: Jesse Young <jlyo@jlyo.org>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied and queued up for -stable, thanks!
^ permalink raw reply
* Re: [PATCH v2] ipv4 : igmp : optimize timer modify logic in igmp_mod_timer()
From: David Miller @ 2011-11-23 22:28 UTC (permalink / raw)
To: mypopydev; +Cc: eric.dumazet, netdev
In-Reply-To: <1322066322-4782-1-git-send-email-mypopydev@gmail.com>
From: Jun Zhao <mypopydev@gmail.com>
Date: Thu, 24 Nov 2011 00:38:42 +0800
> When timer is pending and expires less-than-or-equal-to new delay,
> we need not used del_timer()/add_timer().
>
> Signed-off-by: Jun Zhao <mypopydev@gmail.com>
You did not answer Eric's question, why are you optimizing this
less-used code path?
^ 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