<< | Thread Index | >> ]    [ << | Date Index | >> ]

To: Olaf Titz <olaf,AT,bigred,DOT,inka,DOT,de>
Subject: Re: Problem with short udp packets
From: Stephan von Krawczynski <skraw,AT,ithnet,DOT,com>
Date: Sun, 4 Jan 2004 12:51:48 +0100
Cc: cipe-l,AT,inka,DOT,de
In-reply-to: <E1AcspX-00031W-00@bigred.inka.de>
Organization: ith Kommunikationstechnik GmbH
References: <20031228123556.2854e613.skraw@ithnet.com><E1AcspX-00031W-00@bigred.inka.de>

On Sat, 03 Jan 2004 21:57:07 +0100
Olaf Titz <olaf,AT,bigred,DOT,inka,DOT,de> wrote:

> > kernel: UDP: short packet: a.b.c.d:40025 1528/1512 to e.f.g.h:40025
> >
> > on the e.f.g.h side. The interesting part about it is that all this udp
> > packets contain an ID-field of 0, whereas the normal data packets are
> > counted up. After these appear the throughput seems to drop dramatically.
> > Can this be a problem in key exchange?
> 
> This looks like these are in fact the KX packets. Apparently the KX
> packets, which are sent out via the regular UDP sendmsg() mechanism,
> are treated differently wrt. IP ID generation than data packets, where
> we fill all IP headers ourselves. Being able to tell KX packets apart
> this way is a security problem, this must be fixed.
> 
> Which kernel version? This stuff is highly kernel version dependent.
> 
> For the original problem, can you try if the problem is still present
> in the CVS version?
> 
> Olaf

Hi Olaf,

I found this one, and it is pretty - well - odd. It does only occur under
kernel 2.4 and the reason is this code in output.c :

        /*
         *      Push down and install the CIPE/UDP header.
         */
        iph->version    =       4;
        iph->ihl        =       sizeof(struct iphdr)>>2;
        iph->tos        =       tos;
        iph->tot_len    =       htons(skb->len);
        ip_ident(iph, &rt->u.dst);
        iph->frag_off   =       df;
        iph->ttl        =       ttl;
        iph->protocol   =       IPPROTO_UDP;
        iph->saddr      =       rt->rt_src;
        iph->daddr      =       rt->rt_dst;

        ip_send_check(iph);

You won't believe it, this code has a major bug. The correct version looks 
like
this:

        /*
         *      Push down and install the CIPE/UDP header.
         */
        iph->version    =       4;
        iph->ihl        =       sizeof(struct iphdr)>>2;
        iph->tos        =       tos;
        iph->tot_len    =       htons(skb->len);
        iph->frag_off   =       df;
        iph->ttl        =       ttl;
        iph->protocol   =       IPPROTO_UDP;
        iph->saddr      =       rt->rt_src;
        iph->daddr      =       rt->rt_dst;

        ip_ident(iph, &rt->u.dst);
        ip_send_check(iph);

And the reason is in the kernel tree under /include/net/ip.h:

static inline void ip_select_ident(struct iphdr *iph, struct dst_entry *dst,
struct sock *sk)
{
        if (iph->frag_off&__constant_htons(IP_DF)) {
                /* This is only to work around buggy Windows95/2000
                 * VJ compression implementations.  If the ID field
                 * does not change, they drop every other packet in
                 * a TCP stream using header compression.
                 */ 
                iph->id = ((sk && sk->daddr) ? 
htons(sk->protinfo.af_inet.id++)
: 0);
        } else
                __ip_select_ident(iph, dst);
}

As you can see the ident-code relies on setting the frag_off as well as
sk->daddr. If you do not do that, everything runs well under low load. But as
soon as you have high load (and lars' ignoredf/forcemtu patch) you notice my
described phenomenon, remember skbs are heavily reused mem and may contain 
just
about any data...
It took me several days to figure that one out.

Few lines below this problem in output.c there is yet another one. I wonder if
anybody ever noticed that the interface TX byte-counters under kernel 2.4 are
plain wrong, doubled to name it. Reason is this:

        tunnel->stat.tx_bytes += skb->len;
        tunnel->stat.tx_packets++;
        dprintk(DEB_OUT, (KERN_INFO
                          "%s: sending %d from %s:%d to %s:%d\n",
                          dev->name, skb->len,
                          cipe_ntoa(iph->saddr), ntohs(udph->source),
                          cipe_ntoa(iph->daddr), ntohs(udph->dest)));
        if (tunnel->sockshost)
            dprintk(DEB_OUT, (KERN_INFO "%s: via SOCKS to %s:%d\n", dev->name,
                              cipe_ntoa(tunnel->sockshost),
                              ntohs(tunnel->socksport)));  
#if 0
        dprintk(DEB_OUT, (KERN_INFO "dst: (%d,%d) %s %d %d\n",
                          skb->dst->refcnt, skb->dst->use,
                          skb->dst->dev->name, skb->dst->pmtu,
                          skb->dst->error));
#endif
#ifdef DEBUG
        if (cipe_debug&DEB_PKOU)
            cipe_dump_packet("sending", skb, 0);
#endif
        nf_conntrack_null(skb);
#ifdef LINUX_24
        {
            int err = NF_HOOK(PF_INET, NF_IP_LOCAL_OUT, skb, NULL,
                              rt->u.dst.dev, ip_send);
            if (err==NET_XMIT_SUCCESS || err==NET_XMIT_CN) {
                tunnel->stat.tx_bytes += skb->len;
                tunnel->stat.tx_packets++;
            } else {
                tunnel->stat.tx_errors++;
                tunnel->stat.tx_aborted_errors++;
            }
        }
#else
        ip_send(skb);
#endif

As you can see they are simply added _twice_. Obviously the first two lines
should move inside the #else branch (which is kernel 2.2).

I would suggest to make a 1.5.5 release containing Lars Fennebergs
ignoredf/forcemtu patch (works really well), as well as fixes for above stuff
(I don't attach a patch because they are so simple). This should be the first
release of cipe really working with 2.4 kernel and mtu 1500 ;-)

Regards,
Stephan


<< | Thread Index | >> ]    [ << | Date Index | >> ]