| 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
| ||
|
Date: Thu, 11 Nov 2021 12:27:37 +0000
From: Alvin Šipraga <ALSI@...g-olufsen.dk>
To: Vladimir Oltean <vladimir.oltean@....com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC: Florian Fainelli <f.fainelli@...il.com>,
Andrew Lunn <andrew@...n.ch>,
Vivien Didelot <vivien.didelot@...il.com>,
Tobias Waldekranz <tobias@...dekranz.com>,
DENG Qingfang <dqfext@...il.com>
Subject: Re: [RFC PATCH net-next 2/6] net: dsa: assign a bridge number even
without TX forwarding offload
On 10/26/21 18:26, Vladimir Oltean wrote:
> The service where DSA assigns a unique bridge number for each forwarding
> domain is useful even for drivers which do not implement the TX
> forwarding offload feature.
>
> For example, drivers might use the dp->bridge_num for FDB isolation.
>
> So rename ds->num_fwd_offloading_bridges to ds->max_num_bridges, and
> calculate a unique bridge_num for all drivers that set this value.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
> ---
Thanks for this, it makes FDB isolation much more straightforward to handle.
Reviewed-by: Alvin Šipraga <alsi@...g-olufsen.dk>
> drivers/net/dsa/mv88e6xxx/chip.c | 4 +-
> drivers/net/dsa/sja1105/sja1105_main.c | 2 +-
> include/net/dsa.h | 10 ++--
> net/dsa/port.c | 81 +++++++++++++++++---------
> 4 files changed, 63 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 0d0ccb7f8ccd..ae09cbc2f42f 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -3183,8 +3183,8 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
> * time.
> */
> if (mv88e6xxx_has_pvt(chip))
> - ds->num_fwd_offloading_bridges = MV88E6XXX_MAX_PVT_SWITCHES -
> - ds->dst->last_switch - 1;
> + ds->max_num_bridges = MV88E6XXX_MAX_PVT_SWITCHES -
> + ds->dst->last_switch - 1;
>
> mv88e6xxx_reg_lock(chip);
>
> diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
> index c343effe2e96..355b56cf94d8 100644
> --- a/drivers/net/dsa/sja1105/sja1105_main.c
> +++ b/drivers/net/dsa/sja1105/sja1105_main.c
> @@ -3139,7 +3139,7 @@ static int sja1105_setup(struct dsa_switch *ds)
> ds->vlan_filtering_is_global = true;
> ds->untag_bridge_pvid = true;
> /* tag_8021q has 3 bits for the VBID, and the value 0 is reserved */
> - ds->num_fwd_offloading_bridges = 7;
> + ds->max_num_bridges = 7;
>
> /* Advertise the 8 egress queues */
> ds->num_tx_queues = SJA1105_NUM_TC;
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 56a90f05df49..970bc89a0062 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -413,12 +413,12 @@ struct dsa_switch {
> */
> unsigned int num_lag_ids;
>
> - /* Drivers that support bridge forwarding offload should set this to
> - * the maximum number of bridges spanning the same switch tree (or all
> - * trees, in the case of cross-tree bridging support) that can be
> - * offloaded.
> + /* Drivers that support bridge forwarding offload or FDB isolation
> + * should set this to the maximum number of bridges spanning the same
> + * switch tree (or all trees, in the case of cross-tree bridging
> + * support) that can be offloaded.
> */
> - unsigned int num_fwd_offloading_bridges;
> + unsigned int max_num_bridges;
>
> size_t num_ports;
> };
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index 1772817a1214..b7867746af15 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -271,19 +271,15 @@ static void dsa_port_switchdev_unsync_attrs(struct dsa_port *dp)
> }
>
> static void dsa_port_bridge_tx_fwd_unoffload(struct dsa_port *dp,
> - struct net_device *bridge_dev)
> + struct net_device *bridge_dev,
> + unsigned int bridge_num)
> {
> - unsigned int bridge_num = dp->bridge_num;
> struct dsa_switch *ds = dp->ds;
>
> /* No bridge TX forwarding offload => do nothing */
> - if (!ds->ops->port_bridge_tx_fwd_unoffload || !dp->bridge_num)
> + if (!ds->ops->port_bridge_tx_fwd_unoffload || !bridge_num)
> return;
>
> - dp->bridge_num = 0;
> -
> - dsa_bridge_num_put(bridge_dev, bridge_num);
> -
> /* Notify the chips only once the offload has been deactivated, so
> * that they can update their configuration accordingly.
> */
> @@ -292,31 +288,60 @@ static void dsa_port_bridge_tx_fwd_unoffload(struct dsa_port *dp,
> }
>
> static bool dsa_port_bridge_tx_fwd_offload(struct dsa_port *dp,
> - struct net_device *bridge_dev)
> + struct net_device *bridge_dev,
> + unsigned int bridge_num)
> {
> struct dsa_switch *ds = dp->ds;
> - unsigned int bridge_num;
> int err;
>
> - if (!ds->ops->port_bridge_tx_fwd_offload)
> - return false;
> -
> - bridge_num = dsa_bridge_num_get(bridge_dev,
> - ds->num_fwd_offloading_bridges);
> - if (!bridge_num)
> + /* FDB isolation is required for TX forwarding offload */
> + if (!ds->ops->port_bridge_tx_fwd_offload || !bridge_num)
> return false;
>
> - dp->bridge_num = bridge_num;
> -
> /* Notify the driver */
> err = ds->ops->port_bridge_tx_fwd_offload(ds, dp->index, bridge_dev,
> bridge_num);
> - if (err) {
> - dsa_port_bridge_tx_fwd_unoffload(dp, bridge_dev);
> - return false;
> +
> + return err ? false : true;
> +}
> +
> +static int dsa_port_bridge_create(struct dsa_port *dp,
> + struct net_device *br,
> + struct netlink_ext_ack *extack)
> +{
> + struct dsa_switch *ds = dp->ds;
> + unsigned int bridge_num;
> +
> + dp->bridge_dev = br;
> +
> + if (!ds->max_num_bridges)
> + return 0;
> +
> + bridge_num = dsa_bridge_num_get(br, ds->max_num_bridges);
> + if (!bridge_num) {
> + NL_SET_ERR_MSG_MOD(extack,
> + "Range of offloadable bridges exceeded");
> + return -EOPNOTSUPP;
> }
>
> - return true;
> + dp->bridge_num = bridge_num;
> +
> + return 0;
> +}
> +
> +static void dsa_port_bridge_destroy(struct dsa_port *dp,
> + const struct net_device *br)
> +{
> + struct dsa_switch *ds = dp->ds;
> +
> + dp->bridge_dev = NULL;
> +
> + if (ds->max_num_bridges) {
> + int bridge_num = dp->bridge_num;
> +
> + dp->bridge_num = 0;
> + dsa_bridge_num_put(br, bridge_num);
> + }
> }
>
> int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
> @@ -336,7 +361,9 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
> /* Here the interface is already bridged. Reflect the current
> * configuration so that drivers can program their chips accordingly.
> */
> - dp->bridge_dev = br;
> + err = dsa_port_bridge_create(dp, br, extack);
> + if (err)
> + return err;
>
> brport_dev = dsa_port_to_bridge_port(dp);
>
> @@ -344,7 +371,8 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
> if (err)
> goto out_rollback;
>
> - tx_fwd_offload = dsa_port_bridge_tx_fwd_offload(dp, br);
> + tx_fwd_offload = dsa_port_bridge_tx_fwd_offload(dp, br,
> + dp->bridge_num);
>
> err = switchdev_bridge_port_offload(brport_dev, dev, dp,
> &dsa_slave_switchdev_notifier,
> @@ -366,7 +394,7 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
> out_rollback_unbridge:
> dsa_broadcast(DSA_NOTIFIER_BRIDGE_LEAVE, &info);
> out_rollback:
> - dp->bridge_dev = NULL;
> + dsa_port_bridge_destroy(dp, br);
> return err;
> }
>
> @@ -393,14 +421,15 @@ void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br)
> .port = dp->index,
> .br = br,
> };
> + int bridge_num = dp->bridge_num;
> int err;
>
> /* Here the port is already unbridged. Reflect the current configuration
> * so that drivers can program their chips accordingly.
> */
> - dp->bridge_dev = NULL;
> + dsa_port_bridge_destroy(dp, br);
>
> - dsa_port_bridge_tx_fwd_unoffload(dp, br);
> + dsa_port_bridge_tx_fwd_unoffload(dp, br, bridge_num);
>
> err = dsa_broadcast(DSA_NOTIFIER_BRIDGE_LEAVE, &info);
> if (err)
>
Powered by blists - more mailing lists