Checksum problem in tcp-window-tracking

Checksum problem in tcp-window-tracking

[Martin Josefsson]

[-- Attachment #1: Type: text/plain, Size: 1321 bytes --]

Hi Jozsef

Upgraded my kernel and started seeing lots of complaints from the
tcp-window-tracking code.

I only get the complaints on localhost<->localhost traffic, all other
traffic is complaint-free and working great :)

Jun 12 21:14:30 tux ip_conntrack_tcp: INVALID: bad T IN= OUT= MAC=
SRC=127.0.0.1 DST=127.0.0.1 LEN=93 TOS=00 PREC=0x00 TTL=64 ID=6306 DF
PROTO=TCP SPT=6667 DPT=32813 SEQ=2226635880 ACK=2237541190 WINDOW=4095
ACK PSH URGP=0

"bad T" can only be this code from unclean()

        /* Checksum invalid?  Ignore. */
        /* FIXME: Source route IP option packets --RR */
        if (csum_tcpudp_magic(iph->saddr, iph->daddr,
                              tcplen, IPPROTO_TCP,
                              skb->ip_summed == CHECKSUM_HW
                                ? skb->csum
                                : skb_checksum(skb, iph->ihl*4, tcplen, 0))) {
                if (NET_RATELIMIT(ip_ct_tcp_log_invalid))
                        nf_log_packet(PF_INET, 0, skb, NULL, NULL,
                                  "ip_conntrack_tcp: INVALID: "
                                  "bad TCP checksum ");
                return 1;
        }

According to tcpdump the checksum is correct (otherwise we'd have a
serious bug in the tcp-stack :)

Any ideas?

-- 
/Martin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Checksum problem in tcp-window-tracking

[Patrick McHardy]

Martin Josefsson wrote:
> Hi Jozsef
> 
> Upgraded my kernel and started seeing lots of complaints from the
> tcp-window-tracking code.
> 
> I only get the complaints on localhost<->localhost traffic, all other
> traffic is complaint-free and working great :)
> 
> Jun 12 21:14:30 tux ip_conntrack_tcp: INVALID: bad T IN= OUT= MAC=
> SRC=127.0.0.1 DST=127.0.0.1 LEN=93 TOS=00 PREC=0x00 TTL=64 ID=6306 DF
> PROTO=TCP SPT=6667 DPT=32813 SEQ=2226635880 ACK=2237541190 WINDOW=4095
> ACK PSH URGP=0
> 
> "bad T" can only be this code from unclean()
> 
>         /* Checksum invalid?  Ignore. */
>         /* FIXME: Source route IP option packets --RR */
>         if (csum_tcpudp_magic(iph->saddr, iph->daddr,
>                               tcplen, IPPROTO_TCP,
>                               skb->ip_summed == CHECKSUM_HW
>                                 ? skb->csum
>                                 : skb_checksum(skb, iph->ihl*4, tcplen, 0))) {
>                 if (NET_RATELIMIT(ip_ct_tcp_log_invalid))
>                         nf_log_packet(PF_INET, 0, skb, NULL, NULL,
>                                   "ip_conntrack_tcp: INVALID: "
>                                   "bad TCP checksum ");
>                 return 1;
>         }
> 
> According to tcpdump the checksum is correct (otherwise we'd have a
> serious bug in the tcp-stack :)
> 
> Any ideas?
> 

It probably fails on the outgoing path, where the meaning of skb->csum
is different. When CHECKSUM_HW on an outgoing packet is set, the device
should compute and store the checksum at skb->h.raw + skb->csum. tcp
sets CHECKSUM_HW in tcp_sendmsg() and do_tcp_sendpages() when the device
has NETIF_F_NO_CSUM set. Using skb_checksum() unconditionally should
fix it.

Regards
Patrick

Re: Checksum problem in tcp-window-tracking

[Jozsef Kadlecsik]

On Sun, 13 Jun 2004, Patrick McHardy wrote:

> > I only get the complaints on localhost<->localhost traffic, all other
> > traffic is complaint-free and working great :)
> >
> > Jun 12 21:14:30 tux ip_conntrack_tcp: INVALID: bad T IN= OUT= MAC=
> > SRC=127.0.0.1 DST=127.0.0.1 LEN=93 TOS=00 PREC=0x00 TTL=64 ID=6306 DF
> > PROTO=TCP SPT=6667 DPT=32813 SEQ=2226635880 ACK=2237541190 WINDOW=4095
> > ACK PSH URGP=0
> >
> > "bad T" can only be this code from unclean()
> >
> >         /* Checksum invalid?  Ignore. */
> >         /* FIXME: Source route IP option packets --RR */
> >         if (csum_tcpudp_magic(iph->saddr, iph->daddr,
> >                               tcplen, IPPROTO_TCP,
> >                               skb->ip_summed == CHECKSUM_HW
> >                                 ? skb->csum
> >                                 : skb_checksum(skb, iph->ihl*4, tcplen, 0))) {
> >                 if (NET_RATELIMIT(ip_ct_tcp_log_invalid))
> >                         nf_log_packet(PF_INET, 0, skb, NULL, NULL,
> >                                   "ip_conntrack_tcp: INVALID: "
> >                                   "bad TCP checksum ");
> >                 return 1;
> >         }
> >
> > According to tcpdump the checksum is correct (otherwise we'd have a
> > serious bug in the tcp-stack :)
>
> It probably fails on the outgoing path, where the meaning of skb->csum
> is different. When CHECKSUM_HW on an outgoing packet is set, the device
> should compute and store the checksum at skb->h.raw + skb->csum. tcp
> sets CHECKSUM_HW in tcp_sendmsg() and do_tcp_sendpages() when the device
> has NETIF_F_NO_CSUM set. Using skb_checksum() unconditionally should
> fix it.

Thank you both the report and the explanation. I'm going to fix it
by checking the hook in order not to loose the fast path in the
hardware-checksummed case.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@sunserv.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: Checksum problem in tcp-window-tracking

[Jozsef Kadlecsik]

On Sun, 13 Jun 2004, Jozsef Kadlecsik wrote:

> On Sun, 13 Jun 2004, Patrick McHardy wrote:
>
> > > I only get the complaints on localhost<->localhost traffic, all other
> > > traffic is complaint-free and working great :)
> > >
> > > Jun 12 21:14:30 tux ip_conntrack_tcp: INVALID: bad T IN= OUT= MAC=
> > > SRC=127.0.0.1 DST=127.0.0.1 LEN=93 TOS=00 PREC=0x00 TTL=64 ID=6306 DF
> > > PROTO=TCP SPT=6667 DPT=32813 SEQ=2226635880 ACK=2237541190 WINDOW=4095
> > > ACK PSH URGP=0
> Thank you both the report and the explanation. I'm going to fix it
> by checking the hook in order not to loose the fast path in the
> hardware-checksummed case.

I committed the fixed version of the patch in cvs. Besides the fix the
talkative log messages were trimmed in order to fit (better) into the
ULOG buffer.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@sunserv.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: Checksum problem in tcp-window-tracking

[Patrick McHardy]

Hi Jozsef,

Jozsef Kadlecsik wrote:
> I committed the fixed version of the patch in cvs. Besides the fix the
> talkative log messages were trimmed in order to fit (better) into the
> ULOG buffer.

I think there is still a problem.

+       if (csum_tcpudp_magic(iph->saddr, iph->daddr,
+                             tcplen, IPPROTO_TCP,
+                             skb->ip_summed == CHECKSUM_HW
+                             && hooknum == NF_IP_PRE_ROUTING
+                               ? skb->csum
+                               : skb_checksum(skb, iph->ihl*4, tcplen, 
0))) {

On the outgoing path, the NIC is supposed to checksum the packet when
CHECKSUM_HW is set, so it hasn't got a valid checksum when the check is
performed. Do we need to check the checksum in LOCAL_OUT at all ?
Other than from raw-sockets, invalid packets can not occur.

Regards
Patrick

PS: Martin, how did you trigger these errors ?

Re: Checksum problem in tcp-window-tracking

[Martin Josefsson]

[-- Attachment #1: Type: text/plain, Size: 1415 bytes --]

On Tue, 2004-06-15 at 20:32, Patrick McHardy wrote:

Hi Patrick

> Jozsef Kadlecsik wrote:
> > I committed the fixed version of the patch in cvs. Besides the fix the
> > talkative log messages were trimmed in order to fit (better) into the
> > ULOG buffer.
> 
> I think there is still a problem.
> 
> +       if (csum_tcpudp_magic(iph->saddr, iph->daddr,
> +                             tcplen, IPPROTO_TCP,
> +                             skb->ip_summed == CHECKSUM_HW
> +                             && hooknum == NF_IP_PRE_ROUTING
> +                               ? skb->csum
> +                               : skb_checksum(skb, iph->ihl*4, tcplen, 
> 0))) {
> 
> On the outgoing path, the NIC is supposed to checksum the packet when
> CHECKSUM_HW is set, so it hasn't got a valid checksum when the check is
> performed. Do we need to check the checksum in LOCAL_OUT at all ?
> Other than from raw-sockets, invalid packets can not occur.
> 
> Regards
> Patrick
> 
> PS: Martin, how did you trigger these errors ?

Upgraded the kernel on my desktop-machine :)

Kernel 2.6.7-rc3-bk4 + tcp-windowtracking without the latest change made
by Jozsef.

It's just local imap and irc servers (bitlbee for icq) and clients
connecting to 127.0.0.1

I get one logentry per packet or so, in both directions.

I now have 28.000 lines of these complains in a logfile :)

-- 
/Martin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Checksum problem in tcp-window-tracking

[Jozsef Kadlecsik]

Hi Patrick,

On Tue, 15 Jun 2004, Patrick McHardy wrote:

> > I committed the fixed version of the patch in cvs. Besides the fix the
> > talkative log messages were trimmed in order to fit (better) into the
> > ULOG buffer.
>
> I think there is still a problem.
>
> +       if (csum_tcpudp_magic(iph->saddr, iph->daddr,
> +                             tcplen, IPPROTO_TCP,
> +                             skb->ip_summed == CHECKSUM_HW
> +                             && hooknum == NF_IP_PRE_ROUTING
> +                               ? skb->csum
> +                               : skb_checksum(skb, iph->ihl*4, tcplen,
> 0))) {
>
> On the outgoing path, the NIC is supposed to checksum the packet when
> CHECKSUM_HW is set, so it hasn't got a valid checksum when the check
> is performed.  Do we need to check the checksum in LOCAL_OUT at all ?
> Other than from raw-sockets, invalid packets can not occur.

Finally, finally I see. Then we have to get rid of the checksum checking
on the outgoing path, indeed:

+       if (hooknum == NF_IP_PRE_ROUTING
+	    && csum_tcpudp_magic(iph->saddr, iph->daddr,
+                                tcplen, IPPROTO_TCP,
+                                skb->ip_summed == CHECKSUM_HW
+                                  ? skb->csum
+                                  : skb_checksum(skb, iph->ihl*4, tcplen, 0))) {

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@sunserv.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: Checksum problem in tcp-window-tracking

[Jozsef Kadlecsik]

Hi Martin,

On Sat, 12 Jun 2004, Martin Josefsson wrote:

> I only get the complaints on localhost<->localhost traffic, all other
> traffic is complaint-free and working great :)
>
> Jun 12 21:14:30 tux ip_conntrack_tcp: INVALID: bad T IN= OUT= MAC=
> SRC=127.0.0.1 DST=127.0.0.1 LEN=93 TOS=00 PREC=0x00 TTL=64 ID=6306 DF
> PROTO=TCP SPT=6667 DPT=32813 SEQ=2226635880 ACK=2237541190 WINDOW=4095
> ACK PSH URGP=0
>
> "bad T" can only be this code from unclean()

Why did the prefix get truncated to 'ip_conntrack_tcp: INVALID: bad T'???
Anything special with your kernel?

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@sunserv.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: Checksum problem in tcp-window-tracking

[Martin Josefsson]

On Mon, 14 Jun 2004, Jozsef Kadlecsik wrote:

> Hi Martin,

Hi Jozsef

> > Jun 12 21:14:30 tux ip_conntrack_tcp: INVALID: bad T IN= OUT= MAC=
> > SRC=127.0.0.1 DST=127.0.0.1 LEN=93 TOS=00 PREC=0x00 TTL=64 ID=6306 DF
> > PROTO=TCP SPT=6667 DPT=32813 SEQ=2226635880 ACK=2237541190 WINDOW=4095
> > ACK PSH URGP=0
> >
> > "bad T" can only be this code from unclean()
>
> Why did the prefix get truncated to 'ip_conntrack_tcp: INVALID: bad T'???
> Anything special with your kernel?

No nothing special, using ULOG for logging.
I thought it was weird as well but I didn't trace the problem. Maybe I'll
look at it later today.

/Martin

Re: Checksum problem in tcp-window-tracking

[Jozsef Kadlecsik]

On Mon, 14 Jun 2004, Martin Josefsson wrote:

> > Why did the prefix get truncated to 'ip_conntrack_tcp: INVALID: bad T'???
> > Anything special with your kernel?
>
> No nothing special, using ULOG for logging.
> I thought it was weird as well but I didn't trace the problem. Maybe I'll
> look at it later today.

I found it: the ulog prefix length is 32. I'll try to cut back the strings
to fit into that buffer.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@sunserv.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: Checksum problem in tcp-window-tracking

[Martin Josefsson]

On Mon, 14 Jun 2004, Jozsef Kadlecsik wrote:

> I found it: the ulog prefix length is 32. I'll try to cut back the strings
> to fit into that buffer.

Ah ok, I thought it might be something like that.

/Martin