diff mbox

[net-next,10/13] net: ena: add mtu limitation in ena_change_mtu()

Message ID 1497785298-13468-11-git-send-email-netanel@amazon.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Belgazal, Netanel June 18, 2017, 11:28 a.m. UTC 
From: Netanel Belgazal <netanel@amazon.com>

Signed-off-by: Netanel Belgazal <netanel@amazon.com>
---
 drivers/net/ethernet/amazon/ena/ena_netdev.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

David Miller June 18, 2017, 4:21 p.m. UTC | #1 
From: <netanel@amazon.com>
Date: Sun, 18 Jun 2017 14:28:15 +0300

> From: Netanel Belgazal <netanel@amazon.com>
> 
> Signed-off-by: Netanel Belgazal <netanel@amazon.com>

I don't understand this at all.

This whole reason we have those:

> @@ -3008,8 +3015,6 @@ static void ena_set_conf_feat_params(struct ena_adapter *adapter,
>  	ena_set_dev_offloads(feat, netdev);
>  
>  	adapter->max_mtu = feat->dev_attr.max_mtu;
> -	netdev->max_mtu = adapter->max_mtu;
> -	netdev->min_mtu = ENA_MIN_MTU;
>  }
>  

assignments you are removing is so that the core networking can
validate the request and therefore individual drivers don't have to.

If it's just to get that silly driver message out when the MTU asked
for is too large, that's not a good reason to bypass the
infrastructure for MTU changes that we've provided for drivers in the
core.

I don't like this change at all.

You could have helped things a lot by actually writing a commit log
message explaining what you are really doing, and why you are doing
it.  Empty commit log messages are never a good idea.
Belgazal, Netanel June 19, 2017, 12:26 p.m. UTC | #2 
I'll discard this patch.
diff mbox

Patch

diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index 7dee448157bb..7f31f4ce7b8e 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -108,6 +108,13 @@  static int ena_change_mtu(struct net_device *dev, int new_mtu)
 	struct ena_adapter *adapter = netdev_priv(dev);
 	int ret;
 
+	if ((new_mtu > adapter->max_mtu) || (new_mtu < ENA_MIN_MTU)) {
+		netif_err(adapter, drv, dev,
+			  "Invalid MTU setting. new_mtu: %d max mtu: %d min mtu: %d\n",
+			  new_mtu, adapter->max_mtu, ENA_MIN_MTU);
+		return -EINVAL;
+	}
+
 	ret = ena_com_set_dev_mtu(adapter->ena_dev, new_mtu);
 	if (!ret) {
 		netif_dbg(adapter, drv, dev, "set MTU to %d\n", new_mtu);
@@ -3008,8 +3015,6 @@  static void ena_set_conf_feat_params(struct ena_adapter *adapter,
 	ena_set_dev_offloads(feat, netdev);
 
 	adapter->max_mtu = feat->dev_attr.max_mtu;
-	netdev->max_mtu = adapter->max_mtu;
-	netdev->min_mtu = ENA_MIN_MTU;
 }
 
 static int ena_rss_init_default(struct ena_adapter *adapter)