Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Michal Kubecek <mkubecek@suse.cz>
To: Eric Dumazet <edumazet@google.com>
Cc: "Kevin(Yudong) Yang" <yyd@google.com>,
	netdev <netdev@vger.kernel.org>,
	Neal Cardwell <ncardwell@google.com>
Subject: Re: [PATCH ethtool,v2] ethtool: add support show/set-time-stamping
Date: Mon, 7 Sep 2020 23:25:42 +0200	[thread overview]
Message-ID: <20200907212542.rnwzu3cn24uewyk4@lion.mk-sys.cz> (raw)
In-Reply-To: <CANn89iKZ19+AJOf5_5orPrUObYef+L-HrwF_Oay6o75ZbG7UhQ@mail.gmail.com>

On Mon, Sep 07, 2020 at 06:56:20PM +0200, Eric Dumazet wrote:
> On Mon, Sep 7, 2020 at 2:53 PM Michal Kubecek <mkubecek@suse.cz> wrote:
> >
> > As I said in response to v1 patch, I don't like the idea of adding a new
> > ioctl interface to ethool when we are working on replacing and
> > deprecating the existing ones. Is there a strong reason why this feature
> > shouldn't be implemented using netlink?
> 
> I do not think this is a fair request.
> 
> All known kernels support the ioctl(), none of them support netlink so far.

Several years ago, exactly the same was true for bonding, bridge or vlan
configuration: all known kernels supported ioctl() or sysfs interfaces
for them, none supported netlink at that point. By your logic, the right
course of action would have been using ioctl() and sysfs for iproute2
support. Instead, rtnetlink interfaces were implemented and used by
iproute2. I believe it was the right choice.

> Are you working on the netlink interface, or are you requesting us to
> implement it ?

If it helps, I'm willing to write the kernel side. Or both, if
necessary, just to avoid adding another ioctl monument that would have
to be kept and maintained for many years, maybe forever.

> The ioctl has been added years ago, and Kevin patch is reasonable enough.

And there is a utility using the ioctl, as Andrew pointed out. Just like
there were brctl and vconfig and ioctl they were using. The existence of
those ioctl was not considered sufficient reason to use them when bridge
and vlan support was added to iproute2. I don't believe today's
situation with ethtool is different.

Michal

  reply	other threads:[~2020-09-07 21:26 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-03 14:07 Kevin(Yudong) Yang
2020-09-07 12:53 ` Michal Kubecek
2020-09-07 16:56   ` Eric Dumazet
2020-09-07 21:25     ` Michal Kubecek [this message]
2020-09-08  5:35       ` Eric Dumazet
2020-09-08 10:37         ` Michal Kubecek
2020-09-08 11:17           ` Eric Dumazet

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=20200907212542.rnwzu3cn24uewyk4@lion.mk-sys.cz \
    --to=mkubecek@suse.cz \
    --cc=edumazet@google.com \
    --cc=ncardwell@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=yyd@google.com \
    --subject='Re: [PATCH ethtool,v2] ethtool: add support show/set-time-stamping' \
    /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

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).