* [Intel-wired-lan] [PATCH net v1] iavf: Fix 'tc qdisc show' list too many queues
@ 2022-06-15 8:33 Jedrzej Jagielski
2022-06-15 9:16 ` Paul Menzel
0 siblings, 1 reply; 4+ messages in thread
From: Jedrzej Jagielski @ 2022-06-15 8:33 UTC (permalink / raw)
To: intel-wired-lan; +Cc: Przemyslaw Patynowski, Kiran Patil, Jedrzej Jagielski
From: Przemyslaw Patynowski <przemyslawx.patynowski@intel.com>
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.
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;
+ /* Store queue info based on TC so that, VF gets configured
+ * 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;
}
+
if (v_retval) {
switch (v_opcode) {
case VIRTCHNL_OP_ADD_VLAN:
--
2.27.0
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Intel-wired-lan] [PATCH net v1] iavf: Fix 'tc qdisc show' list too many queues
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
2022-06-15 9:20 ` Paul Menzel
0 siblings, 1 reply; 4+ messages in thread
From: Paul Menzel @ 2022-06-15 9:16 UTC (permalink / raw)
To: Jedrzej Jagielski, Przemyslaw Patynowski; +Cc: intel-wired-lan, Kiran Patil
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Intel-wired-lan] [PATCH net v1] iavf: Fix 'tc qdisc show' list too many queues
2022-06-15 9:16 ` Paul Menzel
@ 2022-06-15 9:20 ` Paul Menzel
2022-06-15 13:32 ` Jagielski, Jedrzej
0 siblings, 1 reply; 4+ messages in thread
From: Paul Menzel @ 2022-06-15 9:20 UTC (permalink / raw)
To: Jedrzej Jagielski, Przemyslaw Patynowski; +Cc: intel-wired-lan
[Remove Kiran from Cc; and correct s/lieber/dear/]
Am 15.06.22 um 11:16 schrieb Paul Menzel:
> Dear Jedrzej, dear 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Intel-wired-lan] [PATCH net v1] iavf: Fix 'tc qdisc show' list too many queues
2022-06-15 9:20 ` Paul Menzel
@ 2022-06-15 13:32 ` Jagielski, Jedrzej
0 siblings, 0 replies; 4+ messages in thread
From: Jagielski, Jedrzej @ 2022-06-15 13:32 UTC (permalink / raw)
To: Paul Menzel, Patynowski, PrzemyslawX; +Cc: intel-wired-lan
Hello Paul,
>Am 15.06.22 um 11:16 schrieb Paul Menzel:
>> Dear Jedrzej, dear 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.
Sure, I will.
>>
>>> 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?
Sure.
>>
>>> + * 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.
Oh I didn't notice that. Thanks for pointing it out.
>>
>>> if (v_retval) {
>>> switch (v_opcode) {
>>> case VIRTCHNL_OP_ADD_VLAN:
>>
>>
>> Kind regards,
>>
>> Paul
Thanks for suggestions.
Best Regards,
Jedrzej
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-06-15 13:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
2022-06-15 9:20 ` Paul Menzel
2022-06-15 13:32 ` Jagielski, Jedrzej
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).