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 2020/05/15 00:21:10 UTC
[GitHub] [incubator-nuttx] masayuki2009 opened a new pull request #1052: drivers: wireless: Apply max payload size to gs2200m.c
masayuki2009 opened a new pull request #1052:
URL: https://github.com/apache/incubator-nuttx/pull/1052
## Summary
- This PR is to apply max payload size to gs2200m.c to avoid crash when sending a large packet. (e.g 4096)
- Also, remove unnecessary initialization for TX packet.
## Impact
- This PR affects gs2200m driver only. More specifically in case of sending a large packet.
## Testing
- Add CONFIG_LIB_SENDFILE_BUFSIZE=4096 to spresense:wifi
- Run webserver and access a large files on uSD card with Web browser from PC.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] masayuki2009 commented on a change in pull request #1052: drivers: wireless: Apply max payload size to gs2200m.c
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #1052:
URL: https://github.com/apache/incubator-nuttx/pull/1052#discussion_r425559820
##########
File path: drivers/wireless/gs2200m.c
##########
@@ -85,6 +85,7 @@
#define NRESPMSG (16 + 2)
#define MAX_PKT_LEN 1500
+#define MAX_PAYLOAD (MAX_PKT_LEN - 36)
Review comment:
>Anyway I'll define the value later.
Hi @patacongo,
I've just pushed the code again.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #1052: drivers: wireless: Apply max payload size to gs2200m.c
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #1052:
URL: https://github.com/apache/incubator-nuttx/pull/1052#issuecomment-629218983
> Thanks! So that mysterious 36 was not what I thought it was. Still better to give it a name to help understanding. Thanks!
@patacongo
Sorry for confusing you.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #1052: drivers: wireless: Apply max payload size to gs2200m.c
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1052:
URL: https://github.com/apache/incubator-nuttx/pull/1052#issuecomment-629220910
> Sorry for confusing you.
No problem... I am already confused most of the time anyway.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] masayuki2009 commented on a change in pull request #1052: drivers: wireless: Apply max payload size to gs2200m.c
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #1052:
URL: https://github.com/apache/incubator-nuttx/pull/1052#discussion_r425538667
##########
File path: drivers/wireless/gs2200m.c
##########
@@ -85,6 +85,7 @@
#define NRESPMSG (16 + 2)
#define MAX_PKT_LEN 1500
+#define MAX_PAYLOAD (MAX_PKT_LEN - 36)
Review comment:
Hello @patacongo
Thanks fot the comments.
Actually, It it not an IP header nor TCP header.
It's just a GS2200M command header for bulk data + guard size.
Anyway I'll define the value later.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] masayuki2009 commented on a change in pull request #1052: drivers: wireless: Apply max payload size to gs2200m.c
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #1052:
URL: https://github.com/apache/incubator-nuttx/pull/1052#discussion_r425538667
##########
File path: drivers/wireless/gs2200m.c
##########
@@ -85,6 +85,7 @@
#define NRESPMSG (16 + 2)
#define MAX_PKT_LEN 1500
+#define MAX_PAYLOAD (MAX_PKT_LEN - 36)
Review comment:
Hello @patacongo
Thanks fot the comments.
Actually, It it not an IP header nor TCP header.
It's just a GS2200M command header + guard size.
Anyway I'll define the value later.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo merged pull request #1052: drivers: wireless: Apply max payload size to gs2200m.c
Posted by GitBox <gi...@apache.org>.
patacongo merged pull request #1052:
URL: https://github.com/apache/incubator-nuttx/pull/1052
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #1052: drivers: wireless: Apply max payload size to gs2200m.c
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #1052:
URL: https://github.com/apache/incubator-nuttx/pull/1052#discussion_r425511715
##########
File path: drivers/wireless/gs2200m.c
##########
@@ -85,6 +85,7 @@
#define NRESPMSG (16 + 2)
#define MAX_PKT_LEN 1500
+#define MAX_PAYLOAD (MAX_PKT_LEN - 36)
Review comment:
You should not hardcode the number 36.
What is this numberanyway. It should:
The size of the Ethernet header (14) +
The size of IPv4 or IPv6 header (20 or 40) +
Any other headers. UDP? TCP?
See:
ethernet.h:#define ETH_HDRLEN 14 /* Header size: 2*6 + 2 */
ip.h:# define IPv4_HDRLEN 20 /* Size of IPv4 header (without options) */
ip.h:# define IPv6_HDRLEN 40 /* Size of IPv6 header */
The IP headers can be longer if there are options included in the IP header. The correct calculation can be a little complex.
But 36 does not see right? Is there a 2 byte FCS? Shouldn't this handle IPv6 too?
Let's not use hard-coded values.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #1052: drivers: wireless: Apply max payload size to gs2200m.c
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #1052:
URL: https://github.com/apache/incubator-nuttx/pull/1052#discussion_r425511715
##########
File path: drivers/wireless/gs2200m.c
##########
@@ -85,6 +85,7 @@
#define NRESPMSG (16 + 2)
#define MAX_PKT_LEN 1500
+#define MAX_PAYLOAD (MAX_PKT_LEN - 36)
Review comment:
You should not hardcode the number 36.
What is this numberanyway. It should:
The size of the Ethernet header (14) +
The size of IPv4 or IPv6 header (20 or 40) +
Any other headers.
See:
ethernet.h:#define ETH_HDRLEN 14 /* Header size: 2*6 + 2 */
ip.h:# define IPv4_HDRLEN 20 /* Size of IPv4 header (without options) */
ip.h:# define IPv6_HDRLEN 40 /* Size of IPv6 header */
36 does not see right? Is ther ea 2 byte FCS? Shouldn't this handle IPv6 too?
Let's not use hard-coded values.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #1052: drivers: wireless: Apply max payload size to gs2200m.c
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1052:
URL: https://github.com/apache/incubator-nuttx/pull/1052#issuecomment-629216992
Thanks! So that mysterious 36 was not what I thought it was. Still better to give it a name to help understanding. Thanks!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #1052: drivers: wireless: Apply max payload size to gs2200m.c
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #1052:
URL: https://github.com/apache/incubator-nuttx/pull/1052#discussion_r425511715
##########
File path: drivers/wireless/gs2200m.c
##########
@@ -85,6 +85,7 @@
#define NRESPMSG (16 + 2)
#define MAX_PKT_LEN 1500
+#define MAX_PAYLOAD (MAX_PKT_LEN - 36)
Review comment:
You should not hardcode the number 36.
What is this numberanyway. It should:
The size of the Ethernet header (14) +
The size of IPv4 or IPv6 header (20 or 40) +
Any other headers.
See:
ethernet.h:#define ETH_HDRLEN 14 /* Header size: 2*6 + 2 */
ip.h:# define IPv4_HDRLEN 20 /* Size of IPv4 header (without options) */
ip.h:# define IPv6_HDRLEN 40 /* Size of IPv6 header */
The IP headers can be longer if there are options included in the IP header. The correct calculation can be a little complex.
But 36 does not see right? Is there a 2 byte FCS? Shouldn't this handle IPv6 too?
Let's not use hard-coded values.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] masayuki2009 commented on a change in pull request #1052: drivers: wireless: Apply max payload size to gs2200m.c
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #1052:
URL: https://github.com/apache/incubator-nuttx/pull/1052#discussion_r425538667
##########
File path: drivers/wireless/gs2200m.c
##########
@@ -85,6 +85,7 @@
#define NRESPMSG (16 + 2)
#define MAX_PKT_LEN 1500
+#define MAX_PAYLOAD (MAX_PKT_LEN - 36)
Review comment:
Hello @patacongo
Thanks fot the comments.
Actually, It it not an IP header nor a TCP/UDP header.
It's just a GS2200M command header for bulk data + guard size.
Anyway I'll define the value later.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org