Re: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS contexts to handle RSS tables
On Wed, May 20, 2020 at 01:16:25PM +0200, Matteo Croce wrote: > On Wed, May 20, 2020 at 1:11 PM Russell King - ARM Linux admin > wrote: > > > > On Tue, May 19, 2020 at 07:05:34PM +0200, Matteo Croce wrote: > > > On Tue, 19 May 2020 12:05:20 +0200 > > > Matteo Croce wrote: > > > > > > Hi, > > > > > > The patch seems to work. I'm generating traffic with random MAC and IP > > > addresses, to have many flows: > > > > > > # tcpdump -tenni eth2 > > > 9a:a9:b1:3a:b1:6b > 00:51:82:11:22:02, ethertype IPv4 (0x0800), length > > > 60: 10.0.0.4.0 > 192.168.0.4.0: UDP, length 12 > > > 9e:92:fd:f8:7f:0a > 00:51:82:11:22:02, ethertype IPv4 (0x0800), length > > > 60: 10.0.0.4.0 > 192.168.0.4.0: UDP, length 12 > > > 66:b7:11:8a:c2:1f > 00:51:82:11:22:02, ethertype IPv4 (0x0800), length > > > 60: 10.0.0.1.0 > 192.168.0.1.0: UDP, length 12 > > > 7a:ba:58:bd:9a:62 > 00:51:82:11:22:02, ethertype IPv4 (0x0800), length > > > 60: 10.0.0.1.0 > 192.168.0.1.0: UDP, length 12 > > > 7e:78:a9:97:70:3a > 00:51:82:11:22:02, ethertype IPv4 (0x0800), length > > > 60: 10.0.0.2.0 > 192.168.0.2.0: UDP, length 12 > > > b2:81:91:34:ce:42 > 00:51:82:11:22:02, ethertype IPv4 (0x0800), length > > > 60: 10.0.0.2.0 > 192.168.0.2.0: UDP, length 12 > > > 2a:05:52:d0:d9:3f > 00:51:82:11:22:02, ethertype IPv4 (0x0800), length > > > 60: 10.0.0.3.0 > 192.168.0.3.0: UDP, length 12 > > > ee:ee:47:35:fa:81 > 00:51:82:11:22:02, ethertype IPv4 (0x0800), length > > > 60: 10.0.0.3.0 > 192.168.0.3.0: UDP, length 12 > > > > > > This is the default rate, with rxhash off: > > > > > > # utraf eth2 > > > tx: 0 bps 0 pps rx: 397.4 Mbps 827.9 Kpps > > > tx: 0 bps 0 pps rx: 396.3 Mbps 825.7 Kpps > > > tx: 0 bps 0 pps rx: 396.6 Mbps 826.3 Kpps > > > tx: 0 bps 0 pps rx: 396.5 Mbps 826.1 Kpps > > > > > > PID USER PR NIVIRTRESSHR S %CPU %MEM TIME+ > > > COMMAND > > > 9 root 20 0 0 0 0 R 99.7 0.0 7:02.58 > > > ksoftirqd/0 > > > 15 root 20 0 0 0 0 S 0.0 0.0 0:00.00 > > > ksoftirqd/1 > > > 20 root 20 0 0 0 0 S 0.0 0.0 2:01.48 > > > ksoftirqd/2 > > > 25 root 20 0 0 0 0 S 0.0 0.0 0:32.86 > > > ksoftirqd/3 > > > > > > and this with rx hashing enabled: > > > > > > # ethtool -K eth2 rxhash on > > > # utraf eth2 > > > tx: 0 bps 0 pps rx: 456.4 Mbps 950.8 Kpps > > > tx: 0 bps 0 pps rx: 458.4 Mbps 955.0 Kpps > > > tx: 0 bps 0 pps rx: 457.6 Mbps 953.3 Kpps > > > tx: 0 bps 0 pps rx: 462.2 Mbps 962.9 Kpps > > > > > > PID USER PR NIVIRTRESSHR S %CPU %MEM TIME+ > > > COMMAND > > > 20 root 20 0 0 0 0 R 0.7 0.0 2:02.34 > > > ksoftirqd/2 > > > 25 root 20 0 0 0 0 S 0.3 0.0 0:33.25 > > > ksoftirqd/3 > > > 9 root 20 0 0 0 0 S 0.0 0.0 7:52.57 > > > ksoftirqd/0 > > > 15 root 20 0 0 0 0 S 0.0 0.0 0:00.00 > > > ksoftirqd/1 > > > > > > > > > The throughput doesn't increase so much, maybe we hit an HW limit of > > > the gigabit port. The interesting thing is how the global CPU usage > > > drops from 25% to 1%. > > > I can't explain this, it could be due to the reduced contention? > > > > Hi Matteo, > > > > Can I take that as a Tested-by ? > > > > Thanks. > > > > -- > > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > > FTTC for 0.8m (est. 1762m) line in suburbia: sync at 13.1Mbps down 424kbps > > up > > > > Tested-by: Matteo Croce > > probably also: > > Reported-by: Matteo Croce Thanks! -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC for 0.8m (est. 1762m) line in suburbia: sync at 13.1Mbps down 424kbps up
Re: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS contexts to handle RSS tables
On Wed, May 20, 2020 at 1:11 PM Russell King - ARM Linux admin wrote: > > On Tue, May 19, 2020 at 07:05:34PM +0200, Matteo Croce wrote: > > On Tue, 19 May 2020 12:05:20 +0200 > > Matteo Croce wrote: > > > > Hi, > > > > The patch seems to work. I'm generating traffic with random MAC and IP > > addresses, to have many flows: > > > > # tcpdump -tenni eth2 > > 9a:a9:b1:3a:b1:6b > 00:51:82:11:22:02, ethertype IPv4 (0x0800), length 60: > > 10.0.0.4.0 > 192.168.0.4.0: UDP, length 12 > > 9e:92:fd:f8:7f:0a > 00:51:82:11:22:02, ethertype IPv4 (0x0800), length 60: > > 10.0.0.4.0 > 192.168.0.4.0: UDP, length 12 > > 66:b7:11:8a:c2:1f > 00:51:82:11:22:02, ethertype IPv4 (0x0800), length 60: > > 10.0.0.1.0 > 192.168.0.1.0: UDP, length 12 > > 7a:ba:58:bd:9a:62 > 00:51:82:11:22:02, ethertype IPv4 (0x0800), length 60: > > 10.0.0.1.0 > 192.168.0.1.0: UDP, length 12 > > 7e:78:a9:97:70:3a > 00:51:82:11:22:02, ethertype IPv4 (0x0800), length 60: > > 10.0.0.2.0 > 192.168.0.2.0: UDP, length 12 > > b2:81:91:34:ce:42 > 00:51:82:11:22:02, ethertype IPv4 (0x0800), length 60: > > 10.0.0.2.0 > 192.168.0.2.0: UDP, length 12 > > 2a:05:52:d0:d9:3f > 00:51:82:11:22:02, ethertype IPv4 (0x0800), length 60: > > 10.0.0.3.0 > 192.168.0.3.0: UDP, length 12 > > ee:ee:47:35:fa:81 > 00:51:82:11:22:02, ethertype IPv4 (0x0800), length 60: > > 10.0.0.3.0 > 192.168.0.3.0: UDP, length 12 > > > > This is the default rate, with rxhash off: > > > > # utraf eth2 > > tx: 0 bps 0 pps rx: 397.4 Mbps 827.9 Kpps > > tx: 0 bps 0 pps rx: 396.3 Mbps 825.7 Kpps > > tx: 0 bps 0 pps rx: 396.6 Mbps 826.3 Kpps > > tx: 0 bps 0 pps rx: 396.5 Mbps 826.1 Kpps > > > > PID USER PR NIVIRTRESSHR S %CPU %MEM TIME+ > > COMMAND > > 9 root 20 0 0 0 0 R 99.7 0.0 7:02.58 > > ksoftirqd/0 > > 15 root 20 0 0 0 0 S 0.0 0.0 0:00.00 > > ksoftirqd/1 > > 20 root 20 0 0 0 0 S 0.0 0.0 2:01.48 > > ksoftirqd/2 > > 25 root 20 0 0 0 0 S 0.0 0.0 0:32.86 > > ksoftirqd/3 > > > > and this with rx hashing enabled: > > > > # ethtool -K eth2 rxhash on > > # utraf eth2 > > tx: 0 bps 0 pps rx: 456.4 Mbps 950.8 Kpps > > tx: 0 bps 0 pps rx: 458.4 Mbps 955.0 Kpps > > tx: 0 bps 0 pps rx: 457.6 Mbps 953.3 Kpps > > tx: 0 bps 0 pps rx: 462.2 Mbps 962.9 Kpps > > > > PID USER PR NIVIRTRESSHR S %CPU %MEM TIME+ > > COMMAND > > 20 root 20 0 0 0 0 R 0.7 0.0 2:02.34 > > ksoftirqd/2 > > 25 root 20 0 0 0 0 S 0.3 0.0 0:33.25 > > ksoftirqd/3 > > 9 root 20 0 0 0 0 S 0.0 0.0 7:52.57 > > ksoftirqd/0 > > 15 root 20 0 0 0 0 S 0.0 0.0 0:00.00 > > ksoftirqd/1 > > > > > > The throughput doesn't increase so much, maybe we hit an HW limit of > > the gigabit port. The interesting thing is how the global CPU usage > > drops from 25% to 1%. > > I can't explain this, it could be due to the reduced contention? > > Hi Matteo, > > Can I take that as a Tested-by ? > > Thanks. > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTC for 0.8m (est. 1762m) line in suburbia: sync at 13.1Mbps down 424kbps up > Tested-by: Matteo Croce probably also: Reported-by: Matteo Croce Thanks, -- Matteo Croce per aspera ad upstream
Re: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS contexts to handle RSS tables
On Tue, May 19, 2020 at 07:05:34PM +0200, Matteo Croce wrote: > On Tue, 19 May 2020 12:05:20 +0200 > Matteo Croce wrote: > > Hi, > > The patch seems to work. I'm generating traffic with random MAC and IP > addresses, to have many flows: > > # tcpdump -tenni eth2 > 9a:a9:b1:3a:b1:6b > 00:51:82:11:22:02, ethertype IPv4 (0x0800), length 60: > 10.0.0.4.0 > 192.168.0.4.0: UDP, length 12 > 9e:92:fd:f8:7f:0a > 00:51:82:11:22:02, ethertype IPv4 (0x0800), length 60: > 10.0.0.4.0 > 192.168.0.4.0: UDP, length 12 > 66:b7:11:8a:c2:1f > 00:51:82:11:22:02, ethertype IPv4 (0x0800), length 60: > 10.0.0.1.0 > 192.168.0.1.0: UDP, length 12 > 7a:ba:58:bd:9a:62 > 00:51:82:11:22:02, ethertype IPv4 (0x0800), length 60: > 10.0.0.1.0 > 192.168.0.1.0: UDP, length 12 > 7e:78:a9:97:70:3a > 00:51:82:11:22:02, ethertype IPv4 (0x0800), length 60: > 10.0.0.2.0 > 192.168.0.2.0: UDP, length 12 > b2:81:91:34:ce:42 > 00:51:82:11:22:02, ethertype IPv4 (0x0800), length 60: > 10.0.0.2.0 > 192.168.0.2.0: UDP, length 12 > 2a:05:52:d0:d9:3f > 00:51:82:11:22:02, ethertype IPv4 (0x0800), length 60: > 10.0.0.3.0 > 192.168.0.3.0: UDP, length 12 > ee:ee:47:35:fa:81 > 00:51:82:11:22:02, ethertype IPv4 (0x0800), length 60: > 10.0.0.3.0 > 192.168.0.3.0: UDP, length 12 > > This is the default rate, with rxhash off: > > # utraf eth2 > tx: 0 bps 0 pps rx: 397.4 Mbps 827.9 Kpps > tx: 0 bps 0 pps rx: 396.3 Mbps 825.7 Kpps > tx: 0 bps 0 pps rx: 396.6 Mbps 826.3 Kpps > tx: 0 bps 0 pps rx: 396.5 Mbps 826.1 Kpps > > PID USER PR NIVIRTRESSHR S %CPU %MEM TIME+ COMMAND > 9 root 20 0 0 0 0 R 99.7 0.0 7:02.58 > ksoftirqd/0 > 15 root 20 0 0 0 0 S 0.0 0.0 0:00.00 > ksoftirqd/1 > 20 root 20 0 0 0 0 S 0.0 0.0 2:01.48 > ksoftirqd/2 > 25 root 20 0 0 0 0 S 0.0 0.0 0:32.86 > ksoftirqd/3 > > and this with rx hashing enabled: > > # ethtool -K eth2 rxhash on > # utraf eth2 > tx: 0 bps 0 pps rx: 456.4 Mbps 950.8 Kpps > tx: 0 bps 0 pps rx: 458.4 Mbps 955.0 Kpps > tx: 0 bps 0 pps rx: 457.6 Mbps 953.3 Kpps > tx: 0 bps 0 pps rx: 462.2 Mbps 962.9 Kpps > > PID USER PR NIVIRTRESSHR S %CPU %MEM TIME+ COMMAND > 20 root 20 0 0 0 0 R 0.7 0.0 2:02.34 > ksoftirqd/2 > 25 root 20 0 0 0 0 S 0.3 0.0 0:33.25 > ksoftirqd/3 > 9 root 20 0 0 0 0 S 0.0 0.0 7:52.57 > ksoftirqd/0 > 15 root 20 0 0 0 0 S 0.0 0.0 0:00.00 > ksoftirqd/1 > > > The throughput doesn't increase so much, maybe we hit an HW limit of > the gigabit port. The interesting thing is how the global CPU usage > drops from 25% to 1%. > I can't explain this, it could be due to the reduced contention? Hi Matteo, Can I take that as a Tested-by ? Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC for 0.8m (est. 1762m) line in suburbia: sync at 13.1Mbps down 424kbps up
Re: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS contexts to handle RSS tables
On Tue, 19 May 2020 12:05:20 +0200 Matteo Croce wrote: > On Tue, May 19, 2020 at 11:54 AM Russell King - ARM Linux admin > wrote: > > > > On Sat, May 09, 2020 at 09:20:50PM +0100, Russell King - ARM Linux > > admin wrote: > > > On Sat, May 09, 2020 at 08:52:46PM +0100, Russell King - ARM > > > Linux admin wrote: > > > > It is highly likely that 895586d5dc32 is responsible for this > > > > breakage. I've been investigating this afternoon, and what I've > > > > found, comparing a kernel without 895586d5dc32 and with > > > > 895586d5dc32 applied is: > > > > > > > > - The table programmed into the hardware via > > > > mvpp22_rss_fill_table() appears to be identical with or without > > > > the commit. > > > > > > > > - When rxhash is enabled on eth2, mvpp2_rss_port_c2_enable() > > > > reports that c2.attr[0] and c2.attr[2] are written back > > > > containing: > > > > > > > >- with 895586d5dc32, failing:0020 4000 > > > >- without 895586d5dc32, working: 0400 4000 > > > > > > > > - When disabling rxhash, c2.attr[0] and c2.attr[2] are written > > > > back as: > > > > > > > >0400 > > > > > > > > The second value represents the MVPP22_CLS_C2_ATTR2_RSS_EN bit, > > > > the first value is the queue number, which comprises two > > > > fields. The high 5 bits are 24:29 and the low three are 21:23 > > > > inclusive. This comes from: > > > > > > > >c2.attr[0] = MVPP22_CLS_C2_ATTR0_QHIGH(qh) | > > > > MVPP22_CLS_C2_ATTR0_QLOW(ql); > > > > #define MVPP22_CLS_C2_ATTR0_QHIGH(qh) (((qh) & 0x1f) > > > > << 24) #define MVPP22_CLS_C2_ATTR0_QLOW(ql)(((ql) & > > > > 0x7) << 21) > > > > > > > > So, the working case gives eth2 a queue id of 4.0, or 32 as per > > > > port->first_rxq, and the non-working case a queue id of 0.1, or > > > > 1. > > > > > > > > The allocation of queue IDs seems to be in mvpp2_port_probe(): > > > > > > > > if (priv->hw_version == MVPP21) > > > > port->first_rxq = port->id * port->nrxqs; > > > > else > > > > port->first_rxq = port->id * > > > > priv->max_port_rxqs; > > > > > > > > Where: > > > > > > > > if (priv->hw_version == MVPP21) > > > > priv->max_port_rxqs = 8; > > > > else > > > > priv->max_port_rxqs = 32; > > > > > > > > Making the port 0 (eth0 / eth1) have port->first_rxq = 0, and > > > > port 1 (eth2) be 32. It seems the idea is that the first 32 > > > > queues belong to port 0, the second 32 queues belong to port 1, > > > > etc. > > > > > > > > mvpp2_rss_port_c2_enable() gets the queue number from it's > > > > parameter, 'ctx', which comes from mvpp22_rss_ctx(port, 0). > > > > This returns port->rss_ctx[0]. > > > > > > > > mvpp22_rss_context_create() is responsible for allocating that, > > > > which it does by looking for an unallocated priv->rss_tables[] > > > > pointer. This table is shared amongst all ports on the CP > > > > silicon. > > > > > > > > When we write the tables in mvpp22_rss_fill_table(), the RSS > > > > table entry is defined by: > > > > > > > > u32 sel = MVPP22_RSS_INDEX_TABLE(rss_ctx) | > > > > MVPP22_RSS_INDEX_TABLE_ENTRY(i); > > > > > > > > where rss_ctx is the context ID (queue number) and i is the > > > > index in the table. > > > > > > > > #define MVPP22_RSS_INDEX_TABLE_ENTRY(idx) (idx) > > > > #define MVPP22_RSS_INDEX_TABLE(idx) ((idx) << 8) > > > > #define MVPP22_RSS_INDEX_QUEUE(idx) ((idx) << 16) > > > > > > > > If we look at what is written: > > > > > > > > - The first table to be written has "sel" values of > > > > ..001f, containing values 0..3. This appears to be > > > > for eth1. This is table 0, RX queue number 0. > > > > - The second table has "sel" values of 0100..011f, and > > > > appears to be for eth2. These contain values 0x20..0x23. This > > > > is table 1, RX queue number 0. > > > > - The third table has "sel" values of 0200..021f, and > > > > appears to be for eth3. These contain values 0x40..0x43. This > > > > is table 2, RX queue number 0. > > > > > > > > Okay, so how do queue numbers translate to the RSS table? > > > > There is another table - the RXQ2RSS table, indexed by the > > > > MVPP22_RSS_INDEX_QUEUE field of MVPP22_RSS_INDEX and accessed > > > > through the MVPP22_RXQ2RSS_TABLE register. Before > > > > 895586d5dc32, it was: > > > > > > > >mvpp2_write(priv, MVPP22_RSS_INDEX, > > > >MVPP22_RSS_INDEX_QUEUE(port->first_rxq)); > > > >mvpp2_write(priv, MVPP22_RXQ2RSS_TABLE, > > > >MVPP22_RSS_TABLE_POINTER(port->id)); > > > > > > > > and after: > > > > > > > >mvpp2_write(priv, MVPP22_RSS_INDEX, > > > > MVPP22_RSS_INDEX_QUEUE(ctx)); mvpp2_write(priv, > > > > MVPP22_RXQ2RSS_TABLE, MVPP22_RSS_TABLE_POINTER(ctx)); > > > > > > > > So, before the commit, for eth2, that would've contained '32' > > > >
Re: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS contexts to handle RSS tables
On Tue, May 19, 2020 at 11:54 AM Russell King - ARM Linux admin wrote: > > On Sat, May 09, 2020 at 09:20:50PM +0100, Russell King - ARM Linux admin > wrote: > > On Sat, May 09, 2020 at 08:52:46PM +0100, Russell King - ARM Linux admin > > wrote: > > > It is highly likely that 895586d5dc32 is responsible for this breakage. > > > I've been investigating this afternoon, and what I've found, comparing > > > a kernel without 895586d5dc32 and with 895586d5dc32 applied is: > > > > > > - The table programmed into the hardware via mvpp22_rss_fill_table() > > > appears to be identical with or without the commit. > > > > > > - When rxhash is enabled on eth2, mvpp2_rss_port_c2_enable() reports > > > that c2.attr[0] and c2.attr[2] are written back containing: > > > > > >- with 895586d5dc32, failing:0020 4000 > > >- without 895586d5dc32, working: 0400 4000 > > > > > > - When disabling rxhash, c2.attr[0] and c2.attr[2] are written back as: > > > > > >0400 > > > > > > The second value represents the MVPP22_CLS_C2_ATTR2_RSS_EN bit, the > > > first value is the queue number, which comprises two fields. The high > > > 5 bits are 24:29 and the low three are 21:23 inclusive. This comes > > > from: > > > > > >c2.attr[0] = MVPP22_CLS_C2_ATTR0_QHIGH(qh) | > > > MVPP22_CLS_C2_ATTR0_QLOW(ql); > > > #define MVPP22_CLS_C2_ATTR0_QHIGH(qh) (((qh) & 0x1f) << 24) > > > #define MVPP22_CLS_C2_ATTR0_QLOW(ql)(((ql) & 0x7) << 21) > > > > > > So, the working case gives eth2 a queue id of 4.0, or 32 as per > > > port->first_rxq, and the non-working case a queue id of 0.1, or 1. > > > > > > The allocation of queue IDs seems to be in mvpp2_port_probe(): > > > > > > if (priv->hw_version == MVPP21) > > > port->first_rxq = port->id * port->nrxqs; > > > else > > > port->first_rxq = port->id * priv->max_port_rxqs; > > > > > > Where: > > > > > > if (priv->hw_version == MVPP21) > > > priv->max_port_rxqs = 8; > > > else > > > priv->max_port_rxqs = 32; > > > > > > Making the port 0 (eth0 / eth1) have port->first_rxq = 0, and port 1 > > > (eth2) be 32. It seems the idea is that the first 32 queues belong to > > > port 0, the second 32 queues belong to port 1, etc. > > > > > > mvpp2_rss_port_c2_enable() gets the queue number from it's parameter, > > > 'ctx', which comes from mvpp22_rss_ctx(port, 0). This returns > > > port->rss_ctx[0]. > > > > > > mvpp22_rss_context_create() is responsible for allocating that, which > > > it does by looking for an unallocated priv->rss_tables[] pointer. This > > > table is shared amongst all ports on the CP silicon. > > > > > > When we write the tables in mvpp22_rss_fill_table(), the RSS table > > > entry is defined by: > > > > > > u32 sel = MVPP22_RSS_INDEX_TABLE(rss_ctx) | > > > MVPP22_RSS_INDEX_TABLE_ENTRY(i); > > > > > > where rss_ctx is the context ID (queue number) and i is the index in > > > the table. > > > > > > #define MVPP22_RSS_INDEX_TABLE_ENTRY(idx) (idx) > > > #define MVPP22_RSS_INDEX_TABLE(idx) ((idx) << 8) > > > #define MVPP22_RSS_INDEX_QUEUE(idx) ((idx) << 16) > > > > > > If we look at what is written: > > > > > > - The first table to be written has "sel" values of ..001f, > > > containing values 0..3. This appears to be for eth1. This is table 0, > > > RX queue number 0. > > > - The second table has "sel" values of 0100..011f, and appears > > > to be for eth2. These contain values 0x20..0x23. This is table 1, > > > RX queue number 0. > > > - The third table has "sel" values of 0200..021f, and appears > > > to be for eth3. These contain values 0x40..0x43. This is table 2, > > > RX queue number 0. > > > > > > Okay, so how do queue numbers translate to the RSS table? There is > > > another table - the RXQ2RSS table, indexed by the MVPP22_RSS_INDEX_QUEUE > > > field of MVPP22_RSS_INDEX and accessed through the MVPP22_RXQ2RSS_TABLE > > > register. Before 895586d5dc32, it was: > > > > > >mvpp2_write(priv, MVPP22_RSS_INDEX, > > >MVPP22_RSS_INDEX_QUEUE(port->first_rxq)); > > >mvpp2_write(priv, MVPP22_RXQ2RSS_TABLE, > > >MVPP22_RSS_TABLE_POINTER(port->id)); > > > > > > and after: > > > > > >mvpp2_write(priv, MVPP22_RSS_INDEX, MVPP22_RSS_INDEX_QUEUE(ctx)); > > >mvpp2_write(priv, MVPP22_RXQ2RSS_TABLE, > > > MVPP22_RSS_TABLE_POINTER(ctx)); > > > > > > So, before the commit, for eth2, that would've contained '32' for the > > > index and '1' for the table pointer - mapping queue 32 to table 1. > > > Remember that this is queue-high.queue-low of 4.0. > > > > > > After the commit, we appear to map queue 1 to table 1. That again > > > looks fine on the face of it. > > > > > > Section 9.3.1 of the A8040 manual seems indicate the reason
Re: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS contexts to handle RSS tables
On Sat, May 09, 2020 at 09:20:50PM +0100, Russell King - ARM Linux admin wrote: > On Sat, May 09, 2020 at 08:52:46PM +0100, Russell King - ARM Linux admin > wrote: > > It is highly likely that 895586d5dc32 is responsible for this breakage. > > I've been investigating this afternoon, and what I've found, comparing > > a kernel without 895586d5dc32 and with 895586d5dc32 applied is: > > > > - The table programmed into the hardware via mvpp22_rss_fill_table() > > appears to be identical with or without the commit. > > > > - When rxhash is enabled on eth2, mvpp2_rss_port_c2_enable() reports > > that c2.attr[0] and c2.attr[2] are written back containing: > > > >- with 895586d5dc32, failing:0020 4000 > >- without 895586d5dc32, working: 0400 4000 > > > > - When disabling rxhash, c2.attr[0] and c2.attr[2] are written back as: > > > >0400 > > > > The second value represents the MVPP22_CLS_C2_ATTR2_RSS_EN bit, the > > first value is the queue number, which comprises two fields. The high > > 5 bits are 24:29 and the low three are 21:23 inclusive. This comes > > from: > > > >c2.attr[0] = MVPP22_CLS_C2_ATTR0_QHIGH(qh) | > > MVPP22_CLS_C2_ATTR0_QLOW(ql); > > #define MVPP22_CLS_C2_ATTR0_QHIGH(qh) (((qh) & 0x1f) << 24) > > #define MVPP22_CLS_C2_ATTR0_QLOW(ql)(((ql) & 0x7) << 21) > > > > So, the working case gives eth2 a queue id of 4.0, or 32 as per > > port->first_rxq, and the non-working case a queue id of 0.1, or 1. > > > > The allocation of queue IDs seems to be in mvpp2_port_probe(): > > > > if (priv->hw_version == MVPP21) > > port->first_rxq = port->id * port->nrxqs; > > else > > port->first_rxq = port->id * priv->max_port_rxqs; > > > > Where: > > > > if (priv->hw_version == MVPP21) > > priv->max_port_rxqs = 8; > > else > > priv->max_port_rxqs = 32; > > > > Making the port 0 (eth0 / eth1) have port->first_rxq = 0, and port 1 > > (eth2) be 32. It seems the idea is that the first 32 queues belong to > > port 0, the second 32 queues belong to port 1, etc. > > > > mvpp2_rss_port_c2_enable() gets the queue number from it's parameter, > > 'ctx', which comes from mvpp22_rss_ctx(port, 0). This returns > > port->rss_ctx[0]. > > > > mvpp22_rss_context_create() is responsible for allocating that, which > > it does by looking for an unallocated priv->rss_tables[] pointer. This > > table is shared amongst all ports on the CP silicon. > > > > When we write the tables in mvpp22_rss_fill_table(), the RSS table > > entry is defined by: > > > > u32 sel = MVPP22_RSS_INDEX_TABLE(rss_ctx) | > > MVPP22_RSS_INDEX_TABLE_ENTRY(i); > > > > where rss_ctx is the context ID (queue number) and i is the index in > > the table. > > > > #define MVPP22_RSS_INDEX_TABLE_ENTRY(idx) (idx) > > #define MVPP22_RSS_INDEX_TABLE(idx) ((idx) << 8) > > #define MVPP22_RSS_INDEX_QUEUE(idx) ((idx) << 16) > > > > If we look at what is written: > > > > - The first table to be written has "sel" values of ..001f, > > containing values 0..3. This appears to be for eth1. This is table 0, > > RX queue number 0. > > - The second table has "sel" values of 0100..011f, and appears > > to be for eth2. These contain values 0x20..0x23. This is table 1, > > RX queue number 0. > > - The third table has "sel" values of 0200..021f, and appears > > to be for eth3. These contain values 0x40..0x43. This is table 2, > > RX queue number 0. > > > > Okay, so how do queue numbers translate to the RSS table? There is > > another table - the RXQ2RSS table, indexed by the MVPP22_RSS_INDEX_QUEUE > > field of MVPP22_RSS_INDEX and accessed through the MVPP22_RXQ2RSS_TABLE > > register. Before 895586d5dc32, it was: > > > >mvpp2_write(priv, MVPP22_RSS_INDEX, > >MVPP22_RSS_INDEX_QUEUE(port->first_rxq)); > >mvpp2_write(priv, MVPP22_RXQ2RSS_TABLE, > >MVPP22_RSS_TABLE_POINTER(port->id)); > > > > and after: > > > >mvpp2_write(priv, MVPP22_RSS_INDEX, MVPP22_RSS_INDEX_QUEUE(ctx)); > >mvpp2_write(priv, MVPP22_RXQ2RSS_TABLE, > > MVPP22_RSS_TABLE_POINTER(ctx)); > > > > So, before the commit, for eth2, that would've contained '32' for the > > index and '1' for the table pointer - mapping queue 32 to table 1. > > Remember that this is queue-high.queue-low of 4.0. > > > > After the commit, we appear to map queue 1 to table 1. That again > > looks fine on the face of it. > > > > Section 9.3.1 of the A8040 manual seems indicate the reason that the > > queue number is separated. queue-low seems to always come from the > > classifier, whereas queue-high can be from the ingress physical port > > number or the classifier depending on the MVPP2_CLS_SWFWD_PCTRL_REG. > > > > We set the port bit in
Re: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS contexts to handle RSS tables
On Sat, May 09, 2020 at 08:52:46PM +0100, Russell King - ARM Linux admin wrote:
> On Sat, May 09, 2020 at 03:14:05PM +0200, Matteo Croce wrote:
> > Hi,
> >
> > When git bisect pointed to 895586d5dc32 ("net: mvpp2: cls: Use RSS
> > contexts to handle RSS tables"), which was merged
> > almost an year after d33ec4525007 ("net: mvpp2: add an RSS
> > classification step for each flow"), so I assume that between these
> > two commits either the feature was working or it was disable and we
> > didn't notice
> >
> > Without knowing what was happening, which commit should my Fixes tag point
> > to?
>
> It is highly likely that 895586d5dc32 is responsible for this breakage.
> I've been investigating this afternoon, and what I've found, comparing
> a kernel without 895586d5dc32 and with 895586d5dc32 applied is:
>
> - The table programmed into the hardware via mvpp22_rss_fill_table()
> appears to be identical with or without the commit.
>
> - When rxhash is enabled on eth2, mvpp2_rss_port_c2_enable() reports
> that c2.attr[0] and c2.attr[2] are written back containing:
>
>- with 895586d5dc32, failing:0020 4000
>- without 895586d5dc32, working: 0400 4000
>
> - When disabling rxhash, c2.attr[0] and c2.attr[2] are written back as:
>
>0400
>
> The second value represents the MVPP22_CLS_C2_ATTR2_RSS_EN bit, the
> first value is the queue number, which comprises two fields. The high
> 5 bits are 24:29 and the low three are 21:23 inclusive. This comes
> from:
>
>c2.attr[0] = MVPP22_CLS_C2_ATTR0_QHIGH(qh) |
> MVPP22_CLS_C2_ATTR0_QLOW(ql);
> #define MVPP22_CLS_C2_ATTR0_QHIGH(qh) (((qh) & 0x1f) << 24)
> #define MVPP22_CLS_C2_ATTR0_QLOW(ql)(((ql) & 0x7) << 21)
>
> So, the working case gives eth2 a queue id of 4.0, or 32 as per
> port->first_rxq, and the non-working case a queue id of 0.1, or 1.
>
> The allocation of queue IDs seems to be in mvpp2_port_probe():
>
> if (priv->hw_version == MVPP21)
> port->first_rxq = port->id * port->nrxqs;
> else
> port->first_rxq = port->id * priv->max_port_rxqs;
>
> Where:
>
> if (priv->hw_version == MVPP21)
> priv->max_port_rxqs = 8;
> else
> priv->max_port_rxqs = 32;
>
> Making the port 0 (eth0 / eth1) have port->first_rxq = 0, and port 1
> (eth2) be 32. It seems the idea is that the first 32 queues belong to
> port 0, the second 32 queues belong to port 1, etc.
>
> mvpp2_rss_port_c2_enable() gets the queue number from it's parameter,
> 'ctx', which comes from mvpp22_rss_ctx(port, 0). This returns
> port->rss_ctx[0].
>
> mvpp22_rss_context_create() is responsible for allocating that, which
> it does by looking for an unallocated priv->rss_tables[] pointer. This
> table is shared amongst all ports on the CP silicon.
>
> When we write the tables in mvpp22_rss_fill_table(), the RSS table
> entry is defined by:
>
> u32 sel = MVPP22_RSS_INDEX_TABLE(rss_ctx) |
> MVPP22_RSS_INDEX_TABLE_ENTRY(i);
>
> where rss_ctx is the context ID (queue number) and i is the index in
> the table.
>
> #define MVPP22_RSS_INDEX_TABLE_ENTRY(idx) (idx)
> #define MVPP22_RSS_INDEX_TABLE(idx) ((idx) << 8)
> #define MVPP22_RSS_INDEX_QUEUE(idx) ((idx) << 16)
>
> If we look at what is written:
>
> - The first table to be written has "sel" values of ..001f,
> containing values 0..3. This appears to be for eth1. This is table 0,
> RX queue number 0.
> - The second table has "sel" values of 0100..011f, and appears
> to be for eth2. These contain values 0x20..0x23. This is table 1,
> RX queue number 0.
> - The third table has "sel" values of 0200..021f, and appears
> to be for eth3. These contain values 0x40..0x43. This is table 2,
> RX queue number 0.
>
> Okay, so how do queue numbers translate to the RSS table? There is
> another table - the RXQ2RSS table, indexed by the MVPP22_RSS_INDEX_QUEUE
> field of MVPP22_RSS_INDEX and accessed through the MVPP22_RXQ2RSS_TABLE
> register. Before 895586d5dc32, it was:
>
>mvpp2_write(priv, MVPP22_RSS_INDEX,
>MVPP22_RSS_INDEX_QUEUE(port->first_rxq));
>mvpp2_write(priv, MVPP22_RXQ2RSS_TABLE,
>MVPP22_RSS_TABLE_POINTER(port->id));
>
> and after:
>
>mvpp2_write(priv, MVPP22_RSS_INDEX, MVPP22_RSS_INDEX_QUEUE(ctx));
>mvpp2_write(priv, MVPP22_RXQ2RSS_TABLE, MVPP22_RSS_TABLE_POINTER(ctx));
>
> So, before the commit, for eth2, that would've contained '32' for the
> index and '1' for the table pointer - mapping queue 32 to table 1.
> Remember that this is queue-high.queue-low of 4.0.
>
> After the commit, we appear to map queue 1 to table 1. That again
> looks fine on the face of it.
>
> Section 9.3.1 of the A8040 manual seems indicate the reason that the
> queue number is separated.
Re: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS contexts to handle RSS tables
On Sat, May 09, 2020 at 03:14:05PM +0200, Matteo Croce wrote:
> Hi,
>
> When git bisect pointed to 895586d5dc32 ("net: mvpp2: cls: Use RSS
> contexts to handle RSS tables"), which was merged
> almost an year after d33ec4525007 ("net: mvpp2: add an RSS
> classification step for each flow"), so I assume that between these
> two commits either the feature was working or it was disable and we
> didn't notice
>
> Without knowing what was happening, which commit should my Fixes tag point to?
It is highly likely that 895586d5dc32 is responsible for this breakage.
I've been investigating this afternoon, and what I've found, comparing
a kernel without 895586d5dc32 and with 895586d5dc32 applied is:
- The table programmed into the hardware via mvpp22_rss_fill_table()
appears to be identical with or without the commit.
- When rxhash is enabled on eth2, mvpp2_rss_port_c2_enable() reports
that c2.attr[0] and c2.attr[2] are written back containing:
- with 895586d5dc32, failing:0020 4000
- without 895586d5dc32, working: 0400 4000
- When disabling rxhash, c2.attr[0] and c2.attr[2] are written back as:
0400
The second value represents the MVPP22_CLS_C2_ATTR2_RSS_EN bit, the
first value is the queue number, which comprises two fields. The high
5 bits are 24:29 and the low three are 21:23 inclusive. This comes
from:
c2.attr[0] = MVPP22_CLS_C2_ATTR0_QHIGH(qh) |
MVPP22_CLS_C2_ATTR0_QLOW(ql);
#define MVPP22_CLS_C2_ATTR0_QHIGH(qh) (((qh) & 0x1f) << 24)
#define MVPP22_CLS_C2_ATTR0_QLOW(ql)(((ql) & 0x7) << 21)
So, the working case gives eth2 a queue id of 4.0, or 32 as per
port->first_rxq, and the non-working case a queue id of 0.1, or 1.
The allocation of queue IDs seems to be in mvpp2_port_probe():
if (priv->hw_version == MVPP21)
port->first_rxq = port->id * port->nrxqs;
else
port->first_rxq = port->id * priv->max_port_rxqs;
Where:
if (priv->hw_version == MVPP21)
priv->max_port_rxqs = 8;
else
priv->max_port_rxqs = 32;
Making the port 0 (eth0 / eth1) have port->first_rxq = 0, and port 1
(eth2) be 32. It seems the idea is that the first 32 queues belong to
port 0, the second 32 queues belong to port 1, etc.
mvpp2_rss_port_c2_enable() gets the queue number from it's parameter,
'ctx', which comes from mvpp22_rss_ctx(port, 0). This returns
port->rss_ctx[0].
mvpp22_rss_context_create() is responsible for allocating that, which
it does by looking for an unallocated priv->rss_tables[] pointer. This
table is shared amongst all ports on the CP silicon.
When we write the tables in mvpp22_rss_fill_table(), the RSS table
entry is defined by:
u32 sel = MVPP22_RSS_INDEX_TABLE(rss_ctx) |
MVPP22_RSS_INDEX_TABLE_ENTRY(i);
where rss_ctx is the context ID (queue number) and i is the index in
the table.
#define MVPP22_RSS_INDEX_TABLE_ENTRY(idx) (idx)
#define MVPP22_RSS_INDEX_TABLE(idx) ((idx) << 8)
#define MVPP22_RSS_INDEX_QUEUE(idx) ((idx) << 16)
If we look at what is written:
- The first table to be written has "sel" values of ..001f,
containing values 0..3. This appears to be for eth1. This is table 0,
RX queue number 0.
- The second table has "sel" values of 0100..011f, and appears
to be for eth2. These contain values 0x20..0x23. This is table 1,
RX queue number 0.
- The third table has "sel" values of 0200..021f, and appears
to be for eth3. These contain values 0x40..0x43. This is table 2,
RX queue number 0.
Okay, so how do queue numbers translate to the RSS table? There is
another table - the RXQ2RSS table, indexed by the MVPP22_RSS_INDEX_QUEUE
field of MVPP22_RSS_INDEX and accessed through the MVPP22_RXQ2RSS_TABLE
register. Before 895586d5dc32, it was:
mvpp2_write(priv, MVPP22_RSS_INDEX,
MVPP22_RSS_INDEX_QUEUE(port->first_rxq));
mvpp2_write(priv, MVPP22_RXQ2RSS_TABLE,
MVPP22_RSS_TABLE_POINTER(port->id));
and after:
mvpp2_write(priv, MVPP22_RSS_INDEX, MVPP22_RSS_INDEX_QUEUE(ctx));
mvpp2_write(priv, MVPP22_RXQ2RSS_TABLE, MVPP22_RSS_TABLE_POINTER(ctx));
So, before the commit, for eth2, that would've contained '32' for the
index and '1' for the table pointer - mapping queue 32 to table 1.
Remember that this is queue-high.queue-low of 4.0.
After the commit, we appear to map queue 1 to table 1. That again
looks fine on the face of it.
Section 9.3.1 of the A8040 manual seems indicate the reason that the
queue number is separated. queue-low seems to always come from the
classifier, whereas queue-high can be from the ingress physical port
number or the classifier depending on the MVPP2_CLS_SWFWD_PCTRL_REG.
We set the port bit in MVPP2_CLS_SWFWD_PCTRL_REG, meaning that queue-high
comes from the MVPP2_CLS_SWFWD_P2HQ_REG() register... and this
Re: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS contexts to handle RSS tables
On Sat, May 9, 2020 at 4:49 PM Russell King - ARM Linux admin
wrote:
>
> On Sat, May 09, 2020 at 02:51:05PM +0100, Russell King - ARM Linux admin
> wrote:
> > On Sat, May 09, 2020 at 03:14:05PM +0200, Matteo Croce wrote:
> > > On Sat, May 9, 2020 at 1:45 PM Russell King - ARM Linux admin
> > > wrote:
> > > >
> > > > On Sat, May 09, 2020 at 11:15:58AM +, Stefan Chulski wrote:
> > > > >
> > > > >
> > > > > > -Original Message-
> > > > > > From: Matteo Croce
> > > > > > Sent: Saturday, May 9, 2020 3:13 AM
> > > > > > To: David S . Miller
> > > > > > Cc: Maxime Chevallier ; netdev
> > > > > > ; LKML ;
> > > > > > Antoine
> > > > > > Tenart ; Thomas Petazzoni
> > > > > > ; gregory.clem...@bootlin.com;
> > > > > > miquel.ray...@bootlin.com; Nadav Haklai ; Stefan
> > > > > > Chulski ; Marcin Wojtas ;
> > > > > > Linux
> > > > > > ARM ; Russell King - ARM
> > > > > > Linux admin
> > > > > >
> > > > > > Subject: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS
> > > > > > contexts to
> > > > > > handle RSS tables
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > What do you think about temporarily disabling it like this?
> > > > > >
> > > > > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > > > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > > > > @@ -5775,7 +5775,8 @@ static int mvpp2_port_probe(struct
> > > > > > platform_device
> > > > > > *pdev,
> > > > > > NETIF_F_HW_VLAN_CTAG_FILTER;
> > > > > >
> > > > > > if (mvpp22_rss_is_supported()) {
> > > > > > - dev->hw_features |= NETIF_F_RXHASH;
> > > > > > + if (port->phy_interface != PHY_INTERFACE_MODE_SGMII)
> > > > > > + dev->hw_features |= NETIF_F_RXHASH;
> > > > > > dev->features |= NETIF_F_NTUPLE;
> > > > > > }
> > > > > >
> > > > > >
> > > > > > David, is this "workaround" too bad to get accepted?
> > > > >
> > > > > Not sure that RSS related to physical interface(SGMII), better just
> > > > > remove NETIF_F_RXHASH as "workaround".
> > > >
> > > > Hmm, I'm not sure this is the right way forward. This patch has the
> > > > effect of disabling:
> > > >
> > > > d33ec4525007 ("net: mvpp2: add an RSS classification step for each
> > > > flow")
> > > >
> > > > but the commit you're pointing at which caused the regression is:
> > > >
> > > > 895586d5dc32 ("net: mvpp2: cls: Use RSS contexts to handle RSS tables")
> > > >
> > > >
> > >
> > > Hi,
> > >
> > > When git bisect pointed to 895586d5dc32 ("net: mvpp2: cls: Use RSS
> > > contexts to handle RSS tables"), which was merged
> > > almost an year after d33ec4525007 ("net: mvpp2: add an RSS
> > > classification step for each flow"), so I assume that between these
> > > two commits either the feature was working or it was disable and we
> > > didn't notice
> > >
> > > Without knowing what was happening, which commit should my Fixes tag
> > > point to?
> >
> > Let me make sure that I get this clear:
> >
> > - Prior to 895586d5dc32, you can turn on and off rxhash without issue
> > on any port.
> > - After 895586d5dc32, turning rxhash on eth2 prevents reception.
> >
> > Prior to 895586d5dc32, with rxhash on, it looks like hashing using
> > CRC32 is supported but only one context. So, if it's possible to
> > enable rxhash on any port on the mcbin without 895586d5dc32, and the
> > port continues to work, I'd say the bug was introduced by
> > 895586d5dc32.
> >
> > Of course, that would be reinforced if there was a measurable
> > difference in performance due to rxhash on each port.
>
> I've just run this test, but I can detect no difference in performance
> with or without 895586d5dc32 on eth0 or eth2 on the mcbin (a
Re: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS contexts to handle RSS tables
On Sat, May 09, 2020 at 02:51:05PM +0100, Russell King - ARM Linux admin wrote:
> On Sat, May 09, 2020 at 03:14:05PM +0200, Matteo Croce wrote:
> > On Sat, May 9, 2020 at 1:45 PM Russell King - ARM Linux admin
> > wrote:
> > >
> > > On Sat, May 09, 2020 at 11:15:58AM +, Stefan Chulski wrote:
> > > >
> > > >
> > > > > -Original Message-
> > > > > From: Matteo Croce
> > > > > Sent: Saturday, May 9, 2020 3:13 AM
> > > > > To: David S . Miller
> > > > > Cc: Maxime Chevallier ; netdev
> > > > > ; LKML ; Antoine
> > > > > Tenart ; Thomas Petazzoni
> > > > > ; gregory.clem...@bootlin.com;
> > > > > miquel.ray...@bootlin.com; Nadav Haklai ; Stefan
> > > > > Chulski ; Marcin Wojtas ;
> > > > > Linux
> > > > > ARM ; Russell King - ARM Linux
> > > > > admin
> > > > >
> > > > > Subject: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS
> > > > > contexts to
> > > > > handle RSS tables
> > > > >
> > > > > Hi,
> > > > >
> > > > > What do you think about temporarily disabling it like this?
> > > > >
> > > > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > > > @@ -5775,7 +5775,8 @@ static int mvpp2_port_probe(struct
> > > > > platform_device
> > > > > *pdev,
> > > > > NETIF_F_HW_VLAN_CTAG_FILTER;
> > > > >
> > > > > if (mvpp22_rss_is_supported()) {
> > > > > - dev->hw_features |= NETIF_F_RXHASH;
> > > > > + if (port->phy_interface != PHY_INTERFACE_MODE_SGMII)
> > > > > + dev->hw_features |= NETIF_F_RXHASH;
> > > > > dev->features |= NETIF_F_NTUPLE;
> > > > > }
> > > > >
> > > > >
> > > > > David, is this "workaround" too bad to get accepted?
> > > >
> > > > Not sure that RSS related to physical interface(SGMII), better just
> > > > remove NETIF_F_RXHASH as "workaround".
> > >
> > > Hmm, I'm not sure this is the right way forward. This patch has the
> > > effect of disabling:
> > >
> > > d33ec4525007 ("net: mvpp2: add an RSS classification step for each flow")
> > >
> > > but the commit you're pointing at which caused the regression is:
> > >
> > > 895586d5dc32 ("net: mvpp2: cls: Use RSS contexts to handle RSS tables")
> > >
> > >
> >
> > Hi,
> >
> > When git bisect pointed to 895586d5dc32 ("net: mvpp2: cls: Use RSS
> > contexts to handle RSS tables"), which was merged
> > almost an year after d33ec4525007 ("net: mvpp2: add an RSS
> > classification step for each flow"), so I assume that between these
> > two commits either the feature was working or it was disable and we
> > didn't notice
> >
> > Without knowing what was happening, which commit should my Fixes tag point
> > to?
>
> Let me make sure that I get this clear:
>
> - Prior to 895586d5dc32, you can turn on and off rxhash without issue
> on any port.
> - After 895586d5dc32, turning rxhash on eth2 prevents reception.
>
> Prior to 895586d5dc32, with rxhash on, it looks like hashing using
> CRC32 is supported but only one context. So, if it's possible to
> enable rxhash on any port on the mcbin without 895586d5dc32, and the
> port continues to work, I'd say the bug was introduced by
> 895586d5dc32.
>
> Of course, that would be reinforced if there was a measurable
> difference in performance due to rxhash on each port.
I've just run this test, but I can detect no difference in performance
with or without 895586d5dc32 on eth0 or eth2 on the mcbin (apart from
eth2 stopping working with 895586d5dc32 applied.) I tested this by
reverting almost all changes to the mvpp2 driver between 5.6 and that
commit.
That's not too surprising; I'm using my cex7 platform with the Mellanox
card in for one end of the 10G link, and that platform doesn't seem to
be able to saturdate a 10G link - it only seems to manage around 4Gbps.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
Re: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS contexts to handle RSS tables
On Sat, May 09, 2020 at 03:14:05PM +0200, Matteo Croce wrote:
> On Sat, May 9, 2020 at 1:45 PM Russell King - ARM Linux admin
> wrote:
> >
> > On Sat, May 09, 2020 at 11:15:58AM +, Stefan Chulski wrote:
> > >
> > >
> > > > -Original Message-
> > > > From: Matteo Croce
> > > > Sent: Saturday, May 9, 2020 3:13 AM
> > > > To: David S . Miller
> > > > Cc: Maxime Chevallier ; netdev
> > > > ; LKML ; Antoine
> > > > Tenart ; Thomas Petazzoni
> > > > ; gregory.clem...@bootlin.com;
> > > > miquel.ray...@bootlin.com; Nadav Haklai ; Stefan
> > > > Chulski ; Marcin Wojtas ; Linux
> > > > ARM ; Russell King - ARM Linux
> > > > admin
> > > >
> > > > Subject: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS
> > > > contexts to
> > > > handle RSS tables
> > > >
> > > > Hi,
> > > >
> > > > What do you think about temporarily disabling it like this?
> > > >
> > > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > > @@ -5775,7 +5775,8 @@ static int mvpp2_port_probe(struct platform_device
> > > > *pdev,
> > > > NETIF_F_HW_VLAN_CTAG_FILTER;
> > > >
> > > > if (mvpp22_rss_is_supported()) {
> > > > - dev->hw_features |= NETIF_F_RXHASH;
> > > > + if (port->phy_interface != PHY_INTERFACE_MODE_SGMII)
> > > > + dev->hw_features |= NETIF_F_RXHASH;
> > > > dev->features |= NETIF_F_NTUPLE;
> > > > }
> > > >
> > > >
> > > > David, is this "workaround" too bad to get accepted?
> > >
> > > Not sure that RSS related to physical interface(SGMII), better just
> > > remove NETIF_F_RXHASH as "workaround".
> >
> > Hmm, I'm not sure this is the right way forward. This patch has the
> > effect of disabling:
> >
> > d33ec4525007 ("net: mvpp2: add an RSS classification step for each flow")
> >
> > but the commit you're pointing at which caused the regression is:
> >
> > 895586d5dc32 ("net: mvpp2: cls: Use RSS contexts to handle RSS tables")
> >
> >
>
> Hi,
>
> When git bisect pointed to 895586d5dc32 ("net: mvpp2: cls: Use RSS
> contexts to handle RSS tables"), which was merged
> almost an year after d33ec4525007 ("net: mvpp2: add an RSS
> classification step for each flow"), so I assume that between these
> two commits either the feature was working or it was disable and we
> didn't notice
>
> Without knowing what was happening, which commit should my Fixes tag point to?
Let me make sure that I get this clear:
- Prior to 895586d5dc32, you can turn on and off rxhash without issue
on any port.
- After 895586d5dc32, turning rxhash on eth2 prevents reception.
Prior to 895586d5dc32, with rxhash on, it looks like hashing using
CRC32 is supported but only one context. So, if it's possible to
enable rxhash on any port on the mcbin without 895586d5dc32, and the
port continues to work, I'd say the bug was introduced by
895586d5dc32.
Of course, that would be reinforced if there was a measurable
difference in performance due to rxhash on each port.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
Re: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS contexts to handle RSS tables
On Sat, May 09, 2020 at 12:31:21PM +, Stefan Chulski wrote: > > -Original Message- > > From: Matteo Croce > > Sent: Saturday, May 9, 2020 3:16 PM > > To: Stefan Chulski > > Cc: David S . Miller ; Maxime Chevallier > > ; netdev ; LKML > > ; Antoine Tenart > > ; Thomas Petazzoni > > ; gregory.clem...@bootlin.com; > > miquel.ray...@bootlin.com; Nadav Haklai ; Marcin > > Wojtas ; Linux ARM > ker...@lists.infradead.org>; Russell King - ARM Linux admin > > > > Subject: Re: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS > > contexts to > > handle RSS tables > > > > Hi, > > > > The point is that RXHASH works fine on all interfaces, but on the gigabit > > one > > (eth2 usually). > > And on the 10 gbit interface is very very effective, the throughput goes 4x > > when > > enabled, so it would be a big drawback to disable it on all interfaces. > > > > Honestly I don't have any 2.5 gbit hardware to test it on eth3, so I don't > > know if > > rxhash actually only works on the first interface of a unit (so eth0 and > > eth1), or > > if it just doesn't work on the gigabit one. > > > > If someone could test it on the 2.5 gbit port, this will be helpful. > > RSS tables is part of Packet Processor IP, not MAC(so it's not related to > specific speed). Probably issue exist on specific packet processor ports. > Since RSS work fine on first port of the CP, we can do the following: > if (port-> id == 0) > dev->hw_features |= NETIF_F_RXHASH; I can confirm that Macchiatobin Single Shot eth0 port works with a 1G Fibre SFP or 10G DA SFP with or without rxhash on. So it seems Stefan's hunch that it is port related is correct. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
Re: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS contexts to handle RSS tables
On Sat, May 9, 2020 at 1:45 PM Russell King - ARM Linux admin
wrote:
>
> On Sat, May 09, 2020 at 11:15:58AM +, Stefan Chulski wrote:
> >
> >
> > > -Original Message-
> > > From: Matteo Croce
> > > Sent: Saturday, May 9, 2020 3:13 AM
> > > To: David S . Miller
> > > Cc: Maxime Chevallier ; netdev
> > > ; LKML ; Antoine
> > > Tenart ; Thomas Petazzoni
> > > ; gregory.clem...@bootlin.com;
> > > miquel.ray...@bootlin.com; Nadav Haklai ; Stefan
> > > Chulski ; Marcin Wojtas ; Linux
> > > ARM ; Russell King - ARM Linux admin
> > >
> > > Subject: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS contexts
> > > to
> > > handle RSS tables
> > >
> > > Hi,
> > >
> > > What do you think about temporarily disabling it like this?
> > >
> > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > @@ -5775,7 +5775,8 @@ static int mvpp2_port_probe(struct platform_device
> > > *pdev,
> > > NETIF_F_HW_VLAN_CTAG_FILTER;
> > >
> > > if (mvpp22_rss_is_supported()) {
> > > - dev->hw_features |= NETIF_F_RXHASH;
> > > + if (port->phy_interface != PHY_INTERFACE_MODE_SGMII)
> > > + dev->hw_features |= NETIF_F_RXHASH;
> > > dev->features |= NETIF_F_NTUPLE;
> > > }
> > >
> > >
> > > David, is this "workaround" too bad to get accepted?
> >
> > Not sure that RSS related to physical interface(SGMII), better just remove
> > NETIF_F_RXHASH as "workaround".
>
> Hmm, I'm not sure this is the right way forward. This patch has the
> effect of disabling:
>
> d33ec4525007 ("net: mvpp2: add an RSS classification step for each flow")
>
> but the commit you're pointing at which caused the regression is:
>
> 895586d5dc32 ("net: mvpp2: cls: Use RSS contexts to handle RSS tables")
>
>
Hi,
When git bisect pointed to 895586d5dc32 ("net: mvpp2: cls: Use RSS
contexts to handle RSS tables"), which was merged
almost an year after d33ec4525007 ("net: mvpp2: add an RSS
classification step for each flow"), so I assume that between these
two commits either the feature was working or it was disable and we
didn't notice
Without knowing what was happening, which commit should my Fixes tag point to?
Regards,
--
Matteo Croce
per aspera ad upstream
Re: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS contexts to handle RSS tables
On Sat, 9 May 2020 13:48:43 +0100 Russell King - ARM Linux admin wrote: > > Unfortunately, we are no longer actively working on Marvell platform > > support at the moment. We might have a look on a best effort basis, but > > this is potentially a non-trivial issue, so I'm not sure when we will > > have the chance to investigate and fix this. > > That may be the case, but that doesn't excuse the fact that we have a > regression and we need to do something. Absolutely. > Please can you suggest how we resolve this regression prior to > 5.7-final? Since 5.7 is really close, I would suggest to disable the functionality. Best regards, Thomas -- Thomas Petazzoni, CTO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS contexts to handle RSS tables
On Sat, May 09, 2020 at 02:16:44PM +0200, Thomas Petazzoni wrote: > Hello, > > On Sat, 9 May 2020 12:45:18 +0100 > Russell King - ARM Linux admin wrote: > > > Looking at the timeline here, it looks like Matteo raised the issue > > very quickly after the patch was sent on the 14th April, and despite > > following up on it, despite me following up on it, bootlin have > > remained quiet. > > Unfortunately, we are no longer actively working on Marvell platform > support at the moment. We might have a look on a best effort basis, but > this is potentially a non-trivial issue, so I'm not sure when we will > have the chance to investigate and fix this. That may be the case, but that doesn't excuse the fact that we have a regression and we need to do something. Please can you suggest how we resolve this regression prior to 5.7-final? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
RE: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS contexts to handle RSS tables
> -Original Message-
> From: Matteo Croce
> Sent: Saturday, May 9, 2020 3:16 PM
> To: Stefan Chulski
> Cc: David S . Miller ; Maxime Chevallier
> ; netdev ; LKML
> ; Antoine Tenart
> ; Thomas Petazzoni
> ; gregory.clem...@bootlin.com;
> miquel.ray...@bootlin.com; Nadav Haklai ; Marcin
> Wojtas ; Linux ARM ker...@lists.infradead.org>; Russell King - ARM Linux admin
>
> Subject: Re: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS contexts
> to
> handle RSS tables
>
> On Sat, May 9, 2020 at 1:16 PM Stefan Chulski wrote:
> > > Hi,
> > >
> > > What do you think about temporarily disabling it like this?
> > >
> > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > @@ -5775,7 +5775,8 @@ static int mvpp2_port_probe(struct
> > > platform_device *pdev,
> > > NETIF_F_HW_VLAN_CTAG_FILTER;
> > >
> > > if (mvpp22_rss_is_supported()) {
> > > - dev->hw_features |= NETIF_F_RXHASH;
> > > + if (port->phy_interface != PHY_INTERFACE_MODE_SGMII)
> > > + dev->hw_features |= NETIF_F_RXHASH;
> > > dev->features |= NETIF_F_NTUPLE;
> > > }
> > >
> > >
> > > David, is this "workaround" too bad to get accepted?
> >
> > Not sure that RSS related to physical interface(SGMII), better just remove
> NETIF_F_RXHASH as "workaround".
> >
> > Stefan.
>
> Hi,
>
> The point is that RXHASH works fine on all interfaces, but on the gigabit one
> (eth2 usually).
> And on the 10 gbit interface is very very effective, the throughput goes 4x
> when
> enabled, so it would be a big drawback to disable it on all interfaces.
>
> Honestly I don't have any 2.5 gbit hardware to test it on eth3, so I don't
> know if
> rxhash actually only works on the first interface of a unit (so eth0 and
> eth1), or
> if it just doesn't work on the gigabit one.
>
> If someone could test it on the 2.5 gbit port, this will be helpful.
RSS tables is part of Packet Processor IP, not MAC(so it's not related to
specific speed). Probably issue exist on specific packet processor ports.
Since RSS work fine on first port of the CP, we can do the following:
if (port-> id == 0)
dev->hw_features |= NETIF_F_RXHASH;
Regards.
Re: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS contexts to handle RSS tables
On Sat, May 9, 2020 at 1:16 PM Stefan Chulski wrote:
> > Hi,
> >
> > What do you think about temporarily disabling it like this?
> >
> > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > @@ -5775,7 +5775,8 @@ static int mvpp2_port_probe(struct platform_device
> > *pdev,
> > NETIF_F_HW_VLAN_CTAG_FILTER;
> >
> > if (mvpp22_rss_is_supported()) {
> > - dev->hw_features |= NETIF_F_RXHASH;
> > + if (port->phy_interface != PHY_INTERFACE_MODE_SGMII)
> > + dev->hw_features |= NETIF_F_RXHASH;
> > dev->features |= NETIF_F_NTUPLE;
> > }
> >
> >
> > David, is this "workaround" too bad to get accepted?
>
> Not sure that RSS related to physical interface(SGMII), better just remove
> NETIF_F_RXHASH as "workaround".
>
> Stefan.
Hi,
The point is that RXHASH works fine on all interfaces, but on the
gigabit one (eth2 usually).
And on the 10 gbit interface is very very effective, the throughput
goes 4x when enabled, so it would be a big drawback to disable it on
all interfaces.
Honestly I don't have any 2.5 gbit hardware to test it on eth3, so I
don't know if rxhash actually only works on the first interface of a
unit (so eth0 and eth1),
or if it just doesn't work on the gigabit one.
If someone could test it on the 2.5 gbit port, this will be helpful.
Regards,
--
Matteo Croce
per aspera ad upstream
Re: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS contexts to handle RSS tables
Hello, On Sat, 9 May 2020 12:45:18 +0100 Russell King - ARM Linux admin wrote: > Looking at the timeline here, it looks like Matteo raised the issue > very quickly after the patch was sent on the 14th April, and despite > following up on it, despite me following up on it, bootlin have > remained quiet. Unfortunately, we are no longer actively working on Marvell platform support at the moment. We might have a look on a best effort basis, but this is potentially a non-trivial issue, so I'm not sure when we will have the chance to investigate and fix this. Best regards, Thomas -- Thomas Petazzoni, CTO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS contexts to handle RSS tables
On Sat, May 09, 2020 at 11:15:58AM +, Stefan Chulski wrote:
>
>
> > -Original Message-
> > From: Matteo Croce
> > Sent: Saturday, May 9, 2020 3:13 AM
> > To: David S . Miller
> > Cc: Maxime Chevallier ; netdev
> > ; LKML ; Antoine
> > Tenart ; Thomas Petazzoni
> > ; gregory.clem...@bootlin.com;
> > miquel.ray...@bootlin.com; Nadav Haklai ; Stefan
> > Chulski ; Marcin Wojtas ; Linux
> > ARM ; Russell King - ARM Linux admin
> >
> > Subject: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS contexts to
> > handle RSS tables
> >
> > Hi,
> >
> > What do you think about temporarily disabling it like this?
> >
> > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > @@ -5775,7 +5775,8 @@ static int mvpp2_port_probe(struct platform_device
> > *pdev,
> > NETIF_F_HW_VLAN_CTAG_FILTER;
> >
> > if (mvpp22_rss_is_supported()) {
> > - dev->hw_features |= NETIF_F_RXHASH;
> > + if (port->phy_interface != PHY_INTERFACE_MODE_SGMII)
> > + dev->hw_features |= NETIF_F_RXHASH;
> > dev->features |= NETIF_F_NTUPLE;
> > }
> >
> >
> > David, is this "workaround" too bad to get accepted?
>
> Not sure that RSS related to physical interface(SGMII), better just remove
> NETIF_F_RXHASH as "workaround".
Hmm, I'm not sure this is the right way forward. This patch has the
effect of disabling:
d33ec4525007 ("net: mvpp2: add an RSS classification step for each flow")
but the commit you're pointing at which caused the regression is:
895586d5dc32 ("net: mvpp2: cls: Use RSS contexts to handle RSS tables")
Looking at the timeline here, it looks like Matteo raised the issue
very quickly after the patch was sent on the 14th April, and despite
following up on it, despite me following up on it, bootlin have
remained quiet. For a regression, that's not particularly good, and
doesn't leave many options but to ask davem to revert a commit, or
if possible fix it (which there doesn't seem to be any willingness
for either - maybe it's a feature no one uses on this platform?)
Would reverting the commit you point to as the cause (895586d5dc32)
resolve the problem, and have any advantage over entirely disabling
RSS?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
RE: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS contexts to handle RSS tables
> -Original Message- > From: Matteo Croce > Sent: Saturday, May 9, 2020 3:13 AM > To: David S . Miller > Cc: Maxime Chevallier ; netdev > ; LKML ; Antoine > Tenart ; Thomas Petazzoni > ; gregory.clem...@bootlin.com; > miquel.ray...@bootlin.com; Nadav Haklai ; Stefan > Chulski ; Marcin Wojtas ; Linux > ARM ; Russell King - ARM Linux admin > > Subject: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS contexts to > handle RSS tables > > External Email > > -- > On Thu, Apr 23, 2020 at 7:00 PM Russell King - ARM Linux admin > wrote: > > > > On Tue, Apr 14, 2020 at 01:43:02AM +0200, Matteo Croce wrote: > > > On Tue, Apr 14, 2020 at 1:21 AM Maxime Chevallier > > > wrote: > > > > > > > > The PPv2 controller has 8 RSS tables that are shared across all > > > > ports on a given PPv2 instance. The previous implementation > > > > allocated one table per port, leaving others unused. > > > > > > > > By using RSS contexts, we can make use of multiple RSS tables per > > > > port, one being the default table (always id 0), the other ones > > > > being used as destinations for flow steering, in the same way as rx > > > > rings. > > > > > > > > This commit introduces RSS contexts management in the PPv2 driver. > > > > We always reserve one table per port, allocated when the port is probed. > > > > > > > > The global table list is stored in the struct mvpp2, as it's a > > > > global resource. Each port then maintains a list of indices in > > > > that global table, that way each port can have it's own numbering > > > > scheme starting from 0. > > > > > > > > One limitation that seems unavoidable is that the hashing > > > > parameters are shared across all RSS contexts for a given port. > > > > Hashing parameters for ctx 0 will be applied to all contexts. > > > > > > > > Signed-off-by: Maxime Chevallier > > > > > > Hi all, > > > > > > I noticed that enabling rxhash blocks the RX on my Macchiatobin. It > > > works fine with the 10G ports (the RX rate goes 4x up) but it > > > completely kills the gigabit interface. > > > > > > # 10G port > > > root@macchiatobin:~# iperf3 -c 192.168.0.2 Connecting to host > > > 192.168.0.2, port 5201 [ 5] local 192.168.0.1 port 42394 connected > > > to 192.168.0.2 port 5201 > > > [ ID] Interval Transfer Bitrate Retr Cwnd > > > [ 5] 0.00-1.00 sec 941 MBytes 7.89 Gbits/sec 4030250 KBytes > > > [ 5] 1.00-2.00 sec 933 MBytes 7.82 Gbits/sec 4393240 KBytes > > > root@macchiatobin:~# ethtool -K eth0 rxhash on root@macchiatobin:~# > > > iperf3 -c 192.168.0.2 Connecting to host 192.168.0.2, port 5201 [ > > > 5] local 192.168.0.1 port 42398 connected to 192.168.0.2 port 5201 > > > [ ID] Interval Transfer Bitrate Retr Cwnd > > > [ 5] 0.00-1.00 sec 860 MBytes 7.21 Gbits/sec 428410 KBytes > > > [ 5] 1.00-2.00 sec 859 MBytes 7.20 Gbits/sec 185563 KBytes > > > > > > # gigabit port > > > root@macchiatobin:~# iperf3 -c turbo Connecting to host turbo, port > > > 5201 [ 5] local 192.168.85.42 port 45144 connected to 192.168.85.6 > > > port 5201 > > > [ ID] Interval Transfer Bitrate Retr Cwnd > > > [ 5] 0.00-1.00 sec 113 MBytes 948 Mbits/sec0407 KBytes > > > [ 5] 1.00-2.00 sec 112 MBytes 942 Mbits/sec0428 KBytes > > > root@macchiatobin:~# ethtool -K eth2 rxhash on root@macchiatobin:~# > > > iperf3 -c turbo > > > iperf3: error - unable to connect to server: Resource temporarily > > > unavailable > > > > > > I've bisected and it seems that this commit causes the issue. I > > > tried to revert it on nex-next as a second test, but the code has > > > changed a lot much since, generating too much conflicts. > > > Can you have a look into this? > > > > This behaviour on eth2 is confirmed here on v5.6. Turning on rxhash > > appears to prevent eth2 working. > > > > Maxime, please look into this regression, thanks. > > > > -- > > RMK's Patch system: > > https://urldefense.proofpoint.com/v2/url?u=https-3A__www.armlinux.org. > > > uk_developer_patches_=DwIBaQ=nKjWec2b6R0mOyPaz7xtfQ=DDQ3dK > wkTIxK > > Al6_Bs7GMx4
Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS contexts to handle RSS tables
On Thu, Apr 23, 2020 at 7:00 PM Russell King - ARM Linux admin
wrote:
>
> On Tue, Apr 14, 2020 at 01:43:02AM +0200, Matteo Croce wrote:
> > On Tue, Apr 14, 2020 at 1:21 AM Maxime Chevallier
> > wrote:
> > >
> > > The PPv2 controller has 8 RSS tables that are shared across all ports on
> > > a given PPv2 instance. The previous implementation allocated one table
> > > per port, leaving others unused.
> > >
> > > By using RSS contexts, we can make use of multiple RSS tables per
> > > port, one being the default table (always id 0), the other ones being
> > > used as destinations for flow steering, in the same way as rx rings.
> > >
> > > This commit introduces RSS contexts management in the PPv2 driver. We
> > > always reserve one table per port, allocated when the port is probed.
> > >
> > > The global table list is stored in the struct mvpp2, as it's a global
> > > resource. Each port then maintains a list of indices in that global
> > > table, that way each port can have it's own numbering scheme starting
> > > from 0.
> > >
> > > One limitation that seems unavoidable is that the hashing parameters are
> > > shared across all RSS contexts for a given port. Hashing parameters for
> > > ctx 0 will be applied to all contexts.
> > >
> > > Signed-off-by: Maxime Chevallier
> >
> > Hi all,
> >
> > I noticed that enabling rxhash blocks the RX on my Macchiatobin. It
> > works fine with the 10G ports (the RX rate goes 4x up) but it
> > completely kills the gigabit interface.
> >
> > # 10G port
> > root@macchiatobin:~# iperf3 -c 192.168.0.2
> > Connecting to host 192.168.0.2, port 5201
> > [ 5] local 192.168.0.1 port 42394 connected to 192.168.0.2 port 5201
> > [ ID] Interval Transfer Bitrate Retr Cwnd
> > [ 5] 0.00-1.00 sec 941 MBytes 7.89 Gbits/sec 4030250 KBytes
> > [ 5] 1.00-2.00 sec 933 MBytes 7.82 Gbits/sec 4393240 KBytes
> > root@macchiatobin:~# ethtool -K eth0 rxhash on
> > root@macchiatobin:~# iperf3 -c 192.168.0.2
> > Connecting to host 192.168.0.2, port 5201
> > [ 5] local 192.168.0.1 port 42398 connected to 192.168.0.2 port 5201
> > [ ID] Interval Transfer Bitrate Retr Cwnd
> > [ 5] 0.00-1.00 sec 860 MBytes 7.21 Gbits/sec 428410 KBytes
> > [ 5] 1.00-2.00 sec 859 MBytes 7.20 Gbits/sec 185563 KBytes
> >
> > # gigabit port
> > root@macchiatobin:~# iperf3 -c turbo
> > Connecting to host turbo, port 5201
> > [ 5] local 192.168.85.42 port 45144 connected to 192.168.85.6 port 5201
> > [ ID] Interval Transfer Bitrate Retr Cwnd
> > [ 5] 0.00-1.00 sec 113 MBytes 948 Mbits/sec0407 KBytes
> > [ 5] 1.00-2.00 sec 112 MBytes 942 Mbits/sec0428 KBytes
> > root@macchiatobin:~# ethtool -K eth2 rxhash on
> > root@macchiatobin:~# iperf3 -c turbo
> > iperf3: error - unable to connect to server: Resource temporarily
> > unavailable
> >
> > I've bisected and it seems that this commit causes the issue. I tried
> > to revert it on nex-next as a second test, but the code has changed a
> > lot much since, generating too much conflicts.
> > Can you have a look into this?
>
> This behaviour on eth2 is confirmed here on v5.6. Turning on rxhash
> appears to prevent eth2 working.
>
> Maxime, please look into this regression, thanks.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
>
Hi,
What do you think about temporarily disabling it like this?
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -5775,7 +5775,8 @@ static int mvpp2_port_probe(struct platform_device *pdev,
NETIF_F_HW_VLAN_CTAG_FILTER;
if (mvpp22_rss_is_supported()) {
- dev->hw_features |= NETIF_F_RXHASH;
+ if (port->phy_interface != PHY_INTERFACE_MODE_SGMII)
+ dev->hw_features |= NETIF_F_RXHASH;
dev->features |= NETIF_F_NTUPLE;
}
David, is this "workaround" too bad to get accepted?
Bye,
--
Matteo Croce
per aspera ad upstream
