Messages in this thread Patch in this message | | | Date | Tue, 23 Apr 2024 11:15:15 +0000 | From | Daniel Machon <> | Subject | Re: [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
| |