public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Florian Fainelli <f.fainelli@gmail.com>
To: David Miller <davem@davemloft.net>, jiri@resnulli.us
Cc: mkubecek@suse.cz, netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 0/9] ethtool netlink interface (WiP)
Date: Mon, 11 Dec 2017 11:03:42 -0800	[thread overview]
Message-ID: <fbc04937-6fa5-b94d-3115-2350c9a7fa3d@gmail.com> (raw)
In-Reply-To: <20171211.120144.1060832843526341781.davem@davemloft.net>

On 12/11/2017 09:01 AM, David Miller wrote:
> From: Jiri Pirko <jiri@resnulli.us>
> Date: Mon, 11 Dec 2017 17:32:46 +0100
> 
>> I think that it does not make sense to convert ethtool->netlink_ethtool
>> 1:1 feature wise. Now we have devlink, ritch switch representation
>> model, tc offload and many others. Lot of things that are in
>> ethtool, should be done in devlink. Also, there are couple of things
>> that should just die - nice example is ethtool --config-ntuple - we
>> should use tc for that.
> 
> Whilst I do agree that devlink is probably a good place for this stuff
> (we want to be able to do ethetool things on objects that lack a netdev)
> I do not agree with the tc angle.
> 
> It is entirely appropriate to set the ntuple settings of a driver
> without being required to use TC or similar.
> 
> All you are going to do with your suggestion is make people keep using
> the existing ethtool ioctl, because they'll say "screw this, I'm not
> using TC I have something which works just fine already".  And that's
> not the goal of putting this stuff into netlink, we want people to
> use the new facilities and move off of the ioctl.

I agree, we can't walk away from that feature today, there are many more
drivers implementing ethtool::ntuple (counting 22) than there are
implementing cls_flower (counting 6), also they are not strictly
equivalent feature wise, one thing critically missing in cls_flower
(last I looked) is the ability to either auto-place (RX_CLS_LOC_ANY) a
matching rule, or select a particular location. For specifying what to
match in a packet and how, cls_flower is superior.

We can probably advise people not to use that feature and request their
driver provider to switch over to cls_flower, but considering how many
more LOCs are needed in the driver to implement that (as opposed to
ethtool ntuple), I just don't see it being something people will be
thrilled to do.

Writing a shim that converts from ethtool ntuple to the equivalent in
kernel space representation of the same call through cls_flower would be
reasonably easy, but same thing, that still requires migrating your
kernel driver over to the cls_flower interface, which is the harder part.

That being said, we should probably discourage implementing both for new
drivers, since they are likely to use the same centralized HW resources
towards the same goals: match + actions, with no visibility into one
another (can't see rules set-up in ethtool via cls_flower and vice
versa) and that just sucks.
-- 
Florian

  parent reply	other threads:[~2017-12-11 19:03 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-11 13:53 [RFC PATCH 0/9] ethtool netlink interface (WiP) Michal Kubecek
2017-12-11 13:53 ` [RFC PATCH 1/9] netlink: introduce nla_put_bitfield32() Michal Kubecek
2017-12-11 13:53 ` [RFC PATCH 2/9] ethtool: introduce ethtool netlink interface Michal Kubecek
2017-12-11 16:02   ` Jiri Pirko
2017-12-11 16:56     ` David Miller
2017-12-11 18:02       ` Jiri Pirko
2017-12-11 18:45         ` David Miller
2017-12-12 23:56           ` Michal Kubecek
2017-12-11 13:53 ` [RFC PATCH 3/9] ethtool: helper functions for " Michal Kubecek
2017-12-11 13:53 ` [RFC PATCH 4/9] ethtool: netlink bitset handling Michal Kubecek
2017-12-11 13:54 ` [RFC PATCH 5/9] ethtool: implement GET_DRVINFO message Michal Kubecek
2017-12-11 16:16   ` Jiri Pirko
2017-12-12 23:54     ` Michal Kubecek
2017-12-13  6:57       ` Jiri Pirko
2017-12-11 13:54 ` [RFC PATCH 6/9] ethtool: implement GET_SETTINGS message Michal Kubecek
2017-12-11 13:54 ` [RFC PATCH 7/9] ethtool: implement SET_SETTINGS message Michal Kubecek
2017-12-11 13:54 ` [RFC PATCH 8/9] ethtool: implement GET_PARAMS message Michal Kubecek
2017-12-11 13:54 ` [RFC PATCH 9/9] ethtool: implement SET_PARAMS message Michal Kubecek
2017-12-11 16:32 ` [RFC PATCH 0/9] ethtool netlink interface (WiP) Jiri Pirko
2017-12-11 17:01   ` David Miller
2017-12-11 17:59     ` Jiri Pirko
2017-12-11 19:03     ` Florian Fainelli [this message]
2017-12-12 15:32 ` Roopa Prabhu
2017-12-12 23:47   ` Jakub Kicinski
2017-12-14 21:07 ` John W. Linville
2017-12-18 19:39   ` David Miller

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=fbc04937-6fa5-b94d-3115-2350c9a7fa3d@gmail.com \
    --to=f.fainelli@gmail.com \
    --cc=davem@davemloft.net \
    --cc=jiri@resnulli.us \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mkubecek@suse.cz \
    --cc=netdev@vger.kernel.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