Re: [PATCH net 1/1] net: bridge: cfm: use a per-bridge frame type

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