Page MenuHomeFreeBSD
Differential D18769

Fix broken IPv6 scope ID checks in outgoing direction
ClosedPublic
Actions

Authored by • hselasky on Jan 7 2019, 12:33 PM.
Tags
None
Referenced Files
Unknown Object (File)
Jan 12 2024, 8:54 AM
Unknown Object (File)
Dec 20 2023, 3:32 AM
Unknown Object (File)
Dec 10 2023, 9:29 PM
Unknown Object (File)
Dec 3 2023, 7:38 AM
Unknown Object (File)
Nov 30 2023, 3:07 PM
Unknown Object (File)
Nov 9 2023, 10:37 PM
Unknown Object (File)
Nov 9 2023, 10:23 PM
Unknown Object (File)
Nov 6 2023, 11:04 AM
View All 24 Files
Subscribers
ae
bz
imp
kib
melifaro
menyy_mellanox.com
slavash

Details

Reviewers
bz
Group Reviewers
network
Commits
rS343139: MFC r342884:
rS343138: MFC r342884:
rS343137: MFC r342884:
rS342884: Fix loopback traffic when using non-lo0 link local IPv6 addresses.
Summary

Fix loopback traffic on link local IPv6 addresses.

The problem is the loopback interface can only receive packets with a single scope ID, namely the scope ID of the loopback interface. To mitigate this packets which use the scope ID are appearing as received by the real network interface, see "origifp" in the patch. The current code would drop packets which are designated for loopback which use a link-local scope ID in the destination address or source address, because they won't match the lo0's scope ID. To fix this restore the network interface pointer from the scope ID in the destination address for the problematic cases. See comments added in patch for a more detailed description.

MFC after: 1 week
Sponsored by: Mellanox Technologies

Test Plan

Run iperf on IPv6 link local addresses and for VLAN ones too.

iperf -s -V
iperf -V -c xxxxx%iface

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

• hselasky created this revision.Jan 7 2019, 12:33 PM
Herald added subscribers: ae, imp. · View Herald TranscriptJan 7 2019, 12:33 PM
Harbormaster completed remote builds in B21858: Diff 52622.Jan 7 2019, 12:33 PM
bz added a subscriber: bz.Jan 7 2019, 1:53 PM

Can you please fold some of the problem description of "why?" this change is needed in the proposed commit message; having some more information available when scrolling through source code management system logs is extremely helpful.

sys/netinet6/ip6_output.c
575 ↗(On Diff #52622)

Comments start upper case and end in punctuation; this can also be folded into a single line now?

590 ↗(On Diff #52622)

Check ... ID.

638 ↗(On Diff #52622)

So this is an empty block? I am also slightly confused by the "code above/below".
I think doing some of the "common code" upfront with error handing such as sa6_recoverscope() would make the if() blocks easier to read; you could then negate this block and make it the "else" case? Might also help putting comments in better places?

• hselasky added a reviewer: bz.Jan 7 2019, 2:07 PM
• hselasky marked 3 inline comments as done.Jan 7 2019, 2:26 PM
• hselasky added inline comments.
sys/netinet6/ip6_output.c
638 ↗(On Diff #52622)

I'll invert the block.

• hselasky updated this revision to Diff 52626.Jan 7 2019, 2:34 PM
• hselasky marked an inline comment as done.
• hselasky edited the summary of this revision. (Show Details)

Address comments by bz@ .

Harbormaster completed remote builds in B21861: Diff 52626.Jan 7 2019, 2:34 PM
bz accepted this revision.Jan 7 2019, 3:52 PM

I like this version better than the one before. Thanks! For as much as I can say it looks OK. I haven't tested it.

This revision is now accepted and ready to land.Jan 7 2019, 3:52 PM
ae added a comment.Jan 7 2019, 6:14 PM

The current code would drop packets which are designated for loopback which use a link-local scope ID in the destination address or source address, because they won't match the lo0's scope ID.

So, why you think that this is wrong and shouldn't happen?

melifaro added a subscriber: melifaro.Jan 7 2019, 6:28 PM

Would it be possible to include firewalls in the test plan?
For example, ensure that ipfw still accepts the packet using something like 'allow ip6 from any to any via lo0' rule.

• hselasky added a comment.Jan 9 2019, 10:43 AM
In D18769#400616, @ae wrote:

The current code would drop packets which are designated for loopback which use a link-local scope ID in the destination address or source address, because they won't match the lo0's scope ID.

So, why you think that this is wrong and shouldn't happen?

Because then TCP loopback on non-lo0 link-local addresses won't work.

• hselasky added a comment.Jan 9 2019, 10:45 AM
In D18769#400622, @melifaro wrote:

Would it be possible to include firewalls in the test plan?
For example, ensure that ipfw still accepts the packet using something like 'allow ip6 from any to any via lo0' rule.

This patch doesn't affect ::1 traffic.

Can you suggest kernel options of IPFW and a test script?

ae added a comment.Jan 9 2019, 11:03 AM
In D18769#400967, @hselasky wrote:

So, why you think that this is wrong and shouldn't happen?

Because then TCP loopback on non-lo0 link-local addresses won't work.

Can you show an example? I just tested and it works:

% nc -6 -l fe80::e6a7:a0ff:fe8e:16bf%lagg0 1234
TEST
% nc -6 fe80::e6a7:a0ff:fe8e:16bf%lagg0 1234
TEST
• hselasky added a comment.Jan 9 2019, 11:16 AM
In D18769#400969, @ae wrote:
In D18769#400967, @hselasky wrote:

So, why you think that this is wrong and shouldn't happen?

Because then TCP loopback on non-lo0 link-local addresses won't work.

Can you show an example? I just tested and it works:

% nc -6 -l fe80::e6a7:a0ff:fe8e:16bf%lagg0 1234
TEST
% nc -6 fe80::e6a7:a0ff:fe8e:16bf%lagg0 1234
TEST

You need to send traffic in both directions and more than one TCP packet.

Try this:

iperf -s -V
iperf -V -c fe80::e6a7:a0ff:fe8e:16bf%lagg0 -t 16 -i 1

ae added a comment.Jan 9 2019, 12:45 PM

Ah, yes, I remembered, this problem was introduced with route caching.

Closed by commit rS342884: Fix loopback traffic when using non-lo0 link local IPv6 addresses. (authored by • hselasky). · Explain WhyJan 9 2019, 2:28 PM
This revision was automatically updated to reflect the committed changes.
melifaro mentioned this in D35117: netinet6: streamline scope6 checks for loopback traffic in ip6_output()..May 3 2022, 2:45 PM

Revision Contents
Changeset List

PathSize
head/
sys/
netinet6/
80 lines

Diff 52694

View Options

head/sys/netinet6/ip6_output.c

Loading...