Re: [PATCH net-next 4/7] net: stmmac: enable ARP Offload on mac_link_up()

[Simon Horman <horms@kernel.org>]

On Tue, Aug 26, 2025 at 01:32:44PM +0200, Konrad Leszczynski wrote:
> From: Karol Jurczenia <karol.jurczenia@intel.com>
> 
> Add Address Resolution Protocol (ARP) Offload support in
> stmmac_mac_link_up() to enable ARP Offload beside the selftests.
> 
> Introduce STMMAC_FLAG_ARP_OFFLOAD_EN flag, which is used to enable the
> feature with the stmmac_set_arp_offload().
> 
> Reviewed-by: Konrad Leszczynski <konrad.leszczynski@intel.com>
> Reviewed-by: Sebastian Basierski <sebastian.basierski@intel.com>
> Signed-off-by: Karol Jurczenia <karol.jurczenia@intel.com>

Konrad,

AFAIK, as you are posting this patches, your SoB line needs to go here.
(And correspondingly, your Reviewed-by line should be removed.)

...

> @@ -1075,6 +1078,20 @@ static void stmmac_mac_link_up(struct phylink_config *config,
>  
>  	if (priv->plat->flags & STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY)
>  		stmmac_hwtstamp_correct_latency(priv, priv);
> +
> +	if (priv->plat->flags & STMMAC_FLAG_ARP_OFFLOAD_EN) {
> +		in_dev = in_dev_get(priv->dev);
> +		if (!in_dev)
> +			return;
> +
> +		ifa = in_dev->ifa_list;

ifa_list is protected by RCU, so I think the code needs to take that into
account. E.g. by doing the following and making sure that the RCU read lock
is held.

		ifa = rcu_dereference(idev->ifa_list);

Flagged by Sparse.


> +		if (!ifa)
> +			return;
> +
> +		stmmac_set_arp_offload(priv, priv->hw, true,
> +				       ntohl(ifa->ifa_address));
> +		in_dev_put(in_dev);
> +	}
>  }
>  
>  static void stmmac_mac_disable_tx_lpi(struct phylink_config *config)
> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> index 26ddf95d23f9..aae522f37710 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -185,6 +185,7 @@ struct dwmac4_addrs {
>  #define STMMAC_FLAG_EN_TX_LPI_CLOCKGATING	BIT(11)
>  #define STMMAC_FLAG_EN_TX_LPI_CLK_PHY_CAP	BIT(12)
>  #define STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY	BIT(13)
> +#define STMMAC_FLAG_ARP_OFFLOAD_EN		BIT(14)

I see that the following patch in this series also makes use of this bit.
But I don't see any thing (platform) that sets this bit. Perhaps I'm
missing something. But I think that would be best included in the patchset
that adds this bit.

>  
>  struct plat_stmmacenet_data {
>  	int bus_id;
> -- 
> 2.34.1
> 
>