Extend XFRM core to allow full offload configuration
Subsystems (9)
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
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
@@ -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
@@ -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 4struct xfrm_userpolicy_default {#define XFRM_USERPOLICY_UNSPEC 0
net/xfrm/xfrm_device.c +3 -0
@@ -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
@@ -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
RFC PATCH v3 2/8 xfrm: allow state full offload mode
In reply to Leon RomanovskyFrom: 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
RFC PATCH v3 3/8 xfrm: add an interface to offload policy
In reply to Leon RomanovskyFrom: 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
@@ -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
@@ -1747,6 +1747,14 @@ static struct xfrm_policy *xfrm_policy_construct(struct net *net, struct xfrm_usif (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
RFC PATCH v3 4/8 xfrm: add TX datapath support for IPsec full offload mode
In reply to Leon RomanovskyFrom: 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
@@ -120,6 +120,16 @@ struct sk_buff *validate_xmit_xfrm(struct sk_buff *skb, netdev_features_t featurif (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
@@ -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
Steffen Klassert
maintainerRe: [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.
Signed-off-by Raed Salem <[email protected]>Signed-off-by Huy Nguyen <[email protected]>Signed-off-by Leon Romanovsky <[email protected]>--- 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@@ -120,6 +120,16 @@ struct sk_buff *validate_xmit_xfrm(struct sk_buff *skb, netdev_features_t featurif (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
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.
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.
Signed-off-by Raed Salem <[email protected]>Signed-off-by Huy Nguyen <[email protected]>Signed-off-by Leon Romanovsky <[email protected]>--- 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@@ -120,6 +120,16 @@ struct sk_buff *validate_xmit_xfrm(struct sk_buff *skb, netdev_features_t featurif (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@@ -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
Steffen Klassert
maintainerRe: [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@@ -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.
RFC PATCH v3 5/8 xfrm: add RX datapath protection for IPsec full offload mode
In reply to Leon RomanovskyFrom: 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
@@ -1094,6 +1094,29 @@ xfrm_state_addr_cmp(const struct xfrm_tmpl *tmpl, const struct xfrm_state *x, unreturn !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
RFC PATCH v3 6/8 xfrm: enforce separation between priorities of HW/SW policies
In reply to Leon RomanovskyFrom: 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
@@ -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
Steffen Klassert
maintainerRe: [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.
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
Steffen Klassert
maintainerRe: [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.
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
RFC PATCH v3 7/8 xfrm: add support to HW update soft and hard limits
In reply to Leon RomanovskyFrom: 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
@@ -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
@@ -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
@@ -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
@@ -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
Steffen Klassert
maintainerRe: [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.Signed-off-by Leon Romanovsky <[email protected]>
This looks good, thanks!
We need this for the other relevant counters too.
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
Signed-off-by Leon Romanovsky <[email protected]>This looks good, thanks!
We need this for the other relevant counters too.
It is in my backlog.
Thanks
Steffen Klassert
maintainerRe: [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
Signed-off-by Leon Romanovsky <[email protected]>This looks good, thanks!
We need this for the other relevant counters too.
It is in my backlog.
Great, thanks!
RFC PATCH v3 8/8 xfrm: document IPsec full offload mode
In reply to Leon RomanovskyFrom: 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
@@ -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
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
Leon Romanovsky
maintainerQuoted 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
Jakub Kicinski
maintainerQuoted 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 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);.....
Steffen Klassert
maintainerQuoted 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!
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
Steffen Klassert
maintainerQuoted 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.
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
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
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
Steffen Klassert
maintainerQuoted 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 modeQuoted 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...