Skip to contentSkip to conversation

Two small fixes to XFRM offload

PATCH Applied Leon Romanovsky 2 participants 4 messages Jul 8, 2024 – Jul 10, 2024

Subsystems (3)
Networking Network drivers Networking IPsec
lore
Share & tools
lore

Leon Romanovsky

author
#0 lore raw

cover Two small fixes to XFRM offload

From: Leon Romanovsky <[email protected]>

Hi,

This series contains two small fixes for XFRM offload.

Thanks

Jianbo Liu (2):
xfrm: fix netdev reference count imbalance
xfrm: call xfrm_dev_policy_delete when kill policy

 net/xfrm/xfrm_policy.c | 5 ++---
 net/xfrm/xfrm_state.c  | 3 +--
 net/xfrm/xfrm_user.c   | 1 -
 3 files changed, 3 insertions(+), 6 deletions(-)
Signature · 1 line
2.45.2
author
#1 lore raw

PATCH 1/2 xfrm: fix netdev reference count imbalance

From: Jianbo Liu <[email protected]>

In cited commit, netdev_tracker_alloc() is called for the newly allocated xfrm state, but dev_hold() is missed, which causes netdev reference count imbalance, because netdev_put() is called when the state is freed in xfrm_dev_state_free(). Fix the issue by replacing netdev_tracker_alloc() with netdev_hold().

Fixes f8a70afafc17 ("xfrm: add TX datapath support for IPsec packet offload mode")
Signed-off-by Jianbo Liu <[email protected]>
Reviewed-by Cosmin Ratiu <[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
---
 net/xfrm/xfrm_state.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
net/xfrm/xfrm_state.c +1 -2
index 5249c3574bb3..bf7904edd2fb 100644
@@ -1274,8 +1274,7 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr,
 			xso->dev = xdo->dev;
 			xso->real_dev = xdo->real_dev;
 			xso->flags = XFRM_DEV_OFFLOAD_FLAG_ACQ;
-			netdev_tracker_alloc(xso->dev, &xso->dev_tracker,
-					     GFP_ATOMIC);
+			netdev_hold(xso->dev, &xso->dev_tracker, GFP_ATOMIC);
 			error = xso->dev->xfrmdev_ops->xdo_dev_state_add(x, NULL);
 			if (error) {
 				xso->dir = 0;
Signature · 1 line
2.45.2
author
#2 lore raw

PATCH 2/2 xfrm: call xfrm_dev_policy_delete when kill policy

In reply to Leon Romanovsky

From: Jianbo Liu <[email protected]>

xfrm_policy_kill() is called at different places to delete xfrm policy. It will call xfrm_pol_put(). But xfrm_dev_policy_delete() is not called to free the policy offloaded to hardware.

The three commits cited here are to handle this issue by calling xfrm_dev_policy_delete() outside xfrm_get_policy(). But they didn't cover all the cases. An example, which is not handled for now, is xfrm_policy_insert(). It is called when XFRM_MSG_UPDPOLICY request is received. Old policy is replaced by new one, but the offloaded policy is not deleted, so driver doesn't have the chance to release hardware resources.

To resolve this issue for all cases, move xfrm_dev_policy_delete() into xfrm_policy_kill(), so the offloaded policy can be deleted from hardware when it is called, which avoids hardware resources leakage.

Fixes 919e43fad516 ("xfrm: add an interface to offload policy")
Fixes bf06fcf4be0f ("xfrm: add missed call to delete offloaded policies")
Fixes 982c3aca8bac ("xfrm: delete offloaded policy")
Signed-off-by Jianbo Liu <[email protected]>
Reviewed-by Cosmin Ratiu <[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
---
 net/xfrm/xfrm_policy.c | 5 ++---
 net/xfrm/xfrm_user.c   | 1 -
 2 files changed, 2 insertions(+), 4 deletions(-)
net/xfrm/xfrm_policy.c +2 -3
index 6603d3bd171f..27117dd7ba60 100644
@@ -452,6 +452,8 @@ EXPORT_SYMBOL(xfrm_policy_destroy);
 
 static void xfrm_policy_kill(struct xfrm_policy *policy)
 {
+	xfrm_dev_policy_delete(policy);
+
 	write_lock_bh(&policy->lock);
 	policy->walk.dead = 1;
 	write_unlock_bh(&policy->lock);

@@ -1850,7 +1852,6 @@ int xfrm_policy_flush(struct net *net, u8 type, bool task_valid)
 
 		__xfrm_policy_unlink(pol, dir);
 		spin_unlock_bh(&net->xfrm.xfrm_policy_lock);
-		xfrm_dev_policy_delete(pol);
 		cnt++;
 		xfrm_audit_policy_delete(pol, 1, task_valid);
 		xfrm_policy_kill(pol);

@@ -1891,7 +1892,6 @@ int xfrm_dev_policy_flush(struct net *net, struct net_device *dev,
 
 		__xfrm_policy_unlink(pol, dir);
 		spin_unlock_bh(&net->xfrm.xfrm_policy_lock);
-		xfrm_dev_policy_delete(pol);
 		cnt++;
 		xfrm_audit_policy_delete(pol, 1, task_valid);
 		xfrm_policy_kill(pol);

@@ -2342,7 +2342,6 @@ int xfrm_policy_delete(struct xfrm_policy *pol, int dir)
 	pol = __xfrm_policy_unlink(pol, dir);
 	spin_unlock_bh(&net->xfrm.xfrm_policy_lock);
 	if (pol) {
-		xfrm_dev_policy_delete(pol);
 		xfrm_policy_kill(pol);
 		return 0;
 	}
net/xfrm/xfrm_user.c +0 -1
index a552cfa623ea..55f039ec3d59 100644
@@ -2466,7 +2466,6 @@ static int xfrm_get_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
 					    NETLINK_CB(skb).portid);
 		}
 	} else {
-		xfrm_dev_policy_delete(xp);
 		xfrm_audit_policy_delete(xp, err ? 0 : 1, true);
 
 		if (err != 0)
Signature · 1 line
2.45.2
maintainer Applied
#3 lore raw
In reply to Leon Romanovsky
Quoted 11 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Hi,

This series contains two small fixes for XFRM offload.

Thanks

Jianbo Liu (2):
xfrm: fix netdev reference count imbalance
xfrm: call xfrm_dev_policy_delete when kill policy

Applied, tanks a lot!