[PATCH] net/tap: add new macpair option for split MAC address

[PATCH] net/tap: add new macpair option for split MAC address

[Isaac Boukris]

Normally, the MAC address of the kernel interface is the same as in the
interface in dpdk, as they represent the same interface. It is useful
to allow viewing them as separate connected interfaces (like ip's veth).

This solves a problem I have running a freebsd-based IPv6 stack on top
of dpdk and using the tap interface, as both the kernel and freebsd
stacks configure the MAC derived IPv6 address on the interface (as can
be seen with ifconfig for the kernel), and they both complain about
duplicate IPv6 address and the freebsd disables IPv6 as a result.

Signed-off-by: Isaac Boukris <iboukris@gmail.com>
---
 doc/guides/nics/tap.rst       | 18 +++++++++++++++++
 drivers/net/tap/rte_eth_tap.c | 37 ++++++++++++++++++++++++++++++-----
 drivers/net/tap/rte_eth_tap.h |  1 +
 3 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/doc/guides/nics/tap.rst b/doc/guides/nics/tap.rst
index f01663c700..fd936c6084 100644
--- a/doc/guides/nics/tap.rst
+++ b/doc/guides/nics/tap.rst
@@ -71,6 +71,24 @@ But this behavior can be overridden by the use of the persist flag, example::
 
   --vdev=net_tap0,iface=tap0,persist ...
 
+Normally, the MAC address of the kernel interface is the same as in the
+interface in dpdk, as they represent the same interface. If a MAC address is set
+by the dpdk application using ``rte_eth_dev_default_mac_addr_set()`` then the
+kernel interface changes accordingly (but not vice versa).
+
+If the ``macpair`` option is given, then the MAC address of the kernel
+interface is initially set to a different random MAC address and it is no
+longer altered by dpdk. This allows to view the two interfaces as separate
+connected interfaces, similar to linux's veth pair interfaces. For instance,
+you can configure an IP address on the kernel interface with the ``ip`` command
+in the same subnet as the one used in dpdk and use it as next gateway.
+
+The ``macpair`` options is incompatible with the ``remote`` option (see above).
+
+Example::
+
+    --vdev=net_tap0,iface=tap0,macpair
+
 
 TUN devices
 -----------
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index c5af5751f6..3cdd130092 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -56,6 +56,7 @@
 #define ETH_TAP_MAC_ARG         "mac"
 #define ETH_TAP_MAC_FIXED       "fixed"
 #define ETH_TAP_PERSIST_ARG     "persist"
+#define ETH_TAP_MACPAIR_ARG     "macpair"
 
 #define ETH_TAP_USR_MAC_FMT     "xx:xx:xx:xx:xx:xx"
 #define ETH_TAP_CMP_MAC_FMT     "0123456789ABCDEFabcdef"
@@ -95,6 +96,7 @@ static const char *valid_arguments[] = {
 	ETH_TAP_REMOTE_ARG,
 	ETH_TAP_MAC_ARG,
 	ETH_TAP_PERSIST_ARG,
+	ETH_TAP_MACPAIR_ARG,
 	NULL
 };
 
@@ -1391,6 +1393,12 @@ tap_mac_set(struct rte_eth_dev *dev, struct rte_ether_addr *mac_addr)
 			dev->device->name);
 		return -EINVAL;
 	}
+
+	if (pmd->mac_pair) {
+		rte_ether_addr_copy(&pmd->eth_addr, mac_addr);
+		return 0;
+	}
+
 	/* Check the actual current MAC address on the tap netdevice */
 	ret = tap_ioctl(pmd, SIOCGIFHWADDR, &ifr, 0, LOCAL_ONLY);
 	if (ret < 0)
@@ -1915,7 +1923,7 @@ static const struct eth_dev_ops ops = {
 static int
 eth_dev_tap_create(struct rte_vdev_device *vdev, const char *tap_name,
 		   char *remote_iface, struct rte_ether_addr *mac_addr,
-		   enum rte_tuntap_type type, int persist)
+		   enum rte_tuntap_type type, int persist, int mac_pair)
 {
 	int numa_node = rte_socket_id();
 	struct rte_eth_dev *dev;
@@ -2023,12 +2031,20 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, const char *tap_name,
 	if (pmd->type == ETH_TUNTAP_TYPE_TAP) {
 		memset(&ifr, 0, sizeof(struct ifreq));
 		ifr.ifr_hwaddr.sa_family = AF_LOCAL;
-		rte_memcpy(ifr.ifr_hwaddr.sa_data, &pmd->eth_addr,
-				RTE_ETHER_ADDR_LEN);
+
+		if (mac_pair) {
+			rte_eth_random_addr((uint8_t *)ifr.ifr_hwaddr.sa_data);
+		} else {
+			memcpy(ifr.ifr_hwaddr.sa_data, &pmd->eth_addr,
+			       RTE_ETHER_ADDR_LEN);
+		}
+
 		if (tap_ioctl(pmd, SIOCSIFHWADDR, &ifr, 0, LOCAL_ONLY) < 0)
 			goto error_exit;
 	}
 
+	pmd->mac_pair = mac_pair;
+
 	/* Make network device persist after application exit */
 	pmd->persist = persist;
 
@@ -2306,7 +2322,7 @@ rte_pmd_tun_probe(struct rte_vdev_device *dev)
 	TAP_LOG(DEBUG, "Initializing pmd_tun for %s", name);
 
 	ret = eth_dev_tap_create(dev, tun_name, remote_iface, 0,
-				 ETH_TUNTAP_TYPE_TUN, 0);
+				 ETH_TUNTAP_TYPE_TUN, 0, 0);
 
 leave:
 	if (ret == -1) {
@@ -2429,6 +2445,7 @@ rte_pmd_tap_probe(struct rte_vdev_device *dev)
 	struct rte_eth_dev *eth_dev;
 	int tap_devices_count_increased = 0;
 	int persist = 0;
+	int mac_pair = 0;
 
 	name = rte_vdev_device_name(dev);
 	params = rte_vdev_device_args(dev);
@@ -2504,6 +2521,16 @@ rte_pmd_tap_probe(struct rte_vdev_device *dev)
 					goto leave;
 			}
 
+			if (rte_kvargs_count(kvlist, ETH_TAP_MACPAIR_ARG) == 1) {
+				if (strlen(remote_iface)) {
+					TAP_LOG(ERR, "mac pair not supported "
+						     "with remote interface.");
+					ret = -1;
+					goto leave;
+				}
+				mac_pair = 1;
+			}
+
 			if (rte_kvargs_count(kvlist, ETH_TAP_MAC_ARG) == 1) {
 				ret = rte_kvargs_process(kvlist,
 							 ETH_TAP_MAC_ARG,
@@ -2533,7 +2560,7 @@ rte_pmd_tap_probe(struct rte_vdev_device *dev)
 	tap_devices_count++;
 	tap_devices_count_increased = 1;
 	ret = eth_dev_tap_create(dev, tap_name, remote_iface, &user_mac,
-				 ETH_TUNTAP_TYPE_TAP, persist);
+				 ETH_TUNTAP_TYPE_TAP, persist, mac_pair);
 
 leave:
 	if (ret == -1) {
diff --git a/drivers/net/tap/rte_eth_tap.h b/drivers/net/tap/rte_eth_tap.h
index ce4322ad04..5a33698f76 100644
--- a/drivers/net/tap/rte_eth_tap.h
+++ b/drivers/net/tap/rte_eth_tap.h
@@ -72,6 +72,7 @@ struct pmd_internals {
 	char name[RTE_ETH_NAME_MAX_LEN];  /* Internal Tap device name */
 	int type;                         /* Type field - TUN|TAP */
 	int persist;			  /* 1 if keep link up, else 0 */
+	int mac_pair;                     /* 1 if mac pair enabled, else 0 */
 	struct rte_ether_addr eth_addr;   /* Mac address of the device port */
 	struct ifreq remote_initial_flags;/* Remote netdevice flags on init */
 	int remote_if_index;              /* remote netdevice IF_INDEX */
-- 
2.45.0

Re: [PATCH] net/tap: add new macpair option for split MAC address

[Isaac Boukris]

Hi,

This is a V2 addressing previous comments, I mistakenly passed the -v2
to send-mail instead of format-patch..

Thanks!

On Tue, Sep 17, 2024 at 2:52 PM Isaac Boukris <iboukris@gmail.com> wrote:
>
> Normally, the MAC address of the kernel interface is the same as in the
> interface in dpdk, as they represent the same interface. It is useful
> to allow viewing them as separate connected interfaces (like ip's veth).
>
> This solves a problem I have running a freebsd-based IPv6 stack on top
> of dpdk and using the tap interface, as both the kernel and freebsd
> stacks configure the MAC derived IPv6 address on the interface (as can
> be seen with ifconfig for the kernel), and they both complain about
> duplicate IPv6 address and the freebsd disables IPv6 as a result.
>
> Signed-off-by: Isaac Boukris <iboukris@gmail.com>
> ---
>  doc/guides/nics/tap.rst       | 18 +++++++++++++++++
>  drivers/net/tap/rte_eth_tap.c | 37 ++++++++++++++++++++++++++++++-----
>  drivers/net/tap/rte_eth_tap.h |  1 +
>  3 files changed, 51 insertions(+), 5 deletions(-)
>
> diff --git a/doc/guides/nics/tap.rst b/doc/guides/nics/tap.rst
> index f01663c700..fd936c6084 100644
> --- a/doc/guides/nics/tap.rst
> +++ b/doc/guides/nics/tap.rst
> @@ -71,6 +71,24 @@ But this behavior can be overridden by the use of the persist flag, example::
>
>    --vdev=net_tap0,iface=tap0,persist ...
>
> +Normally, the MAC address of the kernel interface is the same as in the
> +interface in dpdk, as they represent the same interface. If a MAC address is set
> +by the dpdk application using ``rte_eth_dev_default_mac_addr_set()`` then the
> +kernel interface changes accordingly (but not vice versa).
> +
> +If the ``macpair`` option is given, then the MAC address of the kernel
> +interface is initially set to a different random MAC address and it is no
> +longer altered by dpdk. This allows to view the two interfaces as separate
> +connected interfaces, similar to linux's veth pair interfaces. For instance,
> +you can configure an IP address on the kernel interface with the ``ip`` command
> +in the same subnet as the one used in dpdk and use it as next gateway.
> +
> +The ``macpair`` options is incompatible with the ``remote`` option (see above).
> +
> +Example::
> +
> +    --vdev=net_tap0,iface=tap0,macpair
> +
>
>  TUN devices
>  -----------
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index c5af5751f6..3cdd130092 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -56,6 +56,7 @@
>  #define ETH_TAP_MAC_ARG         "mac"
>  #define ETH_TAP_MAC_FIXED       "fixed"
>  #define ETH_TAP_PERSIST_ARG     "persist"
> +#define ETH_TAP_MACPAIR_ARG     "macpair"
>
>  #define ETH_TAP_USR_MAC_FMT     "xx:xx:xx:xx:xx:xx"
>  #define ETH_TAP_CMP_MAC_FMT     "0123456789ABCDEFabcdef"
> @@ -95,6 +96,7 @@ static const char *valid_arguments[] = {
>         ETH_TAP_REMOTE_ARG,
>         ETH_TAP_MAC_ARG,
>         ETH_TAP_PERSIST_ARG,
> +       ETH_TAP_MACPAIR_ARG,
>         NULL
>  };
>
> @@ -1391,6 +1393,12 @@ tap_mac_set(struct rte_eth_dev *dev, struct rte_ether_addr *mac_addr)
>                         dev->device->name);
>                 return -EINVAL;
>         }
> +
> +       if (pmd->mac_pair) {
> +               rte_ether_addr_copy(&pmd->eth_addr, mac_addr);
> +               return 0;
> +       }
> +
>         /* Check the actual current MAC address on the tap netdevice */
>         ret = tap_ioctl(pmd, SIOCGIFHWADDR, &ifr, 0, LOCAL_ONLY);
>         if (ret < 0)
> @@ -1915,7 +1923,7 @@ static const struct eth_dev_ops ops = {
>  static int
>  eth_dev_tap_create(struct rte_vdev_device *vdev, const char *tap_name,
>                    char *remote_iface, struct rte_ether_addr *mac_addr,
> -                  enum rte_tuntap_type type, int persist)
> +                  enum rte_tuntap_type type, int persist, int mac_pair)
>  {
>         int numa_node = rte_socket_id();
>         struct rte_eth_dev *dev;
> @@ -2023,12 +2031,20 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, const char *tap_name,
>         if (pmd->type == ETH_TUNTAP_TYPE_TAP) {
>                 memset(&ifr, 0, sizeof(struct ifreq));
>                 ifr.ifr_hwaddr.sa_family = AF_LOCAL;
> -               rte_memcpy(ifr.ifr_hwaddr.sa_data, &pmd->eth_addr,
> -                               RTE_ETHER_ADDR_LEN);
> +
> +               if (mac_pair) {
> +                       rte_eth_random_addr((uint8_t *)ifr.ifr_hwaddr.sa_data);
> +               } else {
> +                       memcpy(ifr.ifr_hwaddr.sa_data, &pmd->eth_addr,
> +                              RTE_ETHER_ADDR_LEN);
> +               }
> +
>                 if (tap_ioctl(pmd, SIOCSIFHWADDR, &ifr, 0, LOCAL_ONLY) < 0)
>                         goto error_exit;
>         }
>
> +       pmd->mac_pair = mac_pair;
> +
>         /* Make network device persist after application exit */
>         pmd->persist = persist;
>
> @@ -2306,7 +2322,7 @@ rte_pmd_tun_probe(struct rte_vdev_device *dev)
>         TAP_LOG(DEBUG, "Initializing pmd_tun for %s", name);
>
>         ret = eth_dev_tap_create(dev, tun_name, remote_iface, 0,
> -                                ETH_TUNTAP_TYPE_TUN, 0);
> +                                ETH_TUNTAP_TYPE_TUN, 0, 0);
>
>  leave:
>         if (ret == -1) {
> @@ -2429,6 +2445,7 @@ rte_pmd_tap_probe(struct rte_vdev_device *dev)
>         struct rte_eth_dev *eth_dev;
>         int tap_devices_count_increased = 0;
>         int persist = 0;
> +       int mac_pair = 0;
>
>         name = rte_vdev_device_name(dev);
>         params = rte_vdev_device_args(dev);
> @@ -2504,6 +2521,16 @@ rte_pmd_tap_probe(struct rte_vdev_device *dev)
>                                         goto leave;
>                         }
>
> +                       if (rte_kvargs_count(kvlist, ETH_TAP_MACPAIR_ARG) == 1) {
> +                               if (strlen(remote_iface)) {
> +                                       TAP_LOG(ERR, "mac pair not supported "
> +                                                    "with remote interface.");
> +                                       ret = -1;
> +                                       goto leave;
> +                               }
> +                               mac_pair = 1;
> +                       }
> +
>                         if (rte_kvargs_count(kvlist, ETH_TAP_MAC_ARG) == 1) {
>                                 ret = rte_kvargs_process(kvlist,
>                                                          ETH_TAP_MAC_ARG,
> @@ -2533,7 +2560,7 @@ rte_pmd_tap_probe(struct rte_vdev_device *dev)
>         tap_devices_count++;
>         tap_devices_count_increased = 1;
>         ret = eth_dev_tap_create(dev, tap_name, remote_iface, &user_mac,
> -                                ETH_TUNTAP_TYPE_TAP, persist);
> +                                ETH_TUNTAP_TYPE_TAP, persist, mac_pair);
>
>  leave:
>         if (ret == -1) {
> diff --git a/drivers/net/tap/rte_eth_tap.h b/drivers/net/tap/rte_eth_tap.h
> index ce4322ad04..5a33698f76 100644
> --- a/drivers/net/tap/rte_eth_tap.h
> +++ b/drivers/net/tap/rte_eth_tap.h
> @@ -72,6 +72,7 @@ struct pmd_internals {
>         char name[RTE_ETH_NAME_MAX_LEN];  /* Internal Tap device name */
>         int type;                         /* Type field - TUN|TAP */
>         int persist;                      /* 1 if keep link up, else 0 */
> +       int mac_pair;                     /* 1 if mac pair enabled, else 0 */
>         struct rte_ether_addr eth_addr;   /* Mac address of the device port */
>         struct ifreq remote_initial_flags;/* Remote netdevice flags on init */
>         int remote_if_index;              /* remote netdevice IF_INDEX */
> --
> 2.45.0
>

Re: [PATCH] net/tap: add new macpair option for split MAC address

[Ferruh Yigit]

On 9/17/2024 12:51 PM, Isaac Boukris wrote:
> Normally, the MAC address of the kernel interface is the same as in the
> interface in dpdk, as they represent the same interface. It is useful
> to allow viewing them as separate connected interfaces (like ip's veth).
> 
> This solves a problem I have running a freebsd-based IPv6 stack on top
> of dpdk and using the tap interface, as both the kernel and freebsd
> stacks configure the MAC derived IPv6 address on the interface (as can
> be seen with ifconfig for the kernel), and they both complain about
> duplicate IPv6 address and the freebsd disables IPv6 as a result.
> 

How kernel side knows about the IPv6 address DPDK side uses, what is
your tap usecase?


> Signed-off-by: Isaac Boukris <iboukris@gmail.com>
> ---
>  doc/guides/nics/tap.rst       | 18 +++++++++++++++++
>  drivers/net/tap/rte_eth_tap.c | 37 ++++++++++++++++++++++++++++++-----
>  drivers/net/tap/rte_eth_tap.h |  1 +
>  3 files changed, 51 insertions(+), 5 deletions(-)
> 
> diff --git a/doc/guides/nics/tap.rst b/doc/guides/nics/tap.rst
> index f01663c700..fd936c6084 100644
> --- a/doc/guides/nics/tap.rst
> +++ b/doc/guides/nics/tap.rst
> @@ -71,6 +71,24 @@ But this behavior can be overridden by the use of the persist flag, example::
>  
>    --vdev=net_tap0,iface=tap0,persist ...
>  
> +Normally, the MAC address of the kernel interface is the same as in the
> +interface in dpdk, as they represent the same interface. If a MAC address is set
> +by the dpdk application using ``rte_eth_dev_default_mac_addr_set()`` then the
> +kernel interface changes accordingly (but not vice versa).
> +
> +If the ``macpair`` option is given, then the MAC address of the kernel
> +interface is initially set to a different random MAC address and it is no
> +longer altered by dpdk. This allows to view the two interfaces as separate
> +connected interfaces, similar to linux's veth pair interfaces. For instance,
> +you can configure an IP address on the kernel interface with the ``ip`` command
> +in the same subnet as the one used in dpdk and use it as next gateway.
> +
> +The ``macpair`` options is incompatible with the ``remote`` option (see above).
> +
>

I can see 'macpair' is coming from "veth pair" above, but I don't think
it is very explanatory in the context of tap. Do you have any more
suggestion?

Another concern is how to test this, tap PMD got multiple arguments now,
and as the pmd keeps getting more updates and features, how can we sure
this devarg usecase is not broken?

Re: [PATCH] net/tap: add new macpair option for split MAC address

[Isaac Boukris]

Hi,

On Mon, Sep 30, 2024 at 12:55 AM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
>
> On 9/17/2024 12:51 PM, Isaac Boukris wrote:
> > Normally, the MAC address of the kernel interface is the same as in the
> > interface in dpdk, as they represent the same interface. It is useful
> > to allow viewing them as separate connected interfaces (like ip's veth).
> >
> > This solves a problem I have running a freebsd-based IPv6 stack on top
> > of dpdk and using the tap interface, as both the kernel and freebsd
> > stacks configure the MAC derived IPv6 address on the interface (as can
> > be seen with ifconfig for the kernel), and they both complain about
> > duplicate IPv6 address and the freebsd disables IPv6 as a result.
> >
>
> How kernel side knows about the IPv6 address DPDK side uses, what is
> your tap usecase?

It derives it from the MAC address, which dpdk sets on it via ioclt,
to be the same as the dpdk interface.

As soon as dpdk is up, the interface has this IP configured:

ifconfig dpdk0
dpdk0: flags=4675<UP,BROADCAST,RUNNING,ALLMULTI,MULTICAST>  mtu 1500
        inet6 fe80::c830:cff:feda:9762  prefixlen 64  scopeid 0x20<link>
        ether ca:30:0c:da:97:62  txqueuelen 1000  (Ethernet)
        RX packets 5  bytes 434 (434.0 B)
        RX errors 0  dropped 0  overruns 0  frame 0
        TX packets 3  bytes 266 (266.0 B)
        TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0

dmesg showing the conflict with the stack running on dpdk:
IPv6: dpdk0: IPv6 duplicate address fe80::c830:cff:feda:9762 used by
ca:30:0c:da:97:62 detected!

> I can see 'macpair' is coming from "veth pair" above, but I don't think
> it is very explanatory in the context of tap. Do you have any more
> suggestion?
>
> Another concern is how to test this, tap PMD got multiple arguments now,
> and as the pmd keeps getting more updates and features, how can we sure
> this devarg usecase is not broken?

I'll take a look at the test-suite and try to come up with something better.

Thanks!

Re: [PATCH] net/tap: add new macpair option for split MAC address

[Stephen Hemminger]

On Tue, 17 Sep 2024 14:51:47 +0300
Isaac Boukris <iboukris@gmail.com> wrote:

> Normally, the MAC address of the kernel interface is the same as in the
> interface in dpdk, as they represent the same interface. It is useful
> to allow viewing them as separate connected interfaces (like ip's veth).
> 
> This solves a problem I have running a freebsd-based IPv6 stack on top
> of dpdk and using the tap interface, as both the kernel and freebsd
> stacks configure the MAC derived IPv6 address on the interface (as can
> be seen with ifconfig for the kernel), and they both complain about
> duplicate IPv6 address and the freebsd disables IPv6 as a result.
> 
> Signed-off-by: Isaac Boukris <iboukris@gmail.com>

Makes sense but a couple of small comments and rebase needed.

1. Could use rte_ether_addr_copy hear as is done elsewhere.
   And {} non needed.

@@ -2023,12 +2031,20 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, const char *tap_name,
 	if (pmd->type == ETH_TUNTAP_TYPE_TAP) {
 		memset(&ifr, 0, sizeof(struct ifreq));
 		ifr.ifr_hwaddr.sa_family = AF_LOCAL;
-		rte_memcpy(ifr.ifr_hwaddr.sa_data, &pmd->eth_addr,
-				RTE_ETHER_ADDR_LEN);
+
+		if (mac_pair) {
+			rte_eth_random_addr((uint8_t *)ifr.ifr_hwaddr.sa_data);
+		} else {
+			memcpy(ifr.ifr_hwaddr.sa_data, &pmd->eth_addr,
+			       RTE_ETHER_ADDR_LEN);
		        rte_ether_addr_copy(&pmd->eth_addr, (struct rte_ether_addr *)&ifr.ifr_hwaddr.sa_data);

2. Need more error checks. macpair won't work with TUN device.
   and what happens if mac address is specified?
	

3. Should the mac address devarg, take two args?

Re: [PATCH] net/tap: add new macpair option for split MAC address

[Isaac Boukris]

Hi Stephen,


> 2. Need more error checks. macpair won't work with TUN device.
>    and what happens if mac address is specified?

Following Ferruh's feedback, and since the use-case is a bit
different, I was thinking perhaps it would make more sense to add this
functionality as a new device type rather than piggyback on the tap
device (like --vdev=net_veth). This would allow for better isolation,
both in code and in the configuration options, and would also be more
accurate as the dpdk device is completely virtual while the tap device
is the peer one.

> 3. Should the mac address devarg, take two args?

If we go with net_veth as above, then we can either drop the mac
address option for simplicity, and let the user set them manually (the
dpdk interface via dpdk API, and the kernel interface via ip /
ifconfig), or provide an option to set either or both mac addresses as
preferred.

If it sounds good, I'll rework the patch first to implement it as a
new net_veth device, post it, and then try to work out some basic
tests (the latter might take some time tbh).

Thanks!

RE: [PATCH] net/tap: add new macpair option for split MAC address

[Morten Brørup]

> From: Isaac Boukris [mailto:iboukris@gmail.com]
> Sent: Tuesday, 17 September 2024 08.49
> 
> Hi Stephen
> 
> On Tue, Sep 17, 2024 at 6:36 AM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > On Mon, 16 Sep 2024 20:38:51 +0300
> > Isaac Boukris <iboukris@gmail.com> wrote:
> >
> > > +             if (mac_pair) {
> > > +                     struct rte_ether_addr *mac;
> > > +                     mac = (struct
> rte_ether_addr*)ifr.ifr_hwaddr.sa_data;
> > > +                     mac->addr_bytes[3]++;
> >
> > You need to generate a new MAC to be safe, just incrementing by one can
> > easily cause address conflicts.
> 
> I assumed these two MACs would be the only ones connected to the link,
> so it should be ok. If I generate a new random one, should I just
> assume it is unlikely I got the same?

When not using globally administered MAC addresses (which are globally unique, due to their creation process), there's always a risk of address conflicts.

The best option available to generate a random address is rte_eth_random_addr(), also in this case.
With 46 random bits, it is extremely unlikely that you get the same address. Regardless, it doesn't hurt checking.

Re: [PATCH] net/tap: add new macpair option for split MAC address

[Isaac Boukris]

Hi Stephen

On Tue, Sep 17, 2024 at 6:36 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Mon, 16 Sep 2024 20:38:51 +0300
> Isaac Boukris <iboukris@gmail.com> wrote:
>
> > +             if (mac_pair) {
> > +                     struct rte_ether_addr *mac;
> > +                     mac = (struct rte_ether_addr*)ifr.ifr_hwaddr.sa_data;
> > +                     mac->addr_bytes[3]++;
>
> You need to generate a new MAC to be safe, just incrementing by one can
> easily cause address conflicts.

I assumed these two MACs would be the only ones connected to the link,
so it should be ok. If I generate a new random one, should I just
assume it is unlikely I got the same?

Re: [PATCH] net/tap: add new macpair option for split MAC address

[Stephen Hemminger]

On Mon, 16 Sep 2024 20:38:51 +0300
Isaac Boukris <iboukris@gmail.com> wrote:

> +		if (mac_pair) {
> +			struct rte_ether_addr *mac;
> +			mac = (struct rte_ether_addr*)ifr.ifr_hwaddr.sa_data;
> +			mac->addr_bytes[3]++;

You need to generate a new MAC to be safe, just incrementing by one can
easily cause address conflicts.

Re: [PATCH] net/tap: add new macpair option for split MAC address

[Stephen Hemminger]

On Mon, 16 Sep 2024 20:38:51 +0300
Isaac Boukris <iboukris@gmail.com> wrote:

> +
> +	if (pmd->mac_pair) {
> +		rte_memcpy(&pmd->eth_addr, mac_addr, RTE_ETHER_ADDR_LEN);
> +		return 0;
> +	}

Do not use rte_memcpy for simple mac address copy.
Better to use rte_ether_addr_copy (or just memcpy)

[PATCH] net/tap: add new macpair option for split MAC address

[Isaac Boukris]

Normally, the MAC address of the kernel interface is the same as in the
interface in dpdk, as they represent the same interface. It is useful
to allow viewing them as separate connected interfaces (like ip's veth).

This solves a problem I have running a freebsd-based IPv6 stack on top
of dpdk and using the tap interface, as both the kernel and freebsd
stacks configure the MAC derived IPv6 address on the interface (as can
be seen with ifconfig for the kernel), and they both complain about
duplicate IPv6 address and the freebsd disables IPv6 as a result.

Signed-off-by: Isaac Boukris <iboukris@gmail.com>
---
 doc/guides/nics/tap.rst       | 19 +++++++++++++++++++
 drivers/net/tap/rte_eth_tap.c | 34 +++++++++++++++++++++++++++++++---
 drivers/net/tap/rte_eth_tap.h |  1 +
 3 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/doc/guides/nics/tap.rst b/doc/guides/nics/tap.rst
index f01663c700..c5b08f2470 100644
--- a/doc/guides/nics/tap.rst
+++ b/doc/guides/nics/tap.rst
@@ -71,6 +71,25 @@ But this behavior can be overridden by the use of the persist flag, example::
 
   --vdev=net_tap0,iface=tap0,persist ...
 
+Normally, the MAC address of the kernel interface is the same as in the
+interface in dpdk, as they represent the same interface. If a MAC address is set
+by the dpdk application using ``rte_eth_dev_default_mac_addr_set()`` then the
+kernel interface changes accordingly (but not vice versa).
+
+If the ``macpair`` option is given, then the MAC address of the kernel is
+initially set to a different MAC address than the dpdk one (the fourth octet is
+increamented by 1) and calling ``rte_eth_dev_default_mac_addr_set()`` no longer
+changes the MAC of the kernel interface. This allows to view the two interfaces
+as separate connected interfaces, similar to linux's veth pair interfaces. For
+instance, you can configure an IP address on the kernel interface with the ``ip``
+command in the same subnet as the one used in dpdk and use it as next gateway.
+
+The ``macpair`` options is incompatible with the ``remote`` option (see above).
+
+Example::
+
+    --vdev=net_tap0,iface=tap0,macpair
+
 
 TUN devices
 -----------
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index c5af5751f6..1a70832baa 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -56,6 +56,7 @@
 #define ETH_TAP_MAC_ARG         "mac"
 #define ETH_TAP_MAC_FIXED       "fixed"
 #define ETH_TAP_PERSIST_ARG     "persist"
+#define ETH_TAP_MACPAIR_ARG     "macpair"
 
 #define ETH_TAP_USR_MAC_FMT     "xx:xx:xx:xx:xx:xx"
 #define ETH_TAP_CMP_MAC_FMT     "0123456789ABCDEFabcdef"
@@ -95,6 +96,7 @@ static const char *valid_arguments[] = {
 	ETH_TAP_REMOTE_ARG,
 	ETH_TAP_MAC_ARG,
 	ETH_TAP_PERSIST_ARG,
+	ETH_TAP_MACPAIR_ARG,
 	NULL
 };
 
@@ -1391,6 +1393,12 @@ tap_mac_set(struct rte_eth_dev *dev, struct rte_ether_addr *mac_addr)
 			dev->device->name);
 		return -EINVAL;
 	}
+
+	if (pmd->mac_pair) {
+		rte_memcpy(&pmd->eth_addr, mac_addr, RTE_ETHER_ADDR_LEN);
+		return 0;
+	}
+
 	/* Check the actual current MAC address on the tap netdevice */
 	ret = tap_ioctl(pmd, SIOCGIFHWADDR, &ifr, 0, LOCAL_ONLY);
 	if (ret < 0)
@@ -1915,7 +1923,7 @@ static const struct eth_dev_ops ops = {
 static int
 eth_dev_tap_create(struct rte_vdev_device *vdev, const char *tap_name,
 		   char *remote_iface, struct rte_ether_addr *mac_addr,
-		   enum rte_tuntap_type type, int persist)
+		   enum rte_tuntap_type type, int persist, int mac_pair)
 {
 	int numa_node = rte_socket_id();
 	struct rte_eth_dev *dev;
@@ -2025,10 +2033,19 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, const char *tap_name,
 		ifr.ifr_hwaddr.sa_family = AF_LOCAL;
 		rte_memcpy(ifr.ifr_hwaddr.sa_data, &pmd->eth_addr,
 				RTE_ETHER_ADDR_LEN);
+
+		if (mac_pair) {
+			struct rte_ether_addr *mac;
+			mac = (struct rte_ether_addr*)ifr.ifr_hwaddr.sa_data;
+			mac->addr_bytes[3]++;
+		}
+
 		if (tap_ioctl(pmd, SIOCSIFHWADDR, &ifr, 0, LOCAL_ONLY) < 0)
 			goto error_exit;
 	}
 
+	pmd->mac_pair = mac_pair;
+
 	/* Make network device persist after application exit */
 	pmd->persist = persist;
 
@@ -2306,7 +2323,7 @@ rte_pmd_tun_probe(struct rte_vdev_device *dev)
 	TAP_LOG(DEBUG, "Initializing pmd_tun for %s", name);
 
 	ret = eth_dev_tap_create(dev, tun_name, remote_iface, 0,
-				 ETH_TUNTAP_TYPE_TUN, 0);
+				 ETH_TUNTAP_TYPE_TUN, 0, 0);
 
 leave:
 	if (ret == -1) {
@@ -2429,6 +2446,7 @@ rte_pmd_tap_probe(struct rte_vdev_device *dev)
 	struct rte_eth_dev *eth_dev;
 	int tap_devices_count_increased = 0;
 	int persist = 0;
+	int mac_pair = 0;
 
 	name = rte_vdev_device_name(dev);
 	params = rte_vdev_device_args(dev);
@@ -2504,6 +2522,16 @@ rte_pmd_tap_probe(struct rte_vdev_device *dev)
 					goto leave;
 			}
 
+			if (rte_kvargs_count(kvlist, ETH_TAP_MACPAIR_ARG) == 1) {
+				if (strlen(remote_iface)) {
+					TAP_LOG(ERR, "mac pair not supported "
+						     "with remote interface.");
+					ret = -1;
+					goto leave;
+				}
+				mac_pair = 1;
+			}
+
 			if (rte_kvargs_count(kvlist, ETH_TAP_MAC_ARG) == 1) {
 				ret = rte_kvargs_process(kvlist,
 							 ETH_TAP_MAC_ARG,
@@ -2533,7 +2561,7 @@ rte_pmd_tap_probe(struct rte_vdev_device *dev)
 	tap_devices_count++;
 	tap_devices_count_increased = 1;
 	ret = eth_dev_tap_create(dev, tap_name, remote_iface, &user_mac,
-				 ETH_TUNTAP_TYPE_TAP, persist);
+				 ETH_TUNTAP_TYPE_TAP, persist, mac_pair);
 
 leave:
 	if (ret == -1) {
diff --git a/drivers/net/tap/rte_eth_tap.h b/drivers/net/tap/rte_eth_tap.h
index ce4322ad04..5a33698f76 100644
--- a/drivers/net/tap/rte_eth_tap.h
+++ b/drivers/net/tap/rte_eth_tap.h
@@ -72,6 +72,7 @@ struct pmd_internals {
 	char name[RTE_ETH_NAME_MAX_LEN];  /* Internal Tap device name */
 	int type;                         /* Type field - TUN|TAP */
 	int persist;			  /* 1 if keep link up, else 0 */
+	int mac_pair;                     /* 1 if mac pair enabled, else 0 */
 	struct rte_ether_addr eth_addr;   /* Mac address of the device port */
 	struct ifreq remote_initial_flags;/* Remote netdevice flags on init */
 	int remote_if_index;              /* remote netdevice IF_INDEX */
-- 
2.45.0