Page MenuHomeVyOS Platform

accel-ppp: VPP crashes (SIGBUS in policer_input_node_fn) when sessions with a rate limit end
Open, HighPublicBUG

Description

With the VPP dataplane for PPPoE and a RADIUS rate limit, VPP can crash with SIGBUS in policer_input_node_fn_hsw when sessions end. VPP restarts and every session is dropped.

Seen in T9211 by a tester on a fresh rolling VM (VPP 25.10.0, FreeRADIUS, 2 real CPEs and about 1,700 emulated clients, about 1,000 sessions connecting within a minute):

  • SIGBUS in policer_input_node_fn_hsw at 13:44:52 UTC, VPP restarted, all sessions dropped
  • a second VPP restart at 14:41:22 UTC with no configuration change

I have not reproduced the crash. This is from reading the code, it is a likely cause and not confirmed.

vpppolicer_remove_limiter() in accel-ppp-ng (accel-pppd/session_hooks/vyos_vpp/vpppolicer.c) does not undo vpppolicer_install_limiter() in reverse order. Installing binds the input policer to the interface and then adds the policer-input node to the ip4-unicast, ip6-unicast, ip4-multicast and ip6-multicast feature arcs. Removing unbinds and deletes the input policer first and takes the node out of the arcs afterwards. In between, packets of the interface can still reach policer_input_node_fn while the policer it uses is gone.

Fix proposed by the tester and prepared as a branch: remove the policer-input features first, then unbind and delete the policer (the reverse of the install order). The output policer needs no change.

What is still open:

  • The change is untested. The window is small and a few sessions are not enough to hit it, so it needs a test with many sessions that end at once while traffic flows.
  • It is not known if the order alone is enough, or if deleting a policer while packets are in flight needs more.

Related: T9211 (found there), T9499 (the abort of accel-pppd when a VPP session ends, a different problem).

Details

Version
rolling
Is it a breaking change?
Unspecified (possibly destroys the router)
Issue type
Bug (incorrect behavior)

Event Timeline

rherold created this object in space S1 VyOS Public.
Branch with the change, based on the accel-ppp-ng version that rolling builds (dd07b23):
https://github.com/ruben-herold/accel-ppp-ng/commit/3c44e19 (branch `fix-vpp-policer-remove-order`)

It only moves the four `vpppoe_set_feature(..., 0, "policer-input", ...)` calls in `vpppolicer_remove_limiter()` in front of `vpppolicer_set_input(0, ...)` and the delete 
of the policer, the reverse of `vpppolicer_install_limiter()`.

Status: not confirmed. I have not reproduced the crash. A test build with this change on top of the build of T9211 is in the task T9211 for the tester, who has the load   
for it. A PR to accel-ppp-ng follows when it is confirmed.
Viacheslav raised the priority of this task from Low to High.Thu, Oct 22, 4:04 PM
Viacheslav changed the subtype of this task from "Task" to "Bug".

Test result so far: on a VM with three sessions with a RADIUS rate limit (VPP policers in use) and a ping flood on each, ending all sessions and reconnecting 15 times, VPP did not crash with the old order and not with the new order, and no policer was left behind after any round with the new order. With three sessions the window is too small to hit, so this does not show that the new order is the fix, only that it removes the policers cleanly. A build with the change is in the task T9211 for the tester, who has the load for it.

@saadinnocent909 (a mention subscribes you to this task) for your answers about crashes see https://vyos.dev/T9211, where I ask about the backtrace, the NAT44 output-feature workaround and the policer. The crash of T9501 (NAT44, nat_pre_in2out_output) is a different one from the crash of this task (policer_input_node_fn_hsw). T9211 reports a second VPP restart (14:41:22 UTC) without a cause, which could be either of them. The hook that creates and deletes the session interface is where the policer features are removed here and where marking each pppoe_sessionN as NAT inside (option 1 of T9501) would go, so if that is done the removal has to be the reverse of the setup from the start.