Fix rib generation count for fib algo.
ClosedPublic
Actions

Authored by melifaro on Apr 17 2021, 6:58 PM.

Details

Reviewers

donner

Group Reviewers

network

Commits

rG1899138d5ad5: Fix rib generation count for fib algo.
rG33cb3cb2e321: Fix rib generation count for fib algo.

Summary

Currently, PCB caching mechanism relies on the rib generation counter (rnh_gen) to invalidate cached nhops/LLE entries.

With certain fib algorithms, it is now possible that the datapath lookup state applies RIB changes with some delay.
In that scenario, PCB cache will invalidate on the RIB change, but the new lookup may result in the same nexthop being returned.
When fib algo finally gets in sync with the RIB changes, PCB cache will not receive any notification and will end up caching the stale data.

To fix this, introduce additional counter, rnh_gen_rib, which is used only when FIB_ALGO is enabled.
This counter is incremented by the control plane. Each time when fib algo synchronises with the RIB, it updates rnh_gen to the current rnh_gen_rib value.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

melifaro created this revision.Apr 17 2021, 6:58 PM

Herald added subscribers: ae, imp. · View Herald TranscriptApr 17 2021, 6:58 PM

melifaro requested review of this revision.Apr 17 2021, 6:58 PM

Harbormaster completed remote builds in B38654: Diff 87626.Apr 17 2021, 6:59 PM

melifaro edited the summary of this revision. (Show Details)Apr 17 2021, 7:07 PM

melifaro added a reviewer: network.

donner added a subscriber: donner.Apr 18 2021, 9:59 AM

donner added inline comments.

sys/net/route/fib_algo.c
822	Why is it before "fib_unref_nhop", while it is after that some lines earlier?

melifaro added inline comments.Apr 20 2021, 8:03 AM

sys/net/route/fib_algo.c
822	It has no connection to the mechanics of the `fib_unref_nhop()`, so the ordering doesn't matter. Though, I'll update the one in the `apply_rtable_changes()` to make it consistent.

donner accepted this revision as: donner.Apr 20 2021, 11:55 AM

donner added inline comments.

sys/net/route/fib_algo.c
932	As I do understand the logic, the rib-table version is incremented each time, the rib is modified. This call is done after dumping the table, without modifying it. If this is correct, the increment is not necessary. Otherwise LTGM.

This revision is now accepted and ready to land.Apr 20 2021, 11:55 AM

Reflect donner@ comments.

This revision now requires review to proceed.Apr 20 2021, 10:12 PM

Harbormaster completed remote builds in B38738: Diff 87840.Apr 20 2021, 10:12 PM

melifaro added inline comments.Apr 20 2021, 10:18 PM

sys/net/route/fib_algo.c
932	Yep, you're right w.r.t the table dump. The nuance here is that the framework can re-instantiate algorithm instance at any point in time. For example, the following can happen: inet.0 rib backed by lradix4 receives a route update. As lradix4 is immutable, the framework schedules the algorithm rebuild (e.g. build of a new instance). Until the rebuild is executed and complete, datapath will run with an older version of the rib snapshot. So, when the rebuild is complete, framework needs to update the datapath generation to reflect the fact that it's now synced to the latest rib.