| 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
| ||
|
Message-ID: <23c07e81392bd5ae8f44a5270f91c6ca696baa31.camel@microchip.com>
Date: Mon, 21 Mar 2022 14:24:24 +0100
From: Steen Hegelund <steen.hegelund@...rochip.com>
To: Casper Andersson <casper.casan@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>, <netdev@...r.kernel.org>
CC: <UNGLinuxDriver@...rochip.com>
Subject: Re: [PATCH net-next 2/2] net: sparx5: Add mdb handlers
On Mon, 2022-03-21 at 11:14 +0100, Casper Andersson wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Adds mdb handlers. Uses the PGID arbiter to
> find a free entry in the PGID table for the
> multicast group port mask.
>
> Signed-off-by: Casper Andersson <casper.casan@...il.com>
> ---
> .../microchip/sparx5/sparx5_mactable.c | 33 ++++--
> .../ethernet/microchip/sparx5/sparx5_main.h | 2 +
> .../microchip/sparx5/sparx5_switchdev.c | 111 ++++++++++++++++++
> 3 files changed, 136 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c
> b/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c
> index 82b1b3c9a065..35abb3d0ce19 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c
> @@ -186,11 +186,11 @@ bool sparx5_mact_getnext(struct sparx5 *sparx5,
> return ret == 0;
> }
>
> -static int sparx5_mact_lookup(struct sparx5 *sparx5,
> - const unsigned char mac[ETH_ALEN],
> - u16 vid)
> +bool sparx5_mact_find(struct sparx5 *sparx5,
> + const unsigned char mac[ETH_ALEN], u16 vid, u32 *pcfg2)
> {
> int ret;
> + u32 cfg2;
>
> mutex_lock(&sparx5->lock);
>
> @@ -202,16 +202,29 @@ static int sparx5_mact_lookup(struct sparx5 *sparx5,
> sparx5, LRN_COMMON_ACCESS_CTRL);
>
> ret = sparx5_mact_wait_for_completion(sparx5);
> - if (ret)
> - goto out;
> -
> - ret = LRN_MAC_ACCESS_CFG_2_MAC_ENTRY_VLD_GET
> - (spx5_rd(sparx5, LRN_MAC_ACCESS_CFG_2));
> + if (ret == 0) {
> + cfg2 = spx5_rd(sparx5, LRN_MAC_ACCESS_CFG_2);
> + if (LRN_MAC_ACCESS_CFG_2_MAC_ENTRY_VLD_GET(cfg2))
> + *pcfg2 = cfg2;
> + else
> + ret = -ENOENT;
> + }
>
> -out:
> mutex_unlock(&sparx5->lock);
>
> - return ret;
> + return ret == 0;
> +}
> +
> +static int sparx5_mact_lookup(struct sparx5 *sparx5,
> + const unsigned char mac[ETH_ALEN],
> + u16 vid)
> +{
> + u32 pcfg2;
> +
> + if (sparx5_mact_find(sparx5, mac, vid, &pcfg2))
> + return 1;
> +
> + return 0;
> }
I suggest to drop this and only use your new function (or at least let it return a real error code
like -ENOENT in case of an error).
>
> int sparx5_mact_forget(struct sparx5 *sparx5,
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> index e97fa091c740..7a04b8f2a546 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> @@ -310,6 +310,8 @@ int sparx5_mact_learn(struct sparx5 *sparx5, int port,
> const unsigned char mac[ETH_ALEN], u16 vid);
> bool sparx5_mact_getnext(struct sparx5 *sparx5,
> unsigned char mac[ETH_ALEN], u16 *vid, u32 *pcfg2);
> +bool sparx5_mact_find(struct sparx5 *sparx5,
> + const unsigned char mac[ETH_ALEN], u16 vid, u32 *pcfg2);
> int sparx5_mact_forget(struct sparx5 *sparx5,
> const unsigned char mac[ETH_ALEN], u16 vid);
> int sparx5_add_mact_entry(struct sparx5 *sparx5,
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_switchdev.c
> b/drivers/net/ethernet/microchip/sparx5/sparx5_switchdev.c
> index 2d5de1c06fab..9e1ea35d0c40 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_switchdev.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_switchdev.c
> @@ -366,6 +366,109 @@ static int sparx5_handle_port_vlan_add(struct net_device *dev,
> v->flags & BRIDGE_VLAN_INFO_UNTAGGED);
> }
>
> +static int sparx5_handle_port_mdb_add(struct net_device *dev,
> + struct notifier_block *nb,
> + const struct switchdev_obj_port_mdb *v)
> +{
> + struct sparx5_port *port = netdev_priv(dev);
> + struct sparx5 *spx5 = port->sparx5;
> + u16 pgid_idx, vid;
> + u32 mact_entry;
> + int res, err;
> +
> + /* When VLAN unaware the vlan value is not parsed and we receive vid 0.
> + * Fall back to bridge vid 1.
> + */
> + if (!br_vlan_enabled(spx5->hw_bridge_dev))
> + vid = 1;
> + else
> + vid = v->vid;
> +
> + res = sparx5_mact_find(spx5, v->addr, vid, &mact_entry);
> +
> + if (res) {
> + pgid_idx = LRN_MAC_ACCESS_CFG_2_MAC_ENTRY_ADDR_GET(mact_entry);
> +
> + /* MC_IDX has an offset of 65 in the PGID table. */
> + pgid_idx += PGID_MCAST_START;
This will overlap some of the first ports with the flood masks according to:
https://microchip-ung.github.io/sparx-5_reginfo/reginfo_sparx-5.html?select=ana_ac,pgid
You should use the custom area (PGID_BASE + 8 and onwards) for this new feature.
> + sparx5_pgid_update_mask(port, pgid_idx, true);
> + } else {
> + err = sparx5_pgid_alloc_mcast(spx5, &pgid_idx);
> + if (err) {
> + netdev_warn(dev, "multicast pgid table full\n");
> + return err;
> + }
> + sparx5_pgid_update_mask(port, pgid_idx, true);
> + err = sparx5_mact_learn(spx5, pgid_idx, v->addr, vid);
> + if (err) {
> + netdev_warn(dev, "could not learn mac address %pM\n", v->addr);
> + sparx5_pgid_update_mask(port, pgid_idx, false);
> + return err;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int sparx5_mdb_del_entry(struct net_device *dev,
> + struct sparx5 *spx5,
> + const unsigned char mac[ETH_ALEN],
> + const u16 vid,
> + u16 pgid_idx)
> +{
> + int err;
> +
> + err = sparx5_mact_forget(spx5, mac, vid);
> + if (err) {
> + netdev_warn(dev, "could not forget mac address %pM", mac);
> + return err;
> + }
> + err = sparx5_pgid_free(spx5, pgid_idx);
> + if (err) {
> + netdev_err(dev, "attempted to free already freed pgid\n");
> + return err;
> + }
> + return 0;
> +}
> +
> +static int sparx5_handle_port_mdb_del(struct net_device *dev,
> + struct notifier_block *nb,
> + const struct switchdev_obj_port_mdb *v)
> +{
> + struct sparx5_port *port = netdev_priv(dev);
> + struct sparx5 *spx5 = port->sparx5;
> + u16 pgid_idx, vid;
> + u32 mact_entry, res, pgid_entry[3];
> + int err;
> +
> + if (!br_vlan_enabled(spx5->hw_bridge_dev))
> + vid = 1;
> + else
> + vid = v->vid;
> +
> + res = sparx5_mact_find(spx5, v->addr, vid, &mact_entry);
> +
> + if (res) {
> + pgid_idx = LRN_MAC_ACCESS_CFG_2_MAC_ENTRY_ADDR_GET(mact_entry);
> +
> + /* MC_IDX has an offset of 65 in the PGID table. */
> + pgid_idx += PGID_MCAST_START;
> + sparx5_pgid_update_mask(port, pgid_idx, false);
> +
> + pgid_entry[0] = spx5_rd(spx5, ANA_AC_PGID_CFG(pgid_idx));
> + pgid_entry[1] = spx5_rd(spx5, ANA_AC_PGID_CFG1(pgid_idx));
> + pgid_entry[2] = spx5_rd(spx5, ANA_AC_PGID_CFG2(pgid_idx));
> + if (pgid_entry[0] == 0 && pgid_entry[1] == 0 && pgid_entry[2] == 0) {
Looks like you could use a function that gets the pgid port mask (the inverse of the
sparx5_pgid_update_mask() function)
> + /* No ports are in MC group. Remove entry */
> + err = sparx5_mdb_del_entry(dev, spx5, v->addr, vid, pgid_idx);
> + if (err)
> + return err;
> + }
> + }
> +
> + return 0;
> +}
> +
> static int sparx5_handle_port_obj_add(struct net_device *dev,
> struct notifier_block *nb,
> struct switchdev_notifier_port_obj_info *info)
> @@ -378,6 +481,10 @@ static int sparx5_handle_port_obj_add(struct net_device *dev,
> err = sparx5_handle_port_vlan_add(dev, nb,
> SWITCHDEV_OBJ_PORT_VLAN(obj));
> break;
> + case SWITCHDEV_OBJ_ID_PORT_MDB:
> + err = sparx5_handle_port_mdb_add(dev, nb,
> + SWITCHDEV_OBJ_PORT_MDB(obj));
> + break;
> default:
> err = -EOPNOTSUPP;
> break;
> @@ -426,6 +533,10 @@ static int sparx5_handle_port_obj_del(struct net_device *dev,
> err = sparx5_handle_port_vlan_del(dev, nb,
> SWITCHDEV_OBJ_PORT_VLAN(obj)->vid);
> break;
> + case SWITCHDEV_OBJ_ID_PORT_MDB:
> + err = sparx5_handle_port_mdb_del(dev, nb,
> + SWITCHDEV_OBJ_PORT_MDB(obj));
> + break;
> default:
> err = -EOPNOTSUPP;
> break;
> --
> 2.30.2
>
Best Regards
Steen
Powered by blists - more mailing lists