netdev - Re: [Patch net] net: change addr_list

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200618224632.GE279339@lunn.ch>
Date:   Fri, 19 Jun 2020 00:46:32 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     Cong Wang <xiyou.wangcong@...il.com>,
        netdev <netdev@...r.kernel.org>,
        syzbot+f3a0e80c34b3fc28ac5e@...kaller.appspotmail.com,
        Taehee Yoo <ap420073@...il.com>,
        Dmitry Vyukov <dvyukov@...gle.com>
Subject: Re: [Patch net] net: change addr_list_lock back to static key

> > Hi Vladimir
> >
> > So you are suggesting this?
> >
> > > > +       ret = netdev_upper_dev_link(master, slave_dev, NULL);
> >
> >   Andrew
> 
> Yes, basically this:
> 
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 4c7f086a047b..6aff8cfc9cf1 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -1807,6 +1807,13 @@ int dsa_slave_create(struct dsa_port *port)
>                            ret, slave_dev->name);
>                 goto out_phy;
>         }
> +       rtnl_lock();
> +       ret = netdev_upper_dev_link(master, slave_dev, NULL);
> +       rtnl_unlock();
> +       if (ret) {
> +               unregister_netdevice(slave_dev);
> +               goto out_phy;
> +       }
> 
>         return 0;
> 
> @@ -1826,12 +1833,14 @@ int dsa_slave_create(struct dsa_port *port)
> 
>  void dsa_slave_destroy(struct net_device *slave_dev)
>  {
> +       struct net_device *master = dsa_slave_to_master(slave_dev);
>         struct dsa_port *dp = dsa_slave_to_port(slave_dev);
>         struct dsa_slave_priv *p = netdev_priv(slave_dev);
> 
>         netif_carrier_off(slave_dev);
>         rtnl_lock();
>         phylink_disconnect_phy(dp->pl);
> +       netdev_upper_dev_unlink(master, slave_dev);
>         rtnl_unlock();
> 
>         dsa_slave_notify(slave_dev, DSA_PORT_UNREGISTER);
> 
> Do you see a problem with it?

I was initially not sure you could do this. But it looks like you can
have N : M relationships between uppers and lowers. I suppose it does
make sense. You can have multiple VLAN uppers to one base device. You
can have multiple lowers to one bond device, etc.

I wonder what 'side effects' there are for declaring this linkage. It
is not something i've looked into before, since we never used it. So i
don't see a problem with this, other than i don't know what problems
we might run into :-)

  Andrew