net: sparx5: flower: fix fragment flags handling

I noticed that only 3 out of the 4 input bits were used,
mt.key->flags & FLOW_DIS_IS_FRAGMENT was never checked.

In order to avoid a complicated maze, I converted it to
use a 16 byte mapping table.

As shown in the table below the old heuristics doesn't
always do the right thing, ie. when FLOW_DIS_IS_FRAGMENT=1/1
then it used to only match follow-up fragment packets.

Here are all the combinations, and their resulting new/old
VCAP key/mask filter:

  /- FLOW_DIS_IS_FRAGMENT (key/mask)
  |    /- FLOW_DIS_FIRST_FRAG (key/mask)
  |    |    /-- new VCAP fragment (key/mask)
  v    v    v    v- old VCAP fragment (key/mask)

 0/0  0/0  -/-  -/-     impossible (due to entry cond. on mask)
 0/0  0/1  -/-  0/3 !!  invalid (can't match non-fragment + follow-up frag)
 0/0  1/0  -/-  -/-     impossible (key > mask)
 0/0  1/1  1/3  1/3     first fragment

 0/1  0/0  0/3  3/3 !!  not fragmented
 0/1  0/1  0/3  3/3 !!  not fragmented (+ not first fragment)
 0/1  1/0  -/-  -/-     impossible (key > mask)
 0/1  1/1  -/-  1/3 !!  invalid (non-fragment and first frag)

 1/0  0/0  -/-  -/-     impossible (key > mask)
 1/0  0/1  -/-  -/-     impossible (key > mask)
 1/0  1/0  -/-  -/-     impossible (key > mask)
 1/0  1/1  -/-  -/-     impossible (key > mask)

 1/1  0/0  1/1  3/3 !!  some fragment
 1/1  0/1  3/3  3/3     follow-up fragment
 1/1  1/0  -/-  -/-     impossible (key > mask)
 1/1  1/1  1/3  1/3     first fragment

In the datasheet the VCAP fragment values are documented as:
 0 = no fragment
 1 = initial fragment
 2 = suspicious fragment
 3 = valid follow-up fragment

Result: 3 combinations match the old behavior,
        3 combinations have been corrected,
        2 combinations are now invalid, and fail,
        8 combinations are impossible.

It should now be aligned with how FLOW_DIS_IS_FRAGMENT
and FLOW_DIS_FIRST_FRAG is set in __skb_flow_dissect() in
net/core/flow_dissector.c

Since the VCAP fragment values are not a bitfield, we have
to ignore the suspicious fragment value, eg. when matching
on any kind of fragment with FLOW_DIS_IS_FRAGMENT=1/1.

Only compile tested, and logic tested in userspace, as I
unfortunately don't have access to this switch chip (yet).

Fixes: d6c2964db3fe ("net: microchip: sparx5: Adding more tc flower keys for the IS2 VCAP")
Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
Tested-by: Daniel Machon <daniel.machon@microchip.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Link: https://lore.kernel.org/r/20240411111321.114095-1-ast@fiberby.net
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c b/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c
index 523e0c470894f7fdcf8a995fb821ff146f08fcd9..55f255a3c9db69b92d5743bd42c34cbaba46a0a8 100644 (file)
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c
@@ -36,6 +36,27 @@ struct sparx5_tc_flower_template {
        u16 l3_proto; /* protocol specified in the template */
 };
 
+/* SparX-5 VCAP fragment types:
+ * 0 = no fragment, 1 = initial fragment,
+ * 2 = suspicious fragment, 3 = valid follow-up fragment
+ */
+enum {                   /* key / mask */
+       FRAG_NOT   = 0x03, /* 0 / 3 */
+       FRAG_SOME  = 0x11, /* 1 / 1 */
+       FRAG_FIRST = 0x13, /* 1 / 3 */
+       FRAG_LATER = 0x33, /* 3 / 3 */
+       FRAG_INVAL = 0xff, /* invalid */
+};
+
+/* Flower fragment flag to VCAP fragment type mapping */
+static const u8 sparx5_vcap_frag_map[4][4] = {           /* is_frag */
+       { FRAG_INVAL, FRAG_INVAL, FRAG_INVAL, FRAG_FIRST }, /* 0/0 */
+       { FRAG_NOT,   FRAG_NOT,   FRAG_INVAL, FRAG_INVAL }, /* 0/1 */
+       { FRAG_INVAL, FRAG_INVAL, FRAG_INVAL, FRAG_INVAL }, /* 1/0 */
+       { FRAG_SOME,  FRAG_LATER, FRAG_INVAL, FRAG_FIRST }  /* 1/1 */
+       /* 0/0        0/1         1/0         1/1 <-- first_frag */
+};
+
 static int
 sparx5_tc_flower_es0_tpid(struct vcap_tc_flower_parse_usage *st)
 {
@@ -145,29 +166,27 @@ sparx5_tc_flower_handler_control_usage(struct vcap_tc_flower_parse_usage *st)
        flow_rule_match_control(st->frule, &mt);
 
        if (mt.mask->flags) {
-               if (mt.mask->flags & FLOW_DIS_FIRST_FRAG) {
-                       if (mt.key->flags & FLOW_DIS_FIRST_FRAG) {
-                               value = 1; /* initial fragment */
-                               mask = 0x3;
-                       } else {
-                               if (mt.mask->flags & FLOW_DIS_IS_FRAGMENT) {
-                                       value = 3; /* follow up fragment */
-                                       mask = 0x3;
-                               } else {
-                                       value = 0; /* no fragment */
-                                       mask = 0x3;
-                               }
-                       }
-               } else {
-                       if (mt.mask->flags & FLOW_DIS_IS_FRAGMENT) {
-                               value = 3; /* follow up fragment */
-                               mask = 0x3;
-                       } else {
-                               value = 0; /* no fragment */
-                               mask = 0x3;
-                       }
+               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;
+
+               u8 first_frag_key = !!(mt.key->flags & FLOW_DIS_FIRST_FRAG);
+               u8 first_frag_mask = !!(mt.mask->flags & FLOW_DIS_FIRST_FRAG);
+               u8 first_frag_idx = (first_frag_key << 1) | first_frag_mask;
+
+               /* Lookup verdict based on the 2 + 2 input bits */
+               u8 vdt = sparx5_vcap_frag_map[is_frag_idx][first_frag_idx];
+
+               if (vdt == FRAG_INVAL) {
+                       NL_SET_ERR_MSG_MOD(st->fco->common.extack,
+                                          "Match on invalid fragment flag combination");
+                       return -EINVAL;
                }
 
+               /* Extract VCAP fragment key and mask from verdict */
+               value = (vdt >> 4) & 0x3;
+               mask = vdt & 0x3;
+
                err = vcap_rule_add_key_u32(st->vrule,
                                            VCAP_KF_L3_FRAGMENT_TYPE,
                                            value, mask);