netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vladimir Oltean <vladimir.oltean@nxp.com>
To: netdev@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Claudiu Manoil <claudiu.manoil@nxp.com>,
	Vinicius Costa Gomes <vinicius.gomes@intel.com>,
	Kurt Kanzenbach <kurt@linutronix.de>,
	Jacob Keller <jacob.e.keller@intel.com>,
	Jamal Hadi Salim <jhs@mojatatu.com>,
	Cong Wang <xiyou.wangcong@gmail.com>,
	Jiri Pirko <jiri@resnulli.us>,
	Simon Horman <simon.horman@corigine.com>
Subject: [PATCH v5 net-next 13/17] net/sched: taprio: pass mqprio queue configuration to ndo_setup_tc()
Date: Thu,  2 Feb 2023 02:36:17 +0200	[thread overview]
Message-ID: <20230202003621.2679603-14-vladimir.oltean@nxp.com> (raw)
In-Reply-To: <20230202003621.2679603-1-vladimir.oltean@nxp.com>

The taprio qdisc does not currently pass the mqprio queue configuration
down to the offloading device driver. So the driver cannot act upon the
TXQ counts/offsets per TC, or upon the prio->tc map. It was probably
assumed that the driver only wants to offload num_tc (see
TC_MQPRIO_HW_OFFLOAD_TCS), which it can get from netdev_get_num_tc(),
but there's clearly more to the mqprio configuration than that.

I've considered 2 mechanisms to remedy that. First is to pass a struct
tc_mqprio_qopt_offload as part of the tc_taprio_qopt_offload. The second
is to make taprio actually call TC_SETUP_QDISC_MQPRIO, *in addition to*
TC_SETUP_QDISC_TAPRIO.

The difference is that in the first case, existing drivers (offloading
or not) all ignore taprio's mqprio portion currently, whereas in the
second case, we could control whether to call TC_SETUP_QDISC_MQPRIO,
based on a new capability. The question is which approach would be
better.

I'm afraid that calling TC_SETUP_QDISC_MQPRIO unconditionally (not based
on a taprio capability bit) would risk introducing regressions. For
example, taprio doesn't populate (or validate) qopt->hw, as well as
mqprio.flags, mqprio.shaper, mqprio.min_rate, mqprio.max_rate.

In comparison, adding a capability is functionally equivalent to just
passing the mqprio in a way that drivers can ignore it, except it's
slightly more complicated to use it (need to set the capability).

Ultimately, what made me go for the "mqprio in taprio" variant was that
it's easier for offloading drivers to interpret the mqprio qopt slightly
differently when it comes from taprio vs when it comes from mqprio,
should that ever become necessary.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
---
v4->v5:
- reword commit message
- mqprio_qopt_reconstruct() has been added in a previous patch, to
  consolidate existing code
v2->v4: none
v1->v2: reconstruct the mqprio queue configuration structure

 include/net/pkt_sched.h | 1 +
 net/sched/sch_taprio.c  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h
index 02e3ccfbc7d1..ace8be520fb0 100644
--- a/include/net/pkt_sched.h
+++ b/include/net/pkt_sched.h
@@ -187,6 +187,7 @@ struct tc_taprio_sched_entry {
 };
 
 struct tc_taprio_qopt_offload {
+	struct tc_mqprio_qopt_offload mqprio;
 	u8 enable;
 	ktime_t base_time;
 	u64 cycle_time;
diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index 6b3cecbe9f1f..aba8a16842c1 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -1228,6 +1228,7 @@ static int taprio_enable_offload(struct net_device *dev,
 		return -ENOMEM;
 	}
 	offload->enable = 1;
+	mqprio_qopt_reconstruct(dev, &offload->mqprio.qopt);
 	taprio_sched_to_offload(dev, sched, offload);
 
 	for (tc = 0; tc < TC_MAX_QUEUE; tc++)
-- 
2.34.1


  parent reply	other threads:[~2023-02-02  0:37 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-02  0:36 [PATCH v5 net-next 00/17] ENETC mqprio/taprio cleanup Vladimir Oltean
2023-02-02  0:36 ` [PATCH v5 net-next 01/17] net: enetc: simplify enetc_num_stack_tx_queues() Vladimir Oltean
2023-02-02  0:36 ` [PATCH v5 net-next 02/17] net: enetc: allow the enetc_reconfigure() callback to fail Vladimir Oltean
2023-02-02  0:36 ` [PATCH v5 net-next 03/17] net: enetc: recalculate num_real_tx_queues when XDP program attaches Vladimir Oltean
2023-02-02  0:36 ` [PATCH v5 net-next 04/17] net: enetc: ensure we always have a minimum number of TXQs for stack Vladimir Oltean
2023-02-02  0:36 ` [PATCH v5 net-next 05/17] net/sched: mqprio: refactor nlattr parsing to a separate function Vladimir Oltean
2023-02-02  0:36 ` [PATCH v5 net-next 06/17] net/sched: mqprio: refactor offloading and unoffloading to dedicated functions Vladimir Oltean
2023-02-02  0:36 ` [PATCH v5 net-next 07/17] net/sched: move struct tc_mqprio_qopt_offload from pkt_cls.h to pkt_sched.h Vladimir Oltean
2023-02-02  0:36 ` [PATCH v5 net-next 08/17] net/sched: mqprio: allow reverse TC:TXQ mappings Vladimir Oltean
2023-02-03 16:18   ` Simon Horman
2023-02-05 11:55   ` Gerhard Engleder
2023-02-05 12:22     ` Vladimir Oltean
2023-02-02  0:36 ` [PATCH v5 net-next 09/17] net/sched: mqprio: allow offloading drivers to request queue count validation Vladimir Oltean
2023-02-02  0:36 ` [PATCH v5 net-next 10/17] net/sched: mqprio: add extack messages for " Vladimir Oltean
2023-02-02  0:36 ` [PATCH v5 net-next 11/17] net/sched: taprio: centralize mqprio qopt validation Vladimir Oltean
2023-02-03 16:32   ` Simon Horman
2023-02-05 12:10   ` Gerhard Engleder
2023-02-05 12:23     ` Vladimir Oltean
2023-02-02  0:36 ` [PATCH v5 net-next 12/17] net/sched: refactor mqprio qopt reconstruction to a library function Vladimir Oltean
2023-02-03 16:33   ` Simon Horman
2023-02-02  0:36 ` Vladimir Oltean [this message]
2023-02-02  0:36 ` [PATCH v5 net-next 14/17] net/sched: taprio: only calculate gate mask per TXQ for igc, stmmac and tsnep Vladimir Oltean
2023-02-02  8:04   ` Roger Quadros
2023-02-02 12:31     ` Vladimir Oltean
2023-02-02 20:05       ` Roger Quadros
2023-02-02  0:36 ` [PATCH v5 net-next 15/17] net: enetc: request mqprio to validate the queue counts Vladimir Oltean
2023-02-02  0:36 ` [PATCH v5 net-next 16/17] net: enetc: act upon the requested mqprio queue configuration Vladimir Oltean
2023-02-02  0:36 ` [PATCH v5 net-next 17/17] net: enetc: act upon mqprio queue config in taprio offload Vladimir Oltean
2023-02-02 20:54 ` [PATCH v5 net-next 00/17] ENETC mqprio/taprio cleanup Vladimir Oltean

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=20230202003621.2679603-14-vladimir.oltean@nxp.com \
    --to=vladimir.oltean@nxp.com \
    --cc=claudiu.manoil@nxp.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=jacob.e.keller@intel.com \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=kuba@kernel.org \
    --cc=kurt@linutronix.de \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=simon.horman@corigine.com \
    --cc=vinicius.gomes@intel.com \
    --cc=xiyou.wangcong@gmail.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).