Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

all: add support for thunderbolt networking #97

Merged
merged 1 commit into from Apr 19, 2018

Conversation

gicmo
Copy link
Contributor

@gicmo gicmo commented Apr 18, 2018

First attempt to implement thunderbolt networking. Comments welcome.

Load the thunderbolt-net module if we see a host-to-host connection
and configure the resulting ethernet connection automatically to be
a link-local only one.

@vhumpa
Copy link
Collaborator

vhumpa commented Apr 18, 2018

NM CI: Please confirm test execution over this PR. Comment .ok\W+to\W+test. to confirm and/or .add\W+to\W+whitelist. to add the PR author in whitelist for future.

Copy link
Member

@thom311 thom311 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good to me. Simpler than expected.

# For all thunderbolt network devices, we want to enable link-local configuration
SUBSYSTEM=="net", ENV{ID_NET_DRIVER}=="thunderbolt-net", ENV{NM_AUTO_DEFAULT_LINK_ONLY}="1"

LABEL="nm_thunderbolt_end"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the file must also be marked as EXTRA_DIST in Makefile.am and marked for installation. Similar, in data/meson.build.

@@ -1481,6 +1484,26 @@ new_default_connection (NMDevice *self)
g_object_set (setting, NM_SETTING_WIRED_MAC_ADDRESS, perm_hw_addr, NULL);
nm_connection_add_setting (connection, setting);

/* Check if we should create a Link-Local only connection */
dev = nm_platform_link_get_udev_device (nm_device_get_platform (NM_DEVICE (self)), nm_device_get_ifindex (self));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be nm_device_get_ip_ifindex (self).


setting = nm_setting_ip6_config_new ();
g_object_set (setting,
NM_SETTING_IP_CONFIG_METHOD, NM_SETTING_IP6_CONFIG_METHOD_LINK_LOCAL,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indentation should be with spaces.

@thom311
Copy link
Member

thom311 commented Apr 19, 2018

ah, maybe NM_AUTO_DEFAULT_LINK_ONLY should be called NM_AUTO_DEFAULT_LINK_LOCAL_ONLY.

@bengal
Copy link
Contributor

bengal commented Apr 19, 2018

ah, maybe NM_AUTO_DEFAULT_LINK_ONLY should be called NM_AUTO_DEFAULT_LINK_LOCAL_ONLY.

+1

Looks good to me, with changes suggested by Thomas.

@gicmo gicmo force-pushed the thunderbolt branch 2 times, most recently from 3f7a556 to 2ba1d9b Compare April 19, 2018 12:06
@gicmo gicmo changed the title [WIP] all: add support for thunderbolt networking all: add support for thunderbolt networking Apr 19, 2018
@gicmo
Copy link
Contributor Author

gicmo commented Apr 19, 2018

I hope I got the whitespaces correct. I tested it locally with F27 <-> Windows and successfully got a link-local address that was ping-able from windows.

Load the thunderbolt-net module if we see a host-to-host connection
and configure the resulting ethernet connection automatically to be
a link-local only one. The latter is done by setting a new udev
property "NM_AUTO_DEFAULT_LINK_LOCAL_ONLY" which is picked up when
we configure the connection for the device.

NetworkManager#97
@thom311 thom311 merged commit 89af7fb into NetworkManager:master Apr 19, 2018
@gicmo gicmo deleted the thunderbolt branch April 19, 2018 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants