From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jakub Kicinski Subject: Re: [net-next 1/6] ixgbe: Do not allow LRO or MTU change with XDP Date: Thu, 26 Jul 2018 14:21:08 -0700 Message-ID: <20180726142108.158b69a1@cakuba.netronome.com> References: <20180726174050.8923-1-jeffrey.t.kirsher@intel.com> <20180726174050.8923-2-jeffrey.t.kirsher@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: davem@davemloft.net, Tony Nguyen , netdev@vger.kernel.org, nhorman@redhat.com, sassmann@redhat.com, jogreene@redhat.com To: Jeff Kirsher Return-path: Received: from mail-qk0-f196.google.com ([209.85.220.196]:45468 "EHLO mail-qk0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730452AbeGZWjv (ORCPT ); Thu, 26 Jul 2018 18:39:51 -0400 Received: by mail-qk0-f196.google.com with SMTP id c192-v6so1993804qkg.12 for ; Thu, 26 Jul 2018 14:21:12 -0700 (PDT) In-Reply-To: <20180726174050.8923-2-jeffrey.t.kirsher@intel.com> Sender: netdev-owner@vger.kernel.org List-ID: On Thu, 26 Jul 2018 10:40:45 -0700, Jeff Kirsher wrote: > From: Tony Nguyen > > XDP does not support jumbo frames or LRO. These checks are being made > outside the driver when an XDP program is loaded, however, there is > nothing preventing these from changing after an XDP program is loaded. > Add the checks so that while an XDP program is loaded, do not allow MTU > to be changed or LRO to be enabled. > > Signed-off-by: Tony Nguyen > Tested-by: Andrew Bowers > Signed-off-by: Jeff Kirsher > --- > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > index 5a6600f7b382..c42256e91997 100644 > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > @@ -6469,6 +6469,11 @@ static int ixgbe_change_mtu(struct net_device *netdev, int new_mtu) > { > struct ixgbe_adapter *adapter = netdev_priv(netdev); > > + if (adapter->xdp_prog) { > + e_warn(probe, "MTU cannot be changed while XDP program is loaded\n"); > + return -EPERM; EPERM looks wrong, EINVAL is common. Also most drivers will just check the bounds like you do in ixgbe_xdp_setup(), allowing the change if new MTU still fits constraints. FWIW are the IXGBE_FLAG_SRIOV_ENABLED and IXGBE_FLAG_DCB_ENABLED flag changes also covered while xdp is enabled? Quick grep doesn't reveal them being checked against xdp_prog other than in ixgbe_xdp_setup(). > + } > + > /* > * For 82599EB we cannot allow legacy VFs to enable their receive > * paths when MTU greater than 1500 is configured. So display a > @@ -9407,6 +9412,11 @@ static netdev_features_t ixgbe_fix_features(struct net_device *netdev, > if (!(adapter->flags2 & IXGBE_FLAG2_RSC_CAPABLE)) > features &= ~NETIF_F_LRO; > > + if (adapter->xdp_prog && (features & NETIF_F_LRO)) { > + e_dev_err("LRO is not supported with XDP\n"); > + features &= ~NETIF_F_LRO; > + } > + > return features; > } >