On Fri, Feb 14, 2025 at 03:25:22PM +0100, Alexander Bluhm wrote:
> Hi,
> 
> Shared net lock and socket lock should be sufficient for vxlan.
> 
> While there, check if mbuf list is empty before grabbing lock.  This
> way is done in multiple places.
> 
> Also do not pass around uninitialized structures from the stack.
> Use memset() to start with clean memory.
> 
> ok?

I already have a positive test report.
any reviewers?

> 
> bluhm
> 
> Index: net/if_vxlan.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_vxlan.c,v
> diff -u -p -r1.100 if_vxlan.c
> --- net/if_vxlan.c	31 Oct 2024 11:41:31 -0000	1.100
> +++ net/if_vxlan.c	13 Feb 2025 14:36:19 -0000
> @@ -514,16 +514,19 @@ vxlan_send_ipv4(struct vxlan_softc *sc,
>  	struct mbuf *m;
>  	uint64_t oerrors = 0;
> 
> +	if (ml_empty(ml))
> +		return (0);
> +
> +	memset(&imo, 0, sizeof(struct ip_moptions));
>  	imo.imo_ifidx = sc->sc_if_index0;
>  	imo.imo_ttl = sc->sc_ttl;
> -	imo.imo_loop = 0;
> 
> -	NET_LOCK();
> +	NET_LOCK_SHARED();
>  	while ((m = ml_dequeue(ml)) != NULL) {
>  		if (ip_output(m, NULL, NULL, IP_RAWOUTPUT, &imo, NULL, 0) != 0)
>  			oerrors++;
>  	}
> -	NET_UNLOCK();
> +	NET_UNLOCK_SHARED();
> 
>  	return (oerrors);
>  }
> @@ -536,16 +539,19 @@ vxlan_send_ipv6(struct vxlan_softc *sc,
>  	struct mbuf *m;
>  	uint64_t oerrors = 0;
> 
> +	if (ml_empty(ml))
> +		return (0);
> +
> +	memset(&im6o, 0, sizeof(struct ip6_moptions));
>  	im6o.im6o_ifidx = sc->sc_if_index0;
>  	im6o.im6o_hlim = sc->sc_ttl;
> -	im6o.im6o_loop = 0;
> 
> -	NET_LOCK();
> +	NET_LOCK_SHARED();
>  	while ((m = ml_dequeue(ml)) != NULL) {
>  		if (ip6_output(m, NULL, NULL, 0, &im6o, NULL) != 0)
>  			oerrors++;
>  	}
> -	NET_UNLOCK();
> +	NET_UNLOCK_SHARED();
> 
>  	return (oerrors);
>  }
> @@ -932,10 +938,10 @@ vxlan_tep_add_addr(struct vxlan_softc *s
>  	if (error != 0)
>  		goto free;
> 
> -	solock(so);
> +	solock_shared(so);
>  	sotoinpcb(so)->inp_upcall = vxlan_input;
>  	sotoinpcb(so)->inp_upcall_arg = vt;
> -	sounlock(so);
> +	sounlock_shared(so);
> 
>  	m_inithdr(&m);
>  	m.m_len = sizeof(vt->vt_rdomain);
> @@ -972,9 +978,9 @@ vxlan_tep_add_addr(struct vxlan_softc *s
>  		unhandled_af(vt->vt_af);
>  	}
> 
> -	solock(so);
> +	solock_shared(so);
>  	error = sobind(so, &m, curproc);
> -	sounlock(so);
> +	sounlock_shared(so);
>  	if (error != 0)
>  		goto close;
>