[OpenWrt-Devel] ipq40xx: remove redundant ucidef_set_interfaces_* in 02_network

If already included in ucidef_add_switch, you do not have to
additionally set the interface mode in ucidef_set_interfaces_*
functions.

This patch removes/adjusts such redundant cases.

Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>

---

This is not tested on the affected devices.

However, the same approach is taken for other targets, and
network setup is essentially device-independent concerning the
changes done here.
---
 target/linux/ipq40xx/base-files/etc/board.d/02_network | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

пн, 23 сент. 2019 г. в 16:34, Adrian Schmutzler <freifunk@adrianschmutzler.de>:
>
> If already included in ucidef_add_switch, you do not have to
> additionally set the interface mode in ucidef_set_interfaces_*
> functions.
>
> This patch removes/adjusts such redundant cases.
>
> Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
>
> ---
>
> This is not tested on the affected devices.
>
> However, the same approach is taken for other targets, and
> network setup is essentially device-independent concerning the
> changes done here.
> ---
>  target/linux/ipq40xx/base-files/etc/board.d/02_network | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/target/linux/ipq40xx/base-files/etc/board.d/02_network b/target/linux/ipq40xx/base-files/etc/board.d/02_network
> index e5ba7260f3..b7631a301c 100755
> --- a/target/linux/ipq40xx/base-files/etc/board.d/02_network
> +++ b/target/linux/ipq40xx/base-files/etc/board.d/02_network
> @@ -24,14 +24,14 @@ ipq40xx_setup_interfaces()
>                 ;;
>         asus,rt-ac58u|\
>         zyxel,nbg6617)
> -               ucidef_set_interfaces_lan_wan "eth0" "eth1"
> +               ucidef_set_interface_wan "eth1"
>                 ucidef_add_switch "switch0" \
>                         "0u@eth0" "1:lan:4" "2:lan:3" "3:lan:2" "4:lan:1"
>                 ;;

I planned to investigate this, but didn't find time yet. The config
initially is wrong.
For  asus,rt-ac58u|\
>         zyxel,nbg6617)

There should be port 5 on vid 2 for eth 1 to enable vlan working.
Currently saving any config in luci breaks vlans, because port 5 isn't
defined here. The port 5 is removed.
I suggest lo leave this alone for a while. I am going to find some
kind of a solultion and suggest it.

Hi Dmitry,

> -----Original Message-----
> From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org] On Behalf Of Dmitry Tunin
> Sent: Montag, 23. September 2019 17:12
> To: Adrian Schmutzler <freifunk@adrianschmutzler.de>
> Cc: OpenWrt Development List <openwrt-devel@lists.openwrt.org>
> Subject: Re: [OpenWrt-Devel] [PATCH] ipq40xx: remove redundant ucidef_set_interfaces_* in 02_network
> 
> пн, 23 сент. 2019 г. в 16:34, Adrian Schmutzler <freifunk@adrianschmutzler.de>:
> >
> > If already included in ucidef_add_switch, you do not have to
> > additionally set the interface mode in ucidef_set_interfaces_*
> > functions.
> >
> > This patch removes/adjusts such redundant cases.
> >
> > Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
> >
> > ---
> >
> > This is not tested on the affected devices.
> >
> > However, the same approach is taken for other targets, and
> > network setup is essentially device-independent concerning the
> > changes done here.
> > ---
> >  target/linux/ipq40xx/base-files/etc/board.d/02_network | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/target/linux/ipq40xx/base-files/etc/board.d/02_network b/target/linux/ipq40xx/base-files/etc/board.d/02_network
> > index e5ba7260f3..b7631a301c 100755
> > --- a/target/linux/ipq40xx/base-files/etc/board.d/02_network
> > +++ b/target/linux/ipq40xx/base-files/etc/board.d/02_network
> > @@ -24,14 +24,14 @@ ipq40xx_setup_interfaces()
> >                 ;;
> >         asus,rt-ac58u|\
> >         zyxel,nbg6617)
> > -               ucidef_set_interfaces_lan_wan "eth0" "eth1"
> > +               ucidef_set_interface_wan "eth1"
> >                 ucidef_add_switch "switch0" \
> >                         "0u@eth0" "1:lan:4" "2:lan:3" "3:lan:2" "4:lan:1"
> >                 ;;
> 
> I planned to investigate this, but didn't find time yet. The config
> initially is wrong.
> For  asus,rt-ac58u|\
> >         zyxel,nbg6617)
> 
> There should be port 5 on vid 2 for eth 1 to enable vlan working.
> Currently saving any config in luci breaks vlans, because port 5 isn't
> defined here. The port 5 is removed.
> I suggest lo leave this alone for a while. I am going to find some
> kind of a solultion and suggest it.

Fine with me, as my patch is purely cosmetical and low prio anyway.

Are you just talking about asus/zyxel case or should I remove the entire patch (-> mark as deferred)?

Best

Adrian


> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Hello Adrian,

On 9/23/19 3:34 PM, Adrian Schmutzler wrote:
>  	qxwlan,e2600ac-c1 |\
>  	qxwlan,e2600ac-c2)
> -		ucidef_set_interfaces_lan_wan "eth0" "eth1"
>  		ucidef_add_switch "switch0" \
>  			"0u@eth0" "3:lan" "4:lan" "0u@eth1" "5:wan"
>  		;;

While from the coding it looks like multiple cpu port distribution is supported,
I'm not sure if this is handled correctly for untagged ports.

Furthermore, the switch initialization is a bit "messed up", as the mapping between
external phy <-> ess-switch <--> cpu interfaces is not very obvious.

1) All devices with eth1 interface have it utilize the 5th PHY (displayed
as switchport 5)
2) All devices with a QCA8072 utilize PHY 4 (displayed as switchport 4) for eth0
3) Devices using an RGMII attached PHY (currently only MR33) do not have a switch-construct.

So if we really want to clean this up correctly, there are some more things to consider
(which also brings the opportunity to simplify this further). Also, the first point gives
you the opportunity to test the dual-untagged-cpuport on another ipq40xx device. :)

On the other hand there's ipqess and qca8k on the horizon to end the ar40xx mess.
But i don't think this is a showstopper for this.

Best wishes
David

> Are you just talking about asus/zyxel case or should I remove the entire patch (-> mark as deferred)?
I mean all ipq40xx devices with two MACs. I mentioned the two I have.

Hi David,
> On the other hand there's ipqess and qca8k on the horizon to end the ar40xx mess.
> But i don't think this is a showstopper for this.

That stopped me from doing this fast. ipqess will solve it anyway.
Maybe it is not worth the effort now. I don't see a straightforward
simple solution for now.