Last remaining insque / remque uses

[Iain Hibbert]

3 if_eon.c eonrtrequest 269 remque(&el->el_qhdr);

Given that many of these pieces of sh^W^W^Wlovely source files
don't actually bother using 'struct queue', and far more often
monkey around with the affected structures themselves - it might
be better to just embed local copies of the routines into the
affected sources. (As the defs are all of five lines of code each,
I don't think it'd be a horrible burden.)

'options EON' brings in the eon code, which accounts for one of these
usages, patch below. It builds but I can't test it as I have no idea what
EON is. If it a) looks fine, or b) somebody can test then I'll commit
that.

(the list is otherwise unused, it could as easily be removed)

I was looking at the other netiso usages and yeah, that code is gangly..
how much effort is too much effort for this?

iain

--- /usr/src/sys/netiso/eonvar.h 2007-07-20 22:24:51.000000000 +0100
+++ eonvar.h 2007-10-23 21:40:09.000000000 +0100
@@ -159,18 +159,14 @@
#undef IncStat
#define IncStat(xxx) eonstat.xxx++

-typedef struct qhdr {
- struct qhdr *link, *rlink;
-} *queue_t;
-
struct eon_llinfo {
- struct qhdr el_qhdr;/* keep all in a list */
int el_flags; /* cache valid ? */
int el_snpaoffset; /* IP address contained in dst nsap */
struct rtentry *el_rt; /* back pointer to parent route */
struct eon_iphdr el_ei; /* precomputed portion of hdr */
struct route el_iproute; /* if direct route cache IP info */
/* if gateway, cache secondary route */
+ LIST_ENTRY(eon_llinfo) el_next; /* next entry */
};
#define el_iphdr el_ei.ei_ip
#define el_eonhdr el_ei.ei_eh
--- /usr/src/sys/netiso/if_eon.c 2007-09-05 22:23:55.000000000 +0100
+++ if_eon.c 2007-10-23 22:53:35.000000000 +0100
@@ -80,6 +80,7 @@
#include <sys/mbuf.h>
#include <sys/buf.h>
#include <sys/protosw.h>
+#include <sys/queue.h>
#include <sys/socket.h>
#include <sys/ioctl.h>
#include <sys/errno.h>
@@ -119,7 +120,7 @@
(void) eonattach();
}

-struct eon_llinfo eon_llinfo;
+LIST_HEAD(eon_llhead, eon_llinfo) eon_llhead;
#define PROBE_OK 0;


@@ -155,8 +156,7 @@
if_attach(ifp);
if_alloc_sadl(ifp);
eonioctl(ifp, SIOCSIFADDR, (void *) ifp->if_addrlist.tqh_first);
- eon_llinfo.el_qhdr.link =
- eon_llinfo.el_qhdr.rlink = &(eon_llinfo.el_qhdr);
+ LIST_INIT(&eon_llhead);

#ifdef ARGO_DEBUG
if (argo_debug[D_EON]) {
@@ -266,7 +266,7 @@
switch (cmd) {
case RTM_DELETE:
if (el) {
- remque(&el->el_qhdr);
+ LIST_REMOVE(el, el_next);
rtcache_free(&el->el_iproute);
Free(el);
rt->rt_llinfo = NULL;
@@ -281,7 +281,7 @@
if (el == NULL)
return;
memset(el, 0, sizeof(*el));
- insque(&el->el_qhdr, &eon_llinfo.el_qhdr);
+ LIST_INSERT_HEAD(&eon_llhead, el, el_next);
el->el_rt = rt;
break;
}

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Adrian Portelli]

Rumor has it that the code is used people using the IS-IS routing
protocol, which runs over the ISO stack.

I asked around various people on multiple conferences and noone could
really say whether (a) IS-IS is done using ISO and (b) whether the
relevant routing deamons are not already doing it completely in
userland.
Of course, there's also (c) whether it is relevant at all.
Joerg

FWIW I received no objections when I pulled it from GENERIC kernels.

adrian.

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Steven M. Bellovin]

On Wed, 24 Oct 2007 16:34:05 -0400

Post by Iain Hibbert
I was looking at the other netiso usages and yeah, that code is
gangly.. how much effort is too much effort for this?

Does anyone use/test/maintain netiso on NetBSD-current? If we
cannot get a maintainer, then I think that we should delete it
before 5.0.

Rumor has it that the code is used people using the IS-IS routing
protocol, which runs over the ISO stack.

There were explicit posts on this point. See
http://mail-index.netbsd.org/tech-net/2005/04/09/0000.html
http://mail-index.netbsd.org/tech-net/2005/04/11/0001.html
http://mail-index.netbsd.org/tech-net/2005/04/11/0004.html


--Steve Bellovin, http://www.cs.columbia.edu/~smb

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Ignatios Souvatzis]

Rumor has it that the code is used people using the IS-IS routing
protocol, which runs over the ISO stack.

I asked around various people on multiple conferences and noone could
really say whether (a) IS-IS is done using ISO and (b) whether the
relevant routing deamons are not already doing it completely in
userland.

You should have asked Chris Hopps, and no, it doesn't.

I have some demo packet sending / receiving code for sending simple
datagram packets; I should be able to do function tests for that
code path.

-is

[David Young]

Given that many of these pieces of sh^W^W^Wlovely source files
don't actually bother using 'struct queue', and far more often
monkey around with the affected structures themselves - it might
be better to just embed local copies of the routines into the
affected sources. (As the defs are all of five lines of code each,
I don't think it'd be a horrible burden.)

Having looked at it in a bit more depth, I think that if no testers can be
found for ISO/EON/TPIP then just hardcoding it is the cheapest option and
--- /usr/src/sys/netiso/iso_pcb.c 2007-07-20 22:24:52.000000000 +0100
+++ iso_pcb.c 2007-10-26 12:30:30.000000000 +0100
@@ -118,7 +118,13 @@
return ENOBUFS;
isop->isop_head = head;
isop->isop_socket = so;
- insque(isop, head);
+
+ /* insert at head of pcb list */
+ isop->isop_next = head->isop_next;
+ isop->isop_prev = head;
+ head->isop_next = isop;
+ isop->isop_next->isop_prev = isop;
+

Please, don't do that. Use queue(3) macros. They work, developers
believe they work, and LIST_INSERT_HEAD() will reduce the lines of code
and comments in that example from five to one.

Dave

[Ignatios Souvatzis]

Hi Iain,

However, if the usage is removed from coda as per my other suggestion,
then this is the only thing left using insque/remque - we could do as Tom
suggested and embed them inside the netiso code as below. Its somewhat
cleaner and just as cheap - but is it good enough?

[...]

- remque(&el->el_qhdr);
+ iso_remque(&el->el_qhdr);

[...]
Why shouldn't it? At least as an intermediate measure, it's fine IMO
(and it would allow easliy resurrecting the x.25 related stuff if
somebody needs it in the future).

I've tested it on -current as well as backported to 4.0_RC4 (both
with 4.0_RC3 userland) for CLTP on CLNP on the Ethernet LAN.

I say - (check that the Copyright Notice and License that goes with insque
/remque is in iso.c, then) commit.

Regards,
-is

[Iain Hibbert]

Post by Ignatios Souvatzis

However, if the usage is removed from coda as per my other suggestion,

[somebody is testing that]

Post by Ignatios Souvatzis
I say - (check that the Copyright Notice and License that goes with insque
/remque is in iso.c, then) commit.

Ok thanks, I will do that - I got carried away with something else and
stalled on the proper conversion, but I will continue hacking on it.

iain

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Jason Thorpe]

Queue(3), please.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

C symbol: insque
File Function Line
0 coda_namecache.c <global> 96 #ifndef insque

Coda is much cleaner than netiso, and its not too difficult to tidy
that
away. There are two ways through it and I'm not sure which is better.
a) If we own that code and want to use queue(3) macros, then
diff.queue
attached does handle that. There is not an exact match between queue
(3)
and the macros that were being used and the original macros were not a
complete abstraction, so I've just removed them and replaced with the
queue(3) macros in plain view.
b) If the coda project owns that code and it would be difficult to
maintain differences, then diff.insque attached removes the
dependency by
giving coda_namecache a private copy of insque/remque and changing
nothing
else.
I notice that FreeBSD uses the same kernel code as us, and could no
doubt
update the same way (they have an open PR to do the second method
though
the patch supplied is much more invasive). Linux has different, so
maybe
the coda project does not have a great claim on it. I prefer the
queue(3)
method, but offer both for comment.
I don't use coda and only have one computer, so testing is needed..
iain
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (NetBSD)
iQEVAwUBRySLrvFJxoMWDXVDAQIAjAgAm1ugd5OU2DH8tRwvCFQrp/HFzeNi9BZy
XBjIV0UWH+TNeruxv+P/SE5AlmdOmzklRflxvaPuE+seRvYlTpeC4xfnDUABu3iL
O6v4xY+PDl906lzVN9bYpdYVxWbqdp1wZPV9wiymatio5h42jKfcbXXCsIooMDmj
7GOq73++PWncfxdXyfpk7AUMrbKEW7sS7gWsPH7tEaGm/yEW5BXHpOnqYc5CVMGD
N9U7Xfc28dYhzSMhMmSrJGEKjJY5cKikWJxni1oaSNIA1J6DCy6+nRXnxsid20ST
3ma9BLGTLhkXLxr/To5pFolE2FNmlfzhuJOPY6jsEi17Ugqe53iG5g==
=IFwZ
-----END PGP SIGNATURE-----
<diff.queue>
<diff.insque>

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de