Hi Rao,
On Thu, Mar 22, 2018 at 7:02 PM, Rao Shoaib <rao.shoaib(a)oracle.com> wrote:
Hi Matthieu,
It does not seem like you have read through my emails, please see in-line
On 03/22/2018 03:28 AM, Matthieu Baerts wrote:
Hi Rao,
Thank you for your answer!
On Wed, Mar 21, 2018 at 9:59 PM, Rao Shoaib <rao.shoaib(a)oracle.com> wrote:
> On 03/21/2018 06:34 AM, Matthieu Baerts wrote:
>
> To be honest, I was also thinking about submitting a proposal for either
> a talk or a workshop.
> I also think it would be very interesting to join forces. It is maybe in
> my genes to propose that because the motto of my country is "Unity makes
> Strength" but I really think that it could help to have one single
> presentation about that. Note that in my country, we usually use this motto
> when we need to drink all available beers but that's something else :)
>
> If we did not want to co-operate we would not be sharing the code and
> asking for input. The reason I proposed individual submission is because it
> was claimed that going to the community with my idea would make everyone
> look like a fool.
>
> I understand why you have the impression that you have. In private emails
> I have raised that concern that the comments should be kept professional
> and technical. If someone has a non technical issue they are welcome to
> contact me privately.
>
> BTW I am a big Warriors (Basket Ball Team) fan and their slogan is --
> Strength in Numbers
>
Sorry I was not clear, indeed we are co-operating in term of sharing code
and info.
I also understand why Christoph and Mat were not more supportive regarding
your patch-set after their previous submission (TCP options framework).
Can you explain what do you understand because I don't.
*) The Patch that was submitted had nothing to do with MPTCP so talking
about it is irrelevant.
*) The claim that the patch was rejected because function pointers were
used is incorrect and I have highlighted David's comments. Additionally, I
forgot to mention that David was not interested in the patch even when it
was suggested that the feature that uses function pointers be removed. If I
am wrong, ask David, I am not buying the claim that we know or else stop
using this patch as an example.
*) The patch that we submitted was rejected without any serious review,
for example, it was claimed that it would blow up memory consumption. It
would be obvious to anyone looking at the code that the tables are shared.
I have addressed other comments and none are valid, please read my email.
One issue that was recently raised was that too many function pointers
indicate too much disruption in the tcp code. Well MPTCP is VERY intrusive
so there will be changes but this is not the way to evaluate. For example
if the code has functions called tcp__A ---- tcp_Z and I create functions
mptcp_A --- mptcp_Z and use function pointers to call the appropriate
function, does that mean too much disruption to tcp code because now I have
26 function pointers?
I am not saying that there are no issues with the code. There are several
issues with the code and I can see them myself but they are not serious
enough to not submit the patch as an RFC and start a dialog.
Here is what I know: before your submission, I tried to understand why the
TCP options framework was rejected and at that time, I though it was
because:
* it seems we should not simplify the way devs can add TCP options to have
them really think about the best way to do so. (yes, a bit surprising, not
really supportive as you can see ; after that you really don't know what we
can propose)
* it seems we should reduce function pointers usage to only when it is
really needed.
Knowing that, I understand why at this time, some people here may have been
confused about what to do because I was also maybe like them: surprised by
the first point of this feedback and a bit annoyed by the second one :-/
The situation is no longer the same, patches can be now read with different
preconceptions in mind.
Also note that from what I have read, I understood there would be a new
version of these RFC patches. I maybe misunderstood but maybe other people
are also waiting for a new version before looking at the patches in details
with the new info we recently got.
May you clarify if a new version will be shared?
From my point of view, I am fine to try to get more info using
different
names if it is needed. I am very glad to see that you first asked some
questions and you got answers from NetDev maintainer!
Thank you. However it was not welcomed and I was discouraged in a way that
I take offense to.
OK, sorry, English is not my first language. From these discussions, I
thought it was only discouraged to directly send big patches without asking
these questions you asked first.
But anyway, that's the past and we got clearer info from DaveM now.
I don't think I am wrong to say that we all want to upstream
MPTCP without
> having to rework all the current implementation. Like you, our main concern
> is to adapt both TCP and MPTCP stacks to fit with NetDev maintainer's
> expectations. These expectations are not always clear and we agree that
> NetDevConf is a very good place to discuss about that.
>
> Yup. The RFC patch is intended to do exactly that. When I talked to David
> and Eric about how they want MPTCP implemented, they wanted to see an
> implementation as a starting point. We produced one as no one else was
> working on it. However, it was summarily dismissed. The comments make it
> obvious that no one looked deeper.
>
I understand these guys would prefer to see code to comment it. But I
don't know how to produce a very small version of MPTCP implementation. If
it is not small, it seems it is very hard to get very interesting comments
about implementation details.
So indeed, it seems we have to produce code just to get comments. So what
you are doing seems totally good :-)
We have a lot of experience starting a project like this. I have tried to
share my ideas but eventually I had to take the bull by the horn.
But I guess the message that was shared on this ML is that we need to be
careful not to irritate NetDev community.
Where is this coming from, there is no such issue because this is how open
source works. I have talked to numerous engineers working on Linux and
everyone has said, submit an RFC and get comments. Can you point me to an
example ?
This sentence was linked to the next one. If we want to introduce new
indirect functions calls, we certainly need to clearly document them and
prepare questions from maintainers.
That's a sensible subject, especially for the moment. We could think that
by default, they will say no but it will depend of the
context/justification. Without the questions you asked and without specific
comments about that in the commit message and/or the code, I could
understand why people would not look at them in details and only focus on
these indirect functions calls. It will certainly help if they are
documented and if there are some dedicated lines in the cover letter to
prevent limiting the review to them.
We need to broach the subject of indirect function calls cautiously.
I
understand that after the previous reject regarding the TCP options
framework, it was difficult to see a bunch of new callbacks in TCP. So that
was good to see your question regarding this topic on NetDev ML before
sending patches!
Where is the proof that the patch was rejected because of function calls,
see my comment above. Either provide a proof or please stop using that
example.
Even if the patch was rejected because of function pointers, it is
irrelevant. David says the evaluation is done on a case by case basis.
Mellanox just added a new function callback in the fast path but that is
OK because it makes sense. David has also allowed new function calls in
tcp_transmit_skb() because they enhance the stack.
When I asked David he clearly pointed out that the reason for not allowing
function pointers is due to recent security vulnerabilities and nothing
else -- My performance analysis has not be questioned by anyone. Intel
folks can even take my comments and code to their performance guys and have
them review it and correct me.
Yes I agree with you. Last month, at the time of your submission and after
the feedback related to the TCP options framework, there were many
questions about them.
Now it is clearer: these indirect function calls are not forbidden but they
need to be well justified.
Is okay for you to summarise these answers by saying that: NetDev
community doesn't want a lot of new indirect function calls but they don't
have alternatives (yet) and we need them to have a clean MPTCP
implementation.
The good thing is that they are certainly looking at alternatives as well
and when a good one will be found, we can switch to this new solution. So
they will not reject patches just because there are new abstractions, right?
NetDev community has nothing to do with that. The security issues
discovered recently have resulted in turning off optimizations for indirect
function calls but only for certain CPU manufacturers. See below
https://lkml.org/lkml/2017/12/27/2
Also note that contrary to a statement made in some email, these
optimizations have always been enabled in the kernel until recently.
Indirect function pointers are integral to kernel programming.
Indeed, sorry for the mistake, I didn't want to talk about NetDev community
specifically here.
The current issue boils down to the fact that we were not treated as an
equal contributor. Our contribution was not given the consideration
that it
deserved. It was rejected by making some superfluous remarks. If function
pointers were an issue the right thing to do was to work the issue, not
reject the patch, considering that we have nothing else after one year.
Let's start by reviewing the code and providing true technical comments.
As far as function pointers are concerned, that is a minor issue and we
have the expertise to work with upstream to resolve it.
In short, we want to work with the community and that is why we are here,
but we want to participate as equal partners and only David Miller has the
right to reject anything patch without providing a technical reason and
engaging is a details technical dialog.
Again it is maybe because my English is not perfect but from what I
understood, the patches were not rejected. Some comments have been posted
and I thought a second version would come. If not, please simply mention
that there is no expected new version for the moment.
Also I think what could help is a diff between what you propose and what we
can find in the current MPTCP implementation (regarding the TCP stack of
course, not MPTCP itself which is not included in your patches). I think
that would help us (or just me?) to know what to look at in more details as
many here already know the current MPTCP implementation (I mean this one:
https://github.com/multipath-tcp/mptcp) and the code you modify is quite
big.
But I can be wrong, maybe only useful for me.
Feel free to come to our weekly meetings today to discuss about
these
technical topics :)
I missed the meeting but perhaps the next meeting. Can you put discussion
of our patch on the agenda. I see that others are working on another
implementation in parallel, for which no detail is available. That does not
make sense since we have one, let's first discuss what is wrong with the
one we have. It has all the features that folks claim they are working on.
I just added this topic in the public notes. Feel free to add any other
ones.
That would indeed be very good to discuss them at these meetings. I think
and hope face to face meetings help clarifying stuffs.
Thanks to these emails, I can see I didn't understand some topics like you
did, my bad. Certainly because there are here people from different areas,
not with the same languages and expressions.
But knowing that, that should be easy to fix, no? :)
Cheers,
Matthieu
--
[image: Tessares SA] <
http://www.tessares.net> Matthieu Baerts | R&D
Engineer
matthieu.baerts(a)tessares.net
Tessares SA | Hybrid Access Solutions
www.tessares.net
1 Avenue Jean Monnet, 1348 Louvain-la-Neuve, Belgium
<
https://www.google.com/maps?q=1+Avenue+Jean+Monnet,+1348+Ottignies-Louvai...
--
------------------------------
DISCLAIMER.
This email and any files transmitted with it are confidential and intended
solely for the use of the individual or entity to whom they are addressed.
If you have received this email in error please notify the system manager.
This message contains confidential information and is intended only for the
individual named. If you are not the named addressee you should not
disseminate, distribute or copy this e-mail. Please notify the sender
immediately by e-mail if you have received this e-mail by mistake and
delete this e-mail from your system. If you are not the intended recipient
you are notified that disclosing, copying, distributing or taking any
action in reliance on the contents of this information is strictly
prohibited.