On 12 May 2014, at 03:45, Yonghyeon PYUN <[email protected]> wrote: > On Fri, May 09, 2014 at 04:22:36PM +0200, Michael Tuexen wrote: >> >> On 09 May 2014, at 12:46, Michael Tuexen <[email protected]> >> wrote: >> >>> On 09 May 2014, at 03:35, Yonghyeon PYUN <[email protected]> wrote: >>> >>>> On Thu, May 08, 2014 at 08:40:22PM +0200, Michael Tuexen wrote: >>>>> On 07 May 2014, at 10:37, Yonghyeon PYUN <[email protected]> wrote: >>>>> >>>>>> On Wed, May 07, 2014 at 10:07:09AM +0200, Michael Tuexen wrote: >>>>>>> On 07 May 2014, at 09:56, Yonghyeon PYUN <[email protected]> wrote: >>>>>>> >>>>>>>> On Sat, May 03, 2014 at 11:52:47AM +0200, Michael Tuexen wrote: >>>>>>>>> On 02 May 2014, at 16:02, Bjoern A. Zeeb <[email protected]> wrote: >>>>>>>>> >>>>>>>>>> >>>>>>>>>> On 02 May 2014, at 10:22 , Michael Tuexen >>>>>>>>>> <[email protected]> wrote: >>>>>>>>>> >>>>>>>>>>> Dear all, >>>>>>>>>>> >>>>>>>>>>> during testing I found that FreeBSD head (on a raspberry pi) >>>>>>>>>>> accepts SCTP packet >>>>>>>>>>> with bad checksums. After debugging this I figured out that this is >>>>>>>>>>> a problem with >>>>>>>>>>> the csum_flags defined in mbuf.h. >>>>>>>>>>> >>>>>>>>>>> The SCTP code on its input path checks for CSUM_SCTP_VALID, which >>>>>>>>>>> is defined in mbuf.h: >>>>>>>>>>> #define CSUM_SCTP_VALID CSUM_L4_VALID >>>>>>>>>>> This makes sense: If CSUM_SCTP_VALID is set in csum_flags, the >>>>>>>>>>> packet is considered >>>>>>>>>>> to have a correct checksum. >>>>>>>>>>> >>>>>>>>>>> For UDP and TCP some drivers calculate the UDP/TCP checksum and set >>>>>>>>>>> CSUM_DATA_VALID in >>>>>>>>>>> csum_flags to indicate that the UDP/TCP should consider csum_data >>>>>>>>>>> to figure out if >>>>>>>>>>> the packet has a correct checksum. The problem is that >>>>>>>>>>> CSUM_DATA_VALID is defined as >>>>>>>>>>> #define CSUM_DATA_VALID CSUM_L4_VALID >>>>>>>>>>> In this case the semantic is not that the packet has a valid >>>>>>>>>>> checksum, but the csum_data >>>>>>>>>>> field contains information. >>>>>>>>>>> >>>>>>>>>>> Now the following happens (on the raspberry pi the driver used is >>>>>>>>>>> dev/usb/net/if_smsc.c >>>>>>>>>>> >>>>>>>>>>> 1. A packet is received and if it is not too short, the checksum >>>>>>>>>>> computed >>>>>>>>>>> is stored in csum_data and the flag CSUM_DATA_VALID is set. This >>>>>>>>>>> happens >>>>>>>>>>> for all IP packets, not only for UDP and TCP packets. >>>>>>>>>>> 2. In case of SCTP packets, the SCTP interprets CSUM_DATA_VALID as >>>>>>>>>>> CSUM_SCTP_VALID >>>>>>>>>>> and accepts the packet. So no SCTP checksum check ever happened. >>>>>>>>>>> >>>>>>>>>>> Alternatives to fix this: >>>>>>>>>>> >>>>>>>>>>> 1. Change all drivers to set CSUM_DATA_VALID only in case of UDP or >>>>>>>>>>> TCP packets, since >>>>>>>>>>> it only makes sense in these cases. >>>>>>>>>> >>>>>>>>>> Wait, or for SCTP in cad the crc32 (I think it was) was actually >>>>>>>>>> checked but not otherwise. This is how it should be imho. It >>>>>>>>>> seems like a driver bug. >>>>>>>>> I went through the list of drivers and you are right, it seems to be >>>>>>>>> a bug >>>>>>>>> in if_smsc.c. Most of the other drivers check for UDP/TCP, a small >>>>>>>>> set I can't tell. >>>>>>>>> >>>>>>>> >>>>>>>> I'm not sure how the controller computes TCP/UDP checksum values. >>>>>>>> It seems the publicly available data sheet was highly sanitized so >>>>>>>> it was useless to me. The comment in the driver says that the >>>>>>> Same for me... >>>>>>>> controller computes RX checksum after the IPv4 header to the end of >>>>>>>> ethernet frame. After seeing that comment, three questions popped >>>>>>>> up: >>>>>>>> >>>>> OK, I did some testing. It looks like the card is just computing the >>>>> checksum over the IP payload taking the correct IP header length into >>>>> account. >>>>>>>> 1. Is the controller smart enough to skip IP options header in >>>>>>>> TCP/UDP checksum offloading? >>>>> Yes, I can send fragmented and un-fragmented UDP packets with IP options >>>>> and they are handled correctly. Even if the last fragment is too short. >>>> >>>> I'm assuming you're taking about receiving fragmented UDP packets >>>> with RX checksum offloading, right? >>> Correct. >>>> >>>>>>>> 2. How controller handles UDP checksum value 0x0000(i.e. sender >>>>>>>> didn't compute UDP checksum)? >>>>> This case isn't handled. However, udp_input() looks first for zero >>>>> checksums >>>>> and only after that in the csum_flags. So it doesn't result in any >>>>> problems. >>>>> Would you prefer not to set CSUM_DATA_VALID in this case? >>>> >>>> At least, it correctly updates UDP stats of netstat(1). >>> Let me double check that... >> I double checked it. The statistic counters are incremented. >> Please note that we had a bug in the sending code of head, which >> made it impossible to send UDP packets with 0 checksum. That is >> fixed in >> http://svnweb.freebsd.org/base?view=revision&revision=265776 > > Thanks. > >> >> So any preference whether to report CSUM_DATA_VALID if a UDP packet >> with checksum 0 is received or not? I'm pretty open, since it does >> not have any effect right now... >> > > Because upper stack correctly counts for these packets, your change > (report CSUM_DATA_VALID for UDP packet with checksum value 0) looks > fine. I don't remember how pf/ipf handles that case though but we > can easily fix pf/ipf once we see breakage. OK.
Best regards Michael > >> Best regards >> Michael > _______________________________________________ [email protected] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "[email protected]"
