|
Hi Ratz,
Hello Horms,
thanks for your comments. I have a new version of the patch attached.
Mucho better! I assume the patch is against ipvs-1.0.7. I wasn't sure because
you named your patch ipvs-0.1.7-*.
+/*
+ * 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?
Ok, finally I could download the source. They define the offsets for the
vs_tcp_state_idx in a rather strange but possible way. In case of interest, follow:
static inline int vs_tcp_state_idx(struct tcphdr *th, int state_off) {
/*
* [0-3]: input states, [4-7]: output, [8-11] input only states.
*/
if (th->rst)
return state_off+3;
if (th->syn)
return state_off+0;
if (th->fin)
return state_off+1;
if (th->ack)
return state_off+2;
return -1;
}
But last time Wensong told me that the 2.5.x LVS code will be made much nicer :)
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.
The names confused me a bit but I also asked because in your first patch, you
must have forgotten the chunk where you took them from.
#define IP_VS_SYNC_PORT 8848 /* multicast port */
I wonder if this should also be made selectable?
Good point.
Can be done in the next round, when this patch is in.
+/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.
I see.
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.
Yep, that's my impression too.
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.
Right. We have to see what Julian and Wensong think about all this.
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.
Definitely, thanks.
+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.
Yes :). Sorry for being so picky and a general pain in the butt.
+struct ip_vs_sync_mesg {
+ __u8 nr_conns;
+ __u8 reserved;
+ __u16 size;
+
+ /* ip_vs_sync_conn entries start here */
+};
Hehe, I know why nr_conns is __u8. struct ip_vs_sync_mesg will be 4 bytes, no
alignment, no dirty L1 cache lines, simply a nice happy chuck for the stack, but
there is certainly real truth behind this. :)
-#define VS_STATE_INPUT 0
-#define VS_STATE_OUTPUT 4
-#define VS_STATE_INPUT_ONLY 8
-
This I couldn't find in the former patch of yours, that's why I was confused. I
didn't have the code handy:
ratz@zar:~/down/ipvs > grep VS_STATE_INPUT_ONLY ipvs-0.1.7-sync_proc.*
ipvs-0.1.7-sync_proc.2.patch:+#define VS_STATE_INPUT_ONLY 8
ipvs-0.1.7-sync_proc.2.patch:-#define VS_STATE_INPUT_ONLY 8
ipvs-0.1.7-sync_proc.patch:+#define VS_STATE_INPUT_ONLY 8
ratz@zar:~/down/ipvs >
- len = (cp->flags & IP_VS_CONN_F_SEQ_MASK) ? FULL_CONN_SIZE :
- SIMPLE_CONN_SIZE;
+ len = (cp->flags & IP_VS_CONN_F_SEQ_MASK) ? IP_VS_SYNC_FULL_CONN_SIZE :
+ IP_VS_SYNC_SIMPLE_CONN_SIZE;
m = curr_sb->mesg;
s = (struct ip_vs_sync_conn *)curr_sb->head;
@@ -237,7 +174,8 @@
curr_sb->head += len;
/* check if there is a space for next one */
- if (curr_sb->head+FULL_CONN_SIZE > curr_sb->end) {
+ if (curr_sb->head+IP_VS_SYNC_FULL_CONN_SIZE > curr_sb->end ||
+ m->nr_conns == 255) {
Something I saw which is not only in your patch: Why don't we define something
stupid like (does not work of course, but you get the point):
#define NR_CONNS (1 << (8 * sizeof((struct ip_vs_sync_mesg *)->nr_conns))) - 1
+/proc/sys/net/ipv4/vs/sync_threshold sets the synchronisation threshold \-
+the minimum number of packets a connection needs to receive before it will
+be synchronised. The default is 3 and the vaild range is from 0 up to
s/vaild/valid/
+sync_frequency. Once sync_threshold is passed, the connection will be
+synchronised every sync_frequency packets. For example, using the default
+sync_frequency of 50 and the default sync_threshold of 3, synchronisation
+will occur on once the 3rd packet is received and then every 50th packet.
Now I like it, thanks.
Best regards,
Roberto Nibali, ratz
--
echo '[q]sa[ln0=aln256%Pln256/snlbx]sb3135071790101768542287578439snlbxq' | dc
| 