Subject: Re: [PATCH net-next v5 11/25] ovpn: implement basic RX path (UDP)

From: Sabrina Dubroca <[email-masked]>
Date: Mon, 8 Jul 2024 18:11:02 +0200

2024-06-27, 15:08:29 +0200, Antonio Quartulli wrote:
> +static void ovpn_netdev_write(struct ovpn_peer *peer, struct sk_buff *skb)
> +{
[...]
> +	/* cause packet to be "received" by the interface */
> +	if (likely(gro_cells_receive(&peer->ovpn->gro_cells,
> +				     skb) == NET_RX_SUCCESS))
> +		/* update RX stats with the size of decrypted packet */
> +		dev_sw_netstats_rx_add(peer->ovpn->dev, skb->len);
> +	else
> +		dev_core_stats_rx_dropped_inc(peer->ovpn->dev);

Not needed AFAICT, gro_cells_receive already does
dev_core_stats_rx_dropped_inc(skb->dev) when it drops the packet.

> +}
> +
> +static void ovpn_decrypt_post(struct sk_buff *skb, int ret)
> +{
> +	struct ovpn_peer *peer = ovpn_skb_cb(skb)->peer;
> +
> +	if (unlikely(ret < 0))
> +		goto drop;
> +
> +	ovpn_netdev_write(peer, skb);
> +	/* skb is passed to upper layer - don't free it */
> +	skb = NULL;
> +drop:

I really find this "common" return code confusing. The only thing the
two cases have in common is dropping the peer reference.

> +	if (unlikely(skb))
> +		dev_core_stats_rx_dropped_inc(peer->ovpn->dev);
> +	kfree_skb(skb);
> +	ovpn_peer_put(peer);
> +}

-- 
Sabrina