Re: [Patch] fix packet loss and massive ping spikes with PPP multi-link

Previous thread: Re: [Xen-devel] [PATCH 0 of 12] PV on HVM Xen by Stefano Stabellini on Tuesday, May 25, 2010 - 2:55 am. (1 message)

Next thread: [PATCH] x86/pat: fix memory leak in free_memtype by Xiaotian Feng on Tuesday, May 25, 2010 - 2:52 am. (6 messages)
From: Richard Hartmann
Date: Tuesday, May 25, 2010 - 2:52 am

Hi all,

unfortunately, I lack the thread [1] in which John and Erik so I am
replying in my initial thread.

Unfortunately, our own patch is still in its initial state even though
we tried to clean it up to the Kernel's standard.

As you can see in [1], there is a real problem with multilink ppp. Thus,
I wanted to take the opportunity to ask for someone who is more familiar
with both C and the Linux Kernel than we are to have a go at getting
this code cleaned up and into mainline.


Any and all feedback appreciated,
Richard

[1] http://marc.info/?l=linux-ppp&m=127429724125841&w=2
--

From: Richard Hartmann
Date: Wednesday, May 26, 2010 - 1:47 am

==It seems LKML & netdev were dropped from the To list, re-adding==


rrsched and i do change when appropriate. As they are used as a cheap
way to get round robin, rrsched is not even initialized. One can argue
that this should be done, but as it literally does not matter where the


Leftover from various printks for debugging reasons.


Thanks for your feedback,
Richard
--

From: walter harms
Date: Friday, May 28, 2010 - 12:28 am

yep,
the problem is that you will trigger a warning "variable uninitialised".
And as programmer you are trained to spot such kind of code.
in short you violated "the rule of least surprise", simply set it to 99
and add a comment that the value does not matter because it is actualy a
random seed.
Basicly the same reason for the ppp->rrsched %= ppp->n_channels outside
the loop. 1. people/compiler are happy because they see the variable
is used. 2. no need to recalculate the if in a loop (never trust optimisers).

/*
perhaps rr_chanel is a better name ? round robin channel
that would requiere the changes but explain what it actualy is

please add a comment about that. i can garantee you someone will spot it
and remove either the pch->chan == NULL or the else.



just my 2 cents,
--

Previous thread: Re: [Xen-devel] [PATCH 0 of 12] PV on HVM Xen by Stefano Stabellini on Tuesday, May 25, 2010 - 2:55 am. (1 message)

Next thread: [PATCH] x86/pat: fix memory leak in free_memtype by Xiaotian Feng on Tuesday, May 25, 2010 - 2:52 am. (6 messages)