<div dir="auto"><div><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Feb 26, 2021, 11:49 AM Chris Johns <<a href="mailto:chrisj@rtems.org">chrisj@rtems.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 27/2/21 4:40 am, Vijay Kumar Banerjee wrote:<br>
> Hi all,<br>
> <br>
> Thanks for reviewing<br>
> <br>
> On Fri, Feb 26, 2021 at 10:13 AM Joel Sherrill <<a href="mailto:joel@rtems.org" target="_blank" rel="noreferrer">joel@rtems.org</a>> wrote:<br>
>><br>
>> Some odd questions that are mostly about making this a self-contained entity with no loose ends.<br>
>><br>
>> + Can the network demos be merged also?<br>
>><br>
> Are we talking about testsuites tests that use legacy networking? If<br>
> so, then I have already shifted the networking01.exe and will shift<br>
> other tests using the same approach.<br>
> <br>
>> + rtems-docs has the Network Users Guide which is legacy only. As a minimum, it needs to be renamed to have Legacy in the title. Better would be to convert it to markdown/asciidoc and just toss it in the legacy repo.<br>
>><br>
> This is a good point! I'll probably just keep it as a README in the<br>
> net-legacy repo.<br>
> <br>
>> + Gaisler needs a poke about the grlib NIC drivers. And Daniel expects it. File a ticket that it is time for them to support libbsd and assign it to him. :)<br>
>><br>
>> I'm ok with Chris' proposal to give notice  Grep'ing for NETWORK_DRIVER_NAME did turn up more files than I expected. Perhaps that is simply a list of driver names and attach functions for a readme in the repo. That's all that should have been in the bsp.h files.<br>
>><br>
> Yes, these are mostly bsp.h files. I'll file a ticket and post to<br>
> users and devel about it. There are also quite a few with<br>
> RTEMS_NETWORKING defined.<br>
> <br>
>> This is awesome work and much appreciated.<br>
>><br>
> Thank you. :)<br>
> <br>
>> On Fri, Feb 26, 2021 at 12:12 AM Gedare Bloom <<a href="mailto:gedare@rtems.org" target="_blank" rel="noreferrer">gedare@rtems.org</a>> wrote:<br>
>>><br>
>>> On Thu, Feb 25, 2021 at 6:06 PM Chris Johns <<a href="mailto:chrisj@rtems.org" target="_blank" rel="noreferrer">chrisj@rtems.org</a>> wrote:<br>
>>>><br>
>>>> On 26/2/21 4:49 am, Vijay Kumar Banerjee wrote:<br>
>>>>> The stand-alone repository is very close to completion now and I could<br>
>>>>> use the networking01 test with the standalone repo and it successfully<br>
>>>>> runs on pc-qemu.<br>
>>>><br>
>>>> Fantastic news.<br>
>>>><br>
>>>>> The following are the links to the branches with the<br>
>>>>> final version of the commits and I would really appreciate a review<br>
>>>>> and suggestions on what else needs to be done (I'm not sending patches<br>
>>>>> as they're big and would hit the devel limit):<br>
>>>><br>
>>>> I am fine reviewing the changes in the repos.<br>
>>>><br>
>>>>> RTEMS: <a href="https://git.rtems.org/vijay/rtems.git/log/?h=devel-no-libnet" rel="noreferrer noreferrer" target="_blank">https://git.rtems.org/vijay/rtems.git/log/?h=devel-no-libnet</a><br>
>>>><br>
>>>> Looks good. The only observation is a bisect will probability break as the<br>
>>>> nfsclient depends on rpc but I am OK with now things are.<br>
>>>><br>
>>>> I checked rtems_waf and I think it is OK dealing with no networking defined in<br>
>>>> the RTEMS opts header.<br>
>>>><br>
> Great!<br>
> <br>
>>>>> rtems-net-legacy: <a href="https://git.rtems.org/vijay/rtems-net-legacy.git/log/?h=main" rel="noreferrer noreferrer" target="_blank">https://git.rtems.org/vijay/rtems-net-legacy.git/log/?h=main</a><br>
>>>><br>
>>>> Would calling lnetwork.py netlegacy.py be a better match for that name? Closer<br>
>>>> to the repo naming.<br>
>>>><br>
> Sure, I'll rename it and force push.<br>
<br>
Thanks<br>
<br>
> <br>
>>>> Do the new python files need to pep8 formatted? :)<br>
>>>> [ <a href="https://gitlab.com/ita1024/waf/-/tree/master/playground/pep8" rel="noreferrer noreferrer" target="_blank">https://gitlab.com/ita1024/waf/-/tree/master/playground/pep8</a> ]<br>
>>>><br>
> pep8 does work for me when used manually but with waf module I'm<br>
> getting the following error:<br>
> <br>
> ```<br>
>   File "/home/vijay/.local/lib/python3.8/site-packages/pep8.py", line<br>
> 253, in maximum_line_length<br>
>     if length > options.max_line_length:<br>
> AttributeError: 'Values' object has no attribute 'max_line_length'<br>
> ```<br>
> This is strange because it looks like an error in the pep8 module itself.<br>
> <br>
> I have tried different versions of pep8 and it looks like each version<br>
> has a different error. I think this needs some work from the waf side<br>
> to get it working with the new pycodestyle instead of the pep8 module.<br>
<br>
Running manually is fine.<br>
<br>
>>>> In bsp_drivers.py is there a waf node way to find the sources rather than<br>
>>>> python os walk?<br>
>>>> [ <a href="https://waf.io/apidocs/Node.html#waflib.Node.Node.ant_glob" rel="noreferrer noreferrer" target="_blank">https://waf.io/apidocs/Node.html#waflib.Node.Node.ant_glob</a> ]<br>
>>>><br>
> I gave it a few shots but it didn't quite work out well for me.  I do<br>
> get the generator from it but for some reason, it's not building. We<br>
> would also need the list of headers for the install, for which I think<br>
> os.walk might be needed.<br>
> <br>
> Is this a blocker to merging? If so, then I can put more time into it<br>
> and try to get it working. If you want it as an optimization, maybe we<br>
> could merge it and file a ticket? I can take more time and fix it<br>
> later.<br>
<br>
Thanks for looking, the os.walk is fine.<br>
<br>
>>>> Should the README reference rtems_waf and all the configure options it supports?<br>
>>>><br>
> This is a good point, The README needs some update for sure. I'll<br>
> follow the README.waf from other repos and follow the same pattern.<br>
> <br>
>>>> Do we need a LICENSE file?<br>
>>>><br>
> Do we?<br>
<br>
I think it helps but I am not sure what it would contain. Joel?<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">It would be hard to have a completely accurate one if it has to account for every BSD file with unique copyright holders a d two vs three paragraph license </div><div dir="auto"><br></div><div dir="auto">Perhaps descriptive contents that says it contains code under multiple permissive licenses. See the specific files for details.</div><div dir="auto"><br></div><div dir="auto">It's good to have one but not worth the effort to do more than that.</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
>>>>><br>
>>>>> There are at least two things that need to be done:<br>
>>>>> 1. Shift the tests like mghttpd01 that use the libnetworking stack, to<br>
>>>>> the standalone repo like networking01<br>
>>>><br>
>>>> OK<br>
> I'll do it along with the README and send it for review.<br>
<br>
Thanks<br>
<br>
>>>><br>
>>>>> 2. There are still codes that use the #ifdef RTEMS_NETWORKING. What do<br>
>>>>> we want to do about those?<br>
>>>><br>
>>>> How many BSPs/places/areas are we talking about?<br>
>>>><br>
>>>> Would it be practical to add a cgit link to a ticket and then post an email to<br>
>>>> user and devel stating those interested in BSPs x,y,z to review the ticket? We<br>
>>>> then wait a week and after that the remaining defines are removed.<br>
>>>><br>
> <br>
> grep shows this:<br>
> ```<br>
> cpukit/libfs/src/ftpfs/tftpDriver.c:#ifdef RTEMS_NETWORKING<br>
> cpukit/libfs/src/ftpfs/tftpDriver.c:#ifdef RTEMS_NETWORKING<br>
> cpukit/libfs/src/ftpfs/tftpDriver.c:#ifdef RTEMS_NETWORKING<br>
> cpukit/libfs/src/ftpfs/tftpDriver.c:#ifdef RTEMS_NETWORKING<br>
> cpukit/telnetd/telnetd.c:#ifdef RTEMS_NETWORKING<br>
> cpukit/telnetd/telnetd.c:#ifdef RTEMS_NETWORKING<br>
> cpukit/ftpd/ftpd.c:#ifndef RTEMS_NETWORKING<br>
> cpukit/libtest/testbeginend.c:#if RTEMS_NETWORKING<br>
> cpukit/libtest/testbeginend.c:    " RTEMS_NETWORKING"<br>
> cpukit/include/rtems/monitor.h:#if defined(RTEMS_NETWORKING)<br>
> cpukit/include/rtems/shellconfig.h:#if RTEMS_NETWORKING<br>
> cpukit/include/rtems/shellconfig.h:    #if RTEMS_NETWORKING<br>
> spec/build/testsuites/samples/pppd.yml:  - RTEMS_NETWORKING<br>
> spec/build/testsuites/samples/loopback.yml:- RTEMS_NETWORKING<br>
> spec/build/testsuites/libtests/telnetd01.yml:- RTEMS_NETWORKING<br>
> spec/build/testsuites/libtests/mghttpd01.yml:  - RTEMS_NETWORKING<br>
> spec/build/testsuites/libtests/syscall01.yml:- RTEMS_NETWORKING<br>
> spec/build/testsuites/libtests/networking01.yml:- RTEMS_NETWORKING<br>
> spec/build/testsuites/libtests/ftp01.yml:- RTEMS_NETWORKING<br>
> spec/build/bsps/powerpc/motorola_powerpc/objqemunet.yml:  - RTEMS_NETWORKING<br>
> spec/build/bsps/objnetnosmp.yml:  - RTEMS_NETWORKING<br>
> spec/build/bsps/riscv/griscv/objnetnosmp.yml:  - RTEMS_NETWORKING<br>
> testsuites/libtests/record01/init.c:#ifdef RTEMS_NETWORKING<br>
> testsuites/libtests/record01/init.c:#ifdef RTEMS_NETWORKING<br>
> testsuites/libtests/record01/init.c:#ifdef RTEMS_NETWORKING<br>
> testsuites/libtests/record01/init.c:#ifdef RTEMS_NETWORKING<br>
> bsps/powerpc/beatnik/include/bsp.h:#if defined(RTEMS_NETWORKING)<br>
> bsps/lm32/milkymist/include/bsp.h:#if defined(RTEMS_NETWORKING)<br>
> bsps/lm32/lm32_evr/include/bsp.h:#if defined(RTEMS_NETWORKING)<br>
> <br>
> ```<br>
> I can already see a small issue from my side. The networking01.yml is<br>
> there. That will go away, along with some other testsuites yml that<br>
> I'll shift now. Do we need ticket for the rest?<br>
<br>
A single ticket for this task is fine.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">I would say we always build with networking in in the future. The standard headers are there always. </div><div dir="auto"><br></div><div dir="auto">Perhaps ones in telnetd and similar can go away if that decision is made versus saying it disables common network services.</div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> <br>
>>>> Do we have a ticket for this task?<br>
>>>><br>
>>> <a href="https://devel.rtems.org/ticket/3850" rel="noreferrer noreferrer" target="_blank">https://devel.rtems.org/ticket/3850</a><br>
>>><br>
> Thanks.<br>
<br>
Thanks.<br>
<br>
Chris<br>
<br>
> <br>
>>> I'll let Vijay answer the rest.<br>
>>><br>
>>>>> Apart from these two points above, do the commits and the standalone<br>
>>>>> repo look OK (close to mergeable)?<br>
>>>><br>
>>>> For me this is very close and a welcomed change for RTEMS 6. Really nice work.<br>
>>>><br>
> Thank you!<br>
> <br>
> Best regards,<br>
> Vijay<br>
> <br>
>>>> Thanks<br>
>>>> Chris<br>
>>>> _______________________________________________<br>
>>>> devel mailing list<br>
>>>> <a href="mailto:devel@rtems.org" target="_blank" rel="noreferrer">devel@rtems.org</a><br>
>>>> <a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a><br>
>>> _______________________________________________<br>
>>> devel mailing list<br>
>>> <a href="mailto:devel@rtems.org" target="_blank" rel="noreferrer">devel@rtems.org</a><br>
>>> <a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a><br>
>><br>
>> _______________________________________________<br>
>> devel mailing list<br>
>> <a href="mailto:devel@rtems.org" target="_blank" rel="noreferrer">devel@rtems.org</a><br>
>> <a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a><br>
> _______________________________________________<br>
> devel mailing list<br>
> <a href="mailto:devel@rtems.org" target="_blank" rel="noreferrer">devel@rtems.org</a><br>
> <a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a><br>
> <br>
</blockquote></div></div></div>