Merge pull request #14126 from LabNConsulting/ziemba-pbr-actions-mangling

pbrd: (3/3) add packet mangling actions (src/dst ip-addr/port, dscp, ecn)

bgpd/bgp_zebra.c

@@ -2760,6 +2760,7 @@ static void bgp_encode_pbr_rule_action(struct stream *s,
 	else
 		r.unique = pbra->unique;
 
+	r.family = fam;
 
 	/* filter */
 

doc/user/pbr.rst

@@ -161,6 +161,26 @@ specified in the rule are also applied to the packet.
    VLAN-matching facilities,
    so this field will be ignored unless other dataplane providers are used.
 
+.. clicmd:: set nexthop-group NAME
+
+   Action:
+   forward the packet using nexthop-group NAME.
+
+.. clicmd:: set nexthop [A.B.C.D|X:X::X:XX|blackhole] [interface] [nexthop-vrf NAME]
+
+   Action:
+   forward the packet using the specified single nexthop.
+   If `blackhole`, packets will be sent to a blackhole route and dropped.
+
+.. clicmd:: set vrf unchanged|NAME
+
+   Action:
+   If set to ``unchanged``, the rule will use the vrf table the interface
+   is in as its lookup.
+   If set to NAME, the rule will use that vrf table as its lookup.
+
+   Not supported with NETNS VRF backend.
+
 .. clicmd:: set queue-id (1-65535)
 
    Action:
@@ -195,24 +215,57 @@ specified in the rule are also applied to the packet.
    so this field will be ignored unless another dataplane provider is used.
    It is invalid to specify both a `strip` and `set vlan` action.
 
-.. clicmd:: set nexthop-group NAME
+.. clicmd:: set src-ip [A.B.C.D/M|X:X::X:X/M]
 
    Action:
-   forward the packet using nexthop-group NAME.
+   Set the source IP address of matched packets, possibly using a mask `M`.
+   The Linux Kernel dataplane provider does not currently support
+   packet mangling,
+   so this field will be ignored unless another dataplane provider is used.
 
-.. clicmd:: set nexthop [A.B.C.D|X:X::X:XX] [interface] [nexthop-vrf NAME]
+.. clicmd:: set dst-ip [A.B.C.D/M|X:X::X:X/M]
 
    Action:
-   forward the packet using the specified single nexthop.
+   set the destination IP address of matched packets, possibly using a mask
+   `M`.
+   The Linux Kernel dataplane provider does not currently support
+   packet mangling,
+   so this field will be ignored unless another dataplane provider is used.
 
-.. clicmd:: set vrf unchanged|NAME
+.. clicmd:: set src-port (1-65535)
 
    Action:
-   If set to ``unchanged``, the rule will use the vrf table the interface
+   set the source port of matched packets. Note that this action only makes
-   is in as its lookup.
+   sense with layer 4 protocols that use ports, such as TCP, UDP, and SCTP.
-   If set to NAME, the rule will use that vrf table as its lookup.
+   The Linux Kernel dataplane provider does not currently support
+   packet mangling,
+   so this field will be ignored unless another dataplane provider is used.
 
-   Not supported with NETNS VRF backend.
+.. clicmd:: set dst-port (1-65535)
+
+   Action:
+   set the destination port of matched packets. Note that this action only
+   makes sense with layer 4 protocols that use ports, such as TCP, UDP, and
+   SCTP.
+   The Linux Kernel dataplane provider does not currently support
+   packet mangling,
+   so this field will be ignored unless another dataplane provider is used.
+
+.. clicmd:: set dscp DSCP
+
+   Action:
+   set the differentiated services code point (DSCP) of matched packets.
+   The Linux Kernel dataplane provider does not currently support
+   this action,
+   so this field will be ignored unless another dataplane provider is used.
+
+.. clicmd:: set ecn (0-3)
+
+   Action:
+   set the explicit congestion notification (ECN) of matched packets.
+   The Linux Kernel dataplane provider does not currently support
+   this action,
+   so this field will be ignored unless another dataplane provider is used.
 
 .. clicmd:: show pbr map [NAME] [detail|json]
 

lib/nexthop_group.c

@@ -1047,6 +1047,7 @@ void nexthop_group_write_nexthop_simple(struct vty *vty,
 		vty_out(vty, "%pI6 %s", &nh->gate.ipv6, ifname);
 		break;
 	case NEXTHOP_TYPE_BLACKHOLE:
+		vty_out(vty, "%s", "drop");
 		break;
 	}
 }

lib/pbr.h

@@ -34,17 +34,18 @@ struct pbr_filter {
 #define PBR_FILTER_SRC_PORT       (1 << 2)
 #define PBR_FILTER_DST_PORT       (1 << 3)
 #define PBR_FILTER_FWMARK         (1 << 4)
-#define PBR_FILTER_PROTO          (1 << 5)
+#define PBR_FILTER_IP_PROTOCOL	  (1 << 5)
 #define PBR_FILTER_SRC_PORT_RANGE (1 << 6)
 #define PBR_FILTER_DST_PORT_RANGE (1 << 7)
-#define PBR_FILTER_DSFIELD        (1 << 8)
+#define PBR_FILTER_DSCP		  (1 << 8)
-#define PBR_FILTER_IP_PROTOCOL    (1 << 9)
+#define PBR_FILTER_ECN		  (1 << 9)
 #define PBR_FILTER_PCP            (1 << 10)
 #define PBR_FILTER_VLAN_FLAGS     (1 << 11)
 #define PBR_FILTER_VLAN_ID        (1 << 12)
 
 #define PBR_DSFIELD_DSCP (0xfc) /* Upper 6 bits of DS field: DSCP */
 #define PBR_DSFIELD_ECN (0x03)	/* Lower 2 bits of DS field: BCN */
+
 #define PBR_PCP (0x07)		/* 3-bit value 0..7 for prioritization*/
 
 #define PBR_VLAN_FLAGS_NO_WILD	  0
@@ -56,7 +57,7 @@ struct pbr_filter {
 	struct prefix src_ip;
 	struct prefix dst_ip;
 
-	/* Source and Destination higher-layer (TCP/UDP) port numbers */
+	/* Source and Destination layer 4 (TCP/UDP/etc.) port numbers */
 	uint16_t src_port;
 	uint16_t dst_port;
 
@@ -87,11 +88,17 @@ struct pbr_filter {
 struct pbr_action {
 	uint32_t flags;
 
-#define PBR_ACTION_TABLE      (1 << 0)
+#define PBR_ACTION_TABLE		(1 << 0)
-#define PBR_ACTION_QUEUE_ID   (1 << 1)
+#define PBR_ACTION_QUEUE_ID		(1 << 1)
-#define PBR_ACTION_PCP        (1 << 2)
+#define PBR_ACTION_PCP			(1 << 2)
-#define PBR_ACTION_VLAN_ID    (1 << 3)
+#define PBR_ACTION_VLAN_ID		(1 << 3)
-#define PBR_ACTION_VLAN_FLAGS (1 << 4)
+#define PBR_ACTION_VLAN_STRIP_INNER_ANY (1 << 4)
+#define PBR_ACTION_SRC_IP		(1 << 5)
+#define PBR_ACTION_DST_IP		(1 << 6)
+#define PBR_ACTION_SRC_PORT		(1 << 7)
+#define PBR_ACTION_DST_PORT		(1 << 8)
+#define PBR_ACTION_DSCP			(1 << 9)
+#define PBR_ACTION_ECN			(1 << 10)
 
 	uint32_t table;
 	uint32_t queue_id;
@@ -99,9 +106,18 @@ struct pbr_action {
 	/* VLAN */
 	uint8_t pcp;
 	uint16_t vlan_id;
-	uint16_t vlan_flags;
 
+	/* Source and Destination IP addresses */
+	union sockunion src_ip;
+	union sockunion dst_ip;
 
+	/* Source and Destination layer 4 (TCP/UDP/etc.) port numbers */
+	uint32_t src_port;
+	uint32_t dst_port;
+
+	/* Differentiated Services field  */
+	uint8_t dscp; /* stored here already shifted to upper 6 bits */
+	uint8_t ecn;  /* stored here as lower 2 bits */
 };
 
 /*
@@ -113,6 +129,7 @@ struct pbr_action {
  */
 struct pbr_rule {
 	vrf_id_t vrf_id;
+	uint8_t family; /* netlink: select which rule database */
 
 	uint32_t seq;
 	uint32_t priority;

lib/zclient.c

@@ -1622,6 +1622,47 @@ stream_failure:
 	return false;
 }
 
+static void zapi_encode_sockunion(struct stream *s, const union sockunion *su)
+{
+	int family = sockunion_family(su);
+	size_t addrlen = family2addrsize(family);
+
+	/*
+	 * Must know length to encode
+	 */
+	assert(addrlen);
+
+	stream_putc(s, (uint8_t)family);
+
+	stream_write(s, sockunion_get_addr(su), addrlen);
+}
+
+static bool zapi_decode_sockunion(struct stream *s, union sockunion *su)
+{
+	uint8_t family;
+	size_t addrlen;
+	uint8_t buf[sizeof(union sockunion)];
+
+	memset(su, 0, sizeof(*su));
+
+	STREAM_GETC(s, family);
+	sockunion_family(su) = family;
+
+	addrlen = family2addrsize(family);
+	if (!addrlen)
+		return false;
+
+	if (addrlen > sizeof(buf))
+		return false;
+
+	STREAM_GET(buf, s, addrlen);
+	sockunion_set(su, family, buf, addrlen);
+	return true;
+
+stream_failure:
+	return false;
+}
+
 /*
  * Encode filter subsection of pbr_rule
  */
@@ -1631,40 +1672,79 @@ static void zapi_pbr_rule_filter_encode(struct stream *s, struct pbr_filter *f)
 	assert((f->src_ip.family == AF_INET) || (f->src_ip.family == AF_INET6));
 
 	stream_putl(s, f->filter_bm);
-	stream_putc(s, f->ip_proto);
+
+	if (CHECK_FLAG(f->filter_bm, PBR_FILTER_IP_PROTOCOL))
+		stream_putc(s, f->ip_proto);
 
 	/* addresses */
-	zapi_encode_prefix(s, &f->src_ip, f->src_ip.family);
+	if (CHECK_FLAG(f->filter_bm, PBR_FILTER_SRC_IP))
-	zapi_encode_prefix(s, &f->dst_ip, f->dst_ip.family);
+		zapi_encode_prefix(s, &f->src_ip, f->src_ip.family);
+	if (CHECK_FLAG(f->filter_bm, PBR_FILTER_DST_IP))
+		zapi_encode_prefix(s, &f->dst_ip, f->dst_ip.family);
 
 	/* port numbers */
-	stream_putw(s, f->src_port);
+	if (CHECK_FLAG(f->filter_bm, PBR_FILTER_SRC_PORT))
-	stream_putw(s, f->dst_port);
+		stream_putw(s, f->src_port);
+	if (CHECK_FLAG(f->filter_bm, PBR_FILTER_DST_PORT))
+		stream_putw(s, f->dst_port);
+
+	if (CHECK_FLAG(f->filter_bm, PBR_FILTER_DSCP))
+		stream_putc(s, f->dsfield & PBR_DSFIELD_DSCP);
+	if (CHECK_FLAG(f->filter_bm, PBR_FILTER_ECN))
+		stream_putc(s, f->dsfield & PBR_DSFIELD_ECN);
 
 	/* vlan */
-	stream_putc(s, f->pcp);
+	if (CHECK_FLAG(f->filter_bm, PBR_FILTER_PCP))
-	stream_putw(s, f->vlan_id);
+		stream_putc(s, f->pcp);
-	stream_putw(s, f->vlan_flags);
+	if (CHECK_FLAG(f->filter_bm, PBR_FILTER_VLAN_ID))
+		stream_putw(s, f->vlan_id);
+	if (CHECK_FLAG(f->filter_bm, PBR_FILTER_VLAN_FLAGS))
+		stream_putw(s, f->vlan_flags);
 
-	stream_putc(s, f->dsfield);
+
-	stream_putl(s, f->fwmark);
+	if (CHECK_FLAG(f->filter_bm, PBR_FILTER_FWMARK))
+		stream_putl(s, f->fwmark);
 }
 
 static bool zapi_pbr_rule_filter_decode(struct stream *s, struct pbr_filter *f)
 {
+	uint8_t dscp = 0;
+	uint8_t ecn = 0;
+
 	STREAM_GETL(s, f->filter_bm);
-	STREAM_GETC(s, f->ip_proto);
+
-	if (!zapi_decode_prefix(s, &(f->src_ip)))
+	if (CHECK_FLAG(f->filter_bm, PBR_FILTER_IP_PROTOCOL))
-		goto stream_failure;
+		STREAM_GETC(s, f->ip_proto);
-	if (!zapi_decode_prefix(s, &(f->dst_ip)))
+
-		goto stream_failure;
+	if (CHECK_FLAG(f->filter_bm, PBR_FILTER_SRC_IP))
-	STREAM_GETW(s, f->src_port);
+		if (!zapi_decode_prefix(s, &(f->src_ip)))
-	STREAM_GETW(s, f->dst_port);
+			goto stream_failure;
-	STREAM_GETC(s, f->pcp);
+	if (CHECK_FLAG(f->filter_bm, PBR_FILTER_DST_IP))
-	STREAM_GETW(s, f->vlan_id);
+		if (!zapi_decode_prefix(s, &(f->dst_ip)))
-	STREAM_GETW(s, f->vlan_flags);
+			goto stream_failure;
-	STREAM_GETC(s, f->dsfield);
+
-	STREAM_GETL(s, f->fwmark);
+	if (CHECK_FLAG(f->filter_bm, PBR_FILTER_SRC_PORT))
+		STREAM_GETW(s, f->src_port);
+	if (CHECK_FLAG(f->filter_bm, PBR_FILTER_DST_PORT))
+		STREAM_GETW(s, f->dst_port);
+
+	if (CHECK_FLAG(f->filter_bm, PBR_FILTER_DSCP))
+		STREAM_GETC(s, dscp);
+	if (CHECK_FLAG(f->filter_bm, PBR_FILTER_ECN))
+		STREAM_GETC(s, ecn);
+	f->dsfield = (dscp & PBR_DSFIELD_DSCP) | (ecn & PBR_DSFIELD_ECN);
+
+	/* vlan */
+	if (CHECK_FLAG(f->filter_bm, PBR_FILTER_PCP))
+		STREAM_GETC(s, f->pcp);
+	if (CHECK_FLAG(f->filter_bm, PBR_FILTER_VLAN_ID))
+		STREAM_GETW(s, f->vlan_id);
+	if (CHECK_FLAG(f->filter_bm, PBR_FILTER_VLAN_FLAGS))
+		STREAM_GETW(s, f->vlan_flags);
+
+	if (CHECK_FLAG(f->filter_bm, PBR_FILTER_FWMARK))
+		STREAM_GETL(s, f->fwmark);
+
 	return true;
 
 stream_failure:
@@ -1674,21 +1754,72 @@ stream_failure:
 static void zapi_pbr_rule_action_encode(struct stream *s, struct pbr_action *a)
 {
 	stream_putl(s, a->flags);
-	stream_putl(s, a->table);
+
-	stream_putl(s, a->queue_id);
+	if (CHECK_FLAG(a->flags, PBR_ACTION_TABLE))
-	stream_putc(s, a->pcp);
+		stream_putl(s, a->table);
-	stream_putw(s, a->vlan_id);
+	if (CHECK_FLAG(a->flags, PBR_ACTION_QUEUE_ID))
-	stream_putw(s, a->vlan_flags);
+		stream_putl(s, a->queue_id);
+
+	/* L3 */
+	if (CHECK_FLAG(a->flags, PBR_ACTION_SRC_IP))
+		zapi_encode_sockunion(s, &a->src_ip);
+	if (CHECK_FLAG(a->flags, PBR_ACTION_DST_IP))
+		zapi_encode_sockunion(s, &a->dst_ip);
+	if (CHECK_FLAG(a->flags, PBR_ACTION_SRC_PORT))
+		stream_putw(s, a->src_port);
+	if (CHECK_FLAG(a->flags, PBR_ACTION_DST_PORT))
+		stream_putw(s, a->dst_port);
+
+	if (CHECK_FLAG(a->flags, PBR_ACTION_DSCP))
+		stream_putc(s, a->dscp & PBR_DSFIELD_DSCP);
+	if (CHECK_FLAG(a->flags, PBR_ACTION_ECN))
+		stream_putc(s, a->ecn & PBR_DSFIELD_ECN);
+
+	/* L2 */
+	if (CHECK_FLAG(a->flags, PBR_ACTION_PCP))
+		stream_putc(s, a->pcp);
+	if (CHECK_FLAG(a->flags, PBR_ACTION_VLAN_ID))
+		stream_putw(s, a->vlan_id);
 }
 
 static bool zapi_pbr_rule_action_decode(struct stream *s, struct pbr_action *a)
 {
 	STREAM_GETL(s, a->flags);
-	STREAM_GETL(s, a->table);
+
-	STREAM_GETL(s, a->queue_id);
+	if (CHECK_FLAG(a->flags, PBR_ACTION_TABLE))
-	STREAM_GETC(s, a->pcp);
+		STREAM_GETL(s, a->table);
-	STREAM_GETW(s, a->vlan_id);
+	if (CHECK_FLAG(a->flags, PBR_ACTION_QUEUE_ID))
-	STREAM_GETW(s, a->vlan_flags);
+		STREAM_GETL(s, a->queue_id);
+
+	/* L3 */
+	if (CHECK_FLAG(a->flags, PBR_ACTION_SRC_IP)) {
+		if (!zapi_decode_sockunion(s, &(a->src_ip)))
+			goto stream_failure;
+	}
+	if (CHECK_FLAG(a->flags, PBR_ACTION_DST_IP))
+		if (!zapi_decode_sockunion(s, &(a->dst_ip)))
+			goto stream_failure;
+
+	if (CHECK_FLAG(a->flags, PBR_ACTION_SRC_PORT))
+		STREAM_GETW(s, a->src_port);
+	if (CHECK_FLAG(a->flags, PBR_ACTION_DST_PORT))
+		STREAM_GETW(s, a->dst_port);
+
+	if (CHECK_FLAG(a->flags, PBR_ACTION_DSCP)) {
+		STREAM_GETC(s, a->dscp);
+		a->dscp &= PBR_DSFIELD_DSCP;
+	}
+	if (CHECK_FLAG(a->flags, PBR_ACTION_ECN)) {
+		STREAM_GETC(s, a->ecn);
+		a->ecn &= PBR_DSFIELD_ECN;
+	}
+
+	/* L2 */
+	if (CHECK_FLAG(a->flags, PBR_ACTION_PCP))
+		STREAM_GETC(s, a->pcp);
+	if (CHECK_FLAG(a->flags, PBR_ACTION_VLAN_ID))
+		STREAM_GETW(s, a->vlan_id);
+
 	return true;
 
 stream_failure:
@@ -1702,6 +1833,7 @@ int zapi_pbr_rule_encode(struct stream *s, struct pbr_rule *r)
 	 */
 	stream_putl(s, 1);
 
+	stream_putc(s, r->family);
 	stream_putl(s, r->seq);
 	stream_putl(s, r->priority);
 	stream_putl(s, r->unique);
@@ -1723,6 +1855,7 @@ bool zapi_pbr_rule_decode(struct stream *s, struct pbr_rule *r)
 
 	memset(r, 0, sizeof(*r));
 
+	STREAM_GETC(s, r->family);
 	STREAM_GETL(s, r->seq);
 	STREAM_GETL(s, r->priority);
 	STREAM_GETL(s, r->unique);

pbrd/pbr_map.c

@@ -105,38 +105,6 @@ static bool pbrms_is_installed(const struct pbr_map_sequence *pbrms,
 	return false;
 }
 
-void pbr_set_match_clause_for_pcp(struct pbr_map_sequence *pbrms, bool set,
-				  uint8_t pcp)
-{
-	bool changed = false;
-
-	if (set) {
-		if (!CHECK_FLAG(pbrms->filter_bm, PBR_FILTER_PCP) ||
-		    (pcp != pbrms->match_pcp)) {
-			SET_FLAG(pbrms->filter_bm, PBR_FILTER_PCP);
-			pbrms->match_pcp = pcp;
-			changed = true;
-		}
-	} else {
-		if (CHECK_FLAG(pbrms->filter_bm, PBR_FILTER_PCP)) {
-			UNSET_FLAG(pbrms->filter_bm, PBR_FILTER_PCP);
-			changed = true;
-		}
-	}
-	if (changed)
-		pbr_map_check(pbrms, true);
-}
-
-void pbr_set_match_clause_for_vlan(struct pbr_map_sequence *pbrms,
-				   uint16_t vlan_id, uint16_t vlan_flags)
-{
-	if (pbrms) {
-		pbrms->match_vlan_id = vlan_id;
-		pbrms->match_vlan_flags = vlan_flags;
-		pbr_map_check(pbrms, true);
-	}
-}
-
 /* If any sequence is installed on the interface, assume installed */
 static bool
 pbr_map_interface_is_installed(const struct pbr_map *pbrm,
@@ -562,14 +530,6 @@ struct pbr_map_sequence *pbrms_get(const char *name, uint32_t seqno)
 		pbrms->ruleno = pbr_nht_get_next_rule(seqno);
 		pbrms->parent = pbrm;
 
-		pbrms->match_vlan_id = 0;
-		pbrms->match_vlan_flags = 0;
-		pbrms->match_pcp = 0;
-
-		pbrms->action_vlan_id = 0;
-		pbrms->action_vlan_flags = 0;
-		pbrms->action_pcp = 0;
-
 		pbrms->action_queue_id = PBR_MAP_UNDEFINED_QUEUE_ID;
 
 		pbrms->reason =
@@ -634,13 +594,42 @@ pbr_map_sequence_check_nexthops_valid(struct pbr_map_sequence *pbrms)
 
 static void pbr_map_sequence_check_not_empty(struct pbr_map_sequence *pbrms)
 {
-	if (!pbrms->src && !pbrms->dst && !pbrms->mark && !pbrms->dsfield &&
+	/* clang-format off */
-	    !CHECK_FLAG(pbrms->filter_bm, PBR_FILTER_PCP) &&
+	if (
-	    !pbrms->action_pcp && !pbrms->match_vlan_id &&
+		!CHECK_FLAG(pbrms->filter_bm, (
-	    !pbrms->match_vlan_flags && !pbrms->action_vlan_id &&
+			PBR_FILTER_SRC_IP |
-	    !pbrms->action_vlan_flags &&
+			PBR_FILTER_DST_IP |
-	    pbrms->action_queue_id == PBR_MAP_UNDEFINED_QUEUE_ID)
+			PBR_FILTER_SRC_PORT |
+			PBR_FILTER_DST_PORT |
+
+			PBR_FILTER_IP_PROTOCOL |
+			PBR_FILTER_DSCP |
+			PBR_FILTER_ECN |
+
+			PBR_FILTER_FWMARK |
+			PBR_FILTER_PCP |
+			PBR_FILTER_VLAN_ID |
+			PBR_FILTER_VLAN_FLAGS
+		)) &&
+		!CHECK_FLAG(pbrms->action_bm, (
+			PBR_ACTION_SRC_IP |
+			PBR_ACTION_DST_IP |
+			PBR_ACTION_SRC_PORT |
+			PBR_ACTION_DST_PORT |
+
+			PBR_ACTION_DSCP |
+			PBR_ACTION_ECN |
+
+			PBR_ACTION_PCP |
+			PBR_ACTION_VLAN_ID |
+			PBR_ACTION_VLAN_STRIP_INNER_ANY |
+
+			PBR_ACTION_QUEUE_ID
+		))
+	) {
 		pbrms->reason |= PBR_MAP_INVALID_EMPTY;
+	}
+	/* clang-format on */
 }
 
 static void pbr_map_sequence_check_vlan_actions(struct pbr_map_sequence *pbrms)
@@ -653,7 +642,8 @@ static void pbr_map_sequence_check_vlan_actions(struct pbr_map_sequence *pbrms)
 	 * The strip vlan action removes any inner tag, so it is invalid to
 	 * specify both a set and strip action.
 	 */
-	if ((pbrms->action_vlan_id != 0) && (pbrms->action_vlan_flags != 0))
+	if (CHECK_FLAG(pbrms->action_bm, PBR_ACTION_VLAN_ID) &&
+	    (CHECK_FLAG(pbrms->action_bm, PBR_ACTION_VLAN_STRIP_INNER_ANY)))
 		pbrms->reason |= PBR_MAP_INVALID_SET_STRIP_VLAN;
 }
 

pbrd/pbr_map.h

@@ -3,7 +3,9 @@
  * PBR-map Header
  * Copyright (C) 2018 Cumulus Networks, Inc.
  *               Donald Sharp
- * Copyright (c) 2023 LabN Consulting, L.L.C.
+ * Portions:
+ *	Copyright (c) 2023 LabN Consulting, L.L.C.
+ *	Copyright (c) 2021 The MITRE Corporation
  */
 #ifndef __PBR_MAP_H__
 #define __PBR_MAP_H__
@@ -54,6 +56,14 @@ struct pbr_map_interface {
 	bool delete;
 };
 
+enum pbr_forwarding_type {
+	PBR_FT_UNSPEC = 0,
+	PBR_FT_VRF_UNCHANGED,
+	PBR_FT_SETVRF,
+	PBR_FT_NEXTHOP_GROUP,
+	PBR_FT_NEXTHOP_SINGLE,
+};
+
 struct pbr_map_sequence {
 	struct pbr_map *parent;
 
@@ -109,14 +119,28 @@ struct pbr_map_sequence {
 	 * Action fields
 	 *****************************************************************/
 
+	/*
+	 * same bit definitions as in lib/pbr.h
+	 */
+	uint32_t action_bm;
+
+	union sockunion action_src;
+	union sockunion action_dst;
+
+	uint16_t action_src_port;
+	uint16_t action_dst_port;
+
+	uint8_t action_dscp;
+	uint8_t action_ecn;
+
 	uint8_t action_pcp;
 	uint8_t action_vlan_id;
-#define PBR_MAP_STRIP_INNER_ANY (1 << 0)
-	uint8_t action_vlan_flags;
 
 #define PBR_MAP_UNDEFINED_QUEUE_ID 0
 	uint32_t action_queue_id;
 
+	enum pbr_forwarding_type forwarding_type;
+
 	/*
 	 * Use interface's vrf.
 	 */
@@ -233,9 +257,4 @@ extern void pbr_map_check_vrf_nh_group_change(const char *nh_group,
 extern void pbr_map_check_interface_nh_group_change(const char *nh_group,
 						    struct interface *ifp,
 						    ifindex_t oldifindex);
-extern void pbr_set_match_clause_for_vlan(struct pbr_map_sequence *pbrms,
-					  uint16_t vlan_id,
-					  uint16_t vlan_flags);
-extern void pbr_set_match_clause_for_pcp(struct pbr_map_sequence *pbrms,
-					 bool set, uint8_t pcp);
 #endif

pbrd/pbr_nht.c

@@ -549,6 +549,7 @@ void pbr_nht_set_seq_nhg(struct pbr_map_sequence *pbrms, const char *name)
 		return;
 
 	pbrms->nhgrp_name = XSTRDUP(MTYPE_TMP, name);
+	pbrms->forwarding_type = PBR_FT_NEXTHOP_GROUP;
 
 	nhgc = nhgc_find(name);
 	if (!nhgc)
@@ -572,6 +573,7 @@ void pbr_nht_add_individual_nexthop(struct pbr_map_sequence *pbrms,
 		MTYPE_TMP,
 		pbr_nht_nexthop_make_name(pbrms->parent->name, PBR_NHC_NAMELEN,
 					  pbrms->seqno, buf));
+	pbrms->forwarding_type = PBR_FT_NEXTHOP_SINGLE;
 
 	nh = nexthop_new();
 	memcpy(nh, nhop, sizeof(*nh));

pbrd/pbr_zebra.c

@@ -517,10 +517,14 @@ static bool pbr_encode_pbr_map_sequence(struct stream *s,
 	uint8_t family;
 
 	/*
-	 * There seems to be some effort in pbr_vty.c to keep the three
+	 * Opportunistic address family field is set when any of the IP
-	 * copies of "family" equal. Not sure if the reason goes beyond
+	 * address match/set fields is set, or when a NH/NHG is resolved.
-	 * ensuring consistency in ZAPI encoding. In any case, it might
+	 * The value is needed by zebra for the underlying netlink
-	 * be handled better as an internal matter for the encoder (TBD).
+	 * messaging, particularly in delete operations, because it
+	 * selects the rule database (IPv4 vs. IPv6).
+	 *
+	 * Historically the value has been encoded into any unused
+	 * "match src/dst address" fields and picked off in zebra.
 	 */
 	family = AF_INET;
 	if (pbrms->family)
@@ -539,6 +543,8 @@ static bool pbr_encode_pbr_map_sequence(struct stream *s,
 	r.priority = pbrms->ruleno;
 	r.unique = pbrms->unique;
 
+	r.family = pbrms->family;
+
 	/* filter */
 	r.filter.filter_bm = pbrms->filter_bm;
 	if (pbrms->src)
@@ -558,44 +564,18 @@ static bool pbr_encode_pbr_map_sequence(struct stream *s,
 	r.filter.fwmark = pbrms->mark;
 	r.filter.ip_proto = pbrms->ip_proto;
 
-	/*
+	r.filter.filter_bm = pbrms->filter_bm;
-	 * Fix up filter flags for now, since PBRD doesn't maintain
-	 * them yet (aside from PBR_FILTER_PCP)
-	 */
-	if (!is_default_prefix(&r.filter.src_ip))
-		SET_FLAG(r.filter.filter_bm, PBR_FILTER_SRC_IP);
-	if (!is_default_prefix(&r.filter.dst_ip))
-		SET_FLAG(r.filter.filter_bm, PBR_FILTER_DST_IP);
-	if (r.filter.src_port)
-		SET_FLAG(r.filter.filter_bm, PBR_FILTER_SRC_PORT);
-	if (r.filter.dst_port)
-		SET_FLAG(r.filter.filter_bm, PBR_FILTER_DST_PORT);
-	if (r.filter.vlan_id)
-		SET_FLAG(r.filter.filter_bm, PBR_FILTER_VLAN_ID);
-	if (r.filter.vlan_flags)
-		SET_FLAG(r.filter.filter_bm, PBR_FILTER_VLAN_FLAGS);
-	if (r.filter.dsfield)
-		SET_FLAG(r.filter.filter_bm, PBR_FILTER_DSFIELD);
-	if (r.filter.fwmark)
-		SET_FLAG(r.filter.filter_bm, PBR_FILTER_FWMARK);
-	if (r.filter.ip_proto)
-		SET_FLAG(r.filter.filter_bm, PBR_FILTER_IP_PROTOCOL);
 
 	/* actions */
 
+	SET_FLAG(r.action.flags, PBR_ACTION_TABLE); /* always valid */
+
 	/*
 	 * PBR should maintain its own set of action flags that we
 	 * can copy here instead of trying to infer from magic values.
 	 */
-	SET_FLAG(r.action.flags, PBR_ACTION_TABLE); /* always valid */
+
-	if (pbrms->action_queue_id != PBR_MAP_UNDEFINED_QUEUE_ID)
+	r.action.flags = pbrms->action_bm;
-		SET_FLAG(r.action.flags, PBR_ACTION_QUEUE_ID);
-	if (pbrms->action_pcp != 0)
-		SET_FLAG(r.action.flags, PBR_ACTION_PCP);
-	if (pbrms->action_vlan_id != 0)
-		SET_FLAG(r.action.flags, PBR_ACTION_VLAN_ID);
-	if (pbrms->action_vlan_flags != 0)
-		SET_FLAG(r.action.flags, PBR_ACTION_VLAN_FLAGS);
 
 	/*
 	 * if the user does not use the command "set vrf name unchanged"
@@ -613,9 +593,18 @@ static bool pbr_encode_pbr_map_sequence(struct stream *s,
 	}
 
 	r.action.queue_id = pbrms->action_queue_id;
+
+	r.action.src_ip = pbrms->action_src;
+	r.action.dst_ip = pbrms->action_dst;
+
+	r.action.src_port = pbrms->action_src_port;
+	r.action.dst_port = pbrms->action_dst_port;
+
+	r.action.dscp = pbrms->action_dscp;
+	r.action.ecn = pbrms->action_ecn;
+
 	r.action.pcp = pbrms->action_pcp;
 	r.action.vlan_id = pbrms->action_vlan_id;
-	r.action.vlan_flags = pbrms->action_vlan_flags;
 
 	strlcpy(r.ifname, ifp->name, sizeof(r.ifname));
 

tests/topotests/pbr_topo1/test_pbr_topo1.py

@@ -201,27 +201,38 @@ ftest = [
     {"c": "match vlan untagged", "tm": r"VLAN Flags Match: untagged$"},
     {"c": "match vlan untagged-or-zero", "tm": r"VLAN Flags Match: untagged-or-zero$"},
     {"c": "no match vlan tagged", "tN": r"VLAN Flags Match:"},
-
     {"c": "match src-ip 37.49.22.0/24", "tm": r"SRC IP Match: 37.49.22.0/24$"},
     {"c": "no match src-ip 37.49.22.0/24", "tN": r"SRC IP Match: 37.49.22.0/24$"},
-
+    {
-    {"c": "match dst-ip 38.41.29.0/25", "cDN": "foo", "tm": r"DST IP Match: 38.41.29.0/25$"},
+        "c": "match dst-ip 38.41.29.0/25",
+        "cDN": "foo",
+        "tm": r"DST IP Match: 38.41.29.0/25$",
+    },
     {"c": "no match dst-ip 38.41.29.0/25", "tN": r"DST IP Match: 38.41.29.0/25$"},
-
     {"c": "match src-port 117", "tm": r"SRC Port Match: 117$"},
     {"c": "no match src-port 117", "tN": r"SRC Port Match: 117$"},
-
     {"c": "match dst-port 119", "tm": r"DST Port Match: 119$"},
     {"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 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$"},
     {"c": "no match mark 337", "tN": r"MARK Match: 337$"},
+    {"c": "set src-ip 44.100.1.1", "tm": r"Set SRC IP: 44.100.1.1$"},
+    {"c": "no set src-ip 44.100.1.1", "tN": r"Set SRC IP: 44.100.1.1$"},
+    {"c": "set dst-ip 44.105.1.1", "tm": r"Set DST IP: 44.105.1.1$"},
+    {"c": "no set dst-ip 44.105.1.1", "tN": r"Set DST IP: 44.105.1.1$"},
+    {"c": "set src-port 41", "tm": r"Set SRC PORT: 41$"},
+    {"c": "no set src-port 41", "tN": r"Set SRC PORT: 41$"},
+    {"c": "set dst-port 43", "tm": r"Set DST PORT: 43$"},
+    {"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 ecn 1", "tm": r"Set ECN: 1$"},
+    {"c": "no set ecn 1", "tN": r"Set ECN: 1$"},
 ]
 
 

zebra/rule_netlink.c

@@ -117,8 +117,8 @@ static ssize_t netlink_rule_msg_encode(
 	}
 
 	/* dsfield, if specified; mask off the ECN bits */
-	if (filter_bm & PBR_FILTER_DSFIELD)
+	if (filter_bm & PBR_FILTER_DSCP)
-		req->frh.tos = dsfield & 0xfc;
+		req->frh.tos = dsfield & PBR_DSFIELD_DSCP;
 
 	/* protocol to match on */
 	if (filter_bm & PBR_FILTER_IP_PROTOCOL)

zebra/zapi_msg.c

@@ -3188,10 +3188,32 @@ static inline void zread_rule(ZAPI_HANDLER_ARGS)
 
 		strlcpy(zpr.ifname, zpr.rule.ifname, sizeof(zpr.ifname));
 
+		if ((zpr.rule.family != AF_INET) &&
+		    (zpr.rule.family != AF_INET6)) {
+			zlog_warn("Unsupported PBR source IP family: %s (%hhu)",
+				  family2str(zpr.rule.family), zpr.rule.family);
+			return;
+		}
+
+		/*
+		 * Fixup filter src/dst IP addresses if they are unset
+		 * because the netlink code currently obtains address family
+		 * from them. Address family is used to specify which
+		 * kernel database to use when adding/deleting rule.
+		 *
+		 * TBD: propagate zpr.rule.family into dataplane and
+		 * netlink code so they can stop using filter src/dst addrs.
+		 */
+		if (!CHECK_FLAG(zpr.rule.filter.filter_bm, PBR_FILTER_SRC_IP))
+			zpr.rule.filter.src_ip.family = zpr.rule.family;
+		if (!CHECK_FLAG(zpr.rule.filter.filter_bm, PBR_FILTER_DST_IP))
+			zpr.rule.filter.dst_ip.family = zpr.rule.family;
+
+		/* TBD delete below block when netlink code gets family from zpr.rule.family */
 		if (!(zpr.rule.filter.src_ip.family == AF_INET
 		      || zpr.rule.filter.src_ip.family == AF_INET6)) {
 			zlog_warn(
-				"Unsupported PBR source IP family: %s (%hhu)",
+				"Unsupported PBR source IP family: %s (%u)",
 				family2str(zpr.rule.filter.src_ip.family),
 				zpr.rule.filter.src_ip.family);
 			return;
@@ -3199,11 +3221,12 @@ static inline void zread_rule(ZAPI_HANDLER_ARGS)
 		if (!(zpr.rule.filter.dst_ip.family == AF_INET
 		      || zpr.rule.filter.dst_ip.family == AF_INET6)) {
 			zlog_warn(
-				"Unsupported PBR destination IP family: %s (%hhu)",
+				"Unsupported PBR destination IP family: %s (%u)",
 				family2str(zpr.rule.filter.dst_ip.family),
 				zpr.rule.filter.dst_ip.family);
 			return;
 		}
+		/* TBD delete above block when netlink code gets family from zpr.rule.family */
 
 
 		zpr.vrf_id = zvrf->vrf->vrf_id;

zebra/zebra_pbr.c

@@ -507,6 +507,7 @@ void zebra_pbr_show_rule_unit(struct zebra_pbr_rule *rule, struct vty *vty)
 {
 	struct pbr_rule *prule = &rule->rule;
 	struct zebra_pbr_action *zaction = &rule->action;
+	struct pbr_action *pa = &prule->action;
 
 	vty_out(vty, "Rules if %s\n", rule->ifname);
 	vty_out(vty, "  Seq %u pri %u\n", prule->seq, prule->priority);
@@ -522,12 +523,12 @@ void zebra_pbr_show_rule_unit(struct zebra_pbr_rule *rule, struct vty *vty)
 	if (prule->filter.filter_bm & PBR_FILTER_DST_PORT)
 		vty_out(vty, "  DST Port Match: %u\n", prule->filter.dst_port);
 
-	if (prule->filter.filter_bm & PBR_FILTER_DSFIELD) {
+	if (prule->filter.filter_bm & PBR_FILTER_DSCP)
 		vty_out(vty, "  DSCP Match: %u\n",
 			(prule->filter.dsfield & PBR_DSFIELD_DSCP) >> 2);
+	if (prule->filter.filter_bm & PBR_FILTER_ECN)
 		vty_out(vty, "  ECN Match: %u\n",
 			prule->filter.dsfield & PBR_DSFIELD_ECN);
-	}
 
 	if (prule->filter.filter_bm & PBR_FILTER_FWMARK)
 		vty_out(vty, "  MARK Match: %u\n", prule->filter.fwmark);
@@ -548,6 +549,30 @@ void zebra_pbr_show_rule_unit(struct zebra_pbr_rule *rule, struct vty *vty)
 		vty_out(vty, "\n");
 	}
 
+	if (CHECK_FLAG(pa->flags, PBR_ACTION_ECN))
+		vty_out(vty, "  Action: Set ECN: %u\n", pa->ecn);
+	if (CHECK_FLAG(pa->flags, PBR_ACTION_DSCP))
+		vty_out(vty, "  Action: Set DSCP: %u\n", pa->dscp >> 2);
+
+	if (CHECK_FLAG(pa->flags, PBR_ACTION_SRC_IP))
+		vty_out(vty, "  Action: Set SRC IP: %pSU\n", &pa->src_ip);
+	if (CHECK_FLAG(pa->flags, PBR_ACTION_DST_IP))
+		vty_out(vty, "  Action: Set DST IP: %pSU\n", &pa->dst_ip);
+	if (CHECK_FLAG(pa->flags, PBR_ACTION_SRC_PORT))
+		vty_out(vty, "  Action: Set SRC PORT: %u\n", pa->src_port);
+	if (CHECK_FLAG(pa->flags, PBR_ACTION_DST_PORT))
+		vty_out(vty, "  Action: Set DST PORT: %u\n", pa->dst_port);
+
+	if (CHECK_FLAG(pa->flags, PBR_ACTION_QUEUE_ID))
+		vty_out(vty, "  Action: Set Queue ID: %u\n", pa->queue_id);
+
+	if (CHECK_FLAG(pa->flags, PBR_ACTION_PCP))
+		vty_out(vty, "  Action: Set PCP: %u\n", pa->pcp);
+	if (CHECK_FLAG(pa->flags, PBR_ACTION_VLAN_ID))
+		vty_out(vty, "  Action: Set VLAN ID: %u\n", pa->vlan_id);
+	if (CHECK_FLAG(pa->flags, PBR_ACTION_VLAN_STRIP_INNER_ANY))
+		vty_out(vty, "  Action: Strip VLAN ID\n");
+
 	vty_out(vty, "  Tableid: %u\n", prule->action.table);
 	if (zaction->afi == AFI_IP)
 		vty_out(vty, "  Action: nh: %pI4 intf: %s\n",

zebra/zebra_pbr.h

@@ -61,8 +61,6 @@ struct zebra_pbr_rule {
 	(r->rule.filter.filter_bm & PBR_FILTER_SRC_PORT)
 #define IS_RULE_FILTERING_ON_DST_PORT(r) \
 	(r->rule.filter.filter_bm & PBR_FILTER_DST_PORT)
-#define IS_RULE_FILTERING_ON_DSFIELD(r) \
-	(r->rule.filter.filter_bm & PBR_FILTER_DSFIELD)
 #define IS_RULE_FILTERING_ON_FWMARK(r) \
 	(r->rule.filter.filter_bm & PBR_FILTER_FWMARK)