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

[Nikolay Aleksandrov <razor@blackwall.org>]

On 28/08/2026 22:44, Yuan Tan wrote:
> On Fri, Aug 28, 2026 at 3:23 AM Nikolay Aleksandrov <razor@blackwall.org> wrote:
>>
>> 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 <razor@blackwall.org> wrote:
>>>>>
>>>>> On 10/05/2026 13:05, Nikolay Aleksandrov wrote:
>>>>>> On 10/05/2026 12:15, Ren Wei wrote:
>>>>>>> From: Yilin Zhu <zylzyl2333@gmail.com>
>>>>>>>
>>>>>>> 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: stable@kernel.org
>>>>>>> Reported-by: Yuan Tan <yuantan098@gmail.com>
>>>>>>> Reported-by: Yifan Wu <yifanwucs@gmail.com>
>>>>>>> Reported-by: Juefei Pu <tomapufckgml@gmail.com>
>>>>>>> Reported-by: Xin Liu <bird@lzu.edu.cn>
>>>>>>> Co-developed-by: Peihan Liu <ronbogo@outlook.com>
>>>>>>> Signed-off-by: Peihan Liu <ronbogo@outlook.com>
>>>>>>> Signed-off-by: Yilin Zhu <zylzyl2333@gmail.com>
>>>>>>> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
>>>>>>> ---
>>>>>>>     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
>>
>>
> 
> Hi Nik,
> 
> I’m very sorry for the long delay. Yilin has gone back to continue his
> studies and unfortunately no longer has the time to finish this patch.
> 
> There was also an issue on my side in keeping track of this work, and
> I failed to notice that the patch had been left without a follow-up
> for so long. I apologize for that.
> 
> I can take over the patch, address the remaining feedback, and send a
> new version. That said, I’m not sure whether it might actually be
> easier for you to make the changes directly, if you are willing to do
> so. I’ve heard from maintainers that reviewing and iterating on
> someone else’s patch can sometimes take more effort than simply making
> the fix themselves, so I wanted to ask which approach you would
> prefer.
> 
> If you would prefer to fix the issue directly, could you please
> include the following tags?
> Reported-by: zylzyl2333@gmail.com
> Reported-by: vega@nebusec.ai
> 
> Otherwise, I’m happy to take over and prepare the next revision.
> What would you prefer?
> 
> Best,
> Yuan

Please take over and do the next revision. Just make sure the new cfm/mrp
option bits are checked together first and as an unlikely case.

Thanks,
  Nik