net: tcp: Parse and store send MSS #4657
Conversation
423f102 to
efba11c
Compare
subsys/net/ip/tcp.c
Outdated
| u8_t opt, optlen; | ||
|
|
||
| while (opt_totlen) { | ||
| frag = net_frag_read(frag, pos, &pos, sizeof(opt), &opt); |
There was a problem hiding this comment.
We need to check the return values here so that we now whether the packet is malformed or not.
There was a problem hiding this comment.
Along the discussion on IRC, I'm adding single check at the top of function.
| goto error; | ||
| } | ||
|
|
||
| frag = net_frag_read(frag, pos, &pos, sizeof(optlen), &optlen); |
There was a problem hiding this comment.
Along the discussion on IRC, I'm adding single check at the top of function.
| if (optlen != 2) { | ||
| goto error; | ||
| } | ||
| frag = net_frag_read(frag, pos, &pos, |
There was a problem hiding this comment.
Along the discussion on IRC, I'm adding single check at the top of function.
subsys/net/ip/tcp.c
Outdated
| optlen, (u8_t *)mss); | ||
| break; | ||
| default: | ||
| frag = net_frag_skip(frag, pos, &pos, optlen); |
There was a problem hiding this comment.
Along the discussion on IRC, I'm adding single check at the top of function.
subsys/net/ip/tcp.c
Outdated
| @@ -1230,3 +1231,71 @@ struct net_buf *net_tcp_set_chksum(struct net_pkt *pkt, struct net_buf *frag) | |||
|
|
|||
| return frag; | |||
| } | |||
|
|
|||
| int tcp_parse_opts(struct net_pkt *pkt, int opt_totlen, u16_t *mss) | |||
There was a problem hiding this comment.
Better would be to give also the option we are interested in like NET_TCP_MSS_OPT as a parameter, and then return the value according to wanted option.
There was a problem hiding this comment.
I don't think that would scale or follow any realistic processing routine. Realistically, we will always parse options once, and get all the interesting options during this parse too. So, I had an idea to introduce struct tcp_options, but it would contain only MSS now. So, I considered that over-engineering and applied YAGNI - indeed, it doesn't make sense to remove unused things in one place (#4653 (comment)) and add unused stuff in another.
There was a problem hiding this comment.
Your "struct tcp_options" sounds like a good plan even if it would contain only MSS for now. Now the function is quite misleading as it only parses mss but you call it tcp_parse_opts(). I did not understand your last sentence, what we are removing and adding?
There was a problem hiding this comment.
I did not understand your last sentence, what we are removing and adding?
What I meant that in #4653, an used structure field being reported, and e.g. @lpereira's suggestion was "remove it". Then we could follow that idea and not add fields/structures unless really needed.
Anyway, if you find that introducing struct tcp_options would deconfuse this patch, let me just do it.
subsys/net/ip/tcp.c
Outdated
| } | ||
|
|
||
| opt_totlen -= optlen; | ||
|
|
There was a problem hiding this comment.
This extra empty line can be removed
subsys/net/ip/tcp.c
Outdated
| u16_t pos = net_pkt_ip_hdr_len(pkt) | ||
| + net_pkt_ipv6_ext_len(pkt) | ||
| + sizeof(struct net_tcp_hdr); | ||
|
|
ba3584e to
635e6bc
Compare
MSS is Maximum Segment Size (data payload) of TCP. In SYN packets, each side of the connection shares an MSS it wants to use (receive) via the corresponding TCP option. If the option is not available, the RFC mandates use of the value 536. This patch handles storage of the send MSS (in the TCP structure, in TCP backlog), with follow up patch handling actual parsing it from the SYN TCP options. Signed-off-by: Paul Sokolovsky <paul.sokolovsky@linaro.org>
Calculates full TCP header length (with options). Macro introduced for reuse, to avoid "magic formula". (E.g., it would be needed to parse TCP options). Signed-off-by: Paul Sokolovsky <paul.sokolovsky@linaro.org>
Add a generic function for TCP option parsing. So far we're interested only in MSS option value, so that's what it handles. Use it to parse MSS value in net_context incoming SYN packet handler. Signed-off-by: Paul Sokolovsky <paul.sokolovsky@linaro.org>
635e6bc to
9890e72
Compare
|
@jukkar : Updated to add |
MSS is Maximum Segment Size (data payload) of TCP. In SYN packets,
each side of the connection shares an MSS it wants to use (receive)
via the corresponding TCP option. If the option is not available,
the RFC mandates use of the value 536.
This patchset is a prerequisite for proper implementing #119
Signed-off-by: Paul Sokolovsky paul.sokolovsky@linaro.org