netdev - Re: [PATCH net-next v7 1/2] Add nolocalbypass option to vxlan.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 4 May 2023 16:05:10 +0300
From: Ido Schimmel <idosch@...sch.org>
To: Vladimir Nikishkin <vladimir@...ishkin.pw>
Cc: netdev@...r.kernel.org, davem@...emloft.net, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com,
	eng.alaamohamedsoliman.am@...il.com, gnault@...hat.com,
	razor@...ckwall.org, idosch@...dia.com, liuhangbin@...il.com,
	eyal.birger@...il.com, jtoppins@...hat.com, shuah@...nel.org,
	linux-kselftest@...r.kernel.org
Subject: Re: [PATCH net-next v7 1/2] Add nolocalbypass option to vxlan.

On Tue, May 02, 2023 at 12:25:29AM +0800, Vladimir Nikishkin wrote:
> diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
> index 561fe1b314f5..ede98b879257 100644
> --- a/drivers/net/vxlan/vxlan_core.c
> +++ b/drivers/net/vxlan/vxlan_core.c
> @@ -2355,11 +2355,13 @@ static int encap_bypass_if_local(struct sk_buff *skb, struct net_device *dev,
>  	    !(rt_flags & (RTCF_BROADCAST | RTCF_MULTICAST))) {
>  		struct vxlan_dev *dst_vxlan;
>  
> -		dst_release(dst);
>  		dst_vxlan = vxlan_find_vni(vxlan->net, dst_ifindex, vni,
>  					   daddr->sa.sa_family, dst_port,
>  					   vxlan->cfg.flags);
>  		if (!dst_vxlan) {
> +			if (!(vxlan->cfg.flags & VXLAN_F_LOCALBYPASS))
> +				return 0;
> +			dst_release(dst);

Thinking about it again, now that we have a new flag to signal the
desired behavior, why do we care if there is a local VXLAN device
listening or not? If 'VXLAN_F_LOCALBYPASS' is not set we don't want to
deliver the packet to a local VXLAN device even if one exists.

IOW, can't the diff simply be:

diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
index 561fe1b314f5..1a1dfe6be92d 100644
--- a/drivers/net/vxlan/vxlan_core.c
+++ b/drivers/net/vxlan/vxlan_core.c
@@ -2352,7 +2352,8 @@ static int encap_bypass_if_local(struct sk_buff *skb, struct net_device *dev,
 #endif
        /* Bypass encapsulation if the destination is local */
        if (rt_flags & RTCF_LOCAL &&
-           !(rt_flags & (RTCF_BROADCAST | RTCF_MULTICAST))) {
+           !(rt_flags & (RTCF_BROADCAST | RTCF_MULTICAST)) &&
+           vxlan->cfg.flags & VXLAN_F_LOCALBYPASS) {
                struct vxlan_dev *dst_vxlan;
 
                dst_release(dst);

?

>  			dev->stats.tx_errors++;
>  			vxlan_vnifilter_count(vxlan, vni, NULL,
>  					      VXLAN_VNI_STATS_TX_ERRORS, 0);
> @@ -2367,6 +2369,7 @@ static int encap_bypass_if_local(struct sk_buff *skb, struct net_device *dev,
>  
>  			return -ENOENT;
>  		}
> +		dst_release(dst);
>  		vxlan_encap_bypass(skb, vxlan, dst_vxlan, vni, true);
>  		return 1;
>  	}
> @@ -2568,7 +2571,6 @@ void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
>  
>  		if (!info) {
>  			u32 rt6i_flags = ((struct rt6_info *)ndst)->rt6i_flags;
> -

Unnecessary change. Please remove. Probably a leftover from previous
versions

>  			err = encap_bypass_if_local(skb, dev, vxlan, dst,
>  						    dst_port, ifindex, vni,
>  						    ndst, rt6i_flags);

> @@ -3172,6 +3174,7 @@ static void vxlan_raw_setup(struct net_device *dev)
>  }
>  
>  static const struct nla_policy vxlan_policy[IFLA_VXLAN_MAX + 1] = {
> +	[IFLA_VXLAN_UNSPEC]     = { .strict_start_type = IFLA_VXLAN_LOCALBYPASS },
>  	[IFLA_VXLAN_ID]		= { .type = NLA_U32 },
>  	[IFLA_VXLAN_GROUP]	= { .len = sizeof_field(struct iphdr, daddr) },
>  	[IFLA_VXLAN_GROUP6]	= { .len = sizeof(struct in6_addr) },
> @@ -3202,6 +3205,7 @@ static const struct nla_policy vxlan_policy[IFLA_VXLAN_MAX + 1] = {
>  	[IFLA_VXLAN_TTL_INHERIT]	= { .type = NLA_FLAG },
>  	[IFLA_VXLAN_DF]		= { .type = NLA_U8 },
>  	[IFLA_VXLAN_VNIFILTER]	= { .type = NLA_U8 },
> +	[IFLA_VXLAN_LOCALBYPASS]	= NLA_POLICY_MAX(NLA_U8, 1),
>  };

The rest looks fine to me