Enter network epoch for network interrupts
ClosedPublic
Actions

Authored by glebius on Jan 17 2020, 9:05 PM.

Details

Reviewers

shurd
gallatin
melifaro
bz

Group Reviewers

network
iflib

Commits

rS357004: Enter the network epoch for interrupt handlers of INTR_TYPE_NET.

Summary

This change pushes the epoch further up in the stack. Basically any
context that is associated with networking and isn't allowed to sleep
runs in epoch.

There are few interfaces that have special context where they
process incoming packets and those are marked with IFF_NEEDSEPOCH.
For such interfaces ether_input() would enter epoch. The plan is
to eliminate this crutch before 13.0-RELEASE.

To prevent endless epoch under heavy interrupt storm there is a
tunable that stops batching packet processing at certain count.

Rough commit split: https://github.com/glebius/FreeBSD/commits/intr-epoch

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 28745
Build 26757: arc lint + arc unit

Event Timeline

glebius created this revision.Jan 17 2020, 9:05 PM

Herald added a reviewer: shurd. · View Herald TranscriptJan 17 2020, 9:05 PM

Herald added a reviewer: iflib. · View Herald Transcript

Herald added subscribers: ae, jhibbits, asomers. · View Herald Transcript

Harbormaster completed remote builds in B28744: Diff 66933.Jan 17 2020, 9:05 PM

glebius added a subscriber: olivier.Jan 17 2020, 9:06 PM

I'm pretty sure all the NET_EPOCH's in the cxgbe(4) TOM code can go away as part of this.

sys/net/if.h
147	Does this mean "driver calls if_input without epoch", or "network stack should call if_input without epoch"? I think the former, but the comment is a bit ambiguous.

Clarify unclear comment.

Harbormaster completed remote builds in B28745: Diff 66936.Jan 17 2020, 9:47 PM

glebius marked an inline comment as done.Jan 17 2020, 9:47 PM

First forwarding performance impact benches.
On an old PC Engines APU2 (AMD GX 4 core 1Ghz and Intel Gigabit) there is a +6% improvement of inet4 packets-per-second forwarded:

x r356840
+ r356840 with D23242
+--------------------------------------------------------------------------+
|x x   xx   x                                               +   +    ++   +|
| |___AM___|                                                               |
|                                                             |____A_M___| |
+--------------------------------------------------------------------------+
    N           Min           Max        Median           Avg        Stddev
x   5      763950.5      772812.5      768790.5      768164.7     3504.7937
+   5      812003.5        823150        818868      817803.5     4336.1657
Difference at 95.0% confidence
        49638.8 +/- 5749.84
        6.462% +/- 0.767994%
        (Student's t, pooled s = 3942.46)

On a Atom device (8core and 10G Chelsio) there is no improvement on the packets-per-second forwarded rate:

x 356840
+ 356840 with D23242
+--------------------------------------------------------------------------+
|x                              xx        x  ++            +        +   + x|
|         |______________________M__A__________________________|           |
|                                             |___________AM__________|    |
+--------------------------------------------------------------------------+
    N           Min           Max        Median           Avg        Stddev
x   5       4309884       4606689       4439183     4454028.1     106157.11
+   5       4487479       4598784       4547141     4542038.6     50110.922
No difference proven at 95.0% confidence

Result on bigger hardware later (16 cores Xeon and 40G).

And on 16core with 40Gb NIC, + 4% of forwarding speed improvement (pps unit):

x 356840
+ 356840 with D23242
+--------------------------------------------------------------------------+
|x     x    xx      x                                    +     +   + +    +|
|  |_______AM_____|                                                        |
|                                                           |_____AM____|  |
+--------------------------------------------------------------------------+
    N           Min           Max        Median           Avg        Stddev
x   5      30651580      31116776      30914356      30882113     172145.91
+   5      32002188      32402450      32244079      32214895     150605.94
Difference at 95.0% confidence
        1.33278e+06 +/- 235881
        4.31571% +/- 0.782654%
        (Student's t, pooled s = 161735)

gallatin accepted this revision.Jan 20 2020, 5:40 PM

This revision is now accepted and ready to land.Jan 20 2020, 5:40 PM

I'd really appreciate some reviewing. I'm going to commit this week.

melifaro accepted this revision as: melifaro.Jan 21 2020, 8:39 AM

melifaro added a subscriber: melifaro.

melifaro added inline comments.

sys/kern/kern_intr.c
98	Could you please elaborate a bit on the reasoning of picking this relatively high default? I was under impression that batch size of 16 or 32 could amortise the costs pretty close to "ideal", while maintaining worst-case epoch duration lower.

Can you please update the diff? I doen't apply cleanly to head (anymore):

bz% patch -C -s -p0 < D23242\?download=true
Reversed (or previously applied) patch detected! Assume -R? [y] n
Apply anyway? [n] n
2 out of 2 hunks ignored while patching sys/kern/uipc_domain.c
Reversed (or previously applied) patch detected! Assume -R? [y] n
Apply anyway? [n] n
2 out of 2 hunks ignored while patching sys/net/if.c
Reversed (or previously applied) patch detected! Assume -R? [y] n
Apply anyway? [n] n
2 out of 2 hunks ignored while patching sys/net/if_var.h
2 out of 6 hunks failed while patching sys/net/pfil.c
bz% svn status sys/kern/uipc_domain.c sys/net/if.c sys/net/if_var.h sys/net/pfil.c
bz%

Rebased to fresh master.

This revision now requires review to proceed.Jan 22 2020, 3:09 AM

Harbormaster completed remote builds in B28838: Diff 67133.Jan 22 2020, 3:09 AM

glebius added inline comments.Jan 22 2020, 3:11 AM

sys/kern/kern_intr.c
98	Number is pretty arbitrary. I didn't do much experimenting with it, yet.

One small nit on a comment.

If you do the split commits can you please try to tell people as to why the bits are needed the way they are as well. The split github changes are massively helpful to see and understand individual changes, the commit messages there not so much ;-)

sys/sys/interrupt.h
122	Start with upper case and ends in '.'

This revision is now accepted and ready to land.Jan 22 2020, 2:59 PM

Closed by commit rS357004: Enter the network epoch for interrupt handlers of INTR_TYPE_NET. (authored by glebius). · Explain WhyJan 23 2020, 1:25 AM

This revision was automatically updated to reflect the committed changes.

glebius added a commit: rS357004: Enter the network epoch for interrupt handlers of INTR_TYPE_NET..

Herald added a subscriber: imp. · View Herald TranscriptJan 23 2020, 1:25 AM

glebius mentioned this in D23348: Widen EPOCH(9) usage in network drivers (as a pre-step for D23347).Jan 27 2020, 7:58 PM

Revision Contents
Changeset List

Path

Size

sys/

dev/

altera/

atse/

if_atse.c

3 lines

beri/

virtio/

network/

if_vtbe.c

2 lines

dpaa/

if_dtsec.c

2 lines

hyperv/

netvsc/

if_hn.c

3 lines

if_ndis/

if_ndis.c

3 lines

netmap/

netmap.c

5 lines

ntb/

if_ntb/

if_ntb.c

3 lines

sbni/

if_sbni.c

3 lines

xen/

netback/

netback.c

2 lines

kern/

kern_intr.c

39 lines

kern_poll.c

6 lines

uipc_domain.c

3 lines

mips/

nlm/

dev/

net/

xlpge.c

3 lines

net/

2 lines

2 lines

6 lines

3 lines

6 lines

8 lines

5 lines

19 lines

powerpc/

pseries/

phyp_llan.c

2 lines

sys/

epoch.h

10 lines

interrupt.h

3 lines

Commit	Tree	Parents	Author	Summary	Date
7ea110a31a4a	8249123c8612	eac80993531d	Gleb Smirnoff	fixup "Introduce IFF_NEEDSEPOCH"	Jan 17 2020, 9:46 PM
eac80993531d	9bdf8f3185a5	f3a224963fdd	Gleb Smirnoff	Since now all network interrupts run in epoch, pfil_run_hooks is guaranteed to… (Show More…)	Jan 10 2020, 8:50 PM
f3a224963fdd	af5ae0c9d41e	b1397dcc870e	Gleb Smirnoff	Enter network epoch in tunwrite().	Dec 20 2019, 9:40 PM
b1397dcc870e	d2c3b4b086c4	ab14e1d17948	Gleb Smirnoff	IFF_NEEDSEPOCH	Dec 17 2019, 10:38 PM
ab14e1d17948	dfae1eec1817	9b9e70ba5886	Gleb Smirnoff	Should be INTR_TYPE_NET, not INTR_TYPE_MISC.	Dec 17 2019, 10:31 PM
9b9e70ba5886	03b7b56bb308	6b7e54a2f382	Gleb Smirnoff	Enter network epoch when processing input mbuf queue.	Dec 17 2019, 10:21 PM
6b7e54a2f382	42656d57f4c8	1f595f4217ea	Gleb Smirnoff	IFF_NEEDSEPOCH that needs to be reviewed	Dec 7 2019, 2:51 AM
1f595f4217ea	07e61730f7b4	56ad9613ba37	Gleb Smirnoff	IFF_NEEDSEPOCH	Dec 7 2019, 2:40 AM
56ad9613ba37	b7ece05c62cc	3f627a5af92a	Gleb Smirnoff	IFF_NEEDSEPOCH	Dec 7 2019, 2:38 AM
3f627a5af92a	ca928e437ec3	08019c275c02	Gleb Smirnoff	Should be INTR_TYPE_NET, not INTR_TYPE_BIO.	Dec 7 2019, 2:38 AM
08019c275c02	8b775ec051f9	ec96de0dd218	Gleb Smirnoff	IFF_NEEDSEPOCH	Dec 7 2019, 2:14 AM
ec96de0dd218	458b586bc375	e6eff26ba8c8	Gleb Smirnoff	IFF_NEEDSEPOCH	Dec 7 2019, 1:28 AM
e6eff26ba8c8	9e071c423fae	a645c4b1529d	Gleb Smirnoff	Enter network epoch in idle poll.	Dec 7 2019, 1:13 AM
a645c4b1529d	cf05c00a2173	b8240b95701c	Gleb Smirnoff	Mark interfaces that supposedly don't run interrupt handler in epoch.	Dec 6 2019, 9:58 PM
b8240b95701c	1d058972378d	4dea410f8fea	Gleb Smirnoff	Introduce IFF_NEEDSEPOCH	Dec 6 2019, 9:58 PM
4dea410f8fea	a268be5fde60	6ddd2f28c8b2	Gleb Smirnoff	Enter network epoch in iflib rxeof task. (Show More…)	Jan 15 2020, 10:50 PM
6ddd2f28c8b2	cdbd168bb8af	be0186065e2f	Gleb Smirnoff	Mark swi_net() as INTR_TYPE_NET and stop entering epoch there.	Nov 9 2019, 7:02 AM
be0186065e2f	dd00dcadec64	485edb9a021e	Gleb Smirnoff	Enter the network epoch for interrupt handlers of INTR_TYPE_NET. Provide… (Show More…)	Nov 9 2019, 6:52 AM
485edb9a021e	efda43fffc36	9af03462d7fe	Gleb Smirnoff	Add ie_hflags to struct intr_event, which accumulates flags from all handlers. (Show More…)	Nov 9 2019, 6:28 AM
9af03462d7fe	515c7b2a4417	64967afc7c21	Gleb Smirnoff	- Move global network epoch definition to epoch.h, as more different… (Show More…)	Nov 5 2019, 12:39 AM