netdev - Re: [next-queue PATCH 7/8] igb: Add support for adding offloaded clsflower filters

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]

Date:   Mon, 26 Feb 2018 16:40:06 -0800
From:   Vinicius Costa Gomes <vinicius.gomes@...el.com>
To:     Florian Fainelli <f.fainelli@...il.com>,
        intel-wired-lan@...ts.osuosl.org
Cc:     jeffrey.t.kirsher@...el.com, netdev@...r.kernel.org,
        jesus.sanchez-palencia@...el.com
Subject: Re: [next-queue PATCH 7/8] igb: Add support for adding offloaded clsflower filters

Hi,

Florian Fainelli <f.fainelli@...il.com> writes:

> On February 23, 2018 5:20:35 PM PST, Vinicius Costa Gomes <vinicius.gomes@...el.com> wrote:
>>This allows filters added by tc-flower and specifying MAC addresses,
>>Ethernet types, and the VLAN priority field, to be offloaded to the
>>controller.
>>
>>This reuses most of the infrastructure used by ethtool, ethtool can be
>>used to read these filters, but modification and deletion can only be
>>done via tc-flower.
>
> You would want to check what other drivers supporting both
> ethtool::rxnfc and cls_flower do, but this can be seriously confusing
> to an user. As an user I would be more comfortable with seeing only
> rules added through ethtool via ethtool and those added by cls_flower
> via cls_flower. They will both access a shared set of resources but it
> seems easier for me to dump rules with both tools to figure out why
> -ENOSPC was returned rather than seeing something I did not add.
> Others might see it entirely differently.

I took a closer look at mlx5 and i40e, and they seem to use different
hardware capabilities (even incompatible in the case of i40e, which
returns an error when tring to add cls_flower filter when an ethtool
based filter exists) for ethtool and cls_flower. So I don't think the
model applies exactly here.

As they keep the filters separated for the user perspective, I could do
the same, in the name of convention, it's just that the separation is
more "artificial". But I have no strong opinions either way.

>
> If you added the ability for cls_flower to indicate a queue number and
> either a fixed rule index or auto-placement (RX_CLS_LOC_ANY), could
> that eliminate entirely the need for adding MAC address steering in
> earlier patches?

I am not sure that I understand. 'cls_flower' already has support for
indicating a queue number (expressed via the 'hw_tc' parameter to tc)
(commit 384c181e3780 ("net: sched: Identify hardware traffic classes
using classid").

And adding more control for the allocation of indexes for the rules seem
not to help much in reducing the size/complexity of this series. I.e.
this series has 4 parts: bug fixes, adding support for source addresses
for MAC filters, adding ethtool support MAC address filters (as it was
only missing some glue code), and adding offloading for some types of
cls_flower filters. More control for the allocation of rule indexes would
only affect the cls_flower part.

But perhaps I could be missing something here.

>
> -- 
> Florian