| Message ID | 1496300276-30901-1-git-send-email-liuhangbin@gmail.com |
|---|---|
| State | Awaiting Upstream, archived |
| Delegated to: | David Miller |
| Headers | show |
On Thu, Jun 01, 2017 at 02:57:56PM +0800, Hangbin Liu wrote: > In commit d77e38e612a0 ("xfrm: Add an IPsec hardware offloading API") we > make xfrm_device.o only compiled when enable option CONFIG_XFRM_OFFLOAD. > But this will make xfrm_dev_event() missing if we only enable default XFRM > options. > > Then if we set down and unregister an interface with IPsec on it. You should not be able to register an interface with IPsec offload without CONFIG_XFRM_OFFLOAD. > there > will no xfrm_garbage_collect(), which will cause dev usage count hold and > get error like: > > unregister_netdevice: waiting for <dev> to become free. Usage count = 4 Can you explain how to reproduce this?
On Tue, Jun 06, 2017 at 10:06:58AM +0200, Steffen Klassert wrote: > On Thu, Jun 01, 2017 at 02:57:56PM +0800, Hangbin Liu wrote: > > In commit d77e38e612a0 ("xfrm: Add an IPsec hardware offloading API") we > > make xfrm_device.o only compiled when enable option CONFIG_XFRM_OFFLOAD. > > But this will make xfrm_dev_event() missing if we only enable default XFRM > > options. > > > > Then if we set down and unregister an interface with IPsec on it. > > You should not be able to register an interface with IPsec offload > without CONFIG_XFRM_OFFLOAD. Yes, I mean when compile with default CONFIG_XFRM, the xfrm_dev_event() -> xfrm_dev_down() -> xfrm_garbage_collect() will missing. > > > there > > will no xfrm_garbage_collect(), which will cause dev usage count hold and > > get error like: > > > > unregister_netdevice: waiting for <dev> to become free. Usage count = 4 > > Can you explain how to reproduce this? > Sure, I didn't try physical drivers, just test latest net-next with bridge/bonding and could reproduce it everytime. ``` # cat rep.sh #!/bin/bash iface=$1 src=$2 dst=$3 run=$4 brctl addbr br0 brctl addif br0 ${iface} ip link set ${iface} up ip link set br0 up ip addr add ${src}/24 dev br0 ip xfrm state flush && ip xfrm policy flush ip xfrm state add src ${src} dst ${dst} spi 1000 proto esp enc des3_ede _I_want_to_have_chicken_ auth sha1 beef_fish_pork_salad mode transport ip xfrm state add src ${dst} dst ${src} spi 1000 proto esp enc des3_ede _I_want_to_have_chicken_ auth sha1 beef_fish_pork_salad mode transport ip xfrm policy add src ${src} dst ${dst} dir out tmpl proto esp spi 1000 mode transport ip xfrm policy add src ${dst} dst ${src} dir in tmpl proto esp spi 1000 mode transport if [ "$run" ]; then sleep 1 ping ${dst} -c 4 ip link set br0 down ip link del br0 fi ``` On host A run : # ./rep.sh eth1 192.168.1.1 192.168.1.2 On host B run : # ./rep.sh eth1 192.168.1.2 192.168.1.1 run Then we will see error like kernel:unregister_netdevice: waiting for br0 to become free. Usage count = 3 Thanks Hangbin
On Tue, Jun 06, 2017 at 05:26:01PM +0800, Hangbin Liu wrote: > On Tue, Jun 06, 2017 at 10:06:58AM +0200, Steffen Klassert wrote: > > On Thu, Jun 01, 2017 at 02:57:56PM +0800, Hangbin Liu wrote: > > > In commit d77e38e612a0 ("xfrm: Add an IPsec hardware offloading API") we > > > make xfrm_device.o only compiled when enable option CONFIG_XFRM_OFFLOAD. > > > But this will make xfrm_dev_event() missing if we only enable default XFRM > > > options. > > > > > > Then if we set down and unregister an interface with IPsec on it. > > > > You should not be able to register an interface with IPsec offload > > without CONFIG_XFRM_OFFLOAD. > > Yes, I mean when compile with default CONFIG_XFRM, the xfrm_dev_event() -> > xfrm_dev_down() -> xfrm_garbage_collect() will missing. Ok, I see what you mean now. Thanks for the explanation!
On Thu, Jun 01, 2017 at 02:57:56PM +0800, Hangbin Liu wrote: > In commit d77e38e612a0 ("xfrm: Add an IPsec hardware offloading API") we > make xfrm_device.o only compiled when enable option CONFIG_XFRM_OFFLOAD. > But this will make xfrm_dev_event() missing if we only enable default XFRM > options. > > Then if we set down and unregister an interface with IPsec on it. there > will no xfrm_garbage_collect(), which will cause dev usage count hold and > get error like: > > unregister_netdevice: waiting for <dev> to become free. Usage count = 4 > > Fixes: d77e38e612a0 ("xfrm: Add an IPsec hardware offloading API") > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> Applied, thanks Hangbin!
diff --git a/include/net/xfrm.h b/include/net/xfrm.h index 7e7e2b0..62f5a25 100644 --- a/include/net/xfrm.h +++ b/include/net/xfrm.h @@ -1850,8 +1850,9 @@ static inline struct xfrm_offload *xfrm_offload(struct sk_buff *skb) } #endif -#ifdef CONFIG_XFRM_OFFLOAD void __net_init xfrm_dev_init(void); + +#ifdef CONFIG_XFRM_OFFLOAD int validate_xmit_xfrm(struct sk_buff *skb, netdev_features_t features); int xfrm_dev_state_add(struct net *net, struct xfrm_state *x, struct xfrm_user_offload *xuo); @@ -1877,10 +1878,6 @@ static inline void xfrm_dev_state_free(struct xfrm_state *x) } } #else -static inline void __net_init xfrm_dev_init(void) -{ -} - static inline int validate_xmit_xfrm(struct sk_buff *skb, netdev_features_t features) { return 0; diff --git a/net/xfrm/Makefile b/net/xfrm/Makefile index abf81b3..55b2ac3 100644 --- a/net/xfrm/Makefile +++ b/net/xfrm/Makefile @@ -4,8 +4,7 @@ obj-$(CONFIG_XFRM) := xfrm_policy.o xfrm_state.o xfrm_hash.o \ xfrm_input.o xfrm_output.o \ - xfrm_sysctl.o xfrm_replay.o -obj-$(CONFIG_XFRM_OFFLOAD) += xfrm_device.o + xfrm_sysctl.o xfrm_replay.o xfrm_device.o obj-$(CONFIG_XFRM_STATISTICS) += xfrm_proc.o obj-$(CONFIG_XFRM_ALGO) += xfrm_algo.o obj-$(CONFIG_XFRM_USER) += xfrm_user.o diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c index 574e6f3..5aba036 100644 --- a/net/xfrm/xfrm_device.c +++ b/net/xfrm/xfrm_device.c @@ -22,6 +22,7 @@ #include <net/xfrm.h> #include <linux/notifier.h> +#ifdef CONFIG_XFRM_OFFLOAD int validate_xmit_xfrm(struct sk_buff *skb, netdev_features_t features) { int err; @@ -137,6 +138,7 @@ bool xfrm_dev_offload_ok(struct sk_buff *skb, struct xfrm_state *x) return true; } EXPORT_SYMBOL_GPL(xfrm_dev_offload_ok); +#endif int xfrm_dev_register(struct net_device *dev) {
In commit d77e38e612a0 ("xfrm: Add an IPsec hardware offloading API") we make xfrm_device.o only compiled when enable option CONFIG_XFRM_OFFLOAD. But this will make xfrm_dev_event() missing if we only enable default XFRM options. Then if we set down and unregister an interface with IPsec on it. there will no xfrm_garbage_collect(), which will cause dev usage count hold and get error like: unregister_netdevice: waiting for <dev> to become free. Usage count = 4 Fixes: d77e38e612a0 ("xfrm: Add an IPsec hardware offloading API") Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> --- include/net/xfrm.h | 7 ++----- net/xfrm/Makefile | 3 +-- net/xfrm/xfrm_device.c | 2 ++ 3 files changed, 5 insertions(+), 7 deletions(-)