2002-10-17 00:55:48

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH] ipv4: make arp seq_file show method only produce one record per call

David,

Please pull from:

master.kernel.org:/home/acme/BK/net-2.5

- Arnaldo


You can import this changeset into BK by piping this whole message to:
'| bk receive [path to repository]' or apply the patch as usual.

===================================================================


[email protected], 2002-10-16 21:41:11-03:00, [email protected]
o ipv4: make arp seq_file show method only produce one record per call


ip_proc.c | 214 +++++++++++++++++++++++++++++++++++++++++++-------------------
1 files changed, 150 insertions(+), 64 deletions(-)


diff -Nru a/net/ipv4/ip_proc.c b/net/ipv4/ip_proc.c
--- a/net/ipv4/ip_proc.c Wed Oct 16 21:44:36 2002
+++ b/net/ipv4/ip_proc.c Wed Oct 16 21:44:36 2002
@@ -67,99 +67,185 @@
}
#endif /* CONFIG_AX25 */

+struct arp_iter_state {
+ loff_t is_pneigh: 1,
+ bucket: 6,
+ pos: sizeof(loff_t) * 8 - 7;
+};
+
+static __inline__ struct neighbour *neigh_get_bucket(loff_t *pos)
+{
+ struct neighbour *n = NULL;
+ struct arp_iter_state* state = (struct arp_iter_state *)pos;
+ loff_t l = state->pos;
+ int i, bucket = state->bucket;
+
+ for (; bucket <= NEIGH_HASHMASK; ++bucket)
+ for (i = 0, n = arp_tbl.hash_buckets[bucket]; n;
+ ++i, n = n->next)
+ /* Do not confuse users "arp -a" with magic entries */
+ if ((n->nud_state & ~NUD_NOARP) && !l--) {
+ state->pos = i;
+ state->bucket = bucket;
+ goto out;
+ }
+out:
+ return n;
+}
+
+static __inline__ struct pneigh_entry *pneigh_get_bucket(loff_t *pos)
+{
+ struct pneigh_entry *n = NULL;
+ struct arp_iter_state* state = (struct arp_iter_state *)pos;
+ loff_t l = state->pos;
+ int i, bucket = state->bucket;
+
+ for (; bucket <= PNEIGH_HASHMASK; ++bucket)
+ for (i = 0, n = arp_tbl.phash_buckets[bucket]; n;
+ ++i, n = n->next)
+ if (!l--) {
+ state->pos = i;
+ state->bucket = bucket;
+ goto out;
+ }
+out:
+ return n;
+}
+
+static __inline__ void *arp_get_bucket(struct seq_file *seq, loff_t *pos)
+{
+ void *rc = neigh_get_bucket(pos);
+
+ if (!rc) {
+ struct arp_iter_state* state = (struct arp_iter_state *)pos;
+
+ read_unlock_bh(&arp_tbl.lock);
+ state->is_pneigh = 1;
+ state->bucket = 0;
+ state->pos = 0;
+ /* HACK: till there is state we can pass to seq_show... */
+ seq->private = (void *)1;
+ rc = pneigh_get_bucket(pos);
+ }
+ return rc;
+}
+
static void *arp_seq_start(struct seq_file *seq, loff_t *pos)
{
- return (void *)(unsigned long)++*pos;
+ read_lock_bh(&arp_tbl.lock);
+ return *pos ? arp_get_bucket(seq, pos) : (void *)1;
}

static void *arp_seq_next(struct seq_file *seq, void *v, loff_t *pos)
{
- return (void *)(unsigned long)((++*pos) >=
- (NEIGH_HASHMASK +
- PNEIGH_HASHMASK - 1) ? 0 : *pos);
+ void *rc;
+ struct arp_iter_state* state;
+
+ if (v == (void *)1) {
+ rc = arp_get_bucket(seq, pos);
+ goto out;
+ }
+
+ state = (struct arp_iter_state *)pos;
+ if (!state->is_pneigh) {
+ struct neighbour *n = v;
+
+ rc = n = n->next;
+ if (n)
+ goto out;
+ state->pos = 0;
+ ++state->bucket;
+ rc = neigh_get_bucket(pos);
+ if (rc)
+ goto out;
+ read_unlock_bh(&arp_tbl.lock);
+ /* HACK: till there is state we can pass to seq_show... */
+ seq->private = (void *)1;
+ state->is_pneigh = 1;
+ state->bucket = 0;
+ state->pos = 0;
+ rc = pneigh_get_bucket(pos);
+ } else {
+ struct pneigh_entry *pn = v;
+
+ pn = pn->next;
+ if (!pn) {
+ ++state->bucket;
+ state->pos = 0;
+ pn = pneigh_get_bucket(pos);
+ }
+ rc = pn;
+ }
+out:
+ ++*pos;
+ return rc;
}

static void arp_seq_stop(struct seq_file *seq, void *v)
{
+ if (!seq->private)
+ read_unlock_bh(&arp_tbl.lock);
}

#define HBUFFERLEN 30

-static __inline__ void arp_format_neigh_table(struct seq_file *seq, int entry)
+static __inline__ void arp_format_neigh_entry(struct seq_file *seq,
+ struct neighbour *n)
{
char hbuffer[HBUFFERLEN];
const char hexbuf[] = "0123456789ABCDEF";
- struct neighbour *n;
int k, j;
+ char tbuf[16];
+ struct net_device *dev = n->dev;
+ int hatype = dev->type;

- read_lock_bh(&arp_tbl.lock);
- for (n = arp_tbl.hash_buckets[entry]; n; n = n->next) {
- char tbuf[16];
- struct net_device *dev = n->dev;
- int hatype = dev->type;
-
- /* Do not confuse users "arp -a" with magic entries */
- if (!(n->nud_state & ~NUD_NOARP))
- continue;
-
- read_lock(&n->lock);
- /* Convert hardware address to XX:XX:XX:XX ... form. */
+ read_lock(&n->lock);
+ /* Convert hardware address to XX:XX:XX:XX ... form. */
#ifdef CONFIG_AX25
- if (hatype == ARPHRD_AX25 || hatype == ARPHRD_NETROM)
- ax2asc2((ax25_address *)n->ha, hbuffer);
- else {
+ if (hatype == ARPHRD_AX25 || hatype == ARPHRD_NETROM)
+ ax2asc2((ax25_address *)n->ha, hbuffer);
+ else {
#endif
- for (k = 0, j = 0; k < HBUFFERLEN - 3 &&
- j < dev->addr_len; j++) {
- hbuffer[k++] = hexbuf[(n->ha[j] >> 4) & 15];
- hbuffer[k++] = hexbuf[n->ha[j] & 15];
- hbuffer[k++] = ':';
- }
- hbuffer[--k] = 0;
+ for (k = 0, j = 0; k < HBUFFERLEN - 3 && j < dev->addr_len; j++) {
+ hbuffer[k++] = hexbuf[(n->ha[j] >> 4) & 15];
+ hbuffer[k++] = hexbuf[n->ha[j] & 15];
+ hbuffer[k++] = ':';
+ }
+ hbuffer[--k] = 0;
#ifdef CONFIG_AX25
- }
-#endif
- sprintf(tbuf, "%u.%u.%u.%u",
- NIPQUAD(*(u32*)n->primary_key));
- seq_printf(seq, "%-16s 0x%-10x0x%-10x%s"
- " * %s\n",
- tbuf, hatype, arp_state_to_flags(n),
- hbuffer, dev->name);
- read_unlock(&n->lock);
- }
- read_unlock_bh(&arp_tbl.lock);
-}
-
-static __inline__ void arp_format_pneigh_table(struct seq_file *seq, int entry)
-{
- struct pneigh_entry *n;
-
- for (n = arp_tbl.phash_buckets[entry]; n; n = n->next) {
- struct net_device *dev = n->dev;
- int hatype = dev ? dev->type : 0;
- char tbuf[16];
-
- sprintf(tbuf, "%u.%u.%u.%u", NIPQUAD(*(u32*)n->key));
- seq_printf(seq, "%-16s 0x%-10x0x%-10x%s"
- " * %s\n",
- tbuf, hatype, ATF_PUBL | ATF_PERM,
- "00:00:00:00:00:00",
- dev ? dev->name : "*");
}
+#endif
+ sprintf(tbuf, "%u.%u.%u.%u", NIPQUAD(*(u32*)n->primary_key));
+ seq_printf(seq, "%-16s 0x%-10x0x%-10x%s * %s\n",
+ tbuf, hatype, arp_state_to_flags(n), hbuffer, dev->name);
+ read_unlock(&n->lock);
}

-static int arp_seq_show(struct seq_file *seq, void *v)
+static __inline__ void arp_format_pneigh_entry(struct seq_file *seq,
+ struct pneigh_entry *n)
{
- unsigned long l = (unsigned long)v - 1;

- if (!l)
+ struct net_device *dev = n->dev;
+ int hatype = dev ? dev->type : 0;
+ char tbuf[16];
+
+ sprintf(tbuf, "%u.%u.%u.%u", NIPQUAD(*(u32*)n->key));
+ seq_printf(seq, "%-16s 0x%-10x0x%-10x%s * %s\n",
+ tbuf, hatype, ATF_PUBL | ATF_PERM, "00:00:00:00:00:00",
+ dev ? dev->name : "*");
+}
+
+static int arp_seq_show(struct seq_file *seq, void *v)
+{
+ if (v == (void *)1)
seq_puts(seq, "IP address HW type Flags "
"HW address Mask Device\n");
-
- if (l <= NEIGH_HASHMASK)
- arp_format_neigh_table(seq, l);
- else
- arp_format_pneigh_table(seq, l - NEIGH_HASHMASK);
+ else {
+ if (seq->private)
+ arp_format_pneigh_entry(seq, v);
+ else
+ arp_format_neigh_entry(seq, v);
+ }

return 0;
}

===================================================================


This BitKeeper patch contains the following changesets:
1.859
## Wrapped with gzip_uu ##


begin 664 bkpatch14639
M'XL(`/0'KCT``\U8;5/;1A#^;/V*#9D0OULG6;)LQS00D\"$$$K"3&821B/D
M,Q*6)5>2#;2FO[V[=_(+CEU"DDYC9.MT+WN[^SR[>^(IG"4\;N4<=\B5IW`0
M)6DKYT8A=U-_XE3=:%B]B''@-(IPH.9%0U[;>UL+>5K1JH:"(R=.ZGHPX7'2
MRK&J/N]);T>\E3O=?W-VM'NJ*)T.O/*<\))_X"ET.DH:Q1,GZ"4OG=0+HK":
MQDZ8#'DJ]IS.ITXU5=7PSV`-737,*3/5>F/JLAYC3IWQGJK5+;.ND/HO5]5>
MD<)47&PRIM>GAEHWFTH76-4RFJ!J-:;6F`D::]59B[&*JK=4%=8*A1*#BJKL
MP<\UX)7B0@3^:%)OP=`9<'#B$23\#[OO!QP2+[H&E.U%/8C"X!9&<=0;NQP?
M.,3<C>(>C'@,KA,$REM`\^J6<K)PN%)YY$=15$=5=M:[8(KHUTA5_+%1$[?J
MSAS,F&D8NCY5U0;:ZEJ:9>E&W])T4[_0C$>+:ZAJG:G65-5UTQ0<^GHND>GG
MJOEH<4@KM<DLI%6S80E:F:ND4JT'2&6H4$$>_-*\$BB\ATI\+2ZDR<D:0+Z#
M;(=F$TQ=2=)X[*:DHNVG/+:3U$DY_*7D@JC?MU,`/[%'(?<OO1:PLI++78S=
M`<>\!``F/8^B1#PD_I\\ZN?EL@(4P8(*--K*75OYHI!4WP7;]L/`#[EM0[:O
MD'P1C6,HBJ9]R5-;;I&)@B+N4%!0HS5+H`/'9T='[?G@?3N*(,WI0'Z]G<4"
M"F_/C0UPIABH[,A^/TS!+X-4:#$HG\FP7#^*(=^>S7B!^NP?OCFP#W8_'+S;
M_?"V#:62'"N@K\1D'^6H92#=29OT(JAZ3N)E5B>?Y?V\#2$J0)X%E.'+!6%E
M)^0W0E:N5H1N!&&4`E*[/TXXX#=.8(OH5G&VX-I//23@)3J>AVGL\P2*-5KI
M]R&?)U'C7N:';?C[^*QK'[_?/3TIP/8V/`DJE0+1`#\+CY`N'?#;R]USW\R<
M0F.741I!-)9/=PJV6DHNYNDX#LFLNW^CA&2;32K?(OC?2HO[RWY%9IQ\#S5&
MW\4-0OC_@'`2^3THDNY+@&4.GF?`(K;*L`JC7!J[9,DJY#1'^%38%;O2K!\"
M%H6A-4[/'H=!Y`[L"R^_/?,Y=13:RMP_\Q2(4ME2O]0N1Z`M]:+X>1?&Z,'N
MJ[<M2/T@@-3C,<>$FJEXS3')AS!RD@0KD/`/%8=JM0HR4+$'Q<58LZ1!TD4%
MH8)PU-?!(3V%>,V@BEV)5;>A`5,.\5=3I.$;S<Y6$C+P&ZR"2=C1+M!:5JC;
M:(".XIM09PLL'XB^.:03/%`LI$ETA8&;-B</+!&4#,Q]8T`+"JT">X]0*P5F
MDI%%,',19Z0"R0I%P"U'RU*P930HE5;2PTS<!OB$8*3YJN0'"?OS^)9;)=P/
MQ\(#C`4>)'P9AM4ZL(!"M$<K.#P9A5FV6^/M-9C,I&R"X&ZALF"83("E4E'2
M:"F\#BV=@DH2:\F%A8<1ZUH6127^:INR*:W`XC!T4GO)(>N3JDC<LBRL.V`5
ME&Y39(&FCL&:<STGAO1BW/_,S//VTODJM7M\XN.1M(AWR7AL9$7/<^@M$WNQ
MJ[)#;32C:0#3E$.FFO?22WX;E\[(B=Q\%87XTDHRXMZU@]QT>KV82SI^^M2:
M74"4))NKQ,LNOHU0;F%,):W)S3,=.H#GE8/3KKW["=]TIE/X:N!X_^/I^W>$
MA'.C.8FKY?/8,.S9QL4":N@Y9?#0#WT>DZ(9$;N,:="@?2TPLW(^D`7Z2I`(
M!O`"#O;.7K_>/SW:/\;CKDY'IROL%;ZA/>R`AVVX*I4D-[-=/@]*I7.4X?$;
M<G]>Z/#YZAQV=J".YR]@!@&R8?I\]J9YSUO/90V8]5<J@W/)^R[35-`LM*IN
M@:$\Y6'/[R/T2-DP[>>)#678>C:NSJZM,AP?GOQ^MMO-%_-C71,.P]E#)[ZU
M!_RV0!XC%F821(K>>E9A9@+J#=[5F^SV3)P]\,4@^SQ+OH1;97F,D?M*],J"
M\B)@[32R^X%SF6"6G4-4EMX-G2&7U6H>8<MTZS*#$=7I]BVQ-7I4<*T_JU*`
M,4/';?%FR-WQIG]/:&'EG0<8EEI*62OQ^N71L/U'<.U^?&V?G.T=P50V]T_?
MH4Q5;=V_9HN7C",,T;BMXE;AWHF2G"%(D)6H#4=(69XFXOBXYAA!*!#)#YFI
M403/"PS-74W4N8U<$#N)DD`"5J:NGWFW^">=ZW%WD(R'G7[#=7O<U91_`$F,
&HG+_$P``
`
end


2002-10-17 00:59:35

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] ipv4: make arp seq_file show method only produce one record per call

From: Arnaldo Carvalho de Melo <[email protected]>
Date: Wed, 16 Oct 2002 22:01:36 -0300

Please pull from:

master.kernel.org:/home/acme/BK/net-2.5

Pulled, thanks.

Now to help Al create a sane mechanism for carrying private state
around between start/stop :-)

2002-10-17 01:05:15

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] ipv4: make arp seq_file show method only produce one record per call

Em Wed, Oct 16, 2002 at 05:58:09PM -0700, David S. Miller escreveu:
> From: Arnaldo Carvalho de Melo <[email protected]>
> Date: Wed, 16 Oct 2002 22:01:36 -0300
>
> Please pull from:
>
> master.kernel.org:/home/acme/BK/net-2.5
>
> Pulled, thanks.
>
> Now to help Al create a sane mechanism for carrying private state
> around between start/stop :-)

That would be nice, yes, bastardizing pos for this is, humm, ugly, and
it isn't accessible at show time (pun intended 8) ).

But now I have to chainsaw the /proc/net/route support into shape, and
this one will be fun, as I'll have to change the semantics of the
struct fib_table tb_seq_show so that I can grab the lock at fib_seq_start,
and at fib_seq_show pass just one entry, then drop the lock at fib_seq_stop.
So, probably we'll have fib_table::tb_seq_{start, next, show, start}, humm,
this gave another idea... :)

- Arnaldo

2002-10-17 01:17:16

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] ipv4: make arp seq_file show method only produce one record per call

From: Arnaldo Carvalho de Melo <[email protected]>
Date: Wed, 16 Oct 2002 22:11:08 -0300

That would be nice, yes, bastardizing pos for this is, humm, ugly, and
it isn't accessible at show time (pun intended 8) ).

Can you remind me what the original objection was to
just using seq->private? Is it used, or planned to
be used, by something else?

2002-10-17 01:25:13

by NeilBrown

[permalink] [raw]
Subject: Re: [PATCH] ipv4: make arp seq_file show method only produce one record per call

On Wednesday October 16, [email protected] wrote:
> From: Arnaldo Carvalho de Melo <[email protected]>
> Date: Wed, 16 Oct 2002 22:11:08 -0300
>
> That would be nice, yes, bastardizing pos for this is, humm, ugly, and
> it isn't accessible at show time (pun intended 8) ).
>
> Can you remind me what the original objection was to
> just using seq->private? Is it used, or planned to
> be used, by something else?

I use seq->private for private state for /proc/fs/nfs/exports.
It works nicely.
You need to define an 'open' the sets it up, and a 'release' to
tear it down, rather than doing it in start/stop.
See fs/nfsd/fnsctl.c:exports_open

NeilBrown

2002-10-17 01:30:01

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] ipv4: make arp seq_file show method only produce one record per call

From: Neil Brown <[email protected]>
Date: Thu, 17 Oct 2002 11:30:43 +1000

I use seq->private for private state for /proc/fs/nfs/exports.
It works nicely.
You need to define an 'open' the sets it up, and a 'release' to
tear it down, rather than doing it in start/stop.
See fs/nfsd/fnsctl.c:exports_open

Hmmm, Arnaldo? :-)

2002-10-17 01:29:31

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] ipv4: make arp seq_file show method only produce one record per call

Em Wed, Oct 16, 2002 at 06:15:50PM -0700, David S. Miller escreveu:
> From: Arnaldo Carvalho de Melo <[email protected]>
> Date: Wed, 16 Oct 2002 22:11:08 -0300
>
> That would be nice, yes, bastardizing pos for this is, humm, ugly, and
> it isn't accessible at show time (pun intended 8) ).
>
> Can you remind me what the original objection was to
> just using seq->private? Is it used, or planned to
> be used, by something else?

lemme get my logs... here it is:

Quoting Al:

"You're free to use, but keep in mind that it's assumed to be constant over
->start/->stop/->next/->show more accurately, there's no sane way to use it as
part of iterator state it's for "which of iterators with similar code it is?"
e.g. /proc/<pid>/mounts uses the same iterator over different namespaces -
depending on pid. take a look at the way /proc/ksyms is done"

- Arnaldo

2002-10-17 01:54:09

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] ipv4: make arp seq_file show method only produce one record per call

Em Wed, Oct 16, 2002 at 06:28:14PM -0700, David S. Miller escreveu:
> From: Neil Brown <[email protected]>
> Date: Thu, 17 Oct 2002 11:30:43 +1000
>
> I use seq->private for private state for /proc/fs/nfs/exports.
> It works nicely.
> You need to define an 'open' the sets it up, and a 'release' to
> tear it down, rather than doing it in start/stop.
> See fs/nfsd/fnsctl.c:exports_open
>
> Hmmm, Arnaldo? :-)

I know that it works :-) I just refrained from using it because it is
not the designed purpose for this field, as per what the author stated to me,
so I didn't wanted to use in a way that could change under my feet in the
future when Al decided to do some change in seq_file.

But if Al changes his mind and state that this is valid use, great,
I'll happily use it.

See my other post with Al's comments.

- Arnaldo

2002-10-17 02:04:39

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] ipv4: make arp seq_file show method only produce one record per call

Em Wed, Oct 16, 2002 at 11:00:01PM -0300, Arnaldo C. Melo escreveu:
> Em Wed, Oct 16, 2002 at 06:28:14PM -0700, David S. Miller escreveu:
> > From: Neil Brown <[email protected]>
> > Date: Thu, 17 Oct 2002 11:30:43 +1000
> >
> > I use seq->private for private state for /proc/fs/nfs/exports.
> > It works nicely.
> > You need to define an 'open' the sets it up, and a 'release' to
> > tear it down, rather than doing it in start/stop.
> > See fs/nfsd/fnsctl.c:exports_open
> >
> > Hmmm, Arnaldo? :-)
>
> I know that it works :-) I just refrained from using it because it is
> not the designed purpose for this field, as per what the author stated to me,
> so I didn't wanted to use in a way that could change under my feet in the
> future when Al decided to do some change in seq_file.
>
> But if Al changes his mind and state that this is valid use, great,
> I'll happily use it.
>
> See my other post with Al's comments.

Ok, /me knocks his head on the wall, rereading I think that it is OK, just the
_pointer_ can't change, oh well, duh, back to work.

- Arnaldo

2002-10-17 02:17:28

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] ipv4: make arp seq_file show method only produce one record per call

Em Wed, Oct 16, 2002 at 11:10:28PM -0300, Arnaldo C. Melo escreveu:
> Em Wed, Oct 16, 2002 at 11:00:01PM -0300, Arnaldo C. Melo escreveu:
> > Em Wed, Oct 16, 2002 at 06:28:14PM -0700, David S. Miller escreveu:
> > > From: Neil Brown <[email protected]>
> > > Date: Thu, 17 Oct 2002 11:30:43 +1000
> > >
> > > I use seq->private for private state for /proc/fs/nfs/exports.
> > > It works nicely.
> > > You need to define an 'open' the sets it up, and a 'release' to
> > > tear it down, rather than doing it in start/stop.
> > > See fs/nfsd/fnsctl.c:exports_open
> > >
> > > Hmmm, Arnaldo? :-)
> >
> > I know that it works :-) I just refrained from using it because it is
> > not the designed purpose for this field, as per what the author stated to me,
> > so I didn't wanted to use in a way that could change under my feet in the
> > future when Al decided to do some change in seq_file.
> >
> > But if Al changes his mind and state that this is valid use, great,
> > I'll happily use it.
> >
> > See my other post with Al's comments.
>
> Ok, /me knocks his head on the wall, rereading I think that it is OK, just the
> _pointer_ can't change, oh well, duh, back to work.

Oh, and now that I think about it there is a way to do it without even using
seq_file->private when you need state passed, just allocates a state structure
in seq_start and return it, keeping the pointer to the real object inside this
state structure, together with any other state you need, when the the list is
exhausted, just free the state structure and return NULL, or return NULL and
free the state struct in ->stop, like kernel/module.c:{s_start, s_stop} do, but
I think that what Neil do in NFS is OK, and better, so I'll use seq->private.

- Arnaldo