| 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: <133b8da8-a2da-4bac-b0bb-7dcaebc219b9@redhat.com>
Date: Tue, 7 Jan 2025 10:42:27 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: MD Danish Anwar <danishanwar@...com>, Jeongjun Park
<aha310510@...il.com>, Alexander Lobakin <aleksander.lobakin@...el.com>,
Lukasz Majewski <lukma@...x.de>, Meghana Malladi <m-malladi@...com>,
Diogo Ivo <diogo.ivo@...mens.com>, Simon Horman <horms@...nel.org>,
Jakub Kicinski <kuba@...nel.org>, Eric Dumazet <edumazet@...gle.com>,
"David S. Miller" <davem@...emloft.net>, Andrew Lunn
<andrew+netdev@...n.ch>, Roger Quadros <rogerq@...nel.org>
Cc: linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, srk@...com,
Vignesh Raghavendra <vigneshr@...com>,
Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>,
Larysa Zaremba <larysa.zaremba@...el.com>
Subject: Re: [PATCH net-next v3 2/3] net: ti: icssg-prueth: Add Multicast
Filtering support for VLAN in MAC mode
On 1/3/25 10:20 AM, MD Danish Anwar wrote:
> Add multicast filtering support for VLAN interfaces in dual EMAC mode
> for ICSSG driver.
>
> The driver uses vlan_for_each() API to get the list of available
> vlans. The driver then sync mc addr of vlan interface with a locally
> mainatined list emac->vlan_mcast_list[vid] using __hw_addr_sync_multiple()
> API.
>
> The driver then calls the sync / unsync callbacks and based on whether
> the ndev is vlan or not, driver passes appropriate vid to FDB helper
> functions.
>
> This commit also exports __hw_addr_sync_multiple() in order to use it
> from the ICSSG driver.
>
> Signed-off-by: MD Danish Anwar <danishanwar@...com>
> ---
> drivers/net/ethernet/ti/icssg/icssg_prueth.c | 67 ++++++++++++++++----
> drivers/net/ethernet/ti/icssg/icssg_prueth.h | 6 ++
> include/linux/netdevice.h | 3 +
> net/core/dev_addr_lists.c | 7 +-
> 4 files changed, 66 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> index 1663941e59e3..ed8b5a3184d6 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> @@ -472,30 +472,44 @@ const struct icss_iep_clockops prueth_iep_clockops = {
>
> static int icssg_prueth_add_mcast(struct net_device *ndev, const u8 *addr)
> {
> - struct prueth_emac *emac = netdev_priv(ndev);
> - int port_mask = BIT(emac->port_id);
> + struct net_device *real_dev;
> + struct prueth_emac *emac;
> + int port_mask;
> + u8 vlan_id;
>
> - port_mask |= icssg_fdb_lookup(emac, addr, 0);
> - icssg_fdb_add_del(emac, addr, 0, port_mask, true);
> - icssg_vtbl_modify(emac, 0, port_mask, port_mask, true);
> + vlan_id = is_vlan_dev(ndev) ? vlan_dev_vlan_id(ndev) : PRUETH_DFLT_VLAN_MAC;
> + real_dev = is_vlan_dev(ndev) ? vlan_dev_real_dev(ndev) : ndev;
> + emac = netdev_priv(real_dev);
> +
> + port_mask = BIT(emac->port_id) | icssg_fdb_lookup(emac, addr, vlan_id);
> + icssg_fdb_add_del(emac, addr, vlan_id, port_mask, true);
> + icssg_vtbl_modify(emac, vlan_id, port_mask, port_mask, true);
>
> return 0;
> }
>
> static int icssg_prueth_del_mcast(struct net_device *ndev, const u8 *addr)
> {
> - struct prueth_emac *emac = netdev_priv(ndev);
> - int port_mask = BIT(emac->port_id);
> + struct net_device *real_dev;
> + struct prueth_emac *emac;
> int other_port_mask;
> + int port_mask;
> + u8 vlan_id;
> +
> + vlan_id = is_vlan_dev(ndev) ? vlan_dev_vlan_id(ndev) : PRUETH_DFLT_VLAN_MAC;
> + real_dev = is_vlan_dev(ndev) ? vlan_dev_real_dev(ndev) : ndev;
> + emac = netdev_priv(real_dev);
>
> - other_port_mask = port_mask ^ icssg_fdb_lookup(emac, addr, 0);
> + port_mask = BIT(emac->port_id);
> + other_port_mask = port_mask ^ icssg_fdb_lookup(emac, addr, vlan_id);
>
> - icssg_fdb_add_del(emac, addr, 0, port_mask, false);
> - icssg_vtbl_modify(emac, 0, port_mask, port_mask, false);
> + icssg_fdb_add_del(emac, addr, vlan_id, port_mask, false);
> + icssg_vtbl_modify(emac, vlan_id, port_mask, port_mask, false);
>
> if (other_port_mask) {
> - icssg_fdb_add_del(emac, addr, 0, other_port_mask, true);
> - icssg_vtbl_modify(emac, 0, other_port_mask, other_port_mask, true);
> + icssg_fdb_add_del(emac, addr, vlan_id, other_port_mask, true);
> + icssg_vtbl_modify(emac, vlan_id, other_port_mask,
> + other_port_mask, true);
> }
>
> return 0;
> @@ -531,6 +545,25 @@ static int icssg_prueth_hsr_del_mcast(struct net_device *ndev, const u8 *addr)
> return 0;
> }
>
> +static int icssg_update_vlan_mcast(struct net_device *vdev, int vid,
> + void *args)
> +{
> + struct prueth_emac *emac = args;
> +
> + if (!vdev || !vid)
> + return 0;
> +
> + netif_addr_lock_bh(vdev);
> + __hw_addr_sync_multiple(&emac->vlan_mcast_list[vid], &vdev->mc,
> + vdev->addr_len);
> + netif_addr_unlock_bh(vdev);
At this point, isn't emac->vlan_mcast_list[vid] == vdev->mc?
> +
> + __hw_addr_sync_dev(&emac->vlan_mcast_list[vid], vdev,
> + icssg_prueth_add_mcast, icssg_prueth_del_mcast);
If so, can this function be reduced to just:
__dev_mc_sync(vdev, icssg_prueth_add_mcast, icssg_prueth_del_mcast);
?
> diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c
> index 166e404f7c03..90716bd736f3 100644
> --- a/net/core/dev_addr_lists.c
> +++ b/net/core/dev_addr_lists.c
> @@ -242,9 +242,9 @@ static void __hw_addr_unsync_one(struct netdev_hw_addr_list *to_list,
> __hw_addr_del_entry(from_list, ha, false, false);
> }
>
> -static int __hw_addr_sync_multiple(struct netdev_hw_addr_list *to_list,
> - struct netdev_hw_addr_list *from_list,
> - int addr_len)
> +int __hw_addr_sync_multiple(struct netdev_hw_addr_list *to_list,
> + struct netdev_hw_addr_list *from_list,
> + int addr_len)
> {
> int err = 0;
> struct netdev_hw_addr *ha, *tmp;
> @@ -260,6 +260,7 @@ static int __hw_addr_sync_multiple(struct netdev_hw_addr_list *to_list,
> }
> return err;
> }
> +EXPORT_SYMBOL(__hw_addr_sync_multiple);
I'm asking because this additional export looks suspect. How other
drivers cope with similar situation?
Thanks!
Paolo
Powered by blists - more mailing lists