netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>
To: David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>,
	johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	ath10k-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [PATCH 1/3] ethtool: Support ETHTOOL_GSTATS2 command.
Date: Mon, 23 Apr 2018 08:38:25 -0700	[thread overview]
Message-ID: <3fc50436-be80-cd06-b5db-373f3f3e5b15@candelatech.com> (raw)
In-Reply-To: <20180422.145420.1197041027922699603.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

On 04/22/2018 11:54 AM, David Miller wrote:
> From: Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
> Date: Thu, 19 Apr 2018 17:26:57 +0200
>
>> On Thu, 2018-04-19 at 08:25 -0700, Ben Greear wrote:
>>>
>>> Maybe this could be in followup patches?  It's going to touch a lot of files,
>>> and might be hell to get merged all at once, and I've never used spatch, so
>>> just maybe someone else will volunteer that part :)
>>
>> I guess you'll have to ask davem. :)
>
> Well, first of all, I really don't like this.
>
> The first reason is that every time I see interface foo become foo2,
> foo3 is never far behind it.
>
> If foo was not extensible enough such that we needed foo2, we beter
> design the new thing with explicitly better extensibility in mind.
>
> Furthermore, what you want here is a specific filter.  Someone else
> will want to filter on another criteria, and the next person will
> want yet another.
>
> This needs to be properly generalized.
>
> And frankly if we had moved to ethtool netlink/devlink by now, we
> could just add a netlink attribute for filtering and not even be
> having this conversation.

Well, since there are un-defined flags, it would be simple enough to
extend the API further in the future (flag (1<<31) could mean expect
more input members, etc.  And, adding up to 30 more flags to filter on different
things won't change the API and should be backwards compatible.

But, if you don't want it, that is OK by me, I agree it is a fairly
obscure feature.  It would have saved me time if you had said you didn't
want it at the first RFC patch though...

Thanks,
Ben

-- 
Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>
Candela Technologies Inc  http://www.candelatech.com

      parent reply	other threads:[~2018-04-23 15:38 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-18  1:49 [PATCH 1/3] ethtool: Support ETHTOOL_GSTATS2 command greearb-my8/4N5VtI7c+919tysfdA
2018-04-18  1:49 ` [PATCH 2/3] mac80211: Add support for ethtool gstats2 API greearb
2018-04-18  1:49 ` [PATCH 3/3] ath10k: Support " greearb
2018-04-19 11:19   ` kbuild test robot
2018-04-18 21:07 ` [PATCH 1/3] ethtool: Support ETHTOOL_GSTATS2 command Florian Fainelli
2018-04-18 21:26 ` Johannes Berg
2018-04-18 21:51   ` Ben Greear
2018-04-19  6:38     ` Johannes Berg
2018-04-19 15:25       ` Ben Greear
     [not found]         ` <173c5f98-36bc-2e52-1e64-3a5f89008d46-my8/4N5VtI7c+919tysfdA@public.gmane.org>
2018-04-19 15:26           ` Johannes Berg
     [not found]             ` <1524151617.3024.25.camel-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
2018-04-22 18:54               ` David Miller
     [not found]                 ` <20180422.145420.1197041027922699603.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
2018-04-22 21:15                   ` Roopa Prabhu
2018-04-23 15:41                     ` Ben Greear
2018-04-23 15:38                   ` Ben Greear [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3fc50436-be80-cd06-b5db-373f3f3e5b15@candelatech.com \
    --to=greearb-my8/4n5vti7c+919tysfda@public.gmane.org \
    --cc=ath10k-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org \
    --cc=johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org \
    --cc=linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).