Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1493988

Re: [PATCH v2 net-next 1/2] net: centralize net_device min/max MTU checking

Show all headers | View raw

On Wed, Sep 28, 2016 at 10:20 PM GMT, Jarod Wilson wrote:
> While looking into an MTU issue with sfc, I started noticing that almost
> every NIC driver with an ndo_change_mtu function implemented almost
> exactly the same range checks, and in many cases, that was the only
> practical thing their ndo_change_mtu function was doing. Quite a few
> drivers have either 68, 64, 60 or 46 as their minimum MTU value checked,
> and then various sizes from 1500 to 65535 for their maximum MTU value. We
> can remove a whole lot of redundant code here if we simple store min_mtu
> and max_mtu in net_device, and check against those in net/core/dev.c's
> dev_set_mtu().
>
> In theory, there should be zero functional change with this patch, it just
> puts the infrastructure in place. Subsequent patches will attempt to start
> using said infrastructure, with theoretically zero change in
> functionality.
>
> CC: "David S. Miller" <davem@davemloft.net>
> CC: netdev@vger.kernel.org
> Signed-off-by: Jarod Wilson <jarod@redhat.com>
> ---

[...]

> diff --git a/net/core/dev.c b/net/core/dev.c
> index c0c291f..5343799 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -6493,9 +6493,17 @@ int dev_set_mtu(struct net_device *dev, int new_mtu)
>  	if (new_mtu == dev->mtu)
>  		return 0;
>  
> -	/*	MTU must be positive.	 */
> -	if (new_mtu < 0)
> +	if (new_mtu < dev->min_mtu) {

Ouch, integral promotions. Looks like you need to keep the < 0 check.
Otherwise new_mtu gets promoted to unsigned int and negative values will
pass the check.

Thanks,
Jakub

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v2 net-next 1/2] net: centralize net_device min/max MTU checking Jarod Wilson <jarod@redhat.com> - 2016-09-29 00:30 +0200
  Re: [PATCH v2 net-next 1/2] net: centralize net_device min/max MTU checking Jakub Sitnicki <jkbs@redhat.com> - 2016-09-30 11:40 +0200
    Re: [PATCH v2 net-next 1/2] net: centralize net_device min/max MTU  checking David Miller <davem@davemloft.net> - 2016-10-03 04:50 +0200
      Re: [PATCH v2 net-next 1/2] net: centralize net_device min/max MTU  checking Jarod Wilson <jarod@redhat.com> - 2016-10-03 19:50 +0200
      [PATCH v3 net-next 0/2] net: centralize net_device MTU bounds checking Jarod Wilson <jarod@redhat.com> - 2016-10-08 04:10 +0200
        [PATCH v3 net-next 1/2] net: centralize net_device min/max MTU checking Jarod Wilson <jarod@redhat.com> - 2016-10-08 04:10 +0200

csiph-web