Message ID | 1686179653-29750-1-git-send-email-justin.chen@broadcom.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp551868vqr; Wed, 7 Jun 2023 16:38:16 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ67MnuJkKLqPUzSU5twde8YDUaIiEE9k+IloWw9CXF1FApj8+IOfodxw77/39pZSMN2p5jk X-Received: by 2002:a17:902:dacc:b0:1af:feff:5a70 with SMTP id q12-20020a170902dacc00b001affeff5a70mr8121094plx.11.1686181095797; Wed, 07 Jun 2023 16:38:15 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1686181095; cv=none; d=google.com; s=arc-20160816; b=fQSvEuTdC/Ah6paaRbAY/Dez6+2+k9FCcJAX4G4SE3rFlnzmym+ZsFrrdXkl5MzAE1 1+GsF4/sXUMMlbDwO9FCDeg2Ad5ii8fnrlNbq8Di6xJVeLhkLZVRRvfmskS8Otbaoi+x LySk0obwb0AKuHieFP5zBoeTwGkntCUyHmA/9Z50alruNqsugRulouZO8sbF3IMuvSCG Oq1fM1NrxWaeaylhdYXGk4ZuQaVPYURrqEem0wXNuqZZWGMnDvEfAGYUYdqzsA/HEgon TXvg34bURIqmOICK9tZeRoWNnQxGWuMcQlNl1WGRC3NOMGcEOD6qkuYPoemFTRKsvDOd LS7w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:message-id:date:subject:cc:to:from :dkim-signature; bh=FXM2quBWNXOI6ViXw9QSKr/R4Ruji0gMPru1R5l3oXw=; b=Wl8T5qjAd7c1OeKRKnKtftQSXZWfddnnlxCLvBjs8tJ3Jqu6QCmwWP+pVt1GQVFUFJ L7G44ervqHoEhHbtztO7NhyHCMLpw0Mkc6myEAzgtMxEiyxzrZN2X97QLNGrXGFgYpXC abJ4yOtlnvm0DKtbSj/EcQy/MMpE3BrzNGg3+csGmzI7SS7QqZXCcJAY53Y9aSoqKe7F 9L36C+BLCv8pkvWWdFLTX1xqYXhWpNAfiFE/zc9IxHrCY7e9UECnPf1wSIzw8EmEFHfB dfrxGruGAvAi0if8I7y818ulM7q9WAfcWMx2GG201B2McgX4PNU+NRu886yZycVrdcdY vx4A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@broadcom.com header.s=google header.b=GXPe7tah; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=broadcom.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id k18-20020a170902c41200b001a6a3954e0fsi127180plk.18.2023.06.07.16.38.01; Wed, 07 Jun 2023 16:38:15 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@broadcom.com header.s=google header.b=GXPe7tah; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=broadcom.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232130AbjFGXOY (ORCPT <rfc822;literming00@gmail.com> + 99 others); Wed, 7 Jun 2023 19:14:24 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35188 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229646AbjFGXOW (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 7 Jun 2023 19:14:22 -0400 Received: from mail-pl1-x62c.google.com (mail-pl1-x62c.google.com [IPv6:2607:f8b0:4864:20::62c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 544CC1BD4 for <linux-kernel@vger.kernel.org>; Wed, 7 Jun 2023 16:14:21 -0700 (PDT) Received: by mail-pl1-x62c.google.com with SMTP id d9443c01a7336-1b04706c974so69279985ad.2 for <linux-kernel@vger.kernel.org>; Wed, 07 Jun 2023 16:14:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google; t=1686179661; x=1688771661; h=message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=FXM2quBWNXOI6ViXw9QSKr/R4Ruji0gMPru1R5l3oXw=; b=GXPe7tah44WTUqkp11sFtP9JULVYq/PmZBGngH4KzkCVa8Q8GLE8t1D+fWROZ2AHfA sJiEVBOz53LRObW3H4s02NOt/7F3ds1Ym6T9NfistsPNauG4qgG+OIiEMxUjP/aJv2Ce m4SJn9AbbyaVNu38qkWokRCYEcjzlmpdwZo44= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686179661; x=1688771661; h=message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=FXM2quBWNXOI6ViXw9QSKr/R4Ruji0gMPru1R5l3oXw=; b=j5jfmJcDB5/NpLUvU0ldhadtB8TUbwENCHFtJp7+rnIwdC4Ktayp87i+e6CPKmJC09 BevQ8H1EqEX46hYRBWSOI9f8WRL5pUwe6WuurcgLwU9c/079/Zi73iiw0TCbxvhJAbO1 ewiRLuP6SNU3+d1CeSkQP58/2dz8ZSqjLYIWQAnvUnwNYgUcPB2BatdicWJNqBqswJmO zSR6I89lMcbVeQmnLPDePC9APJga7unYicxlZ+YMQJJyH/+9Ntyoj4+w8mo9CFVhWijR BMzakyFp1WSqi3OPbA1I/UjbmTYmlay876v4NtoCQFf1nhghUVcribbv8cgh3RdS1ofg Y8sA== X-Gm-Message-State: AC+VfDyCBcltGn1TPk+zzj0GtBg5FCiNnfPJiE1znV7lfrwetwgTN1+/ lfXe1xkzUHr0UEwbopbmdmw/Aw== X-Received: by 2002:a17:902:c411:b0:1ac:5717:fd5 with SMTP id k17-20020a170902c41100b001ac57170fd5mr9331279plk.60.1686179660780; Wed, 07 Jun 2023 16:14:20 -0700 (PDT) Received: from stbirv-lnx-2.igp.broadcom.net ([192.19.223.252]) by smtp.gmail.com with ESMTPSA id l13-20020a170903244d00b001ae4d4d2676sm16279pls.269.2023.06.07.16.14.19 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Wed, 07 Jun 2023 16:14:20 -0700 (PDT) From: Justin Chen <justin.chen@broadcom.com> To: netdev@vger.kernel.org Cc: bcm-kernel-feedback-list@broadcom.com, florian.fainelli@broadcom.com, Justin Chen <justin.chen@broadcom.com>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Andrew Lunn <andrew@lunn.ch>, Daniil Tatianin <d-tatianin@yandex-team.ru>, Ido Schimmel <idosch@nvidia.com>, Marco Bonelli <marco@mebeim.net>, Wolfram Sang <wsa+renesas@sang-engineering.com>, Jiri Pirko <jiri@resnulli.us>, Gal Pressman <gal@nvidia.com>, Vincent Mailhol <mailhol.vincent@wanadoo.fr>, Kuniyuki Iwashima <kuniyu@amazon.com>, linux-kernel@vger.kernel.org (open list) Subject: [PATCH v2 net-next] ethtool: ioctl: improve error checking for set_wol Date: Wed, 7 Jun 2023 16:14:11 -0700 Message-Id: <1686179653-29750-1-git-send-email-justin.chen@broadcom.com> X-Mailer: git-send-email 2.7.4 Content-Type: multipart/signed; protocol="application/pkcs7-signature"; micalg=sha-256; boundary="000000000000b9f42b05fd924b77" X-Spam-Status: No, score=-1.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF, MIME_HEADER_CTYPE_ONLY,MIME_NO_TEXT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE, SPF_NONE,T_SCC_BODY_TEXT_LINE,T_TVD_MIME_NO_HEADERS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1768089028346415063?= X-GMAIL-MSGID: =?utf-8?q?1768089028346415063?= |
Series |
[v2,net-next] ethtool: ioctl: improve error checking for set_wol
|
|
Commit Message
Justin Chen
June 7, 2023, 11:14 p.m. UTC
The netlink version of set_wol checks for not supported wolopts and avoids setting wol when the correct wolopt is already set. If we do the same with the ioctl version then we can remove these checks from the driver layer. Reviewed-by: Simon Horman <simon.horman@corigine.com> Signed-off-by: Justin Chen <justin.chen@broadcom.com> --- v2 - Return -EINVAL instead of -EOPNOTSUPP net/ethtool/ioctl.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)
Comments
On 6/7/2023 4:14 PM, Justin Chen wrote: > The netlink version of set_wol checks for not supported wolopts and avoids > setting wol when the correct wolopt is already set. If we do the same with > the ioctl version then we can remove these checks from the driver layer. > > Reviewed-by: Simon Horman <simon.horman@corigine.com> > Signed-off-by: Justin Chen <justin.chen@broadcom.com> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Wed, 7 Jun 2023 16:14:11 -0700 you wrote: > The netlink version of set_wol checks for not supported wolopts and avoids > setting wol when the correct wolopt is already set. If we do the same with > the ioctl version then we can remove these checks from the driver layer. > > Reviewed-by: Simon Horman <simon.horman@corigine.com> > Signed-off-by: Justin Chen <justin.chen@broadcom.com> > > [...] Here is the summary with links: - [v2,net-next] ethtool: ioctl: improve error checking for set_wol https://git.kernel.org/netdev/net-next/c/55b24334c0f2 You are awesome, thank you!
On 6/7/23 4:14 PM, Justin Chen wrote: > The netlink version of set_wol checks for not supported wolopts and avoids > setting wol when the correct wolopt is already set. If we do the same with > the ioctl version then we can remove these checks from the driver layer. > > Reviewed-by: Simon Horman <simon.horman@corigine.com> > Signed-off-by: Justin Chen <justin.chen@broadcom.com> > --- > v2 > - Return -EINVAL instead of -EOPNOTSUPP > > net/ethtool/ioctl.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c > index 6bb778e10461..37b582225854 100644 > --- a/net/ethtool/ioctl.c > +++ b/net/ethtool/ioctl.c > @@ -1436,15 +1436,25 @@ static int ethtool_get_wol(struct net_device *dev, char __user *useraddr) > > static int ethtool_set_wol(struct net_device *dev, char __user *useraddr) > { > - struct ethtool_wolinfo wol; > + struct ethtool_wolinfo wol, cur_wol; > int ret; > > - if (!dev->ethtool_ops->set_wol) > + if (!dev->ethtool_ops->get_wol || !dev->ethtool_ops->set_wol) > return -EOPNOTSUPP; > > + memset(&cur_wol, 0, sizeof(struct ethtool_wolinfo)); > + cur_wol.cmd = ETHTOOL_GWOL; > + dev->ethtool_ops->get_wol(dev, &cur_wol); > + > if (copy_from_user(&wol, useraddr, sizeof(wol))) > return -EFAULT; > > + if (wol.wolopts & ~cur_wol.supported) > + return -EINVAL; > + > + if (wol.wolopts == cur_wol.wolopts) > + return 0; > + > ret = dev->ethtool_ops->set_wol(dev, &wol); > if (ret) > return ret; Was thinking more about this patch. I realized we don't account for the different sopass case. # ethtool -s eth0 wol s sopass 11:22:33:44:55:66 # ethtool -s eth0 wol s sopass 22:44:55:66:77:88 For this case, the second sopass values won't be stored. Can you drop this patch? I will submit another version. Thanks, Justin
On Fri, 9 Jun 2023 13:47:22 -0700 Justin Chen wrote: > Was thinking more about this patch. I realized we don't account for the > different sopass case. > # ethtool -s eth0 wol s sopass 11:22:33:44:55:66 > # ethtool -s eth0 wol s sopass 22:44:55:66:77:88 > > For this case, the second sopass values won't be stored. > > Can you drop this patch? I will submit another version. We can't drop patches, it'd mess up commit IDs and basing trees on top of net-next would be a major PITA for people. Please send a fix on top (with a Fixes tag making it clear that the problem has not reached any -rc kernel).
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c index 6bb778e10461..37b582225854 100644 --- a/net/ethtool/ioctl.c +++ b/net/ethtool/ioctl.c @@ -1436,15 +1436,25 @@ static int ethtool_get_wol(struct net_device *dev, char __user *useraddr) static int ethtool_set_wol(struct net_device *dev, char __user *useraddr) { - struct ethtool_wolinfo wol; + struct ethtool_wolinfo wol, cur_wol; int ret; - if (!dev->ethtool_ops->set_wol) + if (!dev->ethtool_ops->get_wol || !dev->ethtool_ops->set_wol) return -EOPNOTSUPP; + memset(&cur_wol, 0, sizeof(struct ethtool_wolinfo)); + cur_wol.cmd = ETHTOOL_GWOL; + dev->ethtool_ops->get_wol(dev, &cur_wol); + if (copy_from_user(&wol, useraddr, sizeof(wol))) return -EFAULT; + if (wol.wolopts & ~cur_wol.supported) + return -EINVAL; + + if (wol.wolopts == cur_wol.wolopts) + return 0; + ret = dev->ethtool_ops->set_wol(dev, &wol); if (ret) return ret;