Re: [PATCH net-next v2 3/9] net: microchip: vcap: make vcap actionset decoding type_id aware

Jakub Kicinski <[email protected]>
Newsgroups org.kernel.vger.linux-hardening,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel,org.kernel.vger.netdev
Message-ID <[email protected]>
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.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.