From owner-freebsd-hackers@FreeBSD.ORG Sun Oct 23 12:06:20 2005 Return-Path: X-Original-To: freebsd-hackers@freebsd.org Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 787A316A41F; Sun, 23 Oct 2005 12:06:20 +0000 (GMT) (envelope-from keramida@ceid.upatras.gr) Received: from rosebud.otenet.gr (rosebud.otenet.gr [195.170.0.94]) by mx1.FreeBSD.org (Postfix) with ESMTP id 8000B43D46; Sun, 23 Oct 2005 12:06:18 +0000 (GMT) (envelope-from keramida@ceid.upatras.gr) Received: from flame.pc (aris.bedc.ondsl.gr [62.103.39.226]) by rosebud.otenet.gr (8.13.4/8.13.4/Debian-1) with SMTP id j9NC6G3M008871; Sun, 23 Oct 2005 15:06:17 +0300 Received: from flame.pc (flame [127.0.0.1]) by flame.pc (8.13.4/8.13.4) with ESMTP id j9NC4gEG030056; Sun, 23 Oct 2005 15:04:42 +0300 (EEST) (envelope-from keramida@ceid.upatras.gr) Received: (from keramida@localhost) by flame.pc (8.13.4/8.13.4/Submit) id j9NC4gmu030055; Sun, 23 Oct 2005 15:04:42 +0300 (EEST) (envelope-from keramida@ceid.upatras.gr) Date: Sun, 23 Oct 2005 15:04:42 +0300 From: Giorgos Keramidas To: kamal kc Message-ID: <20051023120442.GB29924@flame.pc> References: <20051023101919.GA9957@flame.pc> <20051023111333.53650.qmail@web35701.mail.mud.yahoo.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20051023111333.53650.qmail@web35701.mail.mud.yahoo.com> X-Mailman-Approved-At: Sun, 23 Oct 2005 12:13:31 +0000 Cc: freebsd , freebsd Subject: Re: in_cksum() for ip packets with multiple mbufs X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 23 Oct 2005 12:06:20 -0000 On 2005-10-23 04:13, kamal kc wrote: > /* the argument m is the (struct mbuf *) that > * contains the packet data > */ > > void copy_the_memorybuffer(struct mbuf **m) > { > struct mbuf *mbuf_pointer=*m; > struct mbuf **next_packet; > > next_packet=&mbuf_pointer; > > struct ip *my_ip_hdr; > my_ip_hdr=mtod((*next_packet),struct ip *); > my_ip_hdr->ip_tos=64; > my_ip_hdr->ip_sum=0; > > my_ip_hdr->ip_sum= > in_cksum((*next_packet),(my_ip_hdr->ip_hl<<2)); > ....... > } Why all this pointer fun? How do you know that it's safe to dereference `m' when you do: struct mbuf *mbuf_pointer=*m; Why are you dereferencing `m' once to obtain mbuf_pointer and then taking the address of that to obtain next_packet, when you could just do: next_packet = m; There are also several other problems with copy_the_memorybuffer() as it's written above: - It's considered bad style to mix declarations and code the way you have done above - It is probably better to return the copy of the mbuf you're fiddling with, instead of modifying in place a parameter of the function. - If you are not *REALLY* copying the data of the mbuf, then the name of copy_the_memorybuffer() is very confusing. - What is the magic constant 64 that is assigned to ip_tos? What you probably want to do is something like: void ip_set_type_of_service(struct mbuf *m) { struct ip *iph; assert(m != NULL); iph = mtod(m, struct ip *); iph->ip_tos = IPTOS_PREC_IMMEDIATE; iph->ip_sum = 0; iph->ip_sum = in_cksum((uint16_t *)iph, iph->ip_hl << 2); } but that's not copying anything. > but still it doesn't seem to work. and the problem > is still there. You have obviously made a lot of changes that we haven't seen yet. Instead of posting snippets here and there, save a copy of the original sources somewhere, then a copy of the new sources, and run diff(1) on the two directories to extract *ALL* the changes. $ cd /usr/src $ diff -ruN src.old/ src/ > /tmp/patchfile and put the patchfile somewhere online where we can have a look at all the changes.