* [PATCH net-next-2.6] rtnetlink: rtnl_setlink() and rtnl_getlink() changes
@ 2009-10-21 20:59 Eric Dumazet
2009-10-21 21:48 ` Stephen Hemminger
2009-10-22 11:34 ` David Miller
0 siblings, 2 replies; 5+ messages in thread
From: Eric Dumazet @ 2009-10-21 20:59 UTC (permalink / raw)
To: Stephen Hemminger, David S. Miller; +Cc: Linux Netdev List
Stephen, do you think we could change "ip link show dev ethX" to
let it use rtnl_getlink() instead of rtnl_dump_ifinfo() ?
Thanks !
[PATCH net-next-2.6]rtnetlink: rtnl_setlink() and rtnl_getlink() changes
rtnl_getlink() & rtnl_setlink() run with RTNL held, we can use
__dev_get_by_index() and __dev_get_by_name() variants and avoid
dev_hold()/dev_put()
Adds to rtnl_getlink() the capability to find a device by its name,
not only by its index.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/core/rtnetlink.c | 38 +++++++++++++++++++-------------------
1 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index eb42873..ba13b09 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -910,9 +910,9 @@ static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
err = -EINVAL;
ifm = nlmsg_data(nlh);
if (ifm->ifi_index > 0)
- dev = dev_get_by_index(net, ifm->ifi_index);
+ dev = __dev_get_by_index(net, ifm->ifi_index);
else if (tb[IFLA_IFNAME])
- dev = dev_get_by_name(net, ifname);
+ dev = __dev_get_by_name(net, ifname);
else
goto errout;
@@ -922,11 +922,9 @@ static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
}
if ((err = validate_linkmsg(dev, tb)) < 0)
- goto errout_dev;
+ goto errout;
err = do_setlink(dev, ifm, tb, ifname, 0);
-errout_dev:
- dev_put(dev);
errout:
return err;
}
@@ -1154,6 +1152,7 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr* nlh, void *arg)
{
struct net *net = sock_net(skb->sk);
struct ifinfomsg *ifm;
+ char ifname[IFNAMSIZ];
struct nlattr *tb[IFLA_MAX+1];
struct net_device *dev = NULL;
struct sk_buff *nskb;
@@ -1163,19 +1162,23 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr* nlh, void *arg)
if (err < 0)
return err;
+ if (tb[IFLA_IFNAME])
+ nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
+
ifm = nlmsg_data(nlh);
- if (ifm->ifi_index > 0) {
- dev = dev_get_by_index(net, ifm->ifi_index);
- if (dev == NULL)
- return -ENODEV;
- } else
+ if (ifm->ifi_index > 0)
+ dev = __dev_get_by_index(net, ifm->ifi_index);
+ else if (tb[IFLA_IFNAME])
+ dev = __dev_get_by_name(net, ifname);
+ else
return -EINVAL;
+ if (dev == NULL)
+ return -ENODEV;
+
nskb = nlmsg_new(if_nlmsg_size(dev), GFP_KERNEL);
- if (nskb == NULL) {
- err = -ENOBUFS;
- goto errout;
- }
+ if (nskb == NULL)
+ return -ENOBUFS;
err = rtnl_fill_ifinfo(nskb, dev, RTM_NEWLINK, NETLINK_CB(skb).pid,
nlh->nlmsg_seq, 0, 0);
@@ -1183,11 +1186,8 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr* nlh, void *arg)
/* -EMSGSIZE implies BUG in if_nlmsg_size */
WARN_ON(err == -EMSGSIZE);
kfree_skb(nskb);
- goto errout;
- }
- err = rtnl_unicast(nskb, net, NETLINK_CB(skb).pid);
-errout:
- dev_put(dev);
+ } else
+ err = rtnl_unicast(nskb, net, NETLINK_CB(skb).pid);
return err;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH net-next-2.6] rtnetlink: rtnl_setlink() and rtnl_getlink() changes
2009-10-21 20:59 [PATCH net-next-2.6] rtnetlink: rtnl_setlink() and rtnl_getlink() changes Eric Dumazet
@ 2009-10-21 21:48 ` Stephen Hemminger
2009-10-22 2:59 ` Eric Dumazet
2009-10-22 11:34 ` David Miller
1 sibling, 1 reply; 5+ messages in thread
From: Stephen Hemminger @ 2009-10-21 21:48 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, Linux Netdev List
On Wed, 21 Oct 2009 22:59:31 +0200
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Stephen, do you think we could change "ip link show dev ethX" to
> let it use rtnl_getlink() instead of rtnl_dump_ifinfo() ?
>
> Thanks !
>
> [PATCH net-next-2.6]rtnetlink: rtnl_setlink() and rtnl_getlink() changes
>
> rtnl_getlink() & rtnl_setlink() run with RTNL held, we can use
> __dev_get_by_index() and __dev_get_by_name() variants and avoid
> dev_hold()/dev_put()
>
> Adds to rtnl_getlink() the capability to find a device by its name,
> not only by its index.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> net/core/rtnetlink.c | 38 +++++++++++++++++++-------------------
> 1 files changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index eb42873..ba13b09 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -910,9 +910,9 @@ static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
> err = -EINVAL;
> ifm = nlmsg_data(nlh);
> if (ifm->ifi_index > 0)
> - dev = dev_get_by_index(net, ifm->ifi_index);
> + dev = __dev_get_by_index(net, ifm->ifi_index);
> else if (tb[IFLA_IFNAME])
> - dev = dev_get_by_name(net, ifname);
> + dev = __dev_get_by_name(net, ifname);
> else
> goto errout;
>
> @@ -922,11 +922,9 @@ static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
> }
>
> if ((err = validate_linkmsg(dev, tb)) < 0)
> - goto errout_dev;
> + goto errout;
>
> err = do_setlink(dev, ifm, tb, ifname, 0);
> -errout_dev:
> - dev_put(dev);
> errout:
> return err;
> }
> @@ -1154,6 +1152,7 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr* nlh, void *arg)
> {
> struct net *net = sock_net(skb->sk);
> struct ifinfomsg *ifm;
> + char ifname[IFNAMSIZ];
> struct nlattr *tb[IFLA_MAX+1];
> struct net_device *dev = NULL;
> struct sk_buff *nskb;
> @@ -1163,19 +1162,23 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr* nlh, void *arg)
> if (err < 0)
> return err;
>
> + if (tb[IFLA_IFNAME])
> + nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
> +
> ifm = nlmsg_data(nlh);
> - if (ifm->ifi_index > 0) {
> - dev = dev_get_by_index(net, ifm->ifi_index);
> - if (dev == NULL)
> - return -ENODEV;
> - } else
> + if (ifm->ifi_index > 0)
> + dev = __dev_get_by_index(net, ifm->ifi_index);
> + else if (tb[IFLA_IFNAME])
> + dev = __dev_get_by_name(net, ifname);
> + else
> return -EINVAL;
>
> + if (dev == NULL)
> + return -ENODEV;
> +
> nskb = nlmsg_new(if_nlmsg_size(dev), GFP_KERNEL);
> - if (nskb == NULL) {
> - err = -ENOBUFS;
> - goto errout;
> - }
> + if (nskb == NULL)
> + return -ENOBUFS;
>
> err = rtnl_fill_ifinfo(nskb, dev, RTM_NEWLINK, NETLINK_CB(skb).pid,
> nlh->nlmsg_seq, 0, 0);
> @@ -1183,11 +1186,8 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr* nlh, void *arg)
> /* -EMSGSIZE implies BUG in if_nlmsg_size */
> WARN_ON(err == -EMSGSIZE);
> kfree_skb(nskb);
> - goto errout;
> - }
> - err = rtnl_unicast(nskb, net, NETLINK_CB(skb).pid);
> -errout:
> - dev_put(dev);
> + } else
> + err = rtnl_unicast(nskb, net, NETLINK_CB(skb).pid);
>
> return err;
> }
Would work, but not sure what it gains.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net-next-2.6] rtnetlink: rtnl_setlink() and rtnl_getlink() changes
2009-10-21 21:48 ` Stephen Hemminger
@ 2009-10-22 2:59 ` Eric Dumazet
2009-10-22 4:48 ` Eric Dumazet
0 siblings, 1 reply; 5+ messages in thread
From: Eric Dumazet @ 2009-10-22 2:59 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David S. Miller, Linux Netdev List
Stephen Hemminger a écrit :
> On Wed, 21 Oct 2009 22:59:31 +0200
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>> Stephen, do you think we could change "ip link show dev ethX" to
>> let it use rtnl_getlink() instead of rtnl_dump_ifinfo() ?
>>
>
> Would work, but not sure what it gains.
It takes about one second to dump the table when we have 25.000 devices
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next-2.6] rtnetlink: rtnl_setlink() and rtnl_getlink() changes
2009-10-22 2:59 ` Eric Dumazet
@ 2009-10-22 4:48 ` Eric Dumazet
0 siblings, 0 replies; 5+ messages in thread
From: Eric Dumazet @ 2009-10-22 4:48 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David S. Miller, Linux Netdev List
Eric Dumazet a écrit :
> Stephen Hemminger a écrit :
>> On Wed, 21 Oct 2009 22:59:31 +0200
>> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>
>>> Stephen, do you think we could change "ip link show dev ethX" to
>>> let it use rtnl_getlink() instead of rtnl_dump_ifinfo() ?
>>>
>> Would work, but not sure what it gains.
>
> It takes about one second to dump the table when we have 25.000 devices
Adding new links takes also lot of time in rtnl_dump_ifinfo(), we could
optimize it using a 256 fanout (using the ifindex hash table instead
of the single list)
But IMHO rtnl_dump_ifinfo() should be used only when needed, not when
querying/adding a particular device.
------------------------------------------------------------------------------
PerfTop: 42745 irqs/sec kernel:88.0% [100000 cycles], (all, 8 CPUs)
------------------------------------------------------------------------------
samples pcnt kernel function
_______ _____ _______________
231146.00 - 52.4% : rtnl_dump_ifinfo
18491.00 - 4.2% : __register_sysctl_paths
17700.00 - 4.0% : mwait_idle
12883.00 - 2.9% : rtnl_fill_ifinfo
12661.00 - 2.9% : schedule
6324.00 - 1.4% : find_busiest_group
5911.00 - 1.3% : _spin_lock_irqsave
4862.00 - 1.1% : dev_get_stats
4726.00 - 1.1% : copy_to_user
4547.00 - 1.0% : __nla_put
4117.00 - 0.9% : sysfs_find_dirent
4090.00 - 0.9% : sysenter_past_esp
3789.00 - 0.9% : fput
3735.00 - 0.8% : __nla_reserve
3699.00 - 0.8% : read_tsc
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next-2.6] rtnetlink: rtnl_setlink() and rtnl_getlink() changes
2009-10-21 20:59 [PATCH net-next-2.6] rtnetlink: rtnl_setlink() and rtnl_getlink() changes Eric Dumazet
2009-10-21 21:48 ` Stephen Hemminger
@ 2009-10-22 11:34 ` David Miller
1 sibling, 0 replies; 5+ messages in thread
From: David Miller @ 2009-10-22 11:34 UTC (permalink / raw)
To: eric.dumazet; +Cc: shemminger, netdev
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 21 Oct 2009 22:59:31 +0200
> rtnl_getlink() & rtnl_setlink() run with RTNL held, we can use
> __dev_get_by_index() and __dev_get_by_name() variants and avoid
> dev_hold()/dev_put()
>
> Adds to rtnl_getlink() the capability to find a device by its name,
> not only by its index.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Looks good, applied, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-10-22 11:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-10-21 20:59 [PATCH net-next-2.6] rtnetlink: rtnl_setlink() and rtnl_getlink() changes Eric Dumazet
2009-10-21 21:48 ` Stephen Hemminger
2009-10-22 2:59 ` Eric Dumazet
2009-10-22 4:48 ` Eric Dumazet
2009-10-22 11:34 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).