| Message ID | 20240422152626.175569-1-ast@fiberby.net (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 3833e4834d70440582c127443073fde3204d5c07 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] bnxt_en: flower: validate control flags | expand |
On Mon, Apr 22, 2024 at 8:58 PM Asbjørn Sloth Tønnesen <ast@fiberby.net> wrote: > > This driver currently doesn't support any control flags. > > Use flow_rule_match_has_control_flags() to check for control flags, > such as can be set through `tc flower ... ip_flags frag`. > > In case any control flags are masked, flow_rule_match_has_control_flags() > sets a NL extended error message, and we return -EOPNOTSUPP. > > Only compile-tested. > > Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net> > --- > drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c > index 273c9ba48f09..d2ca90407cce 100644 > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c > @@ -370,6 +370,7 @@ static int bnxt_tc_parse_flow(struct bnxt *bp, > struct bnxt_tc_flow *flow) > { > struct flow_rule *rule = flow_cls_offload_flow_rule(tc_flow_cmd); > + struct netlink_ext_ack *extack = tc_flow_cmd->common.extack; > struct flow_dissector *dissector = rule->match.dissector; > > /* KEY_CONTROL and KEY_BASIC are needed for forming a meaningful key */ > @@ -380,6 +381,9 @@ static int bnxt_tc_parse_flow(struct bnxt *bp, > return -EOPNOTSUPP; > } > > + if (flow_rule_match_has_control_flags(rule, extack)) > + return -EOPNOTSUPP; > + > if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_BASIC)) { > struct flow_match_basic match; > > -- > 2.43.0 > > Thanks for this fix, it looks good. I need some time to test this; I will get back to you in a few days. -Harsha
Mon, Apr 22, 2024 at 05:26:23PM CEST, ast@fiberby.net wrote: >This driver currently doesn't support any control flags. > >Use flow_rule_match_has_control_flags() to check for control flags, >such as can be set through `tc flower ... ip_flags frag`. > >In case any control flags are masked, flow_rule_match_has_control_flags() >sets a NL extended error message, and we return -EOPNOTSUPP. > >Only compile-tested. > >Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
On Mon, Apr 22, 2024 at 10:51 PM Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com> wrote: > > On Mon, Apr 22, 2024 at 8:58 PM Asbjørn Sloth Tønnesen <ast@fiberby.net> wrote: > > > > This driver currently doesn't support any control flags. > > > > Use flow_rule_match_has_control_flags() to check for control flags, > > such as can be set through `tc flower ... ip_flags frag`. > > > > In case any control flags are masked, flow_rule_match_has_control_flags() > > sets a NL extended error message, and we return -EOPNOTSUPP. > > > > Only compile-tested. > > > > Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net> > > --- > > drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c > > index 273c9ba48f09..d2ca90407cce 100644 > > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c > > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c > > @@ -370,6 +370,7 @@ static int bnxt_tc_parse_flow(struct bnxt *bp, > > struct bnxt_tc_flow *flow) > > { > > struct flow_rule *rule = flow_cls_offload_flow_rule(tc_flow_cmd); > > + struct netlink_ext_ack *extack = tc_flow_cmd->common.extack; > > struct flow_dissector *dissector = rule->match.dissector; > > > > /* KEY_CONTROL and KEY_BASIC are needed for forming a meaningful key */ > > @@ -380,6 +381,9 @@ static int bnxt_tc_parse_flow(struct bnxt *bp, > > return -EOPNOTSUPP; > > } > > > > + if (flow_rule_match_has_control_flags(rule, extack)) > > + return -EOPNOTSUPP; > > + > > if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_BASIC)) { > > struct flow_match_basic match; > > > > -- > > 2.43.0 > > > > > > Thanks for this fix, it looks good. I need some time to test this; I > will get back to you in a few days. > -Harsha Reviewed-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com> Tested-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com>
Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Mon, 22 Apr 2024 15:26:23 +0000 you wrote: > This driver currently doesn't support any control flags. > > Use flow_rule_match_has_control_flags() to check for control flags, > such as can be set through `tc flower ... ip_flags frag`. > > In case any control flags are masked, flow_rule_match_has_control_flags() > sets a NL extended error message, and we return -EOPNOTSUPP. > > [...] Here is the summary with links: - [net-next] bnxt_en: flower: validate control flags https://git.kernel.org/netdev/net-next/c/3833e4834d70 You are awesome, thank you!
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c index 273c9ba48f09..d2ca90407cce 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c @@ -370,6 +370,7 @@ static int bnxt_tc_parse_flow(struct bnxt *bp, struct bnxt_tc_flow *flow) { struct flow_rule *rule = flow_cls_offload_flow_rule(tc_flow_cmd); + struct netlink_ext_ack *extack = tc_flow_cmd->common.extack; struct flow_dissector *dissector = rule->match.dissector; /* KEY_CONTROL and KEY_BASIC are needed for forming a meaningful key */ @@ -380,6 +381,9 @@ static int bnxt_tc_parse_flow(struct bnxt *bp, return -EOPNOTSUPP; } + if (flow_rule_match_has_control_flags(rule, extack)) + return -EOPNOTSUPP; + if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_BASIC)) { struct flow_match_basic match;
This driver currently doesn't support any control flags. Use flow_rule_match_has_control_flags() to check for control flags, such as can be set through `tc flower ... ip_flags frag`. In case any control flags are masked, flow_rule_match_has_control_flags() sets a NL extended error message, and we return -EOPNOTSUPP. Only compile-tested. Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net> --- drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c | 4 ++++ 1 file changed, 4 insertions(+)