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:47:10 -0700 Message-ID: <20180726144710.0124aee2@cakuba.netronome.com> References: <20180726174050.8923-1-jeffrey.t.kirsher@intel.com> <20180726174050.8923-2-jeffrey.t.kirsher@intel.com> <20180726142108.158b69a1@cakuba.netronome.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-qt0-f196.google.com ([209.85.216.196]:32869 "EHLO mail-qt0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730533AbeGZXF6 (ORCPT ); Thu, 26 Jul 2018 19:05:58 -0400 Received: by mail-qt0-f196.google.com with SMTP id c15-v6so3167003qtp.0 for ; Thu, 26 Jul 2018 14:47:15 -0700 (PDT) In-Reply-To: <20180726142108.158b69a1@cakuba.netronome.com> Sender: netdev-owner@vger.kernel.org List-ID: On Thu, 26 Jul 2018 14:21:08 -0700, Jakub Kicinski wrote: > 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(). Ah, I didn't make the review in time :) Could you follow up?