Extra IPsec cleanup
Subsystems (6)
cover Extra IPsec cleanup
From: Leon Romanovsky <[email protected]>
Changelog:
v1:
* changed target from mlx5-next to net-next.
* Improved commit message in patch #1
* Left function names intact, with _accel_ word in it.
v0: https://lore.kernel.org/all/[email protected]
--------------------
After FPGA IPsec removal, we can go further and make sure that flow steering logic is aligned to mlx5_core standard together with deep cleaning of whole IPsec path.
Thanks
Leon Romanovsky (17):
net/mlx5: Simplify IPsec flow steering init/cleanup functions net/mlx5: Check IPsec TX flow steering namespace in advance net/mlx5: Don't hide fallback to software IPsec in FS code net/mlx5: Reduce useless indirection in IPsec FS add/delete flows net/mlx5: Store IPsec ESN update work in XFRM state net/mlx5: Remove useless validity check
net/mlx5: Merge various control path IPsec headers into one file net/mlx5: Remove indirections from esp functions net/mlx5: Simplify HW context interfaces by using SA entry net/mlx5: Clean IPsec FS add/delete rules
net/mlx5: Make sure that no dangling IPsec FS pointers exist net/mlx5: Don't advertise IPsec netdev support for non-IPsec device net/mlx5: Simplify IPsec capabilities logic
net/mlx5: Remove not-supported ICV length
net/mlx5: Cleanup XFRM attributes struct
net/mlx5: Allow future addition of IPsec object modifiers net/mlx5: Don't perform lookup after already known sec_path
.../net/ethernet/mellanox/mlx5/core/en/fs.h | 1 - .../ethernet/mellanox/mlx5/core/en/params.c | 2 +- .../mellanox/mlx5/core/en_accel/ipsec.c | 174 +++------ .../mellanox/mlx5/core/en_accel/ipsec.h | 85 +++- .../mellanox/mlx5/core/en_accel/ipsec_fs.c | 362 ++++++------------ .../mellanox/mlx5/core/en_accel/ipsec_fs.h | 4 +- .../mlx5/core/en_accel/ipsec_offload.c | 331 +++------------- .../mlx5/core/en_accel/ipsec_offload.h | 14 - .../mellanox/mlx5/core/en_accel/ipsec_rxtx.c | 6 +- .../mellanox/mlx5/core/en_accel/ipsec_stats.c | 4 +- .../net/ethernet/mellanox/mlx5/core/en_main.c | 1 - .../net/ethernet/mellanox/mlx5/core/en_rx.c | 2 +- .../net/ethernet/mellanox/mlx5/core/main.c | 2 +- include/linux/mlx5/accel.h | 153 -------- include/linux/mlx5/mlx5_ifc.h | 2 - 15 files changed, 320 insertions(+), 823 deletions(-) delete mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.h delete mode 100644 include/linux/mlx5/accel.h
Signature · 1 line
2.35.1
PATCH 1/17 net/mlx5: Simplify IPsec flow steering init/cleanup functions
From: Leon Romanovsky <[email protected]>
Remove multiple function wrappers to make sure that IPsec FS initialization and cleanup functions present one place to help with code readability.
--- .../mellanox/mlx5/core/en_accel/ipsec.c | 4 +- .../mellanox/mlx5/core/en_accel/ipsec_fs.c | 73 ++++++------------- .../mellanox/mlx5/core/en_accel/ipsec_fs.h | 4 +- 3 files changed, 27 insertions(+), 54 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +2 -2
@@ -424,7 +424,7 @@ int mlx5e_ipsec_init(struct mlx5e_priv *priv) } priv->ipsec = ipsec; -mlx5e_accel_ipsec_fs_init (priv); +mlx5e_accel_ipsec_fs_init (ipsec);netdev_dbg (priv->netdev,"IPSec attached to netdevice\n" );return 0 ; }
@@ -436,7 +436,7 @@ void mlx5e_ipsec_cleanup(struct mlx5e_priv *priv)if (!ipsec)return ; -mlx5e_accel_ipsec_fs_cleanup (priv); +mlx5e_accel_ipsec_fs_cleanup (ipsec);destroy_workqueue (ipsec->wq);kfree (ipsec);
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +23 -50
104 lines not on this page · raw message
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.h +2 -2
@@ -9,8 +9,8 @@#include "ipsec_offload.h" #include "en/fs.h" -void mlx5e_accel_ipsec_fs_cleanup (struct mlx5e_priv *priv); -int mlx5e_accel_ipsec_fs_init (struct mlx5e_priv *priv); +void mlx5e_accel_ipsec_fs_cleanup (struct mlx5e_ipsec *ipsec); +int mlx5e_accel_ipsec_fs_init (struct mlx5e_ipsec *ipsec); int mlx5e_accel_ipsec_fs_add_rule(struct mlx5e_priv *priv, struct mlx5_accel_esp_xfrm_attrs *attrs, u32 ipsec_obj_id,
Signature · 1 line
2.35.1
PATCH 2/17 net/mlx5: Check IPsec TX flow steering namespace in advance
In reply to Leon RomanovskyFrom: Leon Romanovsky <[email protected]>
Ensure that flow steering is usable as early as possible, to understand if crypto IPsec is supported or not.
--- drivers/net/ethernet/mellanox/mlx5/core/en/fs.h | 1 - .../ethernet/mellanox/mlx5/core/en_accel/ipsec.c | 1 + .../ethernet/mellanox/mlx5/core/en_accel/ipsec.h | 1 + .../mellanox/mlx5/core/en_accel/ipsec_fs.c | 16 +++++++++------- 4 files changed, 11 insertions(+), 8 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en/fs.h +0 -1
@@ -164,7 +164,6 @@ struct mlx5e_ptp_fs;struct mlx5e_flow_steering {struct mlx5_flow_namespace *ns; -struct mlx5_flow_namespace *egress_ns;#ifdef CONFIG_MLX5_EN_RXNFC struct mlx5e_ethtool_steering ethtool;#endif
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +1 -0
@@ -415,6 +415,7 @@ int mlx5e_ipsec_init(struct mlx5e_priv *priv)hash_init (ipsec->sadb_rx);spin_lock_init (&ipsec->sadb_rx_lock); + ipsec->mdev = priv->mdev; ipsec->en_priv = priv; ipsec->wq =alloc_ordered_workqueue ("mlx5e_ipsec: %s" ,0 , priv->netdev->name);
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h +1 -0
@@ -61,6 +61,7 @@ struct mlx5e_accel_fs_esp;struct mlx5e_ipsec_tx ;struct mlx5e_ipsec { +struct mlx5_core_dev *mdev;struct mlx5e_priv *en_priv; DECLARE_HASHTABLE(sadb_rx, MLX5E_IPSEC_SADB_RX_BITS);spinlock_t sadb_rx_lock;/* Protects sadb_rx */
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +9 -7
@@ -35,6 +35,7 @@ struct mlx5e_accel_fs_esp { };struct mlx5e_ipsec_tx { +struct mlx5_flow_namespace *ns;struct mlx5_flow_table *ft;struct mutex mutex;/* Protect IPsec TX steering */ u32 refcnt;
@@ -338,15 +339,9 @@ static int tx_create(struct mlx5e_priv *priv)struct mlx5_flow_table *ft;int err; - priv->fs.egress_ns = -mlx5_get_flow_namespace (priv->mdev, -MLX5_FLOW_NAMESPACE_EGRESS_KERNEL ); -if (!priv->fs.egress_ns) -return -EOPNOTSUPP ; - ft_attr.max_fte =NUM_IPSEC_FTE ; ft_attr.autogroup.max_num_groups =1 ; - ft =mlx5_create_auto_grouped_flow_table (priv->fs.egress_ns, &ft_attr); + ft =mlx5_create_auto_grouped_flow_table (ipsec->tx_fs->ns, &ft_attr);if (IS_ERR (ft)) { err =PTR_ERR (ft);netdev_err (priv->netdev,"fail to create ipsec tx ft err=%d\n" , err);
@@ -658,9 +653,15 @@ int mlx5e_accel_ipsec_fs_init(struct mlx5e_ipsec *ipsec) {struct mlx5e_accel_fs_esp_prot *fs_prot;struct mlx5e_accel_fs_esp *accel_esp; +struct mlx5_flow_namespace *ns;enum accel_fs_esp_type i;int err = -ENOMEM ; + ns =mlx5_get_flow_namespace (ipsec->mdev, +MLX5_FLOW_NAMESPACE_EGRESS_KERNEL ); +if (!ns) +return -EOPNOTSUPP ; + ipsec->tx_fs =kzalloc (sizeof (*ipsec->tx_fs),GFP_KERNEL );if (!ipsec->tx_fs)return -ENOMEM ;
@@ -670,6 +671,7 @@ int mlx5e_accel_ipsec_fs_init(struct mlx5e_ipsec *ipsec) goto err_rx;mutex_init (&ipsec->tx_fs->mutex); + ipsec->tx_fs->ns = ns; accel_esp = ipsec->rx_fs;for (i =0 ; i <ACCEL_FS_ESP_NUM_TYPES ; i++) {
Signature · 1 line
2.35.1
PATCH 3/17 net/mlx5: Don't hide fallback to software IPsec in FS code
In reply to Leon RomanovskyFrom: Leon Romanovsky <[email protected]>
The XFRM code performs fallback to software IPsec if .xdo_dev_state_add() returns -EOPNOTSUPP. This is what mlx5 did very deep in its stack trace, despite have all the knowledge that IPsec is not going to work in very early stage.
This is achieved by making sure that priv->ipsec pointer is valid for fully working and supported hardware crypto IPsec engine.
In case, the hardware IPsec is not supported, the XFRM code will set NULL to xso->dev and it will prevent from calls to various .xdo_dev_state_*() callbacks.
--- .../mellanox/mlx5/core/en_accel/ipsec.c | 41 ++++++++----------- .../mellanox/mlx5/core/en_accel/ipsec_fs.c | 6 --- 2 files changed, 17 insertions(+), 30 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +17 -24
@@ -43,17 +43,7 @@static struct mlx5e_ipsec_sa_entry *to_ipsec_sa_entry (struct xfrm_state *x) { -struct mlx5e_ipsec_sa_entry *sa; - -if (!x) -return NULL ; - - sa = (struct mlx5e_ipsec_sa_entry *)x->xso.offload_handle; -if (!sa) -return NULL ; - -WARN_ON (sa->x != x); -return sa; +return (struct mlx5e_ipsec_sa_entry *)x->xso.offload_handle; } struct xfrm_state *mlx5e_ipsec_sadb_rx_lookup(struct mlx5e_ipsec *ipsec,
@@ -306,6 +296,8 @@ static int mlx5e_xfrm_add_state(struct xfrm_state *x)int err; priv =netdev_priv (netdev); +if (!priv->ipsec) +return -EOPNOTSUPP ; err =mlx5e_xfrm_validate_state (x);if (err)
@@ -375,9 +367,6 @@ static void mlx5e_xfrm_del_state(struct xfrm_state *x) {struct mlx5e_ipsec_sa_entry *sa_entry =to_ipsec_sa_entry (x); -if (!sa_entry) -return ; -if (x->xso.flags &XFRM_OFFLOAD_INBOUND )mlx5e_ipsec_sadb_rx_del (sa_entry); }
@@ -387,9 +376,6 @@ static void mlx5e_xfrm_free_state(struct xfrm_state *x)struct mlx5e_ipsec_sa_entry *sa_entry =to_ipsec_sa_entry (x);struct mlx5e_priv *priv =netdev_priv (x->xso.dev); -if (!sa_entry) -return ; -if (sa_entry->hw_context) {flush_workqueue (sa_entry->ipsec->wq);mlx5e_xfrm_fs_del_rule (priv, sa_entry);
@@ -402,7 +388,8 @@ static void mlx5e_xfrm_free_state(struct xfrm_state *x)int mlx5e_ipsec_init (struct mlx5e_priv *priv) { -struct mlx5e_ipsec *ipsec =NULL ; +struct mlx5e_ipsec *ipsec; +int ret;if (!mlx5_ipsec_device_caps (priv->mdev)) {netdev_dbg (priv->netdev,"Not an IPSec offload device\n" );
@@ -420,14 +407,23 @@ int mlx5e_ipsec_init(struct mlx5e_priv *priv) ipsec->wq =alloc_ordered_workqueue ("mlx5e_ipsec: %s" ,0 , priv->netdev->name);if (!ipsec->wq) { -kfree (ipsec); -return -ENOMEM ; + ret = -ENOMEM ; + goto err_wq; } + ret =mlx5e_accel_ipsec_fs_init (ipsec); +if (ret) + goto err_fs_init; + priv->ipsec = ipsec; -mlx5e_accel_ipsec_fs_init (ipsec);netdev_dbg (priv->netdev,"IPSec attached to netdevice\n" );return 0 ; + +err_fs_init: +destroy_workqueue (ipsec->wq); +err_wq: +kfree (ipsec); +return (ret != -EOPNOTSUPP ) ? ret :0 ; }void mlx5e_ipsec_cleanup (struct mlx5e_priv *priv)
@@ -487,9 +483,6 @@ static void mlx5e_xfrm_advance_esn_state(struct xfrm_state *x)struct mlx5e_ipsec_modify_state_work *modify_work;bool need_update; -if (!sa_entry) -return ; - need_update =mlx5e_ipsec_update_esn_state (sa_entry);if (!need_update)return ;
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +0 -6
@@ -605,9 +605,6 @@ int mlx5e_accel_ipsec_fs_add_rule(struct mlx5e_priv *priv, u32 ipsec_obj_id, struct mlx5e_ipsec_rule *ipsec_rule) { -if (!priv->ipsec->rx_fs) -return -EOPNOTSUPP ; -if (attrs->action ==MLX5_ACCEL_ESP_ACTION_DECRYPT )return rx_add_rule (priv, attrs, ipsec_obj_id, ipsec_rule);else
@@ -618,9 +615,6 @@ void mlx5e_accel_ipsec_fs_del_rule(struct mlx5e_priv *priv, struct mlx5_accel_esp_xfrm_attrs *attrs, struct mlx5e_ipsec_rule *ipsec_rule) { -if (!priv->ipsec->rx_fs) -return ; -if (attrs->action ==MLX5_ACCEL_ESP_ACTION_DECRYPT )rx_del_rule (priv, attrs, ipsec_rule);else
Signature · 1 line
2.35.1
PATCH 4/17 net/mlx5: Reduce useless indirection in IPsec FS add/delete flows
In reply to Leon RomanovskyFrom: Leon Romanovsky <[email protected]>
There is no need in one-liners wrappers to call internal functions. Let's remove them, together with rename of functions to remove _accel_ notation from their names.
--- .../mellanox/mlx5/core/en_accel/ipsec.c | 25 ++++++------------- .../mellanox/mlx5/core/en_accel/ipsec_fs.c | 4 +-- 2 files changed, 9 insertions(+), 20 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +7 -18
@@ -271,21 +271,6 @@ static inline int mlx5e_xfrm_validate_state(struct xfrm_state *x)return 0 ; } -static int mlx5e_xfrm_fs_add_rule (struct mlx5e_priv *priv, -struct mlx5e_ipsec_sa_entry *sa_entry) -{ -return mlx5e_accel_ipsec_fs_add_rule (priv, &sa_entry->xfrm->attrs, - sa_entry->ipsec_obj_id, - &sa_entry->ipsec_rule); -} - -static void mlx5e_xfrm_fs_del_rule (struct mlx5e_priv *priv, -struct mlx5e_ipsec_sa_entry *sa_entry) -{ -mlx5e_accel_ipsec_fs_del_rule (priv, &sa_entry->xfrm->attrs, - &sa_entry->ipsec_rule); -} -static int mlx5e_xfrm_add_state (struct xfrm_state *x) {struct mlx5e_ipsec_sa_entry *sa_entry =NULL ;
@@ -334,7 +319,9 @@ static int mlx5e_xfrm_add_state(struct xfrm_state *x) } sa_entry->ipsec_obj_id = sa_handle; - err =mlx5e_xfrm_fs_add_rule (priv, sa_entry); + err =mlx5e_accel_ipsec_fs_add_rule (priv, &sa_entry->xfrm->attrs, + sa_entry->ipsec_obj_id, + &sa_entry->ipsec_rule);if (err) goto err_hw_ctx;
@@ -351,7 +338,8 @@ static int mlx5e_xfrm_add_state(struct xfrm_state *x) goto out; err_add_rule: -mlx5e_xfrm_fs_del_rule (priv, sa_entry); +mlx5e_accel_ipsec_fs_del_rule (priv, &sa_entry->xfrm->attrs, + &sa_entry->ipsec_rule); err_hw_ctx:mlx5_accel_esp_free_hw_context (priv->mdev, sa_entry->hw_context); err_xfrm:
@@ -378,7 +366,8 @@ static void mlx5e_xfrm_free_state(struct xfrm_state *x)if (sa_entry->hw_context) {flush_workqueue (sa_entry->ipsec->wq); -mlx5e_xfrm_fs_del_rule (priv, sa_entry); +mlx5e_accel_ipsec_fs_del_rule (priv, &sa_entry->xfrm->attrs, + &sa_entry->ipsec_rule);mlx5_accel_esp_free_hw_context (sa_entry->xfrm->mdev, sa_entry->hw_context);mlx5_accel_esp_destroy_xfrm (sa_entry->xfrm); }
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +2 -2
@@ -612,8 +612,8 @@ int mlx5e_accel_ipsec_fs_add_rule(struct mlx5e_priv *priv, }void mlx5e_accel_ipsec_fs_del_rule (struct mlx5e_priv *priv, -struct mlx5_accel_esp_xfrm_attrs *attrs, -struct mlx5e_ipsec_rule *ipsec_rule) +struct mlx5_accel_esp_xfrm_attrs *attrs, +struct mlx5e_ipsec_rule *ipsec_rule) {if (attrs->action ==MLX5_ACCEL_ESP_ACTION_DECRYPT )rx_del_rule (priv, attrs, ipsec_rule);
Signature · 1 line
2.35.1
PATCH 5/17 net/mlx5: Store IPsec ESN update work in XFRM state
In reply to Leon RomanovskyFrom: Leon Romanovsky <[email protected]>
mlx5 IPsec code updated ESN through workqueue with allocation calls in the data path, which can be saved easily if the work is created during XFRM state initialization routine.
The locking used later in the work didn't protect from anything because change of HW context is possible during XFRM state add or delete only, which can cancel work and make sure that it is not running.
--- .../mellanox/mlx5/core/en_accel/ipsec.c | 49 ++++++------------- .../mellanox/mlx5/core/en_accel/ipsec.h | 7 ++- .../mlx5/core/en_accel/ipsec_offload.c | 39 +++------------ include/linux/mlx5/accel.h | 10 ++-- 4 files changed, 33 insertions(+), 72 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +15 -34
@@ -271,6 +271,16 @@ static inline int mlx5e_xfrm_validate_state(struct xfrm_state *x)return 0 ; } +static void _update_xfrm_state (struct work_struct *work) +{ +struct mlx5e_ipsec_modify_state_work *modify_work = + container_of(work, struct mlx5e_ipsec_modify_state_work, work); +struct mlx5e_ipsec_sa_entry *sa_entry =container_of ( + modify_work, struct mlx5e_ipsec_sa_entry, modify_work); + +mlx5_accel_esp_modify_xfrm (sa_entry->xfrm, &modify_work->attrs); +} +static int mlx5e_xfrm_add_state (struct xfrm_state *x) {struct mlx5e_ipsec_sa_entry *sa_entry =NULL ;
@@ -334,6 +344,7 @@ static int mlx5e_xfrm_add_state(struct xfrm_state *x) mlx5e_ipsec_set_iv_esn : mlx5e_ipsec_set_iv; } +INIT_WORK (&sa_entry->modify_work.work, _update_xfrm_state); x->xso.offload_handle = (unsigned long )sa_entry; goto out;
@@ -365,7 +376,7 @@ static void mlx5e_xfrm_free_state(struct xfrm_state *x)struct mlx5e_priv *priv =netdev_priv (x->xso.dev);if (sa_entry->hw_context) { -flush_workqueue (sa_entry->ipsec->wq); +cancel_work_sync (&sa_entry->modify_work.work);mlx5e_accel_ipsec_fs_del_rule (priv, &sa_entry->xfrm->attrs, &sa_entry->ipsec_rule);mlx5_accel_esp_free_hw_context (sa_entry->xfrm->mdev, sa_entry->hw_context);
@@ -392,7 +403,6 @@ int mlx5e_ipsec_init(struct mlx5e_priv *priv)hash_init (ipsec->sadb_rx);spin_lock_init (&ipsec->sadb_rx_lock); ipsec->mdev = priv->mdev; - ipsec->en_priv = priv; ipsec->wq =alloc_ordered_workqueue ("mlx5e_ipsec: %s" ,0 , priv->netdev->name);if (!ipsec->wq) {
@@ -424,7 +434,6 @@ void mlx5e_ipsec_cleanup(struct mlx5e_priv *priv)mlx5e_accel_ipsec_fs_cleanup (ipsec);destroy_workqueue (ipsec->wq); -kfree (ipsec); priv->ipsec =NULL ; }
@@ -444,47 +453,19 @@ static bool mlx5e_ipsec_offload_ok(struct sk_buff *skb, struct xfrm_state *x)return true; } -struct mlx5e_ipsec_modify_state_work { -struct work_struct work; -struct mlx5_accel_esp_xfrm_attrs attrs; -struct mlx5e_ipsec_sa_entry *sa_entry; -}; - -static void _update_xfrm_state (struct work_struct *work) -{ -int ret; -struct mlx5e_ipsec_modify_state_work *modify_work = - container_of(work, struct mlx5e_ipsec_modify_state_work, work); -struct mlx5e_ipsec_sa_entry *sa_entry = modify_work->sa_entry; - - ret =mlx5_accel_esp_modify_xfrm (sa_entry->xfrm, - &modify_work->attrs); -if (ret) -netdev_warn (sa_entry->ipsec->en_priv->netdev, -"Not an IPSec offload device\n" ); - -kfree (modify_work); -} -static void mlx5e_xfrm_advance_esn_state (struct xfrm_state *x) {struct mlx5e_ipsec_sa_entry *sa_entry =to_ipsec_sa_entry (x); -struct mlx5e_ipsec_modify_state_work *modify_work; +struct mlx5e_ipsec_modify_state_work *modify_work = + &sa_entry->modify_work;bool need_update; need_update =mlx5e_ipsec_update_esn_state (sa_entry);if (!need_update)return ; - modify_work =kzalloc (sizeof (*modify_work),GFP_ATOMIC ); -if (!modify_work) -return ; -mlx5e_ipsec_build_accel_xfrm_attrs (sa_entry, &modify_work->attrs); - modify_work->sa_entry = sa_entry; - -INIT_WORK (&modify_work->work, _update_xfrm_state); -WARN_ON (!queue_work (sa_entry->ipsec->wq, &modify_work->work)); +queue_work (sa_entry->ipsec->wq, &modify_work->work); }static const struct xfrmdev_ops mlx5e_ipsec_xfrmdev_ops = {
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h +6 -1
@@ -62,7 +62,6 @@ struct mlx5e_ipsec_tx;struct mlx5e_ipsec {struct mlx5_core_dev *mdev; -struct mlx5e_priv *en_priv; DECLARE_HASHTABLE(sadb_rx, MLX5E_IPSEC_SADB_RX_BITS);spinlock_t sadb_rx_lock;/* Protects sadb_rx */ struct mlx5e_ipsec_sw_stats sw_stats;
@@ -82,6 +81,11 @@ struct mlx5e_ipsec_rule {struct mlx5_modify_hdr *set_modify_hdr; }; +struct mlx5e_ipsec_modify_state_work { +struct work_struct work; +struct mlx5_accel_esp_xfrm_attrs attrs; +}; +struct mlx5e_ipsec_sa_entry {struct hlist_node hlist;/* Item in SADB_RX hashtable */ struct mlx5e_ipsec_esn_state esn_state;
@@ -94,6 +98,7 @@ struct mlx5e_ipsec_sa_entry { struct xfrm_offload *xo);u32 ipsec_obj_id;struct mlx5e_ipsec_rule ipsec_rule; +struct mlx5e_ipsec_modify_state_work modify_work; };int mlx5e_ipsec_init (struct mlx5e_priv *priv);
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c +6 -33
@@ -18,7 +18,6 @@ struct mlx5_ipsec_sa_ctx {struct mlx5_ipsec_esp_xfrm {/* reference counter of SA ctx */ struct mlx5_ipsec_sa_ctx *sa_ctx; -struct mutex lock;/* protects mlx5_ipsec_esp_xfrm */ struct mlx5_accel_esp_xfrm accel_xfrm; };
@@ -117,7 +116,6 @@ mlx5_ipsec_offload_esp_create_xfrm(struct mlx5_core_dev *mdev,if (!mxfrm)return ERR_PTR (-ENOMEM ); -mutex_init (&mxfrm->lock);memcpy (&mxfrm->accel_xfrm.attrs, attrs,sizeof (mxfrm->accel_xfrm.attrs));
@@ -129,8 +127,6 @@ static void mlx5_ipsec_offload_esp_destroy_xfrm(struct mlx5_accel_esp_xfrm *xfrm struct mlx5_ipsec_esp_xfrm *mxfrm = container_of(xfrm, struct mlx5_ipsec_esp_xfrm, accel_xfrm); -/* assuming no sa_ctx are connected to this xfrm_ctx */ -WARN_ON (mxfrm->sa_ctx);kfree (mxfrm); }
@@ -232,7 +228,6 @@ static void *mlx5_ipsec_offload_create_sa_ctx(struct mlx5_core_dev *mdev, sa_ctx->dev = mdev; mxfrm = container_of(accel_xfrm, struct mlx5_ipsec_esp_xfrm, accel_xfrm); -mutex_lock (&mxfrm->lock); sa_ctx->mxfrm = mxfrm;/* key */
@@ -258,14 +253,12 @@ static void *mlx5_ipsec_offload_create_sa_ctx(struct mlx5_core_dev *mdev, *hw_handle = sa_ctx->ipsec_obj_id; mxfrm->sa_ctx = sa_ctx; -mutex_unlock (&mxfrm->lock);return sa_ctx; err_enc_key:mlx5_destroy_encryption_key (mdev, sa_ctx->enc_key_id); err_sa_ctx: -mutex_unlock (&mxfrm->lock);kfree (sa_ctx);return ERR_PTR (err); }
@@ -273,14 +266,10 @@ static void *mlx5_ipsec_offload_create_sa_ctx(struct mlx5_core_dev *mdev,static void mlx5_ipsec_offload_delete_sa_ctx (void *context) {struct mlx5_ipsec_sa_ctx *sa_ctx = (struct mlx5_ipsec_sa_ctx *)context; -struct mlx5_ipsec_esp_xfrm *mxfrm = sa_ctx->mxfrm; -mutex_lock (&mxfrm->lock);mlx5_destroy_ipsec_obj (sa_ctx->dev, sa_ctx->ipsec_obj_id);mlx5_destroy_encryption_key (sa_ctx->dev, sa_ctx->enc_key_id);kfree (sa_ctx); - mxfrm->sa_ctx =NULL ; -mutex_unlock (&mxfrm->lock); } static int mlx5_modify_ipsec_obj(struct mlx5_core_dev *mdev,
@@ -331,29 +320,17 @@ static int mlx5_modify_ipsec_obj(struct mlx5_core_dev *mdev,return mlx5_cmd_exec (mdev, in,sizeof (in), out,sizeof (out)); } -static int mlx5_ipsec_offload_esp_modify_xfrm (struct mlx5_accel_esp_xfrm *xfrm, -const struct mlx5_accel_esp_xfrm_attrs *attrs) +static void mlx5_ipsec_offload_esp_modify_xfrm ( +struct mlx5_accel_esp_xfrm *xfrm, +const struct mlx5_accel_esp_xfrm_attrs *attrs) {struct mlx5_ipsec_obj_attrs ipsec_attrs = {};struct mlx5_core_dev *mdev = xfrm->mdev;struct mlx5_ipsec_esp_xfrm *mxfrm; -int err =0 ; -if (!memcmp (&xfrm->attrs, attrs,sizeof (xfrm->attrs))) -return 0 ; - -if (mlx5_ipsec_offload_esp_validate_xfrm_attrs (mdev, attrs)) -return -EOPNOTSUPP ; - mxfrm = container_of(xfrm, struct mlx5_ipsec_esp_xfrm, accel_xfrm); -mutex_lock (&mxfrm->lock); - -if (!mxfrm->sa_ctx) -/* Not bound xfrm, change only sw attrs */ - goto change_sw_xfrm_attrs; -/* need to add find and replace in ipsec_rhash_sa the sa_ctx */ /* modify device with new hw_sa */ ipsec_attrs.accel_flags = attrs->flags;
@@ -362,12 +339,8 @@ static int mlx5_ipsec_offload_esp_modify_xfrm(struct mlx5_accel_esp_xfrm *xfrm, &ipsec_attrs, mxfrm->sa_ctx->ipsec_obj_id); -change_sw_xfrm_attrs:if (!err)memcpy (&xfrm->attrs, attrs,sizeof (xfrm->attrs)); - -mutex_unlock (&mxfrm->lock); -return err; } void *mlx5_accel_esp_create_hw_context(struct mlx5_core_dev *mdev,
@@ -413,8 +386,8 @@ void mlx5_accel_esp_destroy_xfrm(struct mlx5_accel_esp_xfrm *xfrm)mlx5_ipsec_offload_esp_destroy_xfrm (xfrm); } -int mlx5_accel_esp_modify_xfrm (struct mlx5_accel_esp_xfrm *xfrm, -const struct mlx5_accel_esp_xfrm_attrs *attrs) +void mlx5_accel_esp_modify_xfrm (struct mlx5_accel_esp_xfrm *xfrm, +const struct mlx5_accel_esp_xfrm_attrs *attrs) { -return mlx5_ipsec_offload_esp_modify_xfrm (xfrm, attrs); +mlx5_ipsec_offload_esp_modify_xfrm (xfrm, attrs); }
include/linux/mlx5/accel.h +6 -4
@@ -127,8 +127,8 @@ struct mlx5_accel_esp_xfrm * mlx5_accel_esp_create_xfrm(struct mlx5_core_dev *mdev, const struct mlx5_accel_esp_xfrm_attrs *attrs);void mlx5_accel_esp_destroy_xfrm (struct mlx5_accel_esp_xfrm *xfrm); -int mlx5_accel_esp_modify_xfrm (struct mlx5_accel_esp_xfrm *xfrm, -const struct mlx5_accel_esp_xfrm_attrs *attrs); +void mlx5_accel_esp_modify_xfrm (struct mlx5_accel_esp_xfrm *xfrm, +const struct mlx5_accel_esp_xfrm_attrs *attrs);#else
@@ -145,9 +145,11 @@ mlx5_accel_esp_create_xfrm(struct mlx5_core_dev *mdev, }static inline void mlx5_accel_esp_destroy_xfrm (struct mlx5_accel_esp_xfrm *xfrm) {} -static inline int +static inline void mlx5_accel_esp_modify_xfrm (struct mlx5_accel_esp_xfrm *xfrm, -const struct mlx5_accel_esp_xfrm_attrs *attrs) {return -EOPNOTSUPP ; } +const struct mlx5_accel_esp_xfrm_attrs *attrs) +{ +}#endif /* CONFIG_MLX5_EN_IPSEC */ #endif /* __MLX5_ACCEL_H__ */
Signature · 1 line
2.35.1
PATCH 6/17 net/mlx5: Remove useless validity check
In reply to Leon RomanovskyFrom: Leon Romanovsky <[email protected]>
All callers build xfrm attributes with help of mlx5e_ipsec_build_accel_xfrm_attrs() function that ensure validity of attributes. There is no need to recheck them again.
--- .../mlx5/core/en_accel/ipsec_offload.c | 44 ------------------- include/linux/mlx5/accel.h | 10 ----- 2 files changed, 54 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c +0 -44
@@ -62,55 +62,11 @@ u32 mlx5_ipsec_device_caps(struct mlx5_core_dev *mdev) }EXPORT_SYMBOL_GPL (mlx5_ipsec_device_caps); -static int -mlx5_ipsec_offload_esp_validate_xfrm_attrs (struct mlx5_core_dev *mdev, -const struct mlx5_accel_esp_xfrm_attrs *attrs) -{ -if (attrs->replay_type !=MLX5_ACCEL_ESP_REPLAY_NONE ) { -mlx5_core_err (mdev,"Cannot offload xfrm states with anti replay (replay_type = %d)\n" , - attrs->replay_type); -return -EOPNOTSUPP ; - } - -if (attrs->keymat_type !=MLX5_ACCEL_ESP_KEYMAT_AES_GCM ) { -mlx5_core_err (mdev,"Only aes gcm keymat is supported (keymat_type = %d)\n" , - attrs->keymat_type); -return -EOPNOTSUPP ; - } - -if (attrs->keymat.aes_gcm.iv_algo != -MLX5_ACCEL_ESP_AES_GCM_IV_ALGO_SEQ ) { -mlx5_core_err (mdev,"Only iv sequence algo is supported (iv_algo = %d)\n" , - attrs->keymat.aes_gcm.iv_algo); -return -EOPNOTSUPP ; - } - -if (attrs->keymat.aes_gcm.key_len !=128 && - attrs->keymat.aes_gcm.key_len !=256 ) { -mlx5_core_err (mdev,"Cannot offload xfrm states with key length other than 128/256 bit (key length = %d)\n" , - attrs->keymat.aes_gcm.key_len); -return -EOPNOTSUPP ; - } - -if ((attrs->flags &MLX5_ACCEL_ESP_FLAGS_ESN_TRIGGERED ) && - !MLX5_CAP_IPSEC (mdev, ipsec_esn)) { -mlx5_core_err (mdev,"Cannot offload xfrm states with ESN triggered\n" ); -return -EOPNOTSUPP ; - } - -return 0 ; -} -static struct mlx5_accel_esp_xfrm *mlx5_ipsec_offload_esp_create_xfrm (struct mlx5_core_dev *mdev,const struct mlx5_accel_esp_xfrm_attrs *attrs) {struct mlx5_ipsec_esp_xfrm *mxfrm; -int err =0 ; - - err =mlx5_ipsec_offload_esp_validate_xfrm_attrs (mdev, attrs); -if (err) -return ERR_PTR (err); mxfrm =kzalloc (sizeof (*mxfrm),GFP_KERNEL );if (!mxfrm)
include/linux/mlx5/accel.h +0 -10
@@ -36,10 +36,6 @@#include <linux/mlx5/driver.h> -enum mlx5_accel_esp_aes_gcm_keymat_iv_algo { -MLX5_ACCEL_ESP_AES_GCM_IV_ALGO_SEQ , -}; -enum mlx5_accel_esp_flags {MLX5_ACCEL_ESP_FLAGS_TUNNEL =0 ,/* Default */ MLX5_ACCEL_ESP_FLAGS_TRANSPORT =1UL <<0 ,
@@ -57,14 +53,9 @@ enum mlx5_accel_esp_keymats { MLX5_ACCEL_ESP_KEYMAT_AES_GCM, }; -enum mlx5_accel_esp_replay { -MLX5_ACCEL_ESP_REPLAY_NONE , -MLX5_ACCEL_ESP_REPLAY_BMP , -};struct aes_gcm_keymat {u64 seq_iv; -enum mlx5_accel_esp_aes_gcm_keymat_iv_algo iv_algo;u32 salt;u32 icv_len;
@@ -81,7 +72,6 @@ struct mlx5_accel_esp_xfrm_attrs {u32 tfc_pad;u32 flags;u32 sa_handle; -enum mlx5_accel_esp_replay replay_type; union { struct { u32 size;
Signature · 1 line
2.35.1
PATCH 7/17 net/mlx5: Merge various control path IPsec headers into one file
In reply to Leon RomanovskyFrom: Leon Romanovsky <[email protected]>
The mlx5 IPsec code has logical separation between code that operates with XFRM objects (ipsec.c), HW objects (ipsec_offload.c), flow steering logic (ipsec_fs.c) and data path (ipsec_rxtx.c).
Such separation makes sense for C-files, but isn't needed at all for H-files as they are included in batch anyway.
--- .../ethernet/mellanox/mlx5/core/en/params.c | 2 +- .../mellanox/mlx5/core/en_accel/ipsec.c | 5 +- .../mellanox/mlx5/core/en_accel/ipsec.h | 101 +++++++++++- .../mellanox/mlx5/core/en_accel/ipsec_fs.c | 17 +- .../mlx5/core/en_accel/ipsec_offload.c | 3 +- .../mlx5/core/en_accel/ipsec_offload.h | 14 -- .../mellanox/mlx5/core/en_accel/ipsec_rxtx.c | 5 +- .../mellanox/mlx5/core/en_accel/ipsec_stats.c | 4 +- .../net/ethernet/mellanox/mlx5/core/en_main.c | 1 - .../net/ethernet/mellanox/mlx5/core/en_rx.c | 2 +- .../net/ethernet/mellanox/mlx5/core/main.c | 2 +- include/linux/mlx5/accel.h | 145 ------------------ 12 files changed, 117 insertions(+), 184 deletions(-) delete mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.h delete mode 100644 include/linux/mlx5/accel.h
drivers/net/ethernet/mellanox/mlx5/core/en/params.c +1 -1
@@ -5,7 +5,7 @@#include "en/txrx.h" #include "en/port.h" #include "en_accel/en_accel.h" -#include "en_accel/ipsec_offload.h" +#include "en_accel/ipsec.h" static bool mlx5e_rx_is_xdp (struct mlx5e_params *params,struct mlx5e_xsk_param *xsk)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +2 -3
@@ -37,9 +37,8 @@#include <linux/netdevice.h> #include "en.h" -#include "en_accel/ipsec.h" -#include "en_accel/ipsec_rxtx.h" -#include "en_accel/ipsec_fs.h" +#include "ipsec.h" +#include "ipsec_rxtx.h" static struct mlx5e_ipsec_sa_entry *to_ipsec_sa_entry (struct xfrm_state *x) {
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h +99 -2
122 lines not on this page · raw message
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +9 -8
@@ -2,8 +2,9 @@/* Copyright (c) 2020, Mellanox Technologies inc. All rights reserved. */ #include <linux/netdevice.h> -#include "ipsec_offload.h" -#include "ipsec_fs.h" +#include "en.h" +#include "en/fs.h" +#include "ipsec.h" #include "fs_core.h" #define NUM_IPSEC_FTE BIT(15)
@@ -565,7 +566,7 @@ static int tx_add_rule(struct mlx5e_priv *priv,if (IS_ERR (rule)) { err =PTR_ERR (rule);netdev_err (priv->netdev,"fail to add ipsec rule attrs->action=0x%x, err=%d\n" , - attrs->action, err); + attrs->action, err); goto out; }
@@ -579,8 +580,8 @@ static int tx_add_rule(struct mlx5e_priv *priv, }static void rx_del_rule (struct mlx5e_priv *priv, -struct mlx5_accel_esp_xfrm_attrs *attrs, -struct mlx5e_ipsec_rule *ipsec_rule) +struct mlx5_accel_esp_xfrm_attrs *attrs, +struct mlx5e_ipsec_rule *ipsec_rule) {mlx5_del_flow_rules (ipsec_rule->rule); ipsec_rule->rule =NULL ;
@@ -592,7 +593,7 @@ static void rx_del_rule(struct mlx5e_priv *priv, }static void tx_del_rule (struct mlx5e_priv *priv, -struct mlx5e_ipsec_rule *ipsec_rule) +struct mlx5e_ipsec_rule *ipsec_rule) {mlx5_del_flow_rules (ipsec_rule->rule); ipsec_rule->rule =NULL ;
@@ -612,8 +613,8 @@ int mlx5e_accel_ipsec_fs_add_rule(struct mlx5e_priv *priv, }void mlx5e_accel_ipsec_fs_del_rule (struct mlx5e_priv *priv, -struct mlx5_accel_esp_xfrm_attrs *attrs, -struct mlx5e_ipsec_rule *ipsec_rule) +struct mlx5_accel_esp_xfrm_attrs *attrs, +struct mlx5e_ipsec_rule *ipsec_rule) {if (attrs->action ==MLX5_ACCEL_ESP_ACTION_DECRYPT )rx_del_rule (priv, attrs, ipsec_rule);
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c +1 -2
@@ -2,9 +2,8 @@/* Copyright (c) 2017, Mellanox Technologies inc. All rights reserved. */ #include "mlx5_core.h" -#include "ipsec_offload.h" +#include "ipsec.h" #include "lib/mlx5.h" -#include "en_accel/ipsec_fs.h" struct mlx5_ipsec_sa_ctx {struct rhash_head hash;
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.h deleted +0 -14
@@ -1,14 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB */ -/* Copyright (c) 2020, Mellanox Technologies inc. All rights reserved. */ - -#ifndef __MLX5_IPSEC_OFFLOAD_H__ -#define __MLX5_IPSEC_OFFLOAD_H__ - -#include <linux/mlx5/driver.h> -#include <linux/mlx5/accel.h> - -void *mlx5_accel_esp_create_hw_context (struct mlx5_core_dev *mdev, -struct mlx5_accel_esp_xfrm *xfrm, -u32 *sa_handle); -void mlx5_accel_esp_free_hw_context (struct mlx5_core_dev *mdev,void *context); -#endif /* __MLX5_IPSEC_OFFLOAD_H__ */
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c +2 -3
@@ -34,9 +34,8 @@#include <crypto/aead.h> #include <net/xfrm.h> #include <net/esp.h> -#include "ipsec_offload.h" -#include "en_accel/ipsec_rxtx.h" -#include "en_accel/ipsec.h" +#include "ipsec.h" +#include "ipsec_rxtx.h" #include "en.h" enum {
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_stats.c +1 -3
@@ -35,9 +35,7 @@#include <net/sock.h> #include "en.h" -#include "ipsec_offload.h" -#include "fpga/sdk.h" -#include "en_accel/ipsec.h" +#include "ipsec.h" static const struct counter_desc mlx5e_ipsec_sw_stats_desc[] = { { MLX5E_DECLARE_STAT(struct mlx5e_ipsec_sw_stats, ipsec_rx_drop_sp_alloc) },
drivers/net/ethernet/mellanox/mlx5/core/en_main.c +0 -1
@@ -48,7 +48,6 @@#include "en_accel/ipsec.h" #include "en_accel/en_accel.h" #include "en_accel/ktls.h" -#include "en_accel/ipsec_offload.h" #include "lib/vxlan.h" #include "lib/clock.h" #include "en/port.h"
drivers/net/ethernet/mellanox/mlx5/core/en_rx.c +1 -1
@@ -48,7 +48,7 @@#include "en_rep.h" #include "en/rep/tc.h" #include "ipoib/ipoib.h" -#include "en_accel/ipsec_offload.h" +#include "en_accel/ipsec.h" #include "en_accel/ipsec_rxtx.h" #include "en_accel/ktls_txrx.h" #include "en/xdp.h"
drivers/net/ethernet/mellanox/mlx5/core/main.c +1 -1
@@ -62,7 +62,7 @@#include "lib/mlx5.h" #include "lib/tout.h" #include "fpga/core.h" -#include "en_accel/ipsec_offload.h" +#include "en_accel/ipsec.h" #include "lib/clock.h" #include "lib/vxlan.h" #include "lib/geneve.h"
include/linux/mlx5/accel.h deleted +0 -145
145 lines not on this page · raw message
Signature · 1 line
2.35.1
PATCH 8/17 net/mlx5: Remove indirections from esp functions
In reply to Leon RomanovskyFrom: Leon Romanovsky <[email protected]>
This change cleanups the mlx5 esp interface.
--- .../mlx5/core/en_accel/ipsec_offload.c | 47 +++++-------------- 1 file changed, 12 insertions(+), 35 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c +12 -35
@@ -61,9 +61,9 @@ u32 mlx5_ipsec_device_caps(struct mlx5_core_dev *mdev) }EXPORT_SYMBOL_GPL (mlx5_ipsec_device_caps); -static struct mlx5_accel_esp_xfrm * -mlx5_ipsec_offload_esp_create_xfrm (struct mlx5_core_dev *mdev, -const struct mlx5_accel_esp_xfrm_attrs *attrs) +struct mlx5_accel_esp_xfrm * +mlx5_accel_esp_create_xfrm (struct mlx5_core_dev *mdev, +const struct mlx5_accel_esp_xfrm_attrs *attrs) {struct mlx5_ipsec_esp_xfrm *mxfrm;
@@ -74,10 +74,11 @@ mlx5_ipsec_offload_esp_create_xfrm(struct mlx5_core_dev *mdev,memcpy (&mxfrm->accel_xfrm.attrs, attrs,sizeof (mxfrm->accel_xfrm.attrs)); + mxfrm->accel_xfrm.mdev = mdev;return &mxfrm->accel_xfrm; } -static void mlx5_ipsec_offload_esp_destroy_xfrm (struct mlx5_accel_esp_xfrm *xfrm) +void mlx5_accel_esp_destroy_xfrm (struct mlx5_accel_esp_xfrm *xfrm) { struct mlx5_ipsec_esp_xfrm *mxfrm = container_of(xfrm, struct mlx5_ipsec_esp_xfrm, accel_xfrm);
@@ -275,14 +276,13 @@ static int mlx5_modify_ipsec_obj(struct mlx5_core_dev *mdev,return mlx5_cmd_exec (mdev, in,sizeof (in), out,sizeof (out)); } -static void mlx5_ipsec_offload_esp_modify_xfrm ( -struct mlx5_accel_esp_xfrm *xfrm, -const struct mlx5_accel_esp_xfrm_attrs *attrs) +void mlx5_accel_esp_modify_xfrm (struct mlx5_accel_esp_xfrm *xfrm, +const struct mlx5_accel_esp_xfrm_attrs *attrs) {struct mlx5_ipsec_obj_attrs ipsec_attrs = {};struct mlx5_core_dev *mdev = xfrm->mdev;struct mlx5_ipsec_esp_xfrm *mxfrm; -int err =0 ; +int err; mxfrm = container_of(xfrm, struct mlx5_ipsec_esp_xfrm, accel_xfrm);
@@ -294,8 +294,10 @@ static void mlx5_ipsec_offload_esp_modify_xfrm( &ipsec_attrs, mxfrm->sa_ctx->ipsec_obj_id); -if (!err) -memcpy (&xfrm->attrs, attrs,sizeof (xfrm->attrs)); +if (err) +return ; + +memcpy (&xfrm->attrs, attrs,sizeof (xfrm->attrs)); } void *mlx5_accel_esp_create_hw_context(struct mlx5_core_dev *mdev,
@@ -321,28 +323,3 @@ void mlx5_accel_esp_free_hw_context(struct mlx5_core_dev *mdev, void *context) {mlx5_ipsec_offload_delete_sa_ctx (context); } - -struct mlx5_accel_esp_xfrm * -mlx5_accel_esp_create_xfrm (struct mlx5_core_dev *mdev, -const struct mlx5_accel_esp_xfrm_attrs *attrs) -{ -struct mlx5_accel_esp_xfrm *xfrm; - - xfrm =mlx5_ipsec_offload_esp_create_xfrm (mdev, attrs); -if (IS_ERR (xfrm)) -return xfrm; - - xfrm->mdev = mdev; -return xfrm; -} - -void mlx5_accel_esp_destroy_xfrm (struct mlx5_accel_esp_xfrm *xfrm) -{ -mlx5_ipsec_offload_esp_destroy_xfrm (xfrm); -} - -void mlx5_accel_esp_modify_xfrm (struct mlx5_accel_esp_xfrm *xfrm, -const struct mlx5_accel_esp_xfrm_attrs *attrs) -{ -mlx5_ipsec_offload_esp_modify_xfrm (xfrm, attrs); -}
Signature · 1 line
2.35.1
PATCH 9/17 net/mlx5: Simplify HW context interfaces by using SA entry
In reply to Leon RomanovskyFrom: Leon Romanovsky <[email protected]>
SA context logic used multiple structures to store same data over and over. By simplifying the SA context interfaces, we can remove extra structs.
--- .../mellanox/mlx5/core/en_accel/ipsec.c | 50 ++--- .../mellanox/mlx5/core/en_accel/ipsec.h | 27 ++- .../mlx5/core/en_accel/ipsec_offload.c | 182 ++++-------------- 3 files changed, 62 insertions(+), 197 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +14 -36
@@ -63,9 +63,9 @@ struct xfrm_state *mlx5e_ipsec_sadb_rx_lookup(struct mlx5e_ipsec *ipsec,return ret; } -static int mlx5e_ipsec_sadb_rx_add (struct mlx5e_ipsec_sa_entry *sa_entry, -unsigned int handle) +static int mlx5e_ipsec_sadb_rx_add (struct mlx5e_ipsec_sa_entry *sa_entry) { +unsigned int handle = sa_entry->ipsec_obj_id;struct mlx5e_ipsec *ipsec = sa_entry->ipsec;struct mlx5e_ipsec_sa_entry *_sa_entry;unsigned long flags;
@@ -277,16 +277,14 @@ static void _update_xfrm_state(struct work_struct *work)struct mlx5e_ipsec_sa_entry *sa_entry =container_of ( modify_work, struct mlx5e_ipsec_sa_entry, modify_work); -mlx5_accel_esp_modify_xfrm (sa_entry->xfrm, &modify_work->attrs); +mlx5_accel_esp_modify_xfrm (sa_entry, &modify_work->attrs); }static int mlx5e_xfrm_add_state (struct xfrm_state *x) {struct mlx5e_ipsec_sa_entry *sa_entry =NULL ;struct net_device *netdev = x->xso.real_dev; -struct mlx5_accel_esp_xfrm_attrs attrs;struct mlx5e_priv *priv; -unsigned int sa_handle;int err; priv =netdev_priv (netdev);
@@ -309,33 +307,20 @@ static int mlx5e_xfrm_add_state(struct xfrm_state *x)/* check esn */ mlx5e_ipsec_update_esn_state (sa_entry); -/* create xfrm */ -mlx5e_ipsec_build_accel_xfrm_attrs (sa_entry, &attrs); - sa_entry->xfrm =mlx5_accel_esp_create_xfrm (priv->mdev, &attrs); -if (IS_ERR (sa_entry->xfrm)) { - err =PTR_ERR (sa_entry->xfrm); - goto err_sa_entry; - } - +mlx5e_ipsec_build_accel_xfrm_attrs (sa_entry, &sa_entry->attrs);/* create hw context */ - sa_entry->hw_context = -mlx5_accel_esp_create_hw_context (priv->mdev, - sa_entry->xfrm, - &sa_handle); -if (IS_ERR (sa_entry->hw_context)) { - err =PTR_ERR (sa_entry->hw_context); + err =mlx5_ipsec_create_sa_ctx (sa_entry); +if (err) goto err_xfrm; - } - sa_entry->ipsec_obj_id = sa_handle; - err =mlx5e_accel_ipsec_fs_add_rule (priv, &sa_entry->xfrm->attrs, + err =mlx5e_accel_ipsec_fs_add_rule (priv, &sa_entry->attrs, sa_entry->ipsec_obj_id, &sa_entry->ipsec_rule);if (err) goto err_hw_ctx;if (x->xso.flags &XFRM_OFFLOAD_INBOUND ) { - err =mlx5e_ipsec_sadb_rx_add (sa_entry, sa_handle); + err =mlx5e_ipsec_sadb_rx_add (sa_entry);if (err) goto err_add_rule; }else {
@@ -348,15 +333,12 @@ static int mlx5e_xfrm_add_state(struct xfrm_state *x) goto out; err_add_rule: -mlx5e_accel_ipsec_fs_del_rule (priv, &sa_entry->xfrm->attrs, +mlx5e_accel_ipsec_fs_del_rule (priv, &sa_entry->attrs, &sa_entry->ipsec_rule); err_hw_ctx: -mlx5_accel_esp_free_hw_context (priv->mdev, sa_entry->hw_context); +mlx5_ipsec_free_sa_ctx (sa_entry); err_xfrm: -mlx5_accel_esp_destroy_xfrm (sa_entry->xfrm); -err_sa_entry:kfree (sa_entry); - out:return err; }
@@ -374,14 +356,10 @@ static void mlx5e_xfrm_free_state(struct xfrm_state *x)struct mlx5e_ipsec_sa_entry *sa_entry =to_ipsec_sa_entry (x);struct mlx5e_priv *priv =netdev_priv (x->xso.dev); -if (sa_entry->hw_context) { -cancel_work_sync (&sa_entry->modify_work.work); -mlx5e_accel_ipsec_fs_del_rule (priv, &sa_entry->xfrm->attrs, - &sa_entry->ipsec_rule); -mlx5_accel_esp_free_hw_context (sa_entry->xfrm->mdev, sa_entry->hw_context); -mlx5_accel_esp_destroy_xfrm (sa_entry->xfrm); - } - +cancel_work_sync (&sa_entry->modify_work.work); +mlx5e_accel_ipsec_fs_del_rule (priv, &sa_entry->attrs, + &sa_entry->ipsec_rule); +mlx5_ipsec_free_sa_ctx (sa_entry);kfree (sa_entry); }
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h +11 -16
@@ -102,11 +102,6 @@ struct mlx5_accel_esp_xfrm_attrs {u8 is_ipv6; }; -struct mlx5_accel_esp_xfrm { -struct mlx5_core_dev *mdev; -struct mlx5_accel_esp_xfrm_attrs attrs; -}; -enum mlx5_accel_ipsec_cap {MLX5_ACCEL_IPSEC_CAP_DEVICE =1 <<0 ,MLX5_ACCEL_IPSEC_CAP_ESP =1 <<1 ,
@@ -162,11 +157,11 @@ struct mlx5e_ipsec_sa_entry {unsigned int handle;/* Handle in SADB_RX */ struct xfrm_state *x;struct mlx5e_ipsec *ipsec; -struct mlx5_accel_esp_xfrm *xfrm; -void *hw_context; +struct mlx5_accel_esp_xfrm_attrs attrs;void (*set_iv_op)(struct sk_buff *skb,struct xfrm_state *x,struct xfrm_offload *xo);u32 ipsec_obj_id; +u32 enc_key_id;struct mlx5e_ipsec_rule ipsec_rule;struct mlx5e_ipsec_modify_state_work modify_work; };
@@ -188,19 +183,19 @@ void mlx5e_accel_ipsec_fs_del_rule(struct mlx5e_priv *priv,struct mlx5_accel_esp_xfrm_attrs *attrs, struct mlx5e_ipsec_rule *ipsec_rule); -void *mlx5_accel_esp_create_hw_context (struct mlx5_core_dev *mdev, -struct mlx5_accel_esp_xfrm *xfrm, -u32 *sa_handle); -void mlx5_accel_esp_free_hw_context (struct mlx5_core_dev *mdev,void *context); +int mlx5_ipsec_create_sa_ctx (struct mlx5e_ipsec_sa_entry *sa_entry); +void mlx5_ipsec_free_sa_ctx (struct mlx5e_ipsec_sa_entry *sa_entry);u32 mlx5_ipsec_device_caps (struct mlx5_core_dev *mdev); -struct mlx5_accel_esp_xfrm * -mlx5_accel_esp_create_xfrm (struct mlx5_core_dev *mdev, -const struct mlx5_accel_esp_xfrm_attrs *attrs); -void mlx5_accel_esp_destroy_xfrm (struct mlx5_accel_esp_xfrm *xfrm); -void mlx5_accel_esp_modify_xfrm (struct mlx5_accel_esp_xfrm *xfrm, +void mlx5_accel_esp_modify_xfrm (struct mlx5e_ipsec_sa_entry *sa_entry,const struct mlx5_accel_esp_xfrm_attrs *attrs); + +static inline struct mlx5_core_dev * +mlx5e_ipsec_sa2dev (struct mlx5e_ipsec_sa_entry *sa_entry) +{ +return sa_entry->ipsec->mdev; +}#else static inline int mlx5e_ipsec_init (struct mlx5e_priv *priv) {
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c +37 -145
283 lines not on this page · raw message
Signature · 1 line
2.35.1
Saeed Mahameed
maintainerRe: [PATCH net-next v1 09/17] net/mlx5: Simplify HW context interfaces by using SA entry
Quoted 239 lines · Leon Romanovsky
From: Leon Romanovsky <[email protected]>
SA context logic used multiple structures to store same data over and over. By simplifying the SA context interfaces, we can remove extra structs.
Reviewed-by Raed Salem <[email protected]>Signed-off-by Leon Romanovsky <[email protected]>---
.../mellanox/mlx5/core/en_accel/ipsec.c | 50 ++--- .../mellanox/mlx5/core/en_accel/ipsec.h | 27 ++- .../mlx5/core/en_accel/ipsec_offload.c | 182 ++++-------------- 3 files changed, 62 insertions(+), 197 deletions(-)drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +8
-23@@ -63,9 +63,9 @@ struct xfrm_state *mlx5e_ipsec_sadb_rx_lookup(struct mlx5e_ipsec *ipsec,return ret; } -static int mlx5e_ipsec_sadb_rx_add (struct mlx5e_ipsec_sa_entry *sa_entry, -unsigned int handle) +static int mlx5e_ipsec_sadb_rx_add (struct mlx5e_ipsec_sa_entry *sa_entry) { +unsigned int handle = sa_entry->ipsec_obj_id;struct mlx5e_ipsec *ipsec = sa_entry->ipsec;struct mlx5e_ipsec_sa_entry *_sa_entry;unsigned long flags;@@ -277,16 +277,14 @@ static void _update_xfrm_state(struct work_struct *work)struct mlx5e_ipsec_sa_entry *sa_entry =container_of ( modify_work, struct mlx5e_ipsec_sa_entry, modify_work); -mlx5_accel_esp_modify_xfrm (sa_entry->xfrm, &modify_work->attrs); +mlx5_accel_esp_modify_xfrm (sa_entry, &modify_work->attrs); }static int mlx5e_xfrm_add_state (struct xfrm_state *x) {struct mlx5e_ipsec_sa_entry *sa_entry =NULL ;struct net_device *netdev = x->xso.real_dev; -struct mlx5_accel_esp_xfrm_attrs attrs;struct mlx5e_priv *priv; -unsigned int sa_handle;int err; priv =netdev_priv (netdev);@@ -309,33 +307,20 @@ static int mlx5e_xfrm_add_state(struct xfrm_state *x)/* check esn */ mlx5e_ipsec_update_esn_state (sa_entry); -/* create xfrm */ -mlx5e_ipsec_build_accel_xfrm_attrs (sa_entry, &attrs); - sa_entry->xfrm =mlx5_accel_esp_create_xfrm (priv->mdev, &attrs); -if (IS_ERR (sa_entry->xfrm)) { - err =PTR_ERR (sa_entry->xfrm); - goto err_sa_entry; - } - +mlx5e_ipsec_build_accel_xfrm_attrs (sa_entry, &sa_entry->attrs);/* create hw context */ - sa_entry->hw_context = -mlx5_accel_esp_create_hw_context (priv->mdev, - sa_entry->xfrm, - &sa_handle); -if (IS_ERR (sa_entry->hw_context)) { - err =PTR_ERR (sa_entry->hw_context); + err =mlx5_ipsec_create_sa_ctx (sa_entry); +if (err) goto err_xfrm; - } - sa_entry->ipsec_obj_id = sa_handle; - err =mlx5e_accel_ipsec_fs_add_rule (priv, &sa_entry->xfrm->attrs, + err =mlx5e_accel_ipsec_fs_add_rule (priv, &sa_entry->attrs, sa_entry->ipsec_obj_id, &sa_entry->ipsec_rule);if (err) goto err_hw_ctx;if (x->xso.flags &XFRM_OFFLOAD_INBOUND ) { - err =mlx5e_ipsec_sadb_rx_add (sa_entry, sa_handle); + err =mlx5e_ipsec_sadb_rx_add (sa_entry);if (err) goto err_add_rule; }else {@@ -348,15 +333,12 @@ static int mlx5e_xfrm_add_state(struct xfrm_state *x) goto out;
err_add_rule:drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +1
-1- mlx5e_accel_ipsec_fs_del_rule(priv, &sa_entry->xfrm->attrs, + mlx5e_accel_ipsec_fs_del_rule(priv, &sa_entry->attrs,&sa_entry->ipsec_rule);
err_hw_ctx:drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +1
-1-mlx5_accel_esp_free_hw_context (priv->mdev, sa_entry->hw_context); +mlx5_ipsec_free_sa_ctx (sa_entry);
err_xfrm:drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +0
-2-mlx5_accel_esp_destroy_xfrm (sa_entry->xfrm); -err_sa_entry:kfree(sa_entry); - out: return err; }drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +4
-8@@ -374,14 +356,10 @@ static void mlx5e_xfrm_free_state(struct xfrm_state *x)struct mlx5e_ipsec_sa_entry *sa_entry =to_ipsec_sa_entry (x);struct mlx5e_priv *priv =netdev_priv (x->xso.dev); -if (sa_entry->hw_context) { -cancel_work_sync (&sa_entry->modify_work.work); -mlx5e_accel_ipsec_fs_del_rule (priv, &sa_entry->xfrm->attrs, - &sa_entry->ipsec_rule); -mlx5_accel_esp_free_hw_context (sa_entry->xfrm->mdev, sa_entry->hw_context); -mlx5_accel_esp_destroy_xfrm (sa_entry->xfrm); - } - +cancel_work_sync (&sa_entry->modify_work.work); +mlx5e_accel_ipsec_fs_del_rule (priv, &sa_entry->attrs, + &sa_entry->ipsec_rule); +mlx5_ipsec_free_sa_ctx (sa_entry);kfree (sa_entry); }drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h +11
-16@@ -102,11 +102,6 @@ struct mlx5_accel_esp_xfrm_attrs {u8 is_ipv6; }; -struct mlx5_accel_esp_xfrm { -struct mlx5_core_dev *mdev; -struct mlx5_accel_esp_xfrm_attrs attrs; -}; -enum mlx5_accel_ipsec_cap {MLX5_ACCEL_IPSEC_CAP_DEVICE =1 <<0 ,MLX5_ACCEL_IPSEC_CAP_ESP =1 <<1 ,@@ -162,11 +157,11 @@ struct mlx5e_ipsec_sa_entry {unsigned int handle;/* Handle in SADB_RX */ struct xfrm_state *x;struct mlx5e_ipsec *ipsec; -struct mlx5_accel_esp_xfrm *xfrm; -void *hw_context; +struct mlx5_accel_esp_xfrm_attrs attrs;void (*set_iv_op)(struct sk_buff *skb,struct xfrm_state *x,struct xfrm_offload *xo);u32 ipsec_obj_id; +u32 enc_key_id;struct mlx5e_ipsec_rule ipsec_rule;struct mlx5e_ipsec_modify_state_work modify_work; };@@ -188,19 +183,19 @@ void mlx5e_accel_ipsec_fs_del_rule(struct mlx5e_priv *priv,struct mlx5_accel_esp_xfrm_attrs *attrs, struct mlx5e_ipsec_rule *ipsec_rule); -void *mlx5_accel_esp_create_hw_context (struct mlx5_core_dev *mdev, -struct mlx5_accel_esp_xfrm *xfrm, -u32 *sa_handle); -void mlx5_accel_esp_free_hw_context (struct mlx5_core_dev *mdev,void *context); +int mlx5_ipsec_create_sa_ctx (struct mlx5e_ipsec_sa_entry *sa_entry); +void mlx5_ipsec_free_sa_ctx (struct mlx5e_ipsec_sa_entry *sa_entry);u32 mlx5_ipsec_device_caps (struct mlx5_core_dev *mdev); -struct mlx5_accel_esp_xfrm * -mlx5_accel_esp_create_xfrm (struct mlx5_core_dev *mdev, -const struct mlx5_accel_esp_xfrm_attrs *attrs); -void mlx5_accel_esp_destroy_xfrm (struct mlx5_accel_esp_xfrm *xfrm); -void mlx5_accel_esp_modify_xfrm (struct mlx5_accel_esp_xfrm *xfrm, +void mlx5_accel_esp_modify_xfrm (struct mlx5e_ipsec_sa_entry *sa_entry,const struct mlx5_accel_esp_xfrm_attrs *attrs); + +static inline struct mlx5_core_dev * +mlx5e_ipsec_sa2dev (struct mlx5e_ipsec_sa_entry *sa_entry) +{ +return sa_entry->ipsec->mdev; +}#else static inline int mlx5e_ipsec_init (struct mlx5e_priv *priv) {drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c +1
-39@@ -5,21 +5,6 @@#include "ipsec.h" #include "lib/mlx5.h" -struct mlx5_ipsec_sa_ctx { -struct rhash_head hash; -u32 enc_key_id; -u32 ipsec_obj_id; -/* hw ctx */ -struct mlx5_core_dev *dev; -struct mlx5_ipsec_esp_xfrm *mxfrm; -}; - -struct mlx5_ipsec_esp_xfrm { -/* reference counter of SA ctx */ -struct mlx5_ipsec_sa_ctx *sa_ctx; -struct mlx5_accel_esp_xfrm accel_xfrm; -}; -u32 mlx5_ipsec_device_caps (struct mlx5_core_dev *mdev) {u32 caps;@@ -61,43 +46,11 @@ u32 mlx5_ipsec_device_caps(struct mlx5_core_dev *mdev) }EXPORT_SYMBOL_GPL (mlx5_ipsec_device_caps); -struct mlx5_accel_esp_xfrm * -mlx5_accel_esp_create_xfrm (struct mlx5_core_dev *mdev, -const struct mlx5_accel_esp_xfrm_attrs *attrs) -{ -struct mlx5_ipsec_esp_xfrm *mxfrm; - - mxfrm =kzalloc (sizeof (*mxfrm),GFP_KERNEL ); -if (!mxfrm) -return ERR_PTR (-ENOMEM ); - -memcpy (&mxfrm->accel_xfrm.attrs, attrs, -sizeof (mxfrm->accel_xfrm.attrs)); - - mxfrm->accel_xfrm.mdev = mdev; -return &mxfrm->accel_xfrm; -} - -void mlx5_accel_esp_destroy_xfrm (struct mlx5_accel_esp_xfrm *xfrm) +static int mlx5_create_ipsec_obj (struct mlx5e_ipsec_sa_entry *sa_entry) { - struct mlx5_ipsec_esp_xfrm *mxfrm = container_of(xfrm, struct mlx5_ipsec_esp_xfrm, - accel_xfrm); - -kfree (mxfrm); -} -
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c u32 mlx5_ipsec_device_caps(struct mlx5_core_dev *mdev) · line 98
Quoted 10 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c +0
-10-struct mlx5_ipsec_obj_attrs { -const struct aes_gcm_keymat *aes_gcm; -u32 accel_flags; -u32 esn_msb; -u32 enc_key_id; -}; - -static int mlx5_create_ipsec_obj (struct mlx5_core_dev *mdev, -struct mlx5_ipsec_obj_attrs *attrs, -u32 *ipsec_id)
I don't see the point of this change, the function used to receive two primitives, now it receives a god object, just to grab the two primitives, this breaks the bottom up design, and contaminates the code with the sa_entry container, that only should be visible by high-level ipsec module and the SA DB, all service and low level functions should remain as primitive and simple as possible to avoid future abuse and reduce the scope and visibility of god objects. The effect of this change is more severe in the next patch.
Even within the same file, i still recommend a monotonic bottom up design and keep the complex objects usage to as few hight level functions as possible.
Quoted 233 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c +19
-40-{ -const struct aes_gcm_keymat *aes_gcm = attrs->aes_gcm; +struct mlx5_accel_esp_xfrm_attrs *attrs = &sa_entry->attrs; +struct mlx5_core_dev *mdev =mlx5e_ipsec_sa2dev (sa_entry); +struct aes_gcm_keymat *aes_gcm = &attrs->keymat.aes_gcm;u32 out[MLX5_ST_SZ_DW (general_obj_out_cmd_hdr)];u32 in[MLX5_ST_SZ_DW (create_ipsec_obj_in)] = {};void *obj, *salt_p, *salt_iv_p;@@ -128,14 +81,14 @@ static int mlx5_create_ipsec_obj(struct mlx5_core_dev *mdev, salt_iv_p =MLX5_ADDR_OF (ipsec_obj, obj, implicit_iv);memcpy (salt_iv_p, &aes_gcm->seq_iv,sizeof (aes_gcm->seq_iv));/* esn */ -if (attrs->accel_flags &MLX5_ACCEL_ESP_FLAGS_ESN_TRIGGERED ) { +if (attrs->flags &MLX5_ACCEL_ESP_FLAGS_ESN_TRIGGERED ) {MLX5_SET (ipsec_obj, obj, esn_en,1 ); -MLX5_SET (ipsec_obj, obj, esn_msb, attrs->esn_msb); -if (attrs->accel_flags &MLX5_ACCEL_ESP_FLAGS_ESN_STATE_OVERLAP ) +MLX5_SET (ipsec_obj, obj, esn_msb, attrs->esn); +if (attrs->flags &MLX5_ACCEL_ESP_FLAGS_ESN_STATE_OVERLAP )MLX5_SET (ipsec_obj, obj, esn_overlap,1 ); } -MLX5_SET (ipsec_obj, obj, dekn, attrs->enc_key_id); +MLX5_SET (ipsec_obj, obj, dekn, sa_entry->enc_key_id);/* general object fields set */ MLX5_SET(general_obj_in_cmd_hdr, in, opcode,@@ -145,13 +98,15 @@ static int mlx5_create_ipsec_obj(struct mlx5_core_dev *mdev, err =mlx5_cmd_exec (mdev, in,sizeof (in), out,sizeof (out));if (!err) - *ipsec_id =MLX5_GET (general_obj_out_cmd_hdr, out, obj_id); + sa_entry->ipsec_obj_id = +MLX5_GET (general_obj_out_cmd_hdr, out, obj_id);return err; } -static void mlx5_destroy_ipsec_obj (struct mlx5_core_dev *mdev,u32 ipsec_id) +static void mlx5_destroy_ipsec_obj (struct mlx5e_ipsec_sa_entry *sa_entry) { +struct mlx5_core_dev *mdev =mlx5e_ipsec_sa2dev (sa_entry);u32 in[MLX5_ST_SZ_DW (general_obj_in_cmd_hdr)] = {};u32 out[MLX5_ST_SZ_DW (general_obj_out_cmd_hdr)];@@ -159,79 +114,52 @@ static void mlx5_destroy_ipsec_obj(struct mlx5_core_dev *mdev, u32 ipsec_id) MLX5_CMD_OP_DESTROY_GENERAL_OBJECT);MLX5_SET (general_obj_in_cmd_hdr, in, obj_type,MLX5_GENERAL_OBJECT_TYPES_IPSEC ); -MLX5_SET (general_obj_in_cmd_hdr, in, obj_id, ipsec_id); +MLX5_SET (general_obj_in_cmd_hdr, in, obj_id, sa_entry->ipsec_obj_id);mlx5_cmd_exec (mdev, in,sizeof (in), out,sizeof (out)); } -static void *mlx5_ipsec_offload_create_sa_ctx (struct mlx5_core_dev *mdev, -struct mlx5_accel_esp_xfrm *accel_xfrm, -const __be32 saddr[4 ],const __be32 daddr[4 ], -const __be32 spi,bool is_ipv6,u32 *hw_handle) +int mlx5_ipsec_create_sa_ctx (struct mlx5e_ipsec_sa_entry *sa_entry) { -struct mlx5_accel_esp_xfrm_attrs *xfrm_attrs = &accel_xfrm->attrs; -struct aes_gcm_keymat *aes_gcm = &xfrm_attrs->keymat.aes_gcm; -struct mlx5_ipsec_obj_attrs ipsec_attrs = {}; -struct mlx5_ipsec_esp_xfrm *mxfrm; -struct mlx5_ipsec_sa_ctx *sa_ctx; +struct aes_gcm_keymat *aes_gcm = &sa_entry->attrs.keymat.aes_gcm; +struct mlx5_core_dev *mdev =mlx5e_ipsec_sa2dev (sa_entry);int err; -/* alloc SA context */ - sa_ctx =kzalloc (sizeof (*sa_ctx),GFP_KERNEL ); -if (!sa_ctx) -return ERR_PTR (-ENOMEM ); - - sa_ctx->dev = mdev; - - mxfrm = container_of(accel_xfrm, struct mlx5_ipsec_esp_xfrm, accel_xfrm); - sa_ctx->mxfrm = mxfrm; -/* key */ err =mlx5_create_encryption_key (mdev, aes_gcm->aes_key, aes_gcm->key_len /BITS_PER_BYTE ,MLX5_ACCEL_OBJ_IPSEC_KEY , - &sa_ctx->enc_key_id); + &sa_entry->enc_key_id);if (err) {mlx5_core_dbg (mdev,"Failed to create encryption key (err = %d)\n" , err); - goto err_sa_ctx; +return err; } - ipsec_attrs.aes_gcm = aes_gcm; - ipsec_attrs.accel_flags = accel_xfrm->attrs.flags; - ipsec_attrs.esn_msb = accel_xfrm->attrs.esn; - ipsec_attrs.enc_key_id = sa_ctx->enc_key_id; - err =mlx5_create_ipsec_obj (mdev, &ipsec_attrs, - &sa_ctx->ipsec_obj_id); + err =mlx5_create_ipsec_obj (sa_entry);if (err) {mlx5_core_dbg (mdev,"Failed to create IPsec object (err = %d)\n" , err); goto err_enc_key; } - *hw_handle = sa_ctx->ipsec_obj_id; - mxfrm->sa_ctx = sa_ctx; - -return sa_ctx; +return 0 ;
err_enc_key:drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c +2
-4-mlx5_destroy_encryption_key (mdev, sa_ctx->enc_key_id); -err_sa_ctx: -kfree (sa_ctx); -return ERR_PTR (err); +mlx5_destroy_encryption_key (mdev, sa_entry->enc_key_id); +return err;}
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c +1
-1-static void mlx5_ipsec_offload_delete_sa_ctx (void *context) +void mlx5_ipsec_free_sa_ctx (struct mlx5e_ipsec_sa_entry *sa_entry){
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c +3
-4-struct mlx5_ipsec_sa_ctx *sa_ctx = (struct mlx5_ipsec_sa_ctx *)context; +struct mlx5_core_dev *mdev =mlx5e_ipsec_sa2dev (sa_entry); -mlx5_destroy_ipsec_obj (sa_ctx->dev, sa_ctx->ipsec_obj_id); -mlx5_destroy_encryption_key (sa_ctx->dev, sa_ctx->enc_key_id); -kfree (sa_ctx); +mlx5_destroy_ipsec_obj (sa_entry); +mlx5_destroy_encryption_key (mdev, sa_entry->enc_key_id);}
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c +2
-3-static int mlx5_modify_ipsec_obj (struct mlx5_core_dev *mdev, -struct mlx5_ipsec_obj_attrs *attrs, -u32 ipsec_id) +static int mlx5_modify_ipsec_obj (struct mlx5e_ipsec_sa_entry *sa_entry, +const struct mlx5_accel_esp_xfrm_attrs *attrs){
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c +1
-0+struct mlx5_core_dev *mdev =mlx5e_ipsec_sa2dev (sa_entry);u32 in[MLX5_ST_SZ_DW(modify_ipsec_obj_in)] = {}; u32 out[MLX5_ST_SZ_DW(query_ipsec_obj_out)]; u64 modify_field_select = 0;drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c +8
-44@@ -239,7 +167,7 @@ static int mlx5_modify_ipsec_obj(struct mlx5_core_dev *mdev,void *obj;int err; -if (!(attrs->accel_flags &MLX5_ACCEL_ESP_FLAGS_ESN_TRIGGERED )) +if (!(attrs->flags &MLX5_ACCEL_ESP_FLAGS_ESN_TRIGGERED ))return 0 ; general_obj_types =MLX5_CAP_GEN_64 (mdev, general_obj_types);@@ -249,11 +177,11 @@ static int mlx5_modify_ipsec_obj(struct mlx5_core_dev *mdev,/* general object fields set */ MLX5_SET (general_obj_in_cmd_hdr, in, opcode,MLX5_CMD_OP_QUERY_GENERAL_OBJECT );MLX5_SET (general_obj_in_cmd_hdr, in, obj_type,MLX5_GENERAL_OBJECT_TYPES_IPSEC ); -MLX5_SET (general_obj_in_cmd_hdr, in, obj_id, ipsec_id); +MLX5_SET (general_obj_in_cmd_hdr, in, obj_id, sa_entry->ipsec_obj_id); err =mlx5_cmd_exec (mdev, in,sizeof (in), out,sizeof (out));if (err) {mlx5_core_err (mdev,"Query IPsec object failed (Object id %d), err = %d\n" , - ipsec_id, err); + sa_entry->ipsec_obj_id, err);return err; }@@ -266,8 +194,8 @@ static int mlx5_modify_ipsec_obj(struct mlx5_core_dev *mdev,return -EOPNOTSUPP ; obj =MLX5_ADDR_OF (modify_ipsec_obj_in, in, ipsec_object); -MLX5_SET (ipsec_obj, obj, esn_msb, attrs->esn_msb); -if (attrs->accel_flags &MLX5_ACCEL_ESP_FLAGS_ESN_STATE_OVERLAP ) +MLX5_SET (ipsec_obj, obj, esn_msb, attrs->esn); +if (attrs->flags &MLX5_ACCEL_ESP_FLAGS_ESN_STATE_OVERLAP )MLX5_SET (ipsec_obj, obj, esn_overlap,1 );/* general object fields set */ @@ -276,50 +204,14 @@ static int mlx5_modify_ipsec_obj(struct mlx5_core_dev *mdev,return mlx5_cmd_exec (mdev, in,sizeof (in), out,sizeof (out)); } -void mlx5_accel_esp_modify_xfrm (struct mlx5_accel_esp_xfrm *xfrm, +void mlx5_accel_esp_modify_xfrm (struct mlx5e_ipsec_sa_entry *sa_entry,const struct mlx5_accel_esp_xfrm_attrs *attrs) { -struct mlx5_ipsec_obj_attrs ipsec_attrs = {}; -struct mlx5_core_dev *mdev = xfrm->mdev; -struct mlx5_ipsec_esp_xfrm *mxfrm;int err; - mxfrm = container_of(xfrm, struct mlx5_ipsec_esp_xfrm, accel_xfrm); - -/* need to add find and replace in ipsec_rhash_sa the sa_ctx */ -/* modify device with new hw_sa */ - ipsec_attrs.accel_flags = attrs->flags; - ipsec_attrs.esn_msb = attrs->esn; - err =mlx5_modify_ipsec_obj (mdev, - &ipsec_attrs, - mxfrm->sa_ctx->ipsec_obj_id); - + err =mlx5_modify_ipsec_obj (sa_entry, attrs);if (err)return ; -memcpy (&xfrm->attrs, attrs,sizeof (xfrm->attrs)); -} - -void *mlx5_accel_esp_create_hw_context (struct mlx5_core_dev *mdev, -struct mlx5_accel_esp_xfrm *xfrm, -u32 *sa_handle) -{ -__be32 saddr[4 ] = {}, daddr[4 ] = {}; - -if (!xfrm->attrs.is_ipv6) { - saddr[3 ] = xfrm->attrs.saddr.a4; - daddr[3 ] = xfrm->attrs.daddr.a4; - }else { -memcpy (saddr, xfrm->attrs.saddr.a6,sizeof (saddr)); -memcpy (daddr, xfrm->attrs.daddr.a6,sizeof (daddr)); - } - -return mlx5_ipsec_offload_create_sa_ctx (mdev, xfrm, saddr, daddr, - xfrm->attrs.spi, - xfrm->attrs.is_ipv6, sa_handle); -} - -void mlx5_accel_esp_free_hw_context (struct mlx5_core_dev *mdev,void *context) -{ -mlx5_ipsec_offload_delete_sa_ctx (context); +memcpy (&sa_entry->attrs, attrs,sizeof (sa_entry->attrs)); }Signature · 1 line
2.35.1
Re: [PATCH net-next v1 09/17] net/mlx5: Simplify HW context interfaces by using SA entry
Quoted 14 lines · Saeed Mahameed
Quoted 13 lines · Leon Romanovsky
From: Leon Romanovsky <[email protected]>
SA context logic used multiple structures to store same data over and over. By simplifying the SA context interfaces, we can remove extra structs.
Reviewed-by Raed Salem <[email protected]>Signed-off-by Leon Romanovsky <[email protected]>---
.../mellanox/mlx5/core/en_accel/ipsec.c | 50 ++--- .../mellanox/mlx5/core/en_accel/ipsec.h | 27 ++- .../mlx5/core/en_accel/ipsec_offload.c | 182 ++++-------------- 3 files changed, 62 insertions(+), 197 deletions(-)
<...>
Quoted 13 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c u32 mlx5_ipsec_device_caps(struct mlx5_core_dev *mdev) · line 98
Quoted 3 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c +0
-3-static int mlx5_create_ipsec_obj (struct mlx5_core_dev *mdev, -struct mlx5_ipsec_obj_attrs *attrs, -u32 *ipsec_id)I don't see the point of this change, the function used to receive two primitives, now it receives a god object, just to grab the two primitives, this breaks the bottom up design, and contaminates the code with the
sa_entrycontainer, that only should be visible by high-level ipsec module and the SA DB, all service and low level functions should remain as primitive and simple as possible to avoid future abuse and reduce the scope and visibility of god objects. The effect of this change is more severe in the next patch.
Quoted 3 lines
Even within the same file, i still recommend a monotonic bottom up design and keep the complex objects usage to as few hight level functions as possible.
Like you said: same file, same data copied in and out - it is not bottom up design for me.
Thanks
PATCH 10/17 net/mlx5: Clean IPsec FS add/delete rules
In reply to Leon RomanovskyFrom: Leon Romanovsky <[email protected]>
Reuse existing struct to pass parameters instead of open code them.
--- .../mellanox/mlx5/core/en_accel/ipsec.c | 10 +--- .../mellanox/mlx5/core/en_accel/ipsec.h | 7 +-- .../mellanox/mlx5/core/en_accel/ipsec_fs.c | 55 ++++++++++--------- 3 files changed, 34 insertions(+), 38 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +3 -7
@@ -313,9 +313,7 @@ static int mlx5e_xfrm_add_state(struct xfrm_state *x)if (err) goto err_xfrm; - err =mlx5e_accel_ipsec_fs_add_rule (priv, &sa_entry->attrs, - sa_entry->ipsec_obj_id, - &sa_entry->ipsec_rule); + err =mlx5e_accel_ipsec_fs_add_rule (priv, sa_entry);if (err) goto err_hw_ctx;
@@ -333,8 +331,7 @@ static int mlx5e_xfrm_add_state(struct xfrm_state *x) goto out; err_add_rule: -mlx5e_accel_ipsec_fs_del_rule (priv, &sa_entry->attrs, - &sa_entry->ipsec_rule); +mlx5e_accel_ipsec_fs_del_rule (priv, sa_entry); err_hw_ctx:mlx5_ipsec_free_sa_ctx (sa_entry); err_xfrm:
@@ -357,8 +354,7 @@ static void mlx5e_xfrm_free_state(struct xfrm_state *x)struct mlx5e_priv *priv =netdev_priv (x->xso.dev);cancel_work_sync (&sa_entry->modify_work.work); -mlx5e_accel_ipsec_fs_del_rule (priv, &sa_entry->attrs, - &sa_entry->ipsec_rule); +mlx5e_accel_ipsec_fs_del_rule (priv, sa_entry);mlx5_ipsec_free_sa_ctx (sa_entry);kfree (sa_entry); }
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h +2 -5
@@ -176,12 +176,9 @@ struct xfrm_state *mlx5e_ipsec_sadb_rx_lookup(struct mlx5e_ipsec *dev,void mlx5e_accel_ipsec_fs_cleanup (struct mlx5e_ipsec *ipsec);int mlx5e_accel_ipsec_fs_init (struct mlx5e_ipsec *ipsec);int mlx5e_accel_ipsec_fs_add_rule (struct mlx5e_priv *priv, -struct mlx5_accel_esp_xfrm_attrs *attrs, -u32 ipsec_obj_id, -struct mlx5e_ipsec_rule *ipsec_rule); +struct mlx5e_ipsec_sa_entry *sa_entry);void mlx5e_accel_ipsec_fs_del_rule (struct mlx5e_priv *priv, -struct mlx5_accel_esp_xfrm_attrs *attrs, -struct mlx5e_ipsec_rule *ipsec_rule); +struct mlx5e_ipsec_sa_entry *sa_entry);int mlx5_ipsec_create_sa_ctx (struct mlx5e_ipsec_sa_entry *sa_entry);void mlx5_ipsec_free_sa_ctx (struct mlx5e_ipsec_sa_entry *sa_entry);
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +29 -26
112 lines not on this page · raw message
Signature · 1 line
2.35.1
Saeed Mahameed
maintainerRe: [PATCH net-next v1 10/17] net/mlx5: Clean IPsec FS add/delete rules
Quoted 4 lines · Leon Romanovsky
From: Leon Romanovsky <[email protected]>
Reuse existing struct to pass parameters instead of open code them.
Why? what do you mean "open code them" ? they are not open coded, they are primitive for a reason ! If we go with this reasoning, then let's pass mlx5e_priv to all functions and just forget about modularity.
Quoted 13 lines
Reviewed-by Raed Salem <[email protected]>Signed-off-by Leon Romanovsky <[email protected]>---
.../mellanox/mlx5/core/en_accel/ipsec.c | 10 +--- .../mellanox/mlx5/core/en_accel/ipsec.h | 7 +-- .../mellanox/mlx5/core/en_accel/ipsec_fs.c | 55 ++++++++++--------- 3 files changed, 34 insertions(+), 38 deletions(-)drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +0
-0
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c static int mlx5e_xfrm_add_state(struct xfrm_state *x) · line 316
Quoted 7 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +1
-3@@ -313,9 +313,7 @@ static int mlx5e_xfrm_add_state(struct xfrm_state *x)if (err) goto err_xfrm; - err =mlx5e_accel_ipsec_fs_add_rule (priv, &sa_entry->attrs, - sa_entry->ipsec_obj_id, - &sa_entry->ipsec_rule); + err =mlx5e_accel_ipsec_fs_add_rule (priv, sa_entry);
To add to my comment on the previous patch, in here the issue is more severe as previously ipsec_fs.c was unaware of sa_entry object and used to deal with pure fs related objects, you are peppering the code with sa_entry for no reason, other than reducing function parameters from 4 to 2.
Quoted 167 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +0
-0if (err) goto err_hw_ctx;@@ -333,8 +331,7 @@ static int mlx5e_xfrm_add_state(struct xfrm_state *x) goto out;
err_add_rule:drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +1
-2-mlx5e_accel_ipsec_fs_del_rule (priv, &sa_entry->attrs, - &sa_entry->ipsec_rule); +mlx5e_accel_ipsec_fs_del_rule (priv, sa_entry);
err_hw_ctx:mlx5_ipsec_free_sa_ctx(sa_entry);
err_xfrm:drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +1
-2@@ -357,8 +354,7 @@ static void mlx5e_xfrm_free_state(struct xfrm_state *x)struct mlx5e_priv *priv =netdev_priv (x->xso.dev);cancel_work_sync (&sa_entry->modify_work.work); -mlx5e_accel_ipsec_fs_del_rule (priv, &sa_entry->attrs, - &sa_entry->ipsec_rule); +mlx5e_accel_ipsec_fs_del_rule (priv, sa_entry);mlx5_ipsec_free_sa_ctx (sa_entry);kfree (sa_entry); }drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h +2
-5@@ -176,12 +176,9 @@ struct xfrm_state *mlx5e_ipsec_sadb_rx_lookup(struct mlx5e_ipsec *dev,void mlx5e_accel_ipsec_fs_cleanup (struct mlx5e_ipsec *ipsec);int mlx5e_accel_ipsec_fs_init (struct mlx5e_ipsec *ipsec);int mlx5e_accel_ipsec_fs_add_rule (struct mlx5e_priv *priv, -struct mlx5_accel_esp_xfrm_attrs *attrs, -u32 ipsec_obj_id, -struct mlx5e_ipsec_rule *ipsec_rule); +struct mlx5e_ipsec_sa_entry *sa_entry);void mlx5e_accel_ipsec_fs_del_rule (struct mlx5e_priv *priv, -struct mlx5_accel_esp_xfrm_attrs *attrs, -struct mlx5e_ipsec_rule *ipsec_rule); +struct mlx5e_ipsec_sa_entry *sa_entry);int mlx5_ipsec_create_sa_ctx (struct mlx5e_ipsec_sa_entry *sa_entry);void mlx5_ipsec_free_sa_ctx (struct mlx5e_ipsec_sa_entry *sa_entry);drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +9
-9@@ -454,11 +454,12 @@ static void setup_fte_common(struct mlx5_accel_esp_xfrm_attrs *attrs, }static int rx_add_rule (struct mlx5e_priv *priv, -struct mlx5_accel_esp_xfrm_attrs *attrs, -u32 ipsec_obj_id, -struct mlx5e_ipsec_rule *ipsec_rule) +struct mlx5e_ipsec_sa_entry *sa_entry) {u8 action[MLX5_UN_SZ_BYTES (set_add_copy_action_in_auto)] = {}; +struct mlx5e_ipsec_rule *ipsec_rule = &sa_entry->ipsec_rule; +struct mlx5_accel_esp_xfrm_attrs *attrs = &sa_entry->attrs; +u32 ipsec_obj_id = sa_entry->ipsec_obj_id;struct mlx5_modify_hdr *modify_hdr =NULL ;struct mlx5e_accel_fs_esp_prot *fs_prot;struct mlx5_flow_destination dest = {};@@ -532,9 +533,7 @@ static int rx_add_rule(struct mlx5e_priv *priv, }static int tx_add_rule (struct mlx5e_priv *priv, -struct mlx5_accel_esp_xfrm_attrs *attrs, -u32 ipsec_obj_id, -struct mlx5e_ipsec_rule *ipsec_rule) +struct mlx5e_ipsec_sa_entry *sa_entry) {struct mlx5_flow_act flow_act = {};struct mlx5_flow_handle *rule;@@ -551,7 +550,8 @@ static int tx_add_rule(struct mlx5e_priv *priv, goto out; } -setup_fte_common (attrs, ipsec_obj_id, spec, &flow_act); +setup_fte_common (&sa_entry->attrs, sa_entry->ipsec_obj_id, spec, + &flow_act);/* Add IPsec indicator in metadata_reg_a */ spec->match_criteria_enable |=MLX5_MATCH_MISC_PARAMETERS_2 ;@@ -566,11 +566,11 @@ static int tx_add_rule(struct mlx5e_priv *priv,if (IS_ERR (rule)) { err =PTR_ERR (rule);netdev_err (priv->netdev,"fail to add ipsec rule attrs->action=0x%x, err=%d\n" , - attrs->action, err); + sa_entry->attrs.action, err); goto out; } - ipsec_rule->rule = rule; + sa_entry->ipsec_rule.rule = rule;out:
kvfree(spec);drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +20
-17@@ -580,21 +580,25 @@ static int tx_add_rule(struct mlx5e_priv *priv, }static void rx_del_rule (struct mlx5e_priv *priv, -struct mlx5_accel_esp_xfrm_attrs *attrs, -struct mlx5e_ipsec_rule *ipsec_rule) +struct mlx5e_ipsec_sa_entry *sa_entry) { +struct mlx5e_ipsec_rule *ipsec_rule = &sa_entry->ipsec_rule; +mlx5_del_flow_rules (ipsec_rule->rule); ipsec_rule->rule =NULL ;mlx5_modify_header_dealloc (priv->mdev, ipsec_rule->set_modify_hdr); ipsec_rule->set_modify_hdr =NULL ; -rx_ft_put (priv, attrs->is_ipv6 ?ACCEL_FS_ESP6 :ACCEL_FS_ESP4 ); +rx_ft_put (priv, + sa_entry->attrs.is_ipv6 ?ACCEL_FS_ESP6 :ACCEL_FS_ESP4 ); }static void tx_del_rule (struct mlx5e_priv *priv, -struct mlx5e_ipsec_rule *ipsec_rule) +struct mlx5e_ipsec_sa_entry *sa_entry) { +struct mlx5e_ipsec_rule *ipsec_rule = &sa_entry->ipsec_rule; +mlx5_del_flow_rules (ipsec_rule->rule); ipsec_rule->rule =NULL ;@@ -602,24 +606,23 @@ static void tx_del_rule(struct mlx5e_priv *priv, }int mlx5e_accel_ipsec_fs_add_rule (struct mlx5e_priv *priv, -struct mlx5_accel_esp_xfrm_attrs *attrs, -u32 ipsec_obj_id, -struct mlx5e_ipsec_rule *ipsec_rule) +struct mlx5e_ipsec_sa_entry *sa_entry) { -if (attrs->action ==MLX5_ACCEL_ESP_ACTION_DECRYPT ) -return rx_add_rule (priv, attrs, ipsec_obj_id, ipsec_rule); -else -return tx_add_rule (priv, attrs, ipsec_obj_id, ipsec_rule); +if (sa_entry->attrs.action ==MLX5_ACCEL_ESP_ACTION_ENCRYPT ) +return tx_add_rule (priv, sa_entry); + +return rx_add_rule (priv, sa_entry); }void mlx5e_accel_ipsec_fs_del_rule (struct mlx5e_priv *priv, -struct mlx5_accel_esp_xfrm_attrs *attrs, -struct mlx5e_ipsec_rule *ipsec_rule) +struct mlx5e_ipsec_sa_entry *sa_entry) { -if (attrs->action ==MLX5_ACCEL_ESP_ACTION_DECRYPT ) -rx_del_rule (priv, attrs, ipsec_rule); -else -tx_del_rule (priv, ipsec_rule); +if (sa_entry->attrs.action ==MLX5_ACCEL_ESP_ACTION_ENCRYPT ) { +tx_del_rule (priv, sa_entry); +return ; + } + +rx_del_rule (priv, sa_entry); }void mlx5e_accel_ipsec_fs_cleanup (struct mlx5e_ipsec *ipsec)Signature · 1 line
2.35.1
Re: [PATCH net-next v1 10/17] net/mlx5: Clean IPsec FS add/delete rules
Quoted 9 lines · Saeed Mahameed
Quoted 4 lines · Leon Romanovsky
From: Leon Romanovsky <[email protected]>
Reuse existing struct to pass parameters instead of open code them.
Why? what do you mean "open code them" ? they are not open coded, they are primitive for a reason ! If we go with this reasoning, then let's pass
mlx5e_privto all functions and just forget about modularity.
There is not much value in having modularity between files/layers in same block. These layers are not usable outside of that block (IPsec) and ipsec.c is tightly coupled with ipsec_fs.c anyway by ensuring that unsupported options are handled as early as possible.
The remove of existing artificial layering allows me to see useless fields (see patch #17) and remove code that can't be executed anyway.
Separation between blocks (mlx5e_priv) is good and right thing, separation inside blocks is not.
Thanks
Quoted 193 lines
Quoted 14 lines
Reviewed-by Raed Salem <[email protected]>Signed-off-by Leon Romanovsky <[email protected]>---
.../mellanox/mlx5/core/en_accel/ipsec.c | 10 +--- .../mellanox/mlx5/core/en_accel/ipsec.h | 7 +-- .../mellanox/mlx5/core/en_accel/ipsec_fs.c | 55 ++++++++++--------- 3 files changed, 34 insertions(+), 38 deletions(-)drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +0
-0drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c static int mlx5e_xfrm_add_state(struct xfrm_state *x) · line 316
Quoted 7 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +1
-3@@ -313,9 +313,7 @@ static int mlx5e_xfrm_add_state(struct xfrm_state *x)if (err) goto err_xfrm; - err =mlx5e_accel_ipsec_fs_add_rule (priv, &sa_entry->attrs, - sa_entry->ipsec_obj_id, - &sa_entry->ipsec_rule); + err =mlx5e_accel_ipsec_fs_add_rule (priv, sa_entry);To add to my comment on the previous patch, in here the issue is more severe as previously
ipsec_fs.cwas unaware ofsa_entryobject and used to deal with pure fs related objects, you are peppering the code withsa_entryfor no reason, other than reducing function parameters from 4 to 2.Quoted 167 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +0
-0if (err) goto err_hw_ctx;@@ -333,8 +331,7 @@ static int mlx5e_xfrm_add_state(struct xfrm_state *x) goto out;
err_add_rule:drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +1
-2-mlx5e_accel_ipsec_fs_del_rule (priv, &sa_entry->attrs, - &sa_entry->ipsec_rule); +mlx5e_accel_ipsec_fs_del_rule (priv, sa_entry);
err_hw_ctx:mlx5_ipsec_free_sa_ctx(sa_entry);
err_xfrm:drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +1
-2@@ -357,8 +354,7 @@ static void mlx5e_xfrm_free_state(struct xfrm_state *x)struct mlx5e_priv *priv =netdev_priv (x->xso.dev);cancel_work_sync (&sa_entry->modify_work.work); -mlx5e_accel_ipsec_fs_del_rule (priv, &sa_entry->attrs, - &sa_entry->ipsec_rule); +mlx5e_accel_ipsec_fs_del_rule (priv, sa_entry);mlx5_ipsec_free_sa_ctx (sa_entry);kfree (sa_entry); }drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h +2
-5@@ -176,12 +176,9 @@ struct xfrm_state *mlx5e_ipsec_sadb_rx_lookup(struct mlx5e_ipsec *dev,void mlx5e_accel_ipsec_fs_cleanup (struct mlx5e_ipsec *ipsec);int mlx5e_accel_ipsec_fs_init (struct mlx5e_ipsec *ipsec);int mlx5e_accel_ipsec_fs_add_rule (struct mlx5e_priv *priv, -struct mlx5_accel_esp_xfrm_attrs *attrs, -u32 ipsec_obj_id, -struct mlx5e_ipsec_rule *ipsec_rule); +struct mlx5e_ipsec_sa_entry *sa_entry);void mlx5e_accel_ipsec_fs_del_rule (struct mlx5e_priv *priv, -struct mlx5_accel_esp_xfrm_attrs *attrs, -struct mlx5e_ipsec_rule *ipsec_rule); +struct mlx5e_ipsec_sa_entry *sa_entry);int mlx5_ipsec_create_sa_ctx (struct mlx5e_ipsec_sa_entry *sa_entry);void mlx5_ipsec_free_sa_ctx (struct mlx5e_ipsec_sa_entry *sa_entry);drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +9
-9@@ -454,11 +454,12 @@ static void setup_fte_common(struct mlx5_accel_esp_xfrm_attrs *attrs, }static int rx_add_rule (struct mlx5e_priv *priv, -struct mlx5_accel_esp_xfrm_attrs *attrs, -u32 ipsec_obj_id, -struct mlx5e_ipsec_rule *ipsec_rule) +struct mlx5e_ipsec_sa_entry *sa_entry) {u8 action[MLX5_UN_SZ_BYTES (set_add_copy_action_in_auto)] = {}; +struct mlx5e_ipsec_rule *ipsec_rule = &sa_entry->ipsec_rule; +struct mlx5_accel_esp_xfrm_attrs *attrs = &sa_entry->attrs; +u32 ipsec_obj_id = sa_entry->ipsec_obj_id;struct mlx5_modify_hdr *modify_hdr =NULL ;struct mlx5e_accel_fs_esp_prot *fs_prot;struct mlx5_flow_destination dest = {};@@ -532,9 +533,7 @@ static int rx_add_rule(struct mlx5e_priv *priv, }static int tx_add_rule (struct mlx5e_priv *priv, -struct mlx5_accel_esp_xfrm_attrs *attrs, -u32 ipsec_obj_id, -struct mlx5e_ipsec_rule *ipsec_rule) +struct mlx5e_ipsec_sa_entry *sa_entry) {struct mlx5_flow_act flow_act = {};struct mlx5_flow_handle *rule;@@ -551,7 +550,8 @@ static int tx_add_rule(struct mlx5e_priv *priv, goto out; } -setup_fte_common (attrs, ipsec_obj_id, spec, &flow_act); +setup_fte_common (&sa_entry->attrs, sa_entry->ipsec_obj_id, spec, + &flow_act);/* Add IPsec indicator in metadata_reg_a */ spec->match_criteria_enable |=MLX5_MATCH_MISC_PARAMETERS_2 ;@@ -566,11 +566,11 @@ static int tx_add_rule(struct mlx5e_priv *priv,if (IS_ERR (rule)) { err =PTR_ERR (rule);netdev_err (priv->netdev,"fail to add ipsec rule attrs->action=0x%x, err=%d\n" , - attrs->action, err); + sa_entry->attrs.action, err); goto out; } - ipsec_rule->rule = rule; + sa_entry->ipsec_rule.rule = rule;out:
kvfree(spec);drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +20
-17@@ -580,21 +580,25 @@ static int tx_add_rule(struct mlx5e_priv *priv, }static void rx_del_rule (struct mlx5e_priv *priv, -struct mlx5_accel_esp_xfrm_attrs *attrs, -struct mlx5e_ipsec_rule *ipsec_rule) +struct mlx5e_ipsec_sa_entry *sa_entry) { +struct mlx5e_ipsec_rule *ipsec_rule = &sa_entry->ipsec_rule; +mlx5_del_flow_rules (ipsec_rule->rule); ipsec_rule->rule =NULL ;mlx5_modify_header_dealloc (priv->mdev, ipsec_rule->set_modify_hdr); ipsec_rule->set_modify_hdr =NULL ; -rx_ft_put (priv, attrs->is_ipv6 ?ACCEL_FS_ESP6 :ACCEL_FS_ESP4 ); +rx_ft_put (priv, + sa_entry->attrs.is_ipv6 ?ACCEL_FS_ESP6 :ACCEL_FS_ESP4 ); }static void tx_del_rule (struct mlx5e_priv *priv, -struct mlx5e_ipsec_rule *ipsec_rule) +struct mlx5e_ipsec_sa_entry *sa_entry) { +struct mlx5e_ipsec_rule *ipsec_rule = &sa_entry->ipsec_rule; +mlx5_del_flow_rules (ipsec_rule->rule); ipsec_rule->rule =NULL ;@@ -602,24 +606,23 @@ static void tx_del_rule(struct mlx5e_priv *priv, }int mlx5e_accel_ipsec_fs_add_rule (struct mlx5e_priv *priv, -struct mlx5_accel_esp_xfrm_attrs *attrs, -u32 ipsec_obj_id, -struct mlx5e_ipsec_rule *ipsec_rule) +struct mlx5e_ipsec_sa_entry *sa_entry) { -if (attrs->action ==MLX5_ACCEL_ESP_ACTION_DECRYPT ) -return rx_add_rule (priv, attrs, ipsec_obj_id, ipsec_rule); -else -return tx_add_rule (priv, attrs, ipsec_obj_id, ipsec_rule); +if (sa_entry->attrs.action ==MLX5_ACCEL_ESP_ACTION_ENCRYPT ) +return tx_add_rule (priv, sa_entry); + +return rx_add_rule (priv, sa_entry); }void mlx5e_accel_ipsec_fs_del_rule (struct mlx5e_priv *priv, -struct mlx5_accel_esp_xfrm_attrs *attrs, -struct mlx5e_ipsec_rule *ipsec_rule) +struct mlx5e_ipsec_sa_entry *sa_entry) { -if (attrs->action ==MLX5_ACCEL_ESP_ACTION_DECRYPT ) -rx_del_rule (priv, attrs, ipsec_rule); -else -tx_del_rule (priv, ipsec_rule); +if (sa_entry->attrs.action ==MLX5_ACCEL_ESP_ACTION_ENCRYPT ) { +tx_del_rule (priv, sa_entry); +return ; + } + +rx_del_rule (priv, sa_entry); }void mlx5e_accel_ipsec_fs_cleanup (struct mlx5e_ipsec *ipsec)Signature · 1 line
2.35.1
PATCH 11/17 net/mlx5: Make sure that no dangling IPsec FS pointers exist
In reply to Leon RomanovskyFrom: Leon Romanovsky <[email protected]>
The IPsec FS code was implemented with anti-pattern there failures in create functions left the system with dangling pointers that were cleaned in global routines.
The less error prone approach is to make sure that failed function cleans everything internally.
As part of this change, we remove the batch of one liners and rewrite get/put functions to remove ambiguity.
--- .../mellanox/mlx5/core/en_accel/ipsec_fs.c | 229 ++++++------------ 1 file changed, 73 insertions(+), 156 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +73 -156
@@ -60,7 +60,7 @@ static int rx_err_add_rule(struct mlx5e_priv *priv,struct mlx5_modify_hdr *modify_hdr;struct mlx5_flow_handle *fte;struct mlx5_flow_spec *spec; -int err =0 ; +int err; spec =kvzalloc (sizeof (*spec),GFP_KERNEL );if (!spec)
@@ -96,101 +96,27 @@ static int rx_err_add_rule(struct mlx5e_priv *priv, goto out; } +kvfree (spec); rx_err->rule = fte; rx_err->copy_modify_hdr = modify_hdr; +return 0 ; out: -if (err) -mlx5_modify_header_dealloc (mdev, modify_hdr); +mlx5_modify_header_dealloc (mdev, modify_hdr); out_spec:kvfree (spec);return err; } -static void rx_err_del_rule (struct mlx5e_priv *priv, -struct mlx5e_ipsec_rx_err *rx_err) -{ -if (rx_err->rule) { -mlx5_del_flow_rules (rx_err->rule); - rx_err->rule =NULL ; - } - -if (rx_err->copy_modify_hdr) { -mlx5_modify_header_dealloc (priv->mdev, rx_err->copy_modify_hdr); - rx_err->copy_modify_hdr =NULL ; - } -} - -static void rx_err_destroy_ft (struct mlx5e_priv *priv,struct mlx5e_ipsec_rx_err *rx_err) -{ -rx_err_del_rule (priv, rx_err); - -if (rx_err->ft) { -mlx5_destroy_flow_table (rx_err->ft); - rx_err->ft =NULL ; - } -} - -static int rx_err_create_ft (struct mlx5e_priv *priv, -struct mlx5e_accel_fs_esp_prot *fs_prot, -struct mlx5e_ipsec_rx_err *rx_err) -{ -struct mlx5_flow_table_attr ft_attr = {}; -struct mlx5_flow_table *ft; -int err; - - ft_attr.max_fte =1 ; - ft_attr.autogroup.max_num_groups =1 ; - ft_attr.level =MLX5E_ACCEL_FS_ESP_FT_ERR_LEVEL ; - ft_attr.prio =MLX5E_NIC_PRIO ; - ft =mlx5_create_auto_grouped_flow_table (priv->fs.ns, &ft_attr); -if (IS_ERR (ft)) { - err =PTR_ERR (ft); -netdev_err (priv->netdev,"fail to create ipsec rx inline ft err=%d\n" , err); -return err; - } - - rx_err->ft = ft; - err =rx_err_add_rule (priv, fs_prot, rx_err); -if (err) - goto out_err; - -return 0 ; - -out_err: -mlx5_destroy_flow_table (ft); - rx_err->ft =NULL ; -return err; -} - -static void rx_fs_destroy (struct mlx5e_accel_fs_esp_prot *fs_prot) -{ -if (fs_prot->miss_rule) { -mlx5_del_flow_rules (fs_prot->miss_rule); - fs_prot->miss_rule =NULL ; - } - -if (fs_prot->miss_group) { -mlx5_destroy_flow_group (fs_prot->miss_group); - fs_prot->miss_group =NULL ; - } - -if (fs_prot->ft) { -mlx5_destroy_flow_table (fs_prot->ft); - fs_prot->ft =NULL ; - } -} -static int rx_fs_create (struct mlx5e_priv *priv,struct mlx5e_accel_fs_esp_prot *fs_prot) {int inlen =MLX5_ST_SZ_BYTES (create_flow_group_in); -struct mlx5_flow_table_attr ft_attr = {}; +struct mlx5_flow_table *ft = fs_prot->ft;struct mlx5_flow_group *miss_group;struct mlx5_flow_handle *miss_rule;MLX5_DECLARE_FLOW_ACT (flow_act);struct mlx5_flow_spec *spec; -struct mlx5_flow_table *ft;u32 *flow_group_in;int err =0 ;
@@ -201,20 +127,6 @@ static int rx_fs_create(struct mlx5e_priv *priv, goto out; } -/* Create FT */ - ft_attr.max_fte =NUM_IPSEC_FTE ; - ft_attr.level =MLX5E_ACCEL_FS_ESP_FT_LEVEL ; - ft_attr.prio =MLX5E_NIC_PRIO ; - ft_attr.autogroup.num_reserved_entries =1 ; - ft_attr.autogroup.max_num_groups =1 ; - ft =mlx5_create_auto_grouped_flow_table (priv->fs.ns, &ft_attr); -if (IS_ERR (ft)) { - err =PTR_ERR (ft); -netdev_err (priv->netdev,"fail to create ipsec rx ft err=%d\n" , err); - goto out; - } - fs_prot->ft = ft; -/* Create miss_group */ MLX5_SET (create_flow_group_in, flow_group_in, start_flow_index, ft->max_fte -1 );MLX5_SET (create_flow_group_in, flow_group_in, end_flow_index, ft->max_fte -1 );
@@ -229,19 +141,19 @@ static int rx_fs_create(struct mlx5e_priv *priv,/* Create miss rule */ miss_rule =mlx5_add_flow_rules (ft, spec, &flow_act, &fs_prot->default_dest,1 );if (IS_ERR (miss_rule)) { +mlx5_destroy_flow_group (fs_prot->miss_group); err =PTR_ERR (miss_rule);netdev_err (priv->netdev,"fail to create ipsec rx miss_rule err=%d\n" , err); goto out; } fs_prot->miss_rule = miss_rule; - out:kvfree (flow_group_in);kvfree (spec);return err; } -static int rx_destroy (struct mlx5e_priv *priv,enum accel_fs_esp_type type) +static void rx_destroy (struct mlx5e_priv *priv,enum accel_fs_esp_type type) {struct mlx5e_accel_fs_esp_prot *fs_prot;struct mlx5e_accel_fs_esp *accel_esp;
@@ -251,17 +163,21 @@ static int rx_destroy(struct mlx5e_priv *priv, enum accel_fs_esp_type type)/* The netdev unreg already happened, so all offloaded rule are already removed */ fs_prot = &accel_esp->fs_prot[type]; -rx_fs_destroy (fs_prot); - -rx_err_destroy_ft (priv, &fs_prot->rx_err); +mlx5_del_flow_rules (fs_prot->miss_rule); +mlx5_destroy_flow_group (fs_prot->miss_group); +mlx5_destroy_flow_table (fs_prot->ft); -return 0 ; +mlx5_del_flow_rules (fs_prot->rx_err.rule); +mlx5_modify_header_dealloc (priv->mdev, fs_prot->rx_err.copy_modify_hdr); +mlx5_destroy_flow_table (fs_prot->rx_err.ft); }static int rx_create (struct mlx5e_priv *priv,enum accel_fs_esp_type type) { +struct mlx5_flow_table_attr ft_attr = {};struct mlx5e_accel_fs_esp_prot *fs_prot;struct mlx5e_accel_fs_esp *accel_esp; +struct mlx5_flow_table *ft;int err; accel_esp = priv->ipsec->rx_fs;
@@ -270,14 +186,45 @@ static int rx_create(struct mlx5e_priv *priv, enum accel_fs_esp_type type) fs_prot->default_dest =mlx5_ttc_get_default_dest (priv->fs.ttc,fs_esp2tt (type)); - err =rx_err_create_ft (priv, fs_prot, &fs_prot->rx_err); + ft_attr.max_fte =1 ; + ft_attr.autogroup.max_num_groups =1 ; + ft_attr.level =MLX5E_ACCEL_FS_ESP_FT_ERR_LEVEL ; + ft_attr.prio =MLX5E_NIC_PRIO ; + ft =mlx5_create_auto_grouped_flow_table (priv->fs.ns, &ft_attr); +if (IS_ERR (ft)) +return PTR_ERR (ft); + + fs_prot->rx_err.ft = ft; + err =rx_err_add_rule (priv, fs_prot, &fs_prot->rx_err);if (err) -return err; + goto err_add; + +/* Create FT */ + ft_attr.max_fte =NUM_IPSEC_FTE ; + ft_attr.level =MLX5E_ACCEL_FS_ESP_FT_LEVEL ; + ft_attr.prio =MLX5E_NIC_PRIO ; + ft_attr.autogroup.num_reserved_entries =1 ; + ft_attr.autogroup.max_num_groups =1 ; + ft =mlx5_create_auto_grouped_flow_table (priv->fs.ns, &ft_attr); +if (IS_ERR (ft)) { + err =PTR_ERR (ft); + goto err_fs_ft; + } + fs_prot->ft = ft; err =rx_fs_create (priv, fs_prot);if (err) -rx_destroy (priv, type); + goto err_fs; + +return 0 ; +err_fs: +mlx5_destroy_flow_table (fs_prot->ft); +err_fs_ft: +mlx5_del_flow_rules (fs_prot->rx_err.rule); +mlx5_modify_header_dealloc (priv->mdev, fs_prot->rx_err.copy_modify_hdr); +err_add: +mlx5_destroy_flow_table (fs_prot->rx_err.ft);return err; }
@@ -291,21 +238,21 @@ static int rx_ft_get(struct mlx5e_priv *priv, enum accel_fs_esp_type type) accel_esp = priv->ipsec->rx_fs; fs_prot = &accel_esp->fs_prot[type];mutex_lock (&fs_prot->prot_mutex); -if (fs_prot->refcnt++) - goto out; +if (fs_prot->refcnt) + goto skip;/* create FT */ err =rx_create (priv, type); -if (err) { - fs_prot->refcnt--; +if (err) goto out; - }/* connect */ dest.type =MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE ; dest.ft = fs_prot->ft;mlx5_ttc_fwd_dest (priv->fs.ttc,fs_esp2tt (type), &dest); +skip: + fs_prot->refcnt++; out:mutex_unlock (&fs_prot->prot_mutex);return err;
@@ -319,7 +266,8 @@ static void rx_ft_put(struct mlx5e_priv *priv, enum accel_fs_esp_type type) accel_esp = priv->ipsec->rx_fs; fs_prot = &accel_esp->fs_prot[type];mutex_lock (&fs_prot->prot_mutex); -if (--fs_prot->refcnt) + fs_prot->refcnt--; +if (fs_prot->refcnt) goto out;/* disconnect */
@@ -352,32 +300,20 @@ static int tx_create(struct mlx5e_priv *priv)return 0 ; } -static void tx_destroy (struct mlx5e_priv *priv) -{ -struct mlx5e_ipsec *ipsec = priv->ipsec; - -if (IS_ERR_OR_NULL (ipsec->tx_fs->ft)) -return ; - -mlx5_destroy_flow_table (ipsec->tx_fs->ft); - ipsec->tx_fs->ft =NULL ; -} -static int tx_ft_get (struct mlx5e_priv *priv) {struct mlx5e_ipsec_tx *tx_fs = priv->ipsec->tx_fs;int err =0 ;mutex_lock (&tx_fs->mutex); -if (tx_fs->refcnt++) - goto out; +if (tx_fs->refcnt) + goto skip; err =tx_create (priv); -if (err) { - tx_fs->refcnt--; +if (err) goto out; - } - +skip: + tx_fs->refcnt++; out:mutex_unlock (&tx_fs->mutex);return err;
@@ -388,11 +324,11 @@ static void tx_ft_put(struct mlx5e_priv *priv)struct mlx5e_ipsec_tx *tx_fs = priv->ipsec->tx_fs;mutex_lock (&tx_fs->mutex); -if (--tx_fs->refcnt) + tx_fs->refcnt--; +if (tx_fs->refcnt) goto out; -tx_destroy (priv); - +mlx5_destroy_flow_table (tx_fs->ft); out:mutex_unlock (&tx_fs->mutex); }
@@ -579,32 +515,6 @@ static int tx_add_rule(struct mlx5e_priv *priv,return err; } -static void rx_del_rule (struct mlx5e_priv *priv, -struct mlx5e_ipsec_sa_entry *sa_entry) -{ -struct mlx5e_ipsec_rule *ipsec_rule = &sa_entry->ipsec_rule; - -mlx5_del_flow_rules (ipsec_rule->rule); - ipsec_rule->rule =NULL ; - -mlx5_modify_header_dealloc (priv->mdev, ipsec_rule->set_modify_hdr); - ipsec_rule->set_modify_hdr =NULL ; - -rx_ft_put (priv, - sa_entry->attrs.is_ipv6 ?ACCEL_FS_ESP6 :ACCEL_FS_ESP4 ); -} - -static void tx_del_rule (struct mlx5e_priv *priv, -struct mlx5e_ipsec_sa_entry *sa_entry) -{ -struct mlx5e_ipsec_rule *ipsec_rule = &sa_entry->ipsec_rule; - -mlx5_del_flow_rules (ipsec_rule->rule); - ipsec_rule->rule =NULL ; - -tx_ft_put (priv); -} -int mlx5e_accel_ipsec_fs_add_rule (struct mlx5e_priv *priv,struct mlx5e_ipsec_sa_entry *sa_entry) {
@@ -617,12 +527,19 @@ int mlx5e_accel_ipsec_fs_add_rule(struct mlx5e_priv *priv,void mlx5e_accel_ipsec_fs_del_rule (struct mlx5e_priv *priv,struct mlx5e_ipsec_sa_entry *sa_entry) { +struct mlx5e_ipsec_rule *ipsec_rule = &sa_entry->ipsec_rule; +struct mlx5_core_dev *mdev =mlx5e_ipsec_sa2dev (sa_entry); + +mlx5_del_flow_rules (ipsec_rule->rule); +if (sa_entry->attrs.action ==MLX5_ACCEL_ESP_ACTION_ENCRYPT ) { -tx_del_rule (priv, sa_entry); +tx_ft_put (priv);return ; } -rx_del_rule (priv, sa_entry); +mlx5_modify_header_dealloc (mdev, ipsec_rule->set_modify_hdr); +rx_ft_put (priv, + sa_entry->attrs.is_ipv6 ?ACCEL_FS_ESP6 :ACCEL_FS_ESP4 ); }void mlx5e_accel_ipsec_fs_cleanup (struct mlx5e_ipsec *ipsec)
Signature · 1 line
2.35.1
PATCH 12/17 net/mlx5: Don't advertise IPsec netdev support for non-IPsec device
In reply to Leon RomanovskyFrom: Leon Romanovsky <[email protected]>
Device that lacks proper IPsec capabilities won't pass mlx5e_ipsec_init() later, so no need to advertise HW netdev offload support for something that isn't going to work anyway.
--- drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +3 -0
@@ -454,6 +454,9 @@ void mlx5e_ipsec_build_netdev(struct mlx5e_priv *priv)struct mlx5_core_dev *mdev = priv->mdev;struct net_device *netdev = priv->netdev; +if (!mlx5_ipsec_device_caps (mdev)) +return ; +if (!(mlx5_ipsec_device_caps (mdev) &MLX5_ACCEL_IPSEC_CAP_ESP ) || !MLX5_CAP_ETH (mdev, swp)) {mlx5_core_dbg (mdev,"mlx5e: ESP and SWP offload not supported\n" );
Signature · 1 line
2.35.1
PATCH 13/17 net/mlx5: Simplify IPsec capabilities logic
In reply to Leon RomanovskyFrom: Leon Romanovsky <[email protected]>
Reduce number of hard-coded IPsec capabilities by making sure that mlx5_ipsec_device_caps() sets only supported bits.
As part of this change, remove _accel_ notations from the names and prepare the code to IPsec full offload mode.
--- .../mellanox/mlx5/core/en_accel/ipsec.c | 16 ++------------ .../mellanox/mlx5/core/en_accel/ipsec.h | 9 +++----- .../mlx5/core/en_accel/ipsec_offload.c | 22 +++++++++---------- 3 files changed, 16 insertions(+), 31 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +2 -14
@@ -215,7 +215,7 @@ static inline int mlx5e_xfrm_validate_state(struct xfrm_state *x)return -EINVAL ; }if (x->props.flags &XFRM_STATE_ESN && - !(mlx5_ipsec_device_caps (priv->mdev) &MLX5_ACCEL_IPSEC_CAP_ESN )) { + !(mlx5_ipsec_device_caps (priv->mdev) &MLX5_IPSEC_CAP_ESN )) {netdev_info (netdev,"Cannot offload ESN xfrm states\n" );return -EINVAL ; }
@@ -262,11 +262,6 @@ 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->props.family ==AF_INET6 && - !(mlx5_ipsec_device_caps (priv->mdev) &MLX5_ACCEL_IPSEC_CAP_IPV6 )) { -netdev_info (netdev,"IPv6 xfrm state offload is not supported by this device\n" ); -return -EINVAL ; - }return 0 ; }
@@ -457,12 +452,6 @@ void mlx5e_ipsec_build_netdev(struct mlx5e_priv *priv)if (!mlx5_ipsec_device_caps (mdev))return ; -if (!(mlx5_ipsec_device_caps (mdev) &MLX5_ACCEL_IPSEC_CAP_ESP ) || - !MLX5_CAP_ETH (mdev, swp)) { -mlx5_core_dbg (mdev,"mlx5e: ESP and SWP offload not supported\n" ); -return ; - } -mlx5_core_info (mdev,"mlx5e: IPSec ESP acceleration enabled\n" ); netdev->xfrmdev_ops = &mlx5e_ipsec_xfrmdev_ops; netdev->features |=NETIF_F_HW_ESP ;
@@ -476,8 +465,7 @@ void mlx5e_ipsec_build_netdev(struct mlx5e_priv *priv) netdev->features |=NETIF_F_HW_ESP_TX_CSUM ; netdev->hw_enc_features |=NETIF_F_HW_ESP_TX_CSUM ; -if (!(mlx5_ipsec_device_caps (mdev) &MLX5_ACCEL_IPSEC_CAP_LSO ) || - !MLX5_CAP_ETH (mdev, swp_lso)) { +if (!MLX5_CAP_ETH (mdev, swp_lso)) {mlx5_core_dbg (mdev,"mlx5e: ESP LSO not supported\n" );return ; }
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h +3 -6
@@ -102,12 +102,9 @@ struct mlx5_accel_esp_xfrm_attrs {u8 is_ipv6; }; -enum mlx5_accel_ipsec_cap { -MLX5_ACCEL_IPSEC_CAP_DEVICE =1 <<0 , -MLX5_ACCEL_IPSEC_CAP_ESP =1 <<1 , -MLX5_ACCEL_IPSEC_CAP_IPV6 =1 <<2 , -MLX5_ACCEL_IPSEC_CAP_LSO =1 <<3 , -MLX5_ACCEL_IPSEC_CAP_ESN =1 <<4 , +enum mlx5_ipsec_cap { +MLX5_IPSEC_CAP_CRYPTO =1 <<0 , +MLX5_IPSEC_CAP_ESN =1 <<1 , };struct mlx5e_priv ;
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c +11 -11
@@ -7,7 +7,7 @@u32 mlx5_ipsec_device_caps (struct mlx5_core_dev *mdev) { -u32 caps; +u32 caps =0 ;if (!MLX5_CAP_GEN (mdev, ipsec_offload))return 0 ;
@@ -19,23 +19,23 @@ u32 mlx5_ipsec_device_caps(struct mlx5_core_dev *mdev) MLX5_HCA_CAP_GENERAL_OBJECT_TYPES_IPSEC))return 0 ; -if (!MLX5_CAP_IPSEC (mdev, ipsec_crypto_offload) || - !MLX5_CAP_ETH (mdev, insert_trailer)) -return 0 ; -if (!MLX5_CAP_FLOWTABLE_NIC_TX (mdev, ipsec_encrypt) || !MLX5_CAP_FLOWTABLE_NIC_RX (mdev, ipsec_decrypt))return 0 ; - caps =MLX5_ACCEL_IPSEC_CAP_DEVICE |MLX5_ACCEL_IPSEC_CAP_IPV6 | -MLX5_ACCEL_IPSEC_CAP_LSO ; +if (!MLX5_CAP_IPSEC (mdev, ipsec_crypto_esp_aes_gcm_128_encrypt) || + !MLX5_CAP_IPSEC (mdev, ipsec_crypto_esp_aes_gcm_128_decrypt)) +return 0 ; -if (MLX5_CAP_IPSEC (mdev, ipsec_crypto_esp_aes_gcm_128_encrypt) && -MLX5_CAP_IPSEC (mdev, ipsec_crypto_esp_aes_gcm_128_decrypt)) - caps |=MLX5_ACCEL_IPSEC_CAP_ESP ; +if (MLX5_CAP_IPSEC (mdev, ipsec_crypto_offload) && +MLX5_CAP_ETH (mdev, insert_trailer) &&MLX5_CAP_ETH (mdev, swp)) + caps |=MLX5_IPSEC_CAP_CRYPTO ; + +if (!caps) +return 0 ;if (MLX5_CAP_IPSEC (mdev, ipsec_esn)) - caps |=MLX5_ACCEL_IPSEC_CAP_ESN ; + caps |=MLX5_IPSEC_CAP_ESN ;/* We can accommodate up to 2^24 different IPsec objects * because we use up to 24 bit in flow table metadata
Signature · 1 line
2.35.1
Saeed Mahameed
maintainerRe: [PATCH net-next v1 13/17] net/mlx5: Simplify IPsec capabilities logic
Quoted 8 lines · Leon Romanovsky
From: Leon Romanovsky <[email protected]>
Reduce number of hard-coded IPsec capabilities by making sure that
mlx5_ipsec_device_caps()sets only supported bits.As part of this change, remove _accel_ notations from the names and prepare the code to IPsec full offload mode.
Can you explain why remove __accel__ notation ?__accel__ notation and decoupling from other common netdev features is done for modularity purpose, en_accel directories are separated so we can implement complex/stateful accelerations while avoid contaminating/affecting common data-path performance sensitives flows.
I think keeping __accel__ notations is a must here for the above reasons, unless you have a more strong reason to remove it..
Quoted 126 lines
Reviewed-by Raed Salem <[email protected]>Signed-off-by Leon Romanovsky <[email protected]>---
.../mellanox/mlx5/core/en_accel/ipsec.c | 16 ++------------ .../mellanox/mlx5/core/en_accel/ipsec.h | 9 +++----- .../mlx5/core/en_accel/ipsec_offload.c | 22 +++++++++---------- 3 files changed, 16 insertions(+), 31 deletions(-)drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +2
-14@@ -215,7 +215,7 @@ static inline int mlx5e_xfrm_validate_state(struct xfrm_state *x)return -EINVAL ; }if (x->props.flags &XFRM_STATE_ESN && - !(mlx5_ipsec_device_caps (priv->mdev) &MLX5_ACCEL_IPSEC_CAP_ESN )) { + !(mlx5_ipsec_device_caps (priv->mdev) &MLX5_IPSEC_CAP_ESN )) {netdev_info (netdev,"Cannot offload ESN xfrm states\n" );return -EINVAL ; }@@ -262,11 +262,6 @@ 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->props.family ==AF_INET6 && - !(mlx5_ipsec_device_caps (priv->mdev) &MLX5_ACCEL_IPSEC_CAP_IPV6 )) { -netdev_info (netdev,"IPv6 xfrm state offload is not supported by this device\n" ); -return -EINVAL ; - }return 0 ; }@@ -457,12 +452,6 @@ void mlx5e_ipsec_build_netdev(struct mlx5e_priv *priv)if (!mlx5_ipsec_device_caps (mdev))return ; -if (!(mlx5_ipsec_device_caps (mdev) &MLX5_ACCEL_IPSEC_CAP_ESP ) || - !MLX5_CAP_ETH (mdev, swp)) { -mlx5_core_dbg (mdev,"mlx5e: ESP and SWP offload not supported\n" ); -return ; - } -mlx5_core_info (mdev,"mlx5e: IPSec ESP acceleration enabled\n" ); netdev->xfrmdev_ops = &mlx5e_ipsec_xfrmdev_ops; netdev->features |=NETIF_F_HW_ESP ;@@ -476,8 +465,7 @@ void mlx5e_ipsec_build_netdev(struct mlx5e_priv *priv) netdev->features |=NETIF_F_HW_ESP_TX_CSUM ; netdev->hw_enc_features |=NETIF_F_HW_ESP_TX_CSUM ; -if (!(mlx5_ipsec_device_caps (mdev) &MLX5_ACCEL_IPSEC_CAP_LSO ) || - !MLX5_CAP_ETH (mdev, swp_lso)) { +if (!MLX5_CAP_ETH (mdev, swp_lso)) {mlx5_core_dbg (mdev,"mlx5e: ESP LSO not supported\n" );return ; }drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h +3
-6@@ -102,12 +102,9 @@ struct mlx5_accel_esp_xfrm_attrs {u8 is_ipv6; }; -enum mlx5_accel_ipsec_cap { -MLX5_ACCEL_IPSEC_CAP_DEVICE =1 <<0 , -MLX5_ACCEL_IPSEC_CAP_ESP =1 <<1 , -MLX5_ACCEL_IPSEC_CAP_IPV6 =1 <<2 , -MLX5_ACCEL_IPSEC_CAP_LSO =1 <<3 , -MLX5_ACCEL_IPSEC_CAP_ESN =1 <<4 , +enum mlx5_ipsec_cap { +MLX5_IPSEC_CAP_CRYPTO =1 <<0 , +MLX5_IPSEC_CAP_ESN =1 <<1 , };struct mlx5e_priv ;drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c +11
-11@@ -7,7 +7,7 @@u32 mlx5_ipsec_device_caps (struct mlx5_core_dev *mdev) { -u32 caps; +u32 caps =0 ;if (!MLX5_CAP_GEN (mdev, ipsec_offload))return 0 ;@@ -19,23 +19,23 @@ u32 mlx5_ipsec_device_caps(struct mlx5_core_dev *mdev) MLX5_HCA_CAP_GENERAL_OBJECT_TYPES_IPSEC))return 0 ; -if (!MLX5_CAP_IPSEC (mdev, ipsec_crypto_offload) || - !MLX5_CAP_ETH (mdev, insert_trailer)) -return 0 ; -if (!MLX5_CAP_FLOWTABLE_NIC_TX (mdev, ipsec_encrypt) || !MLX5_CAP_FLOWTABLE_NIC_RX (mdev, ipsec_decrypt))return 0 ; - caps =MLX5_ACCEL_IPSEC_CAP_DEVICE |MLX5_ACCEL_IPSEC_CAP_IPV6 | -MLX5_ACCEL_IPSEC_CAP_LSO ; +if (!MLX5_CAP_IPSEC (mdev, ipsec_crypto_esp_aes_gcm_128_encrypt) || + !MLX5_CAP_IPSEC (mdev, ipsec_crypto_esp_aes_gcm_128_decrypt)) +return 0 ; -if (MLX5_CAP_IPSEC (mdev, ipsec_crypto_esp_aes_gcm_128_encrypt) && -MLX5_CAP_IPSEC (mdev, ipsec_crypto_esp_aes_gcm_128_decrypt)) - caps |=MLX5_ACCEL_IPSEC_CAP_ESP ; +if (MLX5_CAP_IPSEC (mdev, ipsec_crypto_offload) && +MLX5_CAP_ETH (mdev, insert_trailer) &&MLX5_CAP_ETH (mdev, swp)) + caps |=MLX5_IPSEC_CAP_CRYPTO ; + +if (!caps) +return 0 ;if (MLX5_CAP_IPSEC (mdev, ipsec_esn)) - caps |=MLX5_ACCEL_IPSEC_CAP_ESN ; + caps |=MLX5_IPSEC_CAP_ESN ;/* We can accommodate up to 2^24 different IPsec objects * because we use up to 24 bit in flow table metadata Signature · 1 line
2.35.1
Re: [PATCH net-next v1 13/17] net/mlx5: Simplify IPsec capabilities logic
Quoted 10 lines · Saeed Mahameed
Quoted 8 lines · Leon Romanovsky
From: Leon Romanovsky <[email protected]>
Reduce number of hard-coded IPsec capabilities by making sure that
mlx5_ipsec_device_caps()sets only supported bits.As part of this change, remove _accel_ notations from the names and prepare the code to IPsec full offload mode.
Quoted 8 lines
Can you explain why remove
__accel__notation ?__accel__notation and decoupling from other common netdev features is done for modularity purpose,en_acceldirectories are separated so we can implement complex/stateful accelerations while avoid contaminating/affecting common data-path performance sensitives flows.I think keeping
__accel__notations is a must here for the above reasons, unless you have a more strong reason to remove it..
Acceleration and hardware offloads are the same in their end result, but different in meaning and in their implementations.
Accelerators are usually represented by specialized hardware that designed to perform specific tasks. In our case, CX devices provide hardware offload capabilities that extends general purpose NIC and not accelerations.
__accel__ is a wrong word here.
Thanks
Quoted 127 lines
Quoted 127 lines
Reviewed-by Raed Salem <[email protected]>Signed-off-by Leon Romanovsky <[email protected]>---
.../mellanox/mlx5/core/en_accel/ipsec.c | 16 ++------------ .../mellanox/mlx5/core/en_accel/ipsec.h | 9 +++----- .../mlx5/core/en_accel/ipsec_offload.c | 22 +++++++++---------- 3 files changed, 16 insertions(+), 31 deletions(-)drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +2
-14@@ -215,7 +215,7 @@ static inline int mlx5e_xfrm_validate_state(struct xfrm_state *x)return -EINVAL ; }if (x->props.flags &XFRM_STATE_ESN && - !(mlx5_ipsec_device_caps (priv->mdev) &MLX5_ACCEL_IPSEC_CAP_ESN )) { + !(mlx5_ipsec_device_caps (priv->mdev) &MLX5_IPSEC_CAP_ESN )) {netdev_info (netdev,"Cannot offload ESN xfrm states\n" );return -EINVAL ; }@@ -262,11 +262,6 @@ 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->props.family ==AF_INET6 && - !(mlx5_ipsec_device_caps (priv->mdev) &MLX5_ACCEL_IPSEC_CAP_IPV6 )) { -netdev_info (netdev,"IPv6 xfrm state offload is not supported by this device\n" ); -return -EINVAL ; - }return 0 ; }@@ -457,12 +452,6 @@ void mlx5e_ipsec_build_netdev(struct mlx5e_priv *priv)if (!mlx5_ipsec_device_caps (mdev))return ; -if (!(mlx5_ipsec_device_caps (mdev) &MLX5_ACCEL_IPSEC_CAP_ESP ) || - !MLX5_CAP_ETH (mdev, swp)) { -mlx5_core_dbg (mdev,"mlx5e: ESP and SWP offload not supported\n" ); -return ; - } -mlx5_core_info (mdev,"mlx5e: IPSec ESP acceleration enabled\n" ); netdev->xfrmdev_ops = &mlx5e_ipsec_xfrmdev_ops; netdev->features |=NETIF_F_HW_ESP ;@@ -476,8 +465,7 @@ void mlx5e_ipsec_build_netdev(struct mlx5e_priv *priv) netdev->features |=NETIF_F_HW_ESP_TX_CSUM ; netdev->hw_enc_features |=NETIF_F_HW_ESP_TX_CSUM ; -if (!(mlx5_ipsec_device_caps (mdev) &MLX5_ACCEL_IPSEC_CAP_LSO ) || - !MLX5_CAP_ETH (mdev, swp_lso)) { +if (!MLX5_CAP_ETH (mdev, swp_lso)) {mlx5_core_dbg (mdev,"mlx5e: ESP LSO not supported\n" );return ; }drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h +3
-6@@ -102,12 +102,9 @@ struct mlx5_accel_esp_xfrm_attrs {u8 is_ipv6; }; -enum mlx5_accel_ipsec_cap { -MLX5_ACCEL_IPSEC_CAP_DEVICE =1 <<0 , -MLX5_ACCEL_IPSEC_CAP_ESP =1 <<1 , -MLX5_ACCEL_IPSEC_CAP_IPV6 =1 <<2 , -MLX5_ACCEL_IPSEC_CAP_LSO =1 <<3 , -MLX5_ACCEL_IPSEC_CAP_ESN =1 <<4 , +enum mlx5_ipsec_cap { +MLX5_IPSEC_CAP_CRYPTO =1 <<0 , +MLX5_IPSEC_CAP_ESN =1 <<1 , };struct mlx5e_priv ;drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c +11
-11@@ -7,7 +7,7 @@u32 mlx5_ipsec_device_caps (struct mlx5_core_dev *mdev) { -u32 caps; +u32 caps =0 ;if (!MLX5_CAP_GEN (mdev, ipsec_offload))return 0 ;@@ -19,23 +19,23 @@ u32 mlx5_ipsec_device_caps(struct mlx5_core_dev *mdev) MLX5_HCA_CAP_GENERAL_OBJECT_TYPES_IPSEC))return 0 ; -if (!MLX5_CAP_IPSEC (mdev, ipsec_crypto_offload) || - !MLX5_CAP_ETH (mdev, insert_trailer)) -return 0 ; -if (!MLX5_CAP_FLOWTABLE_NIC_TX (mdev, ipsec_encrypt) || !MLX5_CAP_FLOWTABLE_NIC_RX (mdev, ipsec_decrypt))return 0 ; - caps =MLX5_ACCEL_IPSEC_CAP_DEVICE |MLX5_ACCEL_IPSEC_CAP_IPV6 | -MLX5_ACCEL_IPSEC_CAP_LSO ; +if (!MLX5_CAP_IPSEC (mdev, ipsec_crypto_esp_aes_gcm_128_encrypt) || + !MLX5_CAP_IPSEC (mdev, ipsec_crypto_esp_aes_gcm_128_decrypt)) +return 0 ; -if (MLX5_CAP_IPSEC (mdev, ipsec_crypto_esp_aes_gcm_128_encrypt) && -MLX5_CAP_IPSEC (mdev, ipsec_crypto_esp_aes_gcm_128_decrypt)) - caps |=MLX5_ACCEL_IPSEC_CAP_ESP ; +if (MLX5_CAP_IPSEC (mdev, ipsec_crypto_offload) && +MLX5_CAP_ETH (mdev, insert_trailer) &&MLX5_CAP_ETH (mdev, swp)) + caps |=MLX5_IPSEC_CAP_CRYPTO ; + +if (!caps) +return 0 ;if (MLX5_CAP_IPSEC (mdev, ipsec_esn)) - caps |=MLX5_ACCEL_IPSEC_CAP_ESN ; + caps |=MLX5_IPSEC_CAP_ESN ;/* We can accommodate up to 2^24 different IPsec objects * because we use up to 24 bit in flow table metadata Signature · 1 line
2.35.1
PATCH 14/17 net/mlx5: Remove not-supported ICV length
In reply to Leon RomanovskyFrom: Leon Romanovsky <[email protected]>
mlx5 doesn't allow to configure any AEAD ICV length other than 128, so remove the logic that configures other unsupported values.
--- .../mellanox/mlx5/core/en_accel/ipsec_offload.c | 17 +---------------- include/linux/mlx5/mlx5_ifc.h | 2 -- 2 files changed, 1 insertion(+), 18 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c +1 -16
@@ -62,22 +62,7 @@ static int mlx5_create_ipsec_obj(struct mlx5e_ipsec_sa_entry *sa_entry) salt_p =MLX5_ADDR_OF (ipsec_obj, obj, salt);memcpy (salt_p, &aes_gcm->salt,sizeof (aes_gcm->salt)); -switch (aes_gcm->icv_len) { -case 64 : -MLX5_SET (ipsec_obj, obj, icv_length, -MLX5_IPSEC_OBJECT_ICV_LEN_8B ); -break ; -case 96 : -MLX5_SET (ipsec_obj, obj, icv_length, -MLX5_IPSEC_OBJECT_ICV_LEN_12B ); -break ; -case 128 : -MLX5_SET (ipsec_obj, obj, icv_length, -MLX5_IPSEC_OBJECT_ICV_LEN_16B ); -break ; -default : -return -EINVAL ; - } +MLX5_SET (ipsec_obj, obj, icv_length,MLX5_IPSEC_OBJECT_ICV_LEN_16B ); salt_iv_p =MLX5_ADDR_OF (ipsec_obj, obj, implicit_iv);memcpy (salt_iv_p, &aes_gcm->seq_iv,sizeof (aes_gcm->seq_iv));/* esn */
include/linux/mlx5/mlx5_ifc.h +0 -2
@@ -11383,8 +11383,6 @@ enum {enum {MLX5_IPSEC_OBJECT_ICV_LEN_16B , -MLX5_IPSEC_OBJECT_ICV_LEN_12B , -MLX5_IPSEC_OBJECT_ICV_LEN_8B , };struct mlx5_ifc_ipsec_obj_bits {
Signature · 1 line
2.35.1
PATCH 15/17 net/mlx5: Cleanup XFRM attributes struct
In reply to Leon RomanovskyFrom: Leon Romanovsky <[email protected]>
Remove everything that is not used or from mlx5_accel_esp_xfrm_attrs, together with change type of spi to store proper type from the beginning.
--- .../mellanox/mlx5/core/en_accel/ipsec.c | 10 ++------- .../mellanox/mlx5/core/en_accel/ipsec.h | 21 ++----------------- .../mellanox/mlx5/core/en_accel/ipsec_fs.c | 4 ++-- .../mlx5/core/en_accel/ipsec_offload.c | 4 ++-- 4 files changed, 8 insertions(+), 31 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +2 -8
@@ -137,7 +137,7 @@ mlx5e_ipsec_build_accel_xfrm_attrs(struct mlx5e_ipsec_sa_entry *sa_entry, struct mlx5_accel_esp_xfrm_attrs *attrs) {struct xfrm_state *x = sa_entry->x; -struct aes_gcm_keymat *aes_gcm = &attrs->keymat.aes_gcm; +struct aes_gcm_keymat *aes_gcm = &attrs->aes_gcm;struct aead_geniv_ctx *geniv_ctx;struct crypto_aead *aead;unsigned int crypto_data_len, key_len;
@@ -171,12 +171,6 @@ mlx5e_ipsec_build_accel_xfrm_attrs(struct mlx5e_ipsec_sa_entry *sa_entry, attrs->flags |=MLX5_ACCEL_ESP_FLAGS_ESN_STATE_OVERLAP ; } -/* rx handle */ - attrs->sa_handle = sa_entry->handle; - -/* algo type */ - attrs->keymat_type =MLX5_ACCEL_ESP_KEYMAT_AES_GCM ; -/* action */ attrs->action = (!(x->xso.flags &XFRM_OFFLOAD_INBOUND )) ? MLX5_ACCEL_ESP_ACTION_ENCRYPT :
@@ -187,7 +181,7 @@ mlx5e_ipsec_build_accel_xfrm_attrs(struct mlx5e_ipsec_sa_entry *sa_entry,MLX5_ACCEL_ESP_FLAGS_TUNNEL ;/* spi */ - attrs->spi = x->id.spi; + attrs->spi =be32_to_cpu (x->id.spi);/* source , destination ips */ memcpy (&attrs->saddr, x->props.saddr.a6,sizeof (attrs->saddr));
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h +2 -19
@@ -55,11 +55,6 @@ enum mlx5_accel_esp_action { MLX5_ACCEL_ESP_ACTION_ENCRYPT, }; -enum mlx5_accel_esp_keymats { -MLX5_ACCEL_ESP_KEYMAT_AES_NONE , -MLX5_ACCEL_ESP_KEYMAT_AES_GCM , -}; -struct aes_gcm_keymat {u64 seq_iv;
@@ -73,21 +68,9 @@ struct aes_gcm_keymat {struct mlx5_accel_esp_xfrm_attrs {enum mlx5_accel_esp_action action;u32 esn; -__be32 spi; -u32 seq; -u32 tfc_pad; +u32 spi;u32 flags; -u32 sa_handle; -union { -struct { -u32 size; - - } bmp; - } replay; -enum mlx5_accel_esp_keymats keymat_type; -union { -struct aes_gcm_keymat aes_gcm; - } keymat; +struct aes_gcm_keymat aes_gcm;union {__be32 a4;
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +2 -2
@@ -356,8 +356,8 @@ static void setup_fte_common(struct mlx5_accel_esp_xfrm_attrs *attrs,/* SPI number */ MLX5_SET_TO_ONES (fte_match_param, spec->match_criteria, misc_parameters.outer_esp_spi); -MLX5_SET (fte_match_param, spec->match_value, misc_parameters.outer_esp_spi, -be32_to_cpu (attrs->spi)); +MLX5_SET (fte_match_param, spec->match_value, + misc_parameters.outer_esp_spi, attrs->spi);if (ip_version ==4 ) { memcpy(MLX5_ADDR_OF(fte_match_param, spec->match_value,
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c +2 -2
@@ -50,7 +50,7 @@ static int mlx5_create_ipsec_obj(struct mlx5e_ipsec_sa_entry *sa_entry) {struct mlx5_accel_esp_xfrm_attrs *attrs = &sa_entry->attrs;struct mlx5_core_dev *mdev =mlx5e_ipsec_sa2dev (sa_entry); -struct aes_gcm_keymat *aes_gcm = &attrs->keymat.aes_gcm; +struct aes_gcm_keymat *aes_gcm = &attrs->aes_gcm;u32 out[MLX5_ST_SZ_DW (general_obj_out_cmd_hdr)];u32 in[MLX5_ST_SZ_DW (create_ipsec_obj_in)] = {};void *obj, *salt_p, *salt_iv_p;
@@ -106,7 +106,7 @@ static void mlx5_destroy_ipsec_obj(struct mlx5e_ipsec_sa_entry *sa_entry)int mlx5_ipsec_create_sa_ctx (struct mlx5e_ipsec_sa_entry *sa_entry) { -struct aes_gcm_keymat *aes_gcm = &sa_entry->attrs.keymat.aes_gcm; +struct aes_gcm_keymat *aes_gcm = &sa_entry->attrs.aes_gcm;struct mlx5_core_dev *mdev =mlx5e_ipsec_sa2dev (sa_entry);int err;
Signature · 1 line
2.35.1
Saeed Mahameed
maintainerRe: [PATCH net-next v1 15/17] net/mlx5: Cleanup XFRM attributes struct
Quoted 75 lines · Leon Romanovsky
From: Leon Romanovsky <[email protected]>
Remove everything that is not used or from
mlx5_accel_esp_xfrm_attrs, together with change type of spi to store proper type from the beginning.Reviewed-by Raed Salem <[email protected]>Signed-off-by Leon Romanovsky <[email protected]>---
.../mellanox/mlx5/core/en_accel/ipsec.c | 10 ++------- .../mellanox/mlx5/core/en_accel/ipsec.h | 21 ++----------------- .../mellanox/mlx5/core/en_accel/ipsec_fs.c | 4 ++-- .../mlx5/core/en_accel/ipsec_offload.c | 4 ++-- 4 files changed, 8 insertions(+), 31 deletions(-)drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +2
-8@@ -137,7 +137,7 @@ mlx5e_ipsec_build_accel_xfrm_attrs(struct mlx5e_ipsec_sa_entry *sa_entry, struct mlx5_accel_esp_xfrm_attrs *attrs) {struct xfrm_state *x = sa_entry->x; -struct aes_gcm_keymat *aes_gcm = &attrs->keymat.aes_gcm; +struct aes_gcm_keymat *aes_gcm = &attrs->aes_gcm;struct aead_geniv_ctx *geniv_ctx;struct crypto_aead *aead;unsigned int crypto_data_len, key_len;@@ -171,12 +171,6 @@ mlx5e_ipsec_build_accel_xfrm_attrs(struct mlx5e_ipsec_sa_entry *sa_entry, attrs->flags |=MLX5_ACCEL_ESP_FLAGS_ESN_STATE_OVERLAP ; } -/* rx handle */ - attrs->sa_handle = sa_entry->handle; - -/* algo type */ - attrs->keymat_type =MLX5_ACCEL_ESP_KEYMAT_AES_GCM ; -/* action */ attrs->action = (!(x->xso.flags &XFRM_OFFLOAD_INBOUND )) ? MLX5_ACCEL_ESP_ACTION_ENCRYPT :@@ -187,7 +181,7 @@ mlx5e_ipsec_build_accel_xfrm_attrs(struct mlx5e_ipsec_sa_entry *sa_entry,MLX5_ACCEL_ESP_FLAGS_TUNNEL ;/* spi */ - attrs->spi = x->id.spi; + attrs->spi =be32_to_cpu (x->id.spi);/* source , destination ips */ memcpy (&attrs->saddr, x->props.saddr.a6,sizeof (attrs->saddr));drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h +1
-9@@ -55,11 +55,6 @@ enum mlx5_accel_esp_action { MLX5_ACCEL_ESP_ACTION_ENCRYPT, }; -enum mlx5_accel_esp_keymats { -MLX5_ACCEL_ESP_KEYMAT_AES_NONE , -MLX5_ACCEL_ESP_KEYMAT_AES_GCM , -}; -struct aes_gcm_keymat {u64 seq_iv;@@ -73,21 +68,9 @@ struct aes_gcm_keymat {struct mlx5_accel_esp_xfrm_attrs {enum mlx5_accel_esp_action action;u32 esn; -__be32 spi; -u32 seq; -u32 tfc_pad; +u32 spi;u32 flags; -u32 sa_handle;
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h struct aes_gcm_keymat { · line 90
Quoted 10 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h +0
-10-union { -struct { -u32 size; - - } bmp; - } replay; -enum mlx5_accel_esp_keymats keymat_type; -union { -struct aes_gcm_keymat aes_gcm; - } keymat;
Why do we have so many unused fields ? are these leftovers from FPGA ipsec ?
Quoted 44 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h +1
-0+struct aes_gcm_keymat aes_gcm;union {__be32 a4;drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +2
-2@@ -356,8 +356,8 @@ static void setup_fte_common(struct mlx5_accel_esp_xfrm_attrs *attrs,/* SPI number */ MLX5_SET_TO_ONES (fte_match_param, spec->match_criteria, misc_parameters.outer_esp_spi); -MLX5_SET (fte_match_param, spec->match_value, misc_parameters.outer_esp_spi, -be32_to_cpu (attrs->spi)); +MLX5_SET (fte_match_param, spec->match_value, + misc_parameters.outer_esp_spi, attrs->spi);if (ip_version ==4 ) { memcpy(MLX5_ADDR_OF(fte_match_param, spec->match_value,drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c +2
-2@@ -50,7 +50,7 @@ static int mlx5_create_ipsec_obj(struct mlx5e_ipsec_sa_entry *sa_entry) {struct mlx5_accel_esp_xfrm_attrs *attrs = &sa_entry->attrs;struct mlx5_core_dev *mdev =mlx5e_ipsec_sa2dev (sa_entry); -struct aes_gcm_keymat *aes_gcm = &attrs->keymat.aes_gcm; +struct aes_gcm_keymat *aes_gcm = &attrs->aes_gcm;u32 out[MLX5_ST_SZ_DW (general_obj_out_cmd_hdr)];u32 in[MLX5_ST_SZ_DW (create_ipsec_obj_in)] = {};void *obj, *salt_p, *salt_iv_p;@@ -106,7 +106,7 @@ static void mlx5_destroy_ipsec_obj(struct mlx5e_ipsec_sa_entry *sa_entry)int mlx5_ipsec_create_sa_ctx (struct mlx5e_ipsec_sa_entry *sa_entry) { -struct aes_gcm_keymat *aes_gcm = &sa_entry->attrs.keymat.aes_gcm; +struct aes_gcm_keymat *aes_gcm = &sa_entry->attrs.aes_gcm;struct mlx5_core_dev *mdev =mlx5e_ipsec_sa2dev (sa_entry);int err;Signature · 1 line
2.35.1
Re: [PATCH net-next v1 15/17] net/mlx5: Cleanup XFRM attributes struct
Quoted 80 lines · Saeed Mahameed
Quoted 79 lines · Leon Romanovsky
From: Leon Romanovsky <[email protected]>
Remove everything that is not used or from
mlx5_accel_esp_xfrm_attrs, together with change type of spi to store proper type from the beginning.Reviewed-by Raed Salem <[email protected]>Signed-off-by Leon Romanovsky <[email protected]>---
.../mellanox/mlx5/core/en_accel/ipsec.c | 10 ++------- .../mellanox/mlx5/core/en_accel/ipsec.h | 21 ++----------------- .../mellanox/mlx5/core/en_accel/ipsec_fs.c | 4 ++-- .../mlx5/core/en_accel/ipsec_offload.c | 4 ++-- 4 files changed, 8 insertions(+), 31 deletions(-)drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +2
-8@@ -137,7 +137,7 @@ mlx5e_ipsec_build_accel_xfrm_attrs(struct mlx5e_ipsec_sa_entry *sa_entry, struct mlx5_accel_esp_xfrm_attrs *attrs) {struct xfrm_state *x = sa_entry->x; -struct aes_gcm_keymat *aes_gcm = &attrs->keymat.aes_gcm; +struct aes_gcm_keymat *aes_gcm = &attrs->aes_gcm;struct aead_geniv_ctx *geniv_ctx;struct crypto_aead *aead;unsigned int crypto_data_len, key_len;@@ -171,12 +171,6 @@ mlx5e_ipsec_build_accel_xfrm_attrs(struct mlx5e_ipsec_sa_entry *sa_entry, attrs->flags |=MLX5_ACCEL_ESP_FLAGS_ESN_STATE_OVERLAP ; } -/* rx handle */ - attrs->sa_handle = sa_entry->handle; - -/* algo type */ - attrs->keymat_type =MLX5_ACCEL_ESP_KEYMAT_AES_GCM ; -/* action */ attrs->action = (!(x->xso.flags &XFRM_OFFLOAD_INBOUND )) ? MLX5_ACCEL_ESP_ACTION_ENCRYPT :@@ -187,7 +181,7 @@ mlx5e_ipsec_build_accel_xfrm_attrs(struct mlx5e_ipsec_sa_entry *sa_entry,MLX5_ACCEL_ESP_FLAGS_TUNNEL ;/* spi */ - attrs->spi = x->id.spi; + attrs->spi =be32_to_cpu (x->id.spi);/* source , destination ips */ memcpy (&attrs->saddr, x->props.saddr.a6,sizeof (attrs->saddr));drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h +1
-13@@ -55,11 +55,6 @@ enum mlx5_accel_esp_action { MLX5_ACCEL_ESP_ACTION_ENCRYPT, }; -enum mlx5_accel_esp_keymats { -MLX5_ACCEL_ESP_KEYMAT_AES_NONE , -MLX5_ACCEL_ESP_KEYMAT_AES_GCM , -}; -struct aes_gcm_keymat {u64 seq_iv;@@ -73,21 +68,9 @@ struct aes_gcm_keymat {struct mlx5_accel_esp_xfrm_attrs {enum mlx5_accel_esp_action action;u32 esn; - __be32 spi; - u32 seq; - u32 tfc_pad; +u32 spi;u32 flags; - u32 sa_handle; - union { - struct { - u32 size; -
Quoted 8 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h struct aes_gcm_keymat { · line 90
Quoted 6 lines
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h +0
-6- } bmp; - } replay; -enum mlx5_accel_esp_keymats keymat_type; -union { -struct aes_gcm_keymat aes_gcm; - } keymat;Why do we have so many unused fields ? are these leftovers from FPGA ipsec ?
It is combination of leftovers and extra layering.
Thanks
PATCH 16/17 net/mlx5: Allow future addition of IPsec object modifiers
In reply to Leon RomanovskyFrom: Leon Romanovsky <[email protected]>
Currently, all released FW versions support only two IPsec object modifiers, and modify_field_select get and set same value with proper bits.
However, it is not future compatible, as new FW can have more modifiers and "default" will cause to overwrite not-changed fields.
Fix it by setting explicitly fields that need to be overwritten.
--- .../net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c +3 -0
@@ -179,6 +179,9 @@ static int mlx5_modify_ipsec_obj(struct mlx5e_ipsec_sa_entry *sa_entry,return -EOPNOTSUPP ; obj =MLX5_ADDR_OF (modify_ipsec_obj_in, in, ipsec_object); +MLX5_SET64 (ipsec_obj, obj, modify_field_select, +MLX5_MODIFY_IPSEC_BITMASK_ESN_OVERLAP | +MLX5_MODIFY_IPSEC_BITMASK_ESN_MSB );MLX5_SET (ipsec_obj, obj, esn_msb, attrs->esn);if (attrs->flags &MLX5_ACCEL_ESP_FLAGS_ESN_STATE_OVERLAP )MLX5_SET (ipsec_obj, obj, esn_overlap,1 );
Signature · 1 line
2.35.1
Saeed Mahameed
maintainerRe: [PATCH net-next v1 16/17] net/mlx5: Allow future addition of IPsec object modifiers
Quoted 6 lines · Leon Romanovsky
From: Leon Romanovsky <[email protected]>
Currently, all released FW versions support only two IPsec object modifiers, and
modify_field_selectget and set same value with proper bits.
Quoted 6 lines
However, it is not future compatible, as new FW can have more modifiers and "default" will cause to overwrite not-changed fields.
Fix it by setting explicitly fields that need to be overwritten.
Fixes 7ed92f97a1ad ("net/mlx5e: IPsec: Add Connect-X IPsec ESN update offload support")
Will apply this to net-mlx5 and send this to net.
Quoted 24 lines
Signed-off-by Huy Nguyen <[email protected]>Reviewed-by Raed Salem <[email protected]>Signed-off-by Leon Romanovsky <[email protected]>---
.../net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c| 3 +++1 file changed, 3 insertions(+)drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.c +3
-0@@ -179,6 +179,9 @@ static int mlx5_modify_ipsec_obj(struct mlx5e_ipsec_sa_entry *sa_entry,return -EOPNOTSUPP ; obj =MLX5_ADDR_OF (modify_ipsec_obj_in, in, ipsec_object); +MLX5_SET64 (ipsec_obj, obj, modify_field_select, +MLX5_MODIFY_IPSEC_BITMASK_ESN_OVERLAP | +MLX5_MODIFY_IPSEC_BITMASK_ESN_MSB );MLX5_SET (ipsec_obj, obj, esn_msb, attrs->esn);if (attrs->flags &MLX5_ACCEL_ESP_FLAGS_ESN_STATE_OVERLAP )MLX5_SET (ipsec_obj, obj, esn_overlap,1 );Signature · 1 line
2.35.1
PATCH 17/17 net/mlx5: Don't perform lookup after already known sec_path
In reply to Leon RomanovskyFrom: Leon Romanovsky <[email protected]>
There is no need to perform extra lookup in order to get already known sec_path that was set a couple of lines above. Simply reuse it.
--- drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c | 1 - 1 file changed, 1 deletion(-)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c +0 -1
@@ -332,7 +332,6 @@ void mlx5e_ipsec_offload_handle_rx_skb(struct net_device *netdev,return ; } - sp =skb_sec_path (skb); sp->xvec[sp->len++] = xs; sp->olen++;
Signature · 1 line
2.35.1
Leon Romanovsky
maintainerQuoted 9 lines · Leon Romanovsky
From: Leon Romanovsky <[email protected]>
Changelog:
v1:
* changed target from mlx5-next to net-next.
* Improved commit message in patch #1
* Left function names intact, with _accel_ word in it.
v0: https://lore.kernel.org/all/[email protected]
Quoted 6 lines
--------------------
After FPGA IPsec removal, we can go further and make sure that flow steering logic is aligned tomlx5_corestandard together with deep cleaning of whole IPsec path.Thanks
Hi,
I see that this series is marked as "Awaiting Upstream" in patchworks.
https://patchwork.kernel.org/project/netdevbpf/list/?series=633295&state=*
What does it mean? Can you please apply it directly to the netdev tree?
Thanks
Quoted 42 lines
Leon Romanovsky (17):
net/mlx5: Simplify IPsec flow steering init/cleanup functions net/mlx5: Check IPsec TX flow steering namespace in advance net/mlx5: Don't hide fallback to software IPsec in FS code net/mlx5: Reduce useless indirection in IPsec FS add/delete flows net/mlx5: Store IPsec ESN update work in XFRM state net/mlx5: Remove useless validity check
net/mlx5: Merge various control path IPsec headers into one file net/mlx5: Remove indirections from esp functions net/mlx5: Simplify HW context interfaces by using SA entry net/mlx5: Clean IPsec FS add/delete rules
net/mlx5: Make sure that no dangling IPsec FS pointers exist net/mlx5: Don't advertise IPsec netdev support for non-IPsec device net/mlx5: Simplify IPsec capabilities logic
net/mlx5: Remove not-supported ICV length
net/mlx5: Cleanup XFRM attributes struct
net/mlx5: Allow future addition of IPsec object modifiers net/mlx5: Don't perform lookup after already knownsec_path.../net/ethernet/mellanox/mlx5/core/en/fs.h | 1 - .../ethernet/mellanox/mlx5/core/en/params.c | 2 +- .../mellanox/mlx5/core/en_accel/ipsec.c | 174 +++------ .../mellanox/mlx5/core/en_accel/ipsec.h | 85 +++- .../mellanox/mlx5/core/en_accel/ipsec_fs.c | 362 ++++++------------ .../mellanox/mlx5/core/en_accel/ipsec_fs.h | 4 +- .../mlx5/core/en_accel/ipsec_offload.c | 331 +++------------- .../mlx5/core/en_accel/ipsec_offload.h | 14 - .../mellanox/mlx5/core/en_accel/ipsec_rxtx.c | 6 +- .../mellanox/mlx5/core/en_accel/ipsec_stats.c | 4 +- .../net/ethernet/mellanox/mlx5/core/en_main.c | 1 - .../net/ethernet/mellanox/mlx5/core/en_rx.c | 2 +- .../net/ethernet/mellanox/mlx5/core/main.c | 2 +- include/linux/mlx5/accel.h | 153 -------- include/linux/mlx5/mlx5_ifc.h | 2 - 15 files changed, 320 insertions(+), 823 deletions(-) delete mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_offload.h delete mode 100644 include/linux/mlx5/accel.hSignature · 1 line
2.35.1
Saeed Mahameed
maintainerQuoted 15 lines · Leon Romanovsky
Quoted 14 lines · Leon Romanovsky
From: Leon Romanovsky <[email protected]>
Changelog:
v1:
* changed target from mlx5-next to net-next.
* Improved commit message in patch #1
* Left function names intact, with _accel_ word in it.
v0: https://lore.kernel.org/all/[email protected]--------------------
After FPGA IPsec removal, we can go further and make sure that flow steering logic is aligned tomlx5_corestandard together with deep cleaning of whole IPsec path.
Quoted 7 lines
Quoted 1 line
Thanks
Hi,
I see that this series is marked as "Awaiting Upstream" in patchworks.
https://patchwork.kernel.org/project/netdevbpf/list/?series=633295&state=*
What does it mean? Can you please apply it directly to the netdev tree?
It's waiting for me to apply to net-next-mlx5, Please give me a chance to review and apply, i just got back from a long time off and i have a long backlog, i will provide feedback/apply by end of day.
Thanks,
Saeed.