From owner-freebsd-net Mon Jan 8 23: 2:44 2001 Delivered-To: freebsd-net@freebsd.org Received: from gatekeeper.tsc.tdk.com (gatekeeper.tsc.tdk.com [207.113.159.21]) by hub.freebsd.org (Postfix) with ESMTP id E62DA37B400 for ; Mon, 8 Jan 2001 23:02:26 -0800 (PST) Received: from imap.gv.tsc.tdk.com (imap.gv.tsc.tdk.com [192.168.241.198]) by gatekeeper.tsc.tdk.com (8.8.8/8.8.8) with ESMTP id XAA22130; Mon, 8 Jan 2001 23:02:19 -0800 (PST) (envelope-from gdonl@tsc.tdk.com) Received: from salsa.gv.tsc.tdk.com (salsa.gv.tsc.tdk.com [192.168.241.194]) by imap.gv.tsc.tdk.com (8.9.3/8.9.3) with ESMTP id XAA13421; Mon, 8 Jan 2001 23:02:18 -0800 (PST) (envelope-from Don.Lewis@tsc.tdk.com) Received: (from gdonl@localhost) by salsa.gv.tsc.tdk.com (8.8.5/8.8.5) id XAA15712; Mon, 8 Jan 2001 23:02:17 -0800 (PST) From: Don Lewis Message-Id: <200101090702.XAA15712@salsa.gv.tsc.tdk.com> Date: Mon, 8 Jan 2001 23:02:17 -0800 In-Reply-To: <20001231210746.A81834@skriver.dk> References: <20001218182600.C1856@skriver.dk> <20001219222730.A29741@skriver.dk> <200012201046.CAA19456@salsa.gv.tsc.tdk.com> <20001220155118.N81814@skriver.dk> <20001231210746.A81834@skriver.dk> X-Mailer: Mail User's Shell (7.2.6 beta(5) 10/07/98) To: Jesper Skriver , Don Lewis Subject: ICMP error processing (was: Re: what to do now ? Was: cvs commit: src/sys/netinet ip_icmp.c tcp_subr.c tcp_var.h) Cc: freebsd-net@FreeBSD.ORG Sender: owner-freebsd-net@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.org [ cc: trimmed ] On Dec 31, 9:07pm, Jesper Skriver wrote: } Subject: Re: what to do now ? Was: cvs commit: src/sys/netinet ip_icmp.c } On Wed, Dec 20, 2000 at 03:51:18PM +0100, Jesper Skriver wrote: } > On Wed, Dec 20, 2000 at 02:46:21AM -0800, Don Lewis wrote: } > } > > } @@ -714,6 +715,15 @@ } > > } (lport && inp->inp_lport != lport) || } > > } (laddr.s_addr && inp->inp_laddr.s_addr != laddr.s_addr) || } > > } (fport && inp->inp_fport != fport)) { } > > } + inp = inp->inp_list.le_next; } > > } + continue; } > > } > > Wouldn't it be more cleaner (gets rid of the loop) and more efficient (if } > > we're getting blasted with ICMP messages) to use in_pcblookup_hash()? } > } > I didn't change the loop, but I'll have a look at this code, to see if } > we can improve it, but again, to get moving, I'd like to commit this, } > and leave this for a later improvement, ok ? } } I've looked at this, and as far as I can see we cannot use } in_pcblookup_hash, as it lookup a single session, and the code can in } other cases act on multiple sessions, path MTU discovery is such a case. Actually, I don't see where path MTU discovery acts on multiple sessions. It looks like we pass a non-NULL third argument to tcp_ctlinput(), which causes tcp_ctlinput() to pass the optional IP address and port information to in_pcbnotify(), and in_pcbnotify() doesn't handle the PRC_MSGSIZE specially, like it does with PRC_IS_REDIRECT and PRC_HOSTDEAD. Bug? Should path MTU discovery be handled by pfctlinput() to notify all the protocols? My concern is that having to search through all the PCBs each time an ICMP error message is received makes it too easy to DoS a busy server that has a lot of open sockets. The way icmp -> notify code is structured is kind of strange anyway (there's a nice diagram in TCP/IP Illustrated Volume 2 - Figure 22.32). I think it would make more sense to validate the ICMP error and redirect packets fairly early on by doing the PCB lookup and dropping any ICMP packets that don't have a matching PCB. Redirects *and* path MTU discovery messages would then be handled by pfctlinput(), and the other errors by the protocol specific handler. I think it is a mistake to call the same *_ctlinput() functions from both icmp_input() for one connection and pfctlinput() for many connections. I think the call chain should look something like the following, though some more thought needs to be given to sensibly unwinding things to avoid duplicate code and avoid doing duplicate work. icmp_input->{tcp,udp}_ctlinput1->in_pcbnotify1-> {pfctlinput->{tcp,udp}_ctlinput->in_pcbnotify->..., {tcp,udp}_notify, ...} In the current code, it looks like an ICMP error that the addresses and ports zeroed out will erroneously affect many sessions (what do the RFCs say?). Another bit of strangeness is pfctlinput(PRC_IF{UP,DOWN}, ...) which is passed the interface address, which is matched against the *destination* address in the TCP and UDP PCBs. Fortunately these protocols seem to ignore these commands ... To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-net" in the body of the message