netdev - Re: [PATCH net-next 4/7] net: stmmac: enable ARP Offload on mac_link

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250827192816.GR10519@horms.kernel.org>
Date: Wed, 27 Aug 2025 20:28:16 +0100
From: Simon Horman <horms@...nel.org>
To: Konrad Leszczynski <konrad.leszczynski@...el.com>
Cc: davem@...emloft.net, andrew+netdev@...n.ch, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, cezary.rojewski@...el.com,
	sebastian.basierski@...el.com,
	Karol Jurczenia <karol.jurczenia@...el.com>
Subject: Re: [PATCH net-next 4/7] net: stmmac: enable ARP Offload on
 mac_link_up()

On Tue, Aug 26, 2025 at 01:32:44PM +0200, Konrad Leszczynski wrote:
> From: Karol Jurczenia <karol.jurczenia@...el.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@...el.com>
> Reviewed-by: Sebastian Basierski <sebastian.basierski@...el.com>
> Signed-off-by: Karol Jurczenia <karol.jurczenia@...el.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
> 
>