[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z2PLDqqrLdXhLtAF@mev-dev.igk.intel.com>
Date: Thu, 19 Dec 2024 08:28:14 +0100
From: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
To: MD Danish Anwar <danishanwar@...com>
Cc: aleksander.lobakin@...el.com, lukma@...x.de, m-malladi@...com,
diogo.ivo@...mens.com, rdunlap@...radead.org,
schnelle@...ux.ibm.com, vladimir.oltean@....com, horms@...nel.org,
rogerq@...nel.org, pabeni@...hat.com, kuba@...nel.org,
edumazet@...gle.com, davem@...emloft.net, andrew+netdev@...n.ch,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
netdev@...r.kernel.org, srk@...com,
Vignesh Raghavendra <vigneshr@...com>
Subject: Re: [PATCH net-next 3/4] net: ti: icssg-prueth: Add Multicast
Filtering support for VLAN in MAC mode
On Mon, Dec 16, 2024 at 03:30:43PM +0530, 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 | 69 ++++++++++++++++----
> drivers/net/ethernet/ti/icssg/icssg_prueth.h | 6 ++
> include/linux/netdevice.h | 3 +
> net/core/dev_addr_lists.c | 7 +-
> 4 files changed, 68 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> index e031bccf31dc..a18773ef6eab 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> @@ -472,30 +472,43 @@ 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;
Looks like a helper for that can be nice to have.
> + 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 +544,28 @@ 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 net_device *vport_ndev;
> + struct prueth_emac *emac;
> +
> + if (!vdev || !vid)
> + return 0;
> +
> + vport_ndev = vlan_dev_real_dev(vdev);
> + emac = netdev_priv(vport_ndev);
> +
> + netif_addr_lock_bh(vdev);
> + __hw_addr_sync_multiple(&emac->vlan_mcast_list[vid], &vdev->mc, vdev->addr_len);
Only question, why dev_mc_sync_multiple can't be used here?
> + netif_addr_unlock_bh(vdev);
> +
> + __hw_addr_sync_dev(&emac->vlan_mcast_list[vid], vdev,
> + icssg_prueth_add_mcast, icssg_prueth_del_mcast);
> +
> + return 0;
> +}
> +
> /**
> * emac_ndo_open - EMAC device open
> * @ndev: network adapter device
> @@ -772,12 +807,17 @@ static void emac_ndo_set_rx_mode_work(struct work_struct *work)
> return;
> }
>
> - if (emac->prueth->is_hsr_offload_mode)
> + if (emac->prueth->is_hsr_offload_mode) {
> __dev_mc_sync(ndev, icssg_prueth_hsr_add_mcast,
> icssg_prueth_hsr_del_mcast);
> - else
> + } else {
> __dev_mc_sync(ndev, icssg_prueth_add_mcast,
> icssg_prueth_del_mcast);
> + if (rtnl_trylock()) {
> + vlan_for_each(ndev, icssg_update_vlan_mcast, NULL);
> + rtnl_unlock();
> + }
> + }
> }
>
> /**
> @@ -828,6 +868,7 @@ static int emac_ndo_vlan_rx_add_vid(struct net_device *ndev,
> if (prueth->is_hsr_offload_mode)
> port_mask |= BIT(PRUETH_PORT_HOST);
>
> + __hw_addr_init(&emac->vlan_mcast_list[vid]);
> netdev_err(emac->ndev, "VID add vid:%u port_mask:%X untag_mask %X\n",
> vid, port_mask, untag_mask);
>
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> index f5c1d473e9f9..4da8b87408b5 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> @@ -83,6 +83,10 @@
> #define ICSS_CMD_ADD_FILTER 0x7
> #define ICSS_CMD_ADD_MAC 0x8
>
> +/* VLAN Filtering Related MACROs */
> +#define PRUETH_DFLT_VLAN_MAC 0
> +#define MAX_VLAN_ID 256
> +
> /* In switch mode there are 3 real ports i.e. 3 mac addrs.
> * however Linux sees only the host side port. The other 2 ports
> * are the switch ports.
> @@ -201,6 +205,8 @@ struct prueth_emac {
> /* RX IRQ Coalescing Related */
> struct hrtimer rx_hrtimer;
> unsigned long rx_pace_timeout_ns;
> +
> + struct netdev_hw_addr_list vlan_mcast_list[MAX_VLAN_ID];
> };
>
> /**
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index d917949bba03..a5c169b19543 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -4685,6 +4685,9 @@ int devm_register_netdev(struct device *dev, struct net_device *ndev);
> /* General hardware address lists handling functions */
> int __hw_addr_sync(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);
> void __hw_addr_unsync(struct netdev_hw_addr_list *to_list,
> struct netdev_hw_addr_list *from_list, int addr_len);
> int __hw_addr_sync_dev(struct netdev_hw_addr_list *list,
> 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);
>
> /* This function only works where there is a strict 1-1 relationship
> * between source and destination of they synch. If you ever need to
> --
> 2.34.1
Powered by blists - more mailing lists