From: Simon Horman <simon.horman@corigine•com>
To: Pedro Tammela <pctammela@mojatatu•com>
Cc: netdev@vger•kernel.org, jhs@mojatatu•com,
xiyou.wangcong@gmail•com, jiri@resnulli•us, davem@davemloft•net,
edumazet@google•com, kuba@kernel•org, pabeni@redhat•com,
amir@vadai•me, dcaratti@redhat•com, willemb@google•com,
simon.horman@netronome•com, john.hurley@netronome•com,
yotamg@mellanox•com, ozsh@nvidia•com, paulb@nvidia•com
Subject: Re: [PATCH net 1/3] net/sched: act_pedit: fix action bind logic
Date: Sat, 25 Feb 2023 14:08:40 +0100 [thread overview]
Message-ID: <Y/oIWNU5ryYmPPO1@corigine.com> (raw)
In-Reply-To: <20230224150058.149505-2-pctammela@mojatatu.com>
On Fri, Feb 24, 2023 at 12:00:56PM -0300, Pedro Tammela wrote:
> The TC architecture allows filters and actions to be created independently.
> In filters the user can reference action objects using:
> tc action add action pedit ... index 1
> tc filter add ... action pedit index 1
>
> In the current code for act_pedit this is broken as it checks netlink
> attributes for create/update before actually checking if we are binding to an
> existing action.
>
> tdc results:
> 1..69
...
Hi Pedro,
Thanks for running the tests :)
I think this patch looks good - though I am still digesting it.
But I do wonder if you considered adding a test for this condition.
Also, what is the failure mode?
If it is that user's can't bind actions to filters, but the kernel behaves
correctly with configurations it accepts. If so, then perhaps this is more
of a feature than a fix. OTOH, perhaps it's a regression wrt the oldest of
the two patches references below.
I've haven't looked at the other patches in this series yet.
But I expect my comments apply to them too.
> Fixes: 71d0ed7079df ("net/act_pedit: Support using offset relative to the conventional network headers")
> Fixes: f67169fef8db ("net/sched: act_pedit: fix WARN() in the traffic path")
> Reviewed-by: Jamal Hadi Salim <jhs@mojatatu•com>
> Signed-off-by: Pedro Tammela <pctammela@mojatatu•com>
...
next prev parent reply other threads:[~2023-02-25 13:08 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-24 15:00 [PATCH net 0/3] net/sched: fix action bind logic Pedro Tammela
2023-02-24 15:00 ` [PATCH net 1/3] net/sched: act_pedit: " Pedro Tammela
2023-02-25 13:08 ` Simon Horman [this message]
2023-02-25 13:38 ` Pedro Tammela
2023-02-25 16:15 ` Simon Horman
2023-02-27 19:36 ` Jakub Kicinski
2023-02-27 19:51 ` Jamal Hadi Salim
2023-02-27 20:04 ` Jakub Kicinski
2023-02-27 21:41 ` Jakub Kicinski
2023-02-27 21:47 ` Jamal Hadi Salim
2023-02-24 15:00 ` [PATCH net 2/3] net/sched: act_mpls: " Pedro Tammela
2023-02-25 16:17 ` Simon Horman
2023-02-24 15:00 ` [PATCH net 3/3] net/sched: act_sample: " Pedro Tammela
2023-02-25 16:20 ` Simon Horman
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=Y/oIWNU5ryYmPPO1@corigine.com \
--to=simon.horman@corigine$(echo .)com \
--cc=amir@vadai$(echo .)me \
--cc=davem@davemloft$(echo .)net \
--cc=dcaratti@redhat$(echo .)com \
--cc=edumazet@google$(echo .)com \
--cc=jhs@mojatatu$(echo .)com \
--cc=jiri@resnulli$(echo .)us \
--cc=john.hurley@netronome$(echo .)com \
--cc=kuba@kernel$(echo .)org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=ozsh@nvidia$(echo .)com \
--cc=pabeni@redhat$(echo .)com \
--cc=paulb@nvidia$(echo .)com \
--cc=pctammela@mojatatu$(echo .)com \
--cc=simon.horman@netronome$(echo .)com \
--cc=willemb@google$(echo .)com \
--cc=xiyou.wangcong@gmail$(echo .)com \
--cc=yotamg@mellanox$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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