Page MenuHomeFreeBSD

Replace the fastforward path with tryforward which does not require a sysctl and will always be on. The former split between default and fast forwarding is removed by this commit while preserving the ability to use all network stack features.
ClosedPublic

Authored by gnn on Oct 30 2015, 8:26 PM.

Details

Summary

...es. This version addresses
the IPSEC issues now that IPSEC has been updated to be default
in GENERIC

Test Plan

Tested with iperf3 and Conductor and results presented at VBSDCon 2015

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

gnn updated this revision to Diff 9834.Oct 30 2015, 8:26 PM
gnn retitled this revision from to Replace the fastforward path with tryforward which does not require a sysctl and will always be on. The former split between default and fast forwarding is removed by this commit while preserving the ability to use all network stack features..
gnn updated this object.
gnn edited the test plan for this revision. (Show Details)
gnn edited the test plan for this revision. (Show Details)Oct 30 2015, 8:28 PM
loos added a subscriber: loos.Oct 30 2015, 8:49 PM

Just a cosmetic/documentation remark:

In sys/netinet/ip_fastfwd.c, comment "Step 1: check for packet drop conditions (and sanity checks)" was removed.
Then comment "Step 2: fallback conditions to normal ip_input path processing" (lines 186 to 188) should be renamed or removed.

ae added a subscriber: ae.Nov 2 2015, 10:03 AM

You made a system acting as router by default, even when net.inet.ip.forwarding is zero. Is it an intended change?

gnn added a comment.Nov 2 2015, 2:13 PM

Good point. I'll add that check.

gnn updated this revision to Diff 9901.Nov 3 2015, 4:17 AM
gnn edited edge metadata.

Add check of forwarding sysctl before calling into tryforward.

Updating D4042: Replace the fastforward path with tryforward which does not require

a sysctl and will always be on. The former split between default
and fast forwarding is removed by this commit while preserving the
ability to use all network stack features.

gnn updated this revision to Diff 9910.Nov 3 2015, 5:02 PM

Remove unnecessary comment per Olivier Couchard.

Updating D4042: Replace the fastforward path with tryforward which does not require

a sysctl and will always be on. The former split between default
and fast forwarding is removed by this commit while preserving the
ability to use all network stack features.

ae added inline comments.Nov 4 2015, 10:29 AM
sys/netinet/ip_input.c
83 ↗(On Diff #9910)

What you think about reducing namespace pollution and create a simple function in ip_ipsec.c to check SPD availability?

int
ip_ipsec_skip()
{

   return ((key_havesp(IPSEC_DIR_INBOUND) || key_havesp(IPSEC_DIR_OUTBOUND)) == 0);
}
518 ↗(On Diff #9910)

This block now can be moved under one #ifdef IPSEC.

gnn marked an inline comment as done.Nov 4 2015, 2:31 PM
gnn added inline comments.
sys/netinet/ip_input.c
83 ↗(On Diff #9910)

I had thought about this originally but then the number of places it would be used was too small and I didn't think we'd be adding that many more of them. It could be done outside the context of this particular patch set if people feel really strong about it

ae accepted this revision.Nov 4 2015, 4:55 PM
ae added a reviewer: ae.
This revision is now accepted and ready to land.Nov 4 2015, 4:55 PM
melifaro accepted this revision.Nov 4 2015, 6:26 PM
melifaro edited edge metadata.

I did several forwarding benchmarks based on projects/routing patched with diff 9834.
I tested on 2x E5-2660 (HT off) @ Chelsio T580 HW.
Actual test consists of IPv4 forwarding (128x192 src/dst pairs resulting in close-to-equal traffic distribution on NIC RX rings).
Pure projects/routing results:
1 core: 1.10 mpps, 8 cores: 7.8mpps, 16 cores: 12.7 mpps
Projects/routing w/patch:
1 core: 1.07mpps, 8 cores: 7.95 mpps, 16 cores: 12.7 mpps

I have no good explanation for 8 cores result, but anyway, it looks like performance is roughly the same at least on amd64.

This revision was automatically updated to reflect the committed changes.