On 30/07/18 12:41 PM, Paolo Abeni wrote:
On Mon, 2018-07-30 at 10:03 -0400, Jamal Hadi Salim wrote:
On 30/07/18 08:30 AM, Paolo Abeni wrote:
}
+ if (!tcf_action_valid(a->tcfa_action)) {
+ NL_SET_ERR_MSG(extack, "invalid action value, using TC_ACT_UNSPEC
instead");
+ a->tcfa_action = TC_ACT_UNSPEC;
+ }
+
return a;
I think it would make a lot more sense to just reject the entry than
changing it underneath the user to a default value. Least element of
suprise.
I fear that would break existing (bad) users ?!? This way, such users
are notified they are doing something uncorrect, but still continue to
work.
By "bad users" I think you mean someone setting a policy expecting
one behavior but getting a different one? If yes, that policy was
already wrong/buggy. As an example, if i configured:
match xxx action foo action goo action bar action gah
where action goo has a bad opcode
If you "fix it" with TC_ACT_UNSPEC then basically the above
policy is now equivalent to:
match xxx action foo action goo
Infact if there was a lower prio rule in the chain
then lookup will continue there and produce even stranger
results.
cheers,
jamal
The patch can be changed to reject bad actions, if there is agreement,
but it would not look as the safest way to me.
