pbrd: add vty support for vlan filtering and send to zebra #13514

joshua-werner · 2023-05-11T21:54:39Z

This adds VTY and initial Zebra support for VLAN filters. Since the kernel PBR facilities do not provide any way of matching on VLAN fields, another provider may be required (i.e. netfilter). This rebases the previous pull request by Eli Baum PR 9705, now closed, on master since the previous fork can no longer be updated in situ.

Signed-off-by: Josh Werner joshuawerner@mitre.org

NetDEF-CI · 2023-05-12T00:23:15Z

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-11499/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Warnings Generated during build:

Checkout code: Successful with additional warnings

<stdin>:35: trailing whitespace.
interface to allow admins to influence routing on their router.  At this point in 
<stdin>:226: trailing whitespace.
        
<stdin>:669: trailing whitespace.
    key = jhash_3words(rule->rule.filter.pcp, 
warning: 3 lines add whitespace errors.
Report for pbr_map.c | 14 issues
===============================================
< WARNING: line over 80 characters
< #317: FILE: /tmp/f1-3010521/pbr_map.c:317:
< WARNING: please, no spaces at the start of a line
< #512: FILE: /tmp/f1-3010521/pbr_map.c:512:
< ERROR: trailing whitespace
< #551: FILE: /tmp/f1-3010521/pbr_map.c:551:
< ERROR: code indent should use tabs where possible
< #551: FILE: /tmp/f1-3010521/pbr_map.c:551:
< WARNING: please, no spaces at the start of a line
< #551: FILE: /tmp/f1-3010521/pbr_map.c:551:
< ERROR: code indent should use tabs where possible
< #625: FILE: /tmp/f1-3010521/pbr_map.c:625:
< WARNING: please, no spaces at the start of a line
< #625: FILE: /tmp/f1-3010521/pbr_map.c:625:
Report for pbr_vty.c | 8 issues
===============================================
< WARNING: suspect code indent for conditional statements (8, 12)
< #982: FILE: /tmp/f1-3010521/pbr_vty.c:982:
< WARNING: please, no spaces at the start of a line
< #1395: FILE: /tmp/f1-3010521/pbr_vty.c:1395:
< WARNING: suspect code indent for conditional statements (4, 16)
< #1395: FILE: /tmp/f1-3010521/pbr_vty.c:1395:
< WARNING: please, no spaces at the start of a line
< #1506: FILE: /tmp/f1-3010521/pbr_vty.c:1506:
Report for pbr_zebra.c | 4 issues
===============================================
< WARNING: please, no spaces at the start of a line
< #551: FILE: /tmp/f1-3010521/pbr_zebra.c:551:
< WARNING: Block comments should align the * on each line
< #554: FILE: /tmp/f1-3010521/pbr_zebra.c:554:
Report for zebra_dplane.c | 6 issues
===============================================
< WARNING: please, no spaces at the start of a line
< #251: FILE: /tmp/f1-3010521/zebra_dplane.c:251:
< WARNING: please, no spaces at the start of a line
< #274: FILE: /tmp/f1-3010521/zebra_dplane.c:274:
< WARNING: please, no spaces at the start of a line
< #3435: FILE: /tmp/f1-3010521/zebra_dplane.c:3435:
Report for zebra_pbr.c | 12 issues
===============================================
< ERROR: trailing whitespace
< #172: FILE: /tmp/f1-3010521/zebra_pbr.c:172:
< WARNING: please, no spaces at the start of a line
< #172: FILE: /tmp/f1-3010521/zebra_pbr.c:172:
< ERROR: code indent should use tabs where possible
< #173: FILE: /tmp/f1-3010521/zebra_pbr.c:173:
< WARNING: please, no spaces at the start of a line
< #173: FILE: /tmp/f1-3010521/zebra_pbr.c:173:
< ERROR: code indent should use tabs where possible
< #174: FILE: /tmp/f1-3010521/zebra_pbr.c:174:
< WARNING: please, no spaces at the start of a line
< #174: FILE: /tmp/f1-3010521/zebra_pbr.c:174:

ton31337

Minor changes + some contribution guidelines missing (formatting, signed-off tag).

ton31337 · 2023-05-12T06:39:49Z

doc/user/pbr.rst


   When a incoming packet matches the specified ip protocol, take the
-   packet and forward according to the nexthops specified.
+   packet and forward according to the nexthops specified. Protocols are
+   queried from the protocols database (`/etc/protocols`; see `man 5


nit: if you want to highlight /etc/protocols you have to use double-ticks (``)

ton31337 · 2023-05-12T06:45:31Z

lib/pbr.h

+#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)


Can you leave the old (untouched) code as it is (no formating in this commit)?

ton31337 · 2023-05-12T06:46:33Z

pbrd/pbr_map.c

+{
+	if (pbrms) {
+		pbrms->match_pcp = pcp;
+		zlog_info("setting pbrms->match_pcp = %u ", pbrms->match_pcp);


This sounds to be like a debug and should be guarded 🤷

ton31337 · 2023-05-12T06:49:10Z

pbrd/pbr_vty.c

@@ -25,6 +28,80 @@
 #include "pbrd/pbr_debug.h"
 #include "pbrd/pbr_vty_clippy.c"

+DEFPY(pbr_map_match_pcp, pbr_map_match_pcp_cmd, "[no] match pcp <(0-7)$pcp>",
+      NO_STR
+      "match the rest of the command\n"


Use a capital first letter (Match) to comply with other commands.

ton31337 · 2023-05-12T06:55:28Z

pbrd/pbr_vty.c

+      NO_STR
+      "match the rest of the command\n"
+      "match based on 802.1p Priority Code Point (PCP) value\n"
+      "a valid value in range 0..7 \n")


IMO, zero-point writing values as it's displayed by default from the command itself. Also, please drop whitespace before \n.

ton31337 · 2023-05-12T07:01:36Z

pbrd/pbr_vty.c

+		return CMD_WARNING;
+
+	if (!no) {
+		if (strcmp(tag_type, "tagged") == 0) {


nit: strmatch()

ton31337 · 2023-05-12T07:04:40Z

pbrd/pbr_vty.c

-	  <A.B.C.D|X:X::X:X>$addr [INTERFACE$intf]\
-	  |INTERFACE$intf\
-	>\
+		    <A.B.C.D|X:X::X:X>$addr [INTERFACE$intf]\


Please don't apply the formatting here if you don't touch the code. It's hard to read sometimes (even if it's not the logical/real change).

ton31337 · 2023-05-12T07:04:55Z

pbrd/pbr_vty.c

@@ -879,6 +955,12 @@ static void vty_show_pbrms(struct vty *vty,
 			pbrms->installed ? "yes" : "no",
 			pbrms->reason ? rbuf : "Valid");

+    /* match clauses first */
+
+	if (pbrms->dsfield & PBR_DSFIELD_DSCP)


Why moving this here from the below?

ton31337 · 2023-05-12T07:06:57Z

pbrd/pbr_zebra.c

@@ -3,6 +3,9 @@
 * Zebra connect code.
 * Copyright (C) 2018 Cumulus Networks, Inc.
 *               Donald Sharp
+ * Portions:
+ *		Copyright (c) 2021 The MITRE Corporation. All Rights Reserved.


ton31337 · 2023-05-12T07:07:13Z

zebra/zapi_msg.c

@@ -5,6 +5,8 @@
 *   Copyright (C) 1997-1999  Kunihiro Ishiguro
 *   Copyright (C) 2015-2018  Cumulus Networks, Inc.
 *   et al.
+ *   Copyright (c) 2021 The MITRE Corporation. All Rights Reserved.


mjstapp · 2023-05-15T18:01:19Z

.gitignore

+/changelog-auto
+/m4/ac
+/pathd/
+/pceplib/


we can't really ignore these entire components, can we?

mjstapp · 2023-05-15T18:01:31Z

.gitignore

 pceplib/test/*.log
 pceplib/test/*.trs
+*.xref


I see that already, don't need to dup ?

mjstapp · 2023-05-15T18:01:44Z

lib/pbr.h

-#define PBR_FILTER_DST_PORT		(1 << 3)
-#define PBR_FILTER_FWMARK		(1 << 4)
-#define PBR_FILTER_PROTO		(1 << 5)
+	uint32_t filter_bm; /* not encoded by zapi */


I think we may need to encode these bits - how can we tell the difference between "not present" and "value is zero" otherwise?

Sorry, can you explain a little more in depth what you mean? The comment was only changed in the original MR to be on one line instead of 2. I have reverted it per a ci:cd warning.

I don't see anything related to the issues that Paul and I asked about earlier - the encoding issues?

donaldsharp

I don't mind .gitignore file changes but really really needs to be in it's own commit.

NetDEF-CI · 2023-05-25T02:23:23Z

Continuous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-11742/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Topotests Ubuntu 18.04 amd64 part 3: Failed (click for details)

Topotests Ubuntu 18.04 amd64 part 3: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-11742/artifact/TP3U1804AMD64/TopotestDetails/

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TP3U1804AMD64-11742/test

Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 3
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-11742/artifact/TP3U1804AMD64/TopotestLogs/log_topotests.txt

Topotests debian 10 amd64 part 9: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9DEB10AMD64-11742/test

Topology Tests failed for Topotests debian 10 amd64 part 9
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-11742/artifact/TOPO9DEB10AMD64/TopotestLogs/log_topotests.txt
Topotests debian 10 amd64 part 9: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-11742/artifact/TOPO9DEB10AMD64/TopotestDetails/

Topotests Ubuntu 18.04 i386 part 9: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18I386-11742/test

Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-11742/artifact/TOPO9U18I386/TopotestLogs/log_topotests.txt
Topotests Ubuntu 18.04 i386 part 9: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-11742/artifact/TOPO9U18I386/TopotestDetails/

Topotests debian 10 amd64 part 3: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO3DEB10AMD64-11742/test

Topology Tests failed for Topotests debian 10 amd64 part 3
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-11742/artifact/TOPO3DEB10AMD64/TopotestLogs/log_topotests.txt
Topotests debian 10 amd64 part 3: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-11742/artifact/TOPO3DEB10AMD64/TopotestDetails/

Successful on other platforms/tests

Topotests Ubuntu 18.04 arm8 part 7
Topotests Ubuntu 18.04 i386 part 7
Topotests Ubuntu 18.04 arm8 part 8
Addresssanitizer topotests part 7
Topotests Ubuntu 18.04 i386 part 2
Topotests debian 10 amd64 part 0
Topotests Ubuntu 18.04 amd64 part 5
Addresssanitizer topotests part 5
Topotests Ubuntu 18.04 i386 part 6
Topotests Ubuntu 18.04 i386 part 1
Topotests Ubuntu 18.04 amd64 part 0
Topotests Ubuntu 18.04 amd64 part 2
Topotests Ubuntu 18.04 i386 part 8
Topotests Ubuntu 18.04 i386 part 3
Topotests Ubuntu 18.04 amd64 part 7
Debian 10 deb pkg check
Addresssanitizer topotests part 3
Topotests debian 10 amd64 part 8
Topotests Ubuntu 18.04 i386 part 5
Addresssanitizer topotests part 2
Topotests Ubuntu 18.04 arm8 part 9
Topotests Ubuntu 18.04 arm8 part 2
Addresssanitizer topotests part 9
CentOS 7 rpm pkg check
Topotests Ubuntu 18.04 amd64 part 4
Topotests debian 10 amd64 part 7
Topotests Ubuntu 18.04 amd64 part 9
Topotests Ubuntu 18.04 arm8 part 0
Topotests debian 10 amd64 part 2
Topotests Ubuntu 18.04 amd64 part 8
Addresssanitizer topotests part 8
Static analyzer (clang)
Ubuntu 18.04 deb pkg check
Topotests Ubuntu 18.04 i386 part 0
Addresssanitizer topotests part 6
Ubuntu 20.04 deb pkg check
Topotests debian 10 amd64 part 1
Topotests Ubuntu 18.04 amd64 part 1
Topotests Ubuntu 18.04 arm8 part 6
Topotests Ubuntu 18.04 arm8 part 1
Addresssanitizer topotests part 0
Topotests debian 10 amd64 part 6
Topotests Ubuntu 18.04 amd64 part 6
Addresssanitizer topotests part 4
Debian 9 deb pkg check
Topotests Ubuntu 18.04 arm8 part 3
Topotests debian 10 amd64 part 5
Addresssanitizer topotests part 1
Topotests Ubuntu 18.04 i386 part 4
Topotests debian 10 amd64 part 4

Warnings Generated during build:

Checkout code: Successful with additional warnings

Topotests Ubuntu 18.04 amd64 part 3: Failed (click for details)

Topotests Ubuntu 18.04 amd64 part 3: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-11742/artifact/TP3U1804AMD64/TopotestDetails/

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TP3U1804AMD64-11742/test

Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 3
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-11742/artifact/TP3U1804AMD64/TopotestLogs/log_topotests.txt

Topotests debian 10 amd64 part 9: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9DEB10AMD64-11742/test

Topology Tests failed for Topotests debian 10 amd64 part 9
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-11742/artifact/TOPO9DEB10AMD64/TopotestLogs/log_topotests.txt
Topotests debian 10 amd64 part 9: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-11742/artifact/TOPO9DEB10AMD64/TopotestDetails/

Topotests Ubuntu 18.04 i386 part 9: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18I386-11742/test

Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-11742/artifact/TOPO9U18I386/TopotestLogs/log_topotests.txt
Topotests Ubuntu 18.04 i386 part 9: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-11742/artifact/TOPO9U18I386/TopotestDetails/

Topotests debian 10 amd64 part 3: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO3DEB10AMD64-11742/test

Topology Tests failed for Topotests debian 10 amd64 part 3
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-11742/artifact/TOPO3DEB10AMD64/TopotestLogs/log_topotests.txt
Topotests debian 10 amd64 part 3: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-11742/artifact/TOPO3DEB10AMD64/TopotestDetails/

<stdin>:1736: trailing whitespace.
interface to allow admins to influence routing on their router.  At this point 
<stdin>:4104: trailing whitespace.
        
<stdin>:4327: trailing whitespace.
    
<stdin>:5807: trailing whitespace.
                
<stdin>:6062: new blank line at EOF.
+
warning: squelched 11 whitespace errors
warning: 16 lines add whitespace errors.
Report for bgp_route.c | 2 issues
===============================================
< WARNING: Too many leading tabs - consider code refactoring
< #13076: FILE: /tmp/f1-3173070/bgp_route.c:13076:
Report for libfrr.c | 6 issues
===============================================
< ERROR: do not initialise statics to false
< #133: FILE: /tmp/f1-3173070/libfrr.c:133:
< WARNING: Block comments use * on subsequent lines
< #743: FILE: /tmp/f1-3173070/libfrr.c:743:
< WARNING: Block comments use a trailing */ on a separate line
< #744: FILE: /tmp/f1-3173070/libfrr.c:744:
Report for ospf_vty.c | 2 issues
===============================================
< WARNING: line over 80 characters
< #12156: FILE: /tmp/f1-3173070/ospf_vty.c:12156:
Report for pbr_map.c | 24 issues
===============================================
< ERROR: code indent should use tabs where possible
< #317: FILE: /tmp/f1-3173070/pbr_map.c:317:
< WARNING: please, no spaces at the start of a line
< #317: FILE: /tmp/f1-3173070/pbr_map.c:317:
< WARNING: please, no spaces at the start of a line
< #509: FILE: /tmp/f1-3173070/pbr_map.c:509:
< ERROR: trailing whitespace
< #548: FILE: /tmp/f1-3173070/pbr_map.c:548:
< ERROR: code indent should use tabs where possible
< #548: FILE: /tmp/f1-3173070/pbr_map.c:548:
< WARNING: please, no spaces at the start of a line
< #548: FILE: /tmp/f1-3173070/pbr_map.c:548:
< ERROR: code indent should use tabs where possible
< #551: FILE: /tmp/f1-3173070/pbr_map.c:551:
< WARNING: please, no spaces at the start of a line
< #551: FILE: /tmp/f1-3173070/pbr_map.c:551:
< ERROR: code indent should use tabs where possible
< #622: FILE: /tmp/f1-3173070/pbr_map.c:622:
< WARNING: please, no spaces at the start of a line
< #622: FILE: /tmp/f1-3173070/pbr_map.c:622:
< ERROR: code indent should use tabs where possible
< #625: FILE: /tmp/f1-3173070/pbr_map.c:625:
< WARNING: please, no spaces at the start of a line
< #625: FILE: /tmp/f1-3173070/pbr_map.c:625:
Report for pbr_vty.c | 40 issues
===============================================
< WARNING: please, no spaces at the start of a line
< #84: FILE: /tmp/f1-3173070/pbr_vty.c:84:
< ERROR: code indent should use tabs where possible
< #85: FILE: /tmp/f1-3173070/pbr_vty.c:85:
< WARNING: please, no spaces at the start of a line
< #85: FILE: /tmp/f1-3173070/pbr_vty.c:85:
< WARNING: suspect code indent for conditional statements (8, 24)
< #85: FILE: /tmp/f1-3173070/pbr_vty.c:85:
< ERROR: code indent should use tabs where possible
< #88: FILE: /tmp/f1-3173070/pbr_vty.c:88:
< WARNING: please, no spaces at the start of a line
< #88: FILE: /tmp/f1-3173070/pbr_vty.c:88:
< WARNING: suspect code indent for conditional statements (8, 24)
< #88: FILE: /tmp/f1-3173070/pbr_vty.c:88:
< ERROR: code indent should use tabs where possible
< #91: FILE: /tmp/f1-3173070/pbr_vty.c:91:
< WARNING: please, no spaces at the start of a line
< #91: FILE: /tmp/f1-3173070/pbr_vty.c:91:
< WARNING: suspect code indent for conditional statements (8, 24)
< #91: FILE: /tmp/f1-3173070/pbr_vty.c:91:
< WARNING: suspect code indent for conditional statements (8, 12)
< #983: FILE: /tmp/f1-3173070/pbr_vty.c:983:
< ERROR: trailing whitespace
< #987: FILE: /tmp/f1-3173070/pbr_vty.c:987:
< WARNING: please, no spaces at the start of a line
< #987: FILE: /tmp/f1-3173070/pbr_vty.c:987:
< WARNING: please, no spaces at the start of a line
< #988: FILE: /tmp/f1-3173070/pbr_vty.c:988:
< WARNING: suspect code indent for conditional statements (4, 16)
< #988: FILE: /tmp/f1-3173070/pbr_vty.c:988:
< WARNING: please, no spaces at the start of a line
< #1396: FILE: /tmp/f1-3173070/pbr_vty.c:1396:
< WARNING: suspect code indent for conditional statements (4, 16)
< #1396: FILE: /tmp/f1-3173070/pbr_vty.c:1396:
< WARNING: suspect code indent for conditional statements (8, 12)
< #1399: FILE: /tmp/f1-3173070/pbr_vty.c:1399:
< WARNING: please, no spaces at the start of a line
< #1507: FILE: /tmp/f1-3173070/pbr_vty.c:1507:
< WARNING: please, no spaces at the start of a line
< #1512: FILE: /tmp/f1-3173070/pbr_vty.c:1512:
Report for pbr_zebra.c | 4 issues
===============================================
< WARNING: please, no spaces at the start of a line
< #550: FILE: /tmp/f1-3173070/pbr_zebra.c:550:
< WARNING: Block comments should align the * on each line
< #553: FILE: /tmp/f1-3173070/pbr_zebra.c:553:
Report for ripng_cli.c | 4 issues
===============================================
< WARNING: please, no spaces at the start of a line
< #98: FILE: /tmp/f1-3173070/ripng_cli.c:98:
< WARNING: suspect code indent for conditional statements (4, 16)
< #98: FILE: /tmp/f1-3173070/ripng_cli.c:98:
Report for zebra_dplane.c | 12 issues
===============================================
< WARNING: please, no spaces at the start of a line
< #250: FILE: /tmp/f1-3173070/zebra_dplane.c:250:
< WARNING: please, no spaces at the start of a line
< #251: FILE: /tmp/f1-3173070/zebra_dplane.c:251:
< WARNING: please, no spaces at the start of a line
< #273: FILE: /tmp/f1-3173070/zebra_dplane.c:273:
< WARNING: please, no spaces at the start of a line
< #274: FILE: /tmp/f1-3173070/zebra_dplane.c:274:
< WARNING: please, no spaces at the start of a line
< #3434: FILE: /tmp/f1-3173070/zebra_dplane.c:3434:
< WARNING: please, no spaces at the start of a line
< #3435: FILE: /tmp/f1-3173070/zebra_dplane.c:3435:
Report for zebra_pbr.c | 20 issues
===============================================
< ERROR: code indent should use tabs where possible
< #163: FILE: /tmp/f1-3173070/zebra_pbr.c:163:
< ERROR: code indent should use tabs where possible
< #164: FILE: /tmp/f1-3173070/zebra_pbr.c:164:
< ERROR: code indent should use tabs where possible
< #167: FILE: /tmp/f1-3173070/zebra_pbr.c:167:
< WARNING: please, no spaces at the start of a line
< #171: FILE: /tmp/f1-3173070/zebra_pbr.c:171:
< ERROR: code indent should use tabs where possible
< #172: FILE: /tmp/f1-3173070/zebra_pbr.c:172:
< WARNING: please, no spaces at the start of a line
< #172: FILE: /tmp/f1-3173070/zebra_pbr.c:172:
< ERROR: code indent should use tabs where possible
< #173: FILE: /tmp/f1-3173070/zebra_pbr.c:173:
< WARNING: please, no spaces at the start of a line
< #173: FILE: /tmp/f1-3173070/zebra_pbr.c:173:
< ERROR: code indent should use tabs where possible
< #178: FILE: /tmp/f1-3173070/zebra_pbr.c:178:
< WARNING: please, no spaces at the start of a line
< #178: FILE: /tmp/f1-3173070/zebra_pbr.c:178:

mjstapp · 2023-05-25T12:31:56Z

Please just rebase your branch - no merge commits please?

NetDEF-CI · 2023-06-02T23:40:25Z

Continuous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-11965/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Topotests Ubuntu 18.04 amd64 part 7: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO7U18AMD64-11965/test

Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 7
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-11965/artifact/TOPO7U18AMD64/TopotestLogs/log_topotests.txt
Topotests Ubuntu 18.04 amd64 part 7: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-11965/artifact/TOPO7U18AMD64/TopotestDetails/

Successful on other platforms/tests

Topotests debian 10 amd64 part 6
Topotests Ubuntu 18.04 arm8 part 1
Topotests Ubuntu 18.04 arm8 part 6
Topotests Ubuntu 18.04 amd64 part 0
Addresssanitizer topotests part 3
Topotests Ubuntu 18.04 i386 part 5
Addresssanitizer topotests part 2
Topotests debian 10 amd64 part 5
CentOS 7 rpm pkg check
Topotests Ubuntu 18.04 amd64 part 4
Topotests Ubuntu 18.04 amd64 part 3
Topotests debian 10 amd64 part 7
Addresssanitizer topotests part 8
Topotests Ubuntu 18.04 i386 part 7
Topotests Ubuntu 18.04 i386 part 2
Topotests Ubuntu 18.04 arm8 part 8
Topotests Ubuntu 18.04 i386 part 0
Ubuntu 18.04 deb pkg check
Addresssanitizer topotests part 6
Topotests Ubuntu 18.04 amd64 part 1
Topotests Ubuntu 18.04 amd64 part 2
Topotests Ubuntu 18.04 i386 part 8
Topotests Ubuntu 18.04 i386 part 3
Topotests debian 10 amd64 part 8
Topotests Ubuntu 18.04 amd64 part 6
Addresssanitizer topotests part 0
Topotests debian 10 amd64 part 3
Topotests Ubuntu 18.04 arm8 part 9
Addresssanitizer topotests part 4
Topotests Ubuntu 18.04 arm8 part 3
Debian 9 deb pkg check
Topotests debian 10 amd64 part 9
Addresssanitizer topotests part 1
Topotests Ubuntu 18.04 i386 part 4
Topotests debian 10 amd64 part 4
Topotests debian 10 amd64 part 2
Topotests Ubuntu 18.04 amd64 part 9
Addresssanitizer topotests part 9
Topotests Ubuntu 18.04 arm8 part 0
Topotests Ubuntu 18.04 i386 part 9
Topotests Ubuntu 18.04 amd64 part 8
Topotests Ubuntu 18.04 arm8 part 7
Topotests Ubuntu 18.04 arm8 part 2
Static analyzer (clang)
Topotests debian 10 amd64 part 0
Addresssanitizer topotests part 7
Ubuntu 20.04 deb pkg check
Topotests Ubuntu 18.04 amd64 part 5
Topotests Ubuntu 18.04 i386 part 6
Debian 10 deb pkg check
Topotests debian 10 amd64 part 1
Addresssanitizer topotests part 5
Topotests Ubuntu 18.04 i386 part 1

NetDEF-CI · 2023-06-05T22:51:41Z

Continuous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12016/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Topotests debian 10 amd64 part 9: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9DEB10AMD64-12016/test

Topology Tests failed for Topotests debian 10 amd64 part 9
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-12016/artifact/TOPO9DEB10AMD64/TopotestLogs/log_topotests.txt
Topotests debian 10 amd64 part 9: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12016/artifact/TOPO9DEB10AMD64/TopotestDetails/

Successful on other platforms/tests

Topotests Ubuntu 18.04 arm8 part 3
Addresssanitizer topotests part 4
Addresssanitizer topotests part 5
Topotests Ubuntu 18.04 amd64 part 2
Topotests debian 10 amd64 part 4
Topotests Ubuntu 18.04 amd64 part 3
Topotests debian 10 amd64 part 8
Addresssanitizer topotests part 0
Topotests debian 10 amd64 part 3
Topotests Ubuntu 18.04 arm8 part 9
Topotests debian 10 amd64 part 0
Topotests Ubuntu 18.04 amd64 part 5
Topotests Ubuntu 18.04 arm8 part 2
Ubuntu 20.04 deb pkg check
Topotests Ubuntu 18.04 i386 part 1
Topotests Ubuntu 18.04 amd64 part 8
Addresssanitizer topotests part 9
Topotests Ubuntu 18.04 amd64 part 4
Topotests Ubuntu 18.04 arm8 part 0
Topotests Ubuntu 18.04 i386 part 6
Topotests Ubuntu 18.04 i386 part 8
Topotests Ubuntu 18.04 amd64 part 9
Topotests Ubuntu 18.04 i386 part 3
Topotests Ubuntu 18.04 arm8 part 7
Debian 10 deb pkg check
Addresssanitizer topotests part 7
Topotests Ubuntu 18.04 amd64 part 7
Addresssanitizer topotests part 6
Topotests Ubuntu 18.04 i386 part 5
Addresssanitizer topotests part 3
Topotests Ubuntu 18.04 i386 part 0
Topotests debian 10 amd64 part 1
CentOS 7 rpm pkg check
Topotests Ubuntu 18.04 amd64 part 0
Topotests debian 10 amd64 part 2
Topotests Ubuntu 18.04 i386 part 9
Addresssanitizer topotests part 2
Static analyzer (clang)
Debian 9 deb pkg check
Topotests Ubuntu 18.04 amd64 part 1
Addresssanitizer topotests part 1
Topotests Ubuntu 18.04 i386 part 4
Topotests debian 10 amd64 part 5
Ubuntu 18.04 deb pkg check
Topotests debian 10 amd64 part 7
Topotests Ubuntu 18.04 arm8 part 6
Addresssanitizer topotests part 8
Topotests debian 10 amd64 part 6
Topotests Ubuntu 18.04 arm8 part 1
Topotests Ubuntu 18.04 arm8 part 8
Topotests Ubuntu 18.04 i386 part 7
Topotests Ubuntu 18.04 amd64 part 6
Topotests Ubuntu 18.04 i386 part 2

NetDEF-CI · 2023-06-08T23:16:09Z

Continuous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12151/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Topotests Ubuntu 18.04 amd64 part 9: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18AMD64-12151/test

Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 9
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-12151/artifact/TOPO9U18AMD64/TopotestLogs/log_topotests.txt
Topotests Ubuntu 18.04 amd64 part 9: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12151/artifact/TOPO9U18AMD64/TopotestDetails/

Topotests Ubuntu 18.04 arm8 part 7: Failed (click for details)

Topotests Ubuntu 18.04 arm8 part 7: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12151/artifact/TOPO7U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 7: No useful log found

Topotests Ubuntu 18.04 amd64 part 7: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO7U18AMD64-12151/test

Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 7
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-12151/artifact/TOPO7U18AMD64/TopotestLogs/log_topotests.txt
Topotests Ubuntu 18.04 amd64 part 7: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12151/artifact/TOPO7U18AMD64/TopotestDetails/

Topotests Ubuntu 18.04 i386 part 9: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18I386-12151/test

Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-12151/artifact/TOPO9U18I386/TopotestLogs/log_topotests.txt
Topotests Ubuntu 18.04 i386 part 9: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12151/artifact/TOPO9U18I386/TopotestDetails/

Topotests debian 10 amd64 part 7: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO7DEB10AMD64-12151/test

Topology Tests failed for Topotests debian 10 amd64 part 7
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-12151/artifact/TOPO7DEB10AMD64/TopotestLogs/log_topotests.txt
Topotests debian 10 amd64 part 7: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12151/artifact/TOPO7DEB10AMD64/TopotestDetails/

Topotests Ubuntu 18.04 i386 part 7: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO7U18I386-12151/test

Topology Tests failed for Topotests Ubuntu 18.04 i386 part 7
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-12151/artifact/TOPO7U18I386/TopotestLogs/log_topotests.txt
Topotests Ubuntu 18.04 i386 part 7: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12151/artifact/TOPO7U18I386/TopotestDetails/

Successful on other platforms/tests

Addresssanitizer topotests part 5
Addresssanitizer topotests part 4
Topotests Ubuntu 18.04 i386 part 3
Topotests Ubuntu 18.04 amd64 part 2
Topotests Ubuntu 18.04 i386 part 8
Addresssanitizer topotests part 0
Topotests debian 10 amd64 part 8
Topotests debian 10 amd64 part 4
Topotests debian 10 amd64 part 3
Topotests Ubuntu 18.04 arm8 part 9
Topotests Ubuntu 18.04 arm8 part 3
Topotests Ubuntu 18.04 amd64 part 3
Topotests debian 10 amd64 part 9
Addresssanitizer topotests part 1
Static analyzer (clang)
Topotests debian 10 amd64 part 0
Addresssanitizer topotests part 9
Topotests Ubuntu 18.04 arm8 part 0
Topotests Ubuntu 18.04 amd64 part 8
Topotests debian 10 amd64 part 2
Topotests Ubuntu 18.04 arm8 part 2
Topotests Ubuntu 18.04 i386 part 1
Debian 10 deb pkg check
Topotests Ubuntu 18.04 i386 part 6
Addresssanitizer topotests part 7
Topotests Ubuntu 18.04 amd64 part 5
Ubuntu 20.04 deb pkg check
Topotests debian 10 amd64 part 1
Addresssanitizer topotests part 3
Topotests Ubuntu 18.04 arm8 part 1
Topotests debian 10 amd64 part 6
Topotests Ubuntu 18.04 i386 part 0
Topotests Ubuntu 18.04 i386 part 5
CentOS 7 rpm pkg check
Topotests Ubuntu 18.04 amd64 part 4
Addresssanitizer topotests part 2
Topotests Ubuntu 18.04 amd64 part 0
Topotests Ubuntu 18.04 i386 part 4
Debian 9 deb pkg check
Addresssanitizer topotests part 8
Topotests debian 10 amd64 part 5
Topotests Ubuntu 18.04 amd64 part 6
Topotests Ubuntu 18.04 i386 part 2
Topotests Ubuntu 18.04 arm8 part 8
Topotests Ubuntu 18.04 arm8 part 6
Addresssanitizer topotests part 6
Topotests Ubuntu 18.04 amd64 part 1
Ubuntu 18.04 deb pkg check

gpziemba · 2023-06-10T00:06:55Z

pbrd/pbr_map.c

+{
+	if (pbrms) {
+		pbrms->match_pcp = pcp;
+		pbr_map_check(pbrms, true);


How does this work for matching PCP==0 ? Unless I missed something in the code, there is no use of a separate flag to indicate that PCP should be matched vs. ignored.

I believe you're right and that currently match pcp zero is simply ignored since there is a check in pbr_vty.c to see if it is non-zero. My initial thought is to set the default value to 8, rather than 0, in the pbrms_get and then handle 8 as a special ignore case and allow 0 as a match case. However, this will be difficult to test since, again per Eli, the default Linux kernel provider does not support matching PCPs, so actually testing this case differential may be difficult. Might be good to have a quick meeting to discuss if you have any availability?

I think your proposal could work. It would mean that the check for "are we matching" is something like (pcp != 8) and then the value to match against is (pcp & 7).

Another way might be to set the "8" bit for every match and unset for no match. Then the "are we matching check" (e.g., in zapi_msg.c) remains (pcp != 0) and the value to match is (pcp & 7).

I don't have a strong opinion either way - please pick whichever seems clearer to you.

gpziemba · 2023-06-10T00:07:00Z

pbrd/pbr_map.c

+	if (pbrms) {
+		pbrms->match_vlan_id = vlan_id;
+		pbrms->match_vlan_flags = vlan_flags;
+		pbr_map_check(pbrms, true);


Same concern as above for PCP: How does the code distinguish between "match VLAN_ID==0" and "ignore VLAN_ID" ?

In this case, VLAN_ID should only be a value between 1-4094 (see https://datatracker.ietf.org/doc/html/rfc5517). ID 0 is reserved, and handled by the untagged_0 match, so a value of zero is always ignored unless you do match match untagged-or-zero, rather than match 0. Eli documented this in the .rst and it is has special casing already in pbr_vty.c where it specifically excludes the value of 0 and matches instead on the constants defined in the pbr header (PBR_MAP_VLAN_UNTAGGED_0, PBR_MAP_VLAN_UNTAGGED, etc.). So, from my understanding these cases are distinct already.

Thanks - it's a good explanation and I agree.

github-actions · 2023-06-13T14:32:10Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

NetDEF-CI · 2023-06-15T17:56:26Z

Continuous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12352/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Topotests Ubuntu 18.04 i386 part 3: Failed (click for details)

Topotests Ubuntu 18.04 i386 part 3: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12352/artifact/TOPO3U18I386/TopotestDetails/

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO3U18I386-12352/test

Topology Tests failed for Topotests Ubuntu 18.04 i386 part 3
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-12352/artifact/TOPO3U18I386/TopotestLogs/log_topotests.txt

Topotests debian 10 amd64 part 3: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO3DEB10AMD64-12352/test

Topology Tests failed for Topotests debian 10 amd64 part 3
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-12352/artifact/TOPO3DEB10AMD64/TopotestLogs/log_topotests.txt
Topotests debian 10 amd64 part 3: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12352/artifact/TOPO3DEB10AMD64/TopotestDetails/

Successful on other platforms/tests

Topotests Ubuntu 18.04 arm8 part 6
Topotests Ubuntu 18.04 amd64 part 0
Topotests debian 10 amd64 part 6
Topotests Ubuntu 18.04 arm8 part 1
Topotests Ubuntu 18.04 amd64 part 7
Addresssanitizer topotests part 2
Topotests debian 10 amd64 part 0
Topotests Ubuntu 18.04 arm8 part 2
Addresssanitizer topotests part 3
Topotests debian 10 amd64 part 5
Addresssanitizer topotests part 9
CentOS 7 rpm pkg check
Topotests Ubuntu 18.04 amd64 part 4
Topotests Ubuntu 18.04 amd64 part 3
Addresssanitizer topotests part 8
Topotests Ubuntu 18.04 i386 part 7
Topotests Ubuntu 18.04 arm8 part 8
Topotests Ubuntu 18.04 i386 part 2
Topotests Ubuntu 18.04 i386 part 5
Topotests Ubuntu 18.04 amd64 part 1
Addresssanitizer topotests part 6
Topotests Ubuntu 18.04 i386 part 0
Ubuntu 18.04 deb pkg check
Topotests Ubuntu 18.04 amd64 part 2
Topotests debian 10 amd64 part 7
Topotests Ubuntu 18.04 i386 part 8
Topotests debian 10 amd64 part 8
Addresssanitizer topotests part 0
Debian 10 deb pkg check
Topotests Ubuntu 18.04 amd64 part 6
Topotests Ubuntu 18.04 arm8 part 9
Debian 9 deb pkg check
Topotests Ubuntu 18.04 arm8 part 3
Addresssanitizer topotests part 4
Addresssanitizer topotests part 1
Topotests debian 10 amd64 part 9
Topotests Ubuntu 18.04 i386 part 4
Topotests debian 10 amd64 part 4
Topotests Ubuntu 18.04 amd64 part 9
Topotests debian 10 amd64 part 2
Topotests Ubuntu 18.04 i386 part 9
Topotests Ubuntu 18.04 arm8 part 7
Addresssanitizer topotests part 7
Static analyzer (clang)
Topotests Ubuntu 18.04 amd64 part 5
Ubuntu 20.04 deb pkg check
Addresssanitizer topotests part 5
Topotests debian 10 amd64 part 1
Topotests Ubuntu 18.04 i386 part 1
Topotests Ubuntu 18.04 amd64 part 8
Topotests Ubuntu 18.04 arm8 part 0
Topotests Ubuntu 18.04 i386 part 6

mjstapp

still seem to have some open questions?
and I think the commits need to be reorganized - we'd rather not have "fix it" or "undo it" commits when those could be squashed.

mjstapp · 2023-06-15T18:03:04Z

.gitignore

@@ -114,6 +114,5 @@ refix
 .kitchen
 .emacs.desktop*

-/test-suite.log


It's a duplicated line

mjstapp · 2023-06-15T18:07:16Z

lib/pbr.h

-#define PBR_FILTER_DST_PORT		(1 << 3)
-#define PBR_FILTER_FWMARK		(1 << 4)
-#define PBR_FILTER_PROTO		(1 << 5)
+	uint32_t filter_bm; /* not encoded by zapi */


I don't see anything related to the issues that Paul and I asked about earlier - the encoding issues?

joshua-werner · 2023-06-15T18:28:18Z

still seem to have some open questions? and I think the commits need to be reorganized - we'd rather not have "fix it" or "undo it" commits when those could be squashed.

Yes, the latest push is just to rebase on master. I am currently working on the other changes and planned to mark the thread resolved when I have. I was under the impression we would squash the commits at merge time, but you want me to squash them now with an interactive rebase?

NetDEF-CI · 2023-06-21T02:08:10Z

Continuous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12428/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Topotests Ubuntu 18.04 arm8 part 9: Failed (click for details)

Topotests Ubuntu 18.04 arm8 part 9: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12428/artifact/TOPO9U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 9: No useful log found

Topotests Ubuntu 18.04 i386 part 3: Failed (click for details)

Topotests Ubuntu 18.04 i386 part 3: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12428/artifact/TOPO3U18I386/TopotestDetails/

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO3U18I386-12428/test

Topology Tests failed for Topotests Ubuntu 18.04 i386 part 3
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-12428/artifact/TOPO3U18I386/TopotestLogs/log_topotests.txt

Topotests Ubuntu 18.04 amd64 part 9: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18AMD64-12428/test

Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 9
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-12428/artifact/TOPO9U18AMD64/TopotestLogs/log_topotests.txt
Topotests Ubuntu 18.04 amd64 part 9: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12428/artifact/TOPO9U18AMD64/TopotestDetails/

Topotests Ubuntu 18.04 i386 part 9: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18I386-12428/test

Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-12428/artifact/TOPO9U18I386/TopotestLogs/log_topotests.txt
Topotests Ubuntu 18.04 i386 part 9: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12428/artifact/TOPO9U18I386/TopotestDetails/

Topotests debian 10 amd64 part 9: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9DEB10AMD64-12428/test

Topology Tests failed for Topotests debian 10 amd64 part 9
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-12428/artifact/TOPO9DEB10AMD64/TopotestLogs/log_topotests.txt
Topotests debian 10 amd64 part 9: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12428/artifact/TOPO9DEB10AMD64/TopotestDetails/

Successful on other platforms/tests

Ubuntu 20.04 deb pkg check
Ubuntu 18.04 deb pkg check
Debian 10 deb pkg check
Debian 9 deb pkg check
Addresssanitizer topotests part 1
Topotests Ubuntu 18.04 i386 part 4
Topotests Ubuntu 18.04 amd64 part 1
Topotests Ubuntu 18.04 i386 part 8
Addresssanitizer topotests part 8
Topotests Ubuntu 18.04 amd64 part 6
Addresssanitizer topotests part 4
Addresssanitizer topotests part 5
Topotests debian 10 amd64 part 1
Topotests Ubuntu 18.04 arm8 part 3
Topotests debian 10 amd64 part 4
Topotests debian 10 amd64 part 2
Topotests Ubuntu 18.04 arm8 part 0
Topotests Ubuntu 18.04 amd64 part 8
Topotests debian 10 amd64 part 3
Addresssanitizer topotests part 0
Static analyzer (clang)
Addresssanitizer topotests part 2
Topotests Ubuntu 18.04 i386 part 6
Topotests Ubuntu 18.04 amd64 part 5
Topotests Ubuntu 18.04 amd64 part 4
Topotests Ubuntu 18.04 i386 part 1
Addresssanitizer topotests part 9
Topotests Ubuntu 18.04 arm8 part 7
Topotests debian 10 amd64 part 6
Topotests Ubuntu 18.04 arm8 part 6
Topotests Ubuntu 18.04 arm8 part 1
Topotests Ubuntu 18.04 arm8 part 2
Addresssanitizer topotests part 3
Topotests Ubuntu 18.04 amd64 part 7
Topotests Ubuntu 18.04 i386 part 2
Topotests debian 10 amd64 part 5
Topotests debian 10 amd64 part 0
Addresssanitizer topotests part 7
Topotests Ubuntu 18.04 arm8 part 8
Topotests Ubuntu 18.04 i386 part 0
Topotests Ubuntu 18.04 i386 part 7
Addresssanitizer topotests part 6
CentOS 7 rpm pkg check
Topotests debian 10 amd64 part 7
Topotests Ubuntu 18.04 amd64 part 2
Topotests Ubuntu 18.04 amd64 part 3
Topotests Ubuntu 18.04 amd64 part 0
Topotests debian 10 amd64 part 8
Topotests Ubuntu 18.04 i386 part 5

gpziemba · 2023-06-24T21:50:58Z

zebra/zapi_msg.c

@@ -3244,6 +3248,15 @@ static inline void zread_rule(ZAPI_HANDLER_ARGS)
 		if (zpr.rule.filter.fwmark)
 			zpr.rule.filter.filter_bm |= PBR_FILTER_FWMARK;

+		if (zpr.rule.filter.pcp)
+			zpr.rule.filter.filter_bm |= PBR_FILTER_PCP;


Check or mask correctly here?

NetDEF-CI · 2023-06-29T19:29:05Z

Continuous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12649/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Topotests debian 10 amd64 part 9: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9DEB10AMD64-12649/test

Topology Tests failed for Topotests debian 10 amd64 part 9
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-12649/artifact/TOPO9DEB10AMD64/TopotestLogs/log_topotests.txt
Topotests debian 10 amd64 part 9: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12649/artifact/TOPO9DEB10AMD64/TopotestDetails/

Topotests Ubuntu 18.04 arm8 part 9: Failed (click for details)

Topotests Ubuntu 18.04 arm8 part 9: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12649/artifact/TOPO9U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 9: No useful log found

Topotests Ubuntu 18.04 i386 part 3: Failed (click for details)

Topotests Ubuntu 18.04 i386 part 3: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12649/artifact/TOPO3U18I386/TopotestDetails/

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO3U18I386-12649/test

Topology Tests failed for Topotests Ubuntu 18.04 i386 part 3
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-12649/artifact/TOPO3U18I386/TopotestLogs/log_topotests.txt

Topotests Ubuntu 18.04 amd64 part 9: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18AMD64-12649/test

Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 9
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-12649/artifact/TOPO9U18AMD64/TopotestLogs/log_topotests.txt
Topotests Ubuntu 18.04 amd64 part 9: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12649/artifact/TOPO9U18AMD64/TopotestDetails/

Topotests Ubuntu 18.04 i386 part 9: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18I386-12649/test

Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-12649/artifact/TOPO9U18I386/TopotestLogs/log_topotests.txt
Topotests Ubuntu 18.04 i386 part 9: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12649/artifact/TOPO9U18I386/TopotestDetails/

Successful on other platforms/tests

Topotests debian 10 amd64 part 4
Addresssanitizer topotests part 7
Topotests Ubuntu 18.04 arm8 part 8
Addresssanitizer topotests part 4
Topotests Ubuntu 18.04 i386 part 7
Topotests Ubuntu 18.04 i386 part 2
Topotests Ubuntu 18.04 arm8 part 3
Addresssanitizer topotests part 5
Topotests Ubuntu 18.04 amd64 part 2
Topotests Ubuntu 18.04 amd64 part 3
Topotests debian 10 amd64 part 8
Addresssanitizer topotests part 0
Debian 10 deb pkg check
Topotests Ubuntu 18.04 i386 part 1
Addresssanitizer topotests part 2
Topotests Ubuntu 18.04 amd64 part 4
Topotests Ubuntu 18.04 amd64 part 5
Topotests Ubuntu 18.04 i386 part 6
Topotests Ubuntu 18.04 amd64 part 0
Ubuntu 20.04 deb pkg check
Topotests Ubuntu 18.04 arm8 part 7
Addresssanitizer topotests part 9
Topotests Ubuntu 18.04 amd64 part 7
Topotests debian 10 amd64 part 0
Topotests Ubuntu 18.04 i386 part 8
Addresssanitizer topotests part 3
Topotests Ubuntu 18.04 arm8 part 2
CentOS 7 rpm pkg check
Addresssanitizer topotests part 6
Topotests debian 10 amd64 part 1
Topotests Ubuntu 18.04 amd64 part 8
Topotests debian 10 amd64 part 7
Topotests Ubuntu 18.04 arm8 part 0
Static analyzer (clang)
Topotests Ubuntu 18.04 i386 part 5
Topotests Ubuntu 18.04 i386 part 0
Topotests debian 10 amd64 part 2
Topotests debian 10 amd64 part 3
Topotests Ubuntu 18.04 amd64 part 6
Topotests Ubuntu 18.04 amd64 part 1
Topotests Ubuntu 18.04 arm8 part 6
Ubuntu 18.04 deb pkg check
Addresssanitizer topotests part 1
Topotests Ubuntu 18.04 i386 part 4
Topotests Ubuntu 18.04 arm8 part 1
Topotests debian 10 amd64 part 6
Debian 9 deb pkg check
Topotests debian 10 amd64 part 5
Addresssanitizer topotests part 8

Resolves conflict encountered during rebasing on master; new commits add else condition to pbr_encode_pbr_map_sequence--maintained in this PR Signed-off-by: Josh Werner <joshuawerner@mitre.org>

Address formatting warnings from ci:cd; remove unintentional reformatting from initial commit per comments; remove redundant value displays and an unguarded debug statement; standardize output format in vty per comment; realign vty ordering per comment Signed-off-by: Josh Werner <joshuawerner@mitre.org>

Removes files added to the .gitignore per a PR comment. Also deletes erroneous duplicate. Signed-off-by: Josh Werner <joshuawerner@mitre.org>

Simple commit applying style suggestions from frrbot. Signed-off-by: Josh Werner <joshuawerner@mitre.org>

Sets the pbrms match_pcp to default to 8 instead of 0 to distinguish between the case where pcp is ignored and pcp 0 is instead matched Signed-off-by: Josh Werner <joshuawerner@mitre.org>

NetDEF-CI · 2023-06-30T01:04:31Z

Continuous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12659/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Topotests debian 10 amd64 part 9: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9DEB10AMD64-12659/test

Topology Tests failed for Topotests debian 10 amd64 part 9
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-12659/artifact/TOPO9DEB10AMD64/TopotestLogs/log_topotests.txt
Topotests debian 10 amd64 part 9: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12659/artifact/TOPO9DEB10AMD64/TopotestDetails/

Topotests Ubuntu 18.04 arm8 part 9: Failed (click for details)

Topotests Ubuntu 18.04 arm8 part 9: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12659/artifact/TOPO9U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 9: No useful log found

Topotests Ubuntu 18.04 amd64 part 9: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18AMD64-12659/test

Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 9
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-12659/artifact/TOPO9U18AMD64/TopotestLogs/log_topotests.txt
Topotests Ubuntu 18.04 amd64 part 9: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12659/artifact/TOPO9U18AMD64/TopotestDetails/

Topotests Ubuntu 18.04 i386 part 9: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18I386-12659/test

Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-12659/artifact/TOPO9U18I386/TopotestLogs/log_topotests.txt
Topotests Ubuntu 18.04 i386 part 9: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12659/artifact/TOPO9U18I386/TopotestDetails/

Successful on other platforms/tests

Topotests Ubuntu 18.04 arm8 part 3
Addresssanitizer topotests part 5
Topotests debian 10 amd64 part 4
Topotests Ubuntu 18.04 i386 part 3
Topotests Ubuntu 18.04 amd64 part 2
Addresssanitizer topotests part 4
Topotests debian 10 amd64 part 3
Addresssanitizer topotests part 0
Topotests debian 10 amd64 part 8
Topotests Ubuntu 18.04 amd64 part 3
Static analyzer (clang)
Topotests Ubuntu 18.04 arm8 part 2
Topotests debian 10 amd64 part 0
Topotests Ubuntu 18.04 i386 part 6
Topotests debian 10 amd64 part 2
Topotests Ubuntu 18.04 amd64 part 8
Topotests Ubuntu 18.04 arm8 part 7
Addresssanitizer topotests part 9
Topotests Ubuntu 18.04 arm8 part 0
Topotests Ubuntu 18.04 i386 part 8
Topotests Ubuntu 18.04 i386 part 1
Addresssanitizer topotests part 7
Ubuntu 20.04 deb pkg check
Topotests Ubuntu 18.04 amd64 part 5
Debian 10 deb pkg check
Topotests Ubuntu 18.04 amd64 part 7
Topotests debian 10 amd64 part 1
Addresssanitizer topotests part 3
Topotests Ubuntu 18.04 i386 part 5
Topotests Ubuntu 18.04 i386 part 0
CentOS 7 rpm pkg check
Topotests Ubuntu 18.04 amd64 part 4
Addresssanitizer topotests part 2
Topotests Ubuntu 18.04 amd64 part 0
Topotests debian 10 amd64 part 5
Addresssanitizer topotests part 1
Topotests Ubuntu 18.04 i386 part 4
Topotests Ubuntu 18.04 amd64 part 1
Topotests debian 10 amd64 part 7
Debian 9 deb pkg check
Addresssanitizer topotests part 8
Topotests Ubuntu 18.04 i386 part 2
Topotests Ubuntu 18.04 amd64 part 6
Topotests Ubuntu 18.04 arm8 part 8
Ubuntu 18.04 deb pkg check
Topotests Ubuntu 18.04 arm8 part 6
Addresssanitizer topotests part 6
Topotests debian 10 amd64 part 6
Topotests Ubuntu 18.04 i386 part 7
Topotests Ubuntu 18.04 arm8 part 1

gpziemba · 2023-07-18T19:23:46Z

Please close: superseded by #14026

mjstapp · 2023-07-18T19:27:00Z

Closing this: new, revised version referenced above...

frrbot bot added the pbr label May 11, 2023

github-actions bot added size/L master labels May 11, 2023

ton31337 requested changes May 12, 2023

View reviewed changes

mjstapp reviewed May 15, 2023

View reviewed changes

donaldsharp requested changes May 15, 2023

View reviewed changes

frrbot bot added bfd bgp bugfix documentation libfrr ospf ospfv3 pim rip ripng tests Topotests, make check, etc tools zebra labels May 24, 2023

github-actions bot added size/XXL and removed size/L labels May 24, 2023

frrbot bot added eigrp ldp staticd vtysh labels Jun 2, 2023

github-actions bot added the rebase PR needs rebase label Jun 2, 2023

joshua-werner force-pushed the add-vlan-filters branch from 400aa6c to 1ee6580 Compare June 2, 2023 19:56

joshua-werner force-pushed the add-vlan-filters branch from 81651ca to 07c66d5 Compare June 5, 2023 20:09

joshua-werner force-pushed the add-vlan-filters branch from 07c66d5 to 399db32 Compare June 8, 2023 18:10

gpziemba reviewed Jun 10, 2023

View reviewed changes

github-actions bot added the conflicts label Jun 13, 2023

joshua-werner force-pushed the add-vlan-filters branch from 399db32 to 2e4e7b9 Compare June 15, 2023 15:03

github-actions bot removed the conflicts label Jun 15, 2023

mjstapp reviewed Jun 15, 2023

View reviewed changes

joshua-werner force-pushed the add-vlan-filters branch from 2e4e7b9 to ef32761 Compare June 20, 2023 23:26

gpziemba reviewed Jun 24, 2023

View reviewed changes

joshua-werner force-pushed the add-vlan-filters branch from ef32761 to bad3c7e Compare June 29, 2023 16:35

joshua-werner added 5 commits June 29, 2023 18:04

pbrd: resolve rebasing conflict

3b146fd

Resolves conflict encountered during rebasing on master; new commits add else condition to pbr_encode_pbr_map_sequence--maintained in this PR Signed-off-by: Josh Werner <joshuawerner@mitre.org>

.gitignore: undo changes to address PR comments

32cf0c3

Removes files added to the .gitignore per a PR comment. Also deletes erroneous duplicate. Signed-off-by: Josh Werner <joshuawerner@mitre.org>

pbrd: adopt style suggestions

f20b116

Simple commit applying style suggestions from frrbot. Signed-off-by: Josh Werner <joshuawerner@mitre.org>

pbrd: add logic to distinguish between ignoring pcp and matching pcp 0

b21c325

Sets the pbrms match_pcp to default to 8 instead of 0 to distinguish between the case where pcp is ignored and pcp 0 is instead matched Signed-off-by: Josh Werner <joshuawerner@mitre.org>

joshua-werner force-pushed the add-vlan-filters branch from bad3c7e to b21c325 Compare June 29, 2023 22:04

Jafaral mentioned this pull request Jul 18, 2023

pbrd: 1/3: add vty support for vlan filtering and send to zebra #14026

Merged

mjstapp closed this Jul 18, 2023

pbrd: add vty support for vlan filtering and send to zebra #13514

pbrd: add vty support for vlan filtering and send to zebra #13514

Conversation

joshua-werner commented May 11, 2023 • edited Loading

NetDEF-CI commented May 12, 2023 • edited Loading

Continuous Integration Result: SUCCESSFUL

Warnings Generated during build:

ton31337 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

donaldsharp left a comment

Choose a reason for hiding this comment

NetDEF-CI commented May 25, 2023 • edited Loading

Continuous Integration Result: FAILED

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Warnings Generated during build:

mjstapp commented May 25, 2023

NetDEF-CI commented Jun 2, 2023 • edited Loading

Continuous Integration Result: FAILED

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

NetDEF-CI commented Jun 5, 2023 • edited Loading

Continuous Integration Result: FAILED

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

NetDEF-CI commented Jun 8, 2023 • edited Loading

Continuous Integration Result: FAILED

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jun 13, 2023

NetDEF-CI commented Jun 15, 2023 • edited Loading

Continuous Integration Result: FAILED

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

mjstapp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshua-werner commented Jun 15, 2023

NetDEF-CI commented Jun 21, 2023 • edited Loading

Continuous Integration Result: FAILED

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Choose a reason for hiding this comment

NetDEF-CI commented Jun 29, 2023 • edited Loading

Continuous Integration Result: FAILED

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

NetDEF-CI commented Jun 30, 2023

Continuous Integration Result: FAILED

Get source / Pull Request: Successful

joshua-werner commented May 11, 2023 •

edited

Loading

NetDEF-CI commented May 12, 2023 •

edited

Loading

NetDEF-CI commented May 25, 2023 •

edited

Loading

NetDEF-CI commented Jun 2, 2023 •

edited

Loading

NetDEF-CI commented Jun 5, 2023 •

edited

Loading

NetDEF-CI commented Jun 8, 2023 •

edited

Loading

NetDEF-CI commented Jun 15, 2023 •

edited

Loading

NetDEF-CI commented Jun 21, 2023 •

edited

Loading

NetDEF-CI commented Jun 29, 2023 •

edited

Loading