2024-04-23 10:28:34

by Asbjørn Sloth Tønnesen

[permalink] [raw]
Subject: [PATCH net-next 1/2] net: lan966x: cleanup lan966x_tc_flower_handler_control_usage()

Define extack locally, to reduce line lengths and future users.

Rename goto, as the error message is specific to the fragment flags.

Only compile-tested.

Signed-off-by: Asbjørn Sloth Tønnesen <[email protected]>
---
.../net/ethernet/microchip/lan966x/lan966x_tc_flower.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_tc_flower.c b/drivers/net/ethernet/microchip/lan966x/lan966x_tc_flower.c
index d696cf9dbd19..8baec0cd8d95 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_tc_flower.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_tc_flower.c
@@ -45,6 +45,7 @@ static bool lan966x_tc_is_known_etype(struct vcap_tc_flower_parse_usage *st,
static int
lan966x_tc_flower_handler_control_usage(struct vcap_tc_flower_parse_usage *st)
{
+ struct netlink_ext_ack *extack = st->fco->common.extack;
struct flow_match_control match;
int err = 0;

@@ -59,7 +60,7 @@ lan966x_tc_flower_handler_control_usage(struct vcap_tc_flower_parse_usage *st)
VCAP_KF_L3_FRAGMENT,
VCAP_BIT_0);
if (err)
- goto out;
+ goto bad_frag_out;
}

if (match.mask->flags & FLOW_DIS_FIRST_FRAG) {
@@ -72,15 +73,15 @@ lan966x_tc_flower_handler_control_usage(struct vcap_tc_flower_parse_usage *st)
VCAP_KF_L3_FRAG_OFS_GT0,
VCAP_BIT_1);
if (err)
- goto out;
+ goto bad_frag_out;
}

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");
+bad_frag_out:
+ NL_SET_ERR_MSG_MOD(extack, "ip_frag parse error");
return err;
}

--
2.43.0



2024-04-23 10:28:57

by Asbjørn Sloth Tønnesen

[permalink] [raw]
Subject: [PATCH net-next 2/2] net: lan966x: flower: check for unsupported control flags

Use flow_rule_is_supp_control_flags() to reject filters with
unsupported control flags.

In case any unsupported control flags are masked,
flow_rule_is_supp_control_flags() sets a NL extended
error message, and we return -EOPNOTSUPP.

Only compile-tested.

Signed-off-by: Asbjørn Sloth Tønnesen <[email protected]>
---
drivers/net/ethernet/microchip/lan966x/lan966x_tc_flower.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_tc_flower.c b/drivers/net/ethernet/microchip/lan966x/lan966x_tc_flower.c
index 8baec0cd8d95..43913d6204e1 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_tc_flower.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_tc_flower.c
@@ -76,6 +76,11 @@ lan966x_tc_flower_handler_control_usage(struct vcap_tc_flower_parse_usage *st)
goto bad_frag_out;
}

+ if (!flow_rule_is_supp_control_flags(FLOW_DIS_IS_FRAGMENT |
+ FLOW_DIS_FIRST_FRAG,
+ match.mask->flags, extack))
+ return -EOPNOTSUPP;
+
st->used_keys |= BIT_ULL(FLOW_DISSECTOR_KEY_CONTROL);

return err;
--
2.43.0


2024-04-23 14:03:05

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH net-next 1/2] net: lan966x: cleanup lan966x_tc_flower_handler_control_usage()

On Tue, Apr 23, 2024 at 10:27:17AM +0000, Asbjørn Sloth Tønnesen wrote:
> Define extack locally, to reduce line lengths and future users.
>
> Rename goto, as the error message is specific to the fragment flags.
>
> Only compile-tested.
>
> Signed-off-by: Asbjørn Sloth Tønnesen <[email protected]>

Hi Asbjørn,

This patch-set did not apply to net-next at the time it was posted
(although it does now), so it will need to be reposted.

Also, please consider providing a cover-letter for patch-sets with
more than one (but not just one) patch.

--
pw-bot: changes-requested

2024-04-23 14:56:55

by Jiri Pirko

[permalink] [raw]
Subject: Re: [PATCH net-next 1/2] net: lan966x: cleanup lan966x_tc_flower_handler_control_usage()

Tue, Apr 23, 2024 at 12:27:17PM CEST, [email protected] wrote:
>Define extack locally, to reduce line lengths and future users.
>
>Rename goto, as the error message is specific to the fragment flags.

2 changes, 2 patches?

>
>Only compile-tested.
>
>Signed-off-by: Asbj?rn Sloth T?nnesen <[email protected]>
>---
> .../net/ethernet/microchip/lan966x/lan966x_tc_flower.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_tc_flower.c b/drivers/net/ethernet/microchip/lan966x/lan966x_tc_flower.c
>index d696cf9dbd19..8baec0cd8d95 100644
>--- a/drivers/net/ethernet/microchip/lan966x/lan966x_tc_flower.c
>+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_tc_flower.c
>@@ -45,6 +45,7 @@ static bool lan966x_tc_is_known_etype(struct vcap_tc_flower_parse_usage *st,
> static int
> lan966x_tc_flower_handler_control_usage(struct vcap_tc_flower_parse_usage *st)
> {
>+ struct netlink_ext_ack *extack = st->fco->common.extack;
> struct flow_match_control match;
> int err = 0;
>
>@@ -59,7 +60,7 @@ lan966x_tc_flower_handler_control_usage(struct vcap_tc_flower_parse_usage *st)
> VCAP_KF_L3_FRAGMENT,
> VCAP_BIT_0);
> if (err)
>- goto out;
>+ goto bad_frag_out;
> }
>
> if (match.mask->flags & FLOW_DIS_FIRST_FRAG) {
>@@ -72,15 +73,15 @@ lan966x_tc_flower_handler_control_usage(struct vcap_tc_flower_parse_usage *st)
> VCAP_KF_L3_FRAG_OFS_GT0,
> VCAP_BIT_1);
> if (err)
>- goto out;
>+ goto bad_frag_out;
> }
>
> 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");
>+bad_frag_out:
>+ NL_SET_ERR_MSG_MOD(extack, "ip_frag parse error");
> return err;
> }
>
>--
>2.43.0
>
>

2024-04-23 14:59:50

by Asbjørn Sloth Tønnesen

[permalink] [raw]
Subject: Re: [PATCH net-next 1/2] net: lan966x: cleanup lan966x_tc_flower_handler_control_usage()

Hi Simon,

On 4/23/24 2:00 PM, Simon Horman wrote:
> On Tue, Apr 23, 2024 at 10:27:17AM +0000, Asbjørn Sloth Tønnesen wrote:
>> Define extack locally, to reduce line lengths and future users.
>>
>> Rename goto, as the error message is specific to the fragment flags.
>>
>> Only compile-tested.
>>
>> Signed-off-by: Asbjørn Sloth Tønnesen <[email protected]>
>
> Hi Asbjørn,
>
> This patch-set did not apply to net-next at the time it was posted
> (although it does now), so it will need to be reposted.

Weird, patchwork incorrectly matched up the two series. I had
rebased shortly before posting (net-next was at 077633afe07f4).

The 4 emails and their Subject, Message-ID and In-Reply-To:

Subject: [PATCH net-next 1/2] net: sparx5: flower: cleanup sparx5_tc_flower_handler_control_usage()
Message-ID: <[email protected]> (A)

Subject: [PATCH net-next 2/2] net: sparx5: flower: check for unsupported control flags
Message-ID: <[email protected]> (B)
In-Reply-To: <[email protected]> (A)

Subject: [PATCH net-next 1/2] net: lan966x: cleanup lan966x_tc_flower_handler_control_usage()
Message-ID: <[email protected]> (C)

Subject: [PATCH net-next 2/2] net: lan966x: flower: check for unsupported control flags
Message-ID: <[email protected]> (D)
In-Reply-To: <[email protected]> (C)

Patchwork have two series A+D and C+B, instead of A+B and C+D.

> Also, please consider providing a cover-letter for patch-sets with
> more than one (but not just one) patch.

Sure, will do. Didn't know if it was appreciated for so small series,
anyway v2 will have more patches, after splitting this patch up.

--
Best regards
Asbjørn Sloth Tønnesen
Network Engineer
Fiberby - AS42541