From: Vinicius Costa Gomes <vinicius.gomes@intel.com> To: Vladimir Oltean <vladimir.oltean@nxp.com> Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>, "jhs@mojatatu.com" <jhs@mojatatu.com>, "xiyou.wangcong@gmail.com" <xiyou.wangcong@gmail.com>, "jiri@resnulli.us" <jiri@resnulli.us>, "kuba@kernel.org" <kuba@kernel.org>, "Jose.Abreu@synopsys.com" <Jose.Abreu@synopsys.com>, Po Liu <po.liu@nxp.com>, "intel-wired-lan@lists.osuosl.org" <intel-wired-lan@lists.osuosl.org>, "anthony.l.nguyen@intel.com" <anthony.l.nguyen@intel.com>, "mkubecek@suse.cz" <mkubecek@suse.cz> Subject: Re: [PATCH net-next v3 2/8] taprio: Add support for frame preemption offload Date: Fri, 29 Jan 2021 13:13:24 -0800 [thread overview] Message-ID: <87wnvvsayz.fsf@vcostago-mobl2.amr.corp.intel.com> (raw) In-Reply-To: <20210126000924.jjkjruzmh5lgrkry@skbuf> Vladimir Oltean <vladimir.oltean@nxp.com> writes: > On Fri, Jan 22, 2021 at 02:44:47PM -0800, Vinicius Costa Gomes wrote: >> + /* It's valid to enable frame preemption without any kind of >> + * offloading being enabled, so keep it separated. >> + */ >> + if (tb[TCA_TAPRIO_ATTR_PREEMPT_TCS]) { >> + u32 preempt = nla_get_u32(tb[TCA_TAPRIO_ATTR_PREEMPT_TCS]); >> + struct tc_preempt_qopt_offload qopt = { }; >> + >> + if (preempt == U32_MAX) { >> + NL_SET_ERR_MSG(extack, "At least one queue must be not be preemptible"); >> + err = -EINVAL; >> + goto free_sched; >> + } >> + >> + qopt.preemptible_queues = tc_map_to_queue_mask(dev, preempt); >> + >> + err = dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_PREEMPT, >> + &qopt); >> + if (err) >> + goto free_sched; >> + >> + q->preemptible_tcs = preempt; >> + } >> + > > First I'm interested in the means: why check for preempt == U32_MAX when > you determine that all traffic classes are preemptible? What if less > than 32 traffic classes are used by the netdev? The check will be > bypassed, won't it? Good catch :-) I wanted to have this (at least one express queue) handled in a centralized way, but perhaps this should be handled best per driver. > > Secondly, why should at least one queue be preemptible? What's wrong > with frame preemption being triggered by a tc-taprio window smaller than > the packet size? This can happen regardless of traffic class. It's the opposite, at least one queue needs to be marked express/non-preemptible. But as I said above, perhaps this should be handled in a per-driver way. I will remove this from taprio. I think removing this check/limitation from taprio should solve the second part of your question, right? Cheers, -- Vinicius
WARNING: multiple messages have this Message-ID (diff)
From: Vinicius Costa Gomes <vinicius.gomes@intel.com> To: intel-wired-lan@osuosl.org Subject: [Intel-wired-lan] [PATCH net-next v3 2/8] taprio: Add support for frame preemption offload Date: Fri, 29 Jan 2021 13:13:24 -0800 [thread overview] Message-ID: <87wnvvsayz.fsf@vcostago-mobl2.amr.corp.intel.com> (raw) In-Reply-To: <20210126000924.jjkjruzmh5lgrkry@skbuf> Vladimir Oltean <vladimir.oltean@nxp.com> writes: > On Fri, Jan 22, 2021 at 02:44:47PM -0800, Vinicius Costa Gomes wrote: >> + /* It's valid to enable frame preemption without any kind of >> + * offloading being enabled, so keep it separated. >> + */ >> + if (tb[TCA_TAPRIO_ATTR_PREEMPT_TCS]) { >> + u32 preempt = nla_get_u32(tb[TCA_TAPRIO_ATTR_PREEMPT_TCS]); >> + struct tc_preempt_qopt_offload qopt = { }; >> + >> + if (preempt == U32_MAX) { >> + NL_SET_ERR_MSG(extack, "At least one queue must be not be preemptible"); >> + err = -EINVAL; >> + goto free_sched; >> + } >> + >> + qopt.preemptible_queues = tc_map_to_queue_mask(dev, preempt); >> + >> + err = dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_PREEMPT, >> + &qopt); >> + if (err) >> + goto free_sched; >> + >> + q->preemptible_tcs = preempt; >> + } >> + > > First I'm interested in the means: why check for preempt == U32_MAX when > you determine that all traffic classes are preemptible? What if less > than 32 traffic classes are used by the netdev? The check will be > bypassed, won't it? Good catch :-) I wanted to have this (at least one express queue) handled in a centralized way, but perhaps this should be handled best per driver. > > Secondly, why should at least one queue be preemptible? What's wrong > with frame preemption being triggered by a tc-taprio window smaller than > the packet size? This can happen regardless of traffic class. It's the opposite, at least one queue needs to be marked express/non-preemptible. But as I said above, perhaps this should be handled in a per-driver way. I will remove this from taprio. I think removing this check/limitation from taprio should solve the second part of your question, right? Cheers, -- Vinicius
next prev parent reply other threads:[~2021-01-29 21:14 UTC|newest] Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-01-22 22:44 [PATCH net-next v3 0/8] ethtool: Add support for frame preemption Vinicius Costa Gomes 2021-01-22 22:44 ` [Intel-wired-lan] " Vinicius Costa Gomes 2021-01-22 22:44 ` [PATCH net-next v3 1/8] ethtool: Add support for configuring " Vinicius Costa Gomes 2021-01-22 22:44 ` [Intel-wired-lan] " Vinicius Costa Gomes 2021-03-02 14:23 ` Vladimir Oltean 2021-03-02 14:23 ` [Intel-wired-lan] " Vladimir Oltean 2021-03-03 0:40 ` Vinicius Costa Gomes 2021-03-03 0:40 ` [Intel-wired-lan] " Vinicius Costa Gomes 2021-03-03 0:51 ` Vladimir Oltean 2021-03-03 0:51 ` [Intel-wired-lan] " Vladimir Oltean 2021-03-05 22:38 ` Vinicius Costa Gomes 2021-03-05 22:38 ` [Intel-wired-lan] " Vinicius Costa Gomes 2021-01-22 22:44 ` [PATCH net-next v3 2/8] taprio: Add support for frame preemption offload Vinicius Costa Gomes 2021-01-22 22:44 ` [Intel-wired-lan] " Vinicius Costa Gomes 2021-01-26 0:09 ` Vladimir Oltean 2021-01-26 0:09 ` [Intel-wired-lan] " Vladimir Oltean 2021-01-29 21:13 ` Vinicius Costa Gomes [this message] 2021-01-29 21:13 ` Vinicius Costa Gomes 2021-01-29 21:57 ` Jakub Kicinski 2021-01-29 21:57 ` [Intel-wired-lan] " Jakub Kicinski 2021-01-29 23:12 ` Vinicius Costa Gomes 2021-01-29 23:12 ` [Intel-wired-lan] " Vinicius Costa Gomes 2021-01-30 0:27 ` Jakub Kicinski 2021-01-30 0:27 ` [Intel-wired-lan] " Jakub Kicinski 2021-01-29 23:20 ` Vladimir Oltean 2021-01-29 23:20 ` [Intel-wired-lan] " Vladimir Oltean 2021-01-29 23:42 ` Vinicius Costa Gomes 2021-01-29 23:42 ` [Intel-wired-lan] " Vinicius Costa Gomes 2021-01-30 0:25 ` Vladimir Oltean 2021-01-30 0:25 ` [Intel-wired-lan] " Vladimir Oltean 2021-01-22 22:44 ` [PATCH net-next v3 3/8] igc: Set the RX packet buffer size for TSN mode Vinicius Costa Gomes 2021-01-22 22:44 ` [Intel-wired-lan] " Vinicius Costa Gomes 2021-01-22 22:44 ` [PATCH net-next v3 4/8] igc: Only dump registers if configured to dump HW information Vinicius Costa Gomes 2021-01-22 22:44 ` [Intel-wired-lan] " Vinicius Costa Gomes 2021-01-22 22:44 ` [PATCH net-next v3 5/8] igc: Avoid TX Hangs because long cycles Vinicius Costa Gomes 2021-01-22 22:44 ` [Intel-wired-lan] " Vinicius Costa Gomes 2021-01-26 0:02 ` Vladimir Oltean 2021-01-26 0:02 ` [Intel-wired-lan] " Vladimir Oltean 2021-01-27 9:03 ` Kurt Kanzenbach 2021-01-27 9:03 ` [Intel-wired-lan] " Kurt Kanzenbach 2021-01-29 21:01 ` Vinicius Costa Gomes 2021-01-29 21:01 ` [Intel-wired-lan] " Vinicius Costa Gomes 2021-01-22 22:44 ` [PATCH net-next v3 6/8] igc: Add support for tuning frame preemption via ethtool Vinicius Costa Gomes 2021-01-22 22:44 ` [Intel-wired-lan] " Vinicius Costa Gomes 2021-01-26 0:32 ` Vladimir Oltean 2021-01-26 0:32 ` [Intel-wired-lan] " Vladimir Oltean 2021-01-29 21:27 ` Vinicius Costa Gomes 2021-01-29 21:27 ` [Intel-wired-lan] " Vinicius Costa Gomes 2021-01-29 23:16 ` Vladimir Oltean 2021-01-29 23:16 ` [Intel-wired-lan] " Vladimir Oltean 2021-03-03 1:07 ` Vladimir Oltean 2021-03-03 1:07 ` [Intel-wired-lan] " Vladimir Oltean 2021-01-22 22:44 ` [PATCH net-next v3 7/8] igc: Add support for Frame Preemption offload Vinicius Costa Gomes 2021-01-22 22:44 ` [Intel-wired-lan] " Vinicius Costa Gomes 2021-01-22 22:44 ` [PATCH net-next v3 8/8] igc: Separate TSN configurations that can be updated Vinicius Costa Gomes 2021-01-22 22:44 ` [Intel-wired-lan] " Vinicius Costa Gomes 2021-01-29 23:37 ` [PATCH net-next v3 0/8] ethtool: Add support for frame preemption Vladimir Oltean 2021-01-29 23:37 ` [Intel-wired-lan] " Vladimir Oltean 2021-01-30 0:11 ` Vinicius Costa Gomes 2021-01-30 0:11 ` [Intel-wired-lan] " Vinicius Costa Gomes
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=87wnvvsayz.fsf@vcostago-mobl2.amr.corp.intel.com \ --to=vinicius.gomes@intel.com \ --cc=Jose.Abreu@synopsys.com \ --cc=anthony.l.nguyen@intel.com \ --cc=intel-wired-lan@lists.osuosl.org \ --cc=jhs@mojatatu.com \ --cc=jiri@resnulli.us \ --cc=kuba@kernel.org \ --cc=mkubecek@suse.cz \ --cc=netdev@vger.kernel.org \ --cc=po.liu@nxp.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.