From patchwork Wed Jun 28 09:11:48 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Kauer X-Patchwork-Id: 113737 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp8789300vqr; Wed, 28 Jun 2023 02:33:51 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4pVqK9pQSVrHtpVrEyyxEXMfuaLrN6zbAQKUvVCr323OVWTqvW269ZhHxpAHWl98HRJlgM X-Received: by 2002:a05:6402:35c3:b0:51d:9a92:24f0 with SMTP id z3-20020a05640235c300b0051d9a9224f0mr7335179edc.4.1687944831456; Wed, 28 Jun 2023 02:33:51 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1687944831; cv=none; d=google.com; s=arc-20160816; b=UZryth1Gnm6Gy49RHvzncfPdZj7migrGB20/me1PmP0Olfmrp7/6Ura7cuLt4JbQAV FSZlEqgNPyvZQwAkTJtE4kbAsLp1muEeshoQdQD5wE+iexBuTS4Z48IhHxh6wjZiYYRA SqtliEJ7iSf7fwqZQ2S/fwgi4sd2S+PFqsh9QW00NCrmAPWouaQlRSQJJgOQKXUbXp+P 0STxEF1tZFhVrAAdGN3msktmgvoMIpondflakG0LGrwqdbXggOoFsMptjWudqW3hI7Kv TXnSvktZJyw7aiiLgaNDzqW0PohPTi7F3ZcCzvY6Rcbk5loBy6MvfFPOzm1sZ5qc3uvl 8pGg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:dkim-signature:dkim-signature:from; bh=X0JATWY5yUrknqFueeslFGrqKTA/ehIMFyfgxm0DAko=; fh=RnuPkm6+w1bGVhUeXVnXMMiUWX/dUwFLH2W2+z/gfa0=; b=JimYGWgo2hnZj7i68826v8gz2OXr4j45vqc681dH0zoPDizKg2/GSP0WfxuPIj/Zhe z4iLdathVbmIBvo21XN2HIrbnFYKiiR4T76p51q3Vmu0lu7Clr507Hr7+21FwbjO6ZQg m6QQavdq5mOuoou9eymHzX5H2qfNyGilyUD3TEV3h1MYD8R1nWNp3KyQ4RGyPgBtGvfU ehxZWLhajN/6vaKmqm0HVzU4pvw6nd2pc/p3vPfQnTvwPGp/vWOWkQHT8VwOqZza15Hg DR31o7bt7pRf991aPS28Ut24/Wxf6jcaxZwXtclJHYrn6fhoRkXUf1V3QgoSyyQSNWoc OvcQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linutronix.de header.s=2020 header.b=YT9A2pki; dkim=neutral (no key) header.i=@linutronix.de; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=linutronix.de Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id w11-20020a056402070b00b0051dd22715c4si322015edx.196.2023.06.28.02.33.26; Wed, 28 Jun 2023 02:33:51 -0700 (PDT) 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; Authentication-Results: mx.google.com; dkim=pass header.i=@linutronix.de header.s=2020 header.b=YT9A2pki; dkim=neutral (no key) header.i=@linutronix.de; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=linutronix.de Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233564AbjF1J3Q (ORCPT + 99 others); Wed, 28 Jun 2023 05:29:16 -0400 Received: from Galois.linutronix.de ([193.142.43.55]:37354 "EHLO galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237733AbjF1JMS (ORCPT ); Wed, 28 Jun 2023 05:12:18 -0400 From: Florian Kauer DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1687943536; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=X0JATWY5yUrknqFueeslFGrqKTA/ehIMFyfgxm0DAko=; b=YT9A2pkisAHULoNDuMKgcxtJ/xLDjo8Bji2K++hZQNxvSUA9Yt1sDoySb7tQwt3nCLFUkZ i7qA9QV5sRrnTFoqj8oMLSZgCRsCILUuN4H3TWzwZ835PP+BwbXkusjYU9TmbtxXANKTfJ klzqc21STP4LUko/26fsAXqo1FBnGRxEdLWyv450KjemJzKGytKDMMc710ONfVJVVn65By 3cVyQVB6WzZJC4MfoyIVEjgUxZXZ/C78HIICOIkxdapUWP2t7LiBXAZxvRQDpH8ZnSMCQK 2jU5xsCh+oGfBl4+chOwBxyquamM2SCYDC/8VQxKLg7nm+m2WdwWUR6Ooz+oDg== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1687943536; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=X0JATWY5yUrknqFueeslFGrqKTA/ehIMFyfgxm0DAko=; b=OtN3jhn7DV6ojc3N9cWFAKUT7L1z6nG/35cXsR33QVH1cCOhcf5Xgc+kYT2B3SmUgaY2LR Xg1eWIYpRhIzVqDg== To: Jesse Brandeburg , Tony Nguyen , "David S . Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Vedang Patel , Maciej Fijalkowski , Jithu Joseph , Andre Guedes , Simon Horman Cc: intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, kurt@linutronix.de, florian.kauer@linutronix.de Subject: [PATCH net v2] igc: Prevent garbled TX queue with XDP ZEROCOPY Date: Wed, 28 Jun 2023 11:11:48 +0200 Message-Id: <20230628091148.62256-1-florian.kauer@linutronix.de> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1769938439550848975?= X-GMAIL-MSGID: =?utf-8?q?1769938439550848975?= 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 Reviewed-by: Kurt Kanzenbach Tested-by: Kurt Kanzenbach Acked-by: Vinicius Costa Gomes --- v1 -> v2: I added some more context for further clarification, but it is also just how I interpret the code. Also the typo is fixed and it is reverse christmas again ;-) --- drivers/net/ethernet/intel/igc/igc_main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c index eb4f0e562f60..1cb1df613d27 100644 --- a/drivers/net/ethernet/intel/igc/igc_main.c +++ b/drivers/net/ethernet/intel/igc/igc_main.c @@ -2791,15 +2791,15 @@ static void igc_xdp_xmit_zc(struct igc_ring *ring) struct netdev_queue *nq = txring_txq(ring); union igc_adv_tx_desc *tx_desc = NULL; int cpu = smp_processor_id(); - u16 ntu = ring->next_to_use; struct xdp_desc xdp_desc; - u16 budget; + u16 budget, ntu; if (!netif_carrier_ok(ring->netdev)) return; __netif_tx_lock(nq, cpu); + ntu = ring->next_to_use; budget = igc_desc_unused(ring); while (xsk_tx_peek_desc(pool, &xdp_desc) && budget--) {