netdev - Re: [PATCH net-next 10/13] net: ena: add mtu limitation in ena_change

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20170618.122103.1131376179675660541.davem@davemloft.net>
Date:   Sun, 18 Jun 2017 12:21:03 -0400 (EDT)
From:   David Miller <davem@...emloft.net>
To:     netanel@...zon.com
Cc:     netdev@...r.kernel.org, dwmw@...zon.com, zorik@...zon.com,
        matua@...zon.com, saeedb@...zon.com, msw@...zon.com,
        aliguori@...zon.com, nafea@...zon.com, evgenys@...zon.com
Subject: Re: [PATCH net-next 10/13] net: ena: add mtu limitation in
 ena_change_mtu()

From: <netanel@...zon.com>
Date: Sun, 18 Jun 2017 14:28:15 +0300

> From: Netanel Belgazal <netanel@...zon.com>
> 
> Signed-off-by: Netanel Belgazal <netanel@...zon.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.