Merge pull request #14252 from LabNConsulting/ziemba-pbr-bugfix-match…

…-dscp-numeric pbrd: fix dscp field value computation
FRRouting · Aug 29, 2023 · fa0dd3d · fa0dd3d
2 parents 72ff639 + 1efae6b
commit fa0dd3d
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 16 deletions.
diff --git a/pbrd/pbr_vty.c b/pbrd/pbr_vty.c
@@ -443,27 +443,28 @@ DEFPY  (pbr_map_match_dscp,
 
 	unsigned long ul_dscp;
 	char *pend = NULL;
-	uint8_t raw_dscp;
+	uint8_t shifted_dscp;
 
 	assert(dscp);
 	ul_dscp = strtoul(dscp, &pend, 0);
 	if (pend && *pend)
-		raw_dscp = pbr_map_decode_dscp_enum(dscp);
-	else
-		raw_dscp = ul_dscp << 2;
-	if (raw_dscp > PBR_DSFIELD_DSCP) {
+		ul_dscp = pbr_map_decode_dscp_enum(dscp);
+
+	if (ul_dscp > (PBR_DSFIELD_DSCP >> 2)) {
 		vty_out(vty, "Invalid dscp value: %s%s\n", dscp,
 			((pend && *pend) ? "" : " (numeric value must be in range 0-63)"));
 		return CMD_WARNING_CONFIG_FAILED;
 	}
 
+	shifted_dscp = (ul_dscp << 2) & PBR_DSFIELD_DSCP;
+
 	if (CHECK_FLAG(pbrms->filter_bm, PBR_FILTER_DSCP) &&
-	    (((pbrms->dsfield & PBR_DSFIELD_DSCP) >> 2) == raw_dscp)) {
+	    ((pbrms->dsfield & PBR_DSFIELD_DSCP) == shifted_dscp)) {
 		return CMD_SUCCESS;
 	}
 
 	/* Set the DSCP bits of the DSField */
-	pbrms->dsfield = (pbrms->dsfield & ~PBR_DSFIELD_DSCP) | (raw_dscp << 2);
+	pbrms->dsfield = (pbrms->dsfield & ~PBR_DSFIELD_DSCP) | shifted_dscp;
 	SET_FLAG(pbrms->filter_bm, PBR_FILTER_DSCP);
 
 check:
@@ -870,26 +871,27 @@ DEFPY  (pbr_map_action_dscp,
 
 	unsigned long ul_dscp;
 	char *pend = NULL;
-	uint8_t raw_dscp;
+	uint8_t shifted_dscp;
 
 	assert(dscp);
 	ul_dscp = strtoul(dscp, &pend, 0);
 	if (pend && *pend)
-		raw_dscp = pbr_map_decode_dscp_enum(dscp);
-	else
-		raw_dscp = ul_dscp << 2;
+		ul_dscp = pbr_map_decode_dscp_enum(dscp);
 
-	if (raw_dscp > PBR_DSFIELD_DSCP) {
+	if (ul_dscp > (PBR_DSFIELD_DSCP >> 2)) {
 		vty_out(vty, "Invalid dscp value: %s%s\n", dscp,
 			((pend && *pend) ? "" : " (numeric value must be in range 0-63)"));
 		return CMD_WARNING_CONFIG_FAILED;
 	}
+
+	shifted_dscp = (ul_dscp << 2) & PBR_DSFIELD_DSCP;
+
 	if (CHECK_FLAG(pbrms->action_bm, PBR_ACTION_DSCP) &&
-	    (pbrms->action_dscp == raw_dscp)) {
+	    (pbrms->action_dscp == shifted_dscp)) {
 		return CMD_SUCCESS;
 	}
 	SET_FLAG(pbrms->action_bm, PBR_ACTION_DSCP);
-	pbrms->action_dscp = raw_dscp;
+	pbrms->action_dscp = shifted_dscp;
 
 check:
 	pbr_map_check(pbrms, true);

diff --git a/tests/topotests/pbr_topo1/test_pbr_topo1.py b/tests/topotests/pbr_topo1/test_pbr_topo1.py
@@ -215,6 +215,8 @@ def test_pbr_data():
     {"c": "no match dst-port 119", "tN": r"DST Port Match: 119$"},
     {"c": "match dscp cs3", "tm": r"DSCP Match: 24$"},
     {"c": "no match dscp cs3", "tN": r"DSCP Match: 24$"},
+    {"c": "match dscp 5", "tm": r"DSCP Match: 5$"},
+    {"c": "no match dscp 5", "tN": r"DSCP Match: 5$"},
     {"c": "match ecn 2", "tm": r"ECN Match: 2$"},
     {"c": "no match ecn 2", "tN": r"ECN Match: 2$"},
     {"c": "match mark 337", "tm": r"MARK Match: 337$"},
@@ -229,8 +231,8 @@ def test_pbr_data():
     {"c": "no set dst-port 43", "tN": r"Set DST PORT: 43$"},
     {"c": "set dscp 24", "tm": r"Set DSCP: 24$"},
     {"c": "no set dscp 24", "tN": r"Set DSCP: 24$"},
-    {"c": "set dscp cs7", "tm": r"Set DSCP: 14$"},
-    {"c": "no set dscp cs7", "tN": r"Set DSCP: 14$"},
+    {"c": "set dscp cs7", "tm": r"Set DSCP: 56$"},
+    {"c": "no set dscp cs7", "tN": r"Set DSCP: 56$"},
     {"c": "set ecn 1", "tm": r"Set ECN: 1$"},
     {"c": "no set ecn 1", "tN": r"Set ECN: 1$"},
 ]