On 03/02/2025 10:20, Sabrina Dubroca wrote:
2025-01-13, 10:31:36 +0100, Antonio Quartulli wrote:+void ovpn_xmit_special(struct ovpn_peer *peer, const void *data, + const unsigned int len) +{ + struct ovpn_priv *ovpn; + struct sk_buff *skb; + + ovpn = peer->ovpn; + if (unlikely(!ovpn)) + return; + + skb = alloc_skb(256 + len, GFP_ATOMIC); + if (unlikely(!skb)) + return; + + skb_reserve(skb, 128); + skb->priority = TC_PRIO_BESTEFFORT; + __skb_put_data(skb, data, len); + + /* increase reference counter when passing peer to sending queue */ + if (!ovpn_peer_hold(peer)) { + netdev_dbg(ovpn->dev, + "cannot hold peer reference for sending special packet\n"); + kfree_skb(skb); + return; + } + + ovpn_send(ovpn, skb, peer); +}[...]+static void ovpn_peer_keepalive_send(struct work_struct *work) +{ + struct ovpn_peer *peer = container_of(work, struct ovpn_peer, + keepalive_work); + + local_bh_disable(); + ovpn_xmit_special(peer, ovpn_keepalive_message, + sizeof(ovpn_keepalive_message)); + local_bh_enable(); + ovpn_peer_put(peer);nit: could we simply drop this put and the hold in ovpn_xmit_special? ovpn_peer_keepalive_send has a reference on the peer from the WQ, just transfer it to ovpn_xmit_special and then ovpn_send. No need to pretend to acquire one for ovpn_send's sake and drop the one we got from the WQ? (ovpn_xmit_special would need to _put(peer) in case of early return)
Ok, will drop both and add a comment to remember the casual reader that we already hold a reference (+ put() in error path).
Regards,
+}
-- Antonio Quartulli OpenVPN Inc.
