Re: Problem with short udp packets

On Sat, 03 Jan 2004 21:57:07 +0100
Olaf Titz <[email protected]> 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









--
Message sent by the [email protected] mailing list.
Unsubscribe: mail [email protected], "unsubscribe cipe-l" in body
Other commands available with "help" in body to the same address.
CIPE info and list archive: <URL:http://sites.inka.de/~bigred/devel/cipe.html>