Re: [PATCH stable 4.19 up to 6.8] vxlan: Fix regression when dropping packets due to invalid src addresses

[Greg KH <gregkh@linuxfoundation.org>]

On Thu, Jun 06, 2024 at 11:47:33AM +0200, Daniel Borkmann wrote:
> [ Upstream commit 1cd4bc987abb2823836cbb8f887026011ccddc8a ]
> 
> Commit f58f45c1e5b9 ("vxlan: drop packets from invalid src-address")
> has recently been added to vxlan mainly in the context of source
> address snooping/learning so that when it is enabled, an entry in the
> FDB is not being created for an invalid address for the corresponding
> tunnel endpoint.
> 
> Before commit f58f45c1e5b9 vxlan was similarly behaving as geneve in
> that it passed through whichever macs were set in the L2 header. It
> turns out that this change in behavior breaks setups, for example,
> Cilium with netkit in L3 mode for Pods as well as tunnel mode has been
> passing before the change in f58f45c1e5b9 for both vxlan and geneve.
> After mentioned change it is only passing for geneve as in case of
> vxlan packets are dropped due to vxlan_set_mac() returning false as
> source and destination macs are zero which for E/W traffic via tunnel
> is totally fine.
> 
> Fix it by only opting into the is_valid_ether_addr() check in
> vxlan_set_mac() when in fact source address snooping/learning is
> actually enabled in vxlan. This is done by moving the check into
> vxlan_snoop(). With this change, the Cilium connectivity test suite
> passes again for both tunnel flavors.
> 
> Fixes: f58f45c1e5b9 ("vxlan: drop packets from invalid src-address")
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Cc: David Bauer <mail@david-bauer.net>
> Cc: Ido Schimmel <idosch@nvidia.com>
> Cc: Nikolay Aleksandrov <razor@blackwall.org>
> Cc: Martin KaFai Lau <martin.lau@kernel.org>
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>
> Reviewed-by: David Bauer <mail@david-bauer.net>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> [ Backport note: vxlan snooping/learning not supported in 6.8 or older,
>   so commit is simply a revert. ]
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> ---
>  drivers/net/vxlan/vxlan_core.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
> index 0a0b4a9717ce..1d0688610189 100644
> --- a/drivers/net/vxlan/vxlan_core.c
> +++ b/drivers/net/vxlan/vxlan_core.c
> @@ -1615,10 +1615,6 @@ static bool vxlan_set_mac(struct vxlan_dev *vxlan,
>  	if (ether_addr_equal(eth_hdr(skb)->h_source, vxlan->dev->dev_addr))
>  		return false;
>  
> -	/* Ignore packets from invalid src-address */
> -	if (!is_valid_ether_addr(eth_hdr(skb)->h_source))
> -		return false;
> -
>  	/* Get address from the outer IP header */
>  	if (vxlan_get_sk_family(vs) == AF_INET) {
>  		saddr.sin.sin_addr.s_addr = ip_hdr(skb)->saddr;
> -- 
> 2.34.1
> 
> 

All now queued up, thanks.

greg k-h