* [PATCH] tcp: Use structures to construct initial TCP options
@ 2024-10-21 7:40 David Gibson
2024-10-21 18:11 ` Stefano Brivio
0 siblings, 1 reply; 2+ messages in thread
From: David Gibson @ 2024-10-21 7:40 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson
As a rule, we prefer constructing packets with matching C structures,
rather than building them byte by byte. However, one case we still build
byte by byte is the TCP options we include in SYN packets (in fact the only
time we generate TCP options on the tap interface).
Rework this to use a structure and initialisers which make it a bit
clearer what's going on.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
tcp.c | 17 +++----------
tcp_buf.c | 2 +-
tcp_internal.h | 68 ++++++++++++++++++++++++++++++++++++++++++++++----
3 files changed, 67 insertions(+), 20 deletions(-)
diff --git a/tcp.c b/tcp.c
index b2155ab..0d22e07 100644
--- a/tcp.c
+++ b/tcp.c
@@ -1232,7 +1232,7 @@ static void tcp_update_seqack_from_tap(const struct ctx *c,
* 1 otherwise
*/
int tcp_prepare_flags(const struct ctx *c, struct tcp_tap_conn *conn,
- int flags, struct tcphdr *th, char *data,
+ int flags, struct tcphdr *th, struct tcp_syn_opts *opts,
size_t *optlen)
{
struct tcp_info tinfo = { 0 };
@@ -1258,12 +1258,6 @@ int tcp_prepare_flags(const struct ctx *c, struct tcp_tap_conn *conn,
if (flags & SYN) {
int mss;
- /* Options: MSS, NOP and window scale (8 bytes) */
- *optlen = OPT_MSS_LEN + 1 + OPT_WS_LEN;
-
- *data++ = OPT_MSS;
- *data++ = OPT_MSS_LEN;
-
if (c->mtu == -1) {
mss = tinfo.tcpi_snd_mss;
} else {
@@ -1279,16 +1273,11 @@ int tcp_prepare_flags(const struct ctx *c, struct tcp_tap_conn *conn,
else if (mss > PAGE_SIZE)
mss = ROUND_DOWN(mss, PAGE_SIZE);
}
- *(uint16_t *)data = htons(MIN(USHRT_MAX, mss));
-
- data += OPT_MSS_LEN - 2;
conn->ws_to_tap = MIN(MAX_WS, tinfo.tcpi_snd_wscale);
- *data++ = OPT_NOP;
- *data++ = OPT_WS;
- *data++ = OPT_WS_LEN;
- *data++ = conn->ws_to_tap;
+ *opts = TCP_SYN_OPTS(mss, conn->ws_to_tap);
+ *optlen = sizeof(*opts);
} else if (!(flags & RST)) {
flags |= ACK;
}
diff --git a/tcp_buf.c b/tcp_buf.c
index 238827b..44df0e4 100644
--- a/tcp_buf.c
+++ b/tcp_buf.c
@@ -282,7 +282,7 @@ int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
seq = conn->seq_to_tap;
ret = tcp_prepare_flags(c, conn, flags, &payload->th,
- payload->opts, &optlen);
+ &payload->opts, &optlen);
if (ret <= 0) {
if (CONN_V4(conn))
tcp4_flags_used--;
diff --git a/tcp_internal.h b/tcp_internal.h
index 2f74ffe..1ab8ce2 100644
--- a/tcp_internal.h
+++ b/tcp_internal.h
@@ -33,9 +33,7 @@
#define OPT_EOL 0
#define OPT_NOP 1
#define OPT_MSS 2
-#define OPT_MSS_LEN 4
#define OPT_WS 3
-#define OPT_WS_LEN 3
#define OPT_SACKP 4
#define OPT_SACK 5
#define OPT_TS 8
@@ -77,6 +75,65 @@ struct tcp_payload_t {
} __attribute__ ((packed, aligned(__alignof__(unsigned int))));
#endif
+/** struct tcp_opt_nop - TCP NOP option
+ * @kind: Option kind (OPT_NOP = 1)
+ */
+struct tcp_opt_nop {
+ uint8_t kind;
+} __attribute__ ((packed));
+#define TCP_OPT_NOP ((struct tcp_opt_nop){ .kind = OPT_NOP, })
+
+/** struct tcp_opt_mss - TCP MSS option
+ * @kind: Option kind (OPT_MSS == 2)
+ * @len: Option length (4)
+ * @mss: Maximum Segment Size
+ */
+struct tcp_opt_mss {
+ uint8_t kind;
+ uint8_t len;
+ uint16_t mss;
+} __attribute__ ((packed));
+#define TCP_OPT_MSS(mss_) \
+ ((struct tcp_opt_mss) { \
+ .kind = OPT_MSS, \
+ .len = sizeof(struct tcp_opt_mss), \
+ .mss = htons(mss_), \
+ })
+
+/** struct tcp_opt_ws - TCP Window Scaling option
+ * @kind: Option kind (OPT_WS == 3)
+ * @len: Option length (3)
+ * @shift: Window scaling shift
+ */
+struct tcp_opt_ws {
+ uint8_t kind;
+ uint8_t len;
+ uint8_t shift;
+} __attribute__ ((packed));
+#define TCP_OPT_WS(shift_) \
+ ((struct tcp_opt_ws) { \
+ .kind = OPT_WS, \
+ .len = sizeof(struct tcp_opt_ws), \
+ .shift = (shift_), \
+ })
+
+/** struct tcp_syn_opts - TCP options we apply to SYN packets
+ * @mss: Maximum Segment Size (MSS) option
+ * @nop: NOP opt (for alignment)
+ * @ws: Window Scaling (WS) option
+ */
+struct tcp_syn_opts {
+ struct tcp_opt_mss mss;
+ struct tcp_opt_nop nop;
+ struct tcp_opt_ws ws;
+} __attribute__ ((packed));
+#define TCP_SYN_OPTS(mss_, ws_) \
+ ((struct tcp_syn_opts){ \
+ .mss = TCP_OPT_MSS(mss_), \
+ .nop = TCP_OPT_NOP, \
+ .ws = TCP_OPT_WS(ws_), \
+ })
+
/**
* struct tcp_flags_t - TCP header and data to send zero-length
* segments (flags)
@@ -85,7 +142,7 @@ struct tcp_payload_t {
*/
struct tcp_flags_t {
struct tcphdr th;
- char opts[OPT_MSS_LEN + OPT_WS_LEN + 1];
+ struct tcp_syn_opts opts;
#ifdef __AVX2__
} __attribute__ ((packed, aligned(32)));
#else
@@ -124,7 +181,8 @@ size_t tcp_l2_buf_fill_headers(const struct tcp_tap_conn *conn,
bool no_tcp_csum);
int tcp_update_seqack_wnd(const struct ctx *c, struct tcp_tap_conn *conn,
bool force_seq, struct tcp_info *tinfo);
-int tcp_prepare_flags(const struct ctx *c, struct tcp_tap_conn *conn, int flags,
- struct tcphdr *th, char *data, size_t *optlen);
+int tcp_prepare_flags(const struct ctx *c, struct tcp_tap_conn *conn,
+ int flags, struct tcphdr *th, struct tcp_syn_opts *opts,
+ size_t *optlen);
#endif /* TCP_INTERNAL_H */
--
@@ -33,9 +33,7 @@
#define OPT_EOL 0
#define OPT_NOP 1
#define OPT_MSS 2
-#define OPT_MSS_LEN 4
#define OPT_WS 3
-#define OPT_WS_LEN 3
#define OPT_SACKP 4
#define OPT_SACK 5
#define OPT_TS 8
@@ -77,6 +75,65 @@ struct tcp_payload_t {
} __attribute__ ((packed, aligned(__alignof__(unsigned int))));
#endif
+/** struct tcp_opt_nop - TCP NOP option
+ * @kind: Option kind (OPT_NOP = 1)
+ */
+struct tcp_opt_nop {
+ uint8_t kind;
+} __attribute__ ((packed));
+#define TCP_OPT_NOP ((struct tcp_opt_nop){ .kind = OPT_NOP, })
+
+/** struct tcp_opt_mss - TCP MSS option
+ * @kind: Option kind (OPT_MSS == 2)
+ * @len: Option length (4)
+ * @mss: Maximum Segment Size
+ */
+struct tcp_opt_mss {
+ uint8_t kind;
+ uint8_t len;
+ uint16_t mss;
+} __attribute__ ((packed));
+#define TCP_OPT_MSS(mss_) \
+ ((struct tcp_opt_mss) { \
+ .kind = OPT_MSS, \
+ .len = sizeof(struct tcp_opt_mss), \
+ .mss = htons(mss_), \
+ })
+
+/** struct tcp_opt_ws - TCP Window Scaling option
+ * @kind: Option kind (OPT_WS == 3)
+ * @len: Option length (3)
+ * @shift: Window scaling shift
+ */
+struct tcp_opt_ws {
+ uint8_t kind;
+ uint8_t len;
+ uint8_t shift;
+} __attribute__ ((packed));
+#define TCP_OPT_WS(shift_) \
+ ((struct tcp_opt_ws) { \
+ .kind = OPT_WS, \
+ .len = sizeof(struct tcp_opt_ws), \
+ .shift = (shift_), \
+ })
+
+/** struct tcp_syn_opts - TCP options we apply to SYN packets
+ * @mss: Maximum Segment Size (MSS) option
+ * @nop: NOP opt (for alignment)
+ * @ws: Window Scaling (WS) option
+ */
+struct tcp_syn_opts {
+ struct tcp_opt_mss mss;
+ struct tcp_opt_nop nop;
+ struct tcp_opt_ws ws;
+} __attribute__ ((packed));
+#define TCP_SYN_OPTS(mss_, ws_) \
+ ((struct tcp_syn_opts){ \
+ .mss = TCP_OPT_MSS(mss_), \
+ .nop = TCP_OPT_NOP, \
+ .ws = TCP_OPT_WS(ws_), \
+ })
+
/**
* struct tcp_flags_t - TCP header and data to send zero-length
* segments (flags)
@@ -85,7 +142,7 @@ struct tcp_payload_t {
*/
struct tcp_flags_t {
struct tcphdr th;
- char opts[OPT_MSS_LEN + OPT_WS_LEN + 1];
+ struct tcp_syn_opts opts;
#ifdef __AVX2__
} __attribute__ ((packed, aligned(32)));
#else
@@ -124,7 +181,8 @@ size_t tcp_l2_buf_fill_headers(const struct tcp_tap_conn *conn,
bool no_tcp_csum);
int tcp_update_seqack_wnd(const struct ctx *c, struct tcp_tap_conn *conn,
bool force_seq, struct tcp_info *tinfo);
-int tcp_prepare_flags(const struct ctx *c, struct tcp_tap_conn *conn, int flags,
- struct tcphdr *th, char *data, size_t *optlen);
+int tcp_prepare_flags(const struct ctx *c, struct tcp_tap_conn *conn,
+ int flags, struct tcphdr *th, struct tcp_syn_opts *opts,
+ size_t *optlen);
#endif /* TCP_INTERNAL_H */
--
2.47.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] tcp: Use structures to construct initial TCP options
2024-10-21 7:40 [PATCH] tcp: Use structures to construct initial TCP options David Gibson
@ 2024-10-21 18:11 ` Stefano Brivio
0 siblings, 0 replies; 2+ messages in thread
From: Stefano Brivio @ 2024-10-21 18:11 UTC (permalink / raw)
To: David Gibson; +Cc: passt-dev
On Mon, 21 Oct 2024 18:40:29 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:
> As a rule, we prefer constructing packets with matching C structures,
> rather than building them byte by byte. However, one case we still build
> byte by byte is the TCP options we include in SYN packets (in fact the only
> time we generate TCP options on the tap interface).
>
> Rework this to use a structure and initialisers which make it a bit
> clearer what's going on.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Applied.
By the way, since you were wondering: I originally added the NOP option
before the MSS option simply because the Linux kernel does that as
well: typically, "SACK Permitted" and "Timestamps" (12 bytes
altogether, hence aligned) come first, then NOP, then MSS.
--
Stefano
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-10-21 18:11 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-10-21 7:40 [PATCH] tcp: Use structures to construct initial TCP options David Gibson
2024-10-21 18:11 ` Stefano Brivio
Code repositories for project(s) associated with this public inbox
https://passt.top/passt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).