On 9/30/19 4:56 PM, Alexander Duyck wrote:
On Mon, Sep 30, 2019 at 3:13 PM Josh Hunt <joh...@akamai.com> wrote:Prior to this change an application sending <= 1MSS worth of data and enabling UDP GSO would fail if the system had SW GSO enabled, but the same send would succeed if HW GSO offload is enabled. In addition to this inconsistency the error in the SW GSO case does not get back to the application if sending out of a real device so the user is unaware of this failure. With this change we only perform GSO if the # of segments is > 1 even if the application has enabled segmentation. I've also updated the relevant udpgso selftests. Fixes: bec1f6f69736 ("udp: generate gso with UDP_SEGMENT") Signed-off-by: Josh Hunt <joh...@akamai.com> --- net/ipv4/udp.c | 5 +++-- net/ipv6/udp.c | 5 +++-- tools/testing/selftests/net/udpgso.c | 16 ++++------------ 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c index be98d0b8f014..ac0baf947560 100644 --- a/net/ipv4/udp.c +++ b/net/ipv4/udp.c @@ -821,6 +821,7 @@ static int udp_send_skb(struct sk_buff *skb, struct flowi4 *fl4, int is_udplite = IS_UDPLITE(sk); int offset = skb_transport_offset(skb); int len = skb->len - offset; + int datalen = len - sizeof(*uh); __wsum csum = 0; /* @@ -832,7 +833,7 @@ static int udp_send_skb(struct sk_buff *skb, struct flowi4 *fl4, uh->len = htons(len); uh->check = 0; - if (cork->gso_size) { + if (cork->gso_size && datalen > cork->gso_size) { const int hlen = skb_network_header_len(skb) + sizeof(struct udphdr);So what about the datalen == cork->gso_size case? That would only generate one segment wouldn't it? Shouldn't the test really be "datalen < cork->gso_size"? That should be the only check you need since if gso_size is 0 this statement would always fail anyway. Thanks. - Alex
Alex thanks for the review. The intent of the patch is to only use GSO when the # of segs > 1. The two cases you've mentioned are when the # of segs == 1. In those cases we don't want to set gso_size and treat this as a non-GSO case, skipping the if block. Let me know if I misunderstood your points or you'd like further clarification.
Thanks! Josh
