* Re: [PATCH net-next] sysfs: add entry to indicate network interfaces with random MAC address
From: Stefan Assmann @ 2010-07-21 8:10 UTC (permalink / raw)
To: David Miller
Cc: abadea, bhutchings, netdev, linux-kernel, gospo, gregory.v.rose,
alexander.h.duyck, leedom, harald
In-Reply-To: <20100720.131839.134093789.davem@davemloft.net>
On 20.07.2010 22:18, David Miller wrote:
> From: Stefan Assmann <sassmann@redhat.com>
> Date: Tue, 20 Jul 2010 14:17:30 +0200
>
>> On 20.07.2010 13:58, Alex Badea wrote:
>>> Hi,
>>>
>>> On 07/20/2010 02:47 PM, Stefan Assmann wrote:
>>>>> What about devices that 'steal' MAC addresses from slave devices?
>>>
>>> Might I suggest an attribute such as "address_type", which could report
>>> "permanent", "random", "stolen", or something else in the future?
>>
>> In case there's demand for such a multi-purpose attribute I see no
>> reason to object. More thoughts on this?
>
> I think this is a great idea.
>
> Now it makes sense to use a new 'u8' in struct netdevice or similar to
> store this, since we'll have more than a boolean here.
>
I put Alex' idea into code for further discussion, keeping the names
mentioned here until we agree on the scope of this attribute. When we
have settled I'll post a patch with proper patch description.
Stefan
---
include/linux/etherdevice.h | 14 ++++++++++++++
include/linux/netdevice.h | 6 ++++++
net/core/net-sysfs.c | 2 ++
3 files changed, 22 insertions(+), 0 deletions(-)
diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index 3d7a668..f15cac8 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -127,6 +127,20 @@ static inline void random_ether_addr(u8 *addr)
}
/**
+ * dev_hw_addr_random - Create random MAC and set device flag
+ * @dev: pointer to net_device structure
+ * @addr: Pointer to a six-byte array containing the Ethernet address
+ *
+ * Generate random MAC to be used by a device and set addr_type
+ * so the state can be read by sysfs and be used by udev.
+ */
+static inline void dev_hw_addr_random(struct net_device *dev, u8 *hwaddr)
+{
+ dev->addr_type |= NET_ADDR_RANDOM;
+ random_ether_addr(hwaddr);
+}
+
+/**
* compare_ether_addr - Compare two Ethernet addresses
* @addr1: Pointer to a six-byte array containing the Ethernet address
* @addr2: Pointer other six-byte array containing the Ethernet address
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index b626289..2718895 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -66,6 +66,11 @@ struct wireless_dev;
#define HAVE_FREE_NETDEV /* free_netdev() */
#define HAVE_NETDEV_PRIV /* netdev_priv() */
+/* hardware address types */
+#define NET_ADDR_PERM 0 /* address is permanent (default) */
+#define NET_ADDR_RANDOM 1 /* address is generated randomly */
+#define NET_ADDR_STOLEN 2 /* address is stolen from other device */
+
/* Backlog congestion levels */
#define NET_RX_SUCCESS 0 /* keep 'em coming, baby */
#define NET_RX_DROP 1 /* packet dropped */
@@ -920,6 +925,7 @@ struct net_device {
/* Interface address info. */
unsigned char perm_addr[MAX_ADDR_LEN]; /* permanent hw address */
unsigned char addr_len; /* hardware address length */
+ unsigned char addr_type; /* hardware address type */
unsigned short dev_id; /* for shared network cards */
spinlock_t addr_list_lock;
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index d2b5965..052ab14 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -96,6 +96,7 @@ static ssize_t netdev_store(struct device *dev, struct device_attribute *attr,
NETDEVICE_SHOW(dev_id, fmt_hex);
NETDEVICE_SHOW(addr_len, fmt_dec);
+NETDEVICE_SHOW(addr_type, fmt_dec);
NETDEVICE_SHOW(iflink, fmt_dec);
NETDEVICE_SHOW(ifindex, fmt_dec);
NETDEVICE_SHOW(features, fmt_long_hex);
@@ -296,6 +297,7 @@ static ssize_t show_ifalias(struct device *dev,
static struct device_attribute net_class_attributes[] = {
__ATTR(addr_len, S_IRUGO, show_addr_len, NULL),
+ __ATTR(addr_type, S_IRUGO, show_addr_type, NULL),
__ATTR(dev_id, S_IRUGO, show_dev_id, NULL),
__ATTR(ifalias, S_IRUGO | S_IWUSR, show_ifalias, store_ifalias),
__ATTR(iflink, S_IRUGO, show_iflink, NULL),
--
1.7.1
^ permalink raw reply related
* Re: [PATCH net-next-2.6] netlink: netlink_recvmsg() fix
From: Johannes Berg @ 2010-07-21 8:05 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <1279639232.2498.82.camel@edumazet-laptop>
On Tue, 2010-07-20 at 17:20 +0200, Eric Dumazet wrote:
> [PATCH net-next-2.6 v2] netlink: netlink_recvmsg() fix
>
> commit 1dacc76d0014
> (net/compat/wext: send different messages to compat tasks)
> introduced a race condition on netlink, in case MSG_PEEK is used.
>
> An skb given by skb_recv_datagram() might be shared, we must copy it
> before any modification, or risk fatal corruption.
Makes sense to me, seeing that if you MSG_PEEK it just increases
skb->users. But nothing could touch the other skb at the same time?
Although I guess with netlink multicast we have a similar situation.
johannes
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> net/netlink/af_netlink.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> index 7aeaa83..1537fa5 100644
> --- a/net/netlink/af_netlink.c
> +++ b/net/netlink/af_netlink.c
> @@ -1405,7 +1405,7 @@ static int netlink_recvmsg(struct kiocb *kiocb, struct socket *sock,
> struct netlink_sock *nlk = nlk_sk(sk);
> int noblock = flags&MSG_DONTWAIT;
> size_t copied;
> - struct sk_buff *skb, *frag __maybe_unused = NULL;
> + struct sk_buff *skb;
> int err;
>
> if (flags&MSG_OOB)
> @@ -1440,7 +1440,12 @@ static int netlink_recvmsg(struct kiocb *kiocb, struct socket *sock,
> kfree_skb(skb);
> skb = compskb;
> } else {
> - frag = skb_shinfo(skb)->frag_list;
> + skb = skb_unshare(skb, GFP_KERNEL);
> + if (!skb) {
> + err = -ENOMEM;
> + goto out;
> + }
> + kfree_skb(skb_shinfo(skb)->frag_list);
> skb_shinfo(skb)->frag_list = NULL;
> }
> }
> @@ -1477,10 +1482,6 @@ static int netlink_recvmsg(struct kiocb *kiocb, struct socket *sock,
> if (flags & MSG_TRUNC)
> copied = skb->len;
>
> -#ifdef CONFIG_COMPAT_NETLINK_MESSAGES
> - skb_shinfo(skb)->frag_list = frag;
> -#endif
> -
> skb_free_datagram(sk, skb);
>
> if (nlk->cb && atomic_read(&sk->sk_rmem_alloc) <= sk->sk_rcvbuf / 2)
>
>
>
^ permalink raw reply
* [PATCH net-next-2.6] net: RTA_MARK addition
From: Eric Dumazet @ 2010-07-21 8:03 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Add a new rt attribute, RTA_MARK, and use it in
rt_fill_info()/inet_rtm_getroute() to support following commands :
ip route get 192.168.20.110 mark NUMBER
ip route get 192.168.20.108 from 192.168.20.110 iif eth1 mark NUMBER
ip route list cache [192.168.20.110] mark NUMBER
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
include/linux/rtnetlink.h | 1 +
net/ipv4/route.c | 7 +++++++
2 files changed, 8 insertions(+)
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index fbc8cb0..58d4449 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -282,6 +282,7 @@ enum rtattr_type_t {
RTA_SESSION, /* no longer used */
RTA_MP_ALGO, /* no longer used */
RTA_TABLE,
+ RTA_MARK,
__RTA_MAX
};
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 562ce92..3f56b6e 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2878,6 +2878,9 @@ static int rt_fill_info(struct net *net,
if (rtnetlink_put_metrics(skb, rt->dst.metrics) < 0)
goto nla_put_failure;
+ if (rt->fl.mark)
+ NLA_PUT_BE32(skb, RTA_MARK, rt->fl.mark);
+
error = rt->dst.error;
expires = rt->dst.expires ? rt->dst.expires - jiffies : 0;
if (rt->peer) {
@@ -2933,6 +2936,7 @@ static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr* nlh, void
__be32 src = 0;
u32 iif;
int err;
+ int mark;
struct sk_buff *skb;
err = nlmsg_parse(nlh, sizeof(*rtm), tb, RTA_MAX, rtm_ipv4_policy);
@@ -2960,6 +2964,7 @@ static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr* nlh, void
src = tb[RTA_SRC] ? nla_get_be32(tb[RTA_SRC]) : 0;
dst = tb[RTA_DST] ? nla_get_be32(tb[RTA_DST]) : 0;
iif = tb[RTA_IIF] ? nla_get_u32(tb[RTA_IIF]) : 0;
+ mark = tb[RTA_MARK] ? nla_get_u32(tb[RTA_MARK]) : 0;
if (iif) {
struct net_device *dev;
@@ -2972,6 +2977,7 @@ static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr* nlh, void
skb->protocol = htons(ETH_P_IP);
skb->dev = dev;
+ skb->mark = mark;
local_bh_disable();
err = ip_route_input(skb, dst, src, rtm->rtm_tos, dev);
local_bh_enable();
@@ -2989,6 +2995,7 @@ static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr* nlh, void
},
},
.oif = tb[RTA_OIF] ? nla_get_u32(tb[RTA_OIF]) : 0,
+ .mark = mark,
};
err = ip_route_output_key(net, &rt, &fl);
}
^ permalink raw reply related
* CONNMARK getsockopt/setsockopt functionality
From: Daniel Wagner @ 2010-07-21 7:20 UTC (permalink / raw)
To: netdev
Hi,
There used to be a way to mark packets in userspace by invoking the
setsocketopt with SO_CONNMARK as it was proposed by Krisztian [1].
This code was superseeded by Jan's work [2]. Now I wonder how I get
the same result out of the new code. Something with conditions?
thanks,
daniel
[1] http://lists.netfilter.org/pipermail/netfilter-devel/2004-May/015385.html
[2] http://www.kerneltrap.com/mailarchive/git-commits-head/2009/9/14/4291
^ permalink raw reply
* [PATCH] Re: mmotm 2010-07-19 - e1000e vs. pm_qos_update_request issues
From: Florian Mickler @ 2010-07-21 7:12 UTC (permalink / raw)
To: Andrew Morton
Cc: Valdis.Kletnieks, Rafael J. Wysocki, mark gross, e1000-devel,
netdev, linux-kernel, James Bottomley, Thomas Gleixner,
David S. Miller
In-Reply-To: <20100720140751.71ee83a8.akpm@linux-foundation.org>
On Tue, 20 Jul 2010 14:07:51 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:
> On Tue, 20 Jul 2010 16:35:25 -0400
> Valdis.Kletnieks@vt.edu wrote:
>
> > On Mon, 19 Jul 2010 16:38:09 PDT, akpm@linux-foundation.org said:
> > > The mm-of-the-moment snapshot 2010-07-19-16-37 has been uploaded to
> > >
> > > http://userweb.kernel.org/~akpm/mmotm/
> >
> > Throws a warning at boot:
> >
> > [ 1.786060] WARNING: at kernel/pm_qos_params.c:264 pm_qos_update_request+0x28/0x54()
> > [ 1.786088] Hardware name: Latitude E6500
> > [ 1.787045] pm_qos_update_request() called for unknown object
> > [ 1.787966] Modules linked in:
> > [ 1.788940] Pid: 1, comm: swapper Not tainted 2.6.35-rc5-mmotm0719 #1
> > [ 1.790035] Call Trace:
> > [ 1.791121] [<ffffffff81037335>] warn_slowpath_common+0x80/0x98
> > [ 1.792205] [<ffffffff810373e1>] warn_slowpath_fmt+0x41/0x43
> > [ 1.793279] [<ffffffff81057c14>] pm_qos_update_request+0x28/0x54
> > [ 1.794347] [<ffffffff8134889e>] e1000_configure+0x421/0x459
> > [ 1.795393] [<ffffffff8134afbd>] e1000_open+0xbd/0x37c
> > [ 1.796436] [<ffffffff8105743a>] ? raw_notifier_call_chain+0xf/0x11
> > [ 1.797491] [<ffffffff8145f948>] __dev_open+0xae/0xe2
> > [ 1.798547] [<ffffffff8145f997>] dev_open+0x1b/0x49
> > [ 1.799612] [<ffffffff8146e36e>] netpoll_setup+0x84/0x259
> > [ 1.800685] [<ffffffff81b5037c>] init_netconsole+0xbc/0x21f
> > [ 1.801744] [<ffffffff81b5026c>] ? sir_wq_init+0x0/0x35
> > [ 1.802793] [<ffffffff81b502c0>] ? init_netconsole+0x0/0x21f
> > [ 1.803845] [<ffffffff810002ff>] do_one_initcall+0x7a/0x12f
> > [ 1.804885] [<ffffffff81b2ccae>] kernel_init+0x138/0x1c2
> > [ 1.805915] [<ffffffff81003554>] kernel_thread_helper+0x4/0x10
> > [ 1.806937] [<ffffffff81590e00>] ? restore_args+0x0/0x30
> > [ 1.807955] [<ffffffff81b2cb76>] ? kernel_init+0x0/0x1c2
> > [ 1.808958] [<ffffffff81003550>] ? kernel_thread_helper+0x0/0x10
> > [ 1.809958] ---[ end trace 84b562a00a60539e ]---
> >
> > Looks like a repeat of something I reported against -mmotm 2010-05-11, though a
> > WARNING rather than an outright crash - the traceback is pretty much identical.
> > I have *no* idea why -rc3-mmotm0701 doesn't whinge similarly.
> >
>
> I don't recall you reporting that, sorry.
>
> The warning was added by
>
> : commit 82f682514a5df89ffb3890627eebf0897b7a84ec
> : Author: James Bottomley <James.Bottomley@suse.de>
> : AuthorDate: Mon Jul 5 22:53:06 2010 +0200
> : Commit: Rafael J. Wysocki <rjw@sisk.pl>
> : CommitDate: Mon Jul 19 02:00:34 2010 +0200
> :
> : pm_qos: Get rid of the allocation in pm_qos_add_request()
>
>
> It's a pretty crappy warning too. Neither the warning nor the code
> comments provide developers with any hint as to what they have done
> wrong, nor what they must do to fix things. And the patch changelog
> doesn't mention the new warnings *at all*.
>
> So one must assume that the people who stuck this thing in the tree
> have volunteered to fix e1000e. Let's cc 'em.
>
e1000 calls update_request before registering said request with pm_qos.
This was silently ignored before but now emits a warning. The warning
is sound, because it means, that the constraint-request didn't take
effect.
The right thing is probably to register the request before
calling update_request.
Attached patch moves the registering from e1000_up to e1000_open and
the unregistering from e1000_down to e1000_close.
It is only compile-tested as I don't have the hardware.
Cheers,
Flo
p.s.: sorry if this get's mangled or is wrongly formatted, i'm just using
the "insert file" option of my mailclient and crossing my fingers...
>From 693c71b911ff0845c872261d5704a1d40960722d Mon Sep 17 00:00:00 2001
From: Florian Mickler <florian@mickler.org>
Date: Wed, 21 Jul 2010 08:44:21 +0200
Subject: [PATCH] e1000e: register pm_qos request on hardware activation
The pm_qos_add_request call has to register the pm_qos request with the pm_qos
susbsystem before first use of the pm_qos request via
pm_qos_update_request.
As pm_qos changed to use plists there is no benefit in registering and
unregistering the pm_qos request on ifup/ifdown and thus we move the
registering into e1000_open and the unregistering in e1000_close.
This fixes the following warning:
[ 1.786060] WARNING: at kernel/pm_qos_params.c:264
pm_qos_update_request+0x28/0x54()
[ 1.786088] Hardware name: Latitude E6500
[ 1.787045] pm_qos_update_request() called for unknown object
[ 1.787966] Modules linked in:
[ 1.788940] Pid: 1, comm: swapper Not tainted 2.6.35-rc5-mmotm0719 #1
[ 1.790035] Call Trace:
[ 1.791121] [<ffffffff81037335>] warn_slowpath_common+0x80/0x98
[ 1.792205] [<ffffffff810373e1>] warn_slowpath_fmt+0x41/0x43
[ 1.793279] [<ffffffff81057c14>] pm_qos_update_request+0x28/0x54
[ 1.794347] [<ffffffff8134889e>] e1000_configure+0x421/0x459
[ 1.795393] [<ffffffff8134afbd>] e1000_open+0xbd/0x37c
[ 1.796436] [<ffffffff8105743a>] ? raw_notifier_call_chain+0xf/0x11
[ 1.797491] [<ffffffff8145f948>] __dev_open+0xae/0xe2
[ 1.798547] [<ffffffff8145f997>] dev_open+0x1b/0x49
[ 1.799612] [<ffffffff8146e36e>] netpoll_setup+0x84/0x259
[ 1.800685] [<ffffffff81b5037c>] init_netconsole+0xbc/0x21f
[ 1.801744] [<ffffffff81b5026c>] ? sir_wq_init+0x0/0x35
[ 1.802793] [<ffffffff81b502c0>] ? init_netconsole+0x0/0x21f
[ 1.803845] [<ffffffff810002ff>] do_one_initcall+0x7a/0x12f
[ 1.804885] [<ffffffff81b2ccae>] kernel_init+0x138/0x1c2
[ 1.805915] [<ffffffff81003554>] kernel_thread_helper+0x4/0x10
[ 1.806937] [<ffffffff81590e00>] ? restore_args+0x0/0x30
[ 1.807955] [<ffffffff81b2cb76>] ? kernel_init+0x0/0x1c2
[ 1.808958] [<ffffffff81003550>] ? kernel_thread_helper+0x0/0x10
[ 1.809958] ---[ end trace 84b562a00a60539e ]---
Signed-off-by: Florian Mickler <florian@mickler.org>
---
drivers/net/e1000e/netdev.c | 18 +++++++++---------
1 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 8ba366a..1bd9054 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -3218,12 +3218,6 @@ int e1000e_up(struct e1000_adapter *adapter)
{
struct e1000_hw *hw = &adapter->hw;
- /* DMA latency requirement to workaround early-receive/jumbo issue */
- if (adapter->flags & FLAG_HAS_ERT)
- pm_qos_add_request(&adapter->netdev->pm_qos_req,
- PM_QOS_CPU_DMA_LATENCY,
- PM_QOS_DEFAULT_VALUE);
-
/* hardware has been reset, we need to reload some things */
e1000_configure(adapter);
@@ -3287,9 +3281,6 @@ void e1000e_down(struct e1000_adapter *adapter)
e1000_clean_tx_ring(adapter);
e1000_clean_rx_ring(adapter);
- if (adapter->flags & FLAG_HAS_ERT)
- pm_qos_remove_request(&adapter->netdev->pm_qos_req);
-
/*
* TODO: for power management, we could drop the link and
* pci_disable_device here.
@@ -3524,6 +3515,12 @@ static int e1000_open(struct net_device *netdev)
E1000_MNG_DHCP_COOKIE_STATUS_VLAN))
e1000_update_mng_vlan(adapter);
+ /* DMA latency requirement to workaround early-receive/jumbo issue */
+ if (adapter->flags & FLAG_HAS_ERT)
+ pm_qos_add_request(&adapter->netdev->pm_qos_req,
+ PM_QOS_CPU_DMA_LATENCY,
+ PM_QOS_DEFAULT_VALUE);
+
/*
* before we allocate an interrupt, we must be ready to handle it.
* Setting DEBUG_SHIRQ in the kernel makes it fire an interrupt
@@ -3628,6 +3625,9 @@ static int e1000_close(struct net_device *netdev)
if (adapter->flags & FLAG_HAS_AMT)
e1000_release_hw_control(adapter);
+ if (adapter->flags & FLAG_HAS_ERT)
+ pm_qos_remove_request(&adapter->netdev->pm_qos_req);
+
pm_runtime_put_sync(&pdev->dev);
return 0;
--
1.7.1.1
^ permalink raw reply related
* Re: [RFC PATCH] dst: check if dst is freed in dst_check()
From: Nicolas Dichtel @ 2010-07-21 7:03 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1279679288.2492.15.camel@edumazet-laptop>
Hi Eric,
I was thinking to use this function in sctp, but I misread xfrm part.
Sorry for the noise.
Regards,
Nicolas
Le 21.07.2010 04:28, Eric Dumazet a écrit :
> Le mardi 20 juillet 2010 à 11:49 +0200, Nicolas Dichtel a écrit :
>> Hi,
>>
>> I probably missed something, but I cannot find where obsolete field is checked
>> when dst_check() is called. If dst->obsolete is > 1, dst cannot be used!
>>
>> Attached is a proposal to fix this issue.
>>
>>
>
>> diff --git a/include/net/dst.h b/include/net/dst.h
>> index 81d1413..7bf4f9a 100644
>> --- a/include/net/dst.h
>> +++ b/include/net/dst.h
>> @@ -319,6 +319,8 @@ static inline int dst_input(struct sk_buff *skb)
>>
>> static inline struct dst_entry *dst_check(struct dst_entry *dst, u32 cookie)
>> {
>> + if (dst->obsolete > 1)
>> + return NULL;
>> if (dst->obsolete)
>> dst = dst->ops->check(dst, cookie);
>> return dst;
>
> I believe this is not needed and redundant.
>
> In what case do you think this matters ?
>
> To my knowledge dst_check() is only used by net/xfrm/xfrm_policy.c
>
> And xfrm_dst_check() does the necessary checks.
>
> static struct dst_entry *xfrm_dst_check(struct dst_entry *dst, u32 cookie)
> {
> /* Code (such as __xfrm4_bundle_create()) sets dst->obsolete
> * to "-1" to force all XFRM destinations to get validated by
> * dst_ops->check on every use. We do this because when a
> * normal route referenced by an XFRM dst is obsoleted we do
> * not go looking around for all parent referencing XFRM dsts
> * so that we can invalidate them. It is just too much work.
> * Instead we make the checks here on every use. For example:
> *
> * XFRM dst A --> IPv4 dst X
> *
> * X is the "xdst->route" of A (X is also the "dst->path" of A
> * in this example). If X is marked obsolete, "A" will not
> * notice. That's what we are validating here via the
> * stale_bundle() check.
> *
> * When a policy's bundle is pruned, we dst_free() the XFRM
> * dst which causes it's ->obsolete field to be set to a
> * positive non-zero integer. If an XFRM dst has been pruned
> * like this, we want to force a new route lookup.
> */
> if (dst->obsolete < 0 && !stale_bundle(dst))
> return dst;
>
> return NULL;
> }
^ permalink raw reply
* Re: [RFC PATCH v3 4/5] skb: add tracepoints to freeing skb
From: Koki Sanagi @ 2010-07-21 7:02 UTC (permalink / raw)
To: Neil Horman
Cc: netdev, linux-kernel, davem, kaneshige.kenji, izumi.taku,
kosaki.motohiro, laijs, scott.a.mcmillan, rostedt, eric.dumazet,
fweisbec, mathieu.desnoyers
In-Reply-To: <20100720115044.GD1995@hmsreliant.think-freely.org>
(2010/07/20 20:50), Neil Horman wrote:
> On Tue, Jul 20, 2010 at 09:49:10AM +0900, Koki Sanagi wrote:
>> [RFC PATCH v3 4/5] skb: add tracepoints to freeing skb
>> This patch adds tracepoint to consume_skb, dev_kfree_skb_irq and
>> skb_free_datagram_locked. Combinating with tracepoint on dev_hard_start_xmit,
>> we can check how long it takes to free transmited packets. And using it, we can
>> calculate how many packets driver had at that time. It is useful when a drop of
>> transmited packet is a problem.
>>
>> <idle>-0 [001] 241409.218333: consume_skb: skbaddr=dd6b2fb8
>> <idle>-0 [001] 241409.490555: dev_kfree_skb_irq: skbaddr=f5e29840
>>
>> udp-recv-302 [001] 515031.206008: skb_free_datagram_locked: skbaddr=f5b1d900
>>
>>
>> Signed-off-by: Koki Sanagi <sanagi.koki@jp.fujitsu.com>
>> ---
>> include/trace/events/skb.h | 42 ++++++++++++++++++++++++++++++++++++++++++
>> net/core/datagram.c | 1 +
>> net/core/dev.c | 2 ++
>> net/core/skbuff.c | 1 +
>> 4 files changed, 46 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
>> index 4b2be6d..84c9041 100644
>> --- a/include/trace/events/skb.h
>> +++ b/include/trace/events/skb.h
>> @@ -35,6 +35,48 @@ TRACE_EVENT(kfree_skb,
>> __entry->skbaddr, __entry->protocol, __entry->location)
>> );
>>
>> +DECLARE_EVENT_CLASS(free_skb,
>> +
>> + TP_PROTO(struct sk_buff *skb),
>> +
>> + TP_ARGS(skb),
>> +
>> + TP_STRUCT__entry(
>> + __field( void *, skbaddr )
>> + ),
>> +
>> + TP_fast_assign(
>> + __entry->skbaddr = skb;
>> + ),
>> +
>> + TP_printk("skbaddr=%p", __entry->skbaddr)
>> +
>> +);
>> +
>> +DEFINE_EVENT(free_skb, consume_skb,
>> +
>> + TP_PROTO(struct sk_buff *skb),
>> +
>> + TP_ARGS(skb)
>> +
>> +);
>> +
>> +DEFINE_EVENT(free_skb, dev_kfree_skb_irq,
>> +
>> + TP_PROTO(struct sk_buff *skb),
>> +
>> + TP_ARGS(skb)
>> +
>> +);
>> +
>> +DEFINE_EVENT(free_skb, skb_free_datagram_locked,
>> +
>> + TP_PROTO(struct sk_buff *skb),
>> +
>> + TP_ARGS(skb)
>> +
>> +);
>> +
>
> Why create these last two tracepoints at all? dev_kfree_skb_irq will eventually
> pass through kfree_skb anyway, getting picked up by the tracepoint there, the
> while the latter won't (since it uses __kfree_skb instead), I think that could
> be fixed up by add a call to trace_kfree_skb there directly, saving you two
> tracepoints.
>
> Neil
>
I think dev_kfree_skb_irq isn't chased by trace_kfree_skb or trace_consume_skb
completely. Because net_tx_action frees skb by __kfree_skb. So it is better to
add trace_kfree_skb before it. skb_free_datagram_locked is same.
Thanks,
Koki Sanagi.
>>
>
>
^ permalink raw reply
* Re: [RFC PATCH v3 3/5] netdev: add tracepoints to netdev layer
From: Koki Sanagi @ 2010-07-21 7:01 UTC (permalink / raw)
To: Neil Horman
Cc: netdev, linux-kernel, davem, kaneshige.kenji, izumi.taku,
kosaki.motohiro, laijs, scott.a.mcmillan, rostedt, eric.dumazet,
fweisbec, mathieu.desnoyers
In-Reply-To: <20100720114152.GC1995@hmsreliant.think-freely.org>
(2010/07/20 20:41), Neil Horman wrote:
> On Tue, Jul 20, 2010 at 09:47:59AM +0900, Koki Sanagi wrote:
>> This patch adds tracepoint to dev_queue_xmit, dev_hard_start_xmit and
>> netif_receive_skb. These tracepoints help you to monitor network driver's
>> input/output.
>>
>> sshd-4445 [001] 241367.066046: net_dev_queue: dev=eth3 skbaddr=dd6b2538 len=114
>> sshd-4445 [001] 241367.066047: net_dev_xmit: dev=eth3 skbaddr=dd6b2538 len=114 rc=0
>> <idle>-0 [001] 241367.067472: net_dev_receive: dev=eth3 skbaddr=f5e59000 len=52
>>
>> Signed-off-by: Koki Sanagi <sanagi.koki@jp.fujitsu.com>
>> ---
>> include/trace/events/net.h | 75 ++++++++++++++++++++++++++++++++++++++++++++
>> net/core/dev.c | 5 +++
>> net/core/net-traces.c | 1 +
>> 3 files changed, 81 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/trace/events/net.h b/include/trace/events/net.h
>> new file mode 100644
>> index 0000000..8a21361
>> --- /dev/null
>> +++ b/include/trace/events/net.h
>> @@ -0,0 +1,75 @@
>> +#undef TRACE_SYSTEM
>> +#define TRACE_SYSTEM net
>> +
>> +#if !defined(_TRACE_NET_H) || defined(TRACE_HEADER_MULTI_READ)
>> +#define _TRACE_NET_H
>> +
>> +#include <linux/skbuff.h>
>> +#include <linux/netdevice.h>
>> +#include <linux/ip.h>
>> +#include <linux/tracepoint.h>
>> +
>> +TRACE_EVENT(net_dev_xmit,
>> +
>> + TP_PROTO(struct sk_buff *skb,
>> + int rc),
>> +
>> + TP_ARGS(skb, rc),
>> +
>> + TP_STRUCT__entry(
>> + __field( void *, skbaddr )
>> + __field( unsigned int, len )
>> + __field( int, rc )
>> + __string( name, skb->dev->name )
>> + ),
>> +
>> + TP_fast_assign(
>> + __entry->skbaddr = skb;
>> + __entry->len = skb->len;
>> + __entry->rc = rc;
>> + __assign_str(name, skb->dev->name);
>> + ),
>> +
>> + TP_printk("dev=%s skbaddr=%p len=%u rc=%d",
>> + __get_str(name), __entry->skbaddr, __entry->len, __entry->rc)
>> +);
>> +
>> +DECLARE_EVENT_CLASS(net_dev_template,
>> +
>> + TP_PROTO(struct sk_buff *skb),
>> +
>> + TP_ARGS(skb),
>> +
>> + TP_STRUCT__entry(
>> + __field( void *, skbaddr )
>> + __field( unsigned int, len )
>> + __string( name, skb->dev->name )
>> + ),
>> +
>> + TP_fast_assign(
>> + __entry->skbaddr = skb;
>> + __entry->len = skb->len;
>> + __assign_str(name, skb->dev->name);
>> + ),
>> +
>> + TP_printk("dev=%s skbaddr=%p len=%u",
>> + __get_str(name), __entry->skbaddr, __entry->len)
>> +)
>> +
>> +DEFINE_EVENT(net_dev_template, net_dev_queue,
>> +
>> + TP_PROTO(struct sk_buff *skb),
>> +
>> + TP_ARGS(skb)
>> +);
>> +
>> +DEFINE_EVENT(net_dev_template, net_dev_receive,
>> +
>> + TP_PROTO(struct sk_buff *skb),
>> +
>> + TP_ARGS(skb)
>> +);
>> +#endif /* _TRACE_NET_H */
>> +
>> +/* This part must be outside protection */
>> +#include <trace/define_trace.h>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 93b8929..4acfec6 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -130,6 +130,7 @@
>> #include <linux/jhash.h>
>> #include <linux/random.h>
>> #include <trace/events/napi.h>
>> +#include <trace/events/net.h>
>> #include <linux/pci.h>
>>
>> #include "net-sysfs.h"
>> @@ -1955,6 +1956,7 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
>> }
>>
>> rc = ops->ndo_start_xmit(skb, dev);
>> + trace_net_dev_xmit(skb, rc);
>> if (rc == NETDEV_TX_OK)
>> txq_trans_update(txq);
>> return rc;
>> @@ -1975,6 +1977,7 @@ gso:
>> skb_dst_drop(nskb);
>>
>> rc = ops->ndo_start_xmit(nskb, dev);
>> + trace_net_dev_xmit(nskb, rc);
>> if (unlikely(rc != NETDEV_TX_OK)) {
>> if (rc & ~NETDEV_TX_MASK)
>> goto out_kfree_gso_skb;
>> @@ -2165,6 +2168,7 @@ int dev_queue_xmit(struct sk_buff *skb)
>> #ifdef CONFIG_NET_CLS_ACT
>> skb->tc_verd = SET_TC_AT(skb->tc_verd, AT_EGRESS);
>> #endif
>> + trace_net_dev_queue(skb);
>> if (q->enqueue) {
>> rc = __dev_xmit_skb(skb, q, dev, txq);
>> goto out;
>> @@ -2939,6 +2943,7 @@ int netif_receive_skb(struct sk_buff *skb)
>> if (netdev_tstamp_prequeue)
>> net_timestamp_check(skb);
>>
>> + trace_net_dev_receive(skb);
>
> I imagine for completeness you'll want to make a call to this in netif_rx and
> netif_rx_ni as well.
>
It might be better to add tracepoint to netif_rx to determine a time between
netif_rx and netif_receive_skb.
Thanks,
Koki Sanagi.
>
>
>
^ permalink raw reply
* Re: [RFC PATCH v3 2/5] napi: convert trace_napi_poll to TRACE_EVENT
From: Koki Sanagi @ 2010-07-21 7:00 UTC (permalink / raw)
To: Neil Horman
Cc: netdev, linux-kernel, davem, kaneshige.kenji, izumi.taku,
kosaki.motohiro, laijs, scott.a.mcmillan, rostedt, eric.dumazet,
fweisbec, mathieu.desnoyers
In-Reply-To: <20100720110952.GB1995@hmsreliant.think-freely.org>
(2010/07/20 20:09), Neil Horman wrote:
> On Tue, Jul 20, 2010 at 09:46:51AM +0900, Koki Sanagi wrote:
>> From: Neil Horman <nhorman@tuxdriver.com>
>>
>> This patch converts trace_napi_poll from DECLARE_EVENT to TRACE_EVENT to improve
>> the usability of napi_poll tracepoint.
>>
>> <idle>-0 [001] 241302.750777: napi_poll: napi poll on napi struct f6acc480 for device eth3
>> <idle>-0 [000] 241302.852389: napi_poll: napi poll on napi struct f5d0d70c for device eth1
>>
>> An original patch is below.
>> http://marc.info/?l=linux-kernel&m=126021713809450&w=2
>> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
>>
>> And add a fix by Steven Rostedt.
>> http://marc.info/?l=linux-kernel&m=126150506519173&w=2
>>
>> Signed-off-by: Koki Sanagi <sanagi.koki@jp.fujitsu.com>
>> ---
>> include/trace/events/napi.h | 25 +++++++++++++++++++++++--
>> 1 files changed, 23 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/trace/events/napi.h b/include/trace/events/napi.h
>> index 188deca..8fe1e93 100644
>> --- a/include/trace/events/napi.h
>> +++ b/include/trace/events/napi.h
>> @@ -6,10 +6,31 @@
>>
>> #include <linux/netdevice.h>
>> #include <linux/tracepoint.h>
>> +#include <linux/ftrace.h>
>> +
>> +#define NO_DEV "(no_device)"
>> +
>> +TRACE_EVENT(napi_poll,
>>
>> -DECLARE_TRACE(napi_poll,
>> TP_PROTO(struct napi_struct *napi),
>> - TP_ARGS(napi));
>> +
>> + TP_ARGS(napi),
>> +
>> + TP_STRUCT__entry(
>> + __field( struct napi_struct *, napi)
>> + __string( dev_name, napi->dev ? napi->dev->name : NO_DEV)
>> + ),
>> +
>> + TP_fast_assign(
>> + __entry->napi = napi;
>> + __assign_str(dev_name, napi->dev ? napi->dev->name : NO_DEV);
>> + ),
>> +
>> + TP_printk("napi poll on napi struct %p for device %s",
>> + __entry->napi, __get_str(dev_name))
>> +);
>> +
>> +#undef NO_DEV
>>
>> #endif /* _TRACE_NAPI_H_ */
>>
>>
> NAK, This change will create a build break in the drop monitor code. You'll
> need to fix that up if you want to make this change.
> Neil
>
I built a kernel with CONFIG_NET_DROP_MONITOR=y.
But build break did not occur.
Thanks,
Koki Sanagi.
> --
> 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
>
>
^ permalink raw reply
* Re: [RFC PATCH v3 1/5] irq: add tracepoint to softirq_raise
From: Koki Sanagi @ 2010-07-21 6:57 UTC (permalink / raw)
To: Neil Horman
Cc: netdev, linux-kernel, davem, kaneshige.kenji, izumi.taku,
kosaki.motohiro, laijs, scott.a.mcmillan, rostedt, eric.dumazet,
fweisbec, mathieu.desnoyers
In-Reply-To: <20100720110439.GA1995@hmsreliant.think-freely.org>
(2010/07/20 20:04), Neil Horman wrote:
> On Tue, Jul 20, 2010 at 09:45:31AM +0900, Koki Sanagi wrote:
>> From: Lai Jiangshan <laijs@cn.fujitsu.com>
>>
>> Add a tracepoint for tracing when softirq action is raised.
>>
>> It and the existed tracepoints complete softirq's tracepoints:
>> softirq_raise, softirq_entry and softirq_exit.
>>
>> And when this tracepoint is used in combination with
>> the softirq_entry tracepoint we can determine
>> the softirq raise latency.
>>
>> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
>> Acked-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
>> Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
>>
>> [ factorize softirq events with DECLARE_EVENT_CLASS ]
>> Signed-off-by: Koki Sanagi <sanagi.koki@jp.fujitsu.com>
>> ---
>> include/linux/interrupt.h | 8 +++++-
>> include/trace/events/irq.h | 57 ++++++++++++++++++++++++++-----------------
>> kernel/softirq.c | 4 +-
>> 3 files changed, 43 insertions(+), 26 deletions(-)
>>
>> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
>> index c233113..1cb5726 100644
>> --- a/include/linux/interrupt.h
>> +++ b/include/linux/interrupt.h
>> @@ -18,6 +18,7 @@
>> #include <asm/atomic.h>
>> #include <asm/ptrace.h>
>> #include <asm/system.h>
>> +#include <trace/events/irq.h>
>>
>> /*
>> * These correspond to the IORESOURCE_IRQ_* defines in
>> @@ -402,7 +403,12 @@ asmlinkage void do_softirq(void);
>> asmlinkage void __do_softirq(void);
>> extern void open_softirq(int nr, void (*action)(struct softirq_action *));
>> extern void softirq_init(void);
>> -#define __raise_softirq_irqoff(nr) do { or_softirq_pending(1UL << (nr)); } while (0)
>> +static inline void __raise_softirq_irqoff(unsigned int nr)
>> +{
>> + trace_softirq_raise(nr);
>> + or_softirq_pending(1UL << nr);
>> +}
>> +
> We already have tracepoints in irq_enter and irq_exit. If the goal here is to
> detect latency during packet processing, cant the delta in time between those
> two points be used to determine interrupt handling latency?
Certainly, the time between irq_entry and irq_exit is not directly related to
latency during packet processing. But it's indirectly related it.
Because softirq_entry isn't passed until irq exits and softirq_entry time is
related to packet processing latency. So I show it as a reference.
>
>
>> extern void raise_softirq_irqoff(unsigned int nr);
>> extern void raise_softirq(unsigned int nr);
>> extern void wakeup_softirqd(void);
>> diff --git a/include/trace/events/irq.h b/include/trace/events/irq.h
>> index 0e4cfb6..717744c 100644
>> --- a/include/trace/events/irq.h
>> +++ b/include/trace/events/irq.h
>> @@ -5,7 +5,9 @@
>> #define _TRACE_IRQ_H
>>
>> #include <linux/tracepoint.h>
>> -#include <linux/interrupt.h>
>> +
>> +struct irqaction;
>> +struct softirq_action;
>>
>> #define softirq_name(sirq) { sirq##_SOFTIRQ, #sirq }
>> #define show_softirq_name(val) \
>> @@ -84,56 +86,65 @@ TRACE_EVENT(irq_handler_exit,
>>
>> DECLARE_EVENT_CLASS(softirq,
>>
>> - TP_PROTO(struct softirq_action *h, struct softirq_action *vec),
>> + TP_PROTO(unsigned int nr),
>>
>> - TP_ARGS(h, vec),
>> + TP_ARGS(nr),
>>
>> TP_STRUCT__entry(
>> - __field( int, vec )
>> + __field( unsigned int, vec )
>> ),
>>
>> TP_fast_assign(
>> - __entry->vec = (int)(h - vec);
>> + __entry->vec = nr;
>> ),
>>
>> TP_printk("vec=%d [action=%s]", __entry->vec,
>> - show_softirq_name(__entry->vec))
>> + show_softirq_name(__entry->vec))
>> +);
>> +
>> +/**
>> + * softirq_raise - called immediately when a softirq is raised
>> + * @nr: softirq vector number
>> + *
>> + * Tracepoint for tracing when softirq action is raised.
>> + * Also, when used in combination with the softirq_entry tracepoint
>> + * we can determine the softirq raise latency.
>> + */
>> +DEFINE_EVENT(softirq, softirq_raise,
>> +
>> + TP_PROTO(unsigned int nr),
>> +
>> + TP_ARGS(nr)
>> );
>>
>> /**
>> * softirq_entry - called immediately before the softirq handler
>> - * @h: pointer to struct softirq_action
>> - * @vec: pointer to first struct softirq_action in softirq_vec array
>> + * @nr: softirq vector number
>> *
>> - * The @h parameter, contains a pointer to the struct softirq_action
>> - * which has a pointer to the action handler that is called. By subtracting
>> - * the @vec pointer from the @h pointer, we can determine the softirq
>> - * number. Also, when used in combination with the softirq_exit tracepoint
>> + * Tracepoint for tracing when softirq action starts.
>> + * Also, when used in combination with the softirq_exit tracepoint
>> * we can determine the softirq latency.
>> */
>> DEFINE_EVENT(softirq, softirq_entry,
>>
>> - TP_PROTO(struct softirq_action *h, struct softirq_action *vec),
>> + TP_PROTO(unsigned int nr),
>>
>> - TP_ARGS(h, vec)
>> + TP_ARGS(nr)
>> );
>>
>> /**
>> * softirq_exit - called immediately after the softirq handler returns
>> - * @h: pointer to struct softirq_action
>> - * @vec: pointer to first struct softirq_action in softirq_vec array
>> + * @nr: softirq vector number
>> *
>> - * The @h parameter contains a pointer to the struct softirq_action
>> - * that has handled the softirq. By subtracting the @vec pointer from
>> - * the @h pointer, we can determine the softirq number. Also, when used in
>> - * combination with the softirq_entry tracepoint we can determine the softirq
>> - * latency.
>> + * Tracepoint for tracing when softirq action ends.
>> + * Also, when used in combination with the softirq_entry tracepoint
>> + * we can determine the softirq latency.
>> */
>> DEFINE_EVENT(softirq, softirq_exit,
>>
>> - TP_PROTO(struct softirq_action *h, struct softirq_action *vec),
>> + TP_PROTO(unsigned int nr),
>>
>> - TP_ARGS(h, vec)
>> + TP_ARGS(nr)
>> );
>>
>> #endif /* _TRACE_IRQ_H */
>> diff --git a/kernel/softirq.c b/kernel/softirq.c
>> index 825e112..6790599 100644
>> --- a/kernel/softirq.c
>> +++ b/kernel/softirq.c
>> @@ -215,9 +215,9 @@ restart:
>> int prev_count = preempt_count();
>> kstat_incr_softirqs_this_cpu(h - softirq_vec);
>>
>> - trace_softirq_entry(h, softirq_vec);
>> + trace_softirq_entry(h - softirq_vec);
>> h->action(h);
>> - trace_softirq_exit(h, softirq_vec);
>> + trace_softirq_exit(h - softirq_vec);
>
> You're loosing information here by reducing the numbers of parameters in this
> tracepoint. How many other tracepoint scripts rely on having both pointers
> handy? Why not just do the pointer math inside your tracehook instead?
In __raise_softirq_irqoff macro there is no method to refer softirq_vec, so it
can't use softirq DECLARE_EVENT_CLASS as is.
Currently, there is no script using softirq_entry or softirq_exit.
Thanks,
Koki Sanagi.
>
>> if (unlikely(prev_count != preempt_count())) {
>> printk(KERN_ERR "huh, entered softirq %td %s %p"
>> "with preempt_count %08x,"
>>
>> --
>> 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
>>
>
>
^ permalink raw reply
* Re: [PATCH net-next] sysfs: add entry to indicate network interfaces with random MAC address
From: Harald Hoyer @ 2010-07-21 6:47 UTC (permalink / raw)
To: David Miller
Cc: shemminger, bhutchings, sassmann, netdev, linux-kernel, gospo,
gregory.v.rose, alexander.h.duyck, leedom
In-Reply-To: <20100720.233457.267367495.davem@davemloft.net>
On 07/21/2010 08:34 AM, David Miller wrote:
> From: Harald Hoyer<harald@redhat.com>
> Date: Wed, 21 Jul 2010 08:26:27 +0200
>
>> On 07/20/2010 11:20 PM, David Miller wrote:
>>> From: Stephen Hemminger<shemminger@vyatta.com>
>>> Date: Tue, 20 Jul 2010 14:18:16 -0700
>>>
>>>> No one mentioned that the first octet of an Ethernet address already
>>>> indicates "software generated" Ethernet address. Per the standard,
>>>> if bit 1 is set it means address is locally assigned.
>>>>
>>>> static inline bool is_locally_assigned_ether(const u8 *addr)
>>>> {
>>>> return (addr[0]& 0x2) != 0;
>>>> }
>>>
>>> W00t!
>>>
>>> Indeed, can udev just use that? :-)
>>
>> It already does:
>> see /lib/udev/rules.d/75-persistent-net-generator.rules
>
> So... why doesn't this work?
It works.. but the information, that the MAC is randomly generated would be
valuable. So, for the non-random locally assigned MAC (with bit 1), we could
easily make persistent rules based on the MAC, instead of completely ignoring
them, like we do currently.
^ permalink raw reply
* Re: [RFC PATCH] dst: check if dst is freed in dst_check()
From: David Miller @ 2010-07-21 6:41 UTC (permalink / raw)
To: eric.dumazet; +Cc: nicolas.dichtel, netdev
In-Reply-To: <1279679288.2492.15.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 21 Jul 2010 04:28:08 +0200
> Le mardi 20 juillet 2010 à 11:49 +0200, Nicolas Dichtel a écrit :
>> diff --git a/include/net/dst.h b/include/net/dst.h
>> index 81d1413..7bf4f9a 100644
>> --- a/include/net/dst.h
>> +++ b/include/net/dst.h
>> @@ -319,6 +319,8 @@ static inline int dst_input(struct sk_buff *skb)
>>
>> static inline struct dst_entry *dst_check(struct dst_entry *dst, u32 cookie)
>> {
>> + if (dst->obsolete > 1)
>> + return NULL;
>> if (dst->obsolete)
>> dst = dst->ops->check(dst, cookie);
>> return dst;
>
> I believe this is not needed and redundant.
>
> In what case do you think this matters ?
>
> To my knowledge dst_check() is only used by net/xfrm/xfrm_policy.c
>
> And xfrm_dst_check() does the necessary checks.
Right, last time I was snooping around in here I came to the
same conclusion. In fact I think I'm the author of that
enormous comment in xfrm_dst_check(). :-)
^ permalink raw reply
* Re: [patch v2.6 4/4] libxt_ipvs: user-space lib for netfilter matcher xt_ipvs
From: Jan Engelhardt @ 2010-07-21 6:35 UTC (permalink / raw)
To: Simon Horman
Cc: lvs-devel, netdev, linux-kernel, netfilter, netfilter-devel,
Malcolm Turnbull, Wensong Zhang, Julius Volz, Patrick McHardy,
David S. Miller, Hannes Eder
In-Reply-To: <20100721012146.GC22966@verge.net.au>
On Wednesday 2010-07-21 03:21, Simon Horman wrote:
>> +
>> +#define XT_IPVS_IPVS_PROPERTY (1 << 0) /* all other options imply this one */
>> +#define XT_IPVS_PROTO (1 << 1)
>> +#define XT_IPVS_VADDR (1 << 2)
>> +#define XT_IPVS_VPORT (1 << 3)
>> +#define XT_IPVS_DIR (1 << 4)
>> +#define XT_IPVS_METHOD (1 << 5)
>> +#define XT_IPVS_VPORTCTL (1 << 6)
>> +#define XT_IPVS_MASK ((1 << 7) - 1)
>> +#define XT_IPVS_ONCE_MASK (XT_IPVS_MASK & ~XT_IPVS_IPVS_PROPERTY)
Can't these just be an enum?
^ permalink raw reply
* Re: [PATCH net-next] sysfs: add entry to indicate network interfaces with random MAC address
From: David Miller @ 2010-07-21 6:34 UTC (permalink / raw)
To: harald
Cc: shemminger, bhutchings, sassmann, netdev, linux-kernel, gospo,
gregory.v.rose, alexander.h.duyck, leedom
In-Reply-To: <4C469313.6010807@redhat.com>
From: Harald Hoyer <harald@redhat.com>
Date: Wed, 21 Jul 2010 08:26:27 +0200
> On 07/20/2010 11:20 PM, David Miller wrote:
>> From: Stephen Hemminger<shemminger@vyatta.com>
>> Date: Tue, 20 Jul 2010 14:18:16 -0700
>>
>>> No one mentioned that the first octet of an Ethernet address already
>>> indicates "software generated" Ethernet address. Per the standard,
>>> if bit 1 is set it means address is locally assigned.
>>>
>>> static inline bool is_locally_assigned_ether(const u8 *addr)
>>> {
>>> return (addr[0]& 0x2) != 0;
>>> }
>>
>> W00t!
>>
>> Indeed, can udev just use that? :-)
>
> It already does:
> see /lib/udev/rules.d/75-persistent-net-generator.rules
So... why doesn't this work?
^ permalink raw reply
* Re: [PATCH net-next] sysfs: add entry to indicate network interfaces with random MAC address
From: Harald Hoyer @ 2010-07-21 6:26 UTC (permalink / raw)
To: David Miller
Cc: shemminger, bhutchings, sassmann, netdev, linux-kernel, gospo,
gregory.v.rose, alexander.h.duyck, leedom
In-Reply-To: <20100720.142045.32697196.davem@davemloft.net>
On 07/20/2010 11:20 PM, David Miller wrote:
> From: Stephen Hemminger<shemminger@vyatta.com>
> Date: Tue, 20 Jul 2010 14:18:16 -0700
>
>> No one mentioned that the first octet of an Ethernet address already
>> indicates "software generated" Ethernet address. Per the standard,
>> if bit 1 is set it means address is locally assigned.
>>
>> static inline bool is_locally_assigned_ether(const u8 *addr)
>> {
>> return (addr[0]& 0x2) != 0;
>> }
>
> W00t!
>
> Indeed, can udev just use that? :-)
It already does:
see /lib/udev/rules.d/75-persistent-net-generator.rules
^ permalink raw reply
* Re: [PATCH -next] net: NET_DSA depends on NET_ETHERNET
From: Randy Dunlap @ 2010-07-21 5:41 UTC (permalink / raw)
To: David Miller; +Cc: sfr, netdev, linux-next, linux-kernel, buytenh
In-Reply-To: <20100720.174530.139530021.davem@davemloft.net>
On 07/20/10 17:45, David Miller wrote:
> From: Randy Dunlap <randy.dunlap@oracle.com>
> Date: Tue, 20 Jul 2010 16:03:32 -0700
>
>> From: Randy Dunlap <randy.dunlap@oracle.com>
>>
>> NET_DSA code selects and uses PHYLIB code, but PHYLIB depends on
>> NET_ETHERNET. However, "select" does not follow kconfig dependencies,
>> so explicitly list that requirement here instead.
>>
>> Fixes this kconfig warning:
>>
>> warning: (NET_DSA && NET && EXPERIMENTAL && !S390 ...) selects PHYLIB which has unmet direct dependencies (!S390 && NET_ETHERNET)
>>
>> Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
>
> Randy, this has been fixed in net-2.6 for some time now.
OK, I did see the commit get merged today.
> And I'm pretty sure I sent a copy of this to you when I
> checked it in :-)
I missed it somehow. Thanks.
> --------------------
> From 336a283b9cbe47748ccd68fd8c5158f67cee644b Mon Sep 17 00:00:00 2001
> From: David S. Miller <davem@davemloft.net>
> Date: Mon, 12 Jul 2010 20:03:42 -0700
> Subject: [PATCH 09/24] dsa: Fix Kconfig dependencies.
>
> Based upon a report by Randy Dunlap.
>
> DSA needs PHYLIB, but PHYLIB needs NET_ETHERNET. So, in order
> to select PHYLIB we have to make DSA depend upon NET_ETHERNET.
>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
> net/dsa/Kconfig | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
> index c51b554..1120178 100644
> --- a/net/dsa/Kconfig
> +++ b/net/dsa/Kconfig
> @@ -1,7 +1,7 @@
> menuconfig NET_DSA
> bool "Distributed Switch Architecture support"
> default n
> - depends on EXPERIMENTAL && !S390
> + depends on EXPERIMENTAL && NET_ETHERNET && !S390
> select PHYLIB
> ---help---
> This allows you to use hardware switch chips that use
--
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
^ permalink raw reply
* Re: [patch 1/1] drivers/s390/net: use memdup_user
From: Frank Blaschka @ 2010-07-21 5:35 UTC (permalink / raw)
To: akpm; +Cc: netdev, linux-s390
In-Reply-To: <201007202227.o6KMRZ7V021566@imap1.linux-foundation.org>
Hi,
I added this patch to my patch set
Thx
Frank
^ permalink raw reply
* [PATCH 2/2] sysfs: allow creating symlinks from untagged to tagged directories
From: Eric W. Biederman @ 2010-07-21 5:12 UTC (permalink / raw)
To: Greg KH
Cc: Andrew Morton, Greg KH, Rafael J. Wysocki, Maciej W. Rozycki,
Kay Sievers, Johannes Berg, netdev
In-Reply-To: <m1vd894dy5.fsf_-_@fess.ebiederm.org>
Supporting symlinks from untagged to tagged directories is reasonable,
and needed to support CONFIG_SYSFS_DEPRECATED. So don't fail a prior
allowing that case to work.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
fs/sysfs/symlink.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
index 6603833..a7ac78f 100644
--- a/fs/sysfs/symlink.c
+++ b/fs/sysfs/symlink.c
@@ -67,7 +67,8 @@ static int sysfs_do_create_link(struct kobject *kobj, struct kobject *target,
sysfs_addrm_start(&acxt, parent_sd);
/* Symlinks must be between directories with the same ns_type */
- if (ns_type == sysfs_ns_type(sd->s_symlink.target_sd->s_parent)) {
+ if (!ns_type ||
+ (ns_type == sysfs_ns_type(sd->s_symlink.target_sd->s_parent))) {
if (warn)
error = sysfs_add_one(&acxt, sd);
else
--
1.6.5.2.143.g8cc62
^ permalink raw reply related
* [PATCH 1/2] sysfs: sysfs_delete_link handle symlinks from untagged to tagged directories.
From: Eric W. Biederman @ 2010-07-21 5:10 UTC (permalink / raw)
To: Greg KH
Cc: Andrew Morton, Greg KH, Rafael J. Wysocki, Maciej W. Rozycki,
Kay Sievers, Johannes Berg, netdev
In-Reply-To: <m139vd5sms.fsf_-_@fess.ebiederm.org>
This happens for network devices when SYSFS_DEPRECATED is enabled.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
fs/sysfs/symlink.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
index 44bca5f..6603833 100644
--- a/fs/sysfs/symlink.c
+++ b/fs/sysfs/symlink.c
@@ -135,7 +135,7 @@ void sysfs_delete_link(struct kobject *kobj, struct kobject *targ,
{
const void *ns = NULL;
spin_lock(&sysfs_assoc_lock);
- if (targ->sd)
+ if (targ->sd && sysfs_ns_type(kobj->sd))
ns = targ->sd->s_ns;
spin_unlock(&sysfs_assoc_lock);
sysfs_hash_and_remove(kobj->sd, ns, name);
--
1.6.5.2.143.g8cc62
^ permalink raw reply related
* [PATCH 0/2] Support untagged symlinks to tagged directories.
From: Eric W. Biederman @ 2010-07-21 5:08 UTC (permalink / raw)
To: Greg KH
Cc: Andrew Morton, Greg KH, Rafael J. Wysocki, Maciej W. Rozycki,
Kay Sievers, Johannes Berg, netdev
In-Reply-To: <20100720201334.GA11991@suse.de>
Greg KH <gregkh@suse.de> writes:
> On Mon, Jul 19, 2010 at 01:34:51PM -0700, Andrew Morton wrote:
>> On Thu, 8 Jul 2010 16:06:01 -0700
>> Greg KH <greg@kroah.com> wrote:
>>
>> > On Thu, Jul 08, 2010 at 03:28:53PM -0700, Eric W. Biederman wrote:
>> > > Greg KH <greg@kroah.com> writes:
>> > >
>> > > > With this patch, how does the existing code fail as the drivers aren't
>> > > > fixed up?
>> > > >
>> > > > I like this change, just worried it will cause problems if it gets into
>> > > > .35, without your RFC patch. Will it?
>> > >
>>
>> geethanks!
>>
>> On the FC6 test box I have no networking.
>
> Ick.
>
> Eric, any ideas?
Yes. I just found some time to test my fixes and things are looking
good. It really is just two one line fixes.
On the other part of this debug with SYSFS_DEPRECATED enabled it
with mac80211_hwsim drivers works fine no problems. I expect the
bnep driver will also be fine.
What is affecting those two is arguably a bug in the non-deprecated
sysfs mode.
Regardless here are my fixes. I have split this into a patch for
the warning and a patch for sysfs_delete_link. Because at least
the sysfs_delete_link code needs to make into 2.6.35 if we can.
Eric
^ permalink raw reply
* Re: [PATCH] Export SMBIOS provided firmware instance and label to sysfs
From: Greg KH @ 2010-07-21 3:59 UTC (permalink / raw)
To: Narendra K
Cc: netdev, linux-hotplug, linux-pci, matt_domsch, charles_rose,
jordan_hargrave, vijay_nijhawan
In-Reply-To: <20100714121345.GA20411@auslistsprd01.us.dell.com>
On Wed, Jul 14, 2010 at 07:13:45AM -0500, Narendra K wrote:
> @@ -333,6 +358,7 @@ static void __init dmi_decode(const struct dmi_header *dm, void *dummy)
> break;
> case 41: /* Onboard Devices Extended Information */
> dmi_save_extended_devices(dm);
> + break;
Why make this change? It's not relevant to your patch, right?
> +enum smbios_attr_enum {
> + SMBIOS_ATTR_NONE = 0,
> + SMBIOS_ATTR_LABEL_SHOW,
> + SMBIOS_ATTR_INSTANCE_SHOW,
> +};
> +
> +static mode_t
> +find_smbios_instance_string(struct pci_dev *pdev, char *buf, int attribute)
Why isn't 'attribute' an enumerated type like you just defined above?
Extra type-checking is always good, especially as the variable name
'attribute' means something totally different in other parts of this
file.
> +{
> + const struct dmi_device *dmi;
> + struct dmi_dev_onboard *donboard;
> + int bus;
> + int devfn;
> +
> + bus = pdev->bus->number;
> + devfn = pdev->devfn;
> +
> + dmi = NULL;
> + while ((dmi = dmi_find_device(DMI_DEV_TYPE_DEV_ONBOARD,
> + NULL, dmi)) != NULL) {
> + donboard = dmi->device_data;
> + if (donboard && donboard->bus == bus &&
> + donboard->devfn == devfn) {
> + if (buf) {
> + if (attribute == SMBIOS_ATTR_INSTANCE_SHOW)
> + return scnprintf(buf, PAGE_SIZE,
> + "%d\n",
> + donboard->instance);
> + else if (attribute == SMBIOS_ATTR_LABEL_SHOW)
> + return scnprintf(buf, PAGE_SIZE,
> + "%s\n",
> + dmi->name);
> + }
> + return strlen(dmi->name);
> + }
> + }
> + return 0;
> +}
> +
> +static mode_t
> +smbios_instance_string_exist(struct kobject *kobj, struct attribute *attr,
> + int n)
> +{
> + struct device *dev;
> + struct pci_dev *pdev;
> +
> + dev = container_of(kobj, struct device, kobj);
> + pdev = to_pci_dev(dev);
> +
> + return find_smbios_instance_string(pdev, NULL, SMBIOS_ATTR_NONE);
> +}
> +
> +static ssize_t
> +smbioslabel_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct pci_dev *pdev;
> + pdev = to_pci_dev(dev);
> +
> + return find_smbios_instance_string(pdev, buf,
> + SMBIOS_ATTR_LABEL_SHOW);
> +}
> +
> +static ssize_t
> +smbiosinstance_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct pci_dev *pdev;
> + pdev = to_pci_dev(dev);
> +
> + return find_smbios_instance_string(pdev, buf,
> + SMBIOS_ATTR_INSTANCE_SHOW);
> +}
> +
> +static struct device_attribute smbios_attr_label = {
> + .attr = {.name = "label", .mode = 0444, .owner = THIS_MODULE},
> + .show = smbioslabel_show,
> +};
> +
> +static struct device_attribute smbios_attr_instance = {
> + .attr = {.name = "index", .mode = 0444, .owner = THIS_MODULE},
> + .show = smbiosinstance_show,
> +};
> +
> +static struct attribute *smbios_attributes[] = {
> + &smbios_attr_label.attr,
> + &smbios_attr_instance.attr,
> + NULL,
> +};
> +
> +static struct attribute_group smbios_attr_group = {
> + .attrs = smbios_attributes,
> + .is_visible = smbios_instance_string_exist,
> +};
> +
> +static int
> +pci_create_smbiosname_file(struct pci_dev *pdev)
> +{
> + if (!sysfs_create_group(&pdev->dev.kobj, &smbios_attr_group))
> + return 0;
> + return -ENODEV;
> +}
> +
> +static int
> +pci_remove_smbiosname_file(struct pci_dev *pdev)
> +{
> + sysfs_remove_group(&pdev->dev.kobj, &smbios_attr_group);
> + return 0;
> +}
What's with the extra indentation?
Why return a value at all here?
> +
> +int pci_create_firmware_label_files(struct pci_dev *pdev)
> +{
> + if (!pci_create_smbiosname_file(pdev))
> + return 0;
> + return -ENODEV;
> +}
> +
> +int pci_remove_firmware_label_files(struct pci_dev *pdev)
> +{
> + if (!pci_remove_smbiosname_file(pdev))
> + return 0;
> + return -ENODEV;
> +}
Why return values for these two functions if you never check them
anywhere? Either check the return value and do something with it, or
just make them 'void'.
Also, you need to add documentation for what this sysfs file is and does
in the Documentation/ABI/ directory. That must be in this patch to have
it acceptable.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] Export SMBIOS provided firmware instance and label to sysfs
From: Greg KH @ 2010-07-21 3:54 UTC (permalink / raw)
To: Narendra_K
Cc: netdev, linux-hotplug, linux-pci, Matt_Domsch, Charles_Rose,
Jordan_Hargrave, Vijay_Nijhawan
In-Reply-To: <EDA0A4495861324DA2618B4C45DCB3EE612BD1@blrx3m08.blr.amer.dell.com>
On Mon, Jul 19, 2010 at 10:24:39PM +0530, Narendra_K@Dell.com wrote:
> > -----Original Message-----
> > From: netdev-owner@vger.kernel.org [mailto:netdev-
> > owner@vger.kernel.org] On Behalf Of Narendra K
> > Sent: Wednesday, July 14, 2010 5:44 PM
> > To: greg@kroah.com
> > Cc: netdev@vger.kernel.org; linux-hotplug@vger.kernel.org; linux-
> > pci@vger.kernel.org; Domsch, Matt; Rose, Charles; Hargrave, Jordan;
> > Nijhawan, Vijay
> > Subject: Re: [PATCH] Export SMBIOS provided firmware instance and
> label
> > to sysfs
> >
> >
> > V1 -> V2:
> >
> > 1. The 'smbios_attr' buffer is not being used as mentioned above
> >
> > 2. The function 'smbios_instance_string_exist' is split into two
> > functions,
> > the other being 'find_smbios_instance_string' which would print the
> > result
> > into the sysfs provided 'buf' of associated device. The function
> > 'smbios_instance_string_exist' would let us know if the label exists
> or
> > not.
> >
> > Please find the patch with above changes here -
> >
> > From: Narendra K <narendra_k@dell.com>
> > Subject: [PATCH] Export SMBIOS provided firmware instance and label to
> > sysfs
> >
>
> Greg,
>
> Thanks for the review comments.
>
> This version of the patch has all the suggestions incorporated. Please
> let us know if there are any concerns. If the approach is acceptable,
> please consider this patch for inclusion.
What "version"? The previous one you sent? I'll look at it, but note
that I'm not the maintainer who you need to convince to accept it :)
thanks,
greg k-h
^ permalink raw reply
* Re: linux-next: manual merge of the net tree with Linus' tree
From: Stephen Rothwell @ 2010-07-21 3:44 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-next, linux-kernel, herbert
In-Reply-To: <20100720.202759.00438343.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 500 bytes --]
Hi Dave,
On Tue, 20 Jul 2010 20:27:59 -0700 (PDT) David Miller <davem@davemloft.net> wrote:
>
> The net-2.6 changes should be undone as net-next-2.6 has the fixes
> that allow bridge netpoll to work properly, thus in net-next-2.6 the
> net-2.6 commit is completely unnecessary.
>
> I did this when I merged net-2.6 into net-next-2.6 about an hour ago
> :-)
Excellent, thanks.
--
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: linux-next: manual merge of the net tree with Linus' tree
From: David Miller @ 2010-07-21 3:27 UTC (permalink / raw)
To: sfr; +Cc: netdev, linux-next, linux-kernel, herbert
In-Reply-To: <20100721120448.31e325fd.sfr@canb.auug.org.au>
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Wed, 21 Jul 2010 12:04:48 +1000
> Today's linux-next merge of the net tree got a conflict in
> net/bridge/br_device.c between commit
> 573201f36fd9c7c6d5218cdcd9948cee700b277d ("bridge: Partially disable
> netpoll support") from Linus' tree and commit
> 91d2c34a4eed32876ca333b0ca44f3bc56645805 ("bridge: Fix netpoll support")
> from the net tree.
>
> The net tree commit seems to be a fuller fix, so I used that.
The net-2.6 changes should be undone as net-next-2.6 has the fixes
that allow bridge netpoll to work properly, thus in net-next-2.6 the
net-2.6 commit is completely unnecessary.
I did this when I merged net-2.6 into net-next-2.6 about an hour ago
:-)
^ permalink raw reply
* Re: [PATCH net-next-2.6] ixgbe: fix ethtool stats
From: Eric Dumazet @ 2010-07-21 2:38 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: David Miller, Jesse Brandeburg, PJ Waskiewicz, netdev
In-Reply-To: <AANLkTik7JrI3HrtvQRTgrRU30fFb2lrgGxUJsWXedBL0@mail.gmail.com>
Le mardi 20 juillet 2010 à 15:06 -0700, Jeff Kirsher a écrit :
> On Tue, Jul 20, 2010 at 10:28, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > Note : I am currently unable to test following patch, could you please
> > Intel guys test it and Ack (or Nack) it ?
> >
> > Thanks !
> >
> > [PATCH net-next-2.6] ixgbe: fix ethtool stats
> >
> > In latest changes about 64bit stats on 32bit arches,
> > [commit 28172739f0a276eb8 (net: fix 64 bit counters on 32 bit arches)],
> > I missed ixgbe uses a bit of magic in its ixgbe_gstrings_stats
> > definition.
> >
> > IXGBE_NETDEV_STAT() must now assume offsets relative to
> > rtnl_link_stats64, not relative do dev->stats.
> >
> > As a bonus, we also get 64bit stats on ethtool -S
> >
> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> > ---
> > drivers/net/ixgbe/ixgbe_ethtool.c | 42 ++++++++++++++--------------
> > 1 file changed, 21 insertions(+), 21 deletions(-)
> >
>
> Thanks Eric, I have added it to my queue.
>
Thanks !
By the way, my ixgbe conf doesnt like net-next-2.6 at all.
(No link is established in my fiber loop configuration)
current linux-2.6 git runs correctly, link at 10Gb, so there is a
regression somewhere.
As this machine is quite slow (I dont have anymore my Nehalem dev
machine, had to use an old setup), a bisection would take one month...
^ 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