On Mon, 2026-08-17 at 14:13 -0700, Jakub Kicinski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> 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.
Yes I will update the message, and remove the stale remarks.
