Created attachment 207757 [details] remove fixed limit for links on netgraph/ng_bridge The netgraph node ng_bridge has a compile time limit for the number of links, it can handle. This patch removes this limit by allocating more links when needed (assuming sequential assignment). Please note, that this patch is preliminary. It's only done to allow an incremental review of the development. A large number of links require further changes to the code in order to handle all cases efficient. This includes: + replacement of the central "priv->link" table by hook private data + replacement of foreach loops by a linked list over looped hooks In consequence the current requirement to use a compact, sequential assignment of "link%d" hooks can be replaced by arbitrary naming conventions. How should I provide the additional patches? A large patch (overwriting the previous one) or by a series of patches?
Could this be made into a run-time tunable (sysctl), at least as a simplification while auto-incrementing design is worked on/reviewed ?
The idea is to replace the whole array by hook private storage. This will remove the need for any limit control. Should I provide a summary patch as replacement?
What do you think about submitting this patch to the phabricator? https://reviews.freebsd.org/ https://wiki.freebsd.org/Phabricator
Thank you for pointing me to phabricator. I'll try to find out how it works.
Moved to https://reviews.freebsd.org/D21803
manpages review is "green" What are the next steps @Phabricator?
Comment on attachment 207757 [details] remove fixed limit for links on netgraph/ng_bridge ^Triage: Obsoleted by https://reviews.freebsd.org/D21803
A commit references this bug: Author: glebius Date: Thu Oct 3 02:32:56 UTC 2019 New revision: 353026 URL: https://svnweb.freebsd.org/changeset/base/353026 Log: - Remove the compile time limit for number of links a ng_bridge node can handle. Instead using an array on node private data, use per-hook private data. - Use NG_NODE_FOREACH_HOOK() to traverse through hooks instead of array. PR: 240787 Submitted by: Lutz Donnerhacke <lutz donnerhacke.de> Differential Revision: https://reviews.freebsd.org/D21803 Changes: head/share/man/man4/ng_bridge.4 head/sys/netgraph/ng_bridge.c head/sys/netgraph/ng_bridge.h
Is this a merge to stable/* or releng/12.1 candidate?
I would welcome a merge to STABLE, because that's the production environment.
^Triage: 12.1-R candidate
(In reply to Kubilay Kocak from comment #11) Why is this a 12.1 blocker? The change is a welcome improvement but does not fix a regression. Rather it removes an existing limitation in ng_bridge(4). IMHO it is too late to include this in the release.
I totally agree with Mark.
Too late for 12.1, but merging to stable/12 before 12.2 is probably feasible
it changes the API (and the cookie that identifies the API revision) so old binanries will not work. This sort of disqualifies it from 12.anything.. remember that we keep API along a branch lifetime. For now we should just increase the default number of links in 12.
though one could implement the old cookie too and translate to some set of defaults in the new scheme.
^Triage: - Remove block on 12.1-R issue - Decline stable/* due to ABI change (comment 15) If anyone wants to change this, just re-open and set mfc-stable* back to ? accordingly with a comment
https://reviews.freebsd.org/D21961 solves the ABI change issue. It's accepted but not committed.
(In reply to lutz from comment #18) Please ask the committer who intends to commit the change to reference this PR in the commit log message so it and the subsequent MFC can be tracked here
A commit references this bug: Author: bz Date: Sun Jan 5 19:14:17 UTC 2020 New revision: 356386 URL: https://svnweb.freebsd.org/changeset/base/356386 Log: netgraph/ng_bridge: Reestablish old ABI In order to be able to merge r353026 bring back support for the old cookie API for a transition period in 12.x releases (and possibly 13) before the old API can be removed again entirely. Suggested by: julian Submitted by: Lutz Donnerhacke (lutz donnerhacke.de) PR: 240787 Reviewed by: julian MFC after: 2 weeks X-MFC with: r353026 Differential Revision: https://reviews.freebsd.org/D21961 Changes: head/sys/netgraph/ng_bridge.c head/sys/netgraph/ng_bridge.h
^Triage: Over to bz@ for closing pending merge of base r356386
A commit references this bug: Author: bz Date: Tue Jan 28 17:39:03 UTC 2020 New revision: 357206 URL: https://svnweb.freebsd.org/changeset/base/357206 Log: MFC r353026,353030,354244 (glebius), r356386: Remove the compile time limit for number of links a ng_bridge node can handle. Instead using an array on node private data, use per-hook private data. Reestablish old ABI. PR: 240787 Submitted by: Lutz Donnerhacke (lutz donnerhacke.de) Changes: _U stable/12/ stable/12/share/man/man4/ng_bridge.4 stable/12/sys/netgraph/ng_bridge.c stable/12/sys/netgraph/ng_bridge.h
Merged to FreeBSD 12 as discussed in this thread with the follow-up work from Lutz to re-establish the old API.