diff mbox

[LEDE-DEV,2/5] br2684ctl: set the MAC address configured for the atm bridge using

Message ID 1467471640-22044-2-git-send-email-dev@kresin.me
State Changes Requested
Delegated to: John Crispin
Headers show

Commit Message

Mathias Kresin July 2, 2016, 3 p.m. UTC
If an ESI is set, it will be used as MAC address for the nas0
interface.

According to the ESI man page, changes to the ESI are not automatically
propagated throughout the system and therefore esi should be used
early during system.

Setting the ESI before creating the nas interface should be early enough
for all following services.

Signed-off-by: Mathias Kresin <dev@kresin.me>
---
 package/network/utils/linux-atm/Makefile        | 1 +
 package/network/utils/linux-atm/files/br2684ctl | 5 +++++
 2 files changed, 6 insertions(+)

Comments

Felix Fietkau July 3, 2016, 7:47 a.m. UTC | #1
On 2016-07-02 17:00, Mathias Kresin wrote:
> If an ESI is set, it will be used as MAC address for the nas0
> interface.
> 
> According to the ESI man page, changes to the ESI are not automatically
> propagated throughout the system and therefore esi should be used
> early during system.
> 
> Setting the ESI before creating the nas interface should be early enough
> for all following services.
> 
> Signed-off-by: Mathias Kresin <dev@kresin.me>
I'm not sure the atm-bridge section is the right place for the MAC
address. ESI can only be set once per ATM device, whereas the same ATM
device can have multiple atm-bridge instances.

- Felix
Mathias Kresin July 3, 2016, 5:57 p.m. UTC | #2
Hey Felix,

first off all, I've to admin that don't have much knowledge about ATM 
and the more I read about ATM the more I get confused.

The biggest flaw of my patchset is, that I've never described which 
issue I'm trying to fix.

It has been tried for ages now to fix a race condition between applying 
a MAC address to the nas interface and requesting an ip-address via dhcp 
over nas0 for the DGN3500. Which results in an invalid MAC used by the 
dhcp client.

All attempts to fix the issue had either side effects on other boards or 
got broken over time.

03.07.2016 09:47 Felix Fietkau:
> On 2016-07-02 17:00, Mathias Kresin wrote:
>> If an ESI is set, it will be used as MAC address for the nas0
>> interface.
>>
>> According to the ESI man page, changes to the ESI are not automatically
>> propagated throughout the system and therefore esi should be used
>> early during system.
>>
>> Setting the ESI before creating the nas interface should be early enough
>> for all following services.
>>
>> Signed-off-by: Mathias Kresin <dev@kresin.me>
> I'm not sure the atm-bridge section is the right place for the MAC
> address. ESI can only be set once per ATM device, whereas the same ATM
> device can have multiple atm-bridge instances.

I'm still convinced that the atm-bridge is the right place to set the 
MAC. By setting the ESI, the created nas interface is using the desired 
MAC address right from the beginning. No chance for race conditions.

Albeit you told me already that it's possible to have multiple 
atm-bridges for the same atm device, I've realized how it's working only 
now.

In theory adding support for multiple atm-bridge instances is easy. The 
esi command has a parameter which allows to force overwriting an 
existing ESI.

Using the br2684ctl command in background mode on the command line

/usr/sbin/esi -f "11223344556b" 0 && /usr/sbin/br2684ctl -b -c "0" -e 
"llc" -p "1" -a "0.7.35" -S /lib/netifd/br2684-up && /usr/sbin/esi -f 
"11223344556c" 0 && /usr/sbin/br2684ctl -b -c "1" -e "llc" -p "1" -a 
"0.8.30" -S /lib/netifd/br2684-up

works as expected. The resulting nas interfaces are having the desired 
MAC addresses:

nas0      Link encap:Ethernet  HWaddr 11:22:33:44:55:6B
nas1      Link encap:Ethernet  HWaddr 11:22:33:44:55:6C

But so far I wasn't able to get the same logic working in the brctl2684 
init script. The second esi call is executed before the first nas 
interface is created, with the result that both nas interfaces are using 
the same MAC address. I can only guess that it's related to the fact 
that br2684ctl is started in foreground mode using the exec command + 
the way procd is working.

Any help on this is highly welcome.

Mathias
diff mbox

Patch

diff --git a/package/network/utils/linux-atm/Makefile b/package/network/utils/linux-atm/Makefile
index 62d71ea..52f789c 100644
--- a/package/network/utils/linux-atm/Makefile
+++ b/package/network/utils/linux-atm/Makefile
@@ -77,6 +77,7 @@  endef
 
 define Package/br2684ctl
   $(call Package/linux-atm/Default)
+  DEPENDS+=+atm-esi
   TITLE:=ATM Ethernet bridging configuration utility
 endef
 
diff --git a/package/network/utils/linux-atm/files/br2684ctl b/package/network/utils/linux-atm/files/br2684ctl
index 0fa86bd..2563455 100755
--- a/package/network/utils/linux-atm/files/br2684ctl
+++ b/package/network/utils/linux-atm/files/br2684ctl
@@ -44,6 +44,9 @@  start_daemon() {
 	local sendsize
 	config_get sendsize "$cfg" sendsize
 
+	local mac
+	config_get mac "$cfg" mac
+
 	found=
 	for device in /sys/class/atm/*; do
 		[ -d "$device" ] || break
@@ -56,6 +59,8 @@  start_daemon() {
 
 	local circuit="$atmdev.$vpi.$vci"
 
+	[ -n "$mac" ] && /usr/sbin/esi "${mac//:/}" $unit
+
 	procd_open_instance
 	procd_set_param command \
 		/usr/sbin/br2684ctl_wrap "nas$unit" \