* Re: [RFC PATCH] net: Implement read-only protection and COW'ing of metrics.
From: David Miller @ 2011-01-26 23:31 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <20110126.152538.260074157.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Wed, 26 Jan 2011 15:25:38 -0800 (PST)
> Finally, once this change is stabilized we can be a lot smarter about
> what we do at the time an entry is created. For example, when a route
> is looked up for a TCP socket, we essentially know we are going to COW
> the route %99.99999 of the time. So we can pass a hint into TCP's
> route lookups in the flow flags field telling it to pre-COW the route.
>
> TCP pre-COW'ing of metrics will thus save several atomics.
I forgot to mention one other idea I had.
To get rid of the atomics in the non-TCP cases, we note that pretty much
all routes installed have no special metrics attached, the fib_info
metrics are equal to dst_default_metrics.
This means if we check for this case, we can point the dst->_metrics
at dst_default_metrics and then we don't need to do any atomics at
all. Just one straight assignment at creation and then absolutely no
work at all during destroy.
We could even consider allocating fib_info->metrics externally, and point
it directly at dst_default_metrics when possible. This is going to save
an enormous amount of memory as well as get rid of the atomics.
So Eric, I really hope I can sell you on this :-)
^ permalink raw reply
* Simultaneous error on two different machines
From: J.H. @ 2011-01-27 0:36 UTC (permalink / raw)
To: Michael Chan, netdev
Afternoon,
Happened to trip over this yesterday on two machines (one an HP DL380 G6
and one an HP DL 380 G7). Error was within minutes of each other,
identical on the two boxes. Driver seemed to reset (as indicated
below), however loads on the two boxes almost immediately skyrocketed,
eventually leading to a more serious deadlock panic on one of the boxes,
and me just rebooting the other.
The chipsets involved seem to be:
Ethernet controller: Broadcom Corporation NetXtreme II BCM5709 Gigabit
Ethernet (rev 20)
Kernel is from Fedora 13, 2.6.34.7-61.korg.fc13.x86_64. The only
additional patches applied involve fixing an XFS bug.
Here follows what I was able to snag from the system:
------------[ cut here ]------------
WARNING: at net/sched/sch_generic.c:256 dev_watchdog+0xf5/0x197()
Hardware name: ProLiant DL380 G6
NETDEV WATCHDOG: eth0 (bnx2): transmit queue 2 timed out
Modules linked in: ipmi_devintf coretemp ipv6 xt_multiport
iptable_mangle xfs exportfs uinput hpwdt ipmi_si ipmi_msghandler bnx2
iTCO_wdt iTCO_vendor_support serio_raw microcode power_meter raid0 cciss
hpsa radeon ttm drm_kms_helper drm i2c_algo_bit i2c_core [last unloaded:
speedstep_lib]
Pid: 0, comm: swapper Tainted: G M 2.6.34.7-61.korg.fc13.x86_64 #1
Call Trace:
<IRQ> [<ffffffff8104d14f>] warn_slowpath_common+0x7c/0x94
[<ffffffff8104d1be>] warn_slowpath_fmt+0x41/0x43
[<ffffffff813b530f>] ? netif_tx_lock+0x44/0x6d
[<ffffffff813b542d>] dev_watchdog+0xf5/0x197
[<ffffffff81010261>] ? sched_clock+0x9/0xd
[<ffffffff8106b2f7>] ? sched_clock_cpu+0x44/0xce
[<ffffffff810594d6>] run_timer_softirq+0x1bf/0x263
[<ffffffff8106e3f7>] ? ktime_get+0x65/0xbe
[<ffffffff81053285>] __do_softirq+0xe5/0x1a6
[<ffffffff810726d0>] ? tick_program_event+0x2a/0x2c
[<ffffffff8100ab5c>] call_softirq+0x1c/0x30
[<ffffffff8100c342>] do_softirq+0x46/0x83
[<ffffffff810530f6>] irq_exit+0x3b/0x7d
[<ffffffff8144dc30>] smp_apic_timer_interrupt+0x8d/0x9b
[<ffffffff8100a613>] apic_timer_interrupt+0x13/0x20
<EOI> [<ffffffff8127d4c7>] ? acpi_idle_enter_bm+0x288/0x2bc
[<ffffffff8127d4c0>] ? acpi_idle_enter_bm+0x281/0x2bc
[<ffffffff8137596c>] ? menu_select+0x141/0x1f8
[<ffffffff81374b74>] cpuidle_idle_call+0x99/0xf1
[<ffffffff81008c22>] cpu_idle+0xaa/0xe4
[<ffffffff81440b89>] start_secondary+0x253/0x294
---[ end trace d6fc5aa3e2b641f7 ]---
bnx2 0000:02:00.0: eth0: DEBUG: intr_sem[0]
bnx2 0000:02:00.0: eth0: DEBUG: EMAC_TX_STATUS[00000008]
RPM_MGMT_PKT_CTRL[40000088]
bnx2 0000:02:00.0: eth0: DEBUG: MCP_STATE_P0[0003610e]
MCP_STATE_P1[0003600e]
bnx2 0000:02:00.0: eth0: DEBUG: HC_STATS_INTERRUPT_STATUS[01fb0004]
bnx2 0000:02:00.0: eth0: DEBUG: PBA[00000000]
bnx2 0000:02:00.0: eth0: NIC Copper Link is Down
bnx2 0000:02:00.0: eth0: NIC Copper Link is Up, 1000 Mbps full duplex
- John 'Warthog9' Hawley
^ permalink raw reply
* Re: TSO/GRO/LRO/somethingO breaks LVS on 2.6.36
From: Simon Kirby @ 2011-01-27 0:48 UTC (permalink / raw)
To: Simon Horman; +Cc: Eric Dumazet, netdev
In-Reply-To: <20110113063422.GB14643@verge.net.au>
On Thu, Jan 13, 2011 at 03:34:22PM +0900, Simon Horman wrote:
> Hi Simon,
>
> thanks for prodding me to respond to this post offline and sorry for not
> responding earlier.
>
> Firstly, I think that this is a receive-side problem so I don't believe
> that GSO (generic segmentation offload) or other transmit-side options are
> likely to have any affect.
>
> My understanding is that on the receive-side there are two options which
> when enabled can result in the behaviour that you describe.
>
> * LRO (large receive offload)
>
> You have this disabled, and assuming it really is disabled it
> shouldn't be causing a problem.
>
> * GRO (generic receive offload)
>
> This does not seem to be in the output of your ethtool commands at all.
> So I wonder if your ethtool is too old to support this option?
So, this was the case. Our ethtool (lenny) was too old to see the GRO
option, only GSO. Disabling GRO on eth1.39 has no effect, but disabling
it on eth1 caused it to stop receiving the merged frames, fixing the LVS
packet loss (due to no sending GSO support from LVS/IPVS).
Speaking of this, did your patch for LVS/IPVS GSO support go anywhere?
> In any case, I was able to reproduce the problem that you describe (or at
> least something very similar) using 2.6.36 with GRO enabled on eth1.1 and
> the problem did not manifest when I disabled GRO on eth1.1.
It worked for you to do ethtool -K eth1.1 gro off, then? For me on
2.6.37, it seemed to be that "ethtool -K eth1 gro off" was needed, even
though packets arrive on eth1.39.
Also, strangely, 2.6.35.4's default state (with no received merged frames)
has GRO on for eth1 but off for eth1.39:
# ethtool -k eth1
Offload parameters for eth1:
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: off
# ethtool -k eth1.39
Offload parameters for eth1.39:
rx-checksumming: on
tx-checksumming: off
scatter-gather: off
tcp-segmentation-offload: off
udp-fragmentation-offload: off
generic-segmentation-offload: off
generic-receive-offload: off
large-receive-offload: off
ntuple-filters: off
receive-hashing: off
If I set 2.6.37 to have all of the same options, I still see GRO frames
on 2.6.37 (tg3), which is weird.
Cheers,
Simon-
^ permalink raw reply
* Re: Simultaneous error on two different machines
From: Michael Chan @ 2011-01-27 0:52 UTC (permalink / raw)
To: J.H.; +Cc: netdev@vger.kernel.org
In-Reply-To: <4D40BE2A.2010903@kernel.org>
On Wed, 2011-01-26 at 16:36 -0800, J.H. wrote:
> Afternoon,
>
> Happened to trip over this yesterday on two machines (one an HP DL380 G6
> and one an HP DL 380 G7). Error was within minutes of each other,
> identical on the two boxes. Driver seemed to reset (as indicated
> below), however loads on the two boxes almost immediately skyrocketed,
> eventually leading to a more serious deadlock panic on one of the boxes,
> and me just rebooting the other.
>
> The chipsets involved seem to be:
>
> Ethernet controller: Broadcom Corporation NetXtreme II BCM5709 Gigabit
> Ethernet (rev 20)
>
> Kernel is from Fedora 13, 2.6.34.7-61.korg.fc13.x86_64. The only
> additional patches applied involve fixing an XFS bug.
>
Please check if you have this patch in your kernel:
commit c441b8d2cb2194b05550a558d6d95d8944e56a84
bnx2: Fix lost MSI-X problem on 5709 NICs.
^ permalink raw reply
* Re: TSO/GRO/LRO/somethingO breaks LVS on 2.6.36
From: Simon Horman @ 2011-01-27 1:36 UTC (permalink / raw)
To: Simon Kirby; +Cc: Eric Dumazet, netdev
In-Reply-To: <20110127004805.GB11578@hostway.ca>
On Wed, Jan 26, 2011 at 04:48:05PM -0800, Simon Kirby wrote:
> On Thu, Jan 13, 2011 at 03:34:22PM +0900, Simon Horman wrote:
>
> > Hi Simon,
> >
> > thanks for prodding me to respond to this post offline and sorry for not
> > responding earlier.
> >
> > Firstly, I think that this is a receive-side problem so I don't believe
> > that GSO (generic segmentation offload) or other transmit-side options are
> > likely to have any affect.
> >
> > My understanding is that on the receive-side there are two options which
> > when enabled can result in the behaviour that you describe.
> >
> > * LRO (large receive offload)
> >
> > You have this disabled, and assuming it really is disabled it
> > shouldn't be causing a problem.
> >
> > * GRO (generic receive offload)
> >
> > This does not seem to be in the output of your ethtool commands at all.
> > So I wonder if your ethtool is too old to support this option?
>
> So, this was the case. Our ethtool (lenny) was too old to see the GRO
> option, only GSO. Disabling GRO on eth1.39 has no effect, but disabling
> it on eth1 caused it to stop receiving the merged frames, fixing the LVS
> packet loss (due to no sending GSO support from LVS/IPVS).
>
> Speaking of this, did your patch for LVS/IPVS GSO support go anywhere?
The patch for IPVS GRO support has been merged and should appear in 2.6.39.
This is somewhat later than I previously anticipated due to a merge mix-up
on my part.
> > In any case, I was able to reproduce the problem that you describe (or at
> > least something very similar) using 2.6.36 with GRO enabled on eth1.1 and
> > the problem did not manifest when I disabled GRO on eth1.1.
>
> It worked for you to do ethtool -K eth1.1 gro off, then? For me on
> 2.6.37, it seemed to be that "ethtool -K eth1 gro off" was needed, even
> though packets arrive on eth1.39.
I will recheck my results, but in general I think it is a bit
of an open question as to how ethtool settings should be propagated
between related devices.
> Also, strangely, 2.6.35.4's default state (with no received merged frames)
> has GRO on for eth1 but off for eth1.39:
>
> # ethtool -k eth1
> Offload parameters for eth1:
> 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: off
>
> # ethtool -k eth1.39
> Offload parameters for eth1.39:
> rx-checksumming: on
> tx-checksumming: off
> scatter-gather: off
> tcp-segmentation-offload: off
> udp-fragmentation-offload: off
> generic-segmentation-offload: off
> generic-receive-offload: off
> large-receive-offload: off
> ntuple-filters: off
> receive-hashing: off
>
> If I set 2.6.37 to have all of the same options, I still see GRO frames
> on 2.6.37 (tg3), which is weird.
Yes that is a weird.
There has been quite a lot of work on VLANs recently and
I suspect that the behaviour that you are observing with 2.6.37
is a regression that occurred during that work. It would
be good to fix things to restore the 2.6.35 behaviour.
^ permalink raw reply
* i have business for you get back to us
From: MR CHEN GUAN @ 2011-01-26 1:30 UTC (permalink / raw)
I am Mr. Chen Guan, Foreign Operations Manager of the Bank of China (Hong Kong). I have a business suggestion for you of $17,300,000 Million USD. Further details Contact me via email (chenguan000000@yahoo.com.hk)
^ permalink raw reply
* Re: [net-next 08/12] ixgb: convert to new VLAN model
From: Jesse Gross @ 2011-01-27 3:53 UTC (permalink / raw)
To: Tantilov, Emil S
Cc: Kirsher, Jeffrey T, davem@davemloft.net, netdev@vger.kernel.org,
bphilips@novell.com, Pieper, Jeffrey E, Ben Hutchings
In-Reply-To: <EA929A9653AAE14F841771FB1DE5A136602DF59AA3@rrsmsx501.amr.corp.intel.com>
On Tue, Jan 25, 2011 at 10:20 AM, Tantilov, Emil S
<emil.s.tantilov@intel.com> wrote:
>>-----Original Message-----
>>From: Jesse Gross [mailto:jesse@nicira.com]
>>Sent: Tuesday, January 25, 2011 9:23 AM
>>To: Tantilov, Emil S
>>Cc: Kirsher, Jeffrey T; davem@davemloft.net; netdev@vger.kernel.org;
>>bphilips@novell.com; Pieper, Jeffrey E
>>Subject: Re: [net-next 08/12] ixgb: convert to new VLAN model
>>
>>On Sun, Jan 23, 2011 at 4:25 PM, Tantilov, Emil S
>><emil.s.tantilov@intel.com> wrote:
>>> Jesse Gross wrote:
>>>> On Thu, Jan 6, 2011 at 7:29 PM, <jeffrey.t.kirsher@intel.com> wrote:
>>>>> +static int ixgb_set_flags(struct net_device *netdev, u32 data) +{
>>>>> + struct ixgb_adapter *adapter = netdev_priv(netdev); +
>>>>> bool need_reset; + int rc;
>>>>> +
>>>>> + /*
>>>>> + * TX vlan insertion does not work per HW design when Rx
>>>>> stripping is + * disabled. Disable txvlan when rxvlan is
>>>>> off. + */ + if ((data & ETH_FLAG_RXVLAN) !=
>>>>> (netdev->features & NETIF_F_HW_VLAN_RX)) + data ^=
>>>>> ETH_FLAG_TXVLAN;
>>>>
>>>> Does this really do the right thing? If the RX vlan setting is
>>>> changed, it will do the opposite of what the user requested for TX
>>>> vlan?
>>>>
>>>> So if I start with both on (the default) and turn them both off in one
>>>> command (a valid setting), I will get RX off and TX on (an invalid
>>>> setting).
>>>>
>>>> Why not:
>>>>
>>>> if (!(data & ETH_FLAG_RXVLAN))
>>>> data &= ~ETH_FLAG_TXVLAN;
>>>
>>> Yeah that works for disabling rxvlan, but what if rxvlan is disabled, and
>>the user attempts to enable txvlan? At least our validation argued that we
>>should make it work both ways. Perhaps something like the following?
>>>
>>> if (!(data & ETH_FLAG_RXVLAN) &&
>>> (netdev->features & NETIF_F_HW_VLAN_TX))
>>> data &= ~ETH_FLAG_TXVLAN;
>>> else if (data & ETH_FLAG_TXVLAN)
>>> data |= ETH_FLAG_RXVLAN;
>>
>>I think the logic above does what you describe and will always result
>>in a consistent state. Turning dependent features on when needed is a
>>little bit inconsistent with the rest of Ethtool (for example, turning
>>on TSO when checksum offloading is off will not enable checksum
>>offloading, it will produce an error). However, I know that drivers
>
> That is the reason I asked, as I don't want to keep bouncing the patch back and forth. Personally I like the idea of helping the user and adjusting the flags to something that works rather than a generic error message.
I think it is fine to adjust things, especially where the restrictions
are hardware specific and the user is less likely to know what
settings are related. As long as it works, it doesn't matter too much
to me either way, so please do what you think is the most appropriate.
>
>>aren't completely consistent here and the most important part is that
>>it enforces valid states, so I don't have a strong opinion. Ben's
>>previous suggestion of Ethtool querying again after the operation and
>>reporting any flags that were automatically changed would help a lot
>>here.
>
> Sure, but I think a savvy user would always check the result of an ethtool command (ie. `ethtool -K` followed with `ethtool -k`, -A/-a, etc).
Probably, but it seems the less savviness required from the user the
better. Regardless, it doesn't affect anything here, it would just be
a change to the userspace tool.
^ permalink raw reply
* [PATCH net-next-2.6] net: fix dev_seq_next()
From: Eric Dumazet @ 2011-01-27 4:08 UTC (permalink / raw)
To: David Miller, Paul E. McKenney; +Cc: netdev
Paul, the following comment in include/linux/rculist.h is misleading :
"Why is there no list_empty_rcu()? Because list_empty() serves this
purpose..."
This is probably why I made the error ;)
list_empty() has a meaning only if state cannot change right after its
use.
In an rcu_read_lock() section, state _can_ change, so there is no way
list_empty() can be used at all.
Thanks
[PATCH net-next-2.6] net: fix dev_seq_next()
Commit c6d14c84566d (net: Introduce for_each_netdev_rcu() iterator)
added a race in dev_seq_next().
The rcu_dereference() call should be done _before_ testing the end of
list, or we might return a wrong net_device if a concurrent thread
changes net_device list under us.
Note : discovered thanks to a sparse warning :
net/core/dev.c:3919:9: error: incompatible types in comparison expression
(different address spaces)
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
CC: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
Given this was discovered by code analysis rather than a bug report, I
prepared a patch for net-next-2.6. Once fully tested, this could be
backported to 2.6.33
include/linux/netdevice.h | 9 ++++++++-
net/core/dev.c | 11 +++++++----
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 8858422..c7d7074 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1447,7 +1447,7 @@ static inline struct net_device *next_net_device_rcu(struct net_device *dev)
struct net *net;
net = dev_net(dev);
- lh = rcu_dereference(dev->dev_list.next);
+ lh = rcu_dereference(list_next_rcu(&dev->dev_list));
return lh == &net->dev_base_head ? NULL : net_device_entry(lh);
}
@@ -1457,6 +1457,13 @@ static inline struct net_device *first_net_device(struct net *net)
net_device_entry(net->dev_base_head.next);
}
+static inline struct net_device *first_net_device_rcu(struct net *net)
+{
+ struct list_head *lh = rcu_dereference(list_next_rcu(&net->dev_base_head));
+
+ return lh == &net->dev_base_head ? NULL : net_device_entry(lh);
+}
+
extern int netdev_boot_setup_check(struct net_device *dev);
extern unsigned long netdev_boot_base(const char *prefix, int unit);
extern struct net_device *dev_getbyhwaddr_rcu(struct net *net, unsigned short type,
diff --git a/net/core/dev.c b/net/core/dev.c
index 1b4c07f..ddd5df2 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4051,12 +4051,15 @@ void *dev_seq_start(struct seq_file *seq, loff_t *pos)
void *dev_seq_next(struct seq_file *seq, void *v, loff_t *pos)
{
- struct net_device *dev = (v == SEQ_START_TOKEN) ?
- first_net_device(seq_file_net(seq)) :
- next_net_device((struct net_device *)v);
+ struct net_device *dev = v;
+
+ if (v == SEQ_START_TOKEN)
+ dev = first_net_device_rcu(seq_file_net(seq));
+ else
+ dev = next_net_device_rcu(dev);
++*pos;
- return rcu_dereference(dev);
+ return dev;
}
void dev_seq_stop(struct seq_file *seq, void *v)
^ permalink raw reply related
* Re: [net-next 08/12] ixgb: convert to new VLAN model
From: Ben Hutchings @ 2011-01-27 4:18 UTC (permalink / raw)
To: Jesse Gross
Cc: Tantilov, Emil S, Kirsher, Jeffrey T, davem@davemloft.net,
netdev@vger.kernel.org, bphilips@novell.com, Pieper, Jeffrey E
In-Reply-To: <AANLkTi=RU11ibzd3c9sqCLL0pNowvx1_ow7C=qWVoPMt@mail.gmail.com>
On Wed, 2011-01-26 at 19:53 -0800, Jesse Gross wrote:
> On Tue, Jan 25, 2011 at 10:20 AM, Tantilov, Emil S
> <emil.s.tantilov@intel.com> wrote:
[...]
> > Sure, but I think a savvy user would always check the result of an
> > ethtool command (ie. `ethtool -K` followed with `ethtool -k`, -A/-a,
> > etc).
>
> Probably, but it seems the less savviness required from the user the
> better. Regardless, it doesn't affect anything here, it would just be
> a change to the userspace tool.
I am intending to modify ethtool so that it will report any other
offload settings that were changed automatically.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* [PATCH net-next-2.6] drivers/net: remove some rcu sparse warnings
From: Eric Dumazet @ 2011-01-27 5:28 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Michael Chan, Arnd Bergmann, Eilon Greenstein
Add missing __rcu annotations and helpers.
minor : Fix some rcu_dereference() calls in macvtap
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
CC: Arnd Bergmann <arnd@arndb.de>
CC: Michael Chan <mchan@broadcom.com>
CC: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/bnx2.c | 6 ++++--
drivers/net/bnx2.h | 2 +-
drivers/net/bnx2x/bnx2x.h | 2 +-
drivers/net/bnx2x/bnx2x_main.c | 3 ++-
drivers/net/cnic.c | 27 ++++++++++++++++++---------
drivers/net/cnic.h | 2 +-
drivers/net/hamradio/bpqether.c | 5 +++--
drivers/net/macvtap.c | 18 ++++++++++--------
8 files changed, 40 insertions(+), 25 deletions(-)
diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index 231aa97..2c3d747 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -435,7 +435,8 @@ bnx2_cnic_stop(struct bnx2 *bp)
struct cnic_ctl_info info;
mutex_lock(&bp->cnic_lock);
- c_ops = bp->cnic_ops;
+ c_ops = rcu_dereference_protected(bp->cnic_ops,
+ lockdep_is_held(&bp->cnic_lock));
if (c_ops) {
info.cmd = CNIC_CTL_STOP_CMD;
c_ops->cnic_ctl(bp->cnic_data, &info);
@@ -450,7 +451,8 @@ bnx2_cnic_start(struct bnx2 *bp)
struct cnic_ctl_info info;
mutex_lock(&bp->cnic_lock);
- c_ops = bp->cnic_ops;
+ c_ops = rcu_dereference_protected(bp->cnic_ops,
+ lockdep_is_held(&bp->cnic_lock));
if (c_ops) {
if (!(bp->flags & BNX2_FLAG_USING_MSIX)) {
struct bnx2_napi *bnapi = &bp->bnx2_napi[0];
diff --git a/drivers/net/bnx2.h b/drivers/net/bnx2.h
index 5488a2e..6824eba 100644
--- a/drivers/net/bnx2.h
+++ b/drivers/net/bnx2.h
@@ -6758,7 +6758,7 @@ struct bnx2 {
u32 tx_wake_thresh;
#ifdef BCM_CNIC
- struct cnic_ops *cnic_ops;
+ struct cnic_ops __rcu *cnic_ops;
void *cnic_data;
#endif
diff --git a/drivers/net/bnx2x/bnx2x.h b/drivers/net/bnx2x/bnx2x.h
index 8e41837..dfdb9b5 100644
--- a/drivers/net/bnx2x/bnx2x.h
+++ b/drivers/net/bnx2x/bnx2x.h
@@ -1110,7 +1110,7 @@ struct bnx2x {
#define BNX2X_CNIC_FLAG_MAC_SET 1
void *t2;
dma_addr_t t2_mapping;
- struct cnic_ops *cnic_ops;
+ struct cnic_ops __rcu *cnic_ops;
void *cnic_data;
u32 cnic_tag;
struct cnic_eth_dev cnic_eth_dev;
diff --git a/drivers/net/bnx2x/bnx2x_main.c b/drivers/net/bnx2x/bnx2x_main.c
index 8cdcf5b..a2a1bc4 100644
--- a/drivers/net/bnx2x/bnx2x_main.c
+++ b/drivers/net/bnx2x/bnx2x_main.c
@@ -9862,7 +9862,8 @@ static int bnx2x_cnic_ctl_send(struct bnx2x *bp, struct cnic_ctl_info *ctl)
int rc = 0;
mutex_lock(&bp->cnic_mutex);
- c_ops = bp->cnic_ops;
+ c_ops = rcu_dereference_protected(bp->cnic_ops,
+ lockdep_is_held(&bp->cnic_mutex));
if (c_ops)
rc = c_ops->cnic_ctl(bp->cnic_data, ctl);
mutex_unlock(&bp->cnic_mutex);
diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
index 263a294..e12049e 100644
--- a/drivers/net/cnic.c
+++ b/drivers/net/cnic.c
@@ -65,7 +65,14 @@ static LIST_HEAD(cnic_udev_list);
static DEFINE_RWLOCK(cnic_dev_lock);
static DEFINE_MUTEX(cnic_lock);
-static struct cnic_ulp_ops *cnic_ulp_tbl[MAX_CNIC_ULP_TYPE];
+static struct cnic_ulp_ops __rcu *cnic_ulp_tbl[MAX_CNIC_ULP_TYPE];
+
+/* helper function, assuming cnic_lock is held */
+static inline struct cnic_ulp_ops *cnic_ulp_tbl_prot(int type)
+{
+ return rcu_dereference_protected(cnic_ulp_tbl[type],
+ lockdep_is_held(&cnic_lock));
+}
static int cnic_service_bnx2(void *, void *);
static int cnic_service_bnx2x(void *, void *);
@@ -435,7 +442,7 @@ int cnic_register_driver(int ulp_type, struct cnic_ulp_ops *ulp_ops)
return -EINVAL;
}
mutex_lock(&cnic_lock);
- if (cnic_ulp_tbl[ulp_type]) {
+ if (cnic_ulp_tbl_prot(ulp_type)) {
pr_err("%s: Type %d has already been registered\n",
__func__, ulp_type);
mutex_unlock(&cnic_lock);
@@ -478,7 +485,7 @@ int cnic_unregister_driver(int ulp_type)
return -EINVAL;
}
mutex_lock(&cnic_lock);
- ulp_ops = cnic_ulp_tbl[ulp_type];
+ ulp_ops = cnic_ulp_tbl_prot(ulp_type);
if (!ulp_ops) {
pr_err("%s: Type %d has not been registered\n",
__func__, ulp_type);
@@ -529,7 +536,7 @@ static int cnic_register_device(struct cnic_dev *dev, int ulp_type,
return -EINVAL;
}
mutex_lock(&cnic_lock);
- if (cnic_ulp_tbl[ulp_type] == NULL) {
+ if (cnic_ulp_tbl_prot(ulp_type) == NULL) {
pr_err("%s: Driver with type %d has not been registered\n",
__func__, ulp_type);
mutex_unlock(&cnic_lock);
@@ -544,7 +551,7 @@ static int cnic_register_device(struct cnic_dev *dev, int ulp_type,
clear_bit(ULP_F_START, &cp->ulp_flags[ulp_type]);
cp->ulp_handle[ulp_type] = ulp_ctx;
- ulp_ops = cnic_ulp_tbl[ulp_type];
+ ulp_ops = cnic_ulp_tbl_prot(ulp_type);
rcu_assign_pointer(cp->ulp_ops[ulp_type], ulp_ops);
cnic_hold(dev);
@@ -2953,7 +2960,8 @@ static void cnic_ulp_stop(struct cnic_dev *dev)
struct cnic_ulp_ops *ulp_ops;
mutex_lock(&cnic_lock);
- ulp_ops = cp->ulp_ops[if_type];
+ ulp_ops = rcu_dereference_protected(cp->ulp_ops[if_type],
+ lockdep_is_held(&cnic_lock));
if (!ulp_ops) {
mutex_unlock(&cnic_lock);
continue;
@@ -2977,7 +2985,8 @@ static void cnic_ulp_start(struct cnic_dev *dev)
struct cnic_ulp_ops *ulp_ops;
mutex_lock(&cnic_lock);
- ulp_ops = cp->ulp_ops[if_type];
+ ulp_ops = rcu_dereference_protected(cp->ulp_ops[if_type],
+ lockdep_is_held(&cnic_lock));
if (!ulp_ops || !ulp_ops->cnic_start) {
mutex_unlock(&cnic_lock);
continue;
@@ -3041,7 +3050,7 @@ static void cnic_ulp_init(struct cnic_dev *dev)
struct cnic_ulp_ops *ulp_ops;
mutex_lock(&cnic_lock);
- ulp_ops = cnic_ulp_tbl[i];
+ ulp_ops = cnic_ulp_tbl_prot(i);
if (!ulp_ops || !ulp_ops->cnic_init) {
mutex_unlock(&cnic_lock);
continue;
@@ -3065,7 +3074,7 @@ static void cnic_ulp_exit(struct cnic_dev *dev)
struct cnic_ulp_ops *ulp_ops;
mutex_lock(&cnic_lock);
- ulp_ops = cnic_ulp_tbl[i];
+ ulp_ops = cnic_ulp_tbl_prot(i);
if (!ulp_ops || !ulp_ops->cnic_exit) {
mutex_unlock(&cnic_lock);
continue;
diff --git a/drivers/net/cnic.h b/drivers/net/cnic.h
index b328f6c..4456260 100644
--- a/drivers/net/cnic.h
+++ b/drivers/net/cnic.h
@@ -220,7 +220,7 @@ struct cnic_local {
#define ULP_F_INIT 0
#define ULP_F_START 1
#define ULP_F_CALL_PENDING 2
- struct cnic_ulp_ops *ulp_ops[MAX_CNIC_ULP_TYPE];
+ struct cnic_ulp_ops __rcu *ulp_ops[MAX_CNIC_ULP_TYPE];
unsigned long cnic_local_flags;
#define CNIC_LCL_FL_KWQ_INIT 0x0
diff --git a/drivers/net/hamradio/bpqether.c b/drivers/net/hamradio/bpqether.c
index ac1d323..8931168 100644
--- a/drivers/net/hamradio/bpqether.c
+++ b/drivers/net/hamradio/bpqether.c
@@ -400,13 +400,14 @@ static void *bpq_seq_start(struct seq_file *seq, loff_t *pos)
static void *bpq_seq_next(struct seq_file *seq, void *v, loff_t *pos)
{
struct list_head *p;
+ struct bpqdev *bpqdev = v;
++*pos;
if (v == SEQ_START_TOKEN)
- p = rcu_dereference(bpq_devices.next);
+ p = rcu_dereference(list_next_rcu(&bpq_devices));
else
- p = rcu_dereference(((struct bpqdev *)v)->bpq_list.next);
+ p = rcu_dereference(list_next_rcu(&bpqdev->bpq_list));
return (p == &bpq_devices) ? NULL
: list_entry(p, struct bpqdev, bpq_list);
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 5933621..2300e45 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -39,7 +39,7 @@ struct macvtap_queue {
struct socket sock;
struct socket_wq wq;
int vnet_hdr_sz;
- struct macvlan_dev *vlan;
+ struct macvlan_dev __rcu *vlan;
struct file *file;
unsigned int flags;
};
@@ -141,7 +141,8 @@ static void macvtap_put_queue(struct macvtap_queue *q)
struct macvlan_dev *vlan;
spin_lock(&macvtap_lock);
- vlan = rcu_dereference(q->vlan);
+ vlan = rcu_dereference_protected(q->vlan,
+ lockdep_is_held(&macvtap_lock));
if (vlan) {
int index = get_slot(vlan, q);
@@ -219,7 +220,8 @@ static void macvtap_del_queues(struct net_device *dev)
/* macvtap_put_queue can free some slots, so go through all slots */
spin_lock(&macvtap_lock);
for (i = 0; i < MAX_MACVTAP_QUEUES && vlan->numvtaps; i++) {
- q = rcu_dereference(vlan->taps[i]);
+ q = rcu_dereference_protected(vlan->taps[i],
+ lockdep_is_held(&macvtap_lock));
if (q) {
qlist[j++] = q;
rcu_assign_pointer(vlan->taps[i], NULL);
@@ -569,7 +571,7 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q,
}
rcu_read_lock_bh();
- vlan = rcu_dereference(q->vlan);
+ vlan = rcu_dereference_bh(q->vlan);
if (vlan)
macvlan_start_xmit(skb, vlan->dev);
else
@@ -583,7 +585,7 @@ err_kfree:
err:
rcu_read_lock_bh();
- vlan = rcu_dereference(q->vlan);
+ vlan = rcu_dereference_bh(q->vlan);
if (vlan)
vlan->dev->stats.tx_dropped++;
rcu_read_unlock_bh();
@@ -631,7 +633,7 @@ static ssize_t macvtap_put_user(struct macvtap_queue *q,
ret = skb_copy_datagram_const_iovec(skb, 0, iv, vnet_hdr_len, len);
rcu_read_lock_bh();
- vlan = rcu_dereference(q->vlan);
+ vlan = rcu_dereference_bh(q->vlan);
if (vlan)
macvlan_count_rx(vlan, len, ret == 0, 0);
rcu_read_unlock_bh();
@@ -727,7 +729,7 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
case TUNGETIFF:
rcu_read_lock_bh();
- vlan = rcu_dereference(q->vlan);
+ vlan = rcu_dereference_bh(q->vlan);
if (vlan)
dev_hold(vlan->dev);
rcu_read_unlock_bh();
@@ -736,7 +738,7 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
return -ENOLINK;
ret = 0;
- if (copy_to_user(&ifr->ifr_name, q->vlan->dev->name, IFNAMSIZ) ||
+ if (copy_to_user(&ifr->ifr_name, vlan->dev->name, IFNAMSIZ) ||
put_user(q->flags, &ifr->ifr_flags))
ret = -EFAULT;
dev_put(vlan->dev);
^ permalink raw reply related
* Re: [PATCH net-next-2.6] net_sched: sch_mqprio: dont leak kernel memory
From: Joe Perches @ 2011-01-27 6:04 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, netdev, john.r.fastabend
In-Reply-To: <20110126.115530.226756606.davem@davemloft.net>
On Wed, 2011-01-26 at 11:55 -0800, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> Date: Wed, 26 Jan 2011 09:43:43 -0800
> > On Wed, 2011-01-26 at 18:21 +0100, Eric Dumazet wrote:
> >> mqprio_dump() should make sure all fields of struct tc_mqprio_qopt are
> >> initialized.
> >> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> >> CC: John Fastabend <john.r.fastabend@intel.com>
> >> ---
> >> net/sched/sch_mqprio.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/net/sched/sch_mqprio.c b/net/sched/sch_mqprio.c
> >> index fbc6f53..effd4ee 100644
> >> --- a/net/sched/sch_mqprio.c
> >> +++ b/net/sched/sch_mqprio.c
> >> @@ -215,7 +215,7 @@ static int mqprio_dump(struct Qdisc *sch, struct sk_buff *skb)
> >> struct net_device *dev = qdisc_dev(sch);
> >> struct mqprio_sched *priv = qdisc_priv(sch);
> >> unsigned char *b = skb_tail_pointer(skb);
> >> - struct tc_mqprio_qopt opt;
> >> + struct tc_mqprio_qopt opt = { 0 };
> > I think the best style to use memset so that any
> > possible struct padding is guaranteed to be zeroed.
> Such padding does not exist, and we won't add such padding since this is
> a user visible data structure and thus whose layout is cast in stone.
/* MQPRIO */
#define TC_QOPT_BITMASK 15
#define TC_QOPT_MAX_QUEUE 16
struct tc_mqprio_qopt {
__u8 num_tc;
__u8 prio_tc_map[TC_QOPT_BITMASK + 1];
__u8 hw;
__u16 count[TC_QOPT_MAX_QUEUE];
__u16 offset[TC_QOPT_MAX_QUEUE];
};
I believe this struct needs to be declared __packed.
It could otherwise be 24 bytes not 22.
Or if char array declarations have a different
alignment requirement, could be any size.
memset is better than {0}.
cheers, Joe
^ permalink raw reply
* Re: [PATCH net-next-2.6] net_sched: sch_mqprio: dont leak kernel memory
From: Changli Gao @ 2011-01-27 6:54 UTC (permalink / raw)
To: Joe Perches; +Cc: David Miller, eric.dumazet, netdev, john.r.fastabend
In-Reply-To: <1296108251.2448.183.camel@Joe-Laptop>
On Thu, Jan 27, 2011 at 2:04 PM, Joe Perches <joe@perches.com> wrote:
> On Wed, 2011-01-26 at 11:55 -0800, David Miller wrote:
>> From: Joe Perches <joe@perches.com>
>> Date: Wed, 26 Jan 2011 09:43:43 -0800
>> > On Wed, 2011-01-26 at 18:21 +0100, Eric Dumazet wrote:
>> >> mqprio_dump() should make sure all fields of struct tc_mqprio_qopt are
>> >> initialized.
>> >> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>> >> CC: John Fastabend <john.r.fastabend@intel.com>
>> >> ---
>> >> net/sched/sch_mqprio.c | 2 +-
>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/net/sched/sch_mqprio.c b/net/sched/sch_mqprio.c
>> >> index fbc6f53..effd4ee 100644
>> >> --- a/net/sched/sch_mqprio.c
>> >> +++ b/net/sched/sch_mqprio.c
>> >> @@ -215,7 +215,7 @@ static int mqprio_dump(struct Qdisc *sch, struct sk_buff *skb)
>> >> struct net_device *dev = qdisc_dev(sch);
>> >> struct mqprio_sched *priv = qdisc_priv(sch);
>> >> unsigned char *b = skb_tail_pointer(skb);
>> >> - struct tc_mqprio_qopt opt;
>> >> + struct tc_mqprio_qopt opt = { 0 };
>> > I think the best style to use memset so that any
>> > possible struct padding is guaranteed to be zeroed.
>> Such padding does not exist, and we won't add such padding since this is
>> a user visible data structure and thus whose layout is cast in stone.
>
> /* MQPRIO */
> #define TC_QOPT_BITMASK 15
> #define TC_QOPT_MAX_QUEUE 16
>
> struct tc_mqprio_qopt {
> __u8 num_tc; // 1
> __u8 prio_tc_map[TC_QOPT_BITMASK + 1]; // 16
> __u8 hw; // 1
> __u16 count[TC_QOPT_MAX_QUEUE]; // 32
> __u16 offset[TC_QOPT_MAX_QUEUE]; //32
> };
>
> I believe this struct needs to be declared __packed.
>
> It could otherwise be 24 bytes not 22.
> Or if char array declarations have a different
> alignment requirement, could be any size.
>
The total size is 1 + 16 + 1 + 32 + 32 = 82.
How do you get 24 or 22?
> memset is better than {0}.
>
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH net-next-2.6] net_sched: sch_mqprio: dont leak kernel memory
From: Eric Dumazet @ 2011-01-27 7:04 UTC (permalink / raw)
To: Joe Perches; +Cc: David Miller, netdev, john.r.fastabend
In-Reply-To: <1296108251.2448.183.camel@Joe-Laptop>
Le mercredi 26 janvier 2011 à 22:04 -0800, Joe Perches a écrit :
> /* MQPRIO */
> #define TC_QOPT_BITMASK 15
> #define TC_QOPT_MAX_QUEUE 16
>
> struct tc_mqprio_qopt {
> __u8 num_tc;
> __u8 prio_tc_map[TC_QOPT_BITMASK + 1];
> __u8 hw;
> __u16 count[TC_QOPT_MAX_QUEUE];
> __u16 offset[TC_QOPT_MAX_QUEUE];
> };
>
> I believe this struct needs to be declared __packed.
>
Oh my god. Yet another ugly thing.
> It could otherwise be 24 bytes not 22.
22 ? You are kidding probably. Its 82 exactly.
Listen, I doubled check my patch, its good, while your rants are lazy.
> Or if char array declarations have a different
> alignment requirement, could be any size.
>
If if if... could could could...
> memset is better than {0}.
>
You never stop do you ?
The bigger object is u16, therefore alignof() is 2, not 4
No ABI requires a short (u16) is aligned on 4 byte boundary.
If you find a compiler not respecting this, you can bet linux wont run
at all if compiled with it. mqprio 'potential 2 bytes leak' will hardly
be a problem.
^ permalink raw reply
* Re: TSO/GRO/LRO/somethingO breaks LVS on 2.6.36
From: Eric Dumazet @ 2011-01-27 7:42 UTC (permalink / raw)
To: Simon Kirby; +Cc: Simon Horman, netdev
In-Reply-To: <20110127004805.GB11578@hostway.ca>
Le mercredi 26 janvier 2011 à 16:48 -0800, Simon Kirby a écrit :
> On Thu, Jan 13, 2011 at 03:34:22PM +0900, Simon Horman wrote:
>
> > Hi Simon,
> >
> > thanks for prodding me to respond to this post offline and sorry for not
> > responding earlier.
> >
> > Firstly, I think that this is a receive-side problem so I don't believe
> > that GSO (generic segmentation offload) or other transmit-side options are
> > likely to have any affect.
> >
> > My understanding is that on the receive-side there are two options which
> > when enabled can result in the behaviour that you describe.
> >
> > * LRO (large receive offload)
> >
> > You have this disabled, and assuming it really is disabled it
> > shouldn't be causing a problem.
> >
> > * GRO (generic receive offload)
> >
> > This does not seem to be in the output of your ethtool commands at all.
> > So I wonder if your ethtool is too old to support this option?
>
> So, this was the case. Our ethtool (lenny) was too old to see the GRO
> option, only GSO. Disabling GRO on eth1.39 has no effect, but disabling
> it on eth1 caused it to stop receiving the merged frames, fixing the LVS
> packet loss (due to no sending GSO support from LVS/IPVS).
>
> Speaking of this, did your patch for LVS/IPVS GSO support go anywhere?
>
> > In any case, I was able to reproduce the problem that you describe (or at
> > least something very similar) using 2.6.36 with GRO enabled on eth1.1 and
> > the problem did not manifest when I disabled GRO on eth1.1.
>
> It worked for you to do ethtool -K eth1.1 gro off, then? For me on
> 2.6.37, it seemed to be that "ethtool -K eth1 gro off" was needed, even
> though packets arrive on eth1.39.
>
> Also, strangely, 2.6.35.4's default state (with no received merged frames)
> has GRO on for eth1 but off for eth1.39:
>
> # ethtool -k eth1
> Offload parameters for eth1:
> 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: off
>
> # ethtool -k eth1.39
> Offload parameters for eth1.39:
> rx-checksumming: on
> tx-checksumming: off
> scatter-gather: off
> tcp-segmentation-offload: off
> udp-fragmentation-offload: off
> generic-segmentation-offload: off
> generic-receive-offload: off
> large-receive-offload: off
> ntuple-filters: off
> receive-hashing: off
>
> If I set 2.6.37 to have all of the same options, I still see GRO frames
> on 2.6.37 (tg3), which is weird.
>
Weird maybe, but GRO check/handling is done in dev_gro_receive(), on
eth1 receive path.
Frames are assembled by GRO layer using tg3 NAPI structure (holding GRO
machine state) before being delivered to eth1.39
It would be useless/expensive to add another GRO layer on eth1.39
We might not report GRO state on vlan/bonding (or reflect real device
GRO state)
^ permalink raw reply
* Re: [PATCH] pch_gbe: Fix the issue that the receiving data is not normal.
From: Toshiharu Okada @ 2011-01-27 8:45 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-kernel, qi.wang, yong.y.wang, andrew.chih.howe.khor,
joel.clark, kok.howg.ewe
In-Reply-To: <20110125.133240.59688304.davem@davemloft.net>
Hi David
Thank you for your comment.
I will confirm them and will submit the patch modified.
Best regards
Toshiharu Okada(OKI semiconductor)
----- Original Message -----
From: "David Miller" <davem@davemloft.net>
To: <toshiharu-linux@dsn.okisemi.com>
Cc: <netdev@vger.kernel.org>; <linux-kernel@vger.kernel.org>;
<qi.wang@intel.com>; <yong.y.wang@intel.com>;
<andrew.chih.howe.khor@intel.com>; <joel.clark@intel.com>;
<kok.howg.ewe@intel.com>
Sent: Wednesday, January 26, 2011 6:32 AM
Subject: Re: [PATCH] pch_gbe: Fix the issue that the receiving data is not
normal.
From: Toshiharu Okada <toshiharu-linux@dsn.okisemi.com>
Date: Mon, 24 Jan 2011 13:43:31 +0900
> This PCH_GBE driver had an issue that the receiving data is not normal.
> This driver had not removed correctly the padding data
> which the DMA include in receiving data.
>
> This patch fixed this issue.
>
> Signed-off-by: Toshiharu Okada <toshiharu-linux@dsn.okisemi.com>
There are bugs in these changes:
> if (skb_copy_flag) { /* recycle skb */
> struct sk_buff *new_skb;
> new_skb =
> - netdev_alloc_skb(netdev,
> - length + NET_IP_ALIGN);
> + netdev_alloc_skb(netdev, length);
> if (new_skb) {
> if (!skb_padding_flag) {
> skb_reserve(new_skb,
> - NET_IP_ALIGN);
> + PCH_GBE_DMA_PADDING);
> }
> memcpy(new_skb->data, skb->data,
> length);
If "!skb_padding_flag" then you will write past the end of the SKB
data in that memcpy.
You cannot allocate only "length" then proceed to reserve
PCH_GBE_DMA_PADDING
and then add "length" worth of data on top of that. In such a cause you
must allocate at least "length + PCH_GBE_DMA_PADDING".
Furthermore you _MUST_ respect NET_IP_ALIGN. Some platforms set this value
to "0", because otherwise performance suffers greatly.
There are two seperate issues, removing the padding bytes provided by
the device, and aligning the IP headers as wanted by the cpu
architecutre. Therefore they should be handled seperately, and we
therefore should still see references to NET_IP_ALIGN in your patch.
^ permalink raw reply
* Re: [PATCH net-next-2.6] drivers/net: remove some rcu sparse warnings
From: Arnd Bergmann @ 2011-01-27 9:22 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Michael Chan, Eilon Greenstein
In-Reply-To: <1296106103.1783.114.camel@edumazet-laptop>
On Thursday 27 January 2011, Eric Dumazet wrote:
> Add missing __rcu annotations and helpers.
> minor : Fix some rcu_dereference() calls in macvtap
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> CC: Arnd Bergmann <arnd@arndb.de>
> CC: Michael Chan <mchan@broadcom.com>
> CC: Eilon Greenstein <eilong@broadcom.com>
Macvtap bits look good, thanks!
Acked-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply
* [PATCH] netfilter: CONNMARK: support save the mark of the master connection
From: Changli Gao @ 2011-01-27 9:38 UTC (permalink / raw)
To: Patrick McHardy; +Cc: David S. Miller, netfilter-devel, netdev, Changli Gao
In some cases(Policy routing), it is expected that all the sub-connections
share the same mark with their master.
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
include/linux/netfilter/xt_connmark.h | 3 ++-
net/netfilter/xt_connmark.c | 15 +++++++++++++++
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/include/linux/netfilter/xt_connmark.h b/include/linux/netfilter/xt_connmark.h
index efc17a8..4b513f8 100644
--- a/include/linux/netfilter/xt_connmark.h
+++ b/include/linux/netfilter/xt_connmark.h
@@ -15,7 +15,8 @@
enum {
XT_CONNMARK_SET = 0,
XT_CONNMARK_SAVE,
- XT_CONNMARK_RESTORE
+ XT_CONNMARK_RESTORE,
+ XT_CONNMARK_SAVE_MASTER,
};
struct xt_connmark_tginfo1 {
diff --git a/net/netfilter/xt_connmark.c b/net/netfilter/xt_connmark.c
index 7278145..4207bb6 100644
--- a/net/netfilter/xt_connmark.c
+++ b/net/netfilter/xt_connmark.c
@@ -69,6 +69,21 @@ connmark_tg(struct sk_buff *skb, const struct xt_action_param *par)
(ct->mark & info->ctmask);
skb->mark = newmark;
break;
+ case XT_CONNMARK_SAVE_MASTER:
+ if (ct->master) {
+ struct nf_conn *master;
+
+ master = ct->master;
+ while (master->master)
+ master = master->master;
+ newmark = (ct->mark & ~info->ctmask) ^
+ (master->mark & info->nfmask);
+ if (ct->mark != newmark) {
+ ct->mark = newmark;
+ nf_conntrack_event_cache(IPCT_MARK, ct);
+ }
+ }
+ break;
}
return XT_CONTINUE;
^ permalink raw reply related
* Re: [PATCH] netfilter: CONNMARK: support save the mark of the master connection
From: Patrick McHardy @ 2011-01-27 9:49 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, netfilter-devel, netdev
In-Reply-To: <1296121124-7016-1-git-send-email-xiaosuo@gmail.com>
Am 27.01.2011 10:38, schrieb Changli Gao:
> In some cases(Policy routing), it is expected that all the sub-connections
> share the same mark with their master.
We already automatically mark expected connections with the mark of the
master. What purpose does your patch serve on top of that?
^ permalink raw reply
* Re: [PATCH] netfilter: CONNMARK: support save the mark of the master connection
From: Changli Gao @ 2011-01-27 10:01 UTC (permalink / raw)
To: Patrick McHardy; +Cc: David S. Miller, netfilter-devel, netdev
In-Reply-To: <4D413F96.2070309@trash.net>
On Thu, Jan 27, 2011 at 5:49 PM, Patrick McHardy <kaber@trash.net> wrote:
> Am 27.01.2011 10:38, schrieb Changli Gao:
>> In some cases(Policy routing), it is expected that all the sub-connections
>> share the same mark with their master.
>
> We already automatically mark expected connections with the mark of the
> master. What purpose does your patch serve on top of that?
>
>
It is done in init_conntrack(), indeed. Sorry for the noise. Thanks.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [RFC PATCH] net: Implement read-only protection and COW'ing of metrics.
From: Eric Dumazet @ 2011-01-27 10:01 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20110126.152538.260074157.davem@davemloft.net>
Le mercredi 26 janvier 2011 à 15:25 -0800, David Miller a écrit :
> Eric, thanks again for your feedback. I've taken a stab at fixing the
> various races, in particular the one you discovered about metrics
> sharing and how this interacts with fib_info releases.
>
> What I've choosen to do is two-fold:
>
> 1) Update ->_metrics atomically with cmpxchg once a route becomes publicly
> visible.
>
> 2) Remember and grab a reference to the fib_info for shared read-only
> metrics in rt->fi, then release it once the metrics regerence goes
> away.
>
> It sounds expensive but hear me out :-)
>
> First of all, at rt_set_nexthop() time, the atomic we use to grab a
> ref to the fib_info is replacing a 60-byte memcpy() into the dst
> metrics.
>
> Next, the ->_metrics atomic to un-COW the metrics at destroy time
> might in fact be overkill. Especially once writable metrics live in
> the inetpeer cache (that's the next set of patches after this one).
>
> Finally, once this change is stabilized we can be a lot smarter about
> what we do at the time an entry is created. For example, when a route
> is looked up for a TCP socket, we essentially know we are going to COW
> the route %99.99999 of the time. So we can pass a hint into TCP's
> route lookups in the flow flags field telling it to pre-COW the route.
>
> TCP pre-COW'ing of metrics will thus save several atomics.
>
> Anyways, here is the patch, it is only build tested at this point, but
> I wanted to get feedback from you about the basic gist of things
> as soon as possible.
>
> Thanks!
>
Thanks David, I read this (I am a bit busy preparing my travel to
Reunion/Maurice islands). This looks pretty nice. I have one comment :
>
> +u32 *dst_cow_metrics_generic(struct dst_entry *dst, unsigned long old)
> +{
> + u32 *p = kmalloc(sizeof(u32) * RTAX_MAX, GFP_ATOMIC);
> +
> + if (p) {
> + u32 *old_p = __DST_METRICS_PTR(old);
> + unsigned long prev, new;
> +
> + memcpy(p, old_p, sizeof(u32) * RTAX_MAX);
> +
> + new = (unsigned long) p;
> + prev = cmpxchg(&dst->_metrics, old, new);
> +
> + if (prev != old) {
> + kfree(p);
> + p = __DST_METRICS_PTR(prev);
> + if (prev & DST_METRICS_READ_ONLY)
> + p = NULL;
> + }
> + }
> + return p;
> +}
> +EXPORT_SYMBOL(dst_cow_metrics_generic);
> +
...
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 3e5b7cc..7fc6301 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -152,6 +152,36 @@ static void ipv4_dst_ifdown(struct dst_entry *dst, struct net_device *dev,
> {
> }
>
> +static u32 *ipv4_cow_metrics(struct dst_entry *dst, unsigned long old)
> +{
> + u32 *p = kmalloc(sizeof(u32) * RTAX_MAX, GFP_ATOMIC);
> +
> + if (p) {
> + u32 *old_p = __DST_METRICS_PTR(old);
> + unsigned long prev, new;
> +
> + memcpy(p, old_p, sizeof(u32) * RTAX_MAX);
> +
> + new = (unsigned long) p;
> + prev = cmpxchg(&dst->_metrics, old, new);
> +
> + if (prev != old) {
> + kfree(p);
> + p = __DST_METRICS_PTR(prev);
> + if (prev & DST_METRICS_READ_ONLY)
> + p = NULL;
> + } else {
Hmm, I first asked myself why you dont use dst_cow_metrics_generic() to
perform the generic allocation, but saw following :
> + struct rtable *rt = (struct rtable *) dst;
> +
Since you use cmpxchg() to permut the dst->_metrics, I feel this rt->fi
needs some protection as well. Maybe store fi pointer inside the metrics
instead of dst, or else you need a spinlock to perform the whole
transaction (change dst->_metrics & rt->fi) ?
> + if (rt->fi) {
> + fib_info_put(rt->fi);
> + rt->fi = NULL;
> + }
> + }
> + }
> + return p;
> +}
> +
^ permalink raw reply
* Re: [PATCH 12/20] x25: remove the BKL
From: Andrew Hendry @ 2011-01-27 10:07 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linux-kernel, linux-x25, netdev
In-Reply-To: <1295993854-4971-13-git-send-email-arnd@arndb.de>
Left it running and put about 3.0G through x.25, it was running fine
until after about 20 hours.
I was stopping the test programs and hit this.
Jan 27 20:18:34 jaunty kernel: [80403.945790] PGD 1d8b00067 PUD 1ddec3067 PMD 0
Jan 27 20:18:34 jaunty kernel: [80403.945836] CPU 3
Jan 27 20:18:34 jaunty kernel: [80403.945842] Modules linked in: x25
nls_cp437 cifs binfmt_misc kvm_intel kvm snd_hda_codec_via
snd_usb_audio snd_hda_intel snd_hda_codec nouveau snd_pcm_oss
snd_mixer_oss snd_pcm snd_seq_dummy snd_hwdep snd_usbmidi_lib
snd_seq_oss snd_seq_midi snd_rawmidi snd_seq_midi_event snd_seq
psmouse snd_timer snd_seq_device serio_raw fbcon ttm tileblit font
bitblit softcursor xhci_hcd drm_kms_helper snd drm asus_atk0110
soundcore snd_page_alloc i2c_algo_bit video usbhid hid usb_storage
r8169 pata_jmicron ahci mii libahci
Jan 27 20:18:34 jaunty kernel: [80403.946026]
Jan 27 20:18:34 jaunty kernel: [80403.946034] Pid: 28187, comm:
x25echotest Not tainted 2.6.38-rc2+ #41 P7P55D-E PRO/System Product
Name
Jan 27 20:18:34 jaunty kernel: [80403.946050] RIP:
0010:[<ffffffffa026f197>] [<ffffffffa026f197>]
x25_sendmsg+0x1a7/0x530 [x25]
Jan 27 20:18:34 jaunty kernel: [80403.946072] RSP:
0018:ffff880228dbfcb8 EFLAGS: 00010246
Jan 27 20:18:34 jaunty kernel: [80403.946083] RAX: 0000000000000080
RBX: ffff880228dbfd70 RCX: ffff880228dbfce4
Jan 27 20:18:34 jaunty kernel: [80403.946096] RDX: 00000000fffffe00
RSI: 0000000000000000 RDI: ffff8801ba89f050
Jan 27 20:18:34 jaunty kernel: [80403.946109] RBP: ffff880228dbfd18
R08: ffff88022aa91000 R09: 0000000000000000
Jan 27 20:18:34 jaunty kernel: [80403.946482] R10: 0000000000000000
R11: 0000000000000000 R12: ffff8801ba89f000
Jan 27 20:18:34 jaunty kernel: [80403.946495] R13: 0000000000000000
R14: 0000000000000000 R15: 0000000000000000
Jan 27 20:18:34 jaunty kernel: [80403.946509] FS:
00007f09b3013700(0000) GS:ffff8800bf460000(0000)
knlGS:0000000000000000
Jan 27 20:18:34 jaunty kernel: [80403.946523] CS: 0010 DS: 0000 ES:
0000 CR0: 000000008005003b
Jan 27 20:18:34 jaunty kernel: [80403.946534] CR2: 00000000000000b4
CR3: 00000001df992000 CR4: 00000000000006e0
Jan 27 20:18:34 jaunty kernel: [80403.946547] DR0: 0000000000000000
DR1: 0000000000000000 DR2: 0000000000000000
Jan 27 20:18:34 jaunty kernel: [80403.946560] DR3: 0000000000000000
DR6: 00000000ffff0ff0 DR7: 0000000000000400
Jan 27 20:18:34 jaunty kernel: [80403.946574] Process x25echotest
(pid: 28187, threadinfo ffff880228dbe000, task ffff8801d89bc320)
Jan 27 20:18:34 jaunty kernel: [80403.946594] ffff880200000008
0000000000000016 0000303030390009 0000000000000000
Jan 27 20:18:34 jaunty kernel: [80403.946616] ffff880228db0000
fffffe00d8832450 0000000000000000 ffff880228dbfd38
Jan 27 20:18:34 jaunty kernel: [80403.946638] ffff880228dbfec8
ffff880228dbfdf8 ffff8801de73b980 ffff880228dbfd70
Jan 27 20:18:34 jaunty kernel: [80403.946671] [<ffffffff8140cdd3>]
sock_aio_write+0x183/0x1a0
Jan 27 20:18:34 jaunty kernel: [80403.946686] [<ffffffff8110304c>] ?
__pte_alloc+0xdc/0x100
Jan 27 20:18:34 jaunty kernel: [80403.946700] [<ffffffff81138a5a>]
do_sync_write+0xda/0x120
Jan 27 20:18:34 jaunty kernel: [80403.946713] [<ffffffff8140d026>] ?
move_addr_to_user+0x86/0xa0
Jan 27 20:18:34 jaunty kernel: [80403.946729] [<ffffffff812431a3>] ?
security_file_permission+0x23/0x90
Jan 27 20:18:34 jaunty kernel: [80403.946743] [<ffffffff8113903e>]
vfs_write+0x15e/0x180
Jan 27 20:18:34 jaunty kernel: [80403.946757] [<ffffffff81139151>]
sys_write+0x51/0x90
Jan 27 20:18:34 jaunty kernel: [80403.946771] [<ffffffff8100bf42>]
system_call_fastpath+0x16/0x1b
Jan 27 20:18:34 jaunty kernel: [80403.946973] RSP <ffff880228dbfcb8>
Jan 27 20:18:34 jaunty kernel: [80403.950010] ---[ end trace
36cd53b6ce0d6f4b ]---
If i have done it right, x25_sendmsg+0x1a7/0x530 is the skb_reserve
which gets inlined here.
(af_x25.c)
/* Build a packet */
SOCK_DEBUG(sk, "x25_sendmsg: sendto: building packet.\n");
if ((msg->msg_flags & MSG_OOB) && len > 32)
len = 32;
size = len + X25_MAX_L2_LEN + X25_EXT_MIN_LEN;
release_sock(sk);
skb = sock_alloc_send_skb(sk, size, noblock, &rc);
lock_sock(sk);
X25_SKB_CB(skb)->flags = msg->msg_flags;
skb_reserve(skb, X25_MAX_L2_LEN + X25_EXT_MIN_LEN);
/*
* Put the data on the end
*/
SOCK_DEBUG(sk, "x25_sendmsg: Copying user data\n");
objdump -dS show it at 2197 here.
static inline void skb_reserve(struct sk_buff *skb, int len)
{
skb->data += len;
skb->tail += len;
2197: 41 83 87 b4 00 00 00 addl $0x16,0xb4(%r15) <---
219e: 16
219f: 41 89 47 28 mov %eax,0x28(%r15)
21a3: 49 8b 87 c8 00 00 00 mov 0xc8(%r15),%rax
21aa: 48 83 c0 16 add $0x16,%rax
skb_reserve(skb, X25_MAX_L2_LEN + X25_EXT_MIN_LEN);
But im not sure where to go from there...
On Wed, Jan 26, 2011 at 9:17 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>
> This replaces all instances of lock_kernel in x25
> with lock_sock, taking care to release the socket
> lock around sleeping functions (sock_alloc_send_skb
> and skb_recv_datagram). It is not clear whether
> this is a correct solution, but it seem to be what
> other protocols do in the same situation.
>
> Compile-tested only.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Andrew Hendry <andrew.hendry@gmail.com>
> Cc: linux-x25@vger.kernel.org
> Cc: netdev@vger.kernel.org
> ---
> net/x25/Kconfig | 1 -
> net/x25/af_x25.c | 61 ++++++++++++++++------------------------------------
> net/x25/x25_out.c | 7 ++++-
> 3 files changed, 24 insertions(+), 45 deletions(-)
>
> diff --git a/net/x25/Kconfig b/net/x25/Kconfig
> index 2196e55..e6759c9 100644
> --- a/net/x25/Kconfig
> +++ b/net/x25/Kconfig
> @@ -5,7 +5,6 @@
> config X25
> tristate "CCITT X.25 Packet Layer (EXPERIMENTAL)"
> depends on EXPERIMENTAL
> - depends on BKL # should be fixable
> ---help---
> X.25 is a set of standardized network protocols, similar in scope to
> frame relay; the one physical line from your box to the X.25 network
> diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
> index ad96ee9..8f5d1bb 100644
> --- a/net/x25/af_x25.c
> +++ b/net/x25/af_x25.c
> @@ -40,7 +40,6 @@
> #include <linux/errno.h>
> #include <linux/kernel.h>
> #include <linux/sched.h>
> -#include <linux/smp_lock.h>
> #include <linux/timer.h>
> #include <linux/string.h>
> #include <linux/net.h>
> @@ -432,15 +431,6 @@ void x25_destroy_socket_from_timer(struct sock *sk)
> sock_put(sk);
> }
>
> -static void x25_destroy_socket(struct sock *sk)
> -{
> - sock_hold(sk);
> - lock_sock(sk);
> - __x25_destroy_socket(sk);
> - release_sock(sk);
> - sock_put(sk);
> -}
> -
> /*
> * Handling for system calls applied via the various interfaces to a
> * X.25 socket object.
> @@ -647,18 +637,19 @@ static int x25_release(struct socket *sock)
> struct sock *sk = sock->sk;
> struct x25_sock *x25;
>
> - lock_kernel();
> if (!sk)
> - goto out;
> + return 0;
>
> x25 = x25_sk(sk);
>
> + sock_hold(sk);
> + lock_sock(sk);
> switch (x25->state) {
>
> case X25_STATE_0:
> case X25_STATE_2:
> x25_disconnect(sk, 0, 0, 0);
> - x25_destroy_socket(sk);
> + __x25_destroy_socket(sk);
> goto out;
>
> case X25_STATE_1:
> @@ -678,7 +669,8 @@ static int x25_release(struct socket *sock)
>
> sock_orphan(sk);
> out:
> - unlock_kernel();
> + release_sock(sk);
> + sock_put(sk);
> return 0;
> }
>
> @@ -1085,7 +1077,7 @@ static int x25_sendmsg(struct kiocb *iocb, struct socket *sock,
> size_t size;
> int qbit = 0, rc = -EINVAL;
>
> - lock_kernel();
> + lock_sock(sk);
> if (msg->msg_flags & ~(MSG_DONTWAIT|MSG_OOB|MSG_EOR|MSG_CMSG_COMPAT))
> goto out;
>
> @@ -1148,9 +1140,10 @@ static int x25_sendmsg(struct kiocb *iocb, struct socket *sock,
>
> size = len + X25_MAX_L2_LEN + X25_EXT_MIN_LEN;
>
> + release_sock(sk);
> skb = sock_alloc_send_skb(sk, size, noblock, &rc);
> - if (!skb)
> - goto out;
> + lock_sock(sk);
> +
> X25_SKB_CB(skb)->flags = msg->msg_flags;
>
> skb_reserve(skb, X25_MAX_L2_LEN + X25_EXT_MIN_LEN);
> @@ -1231,26 +1224,10 @@ static int x25_sendmsg(struct kiocb *iocb, struct socket *sock,
> len++;
> }
>
> - /*
> - * lock_sock() is currently only used to serialize this x25_kick()
> - * against input-driven x25_kick() calls. It currently only blocks
> - * incoming packets for this socket and does not protect against
> - * any other socket state changes and is not called from anywhere
> - * else. As x25_kick() cannot block and as long as all socket
> - * operations are BKL-wrapped, we don't need take to care about
> - * purging the backlog queue in x25_release().
> - *
> - * Using lock_sock() to protect all socket operations entirely
> - * (and making the whole x25 stack SMP aware) unfortunately would
> - * require major changes to {send,recv}msg and skb allocation methods.
> - * -> 2.5 ;)
> - */
> - lock_sock(sk);
> x25_kick(sk);
> - release_sock(sk);
> rc = len;
> out:
> - unlock_kernel();
> + release_sock(sk);
> return rc;
> out_kfree_skb:
> kfree_skb(skb);
> @@ -1271,7 +1248,7 @@ static int x25_recvmsg(struct kiocb *iocb, struct socket *sock,
> unsigned char *asmptr;
> int rc = -ENOTCONN;
>
> - lock_kernel();
> + lock_sock(sk);
> /*
> * This works for seqpacket too. The receiver has ordered the queue for
> * us! We do one quick check first though
> @@ -1300,8 +1277,10 @@ static int x25_recvmsg(struct kiocb *iocb, struct socket *sock,
> msg->msg_flags |= MSG_OOB;
> } else {
> /* Now we can treat all alike */
> + release_sock(sk);
> skb = skb_recv_datagram(sk, flags & ~MSG_DONTWAIT,
> flags & MSG_DONTWAIT, &rc);
> + lock_sock(sk);
> if (!skb)
> goto out;
>
> @@ -1338,14 +1317,12 @@ static int x25_recvmsg(struct kiocb *iocb, struct socket *sock,
>
> msg->msg_namelen = sizeof(struct sockaddr_x25);
>
> - lock_sock(sk);
> x25_check_rbuf(sk);
> - release_sock(sk);
> rc = copied;
> out_free_dgram:
> skb_free_datagram(sk, skb);
> out:
> - unlock_kernel();
> + release_sock(sk);
> return rc;
> }
>
> @@ -1581,18 +1558,18 @@ out_cud_release:
>
> case SIOCX25CALLACCPTAPPRV: {
> rc = -EINVAL;
> - lock_kernel();
> + lock_sock(sk);
> if (sk->sk_state != TCP_CLOSE)
> break;
> clear_bit(X25_ACCPT_APPRV_FLAG, &x25->flags);
> - unlock_kernel();
> + release_sock(sk);
> rc = 0;
> break;
> }
>
> case SIOCX25SENDCALLACCPT: {
> rc = -EINVAL;
> - lock_kernel();
> + lock_sock(sk);
> if (sk->sk_state != TCP_ESTABLISHED)
> break;
> /* must call accptapprv above */
> @@ -1600,7 +1577,7 @@ out_cud_release:
> break;
> x25_write_internal(sk, X25_CALL_ACCEPTED);
> x25->state = X25_STATE_3;
> - unlock_kernel();
> + release_sock(sk);
> rc = 0;
> break;
> }
> diff --git a/net/x25/x25_out.c b/net/x25/x25_out.c
> index d00649f..f1a6ff1 100644
> --- a/net/x25/x25_out.c
> +++ b/net/x25/x25_out.c
> @@ -68,8 +68,11 @@ int x25_output(struct sock *sk, struct sk_buff *skb)
> frontlen = skb_headroom(skb);
>
> while (skb->len > 0) {
> - if ((skbn = sock_alloc_send_skb(sk, frontlen + max_len,
> - noblock, &err)) == NULL){
> + release_sock(sk);
> + skbn = sock_alloc_send_skb(sk, frontlen + max_len,
> + 1, &err);
> + lock_sock(sk);
> + if (!skbn) {
> if (err == -EWOULDBLOCK && noblock){
> kfree_skb(skb);
> return sent;
> --
> 1.7.1
>
^ permalink raw reply
* SO_REUSEPORT - can it be done in kernel?
From: Daniel Baluta @ 2011-01-27 10:07 UTC (permalink / raw)
To: therbert; +Cc: netdev
Hi Tom,
How did you solved the issue regarding scaling TCP listeners?
I think SO_REUSEPORT proposed by patch [1] can be a good
start. Where there any follow ups?
Also, solving the problem in users pace can be an option. I want
to run multiple instances of a DNS server on a multi core system.
Any suggestions would be welcomed.
SO_REUSEPORT option seems to be already there [2]. Where
there any plans to have a kernel implementation?
thanks,
Daniel.
[1] http://amailbox.org/mailarchive/linux-netdev/2010/4/19/6274993
[2] http://lxr.linux.no/linux+v2.6.37/include/asm-generic/socket.h#L25
^ permalink raw reply
* Re: [RFC PATCH] net: Implement read-only protection and COW'ing of metrics.
From: Eric Dumazet @ 2011-01-27 10:20 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <1296122511.3027.11.camel@edumazet-laptop>
Le jeudi 27 janvier 2011 à 11:01 +0100, Eric Dumazet a écrit :
> Le mercredi 26 janvier 2011 à 15:25 -0800, David Miller a écrit :
> > Eric, thanks again for your feedback. I've taken a stab at fixing the
> > various races, in particular the one you discovered about metrics
> > sharing and how this interacts with fib_info releases.
> >
> > What I've choosen to do is two-fold:
> >
> > 1) Update ->_metrics atomically with cmpxchg once a route becomes publicly
> > visible.
> >
> > 2) Remember and grab a reference to the fib_info for shared read-only
> > metrics in rt->fi, then release it once the metrics regerence goes
> > away.
> >
> > It sounds expensive but hear me out :-)
> >
> > First of all, at rt_set_nexthop() time, the atomic we use to grab a
> > ref to the fib_info is replacing a 60-byte memcpy() into the dst
> > metrics.
> >
> > Next, the ->_metrics atomic to un-COW the metrics at destroy time
> > might in fact be overkill. Especially once writable metrics live in
> > the inetpeer cache (that's the next set of patches after this one).
> >
> > Finally, once this change is stabilized we can be a lot smarter about
> > what we do at the time an entry is created. For example, when a route
> > is looked up for a TCP socket, we essentially know we are going to COW
> > the route %99.99999 of the time. So we can pass a hint into TCP's
> > route lookups in the flow flags field telling it to pre-COW the route.
> >
> > TCP pre-COW'ing of metrics will thus save several atomics.
> >
> > Anyways, here is the patch, it is only build tested at this point, but
> > I wanted to get feedback from you about the basic gist of things
> > as soon as possible.
> >
> > Thanks!
> >
>
> Thanks David, I read this (I am a bit busy preparing my travel to
> Reunion/Maurice islands). This looks pretty nice. I have one comment :
>
> >
> > +u32 *dst_cow_metrics_generic(struct dst_entry *dst, unsigned long old)
> > +{
> > + u32 *p = kmalloc(sizeof(u32) * RTAX_MAX, GFP_ATOMIC);
> > +
> > + if (p) {
> > + u32 *old_p = __DST_METRICS_PTR(old);
> > + unsigned long prev, new;
> > +
> > + memcpy(p, old_p, sizeof(u32) * RTAX_MAX);
> > +
> > + new = (unsigned long) p;
> > + prev = cmpxchg(&dst->_metrics, old, new);
> > +
> > + if (prev != old) {
> > + kfree(p);
> > + p = __DST_METRICS_PTR(prev);
> > + if (prev & DST_METRICS_READ_ONLY)
> > + p = NULL;
> > + }
> > + }
> > + return p;
> > +}
> > +EXPORT_SYMBOL(dst_cow_metrics_generic);
> > +
> ...
>
> > diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> > index 3e5b7cc..7fc6301 100644
> > --- a/net/ipv4/route.c
> > +++ b/net/ipv4/route.c
> > @@ -152,6 +152,36 @@ static void ipv4_dst_ifdown(struct dst_entry *dst, struct net_device *dev,
> > {
> > }
> >
> > +static u32 *ipv4_cow_metrics(struct dst_entry *dst, unsigned long old)
> > +{
> > + u32 *p = kmalloc(sizeof(u32) * RTAX_MAX, GFP_ATOMIC);
> > +
> > + if (p) {
> > + u32 *old_p = __DST_METRICS_PTR(old);
> > + unsigned long prev, new;
> > +
> > + memcpy(p, old_p, sizeof(u32) * RTAX_MAX);
> > +
> > + new = (unsigned long) p;
> > + prev = cmpxchg(&dst->_metrics, old, new);
> > +
> > + if (prev != old) {
> > + kfree(p);
> > + p = __DST_METRICS_PTR(prev);
> > + if (prev & DST_METRICS_READ_ONLY)
> > + p = NULL;
> > + } else {
>
> Hmm, I first asked myself why you dont use dst_cow_metrics_generic() to
> perform the generic allocation, but saw following :
>
> > + struct rtable *rt = (struct rtable *) dst;
> > +
>
> Since you use cmpxchg() to permut the dst->_metrics, I feel this rt->fi
> needs some protection as well. Maybe store fi pointer inside the metrics
> instead of dst, or else you need a spinlock to perform the whole
> transaction (change dst->_metrics & rt->fi) ?
>
> > + if (rt->fi) {
> > + fib_info_put(rt->fi);
> > + rt->fi = NULL;
> > + }
> > + }
> > + }
> > + return p;
> > +}
> > +
>
Hmm, reading again, I realize the rt->fi was set only when installing
the readonly metrics, so ignore my previous mail.
Thanks !
^ permalink raw reply
* Re: Realtek r8168C / r8169 driver VLAN TAG stripping
From: Anand Raj Manickam @ 2011-01-27 10:35 UTC (permalink / raw)
To: Francois Romieu; +Cc: netdev, Hayes
In-Reply-To: <20110118012146.GA3704@electric-eye.fr.zoreil.com>
Hi ,
We upgraded to 2.6.36 kernel . The result is SAME.
The VLAN tag gets stripped ;-)
Do let me know if you need more info .
Thanks,
Anand
On Tue, Jan 18, 2011 at 6:51 AM, Francois Romieu <romieu@fr.zoreil.com> wrote:
> Anand Raj Manickam <anandrm@gmail.com> :
>> On Mon, Jan 17, 2011 at 11:52 AM, Anand Raj Manickam <anandrm@gmail.com> wrote:
> [...]
>> > This is the dmesg for XID
>> >
>> > eth0: RTL8168c/8111c at 0xf9628000, 00:17:54:00:f6:62, XID 1c4000c0 IRQ 31
>> > r8169: mac_version = 0x16
>
> I do not have one of those (RTL_GIGA_MAC_VER_22) to check if it handles vlan
> correctly yet.
>
>> > r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
>> >
>> > Unfortunately , i m not able to upgrade my kernel now . If there is a
>> > Fix for it , that would be great !!
>
> I doubt there is a lot of glamour/fortune/fame in backporting the 89 r8169
> patches between v2.6.30 and v2.6.37 but you may help yourself and give it
> a try.
>
> --
> Ueimor
>
^ permalink raw reply
* [PATCH V10 00/15] ptp: IEEE 1588 hardware clock support
From: Richard Cochran @ 2011-01-27 10:53 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
Alan Cox, Arnd Bergmann, Christoph Lameter, David Miller,
John Stultz, Krzysztof Halasa, Peter Zijlstra, Rodolfo Giometti,
Thomas Gleixner, Benjamin Herrenschmidt, H. Peter Anvin,
Ingo Molnar, Mike Frysinger, Paul Mackerras, Russell King
Here is version 10 of the PHC patch series. We missed the boat for
2.6.38, but I hope this can be merged for the next one.
Reviewers: I think its done. How can I get this merged?
Arch maintainers (arm, blackfin, powerpc, x86): I have added you on
CC in the hope of getting your acks for the new syscall.
Thanks,
Richard
* Patch ChangeLog
** v10
*** posix clocks
- removed the (previously implied) default functions from the k_clocks
- made the ts parameter 'const' in clock_set functions
*** ptp hardware clocks
- updated to changed PPS kernel API
* Introduction
The aim of this patch set is to add support for PTP Hardware Clocks
(PHCs) into the Linux kernel.
The first eleven patches provide infrastructure supporting dynamic
POSIX clock devices. This new code will be useful for other kinds of
new clocks, not just PHCs.
The last four patches implement the PHC code.
Support for obtaining timestamps from a PHC already exists via the
SO_TIMESTAMPING socket option, integrated in kernel version 2.6.30.
This patch set completes the picture by allow user space programs to
adjust the PHC and to control its ancillary features.
* Previous Discussions
This patch set previously appeared on the netdev list. Since V5 of
the character device patch set, the discussion has moved to the
lkml.
- IEEE 1588 hardware clock support [V5]
http://lkml.org/lkml/2010/8/16/90
- POSIX clock tuning syscall with static clock ids
http://lkml.org/lkml/2010/8/23/49
- POSIX clock tuning syscall with dynamic clock ids
http://lkml.org/lkml/2010/9/3/119
- IEEE 1588 hardware clock support [V6]
http://lkml.org/lkml/2010/9/23/310
- Dynamic clock devices [RFC]
http://lkml.org/lkml/2010/11/4/290
- IEEE 1588 hardware clock support [V7]
http://lkml.org/lkml/2010/12/16/195
- IEEE 1588 hardware clock support [V8]
http://lkml.org/lkml/2010/12/31/128
- IEEE 1588 hardware clock support [V9]
http://lkml.org/lkml/2011/1/13/65
John Stultz (1):
time: Introduce timekeeping_inject_offset
Richard Cochran (14):
time: Correct the *settime* parameters
ntp: add ADJ_SETOFFSET mode bit
posix clocks: introduce a syscall for clock tuning.
posix_clocks: add clock_adjtime for arm
posix_clocks: add clock_adjtime for blackfin
posix_clocks: add clock_adjtime for powerpc
posix_clocks: add clock_adjtime for x86
posix clocks: cleanup the CLOCK_DISPTACH macro
posix clocks: remove useless default methods
posix clocks: introduce dynamic clocks
ptp: Added a brand new class driver for ptp clocks.
ptp: Added a clock that uses the eTSEC found on the MPC85xx.
ptp: Added a clock driver for the IXP46x.
ptp: Added a clock driver for the National Semiconductor PHYTER.
Documentation/ABI/testing/sysfs-ptp | 98 +++
Documentation/powerpc/dts-bindings/fsl/tsec.txt | 57 ++
Documentation/ptp/ptp.txt | 89 +++
Documentation/ptp/testptp.c | 352 +++++++++
Documentation/ptp/testptp.mk | 33 +
arch/arm/include/asm/unistd.h | 1 +
arch/arm/kernel/calls.S | 1 +
arch/arm/mach-ixp4xx/include/mach/ixp46x_ts.h | 78 ++
arch/blackfin/include/asm/unistd.h | 3 +-
arch/blackfin/mach-common/entry.S | 1 +
arch/powerpc/boot/dts/mpc8313erdb.dts | 14 +
arch/powerpc/boot/dts/mpc8572ds.dts | 14 +
arch/powerpc/boot/dts/p2020ds.dts | 14 +
arch/powerpc/boot/dts/p2020rdb.dts | 14 +
arch/powerpc/include/asm/systbl.h | 1 +
arch/powerpc/include/asm/unistd.h | 3 +-
arch/x86/ia32/ia32entry.S | 1 +
arch/x86/include/asm/unistd_32.h | 3 +-
arch/x86/include/asm/unistd_64.h | 2 +
arch/x86/kernel/syscall_table_32.S | 1 +
drivers/Kconfig | 2 +
drivers/Makefile | 1 +
drivers/char/mmtimer.c | 3 +-
drivers/net/Makefile | 1 +
drivers/net/arm/ixp4xx_eth.c | 192 +++++-
drivers/net/gianfar_ptp.c | 448 +++++++++++
drivers/net/gianfar_ptp_reg.h | 113 +++
drivers/net/phy/Kconfig | 29 +
drivers/net/phy/Makefile | 1 +
drivers/net/phy/dp83640.c | 896 +++++++++++++++++++++++
drivers/net/phy/dp83640_reg.h | 261 +++++++
drivers/ptp/Kconfig | 53 ++
drivers/ptp/Makefile | 7 +
drivers/ptp/ptp_chardev.c | 144 ++++
drivers/ptp/ptp_clock.c | 315 ++++++++
drivers/ptp/ptp_ixp46x.c | 332 +++++++++
drivers/ptp/ptp_private.h | 85 +++
drivers/ptp/ptp_sysfs.c | 226 ++++++
include/linux/Kbuild | 1 +
include/linux/posix-clock.h | 150 ++++
include/linux/posix-timers.h | 28 +-
include/linux/ptp_clock.h | 79 ++
include/linux/ptp_clock_kernel.h | 141 ++++
include/linux/security.h | 9 +-
include/linux/syscalls.h | 2 +
include/linux/time.h | 8 +-
include/linux/timex.h | 3 +-
kernel/compat.c | 136 +++--
kernel/posix-cpu-timers.c | 6 +
kernel/posix-timers.c | 245 ++++---
kernel/time.c | 2 +-
kernel/time/Makefile | 3 +-
kernel/time/ntp.c | 11 +
kernel/time/posix-clock-syscalls.h | 39 +
kernel/time/posix-clock.c | 432 +++++++++++
kernel/time/timekeeping.c | 38 +-
security/commoncap.c | 2 +-
security/security.c | 2 +-
58 files changed, 5062 insertions(+), 164 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-ptp
create mode 100644 Documentation/ptp/ptp.txt
create mode 100644 Documentation/ptp/testptp.c
create mode 100644 Documentation/ptp/testptp.mk
create mode 100644 arch/arm/mach-ixp4xx/include/mach/ixp46x_ts.h
create mode 100644 drivers/net/gianfar_ptp.c
create mode 100644 drivers/net/gianfar_ptp_reg.h
create mode 100644 drivers/net/phy/dp83640.c
create mode 100644 drivers/net/phy/dp83640_reg.h
create mode 100644 drivers/ptp/Kconfig
create mode 100644 drivers/ptp/Makefile
create mode 100644 drivers/ptp/ptp_chardev.c
create mode 100644 drivers/ptp/ptp_clock.c
create mode 100644 drivers/ptp/ptp_ixp46x.c
create mode 100644 drivers/ptp/ptp_private.h
create mode 100644 drivers/ptp/ptp_sysfs.c
create mode 100644 include/linux/posix-clock.h
create mode 100644 include/linux/ptp_clock.h
create mode 100644 include/linux/ptp_clock_kernel.h
create mode 100644 kernel/time/posix-clock-syscalls.h
create mode 100644 kernel/time/posix-clock.c
^ 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