Re: [PATCH 00/16] RATP logic fixes and improvements
On Mon, Jun 19, 2017 at 11:07:09AM +0200, Aleksander Morgado wrote: > Hey, > > On 19/06/17 08:46, Sascha Hauer wrote: > >> I went through the RFC916 and ended up preparing a set of fixes and > >> improvements for the RATP logic in barebox. > >> Let me know what you think. > > As far as I can say the patches look good. It's quite a while since I > > last looked at the RATP code, so I can't really judge. To which extent > > are the patches tested? Have you explicitly tested for the corner cases > > you fix in each patch? You probably have tested against your new > > library. Have you also tested against the python implementation? > > I did test against bbremote, and also did several fixes there as well. > I haven't tested against the "ratp filesystem support" feature though, > maybe I should do that as well. That would be great, yes. > > Regarding which corner cases are tested, well, some of them apply to > code paths that I believe wouldn't really apply to barebox right now > (e.g. barebox doing active open at the same time as bbremote doing > active open), so that's hard to test. Indeed, yes. This path has been untested before aswell. > I could go one by one over each > patch and try to provide logs before/after applying the patch, how > about that? I don't think that's necessary. It might be worth noting to the commit messages which patches you made because there was something not working and which patches you made because the standard was not correctly implemented. > > BTW; how would you debug barebox (e.g. get the debug messages > generated) while testing the RATP link over the TTY? Right now I > validated the barebox behavior just by looking at which RATP messages > were returned to me. Use different consoles for debug messages and RATP. During development we used a board with two serial ports, but you could also use network console as an alternative console. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 00/16] RATP logic fixes and improvements
Hey, On 19/06/17 08:46, Sascha Hauer wrote: >> I went through the RFC916 and ended up preparing a set of fixes and >> improvements for the RATP logic in barebox. >> Let me know what you think. > As far as I can say the patches look good. It's quite a while since I > last looked at the RATP code, so I can't really judge. To which extent > are the patches tested? Have you explicitly tested for the corner cases > you fix in each patch? You probably have tested against your new > library. Have you also tested against the python implementation? I did test against bbremote, and also did several fixes there as well. I haven't tested against the "ratp filesystem support" feature though, maybe I should do that as well. Regarding which corner cases are tested, well, some of them apply to code paths that I believe wouldn't really apply to barebox right now (e.g. barebox doing active open at the same time as bbremote doing active open), so that's hard to test. I could go one by one over each patch and try to provide logs before/after applying the patch, how about that? BTW; how would you debug barebox (e.g. get the debug messages generated) while testing the RATP link over the TTY? Right now I validated the barebox behavior just by looking at which RATP messages were returned to me. -- Aleksander https://aleksander.es ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 00/16] RATP logic fixes and improvements
Hi Aleksander, On Thu, Jun 15, 2017 at 01:14:04PM +0200, Aleksander Morgado wrote: > Hey Sascha, > > I went through the RFC916 and ended up preparing a set of fixes and > improvements for the RATP logic in barebox. > Let me know what you think. As far as I can say the patches look good. It's quite a while since I last looked at the RATP code, so I can't really judge. To which extent are the patches tested? Have you explicitly tested for the corner cases you fix in each patch? You probably have tested against your new library. Have you also tested against the python implementation? Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH 00/16] RATP logic fixes and improvements
Hey Sascha, I went through the RFC916 and ended up preparing a set of fixes and improvements for the RATP logic in barebox. Let me know what you think. Cheers! Aleksander Morgado (16): ratp: add missing transition to SYN-RECEIVED in behavior B ratp: avoid unnecessary variable initializations ratp: send missing RST in behavior C2 ratp: add missing RST flag in behavior G ratp: completely ignore RST flagged packets in behavior G ratp: fix data presence check ratp: fix single byte sending flagged with SO ratp: remove bogus data checks in behavior C2 ratp: remove FIXME comment: FIN always requires ACK ratp: fix sending ACKs without data ratp: consolidate ratp_sn_expected() and ratp_an_expected() ratp: prefer using ratp_send_ack() in behaviour I1 ratp: send initial data in behaviour B if any pending ratp: don't ignore data that may arrive in behaviour H1 ratp: consolidate setting the next AN or SN flags ratp: user close may happen in SYN-RECEIVED state lib/ratp.c | 116 +++-- scripts/remote/ratp.py | 38 +++- 2 files changed, 78 insertions(+), 76 deletions(-) -- 2.13.1 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
