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

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

[akpm]

From: "Jari Takkala" <Jari.Takkala@Q9.com>

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 <jari.takkala@q9.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---

 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.000000000 -0700
+++ devel-akpm/net/core/neighbour.c	2006-06-01 20:31:49.000000000 -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)
_

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

[David Miller]

From: akpm@osdl.org
Date: Thu, 01 Jun 2006 20:32:26 -0700

> From: "Jari Takkala" <Jari.Takkala@Q9.com>
> 
> 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 <jari.takkala@q9.com>
> Signed-off-by: Andrew Morton <akpm@osdl.org>

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?

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

[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

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

[Herbert Xu]

Jari Takkala <Jari.Takkala@q9.com> 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 ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[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.