From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kevin Easton Subject: Re: [net-next 06/12] i40e/ixgbe/igb: fail on new WoL flag setting WAKE_MAGICSECURE Date: Thu, 8 Nov 2018 06:05:26 +0000 Message-ID: <20181108060526.GA11230@ip-172-31-15-78> References: <20181107224830.9737-1-jeffrey.t.kirsher@intel.com> <20181107224830.9737-7-jeffrey.t.kirsher@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: davem@davemloft.net, Todd Fujinaka , netdev@vger.kernel.org, nhorman@redhat.com, sassmann@redhat.com To: Jeff Kirsher Return-path: Received: from aws.guarana.org ([13.237.110.252]:33302 "EHLO aws.guarana.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726162AbeKHPqp (ORCPT ); Thu, 8 Nov 2018 10:46:45 -0500 Content-Disposition: inline In-Reply-To: <20181107224830.9737-7-jeffrey.t.kirsher@intel.com> Sender: netdev-owner@vger.kernel.org List-ID: On Wed, Nov 07, 2018 at 02:48:24PM -0800, Jeff Kirsher wrote: > From: Todd Fujinaka > > There's a new flag for setting WoL filters that is only > enabled on one manufacturer's NICs, and it's not ours. Fail > with EOPNOTSUPP. > > Signed-off-by: Todd Fujinaka > Tested-by: Andrew Bowers > Signed-off-by: Jeff Kirsher > --- > drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 3 ++- > drivers/net/ethernet/intel/igb/igb_ethtool.c | 2 +- > drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 3 ++- > 3 files changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c > index 9f8464f80783..9c1211ad2c6b 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c > +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c > @@ -2377,7 +2377,8 @@ static int i40e_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol) > return -EOPNOTSUPP; > > /* only magic packet is supported */ > - if (wol->wolopts && (wol->wolopts != WAKE_MAGIC)) > + if (wol->wolopts && (wol->wolopts != WAKE_MAGIC) > + | (wol->wolopts != WAKE_FILTER)) > return -EOPNOTSUPP; This doesn't look right. WAKE_MAGIC and WAKE_FILTER are distinct, so (wol->wolopts != WAKE_MAGIC) | (wol->wolopts != WAKE_FILTER) will always be 1. It looks like the existing test in this driver was fine - it *only* accepted wol->wolopts of either 0 or WAKE_MAGIC, it was already rejecting everything else including WAKE_FILTER. Suggest you drop that hunk. - Kevin > > > /* is this a new value? */ > diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c > index 5acf3b743876..c57671068245 100644 > --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c > +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c > @@ -2113,7 +2113,7 @@ static int igb_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol) > { > struct igb_adapter *adapter = netdev_priv(netdev); > > - if (wol->wolopts & (WAKE_ARP | WAKE_MAGICSECURE)) > + if (wol->wolopts & (WAKE_ARP | WAKE_MAGICSECURE | WAKE_FILTER)) > return -EOPNOTSUPP; > > if (!(adapter->flags & IGB_FLAG_WOL_SUPPORTED)) > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c > index 732b1e6ecc43..acba067cc15a 100644 > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c > @@ -2206,7 +2206,8 @@ static int ixgbe_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol) > { > struct ixgbe_adapter *adapter = netdev_priv(netdev); > > - if (wol->wolopts & (WAKE_PHY | WAKE_ARP | WAKE_MAGICSECURE)) > + if (wol->wolopts & (WAKE_PHY | WAKE_ARP | WAKE_MAGICSECURE | > + WAKE_FILTER)) > return -EOPNOTSUPP; > > if (ixgbe_wol_exclusion(adapter, wol)) > -- > 2.19.1 > > >