* Re: [PATCH] net/sunrpc/auth_gss/gss_krb5_crypto.c: Use normal negative error value return
From: Trond Myklebust @ 2010-12-14 21:34 UTC (permalink / raw)
To: J. Bruce Fields
Cc: Joe Perches, Neil Brown, David S. Miller,
linux-nfs-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20101214211922.GJ24828-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
On Tue, 2010-12-14 at 16:19 -0500, J. Bruce Fields wrote:
> On Sun, Nov 14, 2010 at 06:08:11PM -0800, Joe Perches wrote:
> > And remove unnecessary double semicolon too.
> >
> > No effect to code, as test is != 0.
>
> Hm, the error return's actually ignored. But OK. Applying to my tree
> for 2.6.38 assuming nobody else has picked it up.
There are also gems such as
u32 ret = -EINVAL;
Cheers
Trond
--
Trond Myklebust
Linux NFS client maintainer
NetApp
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org
www.netapp.com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 21/22] netoops: Add user-programmable boot_id
From: Matt Mackall @ 2010-12-14 21:42 UTC (permalink / raw)
To: Mike Waychison
Cc: simon.kagstrom-vI6UBbBVNY+JA8cjQkG2/g,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, nhorman-2XuSBdqkA4R54TAoqtyWWQ,
adurbin-hpIqsD4AKlfQT0dZR+AlfA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
chavey-hpIqsD4AKlfQT0dZR+AlfA, Greg KH,
netdev-u79uwXL29TY76Z2rM5mHXA, Américo Wang,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20101214213048.17022.58746.stgit-tzAwxxnF6Tt6FDdRrpk8kO4/NqBCd+6Q@public.gmane.org>
On Tue, 2010-12-14 at 13:30 -0800, Mike Waychison wrote:
> Add support for letting userland define a 32bit boot id. This is useful
> for users to be able to correlate netoops reports to specific boot
> instances offline.
This sounds a lot like the pre-existing /proc/sys/kernel/random/boot_id
that's used by kerneloops.org.
--
Mathematics is the supreme nostalgia of our time.
^ permalink raw reply
* Re: [PATCH 2.6.36] vlan: Avoid hwaccel vlan packets when vid not used
From: Jesse Gross @ 2010-12-14 21:46 UTC (permalink / raw)
To: Matt Carlson
Cc: Michael Leun, Michael Chan, Eric Dumazet, David Miller,
Ben Greear, linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20101214191500.GD19951@mcarlson.broadcom.com>
On Tue, Dec 14, 2010 at 11:15 AM, Matt Carlson <mcarlson@broadcom.com> wrote:
> @@ -9538,17 +9505,8 @@ static void __tg3_set_rx_mode(struct net_device *dev)
> /* When ASF is in use, we always keep the RX_MODE_KEEP_VLAN_TAG
> * flag clear.
> */
> -#if TG3_VLAN_TAG_USED
> - if (!tp->vlgrp &&
> - !(tp->tg3_flags & TG3_FLAG_ENABLE_ASF))
> - rx_mode |= RX_MODE_KEEP_VLAN_TAG;
> -#else
> - /* By definition, VLAN is disabled always in this
> - * case.
> - */
> if (!(tp->tg3_flags & TG3_FLAG_ENABLE_ASF))
> rx_mode |= RX_MODE_KEEP_VLAN_TAG;
> -#endif
Just one comment:
I don't think this does quite the right thing: it will always disable
vlan stripping unless ASF is in use. However, it's now OK to always
use vlan stripping, so we might as well take advantage of it. Since
without the set_flags Ethtool op there is no way to change this
setting, we should be able to just drop this code block completely
(and the check for RX_MODE_KEEP_VLAN_TAG on receive).
In addition, this should also remove any differences between ASF
enabled/disabled firmware (at least with respect to vlans) since it
will no longer be a factor.
Thanks.
^ permalink raw reply
* Re: [PATCH] ixgb: Convert to new vlan model.
From: Ben Hutchings @ 2010-12-14 21:47 UTC (permalink / raw)
To: Jesse Gross
Cc: Tantilov, Emil S, David Miller, netdev@vger.kernel.org,
Kirsher, Jeffrey T, Duyck, Alexander H
In-Reply-To: <AANLkTimnSedKQhhL8xXQVKnEm3FpbfRQ_Nd9hPmGbHLJ@mail.gmail.com>
On Tue, 2010-12-14 at 13:29 -0800, Jesse Gross wrote:
> On Tue, Dec 14, 2010 at 11:15 AM, Ben Hutchings
> <bhutchings@solarflare.com> wrote:
> > On Tue, 2010-12-14 at 12:08 -0700, Tantilov, Emil S wrote:
> >> Ben Hutchings wrote:
> >> > On Tue, 2010-12-14 at 11:09 -0700, Tantilov, Emil S wrote:
> >> >> Ben Hutchings wrote:
> >> >>> On Mon, 2010-12-13 at 19:42 -0800, Jesse Gross wrote:
[...]
> >> >>> I think this should reject attempts to change just one flag with
> >> >>> -EINVAL, rather than quietly 'fixing' the setting.
[...]
> Ben, I agree that limiting the settings to what is actually supported
> is conceptually cleaner but in practice it's not very intuitive. If
> you try to turn something off and the response is that it's invalid,
> most people are going to assume that you just can't do it. This is
> especially true since you actually can't turn these settings off in
> most drivers.
>
> There's a precedent for this type of thing: turn off TX checksum
> offloading and watch scatter/gather and TSO be automatically disabled
> as well. It makes sense - the user requested a change, we do what is
> necessary to make that happen without requiring them to understand why
> these features are interrelated.
That reflects a general dependency and not a driver- or hardware-
specific restriction. But I see your point.
Perhaps the ethtool utility should check the result after applying
offload changes and report any additional automatic changes.
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
* Re: [PATCH] ixgb: Convert to new vlan model.
From: Jesse Gross @ 2010-12-14 21:53 UTC (permalink / raw)
To: Ben Hutchings
Cc: Tantilov, Emil S, David Miller, netdev@vger.kernel.org,
Kirsher, Jeffrey T, Duyck, Alexander H
In-Reply-To: <1292363231.20458.25.camel@bwh-desktop>
On Tue, Dec 14, 2010 at 1:47 PM, Ben Hutchings
<bhutchings@solarflare.com> wrote:
> On Tue, 2010-12-14 at 13:29 -0800, Jesse Gross wrote:
>> On Tue, Dec 14, 2010 at 11:15 AM, Ben Hutchings
>> <bhutchings@solarflare.com> wrote:
>> > On Tue, 2010-12-14 at 12:08 -0700, Tantilov, Emil S wrote:
>> >> Ben Hutchings wrote:
>> >> > On Tue, 2010-12-14 at 11:09 -0700, Tantilov, Emil S wrote:
>> >> >> Ben Hutchings wrote:
>> >> >>> On Mon, 2010-12-13 at 19:42 -0800, Jesse Gross wrote:
> [...]
>> >> >>> I think this should reject attempts to change just one flag with
>> >> >>> -EINVAL, rather than quietly 'fixing' the setting.
> [...]
>> Ben, I agree that limiting the settings to what is actually supported
>> is conceptually cleaner but in practice it's not very intuitive. If
>> you try to turn something off and the response is that it's invalid,
>> most people are going to assume that you just can't do it. This is
>> especially true since you actually can't turn these settings off in
>> most drivers.
>>
>> There's a precedent for this type of thing: turn off TX checksum
>> offloading and watch scatter/gather and TSO be automatically disabled
>> as well. It makes sense - the user requested a change, we do what is
>> necessary to make that happen without requiring them to understand why
>> these features are interrelated.
>
> That reflects a general dependency and not a driver- or hardware-
> specific restriction. But I see your point.
>
> Perhaps the ethtool utility should check the result after applying
> offload changes and report any additional automatic changes.
That sounds like the best solution to me.
^ permalink raw reply
* Re: [PATCH v3 21/22] netoops: Add user-programmable boot_id
From: Mike Waychison @ 2010-12-14 21:59 UTC (permalink / raw)
To: Matt Mackall
Cc: simon.kagstrom-vI6UBbBVNY+JA8cjQkG2/g,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, nhorman-2XuSBdqkA4R54TAoqtyWWQ,
adurbin-hpIqsD4AKlfQT0dZR+AlfA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
chavey-hpIqsD4AKlfQT0dZR+AlfA, Greg KH,
netdev-u79uwXL29TY76Z2rM5mHXA, Américo Wang,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1292362957.3446.851.camel@calx>
On Tue, Dec 14, 2010 at 1:42 PM, Matt Mackall <mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ@public.gmane.org> wrote:
> On Tue, 2010-12-14 at 13:30 -0800, Mike Waychison wrote:
>> Add support for letting userland define a 32bit boot id. This is useful
>> for users to be able to correlate netoops reports to specific boot
>> instances offline.
>
> This sounds a lot like the pre-existing /proc/sys/kernel/random/boot_id
> that's used by kerneloops.org.
Could be. I'm looking at it now... There is no documentation for this
boot_id field?
Reusing this guy would work, except that it doesn't appear to allow
arbitrary values to be set. We need to inject our boot sequence
number (which is figured out in userland) in the packet somehow as we
need to correlate it to our other monitoring systems.
I agree that having the uuid included in the netoops messages would be
very useful.
netoops is probably the wrong place to have a system-wide boot
sequence ID set in retrospect. For our purposes, I can have it
encoded into the 'user_blob' portion of the packet if folks don't
think the kernel should have any notion of boot sequence number
programmable by userland.
^ permalink raw reply
* Re: [PATCH v3 21/22] netoops: Add user-programmable boot_id
From: Matt Mackall @ 2010-12-14 22:06 UTC (permalink / raw)
To: Mike Waychison
Cc: simon.kagstrom, davem, nhorman, adurbin, linux-kernel, chavey,
Greg KH, netdev, Américo Wang, akpm, linux-api
In-Reply-To: <AANLkTi=Q+LaS7yaU_4ThkhMUM-q8S3JTuwMWqAikQPgH@mail.gmail.com>
On Tue, 2010-12-14 at 13:59 -0800, Mike Waychison wrote:
> On Tue, Dec 14, 2010 at 1:42 PM, Matt Mackall <mpm@selenic.com> wrote:
> > On Tue, 2010-12-14 at 13:30 -0800, Mike Waychison wrote:
> >> Add support for letting userland define a 32bit boot id. This is useful
> >> for users to be able to correlate netoops reports to specific boot
> >> instances offline.
> >
> > This sounds a lot like the pre-existing /proc/sys/kernel/random/boot_id
> > that's used by kerneloops.org.
>
> Could be. I'm looking at it now... There is no documentation for this
> boot_id field?
Probably not. It's just a random number generated at boot.
> Reusing this guy would work, except that it doesn't appear to allow
> arbitrary values to be set. We need to inject our boot sequence
> number (which is figured out in userland) in the packet somehow as we
> need to correlate it to our other monitoring systems.
What happens if you oops before userspace is available?
--
Mathematics is the supreme nostalgia of our time.
^ permalink raw reply
* Re: After memory pressure: can't read from tape anymore
From: Stephen Hemminger @ 2010-12-14 22:23 UTC (permalink / raw)
To: Vladislav Bolkhovitin
Cc: James Bottomley, Lukas Kolbe, Kai Mäkisara, FUJITA Tomonori,
linux-scsi, Kashyap Desai, netdev
In-Reply-To: <4D07D519.70800@vlnb.net>
On Tue, 14 Dec 2010 23:35:37 +0300
Vladislav Bolkhovitin <vst@vlnb.net> wrote:
> What is interesting to me in this regard is how networking with 9K jumbo
> frames manages to work acceptably reliable? Jumbo frames used
> sufficiently often, including under high memory pressure.
>
> I'm not a deep networking guru, but network drivers need to allocate
> physically continual memory for skbs, which means 16K per 9K packet,
> which means order 2 allocations per skb.
Good network drivers support fragmentation and allocate a small portion
for the header and allocate pages for the rest. This requires no higher
order allocation. The networking stack takes fragmented data coming
in and does the necessary copy/merging to access contiguous headers.
There are still some crap network drivers that require large contiguous
allocation. These should not be used with jumbo frames in real
environments.
--
^ permalink raw reply
* Re: [PATCH v3 21/22] netoops: Add user-programmable boot_id
From: Mike Waychison @ 2010-12-14 22:33 UTC (permalink / raw)
To: Matt Mackall
Cc: simon.kagstrom, davem, nhorman, adurbin, linux-kernel, chavey,
Greg KH, netdev, Américo Wang, akpm, linux-api
In-Reply-To: <1292364378.3446.854.camel@calx>
On Tue, Dec 14, 2010 at 2:06 PM, Matt Mackall <mpm@selenic.com> wrote:
> On Tue, 2010-12-14 at 13:59 -0800, Mike Waychison wrote:
>> On Tue, Dec 14, 2010 at 1:42 PM, Matt Mackall <mpm@selenic.com> wrote:
>> > On Tue, 2010-12-14 at 13:30 -0800, Mike Waychison wrote:
>> >> Add support for letting userland define a 32bit boot id. This is useful
>> >> for users to be able to correlate netoops reports to specific boot
>> >> instances offline.
>> >
>> > This sounds a lot like the pre-existing /proc/sys/kernel/random/boot_id
>> > that's used by kerneloops.org.
>>
>> Could be. I'm looking at it now... There is no documentation for this
>> boot_id field?
>
> Probably not. It's just a random number generated at boot.
>
>> Reusing this guy would work, except that it doesn't appear to allow
>> arbitrary values to be set. We need to inject our boot sequence
>> number (which is figured out in userland) in the packet somehow as we
>> need to correlate it to our other monitoring systems.
>
> What happens if you oops before userspace is available?
>
Either one of two general cases:
- The crash is a one-off and the machine comes back. The boot
number sequence will see a hole in it, which is a clue that something
bad happened.
- The machine is in a crash loop. This has the same failure mode
for us as if the machine never made it onto the network due to
whatever reason: bad cables, bad firmware, bad ram, ...
In both cases, we can detect that something is wrong and handle it.
Note that our firmware is responsible for incrementing the boot
sequence at bootup, which is why the above works. In general though,
our machines do make it up to userland -- staying alive once booted is
the hard part ;)
^ permalink raw reply
* [PATCH] net/veth: Fix packet checksumming
From: Michał Mirosław @ 2010-12-14 22:35 UTC (permalink / raw)
To: netdev
We can't change ip_summed from CHECKSUM_PARTIAL to CHECKSUM_NONE
or CHECKSUM_UNNECESSARY because checksum in packet's headers is
not valid and will cause invalid checksum when frame is forwarded.
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
drivers/net/veth.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 0bbc0c3..cca6a71 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -166,7 +166,9 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
if (!(rcv->flags & IFF_UP))
goto tx_drop;
- if (dev->features & NETIF_F_NO_CSUM)
+ /* don't change ip_summed == CHECKSUM_PARTIAL, as that
+ will cause bad checksum on forwarded packets */
+ if (skb->ip_summed == CHECKSUM_NONE)
skb->ip_summed = rcv_priv->ip_summed;
length = skb->len + ETH_HLEN;
--
1.7.2.3
^ permalink raw reply related
* Re: [PATCH v3 21/22] netoops: Add user-programmable boot_id
From: Matt Mackall @ 2010-12-14 22:47 UTC (permalink / raw)
To: Mike Waychison
Cc: simon.kagstrom-vI6UBbBVNY+JA8cjQkG2/g,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, nhorman-2XuSBdqkA4R54TAoqtyWWQ,
adurbin-hpIqsD4AKlfQT0dZR+AlfA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
chavey-hpIqsD4AKlfQT0dZR+AlfA, Greg KH,
netdev-u79uwXL29TY76Z2rM5mHXA, Américo Wang,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <AANLkTineeFcSNx09P=2SZGcBHeq5p_LQ54nU=uGVv_Ck-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Tue, 2010-12-14 at 14:33 -0800, Mike Waychison wrote:
> On Tue, Dec 14, 2010 at 2:06 PM, Matt Mackall <mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ@public.gmane.org> wrote:
> > On Tue, 2010-12-14 at 13:59 -0800, Mike Waychison wrote:
> >> On Tue, Dec 14, 2010 at 1:42 PM, Matt Mackall <mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ@public.gmane.org> wrote:
> >> > On Tue, 2010-12-14 at 13:30 -0800, Mike Waychison wrote:
> >> >> Add support for letting userland define a 32bit boot id. This is useful
> >> >> for users to be able to correlate netoops reports to specific boot
> >> >> instances offline.
> >> >
> >> > This sounds a lot like the pre-existing /proc/sys/kernel/random/boot_id
> >> > that's used by kerneloops.org.
> >>
> >> Could be. I'm looking at it now... There is no documentation for this
> >> boot_id field?
> >
> > Probably not. It's just a random number generated at boot.
> >
> >> Reusing this guy would work, except that it doesn't appear to allow
> >> arbitrary values to be set. We need to inject our boot sequence
> >> number (which is figured out in userland) in the packet somehow as we
> >> need to correlate it to our other monitoring systems.
> >
> > What happens if you oops before userspace is available?
> >
>
> Either one of two general cases:
> - The crash is a one-off and the machine comes back. The boot
> number sequence will see a hole in it, which is a clue that something
> bad happened.
> - The machine is in a crash loop. This has the same failure mode
> for us as if the machine never made it onto the network due to
> whatever reason: bad cables, bad firmware, bad ram, ...
>
> In both cases, we can detect that something is wrong and handle it.
> Note that our firmware is responsible for incrementing the boot
> sequence at bootup, which is why the above works. In general though,
> our machines do make it up to userland -- staying alive once booted is
> the hard part ;)
Interesting. Is this Google-specific firmware magic? I'd probably accept
a hook in random.c to fold a number into the UUID, which would unify
things.
--
Mathematics is the supreme nostalgia of our time.
^ permalink raw reply
* Re: [PATCH 12/12] cxgb4: NUMA-aware Tx queue allocations
From: Dimitris Michailidis @ 2010-12-14 22:51 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1292361479.2478.2.camel@edumazet-laptop>
Eric Dumazet wrote:
> Le mardi 14 décembre 2010 à 12:18 -0800, Dimitris Michailidis a écrit :
>> Allocate Tx queue memory on the node indicated by the new
>> netdev_queue_numa_node_read. If that fails we allocate on any node.
>>
>> Signed-off-by: Dimitris Michailidis <dm@chelsio.com>
>> ---
>> drivers/net/cxgb4/sge.c | 20 +++++++++++++-------
>> 1 files changed, 13 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/net/cxgb4/sge.c b/drivers/net/cxgb4/sge.c
>> index cc0b997..ed98b8a 100644
>> --- a/drivers/net/cxgb4/sge.c
>> +++ b/drivers/net/cxgb4/sge.c
>> @@ -579,6 +579,7 @@ static inline void __refill_fl(struct adapter *adap, struct sge_fl *fl)
>> * @phys: the physical address of the allocated ring
>> * @metadata: address of the array holding the SW state for the ring
>> * @stat_size: extra space in HW ring for status information
>> + * @node: preferred node for memory allocations
>> *
>> * Allocates resources for an SGE descriptor ring, such as Tx queues,
>> * free buffer lists, or response queues. Each SGE ring requires
>> @@ -590,7 +591,7 @@ static inline void __refill_fl(struct adapter *adap, struct sge_fl *fl)
>> */
>> static void *alloc_ring(struct device *dev, size_t nelem, size_t elem_size,
>> size_t sw_size, dma_addr_t *phys, void *metadata,
>> - size_t stat_size)
>> + size_t stat_size, int node)
>> {
>> size_t len = nelem * elem_size + stat_size;
>> void *s = NULL;
>> @@ -599,7 +600,10 @@ static void *alloc_ring(struct device *dev, size_t nelem, size_t elem_size,
>> if (!p)
>> return NULL;
>> if (sw_size) {
>> - s = kcalloc(nelem, sw_size, GFP_KERNEL);
>> + if (node >= 0)
>> + s = kzalloc_node(nelem * sw_size, GFP_KERNEL, node);
>
> kzalloc_node() has a fallback, you dont need to retry with kcalloc()
I took this retry part from ixgbe but I can remove it if it's not needed.
Luckily it's the last patch in the series.
>
>> + if (!s)
>> + s = kcalloc(nelem, sw_size, GFP_KERNEL);
>>
>> if (!s) {
>> dma_free_coherent(dev, len, p, *phys);
>
> Also, I am not sure it is going to work, since we can setup XPS only
> after device being setup ?
>
> By the time your driver allocates rings, we probably read
> -1/NUMA_NO_NODE
XPS is available after registration. The queues are allocated at open time,
if one configures XPS prior to that the allocations happen on the right
nodes. I've tried this and this is the behavior I see. It is true that
setting XPS after open doesn't affect the queue allocations.
^ permalink raw reply
* RE: [RFC][net-next-2.6 PATCH 0/2] rtnetlink: New IFLA_PORT_PROTO_* attr
From: Christian Benvenuti (benve) @ 2010-12-14 22:59 UTC (permalink / raw)
To: Arnd Bergmann
Cc: netdev, davem, Stefan Berger, Roopa Prabhu (roprabhu),
David Wang (dwang2)
In-Reply-To: <201012141647.38303.arnd@arndb.de>
Hi,
> > In order to be able to scope a port profile, as part of the 802.1Qbh
> > implementation we would like to add a new attribute:
> > IFLA_PORT_CLUSTER_UUID.
> > This parameter (perhaps known under a different name) is already in
> > use (or going to be added) by most Virtual Machine Managers to
> > define migration domains.
> > In the case of 802.1Qbh a port profile would most likely be scoped
> > using the same ID used by VM manager to represent the migration
> > domain.
> >
> > Adding another attribute (IFLA_PORT_CLUSTER_UUID in this case) to
> > the list of IFLA_PORT_* attributes is an option.
>
> Sounds reasonable.
Good.
> > However, we thought that it would be better to 1st re-arrange the
> > current Netlink attribute scheme in order to better group the
> > IFLA_PORT_* attributes (for example by protocol).
>
> We don't normally rearrange protocols once they are in an upstream
> release. Having to maintain compatibility to two different versions of
> the API is a huge burden for maintainance, so IMHO the only reason why
> we should deprecate the current API and introduce a new one is if it
> is absolutely impossible to implement necessary features without
> breaking compatibility. Your explanations are very detailed and well
> explained, but I have not found anything in there that describes why
> it cannot be done without changing the existing interface.
You are right, we do not need to change the current interface in
order to add a new attribute like IFLA_PORT_CLUSTER_UUID.
We just thought that before to add a new attribute it would have
made sense to 1st re-organize (cleanup) the attributes and group
them by protocol, given that:
- the current list of attributes is a mix of attributes from two
protocols (ie, not all of them are shared)
and
- new (not shared) attributes may need to be added in the future
(given the state of the protocols)
If we do not do it now, for sure later it will be either more
painful or impossible (as of now we would need to deprecate only
two attributes and nothing would break).
It is not a mandatory step, but to us it would make sense to think
in the long term too.
> > --------------------------------------------------
> > 2) REASON FOR THIS CHANGE
> > --------------------------------------------------
> > We would like to add one more attribute (IFLA_PORT_CLUSTER_UUID),
> > and the list of IFLA_PORT_* attributes may need to grow again due to
> > the changes that may be required by the two still evolving standard
> > protocols 802.1Qbh/802.1Qbg.
> > Because of that, if you see a value in the re-organization of the
> > IFLA_PORT_* attributes that we are proposing, it would be better to
> > address such changes sooner than later, in order to reduce the
> > impact of backward compatibility issues later.
>
> The changes you propose now seem reasonable and we could probably have
> done it that way initially, but as far as I'm concerned they are too
> late.
> The burden imposed by the change is larger than the risk of breaking
> backwards compatibility later by not fixing it now, as far as I'm
> concerned.
We would only deprecate two attributes and nothing would break actually.
> To give another example, the split between IFLA_VFINFO_LIST and
> IFLA_VF_PORTS is totally arbitrary, we should have merged them at the
> time, but because of timing concerns of the two going in during the
> merge window, we are stuck with two separate lists of VFs now, and I
> don't think we should change them any more.
> > --------------------------------------------------
> > 4) IFLA_* versus IFLA_PORT_*
> > --------------------------------------------------
> >
> > Here is an alternative way to introduce the new Netlink attribute
> > scheme. We personally like better the previous scheme, but I'll
> > include this one too should someone find it interesting.
>
> The impact of doing this would be even bigger.
>
> > OPTION_3: According to the new Netlink attribute scheme that we
> > are proposing, each protocol has its own set of
> > attributes and therefore it would not be considered
> > superfluous to have the same (or a similar) attribute
> > defined for both protocols.
> > (in this case it would be manager_ID for 802.1qbg and
> > cluster_uuid for 802.1qbh).
> >
> > To us OPTION_3 looks like the option that offers most flexibility.
>
> I don't see this depending on the change to split attributes per
> protocol. Just introducing a new IFLA_PORT_CLUSTER_UUID should be all
> you need. Since a cluster UUID is not exactly the same concept as a
> vsi manager id, there is no need to share the same netlink attribute.
Right now it is all we need (well, we need a CLIENT_TYPE too actually).
However, the redesign we proposed was not just to get CLUSTER_UUID
in, but it would rather be a change aimed at keeping the code cleaner
in the long term too (I think it is reasonable to think that more
changes to that IFLA_PORT_* list are likely to happen given the
focus there is nowadays on the virt area (new protocols, new extensions, etc).
> > --------------------------------------------------
> > 8) OPT1: MORE CONFIGURATION FLEXIBILITY
> > --------------------------------------------------
> > The change described in this section is orthogonal to the ones
> > discussed above.
> > We believe it would add value to the new scheme.
> > We would like to include it as part of the new Netlink scheme (but
> > the current patch does not include it).
> >
> > In order to allow device drivers (or a generic consumer of the
> > Netlink messages) to provide extra features or simple optimizations
> > I would suggest the introduction of a new nested attribute that I
> > will call IFLA_PORT_DATA for now.
> >
> > This attribute would allow the use of extra attributes that are not
> > part of the official protocol specs (802.1Qbg/bh for now) or simply
> > allow device drivers to start supporting pre-standard parameters
> > that would not be included in the Netlink scheme before they reach
> > some stability.
>
> I really don't think that you should add per-driver attributes. If we
> believe that we need an extension for a specific feature in the
> netlink interface, it should be defined in a way that is generic
> enough to work for other hardware implementing the same feature.
In general, yes, that's the way to go.
However, the use case of IFLA_PORT_DATA would be that where the
"extra config" is NOT generic enough to be shared with other hardware
implementing the same feature. And because of that it may be hard
to justify a change to the "shared" attribute scheme to include
support for a vendor specific extra attribute.
> > Here are a couple of examples of use.
> > Let's suppose that driver ABC needed to receive a couple of
> > parameters more (that are not part of the official 802.1Qbh/bg
> > protocols).
> > In this case driver ABC can use the new attribute IFLA_PORT_DATA to
> > receive its two additional parameters without any need to
> > touch/modify the IFLA_PORT_* list of attributes.
>
> We can in theory add features that are not part of the official
> standard. IMHO it is more important that the features are of general
> interest and are being actively used. They should of course not
> conflict with other features or the standard.
I agree.
However, from a vendor/driver perspective, what matters more is the
functionality/usefulness/performance_gain of the optional feature, and
not the likelihood of having other vendors support the same feature.
The idea behind IFLA_PORT_DATA is that of not having to change the
Netlink (shared) attribute scheme to support anything that is not
yet standard or will never be. As I said in the original email, it
is just a way to pass more info down the stack synchronously with
the shared attributes/data.
Here is an example. Supposing there was _not_ agreement on the
introduction of the a new IFLA_PORT_XYZ attribute because considered
too vendor/driver-centric.
In that case you either pass (async) XYZ to the driver using something
like the Generic Netlink proto or sysfs or ... one cleaner option would
be that of using IFLA_PORT_DATA, which would guarantee that the extra
config gets sent synchronously with the rest of the config.
> > If in the future driver ABC needed to change any of its private
> > parameters (those it receives through the IFLA_PORT_DATA attribute),
> > it can do it by updating its parsing routine (of course it would
> > need to implement a basic versioning scheme for its private
> > attributes), but no change would be required in the core Netlink
> > code.
>
> A data structure being private to a driver would not save you from
> maintaining backwards compatibility, you still cannot just go and
> change it as you like.
Yes of course.
However in this case the driver itself would take care of it
(RTNetlink does not need to know what is inside the IFLA_PORT_DATA
attribute).
> > If we do not want to add IFLA_PORT_DATA, an alternative solution
> > would be that of using a separate control channel to provide that
> > extra info, for example based on something like the NETLINK_GENERIC
> > Netlink protocol.
> > This alternative approach would offer the same flexibility, but I
> > can see One drawback: this solution would require some extra code to
> > synchronize the two control channels (generic
> > NETLINK_ROUTE/IFLA_PORT_XXX and NETLINK_GENERIC/Driver).
>
> Right, using generic netlink for this does not help, it has all the
> problems of your IFLA_PORT_DATA suggestions and is more complex.
Agree.
> Just don't add driver-private interfaces, make them official!
I would not look at IFLA_PORT_DATA as a private-interface replacing
the public one/s, but rather as a way to make it easier to configure
new extensions without having to wait for a standard/spec to show up
and make a case for the change to the shared Netlink scheme.
Anyway, IFLA_PORT_DATA was just a proposal (which I think makes
sense) and can be added/revisited at any time, not necessarily now.
Since we were proposing a change to the Netlink scheme, I thought
it was the right context to mention it too.
> If we give driver writers a way to add their own interfaces, there is
> a very realistic risk of these interface being defined in a broken
> way, with people relying on them before the code gets submitted for
> mainline inclusion.
Of course there is the risk that some device driver writers will
abuse it. I think that by properly documenting it, device driver
writers should think twice before abusing it.
In summary, here are the three points I would like to reach an
agreement on:
(A) Redesign of IFLA_PORT_* attributes as described in the
original patch 0/2 post
YES/NO?
(I vote for "Yes")
(B) Introduction of the following new attributes (with
CLUSTER_UUID being top priority):
- IFLA_PORT_CLUSTER_UUID : string UUID
Used to scope the port profile
- IFLA_PORT_CLIENT_TYPE : string
Used to identify the type of entity using the port
profile (OS type, etc).
If we go for A/YES, the above two attributes would go
into the new IFLA_PORT_8021QBH_* attr list.
If we go for A/NO, they would be added to the
IFLA_PORT_* attribute list and would be usable by all
protocols.
(C) Attribute versioning: #ifdef vs GET_VERSION
On the libvirt side we need to #ifdef each time the attribute
list changes. This is the default way of handling this kind
of situation, however, by adding support for versioning (see
my previous post) libvirt could detect the attribute "version"
at run-time.
I am fine with going with #ifdefs, unless someone expresses some
interest in adding support for versioning (I would add it).
I am OK in both cases (I think versioning is better but I
understand the counter-argument).
As soon as we converge on (A)/(B)/(C) I'll post the patches for
- Kernel
- libvirt
- iproute2.
/Christian
^ permalink raw reply
* Re: [PATCH v3 02/22] netconsole: Introduce locking over the netpoll fields
From: Michał Mirosław @ 2010-12-14 23:15 UTC (permalink / raw)
To: Mike Waychison
Cc: simon.kagstrom-vI6UBbBVNY+JA8cjQkG2/g,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, nhorman-2XuSBdqkA4R54TAoqtyWWQ,
Matt Mackall, adurbin-hpIqsD4AKlfQT0dZR+AlfA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
chavey-hpIqsD4AKlfQT0dZR+AlfA, Greg KH,
netdev-u79uwXL29TY76Z2rM5mHXA, Américo Wang,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20101214212904.17022.16604.stgit-tzAwxxnF6Tt6FDdRrpk8kO4/NqBCd+6Q@public.gmane.org>
2010/12/14 Mike Waychison <mikew-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>:
> The netconsole driver currently doesn't do any locking over its
> configuration fields. This can cause problems if we were to ever have
> concurrent writing to fields while somebody is enabling the service.
>
> For simplicity, this patch extends targets_list_lock to cover all
> configuration fields within the targets. Macros are also added here to
> wrap accessors so that we check whether the target has been enabled with
> locking handled.
>
> Signed-off-by: Mike Waychison <mikew-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> Acked-by: Matt Mackall <mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ@public.gmane.org>
> ---
> drivers/net/netconsole.c | 114 ++++++++++++++++++++++++++--------------------
> 1 files changed, 64 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index c87a49e..6e16888 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -327,6 +327,7 @@ static ssize_t store_enabled(struct netconsole_target *nt,
> const char *buf,
> size_t count)
> {
> + unsigned long flags;
> int err;
> long enabled;
>
> @@ -335,6 +336,10 @@ static ssize_t store_enabled(struct netconsole_target *nt,
> return enabled;
>
> if (enabled) { /* 1 */
> + spin_lock_irqsave(&target_list_lock, flags);
> + if (nt->enabled)
> + goto busy;
> + spin_unlock_irqrestore(&target_list_lock, flags);
>
This looks wrong. Unless there is another lock or mutex covering this
function, at this point (after spin_unlock_irqrestore()) another
thread might set nt->enabled = 1.
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: [PATCH v3 21/22] netoops: Add user-programmable boot_id
From: Mike Waychison @ 2010-12-14 23:19 UTC (permalink / raw)
To: Matt Mackall
Cc: simon.kagstrom-vI6UBbBVNY+JA8cjQkG2/g,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, nhorman-2XuSBdqkA4R54TAoqtyWWQ,
adurbin-hpIqsD4AKlfQT0dZR+AlfA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
chavey-hpIqsD4AKlfQT0dZR+AlfA, Greg KH,
netdev-u79uwXL29TY76Z2rM5mHXA, Américo Wang,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1292366864.3446.875.camel@calx>
On Tue, Dec 14, 2010 at 2:47 PM, Matt Mackall <mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ@public.gmane.org> wrote:
> On Tue, 2010-12-14 at 14:33 -0800, Mike Waychison wrote:
>> On Tue, Dec 14, 2010 at 2:06 PM, Matt Mackall <mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ@public.gmane.org> wrote:
>> > What happens if you oops before userspace is available?
>> >
>>
>> Either one of two general cases:
>> - The crash is a one-off and the machine comes back. The boot
>> number sequence will see a hole in it, which is a clue that something
>> bad happened.
>> - The machine is in a crash loop. This has the same failure mode
>> for us as if the machine never made it onto the network due to
>> whatever reason: bad cables, bad firmware, bad ram, ...
>>
>> In both cases, we can detect that something is wrong and handle it.
>> Note that our firmware is responsible for incrementing the boot
>> sequence at bootup, which is why the above works. In general though,
>> our machines do make it up to userland -- staying alive once booted is
>> the hard part ;)
>
> Interesting. Is this Google-specific firmware magic?
Ya, this is a Google-ism. I'd be surprised if there weren't other
platforms that had the same thing though (though I don't know of
anything standard on x86).
> I'd probably accept
> a hook in random.c to fold a number into the UUID, which would unify
> things.
I'm not sure there is a _good_ way to support this, is there? I just
read through RFC 4122 and UUIDs seem to be pretty well structured;
it's probably not such a great idea to allow folks to override
portions of it. Like I mentioned in my last email though, I'm okay
pushing this boot sequence ID down into the user blob which acts like
a place for "vendor extensions" if there isn't a good place for it in
the kernel.
^ permalink raw reply
* Re: [PATCH v3 03/22] netconsole: Introduce 'enabled' state-machine
From: Michał Mirosław @ 2010-12-14 23:21 UTC (permalink / raw)
To: Mike Waychison
Cc: simon.kagstrom-vI6UBbBVNY+JA8cjQkG2/g,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, nhorman-2XuSBdqkA4R54TAoqtyWWQ,
Matt Mackall, adurbin-hpIqsD4AKlfQT0dZR+AlfA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
chavey-hpIqsD4AKlfQT0dZR+AlfA, Greg KH,
netdev-u79uwXL29TY76Z2rM5mHXA, Américo Wang,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20101214212909.17022.96801.stgit-tzAwxxnF6Tt6FDdRrpk8kO4/NqBCd+6Q@public.gmane.org>
2010/12/14 Mike Waychison <mikew-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>:
> Representing the internal state within netconsole isn't really a boolean
> value, but rather a state machine with transitions.
[...]
> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index 6e16888..288a025 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
[...]
> @@ -350,9 +362,9 @@ static ssize_t store_enabled(struct netconsole_target *nt,
> err = netpoll_setup(&nt->np);
> spin_lock_irqsave(&target_list_lock, flags);
> if (err)
> - nt->enabled = 0;
> + nt->np_state = NETPOLL_DISABLED;
> else
> - nt->enabled = 1;
> + nt->np_state = NETPOLL_ENABLED;
> spin_unlock_irqrestore(&target_list_lock, flags);
> if (err)
> return err;
[...]
Since the spinlock protects only nt->np_state setting, you might be
able to remove it altogether and use cmpxchg() where nt->np_state
transitions from enabled or disabled state.
Maybe the locking scheme just needs more thought altogether?
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: [PATCH v3 02/22] netconsole: Introduce locking over the netpoll fields
From: Mike Waychison @ 2010-12-14 23:30 UTC (permalink / raw)
To: Michał Mirosław
Cc: simon.kagstrom, davem, nhorman, Matt Mackall, adurbin,
linux-kernel, chavey, Greg KH, netdev, Américo Wang, akpm,
linux-api
In-Reply-To: <AANLkTi=reqRMrv4k5EZKdj7pmsomApw9=Kwh9nxfKCuJ@mail.gmail.com>
2010/12/14 Michał Mirosław <mirqus@gmail.com>:
> 2010/12/14 Mike Waychison <mikew@google.com>:
>> The netconsole driver currently doesn't do any locking over its
>> configuration fields. This can cause problems if we were to ever have
>> concurrent writing to fields while somebody is enabling the service.
>>
>> For simplicity, this patch extends targets_list_lock to cover all
>> configuration fields within the targets. Macros are also added here to
>> wrap accessors so that we check whether the target has been enabled with
>> locking handled.
>>
>> Signed-off-by: Mike Waychison <mikew@google.com>
>> Acked-by: Matt Mackall <mpm@selenic.com>
>> ---
>> drivers/net/netconsole.c | 114 ++++++++++++++++++++++++++--------------------
>> 1 files changed, 64 insertions(+), 50 deletions(-)
>>
>> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
>> index c87a49e..6e16888 100644
>> --- a/drivers/net/netconsole.c
>> +++ b/drivers/net/netconsole.c
>> @@ -327,6 +327,7 @@ static ssize_t store_enabled(struct netconsole_target *nt,
>> const char *buf,
>> size_t count)
>> {
>> + unsigned long flags;
>> int err;
>> long enabled;
>>
>> @@ -335,6 +336,10 @@ static ssize_t store_enabled(struct netconsole_target *nt,
>> return enabled;
>>
>> if (enabled) { /* 1 */
>> + spin_lock_irqsave(&target_list_lock, flags);
>> + if (nt->enabled)
>> + goto busy;
>> + spin_unlock_irqrestore(&target_list_lock, flags);
>>
>
> This looks wrong. Unless there is another lock or mutex covering this
> function, at this point (after spin_unlock_irqrestore()) another
> thread might set nt->enabled = 1.
>
Agreed that this looks wrong :)
It is fixed in the next patch where a state machine is introduced to
replace the binary flag nt->enabled. The code before this patch had
the a very similar problem in that a target could be enabled twice.
store_enabled() would call netpoll_setup() the second time without
checking to see if it was already enabled.
^ permalink raw reply
* Re: [PATCH v3 03/22] netconsole: Introduce 'enabled' state-machine
From: Mike Waychison @ 2010-12-14 23:33 UTC (permalink / raw)
To: Michał Mirosław
Cc: simon.kagstrom, davem, nhorman, Matt Mackall, adurbin,
linux-kernel, chavey, Greg KH, netdev, Américo Wang, akpm,
linux-api
In-Reply-To: <AANLkTik8wzDPPtMM+YPoOSojiW_Nev-KDkAg8dtDV=g1@mail.gmail.com>
2010/12/14 Michał Mirosław <mirqus@gmail.com>:
> 2010/12/14 Mike Waychison <mikew@google.com>:
>> Representing the internal state within netconsole isn't really a boolean
>> value, but rather a state machine with transitions.
> [...]
>> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
>> index 6e16888..288a025 100644
>> --- a/drivers/net/netconsole.c
>> +++ b/drivers/net/netconsole.c
> [...]
>> @@ -350,9 +362,9 @@ static ssize_t store_enabled(struct netconsole_target *nt,
>> err = netpoll_setup(&nt->np);
>> spin_lock_irqsave(&target_list_lock, flags);
>> if (err)
>> - nt->enabled = 0;
>> + nt->np_state = NETPOLL_DISABLED;
>> else
>> - nt->enabled = 1;
>> + nt->np_state = NETPOLL_ENABLED;
>> spin_unlock_irqrestore(&target_list_lock, flags);
>> if (err)
>> return err;
> [...]
>
> Since the spinlock protects only nt->np_state setting, you might be
> able to remove it altogether and use cmpxchg() where nt->np_state
> transitions from enabled or disabled state.
>
> Maybe the locking scheme just needs more thought altogether?
The target_list_lock protects the list, as well as the state
transitions. This makes iterating through the list and getting a
consistent view of the targets a lot easier when it comes time to
transmitting packets we are guaranteed that nobody is changing the
target state underneath us if nt->np_state == NETPOLL_ENABLED while we
hold the lock.
^ permalink raw reply
* Re: [*v2 PATCH 00/22] IPVS, Network Name Space aware
From: Julian Anastasov @ 2010-12-14 23:43 UTC (permalink / raw)
To: Hans Schillstrom
Cc: horms, daniel.lezcano, wensong, lvs-devel, netdev,
netfilter-devel, hans
In-Reply-To: <1292247510-753-1-git-send-email-hans.schillstrom@ericsson.com>
Hello,
On Mon, 13 Dec 2010, Hans Schillstrom wrote:
> This patch series adds network name space support to the LVS.
>
> REVISION
>
> This is version 2
>
> OVERVIEW
>
> The patch doesn't remove or add any functionality except for netns.
> For users that don't use network name space (netns) this patch is
> completely transparent.
>
> Now it's possible to run LVS in a Linux container (see lxc-tools)
> i.e. a light weight visualization. For example it's possible to run
> one or several lvs on a real server in their own network name spaces.
>> From the LVS point of view it looks like it runs on it's own machine.
>
> IMPLEMENTATION
> Basic requirements for netns awareness
> - Global variables has to be moved to dyn. allocated memory.
> - No or very little performance loss
>
> Large hash tables connection hash and service hashes still resides in
> global memory with net ptr added in hash key.
> Most global variables now resides in a struct ipvs { } in netns/ip_vs.h.
> The size of per name space is 2004 bytes (for x86_64) and a little bit less
> for 32 bit archs.
>
> Statistics counters is now lock-free i.e. incremented per CPU,
> The estimator does a sum when using it.
>
> Procfs ip_vs_stats is also changed to reflect the "per cpu"
> ex.
> # cat /proc/net/ip_vs_stats
> Total Incoming Outgoing Incoming Outgoing
> CPU Conns Packets Packets Bytes Bytes
> 0 0 3 1 9D 34
> 1 0 1 2 49 70
> 2 0 1 2 34 76
> 3 1 2 2 70 74
> ~ 1 7 7 18A 18E
>
> Conns/s Pkts/s Pkts/s Bytes/s Bytes/s
> 0 0 0 0 0
>
> Algorithm files are untouched except for lblc and lblcr.
Great! I have some small comments after first look:
v2 PATCH 01/22 - basic init
- first change in ip_vs_conn.c adds existing code:
/* Compute size and mask */
v2 PATCH 02/22 - services part 1
- net = skb_net(skb) in ip_vs_out must be after
check for skb_dst. The skb_dst checks are in ip_vs_in and
ip_vs_out, so skb_net() can be used only after these checks.
- __ip_vs_service_find and __ip_vs_svc_fwm_find are fast path,
may be net_eq(svc->net, net) check can be last, I assume
the different netns will use different VIPs and VPORTs?
- ip_vs_svc_table and ip_vs_svc_fwm_table are not per-ns,
so we can not use per-ns mutex in patch 17
v2 PATCH 03/22 - lblcr
v2 PATCH 04/22 - lblc
v2 PATCH 05/22 - prepare protocol
v2 PATCH 06/22 - tcp
v2 PATCH 07/22 - udp
v2 PATCH 08/22 - sctp
v2 PATCH 09/22 - AH, ESP
v2 PATCH 10/22 - use ip_vs_proto_data as param
- update_defense_level: are per-ns memory stats/limits possible?
- The pp -> pd conversion should start from functions like
ip_vs_out() that use pp = ip_vs_proto_get(iph.protocol),
now they should use
ip_vs_proto_data_get(net, iph.protocol). If
pp is needed, it is available from pd->pp. Many functions
that provide pp as argument should now provide pd.
Then 2nd lookups for proto like in ip_vs_set_state should
disappear.
- copy-and-paste bug in ip_vs_ctl.c:ip_vs_set_timeout():
pd = ip_vs_proto_data_get(net, IPPROTO_TCP)
should be IPPROTO_UDP
- may be ip_vs_protocol_timeout_change should propagate
event to all pd, not all pp?
v2 PATCH 11/22 - appcnt
v2 PATCH 12/22 - apps
v2 PATCH 13/22 - ip_vs_est
- estimation_timer: what protection is needed for for_each_net?
It is rtnl for user context and RCU for softirq?
May be est_timer must be per NS? Now may be rcu_read_lock is
needed before for_each_net_rcu ? for_each_net can be called
only under rtnl_lock?
v2 PATCH 14/22 - ip_vs_sync
v2 PATCH 15/22 - ip_vs_stats
- This was one of the hurdles for IPVS RCU conversion, the others
being dest->svc->stats and scheduler state. But can this
change break some scripts that parse /proc/net/ip_vs_stats ?
v2 PATCH 16/22 - connection hash
v2 PATCH 17/22 - ip_vs_ctl local vars
- I hope it is not fatal if __ip_vs_mutex remains global
because svc lists are global in patch 2
v2 PATCH 18/22 - defense work
v2 PATCH 19/22 - trash
v2 PATCH 20/22 - global svc counters
v2 PATCH 21/22 - init_net removal
v2 PATCH 22/22 - enable netns
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: [PATCH update 3] firewire: net: add carrier detection
From: Maxim Levitsky @ 2010-12-14 23:46 UTC (permalink / raw)
To: Stefan Richter; +Cc: Ben Hutchings, Peter Stuge, linux1394-devel, netdev
In-Reply-To: <20101214000154.06ec5153@stein>
On Tue, 2010-12-14 at 00:01 +0100, Stefan Richter wrote:
> From: Maxim Levitsky <maximlevitsky@gmail.com>
>
> To make userland, e.g. NetworkManager work with firewire, we need to
> detect whether cable is plugged or not. Simple and correct way of doing
> that is just counting number of peers. No peers - no link and vice
> versa.
>
> (Stefan R.: Combined peer_count inc/dec with tests, added link-down
> recognition in fwnet_open, added include.)
>
> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
> ---
> Update 3: added fwnet_open (ifup) hunk
Thank you very much.
I tested this just in case, and it works just fine.
Especially thanks for the missing carrier state initialization,
Just one note, since you pretty much rewrite that little patch, it would
be correct to attribute you as an author of it.
I was recently (and still am) busy with my DVB card, so I didn't test my
original patch enough.
Beyond this patch, firewire networking is pretty much complete.
The only missing parts are IPV6 and multicast, and the latter works by
using broadcast which for all means is enough.
>
> drivers/firewire/net.c | 29 +++++++++++++++++++++++++----
> 1 file changed, 25 insertions(+), 4 deletions(-)
>
> Index: b/drivers/firewire/net.c
> ===================================================================
> --- a/drivers/firewire/net.c
> +++ b/drivers/firewire/net.c
> @@ -9,6 +9,7 @@
> #include <linux/bug.h>
> #include <linux/delay.h>
> #include <linux/device.h>
> +#include <linux/ethtool.h>
> #include <linux/firewire.h>
> #include <linux/firewire-constants.h>
> #include <linux/highmem.h>
> @@ -179,6 +180,7 @@ struct fwnet_device {
> /* Number of tx datagrams that have been queued but not yet acked */
> int queued_datagrams;
>
> + int peer_count;
> struct list_head peer_list;
> struct fw_card *card;
> struct net_device *netdev;
> @@ -1234,6 +1236,13 @@ static int fwnet_open(struct net_device
> }
> netif_start_queue(net);
>
> + mutex_lock(&fwnet_device_mutex);
> + if (dev->peer_count > 1)
> + netif_carrier_on(net);
> + else
> + netif_carrier_off(net);
> + mutex_unlock(&fwnet_device_mutex);
> +
> return 0;
> }
>
> @@ -1412,6 +1421,10 @@ static const struct net_device_ops fwnet
> .ndo_change_mtu = fwnet_change_mtu,
> };
>
> +static const struct ethtool_ops fwnet_ethtool_ops = {
> + .get_link = ethtool_op_get_link,
> +};
> +
> static void fwnet_init_dev(struct net_device *net)
> {
> net->header_ops = &fwnet_header_ops;
> @@ -1423,6 +1436,7 @@ static void fwnet_init_dev(struct net_de
> net->hard_header_len = FWNET_HLEN;
> net->type = ARPHRD_IEEE1394;
> net->tx_queue_len = FWNET_TX_QUEUE_LEN;
> + net->ethtool_ops = &fwnet_ethtool_ops;
> }
>
> /* caller must hold fwnet_device_mutex */
> @@ -1465,6 +1479,10 @@ static int fwnet_add_peer(struct fwnet_d
> list_add_tail(&peer->peer_link, &dev->peer_list);
> spin_unlock_irq(&dev->lock);
>
> + /* serialized by fwnet_device_mutex */
> + if (++dev->peer_count > 1)
> + netif_carrier_on(dev->netdev);
> +
> return 0;
> }
>
> @@ -1543,13 +1561,16 @@ static int fwnet_probe(struct device *_d
> return ret;
> }
>
> -static void fwnet_remove_peer(struct fwnet_peer *peer)
> +static void fwnet_remove_peer(struct fwnet_peer *peer, struct fwnet_device *dev)
> {
> struct fwnet_partial_datagram *pd, *pd_next;
>
> - spin_lock_irq(&peer->dev->lock);
> + if (--dev->peer_count == 1)
> + netif_carrier_off(dev->netdev);
> +
> + spin_lock_irq(&dev->lock);
> list_del(&peer->peer_link);
> - spin_unlock_irq(&peer->dev->lock);
> + spin_unlock_irq(&dev->lock);
>
> list_for_each_entry_safe(pd, pd_next, &peer->pd_list, pd_link)
> fwnet_pd_delete(pd);
> @@ -1566,7 +1587,7 @@ static int fwnet_remove(struct device *_
>
> mutex_lock(&fwnet_device_mutex);
>
> - fwnet_remove_peer(peer);
> + fwnet_remove_peer(peer, dev);
>
> if (list_empty(&dev->peer_list)) {
> net = dev->netdev;
>
>
^ permalink raw reply
* Re: [PATCH v3 14/22] netpoll: Move target code into netpoll_targets.c
From: Mike Waychison @ 2010-12-15 0:07 UTC (permalink / raw)
To: simon.kagstrom, davem, nhorman, Matt Mackall
Cc: adurbin, linux-kernel, chavey, Greg KH, netdev, Américo Wang,
akpm, linux-api
In-Reply-To: <20101214213010.17022.54049.stgit@mike.mtv.corp.google.com>
Ugh. I just realized that I was missing a MODULE_LICENSE("GPL"); for
this new module, otherwise module load fails to find the link for
cancel_work_sync.
---
netpoll: Add missing module license for netpoll_targets
Add a missing module license declaration to netpoll_targets that allows
it to use exported symbols (cancel_work_sync).
Signed-off-by: Mike Waychison <mikew@google.com>
diff --git a/net/core/netpoll_targets.c b/net/core/netpoll_targets.c
index 5fc84f8c..0887268 100644
--- a/net/core/netpoll_targets.c
+++ b/net/core/netpoll_targets.c
@@ -747,3 +747,4 @@ void unregister_netpoll_targets(struct netpoll_targets *nts)
}
EXPORT_SYMBOL_GPL(unregister_netpoll_targets);
+MODULE_LICENSE("GPL");
^ permalink raw reply related
* Re: [PATCH 4/5 v2] ifb: add multiqueue support
From: Changli Gao @ 2010-12-15 0:20 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Jamal Hadi Salim, David S. Miller, netdev
In-Reply-To: <1292342358.5934.10.camel@edumazet-laptop>
On Tue, Dec 14, 2010 at 11:59 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
> There is a problem here.
>
> You should sum in the loop rx_counters only, (the counters syncp
> protected), and use dev_txq_stats_fold() to get the tx_counters from
> core network.
Hmm, in ifb, txq->tx_counters aren't used in ifb_xmit() but
ri_tasklet(). IOW, rx and tx are reversed. Here I use txq->tx_counters
to reduce duplicate counters in ifb_q_private. Unless these variables
might be accessed outside of ifb, I can use them freely? Thanks.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH 2.6.36] vlan: Avoid hwaccel vlan packets when vid not used
From: Michael Leun @ 2010-12-15 0:24 UTC (permalink / raw)
To: Matt Carlson
Cc: Jesse Gross, Michael Chan, Eric Dumazet, David Miller, Ben Greear,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20101214191500.GD19951@mcarlson.broadcom.com>
On Tue, 14 Dec 2010 11:15:00 -0800
"Matt Carlson" <mcarlson@broadcom.com> wrote:
> On Mon, Dec 13, 2010 at 08:07:20PM -0800, Jesse Gross wrote:
> > On Mon, Dec 13, 2010 at 2:45 PM, Matt Carlson
> > <mcarlson@broadcom.com> wrote:
> > > On Sun, Dec 12, 2010 at 04:11:13PM -0800, Jesse Gross wrote:
> > >> On Mon, Dec 6, 2010 at 1:27 PM, Michael Leun
> > >> <lkml20101129@newton.leun.net> wrote:
> > >> > On Mon, 6 Dec 2010 12:04:48 -0800
> > >> > Jesse Gross <jesse@nicira.com> wrote:
> > >> >
> > >> >> On Mon, Dec 6, 2010 at 11:34 AM, Michael Leun
> > >> >> <lkml20101129@newton.leun.net> wrote:
> > >> >> > On Mon, 6 Dec 2010 10:14:55 -0800
> > >> >> > Jesse Gross <jesse@nicira.com> wrote:
> > >> >> >
> > >> >> >> On Sun, Dec 5, 2010 at 2:44 AM, Michael Leun
> > >> >> >> <lkml20101129@newton.leun.net> wrote:
> > >> >> >> > Hi Jesse,
> > >> >> >> >
> > >> >> >> > On Sun, 5 Dec 2010 10:55:28 +0100
> > >> >> >> > Michael Leun <lkml20101129@newton.leun.net> wrote:
> > >> >> >> >
> > >> >> >> >> On Sun, 05 Dec 2010 09:03:53 +0100
> > >> >> >> >> Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > >> >> >> >>
> > >> >> >> >> > > But on
> > >> >> >> >> > >
> > >> >> >> >> > > hpdl320g5:/home/ml # lspci | grep Eth
> > >> >> >> >> > > 03:04.0 Ethernet controller: Broadcom Corporation
> > >> >> >> >> > > NetXtreme BCM5714 Gigabit Ethernet (rev a3) 03:04.1
> > >> >> >> >> > > Ethernet controller: Broadcom Corporation NetXtreme
> > >> >> >> >> > > BCM5714 Gigabit Ethernet (rev a3)
> > >> >> >> >> > >
> > >> >> >> >> > > the good message is that it also does not crash,
> > >> >> >> >> > > but with tcpdump I see vlan tags when no vlan
> > >> >> >> >> > > devices configured on the respective eth, if so I
> > >> >> >> >> > > do not see tags anymore vlan tags on the trunk
> > >> >> >> >> > > interface.
> > >> >> >> >> > >
> > >> >> >> >> >
> > >> >> >> >> > For all these very specific needs, you'll have to try
> > >> >> >> >> > 2.6.37 I am afraid. Jesse did huge changes to exactly
> > >> >> >> >> > make this working, we wont backport this to 2.6.36,
> > >> >> >> >> > but only avoid crashes.
> > >> >> >> >>
> > >> >> >> >> OK, I'm perfectly fine with that, of course, actually
> > >> >> >> >> nice to hear that the issue already is addressed.
> > >> >> >> >>
> > >> >> >> >> Likely I'll give some rc an shot on this machine (maybe
> > >> >> >> >> over christmas), but it is an production machine
> > >> >> >> >> (acutally testing other devices is the "product"
> > >> >> >> >> produced on this machine), so unfortunately I'm not
> > >> >> >> >> that free in when and what I can do (but the
> > >> >> >> >> possibility to, for example, bridge the trunk interface
> > >> >> >> >> would make testing easier, that justifies something...).
> > >> >> >> >>
> > >> >> >> >> Thank you all very much for your work.
> > >> >> >> >
> > >> >> >> > Are these changes already in 2.6.37-rc4? Or, if not are
> > >> >> >> > they somewhere publically available already?
> > >> >> >> >
> > >> >> >> > I looked into various changelogs but have some
> > >> >> >> > difficulties to identify them...
> > >> >> >> >
> > >> >> >> > Maybe I have some time next days to give them an try...
> > >> >> >>
> > >> >> >> Yes, all of the existing vlan changes are in
> > >> >> >> 2.6.37-rc4. ?There were a number of patches but the main
> > >> >> >> one was 3701e51382a026cba10c60b03efabe534fba4ca4
> > >> >> >
> > >> >> > Then, I'm afraid, this (seeing vlan tags even if vlan
> > >> >> > interfaces are configured) does not work on HP DL320G5 (for
> > >> >> > exact description and examples please see my mail a few
> > >> >> > days ago).
> > >> >>
> > >> >> What driver are you using? ?Is it tg3?
> > >> >>
> > >> >> The vlan changes that I made unfortunately require updating
> > >> >> drivers to get the full benefit. ?I've been busy lately so
> > >> >> tg3 hasn't yet been updated.
> > >> >>
> > >> >> I know that tg3 does some things differently depending on
> > >> >> whether a vlan group is configured, so that would likely be
> > >> >> the cause of what you are seeing. ?I'd have to look at it in
> > >> >> more detail to be sure though.
> > >> >>
> > >> >> You said that everything works on the other Broadcom NIC that
> > >> >> you tested? ?Maybe it uses bnx2 instead?
> > >> >>
> > >> >
> > >> > Both machines use tg3 / 2.6.36.1 - one is opensuse, one ubuntu
> > >> > (but this should not matter, I think).
> > >> >
> > >> > If I can do anything to support your investigations / work
> > >> > (most likely testing / providing information) please let me
> > >> > know.
> > >>
> > >> Unfortunately, I probably won't have time to look at this in the
> > >> near future. ?Given that the test works on one NIC but not
> > >> another that strongly suggests that it is a driver problem, even
> > >> if both NICs use the same driver. ?I see tg3 can do different
> > >> things with vlans depending on the model and what features are
> > >> enabled. ?I also ran a quick test on some of my machines and I
> > >> didn't experience this issue. They are running net-next with
> > >> ixgbe.
> > >>
> > >> One of the main goals of my general vlan changes was to remove
> > >> as much logic as possible from the drivers and put it in the
> > >> networking core, so we should in theory see consistent
> > >> behavior. ?However, in 2.6.36 and earlier, each driver knows
> > >> about what vlan devices are configured and does different things
> > >> with that information.
> > >>
> > >> Given all of that, the most logical step to me is simply to
> > >> convert tg3 to use the new vlan infrastructure. ?It should be
> > >> done regardless and it will probably solve this problem. ?Maybe
> > >> you can convince the Broadcom guys to do that? ?It would be a
> > >> lot faster for them to do it than me.
> > >
> > > Below is the patch that converts the tg3 driver over to the new
> > > API. ?I don't see how it could fix the problem though. ?Maybe the
> > > presence of NETIF_F_HW_VLAN_TX changes things.
> >
> > Thanks Matt.
> >
> > There's actually a little bit more that needs to be done for
> > conversion. All references to the vlan group should be gone since
> > that logic has been moved to the networking core.
> > tg3_vlan_rx_register() completely disappears and all other code
> > contained in TG3_VLAN_TAG_USED is unconditionally active. Ideally,
> > there would be an Ethtool set_flags function so that the vlan
> > offloading features could be enabled/disabled for situations like
> > this to help with debugging.
> >
> > The reason why I think that this might help is that the problem
> > manifests when a vlan group is configured, even if that vlan isn't
> > used. Since this removes all logic about vlan groups from the
> > driver, it should avoid any problems in that area. It's possible
> > that the actual issue is somewhere else but then it should be
> > easier to find since we can separate out the different components.
>
> Thanks for the comments Jesse. Below is an updated patch.
>
> Michael, I'm wondering if the difference in behavior can be explained
> by the presence or absence of management firmware. Can you look at
> the driver sign-on messages in your syslogs for ASF[]? I'm half
> expecting the 5752 to show "ASF[0]" and the 5714 to show "ASF[1]".
> If you see this, and the below patch doesn't fix the problem, let me
> know. I have another test I'd like you to run.
Do I understand this correct? "Management firmware" or ASF is some
feature, vendor decides to built into network card (firmware) or not?
If so, would'nt one expect two oneboard network cards in one server
to look alike?
HP Proliant DL320G5
<6>tg3.c:v3.113 (August 2, 2010)
<6>tg3 0000:03:04.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16
<6>tg3 0000:03:04.0: eth0: Tigon3 [partno(N/A) rev 9003] (PCIX:133MHz:64-bit) MAC address xx:xx:xx:xx:xx:xx
<6>tg3 0000:03:04.0: eth0: attached PHY is 5714 (10/100/1000Base-T Ethernet) (WireSpeed[1])
<6>tg3 0000:03:04.0: eth0: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[1] TSOcap[1]
<6>tg3 0000:03:04.0: eth0: dma_rwctrl[76148000] dma_mask[64-bit]
<6>tg3 0000:03:04.1: PCI INT B -> GSI 17 (level, low) -> IRQ 17
<6>tg3 0000:03:04.1: eth1: Tigon3 [partno(N/A) rev 9003] (PCIX:133MHz:64-bit) MAC address xx:xx:xx:xx:xx:xx
<6>tg3 0000:03:04.1: eth1: attached PHY is 5714 (10/100/1000Base-T Ethernet) (WireSpeed[1])
<6>tg3 0000:03:04.1: eth1: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[0] TSOcap[1]
<6>tg3 0000:03:04.1: eth1: dma_rwctrl[76148000] dma_mask[64-bit]
Lenovo ThinkPad z61m
[ 2.679130] tg3.c:v3.113 (August 2, 2010)
[ 2.679176] tg3 0000:02:00.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16
[ 2.679188] tg3 0000:02:00.0: setting latency timer to 64
[ 2.728572] tg3 0000:02:00.0: eth0: Tigon3 [partno(BCM95752m) rev 6002] (PCI Express) MAC address xx:xx:xx:xx:xx:xx
[ 2.728577] tg3 0000:02:00.0: eth0: attached PHY is 5752 (10/100/1000Base-T Ethernet) (WireSpeed[1])
[ 2.728581] tg3 0000:02:00.0: eth0: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[0] TSOcap[1]
[ 2.728585] tg3 0000:02:00.0: eth0: dma_rwctrl[76180000]
dma_mask[64-bit]
> ----
>
> [PATCH] tg3: Use new VLAN code
Unfortunately had'nt time to try much now, but with 2.6.37-rc5 / your
patch on the DL320, single user mode (nothing configured on eth) just
after ifconfig eth0/eth1 up I see NO vlan tags on eth0 but I see vlan
tags on eth1, so there clearly is a difference.
I should have checked if I still see vlan tags on eth1 if I configure
some vlan there - if helpful maybe I can do this (have to look, when I
can effort another downtime).
I wonder, if the difference in that both onboard cards is really there
or if there is some malfunction in detecion?
--
MfG,
Michael Leun
^ permalink raw reply
* linux-next: manual merge of the net tree with the net-current tree
From: Stephen Rothwell @ 2010-12-15 0:45 UTC (permalink / raw)
To: David Miller, netdev; +Cc: linux-next, linux-kernel, Vladislav Zolotarov
[-- Attachment #1: Type: text/plain, Size: 571 bytes --]
Hi all,
Today's linux-next merge of the net tree got a conflict in
drivers/net/bnx2x/bnx2x.h between commit
9f5449611c9d6d7bdcae8020a197d8b4d9b6443c ("bnx2x: Advance a version
number to 1.60.01-0") from the net-current tree and commit
f404c2fea37e02bec7c8b6edddf5edd22ca60505 ("bnx2x: Update version number
and a date") from the net tree.
So the net tree one has a later date, but a lower version number ... I
just used the version from the net tree.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH 1/9] drivers/net: remove unnecessary flush_scheduled_work() calls
From: Jon Mason @ 2010-12-15 0:54 UTC (permalink / raw)
To: Tejun Heo
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
davem@davemloft.net, Wolfgang Grandegger, Stephen Hemminger,
Ramkrishna Vepa, Sivakumar Subramani, Sreenivasa Honnur
In-Reply-To: <1292169185-10579-2-git-send-email-tj@kernel.org>
On Sun, Dec 12, 2010 at 07:52:57AM -0800, Tejun Heo wrote:
> janz-ican3, sh_eth, skge and vxge don't use workqueue at all and there
> is no reason to flush the system_wq. Drop flush_scheduled_work()
> calls and references to workqueue.
I believe you were referencing an older version of the vxge driver.
These is now a reset task that runs on the system workqueue. Based on
your other patches, the flush_scheduled_work should be replaced with a
cancel_delayed_work_sync(&vdev->reset_task)
Thanks,
Jon
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Wolfgang Grandegger <wg@grandegger.com>
> Cc: Stephen Hemminger <shemminger@linux-foundation.org>
> Cc: Ramkrishna Vepa <ramkrishna.vepa@exar.com>
> Cc: Sivakumar Subramani <sivakumar.subramani@exar.com>
> Cc: Sreenivasa Honnur <sreenivasa.honnur@exar.com>
> Cc: Jon Mason <jon.mason@exar.com>
> Cc: netdev@vger.kernel.org
> ---
> drivers/net/can/janz-ican3.c | 9 ---------
> drivers/net/sh_eth.c | 1 -
> drivers/net/sh_eth.h | 1 -
> drivers/net/skge.c | 2 --
> drivers/net/vxge/vxge-main.c | 2 --
> 5 files changed, 0 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/can/janz-ican3.c b/drivers/net/can/janz-ican3.c
> index 6e533dc..b9a6d7a 100644
> --- a/drivers/net/can/janz-ican3.c
> +++ b/drivers/net/can/janz-ican3.c
> @@ -1114,11 +1114,6 @@ static bool ican3_txok(struct ican3_dev *mod)
> /*
> * Recieve one CAN frame from the hardware
> *
> - * This works like the core of a NAPI function, but is intended to be called
> - * from workqueue context instead. This driver already needs a workqueue to
> - * process control messages, so we use the workqueue instead of using NAPI.
> - * This was done to simplify locking.
> - *
> * CONTEXT: must be called from user context
> */
> static int ican3_recv_skb(struct ican3_dev *mod)
> @@ -1251,7 +1246,6 @@ static irqreturn_t ican3_irq(int irq, void *dev_id)
> * Reset an ICAN module to its power-on state
> *
> * CONTEXT: no network device registered
> - * LOCKING: work function disabled
> */
> static int ican3_reset_module(struct ican3_dev *mod)
> {
> @@ -1262,9 +1256,6 @@ static int ican3_reset_module(struct ican3_dev *mod)
> /* disable interrupts so no more work is scheduled */
> iowrite8(1 << mod->num, &mod->ctrl->int_disable);
>
> - /* flush any pending work */
> - flush_scheduled_work();
> -
> /* the first unallocated page in the DPM is #9 */
> mod->free_page = DPM_FREE_START;
>
> diff --git a/drivers/net/sh_eth.c b/drivers/net/sh_eth.c
> index b12660d..819c175 100644
> --- a/drivers/net/sh_eth.c
> +++ b/drivers/net/sh_eth.c
> @@ -1552,7 +1552,6 @@ static int sh_eth_drv_remove(struct platform_device *pdev)
>
> sh_mdio_release(ndev);
> unregister_netdev(ndev);
> - flush_scheduled_work();
> pm_runtime_disable(&pdev->dev);
> free_netdev(ndev);
> platform_set_drvdata(pdev, NULL);
> diff --git a/drivers/net/sh_eth.h b/drivers/net/sh_eth.h
> index 8b47763..efa6422 100644
> --- a/drivers/net/sh_eth.h
> +++ b/drivers/net/sh_eth.h
> @@ -26,7 +26,6 @@
> #include <linux/module.h>
> #include <linux/kernel.h>
> #include <linux/spinlock.h>
> -#include <linux/workqueue.h>
> #include <linux/netdevice.h>
> #include <linux/phy.h>
>
> diff --git a/drivers/net/skge.c b/drivers/net/skge.c
> index 220e039..8c1404b 100644
> --- a/drivers/net/skge.c
> +++ b/drivers/net/skge.c
> @@ -4012,8 +4012,6 @@ static void __devexit skge_remove(struct pci_dev *pdev)
> if (!hw)
> return;
>
> - flush_scheduled_work();
> -
> dev1 = hw->dev[1];
> if (dev1)
> unregister_netdev(dev1);
> diff --git a/drivers/net/vxge/vxge-main.c b/drivers/net/vxge/vxge-main.c
> index b771e4b..537ad87 100644
> --- a/drivers/net/vxge/vxge-main.c
> +++ b/drivers/net/vxge/vxge-main.c
> @@ -3439,8 +3439,6 @@ static void vxge_device_unregister(struct __vxge_hw_device *hldev)
>
> strncpy(buf, dev->name, IFNAMSIZ);
>
> - flush_scheduled_work();
> -
> /* in 2.6 will call stop() if device is up */
> unregister_netdev(dev);
>
> --
> 1.7.1
>
The information and any attached documents contained in this message
may be confidential and/or legally privileged. The message is
intended solely for the addressee(s). If you are not the intended
recipient, you are hereby notified that any use, dissemination, or
reproduction is strictly prohibited and may be unlawful. If you are
not the intended recipient, please contact the sender immediately by
return e-mail and destroy all copies of the original message.
^ 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