linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Kauer <florian.kauer@linutronix.de>
To: Jesse Brandeburg <jesse.brandeburg@intel.com>,
	Tony Nguyen <anthony.l.nguyen@intel.com>,
	Vinicius Costa Gomes <vinicius.gomes@intel.com>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Tan Tee Min <tee.min.tan@linux.intel.com>,
	Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>,
	Aravindhan Gunasekaran <aravindhan.gunasekaran@intel.com>,
	Malli C <mallikarjuna.chilakala@intel.com>
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 2/6] igc: Do not enable taprio offload for invalid arguments
Date: Mon, 19 Jun 2023 12:08:54 +0200	[thread overview]
Message-ID: <20230619100858.116286-3-florian.kauer@linutronix.de> (raw)
In-Reply-To: <20230619100858.116286-1-florian.kauer@linutronix.de>

Only set adapter->taprio_offload_enable after validating the arguments.
Otherwise, it stays set even if the offload was not enabled.
Since the subsequent code does not get executed in case of invalid
arguments, it will not be read at first.
However, by activating and then deactivating another offload
(e.g. ETF/TX launchtime offload), taprio_offload_enable is read
and erroneously keeps the offload feature of the NIC enabled.

This can be reproduced as follows:

    # TAPRIO offload (flags == 0x2) and negative base-time leading to expected -ERANGE
    sudo tc qdisc replace dev enp1s0 parent root handle 100 stab overhead 24 taprio \
	    num_tc 1 \
	    map 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 \
	    queues 1@0 \
	    base-time -1000 \
	    sched-entry S 01 300000 \
	    flags 0x2

    # IGC_TQAVCTRL is 0x0 as expected (iomem=relaxed for reading register)
    sudo pcimem /sys/bus/pci/devices/0000:01:00.0/resource0 0x3570 w*1

    # Activate ETF offload
    sudo tc qdisc replace dev enp1s0 parent root handle 6666 mqprio \
	    num_tc 3 \
	    map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
	    queues 1@0 1@1 2@2 \
	    hw 0
    sudo tc qdisc add dev enp1s0 parent 6666:1 etf \
	    clockid CLOCK_TAI \
	    delta 500000 \
	    offload

    # IGC_TQAVCTRL is 0x9 as expected
    sudo pcimem /sys/bus/pci/devices/0000:01:00.0/resource0 0x3570 w*1

    # Deactivate ETF offload again
    sudo tc qdisc delete dev enp1s0 parent 6666:1

    # IGC_TQAVCTRL should now be 0x0 again, but is observed as 0x9
    sudo pcimem /sys/bus/pci/devices/0000:01:00.0/resource0 0x3570 w*1

Fixes: e17090eb2494 ("igc: allow BaseTime 0 enrollment for Qbv")
Signed-off-by: Florian Kauer <florian.kauer@linutronix.de>
Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de>
---
 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 dda057a3b5e3..290daa5827f0 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -6053,6 +6053,7 @@ static int igc_tsn_clear_schedule(struct igc_adapter *adapter)
 
 	adapter->base_time = 0;
 	adapter->cycle_time = NSEC_PER_SEC;
+	adapter->taprio_offload_enable = false;
 	adapter->qbv_config_change_errors = 0;
 
 	for (i = 0; i < adapter->num_tx_queues; i++) {
@@ -6075,8 +6076,6 @@ static int igc_save_qbv_schedule(struct igc_adapter *adapter,
 	size_t n;
 	int i;
 
-	adapter->taprio_offload_enable = qopt->enable;
-
 	if (!qopt->enable)
 		return igc_tsn_clear_schedule(adapter);
 
@@ -6091,6 +6090,7 @@ static int igc_save_qbv_schedule(struct igc_adapter *adapter,
 
 	adapter->cycle_time = qopt->cycle_time;
 	adapter->base_time = qopt->base_time;
+	adapter->taprio_offload_enable = true;
 
 	for (n = 0; n < qopt->num_entries; n++) {
 		struct tc_taprio_sched_entry *e = &qopt->entries[n];
-- 
2.39.2


  parent reply	other threads:[~2023-06-19 10:09 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-19 10:08 [PATCH net v2 0/6] igc: Fix corner cases for TSN offload Florian Kauer
2023-06-19 10:08 ` [PATCH net v2 1/6] igc: Rename qbv_enable to taprio_offload_enable Florian Kauer
2023-07-02 13:42   ` [Intel-wired-lan] " naamax.meir
     [not found]   ` <36b57ea5-baff-f964-3088-e1b186532cfe@web.de>
2023-07-02 19:17     ` Florian Kauer
2023-06-19 10:08 ` Florian Kauer [this message]
2023-07-03  8:34   ` [Intel-wired-lan] [PATCH net v2 2/6] igc: Do not enable taprio offload for invalid arguments naamax.meir
2023-06-19 10:08 ` [PATCH net v2 3/6] igc: Handle already enabled taprio offload for basetime 0 Florian Kauer
2023-07-04  5:28   ` [Intel-wired-lan] " naamax.meir
2023-06-19 10:08 ` [PATCH net v2 4/6] igc: No strict mode in pure launchtime/CBS offload Florian Kauer
2023-07-04  9:34   ` [Intel-wired-lan] " naamax.meir
2023-07-04  9:39   ` Paul Menzel
2023-06-19 10:08 ` [PATCH net v2 5/6] igc: Fix launchtime before start of cycle Florian Kauer
2023-07-05 11:06   ` [Intel-wired-lan] " naamax.meir
2023-06-19 10:08 ` [PATCH net v2 6/6] igc: Fix inserting of empty frame for launchtime Florian Kauer
2023-07-06 11:36   ` [Intel-wired-lan] " naamax.meir
2023-06-20 22:57 ` [PATCH net v2 0/6] igc: Fix corner cases for TSN offload Vinicius Costa Gomes
2023-06-21  0:17 ` Zulkifli, Muhammad Husaini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230619100858.116286-3-florian.kauer@linutronix.de \
    --to=florian.kauer@linutronix.de \
    --cc=anthony.l.nguyen@intel.com \
    --cc=aravindhan.gunasekaran@intel.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=jesse.brandeburg@intel.com \
    --cc=kuba@kernel.org \
    --cc=kurt@linutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mallikarjuna.chilakala@intel.com \
    --cc=muhammad.husaini.zulkifli@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=tee.min.tan@linux.intel.com \
    --cc=vinicius.gomes@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).