Hi Rao,

On Thu, Mar 22, 2018 at 7:02 PM, Rao Shoaib <rao.shoaib@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@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
--
Tessares SAMatthieu Baerts | R&D Engineer
matthieu.baerts@tessares.net
Tessares SA | Hybrid Access Solutions
www.tessares.net 
1 Avenue Jean Monnet, 1348 Louvain-la-Neuve, Belgium



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.