netdev - Re: [PATCH net-next v2 2/3] ethtool: Add support for DMA buffer settings

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]

Date:	Wed, 30 Jul 2014 17:53:06 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	_govind@....com
Cc:	ben@...adent.org.uk, netdev@...r.kernel.org, ssujith@...co.com,
	benve@...co.com
Subject: Re: [PATCH net-next v2 2/3] ethtool: Add support for DMA buffer
 settings

From: Govindarajulu Varadarajan <_govind@....com>
Date: Tue, 29 Jul 2014 17:10:38 +0530

> @@ -440,6 +440,20 @@ struct ethtool_ringparam {
>  };
>  
>  /**
> + * struct ethtool_buffparam - DMA buffer parameters
> + * @rx_copybreak_cur: current receive DMA buff rx_copybreak.
> + * @rx_copybreak_min: min rx_copybreak set by driver.
> + * @rx_copybreak_max: Max rx_copybreak set by driver.
> + * @reserved: reserve room for future use.
> + */
> +struct ethtool_buffparam {
> +	__u32	cmd;
> +	__u32	rx_copybreak_cur;
> +	__u32	rx_copybreak_max;
> +	__u8	reserved[84];
> +};

I don't understand the reasoning behind this reserved field.

You can't use it to add more fields later, because right now
we'll let the user put any garbage there and thus if you add
more fields that garbage from older apps would be interpreted
as one of the new values.

Largely we have not been adding reserved fields to new ethtool
structures, and this is the primary reason I guess.

It's a shame that when we want to add a new 32-bit knob we have
to add an entire new struct.

We have ethtool_value, but that's only good for one knob at a time and
we have to allocate an entire new ethtool command value for each such
knob.

I really don't know what the recommend here.

However I wonder what value that "max" thing has, I think setting the
value to infinity is just fine, it just means every packet will be
copied.

So if we remove the 'max', we just have the copybreak itself, and you
can therefore use ethtool_value and allocate the ethtool command
numbers.

What do you think?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html