You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2023/01/08 12:33:39 UTC

[GitHub] [nuttx] wengzhe opened a new pull request, #8056: virtio/net: Try fix virtnet logic

wengzhe opened a new pull request, #8056:
URL: https://github.com/apache/nuttx/pull/8056

   ## Summary
   Try to fix virtnet logic after https://github.com/apache/nuttx/pull/8011
   
   ## The main problem
   In `txpoll`, driver may do one of these three actions:
   1. Send or copyout the `d_buf` immediately and return 0 to indicate next poll may come immediately (like arch/sim/src/sim/sim_netdriver.c)
   https://github.com/apache/nuttx/blob/6214f3cde72d30a8b3a3bf37469eb0d97dad09d6/arch/sim/src/sim/sim_netdriver.c#L263-L276
   2. Save the `d_buf` pointer to somewhere else and prepare another `d_buf` to `dev` (like arch/xtensa/src/esp32/esp32_wlan.c)
   https://github.com/apache/nuttx/blob/6214f3cde72d30a8b3a3bf37469eb0d97dad09d6/arch/xtensa/src/esp32/esp32_wlan.c#L938-L946
   3. Just use the `d_buf` without preparing another, and return a non-zero value to stop polling (like error case in esp32_wlan.c, or bcmf_netdev.c, return an errno or 1 does the same thing.)
   https://github.com/apache/nuttx/blob/6214f3cde72d30a8b3a3bf37469eb0d97dad09d6/arch/xtensa/src/esp32/esp32_wlan.c#L941-L944 https://github.com/apache/nuttx/blob/6214f3cde72d30a8b3a3bf37469eb0d97dad09d6/drivers/wireless/ieee80211/bcm43xxx/bcmf_netdev.c#L401-L415
   
   It seems that we only have one tx buffer `g_pktbuf` in our virtnet, and we cannot send it out before the `txpoll` returns. When `txpoll` is called multiple times in a very short time, it finally results in sending only one packet out (but I don't know the actual reason). So maybe return a non-zero value to stop polling is the fastest way to fix.
   
   ## Timeline
   - Before IOB offload, `devif_poll` can only poll out one packet from each tcp connection at a time, so this problem may not be triggered by iperf.
   - After IOB offload, the `devif_poll` will just stop when the `txpoll` returns 0, which is actually not an expected action. virtnet driver works because `devif_poll` just stops.
   - After https://github.com/apache/nuttx/pull/8011 we may poll out as many packets as possible if the callback says it can handle (introduced by IOB offload but finally works after this PR)
   
   ## Other problems
   - Maybe it's still possible we re-write the data in `g_pktbuf` and try to send it before previous transmission is done if `txavail` is called at a very high freq. Also, I'm not sure how `txq`'s state will be in this case.
   - `virtnet_receive` has changed the `d_buf` to rx buffer but never change it back to `g_pktbuf`, also fix it.
     - But maybe not fixing it can reduce the posibility of writing into same buffer before sending it? Since we're writing into one of the rx buffer :)
   - `txdone` is not implemented, so packets may be stayed in net stack if `txavail` failed to queue a work, and waits until timeout expiry to come back to normal state.
   
   ## Testing
   Using the command in @masayuki2009 mentioned in https://github.com/apache/nuttx/pull/8011, iperf works.
   


-- 
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


[GitHub] [nuttx] wengzhe commented on pull request #8056: virtio/net: Try fix virtnet logic

Posted by GitBox <gi...@apache.org>.
wengzhe commented on PR #8056:
URL: https://github.com/apache/nuttx/pull/8056#issuecomment-1374825986

   @masayuki2009 @anchao A possible way to fix virtnet logic. But I'm not very familiar with virtio, so I'm not very sure if it's correct.


-- 
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


[GitHub] [nuttx] masayuki2009 merged pull request #8056: virtio/net: Try fix virtnet logic

Posted by GitBox <gi...@apache.org>.
masayuki2009 merged PR #8056:
URL: https://github.com/apache/nuttx/pull/8056


-- 
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


[GitHub] [nuttx] masayuki2009 commented on pull request #8056: virtio/net: Try fix virtnet logic

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #8056:
URL: https://github.com/apache/nuttx/pull/8056#issuecomment-1374991130

   @wengzhe 
   Thanks! I confirmed that this PR fixes the issue.
   


-- 
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