Commit 9efd5152 authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

Merge branch 'net-convert-to-skb_dstref_steal-and-skb_dstref_restore'

Stanislav Fomichev says:

====================
net: Convert to skb_dstref_steal and skb_dstref_restore

To diagnose and prevent issues similar to [0], emit warning
(CONFIG_DEBUG_NET) from skb_dst_set and skb_dst_set_noref when
overwriting non-null reference-counted entry. Two new helpers
are added to handle special cases where the entry needs to be
reset and restored: skb_dstref_steal/skb_dstref_restore. The bulk of
the patches in the series converts manual _skb_refst manipulations
to these new helpers.

0: https://lore.kernel.org/netdev/20250723224625.1340224-1-sdf@fomichev.me/T/#u
====================

Link: https://patch.msgid.link/20250818154032.3173645-1-sdf@fomichev.me


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents 0e041220 a890348a
Loading
Loading
Loading
Loading
+5 −5
Original line number Diff line number Diff line
@@ -171,7 +171,7 @@ static void chtls_purge_receive_queue(struct sock *sk)
	struct sk_buff *skb;

	while ((skb = __skb_dequeue(&sk->sk_receive_queue)) != NULL) {
		skb_dst_set(skb, (void *)NULL);
		skb_dstref_steal(skb);
		kfree_skb(skb);
	}
}
@@ -194,7 +194,7 @@ static void chtls_purge_recv_queue(struct sock *sk)
	struct sk_buff *skb;

	while ((skb = __skb_dequeue(&tlsk->sk_recv_queue)) != NULL) {
		skb_dst_set(skb, NULL);
		skb_dstref_steal(skb);
		kfree_skb(skb);
	}
}
@@ -1734,7 +1734,7 @@ static int chtls_rx_data(struct chtls_dev *cdev, struct sk_buff *skb)
		pr_err("can't find conn. for hwtid %u.\n", hwtid);
		return -EINVAL;
	}
	skb_dst_set(skb, NULL);
	skb_dstref_steal(skb);
	process_cpl_msg(chtls_recv_data, sk, skb);
	return 0;
}
@@ -1786,7 +1786,7 @@ static int chtls_rx_pdu(struct chtls_dev *cdev, struct sk_buff *skb)
		pr_err("can't find conn. for hwtid %u.\n", hwtid);
		return -EINVAL;
	}
	skb_dst_set(skb, NULL);
	skb_dstref_steal(skb);
	process_cpl_msg(chtls_recv_pdu, sk, skb);
	return 0;
}
@@ -1855,7 +1855,7 @@ static int chtls_rx_cmp(struct chtls_dev *cdev, struct sk_buff *skb)
		pr_err("can't find conn. for hwtid %u.\n", hwtid);
		return -EINVAL;
	}
	skb_dst_set(skb, NULL);
	skb_dstref_steal(skb);
	process_cpl_msg(chtls_rx_hdr, sk, skb);

	return 0;
+2 −2
Original line number Diff line number Diff line
@@ -171,14 +171,14 @@ static inline void chtls_set_req_addr(struct request_sock *oreq,

static inline void chtls_free_skb(struct sock *sk, struct sk_buff *skb)
{
	skb_dst_set(skb, NULL);
	skb_dstref_steal(skb);
	__skb_unlink(skb, &sk->sk_receive_queue);
	__kfree_skb(skb);
}

static inline void chtls_kfree_skb(struct sock *sk, struct sk_buff *skb)
{
	skb_dst_set(skb, NULL);
	skb_dstref_steal(skb);
	__skb_unlink(skb, &sk->sk_receive_queue);
	kfree_skb(skb);
}
+1 −1
Original line number Diff line number Diff line
@@ -1434,7 +1434,7 @@ static int chtls_pt_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
		continue;
found_ok_skb:
		if (!skb->len) {
			skb_dst_set(skb, NULL);
			skb_dstref_steal(skb);
			__skb_unlink(skb, &sk->sk_receive_queue);
			kfree_skb(skb);

+1 −2
Original line number Diff line number Diff line
@@ -346,8 +346,7 @@ netdev_tx_t cvm_oct_xmit(struct sk_buff *skb, struct net_device *dev)
	 * The skbuff will be reused without ever being freed. We must
	 * cleanup a bunch of core things.
	 */
	dst_release(skb_dst(skb));
	skb_dst_set(skb, NULL);
	skb_dst_drop(skb);
	skb_ext_reset(skb);
	nf_reset_ct(skb);
	skb_reset_redirect(skb);
+41 −0
Original line number Diff line number Diff line
@@ -1159,6 +1159,45 @@ static inline struct dst_entry *skb_dst(const struct sk_buff *skb)
	return (struct dst_entry *)(skb->_skb_refdst & SKB_DST_PTRMASK);
}

static inline void skb_dst_check_unset(struct sk_buff *skb)
{
	DEBUG_NET_WARN_ON_ONCE((skb->_skb_refdst & SKB_DST_PTRMASK) &&
			       !(skb->_skb_refdst & SKB_DST_NOREF));
}

/**
 * skb_dstref_steal() - return current dst_entry value and clear it
 * @skb: buffer
 *
 * Resets skb dst_entry without adjusting its reference count. Useful in
 * cases where dst_entry needs to be temporarily reset and restored.
 * Note that the returned value cannot be used directly because it
 * might contain SKB_DST_NOREF bit.
 *
 * When in doubt, prefer skb_dst_drop() over skb_dstref_steal() to correctly
 * handle dst_entry reference counting.
 *
 * Returns: original skb dst_entry.
 */
static inline unsigned long skb_dstref_steal(struct sk_buff *skb)
{
	unsigned long refdst = skb->_skb_refdst;

	skb->_skb_refdst = 0;
	return refdst;
}

/**
 * skb_dstref_restore() - restore skb dst_entry removed via skb_dstref_steal()
 * @skb: buffer
 * @refdst: dst entry from a call to skb_dstref_steal()
 */
static inline void skb_dstref_restore(struct sk_buff *skb, unsigned long refdst)
{
	skb_dst_check_unset(skb);
	skb->_skb_refdst = refdst;
}

/**
 * skb_dst_set - sets skb dst
 * @skb: buffer
@@ -1169,6 +1208,7 @@ static inline struct dst_entry *skb_dst(const struct sk_buff *skb)
 */
static inline void skb_dst_set(struct sk_buff *skb, struct dst_entry *dst)
{
	skb_dst_check_unset(skb);
	skb->slow_gro |= !!dst;
	skb->_skb_refdst = (unsigned long)dst;
}
@@ -1185,6 +1225,7 @@ static inline void skb_dst_set(struct sk_buff *skb, struct dst_entry *dst)
 */
static inline void skb_dst_set_noref(struct sk_buff *skb, struct dst_entry *dst)
{
	skb_dst_check_unset(skb);
	WARN_ON(!rcu_read_lock_held() && !rcu_read_lock_bh_held());
	skb->slow_gro |= !!dst;
	skb->_skb_refdst = (unsigned long)dst | SKB_DST_NOREF;
Loading