Re: [PATCH net-next v7 07/10] enic: add MBOX PF handlers for VF register and capability

[Jakub Kicinski <kuba@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
enic: add MBOX PF handlers for VF register and capability

This commit implements PF-side mailbox message processing for SR-IOV V2
admin channel communication. It routes messages from VFs to type-specific
handlers and tracks per-VF state.

> diff --git a/drivers/net/ethernet/cisco/enic/enic_mbox.c b/drivers/net/ethernet/cisco/enic/enic_mbox.c
> index 3f95247b8f6ba..0f7f9d2d091f7 100644
> --- a/drivers/net/ethernet/cisco/enic/enic_mbox.c
> +++ b/drivers/net/ethernet/cisco/enic/enic_mbox.c

[ ... ]

> @@ -135,10 +135,177 @@ int enic_mbox_send_msg(struct enic *enic, u8 msg_type, u16 dst_vnic_id,

[ ... ]

> +static int enic_mbox_pf_handle_register(struct enic *enic, void *msg,
> +					u16 vf_id, u64 msg_num)
> +{

[ ... ]

> +	err = enic_mbox_send_link_state(enic, vf_id,
> +					ENIC_MBOX_LINK_STATE_ENABLE);
> +	if (err)
> +		netdev_warn(enic->netdev,
> +			    "VF %u: failed to send initial link state: %d\n",
> +			    vf_id, err);

Could a malicious or misbehaving VF flood the host kernel log here?

Other VF-triggered warnings in this file use net_ratelimit() to prevent a
host-side denial-of-service condition where unbounded kernel logging degrades
system performance. Does this netdev_warn() need a net_ratelimit() check as
well?

> +	/* Registration succeeded; link state will be (re-)sent on next
> +	 * enic_link_check() event.
> +	 */
> +	return 0;
> +}

[ ... ]

> +static void enic_mbox_pf_process_msg(struct enic *enic,
> +				     struct enic_mbox_hdr *hdr, void *payload)
> +{

[ ... ]

> +	switch (hdr->msg_type) {

[ ... ]

> +	case ENIC_MBOX_PF_LINK_STATE_ACK: {
> +		struct enic_mbox_pf_link_state_ack_msg *ack = payload;
> +
> +		if (msg_len < sizeof(*hdr) + sizeof(*ack))
> +			break;
> +		if (le16_to_cpu(ack->ack.ret_major))
> +			netdev_warn(enic->netdev,
> +				    "MBOX: VF %u link state ACK error %u/%u\n",
> +				    vf_id, le16_to_cpu(ack->ack.ret_major),
> +				    le16_to_cpu(ack->ack.ret_minor));

Similarly, if a VF sends continuous ENIC_MBOX_PF_LINK_STATE_ACK messages with
a non-zero ret_major error, could it spam the host logs?

Should this netdev_warn() also be guarded by if (net_ratelimit())?

> +		break;
> +	}

[ ... ]
-- 
pw-bot: cr