[Linux-ha-dev] [resource-agents] [RFC] IPaddr2: Proposal patch to support the dual stack of IPv4 and IPv6. (#97)

Keisuke MORI

2012-10-18 09:56:11 UTC

Hi Lars,

Thank you for your comments,
I'm going to answer to your comments below,
and if you have further comments I would greatly appreciate it.

Post by Lars Ellenberg
Again, apollogies for not having this sent out when I wrote it,
I'm unsure why it "hibernated" in my Draft folder for five month,
but it was not the only one :(
I realize the pull request has meanwhile been closed,
and we do have a findif.sh implementation in current git.
Still I'll just send these comments as I wrote them back then,
maybe some of them still apply.
At the end, there is a bit of comment how to maybe re-implement
the ipcheck and ifcheck functions without grep and awk.
Feel free to ignore for now, though.
I'll try to review again whats in git now,
and send a proper git diff, once I find the time
;-)

This is a proposal enhancement of IPaddr2 to support IPv6 as well as IPv4.
I would appreciate your comments and suggestions for merging this into
the upstream.
NOTE: This pull request is meant for reviewing the code and
discussions, and not intended to be merged as is at this moment.

Github pull request comments are IMO not the best place to discuss these
things, so I send to the linux-ha-dev mailing list as well.

* Unify the usage, behavior and the code maintenance between IPv4 and IPv6 on Linux.
The usage of IPaddr2 and IPv6addr are similar but they have
different parameters and different behaviors.$B!!(BIn particular, they
may choose a different interface depending on your configuration
even if you provided similar parameters in the past.
IPv6addr is written in C and rather hard to make improvements. As
/bin/ip already supports both IPv4 and IPv6, we can share the most
of the code of IPaddr2 written in bash.

IPv6addr is supposed to run on non-Linux as well.
So we better not deprecate it, as long as all the world is not Linux.

Agreed.

Post by Lars Ellenberg

* usable for LVS on IPv6.
IPv6addr does not support lvs_support=true and unfortunately there
is no possible way to use LVS on IPv6 right now.
IPaddr2(/bin/ip) works for LVS configurations without enabling
lvs_support both for IPv4 and IPv6.
(You don't have to remove an address on the loopback interface if
the virtual address is assigned by using /bin/ip.)
http://www.gossamer-threads.com/lists/linuxha/dev/76429#76429
* retire the old 'findif' binary.
'findif' binary is replaced by a shell script version of findif,
findif could be rewritten in shell
* easier support for other pending issues
These pending issues can be fix based on this new IPaddr2. *
ClusterLabs/resource-agents#68 : Allow ipv6addr to mark new address
as deprecated * ClusterLabs/resource-agents#77 : New RA that
controls IPv6 address in loopback interface
* findif semantics changes
There are some incompatibility in deciding which interface to be
used when your configuration is ambiguous. But in reality it should
not be a problem as long as it's configured properly.
The changes mostly came from fixing a bug in the findif binary
(returns a wrong broadcast) or merging the difference between
(old)IPaddr2 and IPv6addr.$B!!(B See the ofct test cases for details.
(case No.6, No.9, No.10, No.12, No.15 in IPaddr2v4 test cases)
Other notable changes are described below.
* "broadcast" parameter for IPv4
"broadcast" parameter may be required along with "cidr_netmask" when
you want use a different subnet mask from the static IP address.
It's because doing such calculation is difficult in the shell script
version of findif.
See the ofct test cases for details. (case No.11, No.14, No.16,
No.17 in IPaddr2v4 test cases)
This limitation may be eliminated if we would remove brd options
from the /bin/ip command line.

If we do not specify the broadcast at all, ip will do the "right thing"
by default, I think. We should only use it on the ip command line, if
it is in the input parameters.
I don't really have a use case for the broadcast address to not be the
"default", so I would be ok with dropping it completely.

It has been fixed and the latest code in the repo now should work like you said.

Post by Lars Ellenberg

* loopback(lo) now requires cidr_netmask or broadcast.
See the ofct test case in the IPaddr2 ocft script. The reason is
similar to the previous one.

We really need to avoid breaking existing configurations.
So we need to fix this.
If we find nothing better, with some heuristic.

It has also been fixed now and loopback can be used as same as before.

Post by Lars Ellenberg

* loose error check for "nic" for a IPv6 link-local address.
IPv6addr was able to check this, but in the shell script it is hard
to determine a link-local address (requires bitmask calculation). I
do not think it's worth to implement it in shell.

There may even be use cases for link-local addresses.
So maybe that is a feature, not a bug ;-)

This check is done by simply matching fe80:: prefix as Alan's suggestion
and I think it is just enough.

Post by Lars Ellenberg
We could always have a few helpers in C, still.
Or see if we can use existing helpers, if present at runtime.
(ipcalc, sipcalc, there may be more).

* send_ua: a new binary
We need one new binary as a replacement of send_arp for IPv6
support. IPv6addr.c is reused to make this command.
Note that IPv6addr RA is still there and you can continue to use it
for the backward compatibility.
## Acknowledgement
Thanks to Tomo Nozawa-san for his hard work for writing and testing
this patch.
Thanks to Lars Ellenberg for the first findif.sh implementation.
git pull https://github.com/kskmori/resource-agents
IPaddr2-dualstack-devel
https://github.com/ClusterLabs/resource-agents/pull/97
-- Commit Summary --
* [RFC] IPaddr2: Proposal patch to support the dual stack of IPv4 and
IPv6.
-- File Changes --
M heartbeat/IPaddr2 (59) M heartbeat/IPv6addr.c (89) M
heartbeat/Makefile.am (9) A heartbeat/findif.sh (184) M
tools/ocft/IPaddr2 (24) A tools/ocft/IPaddr2v4 (315) A
tools/ocft/IPaddr2v6 (257) M tools/ocft/Makefile.am (2)
-- Patch Links --
https://github.com/ClusterLabs/resource-agents/pull/97.patch

diff --git a/heartbeat/findif.sh b/heartbeat/findif.sh
new file mode 100644
index 0000000..6a2807c
--- /dev/null
+++ b/heartbeat/findif.sh
@@ -0,0 +1,184 @@
+#!/bin/sh
+ipcheck_ipv4() {
local r1_to_255="([1-9][0-9]?|1[0-9][0-9]|2[0-4][0-9]|25[0-5])"
local r0_to_255="([0-9][0-9]?|1[0-9][0-9]|2[0-4][0-9]|25[0-5])"
local r_ipv4="^$r1_to_255\.$r0_to_255\.$r0_to_255\.$r0_to_255$"
echo "$1" | grep -q -Ee "$r_ipv4"
}
+ipcheck_ipv6() {
! echo "$1" | grep -qs "[^0-9:a-fA-F]"
}
+ifcheck_ipv4() {
+ local ifcheck=$1
+ local ifstr
+ local counter=0
+ local procfile="/proc/net/dev"
+ while read LINE
+ do
+ if [ $counter -ge 2 ] ; then
+ ifstr=`echo $LINE | cut -d ':' -f 1`
+ if [ "$ifstr" = "$ifcheck" ] ; then
+ return 0
+ fi
+ fi
+ counter=`expr $counter + 1`
+ done < $procfile
local ifstr rest
while read ifstr rest ; do
[ "$ifstr" = "$ifcheck:" ] && return 0
done
return 1
+ return 1
+}
+ifcheck_ipv6() {
+ local ifcheck="$1"
+ local ifstr
+ local procfile="/proc/net/if_inet6"
+ while read LINE
+ do
+ ifstr=`echo $LINE | awk -F ' ' '{print $6}'`
+ if [ "$ifstr" = "$ifcheck" ] ; then
+ return 0
+ fi
+ done < $procfile
# btw, in bash, I tend to use _ as dummy
# much more readable
local tmp ifstr
while read tmp tmp tmp tmp tmp ifstr ; do
[ "$ifstr" = "$ifcheck" ] && return 0
done
+ return 1
+}
_______________________________________________________
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/

Regards,

--
Keisuke MORI