| Message ID | 20240627130843.21042-24-antonio@openvpn.net (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Introducing OpenVPN Data Channel Offload | expand |
2024-06-27, 15:08:41 +0200, Antonio Quartulli wrote: > diff --git a/drivers/net/ovpn/peer.c b/drivers/net/ovpn/peer.c > index 2105bcc981fa..23418204fa8e 100644 > --- a/drivers/net/ovpn/peer.c > +++ b/drivers/net/ovpn/peer.c > @@ -273,6 +273,7 @@ void ovpn_peer_release_kref(struct kref *kref) > > ovpn_peer_release(peer); > netdev_put(peer->ovpn->dev, NULL); I don't think you can access peer->ovpn once you release that reference, if this peer was holding the last reference the netdev (and the ovpn struct) could be gone while we're accessing it in ovpn_nl_notify_del_peer. > + ovpn_nl_notify_del_peer(peer); > kfree_rcu(peer, rcu); > } > > diff --git a/drivers/net/ovpn/peer.h b/drivers/net/ovpn/peer.h > index 8d24a8fdd03e..971603a70090 100644 > --- a/drivers/net/ovpn/peer.h > +++ b/drivers/net/ovpn/peer.h > @@ -129,6 +129,7 @@ static inline bool ovpn_peer_hold(struct ovpn_peer *peer) > > void ovpn_peer_release(struct ovpn_peer *peer); > void ovpn_peer_release_kref(struct kref *kref); > +void ovpn_peer_release(struct ovpn_peer *peer); nit: dupe of 2 lines before
Hi, On 17/07/2024 12:54, Sabrina Dubroca wrote: > 2024-06-27, 15:08:41 +0200, Antonio Quartulli wrote: >> diff --git a/drivers/net/ovpn/peer.c b/drivers/net/ovpn/peer.c >> index 2105bcc981fa..23418204fa8e 100644 >> --- a/drivers/net/ovpn/peer.c >> +++ b/drivers/net/ovpn/peer.c >> @@ -273,6 +273,7 @@ void ovpn_peer_release_kref(struct kref *kref) >> >> ovpn_peer_release(peer); >> netdev_put(peer->ovpn->dev, NULL); > > I don't think you can access peer->ovpn once you release that > reference, if this peer was holding the last reference the netdev (and > the ovpn struct) could be gone while we're accessing it in > ovpn_nl_notify_del_peer. I see what you mean. I will move notify_del_peer up above, before the release > >> + ovpn_nl_notify_del_peer(peer); >> kfree_rcu(peer, rcu); >> } >> >> diff --git a/drivers/net/ovpn/peer.h b/drivers/net/ovpn/peer.h >> index 8d24a8fdd03e..971603a70090 100644 >> --- a/drivers/net/ovpn/peer.h >> +++ b/drivers/net/ovpn/peer.h >> @@ -129,6 +129,7 @@ static inline bool ovpn_peer_hold(struct ovpn_peer *peer) >> >> void ovpn_peer_release(struct ovpn_peer *peer); >> void ovpn_peer_release_kref(struct kref *kref); >> +void ovpn_peer_release(struct ovpn_peer *peer); > > nit: dupe of 2 lines before ops Thanks! >
diff --git a/drivers/net/ovpn/netlink.c b/drivers/net/ovpn/netlink.c index e43bbc9ad5d2..e920bb071750 100644 --- a/drivers/net/ovpn/netlink.c +++ b/drivers/net/ovpn/netlink.c @@ -846,6 +846,55 @@ int ovpn_nl_del_key_doit(struct sk_buff *skb, struct genl_info *info) return 0; } +int ovpn_nl_notify_del_peer(struct ovpn_peer *peer) +{ + struct sk_buff *msg; + struct nlattr *attr; + int ret = -EMSGSIZE; + void *hdr; + + netdev_info(peer->ovpn->dev, "deleting peer with id %u, reason %d\n", + peer->id, peer->delete_reason); + + msg = nlmsg_new(100, GFP_ATOMIC); + if (!msg) + return -ENOMEM; + + hdr = genlmsg_put(msg, 0, 0, &ovpn_nl_family, 0, OVPN_CMD_DEL_PEER); + if (!hdr) { + ret = -ENOBUFS; + goto err_free_msg; + } + + if (nla_put_u32(msg, OVPN_A_IFINDEX, peer->ovpn->dev->ifindex)) + goto err_cancel_msg; + + attr = nla_nest_start(msg, OVPN_A_PEER); + if (!attr) + goto err_cancel_msg; + + if (nla_put_u8(msg, OVPN_A_PEER_DEL_REASON, peer->delete_reason)) + goto err_cancel_msg; + + if (nla_put_u32(msg, OVPN_A_PEER_ID, peer->id)) + goto err_cancel_msg; + + nla_nest_end(msg, attr); + + genlmsg_end(msg, hdr); + + genlmsg_multicast_netns(&ovpn_nl_family, dev_net(peer->ovpn->dev), msg, + 0, OVPN_NLGRP_PEERS, GFP_ATOMIC); + + return 0; + +err_cancel_msg: + genlmsg_cancel(msg, hdr); +err_free_msg: + nlmsg_free(msg); + return ret; +} + int ovpn_nl_notify_swap_keys(struct ovpn_peer *peer) { struct sk_buff *msg; diff --git a/drivers/net/ovpn/netlink.h b/drivers/net/ovpn/netlink.h index c86cd102eeef..8ce58c4ee193 100644 --- a/drivers/net/ovpn/netlink.h +++ b/drivers/net/ovpn/netlink.h @@ -12,6 +12,14 @@ int ovpn_nl_register(void); void ovpn_nl_unregister(void); +/** + * ovpn_nl_notify_del_peer - notify userspace about peer being deleted + * @peer: the peer being deleted + * + * Return: 0 on success or a negative error code otherwise + */ +int ovpn_nl_notify_del_peer(struct ovpn_peer *peer); + /** * ovpn_nl_notify_swap_keys - notify userspace peer's key must be renewed * @peer: the peer whose key needs to be renewed diff --git a/drivers/net/ovpn/peer.c b/drivers/net/ovpn/peer.c index 2105bcc981fa..23418204fa8e 100644 --- a/drivers/net/ovpn/peer.c +++ b/drivers/net/ovpn/peer.c @@ -273,6 +273,7 @@ void ovpn_peer_release_kref(struct kref *kref) ovpn_peer_release(peer); netdev_put(peer->ovpn->dev, NULL); + ovpn_nl_notify_del_peer(peer); kfree_rcu(peer, rcu); } diff --git a/drivers/net/ovpn/peer.h b/drivers/net/ovpn/peer.h index 8d24a8fdd03e..971603a70090 100644 --- a/drivers/net/ovpn/peer.h +++ b/drivers/net/ovpn/peer.h @@ -129,6 +129,7 @@ static inline bool ovpn_peer_hold(struct ovpn_peer *peer) void ovpn_peer_release(struct ovpn_peer *peer); void ovpn_peer_release_kref(struct kref *kref); +void ovpn_peer_release(struct ovpn_peer *peer); /** * ovpn_peer_put - decrease reference counter
Whenever a peer is deleted, send a notification to userspace so that it can react accordingly. This is most important when a peer is deleted due to ping timeout, because it all happens in kernelspace and thus userspace has no direct way to learn about it. Signed-off-by: Antonio Quartulli <antonio@openvpn.net> --- drivers/net/ovpn/netlink.c | 49 ++++++++++++++++++++++++++++++++++++++ drivers/net/ovpn/netlink.h | 8 +++++++ drivers/net/ovpn/peer.c | 1 + drivers/net/ovpn/peer.h | 1 + 4 files changed, 59 insertions(+)