Date: Sun, 26 Jul 2015 15:16:42 +0200 From: Kristof Provost <kp@FreeBSD.org> To: Colin Percival <cperciva@freebsd.org> Cc: Mark Felder <feld@FreeBSD.org>, freebsd-xen@freebsd.org, gnn@freebsd.org Subject: Re: Networking under Xen Message-ID: <20150726131642.GC2484@vega.codepro.be> In-Reply-To: <55A611B1.6000000@freebsd.org> References: <4E7B7075-4E0D-4EA7-9F5D-6D252CFBD487@gmail.com> <1436890526.3162974.323521249.6B73E6E2@webmail.messagingengine.com> <55A55AE8.4090101@freebsd.org> <1436901780.3211878.323698017.360F8D73@webmail.messagingengine.com> <20F2398D-ECDF-4CF4-966D-18C894779C4C@FreeBSD.org> <55A611B1.6000000@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2015-07-15 00:54:25 (-0700), Colin Percival <cperciva@freebsd.org> wrote:
> In my tests, deleting these lines from pf_ioctl.c
>
> 3570 /* We need a proper CSUM befor we start (s. OpenBSD ip_output) */
> 3571 if ((*m)->m_pkthdr.csum_flags & CSUM_DELAY_DATA) {
> 3572 in_delayed_cksum(*m);
> 3573 (*m)->m_pkthdr.csum_flags &= ~CSUM_DELAY_DATA;
> 3574 }
>
> unbreaks pf+TSO on EC2 instances. I'm not entirely sure why these lines
> are there in the first place, which is why I didn't want to simply go in
> and remove them -- but it may be that wrapping those lines in something
> like "if ((csum_flags & CSUM_TSO) == 0)" would solve the problem without
> breaking anything else.
>
I think the reason for this checksum calculation is that pf sometimes
modifies the packet, so it also updates the checksum.
In case of TSO (or CSUM_TCP) that doesn't actually work because the TCP
checksum is the pseudo IP(6) header checksum, not the final checksum.
Starting from a full checksum the update is correct however, so that's
why pf works on non-TSO interfaces.
It doesn't work on Xen TSO interfaces because (I assume) it expects to
get the pseudo header checksum, not the full checksum.
It's not entirely clear to my why it's not broken on my hardware (which
claims TSO support), but perhaps Xen is more picky than actual hardware.
Adding the if (CSUM_TSO) check will improve matters, but it will still
break on packets which get changed by pf (rdr/nat/...).
I think the real solution is to make pf_cksum_fixup() a bit more
intelligent. It should look at m->m_pkthdr.csum_flags to determine if it
should touch the checksum or not.
If CSUM_TCP is set we know we've got a pseudo header checksum and we
shouldn't touch it unless we've modified the source or destination IP
address (or length or protocol, but that's less likely).
If CSUM_TCP is not set we should have a full checksum and should do the
fixup as before.
Actually, perhaps CSUM_DELAY_DATA is better than CSUM_TCP, because we'll
have to do the same for UDP.
Regards,
Kristof
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150726131642.GC2484>