On 12/05/2026 11:09, Nikolay Aleksandrov wrote:
On 12/05/2026 08:21, Yilin Zhu wrote:
On Sun, 10 May 2026 at 04:40, Nikolay Aleksandrov <[email protected]> wrote:
On 10/05/2026 13:05, Nikolay Aleksandrov wrote:
On 10/05/2026 12:15, Ren Wei wrote:
From: Yilin Zhu <[email protected]>
CFM registers a bridge frame handler when the first MEP is created and
unregisters it when the last MEP is deleted. The registered object also
contains the hlist_node used by the bridge-local frame_type_list.
The CFM frame type is currently global, so enabling CFM on multiple
bridges links the same hlist_node into multiple bridge-local lists. A
later unregister on one bridge can then operate on list state belonging
to another bridge.
Move the CFM frame type into struct net_bridge and register/unregister
the bridge-owned object. This keeps frame handler list membership local
to the bridge while preserving the existing first-MEP/last-MEP lifetime.
Fixes: dc32cbb3dbd7 ("bridge: cfm: Kernel space implementation of CFM. CCM
frame RX added.")
Cc: [email protected]
Reported-by: Yuan Tan <[email protected]>
Reported-by: Yifan Wu <[email protected]>
Reported-by: Juefei Pu <[email protected]>
Reported-by: Xin Liu <[email protected]>
Co-developed-by: Peihan Liu <[email protected]>
Signed-off-by: Peihan Liu <[email protected]>
Signed-off-by: Yilin Zhu <[email protected]>
Signed-off-by: Ren Wei <[email protected]>
---
net/bridge/br_cfm.c | 14 ++++++--------
net/bridge/br_private.h | 18 +++++++++++-------
2 files changed, 17 insertions(+), 15 deletions(-)
I think MRP suffers from the same bug, but I also think we can contain the
fix within the packet type structure instead of making the already huge
struct net_bridge even bigger. Also, linking a struct within net_bridge
to a list within the same net_bridge looks weird.
IMO br_add_frame should take a type & a frame_handler, allocate a br_frame_type
dynamically and link it, then br_del_frame should take a type instead of a ptr
and remove that frame type and free it with kfree_rcu. That would require
br_add_frame return value to be checked in the respective cfm/mrp functions.
Warnings for already existing types on add or missing types on del should be
added.
Actually I have a better idea since these handlers can be added only once per
bridge, we can do away with a simple bitmask (e.g. use net_bridge's options
which is in a Rx hot cache line) and even reduce net_bridge size while fixing
these bugs, and also remove a conditional from the fast-path when CFM/MRP are
not compiled in. Would you like me to prepare it or do you want to give it a go?
Hi Nik,
Thanks for the suggestion.
I'll give it a go. IIUC, the idea is to remove the shared
br_frame_type list entries for CFM/MRP, track their per-bridge enable
state with bridge option bits, and dispatch the CFM/MRP handlers directly
from the receive path when the corresponding EtherType and option bit
match.
I'll include MRP in v2.
Thanks,
Yilin
Great, thank you. That should simplify the code and give us 8 bytes of Rx hot
cache line back. The important point is that there haven't been any new
handlers since these were added back in 2020, and also these are not
common, so making their impact on the fast-path as small as possible
while fixing the bug sounds good.
A quick & dirty sketch of the fixes gives me:
5 files changed, 47 insertions(+), 66 deletions(-)
That can probably be improved (i.e. more deletions).
Just please make sure that if CFM/MRP are not enabled in .config, they
would not affect the fast-path at all.
Cheers,
Nik
Hi Yilin,
Are you still considering fixing these problems? A few months passed since
we last spoke.
Cheers,
Nik
