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.
