Re: in_cksum() for ip packets with multiple mbufs

From: Giorgos Keramidas (keramida_at_ceid.upatras.gr)
Date: 10/23/05

  • Next message: Volker: "Re: IPSec tcp session stalling ( me too ) ..." 
    Date: Sun, 23 Oct 2005 15:04:42 +0300
To: kamal kc <kamal_ckk@yahoo.com>

    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.
    _______________________________________________
    freebsd-net@freebsd.org mailing list
    http://lists.freebsd.org/mailman/listinfo/freebsd-net
    To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org"

  • Next message: Volker: "Re: IPSec tcp session stalling ( me too ) ..."

    Relevant Pages