lkml.org 
[lkml]   [2024]   [Apr]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH net-next 1/2] net: sparx5: flower: cleanup sparx5_tc_flower_handler_control_usage()
Hi Asbjørn,

Thank you for your patch!

> Define extack locally, to reduce line lengths and future users.
>
> Only perform fragment handling, when at least one fragment flag is set.
>
> Remove goto, as it's only used once, and the error message is specific
> to that context.
>
> Only compile tested.
>
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
> ---
> .../ethernet/microchip/sparx5/sparx5_tc_flower.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c b/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c
> index 663571fe7b2d..d846edd77a01 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c
> @@ -159,13 +159,14 @@ sparx5_tc_flower_handler_basic_usage(struct vcap_tc_flower_parse_usage *st)
> static int
> sparx5_tc_flower_handler_control_usage(struct vcap_tc_flower_parse_usage *st)
> {
> + struct netlink_ext_ack *extack = st->fco->common.extack;

Could you please update the use of extack in all places inside this
function. You are missing one place.

> struct flow_match_control mt;
> u32 value, mask;
> int err = 0;
>
> flow_rule_match_control(st->frule, &mt);
>
> - if (mt.mask->flags) {
> + if (mt.mask->flags & (FLOW_DIS_IS_FRAGMENT | FLOW_DIS_FIRST_FRAG)) {

Since these flags are used here and in the next patch, maybe assign them
to a variable:

u32 supp_flags = FLOW_DIS_IS_FRAGMENT | FLOW_DIS_FIRST_FRAG

And update the use throughout.

> u8 is_frag_key = !!(mt.key->flags & FLOW_DIS_IS_FRAGMENT);
> u8 is_frag_mask = !!(mt.mask->flags & FLOW_DIS_IS_FRAGMENT);
> u8 is_frag_idx = (is_frag_key << 1) | is_frag_mask;
> @@ -190,17 +191,15 @@ sparx5_tc_flower_handler_control_usage(struct vcap_tc_flower_parse_usage *st)
> err = vcap_rule_add_key_u32(st->vrule,
> VCAP_KF_L3_FRAGMENT_TYPE,
> value, mask);
> - if (err)
> - goto out;
> + if (err) {
> + NL_SET_ERR_MSG_MOD(extack, "ip_frag parse error");
> + return err;
> + }
> }
>
> st->used_keys |= BIT_ULL(FLOW_DISSECTOR_KEY_CONTROL);
>
> return err;
> -
> -out:
> - NL_SET_ERR_MSG_MOD(st->fco->common.extack, "ip_frag parse error");
> - return err;
> }
>
> static int
> --
> 2.43.0

Also I think you missing a cover letter for this series.

I will run the patches through our tests once the issues are addressed.

/Daniel


\
 
 \ /
  Last update: 2024-05-27 17:58    [W:0.057 / U:0.148 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site