From: Jakub Kicinski <kuba@kernel.org>
To: jensemil.schulzostergaard@microchip.com
Cc: Jakub Kicinski <kuba@kernel.org>,
horatiu.vultur@microchip.com, UNGLinuxDriver@microchip.com,
andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
pabeni@redhat.com, daniel.machon@microchip.com,
Steen.Hegelund@microchip.com, kees@kernel.org,
gustavoars@kernel.org, robert.marko@sartura.hr,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-hardening@vger.kernel.org
Subject: Re: [PATCH net-next v2 3/9] net: microchip: vcap: make vcap actionset decoding type_id aware
Date: Mon, 17 Aug 2026 14:13:53 -0700 [thread overview]
Message-ID: <20260817211353.3633288-1-kuba@kernel.org> (raw)
In-Reply-To: <20260810-sparx5_l3_routing-v2-3-59e68cc8c8ca@microchip.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: microchip: vcap: make vcap actionset decoding type_id aware
This adds a helper, vcap_find_stream_type_id(), that pulls the type_id
bits out of stream[0] just after the typegroup bits, and uses it in
vcap_verify_actionstream_actionset() to compare the decoded type_id
against info->type_id instead of always returning true. The code change
itself looks fine; the comment below is only about the commit message.
> net: microchip: vcap: make vcap actionset decoding type_id aware
>
> When reading a rule back from hardware, decoding has to identify which
> actionset the rule was written as. The existing logic was only aware of
> the actionset subword length, which cannot distinguish actionsets that
> share a subword length but differ in their type_id field. The LPM VCAP
> added in a following patch introduces this case: ARP_PTR, L3MC_PTR and
> ARP_ENTRY all occupy one subword and differ only by type_id.
[Severity: Low]
Should the sentence "The LPM VCAP added in a following patch introduces
this case: ARP_PTR, L3MC_PTR and ARP_ENTRY all occupy one subword and
differ only by type_id" be reworded?
Two details don't seem to match the tree. First, there is no L3MC_PTR
symbol anywhere at this commit; grepping for L3MC under
drivers/net/ethernet/microchip/ only matches the unrelated register name
GA_ANA_L3_VLAN_ARP_L3MC_STICKY. The one-subword LPM actionsets that do
exist are just these two, in sparx5_vcap_ag_api.c:
static const struct vcap_set lpm_actionfield_set[] = {
[VCAP_AFS_ARP_PTR] = {
.type_id = 0,
.sw_per_item = 1,
.sw_cnt = 12,
},
[VCAP_AFS_ARP_ENTRY] = {
.type_id = 2,
.sw_per_item = 1,
.sw_cnt = 12,
},
};
Second, the colliding actionset metadata is not added in a following
patch; lpm_actionfield_set, lpm_actionfield_set_map and the
sparx5_vcaps[VCAP_TYPE_LPM] entry that references them are already
present at this commit, added by the preceding "net: microchip: vcap: add
lpm vcap to autogen vcap api". Only the sparx5/lan969x LPM driver
enablement lands later.
Could the message say the LPM actionset model is added by the preceding
patch, and drop the L3MC_PTR reference? The premise that two actionsets
share sw_per_item = 1 and differ only in type_id does hold, so the change
itself still looks justified.
next prev parent reply other threads:[~2026-08-17 21:14 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-10 11:20 [PATCH net-next v2 0/9] net: sparx5: add L3 unicast routing offload Jens Emil Schulz Østergaard
2026-08-10 11:20 ` [PATCH net-next v2 1/9] net: microchip: vcap: fix rule move for rules of coprime size Jens Emil Schulz Østergaard
2026-08-17 21:13 ` Jakub Kicinski
2026-08-10 11:20 ` [PATCH net-next v2 2/9] net: microchip: vcap: add lpm vcap to autogen vcap api Jens Emil Schulz Østergaard
2026-08-10 11:20 ` [PATCH net-next v2 3/9] net: microchip: vcap: make vcap actionset decoding type_id aware Jens Emil Schulz Østergaard
2026-08-17 21:13 ` Jakub Kicinski [this message]
2026-08-10 11:20 ` [PATCH net-next v2 4/9] net: microchip: vcap: expose helpers in vcap api and update debugfs Jens Emil Schulz Østergaard
2026-08-17 21:13 ` Jakub Kicinski
2026-08-10 11:20 ` [PATCH net-next v2 5/9] net: sparx5: add l3 routing registers Jens Emil Schulz Østergaard
2026-08-10 11:20 ` [PATCH net-next v2 6/9] net: sparx5: vcap: add lpm vcap implementation Jens Emil Schulz Østergaard
2026-08-17 21:13 ` Jakub Kicinski
2026-08-10 11:20 ` [PATCH net-next v2 7/9] net: sparx5: add L3 router infrastructure and leg management Jens Emil Schulz Østergaard
2026-08-17 21:13 ` Jakub Kicinski
2026-08-10 11:20 ` [PATCH net-next v2 8/9] net: sparx5: add L3 FIB, nexthop and neighbour entry management Jens Emil Schulz Østergaard
2026-08-17 21:13 ` Jakub Kicinski
2026-08-10 11:20 ` [PATCH net-next v2 9/9] net: sparx5: add neighbour event handling for L3 routing Jens Emil Schulz Østergaard
2026-08-17 21:14 ` Jakub Kicinski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260817211353.3633288-1-kuba@kernel.org \
--to=kuba@kernel.org \
--cc=Steen.Hegelund@microchip.com \
--cc=UNGLinuxDriver@microchip.com \
--cc=andrew+netdev@lunn.ch \
--cc=daniel.machon@microchip.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=gustavoars@kernel.org \
--cc=horatiu.vultur@microchip.com \
--cc=jensemil.schulzostergaard@microchip.com \
--cc=kees@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=robert.marko@sartura.hr \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox