You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "Laczen (via GitHub)" <gi...@apache.org> on 2025/02/20 08:37:45 UTC
[PR] sixlowpan: improve tcp support. [nuttx]
Laczen opened a new pull request, #15877:
URL: https://github.com/apache/nuttx/pull/15877
Update the handling of tcp packets over sixlowpan. `tcp_ipv6_input()` can update the dev->d_iob, so use this to send the response.
## Summary
When using tcp over sixlowpan (e.g. for telnet) the handling of packets trough `tcp_ipv6_input()` can modify `dev->d_iob`. The PR makes sure that the correct `dev->d_iob` is used as returned packets.
## Impact
Allows using tcp over sixlowpan, e.g. telnet over sixlowpan. No impact on build system or other parts.
## Testing
Verified on esp32 using espnow as packet radio with telnetd and telnet.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] sixlowpan: improve tcp support. [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #15877:
URL: https://github.com/apache/nuttx/pull/15877#discussion_r1964065587
##########
net/devif/ipv6_input.c:
##########
@@ -463,9 +463,15 @@ static int ipv6_in(FAR struct net_driver_s *dev)
if ((dev->d_len > 0 && dev->d_lltype == NET_LL_IEEE802154) ||
(dev->d_len > 0 && dev->d_lltype == NET_LL_PKTRADIO))
{
+ /* Let 6LoWPAN handle the TCP output, the TCP output can be
+ * updated in dev->d_iob so we get this first.
+ */
+
+ FAR struct ipv6_hdr_s *ipv6r = IPv6BUF;
+
/* Let 6LoWPAN handle the TCP output */
- sixlowpan_tcp_send(dev, dev, ipv6);
+ sixlowpan_tcp_send(dev, dev, ipv6r);
Review Comment:
Ok, but it may more clear to reassign IPv6BUF to ipv6 instead ipv6r.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] sixlowpan: improve tcp support. [nuttx]
Posted by "Laczen (via GitHub)" <gi...@apache.org>.
Laczen commented on PR #15877:
URL: https://github.com/apache/nuttx/pull/15877#issuecomment-2676063437
> @Laczen please update the git commit message to include more info about the issue (like you explained in the Discord)
@acassis my knowledge of NuttX, 6lowpan, and tcp is too limited to include more info without making wrong assumptions and/or make the issue description disputable. I created a correction based upon a short study of what the code is doing (without a full understanding of the code), and the correction seems simple enough.
Making a wrong description of what is the issue would make the correction doubtable to future users. If there are other users in the NuttX community that do have the knowledge to properly describe the issue please assign them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] sixlowpan: improve tcp support. [nuttx]
Posted by "Laczen (via GitHub)" <gi...@apache.org>.
Laczen commented on PR #15877:
URL: https://github.com/apache/nuttx/pull/15877#issuecomment-2672060550
> @Laczen please include more context in the commit log message, tell us what happened and why this modification was necessary
@acassis, @xiaoxiang781216, I thought it was clear from the commit message and the summary. It is just like resetting `ipv6` to the correct iob. As tcp over sixlowpan was only possible since #15822 it was never discovered.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] sixlowpan: improve tcp support. [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #15877:
URL: https://github.com/apache/nuttx/pull/15877#discussion_r1963851321
##########
net/devif/ipv6_input.c:
##########
@@ -463,9 +463,15 @@ static int ipv6_in(FAR struct net_driver_s *dev)
if ((dev->d_len > 0 && dev->d_lltype == NET_LL_IEEE802154) ||
(dev->d_len > 0 && dev->d_lltype == NET_LL_PKTRADIO))
{
+ /* Let 6LoWPAN handle the TCP output, the TCP output can be
+ * updated in dev->d_iob so we get this first.
+ */
+
+ FAR struct ipv6_hdr_s *ipv6r = IPv6BUF;
+
/* Let 6LoWPAN handle the TCP output */
- sixlowpan_tcp_send(dev, dev, ipv6);
+ sixlowpan_tcp_send(dev, dev, ipv6r);
Review Comment:
what's the difference pass ipv6r instead ipv6?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] sixlowpan: improve tcp support. [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis merged PR #15877:
URL: https://github.com/apache/nuttx/pull/15877
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] sixlowpan: improve tcp support. [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on code in PR #15877:
URL: https://github.com/apache/nuttx/pull/15877#discussion_r1963869478
##########
net/devif/ipv6_input.c:
##########
@@ -463,9 +463,15 @@ static int ipv6_in(FAR struct net_driver_s *dev)
if ((dev->d_len > 0 && dev->d_lltype == NET_LL_IEEE802154) ||
(dev->d_len > 0 && dev->d_lltype == NET_LL_PKTRADIO))
{
+ /* Let 6LoWPAN handle the TCP output, the TCP output can be
+ * updated in dev->d_iob so we get this first.
+ */
+
+ FAR struct ipv6_hdr_s *ipv6r = IPv6BUF;
+
/* Let 6LoWPAN handle the TCP output */
- sixlowpan_tcp_send(dev, dev, ipv6);
+ sixlowpan_tcp_send(dev, dev, ipv6r);
Review Comment:
He explained it in the Discord channel, but missed to include the original issue and the explanation, this is the explanation of the cause of the issue:
"I have found the problem with tcp over sixlowpan. It seems that `tcp_ipv_input` sometimes creates a new iob and this new iob needs to be used in the response. It works (and is snappy), but I am unsure if all used iob's are freed."
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] sixlowpan: improve tcp support. [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on PR #15877:
URL: https://github.com/apache/nuttx/pull/15877#issuecomment-2675284297
@Laczen please update the git commit message to include more info about the issue (like you explained in the Discord)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] sixlowpan: improve tcp support. [nuttx]
Posted by "Laczen (via GitHub)" <gi...@apache.org>.
Laczen commented on PR #15877:
URL: https://github.com/apache/nuttx/pull/15877#issuecomment-2672523654
> @Laczen it is important to describe in the commit log message the problem you are fixing and what was causing it. Imagine someone that just ran gitk and saw your commit, they will ask the same question that Xiang did: why the use a new ipv6 variable instead of using the original?
>
> You comment there is doesn't explain it:
>
> ```
> /* Let 6LoWPAN handle the TCP output, the TCP output can be
> * updated in dev->d_iob so we get this first.
> */
> ```
>
> I think originally it was doing it as well, wasn't it?
I will change the code to reuse `ipv6`.
Originally it was not doing this, the pointer it set at the top of the routine and it points to the data in the iob that is in use at the start of the routine. The tcp processing routine however changes the iob that is used by dev. This can only be updated by defining it again using the macro.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] sixlowpan: improve tcp support. [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on PR #15877:
URL: https://github.com/apache/nuttx/pull/15877#issuecomment-2671681885
@Laczen please include more context in the commit log message, tell us what happened and why this modification was necessary
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] sixlowpan: improve tcp support. [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on PR #15877:
URL: https://github.com/apache/nuttx/pull/15877#issuecomment-2671682865
@wengzhe please take a look too
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] sixlowpan: improve tcp support. [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on PR #15877:
URL: https://github.com/apache/nuttx/pull/15877#issuecomment-2672418242
> > @Laczen please include more context in the commit log message, tell us what happened and why this modification was necessary
>
> @acassis, @xiaoxiang781216, I thought it was clear from the commit message and the summary. It is just like resetting `ipv6` to the correct iob. As tcp over sixlowpan was only possible since #15822 it was never discovered.
@Laczen it is important to describe in the commit log message the problem you are fixing and what was causing it. Imagine someone that just ran gitk and saw your commit, they will ask the same question that Xiang did: why the use a new ipv6 variable instead of using the original?
You comment there is doesn't explain it:
/* Let 6LoWPAN handle the TCP output, the TCP output can be
* updated in dev->d_iob so we get this first.
*/
I think originally it was doing it as well, wasn't it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org