rtsock.c: eliminate masking of gotos, don't abuse M_RTABLE
Hi,
Here's a diff to eliminate the senderr() macro from rtsock.c.
This macro is masking goto statements, which is incredibly bad style,
and makes it difficult to follow the flow of control in the file.
This diff also stops rtsock.c from abusing the M_RTABLE malloc define
for routing socket messages, for the sake of being clearer.
[I would have liked to keep these separate but I haven't setup a local
branch yet.]
The size of an rtmsg can vary, but should perhaps be constant --
there are bugs present, as exercised by some code I posted to this list,
and to ru@ in private, just over a month or so ago which panics the kernel
by passing in a dubiously-formatted PF_ROUTE message.
Making rtmsg constant and no longer using the packed sockaddr format would
make it a candiate for a zone allocator, and would help eliminate some
redundancy between rtsock.c and ifconfig(8)/route(8) in the userland.
Comments?
BMS
Index: rtsock.c
===
RCS file: /home/ncvs/src/sys/net/rtsock.c,v
retrieving revision 1.89
diff -u -r1.89 rtsock.c
--- rtsock.c5 Mar 2003 19:24:22 - 1.89
+++ rtsock.c3 Oct 2003 06:45:18 -
@@ -55,6 +55,14 @@
MALLOC_DEFINE(M_RTABLE, "routetbl", "routing tables");
+MALLOC_DEFINE(M_RTMSG, "rtmsg", "PF_ROUTE message data");
+
+#define RTMSG_MALLOC(p, n) \
+ ((p) = (struct rt_msghdr *) \
+ malloc((unsigned long)(n), M_RTMSG, M_NOWAIT))
+#define RTMSG_FREE(p) \
+ free((caddr_t)(p), M_RTMSG)
+
static struct sockaddr route_dst = { 2, PF_ROUTE, };
static struct sockaddr route_src = { 2, PF_ROUTE, };
static struct sockaddr sa_zero = { sizeof(sa_zero), AF_INET, };
@@ -280,7 +288,6 @@
struct ifnet *ifp = 0;
struct ifaddr *ifa = 0;
-#define senderr(e) { error = e; goto flush;}
if (m == 0 || ((m->m_len < sizeof(long)) &&
(m = m_pullup(m, sizeof(long))) == 0))
return (ENOBUFS);
@@ -290,37 +297,45 @@
if (len < sizeof(*rtm) ||
len != mtod(m, struct rt_msghdr *)->rtm_msglen) {
dst = 0;
- senderr(EINVAL);
+ error = EINVAL;
+ goto flush;
}
- R_Malloc(rtm, struct rt_msghdr *, len);
+ RTMSG_MALLOC(rtm, len);
if (rtm == 0) {
dst = 0;
- senderr(ENOBUFS);
+ error = ENOBUFS;
+ goto flush;
}
m_copydata(m, 0, len, (caddr_t)rtm);
if (rtm->rtm_version != RTM_VERSION) {
dst = 0;
- senderr(EPROTONOSUPPORT);
+ error = EPROTONOSUPPORT;
+ goto flush;
}
rtm->rtm_pid = curproc->p_pid;
bzero(&info, sizeof(info));
info.rti_addrs = rtm->rtm_addrs;
if (rt_xaddrs((caddr_t)(rtm + 1), len + (caddr_t)rtm, &info)) {
dst = 0;
- senderr(EINVAL);
+ error = EINVAL;
+ goto flush;
}
info.rti_flags = rtm->rtm_flags;
if (dst == 0 || (dst->sa_family >= AF_MAX)
- || (gate != 0 && (gate->sa_family >= AF_MAX)))
- senderr(EINVAL);
+ || (gate != 0 && (gate->sa_family >= AF_MAX))) {
+ error = EINVAL;
+ goto flush;
+ }
if (genmask) {
struct radix_node *t;
t = rn_addmask((caddr_t)genmask, 0, 1);
if (t && Bcmp((caddr_t *)genmask + 1, (caddr_t *)t->rn_key + 1,
*(u_char *)t->rn_key - 1) == 0)
genmask = (struct sockaddr *)(t->rn_key);
- else
- senderr(ENOBUFS);
+ else {
+ error = ENOBUFS;
+ goto flush;
+ }
}
/*
@@ -328,13 +343,15 @@
* is the only operation the non-superuser is allowed.
*/
if (rtm->rtm_type != RTM_GET && (error = suser(curthread)) != 0)
- senderr(error);
+ goto flush;
switch (rtm->rtm_type) {
case RTM_ADD:
- if (gate == 0)
- senderr(EINVAL);
+ if (gate == 0) {
+ error = EINVAL;
+ goto flush;
+ }
error = rtrequest1(RTM_ADD, &info, &saved_nrt);
if (error == 0 && saved_nrt) {
rt_setmetrics(rtm->rtm_inits,
@@ -359,15 +376,18 @@
case RTM_CHANGE:
case RTM_LOCK:
if ((rnh = rt_tables[dst->sa_family]) == 0) {
- senderr(EAFNOSUPPORT);
+ error = EAFNOSUPPORT;
+ goto flush;
}
RADIX_NODE_HEAD_LOCK(rnh);
rt = (struct rtentry *) rnh->rnh_lookup(dst, netmask, rnh);
RADIX_NODE_HEAD_UNLOCK(rnh);
if (rt != NULL)
Re: rtsock.c: eliminate masking of gotos, don't abuse M_RTABLE
On Fri, 3 Oct 2003, Bruce M Simpson wrote: > Comments? > > BMS I think that anything to clean up the routing table is a good idea... however, aren't there a few major non-committed patches to this area which are almost ready? (Sam's locking, someone else's moving cloned routes into a tcpstatcache, more?) You should probably check on the status of those projects first and make sure that you won't interfere with their integration. Mike "Silby" Silbersack ___ [EMAIL PROTECTED] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Re: rtsock.c: eliminate masking of gotos, don't abuse M_RTABLE
On Fri, Oct 03, 2003 at 02:40:22AM -0500, Mike Silbersack wrote: > > On Fri, 3 Oct 2003, Bruce M Simpson wrote: > > > Comments? > > > > BMS > > I think that anything to clean up the routing table is a good idea... > however, aren't there a few major non-committed patches to this area which > are almost ready? (Sam's locking, someone else's moving cloned routes > into a tcpstatcache, more?) You should probably check on the status of > those projects first and make sure that you won't interfere with their > integration. > I think these uncommitted patches will mostly affect route.c, while this patch is for rtsock.c, the route(4) interface with the kernel, which is unlikely to change a lot. Cheers, -- Ruslan Ermilov Sysadmin and DBA, [EMAIL PROTECTED] Sunbay Software Ltd, [EMAIL PROTECTED] FreeBSD committer pgp0.pgp Description: PGP signature
Re: rtsock.c: eliminate masking of gotos, don't abuse M_RTABLE
On Fri, Oct 03, 2003 at 11:00:10AM +0300, Ruslan Ermilov wrote:
> I think these uncommitted patches will mostly affect route.c, while this
> patch is for rtsock.c, the route(4) interface with the kernel, which is
> unlikely to change a lot.
Much-improved patch to cleanup rtsock.c at bde's prodding attached.
I'm proposing doing another pass after this to clear up the bad
condition checking style, parentheses, and whitespace.
BMS
Index: route.h
===
RCS file: /home/ncvs/src/sys/net/route.h,v
retrieving revision 1.47
diff -u -r1.47 route.h
--- route.h 5 Mar 2003 19:24:22 - 1.47
+++ route.h 3 Oct 2003 08:09:36 -
@@ -262,6 +262,11 @@
};
#ifdef _KERNEL
+MALLOC_DECLARE(M_RTMSG);
+
+#defineRTMSG_MALLOC(p, n) (p) = malloc((n), M_RTMSG, M_NOWAIT)
+#defineRTMSG_FREE(p) free((p), M_RTMSG)
+
#defineRTFREE(rt) \
do { \
if ((rt)->rt_refcnt <= 1) \
@@ -296,6 +301,9 @@
struct sockaddr *, struct sockaddr *, int, struct rtentry **);
int rtrequest1(int, struct rt_addrinfo *, struct rtentry **);
int rt_check(struct rtentry **, struct rtentry **, struct sockaddr *);
+#else
+#define RTMSG_MALLOC(p, n) (p) = malloc((u_long)(n))
+#define RTMSG_FREE(p) free((p))
#endif
#endif
Index: rtsock.c
===
RCS file: /home/ncvs/src/sys/net/rtsock.c,v
retrieving revision 1.89
diff -u -r1.89 rtsock.c
--- rtsock.c5 Mar 2003 19:24:22 - 1.89
+++ rtsock.c3 Oct 2003 08:11:06 -
@@ -280,7 +280,6 @@
struct ifnet *ifp = 0;
struct ifaddr *ifa = 0;
-#define senderr(e) { error = e; goto flush;}
if (m == 0 || ((m->m_len < sizeof(long)) &&
(m = m_pullup(m, sizeof(long))) == 0))
return (ENOBUFS);
@@ -290,37 +289,45 @@
if (len < sizeof(*rtm) ||
len != mtod(m, struct rt_msghdr *)->rtm_msglen) {
dst = 0;
- senderr(EINVAL);
+ error = EINVAL;
+ goto flush;
}
- R_Malloc(rtm, struct rt_msghdr *, len);
+ RTMSG_MALLOC(rtm, len);
if (rtm == 0) {
dst = 0;
- senderr(ENOBUFS);
+ error = ENOBUFS;
+ goto flush;
}
m_copydata(m, 0, len, (caddr_t)rtm);
if (rtm->rtm_version != RTM_VERSION) {
dst = 0;
- senderr(EPROTONOSUPPORT);
+ error = EPROTONOSUPPORT;
+ goto flush;
}
rtm->rtm_pid = curproc->p_pid;
bzero(&info, sizeof(info));
info.rti_addrs = rtm->rtm_addrs;
if (rt_xaddrs((caddr_t)(rtm + 1), len + (caddr_t)rtm, &info)) {
dst = 0;
- senderr(EINVAL);
+ error = EINVAL;
+ goto flush;
}
info.rti_flags = rtm->rtm_flags;
if (dst == 0 || (dst->sa_family >= AF_MAX)
- || (gate != 0 && (gate->sa_family >= AF_MAX)))
- senderr(EINVAL);
+ || (gate != 0 && (gate->sa_family >= AF_MAX))) {
+ error = EINVAL;
+ goto flush;
+ }
if (genmask) {
struct radix_node *t;
t = rn_addmask((caddr_t)genmask, 0, 1);
if (t && Bcmp((caddr_t *)genmask + 1, (caddr_t *)t->rn_key + 1,
*(u_char *)t->rn_key - 1) == 0)
genmask = (struct sockaddr *)(t->rn_key);
- else
- senderr(ENOBUFS);
+ else {
+ error = ENOBUFS;
+ goto flush;
+ }
}
/*
@@ -328,13 +335,15 @@
* is the only operation the non-superuser is allowed.
*/
if (rtm->rtm_type != RTM_GET && (error = suser(curthread)) != 0)
- senderr(error);
+ goto flush;
switch (rtm->rtm_type) {
case RTM_ADD:
- if (gate == 0)
- senderr(EINVAL);
+ if (gate == 0) {
+ error = EINVAL;
+ goto flush;
+ }
error = rtrequest1(RTM_ADD, &info, &saved_nrt);
if (error == 0 && saved_nrt) {
rt_setmetrics(rtm->rtm_inits,
@@ -359,15 +368,18 @@
case RTM_CHANGE:
case RTM_LOCK:
if ((rnh = rt_tables[dst->sa_family]) == 0) {
- senderr(EAFNOSUPPORT);
+ error = EAFNOSUPPORT;
+ goto flush;
}
RADIX_NODE_HEAD_LOCK(rnh);
rt = (struct rtentry *) rnh->rnh_lookup(dst, netmask, rnh);
RADIX_NODE_HEAD_UNLOCK(rnh);
if (rt != NULL)
rt->rt_refcnt++;
- else
-
Re: rtsock.c: eliminate masking of gotos, don't abuse M_RTABLE
Mike Silbersack wrote: > > On Fri, 3 Oct 2003, Bruce M Simpson wrote: > > > Comments? > > > > BMS > > I think that anything to clean up the routing table is a good idea... > however, aren't there a few major non-committed patches to this area which > are almost ready? (Sam's locking, someone else's moving cloned routes > into a tcpstatcache, more?) You should probably check on the status of > those projects first and make sure that you won't interfere with their > integration. This rtsock.c patch does not interfere with the tcp_hostcache work I'm doing at the moment. I've got stuck a little bit in the tcp6 stuff in tcp. The past weeks were quite busy for me but this weekend I've got time to carry the tcp_hostcache (and nuking of PR_CLONING) forward. -- Andre ___ [EMAIL PROTECTED] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Re: ng_ip_input use case
В пт, 03.10.2003, в 03:09, Julian Elischer пишет: > > > I have no idea what it is used for.. similar effect can be made by > > > using the ng_iface node, but I think that they didn't want a separate > > > interface for each packet source.. > > > > > > I suggest we ask brooks..(cc'd). I could imagine it somehow connected > > > with the 300 processor FreeBSD based cluster that he is working on at > > > hos job (especially as it has their copyright). > > > > It was for a network emulator we were trying to build to allow us to > > run real, unmodified programs in an environment where each program > > was assumed to be an independent agent and the communication topology > > between agents changed in realtime. Each agent was to bind to the IP > > address of an IP-over-IP tunnel (gif on the FreeBSD end). We would then > > take the packets, mangle them slightly and send them off to the emulator > > (I think part of the goal was to let us use more or less off the shelf > > emulation gear in the middle without needing a workstation for each > > agent). When we got them back, we'd remanged them and stuff them back > > into the IP stack with ng_ip_input so it could decide which gif tunnel > > to send it back down. The emulation part of the project died when our > > funding for it dried up so we've not actually using this module for > > anything. > > was there a reason to not use the ng_iface node? > (packets enterred on the INET hook if an iface node will be injected > into the ip stack) (from memory) ng_iface does much more then ability to inject packets into IP-stack, as minimum it creates interface, on hosts with dynamic routing software you should start care about this (unused !) interface, If you have number of ng_iface-s used for some real task and some interfaces used only as ip_input gateway you can completely mix them all in mind. I do not think that it is good idea to create one more interface only to inject packets into IP-stack. PS: Anyway, I can suggest one another way to do this task: use ng_ksocket with socket type divert and ipfw divert rule. But this way have disadvantage: packet can easy enter to infinite loop. So use with care. > > -- Brooks -- Vladimir B. Grebenschikov <[EMAIL PROTECTED]> SWsoft Inc. ___ [EMAIL PROTECTED] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Re: ADSL PPoA or RFC1483, any solutions ?
On Thu, Oct 02, 2003 at 09:53:08AM +0200, Harti Brandt wrote: > Does PPPoA really need signalling? I tried to find any pointers to PPPoA > specification, but this seems to be not easy to find. I was probably half asleep when I wrote that answer :) it's been a stressful week. I should correct myself - more often than not ISPs just use PVCs. The userland PPP could probably be run on top of a device node exporting the PVC. I prefer the idea of in-kernel ppp, though, for 1Mbps+ xDSL use. BMS ___ [EMAIL PROTECTED] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "[EMAIL PROTECTED]"
