* [PATCH 7/6] bond: Get the rtnl_link_ops support correct
From: Eric W. Biederman @ 2009-10-30 9:58 UTC (permalink / raw)
To: Patrick McHardy; +Cc: David Miller, netdev, Jay Vosburgh, Eric W. Biederman
In-Reply-To: <4AEAB2F1.5070200@trash.net>
- Don't call rtnl_link_unregister if rtnl_link_register fails
- Set .priv_size so we aren't stomping on uninitialized memory
when we use netdev_priv, on bond devices created with
ip link add type bond.
Signed-off-by: Eric W. Biederman <ebiederm@aristanetworks.com>
---
drivers/net/bonding/bond_main.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index fed44e6..0ef051c 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -5004,6 +5004,7 @@ static int bond_validate(struct nlattr *tb[], struct nlattr *data[])
static struct rtnl_link_ops bond_link_ops __read_mostly = {
.kind = "bond",
+ .priv_size = sizeof(struct bonding),
.setup = bond_setup,
.validate = bond_validate,
};
@@ -5105,7 +5106,7 @@ static int __init bonding_init(void)
res = rtnl_link_register(&bond_link_ops);
if (res)
- goto err;
+ goto err_link;
for (i = 0; i < max_bonds; i++) {
res = bond_create(&init_net, NULL);
@@ -5124,6 +5125,7 @@ out:
return res;
err:
rtnl_link_unregister(&bond_link_ops);
+err_link:
unregister_pernet_gen_subsys(bond_net_id, &bond_net_ops);
goto out;
--
1.6.3.1.54.g99dd.dirty
^ permalink raw reply related
* Re: WARNING: at net/ipv4/af_inet.c:154 inet_sock_destruct
From: Eric Dumazet @ 2009-10-30 9:41 UTC (permalink / raw)
To: Francis Moreau
Cc: Linux Kernel Mailing List, Linux Netdev List, David S. Miller
In-Reply-To: <38b2ab8a0910300144i7a3c190fi9aa3d079c9cdb754@mail.gmail.com>
Francis Moreau a écrit :
> Hello Eric,
>
> It seems I still have a related bug, please have a look to the following oops.
>
> This happened on a 2.6.32-rc5 where your patch is included.
>
> [107304.558821] nfsd: last server has exited, flushing export cache
> [107304.558848] ------------[ cut here ]------------
> [107304.558858] WARNING: at net/ipv4/af_inet.c:153
> inet_sock_destruct+0x161/0x17c()
> [107304.558862] Hardware name: P5K-VM
> [107304.558865] Modules linked in: kvm_intel kvm jfs loop nfsd lockd
> nfs_acl auth_rpcgss exportfs sunrpc [last unloaded: microcode]
> [107304.558889] Pid: 8198, comm: nfsd Tainted: G M 2.6.32-rc5 #25
> [107304.558892] Call Trace:
> [107304.558899] [<ffffffff81429f19>] ? inet_sock_destruct+0x161/0x17c
> [107304.558907] [<ffffffff810487e9>] warn_slowpath_common+0x7c/0xa9
> [107304.558914] [<ffffffff8104882a>] warn_slowpath_null+0x14/0x16
> [107304.558920] [<ffffffff81429f19>] inet_sock_destruct+0x161/0x17c
> [107304.558927] [<ffffffff813c8741>] __sk_free+0x23/0xe7
> [107304.558933] [<ffffffff813c8881>] sk_free+0x1f/0x21
> [107304.558939] [<ffffffff813c894b>] sk_common_release+0xc8/0xcd
> [107304.558944] [<ffffffff81420b59>] udp_lib_close+0xe/0x10
> [107304.558951] [<ffffffff814299bf>] inet_release+0x55/0x5c
> [107304.558957] [<ffffffff813c5aa9>] sock_release+0x1f/0x71
> [107304.558962] [<ffffffff813c5b22>] sock_close+0x27/0x2b
> [107304.558968] [<ffffffff810eb60f>] __fput+0xfb/0x1c0
> [107304.558973] [<ffffffff810eb6f1>] fput+0x1d/0x1f
> [107304.558995] [<ffffffffa0013e23>] svc_sock_free+0x40/0x56 [sunrpc]
> [107304.559018] [<ffffffffa001f392>] svc_xprt_free+0x43/0x53 [sunrpc]
> [107304.559038] [<ffffffffa001f34f>] ? svc_xprt_free+0x0/0x53 [sunrpc]
> [107304.559048] [<ffffffff811d9275>] kref_put+0x43/0x4f
> [107304.559069] [<ffffffffa001e67a>] svc_close_xprt+0x55/0x5e [sunrpc]
> [107304.559088] [<ffffffffa001e6d3>] svc_close_all+0x50/0x69 [sunrpc]
> [107304.559107] [<ffffffffa0012a2b>] svc_destroy+0x9e/0x142 [sunrpc]
> [107304.559126] [<ffffffffa0012b88>] svc_exit_thread+0xb9/0xc2 [sunrpc]
> [107304.559138] [<ffffffffa008981b>] ? nfsd+0x0/0x13f [nfsd]
> [107304.559149] [<ffffffffa0089940>] nfsd+0x125/0x13f [nfsd]
> [107304.559157] [<ffffffff810685e3>] kthread+0x82/0x8a
> [107304.559164] [<ffffffff8100c13a>] child_rip+0xa/0x20
> [107304.559172] [<ffffffff8100baad>] ? restore_args+0x0/0x30
> [107304.559179] [<ffffffff81068561>] ? kthread+0x0/0x8a
> [107304.559185] [<ffffffff8100c130>] ? child_rip+0x0/0x20
> [107304.559191] ---[ end trace c107131f4762168c ]---
> [107304.927931] NFSD: Using /var/lib/nfs/v4recovery as the NFSv4 state
> recovery directory
> [107304.932765] NFSD: starting 90-second grace period
>
Thanks Francis, I think I found the problem.
I am preparing a patch, test it, and submit it in couple of hours
^ permalink raw reply
* Re: [PATCH 5/6] bond: Implement a basic set of rtnl link ops
From: Patrick McHardy @ 2009-10-30 9:33 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: David Miller, netdev, Jay Vosburgh, Eric W. Biederman
In-Reply-To: <m1eiols0a4.fsf@fess.ebiederm.org>
Eric W. Biederman wrote:
> Patrick McHardy <kaber@trash.net> writes:
>
>> Eric W. Biederman wrote:
>>> +static struct rtnl_link_ops bond_link_ops __read_mostly = {
>>> + .kind = "bond",
>>> + .setup = bond_setup,
>>> + .validate = bond_validate,
>>> +};
>> One more thing - you need to initialize .priv_size here so
>> the devices created through rtnl_link have enough private
>> room allocated.
>
> Wow and the code works when I test it without that ouch!
>
> As for rtnl_link_register it always succeeds so let's just
> remove the return code and call it good.
You need unroll anyways for the other failure conditions, so
why not simply add an err1/2 and be safe for future changes?
^ permalink raw reply
* Re: [PATCH 5/6] bond: Implement a basic set of rtnl link ops
From: Eric W. Biederman @ 2009-10-30 9:23 UTC (permalink / raw)
To: Patrick McHardy; +Cc: David Miller, netdev, Jay Vosburgh, Eric W. Biederman
In-Reply-To: <4AEAA403.7080204@trash.net>
Patrick McHardy <kaber@trash.net> writes:
> Eric W. Biederman wrote:
>> +static struct rtnl_link_ops bond_link_ops __read_mostly = {
>> + .kind = "bond",
>> + .setup = bond_setup,
>> + .validate = bond_validate,
>> +};
>
> One more thing - you need to initialize .priv_size here so
> the devices created through rtnl_link have enough private
> room allocated.
Wow and the code works when I test it without that ouch!
As for rtnl_link_register it always succeeds so let's just
remove the return code and call it good.
Eric
^ permalink raw reply
* [RFC, PATCH] net: suspicious test in dev_change_name()
From: Eric Dumazet @ 2009-10-30 9:20 UTC (permalink / raw)
To: David S. Miller; +Cc: Linux Netdev List
While preparing a patch for net-next-2.6, I noticed following code in dev_change_name()
int err = 0;
...
ret = call_netdevice_notifiers(NETDEV_CHANGENAME, dev);
ret = notifier_to_errno(ret);
if (ret) {
<< HERE >> if (err) {
printk(KERN_ERR
"%s: name change rollback failed: %d.\n",
dev->name, ret);
} else {
err = ret;
memcpy(dev->name, oldname, IFNAMSIZ);
goto rollback;
}
}
It seems intent was to test if notifier_to_errno() was null ?
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/net/core/dev.c b/net/core/dev.c
index b8f74cf..029cd41 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -939,9 +939,9 @@ rollback:
write_unlock_bh(&dev_base_lock);
ret = call_netdevice_notifiers(NETDEV_CHANGENAME, dev);
- ret = notifier_to_errno(ret);
if (ret) {
+ err = notifier_to_errno(ret);
if (err) {
printk(KERN_ERR
"%s: name change rollback failed: %d.\n",
^ permalink raw reply related
* Re: [PATCH v3 4/7] fsl_pq_mdio: Add Suport for etsec2.0 devices.
From: David Miller @ 2009-10-30 8:44 UTC (permalink / raw)
To: B05799; +Cc: netdev, afleming
In-Reply-To: <9F4C7D19E8361D4C94921B95BE08B81B9503DF@zin33exm22.fsl.freescale.net>
From: "Kumar Gopalpet-B05799" <B05799@freescale.com>
Date: Fri, 30 Oct 2009 14:12:22 +0530
> We were referring to the logs shown on the gitweb.
> The tree log does not show this commit.
> (search for this commit id failed)
> http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Fdavem%2Fnet-next-2.6.git
> &a=search&h=29906f6a427d2004a515ebbcdc7b28bae8f6c19c&st=commit&s=e72701a
> cbe0b35e52d3f04d442837c06b4e64f1c
>
> Whereas, the log for the file does show the commit.
> http://git.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git;a=histo
> ry;f=drivers/net/fsl_pq_mdio.c;h=6ac4648669728421b934d32646ffbb626128387
> 6;hb=HEAD
>
> Why is there such a mismatch on the tree itself?
If you are using HTTP for accessing my GIT tree(s) that's your
problem.
I do not update the web based GIT information when I push into my
tree(s), only the GIT and SSH protocols are reliable ways to clone and
access my tree(s).
^ permalink raw reply
* Re: WARNING: at net/ipv4/af_inet.c:154 inet_sock_destruct
From: Francis Moreau @ 2009-10-30 8:44 UTC (permalink / raw)
To: Eric Dumazet
Cc: Linux Kernel Mailing List, Linux Netdev List, David S. Miller
In-Reply-To: <4AC1D0F5.4050709@gmail.com>
Hello Eric,
It seems I still have a related bug, please have a look to the following oops.
This happened on a 2.6.32-rc5 where your patch is included.
[107304.558821] nfsd: last server has exited, flushing export cache
[107304.558848] ------------[ cut here ]------------
[107304.558858] WARNING: at net/ipv4/af_inet.c:153
inet_sock_destruct+0x161/0x17c()
[107304.558862] Hardware name: P5K-VM
[107304.558865] Modules linked in: kvm_intel kvm jfs loop nfsd lockd
nfs_acl auth_rpcgss exportfs sunrpc [last unloaded: microcode]
[107304.558889] Pid: 8198, comm: nfsd Tainted: G M 2.6.32-rc5 #25
[107304.558892] Call Trace:
[107304.558899] [<ffffffff81429f19>] ? inet_sock_destruct+0x161/0x17c
[107304.558907] [<ffffffff810487e9>] warn_slowpath_common+0x7c/0xa9
[107304.558914] [<ffffffff8104882a>] warn_slowpath_null+0x14/0x16
[107304.558920] [<ffffffff81429f19>] inet_sock_destruct+0x161/0x17c
[107304.558927] [<ffffffff813c8741>] __sk_free+0x23/0xe7
[107304.558933] [<ffffffff813c8881>] sk_free+0x1f/0x21
[107304.558939] [<ffffffff813c894b>] sk_common_release+0xc8/0xcd
[107304.558944] [<ffffffff81420b59>] udp_lib_close+0xe/0x10
[107304.558951] [<ffffffff814299bf>] inet_release+0x55/0x5c
[107304.558957] [<ffffffff813c5aa9>] sock_release+0x1f/0x71
[107304.558962] [<ffffffff813c5b22>] sock_close+0x27/0x2b
[107304.558968] [<ffffffff810eb60f>] __fput+0xfb/0x1c0
[107304.558973] [<ffffffff810eb6f1>] fput+0x1d/0x1f
[107304.558995] [<ffffffffa0013e23>] svc_sock_free+0x40/0x56 [sunrpc]
[107304.559018] [<ffffffffa001f392>] svc_xprt_free+0x43/0x53 [sunrpc]
[107304.559038] [<ffffffffa001f34f>] ? svc_xprt_free+0x0/0x53 [sunrpc]
[107304.559048] [<ffffffff811d9275>] kref_put+0x43/0x4f
[107304.559069] [<ffffffffa001e67a>] svc_close_xprt+0x55/0x5e [sunrpc]
[107304.559088] [<ffffffffa001e6d3>] svc_close_all+0x50/0x69 [sunrpc]
[107304.559107] [<ffffffffa0012a2b>] svc_destroy+0x9e/0x142 [sunrpc]
[107304.559126] [<ffffffffa0012b88>] svc_exit_thread+0xb9/0xc2 [sunrpc]
[107304.559138] [<ffffffffa008981b>] ? nfsd+0x0/0x13f [nfsd]
[107304.559149] [<ffffffffa0089940>] nfsd+0x125/0x13f [nfsd]
[107304.559157] [<ffffffff810685e3>] kthread+0x82/0x8a
[107304.559164] [<ffffffff8100c13a>] child_rip+0xa/0x20
[107304.559172] [<ffffffff8100baad>] ? restore_args+0x0/0x30
[107304.559179] [<ffffffff81068561>] ? kthread+0x0/0x8a
[107304.559185] [<ffffffff8100c130>] ? child_rip+0x0/0x20
[107304.559191] ---[ end trace c107131f4762168c ]---
[107304.927931] NFSD: Using /var/lib/nfs/v4recovery as the NFSv4 state
recovery directory
[107304.932765] NFSD: starting 90-second grace period
--
Francis
^ permalink raw reply
* RE: [PATCH v3 4/7] fsl_pq_mdio: Add Suport for etsec2.0 devices.
From: Kumar Gopalpet-B05799 @ 2009-10-30 8:42 UTC (permalink / raw)
To: netdev; +Cc: Fleming Andy-AFLEMING, David Miller
In-Reply-To: <20091030.005924.56385149.davem@davemloft.net>
>-----Original Message-----
>From: David Miller [mailto:davem@davemloft.net]
>Sent: Friday, October 30, 2009 1:29 PM
>To: Kumar Gopalpet-B05799
>Cc: netdev@vger.kernel.org; Fleming Andy-AFLEMING
>Subject: Re: [PATCH v3 4/7] fsl_pq_mdio: Add Suport for
>etsec2.0 devices.
>
>From: "Kumar Gopalpet-B05799" <B05799@freescale.com>
>Date: Fri, 30 Oct 2009 12:52:29 +0530
>
>>
>> 1. The patch failure is due to the absence of following commit in
>> net-next-2.6 tree.
>>
>> commit e72701acbe0b35e52d3f04d442837c06b4e64f1c
>> Author: Anton Vorontsov <avorontsov@ru.mvista.com>
>> Date: Wed Oct 14 14:54:52 2009 -0700
>
>It is in the net-next-2.6 tree, something is wrong with your clone.
We were referring to the logs shown on the gitweb.
The tree log does not show this commit.
(search for this commit id failed)
http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Fdavem%2Fnet-next-2.6.git
&a=search&h=29906f6a427d2004a515ebbcdc7b28bae8f6c19c&st=commit&s=e72701a
cbe0b35e52d3f04d442837c06b4e64f1c
Whereas, the log for the file does show the commit.
http://git.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git;a=histo
ry;f=drivers/net/fsl_pq_mdio.c;h=6ac4648669728421b934d32646ffbb626128387
6;hb=HEAD
Why is there such a mismatch on the tree itself?
>
>davem@sunset:~/src/GIT/net-next-2.6$ git describe
>e72701acbe0b35e52d3f04d442837c06b4e64f1c
>v2.6.31-9041-ge72701a
>davem@sunset:~/src/GIT/net-next-2.6$
Even we get this on our clone.
However, the changes to the file itself are absent.
Shall we generate the patch series against the snapshot (which contains
the changes)?
--
Thanks
Sandeep
^ permalink raw reply
* Re: [PATCH net-next-2.6] net: use hlist_for_each_entry()
From: David Miller @ 2009-10-30 8:40 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <4AEAA5DD.3040704@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 30 Oct 2009 09:37:49 +0100
> Small cleanup of __dev_get_by_name() and __dev_get_by_index()
> to use hlist_for_each_entry() : They'll look like their _rcu variant.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks Eric.
^ permalink raw reply
* [PATCH net-next-2.6] net: use hlist_for_each_entry()
From: Eric Dumazet @ 2009-10-30 8:37 UTC (permalink / raw)
To: David S. Miller; +Cc: Linux Netdev List
Small cleanup of __dev_get_by_name() and __dev_get_by_index()
to use hlist_for_each_entry() : They'll look like their _rcu variant.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/core/dev.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 631cc40..94f42a1 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -587,13 +587,13 @@ __setup("netdev=", netdev_boot_setup);
struct net_device *__dev_get_by_name(struct net *net, const char *name)
{
struct hlist_node *p;
+ struct net_device *dev;
+ struct hlist_head *head = dev_name_hash(net, name);
- hlist_for_each(p, dev_name_hash(net, name)) {
- struct net_device *dev
- = hlist_entry(p, struct net_device, name_hlist);
+ hlist_for_each_entry(dev, p, head, name_hlist)
if (!strncmp(dev->name, name, IFNAMSIZ))
return dev;
- }
+
return NULL;
}
EXPORT_SYMBOL(__dev_get_by_name);
@@ -638,13 +638,13 @@ EXPORT_SYMBOL(dev_get_by_name);
struct net_device *__dev_get_by_index(struct net *net, int ifindex)
{
struct hlist_node *p;
+ struct net_device *dev;
+ struct hlist_head *head = dev_index_hash(net, ifindex);
- hlist_for_each(p, dev_index_hash(net, ifindex)) {
- struct net_device *dev
- = hlist_entry(p, struct net_device, index_hlist);
+ hlist_for_each_entry(dev, p, head, index_hlist)
if (dev->ifindex == ifindex)
return dev;
- }
+
return NULL;
}
EXPORT_SYMBOL(__dev_get_by_index);
^ permalink raw reply related
* Re: [PATCH 5/6] bond: Implement a basic set of rtnl link ops
From: Patrick McHardy @ 2009-10-30 8:29 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: David Miller, netdev, Jay Vosburgh, Eric W. Biederman
In-Reply-To: <1256861906-5059-5-git-send-email-ebiederm@xmission.com>
Eric W. Biederman wrote:
> +static struct rtnl_link_ops bond_link_ops __read_mostly = {
> + .kind = "bond",
> + .setup = bond_setup,
> + .validate = bond_validate,
> +};
One more thing - you need to initialize .priv_size here so
the devices created through rtnl_link have enough private
room allocated.
^ permalink raw reply
* Re: [PATCH 5/6] bond: Implement a basic set of rtnl link ops
From: Patrick McHardy @ 2009-10-30 8:06 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: David Miller, netdev, Jay Vosburgh, Eric W. Biederman
In-Reply-To: <1256861906-5059-5-git-send-email-ebiederm@xmission.com>
Eric W. Biederman wrote:
> @@ -5115,6 +5118,10 @@ static int __init bonding_init(void)
>
> bond_create_proc_dir();
>
> + res = rtnl_link_register(&bond_link_ops);
> + if (res)
> + goto err;
The error handling looks wrong, at err: you call rtnl_link_unregister()
after rtnl_link_register() failed.
> +
> for (i = 0; i < max_bonds; i++) {
> res = bond_create(NULL);
> if (res)
> @@ -5128,14 +5135,12 @@ static int __init bonding_init(void)
> register_netdevice_notifier(&bond_netdev_notifier);
> register_inetaddr_notifier(&bond_inetaddr_notifier);
> bond_register_ipv6_notifier();
> -
> - goto out;
> -err:
> - rtnl_lock();
> - bond_free_all();
> - rtnl_unlock();
> out:
> return res;
> +err:
> + rtnl_link_unregister(&bond_link_ops);
> + bond_destroy_proc_dir();
> + goto out;
>
> }
^ permalink raw reply
* Re: [PATCH net-next-2.6] veth: Fix veth_dellink method
From: David Miller @ 2009-10-30 8:00 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, xemul
In-Reply-To: <4AEA8F0F.4030100@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 30 Oct 2009 08:00:31 +0100
> In commit 23289a37e2b127dfc4de1313fba15bb4c9f0cd5b
> (net: add a list_head parameter to dellink() method),
> I forgot to actually use this parameter in veth_dellink.
>
> I remember feeling a bit uncomfortable about veth_close(),
> because it does :
>
> netif_carrier_off(dev);
> netif_carrier_off(priv->peer);
>
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks Eric.
^ permalink raw reply
* Re: [PATCH v3 4/7] fsl_pq_mdio: Add Suport for etsec2.0 devices.
From: David Miller @ 2009-10-30 7:59 UTC (permalink / raw)
To: B05799; +Cc: netdev, afleming
In-Reply-To: <9F4C7D19E8361D4C94921B95BE08B81B9503CA@zin33exm22.fsl.freescale.net>
From: "Kumar Gopalpet-B05799" <B05799@freescale.com>
Date: Fri, 30 Oct 2009 12:52:29 +0530
>
> 1. The patch failure is due to the absence of following commit in
> net-next-2.6 tree.
>
> commit e72701acbe0b35e52d3f04d442837c06b4e64f1c
> Author: Anton Vorontsov <avorontsov@ru.mvista.com>
> Date: Wed Oct 14 14:54:52 2009 -0700
It is in the net-next-2.6 tree, something is wrong with your
clone.
davem@sunset:~/src/GIT/net-next-2.6$ git describe e72701acbe0b35e52d3f04d442837c06b4e64f1c
v2.6.31-9041-ge72701a
davem@sunset:~/src/GIT/net-next-2.6$
^ permalink raw reply
* Re: [PATCH 0/6] Bonding simplifications and netns support
From: Eric W. Biederman @ 2009-10-30 7:50 UTC (permalink / raw)
To: David Miller; +Cc: netdev, fubar
In-Reply-To: <20091029.232523.66969302.davem@davemloft.net>
David Miller <davem@davemloft.net> writes:
> From: ebiederm@xmission.com (Eric W. Biederman)
> Date: Thu, 29 Oct 2009 17:16:54 -0700
>
>> I recently had it pointed out to me that the bonding driver does not
>> work in a network namespace. So I have simplified the bonding driver
>> a bit, added support for ip link add and ip link del, and finally made
>> the bonding driver work in multiple network namespaces.
>>
>> The most note worthy change in the patchset is the addition of support
>> in the networking core for registering a sysfs group for a device.
>>
>> Using this in the bonding driver simplifies the code and removes a
>> userspace race between actions triggered by the netlink event and the
>> bonding sysfs attributes appearing.
>
> I have no objections to these patches, but I'd like the bonding
> folks to have a chance to look at it before I apply to net-next-2.6
Sure.
> One question though, are you sure this clever extra slot scheme
> in patch #1 works for, f.e., a bond of wireless devices? It seems
> like it would work out, but I wanted to ask to make sure you
> considered that case.
I have not explicitly tested wireless devices. But I did make certain
we have enough slots in the array. I did write the code so that a
device driver can use at most one slot (the next slot gets
unconditionally stomped). Other that it is just shifting of where
sysfs_create_group and sysfs_remove_group are called. So I would
be totally stunned if bonded wireless devices started failing from
this change.
Sometime when I have sufficient ambition I intend to reorganize all
callers of sysfs_create_group, sysfs_create_file so that device_add
does all of the work, allowing userspace that responds to hotplug
events to count on everything being there. The current situation is
inherently racy which is a unnecessary pain.
Eric
^ permalink raw reply
* Re: [PATCH] udev: create empty regular files to represent net interfaces
From: Hannes Reinecke @ 2009-10-30 7:45 UTC (permalink / raw)
To: Kay Sievers
Cc: Matt Domsch, dann frazier, linux-hotplug, Narendra_K, netdev,
Jordan_Hargrave, Charles_Rose, Ben Hutchings
In-Reply-To: <ac3eb2510910280123g3c0e3d95wb38a239238906027@mail.gmail.com>
Kay Sievers wrote:
> On Tue, Oct 27, 2009 at 21:55, Matt Domsch <Matt_Domsch@dell.com> wrote:
>> On Thu, Oct 22, 2009 at 12:36:20AM -0600, dann frazier wrote:
>>> Here's a proof of concept to further the discussion..
>>>
>>> The default filename uses the format:
>>> /dev/netdev/by-ifindex/$ifindex
>>>
>>> This provides the infrastructure to permit udev rules to create aliases for
>>> network devices using symlinks, for example:
>>>
>>> /dev/netdev/by-name/eth0 -> ../by-ifindex/1
>>> /dev/netdev/by-biosname/LOM0 -> ../by-ifindex/3
>>>
>>> A library (such as the proposed libnetdevname) could use this information
>>> to provide an alias->realname mapping for network utilities.
>> yes, this could work, as IFINDEX is already exported in the uevents,
>> and that's the primary value udev needs to set up the mapping.
>>
>> While I like the little ifindex2name script you've got, I think udev
>> could simply call if_indextoname() to get this, and not call an
>> external program? I suppose it could be a really really simple
>> external program too.
>
> What's the point of all this? Why would udev ever need to find the
> name of a device by the ifindex? The device name is the primary value
> for the kernel events udev acts on.
>
>> I'd be fine with this approach. It has the advantages of not
>> requiring a kernel change at all, and not creating a whole character
>> device which would be useless. And it doesn't preclude someone in the
>> future from creating a char device for network devices should they so
>> choose.
>>
>> Kay, what say you as udev owner?
>
> That all sounds very much like something which will hit us back some
> day. I'm not sure, if udev should publish such dead text files in
> /dev, it does not seem to fit the usual APIs/assumptions where /sys
> and /dev match, and libudev provides access to both. It all sounds
> more like a database for a possible netdevname library, which does not
> need to be public in /dev, right?
>
And to throw in some bit of useless information;
I've pondered the idea of persistent device names for network interfaces
a while back when designing the original layout of the persistent
device naming scheme.
The one reason I didn't to this was that a network interface is
_not_ a file, but rather an abstract type which is known only internally
in the kernel (ie the one exemption from the 'everything is a file'
UNIX rule).
And as udev is primarily concerned with the _files_, using it for
network interfaces would be a workaround at best.
When I were to design this, I would be implementing network interface
_aliases_, so that a network interface could be accessed either by
name or by alias. This mechanism can then be managed by udev, much
like we (ie SUSE) is using it nowadays to assign the network interface
numbers.
But the mechanism for the aliases has to live in the same instance which
also handles the network interface nowadays, ie the net subsystem of
the kernel. Implementing this in udev is _not_ the way to go.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: [PATCH v3 4/7] fsl_pq_mdio: Add Suport for etsec2.0 devices.
From: Kumar Gopalpet-B05799 @ 2009-10-30 7:22 UTC (permalink / raw)
To: netdev; +Cc: Fleming Andy-AFLEMING, David Miller
In-Reply-To: <20091029.212142.189578068.davem@davemloft.net>
>Did you try the bundle I posted yesterday which failed? This
>one is identical, you didn't change anything.
>
>This is getting rediculious.
>
>Try again, and when you do take the patches from patchwork at:
>
> http://patchwork.ozlabs.org/project/netdev/list/
>
>and you try to apply them to net-next-2.6 using git (_not_
>'patch', 'patch' is way too lenient about fuzz and line
>offsets) so that you can see what's happening.
>
>I'm completely ignoring your patch postings until you sort this out.
1. The patch failure is due to the absence of following commit in
net-next-2.6 tree.
commit e72701acbe0b35e52d3f04d442837c06b4e64f1c
Author: Anton Vorontsov <avorontsov@ru.mvista.com>
Date: Wed Oct 14 14:54:52 2009 -0700
net: Fix OF platform drivers coldplug/hotplug when compiled
as modules
Some OF platform drivers are missing module device tables,
so they won't
load automatically on boot. This patch fixes the issue by
adding proper
MODULE_DEVICE_TABLE() macros to the drivers.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
2. The git clone DOES NOT show the changes affected by the above patch
3. If we take a log of the affected file(fsl_pq_mdio.c), then it DOES
show the commit/changes
(Gitweb:
http://git.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git;a=histo
ry;f=drivers/net/fsl_pq_mdio.c;h=6ac4648669728421b934d32646ffbb626128387
6;hb=HEAD)
4. If we take the log of the tree, the same commit is NOT visible.
(Gitweb:
http://git.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git;a=log)
5. If we take a Snapshot of the tree, the changes to the file are
present. (in the tarball)
To sum up, the Clone and Tree log DOES NOT show the changes.
The Snapshot and file log DOES show it.
We have tried it on multiple machine/tree clones here.
Is there some problem with the Git tree itself?
>
>You cannot have line offsets or fuzz when you send patches
>that are to be applied by GIT, and that is the problem with patch #4:
>
>davem@sunset:~/src/GIT/net-next-2.6$ patch -p1 <diff patching
>file drivers/net/fsl_pq_mdio.c Hunk #6 succeeded at 436 with fuzz 1.
>patching file drivers/net/fsl_pq_mdio.h
>
>GIT does not allow line fuzz, you must make absolutely
>pristine patches, and therefore if you are testing your
>submission with just 'patch' that absolutely will not work.
No we are using git-am.
We did try this on multiple clones of the net-next-2.6 tree.
>
>Thanks.
>--
>To unsubscribe from this list: send the line "unsubscribe
>netdev" in the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Thanks
Sandeep
^ permalink raw reply
* [PATCH net-next-2.6] veth: Fix veth_dellink method
From: Eric Dumazet @ 2009-10-30 7:00 UTC (permalink / raw)
To: David S. Miller; +Cc: Linux Netdev List, Pavel Emelyanov
In commit 23289a37e2b127dfc4de1313fba15bb4c9f0cd5b
(net: add a list_head parameter to dellink() method),
I forgot to actually use this parameter in veth_dellink.
I remember feeling a bit uncomfortable about veth_close(),
because it does :
netif_carrier_off(dev);
netif_carrier_off(priv->peer);
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index ffb502d..9bed694 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -450,8 +450,8 @@ static void veth_dellink(struct net_device *dev, struct list_head *head)
priv = netdev_priv(dev);
peer = priv->peer;
- unregister_netdevice(dev);
- unregister_netdevice(peer);
+ unregister_netdevice_queue(dev, head);
+ unregister_netdevice_queue(peer, head);
}
static const struct nla_policy veth_policy[VETH_INFO_MAX + 1];
^ permalink raw reply related
* Re: pull request: wireless-next-2.6 2009-10-28
From: Johannes Berg @ 2009-10-30 7:00 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: Pekka Enberg, David Miller, linux-wireless, netdev, linux-kernel,
linville
In-Reply-To: <200910292248.42229.bzolnier@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1058 bytes --]
On Thu, 2009-10-29 at 22:48 +0100, Bartlomiej Zolnierkiewicz wrote:
> I don't think that my technical arguments are pointless.
>
> Quite the contrary, I'm pretty confident that addressing my review concerns
> would result in better RT28x00 / RT30x0 support in the very near future.
So you know how to make the driver better, whine that it doesn't work,
but don't help out either. Hey, good way to collaborate!
> > It should be pretty obvious by now that the best way to improve things
> > is to work with the relevant maintainers, not against them. (Unless
> > you wish your work to be ignored, of course.)
>
> I work with a lot of other maintainers. I would say that providing valuable
> review feedback is also "working with" (at least I would be very happy with
> such feedback in my projects).
Face it though, people have read your feedback, thought about it, and
decided to disagree. You seem to have a childish problem with that,
appealing to higher authority to get your way just makes you look more
childish.
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH 4/6] vlan: Optimize multiple unregistration
From: David Miller @ 2009-10-30 6:43 UTC (permalink / raw)
To: eric.dumazet; +Cc: kaber, netdev
In-Reply-To: <4AEA8813.9090505@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 30 Oct 2009 07:30:43 +0100
> Patrick McHardy a écrit :
>> Eric Dumazet wrote:
>>> Patrick McHardy a écrit :
>>>
>>>> Indeed, but unregister_vlan_dev() destroys the group once the
>>>> count has reached zero, so we must not access it after that.
>>> Well, I hoped call_rcu() callback doesnt fire and kfree(grp) until we exited
>>> from unregister_vlan_dev_alls(), with RTNL locked...
>>
>> The RTNL is a mutex, so it shouldn't prevent call_rcu from firing.
>
> Oops this is totally right of course, so your patch is actually a bug fix :)
>
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
I've applied Patrick's patch, thanks everyone.
^ permalink raw reply
* Re: [PATCH 0/6] Bonding simplifications and netns support
From: David Miller @ 2009-10-30 6:40 UTC (permalink / raw)
To: dada1; +Cc: ebiederm, netdev, fubar
In-Reply-To: <4AEA89E9.30902@cosmosbay.com>
From: Eric Dumazet <dada1@cosmosbay.com>
Date: Fri, 30 Oct 2009 07:38:33 +0100
> I have patches against bond driver for unregister_netdevice_queue() stuff,
> I'll wait for Eric work being committed first.
Ok, thanks for the heads up.
^ permalink raw reply
* Re: [PATCH 0/6] Bonding simplifications and netns support
From: Eric Dumazet @ 2009-10-30 6:38 UTC (permalink / raw)
To: David Miller; +Cc: ebiederm, netdev, fubar
In-Reply-To: <20091029.232523.66969302.davem@davemloft.net>
David Miller a écrit :
> From: ebiederm@xmission.com (Eric W. Biederman)
> Date: Thu, 29 Oct 2009 17:16:54 -0700
>
>> I recently had it pointed out to me that the bonding driver does not
>> work in a network namespace. So I have simplified the bonding driver
>> a bit, added support for ip link add and ip link del, and finally made
>> the bonding driver work in multiple network namespaces.
>>
>> The most note worthy change in the patchset is the addition of support
>> in the networking core for registering a sysfs group for a device.
>>
>> Using this in the bonding driver simplifies the code and removes a
>> userspace race between actions triggered by the netlink event and the
>> bonding sysfs attributes appearing.
>
> I have no objections to these patches, but I'd like the bonding
> folks to have a chance to look at it before I apply to net-next-2.6
>
> One question though, are you sure this clever extra slot scheme
> in patch #1 works for, f.e., a bond of wireless devices? It seems
> like it would work out, but I wanted to ask to make sure you
> considered that case.
>
I have patches against bond driver for unregister_netdevice_queue() stuff,
I'll wait for Eric work being committed first.
Thanks
^ permalink raw reply
* Re: [PATCH 4/6] vlan: Optimize multiple unregistration
From: Eric Dumazet @ 2009-10-30 6:30 UTC (permalink / raw)
To: Patrick McHardy; +Cc: David S. Miller, Linux Netdev List
In-Reply-To: <4AE9D000.90603@trash.net>
Patrick McHardy a écrit :
> Eric Dumazet wrote:
>> Patrick McHardy a écrit :
>>
>>> Indeed, but unregister_vlan_dev() destroys the group once the
>>> count has reached zero, so we must not access it after that.
>> Well, I hoped call_rcu() callback doesnt fire and kfree(grp) until we exited
>> from unregister_vlan_dev_alls(), with RTNL locked...
>
> The RTNL is a mutex, so it shouldn't prevent call_rcu from firing.
Oops this is totally right of course, so your patch is actually a bug fix :)
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply
* Re: [PATCH 0/6] Bonding simplifications and netns support
From: David Miller @ 2009-10-30 6:25 UTC (permalink / raw)
To: ebiederm; +Cc: netdev, fubar
In-Reply-To: <m1tyxhwxah.fsf@fess.ebiederm.org>
From: ebiederm@xmission.com (Eric W. Biederman)
Date: Thu, 29 Oct 2009 17:16:54 -0700
> I recently had it pointed out to me that the bonding driver does not
> work in a network namespace. So I have simplified the bonding driver
> a bit, added support for ip link add and ip link del, and finally made
> the bonding driver work in multiple network namespaces.
>
> The most note worthy change in the patchset is the addition of support
> in the networking core for registering a sysfs group for a device.
>
> Using this in the bonding driver simplifies the code and removes a
> userspace race between actions triggered by the netlink event and the
> bonding sysfs attributes appearing.
I have no objections to these patches, but I'd like the bonding
folks to have a chance to look at it before I apply to net-next-2.6
One question though, are you sure this clever extra slot scheme
in patch #1 works for, f.e., a bond of wireless devices? It seems
like it would work out, but I wanted to ask to make sure you
considered that case.
Thanks.
^ permalink raw reply
* Re: [PATCH] udev: create empty regular files to represent net interfaces
From: Marco d'Itri @ 2009-10-30 6:22 UTC (permalink / raw)
To: dann frazier
Cc: Greg KH, Matt Domsch, Kay Sievers, linux-hotplug, Narendra_K,
netdev, Jordan_Hargrave, Charles_Rose, Ben Hutchings
In-Reply-To: <20091030053824.GA25540@ldl.fc.hp.com>
[-- Attachment #1: Type: text/plain, Size: 654 bytes --]
On Oct 30, dann frazier <dannf@hp.com> wrote:
> On Fri, Oct 30, 2009 at 04:30:29AM +0100, Marco d'Itri wrote:
> > On Oct 29, dann frazier <dannf@dannf.org> wrote:
> >
> > > That would make a lot of users much happier (myself included), but it
> > > does restrict us into one view. At different times, admins think of
> > > their NICs by different properties. I may want to do IP assignment by
> > [Citation needed]
> Is "I" not clear, or do you just not believe I admin servers?
I was referring to "At different times, admins think of their NICs by
different properties", which I am not sure is a valid generalization.
--
ciao,
Marco
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ 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