network: introduce multipath routing #14194

yuwata · 2019-11-28T16:48:41Z

Closes #12541.
Replaces #14063.

cc @ssahani and @n0rad.

ssahani · 2019-11-29T10:59:07Z

Thanks for working on it. Let's review this after 244. I am working on openvswitch integration.

yuwata · 2019-12-06T14:21:25Z

@ssahani and @keszybz PTAL.

ssahani · 2019-12-07T06:39:11Z

Look almost good . CI is failing.

======================================================================
FAIL: test_route_static (__main__.NetworkdNetworkTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "./test/test-network/systemd-networkd-tests.py", line 1746, in test_route_static
    self.assertRegex(output, '2001:1234:5:7fff:ff:ff:ff:ff proto static metric 1024')
AssertionError: Regex didn't match: '2001:1234:5:7fff:ff:ff:ff:ff proto static metric 1024' not found in '2001:1234:5:7fff:ff:ff:ff:ff via 2001:1234:5:8fff:ff:ff:ff:ff dev dummy98 proto static metric 1024 pref medium\n2001:1234:5:7fff:ff:ff:ff:ff via 2001:1234:5:9fff:ff:ff:ff:ff dev dummy98 proto static metric 1024 pref medium'

----------------------------------------------------------------------
Ran 111 tests in 2201.561s

FAILED (failures=1, expected failures=13)
[TASK END] Fri  6 Dec 17:09:28 CET 2019
[RESULT] systemd-networkd-tests.py - FAIL (EC: 1) (log file: /root/testsuite-logs-upstream.rnA/systemd-networkd-tests.py_FAIL.log)

[TASK] coredumpctl_collect
Waiting for PID 567553 to finish

man/systemd.network.xml

src/network/networkd-route.c

yuwata · 2019-12-07T14:34:56Z

@ssahani Thank you for the review. I addressed your comments except for the one about the debug log. And now I added a test for the case that IPv4 destination with IPv6 gateways (RTA_VIA). PTAL.

ssahani · 2019-12-07T14:38:31Z

LGTM

man/systemd.network.xml

src/network/networkd-route.c

yuwata · 2019-12-09T17:29:10Z

@poettering Thank you for the review. I've force-pushed a revised version. Now I hope all your comments are addressed. PTAL.

mrc0mmand · 2019-12-09T21:24:45Z

Almost there:

Dec 09 22:01:03 arch.localdomain systemd-networkd[30848]: ==30848==ERROR: AddressSanitizer: attempting free on address which was not malloc()-ed: 0x7ffd655758c0 in thread T0
Dec 09 22:01:03 arch.localdomain systemd-udevd[30850]: Using default interface naming scheme 'v243'.
Dec 09 22:01:03 arch.localdomain systemd-networkd[30848]:     #0 0x7fe2680917c9 in free (/usr/lib/clang/9.0.0/lib/linux/libclang_rt.asan-x86_64.so+0xeb7c9)
Dec 09 22:01:03 arch.localdomain systemd-networkd[30848]:     #1 0x55bc7f4b25a5 in freep /build/build/../src/basic/alloc-util.h:83:9
Dec 09 22:01:03 arch.localdomain systemd-networkd[30848]:     #2 0x55bc7f4d00d9 in config_parse_multipath_route /build/build/../src/network/networkd-route.c:1676:1
Dec 09 22:01:03 arch.localdomain systemd-networkd[30848]:     #3 0x7fe26722c889 in next_assignment /build/build/../src/shared/conf-parser.c:132:32
Dec 09 22:01:03 arch.localdomain systemd-networkd[30848]:     #4 0x7fe267220b8f in parse_line /build/build/../src/shared/conf-parser.c:270:16
Dec 09 22:01:03 arch.localdomain systemd-networkd[30848]:     #5 0x7fe26721df4e in config_parse /build/build/../src/shared/conf-parser.c:391:21
Dec 09 22:01:03 arch.localdomain systemd-networkd[30848]:     #6 0x7fe26722109f in config_parse_many_files /build/build/../src/shared/conf-parser.c:448:21
Dec 09 22:01:03 arch.localdomain systemd-networkd[30848]:     #7 0x7fe267221e7e in config_parse_many /build/build/../src/shared/conf-parser.c:507:16
Dec 09 22:01:03 arch.localdomain systemd-networkd[30848]:     #8 0x55bc7f4881b6 in network_load_one /build/build/../src/network/networkd-network.c:461:13
Dec 09 22:01:03 arch.localdomain systemd-networkd[30848]:     #9 0x55bc7f48976d in network_load /build/build/../src/network/networkd-network.c:536:21
Dec 09 22:01:03 arch.localdomain systemd-networkd[30848]:     #10 0x55bc7f460574 in manager_load_config /build/build/../src/network/networkd-manager.c:1870:13
Dec 09 22:01:03 arch.localdomain systemd-networkd[30848]:     #11 0x55bc7f430959 in run /build/build/../src/network/networkd.c:87:13
Dec 09 22:01:03 arch.localdomain systemd-networkd[30848]:     #12 0x55bc7f42f04d in main /build/build/../src/network/networkd.c:130:1
Dec 09 22:01:03 arch.localdomain systemd-networkd[30848]:     #13 0x7fe266a1b152 in __libc_start_main (/lib64/libc.so.6+0x27152)
Dec 09 22:01:03 arch.localdomain systemd-networkd[30848]:     #14 0x55bc7f42ef4d in _start (/build/build/systemd-networkd+0x255f4d)
Dec 09 22:01:03 arch.localdomain systemd-networkd[30848]: Address 0x7ffd655758c0 is located in stack of thread T0
Dec 09 22:01:03 arch.localdomain systemd-networkd[30848]: SUMMARY: AddressSanitizer: bad-free (/usr/lib/clang/9.0.0/lib/linux/libclang_rt.asan-x86_64.so+0xeb7c9) in free

Possibly relevant full coredump from CentOS 7: https://ci.centos.org/job/systemd-pr-build/9232/artifact//systemd-centos-ci/artifacts_I6h7a0/testsuite-logs-upstream.hSQ/coredumpctl_collect_FAIL.log

yuwata · 2019-12-10T01:59:46Z

@mrc0mmand Thank you for catching the issue. It is now fixed. PTAL.

It will be used in later commit.

Closes systemd#12541.

poettering · 2020-01-03T14:35:08Z

src/libsystemd/sd-netlink/netlink-util.c

+
+void rtattr_append_attribute_internal(struct rtattr *rta, unsigned short type, const void *data, size_t data_length) {
+        size_t padding_length;
+        char *padding;


doesn't matter much, but to me "char" indicates this was actually character data... I'd always use "uint8_t" for this, since these are "arbitrary padding bytes"

poettering · 2020-01-03T14:37:57Z

CI issue appears unrelated. Merging.

aep · 2023-11-06T16:56:46Z

bit late maybe but its not entirely clear how this is supposed to work at all.
a [Route] is attached to a network in systemd-networkd, so its removed if one link in an ecmp goes down.

That's not.. "multi" path?

the test also indicates this only works when both routes are on the same link, but whats the point of that

https://github.com/systemd/systemd/pull/14194/files#diff-61992e6b0355a76830b1a4fa191babb3532354a3f625edf802ff20acb1adf23cR82

yuwata · 2023-11-06T18:09:22Z

@aep

the test also indicates this only works when both routes are on the same link, but whats the point of that

The setting can take another interface name. Though, actually it is not tested.

a [Route] is attached to a network in systemd-networkd, so its removed if one link in an ecmp goes down.

Could you open a new issue page with more details?

Prompted by systemd#14194 (comment).

Prompted by #14194 (comment).

Prompted by systemd#14194 (comment).

yuwata added network new-feature labels Nov 28, 2019

yuwata force-pushed the network-multipath-routing-12541 branch from 507516b to 1ad1351 Compare November 28, 2019 17:07

yuwata added this to the v245 milestone Nov 28, 2019

yuwata force-pushed the network-multipath-routing-12541 branch 3 times, most recently from 3fba607 to beb2d2d Compare December 6, 2019 14:20

ssahani reviewed Dec 7, 2019

View reviewed changes

man/systemd.network.xml Outdated Show resolved Hide resolved

ssahani reviewed Dec 7, 2019

View reviewed changes

src/network/networkd-route.c Outdated Show resolved Hide resolved

ssahani reviewed Dec 7, 2019

View reviewed changes

src/network/networkd-route.c Show resolved Hide resolved

yuwata force-pushed the network-multipath-routing-12541 branch from beb2d2d to 5716937 Compare December 7, 2019 14:18

yuwata added the good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed label Dec 7, 2019

yuwata force-pushed the network-multipath-routing-12541 branch 2 times, most recently from 9bb06b9 to a2e3092 Compare December 7, 2019 20:59

poettering reviewed Dec 9, 2019

View reviewed changes

man/systemd.network.xml Outdated Show resolved Hide resolved

poettering reviewed Dec 9, 2019

View reviewed changes

src/network/networkd-route.c Show resolved Hide resolved

poettering reviewed Dec 9, 2019

View reviewed changes

src/network/networkd-route.c Outdated Show resolved Hide resolved

yuwata added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed labels Dec 9, 2019

yuwata force-pushed the network-multipath-routing-12541 branch from a2e3092 to 198fa4f Compare December 9, 2019 17:23

yuwata removed the reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks label Dec 9, 2019

yuwata force-pushed the network-multipath-routing-12541 branch from 198fa4f to cbd21a7 Compare December 9, 2019 17:29

mrc0mmand added the ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR label Dec 9, 2019

yuwata force-pushed the network-multipath-routing-12541 branch from cbd21a7 to e3d8dcd Compare December 10, 2019 01:58

yuwata removed the ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR label Dec 10, 2019

yuwata added 3 commits December 18, 2019 22:12

sd-netlink: introduce rtattr_append_attribute()

6497a8a

It will be used in later commit.

network: introduce multipath route

6ff5cc6

Closes systemd#12541.

test-network: add test case for multipath routing

a0ce990

yuwata force-pushed the network-multipath-routing-12541 branch from e3d8dcd to a0ce990 Compare December 18, 2019 13:13

poettering reviewed Jan 3, 2020

View reviewed changes

poettering merged commit dc57374 into systemd:master Jan 3, 2020

yuwata deleted the network-multipath-routing-12541 branch January 3, 2020 16:35

yuwata mentioned this pull request Jan 6, 2020

sd-netlink: use uint8_t* for non-character data #14480

Merged

yuwata added a commit to yuwata/systemd that referenced this pull request Nov 6, 2023

test-network: test MultiPathRoute= via another interface

f160ace

Prompted by systemd#14194 (comment).

yuwata mentioned this pull request Nov 6, 2023

test-network: test MultiPathRoute= via another interface #29891

Merged

bluca pushed a commit that referenced this pull request Nov 6, 2023

test-network: test MultiPathRoute= via another interface

d377949

Prompted by #14194 (comment).

lnykryn pushed a commit to lnykryn/systemd that referenced this pull request Nov 8, 2023

test-network: test MultiPathRoute= via another interface

40781e8

Prompted by systemd#14194 (comment).

ssahani pushed a commit to ssahani/systemd that referenced this pull request Nov 23, 2023

test-network: test MultiPathRoute= via another interface

3c2c033

Prompted by systemd#14194 (comment).

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

network: introduce multipath routing #14194

network: introduce multipath routing #14194

yuwata commented Nov 28, 2019

ssahani commented Nov 29, 2019

yuwata commented Dec 6, 2019

ssahani commented Dec 7, 2019

yuwata commented Dec 7, 2019 •

edited

Loading

ssahani commented Dec 7, 2019

yuwata commented Dec 9, 2019

mrc0mmand commented Dec 9, 2019

yuwata commented Dec 10, 2019

poettering Jan 3, 2020

poettering commented Jan 3, 2020

aep commented Nov 6, 2023

yuwata commented Nov 6, 2023

network: introduce multipath routing #14194

network: introduce multipath routing #14194

Conversation

yuwata commented Nov 28, 2019

ssahani commented Nov 29, 2019

yuwata commented Dec 6, 2019

ssahani commented Dec 7, 2019

yuwata commented Dec 7, 2019 • edited Loading

ssahani commented Dec 7, 2019

yuwata commented Dec 9, 2019

mrc0mmand commented Dec 9, 2019

yuwata commented Dec 10, 2019

poettering Jan 3, 2020

Choose a reason for hiding this comment

poettering commented Jan 3, 2020

aep commented Nov 6, 2023

yuwata commented Nov 6, 2023

yuwata commented Dec 7, 2019 •

edited

Loading