| Message ID | 20201001103019.1342470-2-henrik.bjoernlund@microchip.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | net: bridge: cfm: Add support for Connectivity Fault Management(CFM) | expand |
Hi Henrik,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Henrik-Bjoernlund/net-bridge-cfm-Add-support-for-Connectivity-Fault-Management-CFM/20201001-184031
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git f2e834694b0d92187d889172da842e27829df371
config: c6x-randconfig-r032-20200930 (attached as .config)
compiler: c6x-elf-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/1c0b81655468c16fd143f325138a856ca7727071
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Henrik-Bjoernlund/net-bridge-cfm-Add-support-for-Connectivity-Fault-Management-CFM/20201001-184031
git checkout 1c0b81655468c16fd143f325138a856ca7727071
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=c6x
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
In file included from net/bridge/br_private_mrp.h:6,
from net/bridge/br_mrp.c:4:
>> net/bridge/br_private.h:94:32: warning: 'struct net_bridge_port' declared inside parameter list will not be visible outside of this definition or declaration
94 | int (*frame_handler)(struct net_bridge_port *port,
| ^~~~~~~~~~~~~~~
>> net/bridge/br_mrp.c:13:19: error: initialization of 'int (*)(struct net_bridge_port *, struct sk_buff *)' from incompatible pointer type 'int (*)(struct net_bridge_port *, struct sk_buff *)' [-Werror=incompatible-pointer-types]
13 | .frame_handler = br_mrp_process,
| ^~~~~~~~~~~~~~
net/bridge/br_mrp.c:13:19: note: (near initialization for 'mrp_frame_type.frame_handler')
cc1: some warnings being treated as errors
--
In file included from net/bridge/br.c:20:
>> net/bridge/br_private.h:94:32: warning: 'struct net_bridge_port' declared inside parameter list will not be visible outside of this definition or declaration
94 | int (*frame_handler)(struct net_bridge_port *port,
| ^~~~~~~~~~~~~~~
--
In file included from include/trace/events/bridge.h:10,
from net/bridge/br_fdb.c:24:
>> include/trace/events/../../../net/bridge/br_private.h:94:32: warning: 'struct net_bridge_port' declared inside parameter list will not be visible outside of this definition or declaration
94 | int (*frame_handler)(struct net_bridge_port *port,
| ^~~~~~~~~~~~~~~
--
In file included from net/bridge/br_input.c:23:
>> net/bridge/br_private.h:94:32: warning: 'struct net_bridge_port' declared inside parameter list will not be visible outside of this definition or declaration
94 | int (*frame_handler)(struct net_bridge_port *port,
| ^~~~~~~~~~~~~~~
net/bridge/br_input.c: In function 'br_process_frame_type':
>> net/bridge/br_input.c:267:30: error: passing argument 1 of 'tmp->frame_handler' from incompatible pointer type [-Werror=incompatible-pointer-types]
267 | return tmp->frame_handler(p, skb);
| ^
| |
| struct net_bridge_port *
net/bridge/br_input.c:267:30: note: expected 'struct net_bridge_port *' but argument is of type 'struct net_bridge_port *'
cc1: some warnings being treated as errors
--
In file included from net/bridge/br_netlink_tunnel.c:18:
>> net/bridge/br_private.h:94:32: warning: 'struct net_bridge_port' declared inside parameter list will not be visible outside of this definition or declaration
94 | int (*frame_handler)(struct net_bridge_port *port,
| ^~~~~~~~~~~~~~~
net/bridge/br_netlink_tunnel.c:29:6: warning: no previous prototype for 'vlan_tunid_inrange' [-Wmissing-prototypes]
29 | bool vlan_tunid_inrange(const struct net_bridge_vlan *v_curr,
| ^~~~~~~~~~~~~~~~~~
net/bridge/br_netlink_tunnel.c:196:5: warning: no previous prototype for 'br_vlan_tunnel_info' [-Wmissing-prototypes]
196 | int br_vlan_tunnel_info(const struct net_bridge_port *p, int cmd,
| ^~~~~~~~~~~~~~~~~~~
--
In file included from net/atm/lec.c:36:
>> net/atm/../bridge/br_private.h:94:32: warning: 'struct net_bridge_port' declared inside parameter list will not be visible outside of this definition or declaration
94 | int (*frame_handler)(struct net_bridge_port *port,
| ^~~~~~~~~~~~~~~
vim +13 net/bridge/br_mrp.c
10
11 static struct br_frame_type mrp_frame_type __read_mostly = {
12 .type = cpu_to_be16(ETH_P_MRP),
> 13 .frame_handler = br_mrp_process,
14 };
15
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Henrik,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Henrik-Bjoernlund/net-bridge-cfm-Add-support-for-Connectivity-Fault-Management-CFM/20201001-184031
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git f2e834694b0d92187d889172da842e27829df371
config: x86_64-randconfig-a005-20200930 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project bcd05599d0e53977a963799d6ee4f6e0bc21331b)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# https://github.com/0day-ci/linux/commit/1c0b81655468c16fd143f325138a856ca7727071
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Henrik-Bjoernlund/net-bridge-cfm-Add-support-for-Connectivity-Fault-Management-CFM/20201001-184031
git checkout 1c0b81655468c16fd143f325138a856ca7727071
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from net/ipv6/netfilter.c:21:
>> net/ipv6/../bridge/br_private.h:94:32: warning: declaration of 'struct net_bridge_port' will not be visible outside of this function [-Wvisibility]
int (*frame_handler)(struct net_bridge_port *port,
^
1 warning generated.
vim +94 net/ipv6/../bridge/br_private.h
91
92 struct br_frame_type {
93 __be16 type;
> 94 int (*frame_handler)(struct net_bridge_port *port,
95 struct sk_buff *skb);
96 struct hlist_node list;
97 };
98
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Henrik,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Henrik-Bjoernlund/net-bridge-cfm-Add-support-for-Connectivity-Fault-Management-CFM/20201001-184031
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git f2e834694b0d92187d889172da842e27829df371
config: x86_64-randconfig-a003-20200930 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project bcd05599d0e53977a963799d6ee4f6e0bc21331b)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# https://github.com/0day-ci/linux/commit/1c0b81655468c16fd143f325138a856ca7727071
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Henrik-Bjoernlund/net-bridge-cfm-Add-support-for-Connectivity-Fault-Management-CFM/20201001-184031
git checkout 1c0b81655468c16fd143f325138a856ca7727071
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
In file included from net/atm/lec.c:36:
>> net/atm/../bridge/br_private.h:94:32: warning: declaration of 'struct net_bridge_port' will not be visible outside of this function [-Wvisibility]
int (*frame_handler)(struct net_bridge_port *port,
^
1 warning generated.
--
In file included from net/bridge/br_sysfs_if.c:18:
>> net/bridge/br_private.h:94:32: warning: declaration of 'struct net_bridge_port' will not be visible outside of this function [-Wvisibility]
int (*frame_handler)(struct net_bridge_port *port,
^
1 warning generated.
--
In file included from net/bridge/br_input.c:23:
>> net/bridge/br_private.h:94:32: warning: declaration of 'struct net_bridge_port' will not be visible outside of this function [-Wvisibility]
int (*frame_handler)(struct net_bridge_port *port,
^
>> net/bridge/br_input.c:267:30: error: incompatible pointer types passing 'struct net_bridge_port *' to parameter of type 'struct net_bridge_port *' [-Werror,-Wincompatible-pointer-types]
return tmp->frame_handler(p, skb);
^
1 warning and 1 error generated.
--
In file included from net/bridge/br_mrp.c:4:
In file included from net/bridge/br_private_mrp.h:6:
>> net/bridge/br_private.h:94:32: warning: declaration of 'struct net_bridge_port' will not be visible outside of this function [-Wvisibility]
int (*frame_handler)(struct net_bridge_port *port,
^
>> net/bridge/br_mrp.c:13:19: error: incompatible function pointer types initializing 'int (*)(struct net_bridge_port *, struct sk_buff *)' with an expression of type 'int (struct net_bridge_port *, struct sk_buff *)' [-Werror,-Wincompatible-function-pointer-types]
.frame_handler = br_mrp_process,
^~~~~~~~~~~~~~
1 warning and 1 error generated.
--
In file included from net/bridge/br_fdb.c:24:
In file included from include/trace/events/bridge.h:10:
>> include/trace/events/../../../net/bridge/br_private.h:94:32: warning: declaration of 'struct net_bridge_port' will not be visible outside of this function [-Wvisibility]
int (*frame_handler)(struct net_bridge_port *port,
^
1 warning generated.
vim +267 net/bridge/br_input.c
256
257 /* Return 0 if the frame was not processed otherwise 1
258 * note: already called with rcu_read_lock
259 */
260 static int br_process_frame_type(struct net_bridge_port *p,
261 struct sk_buff *skb)
262 {
263 struct br_frame_type *tmp;
264
265 hlist_for_each_entry_rcu(tmp, &p->br->frame_type_list, list)
266 if (unlikely(tmp->type == skb->protocol))
> 267 return tmp->frame_handler(p, skb);
268
269 return 0;
270 }
271
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Thu, 2020-10-01 at 10:30 +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. > > Reviewed-by: Horatiu Vultur <horatiu.vultur@microchip.com> > Signed-off-by: Henrik Bjoernlund <henrik.bjoernlund@microchip.com> > --- > net/bridge/br_device.c | 1 + > net/bridge/br_input.c | 31 ++++++++++++++++++++++++++++++- > net/bridge/br_mrp.c | 19 +++++++++++++++---- > net/bridge/br_private.h | 18 ++++++++++++------ > 4 files changed, 58 insertions(+), 11 deletions(-) > Hi, Mostly looks good, one comment below. > 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 > [snip] > @@ -380,3 +395,17 @@ 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); This hasn't crashed only because you're using hlist_del_rcu(), otherwise it's wrong. You should use hlist_for_each_entry_safe() when deleting from the list while walking it or you should end the walk after the delete since there can't be two elements with the same address anyway. Thanks, Nik
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..8b2638cb550d 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,17 @@ 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); +} 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..747f6f08f439 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -89,6 +89,13 @@ struct bridge_mcast_stats { }; #endif +struct br_frame_type { + __be16 type; + int (*frame_handler)(struct net_bridge_port *port, + struct sk_buff *skb); + struct hlist_node list; +}; + struct br_vlan_stats { u64 rx_bytes; u64 rx_packets; @@ -480,6 +487,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 +764,9 @@ 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); +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;