Re: [PATCH V2 2/3] net-next: dsa: add multi cpu port support

[Andrew Lunn]

On Tue, May 30, 2017 at 05:16:27PM -0700, Florian Fainelli wrote:
> On 05/30/2017 05:06 PM, Andrew Lunn wrote:
> >> - past the initial setup, if we start creating bridge devices and so on,
> >> we have no way to tell: group Ports 0-3 together and send traffic to CPU
> >> port 0, then let Port 5 alone and send traffic to CPU port 1, that's a
> >> DSA-only problem though, because we still have the CPU port(s) as
> >> independent network interfaces.
> > 
> > What is the problem here? Frames come out the master interface, get
> > untagged and passed to the slave interface and go upto the bridge. It
> > should all just work. Same in the reverse direction.
> 
> The problem is really that is you have multiple CPU ports, how do you
> define which one gets all the traffic by default? Ascending order of
> port number? Descending order?

I would probably default to round robin when allocating user ports to
CPU ports. That probably gives you the best default.

> I actually tend to think that most use cases our there are in the order
> of dedicating one CPU port to one corresponding switch port (user
> facing, or internal) in order to provided guaranteed bandwidth for that
> port.

Which is generally a waste of bandwidth. Best case, i get 40Mbps
Internet access. Meaning 960Mbps of a dedicated cpu port would be
wasted.

>  But as an user, I want to choose how the grouping is going to
> work, and right now, I cannot, unless this is hardcoded in Device Tree,
> which sounds both wrong and inadequate.

So how about round-robin default, and then devlink to move a user port
to a specific cpu port?

We also need to watch out for asymmetry. I think newer marvell chips
don't support egress to multiple CPU ports. Ingress to the switch i
think is unlimited. The older chips are more flexible in this
respect. So we need some degree of flexibility here.
 
Andrew

Re: [PATCH V2 2/3] net-next: dsa: add multi cpu port support

[Florian Fainelli]

On 05/30/2017 05:06 PM, Andrew Lunn wrote:
>> - past the initial setup, if we start creating bridge devices and so on,
>> we have no way to tell: group Ports 0-3 together and send traffic to CPU
>> port 0, then let Port 5 alone and send traffic to CPU port 1, that's a
>> DSA-only problem though, because we still have the CPU port(s) as
>> independent network interfaces.
> 
> What is the problem here? Frames come out the master interface, get
> untagged and passed to the slave interface and go upto the bridge. It
> should all just work. Same in the reverse direction.

The problem is really that is you have multiple CPU ports, how do you
define which one gets all the traffic by default? Ascending order of
port number? Descending order?

> 
> In order to make best use of the extra bandwidth of having two cpu
> ports, i probably want the user ports reasonably evenly distributed
> between the CPU ports. Dedicating one CPU port to one user port is
> probably sub-optimal. How many people have 1Gbps Fibre to the home,
> which could fully utilise a one-to-one mapping for the WAN port?

I actually tend to think that most use cases our there are in the order
of dedicating one CPU port to one corresponding switch port (user
facing, or internal) in order to provided guaranteed bandwidth for that
port. But as an user, I want to choose how the grouping is going to
work, and right now, I cannot, unless this is hardcoded in Device Tree,
which sounds both wrong and inadequate.

> 
>> Now, that would still force the user to configure two bridges in order
>> to properly steer traffic towards the requested ports but it would allow
>> us to be very flexible (which is probably desired here) in how ports are
>> grouped together.
> 
> We want a sensible default, spreading the slave ports evenly over the
> CPU ports. We could add a devlink command to change the defaults at
> runtime.

Sensible default is fine for the first time boot, but we should let
users be entirely flexible in how they want their user-facing ports to
map to a CPU port as you say, and IMHO using separate bridges to
configure that is a possible way to go since there is already knowledge
in the bridge join/leave code in DSA that already knows the
dwnstream/user-facing ports, but does not yet know about CPU ports.

Code speaks better, so let me see if I can cook something to illustrate
this.

Thanks!
-- 
Florian

Re: [PATCH V2 2/3] net-next: dsa: add multi cpu port support

[Andrew Lunn]

> - past the initial setup, if we start creating bridge devices and so on,
> we have no way to tell: group Ports 0-3 together and send traffic to CPU
> port 0, then let Port 5 alone and send traffic to CPU port 1, that's a
> DSA-only problem though, because we still have the CPU port(s) as
> independent network interfaces.

What is the problem here? Frames come out the master interface, get
untagged and passed to the slave interface and go upto the bridge. It
should all just work. Same in the reverse direction.

In order to make best use of the extra bandwidth of having two cpu
ports, i probably want the user ports reasonably evenly distributed
between the CPU ports. Dedicating one CPU port to one user port is
probably sub-optimal. How many people have 1Gbps Fibre to the home,
which could fully utilise a one-to-one mapping for the WAN port?

> Now, that would still force the user to configure two bridges in order
> to properly steer traffic towards the requested ports but it would allow
> us to be very flexible (which is probably desired here) in how ports are
> grouped together.

We want a sensible default, spreading the slave ports evenly over the
CPU ports. We could add a devlink command to change the defaults at
runtime.

Andrew

Re: [PATCH V2 2/3] net-next: dsa: add multi cpu port support

[Florian Fainelli]

+Jiri, Ido,

On 05/30/2017 03:44 AM, John Crispin wrote:
> Some boards have two CPU interfaces connected to the switch, e.g. WiFi
> access points, with 1 port labeled WAN, 4 ports labeled lan1-lan4, and
> two port connected to the SoC.
> 
> This patch extends DSA to allows both CPU ports to be used. The "cpu"
> node in the DSA tree can now have a phandle to the host interface it
> connects to. Each user port can have a phandle to a cpu port which
> should be used for traffic between the port and the CPU. Thus simple
> load sharing over the two CPU ports can be achieved.

Part of the problem here is that:

- we have the initial state where we only allow the user-ports to be
talking to the CPU port, now we have more than one CPU port, so which
one do we choose? You have proposed a Device Tree change for that, and
while this works, we are going beyond pure HW description and we are
already describing a desired policy, not ideal

- past the initial setup, if we start creating bridge devices and so on,
we have no way to tell: group Ports 0-3 together and send traffic to CPU
port 0, then let Port 5 alone and send traffic to CPU port 1, that's a
DSA-only problem though, because we still have the CPU port(s) as
independent network interfaces.

Right now, we cannot enslave master network devices into the bridge when
a tagging protocol is used (see
8db0a2ee2c6302a1dcbcdb93cb731dfc6c0cdb5e) so we cannot quite associate a
bridge device with its underlying CPU port.

I suppose we could revise that commit and let the enslaving happen in
order for the switch drivers to "learn" which CPU/master network device
is being added to the bridge, and just not register the RX handler for
that interface.

Now, that would still force the user to configure two bridges in order
to properly steer traffic towards the requested ports but it would allow
us to be very flexible (which is probably desired here) in how ports are
grouped together.

Does that make sense?

> 
> Signed-off-by: John Crispin 
> ---
>  include/net/dsa.h  | 21 -
>  net/dsa/dsa2.c | 35 +++
>  net/dsa/dsa_priv.h |  1 +
>  net/dsa/slave.c| 26 --
>  4 files changed, 68 insertions(+), 15 deletions(-)
> 
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index c0e567c0c824..d2994bd2c507 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -186,6 +186,8 @@ struct dsa_port {
>   u8  stp_state;
>   struct net_device   *bridge_dev;
>   struct devlink_port devlink_port;
> + struct net_device   *ethernet;
> + int upstream;
>  };
>  
>  struct dsa_switch {
> @@ -251,7 +253,7 @@ struct dsa_switch {
>  
>  static inline bool dsa_is_cpu_port(struct dsa_switch *ds, int p)
>  {
> - return ds->dst->cpu_dp == >ports[p];
> + return !!(ds->cpu_port_mask & (1 << p));
>  }
>  
>  static inline bool dsa_is_dsa_port(struct dsa_switch *ds, int p)
> @@ -269,6 +271,11 @@ static inline bool dsa_is_port_initialized(struct 
> dsa_switch *ds, int p)
>   return ds->enabled_port_mask & (1 << p) && ds->ports[p].netdev;
>  }
>  
> +static inline bool dsa_is_upstream_port(struct dsa_switch *ds, int p)
> +{
> + return dsa_is_cpu_port(ds, p) || dsa_is_dsa_port(ds, p);
> +}
> +
>  static inline u8 dsa_upstream_port(struct dsa_switch *ds)
>  {
>   struct dsa_switch_tree *dst = ds->dst;
> @@ -285,6 +292,18 @@ static inline u8 dsa_upstream_port(struct dsa_switch *ds)
>   return ds->rtable[dst->cpu_dp->ds->index];
>  }
>  
> +static inline u8 dsa_port_upstream_port(struct dsa_switch *ds, int port)
> +{
> + /*
> +  * If this port has a specific upstream cpu port, use it,
> +  * otherwise use the switch default.
> +  */
> + if (ds->ports[port].upstream)
> + return ds->ports[port].upstream;
> + else
> + return dsa_upstream_port(ds);
> +}
> +
>  struct dsa_switch_ops {
>   /*
>* Legacy probing.
> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
> index 4301f52e4f5a..8b13aa735c40 100644
> --- a/net/dsa/dsa2.c
> +++ b/net/dsa/dsa2.c
> @@ -253,8 +253,6 @@ static int dsa_cpu_port_apply(struct dsa_port *port, u32 
> index,
>   return err;
>   }
>  
> - ds->cpu_port_mask |= BIT(index);
> -
>   memset(>ports[index].devlink_port, 0,
>  sizeof(ds->ports[index].devlink_port));
>   err = devlink_port_register(ds->devlink, >ports[index].devlink_port,
> @@ -269,6 +267,10 @@ static void dsa_cpu_port_unapply(struct dsa_port *port, 
> u32 index,
>   dsa_cpu_dsa_destroy(port);
>   ds->cpu_port_mask &= ~BIT(index);
>  
> + if (ds->ports[index].ethernet) {
> + dev_put(ds->ports[index].ethernet);
> + ds->ports[index].ethernet = NULL;
> + }
>  }
>  
>  static int dsa_user_port_apply(struct dsa_port *port, u32 index,
> @@ -530,6 +532,29 @@ static int dsa_cpu_parse(struct dsa_port 

Re: [PATCH V2 2/3] net-next: dsa: add multi cpu port support

[Vivien Didelot]

Hi John,

Vivien Didelot  writes:

>> +int port_cpu = ds->ports[port].upstream;
>
> ds->ports[port] is p->dp.

I misread this part, p is not yet allocated in that chunk, please ignore
this one comment ;-)

Thanks,

Vivien

Re: [PATCH V2 2/3] net-next: dsa: add multi cpu port support

[Vivien Didelot]

Hi John,

John Crispin  writes:

> +static inline bool dsa_is_upstream_port(struct dsa_switch *ds, int p)
> +{
> + return dsa_is_cpu_port(ds, p) || dsa_is_dsa_port(ds, p);
> +}

This looks confusing to me. What DSA calls an "upstream" port for the
moment is the port which goes to the CPU interface.

 CPU0 (eth0)
   |
   | sw0 sw1 sw2
 [p0 p1 p2]--[p0 p1 p2]--[p0 p1 p2]
  |   |   |  |
  eth1 eth2eth3  eth4

So in the example above, sw1p0 is an upstream port, but sw1p2 is not.
This is why dsa_upstream_port makes use of ds->rtable.

> @@ -1140,11 +1140,16 @@ int dsa_slave_create(struct dsa_switch *ds, struct 
> device *parent,
>   struct net_device *master;
>   struct net_device *slave_dev;
>   struct dsa_slave_priv *p;
> + int port_cpu = ds->ports[port].upstream;

ds->ports[port] is p->dp.

>   int ret;
>  
> - master = ds->dst->master_netdev;
> - if (ds->master_netdev)
> + if (port_cpu && ds->ports[port_cpu].ethernet)

0 is a valid port index for a CPU, e.g. Marvell 88E6390.

> + master = ds->ports[port_cpu].ethernet;
> + else if (ds->master_netdev)
>   master = ds->master_netdev;
> + else
> + master = ds->dst->master_netdev;
> + master->dsa_ptr = (void *)ds->dst;
>  
>   slave_dev = alloc_netdev(sizeof(struct dsa_slave_priv), name,
>NET_NAME_UNKNOWN, ether_setup);
> @@ -1173,6 +1178,7 @@ int dsa_slave_create(struct dsa_switch *ds, struct 
> device *parent,
>   p->dp = >ports[port];
>   INIT_LIST_HEAD(>mall_tc_list);
>   p->xmit = dst->tag_ops->xmit;
> + p->master = master;

I'm a bit confused why we need all these references to net devices. We
now have ds->master_netdev, dst->master_netdev, dp->ethernet and
p->master...

Wouldn't it be simpler if we only had dp->ethernet (or whichever more
explicit name) for the conduit interface used to send/receive frames?

Maybe I am missing something, in which case I'm sorry in advance.


Thanks,

Vivien

Re: [PATCH V2 2/3] net-next: dsa: add multi cpu port support

[Florian Fainelli]

On 05/30/2017 12:15 PM, Florian Fainelli wrote:
> Hi John,
> 
> On 05/30/2017 11:37 AM, John Crispin wrote:
>> Hi,
>>
>> the patch series is based on net-next from 12 hours ago and works fine
>> on that tree. I compile and runtime tested it quite intensively on
>> various boards
> 
> The warning is legit though:
> 
> 572if (dsa_port_is_cpu(port))
> 573err = dsa_cpu_parse(port, index, dst, ds);
>   > 574else if (dsa_is_normal_port(port))
> 575err = dsa_user_parse(port->dn, index,  ds);
> 
> and after applying your patches I also met it:
> 
> net/dsa/dsa2.c: In function 'dsa_ds_parse':
> net/dsa/dsa2.c:574:3: warning: passing argument 1 of
> 'dsa_is_normal_port' from incompatible pointer type [enabled by default]
>else if (dsa_is_normal_port(port))
>^
> In file included from net/dsa/dsa_priv.h:17:0,
>  from net/dsa/dsa2.c:22:
> ./include/net/dsa.h:264:20: note: expected 'struct dsa_switch *' but
> argument is of type 'struct dsa_port *'
>  static inline bool dsa_is_normal_port(struct dsa_switch *ds, int p)
> ^
> net/dsa/dsa2.c:574:3: error: too few arguments to function
> 'dsa_is_normal_port'
>else if (dsa_is_normal_port(port))
>^
> In file included from net/dsa/dsa_priv.h:17:0,
>  from net/dsa/dsa2.c:22:
> ./include/net/dsa.h:264:20: note: declared here
>  static inline bool dsa_is_normal_port(struct dsa_switch *ds, int p)
> ^
>   CC  net/bridge/br_stp.o
> scripts/Makefile.build:302: recipe for target 'net/dsa/dsa2.o' failed
> make[4]: *** [net/dsa/dsa2.o] Error 1
> scripts/Makefile.build:561: recipe for target 'net/dsa' failed
> make[3]: *** [net/dsa] Error 2
> make[3]: *** Waiting for unfinished jobs
> 
> We need something like this, I have some comments on your patches that I
> will send shortly. Thanks!
> 
> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
> index 8b13aa735c40..124c5acfa123 100644
> --- a/net/dsa/dsa2.c
> +++ b/net/dsa/dsa2.c
> @@ -539,14 +539,14 @@ static int dsa_cpu_parse(struct dsa_port *port,
> u32 index,
> return 0;
>  }
> 
> -static int dsa_user_parse(struct device_node *port, u32 index,
> +static int dsa_user_parse(struct dsa_port *port, u32 index,
>   struct dsa_switch *ds)
>  {
> struct device_node *cpu_port;
> const unsigned int *cpu_port_reg;
> int cpu_port_index;
> 
> -   cpu_port = of_parse_phandle(port, "cpu", 0);
> +   cpu_port = of_parse_phandle(port->dn, "cpu", 0);
> if (cpu_port) {
> cpu_port_reg = of_get_property(cpu_port, "reg", NULL);
> if (!cpu_port_reg)
> @@ -572,7 +572,7 @@ static int dsa_ds_parse(struct dsa_switch_tree *dst,
> struct dsa_switch *ds)
> if (dsa_port_is_cpu(port))
> err = dsa_cpu_parse(port, index, dst, ds);
> else if (dsa_is_normal_port(port))
> -   err = dsa_user_parse(port->dn, index,  ds);
> +   err = dsa_user_parse(port, index,  ds);
> 
> if (err)
> return err;

here is a version that builds, I missed one hunk:

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 8b13aa735c40..d71a2a610340 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -539,14 +539,14 @@ static int dsa_cpu_parse(struct dsa_port *port,
u32 index,
return 0;
 }

-static int dsa_user_parse(struct device_node *port, u32 index,
+static int dsa_user_parse(struct dsa_port *port, u32 index,
  struct dsa_switch *ds)
 {
struct device_node *cpu_port;
const unsigned int *cpu_port_reg;
int cpu_port_index;

-   cpu_port = of_parse_phandle(port, "cpu", 0);
+   cpu_port = of_parse_phandle(port->dn, "cpu", 0);
if (cpu_port) {
cpu_port_reg = of_get_property(cpu_port, "reg", NULL);
if (!cpu_port_reg)
@@ -571,8 +571,8 @@ static int dsa_ds_parse(struct dsa_switch_tree *dst,
struct dsa_switch *ds)

if (dsa_port_is_cpu(port))
err = dsa_cpu_parse(port, index, dst, ds);
-   else if (dsa_is_normal_port(port))
-   err = dsa_user_parse(port->dn, index,  ds);
+   else if (dsa_is_normal_port(ds, index))
+   err = dsa_user_parse(port, index,  ds);

if (err)
return err;

-- 
Florian

Re: [PATCH V2 2/3] net-next: dsa: add multi cpu port support

[Florian Fainelli]

Hi John,

On 05/30/2017 11:37 AM, John Crispin wrote:
> Hi,
> 
> the patch series is based on net-next from 12 hours ago and works fine
> on that tree. I compile and runtime tested it quite intensively on
> various boards

The warning is legit though:

572if (dsa_port_is_cpu(port))
573err = dsa_cpu_parse(port, index, dst, ds);
  > 574else if (dsa_is_normal_port(port))
575err = dsa_user_parse(port->dn, index,  ds);

and after applying your patches I also met it:

net/dsa/dsa2.c: In function 'dsa_ds_parse':
net/dsa/dsa2.c:574:3: warning: passing argument 1 of
'dsa_is_normal_port' from incompatible pointer type [enabled by default]
   else if (dsa_is_normal_port(port))
   ^
In file included from net/dsa/dsa_priv.h:17:0,
 from net/dsa/dsa2.c:22:
./include/net/dsa.h:264:20: note: expected 'struct dsa_switch *' but
argument is of type 'struct dsa_port *'
 static inline bool dsa_is_normal_port(struct dsa_switch *ds, int p)
^
net/dsa/dsa2.c:574:3: error: too few arguments to function
'dsa_is_normal_port'
   else if (dsa_is_normal_port(port))
   ^
In file included from net/dsa/dsa_priv.h:17:0,
 from net/dsa/dsa2.c:22:
./include/net/dsa.h:264:20: note: declared here
 static inline bool dsa_is_normal_port(struct dsa_switch *ds, int p)
^
  CC  net/bridge/br_stp.o
scripts/Makefile.build:302: recipe for target 'net/dsa/dsa2.o' failed
make[4]: *** [net/dsa/dsa2.o] Error 1
scripts/Makefile.build:561: recipe for target 'net/dsa' failed
make[3]: *** [net/dsa] Error 2
make[3]: *** Waiting for unfinished jobs

We need something like this, I have some comments on your patches that I
will send shortly. Thanks!

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 8b13aa735c40..124c5acfa123 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -539,14 +539,14 @@ static int dsa_cpu_parse(struct dsa_port *port,
u32 index,
return 0;
 }

-static int dsa_user_parse(struct device_node *port, u32 index,
+static int dsa_user_parse(struct dsa_port *port, u32 index,
  struct dsa_switch *ds)
 {
struct device_node *cpu_port;
const unsigned int *cpu_port_reg;
int cpu_port_index;

-   cpu_port = of_parse_phandle(port, "cpu", 0);
+   cpu_port = of_parse_phandle(port->dn, "cpu", 0);
if (cpu_port) {
cpu_port_reg = of_get_property(cpu_port, "reg", NULL);
if (!cpu_port_reg)
@@ -572,7 +572,7 @@ static int dsa_ds_parse(struct dsa_switch_tree *dst,
struct dsa_switch *ds)
if (dsa_port_is_cpu(port))
err = dsa_cpu_parse(port, index, dst, ds);
else if (dsa_is_normal_port(port))
-   err = dsa_user_parse(port->dn, index,  ds);
+   err = dsa_user_parse(port, index,  ds);

if (err)
return err;


> 
> John
> 
> 
> On 30/05/17 17:38, kbuild test robot wrote:
>> Hi John,
>>
>> [auto build test ERROR on net-next/master]
>> [also build test ERROR on next-20170530]
>> [cannot apply to v4.12-rc3]
>> [if your patch is applied to the wrong git tree, please drop us a note
>> to help improve the system]
>>
>> url:   
>> https://github.com/0day-ci/linux/commits/John-Crispin/Documentation-devicetree-add-multiple-cpu-port-DSA-binding/20170530-224954
>>
>> config: x86_64-randconfig-x014-201722 (attached as .config)
>> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
>> reproduce:
>>  # save the attached .config to linux build tree
>>  make ARCH=x86_64
>>
>> All error/warnings (new ones prefixed by >>):
>>
>> In file included from include/linux/ioport.h:12:0,
>>  from include/linux/device.h:16,
>>  from net//dsa/dsa2.c:13:
>> net//dsa/dsa2.c: In function 'dsa_ds_parse':
 net//dsa/dsa2.c:574:31: error: passing argument 1 of
 'dsa_is_normal_port' from incompatible pointer type
 [-Werror=incompatible-pointer-types]
>>else if (dsa_is_normal_port(port))
>>^
>> include/linux/compiler.h:160:30: note: in definition of macro
>> '__trace_if'
>>   if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
>>   ^~~~
 net//dsa/dsa2.c:574:8: note: in expansion of macro 'if'
>>else if (dsa_is_normal_port(port))
>> ^~
>> In file included from net//dsa/dsa_priv.h:17:0,
>>  from net//dsa/dsa2.c:22:
>> include/net/dsa.h:264:20: note: expected 'struct dsa_switch *' but
>> argument is of type 'struct dsa_port *'
>>  static inline bool dsa_is_normal_port(struct dsa_switch *ds, int p)
>> ^~
>> In file included from include/linux/ioport.h:12:0,
>>  from include/linux/device.h:16,
>>  from net//dsa/dsa2.c:13:
 

Re: [PATCH V2 2/3] net-next: dsa: add multi cpu port support

[John Crispin]

Hi,

the patch series is based on net-next from 12 hours ago and works fine 
on that tree. I compile and runtime tested it quite intensively on 
various boards


John


On 30/05/17 17:38, kbuild test robot wrote:

Hi John,

[auto build test ERROR on net-next/master]
[also build test ERROR on next-20170530]
[cannot apply to v4.12-rc3]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/John-Crispin/Documentation-devicetree-add-multiple-cpu-port-DSA-binding/20170530-224954
config: x86_64-randconfig-x014-201722 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
 # save the attached .config to linux build tree
 make ARCH=x86_64

All error/warnings (new ones prefixed by >>):

In file included from include/linux/ioport.h:12:0,
 from include/linux/device.h:16,
 from net//dsa/dsa2.c:13:
net//dsa/dsa2.c: In function 'dsa_ds_parse':

net//dsa/dsa2.c:574:31: error: passing argument 1 of 'dsa_is_normal_port' from 
incompatible pointer type [-Werror=incompatible-pointer-types]

   else if (dsa_is_normal_port(port))
   ^
include/linux/compiler.h:160:30: note: in definition of macro '__trace_if'
  if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
  ^~~~

net//dsa/dsa2.c:574:8: note: in expansion of macro 'if'

   else if (dsa_is_normal_port(port))
^~
In file included from net//dsa/dsa_priv.h:17:0,
 from net//dsa/dsa2.c:22:
include/net/dsa.h:264:20: note: expected 'struct dsa_switch *' but argument 
is of type 'struct dsa_port *'
 static inline bool dsa_is_normal_port(struct dsa_switch *ds, int p)
^~
In file included from include/linux/ioport.h:12:0,
 from include/linux/device.h:16,
 from net//dsa/dsa2.c:13:

net//dsa/dsa2.c:574:12: error: too few arguments to function 
'dsa_is_normal_port'

   else if (dsa_is_normal_port(port))
^
include/linux/compiler.h:160:30: note: in definition of macro '__trace_if'
  if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
  ^~~~

net//dsa/dsa2.c:574:8: note: in expansion of macro 'if'

   else if (dsa_is_normal_port(port))
^~
In file included from net//dsa/dsa_priv.h:17:0,
 from net//dsa/dsa2.c:22:
include/net/dsa.h:264:20: note: declared here
 static inline bool dsa_is_normal_port(struct dsa_switch *ds, int p)
^~
In file included from include/linux/ioport.h:12:0,
 from include/linux/device.h:16,
 from net//dsa/dsa2.c:13:

net//dsa/dsa2.c:574:31: error: passing argument 1 of 'dsa_is_normal_port' from 
incompatible pointer type [-Werror=incompatible-pointer-types]

   else if (dsa_is_normal_port(port))
   ^
include/linux/compiler.h:160:42: note: in definition of macro '__trace_if'
  if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
  ^~~~

net//dsa/dsa2.c:574:8: note: in expansion of macro 'if'

   else if (dsa_is_normal_port(port))
^~
In file included from net//dsa/dsa_priv.h:17:0,
 from net//dsa/dsa2.c:22:
include/net/dsa.h:264:20: note: expected 'struct dsa_switch *' but argument 
is of type 'struct dsa_port *'
 static inline bool dsa_is_normal_port(struct dsa_switch *ds, int p)
^~
In file included from include/linux/ioport.h:12:0,
 from include/linux/device.h:16,
 from net//dsa/dsa2.c:13:

net//dsa/dsa2.c:574:12: error: too few arguments to function 
'dsa_is_normal_port'

   else if (dsa_is_normal_port(port))
^
include/linux/compiler.h:160:42: note: in definition of macro '__trace_if'
  if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
  ^~~~

net//dsa/dsa2.c:574:8: note: in expansion of macro 'if'

   else if (dsa_is_normal_port(port))
^~
In file included from net//dsa/dsa_priv.h:17:0,
 from net//dsa/dsa2.c:22:
include/net/dsa.h:264:20: note: declared here
 static inline bool dsa_is_normal_port(struct dsa_switch *ds, int p)
^~
In file included from include/linux/ioport.h:12:0,
 from include/linux/device.h:16,
 from net//dsa/dsa2.c:13:

net//dsa/dsa2.c:574:31: error: passing argument 1 of 'dsa_is_normal_port' from 
incompatible pointer type [-Werror=incompatible-pointer-types]

   else if (dsa_is_normal_port(port))
   ^

Re: [PATCH V2 2/3] net-next: dsa: add multi cpu port support

[kbuild test robot]

Hi John,

[auto build test ERROR on net-next/master]
[also build test ERROR on next-20170530]
[cannot apply to v4.12-rc3]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/John-Crispin/Documentation-devicetree-add-multiple-cpu-port-DSA-binding/20170530-224954
config: x86_64-randconfig-x014-201722 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

   In file included from include/linux/ioport.h:12:0,
from include/linux/device.h:16,
from net//dsa/dsa2.c:13:
   net//dsa/dsa2.c: In function 'dsa_ds_parse':
>> net//dsa/dsa2.c:574:31: error: passing argument 1 of 'dsa_is_normal_port' 
>> from incompatible pointer type [-Werror=incompatible-pointer-types]
  else if (dsa_is_normal_port(port))
  ^
   include/linux/compiler.h:160:30: note: in definition of macro '__trace_if'
 if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
 ^~~~
>> net//dsa/dsa2.c:574:8: note: in expansion of macro 'if'
  else if (dsa_is_normal_port(port))
   ^~
   In file included from net//dsa/dsa_priv.h:17:0,
from net//dsa/dsa2.c:22:
   include/net/dsa.h:264:20: note: expected 'struct dsa_switch *' but argument 
is of type 'struct dsa_port *'
static inline bool dsa_is_normal_port(struct dsa_switch *ds, int p)
   ^~
   In file included from include/linux/ioport.h:12:0,
from include/linux/device.h:16,
from net//dsa/dsa2.c:13:
>> net//dsa/dsa2.c:574:12: error: too few arguments to function 
>> 'dsa_is_normal_port'
  else if (dsa_is_normal_port(port))
   ^
   include/linux/compiler.h:160:30: note: in definition of macro '__trace_if'
 if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
 ^~~~
>> net//dsa/dsa2.c:574:8: note: in expansion of macro 'if'
  else if (dsa_is_normal_port(port))
   ^~
   In file included from net//dsa/dsa_priv.h:17:0,
from net//dsa/dsa2.c:22:
   include/net/dsa.h:264:20: note: declared here
static inline bool dsa_is_normal_port(struct dsa_switch *ds, int p)
   ^~
   In file included from include/linux/ioport.h:12:0,
from include/linux/device.h:16,
from net//dsa/dsa2.c:13:
>> net//dsa/dsa2.c:574:31: error: passing argument 1 of 'dsa_is_normal_port' 
>> from incompatible pointer type [-Werror=incompatible-pointer-types]
  else if (dsa_is_normal_port(port))
  ^
   include/linux/compiler.h:160:42: note: in definition of macro '__trace_if'
 if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
 ^~~~
>> net//dsa/dsa2.c:574:8: note: in expansion of macro 'if'
  else if (dsa_is_normal_port(port))
   ^~
   In file included from net//dsa/dsa_priv.h:17:0,
from net//dsa/dsa2.c:22:
   include/net/dsa.h:264:20: note: expected 'struct dsa_switch *' but argument 
is of type 'struct dsa_port *'
static inline bool dsa_is_normal_port(struct dsa_switch *ds, int p)
   ^~
   In file included from include/linux/ioport.h:12:0,
from include/linux/device.h:16,
from net//dsa/dsa2.c:13:
>> net//dsa/dsa2.c:574:12: error: too few arguments to function 
>> 'dsa_is_normal_port'
  else if (dsa_is_normal_port(port))
   ^
   include/linux/compiler.h:160:42: note: in definition of macro '__trace_if'
 if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
 ^~~~
>> net//dsa/dsa2.c:574:8: note: in expansion of macro 'if'
  else if (dsa_is_normal_port(port))
   ^~
   In file included from net//dsa/dsa_priv.h:17:0,
from net//dsa/dsa2.c:22:
   include/net/dsa.h:264:20: note: declared here
static inline bool dsa_is_normal_port(struct dsa_switch *ds, int p)
   ^~
   In file included from include/linux/ioport.h:12:0,
from include/linux/device.h:16,
from net//dsa/dsa2.c:13:
>> net//dsa/dsa2.c:574:31: error: passing argument 1 of 'dsa_is_normal_port' 
>> from incompatible pointer type [-Werror=incompatible-pointer-types]
  else if (dsa_is_normal_port(port))
  ^
   include/linux/compiler.h:171:16: note: in definition of macro '__trace_if'
  __r = !!(cond); \
   ^~~~
>> net//dsa/dsa2.c:574:8: note: in expansion of macro 'if'
  else if (dsa_is_normal_port(port))
   ^~
   In file included from 

[PATCH V2 2/3] net-next: dsa: add multi cpu port support

[John Crispin]

Some boards have two CPU interfaces connected to the switch, e.g. WiFi
access points, with 1 port labeled WAN, 4 ports labeled lan1-lan4, and
two port connected to the SoC.

This patch extends DSA to allows both CPU ports to be used. The "cpu"
node in the DSA tree can now have a phandle to the host interface it
connects to. Each user port can have a phandle to a cpu port which
should be used for traffic between the port and the CPU. Thus simple
load sharing over the two CPU ports can be achieved.

Signed-off-by: John Crispin 
---
 include/net/dsa.h  | 21 -
 net/dsa/dsa2.c | 35 +++
 net/dsa/dsa_priv.h |  1 +
 net/dsa/slave.c| 26 --
 4 files changed, 68 insertions(+), 15 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index c0e567c0c824..d2994bd2c507 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -186,6 +186,8 @@ struct dsa_port {
u8  stp_state;
struct net_device   *bridge_dev;
struct devlink_port devlink_port;
+   struct net_device   *ethernet;
+   int upstream;
 };
 
 struct dsa_switch {
@@ -251,7 +253,7 @@ struct dsa_switch {
 
 static inline bool dsa_is_cpu_port(struct dsa_switch *ds, int p)
 {
-   return ds->dst->cpu_dp == >ports[p];
+   return !!(ds->cpu_port_mask & (1 << p));
 }
 
 static inline bool dsa_is_dsa_port(struct dsa_switch *ds, int p)
@@ -269,6 +271,11 @@ static inline bool dsa_is_port_initialized(struct 
dsa_switch *ds, int p)
return ds->enabled_port_mask & (1 << p) && ds->ports[p].netdev;
 }
 
+static inline bool dsa_is_upstream_port(struct dsa_switch *ds, int p)
+{
+   return dsa_is_cpu_port(ds, p) || dsa_is_dsa_port(ds, p);
+}
+
 static inline u8 dsa_upstream_port(struct dsa_switch *ds)
 {
struct dsa_switch_tree *dst = ds->dst;
@@ -285,6 +292,18 @@ static inline u8 dsa_upstream_port(struct dsa_switch *ds)
return ds->rtable[dst->cpu_dp->ds->index];
 }
 
+static inline u8 dsa_port_upstream_port(struct dsa_switch *ds, int port)
+{
+   /*
+* If this port has a specific upstream cpu port, use it,
+* otherwise use the switch default.
+*/
+   if (ds->ports[port].upstream)
+   return ds->ports[port].upstream;
+   else
+   return dsa_upstream_port(ds);
+}
+
 struct dsa_switch_ops {
/*
 * Legacy probing.
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 4301f52e4f5a..8b13aa735c40 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -253,8 +253,6 @@ static int dsa_cpu_port_apply(struct dsa_port *port, u32 
index,
return err;
}
 
-   ds->cpu_port_mask |= BIT(index);
-
memset(>ports[index].devlink_port, 0,
   sizeof(ds->ports[index].devlink_port));
err = devlink_port_register(ds->devlink, >ports[index].devlink_port,
@@ -269,6 +267,10 @@ static void dsa_cpu_port_unapply(struct dsa_port *port, 
u32 index,
dsa_cpu_dsa_destroy(port);
ds->cpu_port_mask &= ~BIT(index);
 
+   if (ds->ports[index].ethernet) {
+   dev_put(ds->ports[index].ethernet);
+   ds->ports[index].ethernet = NULL;
+   }
 }
 
 static int dsa_user_port_apply(struct dsa_port *port, u32 index,
@@ -530,6 +532,29 @@ static int dsa_cpu_parse(struct dsa_port *port, u32 index,
 
dst->rcv = dst->tag_ops->rcv;
 
+   dev_hold(ethernet_dev);
+   ds->ports[index].ethernet = ethernet_dev;
+   ds->cpu_port_mask |= BIT(index);
+
+   return 0;
+}
+
+static int dsa_user_parse(struct device_node *port, u32 index,
+ struct dsa_switch *ds)
+{
+   struct device_node *cpu_port;
+   const unsigned int *cpu_port_reg;
+   int cpu_port_index;
+
+   cpu_port = of_parse_phandle(port, "cpu", 0);
+   if (cpu_port) {
+   cpu_port_reg = of_get_property(cpu_port, "reg", NULL);
+   if (!cpu_port_reg)
+   return -EINVAL;
+   cpu_port_index = be32_to_cpup(cpu_port_reg);
+   ds->ports[index].upstream = cpu_port_index;
+   }
+
return 0;
 }
 
@@ -544,11 +569,13 @@ static int dsa_ds_parse(struct dsa_switch_tree *dst, 
struct dsa_switch *ds)
if (!dsa_port_is_valid(port))
continue;
 
-   if (dsa_port_is_cpu(port)) {
+   if (dsa_port_is_cpu(port))
err = dsa_cpu_parse(port, index, dst, ds);
+   else if (dsa_is_normal_port(port))
+   err = dsa_user_parse(port->dn, index,  ds);
+
if (err)
return err;
-   }
}
 
pr_info("DSA: switch %d %d parsed\n", dst->tree, ds->index);
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index c1d4180651af..91fdc16befb2 100644
--- a/net/dsa/dsa_priv.h
+++