All of lore.kernel.org
 help / color / mirror / Atom feed
From: Florian Fainelli <f.fainelli@gmail.com>
To: Vinicius Costa Gomes <vinicius.gomes@intel.com>,
	intel-wired-lan@lists.osuosl.org
Cc: jeffrey.t.kirsher@intel.com, netdev@vger.kernel.org,
	jesus.sanchez-palencia@intel.com
Subject: Re: [next-queue PATCH 7/8] igb: Add support for adding offloaded clsflower filters
Date: Fri, 23 Feb 2018 20:45:44 -0800	[thread overview]
Message-ID: <9F146DDF-A3CC-404D-AA36-F4A4E444DD2D@gmail.com> (raw)
In-Reply-To: <20180224012036.5834-8-vinicius.gomes@intel.com>

On February 23, 2018 5:20:35 PM PST, Vinicius Costa Gomes <vinicius.gomes@intel.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.

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?

-- 
Florian

WARNING: multiple messages have this Message-ID (diff)
From: Florian Fainelli <f.fainelli@gmail.com>
To: intel-wired-lan@osuosl.org
Subject: [Intel-wired-lan] [next-queue PATCH 7/8] igb: Add support for adding offloaded clsflower filters
Date: Fri, 23 Feb 2018 20:45:44 -0800	[thread overview]
Message-ID: <9F146DDF-A3CC-404D-AA36-F4A4E444DD2D@gmail.com> (raw)
In-Reply-To: <20180224012036.5834-8-vinicius.gomes@intel.com>

On February 23, 2018 5:20:35 PM PST, Vinicius Costa Gomes <vinicius.gomes@intel.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.

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?

-- 
Florian

  reply	other threads:[~2018-02-24  4:45 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-24  1:20 [next-queue PATCH 0/8] igb: offloading of receive filters Vinicius Costa Gomes
2018-02-24  1:20 ` [Intel-wired-lan] " Vinicius Costa Gomes
2018-02-24  1:20 ` [next-queue PATCH 1/8] igb: Fix not adding filter elements to the list Vinicius Costa Gomes
2018-02-24  1:20   ` [Intel-wired-lan] " Vinicius Costa Gomes
2018-02-24  1:20 ` [next-queue PATCH 2/8] igb: Fix queue selection on MAC filters on i210 and i211 Vinicius Costa Gomes
2018-02-24  1:20   ` [Intel-wired-lan] " Vinicius Costa Gomes
2018-02-24  1:20 ` [next-queue PATCH 3/8] igb: Enable the hardware traffic class feature bit for igb models Vinicius Costa Gomes
2018-02-24  1:20   ` [Intel-wired-lan] " Vinicius Costa Gomes
2018-02-25 22:37   ` Alexander Duyck
2018-02-25 22:37     ` Alexander Duyck
2018-02-26 18:49     ` Vinicius Costa Gomes
2018-02-26 18:49       ` Vinicius Costa Gomes
2018-02-24  1:20 ` [next-queue PATCH 4/8] igb: Add support for MAC address filters specifying source addresses Vinicius Costa Gomes
2018-02-24  1:20   ` [Intel-wired-lan] " Vinicius Costa Gomes
2018-02-25 22:42   ` Alexander Duyck
2018-02-25 22:42     ` Alexander Duyck
2018-02-26 19:24     ` Vinicius Costa Gomes
2018-02-26 19:24       ` Vinicius Costa Gomes
2018-02-24  1:20 ` [next-queue PATCH 5/8] igb: Add support for ethtool MAC address filters Vinicius Costa Gomes
2018-02-24  1:20   ` [Intel-wired-lan] " Vinicius Costa Gomes
2018-02-24  4:38   ` Florian Fainelli
2018-02-24  4:38     ` [Intel-wired-lan] " Florian Fainelli
2018-02-26 19:30     ` Vinicius Costa Gomes
2018-02-26 19:30       ` [Intel-wired-lan] " Vinicius Costa Gomes
2018-02-24  1:20 ` [next-queue PATCH 6/8] igb: Add the skeletons for tc-flower offloading Vinicius Costa Gomes
2018-02-24  1:20   ` [Intel-wired-lan] " Vinicius Costa Gomes
2018-02-24  1:20 ` [next-queue PATCH 7/8] igb: Add support for adding offloaded clsflower filters Vinicius Costa Gomes
2018-02-24  1:20   ` [Intel-wired-lan] " Vinicius Costa Gomes
2018-02-24  4:45   ` Florian Fainelli [this message]
2018-02-24  4:45     ` Florian Fainelli
2018-02-27  0:40     ` Vinicius Costa Gomes
2018-02-27  0:40       ` [Intel-wired-lan] " Vinicius Costa Gomes
2018-02-27  0:51       ` Florian Fainelli
2018-02-27  0:51         ` [Intel-wired-lan] " Florian Fainelli
2018-02-27  1:51         ` Vinicius Costa Gomes
2018-02-27  1:51           ` [Intel-wired-lan] " Vinicius Costa Gomes
2018-02-26  0:12   ` kbuild test robot
2018-02-26  0:12     ` kbuild test robot
2018-02-24  1:20 ` [next-queue PATCH 8/8] igb: Add support for removing offloaded tc-flower filters Vinicius Costa Gomes
2018-02-24  1:20   ` [Intel-wired-lan] " Vinicius Costa Gomes
2018-02-24  4:36   ` Florian Fainelli
2018-02-24  4:36     ` [Intel-wired-lan] " Florian Fainelli
2018-02-26 20:59     ` Vinicius Costa Gomes
2018-02-26 20:59       ` [Intel-wired-lan] " Vinicius Costa Gomes

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=9F146DDF-A3CC-404D-AA36-F4A4E444DD2D@gmail.com \
    --to=f.fainelli@gmail.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=jeffrey.t.kirsher@intel.com \
    --cc=jesus.sanchez-palencia@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=vinicius.gomes@intel.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.