in_cksum() for ip packets with multiple mbufs

[kamal kc]

> > 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.
 
one thing i would like to ask?

does it make any difference if i free the mbuf 'm'
passed to if_output() and pass my own mbuf to 
if_output. 

is the original mbuf referenced by any
other pointers or global variables ?? 

i couldn't figure out much from the sources.
 
>     - If you are not *REALLY* copying the data of
> the mbuf, then
>       the name of copy_the_memorybuffer() is very
> confusing.

i didn't showed in the above code snippet but
actually i am copying the data contained in the 
mbufs in a character array. 

my purpose is to compress the data contained in
the ip packet
 
>     - What is the magic constant 64 that is assigned
> to ip_tos?

that was just to see that i could actually modify
the ip header.

> 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);
>     }

thanks i will try this code and try to make the 
code simpler next time.

> 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.

i am new to kernel sources and kernel programming and
thank you for informing me on the diff(1).

thanks to you that the problem was solved (i don't 
know if it is completely ok). i found that
   - i had made mistake in computing checksum. 
      earlier checksum was computed over the whole 
dat

I may soon put up patchfiles on web. 
thanks.



 


__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com