Re: [patch 06/17] neighbour.c, pneigh_get_next() skips published entry

2006-06-09 Thread Jari Takkala 

On Fri, 9 Jun 2006, Herbert Xu wrote:

 Could you post an exact sequence of commands that reproduces the bug?
 That would help us in verifying your fix.
 

Publish a large number of ARP entries (greater than 10 required on my
system):
'arp -Ds IP iface pub'

View output of /proc/net/arp:

'dd if=/proc/net/arp of=arp-1024.out bs=1024'

The produced output will be missing on average one entry for every ten
entries published. Occasionally, the output will vary and the missing
entry will be displayed.


-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 06/17] neighbour.c, pneigh_get_next() skips published entry

2006-06-08 Thread Jari Takkala 

On Mon, 5 Jun 2006, David Miller wrote:

 This patch doesn't make any sense, I've been over it a few times.
 
 The seqfile layer should take care of that user buffering issue
transparently as
 long as we implement the interface callbacks properly.

 Even if something needs to be fixed in the pneigh dumper, special
casing *pos==1
 doesn't look right.  Also, if pneigh has this problem, how come the
neigh seqfile
 iterators don't have the same problem or do they?

From my analysis, the problem is that pneigh_get_next() ends up
assigning pn to one entry ahead of what it should be pointing too. This
only occurs when the user's read buffer fills up, where pneigh_get_idx()
is called, which calls pneigh_get_next() until *pos is not true.

I have not checked neigh seqfile iterators, the problem may exist in
there as well. My patch solves this issue for us, however a more elegant
solution would be most welcome.  Could the root of the problem be that
*pos is off by one when pneigh_get_idx() is called?

Jari

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 06/17] neighbour.c, pneigh_get_next() skips published entry

2006-06-08 Thread Herbert Xu 
Jari Takkala [EMAIL PROTECTED] wrote:
 
 I have not checked neigh seqfile iterators, the problem may exist in
 there as well. My patch solves this issue for us, however a more elegant
 solution would be most welcome.  Could the root of the problem be that
 *pos is off by one when pneigh_get_idx() is called?

Could you post an exact sequence of commands that reproduces the bug?
That would help us in verifying your fix.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 06/17] neighbour.c, pneigh_get_next() skips published entry

2006-06-05 Thread David Miller 
From: [EMAIL PROTECTED]
Date: Thu, 01 Jun 2006 20:32:26 -0700

 From: Jari Takkala [EMAIL PROTECTED]
 
 Fix a problem where output from /proc/net/arp skips a record when the full
 output does not fit into the users read() buffer.
 
 To reproduce: publish a large number of ARP entries (more than 10 required
 on my system).  Run 'dd if=/proc/net/arp of=arp-1024.out bs=1024'.  View
 the output, one entry will be missing.
 
 Signed-off-by: Jari Takkala [EMAIL PROTECTED]
 Signed-off-by: Andrew Morton [EMAIL PROTECTED]

This patch doesn't make any sense, I've been over it a few
times.

The seqfile layer should take care of that user buffering
issue transparently as long as we implement the interface
callbacks properly.

Even if something needs to be fixed in the pneigh dumper,
special casing *pos==1 doesn't look right.  Also, if pneigh
has this problem, how come the neigh seqfile iterators don't
have the same problem or do they?
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[patch 06/17] neighbour.c, pneigh_get_next() skips published entry

2006-06-01 Thread akpm 

From: Jari Takkala [EMAIL PROTECTED]

Fix a problem where output from /proc/net/arp skips a record when the full
output does not fit into the users read() buffer.

To reproduce: publish a large number of ARP entries (more than 10 required
on my system).  Run 'dd if=/proc/net/arp of=arp-1024.out bs=1024'.  View
the output, one entry will be missing.

Signed-off-by: Jari Takkala [EMAIL PROTECTED]
Signed-off-by: Andrew Morton [EMAIL PROTECTED]
---

 net/core/neighbour.c |6 ++
 1 file changed, 6 insertions(+)

diff -puN net/core/neighbour.c~neighbourc-pneigh_get_next-skips-published-entry 
net/core/neighbour.c
--- devel/net/core/neighbour.c~neighbourc-pneigh_get_next-skips-published-entry 
2006-06-01 20:31:49.0 -0700
+++ devel-akpm/net/core/neighbour.c 2006-06-01 20:31:49.0 -0700
@@ -2138,6 +2138,12 @@ static struct pneigh_entry *pneigh_get_n
struct neigh_seq_state *state = seq-private;
struct neigh_table *tbl = state-tbl;
 
+   if (pos != NULL  *pos == 1 
+   (pn-next || tbl-phash_buckets[state-bucket])) {
+   --(*pos);
+   return pn;
+   }
+
pn = pn-next;
while (!pn) {
if (++state-bucket  PNEIGH_HASHMASK)
_
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html