* [PATCH net 1/1] net: bridge: cfm: use a per-bridge frame type [not found] <cover.1778378864.git.zylzyl2333@gmail.com> @ 2026-05-10 9:15 ` Ren Wei 2026-05-10 10:05 ` Nikolay Aleksandrov 0 siblings, 1 reply; 8+ messages in thread From: Ren Wei @ 2026-05-10 9:15 UTC (permalink / raw) To: bridge, netdev Cc: razor, idosch, horatiu.vultur, kuba, henrik.bjoernlund, yuantan098, yifanwucs, tomapufckgml, bird, ronbogo, zylzyl2333, n05ec 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(-) diff --git a/net/bridge/br_cfm.c b/net/bridge/br_cfm.c index 118c7ea48c35..547c7415c0ea 100644 --- a/net/bridge/br_cfm.c +++ b/net/bridge/br_cfm.c @@ -489,11 +489,6 @@ static int br_cfm_frame_rx(struct net_bridge_port *port, struct sk_buff *skb) return 1; } -static struct br_frame_type cfm_frame_type __read_mostly = { - .type = cpu_to_be16(ETH_P_CFM), - .frame_handler = br_cfm_frame_rx, -}; - int br_cfm_mep_create(struct net_bridge *br, const u32 instance, struct br_cfm_mep_create *const create, @@ -558,8 +553,11 @@ int br_cfm_mep_create(struct net_bridge *br, INIT_HLIST_HEAD(&mep->peer_mep_list); INIT_DELAYED_WORK(&mep->ccm_tx_dwork, ccm_tx_work_expired); - if (hlist_empty(&br->mep_list)) - br_add_frame(br, &cfm_frame_type); + if (hlist_empty(&br->mep_list)) { + br->cfm_frame_type.type = cpu_to_be16(ETH_P_CFM); + br->cfm_frame_type.frame_handler = br_cfm_frame_rx; + br_add_frame(br, &br->cfm_frame_type); + } hlist_add_tail_rcu(&mep->head, &br->mep_list); @@ -588,7 +586,7 @@ static void mep_delete_implementation(struct net_bridge *br, kfree_rcu(mep, rcu); if (hlist_empty(&br->mep_list)) - br_del_frame(br, &cfm_frame_type); + br_del_frame(br, &br->cfm_frame_type); } int br_cfm_mep_delete(struct net_bridge *br, diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index bed1b1d9b282..a3ed9ee826f5 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -61,6 +61,9 @@ typedef struct bridge_id bridge_id; typedef struct mac_addr mac_addr; typedef __u16 port_id; +struct net_bridge_port; +struct sk_buff; + struct bridge_id { unsigned char prio[2]; unsigned char addr[ETH_ALEN]; @@ -70,6 +73,13 @@ struct mac_addr { unsigned char addr[ETH_ALEN]; }; +struct br_frame_type { + __be16 type; + int (*frame_handler)(struct net_bridge_port *port, + struct sk_buff *skb); + struct hlist_node list; +}; + #ifdef CONFIG_BRIDGE_IGMP_SNOOPING /* our own querier */ struct bridge_mcast_own_query { @@ -585,6 +595,7 @@ struct net_bridge { struct hlist_head mrp_list; #endif #if IS_ENABLED(CONFIG_BRIDGE_CFM) + struct br_frame_type cfm_frame_type; struct hlist_head mep_list; #endif }; @@ -926,13 +937,6 @@ int nbp_backup_change(struct net_bridge_port *p, struct net_device *backup_dev); int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb); rx_handler_func_t *br_get_rx_handler(const struct net_device *dev); -struct br_frame_type { - __be16 type; - int (*frame_handler)(struct net_bridge_port *port, - struct sk_buff *skb); - struct hlist_node list; -}; - void br_add_frame(struct net_bridge *br, struct br_frame_type *ft); void br_del_frame(struct net_bridge *br, struct br_frame_type *ft); -- 2.47.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net 1/1] net: bridge: cfm: use a per-bridge frame type 2026-05-10 9:15 ` [PATCH net 1/1] net: bridge: cfm: use a per-bridge frame type Ren Wei @ 2026-05-10 10:05 ` Nikolay Aleksandrov 2026-05-10 11:40 ` Nikolay Aleksandrov 0 siblings, 1 reply; 8+ messages in thread From: Nikolay Aleksandrov @ 2026-05-10 10:05 UTC (permalink / raw) To: Ren Wei, bridge, netdev Cc: idosch, horatiu.vultur, kuba, henrik.bjoernlund, yuantan098, yifanwucs, tomapufckgml, bird, ronbogo, zylzyl2333 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. Would you please take care of MRP as well? Cheers, Nik > diff --git a/net/bridge/br_cfm.c b/net/bridge/br_cfm.c > index 118c7ea48c35..547c7415c0ea 100644 > --- a/net/bridge/br_cfm.c > +++ b/net/bridge/br_cfm.c > @@ -489,11 +489,6 @@ static int br_cfm_frame_rx(struct net_bridge_port *port, struct sk_buff *skb) > return 1; > } > > -static struct br_frame_type cfm_frame_type __read_mostly = { > - .type = cpu_to_be16(ETH_P_CFM), > - .frame_handler = br_cfm_frame_rx, > -}; > - > int br_cfm_mep_create(struct net_bridge *br, > const u32 instance, > struct br_cfm_mep_create *const create, > @@ -558,8 +553,11 @@ int br_cfm_mep_create(struct net_bridge *br, > INIT_HLIST_HEAD(&mep->peer_mep_list); > INIT_DELAYED_WORK(&mep->ccm_tx_dwork, ccm_tx_work_expired); > > - if (hlist_empty(&br->mep_list)) > - br_add_frame(br, &cfm_frame_type); > + if (hlist_empty(&br->mep_list)) { > + br->cfm_frame_type.type = cpu_to_be16(ETH_P_CFM); > + br->cfm_frame_type.frame_handler = br_cfm_frame_rx; > + br_add_frame(br, &br->cfm_frame_type); > + } > > hlist_add_tail_rcu(&mep->head, &br->mep_list); > > @@ -588,7 +586,7 @@ static void mep_delete_implementation(struct net_bridge *br, > kfree_rcu(mep, rcu); > > if (hlist_empty(&br->mep_list)) > - br_del_frame(br, &cfm_frame_type); > + br_del_frame(br, &br->cfm_frame_type); > } > > int br_cfm_mep_delete(struct net_bridge *br, > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h > index bed1b1d9b282..a3ed9ee826f5 100644 > --- a/net/bridge/br_private.h > +++ b/net/bridge/br_private.h > @@ -61,6 +61,9 @@ typedef struct bridge_id bridge_id; > typedef struct mac_addr mac_addr; > typedef __u16 port_id; > > +struct net_bridge_port; > +struct sk_buff; > + > struct bridge_id { > unsigned char prio[2]; > unsigned char addr[ETH_ALEN]; > @@ -70,6 +73,13 @@ struct mac_addr { > unsigned char addr[ETH_ALEN]; > }; > > +struct br_frame_type { > + __be16 type; > + int (*frame_handler)(struct net_bridge_port *port, > + struct sk_buff *skb); > + struct hlist_node list; > +}; > + > #ifdef CONFIG_BRIDGE_IGMP_SNOOPING > /* our own querier */ > struct bridge_mcast_own_query { > @@ -585,6 +595,7 @@ struct net_bridge { > struct hlist_head mrp_list; > #endif > #if IS_ENABLED(CONFIG_BRIDGE_CFM) > + struct br_frame_type cfm_frame_type; > struct hlist_head mep_list; > #endif > }; > @@ -926,13 +937,6 @@ int nbp_backup_change(struct net_bridge_port *p, struct net_device *backup_dev); > int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb); > rx_handler_func_t *br_get_rx_handler(const struct net_device *dev); > > -struct br_frame_type { > - __be16 type; > - int (*frame_handler)(struct net_bridge_port *port, > - struct sk_buff *skb); > - struct hlist_node list; > -}; > - > void br_add_frame(struct net_bridge *br, struct br_frame_type *ft); > void br_del_frame(struct net_bridge *br, struct br_frame_type *ft); > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net 1/1] net: bridge: cfm: use a per-bridge frame type 2026-05-10 10:05 ` Nikolay Aleksandrov @ 2026-05-10 11:40 ` Nikolay Aleksandrov 2026-05-12 5:21 ` Yilin Zhu 0 siblings, 1 reply; 8+ messages in thread From: Nikolay Aleksandrov @ 2026-05-10 11:40 UTC (permalink / raw) To: Ren Wei, bridge, netdev Cc: idosch, horatiu.vultur, kuba, henrik.bjoernlund, yuantan098, yifanwucs, tomapufckgml, bird, ronbogo, zylzyl2333 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? > Would you please take care of MRP as well? > > Cheers, > Nik > >> diff --git a/net/bridge/br_cfm.c b/net/bridge/br_cfm.c >> index 118c7ea48c35..547c7415c0ea 100644 >> --- a/net/bridge/br_cfm.c >> +++ b/net/bridge/br_cfm.c >> @@ -489,11 +489,6 @@ static int br_cfm_frame_rx(struct net_bridge_port *port, >> struct sk_buff *skb) >> return 1; >> } >> -static struct br_frame_type cfm_frame_type __read_mostly = { >> - .type = cpu_to_be16(ETH_P_CFM), >> - .frame_handler = br_cfm_frame_rx, >> -}; >> - >> int br_cfm_mep_create(struct net_bridge *br, >> const u32 instance, >> struct br_cfm_mep_create *const create, >> @@ -558,8 +553,11 @@ int br_cfm_mep_create(struct net_bridge *br, >> INIT_HLIST_HEAD(&mep->peer_mep_list); >> INIT_DELAYED_WORK(&mep->ccm_tx_dwork, ccm_tx_work_expired); >> - if (hlist_empty(&br->mep_list)) >> - br_add_frame(br, &cfm_frame_type); >> + if (hlist_empty(&br->mep_list)) { >> + br->cfm_frame_type.type = cpu_to_be16(ETH_P_CFM); >> + br->cfm_frame_type.frame_handler = br_cfm_frame_rx; >> + br_add_frame(br, &br->cfm_frame_type); >> + } >> hlist_add_tail_rcu(&mep->head, &br->mep_list); >> @@ -588,7 +586,7 @@ static void mep_delete_implementation(struct net_bridge *br, >> kfree_rcu(mep, rcu); >> if (hlist_empty(&br->mep_list)) >> - br_del_frame(br, &cfm_frame_type); >> + br_del_frame(br, &br->cfm_frame_type); >> } >> int br_cfm_mep_delete(struct net_bridge *br, >> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h >> index bed1b1d9b282..a3ed9ee826f5 100644 >> --- a/net/bridge/br_private.h >> +++ b/net/bridge/br_private.h >> @@ -61,6 +61,9 @@ typedef struct bridge_id bridge_id; >> typedef struct mac_addr mac_addr; >> typedef __u16 port_id; >> +struct net_bridge_port; >> +struct sk_buff; >> + >> struct bridge_id { >> unsigned char prio[2]; >> unsigned char addr[ETH_ALEN]; >> @@ -70,6 +73,13 @@ struct mac_addr { >> unsigned char addr[ETH_ALEN]; >> }; >> +struct br_frame_type { >> + __be16 type; >> + int (*frame_handler)(struct net_bridge_port *port, >> + struct sk_buff *skb); >> + struct hlist_node list; >> +}; >> + >> #ifdef CONFIG_BRIDGE_IGMP_SNOOPING >> /* our own querier */ >> struct bridge_mcast_own_query { >> @@ -585,6 +595,7 @@ struct net_bridge { >> struct hlist_head mrp_list; >> #endif >> #if IS_ENABLED(CONFIG_BRIDGE_CFM) >> + struct br_frame_type cfm_frame_type; >> struct hlist_head mep_list; >> #endif >> }; >> @@ -926,13 +937,6 @@ int nbp_backup_change(struct net_bridge_port *p, struct >> net_device *backup_dev); >> int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff >> *skb); >> rx_handler_func_t *br_get_rx_handler(const struct net_device *dev); >> -struct br_frame_type { >> - __be16 type; >> - int (*frame_handler)(struct net_bridge_port *port, >> - struct sk_buff *skb); >> - struct hlist_node list; >> -}; >> - >> void br_add_frame(struct net_bridge *br, struct br_frame_type *ft); >> void br_del_frame(struct net_bridge *br, struct br_frame_type *ft); > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net 1/1] net: bridge: cfm: use a per-bridge frame type 2026-05-10 11:40 ` Nikolay Aleksandrov @ 2026-05-12 5:21 ` Yilin Zhu 2026-05-12 8:09 ` Nikolay Aleksandrov 0 siblings, 1 reply; 8+ messages in thread From: Yilin Zhu @ 2026-05-12 5:21 UTC (permalink / raw) To: Nikolay Aleksandrov Cc: Ren Wei, bridge, netdev, idosch, horatiu.vultur, kuba, henrik.bjoernlund, yuantan098, yifanwucs, tomapufckgml, bird, ronbogo 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 > > Would you please take care of MRP as well? > > > > Cheers, > > Nik > > > >> diff --git a/net/bridge/br_cfm.c b/net/bridge/br_cfm.c > >> index 118c7ea48c35..547c7415c0ea 100644 > >> --- a/net/bridge/br_cfm.c > >> +++ b/net/bridge/br_cfm.c > >> @@ -489,11 +489,6 @@ static int br_cfm_frame_rx(struct net_bridge_port *port, > >> struct sk_buff *skb) > >> return 1; > >> } > >> -static struct br_frame_type cfm_frame_type __read_mostly = { > >> - .type = cpu_to_be16(ETH_P_CFM), > >> - .frame_handler = br_cfm_frame_rx, > >> -}; > >> - > >> int br_cfm_mep_create(struct net_bridge *br, > >> const u32 instance, > >> struct br_cfm_mep_create *const create, > >> @@ -558,8 +553,11 @@ int br_cfm_mep_create(struct net_bridge *br, > >> INIT_HLIST_HEAD(&mep->peer_mep_list); > >> INIT_DELAYED_WORK(&mep->ccm_tx_dwork, ccm_tx_work_expired); > >> - if (hlist_empty(&br->mep_list)) > >> - br_add_frame(br, &cfm_frame_type); > >> + if (hlist_empty(&br->mep_list)) { > >> + br->cfm_frame_type.type = cpu_to_be16(ETH_P_CFM); > >> + br->cfm_frame_type.frame_handler = br_cfm_frame_rx; > >> + br_add_frame(br, &br->cfm_frame_type); > >> + } > >> hlist_add_tail_rcu(&mep->head, &br->mep_list); > >> @@ -588,7 +586,7 @@ static void mep_delete_implementation(struct net_bridge *br, > >> kfree_rcu(mep, rcu); > >> if (hlist_empty(&br->mep_list)) > >> - br_del_frame(br, &cfm_frame_type); > >> + br_del_frame(br, &br->cfm_frame_type); > >> } > >> int br_cfm_mep_delete(struct net_bridge *br, > >> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h > >> index bed1b1d9b282..a3ed9ee826f5 100644 > >> --- a/net/bridge/br_private.h > >> +++ b/net/bridge/br_private.h > >> @@ -61,6 +61,9 @@ typedef struct bridge_id bridge_id; > >> typedef struct mac_addr mac_addr; > >> typedef __u16 port_id; > >> +struct net_bridge_port; > >> +struct sk_buff; > >> + > >> struct bridge_id { > >> unsigned char prio[2]; > >> unsigned char addr[ETH_ALEN]; > >> @@ -70,6 +73,13 @@ struct mac_addr { > >> unsigned char addr[ETH_ALEN]; > >> }; > >> +struct br_frame_type { > >> + __be16 type; > >> + int (*frame_handler)(struct net_bridge_port *port, > >> + struct sk_buff *skb); > >> + struct hlist_node list; > >> +}; > >> + > >> #ifdef CONFIG_BRIDGE_IGMP_SNOOPING > >> /* our own querier */ > >> struct bridge_mcast_own_query { > >> @@ -585,6 +595,7 @@ struct net_bridge { > >> struct hlist_head mrp_list; > >> #endif > >> #if IS_ENABLED(CONFIG_BRIDGE_CFM) > >> + struct br_frame_type cfm_frame_type; > >> struct hlist_head mep_list; > >> #endif > >> }; > >> @@ -926,13 +937,6 @@ int nbp_backup_change(struct net_bridge_port *p, struct > >> net_device *backup_dev); > >> int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff > >> *skb); > >> rx_handler_func_t *br_get_rx_handler(const struct net_device *dev); > >> -struct br_frame_type { > >> - __be16 type; > >> - int (*frame_handler)(struct net_bridge_port *port, > >> - struct sk_buff *skb); > >> - struct hlist_node list; > >> -}; > >> - > >> void br_add_frame(struct net_bridge *br, struct br_frame_type *ft); > >> void br_del_frame(struct net_bridge *br, struct br_frame_type *ft); > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net 1/1] net: bridge: cfm: use a per-bridge frame type 2026-05-12 5:21 ` Yilin Zhu @ 2026-05-12 8:09 ` Nikolay Aleksandrov 2026-08-28 10:23 ` Nikolay Aleksandrov 0 siblings, 1 reply; 8+ messages in thread From: Nikolay Aleksandrov @ 2026-05-12 8:09 UTC (permalink / raw) To: Yilin Zhu Cc: Ren Wei, bridge, netdev, idosch, horatiu.vultur, kuba, henrik.bjoernlund, yuantan098, yifanwucs, tomapufckgml, bird, ronbogo 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 >>> Would you please take care of MRP as well? >>> >>> Cheers, >>> Nik >>> >>>> diff --git a/net/bridge/br_cfm.c b/net/bridge/br_cfm.c >>>> index 118c7ea48c35..547c7415c0ea 100644 >>>> --- a/net/bridge/br_cfm.c >>>> +++ b/net/bridge/br_cfm.c >>>> @@ -489,11 +489,6 @@ static int br_cfm_frame_rx(struct net_bridge_port *port, >>>> struct sk_buff *skb) >>>> return 1; >>>> } >>>> -static struct br_frame_type cfm_frame_type __read_mostly = { >>>> - .type = cpu_to_be16(ETH_P_CFM), >>>> - .frame_handler = br_cfm_frame_rx, >>>> -}; >>>> - >>>> int br_cfm_mep_create(struct net_bridge *br, >>>> const u32 instance, >>>> struct br_cfm_mep_create *const create, >>>> @@ -558,8 +553,11 @@ int br_cfm_mep_create(struct net_bridge *br, >>>> INIT_HLIST_HEAD(&mep->peer_mep_list); >>>> INIT_DELAYED_WORK(&mep->ccm_tx_dwork, ccm_tx_work_expired); >>>> - if (hlist_empty(&br->mep_list)) >>>> - br_add_frame(br, &cfm_frame_type); >>>> + if (hlist_empty(&br->mep_list)) { >>>> + br->cfm_frame_type.type = cpu_to_be16(ETH_P_CFM); >>>> + br->cfm_frame_type.frame_handler = br_cfm_frame_rx; >>>> + br_add_frame(br, &br->cfm_frame_type); >>>> + } >>>> hlist_add_tail_rcu(&mep->head, &br->mep_list); >>>> @@ -588,7 +586,7 @@ static void mep_delete_implementation(struct net_bridge *br, >>>> kfree_rcu(mep, rcu); >>>> if (hlist_empty(&br->mep_list)) >>>> - br_del_frame(br, &cfm_frame_type); >>>> + br_del_frame(br, &br->cfm_frame_type); >>>> } >>>> int br_cfm_mep_delete(struct net_bridge *br, >>>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h >>>> index bed1b1d9b282..a3ed9ee826f5 100644 >>>> --- a/net/bridge/br_private.h >>>> +++ b/net/bridge/br_private.h >>>> @@ -61,6 +61,9 @@ typedef struct bridge_id bridge_id; >>>> typedef struct mac_addr mac_addr; >>>> typedef __u16 port_id; >>>> +struct net_bridge_port; >>>> +struct sk_buff; >>>> + >>>> struct bridge_id { >>>> unsigned char prio[2]; >>>> unsigned char addr[ETH_ALEN]; >>>> @@ -70,6 +73,13 @@ struct mac_addr { >>>> unsigned char addr[ETH_ALEN]; >>>> }; >>>> +struct br_frame_type { >>>> + __be16 type; >>>> + int (*frame_handler)(struct net_bridge_port *port, >>>> + struct sk_buff *skb); >>>> + struct hlist_node list; >>>> +}; >>>> + >>>> #ifdef CONFIG_BRIDGE_IGMP_SNOOPING >>>> /* our own querier */ >>>> struct bridge_mcast_own_query { >>>> @@ -585,6 +595,7 @@ struct net_bridge { >>>> struct hlist_head mrp_list; >>>> #endif >>>> #if IS_ENABLED(CONFIG_BRIDGE_CFM) >>>> + struct br_frame_type cfm_frame_type; >>>> struct hlist_head mep_list; >>>> #endif >>>> }; >>>> @@ -926,13 +937,6 @@ int nbp_backup_change(struct net_bridge_port *p, struct >>>> net_device *backup_dev); >>>> int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff >>>> *skb); >>>> rx_handler_func_t *br_get_rx_handler(const struct net_device *dev); >>>> -struct br_frame_type { >>>> - __be16 type; >>>> - int (*frame_handler)(struct net_bridge_port *port, >>>> - struct sk_buff *skb); >>>> - struct hlist_node list; >>>> -}; >>>> - >>>> void br_add_frame(struct net_bridge *br, struct br_frame_type *ft); >>>> void br_del_frame(struct net_bridge *br, struct br_frame_type *ft); >>> >> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net 1/1] net: bridge: cfm: use a per-bridge frame type 2026-05-12 8:09 ` Nikolay Aleksandrov @ 2026-08-28 10:23 ` Nikolay Aleksandrov 2026-08-28 19:44 ` Yuan Tan 0 siblings, 1 reply; 8+ messages in thread From: Nikolay Aleksandrov @ 2026-08-28 10:23 UTC (permalink / raw) To: Yilin Zhu Cc: Ren Wei, bridge, netdev, idosch, horatiu.vultur, kuba, henrik.bjoernlund, yuantan098, yifanwucs, tomapufckgml, bird, ronbogo 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net 1/1] net: bridge: cfm: use a per-bridge frame type 2026-08-28 10:23 ` Nikolay Aleksandrov @ 2026-08-28 19:44 ` Yuan Tan 2026-08-30 8:29 ` Nikolay Aleksandrov 0 siblings, 1 reply; 8+ messages in thread From: Yuan Tan @ 2026-08-28 19:44 UTC (permalink / raw) To: Nikolay Aleksandrov Cc: Yilin Zhu, Ren Wei, bridge, netdev, idosch, horatiu.vultur, kuba, henrik.bjoernlund, yifanwucs, tomapufckgml, bird, ronbogo 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net 1/1] net: bridge: cfm: use a per-bridge frame type 2026-08-28 19:44 ` Yuan Tan @ 2026-08-30 8:29 ` Nikolay Aleksandrov 0 siblings, 0 replies; 8+ messages in thread From: Nikolay Aleksandrov @ 2026-08-30 8:29 UTC (permalink / raw) To: Yuan Tan Cc: Yilin Zhu, Ren Wei, bridge, netdev, idosch, horatiu.vultur, kuba, henrik.bjoernlund, yifanwucs, tomapufckgml, bird, ronbogo 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-08-30 8:29 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.1778378864.git.zylzyl2333@gmail.com>
2026-05-10 9:15 ` [PATCH net 1/1] net: bridge: cfm: use a per-bridge frame type Ren Wei
2026-05-10 10:05 ` Nikolay Aleksandrov
2026-05-10 11:40 ` Nikolay Aleksandrov
2026-05-12 5:21 ` Yilin Zhu
2026-05-12 8:09 ` Nikolay Aleksandrov
2026-08-28 10:23 ` Nikolay Aleksandrov
2026-08-28 19:44 ` Yuan Tan
2026-08-30 8:29 ` Nikolay Aleksandrov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox