Re: svn commit: r242161 - in head/sys: net netinet netpfil/pf
On 30.10.2012 03:25, YongHyeon PYUN wrote: On Mon, Oct 29, 2012 at 09:20:59AM +0100, Andre Oppermann wrote: On 29.10.2012 22:40, YongHyeon PYUN wrote: On Mon, Oct 29, 2012 at 09:21:00AM +0400, Gleb Smirnoff wrote: On Mon, Oct 29, 2012 at 01:41:04PM -0700, YongHyeon PYUN wrote: Y On Sun, Oct 28, 2012 at 02:01:37AM +0400, Gleb Smirnoff wrote: Y On Sat, Oct 27, 2012 at 12:58:52PM +0200, Andre Oppermann wrote: Y A On 26.10.2012 23:06, Gleb Smirnoff wrote: Y A Author: glebius Y A Date: Fri Oct 26 21:06:33 2012 Y A New Revision: 242161 Y A URL: http://svn.freebsd.org/changeset/base/242161 Y A Y A Log: Y A o Remove last argument to ip_fragment(), and obtain all needed information Y A on checksums directly from mbuf flags. This simplifies code. Y A o Clear CSUM_IP from the mbuf in ip_fragment() if we did checksums in Y Y I'm not sure whether ti(4)'s checksum offloading for IP fragmented Y packets(CSUM_IP_FRAGS) still works after this change. ti(4) Y requires CSUM_IP should be set for IP fragmented packets. Not sure Y whether it's a bug or not. I have a ti(4) controller but I don't Y remember where I can find it and don't have a link Y parter(1000baseSX) to test it. :-( ti(4) declares both CSUM_IP and CSUM_IP_FRAGS, so ip_fragment() won't do Because it supports both CSUM_IP and CSUM_IP_FRAGS. Probably ti(4) is the only controller that supports TCP/UDP checksum offloading for an IP fragmented packet. This is a bit weird if it doesn't do the fragmentation itself. Computing the IP header checksum doesn't differ for normal and fragmented packets. The protocol checksum (TCP or UDP) stays the same for in the case of IP level fragmentation. It is only visible in the first fragment which includes the protocol header. My interpretation for CSUM_IP_FRAGS works like the following. - Only peuso header checksum for TCP/UDP is computed by upper stack. - Controller has no ability to fragment the packet so it should done in upper stack(i.e. ip_output()). - When ip_output() has to fragment the packet, it just fragments the packet without completing TCP/UDP and IP checksum. If controller does not support CSUM_IP_FRAGS feature, ip_output() can't delay TCP/UDP checksum in this stage. - The fragmented packets are sent to driver. Driver sets appropriate bits of DMA descriptor based on fragmentation field of mbuf(M_FRAG, M_LASTFRAG) and issue the frame to controller. - The firmware of controller queues the fragmented frames up in its internal memory and hold off sending out the frames since it has to compute TCP/UDP checksum. When it sees a frame which indicates the end of fragmented frame it finally computes TCP/UDP checksum and send each frame out to wire by computing IP checksum on the fly. The difference is which one(upper stack vs. controller) computes TCP/UDP/IP checksum. Such a behavior doesn't make much sense and probably wasn't used at all in practice. It's very complex as well. Plus you can't guarantee that there won't be other packet slipping into the interface queue in an SMP world. IP fragmentation really isn't done for TCP within the kernel. We try to prevent it as it would have a huge performance impact. Hence the internal MTU discovery and the Don't Fragment bit set on TCP packets. IP fragmentation does happen for large UDP packet locally generated. There however because of the past absence of UDP fragmentation offload coupled with UDP checksum offloading caused all fragmentation to be done at the UDP level before it hits ip_output. The remaining use of IP fragmentation is when the machine is acting as a router and it has to send packets out on an interface with a smaller MTU than the one it came in on. So the only two useful features regarding UDP+IP fragmentation are: 1. IP fragmentation including UDP checksum calculation for locally generated large UDP packets. This is the TSO for UDP. 2. Pure IP fragmentation for in-transit packets. Here only the IP header checksum needs to be recalculated for each fragment. The layer 4 checksums (UDP, TCP and others) stay the same. -- Andre software checksums, and thus won't clear these flags. Potentially a driver that announces one flag in if_hwassist but relies on couple of flags to be set on mbuf is not correct. If a driver can't do single checksum processing independently from others, then it should set or clear appropriate flags in if_hwassist as a group. Hmm, then what would be best way to achieve CSUM_IP_FRAGS in driver? I don't have clear idea how to utilize the hardware feature. The stack should tell that the mbuf needs TCP/UDP checksum offloading for IP fragmented packet(i.e. CSUM_IP_FRAGS is not set by upper stack). As I said there can't be fragment checksumming without hardware It's up to controller's firmware. It does not send the fragmented frame until it computes TCP/UDP checksum. based fragmentation. We have three cases
Re: svn commit: r242161 - in head/sys: net netinet netpfil/pf
On Wed, Oct 31, 2012 at 08:59:06AM +0100, Andre Oppermann wrote: On 30.10.2012 03:25, YongHyeon PYUN wrote: On Mon, Oct 29, 2012 at 09:20:59AM +0100, Andre Oppermann wrote: On 29.10.2012 22:40, YongHyeon PYUN wrote: On Mon, Oct 29, 2012 at 09:21:00AM +0400, Gleb Smirnoff wrote: On Mon, Oct 29, 2012 at 01:41:04PM -0700, YongHyeon PYUN wrote: Y On Sun, Oct 28, 2012 at 02:01:37AM +0400, Gleb Smirnoff wrote: Y On Sat, Oct 27, 2012 at 12:58:52PM +0200, Andre Oppermann wrote: Y A On 26.10.2012 23:06, Gleb Smirnoff wrote: Y A Author: glebius Y A Date: Fri Oct 26 21:06:33 2012 Y A New Revision: 242161 Y A URL: http://svn.freebsd.org/changeset/base/242161 Y A Y A Log: Y A o Remove last argument to ip_fragment(), and obtain all needed information Y A on checksums directly from mbuf flags. This simplifies code. Y A o Clear CSUM_IP from the mbuf in ip_fragment() if we did checksums in Y Y I'm not sure whether ti(4)'s checksum offloading for IP fragmented Y packets(CSUM_IP_FRAGS) still works after this change. ti(4) Y requires CSUM_IP should be set for IP fragmented packets. Not sure Y whether it's a bug or not. I have a ti(4) controller but I don't Y remember where I can find it and don't have a link Y parter(1000baseSX) to test it. :-( ti(4) declares both CSUM_IP and CSUM_IP_FRAGS, so ip_fragment() won't do Because it supports both CSUM_IP and CSUM_IP_FRAGS. Probably ti(4) is the only controller that supports TCP/UDP checksum offloading for an IP fragmented packet. This is a bit weird if it doesn't do the fragmentation itself. Computing the IP header checksum doesn't differ for normal and fragmented packets. The protocol checksum (TCP or UDP) stays the same for in the case of IP level fragmentation. It is only visible in the first fragment which includes the protocol header. My interpretation for CSUM_IP_FRAGS works like the following. - Only peuso header checksum for TCP/UDP is computed by upper stack. - Controller has no ability to fragment the packet so it should done in upper stack(i.e. ip_output()). - When ip_output() has to fragment the packet, it just fragments the packet without completing TCP/UDP and IP checksum. If controller does not support CSUM_IP_FRAGS feature, ip_output() can't delay TCP/UDP checksum in this stage. - The fragmented packets are sent to driver. Driver sets appropriate bits of DMA descriptor based on fragmentation field of mbuf(M_FRAG, M_LASTFRAG) and issue the frame to controller. - The firmware of controller queues the fragmented frames up in its internal memory and hold off sending out the frames since it has to compute TCP/UDP checksum. When it sees a frame which indicates the end of fragmented frame it finally computes TCP/UDP checksum and send each frame out to wire by computing IP checksum on the fly. The difference is which one(upper stack vs. controller) computes TCP/UDP/IP checksum. Such a behavior doesn't make much sense and probably wasn't used at all in practice. It's very complex as well. Plus you can't guarantee that It's job of firmware running on embedded MIPS R4000 in the controller. ti(4) was one of the best smart controller in the past and it even supported header split! there won't be other packet slipping into the interface queue in an SMP world. Hmm, right, I should have noticed that after FreeBSD's removal for splnet()/splimp(). Since ti(4) is the only controller that supports TCP/UDP checksum offloading on IP fragmented packets and it's rare to see ti(4) controllers in these days, I think there is no much point to make the hardware feature work. I'll remove the feature in ti(4). ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r242161 - in head/sys: net netinet netpfil/pf
On 29.10.2012 22:40, YongHyeon PYUN wrote: On Mon, Oct 29, 2012 at 09:21:00AM +0400, Gleb Smirnoff wrote: On Mon, Oct 29, 2012 at 01:41:04PM -0700, YongHyeon PYUN wrote: Y On Sun, Oct 28, 2012 at 02:01:37AM +0400, Gleb Smirnoff wrote: Y On Sat, Oct 27, 2012 at 12:58:52PM +0200, Andre Oppermann wrote: Y A On 26.10.2012 23:06, Gleb Smirnoff wrote: Y A Author: glebius Y A Date: Fri Oct 26 21:06:33 2012 Y A New Revision: 242161 Y A URL: http://svn.freebsd.org/changeset/base/242161 Y A Y A Log: Y A o Remove last argument to ip_fragment(), and obtain all needed information Y A on checksums directly from mbuf flags. This simplifies code. Y A o Clear CSUM_IP from the mbuf in ip_fragment() if we did checksums in Y Y I'm not sure whether ti(4)'s checksum offloading for IP fragmented Y packets(CSUM_IP_FRAGS) still works after this change. ti(4) Y requires CSUM_IP should be set for IP fragmented packets. Not sure Y whether it's a bug or not. I have a ti(4) controller but I don't Y remember where I can find it and don't have a link Y parter(1000baseSX) to test it. :-( ti(4) declares both CSUM_IP and CSUM_IP_FRAGS, so ip_fragment() won't do Because it supports both CSUM_IP and CSUM_IP_FRAGS. Probably ti(4) is the only controller that supports TCP/UDP checksum offloading for an IP fragmented packet. This is a bit weird if it doesn't do the fragmentation itself. Computing the IP header checksum doesn't differ for normal and fragmented packets. The protocol checksum (TCP or UDP) stays the same for in the case of IP level fragmentation. It is only visible in the first fragment which includes the protocol header. software checksums, and thus won't clear these flags. Potentially a driver that announces one flag in if_hwassist but relies on couple of flags to be set on mbuf is not correct. If a driver can't do single checksum processing independently from others, then it should set or clear appropriate flags in if_hwassist as a group. Hmm, then what would be best way to achieve CSUM_IP_FRAGS in driver? I don't have clear idea how to utilize the hardware feature. The stack should tell that the mbuf needs TCP/UDP checksum offloading for IP fragmented packet(i.e. CSUM_IP_FRAGS is not set by upper stack). As I said there can't be fragment checksumming without hardware based fragmentation. We have three cases here: 1. TSO where the hardware does the segmentation, TCP and IP header checksums for each generated packet. 2. IP packet fragmentation where a packet is split, the IP header checksum is recomputed for each fragment, but the protocol csum stays the same and is not modified. 3. UDP fragmentation where a large packet is sent to the hardware and it generates first the UDP checksum and then splits it into IP fragments each with its own IP header checksum. So we end up with these possible large send hardware offload capabilities: TSO: including IPv4hdr and TCP checksumming UDP fragmentation: including IPv4hdr and UDP checksumming IP fragmentation: including IPv4hdr checksumming Besides that we have the packet = MTU sized offload capabilities: TCP checksumming UDP checksumming SCTP checksumming IPv4hdr checksumming Y A hardware. Some driver may not announce CSUM_IP in theur if_hwassist, Oh, that was a typo! Software was meant. That explains quite a bit of confusion. -- Andre ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r242161 - in head/sys: net netinet netpfil/pf
On Mon, Oct 29, 2012 at 09:20:59AM +0100, Andre Oppermann wrote: On 29.10.2012 22:40, YongHyeon PYUN wrote: On Mon, Oct 29, 2012 at 09:21:00AM +0400, Gleb Smirnoff wrote: On Mon, Oct 29, 2012 at 01:41:04PM -0700, YongHyeon PYUN wrote: Y On Sun, Oct 28, 2012 at 02:01:37AM +0400, Gleb Smirnoff wrote: Y On Sat, Oct 27, 2012 at 12:58:52PM +0200, Andre Oppermann wrote: Y A On 26.10.2012 23:06, Gleb Smirnoff wrote: Y A Author: glebius Y A Date: Fri Oct 26 21:06:33 2012 Y A New Revision: 242161 Y A URL: http://svn.freebsd.org/changeset/base/242161 Y A Y A Log: Y A o Remove last argument to ip_fragment(), and obtain all needed information Y A on checksums directly from mbuf flags. This simplifies code. Y A o Clear CSUM_IP from the mbuf in ip_fragment() if we did checksums in Y Y I'm not sure whether ti(4)'s checksum offloading for IP fragmented Y packets(CSUM_IP_FRAGS) still works after this change. ti(4) Y requires CSUM_IP should be set for IP fragmented packets. Not sure Y whether it's a bug or not. I have a ti(4) controller but I don't Y remember where I can find it and don't have a link Y parter(1000baseSX) to test it. :-( ti(4) declares both CSUM_IP and CSUM_IP_FRAGS, so ip_fragment() won't do Because it supports both CSUM_IP and CSUM_IP_FRAGS. Probably ti(4) is the only controller that supports TCP/UDP checksum offloading for an IP fragmented packet. This is a bit weird if it doesn't do the fragmentation itself. Computing the IP header checksum doesn't differ for normal and fragmented packets. The protocol checksum (TCP or UDP) stays the same for in the case of IP level fragmentation. It is only visible in the first fragment which includes the protocol header. My interpretation for CSUM_IP_FRAGS works like the following. - Only peuso header checksum for TCP/UDP is computed by upper stack. - Controller has no ability to fragment the packet so it should done in upper stack(i.e. ip_output()). - When ip_output() has to fragment the packet, it just fragments the packet without completing TCP/UDP and IP checksum. If controller does not support CSUM_IP_FRAGS feature, ip_output() can't delay TCP/UDP checksum in this stage. - The fragmented packets are sent to driver. Driver sets appropriate bits of DMA descriptor based on fragmentation field of mbuf(M_FRAG, M_LASTFRAG) and issue the frame to controller. - The firmware of controller queues the fragmented frames up in its internal memory and hold off sending out the frames since it has to compute TCP/UDP checksum. When it sees a frame which indicates the end of fragmented frame it finally computes TCP/UDP checksum and send each frame out to wire by computing IP checksum on the fly. The difference is which one(upper stack vs. controller) computes TCP/UDP/IP checksum. software checksums, and thus won't clear these flags. Potentially a driver that announces one flag in if_hwassist but relies on couple of flags to be set on mbuf is not correct. If a driver can't do single checksum processing independently from others, then it should set or clear appropriate flags in if_hwassist as a group. Hmm, then what would be best way to achieve CSUM_IP_FRAGS in driver? I don't have clear idea how to utilize the hardware feature. The stack should tell that the mbuf needs TCP/UDP checksum offloading for IP fragmented packet(i.e. CSUM_IP_FRAGS is not set by upper stack). As I said there can't be fragment checksumming without hardware It's up to controller's firmware. It does not send the fragmented frame until it computes TCP/UDP checksum. based fragmentation. We have three cases here: 1. TSO where the hardware does the segmentation, TCP and IP header checksums for each generated packet. 2. IP packet fragmentation where a packet is split, the IP header checksum is recomputed for each fragment, but the protocol csum stays the same and is not modified. 3. UDP fragmentation where a large packet is sent to the hardware and it generates first the UDP checksum and then splits it into IP fragments each with its own IP header checksum. So we end up with these possible large send hardware offload capabilities: TSO: including IPv4hdr and TCP checksumming UDP fragmentation: including IPv4hdr and UDP checksumming IP fragmentation: including IPv4hdr checksumming Besides that we have the packet = MTU sized offload capabilities: TCP checksumming UDP checksumming SCTP checksumming IPv4hdr checksumming Y A hardware. Some driver may not announce CSUM_IP in theur if_hwassist, Oh, that was a typo! Software was meant. That explains quite a bit of confusion. -- Andre ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to
Re: svn commit: r242161 - in head/sys: net netinet netpfil/pf
On 28.10.2012 00:01, Gleb Smirnoff wrote: On Sat, Oct 27, 2012 at 12:58:52PM +0200, Andre Oppermann wrote: A On 26.10.2012 23:06, Gleb Smirnoff wrote: A Author: glebius A Date: Fri Oct 26 21:06:33 2012 A New Revision: 242161 A URL: http://svn.freebsd.org/changeset/base/242161 A A Log: A o Remove last argument to ip_fragment(), and obtain all needed information A on checksums directly from mbuf flags. This simplifies code. A o Clear CSUM_IP from the mbuf in ip_fragment() if we did checksums in A hardware. Some driver may not announce CSUM_IP in theur if_hwassist, A although try to do checksums if CSUM_IP set on mbuf. Example is em(4). A A I'm not getting your description here? Why work around a bug in a driver A in ip_fragment() when we can fix the bug in the driver? Well, that was actually bug in the stack and a very special driver that demonstrates it. I may even agree that driver is incorrect, but the stack was incorrect, too. Ah, OK. Do you intend to fix the driver as well? -- Andre ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r242161 - in head/sys: net netinet netpfil/pf
On Sun, Oct 28, 2012 at 02:01:37AM +0400, Gleb Smirnoff wrote: On Sat, Oct 27, 2012 at 12:58:52PM +0200, Andre Oppermann wrote: A On 26.10.2012 23:06, Gleb Smirnoff wrote: A Author: glebius A Date: Fri Oct 26 21:06:33 2012 A New Revision: 242161 A URL: http://svn.freebsd.org/changeset/base/242161 A A Log: A o Remove last argument to ip_fragment(), and obtain all needed information A on checksums directly from mbuf flags. This simplifies code. A o Clear CSUM_IP from the mbuf in ip_fragment() if we did checksums in I'm not sure whether ti(4)'s checksum offloading for IP fragmented packets(CSUM_IP_FRAGS) still works after this change. ti(4) requires CSUM_IP should be set for IP fragmented packets. Not sure whether it's a bug or not. I have a ti(4) controller but I don't remember where I can find it and don't have a link parter(1000baseSX) to test it. :-( A hardware. Some driver may not announce CSUM_IP in theur if_hwassist, A although try to do checksums if CSUM_IP set on mbuf. Example is em(4). em(4) had TX IP checksum offloading support but it was removed without justification. There could be some reason on that decision but I don't see any compelling reason. A A I'm not getting your description here? Why work around a bug in a driver A in ip_fragment() when we can fix the bug in the driver? Well, that was actually bug in the stack and a very special driver that demonstrates it. I may even agree that driver is incorrect, but the stack was incorrect, too. -- Totus tuus, Glebius. ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r242161 - in head/sys: net netinet netpfil/pf
On Mon, Oct 29, 2012 at 01:41:04PM -0700, YongHyeon PYUN wrote: Y On Sun, Oct 28, 2012 at 02:01:37AM +0400, Gleb Smirnoff wrote: Y On Sat, Oct 27, 2012 at 12:58:52PM +0200, Andre Oppermann wrote: Y A On 26.10.2012 23:06, Gleb Smirnoff wrote: Y A Author: glebius Y A Date: Fri Oct 26 21:06:33 2012 Y A New Revision: 242161 Y A URL: http://svn.freebsd.org/changeset/base/242161 Y A Y A Log: Y A o Remove last argument to ip_fragment(), and obtain all needed information Y A on checksums directly from mbuf flags. This simplifies code. Y A o Clear CSUM_IP from the mbuf in ip_fragment() if we did checksums in Y Y I'm not sure whether ti(4)'s checksum offloading for IP fragmented Y packets(CSUM_IP_FRAGS) still works after this change. ti(4) Y requires CSUM_IP should be set for IP fragmented packets. Not sure Y whether it's a bug or not. I have a ti(4) controller but I don't Y remember where I can find it and don't have a link Y parter(1000baseSX) to test it. :-( ti(4) declares both CSUM_IP and CSUM_IP_FRAGS, so ip_fragment() won't do software checksums, and thus won't clear these flags. Potentially a driver that announces one flag in if_hwassist but relies on couple of flags to be set on mbuf is not correct. If a driver can't do single checksum processing independently from others, then it should set or clear appropriate flags in if_hwassist as a group. Y A hardware. Some driver may not announce CSUM_IP in theur if_hwassist, Oh, that was a typo! Software was meant. -- Totus tuus, Glebius. ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r242161 - in head/sys: net netinet netpfil/pf
On Mon, Oct 29, 2012 at 09:21:00AM +0400, Gleb Smirnoff wrote: On Mon, Oct 29, 2012 at 01:41:04PM -0700, YongHyeon PYUN wrote: Y On Sun, Oct 28, 2012 at 02:01:37AM +0400, Gleb Smirnoff wrote: Y On Sat, Oct 27, 2012 at 12:58:52PM +0200, Andre Oppermann wrote: Y A On 26.10.2012 23:06, Gleb Smirnoff wrote: Y A Author: glebius Y A Date: Fri Oct 26 21:06:33 2012 Y A New Revision: 242161 Y A URL: http://svn.freebsd.org/changeset/base/242161 Y A Y A Log: Y A o Remove last argument to ip_fragment(), and obtain all needed information Y A on checksums directly from mbuf flags. This simplifies code. Y A o Clear CSUM_IP from the mbuf in ip_fragment() if we did checksums in Y Y I'm not sure whether ti(4)'s checksum offloading for IP fragmented Y packets(CSUM_IP_FRAGS) still works after this change. ti(4) Y requires CSUM_IP should be set for IP fragmented packets. Not sure Y whether it's a bug or not. I have a ti(4) controller but I don't Y remember where I can find it and don't have a link Y parter(1000baseSX) to test it. :-( ti(4) declares both CSUM_IP and CSUM_IP_FRAGS, so ip_fragment() won't do Because it supports both CSUM_IP and CSUM_IP_FRAGS. Probably ti(4) is the only controller that supports TCP/UDP checksum offloading for an IP fragmented packet. software checksums, and thus won't clear these flags. Potentially a driver that announces one flag in if_hwassist but relies on couple of flags to be set on mbuf is not correct. If a driver can't do single checksum processing independently from others, then it should set or clear appropriate flags in if_hwassist as a group. Hmm, then what would be best way to achieve CSUM_IP_FRAGS in driver? I don't have clear idea how to utilize the hardware feature. The stack should tell that the mbuf needs TCP/UDP checksum offloading for IP fragmented packet(i.e. CSUM_IP_FRAGS is not set by upper stack). Y A hardware. Some driver may not announce CSUM_IP in theur if_hwassist, Oh, that was a typo! Software was meant. -- Totus tuus, Glebius. ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r242161 - in head/sys: net netinet netpfil/pf
On 26.10.2012 23:06, Gleb Smirnoff wrote: Author: glebius Date: Fri Oct 26 21:06:33 2012 New Revision: 242161 URL: http://svn.freebsd.org/changeset/base/242161 Log: o Remove last argument to ip_fragment(), and obtain all needed information on checksums directly from mbuf flags. This simplifies code. o Clear CSUM_IP from the mbuf in ip_fragment() if we did checksums in hardware. Some driver may not announce CSUM_IP in theur if_hwassist, although try to do checksums if CSUM_IP set on mbuf. Example is em(4). I'm not getting your description here? Why work around a bug in a driver in ip_fragment() when we can fix the bug in the driver? o While here, consistently use CSUM_IP instead of its alias CSUM_DELAY_IP. After this change CSUM_DELAY_IP vanishes from the stack. Good. :) Submitted by:Sebastian Kuzminsky seb lineratesystems.com -- Andre ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r242161 - in head/sys: net netinet netpfil/pf
On Sat, Oct 27, 2012 at 12:58:52PM +0200, Andre Oppermann wrote: A On 26.10.2012 23:06, Gleb Smirnoff wrote: A Author: glebius A Date: Fri Oct 26 21:06:33 2012 A New Revision: 242161 A URL: http://svn.freebsd.org/changeset/base/242161 A A Log: A o Remove last argument to ip_fragment(), and obtain all needed information A on checksums directly from mbuf flags. This simplifies code. A o Clear CSUM_IP from the mbuf in ip_fragment() if we did checksums in A hardware. Some driver may not announce CSUM_IP in theur if_hwassist, A although try to do checksums if CSUM_IP set on mbuf. Example is em(4). A A I'm not getting your description here? Why work around a bug in a driver A in ip_fragment() when we can fix the bug in the driver? Well, that was actually bug in the stack and a very special driver that demonstrates it. I may even agree that driver is incorrect, but the stack was incorrect, too. -- Totus tuus, Glebius. ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
svn commit: r242161 - in head/sys: net netinet netpfil/pf
Author: glebius
Date: Fri Oct 26 21:06:33 2012
New Revision: 242161
URL: http://svn.freebsd.org/changeset/base/242161
Log:
o Remove last argument to ip_fragment(), and obtain all needed information
on checksums directly from mbuf flags. This simplifies code.
o Clear CSUM_IP from the mbuf in ip_fragment() if we did checksums in
hardware. Some driver may not announce CSUM_IP in theur if_hwassist,
although try to do checksums if CSUM_IP set on mbuf. Example is em(4).
o While here, consistently use CSUM_IP instead of its alias CSUM_DELAY_IP.
After this change CSUM_DELAY_IP vanishes from the stack.
Submitted by: Sebastian Kuzminsky seb lineratesystems.com
Modified:
head/sys/net/if_bridge.c
head/sys/netinet/ip_fastfwd.c
head/sys/netinet/ip_mroute.c
head/sys/netinet/ip_output.c
head/sys/netinet/ip_var.h
head/sys/netpfil/pf/pf.c
Modified: head/sys/net/if_bridge.c
==
--- head/sys/net/if_bridge.cFri Oct 26 20:43:30 2012(r242160)
+++ head/sys/net/if_bridge.cFri Oct 26 21:06:33 2012(r242161)
@@ -3379,8 +3379,8 @@ bridge_fragment(struct ifnet *ifp, struc
goto out;
ip = mtod(m, struct ip *);
- error = ip_fragment(ip, m, ifp-if_mtu, ifp-if_hwassist,
- CSUM_DELAY_IP);
+ m-m_pkthdr.csum_flags |= CSUM_IP;
+ error = ip_fragment(ip, m, ifp-if_mtu, ifp-if_hwassist);
if (error)
goto out;
Modified: head/sys/netinet/ip_fastfwd.c
==
--- head/sys/netinet/ip_fastfwd.c Fri Oct 26 20:43:30 2012
(r242160)
+++ head/sys/netinet/ip_fastfwd.c Fri Oct 26 21:06:33 2012
(r242161)
@@ -542,10 +542,8 @@ passout:
* We have to fragment the packet
*/
m-m_pkthdr.csum_flags |= CSUM_IP;
- if (ip_fragment(ip, m, mtu, ifp-if_hwassist,
- (~ifp-if_hwassist CSUM_DELAY_IP))) {
+ if (ip_fragment(ip, m, mtu, ifp-if_hwassist))
goto drop;
- }
KASSERT(m != NULL, (null mbuf and no error));
/*
* Send off the fragments via outgoing interface
Modified: head/sys/netinet/ip_mroute.c
==
--- head/sys/netinet/ip_mroute.cFri Oct 26 20:43:30 2012
(r242160)
+++ head/sys/netinet/ip_mroute.cFri Oct 26 21:06:33 2012
(r242161)
@@ -2404,7 +2404,8 @@ pim_register_prepare(struct ip *ip, stru
ip-ip_sum = in_cksum(mb_copy, ip-ip_hl 2);
} else {
/* Fragment the packet */
- if (ip_fragment(ip, mb_copy, mtu, 0, CSUM_DELAY_IP) != 0) {
+ mb_copy-m_pkthdr.csum_flags |= CSUM_IP;
+ if (ip_fragment(ip, mb_copy, mtu, 0) != 0) {
m_freem(mb_copy);
return NULL;
}
Modified: head/sys/netinet/ip_output.c
==
--- head/sys/netinet/ip_output.cFri Oct 26 20:43:30 2012
(r242160)
+++ head/sys/netinet/ip_output.cFri Oct 26 21:06:33 2012
(r242161)
@@ -125,7 +125,7 @@ ip_output(struct mbuf *m, struct mbuf *o
struct sockaddr_in *dst;
struct in_ifaddr *ia;
int isbroadcast;
- uint16_t ip_len, ip_off, sw_csum;
+ uint16_t ip_len, ip_off;
struct route iproute;
struct rtentry *rte;/* cache for ro-ro_rt */
struct in_addr odst;
@@ -583,18 +583,16 @@ passout:
}
m-m_pkthdr.csum_flags |= CSUM_IP;
- sw_csum = m-m_pkthdr.csum_flags ~ifp-if_hwassist;
- if (sw_csum CSUM_DELAY_DATA) {
+ if (m-m_pkthdr.csum_flags CSUM_DELAY_DATA ~ifp-if_hwassist) {
in_delayed_cksum(m);
- sw_csum = ~CSUM_DELAY_DATA;
+ m-m_pkthdr.csum_flags = ~CSUM_DELAY_DATA;
}
#ifdef SCTP
- if (sw_csum CSUM_SCTP) {
+ if (m-m_pkthdr.csum_flags CSUM_SCTP ~ifp-if_hwassist) {
sctp_delayed_cksum(m, (uint32_t)(ip-ip_hl 2));
- sw_csum = ~CSUM_SCTP;
+ m-m_pkthdr.csum_flags = ~CSUM_SCTP;
}
#endif
- m-m_pkthdr.csum_flags = ifp-if_hwassist;
/*
* If small enough for interface, or the interface will take
@@ -604,8 +602,10 @@ passout:
(m-m_pkthdr.csum_flags ifp-if_hwassist CSUM_TSO) != 0 ||
((ip_off IP_DF) == 0 (ifp-if_hwassist CSUM_FRAGMENT))) {
ip-ip_sum = 0;
- if (sw_csum CSUM_DELAY_IP)
+ if (m-m_pkthdr.csum_flags CSUM_IP ~ifp-if_hwassist) {
ip-ip_sum = in_cksum(m, hlen);
+ m-m_pkthdr.csum_flags = ~CSUM_IP;
+
