From: Paul Menzel <pmenzel@molgen.mpg.de>
To: Jedrzej Jagielski <jedrzej.jagielski@intel.com>,
Przemyslaw Patynowski <przemyslawx.patynowski@intel.com>
Cc: intel-wired-lan@lists.osuosl.org, Kiran Patil <kiran.patil@intel.com>
Subject: Re: [Intel-wired-lan] [PATCH net v1] iavf: Fix 'tc qdisc show' list too many queues
Date: Wed, 15 Jun 2022 11:16:59 +0200 [thread overview]
Message-ID: <6e59c707-ae30-9471-5669-8fce6f58a7a7@molgen.mpg.de> (raw)
In-Reply-To: <20220615083354.3181885-1-jedrzej.jagielski@intel.com>
Dear Jedrzej, lieber Przemyslaw,
Am 15.06.22 um 10:33 schrieb Jedrzej Jagielski:
> From: Przemyslaw Patynowski <przemyslawx.patynowski@intel.com>
In the summary:
s/list/listing/
> Fix tc qdisc show dev <ethX> root displaying too many fq_codel
> qdiscs.
> tc_modify_qdisc, which is caller of ndo_setup_tc, expects
> driver to call netif_set_real_num_tx_queues, which prepares
> qdiscs.
> Without this patch, fq_codel qdiscs would not be adjusted to
> number of queues on VF.
Please reflow for 75 characters per line.
> e.g.:
> tc qdisc show dev <ethX>
> qdisc mq 0: root
> qdisc fq_codel 0: parent :4 limit 10240p flows 1024 quantum 1514 target 5ms interval 100ms memory_limit 32Mb ecn drop_batch 64
> qdisc fq_codel 0: parent :3 limit 10240p flows 1024 quantum 1514 target 5ms interval 100ms memory_limit 32Mb ecn drop_batch 64
> qdisc fq_codel 0: parent :2 limit 10240p flows 1024 quantum 1514 target 5ms interval 100ms memory_limit 32Mb ecn drop_batch 64
> qdisc fq_codel 0: parent :1 limit 10240p flows 1024 quantum 1514 target 5ms interval 100ms memory_limit 32Mb ecn drop_batch 64
> tc qdisc add dev <ethX> root mqprio num_tc 2 map 1 0 0 0 0 0 0 0 queues 1@0 1@1 hw 1 mode channel shaper bw_rlimit max_rate 5000Mbit 150Mbit
> tc qdisc show dev <ethX>
> qdisc mqprio 8003: root tc 2 map 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
> queues:(0:0) (1:1)
> mode:channel
> shaper:bw_rlimit max_rate:5Gbit 150Mbit
> qdisc fq_codel 0: parent 8003:4 limit 10240p flows 1024 quantum 1514 target 5ms interval 100ms memory_limit 32Mb ecn drop_batch 64
> qdisc fq_codel 0: parent 8003:3 limit 10240p flows 1024 quantum 1514 target 5ms interval 100ms memory_limit 32Mb ecn drop_batch 64
> qdisc fq_codel 0: parent 8003:2 limit 10240p flows 1024 quantum 1514 target 5ms interval 100ms memory_limit 32Mb ecn drop_batch 64
> qdisc fq_codel 0: parent 8003:1 limit 10240p flows 1024 quantum 1514 target 5ms interval 100ms memory_limit 32Mb ecn drop_batch 64
>
> While after fix:
> tc qdisc add dev <ethX> root mqprio num_tc 2 map 1 0 0 0 0 0 0 0 queues 1@0 1@1 hw 1 mode channel shaper bw_rlimit max_rate 5000Mbit 150Mbit
> tc qdisc show dev <ethX> #should show 2, shows 4
> qdisc mqprio 8004: root tc 2 map 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
> queues:(0:0) (1:1)
> mode:channel
> shaper:bw_rlimit max_rate:5Gbit 150Mbit
> qdisc fq_codel 0: parent 8004:2 limit 10240p flows 1024 quantum 1514 target 5ms interval 100ms memory_limit 32Mb ecn drop_batch 64
> qdisc fq_codel 0: parent 8004:1 limit 10240p flows 1024 quantum 1514 target 5ms interval 100ms memory_limit 32Mb ecn drop_batch 64
>
> Fixes: d5b33d024496 ("i40evf: add ndo_setup_tc callback to i40evf")
> Signed-off-by: Przemyslaw Patynowski <przemyslawx.patynowski@intel.com>
> Signed-off-by: Grzegorz Szczurek <grzegorzx.szczurek@intel.com>
> Signed-off-by: Kiran Patil <kiran.patil@intel.com>
> Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
> ---
> drivers/net/ethernet/intel/iavf/iavf.h | 5 +++++
> drivers/net/ethernet/intel/iavf/iavf_main.c | 20 +++++++++++++++++++
> .../net/ethernet/intel/iavf/iavf_virtchnl.c | 1 +
> 3 files changed, 26 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h
> index 49aed3e506a6..05cd2dd5bd36 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf.h
> +++ b/drivers/net/ethernet/intel/iavf/iavf.h
> @@ -427,6 +427,11 @@ struct iavf_adapter {
> /* lock to protect access to the cloud filter list */
> spinlock_t cloud_filter_list_lock;
> u16 num_cloud_filters;
> + /* snapshot of "num_active_queues" before setup_tc for qdisc add
> + * is invoked. This information is useful during qdisc del flow,
> + * to restore correct number of queues
> + */
> + int orig_num_active_queues;
>
> #define IAVF_MAX_FDIR_FILTERS 128 /* max allowed Flow Director filters */
> u16 fdir_active_fltr;
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
> index f3ecb3bca33d..d2220043fd48 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -3411,6 +3411,7 @@ static int __iavf_setup_tc(struct net_device *netdev, void *type_data)
> netif_tx_disable(netdev);
> iavf_del_all_cloud_filters(adapter);
> adapter->aq_required = IAVF_FLAG_AQ_DISABLE_CHANNELS;
> + total_qps = adapter->orig_num_active_queues;
> goto exit;
> } else {
> return -EINVAL;
> @@ -3454,7 +3455,20 @@ static int __iavf_setup_tc(struct net_device *netdev, void *type_data)
> adapter->ch_config.ch_info[i].offset = 0;
> }
> }
> +
> + /* Take snapshot of original config such as "num_active_queues"
> + * It is used later when delete ADQ flow is exercised, so that
> + * once delete ADQ flow completes, VF shall go back to its
> + * original queue configuration
> + */
> +
> + adapter->orig_num_active_queues = adapter->num_active_queues;
It’d move the blank line below.
> + /* Store queue info based on TC so that, VF gets configured
Remove the comma?
> + * with correct number of queues when VF completes ADQ config
> + * flow
> + */
> adapter->ch_config.total_qps = total_qps;
> +
> netif_tx_stop_all_queues(netdev);
> netif_tx_disable(netdev);
> adapter->aq_required |= IAVF_FLAG_AQ_ENABLE_CHANNELS;
> @@ -3471,6 +3485,12 @@ static int __iavf_setup_tc(struct net_device *netdev, void *type_data)
> }
> }
> exit:
> + if (test_bit(__IAVF_IN_REMOVE_TASK, &adapter->crit_section))
> + return 0;
> +
> + netif_set_real_num_rx_queues(netdev, total_qps);
> + netif_set_real_num_tx_queues(netdev, total_qps);
> +
> return ret;
> }
>
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
> index 782450d5c12f..8d5f1d5b49cd 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
> @@ -1920,6 +1920,7 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter,
> }
> return;
> }
> +
Unrelated.
> if (v_retval) {
> switch (v_opcode) {
> case VIRTCHNL_OP_ADD_VLAN:
Kind regards,
Paul
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
next prev parent reply other threads:[~2022-06-15 9:17 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-15 8:33 [Intel-wired-lan] [PATCH net v1] iavf: Fix 'tc qdisc show' list too many queues Jedrzej Jagielski
2022-06-15 9:16 ` Paul Menzel [this message]
2022-06-15 9:20 ` Paul Menzel
2022-06-15 13:32 ` Jagielski, Jedrzej
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=6e59c707-ae30-9471-5669-8fce6f58a7a7@molgen.mpg.de \
--to=pmenzel@molgen.mpg.de \
--cc=intel-wired-lan@lists.osuosl.org \
--cc=jedrzej.jagielski@intel.com \
--cc=kiran.patil@intel.com \
--cc=przemyslawx.patynowski@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).