On 21/05/2019 19:52:55+0300, Claudiu Manoil wrote:
> Doing kmalloc in atomic context is always an issue,
> more so for a list that can grow significantly.
> Turns out that the driver only uses the duplicated
> list of multicast mac addresses to keep track of
> what addresses to delete from h/w before committing
> the new list from kernel to h/w back again via set_rx_mode,
> every time this list gets updated by the kernel.
> Given that the h/w knows how to add and delete mac addresses
> based on the mac address value alone, __dev_mc_sync should be
> the much better choice of kernel API for these operations
> avoiding the considerable overhead of maintaining a duplicated
> list in the driver.
>
> Signed-off-by: Claudiu Manoil <claudiu.man...@nxp.com>
Tested-by: Alexandre Belloni <alexandre.bell...@bootlin.com>
Acked-by: Alexandre Belloni <alexandre.bell...@bootlin.com>
> ---
> Maybe this should go to net, since there were objections
> for abusing kmalloc(GFP_ATOMIC).
>
> drivers/net/ethernet/mscc/ocelot.c | 43 ++++++------------------------
> drivers/net/ethernet/mscc/ocelot.h | 4 ---
> 2 files changed, 8 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/net/ethernet/mscc/ocelot.c
> b/drivers/net/ethernet/mscc/ocelot.c
> index d715ef4fc92f..02ad11e0b0d8 100644
> --- a/drivers/net/ethernet/mscc/ocelot.c
> +++ b/drivers/net/ethernet/mscc/ocelot.c
> @@ -593,45 +593,25 @@ static int ocelot_port_xmit(struct sk_buff *skb, struct
> net_device *dev)
> return NETDEV_TX_OK;
> }
>
> -static void ocelot_mact_mc_reset(struct ocelot_port *port)
> +static int ocelot_mc_unsync(struct net_device *dev, const unsigned char
> *addr)
> {
> - struct ocelot *ocelot = port->ocelot;
> - struct netdev_hw_addr *ha, *n;
> + struct ocelot_port *port = netdev_priv(dev);
>
> - /* Free and forget all the MAC addresses stored in the port private mc
> - * list. These are mc addresses that were previously added by calling
> - * ocelot_mact_mc_add().
> - */
> - list_for_each_entry_safe(ha, n, &port->mc, list) {
> - ocelot_mact_forget(ocelot, ha->addr, port->pvid);
> - list_del(&ha->list);
> - kfree(ha);
> - }
> + return ocelot_mact_forget(port->ocelot, addr, port->pvid);
> }
>
> -static int ocelot_mact_mc_add(struct ocelot_port *port,
> - struct netdev_hw_addr *hw_addr)
> +static int ocelot_mc_sync(struct net_device *dev, const unsigned char *addr)
> {
> - struct ocelot *ocelot = port->ocelot;
> - struct netdev_hw_addr *ha = kzalloc(sizeof(*ha), GFP_ATOMIC);
> -
> - if (!ha)
> - return -ENOMEM;
> -
> - memcpy(ha, hw_addr, sizeof(*ha));
> - list_add_tail(&ha->list, &port->mc);
> -
> - ocelot_mact_learn(ocelot, PGID_CPU, ha->addr, port->pvid,
> - ENTRYTYPE_LOCKED);
> + struct ocelot_port *port = netdev_priv(dev);
>
> - return 0;
> + return ocelot_mact_learn(port->ocelot, PGID_CPU, addr, port->pvid,
> + ENTRYTYPE_LOCKED);
> }
>
> static void ocelot_set_rx_mode(struct net_device *dev)
> {
> struct ocelot_port *port = netdev_priv(dev);
> struct ocelot *ocelot = port->ocelot;
> - struct netdev_hw_addr *ha;
> int i;
> u32 val;
>
> @@ -643,13 +623,7 @@ static void ocelot_set_rx_mode(struct net_device *dev)
> for (i = ocelot->num_phys_ports + 1; i < PGID_CPU; i++)
> ocelot_write_rix(ocelot, val, ANA_PGID_PGID, i);
>
> - /* Handle the device multicast addresses. First remove all the
> - * previously installed addresses and then add the latest ones to the
> - * mac table.
> - */
> - ocelot_mact_mc_reset(port);
> - netdev_for_each_mc_addr(ha, dev)
> - ocelot_mact_mc_add(port, ha);
> + __dev_mc_sync(dev, ocelot_mc_sync, ocelot_mc_unsync);
> }
>
> static int ocelot_port_get_phys_port_name(struct net_device *dev,
> @@ -1657,7 +1631,6 @@ int ocelot_probe_port(struct ocelot *ocelot, u8 port,
> ocelot_port->regs = regs;
> ocelot_port->chip_port = port;
> ocelot_port->phy = phy;
> - INIT_LIST_HEAD(&ocelot_port->mc);
> ocelot->ports[port] = ocelot_port;
>
> dev->netdev_ops = &ocelot_port_netdev_ops;
> diff --git a/drivers/net/ethernet/mscc/ocelot.h
> b/drivers/net/ethernet/mscc/ocelot.h
> index ba3b3380b4d0..541fe41e60b0 100644
> --- a/drivers/net/ethernet/mscc/ocelot.h
> +++ b/drivers/net/ethernet/mscc/ocelot.h
> @@ -441,10 +441,6 @@ struct ocelot_port {
> struct phy_device *phy;
> void __iomem *regs;
> u8 chip_port;
> - /* Keep a track of the mc addresses added to the mac table, so that they
> - * can be removed when needed.
> - */
> - struct list_head mc;
>
> /* Ingress default VLAN (pvid) */
> u16 pvid;
> --
> 2.17.1
>
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
