On Fri, Nov 20, 2020 at 08:23:22AM +0100, Michal Kubecek wrote:

On Fri, Nov 20, 2020 at 10:59:59AM +0800, tanhuazhong wrote:

quoted

On 2020/11/20 6:02, Michal Kubecek wrote:

quoted

We could use a similar approach as struct ethtool_link_ksettings, e.g.

	struct kernel_ethtool_coalesce {
		struct ethtool_coalesce base;
		/* new members which are not part of UAPI */
	}

get_coalesce() and set_coalesce() would get pointer to struct
kernel_ethtool_coalesce and ioctl code would be modified to only touch
the base (legacy?) part.
 > While already changing the ops arguments, we could also add extack
pointer, either as a separate argument or as struct member (I slightly
prefer the former).

If changing the ops arguments, each driver who implement
set_coalesce/get_coalesce of ethtool_ops need to be updated. Is it
acceptable adding two new ops to get/set ext_coalesce info (like
ecc31c60240b ("ethtool: Add link extended state") does)? Maybe i can send V2
in this way, and then could you help to see which one is more suitable?

If it were just this one case, adding an extra op would be perfectly
fine. But from long term point of view, we should expect extending also
other existing ethtool requests and going this way for all of them would
essentially double the number of callbacks in struct ethtool_ops.

coccinella might be useful here.

	   Andrew