On 7/2/2018 6:32 PM, Alexander Duyck wrote:
On Mon, Jul 2, 2018 at 7:46 AM Willem de Bruijn <willemdebruijn.ker...@gmail.com <mailto:willemdebruijn.ker...@gmail.com>> wrote:On Mon, Jul 2, 2018 at 9:34 AM Willem de Bruijn <willemdebruijn.ker...@gmail.com <mailto:willemdebruijn.ker...@gmail.com>> wrote: > > On Mon, Jul 2, 2018 at 1:30 AM Boris Pismenny <bor...@mellanox.com <mailto:bor...@mellanox.com>> wrote: > > > > > > > > On 7/2/2018 4:45 AM, Willem de Bruijn wrote: > > >>> I've noticed that we could get cleaner code in our driver if we remove > > >>> these two lines from net/ipv4/udp_offload.c: > > >>> if (skb_is_gso(segs)) > > >>> mss *= skb_shinfo(segs)->gso_segs; > > >>> > > >>> I think that this is correct in case of GSO_PARTIAL segmentation for the > > >>> following reasons: > > >>> 1. After this change the UDP payload field is consistent with the IP > > >>> header payload length field. Currently, IPv4 length is 1500 and UDP > > >>> total length is the full unsegmented length. > > > > > > How does this simplify the driver? Does it currently have to > > > change the udph->length field to the mss on the wire, because the > > > device only splits + replicates the headers + computes the csum? > > > > Yes, this is the code I have at the moment. > > > > The device's limitation is more subtle than this. It could adjust the > > length, but then the checksum would be wrong. > > I see. We do have to keep in mind other devices. Alexander's ixgbe > RFC patch does not have this logic, so that device must update the > field directly. > > https://patchwork.ozlabs.org/patch/908396/ To be clear, I think it's fine to remove these two lines if it does not cause problems for the ixgbe. It's quite possible that that device sets the udp length field unconditionally, ignoring the previous value. In which case both devices will work after this change without additional driver logic.I would prefer we didn’t modify this. Otherwise we cannot cancel out the length from the partial checksum when the time comes. The ixgbe code was making use of it if I recall.
AFAIU, the ixgbe patch doesn't use this. Instead the length is obtained by the following code for both TCP and UDP segmentation:
paylen = skb->len - l4_offset; Could you please check to see if this is actually required?
