On 4/22/25 5:35 PM, chia-yu.ch...@nokia-bell-labs.com wrote:
> @@ -709,6 +709,8 @@ static __be32 *process_tcp_ao_options(struct tcp_sock *tp,
> return ptr;
> }
>
> +#define NOP_LEFTOVER ((TCPOPT_NOP << 8) | TCPOPT_NOP)
> +
> /* Write previously computed TCP options to the packet.
> *
> * Beware: Something in the Internet is very sensitive to the ordering of
> @@ -727,8 +729,10 @@ static void tcp_options_write(struct tcphdr *th, struct
> tcp_sock *tp,
> struct tcp_out_options *opts,
> struct tcp_key *key)
> {
> + u16 leftover_bytes = NOP_LEFTOVER; /* replace next NOPs if avail */
> __be32 *ptr = (__be32 *)(th + 1);
> u16 options = opts->options; /* mungable copy */
> + int leftover_size = 2;
>
> if (tcp_key_is_md5(key)) {
> *ptr++ = htonl((TCPOPT_NOP << 24) | (TCPOPT_NOP << 16) |
> @@ -763,17 +767,22 @@ static void tcp_options_write(struct tcphdr *th, struct
> tcp_sock *tp,
> }
>
> if (unlikely(OPTION_SACK_ADVERTISE & options)) {
> - *ptr++ = htonl((TCPOPT_NOP << 24) |
> - (TCPOPT_NOP << 16) |
> + *ptr++ = htonl((leftover_bytes << 16) |
> (TCPOPT_SACK_PERM << 8) |
> TCPOLEN_SACK_PERM);
> + leftover_bytes = NOP_LEFTOVER;
Why? isn't leftover_bytes already == NOP_LEFTOVER?
> }
>
> if (unlikely(OPTION_WSCALE & options)) {
> - *ptr++ = htonl((TCPOPT_NOP << 24) |
> + u8 highbyte = TCPOPT_NOP;
> +
> + if (unlikely(leftover_size == 1))
How can the above conditional be true?
> + highbyte = leftover_bytes >> 8;
> + *ptr++ = htonl((highbyte << 24) |
> (TCPOPT_WINDOW << 16) |
> (TCPOLEN_WINDOW << 8) |
> opts->ws);
> + leftover_bytes = NOP_LEFTOVER;
Why? isn't leftover_bytes already == NOP_LEFTOVER?
> }
>
> if (unlikely(opts->num_sack_blocks)) {
> @@ -781,8 +790,7 @@ static void tcp_options_write(struct tcphdr *th, struct
> tcp_sock *tp,
> tp->duplicate_sack : tp->selective_acks;
> int this_sack;
>
> - *ptr++ = htonl((TCPOPT_NOP << 24) |
> - (TCPOPT_NOP << 16) |
> + *ptr++ = htonl((leftover_bytes << 16) |
> (TCPOPT_SACK << 8) |
> (TCPOLEN_SACK_BASE + (opts->num_sack_blocks *
> TCPOLEN_SACK_PERBLOCK)));
> @@ -794,6 +802,10 @@ static void tcp_options_write(struct tcphdr *th, struct
> tcp_sock *tp,
> }
>
> tp->rx_opt.dsack = 0;
> + } else if (unlikely(leftover_bytes != NOP_LEFTOVER)) {
I really feel like I'm missing some code chunk, but I don't see any
possible value for leftover_bytes other than NOP_LEFTOVER
/P
