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

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] [day] [month] [year] [list]

Message-ID: <y45atwebueigfjsbi5d3d4qsf36m3esspgll4ork7fw2su7lrj@26qcv6yvk6mr>
Date: Sat, 30 Aug 2025 05:46:43 +0300
From: Joseph Steel <recv.jo@...il.com>
To: Jacob Keller <jacob.e.keller@...el.com>, 
	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
Subject: Re: [PATCH net-next 0/4] net: stmmac: new features

On Fri, Aug 29, 2025 at 02:23:24PM -0700, Jacob Keller wrote:
> 
> 
> On 8/28/2025 7:45 AM, Konrad Leszczynski wrote:
> > 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/
> > 
> > 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@...el.com>

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

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.

BTW I've been working with the driver for quite some time and AFAICS
Intel contributed if not half but at least quarter of it' mess.

Joseph