Date: Sun, 23 Oct 2005 15:04:42 +0300 From: Giorgos Keramidas <keramida@ceid.upatras.gr> To: kamal kc <kamal_ckk@yahoo.com> Cc: freebsd <freebsd-hackers@freebsd.org>, freebsd <freebsd-net@freebsd.org> Subject: Re: in_cksum() for ip packets with multiple mbufs Message-ID: <20051023120442.GB29924@flame.pc> In-Reply-To: <20051023111333.53650.qmail@web35701.mail.mud.yahoo.com> References: <20051023101919.GA9957@flame.pc> <20051023111333.53650.qmail@web35701.mail.mud.yahoo.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2005-10-23 04:13, kamal kc <kamal_ckk@yahoo.com> 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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20051023120442.GB29924>