Skip to contentSkip to conversation

Extend XFRM core to allow full offload configuration

RFC PATCH Superseded Leon Romanovsky 4 participants 30 messages Sep 4, 2022 – Sep 27, 2022

Subsystems (9)

v3 of 10 Superseded Latest is v10 →

lore
Share & tools
lore
#0 lore raw

v3 cover Extend XFRM core to allow full offload configuration

From: Leon Romanovsky <[email protected]>

Changelog:
v4:
* Changed title from "PATCH" to "PATCH RFC" per-request.
* Added two new patches: one to update hard/soft limits and another initial take on documentation.
* Added more info about lifetime/rekeying flow to cover letter, see relevant section.
* perf traces for crypto mode will come later.
v3: https://lore.kernel.org/all/[email protected]
* I didn't hear any suggestion what term to use instead of
"full offload", so left it as is. It is used in commit messages and documentation only and easy to rename.
* Added performance data and background info to cover letter
* Reused xfrm_output_resume() function to support multiple XFRM transformations
* Add PMTU check in addition to driver .xdo_dev_offload_ok validation
* Documentation is in progress, but not part of this series yet.
v2: https://lore.kernel.org/all/[email protected]
* Rebased to latest 6.0-rc1
* Add an extra check in TX datapath patch to validate packets before forwarding to HW.
* Added policy cleanup logic in case of netdev down event
v1: https://lore.kernel.org/all/[email protected]
* Moved comment to be before if (...) in third patch.
v0: https://lore.kernel.org/all/[email protected]
-----------------------------------------------------------------------

The following series extends XFRM core code to handle a new type of IPsec offload - full offload.

In this mode, the HW is going to be responsible for the whole data path, so both policy and state should be offloaded.

IPsec full offload is an improved version of IPsec crypto mode, In full mode, HW is responsible to trim/add headers in addition to decrypt/encrypt. In this mode, the packet arrives to the stack as already decrypted and vice versa for TX (exits to HW as not-encrypted).

Devices that implement IPsec full offload mode offload policies too. In the RX path, it causes the situation that HW can't effectively handle mixed SW and HW priorities unless users make sure that HW offloaded policies have higher priorities.

To make sure that users have a coherent picture, we require that HW offloaded policies have always (both RX and TX) higher priorities than SW ones.

To not over-engineer the code, HW policies are treated as SW ones and don't take into account netdev to allow reuse of the same priorities for different devices.

There are several deliberate limitations:
* No software fallback
* Fragments are dropped, both in RX and TX
* No sockets policies
* Only IPsec transport mode is implemented

================================================================================
Rekeying:

In order to support rekeying, as XFRM core is skipped, the HW/driver should do the following:
* Count the handled packets
* Raise event that limits are reached
* Drop packets once hard limit is occurred.

The XFRM core calls to newly introduced xfrm_dev_state_update_curlft() function in order to perform sync between device statistics and internal structures. On HW limit event, driver calls to xfrm_state_check_expire() to allow XFRM core take relevant decisions.

This separation between control logic (in XFRM) and data plane allows us to fully reuse SW stack.

================================================================================
Configuration:

iproute2: https://lore.kernel.org/netdev/[email protected]/

Full offload mode:
ip xfrm state offload full dev <if-name> dir <in|out> ip xfrm policy .... offload full dev <if-name>
Crypto offload mode:
ip xfrm state offload crypto dev <if-name> dir <in|out>
or (backward compatibility)
ip xfrm state offload dev <if-name> dir <in|out>

================================================================================
Performance results:

TCP multi-stream, using iperf3 instance per-CPU.

+----------------------+--------+--------+--------+--------+---------+---------+
|                      | 1 CPU  | 2 CPUs | 4 CPUs | 8 CPUs | 16 CPUs | 32 CPUs |
|                      +--------+--------+--------+--------+---------+---------+
|                      |                   BW (Gbps)                           |
+----------------------+--------+--------+-------+---------+---------+---------+
| Baseline             | 27.9   | 59     | 93.1  | 92.8    | 93.7    | 94.4    |
+----------------------+--------+--------+-------+---------+---------+---------+
| Software IPsec       | 6      | 11.9   | 23.3  | 45.9    | 83.8    | 91.8    |
+----------------------+--------+--------+-------+---------+---------+---------+
| IPsec crypto offload | 15     | 29.7   | 58.5  | 89.6    | 90.4    | 90.8    |
+----------------------+--------+--------+-------+---------+---------+---------+
| IPsec full offload   | 28     | 57     | 90.7  | 91      | 91.3    | 91.9    |
+----------------------+--------+--------+-------+---------+---------+---------+

IPsec full offload mode behaves as baseline and reaches linerate with same amount of CPUs.

Setups details (similar for both sides):
* NIC: ConnectX6-DX dual port, 100 Gbps each. Single port used in the tests.
* CPU: Intel(R) Xeon(R) Platinum 8380 CPU @ 2.30GHz

================================================================================
Series together with mlx5 part:
https://git.kernel.org/pub/scm/linux/kernel/git/leon/linux-rdma.git/log/?h=xfrm-next

Thanks

Leon Romanovsky (8):
xfrm: add new full offload flag
xfrm: allow state full offload mode
xfrm: add an interface to offload policy
xfrm: add TX datapath support for IPsec full offload mode
xfrm: add RX datapath protection for IPsec full offload mode
xfrm: enforce separation between priorities of HW/SW policies
xfrm: add support to HW update soft and hard limits
xfrm: document IPsec full offload mode

 Documentation/networking/xfrm_device.rst      |  62 +++++-
 .../inline_crypto/ch_ipsec/chcr_ipsec.c       |   4 +
 .../net/ethernet/intel/ixgbe/ixgbe_ipsec.c    |   5 +
 drivers/net/ethernet/intel/ixgbevf/ipsec.c    |   5 +
 .../mellanox/mlx5/core/en_accel/ipsec.c       |   4 +
 drivers/net/netdevsim/ipsec.c                 |   5 +
 include/linux/netdevice.h                     |   4 +
 include/net/netns/xfrm.h                      |   8 +-
 include/net/xfrm.h                            | 121 +++++++++---
 include/uapi/linux/xfrm.h                     |   6 +
 net/xfrm/xfrm_device.c                        | 103 +++++++++-
 net/xfrm/xfrm_output.c                        |  13 +-
 net/xfrm/xfrm_policy.c                        | 181 ++++++++++++++++++
 net/xfrm/xfrm_state.c                         |   4 +
 net/xfrm/xfrm_user.c                          |  19 ++
 15 files changed, 501 insertions(+), 43 deletions(-)
Signature · 1 line
2.37.2
#1 lore raw

RFC PATCH v3 1/8 xfrm: add new full offload flag

From: Leon Romanovsky <[email protected]>

In the next patches, the xfrm core code will be extended to support new type of offload - full offload. In that mode, both policy and state should be specially configured in order to perform whole offloaded data path.

Full offload takes care of encryption, decryption, encapsulation and other operations with headers.

As this mode is new for XFRM policy flow, we can "start fresh" with flag bits and release first and second bit for future use.

---
 include/net/xfrm.h        | 7 +++++++
 include/uapi/linux/xfrm.h | 6 ++++++
 net/xfrm/xfrm_device.c    | 3 +++
 net/xfrm/xfrm_user.c      | 2 ++
 4 files changed, 18 insertions(+)
include/net/xfrm.h +7 -0
index 6e8fa98f786f..b4d487053dfd 100644
@@ -131,12 +131,19 @@ enum {
 	XFRM_DEV_OFFLOAD_OUT,
 };
 
+enum {
+	XFRM_DEV_OFFLOAD_UNSPECIFIED,
+	XFRM_DEV_OFFLOAD_CRYPTO,
+	XFRM_DEV_OFFLOAD_FULL,
+};
+
 struct xfrm_dev_offload {
 	struct net_device	*dev;
 	netdevice_tracker	dev_tracker;
 	struct net_device	*real_dev;
 	unsigned long		offload_handle;
 	u8			dir : 2;
+	u8			type : 2;
 };
 
 struct xfrm_mode {
include/uapi/linux/xfrm.h +6 -0
index 4f84ea7ee14c..463c6c1af23a 100644
@@ -519,6 +519,12 @@ struct xfrm_user_offload {
  */
 #define XFRM_OFFLOAD_IPV6	1
 #define XFRM_OFFLOAD_INBOUND	2
+/* Two bits above are relevant for state path only, while
+ * offload is used for both policy and state flows.
+ *
+ * In policy offload mode, they are free and can be safely reused.
+ */
+#define XFRM_OFFLOAD_FULL	4
 
 struct xfrm_userpolicy_default {
 #define XFRM_USERPOLICY_UNSPEC	0
net/xfrm/xfrm_device.c +3 -0
index 637ca8838436..6d1124eb1ec8 100644
@@ -270,12 +270,15 @@ int xfrm_dev_state_add(struct net *net, struct xfrm_state *x,
 	else
 		xso->dir = XFRM_DEV_OFFLOAD_OUT;
 
+	xso->type = XFRM_DEV_OFFLOAD_CRYPTO;
+
 	err = dev->xfrmdev_ops->xdo_dev_state_add(x);
 	if (err) {
 		xso->dev = NULL;
 		xso->dir = 0;
 		xso->real_dev = NULL;
 		netdev_put(dev, &xso->dev_tracker);
+		xso->type = XFRM_DEV_OFFLOAD_UNSPECIFIED;
 
 		if (err != -EOPNOTSUPP)
 			return err;
net/xfrm/xfrm_user.c +2 -0
index 2ff017117730..9c0aef815730 100644
@@ -854,6 +854,8 @@ static int copy_user_offload(struct xfrm_dev_offload *xso, struct sk_buff *skb)
 	xuo->ifindex = xso->dev->ifindex;
 	if (xso->dir == XFRM_DEV_OFFLOAD_IN)
 		xuo->flags = XFRM_OFFLOAD_INBOUND;
+	if (xso->type == XFRM_DEV_OFFLOAD_FULL)
+		xuo->flags |= XFRM_OFFLOAD_FULL;
 
 	return 0;
 }
Signature · 1 line
2.37.2
#2 lore raw

RFC PATCH v3 2/8 xfrm: allow state full offload mode

In reply to Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Allow users to configure xfrm states with full offload mode. The full mode must be requested both for policy and state, and such requires us to do not implement fallback.

We explicitly return an error if requested full mode can't be configured.

---
 .../inline_crypto/ch_ipsec/chcr_ipsec.c       |  4 ++++
 .../net/ethernet/intel/ixgbe/ixgbe_ipsec.c    |  5 ++++
 drivers/net/ethernet/intel/ixgbevf/ipsec.c    |  5 ++++
 .../mellanox/mlx5/core/en_accel/ipsec.c       |  4 ++++
 drivers/net/netdevsim/ipsec.c                 |  5 ++++
 net/xfrm/xfrm_device.c                        | 24 +++++++++++++++----
 6 files changed, 42 insertions(+), 5 deletions(-)
drivers/net/ethernet/chelsio/inline_crypto/ch_ipsec/chcr_ipsec.c +4 -0
index 585590520076..ca21794281d6 100644
@@ -283,6 +283,10 @@ static int ch_ipsec_xfrm_add_state(struct xfrm_state *x)
 		pr_debug("Cannot offload xfrm states with geniv other than seqiv\n");
 		return -EINVAL;
 	}
+	if (x->xso.type != XFRM_DEV_OFFLOAD_CRYPTO) {
+		pr_debug("Unsupported xfrm offload\n");
+		return -EINVAL;
+	}
 
 	sa_entry = kzalloc(sizeof(*sa_entry), GFP_KERNEL);
 	if (!sa_entry) {
drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c +5 -0
index 774de63dd93a..53a969e34883 100644
@@ -585,6 +585,11 @@ static int ixgbe_ipsec_add_sa(struct xfrm_state *xs)
 		return -EINVAL;
 	}
 
+	if (xs->xso.type != XFRM_DEV_OFFLOAD_CRYPTO) {
+		netdev_err(dev, "Unsupported ipsec offload type\n");
+		return -EINVAL;
+	}
+
 	if (xs->xso.dir == XFRM_DEV_OFFLOAD_IN) {
 		struct rx_sa rsa;
 
drivers/net/ethernet/intel/ixgbevf/ipsec.c +5 -0
index 9984ebc62d78..c1cf540d162a 100644
@@ -280,6 +280,11 @@ static int ixgbevf_ipsec_add_sa(struct xfrm_state *xs)
 		return -EINVAL;
 	}
 
+	if (xs->xso.type != XFRM_DEV_OFFLOAD_CRYPTO) {
+		netdev_err(dev, "Unsupported ipsec offload type\n");
+		return -EINVAL;
+	}
+
 	if (xs->xso.dir == XFRM_DEV_OFFLOAD_IN) {
 		struct rx_sa rsa;
 
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +4 -0
index 2a8fd7020622..c182b640b80d 100644
@@ -256,6 +256,10 @@ static inline int mlx5e_xfrm_validate_state(struct xfrm_state *x)
 		netdev_info(netdev, "Cannot offload xfrm states with geniv other than seqiv\n");
 		return -EINVAL;
 	}
+	if (x->xso.type != XFRM_DEV_OFFLOAD_CRYPTO) {
+		netdev_info(netdev, "Unsupported xfrm offload type\n");
+		return -EINVAL;
+	}
 	return 0;
 }
 
drivers/net/netdevsim/ipsec.c +5 -0
index 386336a38f34..b93baf5c8bee 100644
@@ -149,6 +149,11 @@ static int nsim_ipsec_add_sa(struct xfrm_state *xs)
 		return -EINVAL;
 	}
 
+	if (xs->xso.type != XFRM_DEV_OFFLOAD_CRYPTO) {
+		netdev_err(dev, "Unsupported ipsec offload type\n");
+		return -EINVAL;
+	}
+
 	/* find the first unused index */
 	ret = nsim_ipsec_find_empty_idx(ipsec);
 	if (ret < 0) {
net/xfrm/xfrm_device.c +19 -5
index 6d1124eb1ec8..5b04e5cdca64 100644
@@ -215,6 +215,7 @@ int xfrm_dev_state_add(struct net *net, struct xfrm_state *x,
 	struct xfrm_dev_offload *xso = &x->xso;
 	xfrm_address_t *saddr;
 	xfrm_address_t *daddr;
+	bool is_full_offload;
 
 	if (!x->type_offload)
 		return -EINVAL;
@@ -223,9 +224,11 @@ int xfrm_dev_state_add(struct net *net, struct xfrm_state *x,
 	if (x->encap || x->tfcpad)
 		return -EINVAL;
 
-	if (xuo->flags & ~(XFRM_OFFLOAD_IPV6 | XFRM_OFFLOAD_INBOUND))
+	if (xuo->flags &
+	    ~(XFRM_OFFLOAD_IPV6 | XFRM_OFFLOAD_INBOUND | XFRM_OFFLOAD_FULL))
 		return -EINVAL;
 
+	is_full_offload = xuo->flags & XFRM_OFFLOAD_FULL;
 	dev = dev_get_by_index(net, xuo->ifindex);
 	if (!dev) {
 		if (!(xuo->flags & XFRM_OFFLOAD_INBOUND)) {
@@ -240,7 +243,7 @@ int xfrm_dev_state_add(struct net *net, struct xfrm_state *x,
 					x->props.family,
 					xfrm_smark_get(0, x));
 		if (IS_ERR(dst))
-			return 0;
+			return (is_full_offload) ? -EINVAL : 0;
 
 		dev = dst->dev;
 
@@ -251,7 +254,7 @@ int xfrm_dev_state_add(struct net *net, struct xfrm_state *x,
 	if (!dev->xfrmdev_ops || !dev->xfrmdev_ops->xdo_dev_state_add) {
 		xso->dev = NULL;
 		dev_put(dev);
-		return 0;
+		return (is_full_offload) ? -EINVAL : 0;
 	}
 
 	if (x->props.flags & XFRM_STATE_ESN &&
@@ -270,7 +273,10 @@ int xfrm_dev_state_add(struct net *net, struct xfrm_state *x,
 	else
 		xso->dir = XFRM_DEV_OFFLOAD_OUT;
 
-	xso->type = XFRM_DEV_OFFLOAD_CRYPTO;
+	if (is_full_offload)
+		xso->type = XFRM_DEV_OFFLOAD_FULL;
+	else
+		xso->type = XFRM_DEV_OFFLOAD_CRYPTO;
 
 	err = dev->xfrmdev_ops->xdo_dev_state_add(x);
 	if (err) {
@@ -280,7 +286,15 @@ int xfrm_dev_state_add(struct net *net, struct xfrm_state *x,
 		netdev_put(dev, &xso->dev_tracker);
 		xso->type = XFRM_DEV_OFFLOAD_UNSPECIFIED;
 
-		if (err != -EOPNOTSUPP)
+		/* User explicitly requested full offload mode and configured
+		 * policy in addition to the XFRM state. So be civil to users,
+		 * and return an error instead of taking fallback path.
+		 *
+		 * This WARN_ON() can be seen as a documentation for driver
+		 * authors to do not return -EOPNOTSUPP in full offload mode.
+		 */
+		WARN_ON(err == -EOPNOTSUPP && is_full_offload);
+		if (err != -EOPNOTSUPP || is_full_offload)
 			return err;
 	}
 
Signature · 1 line
2.37.2
#3 lore raw

RFC PATCH v3 3/8 xfrm: add an interface to offload policy

In reply to Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Extend netlink interface to add and delete XFRM policy from the device. This functionality is a first step to implement full IPsec offload solution.

---
 include/linux/netdevice.h |  3 ++
 include/net/xfrm.h        | 42 ++++++++++++++++++++++++
 net/xfrm/xfrm_device.c    | 61 ++++++++++++++++++++++++++++++++++-
 net/xfrm/xfrm_policy.c    | 68 +++++++++++++++++++++++++++++++++++++++
 net/xfrm/xfrm_user.c      | 17 ++++++++++
 5 files changed, 190 insertions(+), 1 deletion(-)
include/linux/netdevice.h +3 -0
index f0068c1ff1df..c1db9eaa3dca 100644
@@ -1026,6 +1026,9 @@ struct xfrmdev_ops {
 	bool	(*xdo_dev_offload_ok) (struct sk_buff *skb,
 				       struct xfrm_state *x);
 	void	(*xdo_dev_state_advance_esn) (struct xfrm_state *x);
+	int	(*xdo_dev_policy_add) (struct xfrm_policy *x);
+	void	(*xdo_dev_policy_delete) (struct xfrm_policy *x);
+	void	(*xdo_dev_policy_free) (struct xfrm_policy *x);
 };
 #endif
 
include/net/xfrm.h +42 -0

78 lines not on this page · raw message

net/xfrm/xfrm_device.c +60 -1

74 lines not on this page · raw message

net/xfrm/xfrm_policy.c +68 -0

98 lines not on this page · raw message

net/xfrm/xfrm_user.c +17 -0
index 9c0aef815730..698ff84da6ba 100644
@@ -1747,6 +1747,14 @@ static struct xfrm_policy *xfrm_policy_construct(struct net *net, struct xfrm_us
 	if (attrs[XFRMA_IF_ID])
 		xp->if_id = nla_get_u32(attrs[XFRMA_IF_ID]);
 
+	/* configure the hardware if offload is requested */
+	if (attrs[XFRMA_OFFLOAD_DEV]) {
+		err = xfrm_dev_policy_add(net, xp,
+				nla_data(attrs[XFRMA_OFFLOAD_DEV]), p->dir);
+		if (err)
+			goto error;
+	}
+
 	return xp;
  error:
 	*errp = err;
@@ -1785,6 +1793,7 @@ static int xfrm_add_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
 	xfrm_audit_policy_add(xp, err ? 0 : 1, true);
 
 	if (err) {
+		xfrm_dev_policy_delete(xp);
 		security_xfrm_policy_free(xp->security);
 		kfree(xp);
 		return err;
@@ -1897,6 +1906,8 @@ static int dump_one_policy(struct xfrm_policy *xp, int dir, int count, void *ptr
 		err = xfrm_mark_put(skb, &xp->mark);
 	if (!err)
 		err = xfrm_if_id_put(skb, xp->if_id);
+	if (!err && xp->xdo.dev)
+		err = copy_user_offload(&xp->xdo, skb);
 	if (err) {
 		nlmsg_cancel(skb, nlh);
 		return err;
@@ -3213,6 +3224,8 @@ static int build_acquire(struct sk_buff *skb, struct xfrm_state *x,
 		err = xfrm_mark_put(skb, &xp->mark);
 	if (!err)
 		err = xfrm_if_id_put(skb, xp->if_id);
+	if (!err && xp->xdo.dev)
+		err = copy_user_offload(&xp->xdo, skb);
 	if (err) {
 		nlmsg_cancel(skb, nlh);
 		return err;
@@ -3331,6 +3344,8 @@ static int build_polexpire(struct sk_buff *skb, struct xfrm_policy *xp,
 		err = xfrm_mark_put(skb, &xp->mark);
 	if (!err)
 		err = xfrm_if_id_put(skb, xp->if_id);
+	if (!err && xp->xdo.dev)
+		err = copy_user_offload(&xp->xdo, skb);
 	if (err) {
 		nlmsg_cancel(skb, nlh);
 		return err;
@@ -3414,6 +3429,8 @@ static int xfrm_notify_policy(struct xfrm_policy *xp, int dir, const struct km_e
 		err = xfrm_mark_put(skb, &xp->mark);
 	if (!err)
 		err = xfrm_if_id_put(skb, xp->if_id);
+	if (!err && xp->xdo.dev)
+		err = copy_user_offload(&xp->xdo, skb);
 	if (err)
 		goto out_free_skb;
 
Signature · 1 line
2.37.2
#4 lore raw

RFC PATCH v3 4/8 xfrm: add TX datapath support for IPsec full offload mode

In reply to Leon Romanovsky

From: Leon Romanovsky <[email protected]>

In IPsec full mode, the device is going to encrypt and encapsulate packets that are associated with offloaded policy. After successful policy lookup to indicate if packets should be offloaded or not, the stack forwards packets to the device to do the magic.

---
 net/xfrm/xfrm_device.c | 15 +++++++++++++--
 net/xfrm/xfrm_output.c | 12 +++++++++++-
 2 files changed, 24 insertions(+), 3 deletions(-)
net/xfrm/xfrm_device.c +13 -2
index 1cc482e9c87d..2d37bb86914a 100644
@@ -120,6 +120,16 @@ struct sk_buff *validate_xmit_xfrm(struct sk_buff *skb, netdev_features_t featur
 	if (xo->flags & XFRM_GRO || x->xso.dir == XFRM_DEV_OFFLOAD_IN)
 		return skb;
 
+	/* The packet was sent to HW IPsec full offload engine,
+	 * but to wrong device. Drop the packet, so it won't skip
+	 * XFRM stack.
+	 */
+	if (x->xso.type == XFRM_DEV_OFFLOAD_FULL && x->xso.dev != dev) {
+		kfree_skb(skb);
+		dev_core_stats_tx_dropped_inc(dev);
+		return NULL;
+	}
+
 	/* This skb was already validated on the upper/virtual dev */
 	if ((x->xso.dev != dev) && (x->xso.real_dev == dev))
 		return skb;
@@ -369,8 +379,9 @@ bool xfrm_dev_offload_ok(struct sk_buff *skb, struct xfrm_state *x)
 	if (!x->type_offload || x->encap)
 		return false;
 
-	if ((!dev || (dev == xfrm_dst_path(dst)->dev)) &&
-	    (!xdst->child->xfrm)) {
+	if (x->xso.type == XFRM_DEV_OFFLOAD_FULL ||
+	    ((!dev || (dev == xfrm_dst_path(dst)->dev)) &&
+	     !xdst->child->xfrm)) {
 		mtu = xfrm_state_mtu(x, xdst->child_mtu_cached);
 		if (skb->len <= mtu)
 			goto ok;
net/xfrm/xfrm_output.c +11 -1
index 9a5e79a38c67..dde009be8463 100644
@@ -494,7 +494,7 @@ static int xfrm_output_one(struct sk_buff *skb, int err)
 	struct xfrm_state *x = dst->xfrm;
 	struct net *net = xs_net(x);
 
-	if (err <= 0)
+	if (err <= 0 || x->xso.type == XFRM_DEV_OFFLOAD_FULL)
 		goto resume;
 
 	do {
@@ -718,6 +718,16 @@ int xfrm_output(struct sock *sk, struct sk_buff *skb)
 		break;
 	}
 
+	if (x->xso.type == XFRM_DEV_OFFLOAD_FULL) {
+		if (!xfrm_dev_offload_ok(skb, x)) {
+			XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTERROR);
+			kfree_skb(skb);
+			return -EHOSTUNREACH;
+		}
+
+		return xfrm_output_resume(sk, skb, 0);
+	}
+
 	secpath_reset(skb);
 
 	if (xfrm_dev_offload_ok(skb, x)) {
Signature · 1 line
2.37.2
maintainer
#5 lore raw

Re: [PATCH RFC xfrm-next v3 4/8] xfrm: add TX datapath support for IPsec full offload mode

Quoted 53 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

In IPsec full mode, the device is going to encrypt and encapsulate packets that are associated with offloaded policy. After successful policy lookup to indicate if packets should be offloaded or not, the stack forwards packets to the device to do the magic.

---
 net/xfrm/xfrm_device.c | 15 +++++++++++++--
 net/xfrm/xfrm_output.c | 12 +++++++++++-
 2 files changed, 24 insertions(+), 3 deletions(-)
net/xfrm/xfrm_device.c +13 -2
index 1cc482e9c87d..2d37bb86914a 100644
@@ -120,6 +120,16 @@ struct sk_buff *validate_xmit_xfrm(struct sk_buff *skb, netdev_features_t featur
 	if (xo->flags & XFRM_GRO || x->xso.dir == XFRM_DEV_OFFLOAD_IN)
 		return skb;
 
+	/* The packet was sent to HW IPsec full offload engine,
+	 * but to wrong device. Drop the packet, so it won't skip
+	 * XFRM stack.
+	 */
+	if (x->xso.type == XFRM_DEV_OFFLOAD_FULL && x->xso.dev != dev) {
+		kfree_skb(skb);
+		dev_core_stats_tx_dropped_inc(dev);
+		return NULL;
+	}
+
 	/* This skb was already validated on the upper/virtual dev */
 	if ((x->xso.dev != dev) && (x->xso.real_dev == dev))
 		return skb;
@@ -369,8 +379,9 @@ bool xfrm_dev_offload_ok(struct sk_buff *skb, struct xfrm_state *x)
 	if (!x->type_offload || x->encap)
 		return false;
 
-	if ((!dev || (dev == xfrm_dst_path(dst)->dev)) &&
-	    (!xdst->child->xfrm)) {
+	if (x->xso.type == XFRM_DEV_OFFLOAD_FULL ||
+	    ((!dev || (dev == xfrm_dst_path(dst)->dev)) &&
+	     !xdst->child->xfrm)) {
 		mtu = xfrm_state_mtu(x, xdst->child_mtu_cached);
 		if (skb->len <= mtu)
 			goto ok;
net/xfrm/xfrm_output.c +0 -0
index 9a5e79a38c67..dde009be8463 100644

net/xfrm/xfrm_output.c static int xfrm_output_one(struct sk_buff *skb, int err) · line 498

Quoted 6 lines
net/xfrm/xfrm_output.c +1 -1
@@ -494,7 +494,7 @@ static int xfrm_output_one(struct sk_buff *skb, int err)
 	struct xfrm_state *x = dst->xfrm;
 	struct net *net = xs_net(x);
 
-	if (err <= 0)
+	if (err <= 0 || x->xso.type == XFRM_DEV_OFFLOAD_FULL)
 		goto resume;

You check here that the state is marked as 'full offload' before you skip the SW xfrm handling, but I don't see where you check that the policy that led to this state is offloaded too. Also, we have to make sure that both, policy and state is offloaded to the same device. Looks like this part is missing.

#6 lore raw

Re: [PATCH RFC xfrm-next v3 4/8] xfrm: add TX datapath support for IPsec full offload mode

Quoted 57 lines · Steffen Klassert
Quoted 56 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

In IPsec full mode, the device is going to encrypt and encapsulate packets that are associated with offloaded policy. After successful policy lookup to indicate if packets should be offloaded or not, the stack forwards packets to the device to do the magic.

---
 net/xfrm/xfrm_device.c | 15 +++++++++++++--
 net/xfrm/xfrm_output.c | 12 +++++++++++-
 2 files changed, 24 insertions(+), 3 deletions(-)
net/xfrm/xfrm_device.c +13 -2
index 1cc482e9c87d..2d37bb86914a 100644
@@ -120,6 +120,16 @@ struct sk_buff *validate_xmit_xfrm(struct sk_buff *skb, netdev_features_t featur
 	if (xo->flags & XFRM_GRO || x->xso.dir == XFRM_DEV_OFFLOAD_IN)
 		return skb;
 
+	/* The packet was sent to HW IPsec full offload engine,
+	 * but to wrong device. Drop the packet, so it won't skip
+	 * XFRM stack.
+	 */
+	if (x->xso.type == XFRM_DEV_OFFLOAD_FULL && x->xso.dev != dev) {
+		kfree_skb(skb);
+		dev_core_stats_tx_dropped_inc(dev);
+		return NULL;
+	}
+
 	/* This skb was already validated on the upper/virtual dev */
 	if ((x->xso.dev != dev) && (x->xso.real_dev == dev))
 		return skb;
@@ -369,8 +379,9 @@ bool xfrm_dev_offload_ok(struct sk_buff *skb, struct xfrm_state *x)
 	if (!x->type_offload || x->encap)
 		return false;
 
-	if ((!dev || (dev == xfrm_dst_path(dst)->dev)) &&
-	    (!xdst->child->xfrm)) {
+	if (x->xso.type == XFRM_DEV_OFFLOAD_FULL ||
+	    ((!dev || (dev == xfrm_dst_path(dst)->dev)) &&
+	     !xdst->child->xfrm)) {
 		mtu = xfrm_state_mtu(x, xdst->child_mtu_cached);
 		if (skb->len <= mtu)
 			goto ok;
net/xfrm/xfrm_output.c +0 -0
index 9a5e79a38c67..dde009be8463 100644
@@ -494,7 +494,7 @@ static int xfrm_output_one(struct sk_buff *skb, int err)
 	struct xfrm_state *x = dst->xfrm;
 	struct net *net = xs_net(x);
 
Quoted 9 lines

net/xfrm/xfrm_output.c static int xfrm_output_one(struct sk_buff *skb, int err) · line 498

Quoted 3 lines
net/xfrm/xfrm_output.c +1 -1
-	if (err <= 0)
+	if (err <= 0 || x->xso.type == XFRM_DEV_OFFLOAD_FULL)
 		goto resume;

You check here that the state is marked as 'full offload' before you skip the SW xfrm handling, but I don't see where you check that the policy that led to this state is offloaded too. Also, we have to make sure that both, policy and state is offloaded to the same device. Looks like this part is missing.

In SW flow, users are not required to configure policy. If they don't have policy, the packet will be encrypted and sent anyway.

The full offload follows same semantic. The missing offloaded policy is equal to no policy at all.

I don't think that extra checks are needed.

Thanks

Quoted 1 line
maintainer
#7 lore raw

Re: [PATCH RFC xfrm-next v3 4/8] xfrm: add TX datapath support for IPsec full offload mode

Quoted 21 lines · Leon Romanovsky
Quoted 10 lines · Steffen Klassert
Quoted 9 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

net/xfrm/xfrm_output.c +0 -0
index 9a5e79a38c67..dde009be8463 100644
@@ -494,7 +494,7 @@ static int xfrm_output_one(struct sk_buff *skb, int err)
 	struct xfrm_state *x = dst->xfrm;
 	struct net *net = xs_net(x);
 
Quoted 9 lines

net/xfrm/xfrm_output.c static int xfrm_output_one(struct sk_buff *skb, int err) · line 498

Quoted 3 lines
net/xfrm/xfrm_output.c +1 -1
-	if (err <= 0)
+	if (err <= 0 || x->xso.type == XFRM_DEV_OFFLOAD_FULL)
 		goto resume;

You check here that the state is marked as 'full offload' before you skip the SW xfrm handling, but I don't see where you check that the policy that led to this state is offloaded too. Also, we have to make sure that both, policy and state is offloaded to the same device. Looks like this part is missing.

Quoted 2 lines

In SW flow, users are not required to configure policy. If they don't have policy, the packet will be encrypted and sent anyway.

No, it is not! You can't lookup a TX SA without a policy. The lookup happens in two stages. The packet header is matched against the TS of the policy. Then the template found at the policy is used to lookup the SA.

Quoted 2 lines

The full offload follows same semantic. The missing offloaded policy is equal to no policy at all.

No policy at all means that the packets are sent out unencrypted in plaintext, and this is certainly not what you want.

Quoted 1 line

I don't think that extra checks are needed.

We need this checks. This is one of the reasons why I want to separate the SW and HW databases.

#8 lore raw

RFC PATCH v3 5/8 xfrm: add RX datapath protection for IPsec full offload mode

In reply to Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Traffic received by device with enabled IPsec full offload should be forwarded to the stack only after decryption, packet headers and trailers removed.

Such packets are expected to be seen as normal (non-XFRM) ones, while not-supported packets should be dropped by the HW.

---
 include/net/xfrm.h | 55 +++++++++++++++++++++++++++-------------------
 1 file changed, 32 insertions(+), 23 deletions(-)
include/net/xfrm.h +32 -23
index 587697eb1d31..38fff78a1421 100644
@@ -1094,6 +1094,29 @@ xfrm_state_addr_cmp(const struct xfrm_tmpl *tmpl, const struct xfrm_state *x, un
 	return !0;
 }
 
+#ifdef CONFIG_XFRM
+static inline struct xfrm_state *xfrm_input_state(struct sk_buff *skb)
+{
+	struct sec_path *sp = skb_sec_path(skb);
+
+	return sp->xvec[sp->len - 1];
+}
+#endif
+
+static inline struct xfrm_offload *xfrm_offload(struct sk_buff *skb)
+{
+#ifdef CONFIG_XFRM
+	struct sec_path *sp = skb_sec_path(skb);
+
+	if (!sp || !sp->olen || sp->len != sp->olen)
+		return NULL;
+
+	return &sp->ovec[sp->olen - 1];
+#else
+	return NULL;
+#endif
+}
+
 #ifdef CONFIG_XFRM
 int __xfrm_policy_check(struct sock *, int dir, struct sk_buff *skb,
 			unsigned short family);
@@ -1125,10 +1148,19 @@ static inline int __xfrm_policy_check2(struct sock *sk, int dir,
 {
 	struct net *net = dev_net(skb->dev);
 	int ndir = dir | (reverse ? XFRM_POLICY_MASK + 1 : 0);
+	struct xfrm_offload *xo = xfrm_offload(skb);
+	struct xfrm_state *x;
 
 	if (sk && sk->sk_policy[XFRM_POLICY_IN])
 		return __xfrm_policy_check(sk, ndir, skb, family);
 
+	if (xo) {
+		x = xfrm_input_state(skb);
+		if (x->xso.type == XFRM_DEV_OFFLOAD_FULL)
+			return (xo->flags & CRYPTO_DONE) &&
+			       (xo->status & CRYPTO_SUCCESS);
+	}
+
 	return __xfrm_check_nopolicy(net, skb, dir) ||
 	       __xfrm_check_dev_nopolicy(skb, dir, family) ||
 	       __xfrm_policy_check(sk, ndir, skb, family);
@@ -1860,29 +1892,6 @@ static inline void xfrm_states_delete(struct xfrm_state **states, int n)
 }
 #endif
 
-#ifdef CONFIG_XFRM
-static inline struct xfrm_state *xfrm_input_state(struct sk_buff *skb)
-{
-	struct sec_path *sp = skb_sec_path(skb);
-
-	return sp->xvec[sp->len - 1];
-}
-#endif
-
-static inline struct xfrm_offload *xfrm_offload(struct sk_buff *skb)
-{
-#ifdef CONFIG_XFRM
-	struct sec_path *sp = skb_sec_path(skb);
-
-	if (!sp || !sp->olen || sp->len != sp->olen)
-		return NULL;
-
-	return &sp->ovec[sp->olen - 1];
-#else
-	return NULL;
-#endif
-}
-
 void __init xfrm_dev_init(void);
 
 #ifdef CONFIG_XFRM_OFFLOAD
Signature · 1 line
2.37.2
#9 lore raw

RFC PATCH v3 6/8 xfrm: enforce separation between priorities of HW/SW policies

In reply to Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Devices that implement IPsec full offload mode offload policies too. In RX path, it causes to the situation that HW can't effectively handle mixed SW and HW priorities unless users make sure that HW offloaded policies have higher priorities.

In order to make sure that users have coherent picture, let's require that HW offloaded policies have always (both RX and TX) higher priorities than SW ones.

To do not over engineer the code, HW policies are treated as SW ones and don't take into account netdev to allow reuse of same priorities for different devices.

---
 include/net/netns/xfrm.h |   8 ++-
 net/xfrm/xfrm_policy.c   | 113 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 120 insertions(+), 1 deletion(-)
include/net/netns/xfrm.h +7 -1
index bd7c3be4af5d..f0cfa0faf611 100644
@@ -29,6 +29,11 @@ struct xfrm_policy_hthresh {
 	u8			rbits6;
 };
 
+struct xfrm_policy_prio {
+	u32 max_sw_prio;
+	u32 min_hw_prio;
+};
+
 struct netns_xfrm {
 	struct list_head	state_all;
 	/*
@@ -52,7 +57,7 @@ struct netns_xfrm {
 	unsigned int		policy_idx_hmask;
 	struct hlist_head	policy_inexact[XFRM_POLICY_MAX];
 	struct xfrm_policy_hash	policy_bydst[XFRM_POLICY_MAX];
-	unsigned int		policy_count[XFRM_POLICY_MAX * 2];
+	unsigned int		policy_count[XFRM_POLICY_MAX * 3];
 	struct work_struct	policy_hash_work;
 	struct xfrm_policy_hthresh policy_hthresh;
 	struct list_head	inexact_bins;
@@ -67,6 +72,7 @@ struct netns_xfrm {
 	u32			sysctl_acq_expires;
 
 	u8			policy_default[XFRM_POLICY_MAX];
+	struct xfrm_policy_prio	policy_prio[XFRM_POLICY_MAX];
 
 #ifdef CONFIG_SYSCTL
 	struct ctl_table_header	*sysctl_hdr;
net/xfrm/xfrm_policy.c +113 -0

168 lines not on this page · raw message

Signature · 1 line
2.37.2
maintainer
#10 lore raw

Re: [PATCH RFC xfrm-next v3 6/8] xfrm: enforce separation between priorities of HW/SW policies

Quoted 7 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Devices that implement IPsec full offload mode offload policies too. In RX path, it causes to the situation that HW can't effectively handle mixed SW and HW priorities unless users make sure that HW offloaded policies have higher priorities.

Quoted 7 lines

In order to make sure that users have coherent picture, let's require that HW offloaded policies have always (both RX and TX) higher priorities than SW ones.

To do not over engineer the code, HW policies are treated as SW ones and don't take into account netdev to allow reuse of same priorities for different devices.

I think we should split HW and SW SPD (and maybe even SAD) and priorize over the SPDs instead of doing that in one single SPD. Each NIC should maintain its own databases and we should do the lookups from SW with a callback. With the current approach, we still do the costly full policy and state lookup on the TX side in software. On a 'full offload' that should happen in HW too. Also, that will make things easier with tunnel mode whre we can have overlapping traffic selectors.

We can keep a HW SPD in software as a fallback for devices that don't support the offloaded lookup, but on the long run lookups for the RX anf TX path should happen in HW.

#11 lore raw

Re: [PATCH RFC xfrm-next v3 6/8] xfrm: enforce separation between priorities of HW/SW policies

Quoted 12 lines · Steffen Klassert
Quoted 11 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Devices that implement IPsec full offload mode offload policies too. In RX path, it causes to the situation that HW can't effectively handle mixed SW and HW priorities unless users make sure that HW offloaded policies have higher priorities.

In order to make sure that users have coherent picture, let's require that HW offloaded policies have always (both RX and TX) higher priorities than SW ones.

Quoted 8 lines
Quoted 3 lines

To do not over engineer the code, HW policies are treated as SW ones and don't take into account netdev to allow reuse of same priorities for different devices.

I think we should split HW and SW SPD (and maybe even SAD) and priorize over the SPDs instead of doing that in one single SPD. Each NIC should maintain its own databases and we should do the lookups from SW with a callback.

I don't understand how will it work and scale.

Every packet needs to be classified if it is in offloaded path or not. To ensure that, we will need to have two identifiers: targeted device (part of skb, so ok) and relevant SP/SA.

The latter is needed to make sure that we perform lookup only on offloaded SP/SA. It means that we will do lookup twice now. First to see if SP/SA is offloaded and second to see if it in HW.

HW lookup is also misleading name, as the lookup will be in driver code in very similar way to how SADB managed for crypto mode. It makes no sense to convert data from XFRM representation to HW format, execute in HW and convert returned result back. It will be also slow because lookup of SP/SA based on XFRM properties is not datapath.

In any case, you will have double lookup. You will need to query XFRM core DBs and later call to driver DB or vice versa.

Unless you want to perform HW lookup always without relation to SP/SA state and hurt performance for non-offloaded path.

Quoted 3 lines

With the current approach, we still do the costly full policy and state lookup on the TX side in software. On a 'full offload' that should happen in HW too.

In proposed approach, you have only one lookup which is better than two. I'm not even talking about "quality" of driver lookups implementations.

Quoted 2 lines

Also, that will make things easier with tunnel mode whre we can have overlapping traffic selectors.

Can we please put tunnel mode aside? It is a long journey.

Quoted 4 lines

We can keep a HW SPD in software as a fallback for devices that don't support the offloaded lookup, but on the long run lookups for the RX anf TX path should happen in HW.

I doubt about it.

Quoted 1 line
maintainer
#12 lore raw

Re: [PATCH RFC xfrm-next v3 6/8] xfrm: enforce separation between priorities of HW/SW policies

Quoted 17 lines · Leon Romanovsky
Quoted 16 lines · Steffen Klassert
Quoted 14 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Devices that implement IPsec full offload mode offload policies too. In RX path, it causes to the situation that HW can't effectively handle mixed SW and HW priorities unless users make sure that HW offloaded policies have higher priorities.

In order to make sure that users have coherent picture, let's require that HW offloaded policies have always (both RX and TX) higher priorities than SW ones.

To do not over engineer the code, HW policies are treated as SW ones and don't take into account netdev to allow reuse of same priorities for different devices.

Quoted 6 lines
Quoted 4 lines

I think we should split HW and SW SPD (and maybe even SAD) and priorize over the SPDs instead of doing that in one single SPD. Each NIC should maintain its own databases and we should do the lookups from SW with a callback.

I don't understand how will it work and scale.

That is rather easy. HW offload devices register their databases at the xfrm layer with a certain priority higher than the one of the SW databases. The lookup will happen consecutively based on the database priorities. If there are no HW databases are registered everything is like it was before. It gives us a clear separation between HW and SW.

This has the advantage that you don't need to mess with policy priorites. User can keep the priorites as they were before. This is in particular important because usually the IKE daemon chosses the priorities based on some heuristics.

The HW offload has also the advantage that we don't need to search through all SW policies and states in that case.

Quoted 7 lines

Every packet needs to be classified if it is in offloaded path or not. To ensure that, we will need to have two identifiers: targeted device (part of skb, so ok) and relevant SP/SA.

The latter is needed to make sure that we perform lookup only on offloaded SP/SA. It means that we will do lookup twice now. First to see if SP/SA is offloaded and second to see if it in HW.

I think you did not get my point, see above.

Quoted 2 lines

HW lookup is also misleading name, as the lookup will be in driver code in very similar way to how SADB managed for crypto mode.

No, HW lookup means we do the lookup in the hardware and return the result to software. The whole point of a 'full offload' is to get rid of the costly SW database lookups.

Quoted 4 lines

It makes no
sense to convert data from XFRM representation to HW format, execute in HW and convert returned result back. It will be also slow because lookup of SP/SA based on XFRM properties is not datapath.

In case the HW can't do the lookup itself or is considered to be slower than in software, a separated database for HW offload devices can be maintained.

Quoted 11 lines

In any case, you will have double lookup. You will need to query XFRM core DBs and later call to driver DB or vice versa.

Unless you want to perform HW lookup always without relation to SP/SA state and hurt performance for non-offloaded path.

Quoted 3 lines

With the current approach, we still do the costly full policy and state lookup on the TX side in software. On a 'full offload' that should happen in HW too.

In proposed approach, you have only one lookup which is better than two.

In your approach, you still do the lookups in SW. This is not a problem if you have just a handfull policies and SAs, but is problematic when you have 100K policies and SAs. Even if the current HW can not support this, we need to make sure our design allows for it.

Quoted 1 line

I'm not even talking about "quality" of driver lookups implementations.

You don't need to to reimplement the lookups, you just have to create an additional database.

Quoted 5 lines
Quoted 3 lines

Also, that will make things easier with tunnel mode whre we can have overlapping traffic selectors.

Can we please put tunnel mode aside? It is a long journey.

No, we can't. A good design should include transport and tunnel mode. I don't want to change everyting later just because we notice then that it does not work at all for tunnel mode.

#13 lore raw

Re: [PATCH RFC xfrm-next v3 6/8] xfrm: enforce separation between priorities of HW/SW policies

Quoted 32 lines · Steffen Klassert
Quoted 17 lines · Leon Romanovsky
Quoted 16 lines · Steffen Klassert
Quoted 14 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Devices that implement IPsec full offload mode offload policies too. In RX path, it causes to the situation that HW can't effectively handle mixed SW and HW priorities unless users make sure that HW offloaded policies have higher priorities.

In order to make sure that users have coherent picture, let's require that HW offloaded policies have always (both RX and TX) higher priorities than SW ones.

To do not over engineer the code, HW policies are treated as SW ones and don't take into account netdev to allow reuse of same priorities for different devices.

Quoted 6 lines
Quoted 4 lines

I think we should split HW and SW SPD (and maybe even SAD) and priorize over the SPDs instead of doing that in one single SPD. Each NIC should maintain its own databases and we should do the lookups from SW with a callback.

I don't understand how will it work and scale.

That is rather easy. HW offload devices register their databases at the xfrm layer with a certain priority higher than the one of the SW databases. The lookup will happen consecutively based on the database priorities. If there are no HW databases are registered everything is like it was before. It gives us a clear separation between HW and SW.

Quoted 7 lines

This has the advantage that you don't need to mess with policy priorites. User can keep the priorites as they were before. This is in particular important because usually the IKE daemon chosses the priorities based on some heuristics.

The HW offload has also the advantage that we don't need to search through all SW policies and states in that case.

And disadvantage for SW policies, because once you register HW DB, you will first lookup there, won't find and fallback to perform another lookup in SW.

<...>

Quoted 8 lines
Quoted 4 lines

It makes no
sense to convert data from XFRM representation to HW format, execute in HW and convert returned result back. It will be also slow because lookup of SP/SA based on XFRM properties is not datapath.

In case the HW can't do the lookup itself or is considered to be slower than in software, a separated database for HW offload devices can be maintained.

ok, this is my case.
I'll try to see what I can do here.

Thanks

#14 lore raw

RFC PATCH v3 7/8 xfrm: add support to HW update soft and hard limits

In reply to Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Both in RX and TX, the traffic that performs IPsec full offload transformation is accounted by HW. It is needed to properly handle hard limits that require to drop the packet.

It means that XFRM core needs to update internal counters with the one that accounted by the HW, so new callbacks are introduced in this patch.

In case of soft or hard limit is occurred, the driver should call to xfrm_state_check_expire() that will perform key rekeying exactly as done by XFRM core.

---
 include/linux/netdevice.h |  1 +
 include/net/xfrm.h        | 17 +++++++++++++++++
 net/xfrm/xfrm_output.c    |  1 -
 net/xfrm/xfrm_state.c     |  4 ++++
 4 files changed, 22 insertions(+), 1 deletion(-)
include/linux/netdevice.h +1 -0
index c1db9eaa3dca..e38154d7b4cd 100644
@@ -1026,6 +1026,7 @@ struct xfrmdev_ops {
 	bool	(*xdo_dev_offload_ok) (struct sk_buff *skb,
 				       struct xfrm_state *x);
 	void	(*xdo_dev_state_advance_esn) (struct xfrm_state *x);
+	void	(*xdo_dev_state_update_curlft) (struct xfrm_state *x);
 	int	(*xdo_dev_policy_add) (struct xfrm_policy *x);
 	void	(*xdo_dev_policy_delete) (struct xfrm_policy *x);
 	void	(*xdo_dev_policy_free) (struct xfrm_policy *x);
include/net/xfrm.h +17 -0
index 38fff78a1421..100ca45d8172 100644
@@ -1563,6 +1563,23 @@ struct xfrm_state *xfrm_stateonly_find(struct net *net, u32 mark, u32 if_id,
 struct xfrm_state *xfrm_state_lookup_byspi(struct net *net, __be32 spi,
 					      unsigned short family);
 int xfrm_state_check_expire(struct xfrm_state *x);
+#ifdef CONFIG_XFRM_OFFLOAD
+static inline void xfrm_dev_state_update_curlft(struct xfrm_state *x)
+{
+	struct xfrm_dev_offload *xdo = &x->xso;
+	struct net_device *dev = xdo->dev;
+
+	if (x->xso.type != XFRM_DEV_OFFLOAD_FULL)
+		return;
+
+	if (dev && dev->xfrmdev_ops &&
+	    dev->xfrmdev_ops->xdo_dev_state_update_curlft)
+		dev->xfrmdev_ops->xdo_dev_state_update_curlft(x);
+
+}
+#else
+static inline void xfrm_dev_state_update_curlft(struct xfrm_state *x) {}
+#endif
 void xfrm_state_insert(struct xfrm_state *x);
 int xfrm_state_add(struct xfrm_state *x);
 int xfrm_state_update(struct xfrm_state *x);
net/xfrm/xfrm_output.c +0 -1
index dde009be8463..a22033350ddc 100644
@@ -560,7 +560,6 @@ static int xfrm_output_one(struct sk_buff *skb, int err)
 			XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTSTATEPROTOERROR);
 			goto error_nolock;
 		}
-
 		dst = skb_dst_pop(skb);
 		if (!dst) {
 			XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTERROR);
net/xfrm/xfrm_state.c +4 -0
index 91c32a3b6924..83d307cb526f 100644
@@ -549,6 +549,8 @@ static enum hrtimer_restart xfrm_timer_handler(struct hrtimer *me)
 	int err = 0;
 
 	spin_lock(&x->lock);
+	xfrm_dev_state_update_curlft(x);
+
 	if (x->km.state == XFRM_STATE_DEAD)
 		goto out;
 	if (x->km.state == XFRM_STATE_EXPIRED)
@@ -1786,6 +1788,8 @@ EXPORT_SYMBOL(xfrm_state_update);
 
 int xfrm_state_check_expire(struct xfrm_state *x)
 {
+	xfrm_dev_state_update_curlft(x);
+
 	if (!x->curlft.use_time)
 		x->curlft.use_time = ktime_get_real_seconds();
 
Signature · 1 line
2.37.2
maintainer
#15 lore raw

Re: [PATCH RFC xfrm-next v3 7/8] xfrm: add support to HW update soft and hard limits

Quoted 6 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Both in RX and TX, the traffic that performs IPsec full offload transformation is accounted by HW. It is needed to properly handle hard limits that require to drop the packet.

Quoted 8 lines

It means that XFRM core needs to update internal counters with the one that accounted by the HW, so new callbacks are introduced in this patch.

In case of soft or hard limit is occurred, the driver should call to xfrm_state_check_expire() that will perform key rekeying exactly as done by XFRM core.

This looks good, thanks!

We need this for the other relevant counters too.

#16 lore raw

Re: [PATCH RFC xfrm-next v3 7/8] xfrm: add support to HW update soft and hard limits

Quoted 14 lines · Steffen Klassert
Quoted 13 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Both in RX and TX, the traffic that performs IPsec full offload transformation is accounted by HW. It is needed to properly handle hard limits that require to drop the packet.

It means that XFRM core needs to update internal counters with the one that accounted by the HW, so new callbacks are introduced in this patch.

In case of soft or hard limit is occurred, the driver should call to xfrm_state_check_expire() that will perform key rekeying exactly as done by XFRM core.

Quoted 5 lines
Quoted 1 line

This looks good, thanks!

We need this for the other relevant counters too.

It is in my backlog.

Thanks

maintainer
#17 lore raw

Re: [PATCH RFC xfrm-next v3 7/8] xfrm: add support to HW update soft and hard limits

Quoted 15 lines · Leon Romanovsky
Quoted 14 lines · Steffen Klassert
Quoted 13 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Both in RX and TX, the traffic that performs IPsec full offload transformation is accounted by HW. It is needed to properly handle hard limits that require to drop the packet.

It means that XFRM core needs to update internal counters with the one that accounted by the HW, so new callbacks are introduced in this patch.

In case of soft or hard limit is occurred, the driver should call to xfrm_state_check_expire() that will perform key rekeying exactly as done by XFRM core.

Quoted 7 lines
Quoted 5 lines
Quoted 1 line

This looks good, thanks!

We need this for the other relevant counters too.

It is in my backlog.

Great, thanks!

#18 lore raw

RFC PATCH v3 8/8 xfrm: document IPsec full offload mode

In reply to Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Extend XFRM device offload API description with newly added full offload mode.

---
 Documentation/networking/xfrm_device.rst | 62 ++++++++++++++++++++----
 1 file changed, 53 insertions(+), 9 deletions(-)
Documentation/networking/xfrm_device.rst +53 -9
index 01391dfd37d9..007b9de622c0 100644
@@ -5,6 +5,7 @@ XFRM device - offloading the IPsec computations
 ===============================================
 
 Shannon Nelson <[email protected]>
+Leon Romanovsky <[email protected]>
 
 
 Overview
@@ -18,10 +19,21 @@ can radically increase throughput and decrease CPU utilization.  The XFRM
 Device interface allows NIC drivers to offer to the stack access to the
 hardware offload.
 
+Right now, there are two types of hardware offload that kernel supports.
+ * IPsec crypto offload:
+   * NIC performs encrypt/decrypt
+   * Kernel does everything else
+ * IPsec full offload:
+   * NIC performs encrypt/decrypt
+   * NIC does encapsulation
+   * Kernel and NIC have SA and policy in-sync
+   * NIC handles the SA and policies states
+   * The Kernel talks to the keymanager
+
 Userland access to the offload is typically through a system such as
 libreswan or KAME/raccoon, but the iproute2 'ip xfrm' command set can
 be handy when experimenting.  An example command might look something
-like this::
+like this for crypto offload:
 
   ip x s add proto esp dst 14.0.0.70 src 14.0.0.52 spi 0x07 mode transport \
      reqid 0x07 replay-window 32 \
@@ -29,6 +41,17 @@ like this::
      sel src 14.0.0.52/24 dst 14.0.0.70/24 proto tcp \
      offload dev eth4 dir in
 
+and for full offload
+
+  ip x s add proto esp dst 14.0.0.70 src 14.0.0.52 spi 0x07 mode transport \
+     reqid 0x07 replay-window 32 \
+     aead 'rfc4106(gcm(aes))' 0x44434241343332312423222114131211f4f3f2f1 128 \
+     sel src 14.0.0.52/24 dst 14.0.0.70/24 proto tcp \
+     offload full dev eth4 dir in
+
+  ip x p add src 14.0.0.70 dst 14.0.0.52 offload full dev eth4 dir in
+  tmpl src 14.0.0.70 dst 14.0.0.52 proto esp reqid 10000 mode transport
+
 Yes, that's ugly, but that's what shell scripts and/or libreswan are for.
 
 
@@ -40,17 +63,24 @@ Callbacks to implement
 
   /* from include/linux/netdevice.h */
   struct xfrmdev_ops {
+        /* Crypto and Full offload callbacks */
 	int	(*xdo_dev_state_add) (struct xfrm_state *x);
 	void	(*xdo_dev_state_delete) (struct xfrm_state *x);
 	void	(*xdo_dev_state_free) (struct xfrm_state *x);
 	bool	(*xdo_dev_offload_ok) (struct sk_buff *skb,
 				       struct xfrm_state *x);
 	void    (*xdo_dev_state_advance_esn) (struct xfrm_state *x);
+
+        /* Solely full offload callbacks */
+	void    (*xdo_dev_state_update_curlft) (struct xfrm_state *x);
+	int	(*xdo_dev_policy_add) (struct xfrm_policy *x);
+	void	(*xdo_dev_policy_delete) (struct xfrm_policy *x);
+	void	(*xdo_dev_policy_free) (struct xfrm_policy *x);
   };
 
-The NIC driver offering ipsec offload will need to implement these
-callbacks to make the offload available to the network stack's
-XFRM subsystem.  Additionally, the feature bits NETIF_F_HW_ESP and
+The NIC driver offering ipsec offload will need to implement callbacks
+relevant to supported offload to make the offload available to the network
+stack's XFRM subsystem. Additionally, the feature bits NETIF_F_HW_ESP and
 NETIF_F_HW_ESP_TX_CSUM will signal the availability of the offload.
 
 
@@ -79,7 +109,8 @@ and an indication of whether it is for Rx or Tx.  The driver should
 
 		===========   ===================================
 		0             success
-		-EOPNETSUPP   offload not supported, try SW IPsec
+		-EOPNETSUPP   offload not supported, try SW IPsec,
+                              not applicable for full offload mode
 		other         fail the request
 		===========   ===================================
 
@@ -96,6 +127,7 @@ will serviceable.  This can check the packet information to be sure the
 offload can be supported (e.g. IPv4 or IPv6, no IPv4 options, etc) and
 return true of false to signify its support.
 
+Crypto offload mode:
 When ready to send, the driver needs to inspect the Tx packet for the
 offload information, including the opaque context, and set up the packet
 send accordingly::
@@ -139,13 +171,25 @@ the stack in xfrm_input().
 In ESN mode, xdo_dev_state_advance_esn() is called from xfrm_replay_advance_esn().
 Driver will check packet seq number and update HW ESN state machine if needed.
 
+Full offload mode:
+HW adds and deleted XFRM headers. So in RX path, XFRM stack is bypassed if HW
+reported success. In TX path, the packet lefts kernel without extra header
+and not encrypted, the HW is responsible to perform it.
+
 When the SA is removed by the user, the driver's xdo_dev_state_delete()
-is asked to disable the offload.  Later, xdo_dev_state_free() is called
-from a garbage collection routine after all reference counts to the state
+and xdo_dev_policy_delete() are asked to disable the offload.  Later,
+xdo_dev_state_free() and xdo_dev_policy_free() are called from a garbage
+collection routine after all reference counts to the state and policy
 have been removed and any remaining resources can be cleared for the
 offload state.  How these are used by the driver will depend on specific
 hardware needs.
 
 As a netdev is set to DOWN the XFRM stack's netdev listener will call
-xdo_dev_state_delete() and xdo_dev_state_free() on any remaining offloaded
-states.
+xdo_dev_state_delete(), xdo_dev_policy_delete(), xdo_dev_state_free() and
+xdo_dev_policy_free() on any remaining offloaded states.
+
+Outcome of HW handling packets, the XFRM core can't count hard, soft limits.
+The HW/driver are responsible to perform it and provide accurate data when
+xdo_dev_state_update_curlft() is called. In case of one of these limits
+occuried, the driver needs to call to xfrm_state_check_expire() to make sure
+that XFRM performs rekeying sequence.
Signature · 1 line
2.37.2
#19 lore raw
In reply to Leon Romanovsky
Quoted 10 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Changelog:
v4:
* Changed title from "PATCH" to "PATCH RFC" per-request.
* Added two new patches: one to update hard/soft limits and another initial take on documentation.
* Added more info about lifetime/rekeying flow to cover letter, see relevant section.
* perf traces for crypto mode will come later.

<...>

The series is v4 and not as written in subject title.

Thanks

Quoted 3 lines
Signature · 1 line
2.37.2
maintainer
#20 lore raw
In reply to Leon Romanovsky
Quoted 4 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Changelog:
v4:

<...>

Quoted 1 line

* perf traces for crypto mode will come later.

Single core RX:

# Children      Self       Samples  Command        Shared Object       Symbol
# ........  ........  ............  .............  ..................  .......................................

#
99.97% 0.00% 0 ksoftirqd/143 [kernel.vmlinux] [k] ret_from_fork
|
---ret_from_fork
kthread
smpboot_thread_fn
|
--99.96%--run_ksoftirqd
__do_softirq
|
--99.86%--net_rx_action
| |--61.49%--mlx5e_napi_poll
| |
| |--58.43%--mlx5e_poll_rx_cq
| | |
| | --57.27%--mlx5e_handle_rx_cqe
| | |
| | |--33.05%--napi_gro_receive
| | | |
| | | --32.42%--dev_gro_receive
| | | |
| | | --29.64%--inet_gro_receive
| | | |
| | | --27.70%--esp4_gro_receive
| | | |
| | | --25.80%--xfrm_input
| | | |
| | | |--6.86%--xfrm4_transport_finish
| | | | |
| | | | |--4.19%--__memmove
| | | | |
| | | | --1.27%--ip_send_check
| | | |
| | | |--6.02%--esp_input_done2
| | | | |
| | | | --3.26%--skb_copy_bits
| | | | |
| | | | --2.50%--memcpy_erms
| | | |
| | | |--2.19%--_raw_spin_lock
| | | |
| | | |--1.22%--xfrm_rcv_cb
| | | | |
| | | | --0.68%--xfrm4_rcv_cb
| | | |
| | | |--0.97%--xfrm_inner_mode_input.isra.35
| | | |
| | | |--0.97%--gro_cells_receive
| | | |
| | | |--0.69%--esp_input_tail
| | | |
| | | --0.66%--xfrm_parse_spi
| | |
| | |--11.91%--mlx5e_skb_from_cqe_linear
| | | |
| | | --5.63%--build_skb
| | | |
| | | --3.82%--__build_skb
| | | |
| | | --1.97%--kmem_cache_alloc
| | |
| | --9.97%--mlx5e_build_rx_skb
| | |
| | --7.23%--mlx5e_ipsec_offload_handle_rx_skb
| | |
| | |--3.60%--secpath_set
| | | |
| | | --3.41%--skb_ext_add
| | | |
| | | --2.69%--__skb_ext_alloc
| | | |
| | | --2.58%--kmem_cache_alloc
| | | |
| | | --0.60%--__slab_alloc
| | | |
| | | --0.56%--___slab_alloc
| | |
| | --2.52%--mlx5e_ipsec_sadb_rx_lookup
| |
| --2.78%--mlx5e_post_rx_wqes

I have TX traces too and can add if RX are not sufficient.

Thanks

maintainer
#21 lore raw
Quoted 1 line · On Thu, 8 Sep 2022 12:56:16 +0300 Leon Romanovsky wrote:

I have TX traces too and can add if RX are not sufficient.

The perf trace is good, but for those of us not intimately familiar with xfrm, could you provide some analysis here?

#22 lore raw
Quoted 5 lines · Jakub Kicinski
Quoted 1 line · On Thu, 8 Sep 2022 12:56:16 +0300 Leon Romanovsky wrote:

I have TX traces too and can add if RX are not sufficient.

The perf trace is good, but for those of us not intimately familiar with xfrm, could you provide some analysis here?

The perf trace presented is for RX path of IPsec crypto offload mode. In that mode, decrypted packet enters the netdev stack to perform various XFRM specific checks.

The trace presents "the cost" of these checks, which is 25% according to the line "--25.80%--xfrm_input".

The xfrm_input has number of "slow" places (other places are not fast either), which are handled by HW in parallel without any locks in IPsec full offload mode.

The avoided checks include:
* XFRM state lookup. It is linked list iteration.
* Lock of whole xfrm_state. It means that parallel traffic will be congested on this lock.
* Double calculation of replay window protection.
* Update of replay window.

https://elixir.bootlin.com/linux/v6.0-rc6/source/net/xfrm/xfrm_input.c#L459
int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type) {
...

		x = xfrm_state_lookup(net, mark, daddr, spi, nexthdr, family);
...
		spin_lock(&x->lock);
...
		if (xfrm_replay_check(x, skb, seq)) {
...
		spin_unlock(&x->lock);
...
		spin_lock(&x->lock);
...
		if (xfrm_replay_recheck(x, skb, seq)) {
...
		xfrm_replay_advance(x, seq);

.....

maintainer
#23 lore raw
Quoted 10 lines · Leon Romanovsky
Quoted 5 lines · Jakub Kicinski
Quoted 1 line · On Thu, 8 Sep 2022 12:56:16 +0300 Leon Romanovsky wrote:

I have TX traces too and can add if RX are not sufficient.

The perf trace is good, but for those of us not intimately familiar with xfrm, could you provide some analysis here?

The perf trace presented is for RX path of IPsec crypto offload mode. In that mode, decrypted packet enters the netdev stack to perform various XFRM specific checks.

Can you provide the perf traces and analysis for the TX side too? That would be interesting in particular, because the policy and state lookups there happen still in software.

Thanks a lot for your effort on this!

#24 lore raw
Quoted 8 lines · Steffen Klassert
Quoted 7 lines · Leon Romanovsky
Quoted 5 lines · Jakub Kicinski
Quoted 1 line · On Thu, 8 Sep 2022 12:56:16 +0300 Leon Romanovsky wrote:

I have TX traces too and can add if RX are not sufficient.

The perf trace is good, but for those of us not intimately familiar with xfrm, could you provide some analysis here?

Quoted 7 lines
Quoted 3 lines

The perf trace presented is for RX path of IPsec crypto offload mode. In that mode, decrypted packet enters the netdev stack to perform various XFRM specific checks.

Can you provide the perf traces and analysis for the TX side too? That would be interesting in particular, because the policy and state lookups there happen still in software.

Single core TX (crypto mode) from the same run:
Please notice that it is not really bottleneck, probably RX caused to the situation where TX was not executed enough. It is also lighter path than RX.

# Children      Self       Samples  Command          Shared Object       Symbol                                        
# ........  ........  ............  ...............  ..................  ..............................................

#
86.58% 0.00% 0 swapper [kernel.vmlinux] [k] secondary_startup_64_no_verify
|
---secondary_startup_64_no_verify
start_secondary
cpu_startup_entry
do_idle
|
--86.37%--cpu_idle_poll
|
--24.53%--asm_common_interrupt
|
--24.48%--common_interrupt
|
|--23.47%--irq_exit_rcu
| |
| --23.23%--do_softirq_own_stack
| |
| --23.17%--asm_call_irq_on_stack
| __do_softirq
| |
| |--22.23%--net_rx_action
| | |
| | |--20.17%--gro_cell_poll
| | | |
| | | --20.02%--napi_complete_done
| | | |
| | | --19.98%--gro_normal_list.part.154
| | | |
| | | --19.96%--netif_receive_skb_list_internal
| | | |
| | | --19.89%--__netif_receive_skb_list_core
| | | |
| | | --19.77%--ip_list_rcv
| | | |
| | | --19.67%--ip_sublist_rcv
| | | |
| | | --19.56%--ip_sublist_rcv_finish
| | | |
| | | --19.54%--ip_local_deliver
| | | |
| | | --19.49%--ip_local_deliver_finish
| | | |
| | | --19.47%--ip_protocol_deliver_rcu
| | | |
| | | --19.43%--tcp_v4_rcv
| | | |
| | | --18.87%--tcp_v4_do_rcv
| | | |
| | | --18.83%--tcp_rcv_established
| | | |
| | | |--16.41%--__tcp_push_pending_frames
| | | | |
| | | | --16.38%--tcp_write_xmit
| | | | |
| | | | |--6.35%--tcp_event_new_data_sent
| | | | | |
| | | | | --6.22%--sk_reset_timer
| | | | | |
| | | | | --6.21%--mod_timer
| | | | | |
| | | | | --6.10%--get_nohz_timer_target
| | | | | |
| | | | | --1.87%--cpumask_next_and
| | | | | |
| | | | | --1.07%--_find_next_bit.constprop.1
| | | | |
| | | | |--5.50%--tcp_schedule_loss_probe
| | | | | |
| | | | | --5.49%--sk_reset_timer
| | | | | mod_timer
| | | | | |
| | | | | --5.43%--get_nohz_timer_target
| | | | | |
| | | | | --1.37%--cpumask_next_and
| | | | | |
| | | | | --0.71%--_find_next_bit.constprop.1
| | | | |
| | | | --4.31%--__tcp_transmit_skb
| | | | |
| | | | --3.87%--__ip_queue_xmit
| | | | |
| | | | --3.54%--xfrm4_output
| | | | |
| | | | --3.26%--xfrm_output_resume
| | | | |
| | | | --2.88%--ip_output
| | | | |
| | | | --2.78%--ip_finish_output2
| | | | |
| | | | --2.73%--__dev_queue_xmit
| | | | |
| | | | --2.49%--sch_direct_xmit
| | | | |
| | | | |--1.50%--validate_xmit_skb_list
| | | | | | |
| | | | | --1.32%--validate_xmit_skb
| | | | | | |
| | | | | --1.06%--__skb_gso_segment
| | | | | | |
| | | | | --1.04%--skb_mac_gso_segment
| | | | | |
| | | | --1.02%--inet_gso_segment
| | | | | |
| | | | --0.93%--esp4_gso_segment
| | | | | |
| | | | --0.86%--tcp_gso_segment
| | | | |
| | | --0.78%--skb_segment
| | | | |
| | | | --0.77%--dev_hard_start_xmit
| | | | | |
| | | | --0.75%--mlx5e_xmit
| | | |
| | | --1.87%--tcp_ack
| | | |
| | | --1.66%--tcp_clean_rtx_queue
| | | |
| | | --1.35%--__kfree_skb
| | | |
| | | --1.21%--skb_release_data
| | |
| | --1.92%--mlx5e_napi_poll
| | |
| | --1.38%--mlx5e_poll_rx_cq
| | |
| | --1.33%--mlx5e_handle_rx_cqe
| | |
| | --0.53%--napi_gro_receive
| | |
| | --0.52%--dev_gro_receive
| |
| --0.77%--tasklet_action_common.isra.17
|
--0.80%--asm_call_irq_on_stack
|
--0.78%--handle_edge_irq
|
--0.74%--handle_irq_event
|
--0.71%--handle_irq_event_percpu
|
--0.64%--__handle_irq_event_percpu
|
--0.60%--mlx5_irq_int_handler
|
--0.58%--atomic_notifier_call_chain
|
--0.57%--mlx5_eq_comp_int

maintainer
#25 lore raw
Quoted 13 lines · Leon Romanovsky
Quoted 12 lines · Steffen Klassert
Quoted 10 lines · Leon Romanovsky
Quoted 5 lines · Jakub Kicinski
Quoted 1 line · On Thu, 8 Sep 2022 12:56:16 +0300 Leon Romanovsky wrote:

I have TX traces too and can add if RX are not sufficient.

The perf trace is good, but for those of us not intimately familiar with xfrm, could you provide some analysis here?

The perf trace presented is for RX path of IPsec crypto offload mode. In that mode, decrypted packet enters the netdev stack to perform various XFRM specific checks.

Quoted 7 lines
Quoted 3 lines

Can you provide the perf traces and analysis for the TX side too? That would be interesting in particular, because the policy and state lookups there happen still in software.

Single core TX (crypto mode) from the same run:
Please notice that it is not really bottleneck, probably RX caused to the situation where TX was not executed enough. It is also lighter path than RX.

Thanks for this! How many policies and SAs were installed when you ran this? A run with 'many' policies and SAs would be interesting, in particualar a comparison between crypto and full offload. That would show us where the performance of the full offload comes from.

#26 lore raw
Quoted 18 lines · Steffen Klassert
Quoted 17 lines · Leon Romanovsky
Quoted 15 lines · Steffen Klassert
Quoted 10 lines · Leon Romanovsky
Quoted 5 lines · Jakub Kicinski
Quoted 1 line · On Thu, 8 Sep 2022 12:56:16 +0300 Leon Romanovsky wrote:

I have TX traces too and can add if RX are not sufficient.

The perf trace is good, but for those of us not intimately familiar with xfrm, could you provide some analysis here?

The perf trace presented is for RX path of IPsec crypto offload mode. In that mode, decrypted packet enters the netdev stack to perform various XFRM specific checks.

Can you provide the perf traces and analysis for the TX side too? That would be interesting in particular, because the policy and state lookups there happen still in software.

Quoted 8 lines
Quoted 3 lines

Single core TX (crypto mode) from the same run:
Please notice that it is not really bottleneck, probably RX caused to the situation where TX was not executed enough. It is also lighter path than RX.

Thanks for this! How many policies and SAs were installed when you ran this? A run with 'many' policies and SAs would be interesting, in particualar a comparison between crypto and full offload. That would show us where the performance of the full offload comes from.

It was 160 CPU machine with policy/SA per-CPU and direction. In total, 320 policies and 320 SAs.

Thanks

#27 lore raw
In reply to Leon Romanovsky
Quoted 1 line · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

<...>

Quoted 9 lines

Leon Romanovsky (8):
xfrm: add new full offload flag
xfrm: allow state full offload mode
xfrm: add an interface to offload policy
xfrm: add TX datapath support for IPsec full offload mode
xfrm: add RX datapath protection for IPsec full offload mode
xfrm: enforce separation between priorities of HW/SW policies
xfrm: add support to HW update soft and hard limits
xfrm: document IPsec full offload mode

Kindly reminder.

Thanks

#28 lore raw
Quoted 15 lines · Leon Romanovsky
Quoted 1 line · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

<...>

Quoted 9 lines

Leon Romanovsky (8):
xfrm: add new full offload flag
xfrm: allow state full offload mode
xfrm: add an interface to offload policy
xfrm: add TX datapath support for IPsec full offload mode
xfrm: add RX datapath protection for IPsec full offload mode
xfrm: enforce separation between priorities of HW/SW policies
xfrm: add support to HW update soft and hard limits
xfrm: document IPsec full offload mode

Quoted 1 line

Kindly reminder.

Hi Steffen,

Can we please progress with the series? I would like to see it is merged in this merge window, so I will be able to progress with iproute2 and mlx5 code too.

It is approximately 3 weeks already in the ML without any objections. The implementation proposed here has nothing specific to our HW and applicable to other vendors as well.

Thanks

Quoted 2 lines

Thanks

maintainer
#29 lore raw
Quoted 20 lines · Leon Romanovsky
Quoted 15 lines · Leon Romanovsky
Quoted 1 line · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

<...>

Quoted 9 lines

Leon Romanovsky (8):
xfrm: add new full offload flag
xfrm: allow state full offload mode
xfrm: add an interface to offload policy
xfrm: add TX datapath support for IPsec full offload mode
xfrm: add RX datapath protection for IPsec full offload mode
xfrm: enforce separation between priorities of HW/SW policies
xfrm: add support to HW update soft and hard limits
xfrm: document IPsec full offload mode

Quoted 1 line

Kindly reminder.

Hi Steffen,

Quoted 7 lines

Can we please progress with the series? I would like to see it is merged in this merge window, so I will be able to progress with iproute2 and mlx5 code too.

It is approximately 3 weeks already in the ML without any objections. The implementation proposed here has nothing specific to our HW and applicable to other vendors as well.

I've just replied to our private thread about this. I'll have a look at the patches tomorrow. I've just returned from vacation and still a bit backloged...