Re: [PATCH v2 36/46] net/liquidio: add API to set MTU

On 3/21/2017 12:53 PM, Shijith Thotton wrote:

On Tue, Mar 21, 2017 at 12:24:49PM +0000, Ferruh Yigit wrote:

quoted

On 3/2/2017 11:32 AM, Shijith Thotton wrote:

quoted

Signed-off-by: Shijith Thotton <redacted>
Signed-off-by: Jerin Jacob <redacted>
Signed-off-by: Derek Chickles <redacted>
Signed-off-by: Venkat Koppula <redacted>
Signed-off-by: Srisivasubramanian S <redacted>
Signed-off-by: Mallesham Jatharakonda <redacted>

<...>

quoted

 
 static int
+lio_dev_change_vf_mtu(struct rte_eth_dev *eth_dev, uint16_t new_mtu)
+{
+	struct lio_device *lio_dev = LIO_DEV(eth_dev);
+
+	PMD_INIT_FUNC_TRACE();
+
+	if (!lio_dev->intf_open) {
+		lio_dev_err(lio_dev, "Port %d down, can't change MTU\n",
+			    lio_dev->port_id);
+		return -EINVAL;
+	}
+
+	/* Limit the MTU to make sure the ethernet packets are between
+	 * ETHER_MIN_MTU bytes and PF's MTU
+	 */
+	if ((new_mtu < ETHER_MIN_MTU) ||
+			(new_mtu > lio_dev->linfo.link.s.mtu)) {
+		lio_dev_err(lio_dev, "Invalid MTU: %d\n", new_mtu);
+		lio_dev_err(lio_dev, "Valid range %d and %d\n",
+			    ETHER_MIN_MTU, lio_dev->linfo.link.s.mtu);
+		return -EINVAL;
+	}
+
+	return 0;
+}

Is this really sets the MTU?
"new_mtu" seems not used, except limit check, an lio_send_ctrl_pkt()
required perhaps?

It won't set MTU for hardware and is possible only by PF. So
lio_send_ctrl_pkt is not required. VF MTU is limited by PF MTU and is
mentioned under limitations in driver documentation. Here we are
allowing upper layer to set MTU up to the value configured by PF.

I see, but lio_dev_change_vf_mtu() does not set anything at all. If it
is not modifying anything at all, why you claim "MTU update" supported?

And following logic seems wrong for this case:

	...
	if (lio_dev->linfo.link.s.mtu != mtu) {
		ret = lio_dev_change_vf_mtu(eth_dev, mtu);
	...

Should this functions set lio_dev->linfo.link.s.mtu at least, perhaps?

Shijith