Mailing List Archive

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: Linux-HA-Dev@lists.linux-ha.org
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/