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 ;-) On Wed, May 30, 2012 at 11:20:03PM -0700, Keisuke MORI wrote: > 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. > ## Benefits: > > * 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. In 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. > * 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.) > > See also: > 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, > originally developed by lge. See ClusterLabs/resource-agents#53 : > 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 > > > ## Notes / Changes: > > * 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. 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. > * 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. > * 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 ;-) 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. > > > > You can merge this Pull Request by running: > > git pull https://github.com/kskmori/resource-agents > IPaddr2-dualstack-devel > > Or you can view, comment on it, or merge it online at: > > 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 +} _______________________________________________________ Linux-HA-Dev: [email protected] http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev Home Page: http://linux-ha.org/
