Skip to content

Commit 68aba00

Browse files
Asbjørn Sloth Tønnesenkuba-moo
authored andcommitted
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: d6c2964 ("net: microchip: sparx5: Adding more tc flower keys for the IS2 VCAP") Signed-off-by: Asbjørn Sloth Tønnesen <[email protected]> Reviewed-by: Steen Hegelund <[email protected]> Tested-by: Daniel Machon <[email protected]> Reviewed-by: Jacob Keller <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent 27f58f7 commit 68aba00

File tree

1 file changed

+40
-21
lines changed

1 file changed

+40
-21
lines changed

drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c

Lines changed: 40 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,27 @@ struct sparx5_tc_flower_template {
3636
u16 l3_proto; /* protocol specified in the template */
3737
};
3838

39+
/* SparX-5 VCAP fragment types:
40+
* 0 = no fragment, 1 = initial fragment,
41+
* 2 = suspicious fragment, 3 = valid follow-up fragment
42+
*/
43+
enum { /* key / mask */
44+
FRAG_NOT = 0x03, /* 0 / 3 */
45+
FRAG_SOME = 0x11, /* 1 / 1 */
46+
FRAG_FIRST = 0x13, /* 1 / 3 */
47+
FRAG_LATER = 0x33, /* 3 / 3 */
48+
FRAG_INVAL = 0xff, /* invalid */
49+
};
50+
51+
/* Flower fragment flag to VCAP fragment type mapping */
52+
static const u8 sparx5_vcap_frag_map[4][4] = { /* is_frag */
53+
{ FRAG_INVAL, FRAG_INVAL, FRAG_INVAL, FRAG_FIRST }, /* 0/0 */
54+
{ FRAG_NOT, FRAG_NOT, FRAG_INVAL, FRAG_INVAL }, /* 0/1 */
55+
{ FRAG_INVAL, FRAG_INVAL, FRAG_INVAL, FRAG_INVAL }, /* 1/0 */
56+
{ FRAG_SOME, FRAG_LATER, FRAG_INVAL, FRAG_FIRST } /* 1/1 */
57+
/* 0/0 0/1 1/0 1/1 <-- first_frag */
58+
};
59+
3960
static int
4061
sparx5_tc_flower_es0_tpid(struct vcap_tc_flower_parse_usage *st)
4162
{
@@ -145,29 +166,27 @@ sparx5_tc_flower_handler_control_usage(struct vcap_tc_flower_parse_usage *st)
145166
flow_rule_match_control(st->frule, &mt);
146167

147168
if (mt.mask->flags) {
148-
if (mt.mask->flags & FLOW_DIS_FIRST_FRAG) {
149-
if (mt.key->flags & FLOW_DIS_FIRST_FRAG) {
150-
value = 1; /* initial fragment */
151-
mask = 0x3;
152-
} else {
153-
if (mt.mask->flags & FLOW_DIS_IS_FRAGMENT) {
154-
value = 3; /* follow up fragment */
155-
mask = 0x3;
156-
} else {
157-
value = 0; /* no fragment */
158-
mask = 0x3;
159-
}
160-
}
161-
} else {
162-
if (mt.mask->flags & FLOW_DIS_IS_FRAGMENT) {
163-
value = 3; /* follow up fragment */
164-
mask = 0x3;
165-
} else {
166-
value = 0; /* no fragment */
167-
mask = 0x3;
168-
}
169+
u8 is_frag_key = !!(mt.key->flags & FLOW_DIS_IS_FRAGMENT);
170+
u8 is_frag_mask = !!(mt.mask->flags & FLOW_DIS_IS_FRAGMENT);
171+
u8 is_frag_idx = (is_frag_key << 1) | is_frag_mask;
172+
173+
u8 first_frag_key = !!(mt.key->flags & FLOW_DIS_FIRST_FRAG);
174+
u8 first_frag_mask = !!(mt.mask->flags & FLOW_DIS_FIRST_FRAG);
175+
u8 first_frag_idx = (first_frag_key << 1) | first_frag_mask;
176+
177+
/* Lookup verdict based on the 2 + 2 input bits */
178+
u8 vdt = sparx5_vcap_frag_map[is_frag_idx][first_frag_idx];
179+
180+
if (vdt == FRAG_INVAL) {
181+
NL_SET_ERR_MSG_MOD(st->fco->common.extack,
182+
"Match on invalid fragment flag combination");
183+
return -EINVAL;
169184
}
170185

186+
/* Extract VCAP fragment key and mask from verdict */
187+
value = (vdt >> 4) & 0x3;
188+
mask = vdt & 0x3;
189+
171190
err = vcap_rule_add_key_u32(st->vrule,
172191
VCAP_KF_L3_FRAGMENT_TYPE,
173192
value, mask);

0 commit comments

Comments
 (0)