netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Edward Cree <ecree.xilinx@gmail.com>
To: Jamal Hadi Salim <jhs@mojatatu.com>, Paul Blakey <paulb@nvidia.com>
Cc: netdev@vger.kernel.org, Saeed Mahameed <saeedm@nvidia.com>,
	Paolo Abeni <pabeni@redhat.com>, Jakub Kicinski <kuba@kernel.org>,
	Eric Dumazet <edumazet@google.com>,
	Cong Wang <xiyou.wangcong@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Oz Shlomo <ozsh@nvidia.com>, Jiri Pirko <jiri@nvidia.com>,
	Roi Dayan <roid@nvidia.com>, Vlad Buslov <vladbu@nvidia.com>
Subject: Re: [PATCH net-next 0/4] net/sched: cls_api: Support hardware miss to tc action
Date: Fri, 20 Jan 2023 11:54:30 +0000	[thread overview]
Message-ID: <912be77b-3723-33a7-8fee-05262b94efa1@gmail.com> (raw)
In-Reply-To: <CAM0EoM=FaRBWqxPv=jZdV_SZxqw26_yhK__q=o-9vqypSdMV1w@mail.gmail.com>

On 17/01/2023 13:40, Jamal Hadi Salim wrote:
> I agree that with your patch it will be operationally simpler. I hope other
> vendors will be able to use this feature (and the only reason i am saying
> this is because you are making core tc changes).
FTR at AMD/sfc we wouldn't use this as our HW has all action execution after
 all match stages in the pipeline (excepting actions that only control match
 behaviour, i.e. ct lookup), so users on ef100 HW (and I'd imagine probably
 some other vendors' products too) would still need to rewrite their rules
 with skbmark.
I mention this because this feature / patch series disconcerts me.  I wasn't
 even really happy about the 'miss to chain' feature, but even more so 'miss
 to action' feels like it makes the TC-driver offload interface more complex
 than it really ought to be.
Especially because the behaviour in some cases is already weird even with a
 fully offloadable rule; consider a match-all filter with 'action vlan push'
 and no further actions (specifically no redirect).  AIUI the HW will push
 the vlan, then deliver to the default destination (e.g. repr if the packet
 came from a VF), at which point TC SW will apply the same rule and perform
 the vlan push again, leading (incorrectly) to a double-tagged packet.
So it's not really about 'miss', there's a more fundamental issue with how
 HW offload and the SW path interact.  And I don't think it's possible to
 guaranteed-remove that issue without a performance cost (skb ext is
 expensive and we don't want it on *every* RX packet), so users will always
 need to consider this sort of thing when writing their TC rules.

TBH doing an IP address pedit before a CT lookup seems like a fairly
 contrived use case to me.  Is there a pressing real-world need for it, or
 are mlx just adding this support because they can?

-ed

  parent reply	other threads:[~2023-01-20 11:54 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-12 10:58 [PATCH net-next 0/4] net/sched: cls_api: Support hardware miss to tc action Paul Blakey
2023-01-12 10:59 ` [PATCH net-next 1/6] " Paul Blakey
2023-01-12 15:45   ` kernel test robot
2023-01-12 10:59 ` [PATCH net-next 2/6] net/sched: flower: Move filter handle initialization earlier Paul Blakey
2023-01-12 10:59 ` [PATCH net-next 3/6] net/sched: flower: Support hardware miss to tc action Paul Blakey
2023-01-12 10:59 ` [PATCH net-next 4/6] net/mlx5: Refactor tc miss handling to a single function Paul Blakey
2023-01-12 10:59 ` [PATCH net-next 5/6] net/mlx5e: Rename CHAIN_TO_REG to MAPPED_OBJ_TO_REG Paul Blakey
2023-01-12 10:59 ` [PATCH net-next 6/6] net/mlx5: TC, Set CT miss to the specific ct action instance Paul Blakey
2023-01-12 12:24 ` [PATCH net-next 0/4] net/sched: cls_api: Support hardware miss to tc action Jamal Hadi Salim
2023-01-15 12:04   ` Paul Blakey
2023-01-17 13:40     ` Jamal Hadi Salim
2023-01-17 14:48       ` Paul Blakey
2023-01-18 12:54         ` Jamal Hadi Salim
2023-01-18 21:27           ` Paul Blakey
2023-01-23 20:00             ` Jamal Hadi Salim
2023-01-20 11:54       ` Edward Cree [this message]
2023-01-22  9:33         ` Paul Blakey

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=912be77b-3723-33a7-8fee-05262b94efa1@gmail.com \
    --to=ecree.xilinx@gmail.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=jhs@mojatatu.com \
    --cc=jiri@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=ozsh@nvidia.com \
    --cc=pabeni@redhat.com \
    --cc=paulb@nvidia.com \
    --cc=roid@nvidia.com \
    --cc=saeedm@nvidia.com \
    --cc=vladbu@nvidia.com \
    --cc=xiyou.wangcong@gmail.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 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).