Re: [PATCH net-next 0/4] net: stmmac: new features

On 8/29/2025 7:46 PM, Joseph Steel wrote:

On Fri, Aug 29, 2025 at 02:23:24PM -0700, Jacob Keller wrote:

quoted

On 8/28/2025 7:45 AM, Konrad Leszczynski wrote:

quoted

This series adds four new patches which introduce features such as ARP
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@intel.com/ (local)

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(-)

The series looks good to me.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

Not a single comment? Really? Three Rb and three Sb tags from Intel
staff and nobody found even a tiny problem? Sigh...

Not everyone will find every issue. I'm certainly no expert in this
driver. This is why it is good to have many reviewers.

Let's start with an easiest one. What about introducing an unused
platform flag for ARP-offload?

Next is more serious one. What about considering a case that
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? 

Finally did anyone test out the functionality of the patches 1 and
2? What does arping show for instance for just three ARP requests?
Nothing strange?

So to speak at this stage I'd give NAK at least for the patches 1 and
2.