2021-06-11 09:53:53

by Joakim Zhang

[permalink] [raw]
Subject: [PATCH V2 net-next 0/2] net: fec: fix TX bandwidth fluctuations

This patch set intends to fix TX bandwidth fluctuations, any feedback would be appreciated.

---
ChangeLogs:
V1: remove RFC tag, RFC discussions please turn to below:
https://lore.kernel.org/lkml/[email protected]/T/
V2: change functions to be static in this patch set. And add the
t-b tag.

Fugang Duan (1):
net: fec: add ndo_select_queue to fix TX bandwidth fluctuations

Joakim Zhang (1):
net: fec: add FEC_QUIRK_HAS_MULTI_QUEUES represents i.MX6SX ENET IP

drivers/net/ethernet/freescale/fec.h | 5 +++
drivers/net/ethernet/freescale/fec_main.c | 43 ++++++++++++++++++++---
2 files changed, 43 insertions(+), 5 deletions(-)

--
2.17.1


2021-06-11 09:55:31

by Joakim Zhang

[permalink] [raw]
Subject: [PATCH V2 2/2] net: fec: add ndo_select_queue to fix TX bandwidth fluctuations

From: Fugang Duan <[email protected]>

As we know that AVB is enabled by default, and the ENET IP design is
queue 0 for best effort, queue 1&2 for AVB Class A&B. Bandwidth of each
queue 1&2 set in driver is 50%, TX bandwidth fluctuated when selecting
tx queues randomly with FEC_QUIRK_HAS_AVB quirk available.

This patch adds ndo_select_queue callback to select queues for
transmitting to fix this issue. It will always return queue 0 if this is
not a vlan packet, and return queue 1 or 2 based on priority of vlan
packet.

You may complain that in fact we only use single queue for trasmitting
if we are not targeted to VLAN. Yes, but seems we have no choice, since
AVB is enabled when the driver probed, we can't switch this feature
dynamicly. After compare multiple queues to single queue, TX throughput
almost no improvement.

One way we can implemet is to configure the driver to multiple queues
with Round-robin scheme by default. Then add ndo_setup_tc callback to
enable/disable AVB feature for users. Unfortunately, ENET AVB IP seems
not follow the standard 802.1Qav spec. We only can program
DMAnCFG[IDLE_SLOPE] field to calculate bandwidth fraction. And idle
slope is restricted to certain valus (a total of 19). It's far away from
CBS QDisc implemented in Linux TC framework. If you strongly suggest to do
this, I think we only can support limited numbers of bandwidth and reject
others, but it's really urgly and wried.

With this patch, VLAN tagged packets route to queue 0/1/2 based on vlan
priority; VLAN untagged packets route to queue 0.

Tested-by: Frieder Schrempf <[email protected]>
Reported-by: Frieder Schrempf <[email protected]>
Signed-off-by: Fugang Duan <[email protected]>
Signed-off-by: Joakim Zhang <[email protected]>
---
drivers/net/ethernet/freescale/fec_main.c | 32 +++++++++++++++++++++++
1 file changed, 32 insertions(+)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 98cd38379275..00d3fa8ed3a7 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -76,6 +76,8 @@ static void fec_enet_itr_coal_init(struct net_device *ndev);

#define DRIVER_NAME "fec"

+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)
#define FEC_ENET_RSEM_V 0x84
@@ -3240,10 +3242,40 @@ static int fec_set_features(struct net_device *netdev,
return 0;
}

+static u16 fec_enet_get_raw_vlan_tci(struct sk_buff *skb)
+{
+ struct vlan_ethhdr *vhdr;
+ unsigned short vlan_TCI = 0;
+
+ if (skb->protocol == ntohs(ETH_P_ALL)) {
+ vhdr = (struct vlan_ethhdr *)(skb->data);
+ vlan_TCI = ntohs(vhdr->h_vlan_TCI);
+ }
+
+ return vlan_TCI;
+}
+
+static u16 fec_enet_select_queue(struct net_device *ndev, struct sk_buff *skb,
+ struct net_device *sb_dev)
+{
+ struct fec_enet_private *fep = netdev_priv(ndev);
+ u16 vlan_tag;
+
+ if (!(fep->quirks & FEC_QUIRK_HAS_AVB))
+ return netdev_pick_tx(ndev, skb, NULL);
+
+ vlan_tag = fec_enet_get_raw_vlan_tci(skb);
+ if (!vlan_tag)
+ return vlan_tag;
+
+ return fec_enet_vlan_pri_to_queue[vlan_tag >> 13];
+}
+
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.17.1

2021-06-11 20:28:47

by David Miller

[permalink] [raw]
Subject: Re: [PATCH V2 net-next 0/2] net: fec: fix TX bandwidth fluctuations

From: Joakim Zhang <[email protected]>
Date: Fri, 11 Jun 2021 17:50:03 +0800

> This patch set intends to fix TX bandwidth fluctuations, any feedback would be appreciated.
>
> ---
> ChangeLogs:
> V1: remove RFC tag, RFC discussions please turn to below:
> https://lore.kernel.org/lkml/[email protected]/T/
> V2: change functions to be static in this patch set. And add the
> t-b tag.

Please fix these warnings in patch #2:

https://patchwork.hopto.org/static/nipa/498729/12315211/build_allmodconfig_warn/summary

Thank you.

2021-06-15 06:47:23

by Joakim Zhang

[permalink] [raw]
Subject: RE: [PATCH V2 net-next 0/2] net: fec: fix TX bandwidth fluctuations


Hi David,

> -----Original Message-----
> From: David Miller <[email protected]>
> Sent: 2021??6??12?? 4:25
> To: Joakim Zhang <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH V2 net-next 0/2] net: fec: fix TX bandwidth fluctuations
>
> From: Joakim Zhang <[email protected]>
> Date: Fri, 11 Jun 2021 17:50:03 +0800
>
> > This patch set intends to fix TX bandwidth fluctuations, any feedback would
> be appreciated.
> >
> > ---
> > ChangeLogs:
> > V1: remove RFC tag, RFC discussions please turn to below:
> >
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ker
> nel.org%2Flkml%2FYK0Ce5YxR2WYbrAo%40lunn.ch%2FT%2F&amp;data=04%7
> C01%7Cqiangqing.zhang%40nxp.com%7C45b786c85a294b3ea9ec08d92d1709
> 0c%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637590399225645
> 759%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzI
> iLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=CFeL4rNWqNs4Xc
> 2NqtBiQxex4%2FXAUhEXWQreftFhnY4%3D&amp;reserved=0
> > V2: change functions to be static in this patch set. And add the
> > t-b tag.
>
> Please fix these warnings in patch #2:
>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchw
> ork.hopto.org%2Fstatic%2Fnipa%2F498729%2F12315211%2Fbuild_allmodconfi
> g_warn%2Fsummary&amp;data=04%7C01%7Cqiangqing.zhang%40nxp.com%
> 7C45b786c85a294b3ea9ec08d92d17090c%7C686ea1d3bc2b4c6fa92cd99c5c30
> 1635%7C0%7C0%7C637590399225645759%7CUnknown%7CTWFpbGZsb3d8ey
> JWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
> 7C1000&amp;sdata=eb4Ujx2FKOnu5M41j1RnPLCBoEoGfQiCO%2BO5MzIsXyE
> %3D&amp;reserved=0

I can't reproduce these warnings with " make ARCH=arm64 allmodconfig", could you please show me the command you used? Thanks.

Best Regards,
Joakim Zhang
> Thank you.

2021-06-15 12:29:00

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH V2 net-next 0/2] net: fec: fix TX bandwidth fluctuations

> I can't reproduce these warnings with " make ARCH=arm64 allmodconfig", could you please show me the command you used? Thanks.

Try adding W=1

Andrew

2021-06-16 08:31:10

by Joakim Zhang

[permalink] [raw]
Subject: RE: [PATCH V2 net-next 0/2] net: fec: fix TX bandwidth fluctuations


Hi Andrew, David,

> -----Original Message-----
> From: Andrew Lunn <[email protected]>
> Sent: 2021??6??15?? 20:28
> To: Joakim Zhang <[email protected]>
> Cc: David Miller <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH V2 net-next 0/2] net: fec: fix TX bandwidth fluctuations
>
> > I can't reproduce these warnings with " make ARCH=arm64 allmodconfig",
> could you please show me the command you used? Thanks.
>
> Try adding W=1

Thanks.

I try below build options, also can't reproduce this issue, so really don't know how to fix it.

make ARCH=arm64 distclean
make ARCH=arm64 allmodconfig
make -j8 ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- W=1 / make -j8 ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- W=2 / make -j8 ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- W=3

I saw many unrelated warnings...

Best Regards,
Joakim Zhang
> Andrew

2021-06-16 18:42:28

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH V2 net-next 0/2] net: fec: fix TX bandwidth fluctuations

> I try below build options, also can't reproduce this issue, so really don't know how to fix it.
>
> make ARCH=arm64 distclean
> make ARCH=arm64 allmodconfig
> make -j8 ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- W=1 / make -j8 ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- W=2 / make -j8 ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- W=3
>
> I saw many unrelated warnings...

Then it could be sparse. Install sparse and use C=1.

Andrew

2021-06-17 11:47:38

by Joakim Zhang

[permalink] [raw]
Subject: RE: [PATCH V2 net-next 0/2] net: fec: fix TX bandwidth fluctuations


Hi Andrew,

> -----Original Message-----
> From: Andrew Lunn <[email protected]>
> Sent: 2021??6??16?? 21:06
> To: Joakim Zhang <[email protected]>
> Cc: David Miller <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH V2 net-next 0/2] net: fec: fix TX bandwidth fluctuations
>
> > I try below build options, also can't reproduce this issue, so really don't know
> how to fix it.
> >
> > make ARCH=arm64 distclean
> > make ARCH=arm64 allmodconfig
> > make -j8 ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- W=1 / make -j8
> ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- W=2 / make -j8
> ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- W=3
> >
> > I saw many unrelated warnings...
>
> Then it could be sparse. Install sparse and use C=1.

After applying the patch #2, I tried to use C=1 yesterday, I double check it today, still no warnings. Anything I missing?

$ make -j8 ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- W=1,C=1
CALL scripts/atomic/check-atomics.sh
CALL scripts/checksyscalls.sh
CHK include/generated/compile.h
CHK kernel/kheaders_data.tar.xz
CC [M] drivers/net/ethernet/freescale/fec_main.o
LD [M] drivers/net/ethernet/freescale/fec.o
MODPOST modules-only.symvers
GEN Module.symvers
CC [M] drivers/net/ethernet/freescale/fec.mod.o
LD [M] drivers/net/ethernet/freescale/fec.ko

Best Regards,
Joakim Zhang
> Andrew

2021-06-17 14:13:09

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH V2 net-next 0/2] net: fec: fix TX bandwidth fluctuations

On Thu, Jun 17, 2021 at 11:40:58AM +0000, Joakim Zhang wrote:
>
> Hi Andrew,
>
> > -----Original Message-----
> > From: Andrew Lunn <[email protected]>
> > Sent: 2021年6月16日 21:06
> > To: Joakim Zhang <[email protected]>
> > Cc: David Miller <[email protected]>; [email protected];
> > [email protected]; [email protected];
> > [email protected]
> > Subject: Re: [PATCH V2 net-next 0/2] net: fec: fix TX bandwidth fluctuations
> >
> > > I try below build options, also can't reproduce this issue, so really don't know
> > how to fix it.
> > >
> > > make ARCH=arm64 distclean
> > > make ARCH=arm64 allmodconfig
> > > make -j8 ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- W=1 / make -j8
> > ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- W=2 / make -j8
> > ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- W=3
> > >
> > > I saw many unrelated warnings...
> >
> > Then it could be sparse. Install sparse and use C=1.
>
> After applying the patch #2, I tried to use C=1 yesterday, I double check it today, still no warnings. Anything I missing?
>
> $ make -j8 ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- W=1,C=1
> CALL scripts/atomic/check-atomics.sh
> CALL scripts/checksyscalls.sh
> CHK include/generated/compile.h
> CHK kernel/kheaders_data.tar.xz
> CC [M] drivers/net/ethernet/freescale/fec_main.o
> LD [M] drivers/net/ethernet/freescale/fec.o
> MODPOST modules-only.symvers
> GEN Module.symvers
> CC [M] drivers/net/ethernet/freescale/fec.mod.o
> LD [M] drivers/net/ethernet/freescale/fec.ko
>
> Best Regards,
> Joakim Zhang
> > Andrew


If you look at
https://patchwork.hopto.org/static/nipa/498729/12315211/build_32bit/stdout

you see:

Kernel: arch/x86/boot/bzImage is ready (#9396)

So it is building for 32 bit x86. So try

make -j8 ARCH=i386 W=1 C=1

Assuming your host is an x86 machine.

Andrew

2021-06-18 09:22:47

by Joakim Zhang

[permalink] [raw]
Subject: RE: [PATCH V2 net-next 0/2] net: fec: fix TX bandwidth fluctuations


Hi Andrew,

> -----Original Message-----
> From: Andrew Lunn <[email protected]>
> Sent: 2021??6??17?? 21:04
> To: Joakim Zhang <[email protected]>
> Cc: David Miller <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH V2 net-next 0/2] net: fec: fix TX bandwidth fluctuations
>
> On Thu, Jun 17, 2021 at 11:40:58AM +0000, Joakim Zhang wrote:
> >
> > Hi Andrew,
> >
> > > -----Original Message-----
> > > From: Andrew Lunn <[email protected]>
> > > Sent: 2021??6??16?? 21:06
> > > To: Joakim Zhang <[email protected]>
> > > Cc: David Miller <[email protected]>; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]
> > > Subject: Re: [PATCH V2 net-next 0/2] net: fec: fix TX bandwidth
> > > fluctuations
> > >
> > > > I try below build options, also can't reproduce this issue, so
> > > > really don't know
> > > how to fix it.
> > > >
> > > > make ARCH=arm64 distclean
> > > > make ARCH=arm64 allmodconfig
> > > > make -j8 ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- W=1 / make
> > > > -j8
> > > ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- W=2 / make -j8
> > > ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- W=3
> > > >
> > > > I saw many unrelated warnings...
> > >
> > > Then it could be sparse. Install sparse and use C=1.
> >
> > After applying the patch #2, I tried to use C=1 yesterday, I double check it
> today, still no warnings. Anything I missing?
> >
> > $ make -j8 ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- W=1,C=1
> > CALL scripts/atomic/check-atomics.sh
> > CALL scripts/checksyscalls.sh
> > CHK include/generated/compile.h
> > CHK kernel/kheaders_data.tar.xz
> > CC [M] drivers/net/ethernet/freescale/fec_main.o
> > LD [M] drivers/net/ethernet/freescale/fec.o
> > MODPOST modules-only.symvers
> > GEN Module.symvers
> > CC [M] drivers/net/ethernet/freescale/fec.mod.o
> > LD [M] drivers/net/ethernet/freescale/fec.ko
> >
> > Best Regards,
> > Joakim Zhang
> > > Andrew
>
>
> If you look at
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchw
> ork.hopto.org%2Fstatic%2Fnipa%2F498729%2F12315211%2Fbuild_32bit%2Fst
> dout&amp;data=04%7C01%7Cqiangqing.zhang%40nxp.com%7Ce966882aff15
> 41ed54c108d931906ad2%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
> %7C637595318610458379%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
> wMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;s
> data=0o34i6pkvqBN1hPApz5Ja8CHjtqn8iwQB8whxg0p8Rw%3D&amp;reserved
> =0
>
> you see:
>
> Kernel: arch/x86/boot/bzImage is ready (#9396)
>
> So it is building for 32 bit x86. So try
>
> make -j8 ARCH=i386 W=1 C=1
>
> Assuming your host is an x86 machine.

Much thanks.

$ git am 0002-net-fec-add-ndo_select_queue-to-fix-TX-bandwidth-flu.patch
Applying: net: fec: add ndo_select_queue to fix TX bandwidth fluctuations
$ make -j8 ARCH=i386 W=1,C=1
CALL scripts/atomic/check-atomics.sh
CALL scripts/checksyscalls.sh
CHK include/generated/compile.h
CHK kernel/kheaders_data.tar.xz
CC [M] drivers/net/ethernet/freescale/fec_main.o
LD [M] drivers/net/ethernet/freescale/fec.o
MODPOST modules-only.symvers
Kernel: arch/x86/boot/bzImage is ready (#1)
GEN Module.symvers
CC [M] drivers/net/ethernet/freescale/fec.mod.o
LD [M] drivers/net/ethernet/freescale/fec.ko

Unfortunately, I still can't see warnings after changing to build x86 image, a strange phenomenon, "W=1 C=1" seems not work, "W=1,C=1" can work.
I also save all of the build logs to double check, there is no build warnings related to FEC driver.

Best Regards,
Joakim Zhang
> Andrew

2021-06-18 17:58:21

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH V2 2/2] net: fec: add ndo_select_queue to fix TX bandwidth fluctuations

Hi Joakim,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url: https://github.com/0day-ci/linux/commits/Joakim-Zhang/net-fec-fix-TX-bandwidth-fluctuations/20210616-221117
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git c7654495916e109f76a67fd3ae68f8fa70ab4faa
config: parisc-randconfig-s031-20210618 (attached as .config)
compiler: hppa-linux-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.3-341-g8af24329-dirty
# https://github.com/0day-ci/linux/commit/a0ef15273c2798319e07a8277161e99b4a22b41e
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Joakim-Zhang/net-fec-fix-TX-bandwidth-fluctuations/20210616-221117
git checkout a0ef15273c2798319e07a8277161e99b4a22b41e
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=parisc

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>


sparse warnings: (new ones prefixed by >>)
>> drivers/net/ethernet/freescale/fec_main.c:3250:30: sparse: sparse: cast to restricted __be16
>> drivers/net/ethernet/freescale/fec_main.c:3250:16: sparse: sparse: restricted __be16 degrades to integer

vim +3250 drivers/net/ethernet/freescale/fec_main.c

3244
3245 static u16 fec_enet_get_raw_vlan_tci(struct sk_buff *skb)
3246 {
3247 struct vlan_ethhdr *vhdr;
3248 unsigned short vlan_TCI = 0;
3249
> 3250 if (skb->protocol == ntohs(ETH_P_ALL)) {
3251 vhdr = (struct vlan_ethhdr *)(skb->data);
3252 vlan_TCI = ntohs(vhdr->h_vlan_TCI);
3253 }
3254
3255 return vlan_TCI;
3256 }
3257

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (2.10 kB)
.config.gz (33.77 kB)
Download all attachments