Re: Checksum forwarding issue on XEN
On Thu, Nov 05, 2015 at 05:30:06PM +0100, Roger Pau Monné wrote:
> El 05/11/15 a les 17.00, Larry Baird ha escrit:
> > Roger,
> >
> >> Adding the persons that contributed that code in case they can shed some
> >> light.
> >>
> >> El 03/11/15 a les 21.12, Larry Baird ha escrit:
> >>> Has anybody made any progress on "Bug 188261 - [xen] FreeBSD DomU PVHVM
> >>> guests cannot 'route' traffic for other Xen PV guests on same Dom0 Host."
> >>> (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=188261)?
> >>>
> >>> The code for checksum calculation in the function xnb_add_mbuf_cksum()
> >>> looks
> >>> suspect.
> >>>
> >>> switch (iph->ip_p) {
> >>> case IPPROTO_TCP:
> >>> if (mbufc->m_pkthdr.csum_flags & CSUM_IP_VALID) {
> >>> size_t tcplen = ntohs(iph->ip_len) -
> >>> sizeof(struct ip);
> >>> struct tcphdr *th = (struct tcphdr*)(iph + 1);
> >>> th->th_sum = in_pseudo(iph->ip_src.s_addr,
> >>> iph->ip_dst.s_addr, htons(IPPROTO_TCP +
> >>> tcplen));
> >>> th->th_sum = in_cksum_skip(mbufc,
> >>> sizeof(struct ether_header) +
> >>> ntohs(iph->ip_len),
> >>> sizeof(struct ether_header) + (iph->ip_hl <<
> >>> 2));
> >>> }
> >>> break;
> >>> case IPPROTO_UDP:
> >>> if (mbufc->m_pkthdr.csum_flags & CSUM_IP_VALID) {
> >>> size_t udplen = ntohs(iph->ip_len) -
> >>> sizeof(struct ip);
> >>> struct udphdr *uh = (struct udphdr*)(iph + 1);
> >>> uh->uh_sum = in_pseudo(iph->ip_src.s_addr,
> >>> iph->ip_dst.s_addr, htons(IPPROTO_UDP +
> >>> udplen));
> >>> uh->uh_sum = in_cksum_skip(mbufc,
> >>> sizeof(struct ether_header) +
> >>> ntohs(iph->ip_len),
> >>> sizeof(struct ether_header) + (iph->ip_hl <<
> >>> 2));
> >>> }
> >>> break;
> >>> default:
> >>> break;
> >>> }
> >>>
> >>>
> >>> Both in_pseudo() and in_cksum_skip() set the same checksum. Does this
> >>> make since to anybody?
> >>
> >> The bug you are referring to affects FreeBSD when running as a guest
> >> using xen-netfront, but the code snipped above and the function
> >> referenced (xnb_add_mbuf_cksum) is only used on FreeBSD when running as
> >> a host (AKA Dom0) by xen-netback.
> >>
> >> TBH, I don't know that much about FreeBSD network subsystem to have an
> >> opinion, but it certainly looks weird. Patches are welcome :).
> >
> > Xyper-V has a similar forward issue. I found they were misusing csum_flags
> > and were always attempting to do checksum offloading if CSUM_IP_VALID was
> > set. I have given them a patch that fixes the issue. I was hoping that
> > Xen's issue was similar. I found the issue above by looking at all uses
> > of csum_flags in sys/dev/xen. It is hard to tell what the correct fix
> > is, without fulling understand the protocal used when communicating between
> > backend and frontend of Xen.
>
>
>
> > I am sure issue with XEN guest forwarding has to with checksum offloading.
> > If I am not misinterpreting your comments, I can ignore code in netback and
> > concentrate on code in netfront when trying to understand what is going
> > wrong.
>
> Yes, this issue is related to netfront (sys/dev/xen/netfront/netfront.c)
> only, netback code is not involved. The code related to the checksum
> stuff is on line ~1409 for the TX side, and around line 872 for the RX
> side AFAICT.
>
> You can find more information about the protocol itself in
> sys/xen/interface/io/netif.h.
>
> Adding Wei Liu who is also doing some work to improve netfront, and
> knows more about the protocol than myself.
>
Sorry for the late reply.
I think the place to look at, as Roger suggested, is netfront checksum
setup code.
The relevant flags in Xen network protocol are:
XEN_NETTXF_csum_blank -- protocol checksum field is blank
XEN_NETTXF_data_validated -- data has been validated
I'm not entirely sure FreeBSD netfront is mapping its own CSUM_* flags
correctly to Xen's.
I think at some point I can look into it, but not in the near future.
Wei.
> Roger.
___
freebsd-xen@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-xen
To unsubscribe, send any mail to "freebsd-xen-unsubscr...@freebsd.org"
Re: Checksum forwarding issue on XEN
Alan, > Though it looks weird, I think it's actually correct. th->th_sum lies > within the packet that it inspected by in_cksum_skip. in_cksum_skip > skips the ethernet and IP headers, but not the TCP header. So it > consumes whatever value was previously set in th_sum. In any case, it > will be easy to check this code by running the builtin unit tests. Just > build the kernel with XNB_DEBUG defined, instantiate an xnb interface, > and do "sysctl dev.xnb.0.unit_test_results". I agress the code looks strange but is probably correct as far as it goes. The code assumes that there aren't any IP options. I am not sure if this is a valid assumption or not. The code also assumes packet is IPv4. > FWIW, there is a performance problem with Xen and checksums. Whenever > FreeBSD is used as a dom0, checksum offloading should be disabled on the > netfronts. I don't know if the same problem exists on other OSes, but > it might. See xnb(4) for more details about this problem. Ok I read the CAVEATS on xnb(4). Let me play with that for a bit. I have an ugly code patch, that works for me with out disabling checksum offloading. Before I share, let me do a few more tests. Larry -- Larry Baird Global Technology Associates, Inc. 1992-2012| http://www.gta.com Celebrating Twenty Years of Software Innovation | Orlando, FL Email: l...@gta.com | TEL 407-380-0220 ___ freebsd-xen@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-xen To unsubscribe, send any mail to "freebsd-xen-unsubscr...@freebsd.org"
Re: Checksum forwarding issue on XEN
On 11/05/2015 09:00 AM, Larry Baird wrote:
> Roger,
>
>> Adding the persons that contributed that code in case they can shed some
>> light.
Removing John Suykerbuyk's outdated address. I don't have a current
address for him. And replacing my spectralogic email address with my
FreeBSD one.
>>
>> El 03/11/15 a les 21.12, Larry Baird ha escrit:
>>> Has anybody made any progress on "Bug 188261 - [xen] FreeBSD DomU PVHVM
>>> guests cannot 'route' traffic for other Xen PV guests on same Dom0 Host."
>>> (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=188261)?
>>>
>>> The code for checksum calculation in the function xnb_add_mbuf_cksum() looks
>>> suspect.
>>>
>>> switch (iph->ip_p) {
>>> case IPPROTO_TCP:
>>> if (mbufc->m_pkthdr.csum_flags & CSUM_IP_VALID) {
>>> size_t tcplen = ntohs(iph->ip_len) - sizeof(struct
>>> ip);
>>> struct tcphdr *th = (struct tcphdr*)(iph + 1);
>>> th->th_sum = in_pseudo(iph->ip_src.s_addr,
>>> iph->ip_dst.s_addr, htons(IPPROTO_TCP +
>>> tcplen));
>>> th->th_sum = in_cksum_skip(mbufc,
>>> sizeof(struct ether_header) +
>>> ntohs(iph->ip_len),
>>> sizeof(struct ether_header) + (iph->ip_hl <<
>>> 2));
>>> }
>>> break;
>>> case IPPROTO_UDP:
>>> if (mbufc->m_pkthdr.csum_flags & CSUM_IP_VALID) {
>>> size_t udplen = ntohs(iph->ip_len) - sizeof(struct
>>> ip);
>>> struct udphdr *uh = (struct udphdr*)(iph + 1);
>>> uh->uh_sum = in_pseudo(iph->ip_src.s_addr,
>>> iph->ip_dst.s_addr, htons(IPPROTO_UDP +
>>> udplen));
>>> uh->uh_sum = in_cksum_skip(mbufc,
>>> sizeof(struct ether_header) +
>>> ntohs(iph->ip_len),
>>> sizeof(struct ether_header) + (iph->ip_hl <<
>>> 2));
>>> }
>>> break;
>>> default:
>>> break;
>>> }
>>>
>>>
>>> Both in_pseudo() and in_cksum_skip() set the same checksum. Does this
>>> make since to anybody?
Though it looks weird, I think it's actually correct. th->th_sum lies
within the packet that it inspected by in_cksum_skip. in_cksum_skip
skips the ethernet and IP headers, but not the TCP header. So it
consumes whatever value was previously set in th_sum. In any case, it
will be easy to check this code by running the builtin unit tests. Just
build the kernel with XNB_DEBUG defined, instantiate an xnb interface,
and do "sysctl dev.xnb.0.unit_test_results".
>> The bug you are referring to affects FreeBSD when running as a guest
>> using xen-netfront, but the code snipped above and the function
>> referenced (xnb_add_mbuf_cksum) is only used on FreeBSD when running as
>> a host (AKA Dom0) by xen-netback.
>>
>> TBH, I don't know that much about FreeBSD network subsystem to have an
>> opinion, but it certainly looks weird. Patches are welcome :).
> Xyper-V has a similar forward issue. I found they were misusing csum_flags
> and were always attempting to do checksum offloading if CSUM_IP_VALID was
> set. I have given them a patch that fixes the issue. I was hoping that
> Xen's issue was similar. I found the issue above by looking at all uses
> of csum_flags in sys/dev/xen. It is hard to tell what the correct fix
> is, without fulling understand the protocal used when communicating between
> backend and frontend of Xen.
>
> I am sure issue with XEN guest forwarding has to with checksum offloading.
> If I am not misinterpreting your comments, I can ignore code in netback and
> concentrate on code in netfront when trying to understand what is going wrong.
>
> Thank you for some insite,
> Larry
>
>
FWIW, there is a performance problem with Xen and checksums. Whenever
FreeBSD is used as a dom0, checksum offloading should be disabled on the
netfronts. I don't know if the same problem exists on other OSes, but
it might. See xnb(4) for more details about this problem.
-Alan
___
freebsd-xen@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-xen
To unsubscribe, send any mail to "freebsd-xen-unsubscr...@freebsd.org"
Re: Checksum forwarding issue on XEN
El 05/11/15 a les 17.00, Larry Baird ha escrit:
> Roger,
>
>> Adding the persons that contributed that code in case they can shed some
>> light.
>>
>> El 03/11/15 a les 21.12, Larry Baird ha escrit:
>>> Has anybody made any progress on "Bug 188261 - [xen] FreeBSD DomU PVHVM
>>> guests cannot 'route' traffic for other Xen PV guests on same Dom0 Host."
>>> (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=188261)?
>>>
>>> The code for checksum calculation in the function xnb_add_mbuf_cksum() looks
>>> suspect.
>>>
>>> switch (iph->ip_p) {
>>> case IPPROTO_TCP:
>>> if (mbufc->m_pkthdr.csum_flags & CSUM_IP_VALID) {
>>> size_t tcplen = ntohs(iph->ip_len) - sizeof(struct
>>> ip);
>>> struct tcphdr *th = (struct tcphdr*)(iph + 1);
>>> th->th_sum = in_pseudo(iph->ip_src.s_addr,
>>> iph->ip_dst.s_addr, htons(IPPROTO_TCP +
>>> tcplen));
>>> th->th_sum = in_cksum_skip(mbufc,
>>> sizeof(struct ether_header) +
>>> ntohs(iph->ip_len),
>>> sizeof(struct ether_header) + (iph->ip_hl <<
>>> 2));
>>> }
>>> break;
>>> case IPPROTO_UDP:
>>> if (mbufc->m_pkthdr.csum_flags & CSUM_IP_VALID) {
>>> size_t udplen = ntohs(iph->ip_len) - sizeof(struct
>>> ip);
>>> struct udphdr *uh = (struct udphdr*)(iph + 1);
>>> uh->uh_sum = in_pseudo(iph->ip_src.s_addr,
>>> iph->ip_dst.s_addr, htons(IPPROTO_UDP +
>>> udplen));
>>> uh->uh_sum = in_cksum_skip(mbufc,
>>> sizeof(struct ether_header) +
>>> ntohs(iph->ip_len),
>>> sizeof(struct ether_header) + (iph->ip_hl <<
>>> 2));
>>> }
>>> break;
>>> default:
>>> break;
>>> }
>>>
>>>
>>> Both in_pseudo() and in_cksum_skip() set the same checksum. Does this
>>> make since to anybody?
>>
>> The bug you are referring to affects FreeBSD when running as a guest
>> using xen-netfront, but the code snipped above and the function
>> referenced (xnb_add_mbuf_cksum) is only used on FreeBSD when running as
>> a host (AKA Dom0) by xen-netback.
>>
>> TBH, I don't know that much about FreeBSD network subsystem to have an
>> opinion, but it certainly looks weird. Patches are welcome :).
>
> Xyper-V has a similar forward issue. I found they were misusing csum_flags
> and were always attempting to do checksum offloading if CSUM_IP_VALID was
> set. I have given them a patch that fixes the issue. I was hoping that
> Xen's issue was similar. I found the issue above by looking at all uses
> of csum_flags in sys/dev/xen. It is hard to tell what the correct fix
> is, without fulling understand the protocal used when communicating between
> backend and frontend of Xen.
> I am sure issue with XEN guest forwarding has to with checksum offloading.
> If I am not misinterpreting your comments, I can ignore code in netback and
> concentrate on code in netfront when trying to understand what is going wrong.
Yes, this issue is related to netfront (sys/dev/xen/netfront/netfront.c)
only, netback code is not involved. The code related to the checksum
stuff is on line ~1409 for the TX side, and around line 872 for the RX
side AFAICT.
You can find more information about the protocol itself in
sys/xen/interface/io/netif.h.
Adding Wei Liu who is also doing some work to improve netfront, and
knows more about the protocol than myself.
Roger.
___
freebsd-xen@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-xen
To unsubscribe, send any mail to "freebsd-xen-unsubscr...@freebsd.org"
Re: Checksum forwarding issue on XEN
Roger,
> Adding the persons that contributed that code in case they can shed some
> light.
>
> El 03/11/15 a les 21.12, Larry Baird ha escrit:
> > Has anybody made any progress on "Bug 188261 - [xen] FreeBSD DomU PVHVM
> > guests cannot 'route' traffic for other Xen PV guests on same Dom0 Host."
> > (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=188261)?
> >
> > The code for checksum calculation in the function xnb_add_mbuf_cksum() looks
> > suspect.
> >
> > switch (iph->ip_p) {
> > case IPPROTO_TCP:
> > if (mbufc->m_pkthdr.csum_flags & CSUM_IP_VALID) {
> > size_t tcplen = ntohs(iph->ip_len) - sizeof(struct
> > ip);
> > struct tcphdr *th = (struct tcphdr*)(iph + 1);
> > th->th_sum = in_pseudo(iph->ip_src.s_addr,
> > iph->ip_dst.s_addr, htons(IPPROTO_TCP +
> > tcplen));
> > th->th_sum = in_cksum_skip(mbufc,
> > sizeof(struct ether_header) +
> > ntohs(iph->ip_len),
> > sizeof(struct ether_header) + (iph->ip_hl <<
> > 2));
> > }
> > break;
> > case IPPROTO_UDP:
> > if (mbufc->m_pkthdr.csum_flags & CSUM_IP_VALID) {
> > size_t udplen = ntohs(iph->ip_len) - sizeof(struct
> > ip);
> > struct udphdr *uh = (struct udphdr*)(iph + 1);
> > uh->uh_sum = in_pseudo(iph->ip_src.s_addr,
> > iph->ip_dst.s_addr, htons(IPPROTO_UDP +
> > udplen));
> > uh->uh_sum = in_cksum_skip(mbufc,
> > sizeof(struct ether_header) +
> > ntohs(iph->ip_len),
> > sizeof(struct ether_header) + (iph->ip_hl <<
> > 2));
> > }
> > break;
> > default:
> > break;
> > }
> >
> >
> > Both in_pseudo() and in_cksum_skip() set the same checksum. Does this
> > make since to anybody?
>
> The bug you are referring to affects FreeBSD when running as a guest
> using xen-netfront, but the code snipped above and the function
> referenced (xnb_add_mbuf_cksum) is only used on FreeBSD when running as
> a host (AKA Dom0) by xen-netback.
>
> TBH, I don't know that much about FreeBSD network subsystem to have an
> opinion, but it certainly looks weird. Patches are welcome :).
Xyper-V has a similar forward issue. I found they were misusing csum_flags
and were always attempting to do checksum offloading if CSUM_IP_VALID was
set. I have given them a patch that fixes the issue. I was hoping that
Xen's issue was similar. I found the issue above by looking at all uses
of csum_flags in sys/dev/xen. It is hard to tell what the correct fix
is, without fulling understand the protocal used when communicating between
backend and frontend of Xen.
I am sure issue with XEN guest forwarding has to with checksum offloading.
If I am not misinterpreting your comments, I can ignore code in netback and
concentrate on code in netfront when trying to understand what is going wrong.
Thank you for some insite,
Larry
--
Larry Baird
Global Technology Associates, Inc. 1992-2012| http://www.gta.com
Celebrating Twenty Years of Software Innovation | Orlando, FL
Email: l...@gta.com | TEL 407-380-0220
___
freebsd-xen@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-xen
To unsubscribe, send any mail to "freebsd-xen-unsubscr...@freebsd.org"
Re: Checksum forwarding issue on XEN
Hello,
Adding the persons that contributed that code in case they can shed some
light.
El 03/11/15 a les 21.12, Larry Baird ha escrit:
> Has anybody made any progress on "Bug 188261 - [xen] FreeBSD DomU PVHVM
> guests cannot 'route' traffic for other Xen PV guests on same Dom0 Host."
> (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=188261)?
>
> The code for checksum calculation in the function xnb_add_mbuf_cksum() looks
> suspect.
>
> switch (iph->ip_p) {
> case IPPROTO_TCP:
> if (mbufc->m_pkthdr.csum_flags & CSUM_IP_VALID) {
> size_t tcplen = ntohs(iph->ip_len) - sizeof(struct
> ip);
> struct tcphdr *th = (struct tcphdr*)(iph + 1);
> th->th_sum = in_pseudo(iph->ip_src.s_addr,
> iph->ip_dst.s_addr, htons(IPPROTO_TCP + tcplen));
> th->th_sum = in_cksum_skip(mbufc,
> sizeof(struct ether_header) + ntohs(iph->ip_len),
> sizeof(struct ether_header) + (iph->ip_hl << 2));
> }
> break;
> case IPPROTO_UDP:
> if (mbufc->m_pkthdr.csum_flags & CSUM_IP_VALID) {
> size_t udplen = ntohs(iph->ip_len) - sizeof(struct
> ip);
> struct udphdr *uh = (struct udphdr*)(iph + 1);
> uh->uh_sum = in_pseudo(iph->ip_src.s_addr,
> iph->ip_dst.s_addr, htons(IPPROTO_UDP + udplen));
> uh->uh_sum = in_cksum_skip(mbufc,
> sizeof(struct ether_header) + ntohs(iph->ip_len),
> sizeof(struct ether_header) + (iph->ip_hl << 2));
> }
> break;
> default:
> break;
> }
>
>
> Both in_pseudo() and in_cksum_skip() set the same checksum. Does this
> make since to anybody?
The bug you are referring to affects FreeBSD when running as a guest
using xen-netfront, but the code snipped above and the function
referenced (xnb_add_mbuf_cksum) is only used on FreeBSD when running as
a host (AKA Dom0) by xen-netback.
TBH, I don't know that much about FreeBSD network subsystem to have an
opinion, but it certainly looks weird. Patches are welcome :).
Roger.
___
freebsd-xen@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-xen
To unsubscribe, send any mail to "freebsd-xen-unsubscr...@freebsd.org"
Checksum forwarding issue on XEN
Has anybody made any progress on "Bug 188261 - [xen] FreeBSD DomU PVHVM
guests cannot 'route' traffic for other Xen PV guests on same Dom0 Host."
(https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=188261)?
The code for checksum calculation in the function xnb_add_mbuf_cksum() looks
suspect.
switch (iph->ip_p) {
case IPPROTO_TCP:
if (mbufc->m_pkthdr.csum_flags & CSUM_IP_VALID) {
size_t tcplen = ntohs(iph->ip_len) - sizeof(struct ip);
struct tcphdr *th = (struct tcphdr*)(iph + 1);
th->th_sum = in_pseudo(iph->ip_src.s_addr,
iph->ip_dst.s_addr, htons(IPPROTO_TCP + tcplen));
th->th_sum = in_cksum_skip(mbufc,
sizeof(struct ether_header) + ntohs(iph->ip_len),
sizeof(struct ether_header) + (iph->ip_hl << 2));
}
break;
case IPPROTO_UDP:
if (mbufc->m_pkthdr.csum_flags & CSUM_IP_VALID) {
size_t udplen = ntohs(iph->ip_len) - sizeof(struct ip);
struct udphdr *uh = (struct udphdr*)(iph + 1);
uh->uh_sum = in_pseudo(iph->ip_src.s_addr,
iph->ip_dst.s_addr, htons(IPPROTO_UDP + udplen));
uh->uh_sum = in_cksum_skip(mbufc,
sizeof(struct ether_header) + ntohs(iph->ip_len),
sizeof(struct ether_header) + (iph->ip_hl << 2));
}
break;
default:
break;
}
Both in_pseudo() and in_cksum_skip() set the same checksum. Does this
make since to anybody?
Larry
--
Larry Baird
Global Technology Associates, Inc. 1992-2012| http://www.gta.com
Celebrating Twenty Years of Software Innovation | Orlando, FL
Email: l...@gta.com | TEL 407-380-0220
___
freebsd-xen@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-xen
To unsubscribe, send any mail to "freebsd-xen-unsubscr...@freebsd.org"
