From: Ferenc Fejes <fejes@inf.elte.hu>
To: Vladimir Oltean <vladimir.oltean@nxp.com>, 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: Re: [PATCH v6 net-next 02/13] net/sched: mqprio: refactor offloading and unoffloading to dedicated functions
Date: Thu, 16 Feb 2023 14:05:22 +0100 [thread overview]
Message-ID: <ede5e9a2f27bf83bfb86d3e8c4ca7b34093b99e2.camel@inf.elte.hu> (raw)
In-Reply-To: <20230204135307.1036988-3-vladimir.oltean@nxp.com>
Hi!
On Sat, 2023-02-04 at 15:52 +0200, Vladimir Oltean wrote:
> Some more logic will be added to mqprio offloading, so split that
> code
> up from mqprio_init(), which is already large, and create a new
> function, mqprio_enable_offload(), similar to
> taprio_enable_offload().
> Also create the opposite function mqprio_disable_offload().
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
> Reviewed-by: Simon Horman <simon.horman@corigine.com>
> ---
> v1->v6: none
>
> net/sched/sch_mqprio.c | 102 ++++++++++++++++++++++++---------------
> --
> 1 file changed, 59 insertions(+), 43 deletions(-)
>
> diff --git a/net/sched/sch_mqprio.c b/net/sched/sch_mqprio.c
> index d2d8a02ded05..3579a64da06e 100644
> --- a/net/sched/sch_mqprio.c
> +++ b/net/sched/sch_mqprio.c
> @@ -27,6 +27,61 @@ struct mqprio_sched {
> u64 max_rate[TC_QOPT_MAX_QUEUE];
> };
>
> +static int mqprio_enable_offload(struct Qdisc *sch,
> + const struct tc_mqprio_qopt *qopt)
> +{
> + struct tc_mqprio_qopt_offload mqprio = {.qopt = *qopt};
> + struct mqprio_sched *priv = qdisc_priv(sch);
> + struct net_device *dev = qdisc_dev(sch);
> + int err, i;
> +
> + switch (priv->mode) {
> + case TC_MQPRIO_MODE_DCB:
> + if (priv->shaper != TC_MQPRIO_SHAPER_DCB)
> + return -EINVAL;
> + break;
> + case TC_MQPRIO_MODE_CHANNEL:
> + mqprio.flags = priv->flags;
> + if (priv->flags & TC_MQPRIO_F_MODE)
> + mqprio.mode = priv->mode;
> + if (priv->flags & TC_MQPRIO_F_SHAPER)
> + mqprio.shaper = priv->shaper;
> + if (priv->flags & TC_MQPRIO_F_MIN_RATE)
> + for (i = 0; i < mqprio.qopt.num_tc; i++)
> + mqprio.min_rate[i] = priv-
> >min_rate[i];
> + if (priv->flags & TC_MQPRIO_F_MAX_RATE)
> + for (i = 0; i < mqprio.qopt.num_tc; i++)
> + mqprio.max_rate[i] = priv-
> >max_rate[i];
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + err = dev->netdev_ops->ndo_setup_tc(dev,
> TC_SETUP_QDISC_MQPRIO,
> + &mqprio);
> + if (err)
> + return err;
> +
> + priv->hw_offload = mqprio.qopt.hw;
> +
> + return 0;
> +}
> +
> +static void mqprio_disable_offload(struct Qdisc *sch)
> +{
> + struct tc_mqprio_qopt_offload mqprio = { { 0 } };
> + struct mqprio_sched *priv = qdisc_priv(sch);
> + struct net_device *dev = qdisc_dev(sch);
> +
> + switch (priv->mode) {
> + case TC_MQPRIO_MODE_DCB:
> + case TC_MQPRIO_MODE_CHANNEL:
> + dev->netdev_ops->ndo_setup_tc(dev,
> TC_SETUP_QDISC_MQPRIO,
> + &mqprio);
> + break;
> + }
> +}
> +
> static void mqprio_destroy(struct Qdisc *sch)
> {
> struct net_device *dev = qdisc_dev(sch);
> @@ -41,22 +96,10 @@ static void mqprio_destroy(struct Qdisc *sch)
> kfree(priv->qdiscs);
> }
>
> - if (priv->hw_offload && dev->netdev_ops->ndo_setup_tc) {
> - struct tc_mqprio_qopt_offload mqprio = { { 0 } };
> -
> - switch (priv->mode) {
> - case TC_MQPRIO_MODE_DCB:
> - case TC_MQPRIO_MODE_CHANNEL:
> - dev->netdev_ops->ndo_setup_tc(dev,
> -
> TC_SETUP_QDISC_MQPRIO,
> - &mqprio);
> - break;
> - default:
> - return;
> - }
> - } else {
> + if (priv->hw_offload && dev->netdev_ops->ndo_setup_tc)
> + mqprio_disable_offload(sch);
> + else
> netdev_set_num_tc(dev, 0);
> - }
> }
>
> static int mqprio_parse_opt(struct net_device *dev, struct
> tc_mqprio_qopt *qopt)
> @@ -253,36 +296,9 @@ static int mqprio_init(struct Qdisc *sch, struct
> nlattr *opt,
> * supplied and verified mapping
> */
> if (qopt->hw) {
> - struct tc_mqprio_qopt_offload mqprio = {.qopt =
> *qopt};
> -
> - switch (priv->mode) {
> - case TC_MQPRIO_MODE_DCB:
> - if (priv->shaper != TC_MQPRIO_SHAPER_DCB)
> - return -EINVAL;
> - break;
> - case TC_MQPRIO_MODE_CHANNEL:
> - mqprio.flags = priv->flags;
> - if (priv->flags & TC_MQPRIO_F_MODE)
> - mqprio.mode = priv->mode;
> - if (priv->flags & TC_MQPRIO_F_SHAPER)
> - mqprio.shaper = priv->shaper;
> - if (priv->flags & TC_MQPRIO_F_MIN_RATE)
> - for (i = 0; i < mqprio.qopt.num_tc;
> i++)
> - mqprio.min_rate[i] = priv-
> >min_rate[i];
> - if (priv->flags & TC_MQPRIO_F_MAX_RATE)
> - for (i = 0; i < mqprio.qopt.num_tc;
> i++)
> - mqprio.max_rate[i] = priv-
> >max_rate[i];
> - break;
> - default:
> - return -EINVAL;
> - }
> - err = dev->netdev_ops->ndo_setup_tc(dev,
> -
> TC_SETUP_QDISC_MQPRIO,
> - &mqprio);
> + err = mqprio_enable_offload(sch, qopt);
> if (err)
> return err;
> -
> - priv->hw_offload = mqprio.qopt.hw;
> } else {
> netdev_set_num_tc(dev, qopt->num_tc);
> for (i = 0; i < qopt->num_tc; i++)
This patch just code refactoring or it modifies the default behavior of
the offloading too? I'm asking it in regards of the veth interface.
When you configure mqprio, the "hw" parameter is mandatory. By default,
it tries to configure it with "hw 1". However as a result, veth spit
back "Invalid argument" error (before your patches). Same happens after
this patch too, right?
For veth hardware offloading makes no sense, but giving the "hw 0"
argument explicitly as mqprio parameter might counterintuitive.
Best,
Ferenc
next prev parent reply other threads:[~2023-02-16 13:05 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-04 13:52 [PATCH v6 net-next 00/13] ENETC mqprio/taprio cleanup Vladimir Oltean
2023-02-04 13:52 ` [PATCH v6 net-next 01/13] net/sched: mqprio: refactor nlattr parsing to a separate function Vladimir Oltean
2023-02-04 13:52 ` [PATCH v6 net-next 02/13] net/sched: mqprio: refactor offloading and unoffloading to dedicated functions Vladimir Oltean
2023-02-16 13:05 ` Ferenc Fejes [this message]
2023-02-16 14:28 ` Vladimir Oltean
2023-02-16 14:41 ` Ferenc Fejes
2023-02-04 13:52 ` [PATCH v6 net-next 03/13] net/sched: move struct tc_mqprio_qopt_offload from pkt_cls.h to pkt_sched.h Vladimir Oltean
2023-02-04 13:52 ` [PATCH v6 net-next 04/13] net/sched: mqprio: allow reverse TC:TXQ mappings Vladimir Oltean
2023-02-05 12:42 ` Gerhard Engleder
2023-02-04 13:52 ` [PATCH v6 net-next 05/13] net/sched: mqprio: allow offloading drivers to request queue count validation Vladimir Oltean
2023-02-04 13:53 ` [PATCH v6 net-next 06/13] net/sched: mqprio: add extack messages for " Vladimir Oltean
2023-02-04 13:53 ` [PATCH v6 net-next 07/13] net/sched: taprio: centralize mqprio qopt validation Vladimir Oltean
2023-02-05 12:44 ` Gerhard Engleder
2023-02-04 13:53 ` [PATCH v6 net-next 08/13] net/sched: refactor mqprio qopt reconstruction to a library function Vladimir Oltean
2023-02-04 13:53 ` [PATCH v6 net-next 09/13] net/sched: taprio: pass mqprio queue configuration to ndo_setup_tc() Vladimir Oltean
2023-02-04 13:53 ` [PATCH v6 net-next 10/13] net/sched: taprio: only pass gate mask per TXQ for igc, stmmac, tsnep, am65_cpsw Vladimir Oltean
2023-02-04 13:53 ` [PATCH v6 net-next 11/13] net: enetc: request mqprio to validate the queue counts Vladimir Oltean
2023-02-04 13:53 ` [PATCH v6 net-next 12/13] net: enetc: act upon the requested mqprio queue configuration Vladimir Oltean
2023-02-04 13:53 ` [PATCH v6 net-next 13/13] net: enetc: act upon mqprio queue config in taprio offload Vladimir Oltean
2023-02-06 10:10 ` [PATCH v6 net-next 00/13] ENETC mqprio/taprio cleanup patchwork-bot+netdevbpf
2023-02-06 17:51 ` Jacob Keller
2023-02-07 12:48 ` 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=ede5e9a2f27bf83bfb86d3e8c4ca7b34093b99e2.camel@inf.elte.hu \
--to=fejes@inf.elte.hu \
--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=vladimir.oltean@nxp.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