[PATCH net-next 0/4] psp: make tx key ops optional for drivers

[PATCH net-next 0/4] psp: make tx key ops optional for drivers

[Daniel Zahka]

This is the first of two series which together implement rekeying PSP
protected tcp connections. Here are both series together on github:
https://github.com/danieldzahka/linux/commits/psp-rekey-split/

This first series is mostly non-functional changes, except for the minor
difference that netdevsim driver implements tx key ops. Its tx key ops
were basically NOPs, and in the future PSP core can subsume the assoc
counting that it was doing.

The purpose of the refactors is to make code reusable from paths that
will be added to the code in the rekeying series.

Patch 1 exists to setup two paths for tx key deletion, one where keys
are deleted as they are now in psp_assoc_free() when the assoc refcount
drops to 0, and a different path in series two, where the keys will get
put onto a queue for deferred deletion.

Patch 2 exists to allow tx assoc state to be set either from the
connection setup path, or in series two, from a tx rekeying path.

Patch 3 makes implementing tx_key_add and tx_key_del optional for
drivers. These callbacks only make sense for devices that utilize a tx
SADB. This distinction is important once rekeying is involved, because
an SADB implies a race between in flight tx packets and key deletion,
that does not exist for non-SADB implementations.

Patch 4 removes the unnecessary tx key ops in netdevsim.

No new tests are added to this series. There is no real user visible
change of behavior.

Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
---
Daniel Zahka (4):
      psp: refactor psp_dev_tx_key_del()
      psp: move code from psp_sock_assoc_set_tx() into helper functions
      psp: allow drivers to omit tx key add/del ops
      netdevsim: psp: drop tx key ops

 drivers/net/netdevsim/netdevsim.h |   1 -
 drivers/net/netdevsim/psp.c       |  33 -----------
 include/net/psp/functions.h       |   5 --
 include/net/psp/types.h           |   4 ++
 net/psp/psp.h                     |  12 ++++
 net/psp/psp_main.c                |  13 +++--
 net/psp/psp_sock.c                | 115 ++++++++++++++++++++++++--------------
 7 files changed, 97 insertions(+), 86 deletions(-)
---
base-commit: 6ebcf5074cff0402730c6981d2397139fee6322d
change-id: 20260903-psp-prep-8c6b0e4108e9

Best regards,
-- 
Daniel Zahka <daniel.zahka@gmail.com>

[PATCH net-next 1/4] psp: refactor psp_dev_tx_key_del()

[Daniel Zahka]

No functional changes.

Lift the list deletion and tx spi validation code into
callers. Deferred key deletion code paths will need similar checks
that a tx key needs removal from the underlying device, but will
diverge when it comes to list handling and when to call
psp_dev_ops::tx_key_del().

Also, move the predicate for tx key deletion into a helper for
readability and reuse in later patches.

Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
---
 net/psp/psp.h      |  6 ++++++
 net/psp/psp_main.c |  7 +++++--
 net/psp/psp_sock.c | 11 ++++++-----
 3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/net/psp/psp.h b/net/psp/psp.h
index 86eeba823ced..8acf9ca84b55 100644
--- a/net/psp/psp.h
+++ b/net/psp/psp.h
@@ -53,4 +53,10 @@ static inline bool psp_dev_is_registered(struct psp_dev *psd)
 	return !!psd->ops;
 }
 
+static inline bool psp_assoc_needs_tx_key_del(struct psp_assoc *pas)
+{
+	lockdep_assert_held(&pas->psd->lock);
+	return pas->tx.spi;
+}
+
 #endif /* __PSP_PSP_H */
diff --git a/net/psp/psp_main.c b/net/psp/psp_main.c
index c9c1a8826b7f..2556f0d46ef4 100644
--- a/net/psp/psp_main.c
+++ b/net/psp/psp_main.c
@@ -147,8 +147,11 @@ void psp_dev_unregister(struct psp_dev *psd)
 
 	list_splice_init(&psd->active_assocs, &psd->prev_assocs);
 	list_splice_init(&psd->prev_assocs, &psd->stale_assocs);
-	list_for_each_entry_safe(pas, next, &psd->stale_assocs, assocs_list)
-		psp_dev_tx_key_del(psd, pas);
+	list_for_each_entry_safe(pas, next, &psd->stale_assocs, assocs_list) {
+		if (psp_assoc_needs_tx_key_del(pas))
+			psp_dev_tx_key_del(psd, pas);
+		list_del(&pas->assocs_list);
+	}
 
 	list_for_each_entry_safe(entry, entry_tmp, &psd->assoc_dev_list,
 				 dev_list) {
diff --git a/net/psp/psp_sock.c b/net/psp/psp_sock.c
index 1a2a6b7516b0..045389671d7f 100644
--- a/net/psp/psp_sock.c
+++ b/net/psp/psp_sock.c
@@ -85,9 +85,7 @@ static int psp_dev_tx_key_add(struct psp_dev *psd, struct psp_assoc *pas,
 
 void psp_dev_tx_key_del(struct psp_dev *psd, struct psp_assoc *pas)
 {
-	if (pas->tx.spi)
-		psd->ops->tx_key_del(psd, pas);
-	list_del(&pas->assocs_list);
+	psd->ops->tx_key_del(psd, pas);
 }
 
 static void psp_assoc_free(struct work_struct *work)
@@ -96,8 +94,11 @@ static void psp_assoc_free(struct work_struct *work)
 	struct psp_dev *psd = pas->psd;
 
 	mutex_lock(&psd->lock);
-	if (psp_dev_is_registered(psd))
-		psp_dev_tx_key_del(psd, pas);
+	if (psp_dev_is_registered(psd)) {
+		if (psp_assoc_needs_tx_key_del(pas))
+			psp_dev_tx_key_del(psd, pas);
+		list_del(&pas->assocs_list);
+	}
 	mutex_unlock(&psd->lock);
 	psp_dev_put(psd);
 	kfree(pas);

-- 
2.52.0

[PATCH net-next 2/4] psp: move code from psp_sock_assoc_set_tx() into helper functions

[Daniel Zahka]

No functional changes.

Lift code that needs to be called from both initial tx establishment
and tx rekeying into functions that can be reused in both paths.

The plaintext in recv queue checks and mss adjustment only run on
initial tx keying. The dummy psp_assoc machinery will be used in both
paths.

psp_dev_tx_key_add() absorbs the dummy assoc machinery, as its main
purpose is to populate the device specific psp_assoc:drv_data.

psp_assoc_set_tx() exists so that a future change can allow
psp_dev_tx_key_add() to be skipped entirely for devices that don't use
an SADB, while copying of the tx spi and key into the assoc needs to
happen regardless of SADB vs. no SADB.

Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
---
 net/psp/psp_sock.c | 102 ++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 65 insertions(+), 37 deletions(-)

diff --git a/net/psp/psp_sock.c b/net/psp/psp_sock.c
index 045389671d7f..36eb06faa54a 100644
--- a/net/psp/psp_sock.c
+++ b/net/psp/psp_sock.c
@@ -78,9 +78,28 @@ static struct psp_assoc *psp_assoc_dummy(struct psp_assoc *pas)
 }
 
 static int psp_dev_tx_key_add(struct psp_dev *psd, struct psp_assoc *pas,
+			      struct psp_key_parsed *key,
 			      struct netlink_ext_ack *extack)
 {
-	return psd->ops->tx_key_add(psd, pas, extack);
+	struct psp_assoc *dummy;
+	int err;
+
+	/* Pass a fake association to drivers to make sure they don't
+	 * try to store pointers to it. For re-keying we'll need to
+	 * re-allocate the assoc structures.
+	 */
+	dummy = psp_assoc_dummy(pas);
+	if (!dummy)
+		return -ENOMEM;
+
+	memcpy(&dummy->tx, key, sizeof(*key));
+	err = psd->ops->tx_key_add(psd, dummy, extack);
+	if (!err)
+		memcpy(pas->drv_data, dummy->drv_data,
+		       psd->caps->assoc_drv_spc);
+
+	kfree(dummy);
+	return err;
 }
 
 void psp_dev_tx_key_del(struct psp_dev *psd, struct psp_assoc *pas)
@@ -156,6 +175,20 @@ int psp_sock_assoc_set_rx(struct sock *sk, struct psp_assoc *pas,
 	return err;
 }
 
+static int psp_assoc_set_tx(struct psp_dev *psd, struct psp_assoc *pas,
+			    struct psp_key_parsed *key,
+			    struct netlink_ext_ack *extack)
+{
+	int err;
+
+	err = psp_dev_tx_key_add(psd, pas, key, extack);
+	if (err)
+		return err;
+
+	memcpy(&pas->tx, key, sizeof(*key));
+	return 0;
+}
+
 static int psp_sock_recv_queue_check(struct sock *sk, struct psp_assoc *pas)
 {
 	struct psp_skb_ext *pse;
@@ -175,12 +208,40 @@ static int psp_sock_recv_queue_check(struct sock *sk, struct psp_assoc *pas)
 	return 0;
 }
 
+static int
+psp_sock_set_tx_key(struct sock *sk, struct psp_dev *psd, struct psp_assoc *pas,
+		    struct psp_key_parsed *key, struct netlink_ext_ack *extack)
+{
+	struct inet_connection_sock *icsk;
+	int err;
+
+	err = psp_sock_recv_queue_check(sk, pas);
+	if (err) {
+		NL_SET_ERR_MSG(extack,
+			       "Socket has incompatible segments already in the recv queue");
+		return err;
+	}
+
+	err = psp_assoc_set_tx(psd, pas, key, extack);
+	if (err)
+		return err;
+
+	WRITE_ONCE(sk->sk_validate_xmit_skb, psp_validate_xmit);
+	tcp_write_collapse_fence(sk);
+	pas->upgrade_seq = tcp_sk(sk)->rcv_nxt;
+
+	icsk = inet_csk(sk);
+	icsk->icsk_ext_hdr_len += psp_sk_overhead(sk);
+	icsk->icsk_sync_mss(sk, icsk->icsk_pmtu_cookie);
+
+	return err;
+}
+
 int psp_sock_assoc_set_tx(struct sock *sk, struct psp_dev *psd,
 			  u32 version, struct psp_key_parsed *key,
 			  struct netlink_ext_ack *extack)
 {
-	struct inet_connection_sock *icsk;
-	struct psp_assoc *pas, *dummy;
+	struct psp_assoc *pas;
 	int err;
 
 	lock_sock(sk);
@@ -208,40 +269,7 @@ int psp_sock_assoc_set_tx(struct sock *sk, struct psp_dev *psd,
 		goto exit_unlock;
 	}
 
-	err = psp_sock_recv_queue_check(sk, pas);
-	if (err) {
-		NL_SET_ERR_MSG(extack, "Socket has incompatible segments already in the recv queue");
-		goto exit_unlock;
-	}
-
-	/* Pass a fake association to drivers to make sure they don't
-	 * try to store pointers to it. For re-keying we'll need to
-	 * re-allocate the assoc structures.
-	 */
-	dummy = psp_assoc_dummy(pas);
-	if (!dummy) {
-		err = -ENOMEM;
-		goto exit_unlock;
-	}
-
-	memcpy(&dummy->tx, key, sizeof(*key));
-	err = psp_dev_tx_key_add(psd, dummy, extack);
-	if (err)
-		goto exit_free_dummy;
-
-	memcpy(pas->drv_data, dummy->drv_data, psd->caps->assoc_drv_spc);
-	memcpy(&pas->tx, key, sizeof(*key));
-
-	WRITE_ONCE(sk->sk_validate_xmit_skb, psp_validate_xmit);
-	tcp_write_collapse_fence(sk);
-	pas->upgrade_seq = tcp_sk(sk)->rcv_nxt;
-
-	icsk = inet_csk(sk);
-	icsk->icsk_ext_hdr_len += psp_sk_overhead(sk);
-	icsk->icsk_sync_mss(sk, icsk->icsk_pmtu_cookie);
-
-exit_free_dummy:
-	kfree(dummy);
+	err = psp_sock_set_tx_key(sk, psd, pas, key, extack);
 exit_unlock:
 	release_sock(sk);
 	return err;

-- 
2.52.0

[PATCH net-next 3/4] psp: allow drivers to omit tx key add/del ops

[Daniel Zahka]

Drivers that don't use an SADB for tx key storage don't have a use
for psp_dev_ops::tx_key_add and psp_dev_ops::tx_key_del.

Allowing drivers to leave these as NULL gives PSP core a simple way to
determine whether a driver utilizes an SADB, which in turn could
affect how PSP core chooses to handle certain situations.

For example:
- deciding if tx key deletion needs to be delayed during a rekeying
  event to avoid in-flight packets using old key handles.
- choosing whether or not to report device stats like SADB usage to
  userspace, which only make sense if the driver uses on-device key
  storage.

Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
---
 include/net/psp/types.h | 4 ++++
 net/psp/psp.h           | 8 +++++++-
 net/psp/psp_main.c      | 6 +++---
 net/psp/psp_sock.c      | 8 +++++---
 4 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/include/net/psp/types.h b/include/net/psp/types.h
index 87991a1ea02d..b8905efbd604 100644
--- a/include/net/psp/types.h
+++ b/include/net/psp/types.h
@@ -219,12 +219,16 @@ struct psp_dev_ops {
 	 * @tx_key_add: add a Tx key to the device
 	 * Install an association in the device. Core will allocate space
 	 * for the driver to use at drv_data.
+	 * Can be left NULL if device does not store Tx keys and @tx_key_del
+	 * is also NULL.
 	 */
 	int (*tx_key_add)(struct psp_dev *psd, struct psp_assoc *pas,
 			  struct netlink_ext_ack *extack);
 	/**
 	 * @tx_key_del: remove a Tx key from the device
 	 * Remove an association from the device.
+	 * Can be left NULL if device does not store Tx keys and @tx_key_add
+	 * is also NULL.
 	 */
 	void (*tx_key_del)(struct psp_dev *psd, struct psp_assoc *pas);
 
diff --git a/net/psp/psp.h b/net/psp/psp.h
index 8acf9ca84b55..bbb39e2f5b0a 100644
--- a/net/psp/psp.h
+++ b/net/psp/psp.h
@@ -53,10 +53,16 @@ static inline bool psp_dev_is_registered(struct psp_dev *psd)
 	return !!psd->ops;
 }
 
+static inline bool psp_dev_has_sadb(struct psp_dev *psd)
+{
+	lockdep_assert_held(&psd->lock);
+	return !!psd->ops->tx_key_del;
+}
+
 static inline bool psp_assoc_needs_tx_key_del(struct psp_assoc *pas)
 {
 	lockdep_assert_held(&pas->psd->lock);
-	return pas->tx.spi;
+	return psp_dev_has_sadb(pas->psd) && pas->tx.spi;
 }
 
 #endif /* __PSP_PSP_H */
diff --git a/net/psp/psp_main.c b/net/psp/psp_main.c
index 2556f0d46ef4..91473f96ad21 100644
--- a/net/psp/psp_main.c
+++ b/net/psp/psp_main.c
@@ -68,9 +68,9 @@ psp_dev_create(struct net_device *netdev,
 		    !psd_ops->set_config ||
 		    !psd_ops->key_rotate ||
 		    !psd_ops->rx_spi_alloc ||
-		    !psd_ops->tx_key_add ||
-		    !psd_ops->tx_key_del ||
-		    !psd_ops->get_stats))
+		    !psd_ops->get_stats ||
+		    (!psd_ops->tx_key_add != !psd_ops->tx_key_del) ||
+		    (psd_caps->assoc_drv_spc && !psd_ops->tx_key_add)))
 		return ERR_PTR(-EINVAL);
 
 	psd = kzalloc_obj(*psd);
diff --git a/net/psp/psp_sock.c b/net/psp/psp_sock.c
index 36eb06faa54a..6a4becc38b55 100644
--- a/net/psp/psp_sock.c
+++ b/net/psp/psp_sock.c
@@ -181,9 +181,11 @@ static int psp_assoc_set_tx(struct psp_dev *psd, struct psp_assoc *pas,
 {
 	int err;
 
-	err = psp_dev_tx_key_add(psd, pas, key, extack);
-	if (err)
-		return err;
+	if (psp_dev_has_sadb(psd)) {
+		err = psp_dev_tx_key_add(psd, pas, key, extack);
+		if (err)
+			return err;
+	}
 
 	memcpy(&pas->tx, key, sizeof(*key));
 	return 0;

-- 
2.52.0

[PATCH net-next 4/4] netdevsim: psp: drop tx key ops

[Daniel Zahka]

netdevsim has no SADB. The usage of psp_assoc_drv_data() was always
obsolete given psp_validate_xmit(), so we can remove nsim_assoc_add()
and nsim_assoc_del(). netdevsim::assoc_cnt can also be removed. PSP core
can track the balance of tx_key_add vs tx_key_del calls in a future
change.

Delete psp_assoc_drv_data() because there are no more callers left in
the tree. mlx5 accesses pas->drv_data directly.

Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
---
 drivers/net/netdevsim/netdevsim.h |  1 -
 drivers/net/netdevsim/psp.c       | 33 ---------------------------------
 include/net/psp/functions.h       |  5 -----
 3 files changed, 39 deletions(-)

diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 55aec41237b9..181b6baaba7a 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -122,7 +122,6 @@ struct netdevsim {
 		struct dentry *rereg;
 		struct mutex rereg_lock;
 		u32 spi;
-		u32 assoc_cnt;
 	} psp;
 
 	struct nsim_bus_dev *nsim_bus_dev;
diff --git a/drivers/net/netdevsim/psp.c b/drivers/net/netdevsim/psp.c
index 6b3532b5e360..b7452c49b581 100644
--- a/drivers/net/netdevsim/psp.c
+++ b/drivers/net/netdevsim/psp.c
@@ -23,7 +23,6 @@ nsim_do_psp(struct sk_buff *skb, struct netdevsim *ns,
 	struct psp_assoc *pas;
 	struct net *net;
 	int psp_len;
-	void **ptr;
 
 	rcu_read_lock();
 	pas = psp_skb_get_assoc_rcu(skb);
@@ -37,12 +36,6 @@ nsim_do_psp(struct sk_buff *skb, struct netdevsim *ns,
 		goto out_unlock;
 	}
 
-	ptr = psp_assoc_drv_data(pas);
-	if (*ptr != ns) {
-		rc = SKB_DROP_REASON_PSP_OUTPUT;
-		goto out_unlock;
-	}
-
 	net = sock_net(skb->sk);
 	if (!psp_dev_encapsulate(net, skb, pas->tx.spi, pas->version, 0)) {
 		rc = SKB_DROP_REASON_PSP_OUTPUT;
@@ -149,19 +142,6 @@ nsim_rx_spi_alloc(struct psp_dev *psd, u32 version,
 	return 0;
 }
 
-static int nsim_assoc_add(struct psp_dev *psd, struct psp_assoc *pas,
-			  struct netlink_ext_ack *extack)
-{
-	struct netdevsim *ns = psd->drv_priv;
-	void **ptr = psp_assoc_drv_data(pas);
-
-	/* Copy drv_priv from psd to assoc */
-	*ptr = psd->drv_priv;
-	ns->psp.assoc_cnt++;
-
-	return 0;
-}
-
 static int nsim_key_rotate(struct psp_dev *psd, struct netlink_ext_ack *extack)
 {
 	struct netdevsim *ns = psd->drv_priv;
@@ -177,15 +157,6 @@ static int nsim_key_rotate(struct psp_dev *psd, struct netlink_ext_ack *extack)
 	return 0;
 }
 
-static void nsim_assoc_del(struct psp_dev *psd, struct psp_assoc *pas)
-{
-	struct netdevsim *ns = psd->drv_priv;
-	void **ptr = psp_assoc_drv_data(pas);
-
-	*ptr = NULL;
-	ns->psp.assoc_cnt--;
-}
-
 static void nsim_get_stats(struct psp_dev *psd, struct psp_dev_stats *stats)
 {
 	struct netdevsim *ns = psd->drv_priv;
@@ -204,8 +175,6 @@ static void nsim_get_stats(struct psp_dev *psd, struct psp_dev_stats *stats)
 static struct psp_dev_ops nsim_psp_ops = {
 	.set_config	= nsim_psp_set_config,
 	.rx_spi_alloc	= nsim_rx_spi_alloc,
-	.tx_key_add	= nsim_assoc_add,
-	.tx_key_del	= nsim_assoc_del,
 	.key_rotate	= nsim_key_rotate,
 	.get_stats	= nsim_get_stats,
 };
@@ -215,7 +184,6 @@ static struct psp_dev_caps nsim_psp_caps = {
 		    1 << PSP_VERSION_HDR0_AES_GMAC_128 |
 		    1 << PSP_VERSION_HDR0_AES_GCM_256 |
 		    1 << PSP_VERSION_HDR0_AES_GMAC_256,
-	.assoc_drv_spc = sizeof(void *),
 };
 
 static void __nsim_psp_uninit(struct netdevsim *ns, bool teardown)
@@ -230,7 +198,6 @@ static void __nsim_psp_uninit(struct netdevsim *ns, bool teardown)
 		synchronize_rcu();
 		psp_dev_unregister(psd);
 	}
-	WARN_ON(ns->psp.assoc_cnt);
 }
 
 void nsim_psp_uninit(struct netdevsim *ns)
diff --git a/include/net/psp/functions.h b/include/net/psp/functions.h
index c5c23a54774e..b23c30898389 100644
--- a/include/net/psp/functions.h
+++ b/include/net/psp/functions.h
@@ -24,11 +24,6 @@ int psp_dev_rcv(struct sk_buff *skb, u16 dev_id, u8 generation, bool strip_icv);
 /* Kernel-facing API */
 void psp_assoc_put(struct psp_assoc *pas);
 
-static inline void *psp_assoc_drv_data(struct psp_assoc *pas)
-{
-	return pas->drv_data;
-}
-
 #if IS_ENABLED(CONFIG_INET_PSP)
 unsigned int psp_key_size(u32 version);
 void psp_sk_assoc_free(struct sock *sk);

-- 
2.52.0