Re: [PATCH net-next 4/5] net: ethernet: adi: adin1110: Add OA TC6 protocol support for ADIN1110

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] net: adin1110: broken uc and mc filtering in adin1110_set_rx_mode_tc6()
--

commit fee140bec5b369ab45ae064be0620e5878578e1e
Author: Ciprian Regus <ciprian.regus@analog.com>

net: ethernet: adi: adin1110: Add OA TC6 protocol support for ADIN1110

This commit adds OPEN Alliance TC6 SPI protocol support for the ADIN1110
MAC-PHY using the oa_tc6 framework when the adi,spi-oa-tc6 property is
present. It introduces OA TC6 register accessors, net_device_ops callbacks,
and probe handling while sharing the existing MAC configuration logic.

> diff --git a/drivers/net/ethernet/adi/adin1110.c b/drivers/net/ethernet/adi/adin1110.c
> index 53fac3dc8be62..d08b165bf7c48 100644
> --- a/drivers/net/ethernet/adi/adin1110.c
> +++ b/drivers/net/ethernet/adi/adin1110.c

[ ... ]

> @@ -881,6 +920,22 @@ static void adin1110_set_rx_mode(struct net_device *dev)
>  	spin_unlock(&priv->state_lock);
>  }
>  
> +static int adin1110_set_rx_mode_tc6(struct net_device *dev,
> +				    struct netdev_hw_addr_list *uc,
> +				    struct netdev_hw_addr_list *mc)
> +{
> +	struct adin1110_port_priv *port_priv = netdev_priv(dev);
> +	struct adin1110_priv *priv = port_priv->priv;
> +	int ret;
> +
> +	mutex_lock(&priv->lock);
> +	port_priv->flags = dev->flags;
> +	ret = adin1110_setup_rx_mode(port_priv);
> +	mutex_unlock(&priv->lock);
> +
> +	return ret;
> +}

[ ... ]

> @@ -1123,6 +1233,69 @@ static int adin1110_check_spi(struct adin1110_priv *priv)
>  	return 0;
>  }
>  
> +static void adin1110_oa_tc6_remove(void *data)
> +{
> +	oa_tc6_exit(data);
> +}
> +
> +static int adin1110_oa_tc6_probe(struct adin1110_priv *priv)
> +{

[ ... ]

> +	netdev->if_port = IF_PORT_10BASET;
> +	netdev->netdev_ops = &adin1110_tc6_netdev_ops;
> +	netdev->ethtool_ops = &adin1110_ethtool_ops;
> +	netdev->priv_flags |= IFF_UNICAST_FLT;
> +	netdev->netns_immutable = true;
> +
> +	return devm_register_netdev(dev, netdev);
> +}

[Severity: Medium]
In adin1110_oa_tc6_probe(), netdev->priv_flags sets IFF_UNICAST_FLT, which
tells netif_rx_mode_run() -> netif_uc_promisc_update() not to fall back to
IFF_PROMISC when secondary unicast addresses are added to dev->uc.

However, adin1110_set_rx_mode_tc6() ignores both the uc and mc address list
snapshots and only calls adin1110_setup_rx_mode():

drivers/net/ethernet/adi/adin1110.c:adin1110_setup_rx_mode() {
	ret = adin1110_set_promisc_mode(port_priv,
					!!(port_priv->flags & IFF_PROMISC));
	if (ret < 0)
		return ret;

	ret = adin1110_multicast_filter(port_priv, ADIN_MAC_MULTICAST_ADDR_SLOT,
					!!(port_priv->flags & IFF_ALLMULTI));
    ...
}

Because neither dev->uc nor dev->mc is programmed into the hardware MAC
filter slots, and ADIN_MAC_MULTICAST_ADDR_SLOT is only enabled when
IFF_ALLMULTI is set, won't frames destined to secondary unicast addresses
or joined multicast groups (such as IPv6 Neighbor Discovery solicited-node
multicast when IFF_ALLMULTI is not set) be silently dropped in OA TC6 mode?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103160918.409701-1-iustin.stolniceanu@analog.com?part=4