[prev in list] [next in list] [prev in thread] [next in thread]
List: linux-ha-dev
Subject: Re: [Linux-ha-dev] [resource-agents] [RFC] IPaddr2: Proposal patch to support the dual stack of IPv4
From: Keisuke MORI <keisuke.mori+ha () gmail ! com>
Date: 2012-10-18 9:56:11
Message-ID: CAJM6Fh_vbBftyjwnsO8bhgs+BhYsURhz8aJF9mM9jbQUPipmUA () mail ! gmail ! com
[Download RAW message or body]
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.
2012/10/16 Lars Ellenberg <lars@linbit.com>:
>
> 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.
Agreed.
>
>> * 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.
It has been fixed and the latest code in the repo now should work like you said.
>
>> * 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.
>
>> * 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.
>
> 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/
Regards,
--
Keisuke MORI
_______________________________________________________
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/
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic