Pim vrf acl fixes (backport #8637) #9186
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![mergify[bot]](https://avatars.githubusercontent.com/in/10562?s=60&v=4){kind=small}
Merged
polychaeta
reviewed
Jul 26, 2021
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Pylint found errors in source files changed by this PR:
Pylint report for my_frr/tests/topotests/lib/topotest.py:
************* Module lib.topotest
my_frr/tests/topotests/lib/topotest.py:1937:15: E0602: Undefined variable 'unicode' (undefined-variable)
-----------------------------------
Your code has been rated at 9.95/10
Pylint report for my_frr/tests/topotests/pim_acl/test_pim_acl.py:
************* Module test_pim_acl
my_frr/tests/topotests/pim_acl/test_pim_acl.py:101:0: C0413: Import "import json" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:102:0: C0413: Import "import functools" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:103:0: C0413: Import "import os" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:104:0: C0413: Import "import sys" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:105:0: C0413: Import "import pytest" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:106:0: C0413: Import "import re" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:107:0: C0413: Import "import time" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:108:0: C0413: Import "from time import sleep" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:109:0: C0413: Import "import socket" should be placed at the top of the module (wrong-import-position)
-----------------------------------
Your code has been rated at 9.46/10
Pylint report for my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:
************* Module test_pim_vrf
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:88:0: C0413: Import "import json" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:89:0: C0413: Import "import functools" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:90:0: C0413: Import "import os" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:91:0: C0413: Import "import sys" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:92:0: C0413: Import "import pytest" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:93:0: C0413: Import "import re" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:94:0: C0413: Import "import time" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:95:0: C0413: Import "from time import sleep" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:96:0: C0413: Import "import socket" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:419:16: E1305: Too many arguments for format string (too-many-format-args)
-----------------------------------
Your code has been rated at 9.28/10
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/9e46eb84c236f8fd5092cfa03c9c7c95/raw/3a867a64c0275729aba116bba473d6991b42fe10/cr_9186_1627306504.diff | git apply
diff --git a/lib/plist.c b/lib/plist.c
index 2b42c4376..a392bf65f 100644
--- a/lib/plist.c
+++ b/lib/plist.c
@@ -780,11 +780,10 @@ static int prefix_list_entry_match(struct prefix_list_entry *pentry,
return 1;
}
-enum prefix_list_type prefix_list_apply_ext(
- struct prefix_list *plist,
- const struct prefix_list_entry **which,
- union prefixconstptr object,
- bool address_mode)
+enum prefix_list_type
+prefix_list_apply_ext(struct prefix_list *plist,
+ const struct prefix_list_entry **which,
+ union prefixconstptr object, bool address_mode)
{
struct prefix_list_entry *pentry, *pbest = NULL;
@@ -1300,17 +1299,16 @@ DEFPY (clear_ipv6_prefix_list,
return vty_clear_prefix_list(vty, AFI_IP6, prefix_list, prefix_str);
}
-DEFPY (debug_prefix_list_match,
- debug_prefix_list_match_cmd,
- "debug prefix-list WORD$prefix-list match <A.B.C.D/M|X:X::X:X/M>"
- " [address-mode$addr_mode]",
- DEBUG_STR
- "Prefix-list test access\n"
- "Name of a prefix list\n"
- "Test prefix for prefix list result\n"
- "Prefix to test in ip prefix-list\n"
- "Prefix to test in ipv6 prefix-list\n"
- "Use address matching mode (PIM RP)\n")
+DEFPY(debug_prefix_list_match, debug_prefix_list_match_cmd,
+ "debug prefix-list WORD$prefix-list match <A.B.C.D/M|X:X::X:X/M>"
+ " [address-mode$addr_mode]",
+ DEBUG_STR
+ "Prefix-list test access\n"
+ "Name of a prefix list\n"
+ "Test prefix for prefix list result\n"
+ "Prefix to test in ip prefix-list\n"
+ "Prefix to test in ipv6 prefix-list\n"
+ "Use address matching mode (PIM RP)\n")
{
struct prefix_list *plist;
const struct prefix_list_entry *entry = NULL;
@@ -1332,7 +1330,7 @@ DEFPY (debug_prefix_list_match,
if (!entry)
vty_out(vty, "no match found\n");
else {
- vty_out(vty, "matching entry #%"PRId64": %pFX", entry->seq,
+ vty_out(vty, "matching entry #%" PRId64 ": %pFX", entry->seq,
&entry->prefix);
if (entry->ge)
vty_out(vty, " ge %d", entry->ge);
diff --git a/lib/plist.h b/lib/plist.h
index c9507df57..2df909a43 100644
--- a/lib/plist.h
+++ b/lib/plist.h
@@ -72,10 +72,8 @@ extern struct prefix_list *prefix_list_lookup(afi_t, const char *);
extern enum prefix_list_type
prefix_list_apply_ext(struct prefix_list *plist,
const struct prefix_list_entry **matches,
- union prefixconstptr prefix,
- bool address_mode);
-#define prefix_list_apply(A, B) \
- prefix_list_apply_ext((A), NULL, (B), false)
+ union prefixconstptr prefix, bool address_mode);
+#define prefix_list_apply(A, B) prefix_list_apply_ext((A), NULL, (B), false)
extern struct prefix_list *prefix_bgp_orf_lookup(afi_t, const char *);
extern struct stream *prefix_bgp_orf_entry(struct stream *,
diff --git a/pimd/pim_rp.c b/pimd/pim_rp.c
index bda078df8..b78ee7c01 100644
--- a/pimd/pim_rp.c
+++ b/pimd/pim_rp.c
@@ -243,7 +243,8 @@ struct rp_info *pim_rp_find_match_group(struct pim_instance *pim,
plist = prefix_list_lookup(AFI_IP, rp_info->plist);
if (prefix_list_apply_ext(plist, &entry, group, true)
- == PREFIX_DENY || !entry)
+ == PREFIX_DENY
+ || !entry)
continue;
if (!best) {
diff --git a/pimd/pim_sock.c b/pimd/pim_sock.c
index 05b0f92a4..72c699eb9 100644
--- a/pimd/pim_sock.c
+++ b/pimd/pim_sock.c
@@ -117,9 +117,8 @@ int pim_socket_mcast(int protocol, struct in_addr ifaddr, struct interface *ifp,
ret = pim_socket_bind(fd, ifp);
if (ret) {
close(fd);
- zlog_warn(
- "Could not set fd: %d for interface: %s to device",
- fd, ifp->name);
+ zlog_warn("Could not set fd: %d for interface: %s to device",
+ fd, ifp->name);
return PIM_SOCK_ERR_BIND;
}
#else
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 arm8 part 9: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 9: No useful log foundTopotests debian 10 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO9DEB10AMD64-20534/test Topology Tests failed for Topotests debian 10 amd64 part 9: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20534/artifact/TOPO9DEB10AMD64/ErrorLog/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-FRRPULLREQ-TOPO9U18AMD64-20534/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 9: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20534/artifact/TOPO9U18AMD64/ErrorLog/log_topotests.txt Addresssanitizer topotests part 9: Failed (click for details)Addresssanitizer topotests part 9: No useful log foundTopotests Ubuntu 18.04 i386 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO9U18I386-20534/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20534/artifact/TOPO9U18I386/ErrorLog/log_topotests.txt Successful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsTopotests Ubuntu 18.04 arm8 part 9: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 9: No useful log foundTopotests debian 10 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO9DEB10AMD64-20534/test Topology Tests failed for Topotests debian 10 amd64 part 9: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20534/artifact/TOPO9DEB10AMD64/ErrorLog/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-FRRPULLREQ-TOPO9U18AMD64-20534/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 9: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20534/artifact/TOPO9U18AMD64/ErrorLog/log_topotests.txt Addresssanitizer topotests part 9: Failed (click for details)Addresssanitizer topotests part 9: No useful log foundTopotests Ubuntu 18.04 i386 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO9U18I386-20534/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20534/artifact/TOPO9U18I386/ErrorLog/log_topotests.txt |
donaldsharp
requested changes
Jul 27, 2021
|
Disagree. These are fixes. The functionality was there and used to work in 7.4 It was broken since 7.5 |
|
it has new cli this is a forever nack from me |
|
if you need a specific fix break that shit out |
|
Only CLI change is an extra debug command which should (I would say MUST) be allowed for minor changes. The CLI command is required for the unit test. |
eqvinox
force-pushed
the
mergify/bp/stable/8.0/pr-8637
branch
from
1c57ebf to
3f5fe1d
Compare
polychaeta
reviewed
Jul 27, 2021
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Pylint found errors in source files changed by this PR:
Pylint report for my_frr/tests/topotests/lib/topotest.py:
************* Module lib.topotest
my_frr/tests/topotests/lib/topotest.py:1937:15: E0602: Undefined variable 'unicode' (undefined-variable)
-----------------------------------
Your code has been rated at 9.95/10
Pylint report for my_frr/tests/topotests/pim_acl/test_pim_acl.py:
************* Module test_pim_acl
my_frr/tests/topotests/pim_acl/test_pim_acl.py:101:0: C0413: Import "import json" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:102:0: C0413: Import "import functools" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:103:0: C0413: Import "import os" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:104:0: C0413: Import "import sys" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:105:0: C0413: Import "import pytest" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:106:0: C0413: Import "import re" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:107:0: C0413: Import "import time" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:108:0: C0413: Import "from time import sleep" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:109:0: C0413: Import "import socket" should be placed at the top of the module (wrong-import-position)
-----------------------------------
Your code has been rated at 9.46/10
Pylint report for my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:
************* Module test_pim_vrf
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:88:0: C0413: Import "import json" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:89:0: C0413: Import "import functools" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:90:0: C0413: Import "import os" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:91:0: C0413: Import "import sys" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:92:0: C0413: Import "import pytest" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:93:0: C0413: Import "import re" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:94:0: C0413: Import "import time" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:95:0: C0413: Import "from time import sleep" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:96:0: C0413: Import "import socket" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:419:16: E1305: Too many arguments for format string (too-many-format-args)
-----------------------------------
Your code has been rated at 9.28/10
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/ca09e1c1e41ebba83f21be2143318708/raw/9b18476bad4d48013aa6460f32bc61e61fd7f0bd/cr_9186_1627403433.diff | git apply
diff --git a/lib/plist.c b/lib/plist.c
index 1ee855a59..16500ecbe 100644
--- a/lib/plist.c
+++ b/lib/plist.c
@@ -780,11 +780,10 @@ static int prefix_list_entry_match(struct prefix_list_entry *pentry,
return 1;
}
-enum prefix_list_type prefix_list_apply_ext(
- struct prefix_list *plist,
- const struct prefix_list_entry **which,
- union prefixconstptr object,
- bool address_mode)
+enum prefix_list_type
+prefix_list_apply_ext(struct prefix_list *plist,
+ const struct prefix_list_entry **which,
+ union prefixconstptr object, bool address_mode)
{
struct prefix_list_entry *pentry, *pbest = NULL;
diff --git a/lib/plist.h b/lib/plist.h
index c9507df57..2df909a43 100644
--- a/lib/plist.h
+++ b/lib/plist.h
@@ -72,10 +72,8 @@ extern struct prefix_list *prefix_list_lookup(afi_t, const char *);
extern enum prefix_list_type
prefix_list_apply_ext(struct prefix_list *plist,
const struct prefix_list_entry **matches,
- union prefixconstptr prefix,
- bool address_mode);
-#define prefix_list_apply(A, B) \
- prefix_list_apply_ext((A), NULL, (B), false)
+ union prefixconstptr prefix, bool address_mode);
+#define prefix_list_apply(A, B) prefix_list_apply_ext((A), NULL, (B), false)
extern struct prefix_list *prefix_bgp_orf_lookup(afi_t, const char *);
extern struct stream *prefix_bgp_orf_entry(struct stream *,
diff --git a/pimd/pim_rp.c b/pimd/pim_rp.c
index bda078df8..b78ee7c01 100644
--- a/pimd/pim_rp.c
+++ b/pimd/pim_rp.c
@@ -243,7 +243,8 @@ struct rp_info *pim_rp_find_match_group(struct pim_instance *pim,
plist = prefix_list_lookup(AFI_IP, rp_info->plist);
if (prefix_list_apply_ext(plist, &entry, group, true)
- == PREFIX_DENY || !entry)
+ == PREFIX_DENY
+ || !entry)
continue;
if (!best) {
diff --git a/pimd/pim_sock.c b/pimd/pim_sock.c
index 05b0f92a4..72c699eb9 100644
--- a/pimd/pim_sock.c
+++ b/pimd/pim_sock.c
@@ -117,9 +117,8 @@ int pim_socket_mcast(int protocol, struct in_addr ifaddr, struct interface *ifp,
ret = pim_socket_bind(fd, ifp);
if (ret) {
close(fd);
- zlog_warn(
- "Could not set fd: %d for interface: %s to device",
- fd, ifp->name);
+ zlog_warn("Could not set fd: %d for interface: %s to device",
+ fd, ifp->name);
return PIM_SOCK_ERR_BIND;
}
#else
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
|
force-pushed to remove the 3 commits related to the CLI addition |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedAddresssanitizer topotests part 9: Failed (click for details)Addresssanitizer topotests part 9: No useful log foundTopotests debian 10 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO9DEB10AMD64-20580/test Topology Tests failed for Topotests debian 10 amd64 part 9: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20580/artifact/TOPO9DEB10AMD64/ErrorLog/log_topotests.txt Topotests Ubuntu 18.04 arm8 part 9: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 9: No useful log foundTopotests Ubuntu 18.04 i386 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO9U18I386-20580/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20580/artifact/TOPO9U18I386/ErrorLog/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-FRRPULLREQ-TOPO9U18AMD64-20580/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 9: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20580/artifact/TOPO9U18AMD64/ErrorLog/log_topotests.txt Successful on other platforms/tests
|
mwinter-osr
force-pushed
the
mergify/bp/stable/8.0/pr-8637
branch
from
3f5fe1d to
3aefdfa
Compare
polychaeta
reviewed
Jul 29, 2021
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Pylint found errors in source files changed by this PR:
Pylint report for my_frr/tests/topotests/lib/topotest.py:
************* Module lib.topotest
my_frr/tests/topotests/lib/topotest.py:1937:15: E0602: Undefined variable 'unicode' (undefined-variable)
-----------------------------------
Your code has been rated at 9.95/10
Pylint report for my_frr/tests/topotests/pim_acl/test_pim_acl.py:
************* Module test_pim_acl
my_frr/tests/topotests/pim_acl/test_pim_acl.py:101:0: C0413: Import "import json" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:102:0: C0413: Import "import functools" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:103:0: C0413: Import "import os" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:104:0: C0413: Import "import sys" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:105:0: C0413: Import "import pytest" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:106:0: C0413: Import "import re" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:107:0: C0413: Import "import time" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:108:0: C0413: Import "from time import sleep" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:109:0: C0413: Import "import socket" should be placed at the top of the module (wrong-import-position)
-----------------------------------
Your code has been rated at 9.46/10
Pylint report for my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:
************* Module test_pim_vrf
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:88:0: C0413: Import "import json" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:89:0: C0413: Import "import functools" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:90:0: C0413: Import "import os" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:91:0: C0413: Import "import sys" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:92:0: C0413: Import "import pytest" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:93:0: C0413: Import "import re" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:94:0: C0413: Import "import time" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:95:0: C0413: Import "from time import sleep" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:96:0: C0413: Import "import socket" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:419:16: E1305: Too many arguments for format string (too-many-format-args)
-----------------------------------
Your code has been rated at 9.28/10
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/8f658e747d840694dbb3607bb4d5dc0d/raw/9b18476bad4d48013aa6460f32bc61e61fd7f0bd/cr_9186_1627571816.diff | git apply
diff --git a/lib/plist.c b/lib/plist.c
index 1ee855a59..16500ecbe 100644
--- a/lib/plist.c
+++ b/lib/plist.c
@@ -780,11 +780,10 @@ static int prefix_list_entry_match(struct prefix_list_entry *pentry,
return 1;
}
-enum prefix_list_type prefix_list_apply_ext(
- struct prefix_list *plist,
- const struct prefix_list_entry **which,
- union prefixconstptr object,
- bool address_mode)
+enum prefix_list_type
+prefix_list_apply_ext(struct prefix_list *plist,
+ const struct prefix_list_entry **which,
+ union prefixconstptr object, bool address_mode)
{
struct prefix_list_entry *pentry, *pbest = NULL;
diff --git a/lib/plist.h b/lib/plist.h
index c9507df57..2df909a43 100644
--- a/lib/plist.h
+++ b/lib/plist.h
@@ -72,10 +72,8 @@ extern struct prefix_list *prefix_list_lookup(afi_t, const char *);
extern enum prefix_list_type
prefix_list_apply_ext(struct prefix_list *plist,
const struct prefix_list_entry **matches,
- union prefixconstptr prefix,
- bool address_mode);
-#define prefix_list_apply(A, B) \
- prefix_list_apply_ext((A), NULL, (B), false)
+ union prefixconstptr prefix, bool address_mode);
+#define prefix_list_apply(A, B) prefix_list_apply_ext((A), NULL, (B), false)
extern struct prefix_list *prefix_bgp_orf_lookup(afi_t, const char *);
extern struct stream *prefix_bgp_orf_entry(struct stream *,
diff --git a/pimd/pim_rp.c b/pimd/pim_rp.c
index bda078df8..b78ee7c01 100644
--- a/pimd/pim_rp.c
+++ b/pimd/pim_rp.c
@@ -243,7 +243,8 @@ struct rp_info *pim_rp_find_match_group(struct pim_instance *pim,
plist = prefix_list_lookup(AFI_IP, rp_info->plist);
if (prefix_list_apply_ext(plist, &entry, group, true)
- == PREFIX_DENY || !entry)
+ == PREFIX_DENY
+ || !entry)
continue;
if (!best) {
diff --git a/pimd/pim_sock.c b/pimd/pim_sock.c
index 05b0f92a4..72c699eb9 100644
--- a/pimd/pim_sock.c
+++ b/pimd/pim_sock.c
@@ -117,9 +117,8 @@ int pim_socket_mcast(int protocol, struct in_addr ifaddr, struct interface *ifp,
ret = pim_socket_bind(fd, ifp);
if (ret) {
close(fd);
- zlog_warn(
- "Could not set fd: %d for interface: %s to device",
- fd, ifp->name);
+ zlog_warn("Could not set fd: %d for interface: %s to device",
+ fd, ifp->name);
return PIM_SOCK_ERR_BIND;
}
#else
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20637/ This is a comment from an automated CI system. |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 i386 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO9U18I386-21144/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-21144/artifact/TOPO9U18I386/ErrorLog/log_topotests.txt Topotests debian 10 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO9DEB10AMD64-21144/test Topology Tests failed for Topotests debian 10 amd64 part 9: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-21144/artifact/TOPO9DEB10AMD64/ErrorLog/log_topotests.txt Topotests Ubuntu 18.04 arm8 part 9: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 9: No useful log foundTopotests Ubuntu 18.04 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO9U18AMD64-21144/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 9: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-21144/artifact/TOPO9U18AMD64/ErrorLog/log_topotests.txt Addresssanitizer topotests part 9: Failed (click for details)Addresssanitizer topotests part 9: No useful log foundSuccessful on other platforms/tests
|
|
CI:rerun |
mwinter-osr
force-pushed
the
mergify/bp/stable/8.0/pr-8637
branch
from
3f5fe1d to
1e7c70b
Compare
polychaeta
reviewed
Aug 20, 2021
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Pylint found errors in source files changed by this PR:
Pylint report for my_frr/tests/topotests/lib/topotest.py:
************* Module lib.topotest
my_frr/tests/topotests/lib/topotest.py:1937:15: E0602: Undefined variable 'unicode' (undefined-variable)
-----------------------------------
Your code has been rated at 9.95/10
Pylint report for my_frr/tests/topotests/pim_acl/test_pim_acl.py:
************* Module test_pim_acl
my_frr/tests/topotests/pim_acl/test_pim_acl.py:101:0: C0413: Import "import json" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:102:0: C0413: Import "import functools" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:103:0: C0413: Import "import os" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:104:0: C0413: Import "import sys" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:105:0: C0413: Import "import pytest" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:106:0: C0413: Import "import re" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:107:0: C0413: Import "import time" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:108:0: C0413: Import "from time import sleep" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:109:0: C0413: Import "import socket" should be placed at the top of the module (wrong-import-position)
-----------------------------------
Your code has been rated at 9.46/10
Pylint report for my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:
************* Module test_pim_vrf
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:88:0: C0413: Import "import json" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:89:0: C0413: Import "import functools" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:90:0: C0413: Import "import os" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:91:0: C0413: Import "import sys" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:92:0: C0413: Import "import pytest" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:93:0: C0413: Import "import re" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:94:0: C0413: Import "import time" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:95:0: C0413: Import "from time import sleep" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:96:0: C0413: Import "import socket" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:419:16: E1305: Too many arguments for format string (too-many-format-args)
-----------------------------------
Your code has been rated at 9.28/10
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/4852dcbae8e64bd0251771529e17c562/raw/3d1a5a0d408fd55c9bc186eea8f3c0c82c307663/cr_9186_1629501492.diff | git apply
diff --git a/lib/plist.c b/lib/plist.c
index 1ee855a59..16500ecbe 100644
--- a/lib/plist.c
+++ b/lib/plist.c
@@ -780,11 +780,10 @@ static int prefix_list_entry_match(struct prefix_list_entry *pentry,
return 1;
}
-enum prefix_list_type prefix_list_apply_ext(
- struct prefix_list *plist,
- const struct prefix_list_entry **which,
- union prefixconstptr object,
- bool address_mode)
+enum prefix_list_type
+prefix_list_apply_ext(struct prefix_list *plist,
+ const struct prefix_list_entry **which,
+ union prefixconstptr object, bool address_mode)
{
struct prefix_list_entry *pentry, *pbest = NULL;
diff --git a/lib/plist.h b/lib/plist.h
index c9507df57..2df909a43 100644
--- a/lib/plist.h
+++ b/lib/plist.h
@@ -72,10 +72,8 @@ extern struct prefix_list *prefix_list_lookup(afi_t, const char *);
extern enum prefix_list_type
prefix_list_apply_ext(struct prefix_list *plist,
const struct prefix_list_entry **matches,
- union prefixconstptr prefix,
- bool address_mode);
-#define prefix_list_apply(A, B) \
- prefix_list_apply_ext((A), NULL, (B), false)
+ union prefixconstptr prefix, bool address_mode);
+#define prefix_list_apply(A, B) prefix_list_apply_ext((A), NULL, (B), false)
extern struct prefix_list *prefix_bgp_orf_lookup(afi_t, const char *);
extern struct stream *prefix_bgp_orf_entry(struct stream *,
diff --git a/pimd/pim_rp.c b/pimd/pim_rp.c
index 0f3d50f6a..bea84d71c 100644
--- a/pimd/pim_rp.c
+++ b/pimd/pim_rp.c
@@ -244,7 +244,8 @@ struct rp_info *pim_rp_find_match_group(struct pim_instance *pim,
plist = prefix_list_lookup(AFI_IP, rp_info->plist);
if (prefix_list_apply_ext(plist, &entry, group, true)
- == PREFIX_DENY || !entry)
+ == PREFIX_DENY
+ || !entry)
continue;
if (!best) {
diff --git a/pimd/pim_sock.c b/pimd/pim_sock.c
index 05b0f92a4..72c699eb9 100644
--- a/pimd/pim_sock.c
+++ b/pimd/pim_sock.c
@@ -117,9 +117,8 @@ int pim_socket_mcast(int protocol, struct in_addr ifaddr, struct interface *ifp,
ret = pim_socket_bind(fd, ifp);
if (ret) {
close(fd);
- zlog_warn(
- "Could not set fd: %d for interface: %s to device",
- fd, ifp->name);
+ zlog_warn("Could not set fd: %d for interface: %s to device",
+ fd, ifp->name);
return PIM_SOCK_ERR_BIND;
}
#else
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: FailedCheckout code: Failed (click for details)PullReq merge failed. Please rebase your branch: |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: FailedCheckout code: Failed (click for details)PullReq merge failed. Please rebase your branch: |
There's an IGMP socket per interface, so they should be bound to that interface. Which also makes IGMP work in VRFs. Fixes: #7889 Signed-off-by: David Lamparter <[email protected]> (cherry picked from commit f06c6e3)
... the PIM code is kinda misusing prefix lists to match addresses. Considering the weird semantics of access-lists, I can't fault it. However, prefix lists aren't great at matching addresses by default, since they try to match the prefix length too. So, here's an "address match mode" for prefix lists to get that to work more reasonably. Fixes: #8492 Signed-off-by: David Lamparter <[email protected]> (cherry picked from commit 2b6b16f)
Signed-off-by: Martin Winter <[email protected]> (cherry picked from commit 80c5c45)
Signed-off-by: Martin Winter <[email protected]>
Signed-off-by: Martin Winter <[email protected]> (cherry picked from commit fe3c85d)
Signed-off-by: Martin Winter <[email protected]> (cherry picked from commit 5111848)
When we have a "192.0.2.1 peer 192.0.2.2/32" address on an interface, we need to (a) recognize the local address as being on the link for our own packets, and (b) do the IGMP socket lookup with the proper local address rather than the peer prefix. Fixes: efe6f18 ("pimd: fix IGMP receive handling") Cc: Nathan Bahr <[email protected]> Signed-off-by: David Lamparter <[email protected]> (cherry picked from commit a2810d3)
mwinter-osr
force-pushed
the
mergify/bp/stable/8.0/pr-8637
branch
from
1e7c70b to
e01cebf
Compare
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: FailedCheckout code: Failed (click for details)PullReq merge failed. Please rebase your branch: |
polychaeta
reviewed
Aug 21, 2021
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Pylint found errors in source files changed by this PR:
Pylint report for my_frr/tests/topotests/lib/topotest.py:
************* Module lib.topotest
my_frr/tests/topotests/lib/topotest.py:1934:15: E0602: Undefined variable 'unicode' (undefined-variable)
-----------------------------------
Your code has been rated at 9.95/10
Pylint report for my_frr/tests/topotests/pim_acl/test_pim_acl.py:
************* Module test_pim_acl
my_frr/tests/topotests/pim_acl/test_pim_acl.py:101:0: C0413: Import "import json" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:102:0: C0413: Import "import functools" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:103:0: C0413: Import "import os" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:104:0: C0413: Import "import sys" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:105:0: C0413: Import "import pytest" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:106:0: C0413: Import "import re" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:107:0: C0413: Import "import time" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:108:0: C0413: Import "from time import sleep" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_acl/test_pim_acl.py:109:0: C0413: Import "import socket" should be placed at the top of the module (wrong-import-position)
-----------------------------------
Your code has been rated at 9.46/10
Pylint report for my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:
************* Module test_pim_vrf
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:88:0: C0413: Import "import json" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:89:0: C0413: Import "import functools" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:90:0: C0413: Import "import os" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:91:0: C0413: Import "import sys" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:92:0: C0413: Import "import pytest" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:93:0: C0413: Import "import re" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:94:0: C0413: Import "import time" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:95:0: C0413: Import "from time import sleep" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:96:0: C0413: Import "import socket" should be placed at the top of the module (wrong-import-position)
my_frr/tests/topotests/pim_igmp_vrf/test_pim_vrf.py:419:16: E1305: Too many arguments for format string (too-many-format-args)
-----------------------------------
Your code has been rated at 9.28/10
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
|
CI:rerun |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: FailedCheckout code: Failed (click for details)PullReq merge failed. Please rebase your branch: |
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-21265/ This is a comment from an automated CI system. |
|
@donaldsharp |
|
I've had a pretty hard stance on backports must be a clean patch into the system. Why should we change this stance? |
|
It is a clean patch. Just uses a an additional library for topotests which is from a commit which didn't go into 8.0 This specific lib (same as it is in master) is now part of this PR as well. |
|
@donaldsharp sorry, I probably contributed even more confusion. 1771900 is the commit on master that adds the file ( |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an automatic backport of pull request #8637 done by Mergify.
Mergify commands and options
More conditions and actions can be found in the documentation.
You can also trigger Mergify actions by commenting on this pull request:
@Mergifyio refreshwill re-evaluate the rules@Mergifyio rebasewill rebase this PR on its base branch@Mergifyio updatewill merge the base branch into this PR@Mergifyio backport <destination>will backport this PR on<destination>branchAdditionally, on Mergify dashboard you can:
Finally, you can contact us on https://mergify.io/