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;
Date:   Fri, 30 Jun 2023 15:20:54 +0200
From:   Simon Horman <simon.horman@corigine.com>
To:     Florian Kauer <florian.kauer@linutronix.de>
Cc:     Jesse Brandeburg <jesse.brandeburg@intel.com>,
        Tony Nguyen <anthony.l.nguyen@intel.com>,
        "David S . Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Vedang Patel <vedang.patel@intel.com>,
        Maciej Fijalkowski <maciej.fijalkowski@intel.com>,
        Jithu Joseph <jithu.joseph@intel.com>,
        Andre Guedes <andre.guedes@intel.com>,
        intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org, kurt@linutronix.de,
        Vinicius Costa Gomes <vinicius.gomes@intel.com>
Subject: Re: [PATCH net v3] igc: Prevent garbled TX queue with XDP ZEROCOPY
Message-ID: <ZJ7WtoU/doUGD7dW@corigine.com>
References: <20230630120306.8534-1-florian.kauer@linutronix.de>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230630120306.8534-1-florian.kauer@linutronix.de>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?FqYWJkjwQyT+cgy061gQ4nM89lgTDL/j7JuUjT6hhiVTbLzpwMREQzrFyuf4?=
 =?us-ascii?Q?vI+g2wfAFEe5e5VmZ8j2M2aXKRxTesbneVxDsE12cw87o27Lw3Zg/1qBbAbe?=
 =?us-ascii?Q?mo86hAMds8Ep+esemE3qcSZHSRXFWtWGbPsqKkMIxQ+3xxezBIBIKM4fsaB5?=
 =?us-ascii?Q?+Ul0CvGtFlIMAJsw0zi9RAoqKKt1ZOP5TMCvf2DmjErqaNIKzY6U28BwQvmv?=
 =?us-ascii?Q?SKLpsZRA8u9Ce+KkapWRrU1+LBcXcW6A4kVCiVrCyvQZG2yumhimQpoiD0Fc?=
 =?us-ascii?Q?OkHnC2AIRUNQLxwmRvvGheF4cKhjz3EzZMVzpTB/6DotOEsbgXEk/q48PQzv?=
 =?us-ascii?Q?TulpOTqW7a7ze7yhfmQRxskx7uM3r8VIXMPc7BWccYRDus95dl8Nh6eYXGg1?=
 =?us-ascii?Q?siHt6TImedXkzgkSXFF5z5YSMq81lmDIxleGgwoJYMErCXxXEsBL2CXDtXRA?=
 =?us-ascii?Q?RKE1u4uJIvcwW91KdrEffqGV17j6ACjY/TVhyY0MLhgymUlHeqkdMN1A/MWM?=
 =?us-ascii?Q?A+gG/LS46yRaAmud09q9Qm4B9tNUMzmhKbyOXaF1di+UDMsHxPHT5fLfG+De?=
 =?us-ascii?Q?XyPZbpd2UW2LeaVgzeHL5AFHICPs3f2mv4q9xMM3nabggz3uGKWnqrfPI8B+?=
 =?us-ascii?Q?k6pyi2OBYMw2JCDHJmvkY2YPhY1th46IUsZ4k3lNJJhpi9xgmw2lRVkEiBJB?=
 =?us-ascii?Q?sxY3gLBoUy1ETBpWGtApqRq6MYWwdc1Wfcoes1/tAypSqiTk69c8U+QK+dTd?=
 =?us-ascii?Q?KwqO4d3Er0VYZ3AWB2Oxr+IEJxM9BLxztbpw21YBpvL+NFvHrgCSI75LELSF?=
 =?us-ascii?Q?Vx7ye4ka7SrppCxHW+sWXhO2B7eQnF8nwsYm1JxFW6AF2QM3N7g9qFW8/+LZ?=
 =?us-ascii?Q?3weLsrchY5VvwvJHQHhi1guq6k6fuB8qqnMCdX8TOPlv0f5FTdv6iS9+ZFBu?=
 =?us-ascii?Q?JfytMCTBzlC81E1u5T5ZRAZkCzz4UNB/EbMQBaf25A2E3l1wzMoYhi10prNF?=
 =?us-ascii?Q?Anlzut6nwFNIEVNQlBnH1EbqpRUshHag0raBgXI49wCiLvZtBrzxN7PR3j+g?=
 =?us-ascii?Q?JXOBYu8nF6ISXeqf+kGAM4krbZyfq0oyMr4TKG4OQaTUF/+tFOjGf9G0FyqW?=
 =?us-ascii?Q?VoSp538JJRT+IGX/n6Fodhqa6ySZusYdg82frubwbrfbvyxgoPhP+EBUxRrF?=
 =?us-ascii?Q?Tws0xGTYgJ0Kof170fAMIlEwVoxlwC/VKvi7Tgp1bSKH+Kvd3d09FRBIkGvZ?=
 =?us-ascii?Q?ITi1iPWLwMuBck4zDqPrc7iyGxPSnDg4E5zvHkKpdwPbE0+0o0JWsRhXXQAB?=
 =?us-ascii?Q?1YOZPeV7YuMdIFv3pB6NWKhV92OggyCGLetfMYsq39hcSMRroIvUqm51vspV?=
 =?us-ascii?Q?/TW/Y13ppadkhkeIyzi/LRmGHc8LP3peSf3kEdLINWizDd7zhvpzXNA4r5sH?=
 =?us-ascii?Q?xkoQ4ez81AIU/tjsegXxY7xgdW/Qm9cpDVzXJeW7gL8G1fsoYHcHjC5Gcyy6?=
 =?us-ascii?Q?6SAFRbrFHXUbgaksTyIn8Hp9nT18H7BQYogotOgH5oyEnsyf8isieJ83m4/U?=
 =?us-ascii?Q?gd9Y4fn0Zab3JezpX+QAWeX3NuYfuHEPDk2pZkTjKxAK2lyAL+TMD6XMQHxc?=
 =?us-ascii?Q?X1tpy6RQSSPne1TQjn7OlZoEY8eeznIUrxWMMuFdZmxVLZrXBe7fNP+69pGH?=
 =?us-ascii?Q?AElWxg=3D=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 000c107b-4e3f-4b6d-7cba-08db796cd984
X-MS-Exchange-CrossTenant-AuthSource: PH0PR13MB4842.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Jun 2023 13:21:01.6760
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: fe128f2c-073b-4c20-818e-7246a585940c
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Dt6RFtUNodjn36en8xnfNAz8NzmlicESJ5C+eKlbWB2ZMHZeJ0ee0HhqdoY16khz6ClB+vUPeDdcbetkxsWE+4yJyelXd3tpOS8MWgAXBD8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW3PR13MB3995
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,
        T_SCC_BODY_TEXT_LINE autolearn=ham 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

On Fri, Jun 30, 2023 at 02:03:06PM +0200, Florian Kauer wrote:
> In normal operation, each populated queue item has
> next_to_watch pointing to the last TX desc of the packet,
> while each cleaned item has it set to 0. In particular,
> next_to_use that points to the next (necessarily clean)
> item to use has next_to_watch set to 0.
> 
> When the TX queue is used both by an application using
> AF_XDP with ZEROCOPY as well as a second non-XDP application
> generating high traffic, the queue pointers can get in
> an invalid state where next_to_use points to an item
> where next_to_watch is NOT set to 0.
> 
> However, the implementation assumes at several places
> that this is never the case, so if it does hold,
> bad things happen. In particular, within the loop inside
> of igc_clean_tx_irq(), next_to_clean can overtake next_to_use.
> Finally, this prevents any further transmission via
> this queue and it never gets unblocked or signaled.
> Secondly, if the queue is in this garbled state,
> the inner loop of igc_clean_tx_ring() will never terminate,
> completely hogging a CPU core.
> 
> The reason is that igc_xdp_xmit_zc() reads next_to_use
> before acquiring the lock, and writing it back
> (potentially unmodified) later. If it got modified
> before locking, the outdated next_to_use is written
> pointing to an item that was already used elsewhere
> (and thus next_to_watch got written).
> 
> Fixes: 9acf59a752d4 ("igc: Enable TX via AF_XDP zero-copy")
> Signed-off-by: Florian Kauer <florian.kauer@linutronix.de>
> Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de>
> Tested-by: Kurt Kanzenbach <kurt@linutronix.de>
> Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>