On Thu, Nov 21, 2002 at 11:51:19AM +0100, Roberto Nibali wrote:
> Hello,
Hi Ratz,
thanks for your comments. I have a new version of the patch attached.
> >I would like to resumbit this patch, which I have now updated for ipvs
> >1.0.7. The patch adds two new proc entries that effect the operation of
> >the synchronisation daemon.
> >
> >/proc/sys/net/vs/sync_frequency
> >
> >This is a companion to the existing /proc/sys/net/vs/sync_threshold proc
> >entry. It sets how often a packet will be synchronised. The default is
> >50, which was the hard-coded value for this.
> >
> >/proc/sys/net/vs/sync_msg_max_size
> >
> >This sets the maximum size of messages sent by the synchronisation
> >daemon in bytes. The intention is to be able to fine tune this for
> >networks whose MTU may be other than 1500 bytes. One example that
> >springs to mind would be a (Gigabit) network that uses jumbo frames of
> >6000 bytes. The default is 1228 which is the old hard-coded value, plus
>
> Or even 9000 bytes, no?
Yes
> >a little extra space to allow 50 simple connections to be transmitted by
> >having an extra 24 bytes to allow for the case where the last connection
> >is a full connection.
>
> I like the patch just like last time, some minor issues though.
>
> > Updates the ipvsadm(8) man pages to document the functionality of
> > these proc entries, as well as the existing
> > /proc/sys/net/vs/sync_threshold proc entry that was previously
> > undocumented.
>
> At least this should go in.
>
> > Fixes various spelling and gramatical errors in the ipvsadm(8)
> ^^^^^
> I'm not so sure if I should trust you :):) ... see below for more!
I don't really trust myself much either :)
> >@@ -218,6 +218,131 @@
> > };
> >
> >
> >+/*
> >+ * IPVS connection entry hash table
> >+ */
> >+
> >+#define VS_STATE_INPUT 0
> >+#define VS_STATE_OUTPUT 4
> >+#define VS_STATE_INPUT_ONLY 8
>
> What for? And why 0, 4, 8?
They are just the internal states, I have no idea why the numbers 0 4
and 8 were chosen, but it also doesn't seem to be particularly
imporatant either. I just moved these few lines from one part of ip_vs.h
to another.
> >+#define IP_VS_SYNC_CONN_TIMEOUT (3*60*HZ)
> >+#define IP_VS_SYNC_SIMPLE_CONN_SIZE (sizeof(struct ip_vs_sync_conn))
> >+#define IP_VS_SYNC_FULL_CONN_SIZE \
> >+(sizeof(struct ip_vs_sync_conn) + sizeof(struct ip_vs_sync_conn_options))
>
> >+#define IP_VS_SYNC_FREQUENCY_DEFAULT 50
> >+#define IP_VS_SYNC_FREQUENCY \
> >+ ((sysctl_ip_vs_sync_frequency < 1) ? 1 : sysctl_ip_vs_sync_frequency)
> >+
> >+#define IP_VS_SYNC_THRESHOLD_DEFAULT 3
> >+#define IP_VS_SYNC_THRESHOLD \
> >+ ((sysctl_ip_vs_sync_threshold < 0) ? 0 : \
> >+ (sysctl_ip_vs_sync_threshold >= IP_VS_SYNC_FREQUENCY) ? \
> >+ IP_VS_SYNC_FREQUENCY - 1 : sysctl_ip_vs_sync_threshold)
> >+
>
> Looks fine as for a policy.
>
> > #define IP_VS_SYNC_PORT 8848 /* multicast port */
>
> I wonder if this should also be made selectable?
Good point.
> >+running on the backup load balancers receives multicast message and
> >+creates corresponding connections. Then, if the primary load
> >+balancer fails and backup load balancer takes oever, it has the state
>
> +balancer fails and the backup load balancer takes over, it has the state
>
> >+There are 3 proc enties that effect the behaviour of the
>
> s/enties/entries/
>
> >+/proc/sys/net/ipv4/vs/sync_msg_max_size sets the maximum size of messages
> >+sent by the synchronisation daemon in bytes. The default is 1228 and the
> >+useful range is 52 through to 6172.
>
> Why not 52 up to 9k?
Because the packet structure only has one byte for the number of sync
conn entries in a packet. Thus a maximum of 255 sync conn entries in a
packet. The sync conn entries have two vareties, 24 byte
and 48 byte. AFIK 24 byte are much more common, thus there is an
effective limit on the size of a packet around 6k. Not my design.
Actually I think 6172 should be 6148 but it really doesn't make much
difference. 4 bytes header + 255 * 24 byte sync conn + 24 bytes of pad
in case the last sync conn is 48 bytes. The 24 byte pad isn't actuallty
sent over the wire, unless the last entry turns out to be a 48byte
entry.
However, I think this logig is wrong, and we can allow a packet size of
up to 4 + 255 * 48 = 12244 bytes to be set. I have updated the patch
accordingly. Although one notes that if most (all) the entries are the
24 byte variety, you still are stuck at around 6k because of the 255
sync conn limit. So it really doesn't make much difference IMHO.
> >+.sp
> >+/proc/sys/net/ipv4/vs/sync_frequency sets syncrhonisation frequency \-
>
> s/syncrhonisation/synchronisation/
>
> >+the how often a connection is
> >+synchronised in terms of the number of packets received. The default is
> >50
> >+and the useful range is 1 through to 2147483648.
>
> What do you mean with useful when you select sync_frequency=2147483648?
By useful, I mean that the code will actually work. Values outside that
range are invalid. Perhaps vaild would be a better word to use than
useful.
> >+.sp
> >+/proc/sys/net/ipv4/vs/sync_threshold sets the synchronisation threshold \-
> >+the minmum number of packets a connection needs to receive before it will
>
> s/minmum/minimum/
> Maybe you even want to add a 'to' in front of 'the minimum'.
>
> >+be synchronised. The default is 3 and the useful range is from 0 up to
> >the
> >+synchronisation frequency. Once this threshold is passed the connection
> >+will be syncronised each time the number of packets, modulus the
>
> s/syncronised/synchronised/
>
> >+syncronisation will occur on once the 3rd packet is recieved, and every
>
> Holy cow Horms, where you stoned or did I miss recent advances in English?
I've reworded this, hopefully it is clearer now.
--
Horms
ipvs-0.1.7-sync_proc.2.patch
Description: Text document
|