Re: [PATCH net-next 0/4] net: stmmac: new features
From: Konrad Leszczynski
Date: Mon Sep 15 2025 - 09:06:50 EST
On 14-Sep-25 14:53, Joseph Steel wrote:
On Tue, Sep 09, 2025 at 08:26:29PM +0200, Sebastian Basierski wrote:
On 8/30/2025 4:46 AM, Joseph Steel wrote:The reasoning doesn't explain the outcome you provided. Even if the
On Fri, Aug 29, 2025 at 02:23:24PM -0700, Jacob Keller wrote:Hi Joseph,
On 8/28/2025 7:45 AM, Konrad Leszczynski wrote:Not a single comment? Really? Three Rb and three Sb tags from Intel
This series adds four new patches which introduce features such as ARPThe series looks good to me.
Offload support, VLAN protocol detection and TC flower filter support.
Patchset has been created as a result of discussion at [1].
[1] https://lore.kernel.org/netdev/20250826113247.3481273-1-konrad.leszczynski@xxxxxxxxx/
v1 -> v2:
- add missing SoB lines
- place ifa_list under RCU protection
Karol Jurczenia (3):
net: stmmac: enable ARP Offload on mac_link_up()
net: stmmac: set TE/RE bits for ARP Offload when interface down
net: stmmac: add TC flower filter support for IP EtherType
Piotr Warpechowski (1):
net: stmmac: enhance VLAN protocol detection for GRO
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 1 +
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 35
++++++++++++++++---
.../net/ethernet/stmicro/stmmac/stmmac_tc.c | 19 +++++++++-
include/linux/stmmac.h | 1 +
4 files changed, 50 insertions(+), 6 deletions(-)
Reviewed-by: Jacob Keller <jacob.e.keller@xxxxxxxxx>
staff and nobody found even a tiny problem? Sigh...
Thank you for your time and valuable review
Let's start with an easiest one. What about introducing an unusedRight, this patch should not be here. Will be removed in next revision.
platform flag for ARP-offload?
Next is more serious one. What about considering a case thatWhile trying to enable ARP offload,
IP-address can be changed or removed while MAC link is being up?
Why does Intel want to have ARP requests being silently handled even
when a link is completely set down by the host, when PHY-link is
stopped and PHY is disconnected, after net_device::ndo_stop() is
called?
we found out that when interface was set down and up,
MAC_ARP_Address and ARP offload enable bit were reset to default values,
the address was set to 0xFFFFFFFF and ARP offload was disabled.
There was two possible solutions out of this:
a) caching address and ARP offload bit state
b) enabling ARP while interface is down.
We choose to go with second solution.
But given that fact this code depends on unused STMMAC_ARP_OFFLOAD_EN flag,
i guess whether it is fine or not, should not be placed in patchset.
controller is reset on the device switching on/off/on cycles the
driver will re-initialize it anyway. The same could have happened with
the ARP-engine too should the patch 1 be in in-place. But besides of
that you submitted patch 2, which _enables_ the network interface to
respond on the ARP-requests with some initial IP-address even if the
interface is set down by the user. This makes the host being visible
to the surrounding network devices behind the user back if the PHY,
for instance, is unmanaged or reported as fixed. Is that what Intel
wanted?
So did they notice anything strange? Is the validation team qualifiedFinally did anyone test out the functionality of the patches 1 andYes, we have a validation team that verified proposed solution.
2? What does arping show for instance for just three ARP requests?
Nothing strange?
enough to correctly evaluate the change?
Joseph
Hi Joseph,
Thanks for your input. We'll remove both ARP offload-related patches (1 and 2) from this patchset until an actual platform that uses them will be introduced later on. When that time comes we'll also check your comments regarding the "host being visible to the surrounding network devices behind the user" and adjust the patches accordingly.