| Message ID | 20201009143530.2438738-2-henrik.bjoernlund@microchip.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | net: bridge: cfm: Add support for Connectivity Fault Management(CFM) | expand |
On Fri, 2020-10-09 at 14:35 +0000, Henrik Bjoernlund wrote: > This patch extends the processing of frames in the bridge. Currently MRP > frames needs special processing and the current implementation doesn't > allow a nice way to process different frame types. Therefore try to > improve this by adding a list that contains frame types that need > special processing. This list is iterated for each input frame and if > there is a match based on frame type then these functions will be called > and decide what to do with the frame. It can process the frame then the > bridge doesn't need to do anything or don't process so then the bridge > will do normal forwarding. > > Signed-off-by: Henrik Bjoernlund <henrik.bjoernlund@microchip.com> > Reviewed-by: Horatiu Vultur <horatiu.vultur@microchip.com> > --- > net/bridge/br_device.c | 1 + > net/bridge/br_input.c | 33 ++++++++++++++++++++++++++++++++- > net/bridge/br_mrp.c | 19 +++++++++++++++---- > net/bridge/br_private.h | 18 ++++++++++++------ > 4 files changed, 60 insertions(+), 11 deletions(-) > Acked-by: Nikolay Aleksandrov <nikolay@nvidia.com>
On Fri, 2020-10-09 at 14:35 +0000, Henrik Bjoernlund wrote: > This patch extends the processing of frames in the bridge. Currently MRP > frames needs special processing and the current implementation doesn't > allow a nice way to process different frame types. Therefore try to > improve this by adding a list that contains frame types that need > special processing. This list is iterated for each input frame and if > there is a match based on frame type then these functions will be called > and decide what to do with the frame. It can process the frame then the > bridge doesn't need to do anything or don't process so then the bridge > will do normal forwarding. > > Signed-off-by: Henrik Bjoernlund <henrik.bjoernlund@microchip.com> > Reviewed-by: Horatiu Vultur <horatiu.vultur@microchip.com> > --- > net/bridge/br_device.c | 1 + > net/bridge/br_input.c | 33 ++++++++++++++++++++++++++++++++- > net/bridge/br_mrp.c | 19 +++++++++++++++---- > net/bridge/br_private.h | 18 ++++++++++++------ > 4 files changed, 60 insertions(+), 11 deletions(-) > [snip] > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h > index 345118e35c42..3e62ce2ef8a5 100644 > --- a/net/bridge/br_private.h > +++ b/net/bridge/br_private.h > @@ -480,6 +480,8 @@ struct net_bridge { > #endif > struct hlist_head fdb_list; > > + struct hlist_head frame_type_list; Since there will be a v5, I'd suggest to move this struct member in the first cache line as it will be always used in the bridge fast-path for all cases. In order to make room for it there you can move port_list after fdb_hash_tbl and add this in its place, port_list is currently used only when flooding and soon I'll even change that. Thanks, Nik > + > #if IS_ENABLED(CONFIG_BRIDGE_MRP) > struct list_head mrp_list; > #endif > @@ -755,6 +757,16 @@ 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); > + > static inline bool br_rx_handler_check_rcu(const struct net_device *dev) > { > return rcu_dereference(dev->rx_handler) == br_get_rx_handler(dev); > @@ -1417,7 +1429,6 @@ extern int (*br_fdb_test_addr_hook)(struct net_device *dev, unsigned char *addr) > #if IS_ENABLED(CONFIG_BRIDGE_MRP) > int br_mrp_parse(struct net_bridge *br, struct net_bridge_port *p, > struct nlattr *attr, int cmd, struct netlink_ext_ack *extack); > -int br_mrp_process(struct net_bridge_port *p, struct sk_buff *skb); > bool br_mrp_enabled(struct net_bridge *br); > void br_mrp_port_del(struct net_bridge *br, struct net_bridge_port *p); > int br_mrp_fill_info(struct sk_buff *skb, struct net_bridge *br); > @@ -1429,11 +1440,6 @@ static inline int br_mrp_parse(struct net_bridge *br, struct net_bridge_port *p, > return -EOPNOTSUPP; > } > > -static inline int br_mrp_process(struct net_bridge_port *p, struct sk_buff *skb) > -{ > - return 0; > -} > - > static inline bool br_mrp_enabled(struct net_bridge *br) > { > return false;
Thanks for the review. Comments below. The 10/12/2020 09:12, Nikolay Aleksandrov wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On Fri, 2020-10-09 at 14:35 +0000, Henrik Bjoernlund wrote: > > This patch extends the processing of frames in the bridge. Currently MRP > > frames needs special processing and the current implementation doesn't > > allow a nice way to process different frame types. Therefore try to > > improve this by adding a list that contains frame types that need > > special processing. This list is iterated for each input frame and if > > there is a match based on frame type then these functions will be called > > and decide what to do with the frame. It can process the frame then the > > bridge doesn't need to do anything or don't process so then the bridge > > will do normal forwarding. > > > > Signed-off-by: Henrik Bjoernlund <henrik.bjoernlund@microchip.com> > > Reviewed-by: Horatiu Vultur <horatiu.vultur@microchip.com> > > --- > > net/bridge/br_device.c | 1 + > > net/bridge/br_input.c | 33 ++++++++++++++++++++++++++++++++- > > net/bridge/br_mrp.c | 19 +++++++++++++++---- > > net/bridge/br_private.h | 18 ++++++++++++------ > > 4 files changed, 60 insertions(+), 11 deletions(-) > > > [snip] > > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h > > index 345118e35c42..3e62ce2ef8a5 100644 > > --- a/net/bridge/br_private.h > > +++ b/net/bridge/br_private.h > > @@ -480,6 +480,8 @@ struct net_bridge { > > #endif > > struct hlist_head fdb_list; > > > > + struct hlist_head frame_type_list; > > Since there will be a v5, I'd suggest to move this struct member in the first > cache line as it will be always used in the bridge fast-path for all cases. > In order to make room for it there you can move port_list after fdb_hash_tbl and > add this in its place, port_list is currently used only when flooding and soon > I'll even change that. > > Thanks, > Nik > I will do change as requested. > > + > > #if IS_ENABLED(CONFIG_BRIDGE_MRP) > > struct list_head mrp_list; > > #endif > > @@ -755,6 +757,16 @@ 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); > > + > > static inline bool br_rx_handler_check_rcu(const struct net_device *dev) > > { > > return rcu_dereference(dev->rx_handler) == br_get_rx_handler(dev); > > @@ -1417,7 +1429,6 @@ extern int (*br_fdb_test_addr_hook)(struct net_device *dev, unsigned char *addr) > > #if IS_ENABLED(CONFIG_BRIDGE_MRP) > > int br_mrp_parse(struct net_bridge *br, struct net_bridge_port *p, > > struct nlattr *attr, int cmd, struct netlink_ext_ack *extack); > > -int br_mrp_process(struct net_bridge_port *p, struct sk_buff *skb); > > bool br_mrp_enabled(struct net_bridge *br); > > void br_mrp_port_del(struct net_bridge *br, struct net_bridge_port *p); > > int br_mrp_fill_info(struct sk_buff *skb, struct net_bridge *br); > > @@ -1429,11 +1440,6 @@ static inline int br_mrp_parse(struct net_bridge *br, struct net_bridge_port *p, > > return -EOPNOTSUPP; > > } > > > > -static inline int br_mrp_process(struct net_bridge_port *p, struct sk_buff *skb) > > -{ > > - return 0; > > -} > > - > > static inline bool br_mrp_enabled(struct net_bridge *br) > > { > > return false; >
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c index 9a2fb4aa1a10..206c4ba51cd2 100644 --- a/net/bridge/br_device.c +++ b/net/bridge/br_device.c @@ -473,6 +473,7 @@ void br_dev_setup(struct net_device *dev) spin_lock_init(&br->lock); INIT_LIST_HEAD(&br->port_list); INIT_HLIST_HEAD(&br->fdb_list); + INIT_HLIST_HEAD(&br->frame_type_list); #if IS_ENABLED(CONFIG_BRIDGE_MRP) INIT_LIST_HEAD(&br->mrp_list); #endif diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c index 59a318b9f646..bece03bf83c4 100644 --- a/net/bridge/br_input.c +++ b/net/bridge/br_input.c @@ -254,6 +254,21 @@ static int nf_hook_bridge_pre(struct sk_buff *skb, struct sk_buff **pskb) return RX_HANDLER_CONSUMED; } +/* Return 0 if the frame was not processed otherwise 1 + * note: already called with rcu_read_lock + */ +static int br_process_frame_type(struct net_bridge_port *p, + struct sk_buff *skb) +{ + struct br_frame_type *tmp; + + hlist_for_each_entry_rcu(tmp, &p->br->frame_type_list, list) + if (unlikely(tmp->type == skb->protocol)) + return tmp->frame_handler(p, skb); + + return 0; +} + /* * Return NULL if skb is handled * note: already called with rcu_read_lock @@ -343,7 +358,7 @@ static rx_handler_result_t br_handle_frame(struct sk_buff **pskb) } } - if (unlikely(br_mrp_process(p, skb))) + if (unlikely(br_process_frame_type(p, skb))) return RX_HANDLER_PASS; forward: @@ -380,3 +395,19 @@ rx_handler_func_t *br_get_rx_handler(const struct net_device *dev) return br_handle_frame; } + +void br_add_frame(struct net_bridge *br, struct br_frame_type *ft) +{ + hlist_add_head_rcu(&ft->list, &br->frame_type_list); +} + +void br_del_frame(struct net_bridge *br, struct br_frame_type *ft) +{ + struct br_frame_type *tmp; + + hlist_for_each_entry(tmp, &br->frame_type_list, list) + if (ft == tmp) { + hlist_del_rcu(&ft->list); + return; + } +} diff --git a/net/bridge/br_mrp.c b/net/bridge/br_mrp.c index b36689e6e7cb..f94d72bb7c32 100644 --- a/net/bridge/br_mrp.c +++ b/net/bridge/br_mrp.c @@ -6,6 +6,13 @@ static const u8 mrp_test_dmac[ETH_ALEN] = { 0x1, 0x15, 0x4e, 0x0, 0x0, 0x1 }; static const u8 mrp_in_test_dmac[ETH_ALEN] = { 0x1, 0x15, 0x4e, 0x0, 0x0, 0x3 }; +static int br_mrp_process(struct net_bridge_port *p, struct sk_buff *skb); + +static struct br_frame_type mrp_frame_type __read_mostly = { + .type = cpu_to_be16(ETH_P_MRP), + .frame_handler = br_mrp_process, +}; + static bool br_mrp_is_ring_port(struct net_bridge_port *p_port, struct net_bridge_port *s_port, struct net_bridge_port *port) @@ -445,6 +452,9 @@ static void br_mrp_del_impl(struct net_bridge *br, struct br_mrp *mrp) list_del_rcu(&mrp->list); kfree_rcu(mrp, rcu); + + if (list_empty(&br->mrp_list)) + br_del_frame(br, &mrp_frame_type); } /* Adds a new MRP instance. @@ -493,6 +503,9 @@ int br_mrp_add(struct net_bridge *br, struct br_mrp_instance *instance) spin_unlock_bh(&br->lock); rcu_assign_pointer(mrp->s_port, p); + if (list_empty(&br->mrp_list)) + br_add_frame(br, &mrp_frame_type); + INIT_DELAYED_WORK(&mrp->test_work, br_mrp_test_work_expired); INIT_DELAYED_WORK(&mrp->in_test_work, br_mrp_in_test_work_expired); list_add_tail_rcu(&mrp->list, &br->mrp_list); @@ -1172,15 +1185,13 @@ static int br_mrp_rcv(struct net_bridge_port *p, * normal forwarding. * note: already called with rcu_read_lock */ -int br_mrp_process(struct net_bridge_port *p, struct sk_buff *skb) +static int br_mrp_process(struct net_bridge_port *p, struct sk_buff *skb) { /* If there is no MRP instance do normal forwarding */ if (likely(!(p->flags & BR_MRP_AWARE))) goto out; - if (unlikely(skb->protocol == htons(ETH_P_MRP))) - return br_mrp_rcv(p, skb, p->dev); - + return br_mrp_rcv(p, skb, p->dev); out: return 0; } diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index 345118e35c42..3e62ce2ef8a5 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -480,6 +480,8 @@ struct net_bridge { #endif struct hlist_head fdb_list; + struct hlist_head frame_type_list; + #if IS_ENABLED(CONFIG_BRIDGE_MRP) struct list_head mrp_list; #endif @@ -755,6 +757,16 @@ 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); + static inline bool br_rx_handler_check_rcu(const struct net_device *dev) { return rcu_dereference(dev->rx_handler) == br_get_rx_handler(dev); @@ -1417,7 +1429,6 @@ extern int (*br_fdb_test_addr_hook)(struct net_device *dev, unsigned char *addr) #if IS_ENABLED(CONFIG_BRIDGE_MRP) int br_mrp_parse(struct net_bridge *br, struct net_bridge_port *p, struct nlattr *attr, int cmd, struct netlink_ext_ack *extack); -int br_mrp_process(struct net_bridge_port *p, struct sk_buff *skb); bool br_mrp_enabled(struct net_bridge *br); void br_mrp_port_del(struct net_bridge *br, struct net_bridge_port *p); int br_mrp_fill_info(struct sk_buff *skb, struct net_bridge *br); @@ -1429,11 +1440,6 @@ static inline int br_mrp_parse(struct net_bridge *br, struct net_bridge_port *p, return -EOPNOTSUPP; } -static inline int br_mrp_process(struct net_bridge_port *p, struct sk_buff *skb) -{ - return 0; -} - static inline bool br_mrp_enabled(struct net_bridge *br) { return false;