Re: [PATCH net-next v12 09/15] ax88179_178a: Add VLAN offload support for AX88179A

[Birger Koblitz <mail@birger-koblitz.de>]

On 17/09/2026 11:25 pm, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 9 potential
> issue(s) to consider:
> 
> Critical: 0 · High: 4 · Medium: 5 · Low: 0
> 
> - [High] ax88179a_set_features()
>    (drivers/net/usb/ax88179a_devices.c:578-603) clobbers the persistent…
Will fix by moving conditionally setting AX_VLAN_CONTROL_VFE after the
loop over the VLAN-entries.

> - [High] This patch newly installs `.ndo_set_features =
>    ax88179a_set_features` on the AX88179A ops table…
Moved the rx_checksum tracing addition into the patch that adds the
AX88179A HW support. Made setting the checksumming feature on the device
dependent on NETIF_F_RXCSUM flag setting in ax88179a_reset().

> - [High] net: missing endianness conversion for subsequent Rx descriptors
>    in ax88179a_rx_fixup
Will add u64 pkt_desc = le64_to_cpu(*pkt_desc_ptr) within the loop
to account for endianness.

> - [High] net: unswapped read of rx_hdr yields garbage on big-endian
>    architectures
Duplicate of bug fixed in commit adding ax88179a_rx_fixup()

> - [Medium] The series is not bisectable: the VLAN feature bits are
>    advertised three commits earlier than the callbacks this patch adds,…
Will fix by moving the VLAN feature bits into the later patch that
also adds the netdev_ops.

> - [Medium] The new VLAN register sequences ignore every USB register-
>    access error and still report success.
Will fix by checking return codes in ax88179a_vlan_rx_add_vid(),
ax88179a_vlan_rx_kill_vid() and ax88179a_set_features

> - [Medium] All three new functions read AX88179A_VLAN_ID_CONTROL into an
>    uninitialised local u8 and ignore the result, then transmit a value…
Fixed by above checking of return values.

> - [Medium] The new `data->rx_checksum` write in ax88179_set_features()
>    (drivers/net/usb/ax88179_lib.c:369) races with, and is ordered wrongly…
  will clear rx_checksum before disabling the engine and only set the
   flag after the engine has been enabled.


> - [Medium] Enabling NETIF_F_HW_VLAN_CTAG_FILTER makes
>    ax88179a_set_features() (drivers/net/usb/ax88179a_devices.c:582-596)…
> 

You ask: On a busy bus, or when a transfer stalls and hits the timeout, 
this blocks
   all other network configuration in the system for a long time.  Could the
   clear be limited to rows actually in use, batched, or moved out of 
the RTNL
   section?

It is not possible to limit this to rows actually in use, because this is
clearing the rows _before_ use. These rows appear to contain garbage
on reset. Moving them out of RTNL scope would mean moving the clearing
elsewhere, but this also does not appear to solve the RTNL issue:
the logical place would be ax88179a_reset(), which however also runs 
under RTNL.
I do not see how batching could be done. Since the same issue is also
in aqc111, and this is not critical, I would like to push for a later
solution of the issue.