Skip to contentSkip to conversation

Basic implementation for ccid-4 dropped packet option

PATCH In review Leandro 2 participants 2 messages Nov 1, 2007

lore
Share & tools
lore

Leandro

author
#0 lore raw

PATCH 14/25 Basic implementation for ccid-4 dropped packet option

[CCID-4] Basic implementation for ccid-4 dropped packet option as per ccid-4 draft

Index: leandro.new/net/dccp/ccids/lib/tfrc_ccids.h =================================--- leandro.new.orig/net/dccp/ccids/lib/tfrc_ccids.h +++ leandro.new/net/dccp/ccids/lib/tfrc_ccids.h
@@ -36,12 +36,15 @@ enum tfrc_options {
TFRC_OPT_LOSS_EVENT_RATE = 192,
TFRC_OPT_LOSS_INTERVALS = 193,
TFRC_OPT_RECEIVE_RATE = 194,
+ TFRC_OPT_DROPPED_PACKETS = 195, /* as per ccid-4 draft, section 8 */
};

struct tfrc_options_received {
u64 tfrcor_seqno:48,
- tfrcor_loss_intervals_idx:16;
- u16 tfrcor_loss_intervals_len;
+ tfrcor_loss_intervals_idx:16,
+ tfrcor_dropped_packets_idx:16;
+ u16 tfrcor_loss_intervals_len,
+ tfrcor_dropped_packets_len;
u32 tfrcor_loss_event_rate;
u32 tfrcor_receive_rate;
};
Index: leandro.new/net/dccp/ccids/ccid4.c =================================--- leandro.new.orig/net/dccp/ccids/ccid4.c +++ leandro.new/net/dccp/ccids/ccid4.c
@@ -590,6 +590,9 @@ static int ccid4_hc_tx_parse_options(str
opt_recv->tfrcor_receive_rate);
}
break;
+ case TFRC_OPT_DROPPED_PACKETS:
+ /* FIXME: Implement this sock option according to ccid-4 draft */
+ break;
}

return rc;

#1 lore raw

Re: [PATCH 14/25] Basic implementation for ccid-4 dropped packet

Em Wed, Oct 31, 2007 at 09:31:40PM -0300, Leandro escreveu:

Quoted 10 lines

[CCID-4] Basic implementation for ccid-4 dropped packet option as per ccid-4 draft

Index: leandro.new/net/dccp/ccids/lib/tfrc_ccids.h =================================> --- leandro.new.orig/net/dccp/ccids/lib/tfrc_ccids.h +++ leandro.new/net/dccp/ccids/lib/tfrc_ccids.h

(quoted diff) +0 -0
@@ -36,12 +36,15 @@ enum tfrc_options {
 	TFRC_OPT_LOSS_EVENT_RATE = 192,
 	TFRC_OPT_LOSS_INTERVALS	 = 193,

Review enum tfrc_options { · line 45

Quoted 10 lines
(quoted diff) +3 -2
 	TFRC_OPT_RECEIVE_RATE	 = 194,
+	TFRC_OPT_DROPPED_PACKETS = 195, /* as per ccid-4 draft, section 8 */
 };
 
 struct tfrc_options_received {
 	u64 tfrcor_seqno:48,
-	    tfrcor_loss_intervals_idx:16;
-	u16 tfrcor_loss_intervals_len;
+	    tfrcor_loss_intervals_idx:16,
+	    tfrcor_dropped_packets_idx:16;

Just make tfrcor_dropped_packets_idx a plain u16. tfrcor_loss_intervals_idx is part of a bitfield together with tfrcor_seqno because it is 48 bits and we want to use what is left in a u64.

Making tfrcor_dropped_packets_idx a 16 bits entry in a 16 bits bitfield doesn't helps us here and likely produces bad code.

Review enum tfrc_options { · line 50

Quoted 5 lines
(quoted diff) +2 -0
+	u16 tfrcor_loss_intervals_len,
+            tfrcor_dropped_packets_len;
 	u32 tfrcor_loss_event_rate;
 	u32 tfrcor_receive_rate;
 };

And even leads to some confusion, one would think that the second, single entry bitfield would use 64 bits, as it is declared u64, but the rule is:

[acme@doppio ~]$ pahole bitfield

struct tfrc_options_received {
        u64  tfrcor_seqno:48;               /*   0 8 */
        u64  tfrcor_loss_intervals_idx:16;  /*   0 8 */
        u64  tfrcor_dropped_packets_idx:16; /*   8 8 */

        /* Bitfield WARNING: DWARF size=8, real size=2 */

        u16  tfrcor_loss_intervals_len;     /*  10 2 */
        u16  tfrcor_dropped_packets_len;    /*  12 2 */

        /* XXX 2 bytes hole, try to pack */

        u32  tfrcor_loss_event_rate;        /*  16 4 */
        u32  tfrcor_receive_rate;           /*  20 4 */

        /* size: 24, cachelines: 1 */
        /* sum members: 22, holes: 1, sum holes: 2 */
        /* last cacheline: 24 bytes */
};      /* definitions: 1 */

[acme@doppio ~]$

The rule is so confusing that even gcc gets lost and emits debugging information where it says the type of the bitfield is u64, thus 8 bytes wide, but the offset of the next field is 2 bytes after the bitfield :-)

So just make it u16 and we'll get this:

[acme@doppio ~]$ pahole bitfield

[acme@doppio ~]$ pahole bitfield

struct tfrc_options_received {
        u64  tfrcor_seqno:48;		   /*   0 8 */
        u64  tfrcor_loss_intervals_idx:16; /*   0 8 */
        u16  tfrcor_dropped_packets_idx;   /*   8 2 */
        u16  tfrcor_loss_intervals_len;	   /*  10 2 */
        u16  tfrcor_dropped_packets_len;   /*  12 2 */

        /* XXX 2 bytes hole, try to pack */

        u32  tfrcor_loss_event_rate;       /*  16 4 */
        u32  tfrcor_receive_rate;          /*  20 4 */

        /* size: 24, cachelines: 1 */
        /* sum members: 22, holes: 1, sum holes: 2 */
        /* last cacheline: 24 bytes */
};      /* definitions: 1 */

[acme@doppio ~]$

And now we have a 2 bytes hole, so we may as well just stop making the first two fields a bitfield.

- Arnaldo