Share TFRC feedback types struct via ccid34_lib

I have a suggestion regarding types, names, and things shared between CCID3 and CCID4 via dccp_tfrc_lib:

I think it would be best to name all such shared structures / names, when they are to appear in the dccp_tfrc_lib, as tfrc_xxx' instead of ccid34_xxx'

For example,

enum tfrc_fback_type {
// ...
};

instead of:

|  enum ccid34_fback_type {
|  + FBACK_NONE = 0,
|  + FBACK_INITIAL,
|  + FBACK_PERIODIC,
|  + FBACK_PARAM_CHANGE
|  +};

There are no mechanisms other than TFRC (current work builds around TFRC, rather than entirely new schemes) so I think that this naming scheme is safe; and it would be consistent throughout the library.

I'd hope that Arnaldo and Ian add their take if they disagree or have other suggestions.

Em Wed, Oct 31, 2007 at 09:59:14AM +0000, Gerrit Renker escreveu:

I agree that for things which concept comes from TFRC and are used in one of the TFRC based DCCP CCIDs the best possible namespace is tfrc_. If not we'll have to rename everything again when CCID5 comes if it is also based on TFRC 8-)

And after all, even before ccid4 appeared on the radar I created dccp_tfrc_lib, include/linux/tfrc.h, etc exactly for sharing code with potentially new CCIDs that were based on TFRC.

- Arnaldo

Leandro,

following Arnaldo's reply, can you please you the suggested naming scheme (it is also already used in packet_history.c and loss_interval.c): I will be looking into merging the patches with the CCID3 set of patches, to reduce work and duplication.

The whole lot of CCID3 patches will then be re-submitted. Arnaldo, I would like to make your job easier by sending fewer patches (the CCID3 batch is 40 patches, plus Leandro's):
would you be ok with a smaller number, e.g. 5..7?

There will also be a short RFC before this (regarding locking), and I will make sure that the entire batch becomes fully bisectable.

Gerrit

Quoting Arnaldo Carvalho de Melo:

|  > enum tfrc_fback_type {
|  > // ...
|  > };
|  >  
|  > instead of:
|  > 
|  > |  enum ccid34_fback_type {
|  > |  + FBACK_NONE = 0,
|  > |  + FBACK_INITIAL,
|  > |  + FBACK_PERIODIC,
|  > |  + FBACK_PARAM_CHANGE
|  > |  +};
|  > 
|  > There are no mechanisms other than TFRC (current work builds around TFRC, rather than entirely new schemes)
|  > so I think that this naming scheme is safe; and it would be consistent throughout the library.
|  > 
|  > I'd hope that Arnaldo and Ian add their take if they disagree or have other suggestions.
|  
|  I agree that for things which concept comes from TFRC and are used in
|  one of the TFRC based DCCP CCIDs the best possible namespace is tfrc_.
|  If not we'll have to rename everything again when CCID5 comes if it is
|  also based on TFRC 8-)
|  
|  And after all, even before ccid4 appeared on the radar I created
|  dccp_tfrc_lib, include/linux/tfrc.h, etc exactly for sharing code with
|  potentially new CCIDs that were based on TFRC.
|  

2007/10/31, Gerrit Renker <[email protected]>:

Hello Gerrit,
Yes, for sure! I also agree with you and Arnaldo on this and I'm
currently change to the namespace suggested by you and accepted by Arnaldo.

Leandro

[CCID-3/4] Share TFRC feedback types struct via tfrc_ccids

Index: leandro.new/net/dccp/ccids/ccid3.c =================================--- leandro.new.orig/net/dccp/ccids/ccid3.c +++ leandro.new/net/dccp/ccids/ccid3.c
@@ -660,7 +660,7 @@ static inline void ccid3_hc_rx_update_s(
}

static void ccid3_hc_rx_send_feedback(struct sock *sk, struct sk_buff *skb,
- enum ccid3_fback_type fbtype)
+ enum tfrc_fback_type fbtype)
{
struct ccid3_hc_rx_sock *hcrx = ccid3_hc_rx_sk(sk); struct dccp_sock *dp = dccp_sk(sk);
@@ -777,7 +777,7 @@ static u32 ccid3_first_li(struct sock *s
static void ccid3_hc_rx_packet_recv(struct sock *sk, struct sk_buff *skb) {
struct ccid3_hc_rx_sock *hcrx = ccid3_hc_rx_sk(sk);
- enum ccid3_fback_type do_feedback = FBACK_NONE;
+ enum tfrc_fback_type do_feedback = FBACK_NONE;
u64 ndp = dccp_sk(sk)->dccps_options_received.dccpor_ndp; u32 sample, payload_size = skb->len - dccp_hdr(skb)->dccph_doff * 4; u8 is_data_packet = dccp_data_packet(skb);
Index: leandro.new/net/dccp/ccids/ccid3.h =================================--- leandro.new.orig/net/dccp/ccids/ccid3.h +++ leandro.new/net/dccp/ccids/ccid3.h
@@ -88,14 +88,6 @@ static inline struct ccid3_hc_tx_sock *c

     return hctx;

}

-/* CCID3 feedback types */
-enum ccid3_fback_type {
- FBACK_NONE = 0,
- FBACK_INITIAL,
- FBACK_PERIODIC,
- FBACK_PARAM_CHANGE
-};
-
/** struct ccid3_hc_rx_sock - CCID3 receiver half-connection socket
*
* @ccid3hcrx_last_counter - Tracks window counter (RFC 4342, 8.1)
Index: leandro.new/net/dccp/ccids/ccid4.c =================================--- leandro.new.orig/net/dccp/ccids/ccid4.c +++ leandro.new/net/dccp/ccids/ccid4.c
@@ -686,7 +686,7 @@ static inline void ccid4_hc_rx_update_s(
}

static void ccid4_hc_rx_send_feedback(struct sock *sk, struct sk_buff *skb,
- enum ccid4_fback_type fbtype)
+ enum tfrc_fback_type fbtype)
{
struct ccid4_hc_rx_sock *hcrx = ccid4_hc_rx_sk(sk); struct dccp_sock *dp = dccp_sk(sk);
@@ -803,7 +803,7 @@ static u32 ccid4_first_li(struct sock *s
static void ccid4_hc_rx_packet_recv(struct sock *sk, struct sk_buff *skb) {
struct ccid4_hc_rx_sock *hcrx = ccid4_hc_rx_sk(sk);
- enum ccid4_fback_type do_feedback = FBACK_NONE;
+ enum tfrc_fback_type do_feedback = FBACK_NONE;
u64 ndp = dccp_sk(sk)->dccps_options_received.dccpor_ndp; u32 sample, payload_size = skb->len - dccp_hdr(skb)->dccph_doff * 4; u8 is_data_packet = dccp_data_packet(skb);
Index: leandro.new/net/dccp/ccids/ccid4.h =================================--- leandro.new.orig/net/dccp/ccids/ccid4.h +++ leandro.new/net/dccp/ccids/ccid4.h
@@ -111,14 +111,6 @@ static inline struct ccid4_hc_tx_sock *c

     return hctx;

}

-/* CCID4 feedback types */
-enum ccid4_fback_type {
- FBACK_NONE = 0,
- FBACK_INITIAL,
- FBACK_PERIODIC,
- FBACK_PARAM_CHANGE
-};
-
/** struct ccid4_hc_rx_sock - CCID4 receiver half-connection socket
*
* @ccid4hcrx_last_counter - Tracks window counter (RFC 4342, 8.1)
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
@@ -61,3 +61,11 @@ enum tfrc_hc_rx_states {
TFRC_RSTATE_TERM = 127,
};

+/* CCID3/4 feedback types */
+enum tfrc_fback_type {
+ FBACK_NONE = 0,
+ FBACK_INITIAL,
+ FBACK_PERIODIC,
+ FBACK_PARAM_CHANGE
+};
+