diff mbox

[OpenWrt-Devel] vlandev: Add support for setting a unique MAC on a VLAN interface

Message ID 1453664846-4973-1-git-send-email-avalentin@marcant.net
State Rejected
Headers show

Commit Message

André Valentin Jan. 24, 2016, 7:47 p.m. UTC
There are cases where you need a different MAC on a VLAN interface, i.e. if
if have move than one connection to a switch. A later change of the MAC
is not possible on VLAN interfaces, so it has to be done on creation.
The new macaddr option allows to set a unique MAC for a VLAN interface.
Example:
config device 'testvl'
        option type '8021ad'
        option name 'testvl'
        option ifname 'eth0'
        option vid '106'
        option macaddr 'f2:48:00:89:45:4c'

config interface 'testif'
        option ifname 'testvl'
        option proto 'none'
        option auto '1'

Signed-off-by: André Valentin <avalentin@marcant.net>
---
 system-linux.c |  2 ++
 system.h       |  7 +++++++
 vlandev.c      | 17 +++++++++++++++++
 3 files changed, 26 insertions(+)

Comments

André Valentin Jan. 24, 2016, 8:34 p.m. UTC | #1
Am 24.01.2016 um 20:47 schrieb André Valentin:
> There are cases where you need a different MAC on a VLAN interface, i.e. if
> if have move than one connection to a switch. A later change of the MAC
> is not possible on VLAN interfaces, so it has to be done on creation.
> The new macaddr option allows to set a unique MAC for a VLAN interface.
> Example:
> config device 'testvl'
>         option type '8021ad'
>         option name 'testvl'
>         option ifname 'eth0'
>         option vid '106'
>         option macaddr 'f2:48:00:89:45:4c'
> 
> config interface 'testif'
>         option ifname 'testvl'
>         option proto 'none'
>         option auto '1'

Wiki now inlcudes vlandev and the new macaddr option.
https://wiki.openwrt.org/doc/uci/network#devices

Kind regards,

André
André Valentin Jan. 24, 2016, 9 p.m. UTC | #2
Am 24.01.2016 um 21:34 schrieb Gio:
> On Sunday 24 January 2016 20:47:26 André Valentin wrote:
>> There are cases where you need a different MAC on a VLAN interface, i.e. if
>> if have move than one connection to a switch. A later change of the MAC
>> is not possible on VLAN interfaces, so it has to be done on creation.
>> The new macaddr option allows to set a unique MAC for a VLAN interface.
> 
> I am the author of vlandev stuff are you sure that this doesn't break 
> assumptions made by linux kernel networking stack?
> 
> Some time ago i have done some testing (not enough probably) about changing 
> mac address on a vlan and the vlan interface stopped receiving packets, I 
> think that linux kernel was assuming the vlan and the untagged interface have 
> the same mac address my setup was like this
> 
> <br-lan{eth0, bat0, wlan0}>
> <eth0.10>
> 
> My experience was that if eth0 and eth0.10 had differents mac, eth0.10 didn't 
> receiving packets, this may be caused by eth0 being in a bridge
> 
> To overcome this i tried multiple permutations with macvlan in the middle but 
> none of them worked :(
> 
> one of the permutation was this
> <br-lan{eth0, bat0, wlan0}>
> <macvlan0(eth0)>
> <macvlan0.10>
> if i remember well i could not create the macvlan0 interface since eth0 was in 
> the bridge :(
The upper setup was exact that what I not wanted ;-)

I know that there are problems if you create a bridge on the untagged VLAN and
also on tagged ones depending on the kernel version. Personally I do not use such
setup because of its instability.
But with the documented setup below I have no problems, it's verified with several
ar71xx devices.

Kind regards,

Andre


> 
> Cheers!
> 
>> Example:
>> config device 'testvl'
>>         option type '8021ad'
>>         option name 'testvl'
>>         option ifname 'eth0'
>>         option vid '106'
>>         option macaddr 'f2:48:00:89:45:4c'
>>
>> config interface 'testif'
>>         option ifname 'testvl'
>>         option proto 'none'
>>         option auto '1'
>>
André Valentin Jan. 24, 2016, 9:04 p.m. UTC | #3
Am 24.01.2016 um 22:00 schrieb Andre Valentin:
> Am 24.01.2016 um 21:34 schrieb Gio:
>> On Sunday 24 January 2016 20:47:26 André Valentin wrote:
>>> There are cases where you need a different MAC on a VLAN interface, i.e. if
>>> if have move than one connection to a switch. A later change of the MAC
>>> is not possible on VLAN interfaces, so it has to be done on creation.
>>> The new macaddr option allows to set a unique MAC for a VLAN interface.
>>
>> I am the author of vlandev stuff are you sure that this doesn't break 
>> assumptions made by linux kernel networking stack?
>>
>> Some time ago i have done some testing (not enough probably) about changing 
>> mac address on a vlan and the vlan interface stopped receiving packets, I 
>> think that linux kernel was assuming the vlan and the untagged interface have 
>> the same mac address my setup was like this
>>
>> <br-lan{eth0, bat0, wlan0}>
>> <eth0.10>
>>
>> My experience was that if eth0 and eth0.10 had differents mac, eth0.10 didn't 
>> receiving packets, this may be caused by eth0 being in a bridge
>>
>> To overcome this i tried multiple permutations with macvlan in the middle but 
>> none of them worked :(
>>
>> one of the permutation was this
>> <br-lan{eth0, bat0, wlan0}>
>> <macvlan0(eth0)>
>> <macvlan0.10>
>> if i remember well i could not create the macvlan0 interface since eth0 was in 
>> the bridge :(
> The upper setup was exact that what I not wanted ;-)
> 
> I know that there are problems if you create a bridge on the untagged VLAN and
> also on tagged ones depending on the kernel version. Personally I do not use such
> setup because of its instability.
> But with the documented setup below I have no problems, it's verified with several
> ar71xx devices.

By the way, you should always make sure that you use a unicast mac. There are several generators
on the web which do not take care if this. It took me some time to remember this fact yesterday..

Kind regards,

André
Felix Fietkau Jan. 28, 2016, 10:35 p.m. UTC | #4
On 2016-01-24 20:47, André Valentin wrote:
> There are cases where you need a different MAC on a VLAN interface, i.e. if
> if have move than one connection to a switch. A later change of the MAC
> is not possible on VLAN interfaces, so it has to be done on creation.
> The new macaddr option allows to set a unique MAC for a VLAN interface.
> Example:
> config device 'testvl'
>         option type '8021ad'
>         option name 'testvl'
>         option ifname 'eth0'
>         option vid '106'
>         option macaddr 'f2:48:00:89:45:4c'
> 
> config interface 'testif'
>         option ifname 'testvl'
>         option proto 'none'
>         option auto '1'
> 
> Signed-off-by: André Valentin <avalentin@marcant.net>
I don't see the point of this patch - I just tried your example config,
and it works just fine without it ;)

- Felix
André Valentin Jan. 29, 2016, 12:15 p.m. UTC | #5
Hi Felix,

thanks for the hint. Did you configure the MAC in the interface section?
I suggest I had a wrongly created MAC on my first tries which resulted in kernel not accepting it.

Kind regards,

André

Am 28.01.2016 um 23:35 schrieb Felix Fietkau:
> On 2016-01-24 20:47, André Valentin wrote:
>> There are cases where you need a different MAC on a VLAN interface, i.e. if
>> if have move than one connection to a switch. A later change of the MAC
>> is not possible on VLAN interfaces, so it has to be done on creation.
>> The new macaddr option allows to set a unique MAC for a VLAN interface.
>> Example:
>> config device 'testvl'
>>         option type '8021ad'
>>         option name 'testvl'
>>         option ifname 'eth0'
>>         option vid '106'
>>         option macaddr 'f2:48:00:89:45:4c'
>>
>> config interface 'testif'
>>         option ifname 'testvl'
>>         option proto 'none'
>>         option auto '1'
>>
>> Signed-off-by: André Valentin <avalentin@marcant.net>
> I don't see the point of this patch - I just tried your example config,
> and it works just fine without it ;)
> 
> - Felix
>
Felix Fietkau Jan. 29, 2016, 12:33 p.m. UTC | #6
On 2016-01-29 13:15, André Valentin wrote:
> Hi Felix,
> 
> thanks for the hint. Did you configure the MAC in the interface section?
> I suggest I had a wrongly created MAC on my first tries which resulted in kernel not accepting it.
I configured it in the device section - in fact I simply copy&pasted
your exact example config text ;)

- Felix
André Valentin Jan. 31, 2016, 12:26 a.m. UTC | #7
Hi Felix,

Am 29.01.2016 um 13:33 schrieb Felix Fietkau:
> On 2016-01-29 13:15, André Valentin wrote:
>> Hi Felix,
>>
>> thanks for the hint. Did you configure the MAC in the interface section?
>> I suggest I had a wrongly created MAC on my first tries which resulted in kernel not accepting it.
> I configured it in the device section - in fact I simply copy&pasted
> your exact example config text ;)

It works.. Argh. Stupid MAC generator that gave me bad MACs... That made me extend the code...

Kind regards,

André

P.S.: Don't ever use this one:
http://www.miniwebtool.com/mac-address-generator/
diff mbox

Patch

diff --git a/system-linux.c b/system-linux.c
index 909ba0f..ac657c2 100644
--- a/system-linux.c
+++ b/system-linux.c
@@ -1014,6 +1014,8 @@  int system_vlandev_add(struct device *vlandev, struct device *dev, struct vlande
 		return -1;
 
 	nlmsg_append(msg, &iim, sizeof(iim), 0);
+	if (cfg->flags & VLANDEV_OPT_MACADDR)
+		nla_put(msg, IFLA_ADDRESS, sizeof(cfg->macaddr), cfg->macaddr);
 	nla_put_string(msg, IFLA_IFNAME, vlandev->ifname);
 	nla_put_u32(msg, IFLA_LINK, dev->ifindex);
 	
diff --git a/system.h b/system.h
index 97fbc8b..a053072 100644
--- a/system.h
+++ b/system.h
@@ -77,9 +77,16 @@  enum vlan_proto {
 	VLAN_PROTO_8021AD = 0x88A8
 };
 
+enum vlandev_opt {
+	VLANDEV_OPT_MACADDR = (1 << 0),
+};
+
 struct vlandev_config {
 	enum vlan_proto proto;
 	uint16_t vid;
+
+	enum vlandev_opt flags;
+	unsigned char macaddr[6];
 };
 
 static inline int system_get_addr_family(unsigned int flags)
diff --git a/vlandev.c b/vlandev.c
index b93527c..7f1eda7 100644
--- a/vlandev.c
+++ b/vlandev.c
@@ -13,15 +13,22 @@ 
  */
 
 #include <string.h>
+#include <net/ethernet.h>
+
+#ifdef linux
+#include <netinet/ether.h>
+#endif
 
 #include "netifd.h"
 #include "device.h"
 #include "interface.h"
 #include "system.h"
 
+
 enum {
 	VLANDEV_ATTR_TYPE,
 	VLANDEV_ATTR_IFNAME,
+	VLANDEV_ATTR_MACADDR,
 	VLANDEV_ATTR_VID,
 	__VLANDEV_ATTR_MAX
 };
@@ -29,6 +36,7 @@  enum {
 static const struct blobmsg_policy vlandev_attrs[__VLANDEV_ATTR_MAX] = {
 	[VLANDEV_ATTR_TYPE] = { "type", BLOBMSG_TYPE_STRING },
 	[VLANDEV_ATTR_IFNAME] = { "ifname", BLOBMSG_TYPE_STRING },
+	[VLANDEV_ATTR_MACADDR] = { "macaddr", BLOBMSG_TYPE_STRING },
 	[VLANDEV_ATTR_VID] = { "vid", BLOBMSG_TYPE_INT32 },
 };
 
@@ -157,6 +165,7 @@  vlandev_apply_settings(struct vlandev_device *mvdev, struct blob_attr **tb)
 {
 	struct vlandev_config *cfg = &mvdev->config;
 	struct blob_attr *cur;
+	struct ether_addr *ea;
 
 	cfg->proto = VLAN_PROTO_8021Q;
 	cfg->vid = 1;
@@ -169,6 +178,14 @@  vlandev_apply_settings(struct vlandev_device *mvdev, struct blob_attr **tb)
 
 	if ((cur = tb[VLANDEV_ATTR_VID]))
 		cfg->vid = (uint16_t) blobmsg_get_u32(cur);
+
+	if ((cur = tb[VLANDEV_ATTR_MACADDR])) {
+		ea = ether_aton(blobmsg_data(cur));
+		if (ea) {
+			memcpy(cfg->macaddr, ea, 6);
+			cfg->flags |= VLANDEV_OPT_MACADDR;
+		}
+	}
 }
 
 static enum dev_change_type