On 8/21/2023 2:02 PM, Jakub Kicinski wrote:

On Mon, 21 Aug 2023 13:41:15 -0700 Linga, Pavan Kumar wrote:

quoted

On 8/18/2023 11:58 AM, Jakub Kicinski wrote:

quoted

On Tue, 15 Aug 2023 17:43:04 -0700 Tony Nguyen wrote:

quoted

+static u32 idpf_get_rxfh_indir_size(struct net_device *netdev)
+{
+	struct idpf_vport *vport = idpf_netdev_to_vport(netdev);
+	struct idpf_vport_user_config_data *user_config;
+
+	if (!vport)
+		return -EINVAL;

defensive programming? how do we have a netdev and no vport?

During a hardware reset, the control plane will reinitialize its vport
configuration along with the hardware resources which in turn requires
the driver to reallocate the vports as well. For this reason the vports
will be freed, but the netdev will be preserved.

HW reset path should take appropriate locks so that the normal control
path can't be exposed to transient errors.

User space will 100% not know what to do with a GET reporting EINVAL.

Agreed, looking into using locks to protect such cases.

quoted

quoted

quoted

+	dev = &vport->adapter->pdev->dev;
+	if (!(ch->combined_count || (ch->rx_count && ch->tx_count))) {
+		dev_err(dev, "Please specify at least 1 Rx and 1 Tx channel\n");

The error msg doesn't seem to fit the second part of the condition.

The negation part is to the complete check which means it takes 0
[tx|rx]_count into consideration.

Ah, missed the negation. In that case I think the check is not needed,
pretty sure core checks this.

My bad. After taking a closer look, the above check is similar compared 
to that of the core check. Will remove the check as it is redundant.

quoted

quoted

quoted

+		return -EINVAL;
+	}
+
+	num_req_tx_q = ch->combined_count + ch->tx_count;
+	num_req_rx_q = ch->combined_count + ch->rx_count;
+
+	dev = &vport->adapter->pdev->dev;
+	/* It's possible to specify number of queues that exceeds max in a way
+	 * that stack won't catch for us, this should catch that.
+	 */

How, tho?

If the user tries to pass the combined along with the txq or rxq values,
then it is possbile to cross the max supported values. So the following
checks are needed to protect those cases. Core checks the max values for
the individual arguments but not the combined + [tx|rx].

I see, please add something along those lines to the comment.

Sure, will do.

Thanks,
Pavan