Return to BSD News archive
Xref: sserve comp.os.386bsd.bugs:2215 comp.protocols.ppp:4088 Newsgroups: comp.os.386bsd.bugs,comp.protocols.ppp Path: sserve!newshost.anu.edu.au!munnari.oz.au!bunyip.cc.uq.oz.au!harbinger.cc.monash.edu.au!msuinfo!agate!howland.reston.ans.net!math.ohio-state.edu!jussieu.fr!univ-lyon1.fr!frmug.fr.net!fasterix.frmug.fr.net!pb From: pb@fasterix.frmug.fr.net (Pierre Beyssac) Subject: Re: priority queuing bug in *BSD ppp (if_ppp.c) Keywords: ppp BSD References: <1994May7.004353.695@fasterix.frmug.fr.net> <1994May10.130723.28188@zen.void.oz.au> Organization: considered harmful Date: Wed, 11 May 1994 19:38:39 GMT Message-ID: <1994May11.193839.3217@fasterix.frmug.fr.net> Lines: 49 In article <1994May10.130723.28188@zen.void.oz.au>, Simon J. Gerraty <sjg@zen.void.oz.au> wrote: >> if (INTERACTIVE(ntohs(p & 0xffff)) || INTERACTIVE(ntohs(p >> 16))) > >I suspect that > > if (INTERACTIVE(ntohs(p) & 0xffff) || INTERACTIVE(ntohs(p) >> 16)) > >is more likely to resolve the ntohs() problem without changing the >meaning of the code. ntohs(p >> 16) and ntohs(p) >> 16 can >produce _very_ different results. Well, if you really really want something strictly equivalent to the original code, it would look more like : /* no change to the original on the next line -- added for clarity */ register int p = ((int *)ip)[ip->ip_hl]; if (INTERACTIVE(ntohl(p) & 0xffff) || INTERACTIVE(ntohl(p) >> 16)) (ntohl instead of ntohs) This assumes that ntohl(0x04030201) == 0x01020304 on a little endian machine. I'm not very sure, but it might be 0x02010403 instead... Since the port numbers are two separate shorts and not a single int, it is better to use ntohs. Granted, there *is* a difference in the code I gave with the original :-) It will compare the tcp source port first on some machines, and the tcp dest port first on other machines. I don't think this is really a problem :-) The strictly correct way to handle this would be something like : struct tcphdr *tp = (struct tcphdr *)((int *)ip + ip->ip_hl); if (INTERACTIVE(ntohs(tp->th_sport)) || INTERACTIVE(ntohs(tp->th_dport))) But I'm not sure it's worth the trouble :-) (Note the source port is tested first because it is more likely to be the telnet/rlogin/finger port) Also, INTERACTIVE is a macro and evaluates its argument twice : more optimization could be done... -- Pierre Beyssac FreeBSD@home: pb@fasterix.frmug.fr.net FreeBSD, NetBSD, Linux -- Il y a moins bien, mais c'est plus cher. You can also get less bang for more bucks. (translation F. Berjon)