Re: [PATCH net] net: mscc: ocelot: fix incorrect IFH SRC_PORT field in ocelot_ifh_set_basic()

[Simon Horman <horms@kernel.org>]

On Thu, Dec 12, 2024 at 06:55:45PM +0200, Vladimir Oltean wrote:
> Packets injected by the CPU should have a SRC_PORT field equal to the
> CPU port module index in the Analyzer block (ocelot->num_phys_ports).
> 
> The blamed commit copied the ocelot_ifh_set_basic() call incorrectly
> from ocelot_xmit_common() in net/dsa/tag_ocelot.c. Instead of calling
> with "x", it calls with BIT_ULL(x), but the field is not a port mask,
> but rather a single port index.
> 
> [ side note: this is the technical debt of code duplication :( ]
> 
> The error used to be silent and doesn't appear to have other
> user-visible manifestations, but with new changes in the packing
> library, it now fails loudly as follows:
> 
> ------------[ cut here ]------------
> Cannot store 0x40 inside bits 46-43 - will truncate
> sja1105 spi2.0: xmit timed out
> WARNING: CPU: 1 PID: 102 at lib/packing.c:98 __pack+0x90/0x198
> sja1105 spi2.0: timed out polling for tstamp
> CPU: 1 UID: 0 PID: 102 Comm: felix_xmit
> Tainted: G        W        N 6.13.0-rc1-00372-gf706b85d972d-dirty #2605
> Call trace:
>  __pack+0x90/0x198 (P)
>  __pack+0x90/0x198 (L)
>  packing+0x78/0x98
>  ocelot_ifh_set_basic+0x260/0x368
>  ocelot_port_inject_frame+0xa8/0x250
>  felix_port_deferred_xmit+0x14c/0x258
>  kthread_worker_fn+0x134/0x350
>  kthread+0x114/0x138
> 
> The code path pertains to the ocelot switchdev driver and to the felix
> secondary DSA tag protocol, ocelot-8021q. Here seen with ocelot-8021q.
> 
> The messenger (packing) is not really to blame, so fix the original
> commit instead.
> 
> Fixes: e1b9e80236c5 ("net: mscc: ocelot: fix QoS class for injected packets with "ocelot-8021q"")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Simon Horman <horms@kernel.org>