Page MenuHomeFreeBSD

netgraph/ng_bridge: become multithreaded
ClosedPublic

Authored by donner on Jan 12 2021, 9:37 PM.

Details

Summary

The node ng_bridge is currently a single threaded node.
This causes performance issues in our environment.

In order to become multithreaded the recdata function needs to be modified, so that all node and link private storage is const.

The modifications will be child reviews in order to be easier to review. This revision will maintain the high level overview.

Diff Detail

Repository
R10 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

donner retitled this revision from netgraph/ng_bridge:become multithreaded to netgraph/ng_bridge :become multithreaded.Jan 12 2021, 10:32 PM
donner added a reviewer: network.
donner retitled this revision from netgraph/ng_bridge :become multithreaded to netgraph/ng_bridge: become multithreaded.
donner edited the summary of this revision. (Show Details)

Lutz, do you have any plans for the upcoming changes?
I also thought about getting rid of ng_bridge from NG_NODE_FORCE_WRITER. Since rcv_data is always called in the NET_EPOCH context, I think we can do it like @kp did it for if_bridge(4) (see D24250).

Lutz, do you have any plans for the upcoming changes?

Yes, I have to solve the problem ASAP.
Steps include (as written in the comment):

  • modify revdata path so work read only
  • changes to the host or node private data need to be done via control messages (send to self)
  • stats and alive counter will become counters

I also thought about getting rid of ng_bridge from NG_NODE_FORCE_WRITER. Since rcv_data is always called in the NET_EPOCH context, I think we can do it like @kp did it for if_bridge(4) (see D24250).

Thanks, I'll have a look.

Prepare ng_bridge for read-only data paths by providing const pointers.

revert the idea of specialized const pointers, it's too confusing

I also thought about getting rid of ng_bridge from NG_NODE_FORCE_WRITER. Since rcv_data is always called in the NET_EPOCH context, I think we can do it like @kp did it for if_bridge(4) (see D24250).

The idea in of this patch is to enter and leave the epoch for critical paths, and delay destroying actions by callbacks to the epoch framework.
if_bridge can do this, because the epoch framework is voluntary to it's work.

On contrast ng_bridge is called in epoch itself, it's a functional addon to data paths (i.e. if_bridge) which are already covered by epoch. Despite it's unwise to interfere with this out of scope usage (we may break things on physical interface level), there is a the synchronization from the netgraph framework itself. An out-of-epoch callback does not know anything about the current state of the netgraph control message processing (which is not necessarily covered by epoch).

So I fear, that the approach of using epoch will cause subtle race conditions.

For out-of-current callbacks the netgraph framework offers control messages with direct function calls and can end in externally usable table modification. Personally I like the idea of manipulating the host table of ng_bridge from user space. I would allow me to backup and restore the table for maintenance, pre-populate the table for a quicker response (especially if D23963 does not learn all addresses automatically)

So let me try to nail down this path of using the netgraph framework directly.

Because the Phabricator dependencies are now in place: Make it happen

This revision was not accepted when it landed; it landed in state Needs Review.May 13 2021, 3:54 PM
This revision was automatically updated to reflect the committed changes.