* Re: [PATCH -next] sundance: Add initial ethtool stats support
From: Eric Dumazet @ 2010-10-09 12:33 UTC (permalink / raw)
To: Denis Kirjanov; +Cc: davem, netdev
In-Reply-To: <20101009095346.GA12951@hera.kernel.org>
Le samedi 09 octobre 2010 à 09:53 +0000, Denis Kirjanov a écrit :
> Add initial ethtool statistics support
>
> Signed-off-by: Denis Kirjanov <dkirjanov@kernel.org>
> ---
> drivers/net/sundance.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 46 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/sundance.c b/drivers/net/sundance.c
> index 27d69aa..685845b 100644
> --- a/drivers/net/sundance.c
> +++ b/drivers/net/sundance.c
> @@ -1564,6 +1564,18 @@ static int __set_mac_addr(struct net_device *dev)
> return 0;
> }
>
> +static const struct {
> + const char name[ETH_GSTRING_LEN];
> +} sundance_stats[] = {
> + { "tx_packets" },
> + { "tx_bytes" },
> + { "rx_packets" },
> + { "rx_bytes" },
> + { "tx_errors" },
> + { "tx_dropped" },
> + { "rx_errors" },
> +};
> +
> static int check_if_running(struct net_device *dev)
> {
> if (!netif_running(dev))
> @@ -1622,6 +1634,37 @@ static void set_msglevel(struct net_device *dev, u32 val)
> np->msg_enable = val;
> }
>
> +static void get_strings(struct net_device *dev, u32 stringset,
> + u8 *data)
> +{
if (stringset != ETH_SS_STATS)
return;
> + memcpy(data, sundance_stats, sizeof(sundance_stats));
> +}
> +
> +static int get_sset_count(struct net_device *dev, int sset)
> +{
> + switch (sset) {
> + case ETH_SS_STATS:
> + return ARRAY_SIZE(sundance_stats);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static void get_ethtool_stats(struct net_device *dev,
> + struct ethtool_stats *stats, u64 *data)
> +{
> + struct net_device_stats *netdev_stats = get_stats(dev);
> + int i = 0;
> +
> + data[i++] = netdev_stats->tx_packets;
> + data[i++] = netdev_stats->tx_bytes;
> + data[i++] = netdev_stats->rx_packets;
> + data[i++] = netdev_stats->rx_bytes;
> + data[i++] = netdev_stats->tx_errors;
> + data[i++] = netdev_stats->tx_dropped;
> + data[i++] = netdev_stats->rx_errors;
> +}
> +
> static const struct ethtool_ops ethtool_ops = {
> .begin = check_if_running,
> .get_drvinfo = get_drvinfo,
> @@ -1631,6 +1674,9 @@ static const struct ethtool_ops ethtool_ops = {
> .get_link = get_link,
> .get_msglevel = get_msglevel,
> .set_msglevel = set_msglevel,
> + .get_strings = get_strings,
> + .get_sset_count = get_sset_count,
> + .get_ethtool_stats = get_ethtool_stats,
> };
>
> static int netdev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
^ permalink raw reply
* Re: [PATCHv5] net: Add batman-adv meshing protocol
From: Sven Eckelmann @ 2010-10-09 12:34 UTC (permalink / raw)
To: b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, David Miller
In-Reply-To: <20100924.134334.28812338.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
[-- Attachment #1: Type: Text/Plain, Size: 1622 bytes --]
David Miller wrote:
> From: Sven Eckelmann <sven.eckelmann-Mmb7MZpHnFY@public.gmane.org>
> Date: Sat, 18 Sep 2010 21:03:30 +0200
>
> > B.A.T.M.A.N. (better approach to mobile ad-hoc networking) is a routing
> > protocol for multi-hop ad-hoc mesh networks. The networks may be wired or
> > wireless. See http://www.open-mesh.org/ for more information and user
> > space tools.
> >
> > Signed-off-by: Sven Eckelmann <sven.eckelmann-Mmb7MZpHnFY@public.gmane.org>
>
> The only thing remaining which I really don't like is this hash helper
> library thing in here.
>
> It's a terrible abstraction and very inefficient. Iteration uses
> function calls, as does removal. Key comparisons use callbacks, via
> indirection function pointers, also very inefficient.
I would completely agree.
> Just use the "struct hlist_head" and "struct hlist_node" objects we
> have generically already. Inline the list iteration, as well as the
> key comparisons and the node linking/unlinking.
hlist_head and hlist_node is the right thing to do, but I am a little bit
irritated by the rest.
Ok, no hash implementation from the basics, but there is functionality shared
by the four hashing tables used, which I would not like to "implement" again
everywhere. For example the "add to hash if data isn't already added there".
This can easily done using a static inline function which receives a
comparison and choose/key function (which also can be inlined by the compiler)
and does the rest using hlist_*.
Speaks anything against such things shared in inside batman-adv only?
thanks,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH -next] sundance: Add initial ethtool stats support
From: Ben Hutchings @ 2010-10-09 13:27 UTC (permalink / raw)
To: Denis Kirjanov; +Cc: davem, netdev
In-Reply-To: <20101009095346.GA12951@hera.kernel.org>
Denis Kirjanov wrote:
> Add initial ethtool statistics support
[...]
> +static void get_ethtool_stats(struct net_device *dev,
> + struct ethtool_stats *stats, u64 *data)
> +{
> + struct net_device_stats *netdev_stats = get_stats(dev);
> + int i = 0;
> +
> + data[i++] = netdev_stats->tx_packets;
> + data[i++] = netdev_stats->tx_bytes;
> + data[i++] = netdev_stats->rx_packets;
> + data[i++] = netdev_stats->rx_bytes;
> + data[i++] = netdev_stats->tx_errors;
> + data[i++] = netdev_stats->tx_dropped;
> + data[i++] = netdev_stats->rx_errors;
> +}
[...]
There is no point in adding ethtool stats that merely mirror the baseline
net device stats.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH] SIW: Object management
From: Bernard Metzler @ 2010-10-09 14:10 UTC (permalink / raw)
To: Steve Wise
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, Roland Dreier
In-Reply-To: <4CAB464D.5030702-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
<snip>
> >
> >> the post_send/recv and other functions in your driver are called
> >> directly (almost) by kernel users like NFSRDMA. These users may be
> >> calling in an interrupt context and thus you cannot block/sleep.
> >>
> >>
> > OK, very convincing. not a big change since siw_wqe_get/_put()
> > already maintain a list of pre-allocated wqe's (currently for
> > the read.responses).
> > but, would it be ok if the code distinguishes between user
> > land and in-kernel consumers? i would be very happy if we could
> > keep the pre-allocations per user land connection to its very
> > minimum...
> >
> >
>
> I think that's ok, but its bending the core locking rules a little I
> guess. But the intent is that kernel users can definitely
> send/recv/poll in interrupt context, so possibly blocking for user mode
> QPs in on-kernel-bypass operations is probably ok...
>
i think its best if the code can be optimized this way. i will provide
a patch following down that path soon after i am back from vacation
(next week i am off).
while respecting the specific kernel user requirements, i really want
to keep memory allocations small for user land applications.
thanks,
bernard.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH -next] sundance: Add initial ethtool stats support
From: Denis Kirjanov @ 2010-10-09 14:40 UTC (permalink / raw)
To: Ben Hutchings; +Cc: davem, netdev, Eric Dumazet
In-Reply-To: <20101009132749.GA15074@solarflare.com>
On 10/09/2010 05:27 PM, Ben Hutchings wrote:
> Denis Kirjanov wrote:
>> Add initial ethtool statistics support
> [...]
>> +static void get_ethtool_stats(struct net_device *dev,
>> + struct ethtool_stats *stats, u64 *data)
>> +{
>> + struct net_device_stats *netdev_stats = get_stats(dev);
>> + int i = 0;
>> +
>> + data[i++] = netdev_stats->tx_packets;
>> + data[i++] = netdev_stats->tx_bytes;
>> + data[i++] = netdev_stats->rx_packets;
>> + data[i++] = netdev_stats->rx_bytes;
>> + data[i++] = netdev_stats->tx_errors;
>> + data[i++] = netdev_stats->tx_dropped;
>> + data[i++] = netdev_stats->rx_errors;
>> +}
> [...]
>
> There is no point in adding ethtool stats that merely mirror the baseline
> net device stats.
Fair enough,
I'll add extra stats shortly
> Ben.
>
^ permalink raw reply
* [PATCH] net: introduce alloc_skb_order0
From: Eric Dumazet @ 2010-10-09 15:59 UTC (permalink / raw)
To: Stanislaw Gruszka, David Miller; +Cc: Francois Romieu, netdev
In-Reply-To: <20101008160341.GC10393@redhat.com>
Le vendredi 08 octobre 2010 à 18:03 +0200, Stanislaw Gruszka a écrit :
> On Fri, Oct 08, 2010 at 05:04:07PM +0200, Eric Dumazet wrote:
> > Switch to SLAB -> no more problem ;)
>
> yeh, I wish to, but fedora use SLUB because of some debugging
> capabilities.
Yes, of course, I was kidding :)
echo 0 >/sys/kernel/slab/kmalloc-2048/order
echo 0 >/sys/kernel/slab/kmalloc-1024/order
echo 0 >/sys/kernel/slab/kmalloc-512/order
Should do the trick : No more high order allocations for MTU=1500
frames.
For MTU=9000 frames, we probably need something like this patch :
(Not yet for inclusion, this is an RFC, this will need two separate
patches)
[PATCH] net: introduce alloc_skb_order0()
Reception of big frames hit a memory allocation problem, because of high
order pages allocations (order-3 sometimes for MTU=9000). This patch
introduces alloc_skb_order0(), to build skbs with order-0 pages only.
Their headlen is at most SKB_MAX_HEAD(NET_SKB_PAD + NET_IP_ALIGN)
(3648 bytes on x86_64, 3840 bytes on x86_32)
As net drivers might use skb_store_bits() to copy data to this newly
allocated skb, we might even use __GFP_HIGHMEM for the fragments ?
Note : Use GFP_NOWAIT | __GFP_NOWARN mask to allocate pages, since we
dont want to let big packets exhaust GFP_ATOMIC pool.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
drivers/net/r8169.c | 19 ++++++---------
include/linux/skbuff.h | 1
net/core/skbuff.c | 47 +++++++++++++++++++++++++++++++++++++++
3 files changed, 56 insertions(+), 11 deletions(-)
diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index fe3b762..f4220db 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -4468,27 +4468,24 @@ static inline void rtl8169_rx_csum(struct sk_buff *skb, u32 opts1)
skb_checksum_none_assert(skb);
}
-static inline bool rtl8169_try_rx_copy(struct sk_buff **sk_buff,
+static inline bool rtl8169_try_rx_copy(struct sk_buff **pskb,
struct rtl8169_private *tp, int pkt_size,
dma_addr_t addr)
{
struct sk_buff *skb;
- bool done = false;
if (pkt_size >= rx_copybreak)
- goto out;
+ return false;
- skb = netdev_alloc_skb_ip_align(tp->dev, pkt_size);
+ skb = alloc_skb_order0(pkt_size);
if (!skb)
- goto out;
+ return false;
pci_dma_sync_single_for_cpu(tp->pci_dev, addr, pkt_size,
PCI_DMA_FROMDEVICE);
- skb_copy_from_linear_data(*sk_buff, skb->data, pkt_size);
- *sk_buff = skb;
- done = true;
-out:
- return done;
+ skb_store_bits(skb, 0, (*pskb)->data, pkt_size);
+ *pskb = skb;
+ return true;
}
/*
@@ -4559,10 +4556,10 @@ static int rtl8169_rx_interrupt(struct net_device *dev,
pci_unmap_single(pdev, addr, tp->rx_buf_sz,
PCI_DMA_FROMDEVICE);
tp->Rx_skbuff[entry] = NULL;
+ skb_put(skb, pkt_size);
}
rtl8169_rx_csum(skb, status);
- skb_put(skb, pkt_size);
skb->protocol = eth_type_trans(skb, dev);
if (rtl8169_rx_vlan_skb(tp, desc, skb, polling) < 0) {
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 0b53c43..2cc161a 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1841,6 +1841,7 @@ extern int skb_copy_bits(const struct sk_buff *skb, int offset,
void *to, int len);
extern int skb_store_bits(struct sk_buff *skb, int offset,
const void *from, int len);
+extern struct sk_buff *alloc_skb_order0(int pkt_size);
extern __wsum skb_copy_and_csum_bits(const struct sk_buff *skb,
int offset, u8 *to, int len,
__wsum csum);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 752c197..4a6195d 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1664,6 +1664,53 @@ fault:
}
EXPORT_SYMBOL(skb_store_bits);
+/**
+ * alloc_skb_order0 - allocate skb with order-0 requirements
+ * @pkt_size: packet size
+ *
+ * Allocate an skb with a head small enough that skb->data should not
+ * require high order page allocation, and complete with fragments if
+ * pkt_size is too big. Might be use in drivers RX path : We reserve
+ * NET_SKB_PAD + NET_IP_ALIGN bytes and use GFP_ATOMIC allocations.
+ * We also set skb->len to pkt_size, so driver should not call skb_put()
+ */
+struct sk_buff *alloc_skb_order0(int pkt_size)
+{
+ int head = min_t(int, pkt_size, SKB_MAX_HEAD(NET_SKB_PAD + NET_IP_ALIGN));
+ struct sk_buff *skb;
+
+ skb = alloc_skb(head + NET_SKB_PAD + NET_IP_ALIGN,
+ GFP_ATOMIC | __GFP_NOWARN);
+ if (!skb)
+ return NULL;
+ skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
+ skb_put(skb, head);
+ pkt_size -= head;
+
+ skb->len += pkt_size;
+ skb->data_len += pkt_size;
+ skb->truesize += pkt_size;
+ while (pkt_size) {
+ int i = skb_shinfo(skb)->nr_frags++;
+ skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
+ int fragsize = min_t(int, pkt_size, PAGE_SIZE);
+ struct page *page = alloc_page(GFP_NOWAIT | __GFP_NOWARN);
+
+ if (!page)
+ goto error;
+ frag->page = page;
+ frag->size = fragsize;
+ frag->page_offset = 0;
+ pkt_size -= fragsize;
+ }
+ return skb;
+
+error:
+ kfree_skb(skb);
+ return NULL;
+}
+EXPORT_SYMBOL(alloc_skb_order0);
+
/* Checksum skb data. */
__wsum skb_checksum(const struct sk_buff *skb, int offset,
^ permalink raw reply related
* Re: [PATCH 1/2] r8169: allocate with GFP_KERNEL flag when able to sleep
From: David Miller @ 2010-10-09 16:17 UTC (permalink / raw)
To: eric.dumazet; +Cc: sgruszka, romieu, netdev
In-Reply-To: <1286610844.2692.11.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sat, 09 Oct 2010 09:54:04 +0200
> Le vendredi 08 octobre 2010 à 16:25 +0200, Stanislaw Gruszka a écrit :
>> We have fedora bug report where driver fail to initialize after
>> suspend/resume because of memory allocation errors:
>> https://bugzilla.redhat.com/show_bug.cgi?id=629158
>>
>> To fix use GFP_KERNEL allocation where possible.
>>
>> Tested-by: Neal Becker <ndbecker2@gmail.com>
>> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
>
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH 2/2] r8169: use device model DMA API
From: David Miller @ 2010-10-09 16:17 UTC (permalink / raw)
To: eric.dumazet; +Cc: sgruszka, romieu, netdev
In-Reply-To: <1286611056.2692.12.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sat, 09 Oct 2010 09:57:35 +0200
> Le vendredi 08 octobre 2010 à 16:25 +0200, Stanislaw Gruszka a écrit :
>> Use DMA API as PCI equivalents will be deprecated. This change also
>> allow to allocate with GFP_KERNEL where possible.
>>
>> Tested-by: Neal Becker <ndbecker2@gmail.com>
>> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
>> ---
>> drivers/net/r8169.c | 53 +++++++++++++++++++++++++++-----------------------
>> 1 files changed, 29 insertions(+), 24 deletions(-)
>
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH] ehea: Fix a checksum issue on the receive path
From: David Miller @ 2010-10-09 16:20 UTC (permalink / raw)
To: eric.dumazet; +Cc: leitao, netdev, fubar
In-Reply-To: <1286548577.2959.412.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 08 Oct 2010 16:36:17 +0200
> I am pretty sure most (if not all) netdev drivers pass the packet with
> invalid checksum to upper stack, so that we can increment appropriate
> SNMP counters, in IP stack or UDP/TCP/whatever stack.
>
> tg3, bnx2, e1000, skge, sky2, bnx2x, niu, r8169, igb, ... seems to do
> that.
Drivers _must_ send up all packets, even those with bad checksums,
without exception.
Otherwise protocol statistics get lost, netfilter log entries go
missing, etc.
^ permalink raw reply
* Re: [PATCH] ehea: simplify conditional
From: David Miller @ 2010-10-09 16:21 UTC (permalink / raw)
To: leitao; +Cc: nikai, netdev, linux-kernel
In-Reply-To: <4CAE5C71.50304@linux.vnet.ibm.com>
From: Breno Leitao <leitao@linux.vnet.ibm.com>
Date: Thu, 07 Oct 2010 20:49:05 -0300
> On 10/07/2010 08:14 PM, Nicolas Kaiser wrote:
>> Simplify: ((a&& b) || (!a&& !b)) => (a == b)
>>
>> Signed-off-by: Nicolas Kaiser<nikai@nikai.net>
> Acked-by: Breno Leitao <leitao@linux.vnet.ibm.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net_sched: use __TCA_HTB_MAX and TCA_HTB_MAX
From: David Miller @ 2010-10-09 16:23 UTC (permalink / raw)
To: xiaosuo; +Cc: hadi, netdev
In-Reply-To: <1285863464-5299-1-git-send-email-xiaosuo@gmail.com>
From: Changli Gao <xiaosuo@gmail.com>
Date: Fri, 1 Oct 2010 00:17:44 +0800
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] sundance: get_stats proper locking
From: David Miller @ 2010-10-09 16:24 UTC (permalink / raw)
To: eric.dumazet; +Cc: dkirjanov, netdev
In-Reply-To: <1286626621.2692.18.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sat, 09 Oct 2010 14:17:01 +0200
> Le samedi 09 octobre 2010 à 09:53 +0000, Denis Kirjanov a écrit :
>> Add initial ethtool statistics support
>
> OK, I guess its time to add proper locking into sundance after all ;)
>
> [PATCH net-next] sundance: get_stats proper locking
>
> sundance get_stats() should not be run concurrently, add a lock to avoid
> potential losses.
>
> Note: Remove unused rx_lock field
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net/tg3: simplify conditional
From: David Miller @ 2010-10-09 16:23 UTC (permalink / raw)
To: mcarlson; +Cc: nikai, mchan, netdev, linux-kernel
In-Reply-To: <20101008174312.GA3562@mcarlson.broadcom.com>
From: "Matt Carlson" <mcarlson@broadcom.com>
Date: Fri, 8 Oct 2010 10:43:12 -0700
> On Fri, Oct 08, 2010 at 02:29:27AM -0700, Nicolas Kaiser wrote:
>> Simplify: ((a && !b) || (!a && b)) => (a != b)
>>
>> Signed-off-by: Nicolas Kaiser <nikai@nikai.net>
...
> Looks good to me.
Applied.
Matt, you can give an "Acked-by: " tag to indicate your approval
of a patch in the future :-)
Thanks.
^ permalink raw reply
* Re: [PATCH net-next] net: percpu net_device refcount
From: Paul E. McKenney @ 2010-10-09 16:58 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Stephen Hemminger, David Miller, netdev
In-Reply-To: <1286605396.2692.10.camel@edumazet-laptop>
On Sat, Oct 09, 2010 at 08:23:16AM +0200, Eric Dumazet wrote:
> Le vendredi 08 octobre 2010 à 14:56 -0700, Paul E. McKenney a écrit :
> > On Thu, Oct 07, 2010 at 10:30:51AM -0700, Stephen Hemminger wrote:
> > > On Thu, 07 Oct 2010 19:12:35 +0200
> > > Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > >
> > > > We tried very hard to remove all possible dev_hold()/dev_put() pairs in
> > > > network stack, using RCU conversions.
> > > >
> > > > There is still an unavoidable device refcount change for every dst we
> > > > create/destroy, and this can slow down some workloads (routers or some
> > > > app servers)
> > > >
> > > > We can switch to a percpu refcount implementation, now dynamic per_cpu
> > > > infrastructure is mature. On a 64 cpus machine, this consumes 256 bytes
> > > > per device.
> > >
> > > It makes sense, but what about 256 cores and 1024 Vlans?
> > > That adds up to 4M of memory which is might be noticeable.
> >
> > I bet that systems that have 256 cores have >100GB of memory, at which
> > point 4MB is way down in the noise.
>
> Well, first its 1MB added, and secondly we added percpu stats for vlan
> devices, and this consumed 8x more :
>
> (struct vlan_rx_stats is 32 bytes per cpu and per vlan
> 32*256*1024 -> 8 Mbytes
>
> Some strange machines have many cores sharing a small amount of memory,
> but I am not sure they want to run many net devices ;)
I do have to admit that the rapid growth rate in the data required might
well be cause for concern. But only if it continues. ;-)
Thanx, Paul
^ permalink raw reply
* Re: [PATCH] ehea: Fix a checksum issue on the receive path
From: Stephen Hemminger @ 2010-10-09 17:31 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, leitao, netdev, fubar
In-Reply-To: <20101009.092043.58419374.davem@davemloft.net>
On Sat, 09 Oct 2010 09:20:43 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Fri, 08 Oct 2010 16:36:17 +0200
>
> > I am pretty sure most (if not all) netdev drivers pass the packet with
> > invalid checksum to upper stack, so that we can increment appropriate
> > SNMP counters, in IP stack or UDP/TCP/whatever stack.
> >
> > tg3, bnx2, e1000, skge, sky2, bnx2x, niu, r8169, igb, ... seems to do
> > that.
>
> Drivers _must_ send up all packets, even those with bad checksums,
> without exception.
>
> Otherwise protocol statistics get lost, netfilter log entries go
> missing, etc.
Also hardware checksum can be wrong/broken. By passing up a packet
which the driver thinks is bad, the software can still work.
^ permalink raw reply
* Re: [PATCH] forcedeth: reconfigure multicast packet filter only when needed
From: Stephen Hemminger @ 2010-10-09 18:32 UTC (permalink / raw)
To: Jindřich Makovička; +Cc: linux-kernel, netdev, davem, aabdulla, ditto
In-Reply-To: <AANLkTinbgud9enM1XDQrdZVBtGnTFaUeoEL-D8Z2FmJh@mail.gmail.com>
On Sat, 9 Oct 2010 13:26:05 +0200
Jindřich Makovička <makovick@gmail.com> wrote:
> +
> + /* current packet filter state */
> + u32 cur_pff;
> + u32 cur_addr[2];
> + u32 cur_mask[2];
> };
No big deal, but couldn't you just put those temporary variables
on the stack. and reread the current value before stopping.
--
^ permalink raw reply
* Re: [PATCH -next] sundance: Add initial ethtool stats support
From: Denis Kirjanov @ 2010-10-09 19:41 UTC (permalink / raw)
To: netdev; +Cc: davem, Eric Dumazet, Ben Hutchings
In-Reply-To: <20101009132749.GA15074@solarflare.com>
On 10/09/2010 05:27 PM, Ben Hutchings wrote:
> Denis Kirjanov wrote:
>> Add initial ethtool statistics support
> [...]
>> +static void get_ethtool_stats(struct net_device *dev,
>> + struct ethtool_stats *stats, u64 *data)
>> +{
>> + struct net_device_stats *netdev_stats = get_stats(dev);
>> + int i = 0;
>> +
>> + data[i++] = netdev_stats->tx_packets;
>> + data[i++] = netdev_stats->tx_bytes;
>> + data[i++] = netdev_stats->rx_packets;
>> + data[i++] = netdev_stats->rx_bytes;
>> + data[i++] = netdev_stats->tx_errors;
>> + data[i++] = netdev_stats->tx_dropped;
>> + data[i++] = netdev_stats->rx_errors;
>> +}
> [...]
>
> There is no point in adding ethtool stats that merely mirror the baseline
> net device stats.
>
> Ben.
>
[PATCH -next v2] sundance: Add ethtool stats support
Add ethtool stats support
Signed-off-by: Denis Kirjanov <dkirjanov@kernel.org>
---
V2:
check for the ETH_SS_STATS in get_string()
use xstats struct for ethtool stats
drivers/net/sundance.c | 90 ++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 83 insertions(+), 7 deletions(-)
diff --git a/drivers/net/sundance.c b/drivers/net/sundance.c
index 4283cc5..4e3ff71 100644
--- a/drivers/net/sundance.c
+++ b/drivers/net/sundance.c
@@ -363,6 +363,19 @@ struct netdev_private {
dma_addr_t tx_ring_dma;
dma_addr_t rx_ring_dma;
struct timer_list timer; /* Media monitoring timer. */
+ /* ethtool extra stats */
+ struct {
+ unsigned long tx_multiple_collisions;
+ unsigned long tx_single_collisions;
+ unsigned long tx_late_collisions;
+ unsigned long tx_deffered;
+ unsigned long tx_deffered_excessive;
+ unsigned long tx_aborted;
+ unsigned long tx_bcasts;
+ unsigned long rx_bcasts;
+ unsigned long tx_mcasts;
+ unsigned long rx_mcasts;
+ } xstats;
/* Frequently used values: keep some adjacent for cache effect. */
spinlock_t lock;
int msg_enable;
@@ -1486,7 +1499,6 @@ static struct net_device_stats *get_stats(struct net_device *dev)
{
struct netdev_private *np = netdev_priv(dev);
void __iomem *ioaddr = np->base;
- int i;
unsigned long flags;
spin_lock_irqsave(&np->statlock, flags);
@@ -1494,13 +1506,23 @@ static struct net_device_stats *get_stats(struct net_device *dev)
dev->stats.rx_missed_errors += ioread8(ioaddr + RxMissed);
dev->stats.tx_packets += ioread16(ioaddr + TxFramesOK);
dev->stats.rx_packets += ioread16(ioaddr + RxFramesOK);
- dev->stats.collisions += ioread8(ioaddr + StatsLateColl);
- dev->stats.collisions += ioread8(ioaddr + StatsMultiColl);
- dev->stats.collisions += ioread8(ioaddr + StatsOneColl);
dev->stats.tx_carrier_errors += ioread8(ioaddr + StatsCarrierError);
- ioread8(ioaddr + StatsTxDefer);
- for (i = StatsTxDefer; i <= StatsMcastRx; i++)
- ioread8(ioaddr + i);
+
+ np->xstats.tx_multiple_collisions += ioread8(ioaddr + StatsMultiColl);
+ np->xstats.tx_single_collisions += ioread8(ioaddr + StatsOneColl);
+ np->xstats.tx_late_collisions += ioread8(ioaddr + StatsLateColl);
+ dev->stats.collisions += np->xstats.tx_multiple_collisions
+ + np->xstats.tx_single_collisions
+ + np->xstats.tx_late_collisions;
+
+ np->xstats.tx_deffered += ioread8(ioaddr + StatsTxDefer);
+ np->xstats.tx_deffered_excessive += ioread8(ioaddr + StatsTxXSDefer);
+ np->xstats.tx_aborted += ioread8(ioaddr + StatsTxAbort);
+ np->xstats.tx_bcasts += ioread8(ioaddr + StatsBcastTx);
+ np->xstats.rx_bcasts += ioread8(ioaddr + StatsBcastRx);
+ np->xstats.tx_mcasts += ioread8(ioaddr + StatsMcastTx);
+ np->xstats.rx_mcasts += ioread8(ioaddr + StatsMcastRx);
+
dev->stats.tx_bytes += ioread16(ioaddr + TxOctetsLow);
dev->stats.tx_bytes += ioread16(ioaddr + TxOctetsHigh) << 16;
dev->stats.rx_bytes += ioread16(ioaddr + RxOctetsLow);
@@ -1566,6 +1588,21 @@ static int __set_mac_addr(struct net_device *dev)
return 0;
}
+static const struct {
+ const char name[ETH_GSTRING_LEN];
+} sundance_stats[] = {
+ { "tx_multiple_collisions" },
+ { "tx_single_collisions" },
+ { "tx_late_collisions" },
+ { "tx_deffered" },
+ { "tx_deffered_excessive" },
+ { "tx_aborted" },
+ { "tx_bcasts" },
+ { "rx_bcasts" },
+ { "tx_mcasts" },
+ { "rx_mcasts" },
+};
+
static int check_if_running(struct net_device *dev)
{
if (!netif_running(dev))
@@ -1624,6 +1661,42 @@ static void set_msglevel(struct net_device *dev, u32 val)
np->msg_enable = val;
}
+static void get_strings(struct net_device *dev, u32 stringset,
+ u8 *data)
+{
+ if (stringset == ETH_SS_STATS)
+ memcpy(data, sundance_stats, sizeof(sundance_stats));
+}
+
+static int get_sset_count(struct net_device *dev, int sset)
+{
+ switch (sset) {
+ case ETH_SS_STATS:
+ return ARRAY_SIZE(sundance_stats);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static void get_ethtool_stats(struct net_device *dev,
+ struct ethtool_stats *stats, u64 *data)
+{
+ struct netdev_private *np = netdev_priv(dev);
+ int i = 0;
+
+ get_stats(dev);
+ data[i++] = np->xstats.tx_multiple_collisions;
+ data[i++] = np->xstats.tx_single_collisions;
+ data[i++] = np->xstats.tx_late_collisions;
+ data[i++] = np->xstats.tx_deffered;
+ data[i++] = np->xstats.tx_deffered_excessive;
+ data[i++] = np->xstats.tx_aborted;
+ data[i++] = np->xstats.tx_bcasts;
+ data[i++] = np->xstats.rx_bcasts;
+ data[i++] = np->xstats.tx_mcasts;
+ data[i++] = np->xstats.rx_mcasts;
+}
+
static const struct ethtool_ops ethtool_ops = {
.begin = check_if_running,
.get_drvinfo = get_drvinfo,
@@ -1633,6 +1706,9 @@ static const struct ethtool_ops ethtool_ops = {
.get_link = get_link,
.get_msglevel = get_msglevel,
.set_msglevel = set_msglevel,
+ .get_strings = get_strings,
+ .get_sset_count = get_sset_count,
+ .get_ethtool_stats = get_ethtool_stats,
};
static int netdev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
--
1.7.0
^ permalink raw reply related
* Re: [PATCH] forcedeth: reconfigure multicast packet filter only when needed
From: Jindřich Makovička @ 2010-10-09 20:46 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: linux-kernel, netdev, davem, aabdulla, mditto
In-Reply-To: <20101009113213.62c2f131@nehalam>
[-- Attachment #1: Type: text/plain, Size: 661 bytes --]
2010/10/9 Stephen Hemminger <shemminger@vyatta.com>:
> On Sat, 9 Oct 2010 13:26:05 +0200
> Jindřich Makovička <makovick@gmail.com> wrote:
>
>> +
>> + /* current packet filter state */
>> + u32 cur_pff;
>> + u32 cur_addr[2];
>> + u32 cur_mask[2];
>> };
>
> No big deal, but couldn't you just put those temporary variables
> on the stack. and reread the current value before stopping.
Sure, this version should do the same (still untested, I don't have a
machine with nForce here at the moment). I just wanted to avoid more
NIC register accesses, but it's probably a premature optimization.
Regards,
--
Jindrich Makovicka
[-- Attachment #2: forcedeth2.diff --]
[-- Type: text/x-patch, Size: 2001 bytes --]
--- forcedeth.c.orig 2010-10-07 08:56:55.564511153 +0200
+++ forcedeth.c 2010-10-09 22:34:53.151523511 +0200
@@ -3027,9 +3027,15 @@
{
struct fe_priv *np = netdev_priv(dev);
u8 __iomem *base = get_hwbase(dev);
- u32 addr[2];
- u32 mask[2];
- u32 pff = readl(base + NvRegPacketFilterFlags) & NVREG_PFF_PAUSE_RX;
+ u32 addr[2], prev_addr[2];
+ u32 mask[2], prev_mask[2];
+ u32 prev_pff = readl(base + NvRegPacketFilterFlags);
+ u32 pff = prev_pff & NVREG_PFF_PAUSE_RX;
+
+ prev_addr[0] = readl(base + NvRegMulticastAddrA);
+ prev_addr[1] = readl(base + NvRegMulticastAddrB);
+ prev_mask[0] = readl(base + NvRegMulticastMaskA);
+ prev_mask[1] = readl(base + NvRegMulticastMaskB);
memset(addr, 0, sizeof(addr));
memset(mask, 0, sizeof(mask));
@@ -3072,17 +3078,25 @@
}
addr[0] |= NVREG_MCASTADDRA_FORCE;
pff |= NVREG_PFF_ALWAYS;
- spin_lock_irq(&np->lock);
- nv_stop_rx(dev);
- writel(addr[0], base + NvRegMulticastAddrA);
- writel(addr[1], base + NvRegMulticastAddrB);
- writel(mask[0], base + NvRegMulticastMaskA);
- writel(mask[1], base + NvRegMulticastMaskB);
- writel(pff, base + NvRegPacketFilterFlags);
- dprintk(KERN_INFO "%s: reconfiguration for multicast lists.\n",
- dev->name);
- nv_start_rx(dev);
- spin_unlock_irq(&np->lock);
+ if (prev_pff != pff
+ || memcmp(prev_addr, addr, sizeof(prev_addr)) != 0
+ || memcmp(prev_mask, mask, sizeof(prev_mask)) != 0)
+ {
+ dprintk(KERN_INFO "%s: reconfiguration for multicast lists.\n",
+ dev->name);
+ spin_lock_irq(&np->lock);
+ nv_stop_rx(dev);
+ writel(addr[0], base + NvRegMulticastAddrA);
+ writel(addr[1], base + NvRegMulticastAddrB);
+ writel(mask[0], base + NvRegMulticastMaskA);
+ writel(mask[1], base + NvRegMulticastMaskB);
+ writel(pff, base + NvRegPacketFilterFlags);
+ nv_start_rx(dev);
+ spin_unlock_irq(&np->lock);
+ } else {
+ dprintk(KERN_INFO "%s: pff state unchanged - skipping reconfiguration.\n",
+ dev->name);
+ }
}
static void nv_update_pause(struct net_device *dev, u32 pause_flags)
^ permalink raw reply
* Re: [PATCH -next] sundance: Add initial ethtool stats support
From: Jeff Garzik @ 2010-10-09 21:48 UTC (permalink / raw)
To: Denis Kirjanov; +Cc: netdev, davem, Eric Dumazet, Ben Hutchings
In-Reply-To: <4CB0C56C.2000106@kernel.org>
On 10/09/2010 03:41 PM, Denis Kirjanov wrote:
> On 10/09/2010 05:27 PM, Ben Hutchings wrote:
>> Denis Kirjanov wrote:
>>> Add initial ethtool statistics support
>> [...]
>>> +static void get_ethtool_stats(struct net_device *dev,
>>> + struct ethtool_stats *stats, u64 *data)
>>> +{
>>> + struct net_device_stats *netdev_stats = get_stats(dev);
>>> + int i = 0;
>>> +
>>> + data[i++] = netdev_stats->tx_packets;
>>> + data[i++] = netdev_stats->tx_bytes;
>>> + data[i++] = netdev_stats->rx_packets;
>>> + data[i++] = netdev_stats->rx_bytes;
>>> + data[i++] = netdev_stats->tx_errors;
>>> + data[i++] = netdev_stats->tx_dropped;
>>> + data[i++] = netdev_stats->rx_errors;
>>> +}
>> [...]
>>
>> There is no point in adding ethtool stats that merely mirror the baseline
>> net device stats.
>>
>> Ben.
>>
>
> [PATCH -next v2] sundance: Add ethtool stats support
>
> Add ethtool stats support
>
> Signed-off-by: Denis Kirjanov<dkirjanov@kernel.org>
> ---
> V2:
> check for the ETH_SS_STATS in get_string()
> use xstats struct for ethtool stats
>
> drivers/net/sundance.c | 90 ++++++++++++++++++++++++++++++++++++++++++++----
> 1 files changed, 83 insertions(+), 7 deletions(-)
Acked-by: Jeff Garzik <jgarzik@redhat.com>
glad somebody tackled this...
^ permalink raw reply
* [PATCH] Add IRQF_SAMPLE_RANDOM Flag to forcedeth
From: Patrick Simmons @ 2010-10-09 23:45 UTC (permalink / raw)
To: netdev
This patch adds the IRQF_SAMPLE_RANDOM flag to the forcedeth driver,
allowing the interrupt timing for forcedeth to be used for entropy
generation. This should help /dev/random generate more secure random
numbers on machines using this driver.
Signed-off-by: Patrick Simmons <linuxrocks123@netscape.net>
Please CC me with any comments as I am not subscribed to the list.
--- linux/drivers/net/forcedeth.c.orig 2010-10-09 17:12:08.400000015
-0600
+++ linux/drivers/net/forcedeth.c 2010-10-09 17:14:44.880000015 -0600
@@ -3819,7 +3819,7 @@ static int nv_request_irq(struct net_dev
/* Request irq for rx handling */
sprintf(np->name_rx, "%s-rx", dev->name);
if
(request_irq(np->msi_x_entry[NV_MSI_X_VECTOR_RX].vector,
- nv_nic_irq_rx, IRQF_SHARED, np->name_rx, dev)
!= 0) {
+ nv_nic_irq_rx, IRQF_SHARED |
IRQF_SAMPLE_RANDOM, np->name_rx, dev) != 0) {
printk(KERN_INFO "forcedeth: request_irq failed
for rx %d\n", ret);
pci_disable_msix(np->pci_dev);
np->msi_flags &= ~NV_MSI_X_ENABLED;
@@ -3828,7 +3828,7 @@ static int nv_request_irq(struct net_dev
/* Request irq for tx handling */
sprintf(np->name_tx, "%s-tx", dev->name);
if
(request_irq(np->msi_x_entry[NV_MSI_X_VECTOR_TX].vector,
- nv_nic_irq_tx, IRQF_SHARED, np->name_tx, dev)
!= 0) {
+ nv_nic_irq_tx, IRQF_SHARED |
IRQF_SAMPLE_RANDOM, np->name_tx, dev) != 0) {
printk(KERN_INFO "forcedeth: request_irq failed
for tx %d\n", ret);
pci_disable_msix(np->pci_dev);
np->msi_flags &= ~NV_MSI_X_ENABLED;
@@ -3837,7 +3837,7 @@ static int nv_request_irq(struct net_dev
/* Request irq for link and timer handling */
sprintf(np->name_other, "%s-other", dev->name);
if
(request_irq(np->msi_x_entry[NV_MSI_X_VECTOR_OTHER].vector,
- nv_nic_irq_other, IRQF_SHARED, np->name_other,
dev) != 0) {
+ nv_nic_irq_other, IRQF_SHARED |
IRQF_SAMPLE_RANDOM, np->name_other, dev) != 0) {
printk(KERN_INFO "forcedeth: request_irq failed
for link %d\n", ret);
pci_disable_msix(np->pci_dev);
np->msi_flags &= ~NV_MSI_X_ENABLED;
@@ -3851,7 +3851,7 @@ static int nv_request_irq(struct net_dev
set_msix_vector_map(dev, NV_MSI_X_VECTOR_OTHER,
NVREG_IRQ_OTHER);
} else {
/* Request irq for all interrupts */
- if
(request_irq(np->msi_x_entry[NV_MSI_X_VECTOR_ALL].vector, handler,
IRQF_SHARED, dev->name, dev) != 0) {
+ if
(request_irq(np->msi_x_entry[NV_MSI_X_VECTOR_ALL].vector, handler,
IRQF_SHARED | IRQF_SAMPLE_RANDOM, dev->name, dev) != 0) {
printk(KERN_INFO "forcedeth: request_irq failed
%d\n", ret);
pci_disable_msix(np->pci_dev);
np->msi_flags &= ~NV_MSI_X_ENABLED;
@@ -3868,7 +3868,7 @@ static int nv_request_irq(struct net_dev
if ((ret = pci_enable_msi(np->pci_dev)) == 0) {
np->msi_flags |= NV_MSI_ENABLED;
dev->irq = np->pci_dev->irq;
- if (request_irq(np->pci_dev->irq, handler, IRQF_SHARED,
dev->name, dev) != 0) {
+ if (request_irq(np->pci_dev->irq, handler, IRQF_SHARED |
IRQF_SAMPLE_RANDOM, dev->name, dev) != 0) {
printk(KERN_INFO "forcedeth: request_irq failed %d\n",
ret);
pci_disable_msi(np->pci_dev);
np->msi_flags &= ~NV_MSI_ENABLED;
@@ -3884,7 +3884,7 @@ static int nv_request_irq(struct net_dev
}
}
if (ret != 0) {
- if (request_irq(np->pci_dev->irq, handler, IRQF_SHARED,
dev->name, dev) != 0)
+ if (request_irq(np->pci_dev->irq, handler, IRQF_SHARED |
IRQF_SAMPLE_RANDOM, dev->name, dev) != 0)
goto out_err;
}
^ permalink raw reply
* RE: [PATCH net-next] igb: fix stats handling
From: Tantilov, Emil S @ 2010-10-09 23:57 UTC (permalink / raw)
To: Eric Dumazet, Kirsher, Jeffrey T
Cc: Jesper Dangaard Brouer, Duyck, Alexander H,
Jesper Dangaard Brouer, David S. Miller, netdev, Wyborny, Carolyn
In-Reply-To: <1286339791.4861.26.camel@edumazet-laptop>
Eric Dumazet wrote:
> Le mercredi 06 octobre 2010 à 05:28 +0200, Eric Dumazet a écrit :
>
>> I'll let Intel guys doing the backporting work, but for old kernels,
>> you'll probably need to use "unsigned long" instead of "u64"
>>
>> My plan is :
>>
>> - Provide 64bit counters even on 32bit arch
>> - with proper synchro (include/linux/u64_stats_sync.h)
>> - Add a spinlock so we can apply Jesper patch.
>
> Here is the net-next-2.6 patch, I am currently enable to test it, the
> dev machine with IGB NIC cannot be restarted until tomorrow, my son
> Nicolas is currently using it ;)
>
> Could you and/or Jesper test it, possibly on 32 and 64 bit kernels ?
>
> Thanks !
>
> [PATCH net-next] igb: fix stats handling
>
> There are currently some problems with igb.
>
> - On 32bit arches, maintaining 64bit counters without proper
> synchronization between writers and readers.
>
> - Stats updated every two seconds, as reported by Jesper.
> (Jesper provided a patch for this)
>
> - Potential problem between worker thread and ethtool -S
>
> This patch uses u64_stats_sync, and convert everything to be 64bit
> safe,
> SMP safe, even on 32bit arches.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> drivers/net/igb/igb.h | 7 +-
> drivers/net/igb/igb_ethtool.c | 10 +-
> drivers/net/igb/igb_main.c | 111 +++++++++++++++++++++++---------
> 3 files changed, 94 insertions(+), 34 deletions(-)
This patch is causing a hang when testing with 2 sessions in a while loop reading /proc/net/dev/ and ethtool -S. I think even just reading /proc/net/dev/ is sufficient, but have not confirmed it yet. I have seen the hang somewhere between 15 min to an hour. Without the patch same test ran 24+ hours without issues.
There was no trace on the screen, I got this with magic sysrq:
[15388.393579] SysRq : Show Regs
[15388.397341] Modules linked in: igb [last unloaded: scsi_wait_scan] [15388.404846]
[15388.406889] Pid: 16218, comm: kworker/4:1 Not tainted 2.6.36-rc3-net-next-igb-100810+ #2 S5520HC/S5520HC
[15388.418393] EIP: 0060:[<c13fead2>] EFLAGS: 00000297 CPU: 4
[15388.424908] EIP is at _raw_spin_lock+0x13/0x19
[15388.430257] EAX: f6eab55c EBX: f6eab380 ECX: 00000001 EDX: 00004e4a [15388.437629] ESI: f6eab000 EDI: f6eab41c EBP: f3d9bf4c ESP: f3d9bf4c [15388.445011] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
[15388.451422] Process kworker/4:1 (pid: 16218, ti=f3d9a000 task=f5ce0ed0 task.ti=f3d9a000)
[15388.461173] Stack:
[15388.463796] f3d9bf64 f8586c57 f3d9bf5c f6eab41c f6901e00 c8703cc0 f3d9bf90 c1041379
[15388.473116] <0> 00000004 00000000 f8586b16 00000000 c8707b05 c8707b00 f6901e00 c8703cc4
[15388.483379] <0> c8703cc0 f3d9bfb8 c1042879 c16bb640 c8703cc4 00000c54 c16bb640 f6901e10
[15388.494219] Call Trace:
[15388.497336] [<f8586c57>] ? igb_watchdog_task+0x141/0x21a [igb]
[15388.504336] [<c1041379>] ? process_one_work+0x18e/0x265
[15388.510643] [<f8586b16>] ? igb_watchdog_task+0x0/0x21a [igb]
[15388.517455] [<c1042879>] ? worker_thread+0xf3/0x1ef
[15388.523384] [<c1042786>] ? worker_thread+0x0/0x1ef
[15388.529222] [<c104506b>] ? kthread+0x62/0x67
[15388.534475] [<c1045009>] ? kthread+0x0/0x67
[15388.539623] [<c1002d36>] ? kernel_thread_helper+0x6/0x10
[15388.546034] Code: 00 75 05 f0 66 0f b1 0a 0f 94 c1 0f b6 c1 85 c0 0f 95 c0 0f b6 c0 5d c3 55 ba 00 01 00 00 89 e5 f0 66 0f c1 10 38 f2 74 06 f3 90 <8a> 10 eb f6 5d c3 55 89 e5 9c 59 fa ba 00 01 00 00 f0 66 0f c1
Thanks,
Emil
^ permalink raw reply
* Re: [PATCH] Add IRQF_SAMPLE_RANDOM Flag to forcedeth
From: Ben Hutchings @ 2010-10-10 1:09 UTC (permalink / raw)
To: Patrick Simmons; +Cc: netdev
In-Reply-To: <4CB0FEA6.3030206@netscape.net>
Patrick Simmons wrote:
> This patch adds the IRQF_SAMPLE_RANDOM flag to the forcedeth driver,
> allowing the interrupt timing for forcedeth to be used for entropy
> generation. This should help /dev/random generate more secure random
> numbers on machines using this driver.
[...]
We don't enable this for network drivers any more because:
1. At high packet rates, interrupt moderation makes interrupts very
regular.
2. At low packet rates, a malicious sender can control the interrupt
timing.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH] net: Fix sk_dst_check() to reset the obsolete dst_entry of a socket.
From: Chung-Yih Wang (王崇懿) @ 2010-10-10 1:10 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-kernel, timo.teras
In-Reply-To: <AANLkTikz8=H9=O2RR9J0=rHm7pcwabnMupKfiSfGnt1k@mail.gmail.com>
After I gave it a try, that patch worked. Please ignore my patch then. Thanks!
On Thu, Oct 7, 2010 at 12:37 PM, Chung-Yih Wang (王崇懿) <cywang@google.com> wrote:
> As I am testing the l2tp/ipsec client(it is working fine on 2.6.32 but
> failed on 2.6.35 with the same client). Please see the following log
> dump for sk_dst_check().
>
> <2>[ 201.390289] ==== sk_dst_check sk=C7485800 dst=C717AC60
> obsolete=FFFFFFFF cookie=0 check=C0296510
> <2>[ 211.247467] ==== sk_dst_check sk=C7485000 dst=C717AC60
> obsolete=FFFFFFFF cookie=0 check=C0296510
>
> [Basically, the ipsec tunnel is built and then free the dst_entry for
> this l2tp socket. Therefore, the obsolete entry should be reset in
> sk_dst_check(). However, the kernel 2.6.35 will do nothing here since
> the ipv4_dst_check still return the obsolete entry even if it is
> obsolete(dst->obsolete=2)]
>
> <2>[ 216.571350] ==== sk_dst_check sk=C7485400 dst=C6F670E0
> obsolete=00000002 cookie=0 check=C0296510
> <6>[ 218.069396] alg: No test for authenc(hmac(sha1),cbc(des3_ede))
> (authenc(hmac(sha1-generic),cbc(des3_ede-generic)))
> <6>[ 218.164764] batt: 96%, 4114 mV, 0 mA (-6 avg), 27.2 C, 1305 mAh
> <2>[ 218.575561] ==== sk_dst_check sk=C7485400 dst=C6F670E0
> obsolete=00000002 cookie=0 check=C0296510
> <2>[ 220.580535] ==== sk_dst_check sk=C7485400 dst=C6F670E0
> obsolete=00000002 cookie=0 check=C0296510
> <2>[ 222.585754] ==== sk_dst_check sk=C7485400 dst=C6F670E0
> obsolete=00000002 cookie=0 check=C0296510
> <2>[ 224.591522] ==== sk_dst_check sk=C7485400 dst=C6F670E0
> obsolete=00000002 cookie=0 check=C0296510
> <2>[ 226.599212] ==== sk_dst_check sk=C7485400 dst=C6F670E0
> obsolete=00000002 cookie=0 check=C0296510
> <2>[ 228.602600] ==== sk_dst_check sk=C7485400 dst=C6F670E0
> obsolete=00000002 cookie=0 check=C0296510
> <2>[ 230.608062] ==== sk_dst_check sk=C7485400 dst=C6F670E0
> obsolete=00000002 cookie=0 check=C0296510
> <2>[ 232.613464] ==== sk_dst_check sk=C7485400 dst=C6F670E0
> obsolete=00000002 cookie=0 check=C0296510
> <2>[ 234.618896] ==== sk_dst_check sk=C7485400 dst=C6F670E0
> obsolete=00000002 cookie=0 check=C0296510
> <2>[ 236.623504] ==== sk_dst_check sk=C7485400 dst=C6F670E0
> obsolete=00000002 cookie=0 check=C0296510
> <2>[ 238.628936] ==== sk_dst_check sk=C7485400 dst=C6F670E0
> obsolete=00000002 cookie=0 check=C0296510
> <2>[ 240.634338] ==== sk_dst_check sk=C7485400 dst=C6F670E0
> obsolete=00000002 cookie=0 check=C0296510
> <2>[ 242.639709] ==== sk_dst_check sk=C7485400 dst=C6F670E0
> obsolete=00000002 cookie=0 check=C0296510
> <2>[ 244.645111] ==== sk_dst_check sk=C7485400 dst=C6F670E0
> obsolete=00000002 cookie=0 check=C0296510
> <2>[ 246.648864] ==== sk_dst_check sk=C7485400 dst=C6F670E0
> obsolete=00000002 cookie=0 check=C0296510
> <2>[ 248.654693] ==== sk_dst_check sk=C7485400 dst=C6F670E0
> obsolete=00000002 cookie=0 check=C0296510
> <2>[ 250.660125] ==== sk_dst_check sk=C7485400 dst=C6F670E0
> obsolete=00000002 cookie=0 check=C0296510
> <2>[ 252.665527] ==== sk_dst_check sk=C7485400 dst=C6F670E0
> obsolete=00000002 cookie=0 check=C0296510
>
^ permalink raw reply
* Re: [PATCH] Add IRQF_SAMPLE_RANDOM Flag to forcedeth
From: David Miller @ 2010-10-10 3:15 UTC (permalink / raw)
To: bhutchings; +Cc: linuxrocks123, netdev
In-Reply-To: <20101010010924.GB15074@solarflare.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Sun, 10 Oct 2010 02:09:24 +0100
> Patrick Simmons wrote:
>> This patch adds the IRQF_SAMPLE_RANDOM flag to the forcedeth driver,
>> allowing the interrupt timing for forcedeth to be used for entropy
>> generation. This should help /dev/random generate more secure random
>> numbers on machines using this driver.
> [...]
>
> We don't enable this for network drivers any more because:
>
> 1. At high packet rates, interrupt moderation makes interrupts very
> regular.
> 2. At low packet rates, a malicious sender can control the interrupt
> timing.
Agreed on all counts, I'm not applying this patch.
^ permalink raw reply
* Re: [PATCH] Add IRQF_SAMPLE_RANDOM Flag to forcedeth
From: Patrick Simmons @ 2010-10-10 3:23 UTC (permalink / raw)
To: David Miller; +Cc: bhutchings, netdev
In-Reply-To: <20101009.201501.214209347.davem@davemloft.net>
On 10/09/10 21:15, David Miller wrote:
> From: Ben Hutchings<bhutchings@solarflare.com>
> Date: Sun, 10 Oct 2010 02:09:24 +0100
>
>> Patrick Simmons wrote:
>>> This patch adds the IRQF_SAMPLE_RANDOM flag to the forcedeth driver,
>>> allowing the interrupt timing for forcedeth to be used for entropy
>>> generation. This should help /dev/random generate more secure random
>>> numbers on machines using this driver.
>> [...]
>>
>> We don't enable this for network drivers any more because:
>>
>> 1. At high packet rates, interrupt moderation makes interrupts very
>> regular.
>> 2. At low packet rates, a malicious sender can control the interrupt
>> timing.
>
> Agreed on all counts, I'm not applying this patch.
It's enabled for other network drivers, which is where I got the idea
from. Has anyone actually done an experiment to see whether these two
concerns are valid?
--Patrick
--
If I'm not here, I've gone out to find myself. If I get back before I
return, please keep me here.
^ 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