| Message ID | 1521724929-13353-1-git-send-email-roid@mellanox.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,branch-2.8] lib/tc: Handle error parsing action in nl_parse_single_action | expand |
On 22/03/2018 15:22, Roi Dayan wrote: > Raise the error up instead of ignoring it. > Before this commit beside an error an incorrect rule was also printed. > > Signed-off-by: Roi Dayan <roid@mellanox.com> > Reviewed-by: Paul Blakey <paulb@mellanox.com> > Signed-off-by: Simon Horman <simon.horman@netronome.com> oops sorry about the signed off. i cherry-picked from master branch for the backport and forgot to remove. > --- > lib/tc.c | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/lib/tc.c b/lib/tc.c > index 6f8cd1b9faf7..0d099aa1a9c3 100644 > --- a/lib/tc.c > +++ b/lib/tc.c > @@ -570,6 +570,7 @@ nl_parse_single_action(struct nlattr *action, struct tc_flower *flower) > struct nlattr *stats_attrs[ARRAY_SIZE(stats_policy)]; > struct ovs_flow_stats *stats = &flower->stats; > const struct gnet_stats_basic *bs; > + int err = 0; > > if (!nl_parse_nested(action, act_policy, action_attrs, > ARRAY_SIZE(act_policy))) { > @@ -582,16 +583,20 @@ nl_parse_single_action(struct nlattr *action, struct tc_flower *flower) > act_cookie = action_attrs[TCA_ACT_COOKIE]; > > if (!strcmp(act_kind, "gact")) { > - nl_parse_act_drop(act_options, flower); > + err = nl_parse_act_drop(act_options, flower); > } else if (!strcmp(act_kind, "mirred")) { > - nl_parse_act_mirred(act_options, flower); > + err = nl_parse_act_mirred(act_options, flower); > } else if (!strcmp(act_kind, "vlan")) { > - nl_parse_act_vlan(act_options, flower); > + err = nl_parse_act_vlan(act_options, flower); > } else if (!strcmp(act_kind, "tunnel_key")) { > - nl_parse_act_tunnel_key(act_options, flower); > + err = nl_parse_act_tunnel_key(act_options, flower); > } else { > VLOG_ERR_RL(&error_rl, "unknown tc action kind: %s", act_kind); > - return EINVAL; > + err = EINVAL; > + } > + > + if (err) { > + return err; > } > > if (act_cookie) { >
On 22/03/2018 15:44, Roi Dayan wrote: > > > On 22/03/2018 15:22, Roi Dayan wrote: >> Raise the error up instead of ignoring it. >> Before this commit beside an error an incorrect rule was also printed. >> >> Signed-off-by: Roi Dayan <roid@mellanox.com> >> Reviewed-by: Paul Blakey <paulb@mellanox.com> >> Signed-off-by: Simon Horman <simon.horman@netronome.com> > > oops sorry about the signed off. i cherry-picked from master branch for > the backport and forgot to remove. Simon, let me know if you want me to resend it clean. > >> --- >> lib/tc.c | 15 ++++++++++----- >> 1 file changed, 10 insertions(+), 5 deletions(-) >> >> diff --git a/lib/tc.c b/lib/tc.c >> index 6f8cd1b9faf7..0d099aa1a9c3 100644 >> --- a/lib/tc.c >> +++ b/lib/tc.c >> @@ -570,6 +570,7 @@ nl_parse_single_action(struct nlattr *action, >> struct tc_flower *flower) >> struct nlattr *stats_attrs[ARRAY_SIZE(stats_policy)]; >> struct ovs_flow_stats *stats = &flower->stats; >> const struct gnet_stats_basic *bs; >> + int err = 0; >> if (!nl_parse_nested(action, act_policy, action_attrs, >> ARRAY_SIZE(act_policy))) { >> @@ -582,16 +583,20 @@ nl_parse_single_action(struct nlattr *action, >> struct tc_flower *flower) >> act_cookie = action_attrs[TCA_ACT_COOKIE]; >> if (!strcmp(act_kind, "gact")) { >> - nl_parse_act_drop(act_options, flower); >> + err = nl_parse_act_drop(act_options, flower); >> } else if (!strcmp(act_kind, "mirred")) { >> - nl_parse_act_mirred(act_options, flower); >> + err = nl_parse_act_mirred(act_options, flower); >> } else if (!strcmp(act_kind, "vlan")) { >> - nl_parse_act_vlan(act_options, flower); >> + err = nl_parse_act_vlan(act_options, flower); >> } else if (!strcmp(act_kind, "tunnel_key")) { >> - nl_parse_act_tunnel_key(act_options, flower); >> + err = nl_parse_act_tunnel_key(act_options, flower); >> } else { >> VLOG_ERR_RL(&error_rl, "unknown tc action kind: %s", act_kind); >> - return EINVAL; >> + err = EINVAL; >> + } >> + >> + if (err) { >> + return err; >> } >> if (act_cookie) { >>
On Thu, Mar 22, 2018 at 03:46:17PM +0200, Roi Dayan wrote: > > > On 22/03/2018 15:44, Roi Dayan wrote: > > > > > > On 22/03/2018 15:22, Roi Dayan wrote: > > > Raise the error up instead of ignoring it. > > > Before this commit beside an error an incorrect rule was also printed. > > > > > > Signed-off-by: Roi Dayan <roid@mellanox.com> > > > Reviewed-by: Paul Blakey <paulb@mellanox.com> > > > Signed-off-by: Simon Horman <simon.horman@netronome.com> > > > > oops sorry about the signed off. i cherry-picked from master branch for > > the backport and forgot to remove. > > Simon, let me know if you want me to resend it clean. No need to repost for that.
On Thu, Mar 22, 2018 at 03:32:58PM +0100, Simon Horman wrote: > On Thu, Mar 22, 2018 at 03:46:17PM +0200, Roi Dayan wrote: > > > > > > On 22/03/2018 15:44, Roi Dayan wrote: > > > > > > > > > On 22/03/2018 15:22, Roi Dayan wrote: > > > > Raise the error up instead of ignoring it. > > > > Before this commit beside an error an incorrect rule was also printed. > > > > > > > > Signed-off-by: Roi Dayan <roid@mellanox.com> > > > > Reviewed-by: Paul Blakey <paulb@mellanox.com> > > > > Signed-off-by: Simon Horman <simon.horman@netronome.com> > > > > > > oops sorry about the signed off. i cherry-picked from master branch for > > > the backport and forgot to remove. > > > > Simon, let me know if you want me to resend it clean. > > No need to repost for that. Simon, are you still looking at this to possibly apply it to branch-2.8?
On Wed, Apr 04, 2018 at 04:51:01PM -0700, Ben Pfaff wrote: > On Thu, Mar 22, 2018 at 03:32:58PM +0100, Simon Horman wrote: > > On Thu, Mar 22, 2018 at 03:46:17PM +0200, Roi Dayan wrote: > > > > > > > > > On 22/03/2018 15:44, Roi Dayan wrote: > > > > > > > > > > > > On 22/03/2018 15:22, Roi Dayan wrote: > > > > > Raise the error up instead of ignoring it. > > > > > Before this commit beside an error an incorrect rule was also printed. > > > > > > > > > > Signed-off-by: Roi Dayan <roid@mellanox.com> > > > > > Reviewed-by: Paul Blakey <paulb@mellanox.com> > > > > > Signed-off-by: Simon Horman <simon.horman@netronome.com> > > > > > > > > oops sorry about the signed off. i cherry-picked from master branch for > > > > the backport and forgot to remove. > > > > > > Simon, let me know if you want me to resend it clean. > > > > No need to repost for that. > > Simon, are you still looking at this to possibly apply it to branch-2.8? Sorry about letting this slip through the cracks, I'll look into it ASAP.
On Thu, Apr 05, 2018 at 05:16:59PM +0300, Simon Horman wrote: > On Wed, Apr 04, 2018 at 04:51:01PM -0700, Ben Pfaff wrote: > > On Thu, Mar 22, 2018 at 03:32:58PM +0100, Simon Horman wrote: > > > On Thu, Mar 22, 2018 at 03:46:17PM +0200, Roi Dayan wrote: > > > > > > > > > > > > On 22/03/2018 15:44, Roi Dayan wrote: > > > > > > > > > > > > > > > On 22/03/2018 15:22, Roi Dayan wrote: > > > > > > Raise the error up instead of ignoring it. > > > > > > Before this commit beside an error an incorrect rule was also printed. > > > > > > > > > > > > Signed-off-by: Roi Dayan <roid@mellanox.com> > > > > > > Reviewed-by: Paul Blakey <paulb@mellanox.com> > > > > > > Signed-off-by: Simon Horman <simon.horman@netronome.com> > > > > > > > > > > oops sorry about the signed off. i cherry-picked from master branch for > > > > > the backport and forgot to remove. > > > > > > > > Simon, let me know if you want me to resend it clean. > > > > > > No need to repost for that. > > > > Simon, are you still looking at this to possibly apply it to branch-2.8? > > Sorry about letting this slip through the cracks, I'll look into it ASAP. Sorry once again for the delay, I have applied this patch, which I had previously reviewed but somehow not pushed, to branch-2.8.
diff --git a/lib/tc.c b/lib/tc.c index 6f8cd1b9faf7..0d099aa1a9c3 100644 --- a/lib/tc.c +++ b/lib/tc.c @@ -570,6 +570,7 @@ nl_parse_single_action(struct nlattr *action, struct tc_flower *flower) struct nlattr *stats_attrs[ARRAY_SIZE(stats_policy)]; struct ovs_flow_stats *stats = &flower->stats; const struct gnet_stats_basic *bs; + int err = 0; if (!nl_parse_nested(action, act_policy, action_attrs, ARRAY_SIZE(act_policy))) { @@ -582,16 +583,20 @@ nl_parse_single_action(struct nlattr *action, struct tc_flower *flower) act_cookie = action_attrs[TCA_ACT_COOKIE]; if (!strcmp(act_kind, "gact")) { - nl_parse_act_drop(act_options, flower); + err = nl_parse_act_drop(act_options, flower); } else if (!strcmp(act_kind, "mirred")) { - nl_parse_act_mirred(act_options, flower); + err = nl_parse_act_mirred(act_options, flower); } else if (!strcmp(act_kind, "vlan")) { - nl_parse_act_vlan(act_options, flower); + err = nl_parse_act_vlan(act_options, flower); } else if (!strcmp(act_kind, "tunnel_key")) { - nl_parse_act_tunnel_key(act_options, flower); + err = nl_parse_act_tunnel_key(act_options, flower); } else { VLOG_ERR_RL(&error_rl, "unknown tc action kind: %s", act_kind); - return EINVAL; + err = EINVAL; + } + + if (err) { + return err; } if (act_cookie) {