Re: [PATCH net 00/11] net: fix nested device bugs

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu,  5 Sep 2019 03:38:28 +0900
Taehee Yoo <ap420073@gmail.com> wrote:

> This patchset fixes several bugs that are related to nesting
> device infrastructure.
> Current nesting infrastructure code doesn't limit the depth level of
> devices. nested devices could be handled recursively. at that moment,
> it needs huge memory and stack overflow could occur.
> Below devices type have same bug.
> VLAN, BONDING, TEAM, MACSEC, MACVLAN and VXLAN.
> 
> Test commands:
>     ip link add dummy0 type dummy
>     ip link add vlan1 link dummy0 type vlan id 1
> 
>     for i in {2..100}
>     do
> 	    let A=$i-1
> 	    ip link add name vlan$i link vlan$A type vlan id $i
>     done
>     ip link del dummy0
> 
> 1st patch actually fixes the root cause.
> It adds new common variables {upper/lower}_level that represent
> depth level. upper_level variable is depth of upper devices.
> lower_level variable is depth of lower devices.
> 
>       [U][L]       [U][L]
> vlan1  1  5  vlan4  1  4
> vlan2  2  4  vlan5  2  3
> vlan3  3  3    |
>   |            |
>   +------------+
>   |
> vlan6  4  2
> dummy0 5  1
> 
> After this patch, the nesting infrastructure code uses this variable to
> check the depth level.
> 
> 2, 4, 5, 6, 7 patches fix lockdep related problem.
> Before this patch, devices use static lockdep map.
> So, if devices that are same type is nested, lockdep will warn about
> recursive situation.
> These patches make these devices use dynamic lockdep key instead of
> static lock or subclass.
> 
> 3rd patch splits IFF_BONDING flag into IFF_BONDING and IFF_BONDING_SLAVE.
> Before this patch, there is only IFF_BONDING flags, which means
> a bonding master or a bonding slave device.
> But this single flag could be problem when bonding devices are set to
> nested.
> 
> 8th patch fixes a refcnt leak in the macsec module.
> 
> 9th patch adds ignore flag to an adjacent structure.
> In order to exchange an adjacent node safely, ignore flag is needed.
> 
> 10th patch makes vxlan add an adjacent link to limit depth level.
> 
> 11th patch removes unnecessary variables and callback.
> 
> Taehee Yoo (11):
>   net: core: limit nested device depth
>   vlan: use dynamic lockdep key instead of subclass
>   bonding: split IFF_BONDING into IFF_BONDING and IFF_BONDING_SLAVE
>   bonding: use dynamic lockdep key instead of subclass
>   team: use dynamic lockdep key instead of static key
>   macsec: use dynamic lockdep key instead of subclass
>   macvlan: use dynamic lockdep key instead of subclass
>   macsec: fix refcnt leak in module exit routine
>   net: core: add ignore flag to netdev_adjacent structure
>   vxlan: add adjacent link to limit depth level
>   net: remove unnecessary variables and callback
> 
>  drivers/net/bonding/bond_alb.c                |   2 +-
>  drivers/net/bonding/bond_main.c               |  87 ++++--
>  .../net/ethernet/mellanox/mlx5/core/en_tc.c   |   2 +-
>  .../ethernet/qlogic/netxen/netxen_nic_main.c  |   2 +-
>  drivers/net/hyperv/netvsc_drv.c               |   3 +-
>  drivers/net/macsec.c                          |  50 ++--
>  drivers/net/macvlan.c                         |  36 ++-
>  drivers/net/team/team.c                       |  61 ++++-
>  drivers/net/vxlan.c                           |  71 ++++-
>  drivers/scsi/fcoe/fcoe.c                      |   2 +-
>  drivers/target/iscsi/cxgbit/cxgbit_cm.c       |   2 +-
>  include/linux/if_macvlan.h                    |   3 +-
>  include/linux/if_team.h                       |   5 +
>  include/linux/if_vlan.h                       |  13 +-
>  include/linux/netdevice.h                     |  29 +-
>  include/net/bonding.h                         |   4 +-
>  include/net/vxlan.h                           |   1 +
>  net/8021q/vlan.c                              |   1 -
>  net/8021q/vlan_dev.c                          |  32 +--
>  net/core/dev.c                                | 252 ++++++++++++++++--
>  net/core/dev_addr_lists.c                     |  12 +-
>  net/smc/smc_core.c                            |   2 +-
>  net/smc/smc_pnet.c                            |   2 +-
>  23 files changed, 519 insertions(+), 155 deletions(-)
> 

The network receive path already avoids excessive stack
depth. Maybe the real problem is in the lockdep code.