Re: [PATCH net] net: Have netpoll bring-up DSA management interface

[Vladimir Oltean]

On Mon, Oct 19, 2020 at 10:17:44AM -0700, Florian Fainelli wrote:
> DSA network devices rely on having their DSA management interface up and
> running otherwise their ndo_open() will return -ENETDOWN. Without doing
> this it would not be possible to use DSA devices as netconsole when
> configured on the command line. These devices also do not utilize the
> upper/lower linking so the check about the netpoll device having upper
> is not going to be a problem.
> 
> The solution adopted here is identical to the one done for
> net/ipv4/ipconfig.c with 728c02089a0e ("net: ipv4: handle DSA enabled
> master network devices"), with the network namespace scope being
> restricted to that of the process configuring netpoll.

Or to that of the swapper process, when netpoll_setup is called from
initcall context.

> 
> Fixes: 04ff53f96a93 ("net: dsa: Add netconsole support")
> Signed-off-by: Florian Fainelli <f.fainelli@xxxxxxxxx>
> ---

Tested-by: Vladimir Oltean <olteanv@xxxxxxxxx>

>  net/core/netpoll.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/net/core/netpoll.c b/net/core/netpoll.c
> index c310c7c1cef7..960948290001 100644
> --- a/net/core/netpoll.c
> +++ b/net/core/netpoll.c
> @@ -29,6 +29,7 @@
>  #include <linux/slab.h>
>  #include <linux/export.h>
>  #include <linux/if_vlan.h>
> +#include <net/dsa.h>
>  #include <net/tcp.h>
>  #include <net/udp.h>
>  #include <net/addrconf.h>
> @@ -657,15 +658,15 @@ EXPORT_SYMBOL_GPL(__netpoll_setup);
>  
>  int netpoll_setup(struct netpoll *np)
>  {
> -	struct net_device *ndev = NULL;
> +	struct net_device *ndev = NULL, *dev = NULL;
> +	struct net *net = current->nsproxy->net_ns;
>  	struct in_device *in_dev;
>  	int err;
>  
>  	rtnl_lock();
> -	if (np->dev_name[0]) {
> -		struct net *net = current->nsproxy->net_ns;
> +	if (np->dev_name[0])
>  		ndev = __dev_get_by_name(net, np->dev_name);
> -	}
> +
>  	if (!ndev) {
>  		np_err(np, "%s doesn't exist, aborting\n", np->dev_name);
>  		err = -ENODEV;
> @@ -673,6 +674,19 @@ int netpoll_setup(struct netpoll *np)
>  	}
>  	dev_hold(ndev);
>  
> +	/* bring up DSA management network devices up first */
> +	for_each_netdev(net, dev) {
> +		if (!netdev_uses_dsa(dev))
> +			continue;

Not amazing, but does the job.

> +
> +		err = dev_change_flags(dev, dev->flags | IFF_UP, NULL);
> +		if (err < 0) {
> +			np_err(np, "%s failed to open %s\n",
> +			       np->dev_name, dev->name);
> +			goto put;
> +		}
> +	}
> +
>  	if (netdev_master_upper_dev_get(ndev)) {
>  		np_err(np, "%s is a slave device, aborting\n", np->dev_name);
>  		err = -EBUSY;
> -- 
> 2.25.1
>