The following reply was made to PR kern/180430; it has been noted by GNATS.
From: John Baldwin <j...@freebsd.org> To: Meny Yossefi <me...@mellanox.com> Cc: "bug-follo...@freebsd.org" <bug-follo...@freebsd.org> Subject: Re: kern/180430: [ofed] [patch] Bad UDP checksum calc for fragmented packets Date: Mon, 22 Jul 2013 11:40:08 -0400 On Monday, July 22, 2013 10:11:51 am Meny Yossefi wrote: > Hi John, > > > > The problem is that the HW will only calculate csum for parts of the > payload, one fragment at a time, > > while the receiver side, in our case the tcp/ip stack, will expect to > validate the packet's payload as a whole. Ok. > I agree with the change you offered, though one thing bothers me. > > This change will add two conditions to the send flow which will probably > have an effect on performance. > > Maybe 'likely' can be useful here ? FreeBSD tends to not use likely/unlikely unless there is a demonstrable gain via benchmark measurements. However, if the OFED code regularly uses it and you feel this is a case where you would normally use it, it can be added. > BTW, I'm not entirely sure, but I think the CSUM_IP flag is always set, so > maybe the first condition is not necessary. > > What do you think ? If the user uses ifconfig to disable checksum offload and force software checksums the flag will not be set. > -Meny > > > > > > -----Original Message----- > From: John Baldwin [mailto:j...@freebsd.org] > Sent: Friday, July 19, 2013 6:29 PM > To: bug-follo...@freebsd.org; Meny Yossefi > Subject: Re: kern/180430: [ofed] [patch] Bad UDP checksum calc for > fragmented packets > > > > Oops, my previous reply didn't make it to the PR itself. > > > > Is the problem that the hardware checksum overwrites arbitrary data in the > packet (at the location where the UDP header would be)? > > > > Also, I've looked at other drivers, and they all look at the CSUM_* flags to > determine the right settings. IP fragments will not have CSUM_UDP or CSUM_TCP set, so I think the more correct fix is this: > > > > Index: en_tx.c > > =================================================================== > > --- en_tx.c (revision 253470) > > +++ en_tx.c (working copy) > > @@ -780,8 +780,12 @@ retry: > > tx_desc->ctrl.srcrb_flags = > cpu_to_be32(MLX4_WQE_CTRL_CQ_UPDATE | > > > MLX4_WQE_CTRL_SOLICITED); > > if (mb->m_pkthdr.csum_flags & (CSUM_IP|CSUM_TCP|CSUM_UDP)) { > > - tx_desc->ctrl.srcrb_flags |= > cpu_to_be32(MLX4_WQE_CTRL_IP_CSUM | > > - > MLX4_WQE_CTRL_TCP_UDP_CSUM); > > + if (mb->m_pkthdr.csum_flags & CSUM_IP) > > + tx_desc->ctrl.srcrb_flags |= > > + > cpu_to_be32(MLX4_WQE_CTRL_IP_CSUM); > > + if (mb->m_pkthdr.csum_flags & > (CSUM_TCP|CSUM_UDP)) > > + tx_desc->ctrl.srcrb_flags |= > > + > cpu_to_be32(MLX4_WQE_CTRL_TCP_UDP_CSUM); > > priv->port_stats.tx_chksum_offload++; > > } > > > > -- > > John Baldwin > -- John Baldwin _______________________________________________ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"