* Re: UDP path MTU discovery
From: Andi Kleen @ 2010-04-01 0:57 UTC (permalink / raw)
To: Glen Turner; +Cc: Andi Kleen, David Miller, netdev
In-Reply-To: <1270079864.2389.48.camel@ilion>
> > Seems like a big hole not considered by the IPv6 designers?
>
> Yeah. The sockets API for IPv6 required an additional feature that
> the IETF did not foresee.
Linux (or in this concrete case ANK) did foresee it.
-Andi
--
ak@linux.intel.com -- Speaking for myself only.
^ permalink raw reply
* Re: [PATCH] r8169: offical fix for CVE-2009-4537 (overlength frame DMAs)
From: Neil Horman @ 2010-04-01 1:07 UTC (permalink / raw)
To: Brandon Philips
Cc: linux-kernel, netdev, michael.s.gilbert, davem, romieu,
eric.dumazet
In-Reply-To: <20100401002450.GA19117@jenkins.home.ifup.org>
On Wed, Mar 31, 2010 at 05:24:50PM -0700, Brandon Philips wrote:
> On 12:03 Mon 29 Mar 2010, Neil Horman wrote:
> > Signed-off-by: Neil Horman <nhorman@redhat.com>
>
> Cc: stable@kernel.org?
> Tested-by: Brandon Philips <bphilips@suse.de>
>
> > + if (max_frame != 16383)
> > + printk(KERN_WARNING "WARNING! Changing of MTU on this NIC"
> > + "May lead to frame reception errors!\n");
>
> This needs a space to look right. You might as well add the PFX too:
>
> printk(KERN_WARNING PFX "WARNING! Changing of MTU on this "
> "NIC may lead to frame reception errors!\n");
>
> Thanks Neil.
>
> Cheers,
>
> Brandon
>
Daves already merged my patch, but I'll submit a cleanup patch shortly to take
care of this.
Thanks!
Neil
^ permalink raw reply
* Re: [PATCH 5/6] cxgb4: Add main driver file and driver Makefile
From: Dimitris Michailidis @ 2010-04-01 1:34 UTC (permalink / raw)
To: David Miller; +Cc: shemminger, netdev
In-Reply-To: <20100331.172401.00333850.davem@davemloft.net>
David Miller wrote:
>> So, I propose getting rid of 3-4 of these files that are of lesser
>> value and moving the rest to debugfs for now. If some alternative
>> through ethtool or something becomes available I can get rid of
>> anything that can be handled through a more general facility. Would
>> that be acceptable?
>
> You can use sysfs.
sysfs is a possibility but I thought Stephen's initial concern was that I
was adding too many of these proc files and that they were creating a
potential API. sysfs will result in a lot more files with its
value-per-file model and I think sysfs and proc are similar in "APIness".
So it's not clear to me how going to sysfs would address Stephen's point.
The remove-a-few plus move-to-debugfs proposal was in order to end up with
fewer files in a non-API filesystem.
As these builtin switches become more common I expect an official way to
represent and access them will emerge but maybe it's not a good idea to
introduce a sysfs model for them as part of this driver submission.
^ permalink raw reply
* Re: [PATCH 5/6] cxgb4: Add main driver file and driver Makefile
From: David Miller @ 2010-04-01 2:18 UTC (permalink / raw)
To: dm; +Cc: shemminger, netdev
In-Reply-To: <4BB3F827.3010402@chelsio.com>
From: Dimitris Michailidis <dm@chelsio.com>
Date: Wed, 31 Mar 2010 18:34:31 -0700
> David Miller wrote:
>
>>> So, I propose getting rid of 3-4 of these files that are of lesser
>>> value and moving the rest to debugfs for now. If some alternative
>>> through ethtool or something becomes available I can get rid of
>>> anything that can be handled through a more general facility. Would
>>> that be acceptable?
>> You can use sysfs.
>
> sysfs is a possibility but I thought Stephen's initial concern was
> that I was adding too many of these proc files and that they were
> creating a potential API.
Yes, since procfs is essentially deprecated these days.
> sysfs will result in a lot more files with its value-per-file model
> and I think sysfs and proc are similar in "APIness". So it's not
> clear to me how going to sysfs would address Stephen's point. The
> remove-a-few plus move-to-debugfs proposal was in order to end up
> with fewer files in a non-API filesystem.
It's in fact easier to retain API by using sysfs. Instead of having
to worry about the format of a procfs file listing entries one by one
per line, under sysfs you just add a new file to export new values.
> As these builtin switches become more common I expect an official way
> to represent and access them will emerge but maybe it's not a good
> idea to introduce a sysfs model for them as part of this driver
> submission.
What nodes you create under your own device object in sysfs is your
domain and your business. Since it's one value per file there is no
real complexity in making sure tools can display the values properly.
^ permalink raw reply
* Re: [PATCH 3/4] xfrm: remove policy lock when accessing policy->walk.dead
From: jamal @ 2010-04-01 2:19 UTC (permalink / raw)
To: Herbert Xu; +Cc: David Miller, timo.teras, netdev
In-Reply-To: <20100401002244.GA18893@gondor.apana.org.au>
On Thu, 2010-04-01 at 08:22 +0800, Herbert Xu wrote:
> OK, in that case please change xfrm_state_flush too. Right now
> it also notifies on empty.
It shouldnt. I will double check it tomorrow..
cheers,
jamal
^ permalink raw reply
* Re: pull request: wireless-2.6 2010-03-31
From: David Miller @ 2010-04-01 2:33 UTC (permalink / raw)
To: linville-2XuSBdqkA4R54TAoqtyWWQ
Cc: davem-fT/PcQaiUtJ0xCFg78z1Mw,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20100331144649.GC3024-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
From: "John W. Linville" <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
Date: Wed, 31 Mar 2010 10:46:49 -0400
> Here is a batch of fixes intended for 2.6.34. Included are a few
> device IDs, along with several almost-one-liners to fix a variety of
> issues, including a NULL deref, a potential overflow, a misues of the
> USB API, a regulatory error for iwlwifi, a race condition in mac80211,
> and some other more minor fixes.
>
> I saw your note about only "eats someones disk" bugs. I'm not
> sure all of these meet that test, but I hope you will take them.
> I've been sitting on them a while and letting them cook in linux-next,
> mostly to test one particular patch (which I backed-out yesterday).
> I promise I'll tighten-up after this batch! :-)
Ok, pulled, thanks John.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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: [RFC] SPD basic actions per netdev
From: jamal @ 2010-04-01 2:35 UTC (permalink / raw)
To: Herbert Xu; +Cc: Timo Teras, David S. Miller, Patrick McHardy, netdev
In-Reply-To: <20100401003352.GA19147@gondor.apana.org.au>
On Thu, 2010-04-01 at 08:33 +0800, Herbert Xu wrote:
> If we're going to change this then we should just add a second
> interface field to the selector, rather than trying to overload
> the existing one.
Do you mean to have a selector->iif/oif? Sure that makes sense - but is
a much larger surgery and user space will need to be taught.
Did you look at the patch i sent? i tried to retain current behavior
except for the input check path. output path was working in classifying
with specific netdevs.
cheers,
jamal
^ permalink raw reply
* Re: [Patch] bonding: fix potential deadlock in bond_uninit()
From: Cong Wang @ 2010-04-01 2:49 UTC (permalink / raw)
To: Eric W. Biederman
Cc: linux-kernel, Jiri Pirko, Stephen Hemminger, netdev,
David S. Miller, bonding-devel, Jay Vosburgh
In-Reply-To: <m1d3ykzq5a.fsf@fess.ebiederm.org>
Eric W. Biederman wrote:
> Amerigo Wang <amwang@redhat.com> writes:
>
>> bond_uninit() is invoked with rtnl_lock held, when it does destroy_workqueue()
>> which will potentially flush all works in this workqueue, if we hold rtnl_lock
>> again in the work function, it will deadlock.
>>
>> So unlock rtnl_lock before calling destroy_workqueue().
>
> Ouch. That seems rather rude to our caller, and likely very
> dangerous.
This is reasonable, because workqueue flush functions will potentially
call all the work functions which could take the same lock taken before
the flush call, thus deadlock.
>
> Is this a deadlock you actually hit, or is this something lockdep
> warned about?
It's only a lockdep warning.
>
> My gut feel says we need to move the destroy_workqueue into
> the network device destructor.
>
Oh, this seems a better idea, as long as the destructor are not called
with any locks holding.
Thanks!
^ permalink raw reply
* Re: [RFC] SPD basic actions per netdev
From: Herbert Xu @ 2010-04-01 2:52 UTC (permalink / raw)
To: jamal; +Cc: Timo Teras, David S. Miller, Patrick McHardy, netdev
In-Reply-To: <1270089323.26743.138.camel@bigi>
On Wed, Mar 31, 2010 at 10:35:23PM -0400, jamal wrote:
> On Thu, 2010-04-01 at 08:33 +0800, Herbert Xu wrote:
>
> > If we're going to change this then we should just add a second
> > interface field to the selector, rather than trying to overload
> > the existing one.
>
> Do you mean to have a selector->iif/oif? Sure that makes sense - but is
> a much larger surgery and user space will need to be taught.
>
> Did you look at the patch i sent? i tried to retain current behavior
> except for the input check path. output path was working in classifying
> with specific netdevs.
OK, I guess the chances of an existing app breaking is slim.
BTW, you should treat FLOW_DIR_FWD as FLOW_DIR_IN.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: Undefined behaviour of connect(fd, NULL, 0);
From: Changli Gao @ 2010-04-01 3:00 UTC (permalink / raw)
To: Neil Brown; +Cc: David Miller, shemminger, netdev
In-Reply-To: <20100401090756.69bfb57d@notabene.brown>
I think the following patch is what Neil wants. The old code implies that
connect(fd, NULL, 0) is used to check the socket connecting status, but
Stephen's patch breaks it. The old code is wrong when it checks the address's
faimly but not check the sizeof of the address to determine the family member
is valid or not before.
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index be1a6ac..3ff51f0 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -576,7 +576,8 @@ int inet_stream_connect(struct socket *sock,
struct sockaddr *uaddr,
lock_sock(sk);
- if (uaddr->sa_family == AF_UNSPEC) {
+ if (addr_len >= sizeof(uaddr->sa_family) &&
+ uaddr->sa_family == AF_UNSPEC) {
err = sk->sk_prot->disconnect(sk, flags);
sock->state = err ? SS_DISCONNECTING : SS_UNCONNECTED;
goto out;
^ permalink raw reply related
* Re: Undefined behaviour of connect(fd, NULL, 0);
From: Neil Brown @ 2010-04-01 3:38 UTC (permalink / raw)
To: Changli Gao; +Cc: David Miller, shemminger, netdev
In-Reply-To: <n2w412e6f7f1003312000o3a69802j120acd3946458517@mail.gmail.com>
On Thu, 1 Apr 2010 11:00:23 +0800
Changli Gao <xiaosuo@gmail.com> wrote:
> I think the following patch is what Neil wants. The old code implies that
> connect(fd, NULL, 0) is used to check the socket connecting status, but
> Stephen's patch breaks it. The old code is wrong when it checks the address's
> faimly but not check the sizeof of the address to determine the family member
> is valid or not before.
>
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index be1a6ac..3ff51f0 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -576,7 +576,8 @@ int inet_stream_connect(struct socket *sock,
> struct sockaddr *uaddr,
>
> lock_sock(sk);
>
> - if (uaddr->sa_family == AF_UNSPEC) {
> + if (addr_len >= sizeof(uaddr->sa_family) &&
> + uaddr->sa_family == AF_UNSPEC) {
> err = sk->sk_prot->disconnect(sk, flags);
> sock->state = err ? SS_DISCONNECTING : SS_UNCONNECTED;
> goto out;
I'm not sure I'd say that I "want" any particular patch.
I just want to know what "connect(fd, NULL, 0)" is supposed to do, and to
have the kernel be consistent in its behaviour. I'm not really fussed what
the behaviour is.
I suspect the customer wants that patch you have supplied as it would mean
they don't need to change their code. But I only want it if it is "right".
The patch you have provided does what I had assumed Stephen's patch did
before I actually read it properly.
My feeling is that this patch might be more useful than Stephen's as having
connect(fd, NULL, 0) do what the customer expects seems useful, where as
having it do the same as setting AF_UNSPEC doesn't add anything.
I've googled around a bit but cannot find any evidence of anyone passing NULL
to connect like this, and what documentation I can find doesn't really
address the issue at all.
Thanks,
NeilBrown
^ permalink raw reply
* Re: Undefined behaviour of connect(fd, NULL, 0);
From: Changli Gao @ 2010-04-01 4:16 UTC (permalink / raw)
To: Neil Brown; +Cc: David Miller, shemminger, netdev
In-Reply-To: <20100401143805.1f83a161@notabene.brown>
On Thu, Apr 1, 2010 at 11:38 AM, Neil Brown <neilb@suse.de> wrote:
> I'm not sure I'd say that I "want" any particular patch.
> I just want to know what "connect(fd, NULL, 0)" is supposed to do, and to
> have the kernel be consistent in its behaviour. I'm not really fussed what
> the behaviour is.
>
> I suspect the customer wants that patch you have supplied as it would mean
> they don't need to change their code. But I only want it if it is "right".
>
> The patch you have provided does what I had assumed Stephen's patch did
> before I actually read it properly.
>
> My feeling is that this patch might be more useful than Stephen's as having
> connect(fd, NULL, 0) do what the customer expects seems useful, where as
> having it do the same as setting AF_UNSPEC doesn't add anything.
>
> I've googled around a bit but cannot find any evidence of anyone passing NULL
> to connect like this, and what documentation I can find doesn't really
> address the issue at all.
>
I found this from man page for connect(2)
Generally, connection-based protocol sockets may successfully connect()
only once; connectionless protocol sockets may use connect() multiple
times to change their association. Connectionless sockets may dissolve
the association by connecting to an address with the sa_family member
of sockaddr set to AF_UNSPEC (supported on Linux since kernel 2.2).
It only said the meaning of AF_UNSPEC for connectionless sockets, but
not connection sockets like TCP. It means that the behavior of
disconnecting the socket when AF_UNSEPC family address is met is also
undocumented.
The connect() API is a little different. If you try to call connect()
in non-blocking mode, and the API can't connect instantly, it will
return the error code for 'Operation In Progress'. When you call
connect() again, later, it may tell you 'Operation Already In
Progress' to let you know that it's still trying to connect, or it may
give you a successful return code, telling you that the connect has
been made.
from: http://www.scottklement.com/rpg/socktut/nonblocking.html
Someone may use connect() to check if the connection is established or
not. But there is no spec about the addr and addr_len value when
connect(2) is used this way. Since there is no limit of addr and
addr_len, and we supports addr is NULL to check the status of socket
(Although it is buggy). I think we should treat it like a feature, and
the problem Neil reported is a bug.
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH 1/2] phylib: Support phy module autoloading
From: Ben Hutchings @ 2010-04-01 4:34 UTC (permalink / raw)
To: David Woodhouse; +Cc: davem, netdev, 553024
In-Reply-To: <1269998334.18090.278.camel@macbook.infradead.org>
[-- Attachment #1: Type: text/plain, Size: 5111 bytes --]
On Wed, 2010-03-31 at 02:18 +0100, David Woodhouse wrote:
> We don't use the normal hotplug mechanism because it doesn't work. It will
> load the module some time after the device appears, but that's not good
> enough for us -- we need the driver loaded _immediately_ because otherwise
> the NIC driver may just abort and then the phy 'device' goes away.
>
> Instead, we just issue a request_module() directly in phy_device_create().
[...]
Thanks for doing this, David. I had a stab at it earlier when this
problem was reported in Debian <http://bugs.debian.org/553024>. I
didn't complete this because (a) I didn't understand all the details of
adding new device table type, and (b) I tried to avoid duplicating
information, which turns out to be rather difficult in modules with
multiple drivers.
Since you've dealt with (a), and (b) is not really as important, I would
just like to suggest some minor changes to your patch 1 (see below).
Feel free to fold them in. Your patch 2 would then need the
substitutions s/phy_device_id/mdio_device_id/; s/TABLE(phy/TABLE(mdio/.
Ben.
From: Ben Hutchings <ben@decadent.org.uk>
Date: Thu, 1 Apr 2010 05:03:02 +0100
Subject: [PATCH] phylib: Minor cleanup of phylib autoloading
Refer to MDIO, consistent with other module aliases using bus names.
Change type names to __u32, consistent with the rest of the file.
Add kernel-doc comment to struct mdio_device_id.
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
drivers/net/phy/phy_device.c | 2 +-
include/linux/mod_devicetable.h | 22 ++++++++++++++--------
scripts/mod/file2alias.c | 8 ++++----
3 files changed, 19 insertions(+), 13 deletions(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index b35ec7e..b0e54b4 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -188,7 +188,7 @@ struct phy_device* phy_device_create(struct mii_bus *bus, int addr, int phy_id)
around for long enough for the driver to get loaded. With
MDIO, the NIC driver will get bored and give up as soon
as it finds that there's no driver _already_ loaded. */
- sprintf(modid, "phy:" PHYID_FMT, PHYID_ARGS(phy_id));
+ sprintf(modid, MDIO_MODULE_PREFIX MDIO_ID_FMT, MDIO_ID_ARGS(phy_id));
request_module(modid);
#endif
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 0c3e300..55f1f9c 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -474,10 +474,10 @@ struct platform_device_id {
__attribute__((aligned(sizeof(kernel_ulong_t))));
};
-#define PHY_MODULE_PREFIX "phy:"
+#define MDIO_MODULE_PREFIX "mdio:"
-#define PHYID_FMT "%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d"
-#define PHYID_ARGS(_id) \
+#define MDIO_ID_FMT "%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d"
+#define MDIO_ID_ARGS(_id) \
(_id)>>31, ((_id)>>30) & 1, ((_id)>>29) & 1, ((_id)>>28) & 1, \
((_id)>>27) & 1, ((_id)>>26) & 1, ((_id)>>25) & 1, ((_id)>>24) & 1, \
((_id)>>23) & 1, ((_id)>>22) & 1, ((_id)>>21) & 1, ((_id)>>20) & 1, \
@@ -487,11 +487,17 @@ struct platform_device_id {
((_id)>>7) & 1, ((_id)>>6) & 1, ((_id)>>5) & 1, ((_id)>>4) & 1, \
((_id)>>3) & 1, ((_id)>>2) & 1, ((_id)>>1) & 1, (_id) & 1
-
-
-struct phy_device_id {
- uint32_t phy_id;
- uint32_t phy_id_mask;
+/**
+ * struct mdio_device_id - identifies PHY devices on an MDIO/MII bus
+ * @phy_id: The result of
+ * (mdio_read(&MII_PHYSID1) << 16 | mdio_read(&PHYSID2)) & @phy_id_mask
+ * for this PHY type
+ * @phy_id_mask: Defines the significant bits of @phy_id. A value of 0
+ * is used to terminate an array of struct mdio_device_id.
+ */
+struct mdio_device_id {
+ __u32 phy_id;
+ __u32 phy_id_mask;
};
#endif /* LINUX_MOD_DEVICETABLE_H */
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index b412185..0e08b8b 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -797,7 +797,7 @@ static int do_platform_entry(const char *filename,
}
static int do_phy_entry(const char *filename,
- struct phy_device_id *id, char *alias)
+ struct mdio_device_id *id, char *alias)
{
char str[33];
int i;
@@ -813,7 +813,7 @@ static int do_phy_entry(const char *filename,
str[i] = '0';
}
- sprintf(alias, PHY_MODULE_PREFIX "%s", str);
+ sprintf(alias, MDIO_MODULE_PREFIX "%s", str);
return 1;
}
@@ -964,9 +964,9 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
do_table(symval, sym->st_size,
sizeof(struct platform_device_id), "platform",
do_platform_entry, mod);
- else if (sym_is(symname, "__mod_phy_device_table"))
+ else if (sym_is(symname, "__mod_mdio_device_table"))
do_table(symval, sym->st_size,
- sizeof(struct phy_device_id), "phy",
+ sizeof(struct mdio_device_id), "phy",
do_phy_entry, mod);
free(zeros);
}
--
1.7.0.3
--
Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply related
* Re: [RFC] SPD basic actions per netdev
From: Timo Teräs @ 2010-04-01 4:52 UTC (permalink / raw)
To: Herbert Xu; +Cc: jamal, David S. Miller, Patrick McHardy, netdev
In-Reply-To: <20100401025247.GA19994@gondor.apana.org.au>
Herbert Xu wrote:
> On Wed, Mar 31, 2010 at 10:35:23PM -0400, jamal wrote:
>> On Thu, 2010-04-01 at 08:33 +0800, Herbert Xu wrote:
>>
>>> If we're going to change this then we should just add a second
>>> interface field to the selector, rather than trying to overload
>>> the existing one.
>> Do you mean to have a selector->iif/oif? Sure that makes sense - but is
>> a much larger surgery and user space will need to be taught.
>>
>> Did you look at the patch i sent? i tried to retain current behavior
>> except for the input check path. output path was working in classifying
>> with specific netdevs.
>
> OK, I guess the chances of an existing app breaking is slim.
>
> BTW, you should treat FLOW_DIR_FWD as FLOW_DIR_IN.
I think we need iif and oif. The separation is clear in in/fwd policies,
as each is matched properly. But 'out' policies are used for both:
locally generated, and forwarded traffic.
Basically it goes like:
in - for policy_check for traffic that is received locally
fwd - for policy_check for traffic that is forwarded
out - all (local and forwarded) traffic that goes out of box
IMHO, it's slightly confusing that in/fwd is split, but out is not.
But that's the way it works. If you now override the how interface
is checked for 'out' policy, it'll break current behaviour.
- Timo
^ permalink raw reply
* linux-next: build failure after merge of the slabh tree
From: Stephen Rothwell @ 2010-04-01 5:41 UTC (permalink / raw)
To: Tejun Heo
Cc: linux-next, linux-kernel, Sjur Braendeland, David Miller, netdev
Hi Tejun,
After merging the slabh tree, today's linux-next build (x86_64
allmodconfig) failed like this:
net/caif/cfcnfg.c: In function 'cfcnfg_create':
net/caif/cfcnfg.c:68: error: implicit declaration of function 'kmalloc'
net/caif/cfcnfg.c:68: warning: assignment makes pointer from integer without a cast
net/caif/cfcnfg.c:100: error: implicit declaration of function 'kfree'
Caused by commit 15c9ac0c80e390df09ce5730a7b08b13e07a8dd5 ("net-caif: add
CAIF generic caif support functions") from the net interacting with
commit de380b55f92986c1a84198149cb71b7228d15fbd ("percpu: don't
implicitly include slab.h from percpu.h") from the slabh tree.
I have applied the following patch for today. Dave, could you apply this
to the net tree please?
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Thu, 1 Apr 2010 16:31:50 +1100
Subject: [PATCH] net-caif: using kmalloc/kfree requires the include of slab.h
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
net/caif/cfcnfg.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/net/caif/cfcnfg.c b/net/caif/cfcnfg.c
index 70a733d..c873e3d 100644
--- a/net/caif/cfcnfg.c
+++ b/net/caif/cfcnfg.c
@@ -5,6 +5,7 @@
*/
#include <linux/kernel.h>
#include <linux/stddef.h>
+#include <linux/slab.h>
#include <net/caif/caif_layer.h>
#include <net/caif/cfpkt.h>
#include <net/caif/cfcnfg.h>
--
1.7.0.3
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
^ permalink raw reply related
* Re: Undefined behaviour of connect(fd, NULL, 0);
From: Changli Gao @ 2010-04-01 5:50 UTC (permalink / raw)
To: Neil Brown; +Cc: David Miller, shemminger, netdev
In-Reply-To: <x2j412e6f7f1003312116rd3b3ba96t31267545efe7660f@mail.gmail.com>
On Thu, Apr 1, 2010 at 12:16 PM, Changli Gao <xiaosuo@gmail.com> wrote:
>
> I found this from man page for connect(2)
>
> Generally, connection-based protocol sockets may successfully connect()
> only once; connectionless protocol sockets may use connect() multiple
> times to change their association. Connectionless sockets may dissolve
> the association by connecting to an address with the sa_family member
> of sockaddr set to AF_UNSPEC (supported on Linux since kernel 2.2).
>
dissolving the association by connecting to an address with the
sa_family member of sockaddr set to AF_UNSEPC is broken too.
int ip4_datagram_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
{
struct inet_sock *inet = inet_sk(sk);
struct sockaddr_in *usin = (struct sockaddr_in *) uaddr;
struct rtable *rt;
__be32 saddr;
int oif;
int err;
if (addr_len < sizeof(*usin))
return -EINVAL;
if (usin->sin_family != AF_INET)
return -EAFNOSUPPORT;
according to the man page, sin_family == AF_UNSPEC should be allowed.
And netlink's connect doesn't check the addr_len, so it behavior is
also undeterminedl
static int netlink_connect(struct socket *sock, struct sockaddr *addr,
int alen, int flags)
{
int err = 0;
struct sock *sk = sock->sk;
struct netlink_sock *nlk = nlk_sk(sk);
struct sockaddr_nl *nladdr = (struct sockaddr_nl *)addr;
if (addr->sa_family == AF_UNSPEC) {
sk->sk_state = NETLINK_UNCONNECTED;
nlk->dst_pid = 0;
nlk->dst_group = 0;
return 0;
}
If this issues need to be fixed, I'll check all the protocols if their
connect() checkes the sizeof of socket address or not, and post a
patch.
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [RFC] SPD basic actions per netdev
From: Herbert Xu @ 2010-04-01 6:01 UTC (permalink / raw)
To: Timo Teräs; +Cc: jamal, David S. Miller, Patrick McHardy, netdev
In-Reply-To: <4BB42692.9010105@iki.fi>
On Thu, Apr 01, 2010 at 07:52:34AM +0300, Timo Teräs wrote:
>
> IMHO, it's slightly confusing that in/fwd is split, but out is not.
> But that's the way it works. If you now override the how interface
> is checked for 'out' policy, it'll break current behaviour.
Unless I've misunderstood what his patch is trying to do, it would
seem that out policies would be completely unchanged.
Forward policies are not used on output.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* [Patch V2] bonding: fix potential deadlock in bond_uninit()
From: Amerigo Wang @ 2010-04-01 6:06 UTC (permalink / raw)
To: linux-kernel
Cc: Jiri Pirko, Stephen Hemminger, netdev, David S. Miller,
Eric W. Biederman, Amerigo Wang, bonding-devel, Jay Vosburgh
bond_uninit() is invoked with rtnl_lock held, when it does destroy_workqueue()
which will potentially flush all works in this workqueue, if we hold rtnl_lock
again in the work function, it will deadlock.
So move destroy_workqueue() to destructor where rtnl_lock is not held any more,
suggested by Eric.
Signed-off-by: WANG Cong <amwang@redhat.com>
Cc: Jay Vosburgh <fubar@us.ibm.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Stephen Hemminger <shemminger@vyatta.com>
Cc: Jiri Pirko <jpirko@redhat.com>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
---
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 5b92fbf..9f0aaa2 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4450,6 +4450,14 @@ static const struct net_device_ops bond_netdev_ops = {
.ndo_vlan_rx_kill_vid = bond_vlan_rx_kill_vid,
};
+static void bond_destructor(struct net_device *bond_dev)
+{
+ struct bonding *bond = netdev_priv(bond_dev);
+ if (bond->wq)
+ destroy_workqueue(bond->wq);
+ free_netdev(bond_dev);
+}
+
static void bond_setup(struct net_device *bond_dev)
{
struct bonding *bond = netdev_priv(bond_dev);
@@ -4470,7 +4478,7 @@ static void bond_setup(struct net_device *bond_dev)
bond_dev->ethtool_ops = &bond_ethtool_ops;
bond_set_mode_ops(bond, bond->params.mode);
- bond_dev->destructor = free_netdev;
+ bond_dev->destructor = bond_destructor;
/* Initialize the device options */
bond_dev->tx_queue_len = 0;
@@ -4542,9 +4550,6 @@ static void bond_uninit(struct net_device *bond_dev)
bond_remove_proc_entry(bond);
- if (bond->wq)
- destroy_workqueue(bond->wq);
-
netif_addr_lock_bh(bond_dev);
bond_mc_list_destroy(bond);
netif_addr_unlock_bh(bond_dev);
^ permalink raw reply related
* Re: [Patch V2] bonding: fix potential deadlock in bond_uninit()
From: Eric W. Biederman @ 2010-04-01 6:12 UTC (permalink / raw)
To: Amerigo Wang
Cc: linux-kernel, Jiri Pirko, Stephen Hemminger, netdev,
David S. Miller, bonding-devel, Jay Vosburgh
In-Reply-To: <20100401061014.4815.7341.sendpatchset@localhost.localdomain>
Amerigo Wang <amwang@redhat.com> writes:
> bond_uninit() is invoked with rtnl_lock held, when it does destroy_workqueue()
> which will potentially flush all works in this workqueue, if we hold rtnl_lock
> again in the work function, it will deadlock.
>
> So move destroy_workqueue() to destructor where rtnl_lock is not held any more,
> suggested by Eric.
The error handling on creating a bond device needs to be updated as well.
Eric
> Signed-off-by: WANG Cong <amwang@redhat.com>
> Cc: Jay Vosburgh <fubar@us.ibm.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Stephen Hemminger <shemminger@vyatta.com>
> Cc: Jiri Pirko <jpirko@redhat.com>
> Cc: "Eric W. Biederman" <ebiederm@xmission.com>
>
> ---
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 5b92fbf..9f0aaa2 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -4450,6 +4450,14 @@ static const struct net_device_ops bond_netdev_ops = {
> .ndo_vlan_rx_kill_vid = bond_vlan_rx_kill_vid,
> };
>
> +static void bond_destructor(struct net_device *bond_dev)
> +{
> + struct bonding *bond = netdev_priv(bond_dev);
> + if (bond->wq)
> + destroy_workqueue(bond->wq);
> + free_netdev(bond_dev);
> +}
> +
> static void bond_setup(struct net_device *bond_dev)
> {
> struct bonding *bond = netdev_priv(bond_dev);
> @@ -4470,7 +4478,7 @@ static void bond_setup(struct net_device *bond_dev)
> bond_dev->ethtool_ops = &bond_ethtool_ops;
> bond_set_mode_ops(bond, bond->params.mode);
>
> - bond_dev->destructor = free_netdev;
> + bond_dev->destructor = bond_destructor;
>
> /* Initialize the device options */
> bond_dev->tx_queue_len = 0;
> @@ -4542,9 +4550,6 @@ static void bond_uninit(struct net_device *bond_dev)
>
> bond_remove_proc_entry(bond);
>
> - if (bond->wq)
> - destroy_workqueue(bond->wq);
> -
> netif_addr_lock_bh(bond_dev);
> bond_mc_list_destroy(bond);
> netif_addr_unlock_bh(bond_dev);
^ permalink raw reply
* Re: [RFC] SPD basic actions per netdev
From: Timo Teräs @ 2010-04-01 6:20 UTC (permalink / raw)
To: Herbert Xu; +Cc: jamal, David S. Miller, Patrick McHardy, netdev
In-Reply-To: <20100401060145.GB20865@gondor.apana.org.au>
Herbert Xu wrote:
> On Thu, Apr 01, 2010 at 07:52:34AM +0300, Timo Teräs wrote:
>> IMHO, it's slightly confusing that in/fwd is split, but out is not.
>> But that's the way it works. If you now override the how interface
>> is checked for 'out' policy, it'll break current behaviour.
>
> Unless I've misunderstood what his patch is trying to do, it would
> seem that out policies would be completely unchanged.
>
> Forward policies are not used on output.
Oh, that right.
But my statement still holds. If iif/oif is swapped, it's changing
current semantics and can end up breaking setups. Both are still
valid for 'in' and 'fwd' policies too, right? What if I'm using
'in' policy to make sure that all stuff arriving via 'eth0' is
encrypted, but 'eth1' is trusted and does not need xfrm. This
would break.
I do like the idea very much. In fact I remember asking this exact
feature long time ago. But I think it should be done by explicitly
allowing user to specify both iif and oif; even if it's more
intrusive.
^ permalink raw reply
* Re: [RFC] SPD basic actions per netdev
From: Herbert Xu @ 2010-04-01 6:28 UTC (permalink / raw)
To: Timo Teräs; +Cc: jamal, David S. Miller, Patrick McHardy, netdev
In-Reply-To: <4BB43B38.1060004@iki.fi>
On Thu, Apr 01, 2010 at 09:20:40AM +0300, Timo Teräs wrote:
>
> But my statement still holds. If iif/oif is swapped, it's changing
> current semantics and can end up breaking setups. Both are still
> valid for 'in' and 'fwd' policies too, right? What if I'm using
> 'in' policy to make sure that all stuff arriving via 'eth0' is
> encrypted, but 'eth1' is trusted and does not need xfrm. This
> would break.
The thing is if you're currently specifying an ifindex in the
selector for inbound/forward, it probably just won't work as
it'll be matched against oif which is meaningless on inbound
and forward.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [RFC] SPD basic actions per netdev
From: Timo Teräs @ 2010-04-01 6:32 UTC (permalink / raw)
To: Herbert Xu; +Cc: jamal, David S. Miller, Patrick McHardy, netdev
In-Reply-To: <20100401062840.GA21284@gondor.apana.org.au>
Herbert Xu wrote:
> On Thu, Apr 01, 2010 at 09:20:40AM +0300, Timo Teräs wrote:
>> But my statement still holds. If iif/oif is swapped, it's changing
>> current semantics and can end up breaking setups. Both are still
>> valid for 'in' and 'fwd' policies too, right? What if I'm using
>> 'in' policy to make sure that all stuff arriving via 'eth0' is
>> encrypted, but 'eth1' is trusted and does not need xfrm. This
>> would break.
>
> The thing is if you're currently specifying an ifindex in the
> selector for inbound/forward, it probably just won't work as
> it'll be matched against oif which is meaningless on inbound
> and forward.
On inbound it's always loopback interface. Does the same hold
true on forward? I was under the impression that it would
reflect the actual destination interface.
^ permalink raw reply
* Re: [RFC] SPD basic actions per netdev
From: Herbert Xu @ 2010-04-01 6:39 UTC (permalink / raw)
To: Timo Teräs; +Cc: jamal, David S. Miller, Patrick McHardy, netdev
In-Reply-To: <4BB43DE6.9060300@iki.fi>
On Thu, Apr 01, 2010 at 09:32:06AM +0300, Timo Teräs wrote:
>
> On inbound it's always loopback interface. Does the same hold
> true on forward? I was under the impression that it would
> reflect the actual destination interface.
That's a good point. I suppose there might just be some crazy
people out there using it this way.
So Jamal, I think this is a good reason why we do need a new
field instead of just overloading the current one. Otherwise
inbound selectors and forward selectors will have different
semantics which is needlessly confusing.
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [Patch V2] bonding: fix potential deadlock in bond_uninit()
From: Cong Wang @ 2010-04-01 6:54 UTC (permalink / raw)
To: Eric W. Biederman
Cc: linux-kernel, Jiri Pirko, Stephen Hemminger, netdev,
David S. Miller, bonding-devel, Jay Vosburgh
In-Reply-To: <m1r5mzsnuh.fsf@fess.ebiederm.org>
Eric W. Biederman wrote:
> Amerigo Wang <amwang@redhat.com> writes:
>
>> bond_uninit() is invoked with rtnl_lock held, when it does destroy_workqueue()
>> which will potentially flush all works in this workqueue, if we hold rtnl_lock
>> again in the work function, it will deadlock.
>>
>> So move destroy_workqueue() to destructor where rtnl_lock is not held any more,
>> suggested by Eric.
>
> The error handling on creating a bond device needs to be updated as well.
>
You're right, I missed that part. Will update it soon.
Thanks.
^ permalink raw reply
* Re: [PATCH v3 04/12] l2tp: Add ppp device name to L2TP ppp session data
From: James Chapman @ 2010-04-01 7:19 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20100331160811.16351eb7@s6510>
Stephen Hemminger wrote:
> On Wed, 31 Mar 2010 08:43:09 +0100
> James Chapman <jchapman@katalix.com> wrote:
>
>> Stephen Hemminger wrote:
>>> On Tue, 30 Mar 2010 17:17:46 +0100
>>> James Chapman <jchapman@katalix.com> wrote:
>>>
>>>> When dumping L2TP PPP sessions using /proc/net/l2tp, get
>>>> the assigned PPP device name from PPP using ppp_dev_name().
>>>>
>>>> Signed-off-by: James Chapman <jchapman@katalix.com>
>>>> Reviewed-by: Randy Dunlap <randy.dunlap@oracle.com>
>>>>
>>> Why is this a necessary API?
>>> Why not put it in debugfs if just a debugging tool?
>> With the original driver (merged in 2.6.23), some people use horrible
>> hacks in scripts to derive info about their L2TP connections from /proc.
>> So I was reluctant to move it to debugfs in the new driver. If it is ok
>> to move an existing /proc file to debugfs, I'm happy to do so. People
>> should obtain such info from their L2TP userspace daemon, or through
>> netlink anyway.
>>
>>
> Sounds like a good use of sysfs either with attribute or symlink
> back to underlying device
>
There might be thousands of L2TP sessions in some setups. Populating
sysfs with a link for each of those sessions isn't practical. The
existing /proc file dumps its info as a single text file for this
reason. I'd also like to provide the device name in the session netlink
message, which is the interface used by l2tp userspace, so I need a
kernel API to retrieve the device name from ppp.
I like the suggestion of using debugfs for access to driver debug info
though. I propose leaving the /proc file for L2TPv2 only, removing the
L2TPv3 data that I added to the proc file in this patch series, to
retain compatibility with the existing driver. This would show only
L2TPv2 sessions and tunnels. For new driver functionality (L2TPv3 etc),
use debugfs. The debugfs files would dump lists in a similar form to the
current code, listing all tunnels (L2TPv2 and L2TPv3) in a single file.
Using debugfs gives more flexibility for adding additional info later,
as required. How does that sound?
--
James Chapman
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development
^ 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