diff mbox series

[RFC,1/3] net/tun: Ensure tun devices have a MAC address

Message ID 168330132769.5953.7109360341846745035.stgit@oracle-102.nfsv4bat.org (mailing list archive)
State RFC
Delegated to: Netdev Maintainers
Headers show
Series siw on tunnel devices | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/cc_maintainers fail 4 maintainers not CCed: kuba@kernel.org edumazet@google.com davem@davemloft.net pabeni@redhat.com
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 24 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Chuck Lever May 5, 2023, 3:42 p.m. UTC 
From: Chuck Lever <chuck.lever@oracle.com>

A non-zero MAC address enables a network device to be assigned as
the underlying device for a virtual RDMA device. Without a non-
zero MAC address, cma_acquire_dev_by_src_ip() is unable to find the
underlying egress device that corresponds to a source IP address,
and rdma_resolve_address() fails.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 drivers/net/tun.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Stephen Hemminger May 5, 2023, 4:59 p.m. UTC | #1 
On Fri, 05 May 2023 11:42:17 -0400
Chuck Lever <cel@kernel.org> wrote:

> From: Chuck Lever <chuck.lever@oracle.com>
> 
> A non-zero MAC address enables a network device to be assigned as
> the underlying device for a virtual RDMA device. Without a non-
> zero MAC address, cma_acquire_dev_by_src_ip() is unable to find the
> underlying egress device that corresponds to a source IP address,
> and rdma_resolve_address() fails.
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  drivers/net/tun.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index d4d0a41a905a..da85abfcd254 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1384,7 +1384,7 @@ static void tun_net_initialize(struct net_device *dev)
>  
>  		/* Point-to-Point TUN Device */
>  		dev->hard_header_len = 0;
> -		dev->addr_len = 0;
> +		dev->addr_len = ETH_ALEN;
>  		dev->mtu = 1500;
>  
>  		/* Zero header length */

This is a bad idea.
TUN devices are L3 devices without any MAC address.
This patch will change the semantics and break users.

If you want an L2 address, you need to use TAP, not TUN device.
Chuck Lever May 5, 2023, 5:09 p.m. UTC | #2 
> On May 5, 2023, at 12:59 PM, Stephen Hemminger <stephen@networkplumber.org> wrote:
> 
> On Fri, 05 May 2023 11:42:17 -0400
> Chuck Lever <cel@kernel.org> wrote:
> 
>> From: Chuck Lever <chuck.lever@oracle.com>
>> 
>> A non-zero MAC address enables a network device to be assigned as
>> the underlying device for a virtual RDMA device. Without a non-
>> zero MAC address, cma_acquire_dev_by_src_ip() is unable to find the
>> underlying egress device that corresponds to a source IP address,
>> and rdma_resolve_address() fails.
>> 
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>> drivers/net/tun.c |    6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>> index d4d0a41a905a..da85abfcd254 100644
>> --- a/drivers/net/tun.c
>> +++ b/drivers/net/tun.c
>> @@ -1384,7 +1384,7 @@ static void tun_net_initialize(struct net_device *dev)
>> 
>> /* Point-to-Point TUN Device */
>> dev->hard_header_len = 0;
>> - dev->addr_len = 0;
>> + dev->addr_len = ETH_ALEN;
>> dev->mtu = 1500;
>> 
>> /* Zero header length */
> 
> This is a bad idea.
> TUN devices are L3 devices without any MAC address.
> This patch will change the semantics and break users.

I suspected this might be a problem, thanks for the quick
feedback.


> If you want an L2 address, you need to use TAP, not TUN device.

We can't assume how the VPN is implemented. In our case,
it's Tailscale, which creates a tun device. wireguard (in
kernel) is the same.

We would prefer a mechanism that can support tun. Having a
MAC is the easiest way forward, but is not a hard
requirement AFAICT.


--
Chuck Lever
diff mbox series

Patch

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index d4d0a41a905a..da85abfcd254 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1384,7 +1384,7 @@  static void tun_net_initialize(struct net_device *dev)
 
 		/* Point-to-Point TUN Device */
 		dev->hard_header_len = 0;
-		dev->addr_len = 0;
+		dev->addr_len = ETH_ALEN;
 		dev->mtu = 1500;
 
 		/* Zero header length */
@@ -1399,8 +1399,6 @@  static void tun_net_initialize(struct net_device *dev)
 		dev->priv_flags &= ~IFF_TX_SKB_SHARING;
 		dev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
 
-		eth_hw_addr_random(dev);
-
 		/* Currently tun does not support XDP, only tap does. */
 		dev->xdp_features = NETDEV_XDP_ACT_BASIC |
 				    NETDEV_XDP_ACT_REDIRECT |
@@ -1409,6 +1407,8 @@  static void tun_net_initialize(struct net_device *dev)
 		break;
 	}
 
+	eth_hw_addr_random(dev);
+
 	dev->min_mtu = MIN_MTU;
 	dev->max_mtu = MAX_MTU - dev->hard_header_len;
 }