On Tue, 22 Oct 2019 at 09:51, David Ahern wrote:
>
Hi David,
Thank you for the review!
> On 10/21/19 12:47 PM, Taehee Yoo wrote:
> > Current code doesn't limit the number of nested devices.
> > Nested devices would be handled recursively and this needs huge stack
> > memory. So, unlimited nested devices could make stack overflow.
> >
> > This patch adds upper_level and lower_level, they are common variables
> > and represent maximum lower/upper depth.
> > When upper/lower device is attached or dettached,
> > {lower/upper}_level are updated. and if maximum depth is bigger than 8,
> > attach routine fails and returns -EMLINK.
> >
> > In addition, this patch converts recursive routine of
> > netdev_walk_all_{lower/upper} to iterator routine.
>
> They were made recursive because of a particular setup. Did you verify
> your changes did not break it? See commits starting with
> 5bb61cb5fd115bed1814f6b97417e0f397da3c79
>
I didn't change the actual logic of walking APIs.
These walking iterator APIs work as DFS.
So it doesn't break existing codes.
> >
> > Test commands:
> > ip link add dummy0 type dummy
> > ip link add link dummy0 name vlan1 type vlan id 1
> > ip link set vlan1 up
> >
> > for i in {2..55}
> > do
> > let A=$i-1
> >
> > ip link add vlan$i link vlan$A type vlan id $i
> > done
> > ip link del dummy0
>
> 8 levels of nested vlan seems like complete nonsense. Why not just limit
> that stacking and not mess with the rest which can affect real use cases?
VLAN, BONDING, TEAM, MACSEC, MACVLAN, IPVLAN, VIRT_WIFI, and VXLAN
These interface types can be nested and these could be combined.
team6
|
vlan5
|
team4
|
macvlan3
|
bond2
|
vlan1
|
dummy0
There are so many similar cases even they are not real use cases.
So I think generic code is needed.
Thank you
Taehee Yoo
