* Re: [PATCH] correct typo in __xfrm4_bundle_create()
From: David Miller @ 2007-12-05 14:03 UTC (permalink / raw)
To: ianbrn; +Cc: netdev
In-Reply-To: <d0383f90712050559o7ad78dbbm51f74984b591e66d@mail.gmail.com>
From: "Ian Brown" <ianbrn@gmail.com>
Date: Wed, 5 Dec 2007 15:59:52 +0200
> Hello,
> Sorry ; but is this tree can be fetched ? it requires a password.
>
> I try:
> git-clone kernel.org:/pub/scm/linux/kernel/git/davem/net-2.6.25.git net-2.6.25
> and get:
> root@kernel.org's password:
Use the git:// URL prefix.
^ permalink raw reply
* Re: [PATCH] correct typo in __xfrm4_bundle_create()
From: Ian Brown @ 2007-12-05 13:59 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20071205.054711.186439758.davem@davemloft.net>
Hello,
Sorry ; but is this tree can be fetched ? it requires a password.
I try:
git-clone kernel.org:/pub/scm/linux/kernel/git/davem/net-2.6.25.git net-2.6.25
and get:
root@kernel.org's password:
Ian
On Dec 5, 2007 3:47 PM, David Miller <davem@davemloft.net> wrote:
> From: "Ian Brown" <ianbrn@gmail.com>
> Date: Wed, 5 Dec 2007 15:41:37 +0200
>
> > Hello,
> > Sorry; I was not aware of it; I am sending here the patch as an attachment.
> > It is a typo correction in __xfrm4_bundle_create() in net/ipv4/xfrm4_policy.c;
> > it should be "Copy neighbour" instead "Copy neighbout"
>
> __xfrm4_bundle_create() no longer exists in the networking
> development tree:
>
> kernel.org:/pub/scm/linux/kernel/git/davem/net-2.6.25.git
>
> Because Herbert Xu unified much of the IPSEC handling
> paths over the past few weeks.
>
^ permalink raw reply
* Re: [PATCH v2 0/3][BUG-FIX]: Test tree updates and bug fixes
From: Gerrit Renker @ 2007-12-05 13:55 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, dccp, netdev
In-Reply-To: <20071205132345.GC4653@ghostprotocols.net>
| > @@ -788,8 +782,8 @@ static void ccid3_hc_rx_packet_recv(stru
| > if (unlikely(hcrx->ccid3hcrx_state == TFRC_RSTATE_NO_DATA)) {
| > if (is_data_packet) {
| > do_feedback = FBACK_INITIAL;
| > + hcrx->ccid3hcrx_s = payload_size;
| > ccid3_hc_rx_set_state(sk, TFRC_RSTATE_DATA);
| > - ccid3_hc_rx_update_s(hcrx, payload_size);
|
| We have to set ccid3hcrx_bytes_recv to the payload_size here too, I'm
| fixing this on the reworked patch that introduces the RX history.
|
I almost did the same error again by wanting to agree too prematurely.
But updating ccid3hcrx_bytes_recv is in fact not needed here and if it
would be done it would not have a useable effect. The reason is that the
first data packet will trigger the initial feedback; and in the initial
feedback packet X_recv (which is ccid3hcrx_bytes_recv / the_time_spent)
is set to 0 (RFC 3448, 6.3).
For this reason, updating bytes_recv for the first data packet is also not
in the flowchart on
http://www.erg.abdn.ac.uk/users/gerrit/dccp/notes/ccid3_packet_reception/
^ permalink raw reply
* Re: [Fwd: PATCH 2.6.24-rc3-mm2 - build breakage - bnx2x depends on ZLIB_INFLATE]
From: Eliezer Tamir @ 2007-12-05 13:48 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Lee.Schermerhorn, jeff
In-Reply-To: <20071205.054442.110477658.davem@davemloft.net>
I will resend it in a moment.
Thanks,
Eliezer
On Wed, 2007-12-05 at 05:44 -0800, David Miller wrote:
> From: "Eliezer Tamir" <eliezert@broadcom.com>
> Date: Wed, 05 Dec 2007 15:40:15 +0200
>
> > Dave,
> >
> > Lee Schermerhorn sent this patch while you were away,
> > please apply.
>
> I can't because the patch is corrupted by line breaks
> and tabs --> space corruption caused by his mail
> client.
>
>
>
^ permalink raw reply
* Re: [PATCH] correct typo in __xfrm4_bundle_create()
From: David Miller @ 2007-12-05 13:47 UTC (permalink / raw)
To: ianbrn; +Cc: netdev
In-Reply-To: <d0383f90712050541p5d8ffce6q8748d7c6b66ea9c6@mail.gmail.com>
From: "Ian Brown" <ianbrn@gmail.com>
Date: Wed, 5 Dec 2007 15:41:37 +0200
> Hello,
> Sorry; I was not aware of it; I am sending here the patch as an attachment.
> It is a typo correction in __xfrm4_bundle_create() in net/ipv4/xfrm4_policy.c;
> it should be "Copy neighbour" instead "Copy neighbout"
__xfrm4_bundle_create() no longer exists in the networking
development tree:
kernel.org:/pub/scm/linux/kernel/git/davem/net-2.6.25.git
Because Herbert Xu unified much of the IPSEC handling
paths over the past few weeks.
^ permalink raw reply
* Re: [RFC][PATCHES 0/7]: Reorganization of RX history patches
From: Gerrit Renker @ 2007-12-05 13:45 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, netdev, dccp
In-Reply-To: <20071205115226.GM29972@ghostprotocols.net>
| > Patch #3:
| > ---------
| > Renames s/tfrc_tx_hist/tfrc_tx_hist_slab/g; no problems.
|
| This was for consistency with the other slabs in DCCP:
|
| [acme@doppio net-2.6.25]$ find net/dccp/ -name "*.c" | xargs grep 'struct kmem_cache'
Thanks, I will put the same naming scheme also in the test tree (tomorrow).
| > Patch #4:
| > ---------
| > Just a suggestion, it is possible to simplify this further,
| > by doing all the initialisation / cleanup in tfrc.c:
| > int __init {rx,tx}_packet_history_init()
| > {
| > tfrc_{rx,tx}_hist_slab = kmem_cache_create("tfrc_{rx,tx}_hist", ...);
| > return tfrc_{rx,tx}_hist_slab == NULL ? - ENOBUFS : 0;
| > }
| > and then call these successively in tfrc_module_init().
|
| Idea here was to have each C source file to have a init module. Perhaps
| we should try to break packet_history.c into tx_packet_history and
| rx_packet_history.c. We can do that later to try to meet the goal of
| being able to see what is being replaced.
|
I think this is a great idea, since then rx_packet_history.c could also
take up all the internals of the RX packet history list, as it is
currently done for the TX history, and it could also possibly
incorporate. packet_history_internal.h.
|
| > Patch #7:
| > ---------
|
| > * tfrc_rx_hist_entry_data_packet() is not needed:
| > - a similar function, called dccp_data_packet(), was introduced in patch 2/7
|
| I thought about that, but dccp_data_packet is for skbs,
| tfrc_rx_hist_entry_data_packet is for tfrc_rx_hist_entries, I guess we
| should just make dccp_data_packet receive the packet type and not an
| object that has a packet type field.
|
The question which of the two interfaces is generally better to use is
best left to you. Two functions doing almost the same thing can probably
be replaced by just one.
^ permalink raw reply
* Re: [Fwd: PATCH 2.6.24-rc3-mm2 - build breakage - bnx2x depends on ZLIB_INFLATE]
From: David Miller @ 2007-12-05 13:44 UTC (permalink / raw)
To: eliezert; +Cc: netdev, Lee.Schermerhorn, jeff
In-Reply-To: <1196862015.5204.16.camel@lb-tlvb-eliezer.il.broadcom.com>
From: "Eliezer Tamir" <eliezert@broadcom.com>
Date: Wed, 05 Dec 2007 15:40:15 +0200
> Dave,
>
> Lee Schermerhorn sent this patch while you were away,
> please apply.
I can't because the patch is corrupted by line breaks
and tabs --> space corruption caused by his mail
client.
^ permalink raw reply
* Re: [PATCH] correct typo in __xfrm4_bundle_create()
From: Ian Brown @ 2007-12-05 13:41 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20071205.052034.251196808.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 856 bytes --]
Hello,
Sorry; I was not aware of it; I am sending here the patch as an attachment.
It is a typo correction in __xfrm4_bundle_create() in net/ipv4/xfrm4_policy.c;
it should be "Copy neighbour" instead "Copy neighbout"
Ian
On Dec 5, 2007 3:20 PM, David Miller <davem@davemloft.net> wrote:
> From: "Ian Brown" <ianbrn@gmail.com>
> Date: Wed, 5 Dec 2007 12:49:52 +0200
>
> > This is a typo correction in net/ipv4/xfrm4_policy.c; it should be
> > "Copy neighbour" instead "Copy neighbout",
>
> Your email client has corrupted the patch by adding
> newlines among other things.
>
> This happens often for gmail users, because by default
> gmail does text formatting which you don't want it
> to do for patches because it corrupts them.
>
> Please use an attachment or some other method to prevent
> this form happening so that you patch is not corrupted.
>
[-- Attachment #2: patchToNetDev.txt --]
[-- Type: text/plain, Size: 690 bytes --]
Signed-off-by: ianbrn@gmail.com
diff --git a/a/net/ipv4/xfrm4_policy.c b/b/net/ipv4/xfrm4_policy.c
index cc86fb1..390dc7c 100644
--- a/a/net/ipv4/xfrm4_policy.c
+++ b/b/net/ipv4/xfrm4_policy.c
@@ -165,7 +165,7 @@ __xfrm4_bundle_create(struct xfrm_policy *policy, struct xfrm_state **xfrm, int
dst_prev->trailer_len = trailer_len;
memcpy(&dst_prev->metrics, &x->route->metrics, sizeof(dst_prev->metrics));
- /* Copy neighbout for reachability confirmation */
+ /* Copy neighbour for reachability confirmation */
dst_prev->neighbour = neigh_clone(rt->u.dst.neighbour);
dst_prev->input = rt->u.dst.input;
dst_prev->output = dst_prev->xfrm->outer_mode->afinfo->output;
^ permalink raw reply related
* [Fwd: PATCH 2.6.24-rc3-mm2 - build breakage - bnx2x depends on ZLIB_INFLATE]
From: Eliezer Tamir @ 2007-12-05 13:40 UTC (permalink / raw)
To: davem@davemloft.net, netdev@vger.kernel.org
Cc: Lee Schermerhorn, jeff@garzik.org
Dave,
Lee Schermerhorn sent this patch while you were away,
please apply.
Thanks,
Eliezer
-------- Forwarded Message --------
From: Lee Schermerhorn <Lee.Schermerhorn@hp.com>
To: Andrew Morton <akpm@linux-foundation.org>, netdev@vger.kernel.org,
linux-kernel <linux-kernel@vger.kernel.org>
Subject: PATCH 2.6.24-rc3-mm2 - build breakage - bnx2x depends on
ZLIB_INFLATE
Date: Wed, 28 Nov 2007 12:25:24 -0800
Couldn't find one of these on the lists...
PATCH 2.6.24-rc3-mm1: bnx2x depends on ZLIB_INFLATE
The bnx2x module depends on the zlib_inflate functions. The
build will fail if ZLIB_INFLATE has not been selected manually
or by building another module that automatically selects it.
Modify BNX2X config option to 'select ZLIB_INFLATE' like BNX2
and others. This seems to fix it.
Signed-off-by: Lee Schermerhorn <lee.schermerhorn@hp.com>
Acked-by: Eliezer Tamir <eliezert@broadcom.com>
drivers/net/Kconfig | 1 +
1 file changed, 1 insertion(+)
Index: Linux/drivers/net/Kconfig
===================================================================
--- Linux.orig/drivers/net/Kconfig 2007-11-28 10:41:23.000000000
-0500
+++ Linux/drivers/net/Kconfig 2007-11-28 12:40:16.000000000 -0500
@@ -2606,6 +2606,7 @@ config TEHUTI
config BNX2X
tristate "Broadcom NetXtremeII 10Gb support"
depends on PCI
+ select ZLIB_INFLATE
help
This driver supports Broadcom NetXtremeII 10 gigabit Ethernet
cards.
To compile this driver as a module, choose M here: the module
^ permalink raw reply
* Re: [PATCH 7/7] [TFRC] New rx history code
From: Gerrit Renker @ 2007-12-05 13:34 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, netdev, dccp
In-Reply-To: <20071205120801.GN29972@ghostprotocols.net>
| > In the end this went back to the same questions and issues that were tackled between
| > February and March this year. I wish we could have had this dicussion then; it would
| > have made this email unnecessary.
|
| That is why it is important that the changeset comment collects these
| scattered notes and discussions. Think about in some years from now we
| will be left with a situation where we would have to look at many places
| to understando some aspects of what is in the code. While this happens
| and we have google for that I think that since you keep such detailed
| notes it is not a problem to get them in the changesets.
|
I agree, the changelogs are all a bit short. When condensing from 40 to 16 patches the changelogs
were also cut down, for fear of being too verbose. Will go through the patches in the next days and
see if/where this can be improved, that would probably have saved some trouble.
| > * But where I need to interact is when changes are made to the algorithm, even if these may
| > seem small. The underlying reasons that lead to the code may not be immediately clear,
| > but since this is a solution for a specific problem, there are also specific reasons; which
| > is why for algorithm changes (and underlying data structure) I'd ask you to discuss this first
| > before committing the patch.
|
| I did this for the RX handling, where changes were made. Did that for
| the TX but ended up commiting before comments from you, but I think its
| fair to say that the changes for the TX history were more organizational
| than in the essence of the algorithm.
|
It was a similar situation: the RFC patch came out on Thursday afternoon; I replied a bit hurriedly
on Friday that it seemed ok, but found the full confirmation only on Sunday after putting all
recent changes into the test tree. And yes, you made a good job of it.
^ permalink raw reply
* TCP event tracking via netlink...
From: David Miller @ 2007-12-05 13:30 UTC (permalink / raw)
To: ilpo.jarvinen; +Cc: netdev
Ilpo, I was pondering the kind of debugging one does to find
congestion control issues and even SACK bugs and it's currently too
painful because there is no standard way to track state changes.
I assume you're using something like carefully crafted printk's,
kprobes, or even ad-hoc statistic counters. That's what I used to do
:-)
With that in mind it occurred to me that we might want to do something
like a state change event generator.
Basically some application or even a daemon listens on this generic
netlink socket family we create. The header of each event packet
indicates what socket the event is for and then there is some state
information.
Then you can look at a tcpdump and this state dump side by side and
see what the kernel decided to do.
Now there is the question of granularity.
A very important consideration in this is that we want this thing to
be enabled in the distributions, therefore it must be cheap. Perhaps
one test at the end of the packet input processing.
So I say we pick some state to track (perhaps start with tcp_info)
and just push that at the end of every packet input run. Also,
we add some minimal filtering capability (match on specific IP
address and/or port, for example).
Maybe if we want to get really fancy we can have some more-expensive
debug mode where detailed specific events get generated via some
macros we can scatter all over the place. This won't be useful
for general user problem analysis, but it will be excellent for
developers.
Let me know if you think this is useful enough and I'll work on
an implementation we can start playing with.
^ permalink raw reply
* Re: [PATCH v2 0/3][BUG-FIX]: Test tree updates and bug fixes
From: Arnaldo Carvalho de Melo @ 2007-12-05 13:23 UTC (permalink / raw)
To: Gerrit Renker; +Cc: dccp, netdev
In-Reply-To: <11968535861367-git-send-email-gerrit@erg.abdn.ac.uk>
Em Wed, Dec 05, 2007 at 11:19:46AM +0000, Gerrit Renker escreveu:
> This patch removes the following redundancies:
> * ccid3_hc_rx_update_s() is only called for data packets (that is what it should be called for);
> * each call to ccid3_hc_rx_update_s() is wrapped inside a "if (is_data_packet)" test';
> * therefore testing each time if "len > 0" is redundant (pointed out by Arnaldo);
> * no other code calls this function, hence the inline function can go.
>
> Also replaced the first call to updating s with direct assignment of `payload_size'; this has also
> been pointed out by Arnaldo.
>
> Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
> ---
> net/dccp/ccids/ccid3.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> --- a/net/dccp/ccids/ccid3.c
> +++ b/net/dccp/ccids/ccid3.c
> @@ -654,12 +654,6 @@ static void ccid3_hc_rx_set_state(struct
> hcrx->ccid3hcrx_state = state;
> }
>
> -static inline void ccid3_hc_rx_update_s(struct ccid3_hc_rx_sock *hcrx, int len)
> -{
> - if (likely(len > 0)) /* don't update on empty packets (e.g. ACKs) */
> - hcrx->ccid3hcrx_s = tfrc_ewma(hcrx->ccid3hcrx_s, len, 9);
> -}
> -
> static void ccid3_hc_rx_send_feedback(struct sock *sk, struct sk_buff *skb,
> enum ccid3_fback_type fbtype)
> {
> @@ -788,8 +782,8 @@ static void ccid3_hc_rx_packet_recv(stru
> if (unlikely(hcrx->ccid3hcrx_state == TFRC_RSTATE_NO_DATA)) {
> if (is_data_packet) {
> do_feedback = FBACK_INITIAL;
> + hcrx->ccid3hcrx_s = payload_size;
> ccid3_hc_rx_set_state(sk, TFRC_RSTATE_DATA);
> - ccid3_hc_rx_update_s(hcrx, payload_size);
We have to set ccid3hcrx_bytes_recv to the payload_size here too, I'm
fixing this on the reworked patch that introduces the RX history.
> }
> goto update_records;
> }
> @@ -798,7 +792,10 @@ static void ccid3_hc_rx_packet_recv(stru
> goto done_receiving;
>
> if (is_data_packet) {
> - ccid3_hc_rx_update_s(hcrx, payload_size);
> + /*
> + * Update moving-average of s and the sum of received payload bytes
> + */
> + hcrx->ccid3hcrx_s = tfrc_ewma(hcrx->ccid3hcrx_s, payload_size, 9);
> hcrx->ccid3hcrx_bytes_recv += payload_size;
^ permalink raw reply
* Re: [PATCH] correct typo in __xfrm4_bundle_create()
From: David Miller @ 2007-12-05 13:20 UTC (permalink / raw)
To: ianbrn; +Cc: netdev
In-Reply-To: <d0383f90712050249q3bc0e249h4cb66dc0ea97f172@mail.gmail.com>
From: "Ian Brown" <ianbrn@gmail.com>
Date: Wed, 5 Dec 2007 12:49:52 +0200
> This is a typo correction in net/ipv4/xfrm4_policy.c; it should be
> "Copy neighbour" instead "Copy neighbout",
Your email client has corrupted the patch by adding
newlines among other things.
This happens often for gmail users, because by default
gmail does text formatting which you don't want it
to do for patches because it corrupts them.
Please use an attachment or some other method to prevent
this form happening so that you patch is not corrupted.
^ permalink raw reply
* Re: [PATCH] BRIDGE : br_fdb_fini() should be __exit_refok instead of __exit
From: David Miller @ 2007-12-05 13:07 UTC (permalink / raw)
To: dada1; +Cc: netdev
In-Reply-To: <47569C74.9020008@cosmosbay.com>
From: Eric Dumazet <dada1@cosmosbay.com>
Date: Wed, 05 Dec 2007 13:41:24 +0100
> Building last net-2.6.25 git tree gave me this warning :
>
> WARNING: vmlinux.o(.init.text+0x2a957): Section mismatch: reference to
> .exit.text:br_fdb_fini (between 'br_init' and 'br_fdb_init')
>
> This is because br_init() (marked __init) can call br_fdb_fini() (marked __exit).
>
> I am not sure the following is the right fix, since __exit_refok is quite new,
> and seldom used.
>
> [PATCH] BRIDGE : br_fdb_fini() should be __exit_refok instead of __exit
>
> Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
I have another patch like this from Andrew's -mm queue sitting
in my inbox, which I'll review and apply.
Thanks!
^ permalink raw reply
* Re: [PATCH 0/2] netem: trace enhancement
From: Patrick McHardy @ 2007-12-05 13:05 UTC (permalink / raw)
To: Ariane Keller
Cc: Ben Greear, Stephen Hemminger, netdev, herbert, Rainer Baumann
In-Reply-To: <4756A046.1070608@ee.ethz.ch>
Ariane Keller wrote:
> Thanks for your comments!
>
> Patrick McHardy wrote:
>
>>> But with this we would need the tcm_handle, tcm_parent arguments etc.
>>> which are not known in q_netem.c
>>> Therefore we would have to change the parse_qopt() function prototype
>>> in order to pass the whole "req" and not only the nlmsghdr.
>>
>> I assume you mean netem_init, parse_qopt is userspace. But I don't
>> see how that is related, emptying the buffer happens during packet
>> processing, right?
>
> Actually I meant parse_qopt from user space.
> If we would change that function prototype we would have the whole
> message header available in netem_parse_opt() and could pass this to the
> process which is responsible for sending the data to the kernel. This
> process would use this header every time it has to send new values to
> the netem_change() function in the kernel module.
You don't actually want to parse tc output in your program?
Just open a netlink socket and do the necessary processing
yourself, libnl makes this really easy.
> I thought about this because I was not aware of the qdisc_notify function.
> Anyway I've got some troubles with calling qdisc_notify.
> 1. I have to do a EXPORT_SYMBOL(qdisc_notify) (currently it is declared
> static in sch_api.c)
This is fine.
> 2. I'd like to call it from netem_enqueue(), which leads to a "sleeping
> function called from invalid context", since we are still in interrupt
> context. Therefore I think I have to put it in a workqueue.
Just change it to use gfp_any().
^ permalink raw reply
* Re: [PATCH 0/2] netem: trace enhancement
From: Ariane Keller @ 2007-12-05 12:57 UTC (permalink / raw)
To: Patrick McHardy
Cc: Ariane Keller, Ben Greear, Stephen Hemminger, netdev, herbert,
Rainer Baumann
In-Reply-To: <47556B20.2030700@trash.net>
Thanks for your comments!
Patrick McHardy wrote:
> Ariane Keller wrote:
>>
>>>> That sounds like it would also be possible using rtnetlink. You could
>>>> send out a notification whenever you switch the active buffer and have
>>>> userspace listen to these and replace the inactive one.
>>
>> I guess using rtnetlink is possible. However I'm not sure about how to
>> implement it:
>> The first thought was to use RTM_NEWQDISC to send the data to the
>> netem_change() function (similar to tc_qdisc_modify() ).
>
> That sounds reasonable.
>
>> But with this we would need the tcm_handle, tcm_parent arguments etc.
>> which are not known in q_netem.c
>> Therefore we would have to change the parse_qopt() function prototype
>> in order to pass the whole "req" and not only the nlmsghdr.
>
> I assume you mean netem_init, parse_qopt is userspace. But I don't
> see how that is related, emptying the buffer happens during packet
> processing, right?
Actually I meant parse_qopt from user space.
If we would change that function prototype we would have the whole
message header available in netem_parse_opt() and could pass this to the
process which is responsible for sending the data to the kernel. This
process would use this header every time it has to send new values to
the netem_change() function in the kernel module.
I thought about this because I was not aware of the qdisc_notify function.
Anyway I've got some troubles with calling qdisc_notify.
1. I have to do a EXPORT_SYMBOL(qdisc_notify) (currently it is declared
static in sch_api.c)
2. I'd like to call it from netem_enqueue(), which leads to a "sleeping
function called from invalid context", since we are still in interrupt
context. Therefore I think I have to put it in a workqueue.
I hope, this is ok.
>
> I guess I would simply change the qdisc_notify function to not
> require a struct nlmsghdr * (simply pass nlmsg_seq directly) and
> use that to send notifications. The netem dump function would
> add the buffer state. BTW, the parent class id is available in
> sch->parent, the handle in sch->handle, but qdisc_notify should
> take care of everything you need.
^ permalink raw reply
* Re: [PATCH] BRIDGE : br_fdb_fini() should be __exit_refok instead of __exit
From: Stephen Hemminger @ 2007-12-05 12:49 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, Linux Netdev List
In-Reply-To: <47569C74.9020008@cosmosbay.com>
On Wed, 05 Dec 2007 13:41:24 +0100
Eric Dumazet <dada1@cosmosbay.com> wrote:
> Hi David
>
> Building last net-2.6.25 git tree gave me this warning :
>
> WARNING: vmlinux.o(.init.text+0x2a957): Section mismatch: reference to
> .exit.text:br_fdb_fini (between 'br_init' and 'br_fdb_init')
>
> This is because br_init() (marked __init) can call br_fdb_fini() (marked __exit).
>
> I am not sure the following is the right fix, since __exit_refok is quite new,
> and seldom used.
>
> [PATCH] BRIDGE : br_fdb_fini() should be __exit_refok instead of __exit
>
> Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
>
Andrew put a patch in his tree to just take off the __exit.
That seems like the easiest fix for a one line function.
^ permalink raw reply
* [PATCH] BRIDGE : br_fdb_fini() should be __exit_refok instead of __exit
From: Eric Dumazet @ 2007-12-05 12:41 UTC (permalink / raw)
To: David S. Miller; +Cc: Linux Netdev List
[-- Attachment #1: Type: text/plain, Size: 507 bytes --]
Hi David
Building last net-2.6.25 git tree gave me this warning :
WARNING: vmlinux.o(.init.text+0x2a957): Section mismatch: reference to
.exit.text:br_fdb_fini (between 'br_init' and 'br_fdb_init')
This is because br_init() (marked __init) can call br_fdb_fini() (marked __exit).
I am not sure the following is the right fix, since __exit_refok is quite new,
and seldom used.
[PATCH] BRIDGE : br_fdb_fini() should be __exit_refok instead of __exit
Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
[-- Attachment #2: br_fdb_fini.patch --]
[-- Type: text/plain, Size: 311 bytes --]
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index eb57502..6caeba5 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -44,7 +44,7 @@ int __init br_fdb_init(void)
return 0;
}
-void __exit br_fdb_fini(void)
+void __exit_refok br_fdb_fini(void)
{
kmem_cache_destroy(br_fdb_cache);
}
^ permalink raw reply related
* Re: [PATCH 7/7] [TFRC] New rx history code
From: Arnaldo Carvalho de Melo @ 2007-12-05 12:08 UTC (permalink / raw)
To: Gerrit Renker, Arnaldo Carvalho de Melo, netdev, dccp
In-Reply-To: <20071205093530.GA5177@gerrit.erg.abdn.ac.uk>
Em Wed, Dec 05, 2007 at 09:35:30AM +0000, Gerrit Renker escreveu:
> Quoting Arnaldo:
> | Em Tue, Dec 04, 2007 at 06:55:17AM +0000, Gerrit Renker escreveu:
> | > NAK. You have changed the control flow of the algorithm and the underlying
> | > data structure. Originally it had been an array of pointers, and this had
> | > been a design decision right from the first submissions in March. From I
> | > of http://www.erg.abdn.ac.uk/users/gerrit/dccp/notes/ccid3_packet_reception/loss_detection/loss_detection_algorithm_notes.txt
> | >
> | > "1. 'ring' is an array of pointers rather than a contiguous area in
> | > memory. The reason is that, when having to swap adjacent entries
> | > to sort the history, it is easier to swap pointers than to copy
> | > (heavy-weight) history-entry data structs."
> | >
> | > But in your algorithm it becomes a normal linear array.
> | >
> | > As a consequence all subsequent code needs to be rewritten to use
> | > copying instead of swapping pointers. And there is a lot of that, since
> | > the loss detection code makes heavy use of swapping entries in order to
> | > cope with reordered and/or delayed packets.
> |
> | So lets not do that, the decision was made based on the RX history patch
> | alone, where, as pointed out, no swapping occurs.
> |
> | > This is not what I would call "entirely equivalent" as you claim. Your
> | > algorithm is fundamentally different, the control flow is not the same,
> | > nor are the underlying data structures.
> |
> | Its is equivalent up to what is in the tree, thank you for point out
> | that in subsequent patches it will be used.
> |
> | Had this design decision been made explicit in the changeset comment
> | that introduces the data structure I wouldn't have tried to reduce the
> | number of allocations.
> |
> | And you even said that it was a good decision when first reacting to
> | this change, which I saw as positive feedback for the RFC I sent.
> |
> That was my fault. Your solution "looked" much better to me but even I had forgotten
> that the algorithm is based on an array of pointers. When I finally sat down
And you wrote that code, I acted on looking the code together with
reading the changeset comment, to me it also looked much better to do
that, but as there were many changes, I sent an [RFC] message. It served
its purpose, as you after two iterations realised it was in fact not a
good idea as later an array of pointers is required. I should have taken
that swap routine as the definitive hint that indeed it was needed.
> and looked through the patch set I found the original notes from March and
> saw that using a linear array instead of an array of pointers will require quite
> a few changes and means changing the algorithm. I then thought through whether there
> could be any advantage in using a linear array as in this patch, but could find none.
> In the end this went back to the same questions and issues that were tackled between
> February and March this year. I wish we could have had this dicussion then; it would
> have made this email unnecessary.
That is why it is important that the changeset comment collects these
scattered notes and discussions. Think about in some years from now we
will be left with a situation where we would have to look at many places
to understando some aspects of what is in the code. While this happens
and we have google for that I think that since you keep such detailed
notes it is not a problem to get them in the changesets.
> I thank you for your replay and I am sorry if you took offense at my perhaps a little strong
> reaction, here is the essence what I tried to convey but what maybe did not succeed in conveying:
>
> * I am absolutely ok with you making changes to variable names, namespaces, file organisation,
> overall arrangement etc (as per previous email). You have experience and this will often be a
> benefit. For instance, you combined tfrc_entry_from_skb() with tfrc_rx_hist_update() to give
> a new function, tfrc_rx_hist_add_packet(). This is ok since entry_from_skb() is not otherwise
> used (and I only found this out when reading your patch). (On the other hand, since this is a
> 4-element ring buffer, it is no real adding, the same entry will frequently be overwritten,
> this is why it was initially called hist_update().)
>From the RX history API user perspective (CCID3 right now, others may
come) it is adding a packet to the history. In the past it went to a
list, now we have a ring and don't keep more than TFRC_NDUPACK + 1
entries, but this is an internal detail that users don't need to know.
> * I am also not so much concerned about credit. As long as the (changed) end result is as
> least as good as or hopefully better than the submitted input, the author name is a side
> issue; so I do think that your approach is sound and fair. The only place where credits
> are needed is for the SatSix project (www.ist-satsix.org) which funded this work. This is
> why some of the copyrights now included "University of Aberdeen". What in any case I'd like
> to add is at least the Signed-off-by: if it turns out to be a mess I want to contribute my
> part of responsibility.
And that, along with credit to you is being provided on the few patches
I change.
> * But where I need to interact is when changes are made to the algorithm, even if these may
> seem small. The underlying reasons that lead to the code may not be immediately clear,
> but since this is a solution for a specific problem, there are also specific reasons; which
> is why for algorithm changes (and underlying data structure) I'd ask you to discuss this first
> before committing the patch.
I did this for the RX handling, where changes were made. Did that for
the TX but ended up commiting before comments from you, but I think its
fair to say that the changes for the TX history were more organizational
than in the essence of the algorithm.
> * In part you are already doing this (message subject); it may be necessary to adapt the way
> of discussion/presentation. The subject is complex (spent a week with the flow chart only)
> and it is a lot - initially this had been 40 small patches.
>
> | > | I modified it just to try to make it closer to the existing API, hide details from
> | > | the CCIDs and fix a couple bugs found in the initial implementation.
> | > What is "a couple bugs"? So far you have pointed out only one problem and that was
> | > agreed, it can be fixed. But it remains a side issue, it is not a failure of the
> |
> | I pointed two problems one ended up being just the fact that tfrc_ewma
> | checks if the average is zero for every calculation.
> |
> With regard to the tfrc_ewma we need to consider the discussion as I think we are having a
> misunderstanding. It is necessary to
> (a) deal with the case that a peer may be sending zero-sized packets:
> - one solution is to simply omit the check "if (len > 0)" and declare 0-sized data
> - I think you have a point in that the len check is indeed redundant when we are
> already checking that this is a data packet
> packets as pathological (which is probably what they are);
> - a BUG_ON would be not such a good solution since 0-sized packets are legal;
> - my suggestion is to remove the "len > 0" test - then tfrc_ewma() can also be
> used directly, without the surrounding inline function wrapper.
>
> (b) reconsider the control flow: in your patch set you introduced a different control flow
> in the rx_packet_recv() function. I tried to figure out why this was introduced, there
> are two problems, one not immediately obvious at this moment:
> - it does not consider where and how loss detection is done (which is part of a later
> patch). This is a key point why the flow may seem strange: loss detection must be
> done on both non-data and data packets; there are several cases depending on whether
> loss previously occurred; the accounting must also be done in parallel for data packets
> (for instance s and bytes_recvd) and non-data packets (highest-received seqno is always
> - it is not equivalent with the initial one, when comparing this to the flowchart:
> http://www.erg.abdn.ac.uk/users/gerrit/dccp/notes/ccid3_packet_reception/reception-main-flowchart.png
I will revert to your original flow.
> | > I have provided documentation, written test modules, and am able to prove
> | > that the algorithm as submitted works reasonably correct. In addition, the
> | > behaviour has been verified using regression tests.
> | >
> | > I am not prepared to take your claims and expressions of "deepest
> | > respect" at face value since your actions - not your words - show that
> | > you are, in fact, not dealing respectfully with work which has taken
> | > months to complete and verify.
> | >
> | > During 9 months on dccp@vger you did not provide so much as a paragraph
> | > of feedback. Instead you mopped up code from the test tree, modified it
> | > according to your own whims and now, in the circle of your
> | > invitation-only buddies, start to talk about having discussions and
> | > iterations. The only iteration I can see is in fixing up the things you
> | > introduced, so it is not you who has to pay the price.
> | >
> | > Sorry, unless you can offer a more mature model of collaboration,
> | > consider me out of this and the patches summarily withdrawn. I am not
> | > prepared to throw away several months of work just because you feel
> | > inspired to do as you please with the work of others.
> |
> | Again you want to have your patches accepted as-is. Pointed to one case
> | where I gave you credit while improving on your work (TX history) and
> | another where the changes were up for review. I don't consider this a
> | warm welcome for me to finally dedicate time to this effort. Would you
> | prefer to continue working without help? I think we should encourage
> | more people to work on DCCP, you seem to think that changes to your work
> | are not acceptable.
> |
> Yes and no. The internal algorithm I would rather have accepted as-is, unless
> there are good reasons to change it. But this by no means means I want this
> waved through like a diplomat's car at the border: there are reasons why
> it is as it is, and I am happy to discuss these reasons; and if someone
> can point out a better solution, will accept that. It is just that this
> is a solution to a specific problem which so far has proven to work, and
> I therefore think that we can spent the time more efficiently by not
> fixing things that have already been fixed once: there are many more
> problems in DCCP (just recently you mentioned robustness of memory usage,
> then there is the ongoing discussion of how to integrate CCID4 as well);
> I think we could divide up work time with more benefit.
OK, I think I exaggerated on the changes and that you exaggerated on the
reaction, lets try to tone down both behaviours and we'll move on.
> | Perhaps others can comment on what is happening and help us find, as you
> | say, to find a more mature model of collaboration.
> |
> No need to be cynical, you have replied gracefully and with substance.
> For that I thank you. The topic is complicated, adapting how it is
> treated may improve the discussion - for instance, if it helps, we could
> switch back to smaller patch format as original, which you have already
> been introducing.
No intention, I was fearing your reaction would escalate and asking for
other people that are more experienced in maintainership, telling about
my and your mistakes in interacting could help. But I think we're going
back on track by ourselves, thank you.
- Arnaldo
^ permalink raw reply
* Re: [RFC][PATCHES 0/7]: Reorganization of RX history patches
From: Arnaldo Carvalho de Melo @ 2007-12-05 11:52 UTC (permalink / raw)
To: Gerrit Renker, Arnaldo Carvalho de Melo, netdev, dccp
In-Reply-To: <20071205102736.GF5177@gerrit.erg.abdn.ac.uk>
Note: removed Ingo from the CC list, I had added it first just because
he advocated reducing the number of mailing lists, so I wanted him to
know that we're trying to do that.
Em Wed, Dec 05, 2007 at 10:27:36AM +0000, Gerrit Renker escreveu:
> Today being Wednesday, below is some feedback after working through the patch set.
> Hope this is helpful.
>
> Patch #1:
> ---------
> Several new good points are introduced:
> - IP_DCCP_TFRC_DEBUG is made dependent on IP_DCCP_TFRC_DEBUG which is useful
> - the select from CONFIG_IP_DCCP_CCID3 => CONFIG_DCCP_TFRC_LIB
> - the cleanup action in tfrc_module_init() (when packet_history_init() fails)
> was previously missing, this is a good catch.
> Also a note: tfrc_pr_debug() is not currently used (but may be later should the
> code common to both CCID3 and CCID4 be shared).
OK
> Patches #2/#6:
> --------------
> Separated from main patch, no problems (were initially submitted in this format).
> I wonder whether switching back to smaller patch sizes is better?
Patches that do one thing that is logically separated from others are
preferred, as its analisys is made faster.
Patches that rewrite an existing functionality are done best when the
functions being replaced keep as much as possible the same name. For
instance, in the new RX handling code we need to alloc the RX hist
internal structures, in the previous implementation we also needed to do
that, it also needs to purge entries, as the old one needed. Previous
one was a linked list, the new one is a ring. As we go we can and should
add new APIs when needed, but trying to keep an API so that if in the
future we decide to rework the internal structures, this can be done in
a plugin fashion, without changing too much its users (CCIDs in this
case), so that we can get back to the old one with minor disruption to
the users if needed. Sometimes we manage to do that, some times we need
to improve upon the current API, but the goal remains the same.
> Patch #3:
> ---------
> Renames s/tfrc_tx_hist/tfrc_tx_hist_slab/g; no problems.
This was for consistency with the other slabs in DCCP:
[acme@doppio net-2.6.25]$ find net/dccp/ -name "*.c" | xargs grep 'struct kmem_cache'
net/dccp/ccids/lib/packet_history.c:static struct kmem_cache *tfrc_tx_hist_slab;
net/dccp/ccids/lib/packet_history.c:static struct kmem_cache *tfrc_rx_hist_slab;
net/dccp/ccids/lib/loss_interval.c:static struct kmem_cache *dccp_li_cachep __read_mostly;
net/dccp/ackvec.c:static struct kmem_cache *dccp_ackvec_slab;
net/dccp/ackvec.c:static struct kmem_cache *dccp_ackvec_record_slab;
net/dccp/ccid.c: struct kmem_cache *slab;
net/dccp/ccid.c:static void ccid_kmem_cache_destroy(struct kmem_cache *slab)
[acme@doppio net-2.6.25]$
> Patch #4:
> ---------
> packet_history_init() initialises both RX and TX history and is later called by the module_init()
> function in net/dccp/ccids/lib/tfrc.c. Just a suggestion, it is possible to simplify this further,
> by doing all the initialisation / cleanup in tfrc.c:
> int __init {rx,tx}_packet_history_init()
> {
> tfrc_{rx,tx}_hist_slab = kmem_cache_create("tfrc_{rx,tx}_hist", ...);
> return tfrc_{rx,tx}_hist_slab == NULL ? - ENOBUFS : 0;
> }
> and then call these successively in tfrc_module_init().
Idea here was to have each C source file to have a init module. Perhaps
we should try to break packet_history.c into tx_packet_history and
rx_packet_history.c. We can do that later to try to meet the goal of
being able to see what is being replaced.
> Patch #5:
> ---------
> The naming scheme introduced here is
> s/dccp_rx/tfrc_rx/g;
> s/dccphrx/tfrchrx/g;
> I wonder, while at it, would it be possible to extend this and extend this to other parts
> of the code? Basically this is the same discussion as earlier on dccp@vger with Leandro,
> who first came up with this naming scheme. There the same question came up and the result
> of the discussion was that a prefix of `tfrchrx' is cryptic; if something simpler is
> possible then it would be great.
As we did with the ackvecs, will do that later.
> Patch #7:
> ---------
> * ccid3_hc_rx_detect_loss() can be fully removed since no other code references it any
> further.
I left it to try have the diff not mixing up removal of its
implementation with the implementation of the function just after it,
ccid3_hx_rx_packet_recv. Will remove it later.
> * bytes_recv also counts the payload_size's of non-data packets, which is wrong (it should only
> sum up the sum of bytes transferred as data packets)
As said to you in private message I'll get back to the way you wrote, as
you mentioned that upcoming patches will make good use of that.
I'll try to restrain me to not do too many changes.
> * loss handling is not correctly taken care of: unlike in the other part, both data and non-data
> packets are used to detect loss (this is not correctly handled in the current Linux implementation).
See previous comment
> * tfrc_rx_hist_entry_data_packet() is not needed:
> - a similar function, called dccp_data_packet(), was introduced in patch 2/7
I thought about that, but dccp_data_packet is for skbs,
tfrc_rx_hist_entry_data_packet is for tfrc_rx_hist_entries, I guess we
should just make dccp_data_packet receive the packet type and not an
object that has a packet type field.
> - code compiles cleanly without tfrc_rx_hist_entry_data_packet()
> - all references to this function are deleted by patch 7/7
> * is another header file (net/dccp/ccids/lib/packet_history_internal.h) really necessary?
> - net/dccp/ccids/lib has already 3 x header file, 4 x source file
Idea here is to not expose data structures and functions that are used
only by the rx handling and loss history code.
> - with the removal of tfrc_rx_hist_entry_data_packet(), only struct tfrc_rx_hist_entry
> remains as the sole occupant of that file
> - how about hiding the internals of struct tfrc_rx_hist_entry by putting it into packet_history.c,
> as it was done previously in a similar way for the TX packet history?
See previous comment.
> * in ccid3_hc_rx_insert_options(), due to hunk-shifting or something else, there is still the
> call to dccp_insert_option_timestamp(sk, skb)
> --> this was meant to be removed by an earlier patch (which also removed the Elapsed Time option);
> --> in the original submission of this patch the call to dccp_insert_option_timestamp() did no
> longer appear (as can be found in the dccp@vger mailing list archives), and the test tree
> likewise does not use it;
> --> it can be removed with full confidence since no part of the code uses timestamps sent by the
> HC-receiver (only the HC-sender timestamps are used); and it is further not required by the
> spec to send HC-receiver timestamps (RFC 4342, section 6)
I removed that on purpose, it didn't look related to the main goal of
the patch nor was mentioned in the changeset, I'll add it as a separate
patch with the explanation you provided above (and in the past).
> * one of the two variables ccid3hcrx_tstamp_last_feedback and ccid3hcrx_tstamp_last_ack is
> redundant and can be removed (this may be part of a later patch); the variable ccid3hcrx_tstamp_last_feedback
> is very long (function is clear due to type of ktime_t).
will change that later. I.e. these things can be changed later, avoiding
such changes at this point reduces the size of the patch, which is
always good.
> * the inlines are a good idea regarding type safety, but I take it that we can now throw overboard the old rule
> of 80 characters per line? Due to the longer names of the inlines, some expressions end at column 98 (cf.
> tfrc_rx_hist_sample_rtt(); but to be honest I'd rather get rid of that function since the receiver-RTT
> sampling is notoriously inaccurate (wrong place to measure) and then there is little left to argue with the inlines).
We must try as much as possible to not have more than 80 characters per
line, but at the same time the kernel is ever growing, namespacing is
required to be able to use tools such as ctags, and even to help in
decoding oopses I'd say, as we can get more information on a backtrace.
> * with regard to RX history initialisation, would you be amicable to the idea of performing the RX/TX history, and
> loss intervals history in tfrc.c, as suggested for patch 1/7 (shortens the routines)?
See my previous comment.
> * tfrc_rx_hist_entry is lacking documentation (my fault, had been forgotten in the initial submission):
> /**
> * tfrc_rx_hist_entry - Store information about a single received packet
> * @ptype: the type (5.1) of the packet
> ...
> */
>
> * is it really necessary to give the field members of known structures long names such as
> tfrc_rx_hist_loss_prev(h)->tfrchrx_seqno?
> This is the same comment as per patch 5/7 and there has been an earlier discussion on dccp@vger where
> other developers (not just me) agreed that such long names are a burden to write; but we could leave that also for later.
See previous comment, we can do that as we did for the ackvec, in a
separate patch, but lets leave this for later now, trying to use
namespacing but with shorter names, that even looking cryptic can help
in searching for RX stamps, for instance, as we will iterate thru them
using the editor search routine and don't get distracted with the TX
tstamps, for instance, or search for them with grep.
- Arnaldo
^ permalink raw reply
* Re: [PATCH 3/3] [TCP]: NAGLE_PUSH seems to be a wrong way around
From: David Miller @ 2007-12-05 11:33 UTC (permalink / raw)
To: ilpo.jarvinen; +Cc: netdev
In-Reply-To: <Pine.LNX.4.64.0712051242310.18529@kivilampi-30.cs.helsinki.fi>
From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Wed, 5 Dec 2007 13:18:14 +0200 (EET)
> ...If I understood the very old history correctly, this bug was introduced
> in 2.4.0-test12 which inverted !tail incorrectly to nonagle==1.
>
> You can check include/net/tcp.h diffs from this commit:
> http://www.linux-mips.org/git?p=linux.git;a=commitdiff;h=c9c06167e7933d93a6e396174c68abf242294abb
>
> ...Though it's very large one. So I included only the relevant portion
> here below.
Thanks for the info, indeed it seems it has been broken this
way all this time.
^ permalink raw reply
* Re: [NETLINK]: Mark attribute construction exception unlikely
From: David Miller @ 2007-12-05 11:32 UTC (permalink / raw)
To: kaber; +Cc: netdev
In-Reply-To: <47568935.3040704@trash.net>
From: Patrick McHardy <kaber@trash.net>
Date: Wed, 05 Dec 2007 12:19:17 +0100
> [NETLINK]: Mark attribute construction exception unlikely
>
> Signed-off-by: Patrick McHardy <kaber@trash.net>
Applied to net-2.6.25, thanks Patrick.
^ permalink raw reply
* Re: [PATCH net-2.6 0/3]: Three TCP fixes
From: Ilpo Järvinen @ 2007-12-05 11:30 UTC (permalink / raw)
To: David Miller; +Cc: John Heffner, Netdev
In-Reply-To: <20071205.023030.125862645.davem@davemloft.net>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 2325 bytes --]
On Wed, 5 Dec 2007, David Miller wrote:
> From: John Heffner <jheffner@psc.edu>
> Date: Tue, 04 Dec 2007 13:42:41 -0500
>
> > Ilpo Järvinen wrote:
> > > ...I'm still to figure out why tcp_cwnd_down uses snd_ssthresh/2
> > > as lower bound even though the ssthresh was already halved,
> > > so snd_ssthresh should suffice.
> >
> > I remember this coming up at least once before, so it's probably worth a
> > comment in the code. Rate-halving attempts to actually reduce cwnd to
> > half the delivered window. Here, cwnd/4 (ssthresh/2) is a lower bound
> > on how far rate-halving can reduce cwnd. See the "Bounding Parameters"
> > section of <http://www.psc.edu/networking/papers/FACKnotes/current/>.
>
> I assume we're talking about the tcp_cwnd_min() usage in
> tcp_cwnd_down(), I don't see where it's dividing by two
> there.
...Ah, it's because I'm not using cubic which is not setting cwnd_min
callback. In tcp_cong.c it is:
/* Lower bound on congestion window with halving. */
u32 tcp_reno_min_cwnd(const struct sock *sk)
{
const struct tcp_sock *tp = tcp_sk(sk);
return tp->snd_ssthresh/2;
}
That snd_ssthresh is already halved when CA_Recovery/CA_CWR was
entered. Thus the amount of reduction is not 1/2 * orig_cwnd but
1/4 of it.
> Anyways, someone please enlighten me and please also cook
> up a patch to add the descriptive comment :-)
Not sure if a too simple patch here is correct thing to do... Matt has a
point regarding slow-start cwnd behavior. For FACK/SACK which can estimate
losses very accurately it works very well, where as NewReno discovers
those losses only one by one when cumulative ACKs arrive and until then,
the other losses are counted as outstanding segments. As a result, every
undiscovered loss is "missing from ACK clock" which in here results in the
bad performance because ACK clock slows down and (almost) dies out (ie.,
cumulative ACKs keep it going, though its rate is not very astonishing).
There might be some other bug to make it worse in the end of the recovery
but I'm yet to uncover it. With SACK/FACK, ACK clock keeps running at
expected rate and if cwnd becomes < ssthresh, the difference is quickly
regained in slow start.
...I've not yet decided what is the best way to deal with it but I'll try
to figure something out.
--
i.
^ permalink raw reply
* [PATCH v2 0/3][BUG-FIX]: Test tree updates and bug fixes
From: Gerrit Renker @ 2007-12-05 11:19 UTC (permalink / raw)
To: dccp; +Cc: netdev, Gerrit Renker
In-Reply-To: <11968535863312-git-send-email-gerrit@erg.abdn.ac.uk>
This revision fixes a bug present in the per-socket allocation of RX history
entries; identification of this bug is thanks to Arnaldo Carvalho de Melo.
The bug was in not deallocating history entries when the allocation of
one array element failed. The solution in this revised patch set is the
original one written by Arnaldo.
----------------------> Patch v2 <---------------------------------------------
[TFRC]: New RX history implementation
This provides a new, self-contained and generic RX history service for TFRC
based protocols.
Details:
* new data structure, initialisation and cleanup routines;
* allocation of dccp_rx_hist entries local to packet_history.c,
as a service exported by the dccp_tfrc_lib module.
* interface to automatically track highest-received seqno;
* receiver-based RTT estimation (needed for instance by RFC 3448, 6.3.1);
* a generic function to test for `data packets' as per RFC 4340, sec. 7.7.
Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
Signed-off-by: Ian McDonald <ian.mcdonald@jandi.co.nz>
---
net/dccp/ccids/lib/packet_history.c | 126 ++++++++++++++++++++++++++++---
net/dccp/ccids/lib/packet_history.h | 144 +++++++++++++++++++++++++++++++++++-
net/dccp/ccids/lib/tfrc_module.c | 26 ++++--
net/dccp/dccp.h | 12 +++
4 files changed, 285 insertions(+), 23 deletions(-)
--- a/net/dccp/ccids/lib/packet_history.h
+++ b/net/dccp/ccids/lib/packet_history.h
@@ -1,3 +1,5 @@
+#ifndef _DCCP_PKT_HIST_
+#define _DCCP_PKT_HIST_
/*
* Packet RX/TX history data structures and routines for TFRC-based protocols.
*
@@ -32,10 +34,6 @@
* along with this program; if not, write to the Free Software
* Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/
-
-#ifndef _DCCP_PKT_HIST_
-#define _DCCP_PKT_HIST_
-
#include <linux/ktime.h>
#include <linux/list.h>
#include <linux/slab.h>
@@ -43,9 +41,13 @@
/* Number of later packets received before one is considered lost */
#define TFRC_RECV_NUM_LATE_LOSS 3
+/* Number of packets to wait after a missing packet (RFC 4342, 6.1) */
+#define NDUPACK 3
#define TFRC_WIN_COUNT_PER_RTT 4
#define TFRC_WIN_COUNT_LIMIT 16
+/* Subtraction a-b modulo-16, respects circular wrap-around */
+#define SUB16(a,b) (((a) + 16 - (b)) & 0xF)
struct tfrc_tx_hist_entry;
@@ -66,6 +68,23 @@ struct dccp_rx_hist_entry {
ktime_t dccphrx_tstamp;
};
+
+/**
+ * tfrc_rx_hist_entry - Store information about a single received packet
+ * @seqno: DCCP packet sequence number
+ * @ccval: window counter value of packet (RFC 4342, 8.1)
+ * @ptype: the type (5.1) of the packet
+ * @ndp: the NDP count (if any) of the packet
+ * @stamp: actual receive time of packet
+ */
+struct tfrc_rx_hist_entry {
+ u64 seqno:48,
+ ccval:4,
+ ptype:4;
+ u32 ndp;
+ ktime_t stamp;
+};
+
struct dccp_rx_hist {
struct kmem_cache *dccprxh_slab;
};
@@ -73,6 +92,123 @@ struct dccp_rx_hist {
extern struct dccp_rx_hist *dccp_rx_hist_new(const char *name);
extern void dccp_rx_hist_delete(struct dccp_rx_hist *hist);
+/**
+ * tfrc_rx_hist - RX history structure for TFRC-based protocols
+ *
+ * @ring: Packet history for RTT sampling and loss detection
+ * @loss_count: Number of entries in circular history
+ * @loss_start: Movable index (for loss detection)
+ * @rtt_sample_prev: Used during RTT sampling, points to candidate entry
+ */
+struct tfrc_rx_hist {
+ struct tfrc_rx_hist_entry *ring[NDUPACK + 1];
+ u8 loss_count:2,
+ loss_start:2;
+#define rtt_sample_prev loss_start
+};
+
+/*
+ * Macros for loss detection.
+ * @loss_prev: entry with highest-received-seqno before loss was detected
+ * @hist_index: index to reach n-th entry after loss_start
+ * @hist_entry: return the n-th history entry after loss_start
+ * @last_rcv: entry with highest-received-seqno so far
+ */
+#define loss_prev(h) (h)->ring[(h)->loss_start]
+#define hist_index(h, n) (((h)->loss_start + (n)) & NDUPACK)
+#define hist_entry(h, n) (h)->ring[hist_index(h, n)]
+#define last_rcv(h) (h)->ring[hist_index(h, (h)->loss_count)]
+
+/*
+ * Macros to access history entries for RTT sampling.
+ * @rtt_last_s: reference entry to compute RTT samples against
+ * @rtt_prev_s: previously suitable (wrt rtt_last_s) RTT-sampling entry
+ */
+#define rtt_last_s(h) (h)->ring[0]
+#define rtt_prev_s(h) (h)->ring[(h)->rtt_sample_prev]
+
+/* initialise loss detection and disable RTT sampling */
+static inline void tfrc_rx_hist_loss_indicated(struct tfrc_rx_hist *h)
+{
+ h->loss_count = 1;
+}
+
+/* indicate whether previously a packet was detected missing */
+static inline int tfrc_rx_loss_pending(struct tfrc_rx_hist *h)
+{
+ return h->loss_count;
+}
+
+/* any data packets missing between last reception and skb ? */
+static inline int tfrc_rx_new_loss_indicated(struct tfrc_rx_hist *h,
+ struct sk_buff *skb, u32 ndp)
+{
+ int delta = dccp_delta_seqno(last_rcv(h)->seqno,
+ DCCP_SKB_CB(skb)->dccpd_seq);
+
+ if (delta > 1 && ndp < delta)
+ tfrc_rx_hist_loss_indicated(h);
+
+ return tfrc_rx_loss_pending(h);
+}
+
+/* has the packet contained in skb been seen before ? */
+static inline int tfrc_rx_duplicate(struct tfrc_rx_hist *h, struct sk_buff *skb)
+{
+ const u64 seq = DCCP_SKB_CB(skb)->dccpd_seq;
+ int i;
+
+ if (dccp_delta_seqno(loss_prev(h)->seqno, seq) <= 0)
+ return 1;
+
+ for (i = 1; i <= h->loss_count; i++)
+ if (hist_entry(h, i)->seqno == seq)
+ return 1;
+
+ return 0;
+}
+
+/* return the signed modulo-2^48 sequence number distance from entry e1 to e2 */
+static inline s64 tfrc_rx_hist_delta_seqno(struct tfrc_rx_hist *h, u8 e1, u8 e2)
+{
+ DCCP_BUG_ON(e1 > h->loss_count || e2 > h->loss_count);
+
+ return dccp_delta_seqno(hist_entry(h, e1)->seqno,
+ hist_entry(h, e2)->seqno);
+}
+
+static inline void tfrc_rx_hist_swap(struct tfrc_rx_hist_entry **a,
+ struct tfrc_rx_hist_entry **b)
+{
+ struct tfrc_rx_hist_entry *tmp = *a;
+
+ *a = *b;
+ *b = tmp;
+}
+
+static inline void tfrc_rx_hist_entry_from_skb(struct tfrc_rx_hist_entry *new,
+ struct sk_buff *skb, u32 ndp)
+{
+ const struct dccp_hdr *dh = dccp_hdr(skb);
+
+ new->seqno = DCCP_SKB_CB(skb)->dccpd_seq;
+ new->ccval = dh->dccph_ccval;
+ new->ptype = dh->dccph_type;
+ new->ndp = ndp;
+ new->stamp = ktime_get_real();
+}
+
+/* commit packet details of skb to history (record highest received seqno) */
+static inline void tfrc_rx_hist_update(struct tfrc_rx_hist *h,
+ struct sk_buff *skb, u32 ndp)
+{
+ tfrc_rx_hist_entry_from_skb(last_rcv(h), skb, ndp);
+}
+
+extern u32 tfrc_rx_sample_rtt(struct tfrc_rx_hist *, struct sk_buff *);
+extern int tfrc_rx_hist_init(struct tfrc_rx_hist *);
+extern void tfrc_rx_hist_cleanup(struct tfrc_rx_hist *);
+
static inline struct dccp_rx_hist_entry *
dccp_rx_hist_entry_new(struct dccp_rx_hist *hist,
const u32 ndp,
--- a/net/dccp/ccids/lib/packet_history.c
+++ b/net/dccp/ccids/lib/packet_history.c
@@ -34,7 +34,6 @@
* along with this program; if not, write to the Free Software
* Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/
-
#include <linux/string.h>
#include "packet_history.h"
@@ -55,6 +54,22 @@ struct tfrc_tx_hist_entry {
*/
static struct kmem_cache *tfrc_tx_hist;
+int __init tx_packet_history_init(void)
+{
+ tfrc_tx_hist = kmem_cache_create("tfrc_tx_hist",
+ sizeof(struct tfrc_tx_hist_entry), 0,
+ SLAB_HWCACHE_ALIGN, NULL);
+ return tfrc_tx_hist == NULL ? -ENOBUFS : 0;
+}
+
+void tx_packet_history_cleanup(void)
+{
+ if (tfrc_tx_hist != NULL) {
+ kmem_cache_destroy(tfrc_tx_hist);
+ tfrc_tx_hist = NULL;
+ }
+}
+
static struct tfrc_tx_hist_entry *
tfrc_tx_hist_find_entry(struct tfrc_tx_hist_entry *head, u64 seqno)
{
@@ -264,6 +279,55 @@ void dccp_rx_hist_add_packet(struct dccp
EXPORT_SYMBOL_GPL(dccp_rx_hist_add_packet);
+static struct kmem_cache *tfrc_rxh_cache;
+
+int __init rx_packet_history_init(void)
+{
+ tfrc_rxh_cache = kmem_cache_create("tfrc_rxh_cache",
+ sizeof(struct tfrc_rx_hist_entry),
+ 0, SLAB_HWCACHE_ALIGN, NULL);
+ return tfrc_rxh_cache == NULL ? -ENOBUFS : 0;
+}
+
+void rx_packet_history_cleanup(void)
+{
+ if (tfrc_rxh_cache != NULL) {
+ kmem_cache_destroy(tfrc_rxh_cache);
+ tfrc_rxh_cache = NULL;
+ }
+}
+
+int tfrc_rx_hist_init(struct tfrc_rx_hist *h)
+{
+ int i;
+
+ for (i = 0; i <= NDUPACK; i++) {
+ h->ring[i] = kmem_cache_alloc(tfrc_rxh_cache, GFP_ATOMIC);
+ if (h->ring[i] == NULL)
+ goto out_free;
+ }
+ h->loss_count = h->loss_start = 0;
+ return 0;
+
+out_free:
+ while (i-- != 0) {
+ kmem_cache_free(tfrc_rxh_cache, h->ring[i]);
+ h->ring[i] = 0;
+ }
+ return -ENOBUFS;
+}
+EXPORT_SYMBOL_GPL(tfrc_rx_hist_init);
+
+void tfrc_rx_hist_cleanup(struct tfrc_rx_hist *h)
+{
+ int i;
+
+ for (i=0; i <= NDUPACK; i++)
+ if (h->ring[i] != NULL)
+ kmem_cache_free(tfrc_rxh_cache, h->ring[i]);
+}
+EXPORT_SYMBOL_GPL(tfrc_rx_hist_cleanup);
+
void dccp_rx_hist_purge(struct dccp_rx_hist *hist, struct list_head *list)
{
struct dccp_rx_hist_entry *entry, *next;
@@ -276,18 +340,56 @@ void dccp_rx_hist_purge(struct dccp_rx_h
EXPORT_SYMBOL_GPL(dccp_rx_hist_purge);
-int __init packet_history_init(void)
+/**
+ * tfrc_rx_sample_rtt - Sample RTT from timestamp / CCVal
+ * Based on ideas presented in RFC 4342, 8.1. Returns 0 if it was not able
+ * to compute a sample with given data - calling function should check this.
+ */
+u32 tfrc_rx_sample_rtt(struct tfrc_rx_hist *h, struct sk_buff *skb)
{
- tfrc_tx_hist = kmem_cache_create("tfrc_tx_hist",
- sizeof(struct tfrc_tx_hist_entry), 0,
- SLAB_HWCACHE_ALIGN, NULL);
- return tfrc_tx_hist == NULL ? -ENOBUFS : 0;
-}
+ u32 sample = 0,
+ delta_v = SUB16(dccp_hdr(skb)->dccph_ccval, rtt_last_s(h)->ccval);
-void __exit packet_history_exit(void)
-{
- if (tfrc_tx_hist != NULL) {
- kmem_cache_destroy(tfrc_tx_hist);
- tfrc_tx_hist = NULL;
+ if (delta_v < 1 || delta_v > 4) { /* unsuitable CCVal delta */
+
+ if (h->rtt_sample_prev == 2) { /* previous candidate stored */
+ sample = SUB16(rtt_prev_s(h)->ccval,
+ rtt_last_s(h)->ccval);
+ if (sample)
+ sample = 4 / sample
+ * ktime_us_delta(rtt_prev_s(h)->stamp,
+ rtt_last_s(h)->stamp);
+ else /*
+ * FIXME: This condition is in principle not
+ * possible but occurs when CCID is used for
+ * two-way data traffic. I have tried to trace
+ * it, but the cause does not seem to be here.
+ */
+ DCCP_BUG("please report to dccp@vger.kernel.org"
+ " => prev = %u, last = %u",
+ rtt_prev_s(h)->ccval,
+ rtt_last_s(h)->ccval);
+ } else if (delta_v < 1) {
+ h->rtt_sample_prev = 1;
+ goto keep_ref_for_next_time;
+ }
+
+ } else if (delta_v == 4) { /* optimal match */
+ sample = ktime_to_us(net_timedelta(rtt_last_s(h)->stamp));
+
+ } else { /* suboptimal match */
+ h->rtt_sample_prev = 2;
+ goto keep_ref_for_next_time;
+ }
+
+ if (unlikely(sample > DCCP_SANE_RTT_MAX)) {
+ DCCP_WARN("RTT sample %u too large, using max\n", sample);
+ sample = DCCP_SANE_RTT_MAX;
}
+
+ h->rtt_sample_prev = 0; /* use current entry as next reference */
+keep_ref_for_next_time:
+
+ return sample;
}
+EXPORT_SYMBOL_GPL(tfrc_rx_sample_rtt);
--- a/net/dccp/dccp.h
+++ b/net/dccp/dccp.h
@@ -334,6 +334,7 @@ struct dccp_skb_cb {
#define DCCP_SKB_CB(__skb) ((struct dccp_skb_cb *)&((__skb)->cb[0]))
+/* RFC 4340, sec. 7.7 */
static inline int dccp_non_data_packet(const struct sk_buff *skb)
{
const __u8 type = DCCP_SKB_CB(skb)->dccpd_type;
@@ -346,6 +347,17 @@ static inline int dccp_non_data_packet(c
type == DCCP_PKT_SYNCACK;
}
+/* RFC 4340, sec. 7.7 */
+static inline int dccp_data_packet(const struct sk_buff *skb)
+{
+ const __u8 type = DCCP_SKB_CB(skb)->dccpd_type;
+
+ return type == DCCP_PKT_DATA ||
+ type == DCCP_PKT_DATAACK ||
+ type == DCCP_PKT_REQUEST ||
+ type == DCCP_PKT_RESPONSE;
+}
+
static inline int dccp_packet_without_ack(const struct sk_buff *skb)
{
const __u8 type = DCCP_SKB_CB(skb)->dccpd_type;
--- a/net/dccp/ccids/lib/tfrc_module.c
+++ b/net/dccp/ccids/lib/tfrc_module.c
@@ -8,8 +8,10 @@
#include "tfrc.h"
/* Initialisation / Clean-up routines */
-extern int packet_history_init(void);
-extern void packet_history_exit(void);
+extern int tx_packet_history_init(void);
+extern int rx_packet_history_init(void);
+extern void tx_packet_history_cleanup(void);
+extern void rx_packet_history_cleanup(void);
extern int dccp_li_init(void);
extern void dccp_li_exit(void);
@@ -24,11 +26,20 @@ static int __init tfrc_module_init(void)
{
int rc = dccp_li_init();
- if (rc == 0)
- rc = packet_history_init();
- if (rc == 0)
+ if (rc)
goto out;
+ rc = tx_packet_history_init();
+ if (rc)
+ goto out_free_loss_intervals;
+
+ rc = rx_packet_history_init();
+ if (rc)
+ goto out_free_tx_history;
+ return 0;
+
+out_free_tx_history:
+ tx_packet_history_cleanup();
out_free_loss_intervals:
dccp_li_exit();
out:
@@ -38,8 +49,9 @@ module_init(tfrc_module_init);
static void __exit tfrc_module_exit(void)
{
- packet_history_exit();
- dccp_li_exit();
+ rx_packet_history_cleanup();
+ tx_packet_history_cleanup();
+ dccp_li_exit();
}
module_exit(tfrc_module_exit);
^ permalink raw reply
* [PATCH v2 0/3][BUG-FIX]: Test tree updates and bug fixes
From: Gerrit Renker @ 2007-12-05 11:19 UTC (permalink / raw)
To: dccp; +Cc: netdev, Gerrit Renker
In-Reply-To: <11968535864083-git-send-email-gerrit@erg.abdn.ac.uk>
This fixes a problem in the initial revision of the patch: The loss interval
history was not de-allocated when the initialisation of the packet history
failed. The identification of this problem is also thanks due to Arnaldo.
The interdiff to the previous revision is:
--- b/net/dccp/ccids/lib/tfrc_module.c
+++ b/net/dccp/ccids/lib/tfrc_module.c
@@ -26,7 +26,12 @@
if (rc == 0)
rc = packet_history_init();
+ if (rc == 0)
+ goto out;
+out_free_loss_intervals:
+ dccp_li_exit();
+out:
return rc;
}
-------------------------> Patch v2 <---------------------------------------
[TFRC]: Provide central source file and debug facility
This patch changes the tfrc_lib module in the following manner:
(1) a dedicated tfrc_module source file (this is later populated
with TX/RX hist and LI Database routines);
(2) a dedicated tfrc_pr_debug macro with toggle switch `tfrc_debug'.
Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
Signed-off-by: Ian McDonald <ian.mcdonald@jandi.co.nz>
---
net/dccp/ccids/Kconfig | 12 +++++---
net/dccp/ccids/lib/Makefile | 3 +-
net/dccp/ccids/lib/loss_interval.c | 2 -
net/dccp/ccids/lib/packet_history.c | 28 ++------------------
net/dccp/ccids/lib/packet_history.h | 3 --
net/dccp/ccids/lib/tfrc.h | 17 +++++++++---
net/dccp/ccids/lib/tfrc_module.c | 50 ++++++++++++++++++++++++++++++++++++
7 files changed, 78 insertions(+), 37 deletions(-)
--- a/net/dccp/ccids/lib/packet_history.h
+++ b/net/dccp/ccids/lib/packet_history.h
@@ -39,8 +39,7 @@
#include <linux/ktime.h>
#include <linux/list.h>
#include <linux/slab.h>
-
-#include "../../dccp.h"
+#include "tfrc.h"
/* Number of later packets received before one is considered lost */
#define TFRC_RECV_NUM_LATE_LOSS 3
--- a/net/dccp/ccids/lib/packet_history.c
+++ b/net/dccp/ccids/lib/packet_history.c
@@ -35,7 +35,6 @@
* Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/
-#include <linux/module.h>
#include <linux/string.h>
#include "packet_history.h"
@@ -277,39 +276,18 @@ void dccp_rx_hist_purge(struct dccp_rx_h
EXPORT_SYMBOL_GPL(dccp_rx_hist_purge);
-extern int __init dccp_li_init(void);
-extern void dccp_li_exit(void);
-
-static __init int packet_history_init(void)
+int __init packet_history_init(void)
{
- if (dccp_li_init() != 0)
- goto out;
-
tfrc_tx_hist = kmem_cache_create("tfrc_tx_hist",
sizeof(struct tfrc_tx_hist_entry), 0,
SLAB_HWCACHE_ALIGN, NULL);
- if (tfrc_tx_hist == NULL)
- goto out_li_exit;
-
- return 0;
-out_li_exit:
- dccp_li_exit();
-out:
- return -ENOBUFS;
+ return tfrc_tx_hist == NULL ? -ENOBUFS : 0;
}
-module_init(packet_history_init);
-static __exit void packet_history_exit(void)
+void __exit packet_history_exit(void)
{
if (tfrc_tx_hist != NULL) {
kmem_cache_destroy(tfrc_tx_hist);
tfrc_tx_hist = NULL;
}
- dccp_li_exit();
}
-module_exit(packet_history_exit);
-
-MODULE_AUTHOR("Ian McDonald <ian.mcdonald@jandi.co.nz>, "
- "Arnaldo Carvalho de Melo <acme@ghostprotocols.net>");
-MODULE_DESCRIPTION("DCCP TFRC library");
-MODULE_LICENSE("GPL");
--- a/net/dccp/ccids/lib/tfrc.h
+++ b/net/dccp/ccids/lib/tfrc.h
@@ -3,10 +3,11 @@
/*
* net/dccp/ccids/lib/tfrc.h
*
- * Copyright (c) 2005 The University of Waikato, Hamilton, New Zealand.
- * Copyright (c) 2005 Ian McDonald <ian.mcdonald@jandi.co.nz>
- * Copyright (c) 2005 Arnaldo Carvalho de Melo <acme@conectiva.com.br>
- * Copyright (c) 2003 Nils-Erik Mattsson, Joacim Haggmark, Magnus Erixzon
+ * Copyright (c) 2007 The University of Aberdeen, Scotland, UK
+ * Copyright (c) 2005-6 The University of Waikato, Hamilton, New Zealand.
+ * Copyright (c) 2005-6 Ian McDonald <ian.mcdonald@jandi.co.nz>
+ * Copyright (c) 2005 Arnaldo Carvalho de Melo <acme@conectiva.com.br>
+ * Copyright (c) 2003 Nils-Erik Mattsson, Joacim Haggmark, Magnus Erixzon
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -15,6 +16,14 @@
*/
#include <linux/types.h>
#include <asm/div64.h>
+#include "../../dccp.h"
+
+#ifdef CONFIG_IP_DCCP_TFRC_DEBUG
+extern int tfrc_debug;
+#define tfrc_pr_debug(format, a...) DCCP_PR_DEBUG(tfrc_debug, format, ##a)
+#else
+#define tfrc_pr_debug(format, a...)
+#endif
/* integer-arithmetic divisions of type (a * 1000000)/b */
static inline u64 scaled_div(u64 a, u32 b)
--- /dev/null
+++ b/net/dccp/ccids/lib/tfrc_module.c
@@ -0,0 +1,50 @@
+/*
+ * TFRC: main module holding the pieces of the TFRC library together
+ *
+ * Copyright (c) 2007 The University of Aberdeen, Scotland, UK
+ */
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include "tfrc.h"
+
+/* Initialisation / Clean-up routines */
+extern int packet_history_init(void);
+extern void packet_history_exit(void);
+
+extern int dccp_li_init(void);
+extern void dccp_li_exit(void);
+
+#ifdef CONFIG_IP_DCCP_TFRC_DEBUG
+int tfrc_debug;
+module_param(tfrc_debug, bool, 0444);
+MODULE_PARM_DESC(tfrc_debug, "Enable debug messages");
+#endif
+
+static int __init tfrc_module_init(void)
+{
+ int rc = dccp_li_init();
+
+ if (rc == 0)
+ rc = packet_history_init();
+ if (rc == 0)
+ goto out;
+
+out_free_loss_intervals:
+ dccp_li_exit();
+out:
+ return rc;
+}
+module_init(tfrc_module_init);
+
+static void __exit tfrc_module_exit(void)
+{
+ packet_history_exit();
+ dccp_li_exit();
+}
+module_exit(tfrc_module_exit);
+
+MODULE_AUTHOR("Gerrit Renker <gerrit@erg.abdn.ac.uk>, "
+ "Ian McDonald <ian.mcdonald@jandi.co.nz>, "
+ "Arnaldo Carvalho de Melo <acme@ghostprotocols.net>");
+MODULE_DESCRIPTION("DCCP TFRC library");
+MODULE_LICENSE("GPL");
--- a/net/dccp/ccids/lib/Makefile
+++ b/net/dccp/ccids/lib/Makefile
@@ -1,3 +1,4 @@
obj-$(CONFIG_IP_DCCP_TFRC_LIB) += dccp_tfrc_lib.o
-dccp_tfrc_lib-y := loss_interval.o packet_history.o tfrc_equation.o
+dccp_tfrc_lib-y := tfrc_module.o tfrc_equation.o \
+ packet_history.o loss_interval.o
--- a/net/dccp/ccids/Kconfig
+++ b/net/dccp/ccids/Kconfig
@@ -63,10 +63,6 @@ config IP_DCCP_CCID3
If in doubt, say M.
-config IP_DCCP_TFRC_LIB
- depends on IP_DCCP_CCID3
- def_tristate IP_DCCP_CCID3
-
config IP_DCCP_CCID3_DEBUG
bool "CCID3 debugging messages"
depends on IP_DCCP_CCID3
@@ -110,5 +106,13 @@ config IP_DCCP_CCID3_RTO
is serious network congestion: experimenting with larger values should
therefore not be performed on WANs.
+# The TFRC Library: currently only has CCID 3 as customer
+config IP_DCCP_TFRC_LIB
+ depends on IP_DCCP_CCID3
+ def_tristate IP_DCCP_CCID3
+
+config IP_DCCP_TFRC_DEBUG
+ bool
+ default y if IP_DCCP_CCID3_DEBUG
endmenu
--- a/net/dccp/ccids/lib/loss_interval.c
+++ b/net/dccp/ccids/lib/loss_interval.c
@@ -285,7 +285,7 @@ int __init dccp_li_init(void)
return dccp_li_cachep == NULL ? -ENOBUFS : 0;
}
-void dccp_li_exit(void)
+void __exit dccp_li_exit(void)
{
if (dccp_li_cachep != NULL) {
kmem_cache_destroy(dccp_li_cachep);
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox