Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: flowtable direction logic and other fixes #382

Merged
merged 21 commits into from
Feb 4, 2024
Merged

refactor: flowtable direction logic and other fixes #382

merged 21 commits into from
Feb 4, 2024

Conversation

mogaika
Copy link
Collaborator

@mogaika mogaika commented Feb 1, 2024 •
edited
Loading

Functional change:

  • Instead of enabling flowtable timers expiration process only in tests, create flowtable timer node which enabled always. Implemented using process node which triggers interrupts in input node which does timer wheel rotation. For now this is only done as backup mechanism for low-traffic situation and flow timers expected to expire mostly in flow node, so wheel rotation frequency is low. To switch entirely to timer expiration node granularity of time has to be increased to spread timers over time and to not burst them every second.

Changes related to flowtable, but not affecting behavior:

  • In flow structure do not use reversed order from flow hashkey. This eliminates tons of xoring of direction with flow reverse mark. This makes code much clearer. Matching in hashtable still done with "ordered" key which order depends on ip addresses. Flow internal ip and port fields swapped if needed on flow creation and flow removal. Access to flow fields can done directly with direction type without additional xoring.
  • Separate per direction fields of flow to separated struct which to have better logical separation. There is no advantage from performance side to keep it as SoA instead of AoS, since fields used in random order in hot-path and even if performance difference exists, it's in margin of error.
  • In some places replace FT_ORIGIN/FT_REVERS with more meaningful enums for different operations.
  • Remove flow event system and replace it with explicit calls to event handlers.
  • Separate upf_buffer_opaque_t in own header so we could solve dependency and use it in flowtable code.

Unrelated changes:

  • Remove "upf-ipX-proxy-server-no-conn-output" nodes since they make no sense in newer VPP versions (as was mentioned in comments), and also there was a bug where direction variable have been incorrectly reused between packets in one vector.
  • Disable and deprecate "pooling" mode for pfcp. UPG supports interrupts without additional configuration if all VPP nodes are in interrupt mode. Except in vpp v22.10 there is a bug which prevents enabling interrupt mode, and it's fixed in vpp 5f30518.
  • Fix vpp hang when looping of proxy traffic happens if PDI/FAR configured wrongly by dealing with TTL.
  • Simplify optimization possibility of acl matching and do not match spare fields in case of ipv4.

@mogaika
remove flowtable events system
8d30a09
@mogaika
retype previously known flow event handlers
1a7841d
@mogaika
expire flows from separated timer node
2cdca6e
@mogaika
remove pfcp-server-mode since it didn't worked
e8a8a20 
It impossible to change interrupt mode of process node.

Instead for tests use `use-private-rx-mqs` parameter for session node
which should enable interrupt mode for session processing. Which is
only pooling node we using. In current upg version this is broken due
to bug which is fixed in vpp commit 5f30518 and included in v23.06.

For flowtable timer wheel expiration we using separate process which
sends interrupts to worker input nodes to trigger expiration.
@mogaika
fix flowtable expiration
2c7eced
@mogaika
do renaming and documentation of is_reverse stuff
1360a8f
@mogaika
fix typo which led to using wrong pdr in app detection
bc65fb3
@mogaika
rework how flow member fields are accesses
5e547e9 
Use inline func with assert instead of macro, so in next
change we could get rid of xoring of fields.
Also group flow fields for one direction in struct, to improve
cache hit and readability
@mogaika
rework how flow accesses with more controllable meaning
fd428a7
@mogaika
remove obsolete upf-ipX-proxy-server-no-conn-output nodes
2ba0a35
@mogaika
fixes of renew changes
9adf76c
@mogaika
move things around
6d974cf
@mogaika
fix big caused by small type
718e9d6
@mogaika
reverse logic of key_direction to avoid lots of xors
b8ab303
@mogaika
prevent vpp hang if there is proxy loop
ad8fc6e
@mogaika
small code improvements
51cb1e4
@mogaika
better acl inlining for ipv4/6
c977bbd
@mogaika mogaika changed the title Multiple small fixes and readability improvements Rework flowtable direction logic and other fixes Feb 2, 2024
@mogaika mogaika marked this pull request as ready for review February 2, 2024 08:31
mogaika
mogaika commented Feb 2, 2024
View reviewed changes
upf/upf_adf.c Show resolved Hide resolved
@demo-exe
Copy link
Collaborator

demo-exe commented Feb 3, 2024

maybe name to 'refactor: flowtable direction logic and other fixes'

@mogaika mogaika changed the title Rework flowtable direction logic and other fixes refactor: flowtable direction logic and other fixes Feb 4, 2024
@mogaika mogaika merged commit eef744e into master Feb 4, 2024
9 checks passed
@mogaika mogaika deleted the small-improvements-around-flowtable branch February 4, 2024 21:14
@korroot korroot added the fix label Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@demo-exe demo-exe demo-exe approved these changes

Assignees
No one assigned
Labels
fix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mogaika @demo-exe @korroot