Re: [PATCH net-next v18 17/25] ovpn: implement keepalive mechanism

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,

...

+}