RE: [PATCH v5 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2 Ethernet Switch driver
> -Original Message- > From: Andrew Lunn [mailto:and...@lunn.ch] > Sent: Tuesday, March 13, 2018 5:26 PM > To: Razvan Stefanescu <razvan.stefane...@nxp.com> > Cc: gre...@linuxfoundation.org; de...@driverdev.osuosl.org; linux- > ker...@vger.kernel.org; net...@vger.kernel.org; Alexander Graf > <ag...@suse.de>; a...@arndb.de; Alexandru Marginean > <alexandru.margin...@nxp.com>; Ruxandra Ioana Ciocoi Radulescu > <ruxandra.radule...@nxp.com>; Ioana Ciornei <ioana.cior...@nxp.com>; > Laurentiu Tudor <laurentiu.tu...@nxp.com>; stuyo...@gmail.com > Subject: Re: [PATCH v5 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2 > Ethernet Switch driver > > > Hello Andrew, > > > > The current driver implementation uses only a single FDB for the switch, > > so it is not possible configure multiple flooding domains to accommodate > > ports partitioning. > > Ah, O.K. Rather than break somebodies network by wrongly flooding, it > would be better to return -EOPNOTSUPP when the requirement for the > second FDB is met. The offload to hardware will then not happen, and > the software bridge will do all the work. > Thank you for your suggestion. I will add to the TODO list in v6. Razvan > Andrew
RE: [PATCH v5 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2 Ethernet Switch driver
> -Original Message- > From: Andrew Lunn [mailto:and...@lunn.ch] > Sent: Tuesday, March 13, 2018 5:26 PM > To: Razvan Stefanescu > Cc: gre...@linuxfoundation.org; de...@driverdev.osuosl.org; linux- > ker...@vger.kernel.org; net...@vger.kernel.org; Alexander Graf > ; a...@arndb.de; Alexandru Marginean > ; Ruxandra Ioana Ciocoi Radulescu > ; Ioana Ciornei ; > Laurentiu Tudor ; stuyo...@gmail.com > Subject: Re: [PATCH v5 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2 > Ethernet Switch driver > > > Hello Andrew, > > > > The current driver implementation uses only a single FDB for the switch, > > so it is not possible configure multiple flooding domains to accommodate > > ports partitioning. > > Ah, O.K. Rather than break somebodies network by wrongly flooding, it > would be better to return -EOPNOTSUPP when the requirement for the > second FDB is met. The offload to hardware will then not happen, and > the software bridge will do all the work. > Thank you for your suggestion. I will add to the TODO list in v6. Razvan > Andrew
Re: [PATCH v5 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2 Ethernet Switch driver
> Hello Andrew, > > The current driver implementation uses only a single FDB for the switch, > so it is not possible configure multiple flooding domains to accommodate > ports partitioning. Ah, O.K. Rather than break somebodies network by wrongly flooding, it would be better to return -EOPNOTSUPP when the requirement for the second FDB is met. The offload to hardware will then not happen, and the software bridge will do all the work. Andrew
Re: [PATCH v5 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2 Ethernet Switch driver
> Hello Andrew, > > The current driver implementation uses only a single FDB for the switch, > so it is not possible configure multiple flooding domains to accommodate > ports partitioning. Ah, O.K. Rather than break somebodies network by wrongly flooding, it would be better to return -EOPNOTSUPP when the requirement for the second FDB is met. The offload to hardware will then not happen, and the software bridge will do all the work. Andrew
RE: [PATCH v5 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2 Ethernet Switch driver
> -Original Message-
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Tuesday, March 13, 2018 4:23 PM
> To: Razvan Stefanescu <razvan.stefane...@nxp.com>
> Cc: gre...@linuxfoundation.org; de...@driverdev.osuosl.org; linux-
> ker...@vger.kernel.org; net...@vger.kernel.org; Alexander Graf
> <ag...@suse.de>; a...@arndb.de; Alexandru Marginean
> <alexandru.margin...@nxp.com>; Ruxandra Ioana Ciocoi Radulescu
> <ruxandra.radule...@nxp.com>; Ioana Ciornei <ioana.cior...@nxp.com>;
> Laurentiu Tudor <laurentiu.tu...@nxp.com>; stuyo...@gmail.com
> Subject: Re: [PATCH v5 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2
> Ethernet Switch driver
>
> > +/* For the moment, only flood setting needs to be updated */
> > +static int port_bridge_join(struct net_device *netdev,
> > + struct net_device *upper_dev)
> > +{
> > + struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> > + struct ethsw_core *ethsw = port_priv->ethsw_data;
> > + int i, err;
> > +
> > + for (i = 0; i < ethsw->sw_attr.num_ifs; i++)
> > + if (ethsw->ports[i]->bridge_dev &&
> > + (ethsw->ports[i]->bridge_dev != upper_dev)) {
> > + netdev_err(netdev,
> > + "Another switch port is connected to %s\n",
> > + ethsw->ports[i]->bridge_dev->name);
> > + return -EINVAL;
> > + }
> > +
> > + /* Enable flooding */
> > + err = ethsw_port_set_flood(port_priv, 1);
> > + if (!err)
> > + port_priv->bridge_dev = upper_dev;
> > +
> > + return err;
> > +}
>
> Hi Razvan
>
> That is not what i was meaning.
>
> brctl addbr br0
> brctl addbr br1
> brctl addif br0 lan0
> brctl addif br0 lan1
> brctl addif br1 lan2
> brctl addif br1 lan3
>
> Is there somewhere in the code which sets the scope for the flooding?
> lan0 can flood to lan1, but it should not flood to lan2 or lan3, since
> they are in a different bridge. I was expecting that
> ethsw_port_set_flood() takes upper_dev, in order to configure which
> ports it should flood to.
>
> Andrew
Hello Andrew,
The current driver implementation uses only a single FDB for the switch,
so it is not possible configure multiple flooding domains to accommodate
ports partitioning.
The configuration that you mentioned will be possible when support for
multiple FDBs in the switch is added. Ports added to the same bridge
will have their flooding domain limited to a specific FDB.
Switch ports partitioning is a feature on the roadmap. I will add this
information to the TODO list.
Best regards,
Razvan Stefanescu
RE: [PATCH v5 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2 Ethernet Switch driver
> -Original Message-
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Tuesday, March 13, 2018 4:23 PM
> To: Razvan Stefanescu
> Cc: gre...@linuxfoundation.org; de...@driverdev.osuosl.org; linux-
> ker...@vger.kernel.org; net...@vger.kernel.org; Alexander Graf
> ; a...@arndb.de; Alexandru Marginean
> ; Ruxandra Ioana Ciocoi Radulescu
> ; Ioana Ciornei ;
> Laurentiu Tudor ; stuyo...@gmail.com
> Subject: Re: [PATCH v5 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2
> Ethernet Switch driver
>
> > +/* For the moment, only flood setting needs to be updated */
> > +static int port_bridge_join(struct net_device *netdev,
> > + struct net_device *upper_dev)
> > +{
> > + struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> > + struct ethsw_core *ethsw = port_priv->ethsw_data;
> > + int i, err;
> > +
> > + for (i = 0; i < ethsw->sw_attr.num_ifs; i++)
> > + if (ethsw->ports[i]->bridge_dev &&
> > + (ethsw->ports[i]->bridge_dev != upper_dev)) {
> > + netdev_err(netdev,
> > + "Another switch port is connected to %s\n",
> > + ethsw->ports[i]->bridge_dev->name);
> > + return -EINVAL;
> > + }
> > +
> > + /* Enable flooding */
> > + err = ethsw_port_set_flood(port_priv, 1);
> > + if (!err)
> > + port_priv->bridge_dev = upper_dev;
> > +
> > + return err;
> > +}
>
> Hi Razvan
>
> That is not what i was meaning.
>
> brctl addbr br0
> brctl addbr br1
> brctl addif br0 lan0
> brctl addif br0 lan1
> brctl addif br1 lan2
> brctl addif br1 lan3
>
> Is there somewhere in the code which sets the scope for the flooding?
> lan0 can flood to lan1, but it should not flood to lan2 or lan3, since
> they are in a different bridge. I was expecting that
> ethsw_port_set_flood() takes upper_dev, in order to configure which
> ports it should flood to.
>
> Andrew
Hello Andrew,
The current driver implementation uses only a single FDB for the switch,
so it is not possible configure multiple flooding domains to accommodate
ports partitioning.
The configuration that you mentioned will be possible when support for
multiple FDBs in the switch is added. Ports added to the same bridge
will have their flooding domain limited to a specific FDB.
Switch ports partitioning is a feature on the roadmap. I will add this
information to the TODO list.
Best regards,
Razvan Stefanescu
Re: [PATCH v5 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2 Ethernet Switch driver
> +/* For the moment, only flood setting needs to be updated */
> +static int port_bridge_join(struct net_device *netdev,
> + struct net_device *upper_dev)
> +{
> + struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> + struct ethsw_core *ethsw = port_priv->ethsw_data;
> + int i, err;
> +
> + for (i = 0; i < ethsw->sw_attr.num_ifs; i++)
> + if (ethsw->ports[i]->bridge_dev &&
> + (ethsw->ports[i]->bridge_dev != upper_dev)) {
> + netdev_err(netdev,
> +"Another switch port is connected to %s\n",
> +ethsw->ports[i]->bridge_dev->name);
> + return -EINVAL;
> + }
> +
> + /* Enable flooding */
> + err = ethsw_port_set_flood(port_priv, 1);
> + if (!err)
> + port_priv->bridge_dev = upper_dev;
> +
> + return err;
> +}
Hi Razvan
That is not what i was meaning.
brctl addbr br0
brctl addbr br1
brctl addif br0 lan0
brctl addif br0 lan1
brctl addif br1 lan2
brctl addif br1 lan3
Is there somewhere in the code which sets the scope for the flooding?
lan0 can flood to lan1, but it should not flood to lan2 or lan3, since
they are in a different bridge. I was expecting that
ethsw_port_set_flood() takes upper_dev, in order to configure which
ports it should flood to.
Andrew
Re: [PATCH v5 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2 Ethernet Switch driver
> +/* For the moment, only flood setting needs to be updated */
> +static int port_bridge_join(struct net_device *netdev,
> + struct net_device *upper_dev)
> +{
> + struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> + struct ethsw_core *ethsw = port_priv->ethsw_data;
> + int i, err;
> +
> + for (i = 0; i < ethsw->sw_attr.num_ifs; i++)
> + if (ethsw->ports[i]->bridge_dev &&
> + (ethsw->ports[i]->bridge_dev != upper_dev)) {
> + netdev_err(netdev,
> +"Another switch port is connected to %s\n",
> +ethsw->ports[i]->bridge_dev->name);
> + return -EINVAL;
> + }
> +
> + /* Enable flooding */
> + err = ethsw_port_set_flood(port_priv, 1);
> + if (!err)
> + port_priv->bridge_dev = upper_dev;
> +
> + return err;
> +}
Hi Razvan
That is not what i was meaning.
brctl addbr br0
brctl addbr br1
brctl addif br0 lan0
brctl addif br0 lan1
brctl addif br1 lan2
brctl addif br1 lan3
Is there somewhere in the code which sets the scope for the flooding?
lan0 can flood to lan1, but it should not flood to lan2 or lan3, since
they are in a different bridge. I was expecting that
ethsw_port_set_flood() takes upper_dev, in order to configure which
ports it should flood to.
Andrew
