Re: [PATCH net-next v4 01/10] net: bridge: extend the process of special frames

[Nikolay Aleksandrov]

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@xxxxxxxxxxxxx>
> Reviewed-by: Horatiu Vultur  <horatiu.vultur@xxxxxxxxxxxxx>
> ---
>  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;