Date: Tue, 23 Sep 2014 12:56:17 +0900 From: Midori Kato <katoon@sfc.wide.ad.jp> To: Lawrence Stewart <lstewart@freebsd.org> Cc: "Eggert, Lars" <lars@netapp.com>, hiren panchasara <hiren.panchasara@gmail.com>, "freebsd-net@freebsd.org" <net@freebsd.org> Subject: Re: DCTCP implementation Message-ID: <5420EF61.5040509@sfc.wide.ad.jp> In-Reply-To: <540509C1.6090006@freebsd.org> References: <5338FF05.1020302@sfc.wide.ad.jp> <540509C1.6090006@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi Lawrence, Thank you so much for your response. I discuss KPI changes with Hiren over a month and understood your idea. The main point in your discussion thread was the KPI changes in cc_module. So, I describe about it as follows. As you mention, the changes in my implemtation is DCTCP specific ones. It makes sense not to add these two functions (ect_handler and ecnpkt_handler) into the main stream. At the same time, without these functions, it means that we cannot use DCTCP in FreeBSD. We have to find alternative ways to be putting into them. I think the compatibility with RFC3168 is the thing we should consider. The current KPI has not thought about it. There are two idea to achieve this: - append parameters to handle ECN in struct cc_var - change APIs to process ECN The former idea is easy and elegant. Which idea do you like when we implement DCTCP? And let me know if you have more better idea. I love to work on this to complete the DCTCP implementation. Hiren, if I miss something about our discussion, please provide additional explanation. Many thanks, -- Midori (9/2/14, 9:05 AM), Lawrence Stewart wrote: > Hi Midori (& Lars), > > On 03/31/14 16:37, Midori Kato wrote: >> Hi FreeBSD developpers, >> >> I'm Midori Kato. I'm working on the DCTCP implementation in the FreeBSD >> with Lars Eggert. I mail you because I would like to ask you a code >> review and testing. The attached patch is not good enough to test our >> code. Please give me your message. I will send an ECN marking >> implmenetation in dummynet and test scripts personally to you. > [snip] > > Firstly, please let me apologise for not providing follow up feedback > and review after our initial discussion of your work last year. It was > always my intention to come back to this but life has been particularly > crazy the past 12 months and I have been unsuccessful in my attempts to > keep a number of balls in the air. > > Hiren has been diligently working on shepherding your code into the > FreeBSD source tree and has been prodding me for a review which I > finally got around to yesterday. Unfortunately, my understanding is that > non developers are unable to interact with the code review system being > used, so we're moving the discussion back to the mailing list so that > you and others can participate. You can read the review discussion > history to date at [1]. > > Leaving editorial work on the documentation aside for the time being, > I'd like to discuss the KPI changes and stack integration aspects of the > patch to start with. Specifically: > > - The ect_handler KPI addition seems redundant to me and I believe the > patch can be simplified by removing it entirely. > > - The ecnpkt_handler KPI addition takes arguments which are too specific > to DCTCP, and is too indiscriminate in what it matches i.e. all packets > for ECN enabled connections. I would argue we either add a fully > generalist hook which would catch all packets of an ESTABLISHED > connection, and have the dctcp module check if ECN is enabled or not > before continuing processing (not my preferred option), or we extract > the essence of what dctcp_ecnpkt_handler() needs to do into a less > generic hook or bit of meta data passed in to an existing hook via > struct cc_var (I need to study the code a bit more, but will likely > strongly push for this option). > > - I don't believe the code correctly handles the case of dctcp being > used on connections which did not successfully negotiate ECN. > > There is more to discuss but I think getting these high level technical > things resolved first will help guide the remainder of the review > discussion. > > Cheers, > Lawrence > > [1] https://reviews.freebsd.org/D604
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5420EF61.5040509>