On Tue, Jan 12, 2010 at 4:01 AM, Dejan Muhamedagic <[email protected]> wrote:
> Hi,
>
> On Tue, Jan 12, 2010 at 04:54:43PM +0800, Jiaju Zhang wrote:
>> On Mon, Jan 11, 2010 at 9:21 PM, Lars Ellenberg
>> <[email protected]> wrote:
>> > On Mon, Jan 11, 2010 at 06:56:03PM +0800, Jiaju Zhang wrote:
>> >> # HG changeset patch
>> >> # User Jiaju Zhang <[email protected]>
>> >> # Date 1263205796 -28800
>> >> # Node ID 96ffc17dafd253e71f916b4d90ce701e086e2927
>> >> # Parent  c76b4a6eb576feb3b39852aa2349a0716bda1078
>> >> Dev: portblock: Tickle ACK to TCP connections
>> >>
>> >> diff -r c76b4a6eb576 -r 96ffc17dafd2 heartbeat/portblock
>> >> --- a/heartbeat/portblock     Mon Jan 04 14:42:10 2010 +0100
>> >> +++ b/heartbeat/portblock     Mon Jan 11 18:29:56 2010 +0800
>> >
>> > btw, there is dead code:
>> > iptables_spec() { ... }
>> >
>> >
>> >> @@ -14,6 +14,8 @@
>> >>  #            OCF_RESKEY_portno
>> >>  #            OCF_RESKEY_action
>> >>  #            OCF_RESKEY_ip
>> >> +#            OCF_RESKEY_tickle_dir
>> >> +#            OCF_RESKEY_sync_script
>> >>  #######################################################################
>> >>  # Initialization:
>> >>
>> >> @@ -26,6 +28,7 @@
>> >>  : ${OCF_RESKEY_ip=${OCF_RESKEY_ip_default}}
>> >>  #######################################################################
>> >>  CMD=`basename $0`
>> >> +TICKLETCP=$HA_BIN/tickle_tcp
>> >>
>> >>  usage()
>> >>  {
>> >> @@ -58,6 +61,34 @@
>> >>       the server.
>> >>
>> >>       NOTE:  iptables is linux-specific...
>> >> +
>> >> +     An additional feature in the portblock RA is the tickle ACK function
>> >> +        which triggers the clients to faster reconnect their TCP 
>> >> connections
>> >> +     to the fail-overed server.
>> >
>> > either add here:
>> >        enabled by specifying the tickle_dir parameter.
>> >
>> > or move the following paragraph below the "When using the tickle ACK
>> > function ..."
>> >
>> >> +
>> >> +     Please note that this feature is often used for the floating IP 
>> >> fail-
>> >> +     over scenario where the long-lived TCP connections need to be 
>> >> tickled.
>> >> +     It doesn't support the cluster alias IP scenario. And if you want to
>> >> +     tickle the TCP connections to _one_ floating IP(maybe the 
>> >> connections
>> >> +     are to different ports), you only need _one_ portblock resource.
>> >
>> >        ... you should *enable* tickles for _one_ portblock resource only,
>> >        and of course for the action=unblock instance (otherwise that
>> >        won't work; maybe enfoce this?).
>> >
>> > Of course you may have more multiple portblocks per IP.
>> > You may occasionally need several ports (maybe even
>> > more than iptables accepts for one multiport commandline), or udp as
>> > well as tcp blocking, or have other reasons for multiple portblocks.
>> > And typically, you have one portblock action: block, and one portblock
>> > action: unblock, each.
>>
>> Yeah, I'll modify the usage specification to try to clarify these :)
>>
>> >
>> >> +
>> >> +     When using the tickle ACK function, in addition to the normal usage
>> >> +     of portblock RA, the parameter tickle_dir must be specified! The
>> >> +     tickle_dir is a location which stores the established TCP 
>> >> connections.
>> >> +     It can be a shared directory which is cluster-visible to all nodes.
>> >> +     But if you don't have a shared directory, you could also use a local
>> >> +     directory with cysnc2 pre-configured.
>> >> +     For example, if you use the local directory /tmp/tickle as 
>> >> tickle_dir,
>> >> +     you could configure your /etc/csync2/csync2.cfg like:
>> >> +             group ticklegroup {
>> >> +               host node1;
>> >> +               host node2;
>> >> +               key  /etc/csync2/ticklegroup.key;
>> >> +               include /etc/csync2/csync2.cfg;
>> >> +               include /tmp/tickle;
>> >> +               auto younger;
>> >> +             }
>> >> +     Then specify the parameter sync_script as "csync2 -xv".
>> >>
>> >>  END
>> >>  }
>> >> @@ -100,6 +131,25 @@
>> >>  <content type="string" default="${OCF_RESKEY_ip_default}" />
>> >>  </parameter>
>> >>
>> >> +<parameter name="tickle_dir" unique="0" required="0">
>> >> +<longdesc lang="en">
>> >> +The shared or local directory(_must_ be absolute path) which
>> >> +stores the established TCP connections.
>> >> +</longdesc>
>> >> +<shortdesc lang="en">Tickle directory</shortdesc>
>> >> +<content type="string" default="" />
>> >> +</parameter>
>> >> +
>> >> +<parameter name="sync_script" unique="0" required="0">
>> >> +<longdesc lang="en">
>> >> +The script used for synchronizing TCP connection state file, such as
>> >> +csync2, some wrapper of rsync, or whatever.
>> >> +If you used local directory as tickle_dir, you must specify this 
>> >> parameter.
>> >> +</longdesc>
>> >> +<shortdesc lang="en">File sync script</shortdesc>
>> >> +<content type="string" default="csync2 -xv" />
>> >> +</parameter>
>> >> +
>> >>  <parameter name="action" unique="0" required="1">
>> >>  <longdesc lang="en">
>> >>  The action (block/unblock) to be done on the protocol::portno.
>> >> @@ -149,6 +199,33 @@
>> >>  {
>> >>    PAT=`active_grep_pat "$1" "$2" "$3"`
>> >>    $IPTABLES -n -L INPUT | grep "$PAT" >/dev/null
>> >> +}
>> >> +
>> >> +save_tcp_connections()
>> >> +{
>> >> +     [ -z "$OCF_RESKEY_tickle_dir" ] && return
>> >> +     statefile=$OCF_RESKEY_tickle_dir/$OCF_RESKEY_ip
>> >> +     if [ -z "$OCF_RESKEY_sync_script" ]; then
>> >> +             netstat -tn |awk -F '[:[:space:]]+' '
>> >> +                     $8 == "ESTABLISHED" && $4 == "'$OCF_RESKEY_ip'" \
>> >> +                     {printf "%s:%s\t%s:%s\n", $4,$5, $6,$7}' |
>> >> +                     dd of="$statefile".new conv=fsync &&
>> >> +                     mv "$statefile".new "$statefile"
>> >> +     else
>> >> +             netstat -tn |awk -F '[:[:space:]]+' '
>> >> +                     $8 == "ESTABLISHED" && $4 == "'$OCF_RESKEY_ip'" \
>> >> +                     {printf "%s:%s\t%s:%s\n", $4,$5, $6,$7}' \
>> >> +                     > $statefile
>> >> +             $OCF_RESKEY_sync_script $statefile > /dev/null 2>&1 &
>> >> +     fi
>> >> +}
>> >> +
>> >> +run_tickle_tcp()
>> >> +{
>> >> +     [ -z "$OCF_RESKEY_tickle_dir" ] && return
>> >> +     echo 1 > /proc/sys/net/ipv4/tcp_tw_recycle
>> >> +     f=$OCF_RESKEY_tickle_dir/$OCF_RESKEY_ip
>> >> +     [ -f $f ] && cat $f | $TICKLETCP -n 3
>> >>  }
>> >>
>> >>  SayActive()
>> >> @@ -195,8 +272,9 @@
>> >>               ;;
>> >>
>> >>           *)
>> >> -             if ha_pseudo_resource "${OCF_RESOURCE_INSTANCE}" status; 
>> >> then
>> >> +             if ha_pseudo_resource "${OCF_RESOURCE_INSTANCE}" status; 
>> >> then
>> >>                       SayActive $*
>> >> +                     save_tcp_connections
>> >
>> > rather,
>> >        ocf_is_probe || save_tcp_connections
>> > because we do not want to save_tcp_connections if this is a probe
>> > (crm verifying on hosts where this should not run that it is not running)
>> >
>> > though that ha_pseudo_resource thing should only return true if this is
>> > not a probe, right?
>>
>> Yeah, the combination of "chain_isactive" and "ha_pseudo_resource" can
>> differentiate if it is a probe or real monitor.
>>
>> >
>> > so maybe leave it as is, but add a comment indicating that this is only
>> > run on real monitor events.
>>
>> OK, I'll add the comment.
>>
>> >
>> >>                       rc=$OCF_SUCCESS
>> >>               else
>> >>                       SayInactive $*
>> >> @@ -243,7 +321,10 @@
>> >>    ha_pseudo_resource "${OCF_RESOURCE_INSTANCE}" start
>> >>    case $4 in
>> >>      block)   IptablesBLOCK "$@";;
>> >> -    unblock) IptablesUNBLOCK "$@";;
>> >> +    unblock)
>> >> +             IptablesUNBLOCK "$@"
>> >> +             run_tickle_tcp
>> >
>> > problem:
>> > exit code now contains something other than the return code of 
>> > IptablesUNBLOCK
>> >
>> > rather,
>> >                IptablesUNBLOCK "$@"
>> >                rc=$?
>> >                run_tickle_tcp
>> >                # ignore run_tickle_tcp exit code!
>> >                return $rc
>>
>> Thanks for pointing out this :)
>>
>> >
>> >> +             ;;
>> >>      *)               usage; return 1;
>> >>    esac
>> >>
>> >> @@ -256,7 +337,10 @@
>> >>    ha_pseudo_resource "${OCF_RESOURCE_INSTANCE}" stop
>> >>    case $4 in
>> >>      block)   IptablesUNBLOCK "$@";;
>> >> -    unblock) IptablesBLOCK "$@";;
>> >> +    unblock)
>> >> +             save_tcp_connections
>> >> +             IptablesBLOCK "$@"
>> >> +             ;;
>> >>      *)               usage; return 1;;
>> >>    esac
>> >>
>> >> @@ -269,14 +353,7 @@
>> >>  CheckPort() {
>> >>  #    Examples of valid port: "1080", "1", "0080"
>> >>  #    Examples of invalid port: "1080bad", "0", "0000", ""
>> >> -  case "$1" in
>> >> -    *[!0-9]*) #got invalid char
>> >> -     false;;
>> >> -    *[1-9]*) #no invalid char, and has non-zero digit, so is a good port
>> >> -     true;;
>> >> -    *) #empty string, or string of 0's
>> >> -     false;;
>> >> -  esac
>> >> +  echo $1 |egrep -qx '[0-9]+(:[0-9]+)?(,[0-9]+(:[0-9]+)?)*'
>> >
>> > thanks ;-)
>> > though this now would allow the all zero port, which the old one did not 
>> > allow.
>> > but I don't care, and neither does iptables (at least on my box...)
>> >
>> >>  }
>> >>
>> >>  IptablesValidateAll()
>> >> @@ -296,6 +373,13 @@
>> >>    else
>> >>       ocf_log err "Invalid port number $portno!"
>> >>       exit $OCF_ERR_ARGS
>> >> +  fi
>> >> +
>> >> +  if [ -n "$OCF_RESKEY_tickle_dir" ]; then
>> >> +     if [ ! -d "$OCF_RESKEY_tickle_dir" ]; then
>> >> +             ocf_log err "The tickle dir doesn't exist!"
>> >> +             exit $OCF_ERR_ARGS
>> >> +     fi
>> >
>> >
>> > hmm.
>> > as tickle_dir is only used on the unblock action,
>> > maybe enforce that:
>> >        $action = unblock || "tickles are only useful with
>> >        action=unblock", ERR_ARGS ...
>>
>> OK, will enforce that.
>>
>> >
>> >
>> > I think we should include this now.
>>
>> Thanks! New patch attached :)
>
> Many thanks!
>
> I'll move some of the text from usage into the metadata. Nobody's
> going to read usage, let's hope some will take a look at the
> metadata :) After another review, I'll apply the patch. Then if
> there are more issues, we can take it from there.
>

Thanks, guys!
Can't wait to test the patch!

--
Sam
_______________________________________________________
Linux-HA-Dev: [email protected]
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/

Reply via email to