netdev - Re: [RFC PATCH 0/9] ethtool netlink interface (WiP)

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]

Message-ID: <fbc04937-6fa5-b94d-3115-2350c9a7fa3d@gmail.com>
Date:   Mon, 11 Dec 2017 11:03:42 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     David Miller <davem@...emloft.net>, jiri@...nulli.us
Cc:     mkubecek@...e.cz, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 0/9] ethtool netlink interface (WiP)

On 12/11/2017 09:01 AM, David Miller wrote:
> From: Jiri Pirko <jiri@...nulli.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