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

net/iproute2: delete IPv6 tunnel. #26

Merged
merged 1 commit into from Oct 21, 2017
Merged

net/iproute2: delete IPv6 tunnel. #26

merged 1 commit into from Oct 21, 2017

Conversation

stkchp
Copy link
Contributor

@stkchp stkchp commented Oct 17, 2017

ip6ip6/ipip6 tunnel is not shown command ip tunnel show ${IFACE}. should add option -6,

@robbat2
Copy link
Contributor

robbat2 commented Oct 17, 2017

This should be 2 changes at least: ip6tnl0, and then the -6 fallback. I don't like the -6 fallback however, can we detect what type it is so we know up front to use -6 or not?

@stkchp
Copy link
Contributor Author

stkchp commented Oct 17, 2017

ip6tnl0

When ip6_tunnel module loaded, kernel create undeletable device ip6tnl0.
This behavior same as relation between sit module and sit0 device. But if unneeded, I'll remove it.

-6 fallback:

Do you mean this code?

local ipproto=''
[ -n "$(ip -4 tunnel show "${IFACE}" 2>/dev/null)" ] && ipproto='-4'
[ -n "$(ip -6 tunnel show "${IFACE}" 2>/dev/null)" ] && ipproto='-6'
if [ -n "$ipproto" ]; then
	ebegin "Destroying tunnel ${IFACE}"
	veinfo ip $ipproto tunnel del "${IFACE}"
	ip $ipproto tunnel del "${IFACE}"
	eend $?
fi
	

I'll fix it if it's correct.

@robbat2
Copy link
Contributor

robbat2 commented Oct 17, 2017

ip6tnl0: just put it in a separate logical commit

That fallback is pretty good, are there any cases where we'd have to omit -4 and -6 entirely? Maybe drop just if conditional.

Additional idea:
if we set ipproto='-4', we don't need to test for -6?

local ipproto=''
if [ -n "$(ip -4 tunnel show "${IFACE}" 2>/dev/null)" ]; then
  ipproto='-4'
elif  [ -n "$(ip -6 tunnel show "${IFACE}" 2>/dev/null)" ]; then
  ipproto='-6'
fi
ebegin "Destroying tunnel ${IFACE}"
veinfo ip $ipproto tunnel del "${IFACE}"
ip $ipproto tunnel del "${IFACE}"
eend $?
unset ipproto

@stkchp
Copy link
Contributor Author

stkchp commented Oct 18, 2017

I fixed it.

That fallback is pretty good, are there any cases where we'd have to omit -4 and -6 entirely?

No. ip tunnel delete equal ip -4 tunnel delete.
There is a possibility that the tunnel does not exist, and an if [ -n "${ipproto}" ] conditional is added.

@robbat2
Copy link
Contributor

robbat2 commented Oct 19, 2017

Right now the iproute2 codebase the codebase assumes -4 (rather AF_INET) if unspecified, but past development has shown other variations do happen over time. It's reasonable to think there might be ip -f inet_future tunnel or something similar.

If the tunnel doesn't exist, do we lose anything by letting the user try to delete it anyway? Race conditions where it is deleted already would be similar.

As for this PR, please rebase to remove the revert. It should be just 2 commits: ip6tnl0 and the tunnel deletion.

@stkchp
Copy link
Contributor Author

stkchp commented Oct 19, 2017

OK, I tried to fix it!

If the tunnel doesn't exist, do we lose anything by letting the user try to delete it anyway? Race conditions where it is deleted already would be similar.

We will not lose anything. But if the device to be stopped is not tunnel but gretap or vxlan, it will fail with tunnel deletion? There is a code deleting link just below.

It's better not to add option -4. I create _iproute2_tunnel_delete function like link deletion.

I decided this PR not to include ip6tnl0 is handled as a special device.
ip6tnl0 device will fail with not only stop but also start.
If we do, another PR seems to be good.

@robbat2 robbat2 merged commit db06d25 into gentoo:master Oct 21, 2017
gentoo-bot pushed a commit to gentoo/gentoo that referenced this pull request Nov 27, 2017
New functionality & improvements:
- Wireless: 'iw' module to replace older 'iwconfig' module.
  (Brian Evans <grknight@gentoo.org>)
- iproute2: VXLAN & GRETAP support (iplink_$IFVAR)
  (Sergey Popov <pinkbyte@gentoo.org>)
- Bonding: ARP IP targets (Marc Schiffbauer <mschiff@gentoo.org>)
- wpa_supplicant: better matching of wired connections
  (Henning Schild <henning@hennsch.de>)
- IPv6: clearer message for Tentative duplicate address detection (DAD).
- Refactor veinfo printing of iproute2 commands.

Fixes:
- Avoid moduleslist race condition (Hagbard Celine <hagbardcelin@gmail.com>)
- Delete IPv6 tunnel correctly (stkchp <s@tkch.net>)

Bug: https://bugs.gentoo.org/638836
Bug: https://bugs.gentoo.org/637474
Bug: https://bugs.gentoo.org/636846
Bug: gentoo/netifrc#24
Bug: gentoo/netifrc#26
Bug: gentoo/netifrc#25
Bug: gentoo/netifrc#27
Package-Manager: Portage-2.3.16, Repoman-2.3.6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants