From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-11.4 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id F0AD7C35E01 for ; Tue, 25 Feb 2020 17:17:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C0A8C20732 for ; Tue, 25 Feb 2020 17:17:32 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="l2inbIhk" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729867AbgBYRRc (ORCPT ); Tue, 25 Feb 2020 12:17:32 -0500 Received: from mail-wm1-f66.google.com ([209.85.128.66]:34014 "EHLO mail-wm1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727983AbgBYRRb (ORCPT ); Tue, 25 Feb 2020 12:17:31 -0500 Received: by mail-wm1-f66.google.com with SMTP id i10so1553050wmd.1 for ; Tue, 25 Feb 2020 09:17:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=enwbul+2wY5IaL2JlcSBQKs2gWt8zyhe/5zh20sr06g=; b=l2inbIhkh97XFdP0v95gmdfpL/95QPmjNN2xdqL+NyMIrf6u0tuz5y6TApXILUaERI jHVZo1JeZbKMCSYG5pA/cEVrvkQANvgPwB3jROSZ9mRycapVsjLNndk6uJAJCReEzEa/ rm6QVLJsCQERC85nsfiHGfCUdjaX043lRfeX+x8hblbNUh8yihHg6f0G7y4KDnU0zK6U j3N1n74tAECXqoXk9YZ1C0zvYiIM5xrRiKZJAxNf+wRX+3PB3eL9ovft1gkHa492nemP N1cvJ11Ib5H0wiwLo7wrWlzZxxHW0bbWYMbxVypXDn8aW5UgvTnVxZ+BIYbtMcsC+eGe zhWQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=enwbul+2wY5IaL2JlcSBQKs2gWt8zyhe/5zh20sr06g=; b=oGg683NFbQUTymsAYW+CgHiHyqnGS28c9F6afJ6Me1B17HL3GEO8g+Ynt1Ze+DJcw1 p0WlfMWirwMzCUEGOF9RD8JUWpeGL8SCjfNwTR/fhNgt/lrTW37/rwjLlAJJn9Sgwwqq 5mtVjJjDzTy/ZLCzglJLBDNaYA6fypE4obxg5/5t7xs9TwK8bq1Nk4g34REm3AhY4TYR RQLn1MNiSOo+nhoh77WBiimfmOBow8Fxl9Rkmkru5FnjC8utwW5movmd2SsWyV2VESTy WTV8mF1Don9Yj0S84L9SrxvYnq8awcZW+CdbqBc4CPpY3KvAFXBlfj9hF8q4VdKJKLSm AkMw== X-Gm-Message-State: APjAAAUEaPf/WhYui4OXvY8lGm0aQq1hyrMiNmuo1UzktUT9CBjukPw+ p4l60k7i2LX7RE5ZnU9IpgeI2dlGmgxaI21uu83z0g== X-Google-Smtp-Source: APXvYqwAMj6EObdSsgO/Hs6YMVU6vXPQ6obpY2yFXjj/C9UE/aQOU2z0FBHoCHpJvotcSsaAXkMlxHKCAynbEd/+tCs= X-Received: by 2002:a05:600c:217:: with SMTP id 23mr285633wmi.124.1582651048921; Tue, 25 Feb 2020 09:17:28 -0800 (PST) MIME-Version: 1.0 References: <20200225060620.76486-1-arjunroy.kdev@gmail.com> In-Reply-To: From: Soheil Hassas Yeganeh Date: Tue, 25 Feb 2020 12:16:52 -0500 Message-ID: Subject: Re: [PATCH net-next] tcp-zerocopy: Update returned getsockopt() optlen. To: Arjun Roy Cc: Eric Dumazet , Arjun Roy , David Miller , netdev , Willem de Bruijn Content-Type: text/plain; charset="UTF-8" Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Tue, Feb 25, 2020 at 12:04 PM Arjun Roy wrote: > > On Tue, Feb 25, 2020 at 8:48 AM Arjun Roy wrote: > > > > On Mon, Feb 24, 2020 at 10:28 PM Eric Dumazet wrote: > > > > > > On Mon, Feb 24, 2020 at 10:06 PM Arjun Roy wrote: > > > > > > > > From: Arjun Roy > > > > > > > > TCP receive zerocopy currently does not update the returned optlen for > > > > getsockopt(). Thus, userspace cannot properly determine if all the > > > > fields are set in the passed-in struct. This patch sets the optlen > > > > before return, in keeping with the expected operation of getsockopt(). > > > > > > > > Signed-off-by: Arjun Roy > > > > Signed-off-by: Eric Dumazet > > > > Signed-off-by: Soheil Hassas Yeganeh > > > > Signed-off-by: Willem de Bruijn > > > > Fixes: c8856c051454 ("tcp-zerocopy: Return inq along with tcp receive > > > > zerocopy") > > > > > > > > > OK, please note for next time : > > > > > > Fixes: tag should not wrap : It should be a single line. > > > Preferably it should be the first tag (before your Sob) > > > > > > Add v2 as in [PATCH v2 net-next] : so that reviewers can easily see > > > which version is the more recent one. > > > > > > > > > > > > > > + if (!err) { > > > > + if (put_user(len, optlen)) > > > > + return -EFAULT; > > > > > > Sorry for not asking this before during our internal review : > > > > > > Is the cost of the extra STAC / CLAC (on x86) being high enough that it is worth > > > trying to call put_user() only if user provided a different length ? > > > > I'll have to defer to someone with more understanding of the overheads > > involved in this case. > > > > Actually, now that I think about it, the (hopefully) common case is > indeed that the kernel and userspace agree on the size of the struct, > so I think just having just that one extra branch to check before > issuing a put_user() would be well worth it compared to all the > instructions in put_user(). I'll send a v2 patch with the change. Thank you, Arjun. Given that most TCP socket options overwrite the optlen even when returning error, I think we can avoid having the extra branch by simply moving put_user right after the check for "len == sizeof(zc)" and before "switch(len)". Thanks, Soheil > Thanks, > -Arjun > > > -Arjun