From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Chan Subject: Re: [PATCH net-next v3 2/5] net: Disable GRO_HW when generic XDP is installed on a device. Date: Sat, 9 Dec 2017 22:49:39 -0800 Message-ID: References: <1512800879-17934-1-git-send-email-michael.chan@broadcom.com> <1512800879-17934-3-git-send-email-michael.chan@broadcom.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Cc: David Miller , Netdev , Andrew Gospodarek , Ariel Elior , everest-linux-l2@cavium.com To: Alexander Duyck Return-path: Received: from mail-oi0-f67.google.com ([209.85.218.67]:40072 "EHLO mail-oi0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751273AbdLJGtk (ORCPT ); Sun, 10 Dec 2017 01:49:40 -0500 Received: by mail-oi0-f67.google.com with SMTP id w125so9701607oie.7 for ; Sat, 09 Dec 2017 22:49:40 -0800 (PST) In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On Sat, Dec 9, 2017 at 2:37 PM, Alexander Duyck wrote: > On Sat, Dec 9, 2017 at 1:40 PM, Michael Chan wrote: >> On Sat, Dec 9, 2017 at 10:56 AM, Alexander Duyck >> wrote: >>> I think these two lines are redundant in dev_disable_lro, since >>> netdev_update_features should propagate the disable to all of the >>> lower devices. >> >> Right. But for GRO_HW, there is no automatic propagation. > > Right, but that is also an issue since the automatic propagation is > what prevents LRO from being re-enabled on the lower devices. > >>> Also this doesn't prevent the lower devices from >>> re-enabling gro_hw. >> >> Right. You can re-enable LRO on the upper device as well. > > On the upper device yes, but not on the lower devices. That was what I > was getting at. With LRO there is netdev_sync_upper_features() and > that prevents you from enabling it if the upper device has it > disabled. The problem is right now there is nothing that sets it for > the upper devices when they are added to something like a bond so that > is one of the pieces that still has to be worked on before we can just > use the existing sync logic. I understand. But if the user really wants to re-enable LRO, he can just re-enable LRO on the upper device first and then re-enable LRO on the lower devices. To permanently disable a feature, I think additional infrastructure may be required so that the feature can be cleared in hw_features and then re-enabled later when it is allowed.