Re: [PATCH net-next v11 17/23] ovpn: add support for peer floating

On 04/11/2024 12:24, Sabrina Dubroca wrote:

2024-10-29, 11:47:30 +0100, Antonio Quartulli wrote:

...

+static int ovpn_peer_reset_sockaddr(struct ovpn_peer *peer,

• 		    const struct sockaddr_storage *ss,
  

• 		    const u8 *local_ip)
  

• __must_hold(&peer->lock)

+{

• struct ovpn_bind *bind;
• size_t ip_len;
• /* create new ovpn_bind object */
• bind = ovpn_bind_from_sockaddr(ss);
• if (IS_ERR(bind))

• return PTR_ERR(bind);
  

• if (local_ip) {

• if (ss->ss_family == AF_INET) {
  

• 	ip_len = sizeof(struct in_addr);
  

• } else if (ss->ss_family == AF_INET6) {
  

• 	ip_len = sizeof(struct in6_addr);
  

• } else {
  

• 	netdev_dbg(peer->ovpn->dev, "%s: invalid family for remote endpoint\n",
  

• 		   __func__);
  

ratelimited since that can be triggered from packet processing?

ACK

[...]

...

+void ovpn_peer_float(struct ovpn_peer *peer, struct sk_buff *skb) +{

[...]

...

• switch (family) {
• case AF_INET:

• sa = (struct sockaddr_in *)&ss;
  

• sa->sin_family = AF_INET;
  

• sa->sin_addr.s_addr = ip_hdr(skb)->saddr;
  

• sa->sin_port = udp_hdr(skb)->source;
  

• salen = sizeof(*sa);
  

• break;
  

• case AF_INET6:

• sa6 = (struct sockaddr_in6 *)&ss;
  

• sa6->sin6_family = AF_INET6;
  

• sa6->sin6_addr = ipv6_hdr(skb)->saddr;
  

• sa6->sin6_port = udp_hdr(skb)->source;
  

• sa6->sin6_scope_id = ipv6_iface_scope_id(&ipv6_hdr(skb)->saddr,
  

• 					 skb->skb_iif);
  

• salen = sizeof(*sa6);
  

• break;
  

• default:

• goto unlock;
  

• }
• netdev_dbg(peer->ovpn->dev, "%s: peer %d floated to %pIScp", __func__,

                                           %u for peer->id?

and ratelimited too, probably.

(also in ovpn_peer_update_local_endpoint in the previous patch)

Technically we don't expect that frequent float/endpoint updates, but should they happen..better to be protected.

ACK

...

•    peer->id, &ss);
  

• ovpn_peer_reset_sockaddr(peer, (struct sockaddr_storage *)&ss,

• 		 local_ip);
  

skip the rehash if this fails? peer->bind will still be the old one so moving it to the new hash chain won't help (the lookup will fail).

Yeah, it makes sense.

Thanks a lot. Regards,