[PATCH] net: fec: select queue depending on VLAN priority

[PATCH] net: fec: select queue depending on VLAN priority

[Stefan Agner]

Since the addition of the multi queue code with commit 59d0f7465644
("net: fec: init multi queue date structure") the queue selection
has been handelt by the default transmit queue selection
implementation which tries to evenly distribute the traffic across
all available queues. This selection presumes that the queues are
using an equal priority, however, the queues 1 and 2 are actually
of higher priority (the classification of the queues is enabled in
fec_enet_enable_ring).

This can lead to net scheduler warnings and continuous TX ring
dumps when exercising the system with iperf.

Use only queue 0 for all common traffic (no VLAN and P802.1p
priority 0 and 1) and route level 2-7 through queue 1 and 2.

Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
Fixes: 59d0f7465644 ("net: fec: init multi queue date structure")
---
This patch solves the issue documented in this thread:
https://www.spinics.net/lists/netdev/msg430679.html

 drivers/net/ethernet/freescale/fec_main.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 91a16641e851..72343cbfddc9 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -72,6 +72,7 @@ static void fec_enet_itr_coal_init(struct net_device *ndev);
 #define DRIVER_NAME	"fec"
 
 #define FEC_ENET_GET_QUQUE(_x) ((_x == 0) ? 1 : ((_x == 1) ? 2 : 0))
+static const u16 fec_enet_vlan_pri_to_queue[8] = {0, 0, 1, 1, 1, 2, 2, 2};
 
 /* Pause frame feild and FIFO threshold */
 #define FEC_ENET_FCE	(1 << 5)
@@ -3052,10 +3053,28 @@ static int fec_set_features(struct net_device *netdev,
 	return 0;
 }
 
+u16 fec_enet_select_queue(struct net_device *ndev, struct sk_buff *skb,
+			  void *accel_priv, select_queue_fallback_t fallback)
+{
+	struct fec_enet_private *fep = netdev_priv(ndev);
+	u16 vlan_tci, vlan_pcp;
+
+	if (!(fep->quirks & FEC_QUIRK_HAS_AVB))
+		return fallback(ndev, skb);
+
+	/* Assign queue 0 if no VLAN tag present */
+	if (vlan_get_tag(skb, &vlan_tci) < 0)
+		return 0;
+
+	vlan_pcp = (vlan_tci & VLAN_PRIO_MASK) >> VLAN_PRIO_SHIFT;
+	return fec_enet_vlan_pri_to_queue[vlan_pcp];
+}
+
 static const struct net_device_ops fec_netdev_ops = {
 	.ndo_open		= fec_enet_open,
 	.ndo_stop		= fec_enet_close,
 	.ndo_start_xmit		= fec_enet_start_xmit,
+	.ndo_select_queue       = fec_enet_select_queue,
 	.ndo_set_rx_mode	= set_multicast_list,
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_tx_timeout		= fec_timeout,
-- 
2.12.2

Re: [PATCH] net: fec: select queue depending on VLAN priority

[Andrew Lunn]

On Mon, May 08, 2017 at 10:37:08PM -0700, Stefan Agner wrote:
> Since the addition of the multi queue code with commit 59d0f7465644
> ("net: fec: init multi queue date structure") the queue selection
> has been handelt by the default transmit queue selection
> implementation which tries to evenly distribute the traffic across
> all available queues. This selection presumes that the queues are
> using an equal priority, however, the queues 1 and 2 are actually
> of higher priority (the classification of the queues is enabled in
> fec_enet_enable_ring).
> 
> This can lead to net scheduler warnings and continuous TX ring
> dumps when exercising the system with iperf.
> 
> Use only queue 0 for all common traffic (no VLAN and P802.1p
> priority 0 and 1) and route level 2-7 through queue 1 and 2.

Hi Stefan

Did you try:

vconfig set_egress_map eth0.42 0 7
ip addr add 10.42.42.42/24 eth0.42
iperf -c 10.42.42.1

i.e. send a continuous stream on one of the higher priority queues.

>From what was said earlier in this thread, isn't queue 0 going to be
starved? As well as this patch, don't we also need some default
bandwidth allocations to the queues to ensure queue 0 does get some
bandwidth?

	Andrew

Re: [PATCH] net: fec: select queue depending on VLAN priority

[David Miller]

From: Stefan Agner <stefan@agner.ch>
Date: Mon,  8 May 2017 22:37:08 -0700

> Since the addition of the multi queue code with commit 59d0f7465644
> ("net: fec: init multi queue date structure") the queue selection
> has been handelt by the default transmit queue selection
> implementation which tries to evenly distribute the traffic across
> all available queues. This selection presumes that the queues are
> using an equal priority, however, the queues 1 and 2 are actually
> of higher priority (the classification of the queues is enabled in
> fec_enet_enable_ring).
> 
> This can lead to net scheduler warnings and continuous TX ring
> dumps when exercising the system with iperf.
> 
> Use only queue 0 for all common traffic (no VLAN and P802.1p
> priority 0 and 1) and route level 2-7 through queue 1 and 2.
> 
> Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
> Fixes: 59d0f7465644 ("net: fec: init multi queue date structure")

If the queues are used for prioritization, and it does not have
multiple normal priority level queues, multiqueue is not what the
driver should have implemented.

RE: [PATCH] net: fec: select queue depending on VLAN priority

[Andy Duan]

From: David Miller <davem@davemloft.net> Sent: Tuesday, May 09, 2017 9:39 PM
>To: stefan@agner.ch
>Cc: Andy Duan <fugang.duan@nxp.com>; andrew@lunn.ch;
>festevam@gmail.com; netdev@vger.kernel.org; linux-
>kernel@vger.kernel.org
>Subject: Re: [PATCH] net: fec: select queue depending on VLAN priority
>
>From: Stefan Agner <stefan@agner.ch>
>Date: Mon,  8 May 2017 22:37:08 -0700
>
>> Since the addition of the multi queue code with commit 59d0f7465644
>> ("net: fec: init multi queue date structure") the queue selection has
>> been handelt by the default transmit queue selection implementation
>> which tries to evenly distribute the traffic across all available
>> queues. This selection presumes that the queues are using an equal
>> priority, however, the queues 1 and 2 are actually of higher priority
>> (the classification of the queues is enabled in fec_enet_enable_ring).
>>
>> This can lead to net scheduler warnings and continuous TX ring dumps
>> when exercising the system with iperf.
>>
>> Use only queue 0 for all common traffic (no VLAN and P802.1p priority
>> 0 and 1) and route level 2-7 through queue 1 and 2.
>>
>> Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
>> Fixes: 59d0f7465644 ("net: fec: init multi queue date structure")
>
>If the queues are used for prioritization, and it does not have multiple normal
>priority level queues, multiqueue is not what the driver should have
>implemented.
Firstly, HW multiple queues support:
	- Traffic-shaping bandwidth distribution supports credit-based and round-robin-based policies. Either policy can be combined with time-based shaping.
	- AVB (Audio Video Bridging, IEEE 802.1Qav) features:
		* Credit-based bandwidth distribution policy can be combined with time-based shaping
		* AVB endpoint talker and listener support
		* Support for arbitration between different priority traffic (for example, AVB class A, AVB class B, and non-AVB)
Round-robin-based policies:
	It has the same priority for three queues: In the round-robin QoS scheme, each queue is given an equal opportunity to transmit one frame. For example, if queue n has a frame to transmit, the queue transmits its frame. After queue n has transmitted its frame, or if queue n does not have a frame to transmit, queue n+1 is then allowed to transmit its frame, and so on.

Credit-based policies:
	The AVB credit based shaper acts independently, per class, to control the bandwidth distribution between normal traffic and time-sensitive traffic with respect to the total link bandwidth available.
	Credit based shaper conbined with time-based shaping:  
		- priority: ClassA queue > ClassB queue > best effort
		- ensure the queue bandwidth as user set based on time-based shaping algorithms (transmitter transmit frame from three queue in turn based on time-based shaping algorithms)
	And in real AVB case,  each streaming can be independent, and are fixed on related queue. Then driver level should implement .ndo_select_queue() to put the streaming into related queue. That is what the patch did.

The current driver config the three queue to credit-based policies (AVB), the patch seems no problem for the implementation. Do you have any suggestion ?

Andy

Re: [PATCH] net: fec: select queue depending on VLAN priority

[Stefan Agner]

On 2017-05-09 19:42, Andy Duan wrote:
> From: David Miller <davem@davemloft.net> Sent: Tuesday, May 09, 2017 9:39 PM
>>To: stefan@agner.ch
>>Cc: Andy Duan <fugang.duan@nxp.com>; andrew@lunn.ch;
>>festevam@gmail.com; netdev@vger.kernel.org; linux-
>>kernel@vger.kernel.org
>>Subject: Re: [PATCH] net: fec: select queue depending on VLAN priority
>>
>>From: Stefan Agner <stefan@agner.ch>
>>Date: Mon,  8 May 2017 22:37:08 -0700
>>
>>> Since the addition of the multi queue code with commit 59d0f7465644
>>> ("net: fec: init multi queue date structure") the queue selection has
>>> been handelt by the default transmit queue selection implementation
>>> which tries to evenly distribute the traffic across all available
>>> queues. This selection presumes that the queues are using an equal
>>> priority, however, the queues 1 and 2 are actually of higher priority
>>> (the classification of the queues is enabled in fec_enet_enable_ring).
>>>
>>> This can lead to net scheduler warnings and continuous TX ring dumps
>>> when exercising the system with iperf.
>>>
>>> Use only queue 0 for all common traffic (no VLAN and P802.1p priority
>>> 0 and 1) and route level 2-7 through queue 1 and 2.
>>>
>>> Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
>>> Fixes: 59d0f7465644 ("net: fec: init multi queue date structure")
>>
>>If the queues are used for prioritization, and it does not have multiple normal
>>priority level queues, multiqueue is not what the driver should have
>>implemented.
> Firstly, HW multiple queues support:
> 	- Traffic-shaping bandwidth distribution supports credit-based and
> round-robin-based policies. Either policy can be combined with
> time-based shaping.
> 	- AVB (Audio Video Bridging, IEEE 802.1Qav) features:
> 		* Credit-based bandwidth distribution policy can be combined with
> time-based shaping
> 		* AVB endpoint talker and listener support
> 		* Support for arbitration between different priority traffic (for
> example, AVB class A, AVB class B, and non-AVB)
> Round-robin-based policies:
> 	It has the same priority for three queues: In the round-robin QoS
> scheme, each queue is given an equal opportunity to transmit one
> frame. For example, if queue n has a frame to transmit, the queue
> transmits its frame. After queue n has transmitted its frame, or if
> queue n does not have a frame to transmit, queue n+1 is then allowed
> to transmit its frame, and so on.
> 
> Credit-based policies:
> 	The AVB credit based shaper acts independently, per class, to control
> the bandwidth distribution between normal traffic and time-sensitive
> traffic with respect to the total link bandwidth available.
> 	Credit based shaper conbined with time-based shaping:  
> 		- priority: ClassA queue > ClassB queue > best effort
> 		- ensure the queue bandwidth as user set based on time-based shaping
> algorithms (transmitter transmit frame from three queue in turn based
> on time-based shaping algorithms)
> 	And in real AVB case,  each streaming can be independent, and are
> fixed on related queue. Then driver level should implement
> .ndo_select_queue() to put the streaming into related queue. That is
> what the patch did.
> 
> The current driver config the three queue to credit-based policies
> (AVB), the patch seems no problem for the implementation. Do you have
> any suggestion ?
> 

I tried using the round robin mode by adding this:

+       /* Set Round-Robin policy */
+       writel(1, fep->hwp + FEC_QOS_SCHEME);

After a while I got the warning non the less:

WARNING: CPU: 0 PID: 0 at net/sched/sch_generic.c:316
dev_watchdog+0x248/0x24c
NETDEV WATCHDOG: eth0 (fec): transmit queue 2 timed out
Modules linked in:
CPU: 0 PID: 0 Comm: swapper/0 Not tainted
4.11.0-rc1-00058-g56d22eced8bc-dirty #377
Hardware name: Freescale i.MX7 Dual (Device Tree)
[<c02266b0>] (unwind_backtrace) from [<c0222d7c>] (show_stack+0x10/0x14)
[<c0222d7c>] (show_stack) from [<c04d4098>] (dump_stack+0x78/0x8c)
[<c04d4098>] (dump_stack) from [<c0236548>] (__warn+0xe8/0x100)
[<c0236548>] (__warn) from [<c0236598>] (warn_slowpath_fmt+0x38/0x48)
[<c0236598>] (warn_slowpath_fmt) from [<c0805904>]
(dev_watchdog+0x248/0x24c)
[<c0805904>] (dev_watchdog) from [<c028a0e8>] (call_timer_fn+0x28/0x98)
[<c028a0e8>] (call_timer_fn) from [<c028a1f8>] (expire_timers+0xa0/0xac)
[<c028a1f8>] (expire_timers) from [<c028a2a0>]
(run_timer_softirq+0x9c/0x194)
[<c028a2a0>] (run_timer_softirq) from [<c023aaf8>]
(__do_softirq+0x114/0x234)
[<c023aaf8>] (__do_softirq) from [<c023aee4>] (irq_exit+0xcc/0x108)
[<c023aee4>] (irq_exit) from [<c0279920>]
(__handle_domain_irq+0x80/0xec)
[<c0279920>] (__handle_domain_irq) from [<c0201544>]
(gic_handle_irq+0x48/0x8c)
[<c0201544>] (gic_handle_irq) from [<c0223838>] (__irq_svc+0x58/0x8c)
Exception stack(0xc1001f28 to 0xc1001f70)
1f20:                   00000001 00000000 00000000 c022fdc0 c1000000
c1003d80
1f40: c1003d34 c0e72ed0 c0bd9b04 c1001f80 00000000 00000000 00000000
c1001f78
1f60: c022048c c0220490 60000013 ffffffff
[<c0223838>] (__irq_svc) from [<c0220490>] (arch_cpu_idle+0x38/0x3c)
[<c0220490>] (arch_cpu_idle) from [<c026ec60>] (do_idle+0x170/0x204)
[<c026ec60>] (do_idle) from [<c026efac>] (cpu_startup_entry+0x18/0x1c)
[<c026efac>] (cpu_startup_entry) from [<c0e00c88>]
(start_kernel+0x394/0x3a0)
---[ end trace a474f341d40e0705 ]---
fec 30be0000.ethernet eth0: TX ring dump

I disabled the regular ring dump and only printed one line. It seems to
come up every 2 seconds, and checking cat /proc/interrupts showed that
queue 2 stayed at its last value (3562218):

 58:    3091320     GIC-0 150 Level     30be0000.ethernet
 59:    3562218     GIC-0 151 Level     30be0000.ethernet
 60:   13377922     GIC-0 152 Level     30be0000.ethernet


--
Stefan

RE: [PATCH] net: fec: select queue depending on VLAN priority

[Andy Duan]

From: Stefan Agner <stefan@agner.ch> Sent: Thursday, May 11, 2017 12:08 PM
>To: Andy Duan <fugang.duan@nxp.com>
>Cc: David Miller <davem@davemloft.net>; andrew@lunn.ch;
>festevam@gmail.com; netdev@vger.kernel.org; linux-
>kernel@vger.kernel.org
>Subject: Re: [PATCH] net: fec: select queue depending on VLAN priority
>
>On 2017-05-09 19:42, Andy Duan wrote:
>> From: David Miller <davem@davemloft.net> Sent: Tuesday, May 09, 2017
>> 9:39 PM
>>>To: stefan@agner.ch
>>>Cc: Andy Duan <fugang.duan@nxp.com>; andrew@lunn.ch;
>>>festevam@gmail.com; netdev@vger.kernel.org; linux-
>>>kernel@vger.kernel.org
>>>Subject: Re: [PATCH] net: fec: select queue depending on VLAN priority
>>>
>>>From: Stefan Agner <stefan@agner.ch>
>>>Date: Mon,  8 May 2017 22:37:08 -0700
>>>
>>>> Since the addition of the multi queue code with commit 59d0f7465644
>>>> ("net: fec: init multi queue date structure") the queue selection
>>>> has been handelt by the default transmit queue selection
>>>> implementation which tries to evenly distribute the traffic across
>>>> all available queues. This selection presumes that the queues are
>>>> using an equal priority, however, the queues 1 and 2 are actually of
>>>> higher priority (the classification of the queues is enabled in
>fec_enet_enable_ring).
>>>>
>>>> This can lead to net scheduler warnings and continuous TX ring dumps
>>>> when exercising the system with iperf.
>>>>
>>>> Use only queue 0 for all common traffic (no VLAN and P802.1p
>>>> priority
>>>> 0 and 1) and route level 2-7 through queue 1 and 2.
>>>>
>>>> Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
>>>> Fixes: 59d0f7465644 ("net: fec: init multi queue date structure")
>>>
>>>If the queues are used for prioritization, and it does not have
>>>multiple normal priority level queues, multiqueue is not what the
>>>driver should have implemented.
>> Firstly, HW multiple queues support:
>> 	- Traffic-shaping bandwidth distribution supports credit-based and
>> round-robin-based policies. Either policy can be combined with
>> time-based shaping.
>> 	- AVB (Audio Video Bridging, IEEE 802.1Qav) features:
>> 		* Credit-based bandwidth distribution policy can be combined
>with
>> time-based shaping
>> 		* AVB endpoint talker and listener support
>> 		* Support for arbitration between different priority traffic (for
>> example, AVB class A, AVB class B, and non-AVB) Round-robin-based
>> policies:
>> 	It has the same priority for three queues: In the round-robin QoS
>> scheme, each queue is given an equal opportunity to transmit one
>> frame. For example, if queue n has a frame to transmit, the queue
>> transmits its frame. After queue n has transmitted its frame, or if
>> queue n does not have a frame to transmit, queue n+1 is then allowed
>> to transmit its frame, and so on.
>>
>> Credit-based policies:
>> 	The AVB credit based shaper acts independently, per class, to control
>> the bandwidth distribution between normal traffic and time-sensitive
>> traffic with respect to the total link bandwidth available.
>> 	Credit based shaper conbined with time-based shaping:
>> 		- priority: ClassA queue > ClassB queue > best effort
>> 		- ensure the queue bandwidth as user set based on time-
>based shaping
>> algorithms (transmitter transmit frame from three queue in turn based
>> on time-based shaping algorithms)
>> 	And in real AVB case,  each streaming can be independent, and are
>> fixed on related queue. Then driver level should implement
>> .ndo_select_queue() to put the streaming into related queue. That is
>> what the patch did.
>>
>> The current driver config the three queue to credit-based policies
>> (AVB), the patch seems no problem for the implementation. Do you have
>> any suggestion ?
>>
>
>I tried using the round robin mode by adding this:
>
>+       /* Set Round-Robin policy */
>+       writel(1, fep->hwp + FEC_QOS_SCHEME);
>
>After a while I got the warning non the less:
>
>WARNING: CPU: 0 PID: 0 at net/sched/sch_generic.c:316
>dev_watchdog+0x248/0x24c NETDEV WATCHDOG: eth0 (fec): transmit queue
>2 timed out Modules linked in:
>CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.11.0-rc1-00058-g56d22eced8bc-
>dirty #377 Hardware name: Freescale i.MX7 Dual (Device Tree) [<c02266b0>]
>(unwind_backtrace) from [<c0222d7c>] (show_stack+0x10/0x14) [<c0222d7c>]
>(show_stack) from [<c04d4098>] (dump_stack+0x78/0x8c) [<c04d4098>]
>(dump_stack) from [<c0236548>] (__warn+0xe8/0x100) [<c0236548>]
>(__warn) from [<c0236598>] (warn_slowpath_fmt+0x38/0x48) [<c0236598>]
>(warn_slowpath_fmt) from [<c0805904>]
>(dev_watchdog+0x248/0x24c)
>[<c0805904>] (dev_watchdog) from [<c028a0e8>] (call_timer_fn+0x28/0x98)
>[<c028a0e8>] (call_timer_fn) from [<c028a1f8>] (expire_timers+0xa0/0xac)
>[<c028a1f8>] (expire_timers) from [<c028a2a0>]
>(run_timer_softirq+0x9c/0x194)
>[<c028a2a0>] (run_timer_softirq) from [<c023aaf8>]
>(__do_softirq+0x114/0x234)
>[<c023aaf8>] (__do_softirq) from [<c023aee4>] (irq_exit+0xcc/0x108)
>[<c023aee4>] (irq_exit) from [<c0279920>]
>(__handle_domain_irq+0x80/0xec)
>[<c0279920>] (__handle_domain_irq) from [<c0201544>]
>(gic_handle_irq+0x48/0x8c)
>[<c0201544>] (gic_handle_irq) from [<c0223838>] (__irq_svc+0x58/0x8c)
>Exception stack(0xc1001f28 to 0xc1001f70)
>1f20:                   00000001 00000000 00000000 c022fdc0 c1000000
>c1003d80
>1f40: c1003d34 c0e72ed0 c0bd9b04 c1001f80 00000000 00000000 00000000
>c1001f78
>1f60: c022048c c0220490 60000013 ffffffff [<c0223838>] (__irq_svc) from
>[<c0220490>] (arch_cpu_idle+0x38/0x3c) [<c0220490>] (arch_cpu_idle) from
>[<c026ec60>] (do_idle+0x170/0x204) [<c026ec60>] (do_idle) from
>[<c026efac>] (cpu_startup_entry+0x18/0x1c) [<c026efac>]
>(cpu_startup_entry) from [<c0e00c88>]
>(start_kernel+0x394/0x3a0)
>---[ end trace a474f341d40e0705 ]---
>fec 30be0000.ethernet eth0: TX ring dump
>
>I disabled the regular ring dump and only printed one line. It seems to come
>up every 2 seconds, and checking cat /proc/interrupts showed that queue 2
>stayed at its last value (3562218):
>
> 58:    3091320     GIC-0 150 Level     30be0000.ethernet
> 59:    3562218     GIC-0 151 Level     30be0000.ethernet
> 60:   13377922     GIC-0 152 Level     30be0000.ethernet

Pls check ENETx_DMAnCFG[16] whether is set, and disable time-based shaping for round robin.

Re: [PATCH] net: fec: select queue depending on VLAN priority

[Stefan Agner]

On 2017-05-10 21:49, Andy Duan wrote:
> From: Stefan Agner <stefan@agner.ch> Sent: Thursday, May 11, 2017 12:08 PM
>>To: Andy Duan <fugang.duan@nxp.com>
>>Cc: David Miller <davem@davemloft.net>; andrew@lunn.ch;
>>festevam@gmail.com; netdev@vger.kernel.org; linux-
>>kernel@vger.kernel.org
>>Subject: Re: [PATCH] net: fec: select queue depending on VLAN priority
>>
>>On 2017-05-09 19:42, Andy Duan wrote:
>>> From: David Miller <davem@davemloft.net> Sent: Tuesday, May 09, 2017
>>> 9:39 PM
>>>>To: stefan@agner.ch
>>>>Cc: Andy Duan <fugang.duan@nxp.com>; andrew@lunn.ch;
>>>>festevam@gmail.com; netdev@vger.kernel.org; linux-
>>>>kernel@vger.kernel.org
>>>>Subject: Re: [PATCH] net: fec: select queue depending on VLAN priority
>>>>
>>>>From: Stefan Agner <stefan@agner.ch>
>>>>Date: Mon,  8 May 2017 22:37:08 -0700
>>>>
>>>>> Since the addition of the multi queue code with commit 59d0f7465644
>>>>> ("net: fec: init multi queue date structure") the queue selection
>>>>> has been handelt by the default transmit queue selection
>>>>> implementation which tries to evenly distribute the traffic across
>>>>> all available queues. This selection presumes that the queues are
>>>>> using an equal priority, however, the queues 1 and 2 are actually of
>>>>> higher priority (the classification of the queues is enabled in
>>fec_enet_enable_ring).
>>>>>
>>>>> This can lead to net scheduler warnings and continuous TX ring dumps
>>>>> when exercising the system with iperf.
>>>>>
>>>>> Use only queue 0 for all common traffic (no VLAN and P802.1p
>>>>> priority
>>>>> 0 and 1) and route level 2-7 through queue 1 and 2.
>>>>>
>>>>> Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
>>>>> Fixes: 59d0f7465644 ("net: fec: init multi queue date structure")
>>>>
>>>>If the queues are used for prioritization, and it does not have
>>>>multiple normal priority level queues, multiqueue is not what the
>>>>driver should have implemented.
>>> Firstly, HW multiple queues support:
>>> 	- Traffic-shaping bandwidth distribution supports credit-based and
>>> round-robin-based policies. Either policy can be combined with
>>> time-based shaping.
>>> 	- AVB (Audio Video Bridging, IEEE 802.1Qav) features:
>>> 		* Credit-based bandwidth distribution policy can be combined
>>with
>>> time-based shaping
>>> 		* AVB endpoint talker and listener support
>>> 		* Support for arbitration between different priority traffic (for
>>> example, AVB class A, AVB class B, and non-AVB) Round-robin-based
>>> policies:
>>> 	It has the same priority for three queues: In the round-robin QoS
>>> scheme, each queue is given an equal opportunity to transmit one
>>> frame. For example, if queue n has a frame to transmit, the queue
>>> transmits its frame. After queue n has transmitted its frame, or if
>>> queue n does not have a frame to transmit, queue n+1 is then allowed
>>> to transmit its frame, and so on.
>>>
>>> Credit-based policies:
>>> 	The AVB credit based shaper acts independently, per class, to control
>>> the bandwidth distribution between normal traffic and time-sensitive
>>> traffic with respect to the total link bandwidth available.
>>> 	Credit based shaper conbined with time-based shaping:
>>> 		- priority: ClassA queue > ClassB queue > best effort
>>> 		- ensure the queue bandwidth as user set based on time-
>>based shaping
>>> algorithms (transmitter transmit frame from three queue in turn based
>>> on time-based shaping algorithms)
>>> 	And in real AVB case,  each streaming can be independent, and are
>>> fixed on related queue. Then driver level should implement
>>> .ndo_select_queue() to put the streaming into related queue. That is
>>> what the patch did.
>>>
>>> The current driver config the three queue to credit-based policies
>>> (AVB), the patch seems no problem for the implementation. Do you have
>>> any suggestion ?
>>>
>>
>>I tried using the round robin mode by adding this:
>>
>>+       /* Set Round-Robin policy */
>>+       writel(1, fep->hwp + FEC_QOS_SCHEME);
>>
>>After a while I got the warning non the less:
>>
>>WARNING: CPU: 0 PID: 0 at net/sched/sch_generic.c:316
>>dev_watchdog+0x248/0x24c NETDEV WATCHDOG: eth0 (fec): transmit queue
>>2 timed out Modules linked in:
>>CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.11.0-rc1-00058-g56d22eced8bc-
>>dirty #377 Hardware name: Freescale i.MX7 Dual (Device Tree) [<c02266b0>]
>>(unwind_backtrace) from [<c0222d7c>] (show_stack+0x10/0x14) [<c0222d7c>]
>>(show_stack) from [<c04d4098>] (dump_stack+0x78/0x8c) [<c04d4098>]
>>(dump_stack) from [<c0236548>] (__warn+0xe8/0x100) [<c0236548>]
>>(__warn) from [<c0236598>] (warn_slowpath_fmt+0x38/0x48) [<c0236598>]
>>(warn_slowpath_fmt) from [<c0805904>]
>>(dev_watchdog+0x248/0x24c)
>>[<c0805904>] (dev_watchdog) from [<c028a0e8>] (call_timer_fn+0x28/0x98)
>>[<c028a0e8>] (call_timer_fn) from [<c028a1f8>] (expire_timers+0xa0/0xac)
>>[<c028a1f8>] (expire_timers) from [<c028a2a0>]
>>(run_timer_softirq+0x9c/0x194)
>>[<c028a2a0>] (run_timer_softirq) from [<c023aaf8>]
>>(__do_softirq+0x114/0x234)
>>[<c023aaf8>] (__do_softirq) from [<c023aee4>] (irq_exit+0xcc/0x108)
>>[<c023aee4>] (irq_exit) from [<c0279920>]
>>(__handle_domain_irq+0x80/0xec)
>>[<c0279920>] (__handle_domain_irq) from [<c0201544>]
>>(gic_handle_irq+0x48/0x8c)
>>[<c0201544>] (gic_handle_irq) from [<c0223838>] (__irq_svc+0x58/0x8c)
>>Exception stack(0xc1001f28 to 0xc1001f70)
>>1f20:                   00000001 00000000 00000000 c022fdc0 c1000000
>>c1003d80
>>1f40: c1003d34 c0e72ed0 c0bd9b04 c1001f80 00000000 00000000 00000000
>>c1001f78
>>1f60: c022048c c0220490 60000013 ffffffff [<c0223838>] (__irq_svc) from
>>[<c0220490>] (arch_cpu_idle+0x38/0x3c) [<c0220490>] (arch_cpu_idle) from
>>[<c026ec60>] (do_idle+0x170/0x204) [<c026ec60>] (do_idle) from
>>[<c026efac>] (cpu_startup_entry+0x18/0x1c) [<c026efac>]
>>(cpu_startup_entry) from [<c0e00c88>]
>>(start_kernel+0x394/0x3a0)
>>---[ end trace a474f341d40e0705 ]---
>>fec 30be0000.ethernet eth0: TX ring dump
>>
>>I disabled the regular ring dump and only printed one line. It seems to come
>>up every 2 seconds, and checking cat /proc/interrupts showed that queue 2
>>stayed at its last value (3562218):
>>
>> 58:    3091320     GIC-0 150 Level     30be0000.ethernet
>> 59:    3562218     GIC-0 151 Level     30be0000.ethernet
>> 60:   13377922     GIC-0 152 Level     30be0000.ethernet
> 
> Pls check ENETx_DMAnCFG[16] whether is set, and disable time-based
> shaping for round robin.

When I do not set ENETx_DMAnCFG[16] (DMA_CLASS_EN), then networking
stops working (I cannot mount NFS).

Time-based is disabled by default I guess?

RE: [PATCH] net: fec: select queue depending on VLAN priority

[Andy Duan]

From: Stefan Agner <stefan@agner.ch> Sent: Monday, May 15, 2017 1:39 PM
>To: Andy Duan <fugang.duan@nxp.com>
>Cc: David Miller <davem@davemloft.net>; andrew@lunn.ch;
>festevam@gmail.com; netdev@vger.kernel.org; linux-
>kernel@vger.kernel.org
>Subject: Re: [PATCH] net: fec: select queue depending on VLAN priority
>
>On 2017-05-10 21:49, Andy Duan wrote:
>> From: Stefan Agner <stefan@agner.ch> Sent: Thursday, May 11, 2017
>> 12:08 PM
>>>To: Andy Duan <fugang.duan@nxp.com>
>>>Cc: David Miller <davem@davemloft.net>; andrew@lunn.ch;
>>>festevam@gmail.com; netdev@vger.kernel.org; linux-
>>>kernel@vger.kernel.org
>>>Subject: Re: [PATCH] net: fec: select queue depending on VLAN priority
>>>
>>>On 2017-05-09 19:42, Andy Duan wrote:
>>>> From: David Miller <davem@davemloft.net> Sent: Tuesday, May 09, 2017
>>>> 9:39 PM
>>>>>To: stefan@agner.ch
>>>>>Cc: Andy Duan <fugang.duan@nxp.com>; andrew@lunn.ch;
>>>>>festevam@gmail.com; netdev@vger.kernel.org; linux-
>>>>>kernel@vger.kernel.org
>>>>>Subject: Re: [PATCH] net: fec: select queue depending on VLAN
>>>>>priority
>>>>>
>>>>>From: Stefan Agner <stefan@agner.ch>
>>>>>Date: Mon,  8 May 2017 22:37:08 -0700
>>>>>
>>>>>> Since the addition of the multi queue code with commit
>>>>>> 59d0f7465644
>>>>>> ("net: fec: init multi queue date structure") the queue selection
>>>>>> has been handelt by the default transmit queue selection
>>>>>> implementation which tries to evenly distribute the traffic across
>>>>>> all available queues. This selection presumes that the queues are
>>>>>> using an equal priority, however, the queues 1 and 2 are actually
>>>>>> of higher priority (the classification of the queues is enabled in
>>>fec_enet_enable_ring).
>>>>>>
>>>>>> This can lead to net scheduler warnings and continuous TX ring
>>>>>> dumps when exercising the system with iperf.
>>>>>>
>>>>>> Use only queue 0 for all common traffic (no VLAN and P802.1p
>>>>>> priority
>>>>>> 0 and 1) and route level 2-7 through queue 1 and 2.
>>>>>>
>>>>>> Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
>>>>>> Fixes: 59d0f7465644 ("net: fec: init multi queue date structure")
>>>>>
>>>>>If the queues are used for prioritization, and it does not have
>>>>>multiple normal priority level queues, multiqueue is not what the
>>>>>driver should have implemented.
>>>> Firstly, HW multiple queues support:
>>>> 	- Traffic-shaping bandwidth distribution supports credit-based and
>>>> round-robin-based policies. Either policy can be combined with
>>>> time-based shaping.
>>>> 	- AVB (Audio Video Bridging, IEEE 802.1Qav) features:
>>>> 		* Credit-based bandwidth distribution policy can be combined
>>>with
>>>> time-based shaping
>>>> 		* AVB endpoint talker and listener support
>>>> 		* Support for arbitration between different priority traffic (for
>>>> example, AVB class A, AVB class B, and non-AVB) Round-robin-based
>>>> policies:
>>>> 	It has the same priority for three queues: In the round-robin QoS
>>>> scheme, each queue is given an equal opportunity to transmit one
>>>> frame. For example, if queue n has a frame to transmit, the queue
>>>> transmits its frame. After queue n has transmitted its frame, or if
>>>> queue n does not have a frame to transmit, queue n+1 is then allowed
>>>> to transmit its frame, and so on.
>>>>
>>>> Credit-based policies:
>>>> 	The AVB credit based shaper acts independently, per class, to
>>>> control the bandwidth distribution between normal traffic and
>>>> time-sensitive traffic with respect to the total link bandwidth available.
>>>> 	Credit based shaper conbined with time-based shaping:
>>>> 		- priority: ClassA queue > ClassB queue > best effort
>>>> 		- ensure the queue bandwidth as user set based on time-
>>>based shaping
>>>> algorithms (transmitter transmit frame from three queue in turn
>>>> based on time-based shaping algorithms)
>>>> 	And in real AVB case,  each streaming can be independent, and are
>>>> fixed on related queue. Then driver level should implement
>>>> .ndo_select_queue() to put the streaming into related queue. That is
>>>> what the patch did.
>>>>
>>>> The current driver config the three queue to credit-based policies
>>>> (AVB), the patch seems no problem for the implementation. Do you
>>>> have any suggestion ?
>>>>
>>>
>>>I tried using the round robin mode by adding this:
>>>
>>>+       /* Set Round-Robin policy */
>>>+       writel(1, fep->hwp + FEC_QOS_SCHEME);
>>>
>>>After a while I got the warning non the less:
>>>
>>>WARNING: CPU: 0 PID: 0 at net/sched/sch_generic.c:316
>>>dev_watchdog+0x248/0x24c NETDEV WATCHDOG: eth0 (fec): transmit
>queue
>>>2 timed out Modules linked in:
>>>CPU: 0 PID: 0 Comm: swapper/0 Not tainted
>>>4.11.0-rc1-00058-g56d22eced8bc- dirty #377 Hardware name: Freescale
>>>i.MX7 Dual (Device Tree) [<c02266b0>]
>>>(unwind_backtrace) from [<c0222d7c>] (show_stack+0x10/0x14)
>>>[<c0222d7c>]
>>>(show_stack) from [<c04d4098>] (dump_stack+0x78/0x8c) [<c04d4098>]
>>>(dump_stack) from [<c0236548>] (__warn+0xe8/0x100) [<c0236548>]
>>>(__warn) from [<c0236598>] (warn_slowpath_fmt+0x38/0x48)
>[<c0236598>]
>>>(warn_slowpath_fmt) from [<c0805904>]
>>>(dev_watchdog+0x248/0x24c)
>>>[<c0805904>] (dev_watchdog) from [<c028a0e8>]
>>>(call_timer_fn+0x28/0x98) [<c028a0e8>] (call_timer_fn) from
>>>[<c028a1f8>] (expire_timers+0xa0/0xac) [<c028a1f8>] (expire_timers)
>>>from [<c028a2a0>]
>>>(run_timer_softirq+0x9c/0x194)
>>>[<c028a2a0>] (run_timer_softirq) from [<c023aaf8>]
>>>(__do_softirq+0x114/0x234)
>>>[<c023aaf8>] (__do_softirq) from [<c023aee4>] (irq_exit+0xcc/0x108)
>>>[<c023aee4>] (irq_exit) from [<c0279920>]
>>>(__handle_domain_irq+0x80/0xec)
>>>[<c0279920>] (__handle_domain_irq) from [<c0201544>]
>>>(gic_handle_irq+0x48/0x8c)
>>>[<c0201544>] (gic_handle_irq) from [<c0223838>] (__irq_svc+0x58/0x8c)
>>>Exception stack(0xc1001f28 to 0xc1001f70)
>>>1f20:                   00000001 00000000 00000000 c022fdc0 c1000000
>>>c1003d80
>>>1f40: c1003d34 c0e72ed0 c0bd9b04 c1001f80 00000000 00000000 00000000
>>>c1001f78
>>>1f60: c022048c c0220490 60000013 ffffffff [<c0223838>] (__irq_svc)
>>>from [<c0220490>] (arch_cpu_idle+0x38/0x3c) [<c0220490>]
>>>(arch_cpu_idle) from [<c026ec60>] (do_idle+0x170/0x204) [<c026ec60>]
>>>(do_idle) from [<c026efac>] (cpu_startup_entry+0x18/0x1c) [<c026efac>]
>>>(cpu_startup_entry) from [<c0e00c88>]
>>>(start_kernel+0x394/0x3a0)
>>>---[ end trace a474f341d40e0705 ]---
>>>fec 30be0000.ethernet eth0: TX ring dump
>>>
>>>I disabled the regular ring dump and only printed one line. It seems
>>>to come up every 2 seconds, and checking cat /proc/interrupts showed
>>>that queue 2 stayed at its last value (3562218):
>>>
>>> 58:    3091320     GIC-0 150 Level     30be0000.ethernet
>>> 59:    3562218     GIC-0 151 Level     30be0000.ethernet
>>> 60:   13377922     GIC-0 152 Level     30be0000.ethernet
>>
>> Pls check ENETx_DMAnCFG[16] whether is set, and disable time-based
>> shaping for round robin.
>
>When I do not set ENETx_DMAnCFG[16] (DMA_CLASS_EN), then networking
>stops working (I cannot mount NFS).
>
>Time-based is disabled by default I guess?

No, you should clear ENETx_DMAnCFG[15:0],  and set ENETx_DMAnCFG[16].

Andy

Re: [PATCH] net: fec: select queue depending on VLAN priority

[Stefan Agner]

On 2017-05-09 06:39, David Miller wrote:
> From: Stefan Agner <stefan@agner.ch>
> Date: Mon,  8 May 2017 22:37:08 -0700
> 
>> Since the addition of the multi queue code with commit 59d0f7465644
>> ("net: fec: init multi queue date structure") the queue selection
>> has been handelt by the default transmit queue selection
>> implementation which tries to evenly distribute the traffic across
>> all available queues. This selection presumes that the queues are
>> using an equal priority, however, the queues 1 and 2 are actually
>> of higher priority (the classification of the queues is enabled in
>> fec_enet_enable_ring).
>>
>> This can lead to net scheduler warnings and continuous TX ring
>> dumps when exercising the system with iperf.
>>
>> Use only queue 0 for all common traffic (no VLAN and P802.1p
>> priority 0 and 1) and route level 2-7 through queue 1 and 2.
>>
>> Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
>> Fixes: 59d0f7465644 ("net: fec: init multi queue date structure")
> 
> If the queues are used for prioritization, and it does not have
> multiple normal priority level queues, multiqueue is not what the
> driver should have implemented.

As Andy mentioned, there is also a round-robin mode. I'll try that.

What would be the proper way to use the prioritized queues?

--
Stefan