[tcpm] Re: Comments on draft-ietf-tcpm-tcp-edo-13

["[e-mail]" <[e-mail]>]

Hi, Kuniyuki,

Thanks for the detailed feedback. Some notes below.

Joe

—
Dr. Joe Touch, temporal epistemologist
www.strayalpha.com

> On Jul 9, 2024, at 6:39 PM, Iwashima, Kuniyuki <[e-mail]> wrote:
> 
> Hi Joe,
> 
> I wanted to send summary of feedback to EDO draft and related works after IETF 118, but it somehow slipped on my mind.
> Sorry for the long delay. (and thanks for the heads-up, Yoshi).
> 
> 
> ## Suggestion
> 
> I have three suggestions about EDO fallback, EDO Extension limit, and logging.
> 
> 1) EDO fallback
> 
> In the sentence below,
> 
>>> The EDO Extension option MAY be used only if confirmed when the
>   connection transitions to the ESTABLISHED state, e.g., a client is
>   enabled after receiving the EDO Supported option in the SYN/ACK and
>   the server is enabled after seeing the EDO Extension option in the
> 
> -> the server is enabled after seeing the _valid _ EDO Extension option in the <--
> 
>   final ACK of the three-way handshake. If either of those segments
>   lacks the appropriate EDO option, the connection MUST NOT use any
>   EDO options on any other segments.
> 
> I'd suggest adding "valid" like above to make it clear that the following "lacks the appropriate EDO" means
> 
>   i) ACK of 3WHS lacks EDO Extension option
>  ii) ACK of 3WHS has invalid/malformed EDO Extension
> 
> and these segments should be dropped instead of triggering non-EDO fallback.

Will do.

> 2) EDO Extension limit
> 
> As I told before, I think it would be better to clarify that EDO Extension MUST NOT be included more than once in a segment.
> 
> If TCP header includes multiple same options, Linux uses the latest one and ignores the other preceding ones.
> However, EDO Extension requires copying data into the linear buffer, the number of such an operation should be limited within the single segment to avoid DoS.

Agreed; will do.

> 3) Logging
> 
> In the following sentence, the 2nd MUST should be changed to SHOULD.
> 
>>> If EDO has been negotiated, any subsequent segments arriving
>   without the EDO Extension option MUST be silently ignored. Such
>   events MAY be logged as warning errors and logging MUST be rate
> 
> -> events MAY be logged as warning errors and logging SHOULD be rate <-
> 
>   limited.
> 
> Because there are two other places that mentions that EDO SHOULD log some events, not MUST.
> 
>>> When an endpoint receives a segment using the 6-byte EDO
>   Extension option, it MUST validate the Segment_Length field with the
>   length of the segment as indicated in the TCP pseudoheader. If the
>   segment lengths do not match, the segment MUST be discarded and an
>   error SHOULD be logged in a rate-limited manner.
> 
>>> Due to the potential impacts of legacy middleboxes (discussed in
>   Section 7), a TCP implementation supporting EDO SHOULD log any
>   events within an EDO connection when options that are malformed or
>   show other evidence of tampering arrive

Agreed - will do.

> That's all of my suggestions.
> 
> In my implementation, EDO options are parsed as follows:
> 
>  - SYN segments
>    - EDO Supported (length == 2) is parsed
>    - other EDO option (length != 2) is ignored (option is skipped)

Other EDO option (length !=2) should not be ignored; it should generate a RST in response. The option indicates a malformed SYN. I’ll add an explicit rule about that.

>  - non-SYN segments
>    - if not negotiated, all EDO options are ignored (option is skipped)

	Yes - we have to do this - it’s what legacy endpoints (those that don’t understand EDO) will do.

>    - if negotiated,
>      - EDO Supported or EDO option with invalid length (!= 4/6) is ignored (option is skipped)
>      - segment with multiple EDO Extension is dropped
>      - segment with invalid header/segment length is dropped
>      - EDO Extension is parsed
> 
> If this behaviour is correct, I think I read the draft as intended and have no other comments.

Yes, those seem reasonable.

> ## Implementation
> 
> Here's the EDO implementation of Linux kernel, tcpdump, and packetdrill.
> 
> - https://github.com/q2ven/linux/tree/edo
> - https://github.com/nsdyoshi/tcpdump/tree/tcp-edo-option
> - https://github.com/q2ven/packetdrill/tree/edo
> 
> The basic uAPI can be checked in this presentation.
> 
>  https://www.youtube.com/watch?si=bbBuPaAE5y__3IZW&t=2575&v=Na1FYAjLBeU&feature=youtu.be
> 
> Note that the constant TCP_EXT_DATA_OFFSET for setsockopt() is 44.
> 
> 
> The packetdrill repo has some scripts to test all possible EDO fallback scenarios, and SNMP stats can be used to check the negotiation result.
> 
> ---8<---
> # ./packetdrill ../tcp/edo/edo-fallback-server-syn.pkt
> # nstat | grep EDO
> TcpExtTCPEDOFallbackSYN         1                  0.0
> # ./packetdrill ../tcp/edo/edo-fallback-client-synack.pkt
> # nstat | grep EDO
> TcpExtTCPEDOFallbackSYNACK      1                  0.0
> # ./packetdrill ../tcp/edo/edo-fallback-server-ack.pkt
> # nstat | grep EDO
> TcpExtTCPEDOFallbackACK         1                  0.0
> # ./packetdrill ../tcp/edo/edo-fallback-cross-syn.pkt
> # nstat | grep EDO
> TcpExtTCPEDOFallbackSYN         1                  0.0
> # ./packetdrill ../tcp/edo/edo-fallback-cross-synack.pkt
> # nstat | grep EDO
> TcpExtTCPEDOFallbackSYNACK      1                  0.0
> # ./packetdrill ../tcp/edo/edo-sack.pkt 
> # nstat | grep EDO
> TcpExtTCPEDOSuccess             1                  0.0
> ---8<---
> 
> 
> Another script shows that more than 60 bytes header can be sent with EDO Extension.
> In this example, 4 SACK blocks are sent with 8 bytes EDO Extension (including ExID 0x0ed0).
> 
> ---8<---
> # ./packetdrill ../tcp/edo/edo-sack.pkt -v
> ...
> connect syscall: 1720572792.643631
> outbound sniffed packet:  0.000044 S 2116065219:2116065219(0) win 65535 <edoOK,mss 1440,sackOK,TS val 3401991024 ecr 0,nop,wscale 8>
> inbound injected packet:  0.000101 S. 0:0(0) ack 2116065220 win 65535 <nop,nop,edoOK,mss 9000,sackOK>
> outbound sniffed packet:  0.000114 . 2116065220:2116065220(0) ack 1 win 65535 <edo 28 28>
> inbound injected packet:  0.000128 . 12:13(1) ack 2116065220 win 65535 <edo 28 29>
> outbound sniffed packet:  0.000133 . 2116065220:2116065220(0) ack 1 win 65535 <edo 40 40,nop,nop,sack 12:13>
> inbound injected packet:  0.000147 . 10:11(1) ack 2116065220 win 65535 <edo 28 29>
> outbound sniffed packet:  0.000154 . 2116065220:2116065220(0) ack 1 win 65535 <edo 48 48,nop,nop,sack 10:11 12:13>
> inbound injected packet:  0.000168 . 8:9(1) ack 2116065220 win 65535 <edo 28 29>
> outbound sniffed packet:  0.000172 . 2116065220:2116065220(0) ack 1 win 65535 <edo 56 56,nop,nop,sack 8:9 10:11 12:13>
> inbound injected packet:  0.000185 . 6:7(1) ack 2116065220 win 65535 <edo 28 29>
> outbound sniffed packet:  0.000299 . 2116065220:2116065220(0) ack 1 win 65535 <edo 64 64,nop,nop,sack 6:7 8:9 10:11 12:13>
> ---8<---
> 
> 
> Also, with MPTCP, we can see the effect of EDO Extension easily.  Note that GRO must be turned off.

Yes, we’re aware that GRO does not convert packets in ways consistent with TCP options.

> 
> ---8<---
> Client:
> $ sudo ethtool -K enp39s0 gro off
> $ sudo sysctl net.ipv4.tcp_ext_data_offset=2
> $ mptcpize run iperf3 -c 10.0.0.36 -t 3
> 
> Server:
> $ sudo ethtool -K enp39s0 gro off
> $ sudo sysctl net.ipv4.tcp_ext_data_offset=2
> $ mptcpize run iperf3 -s
> 
> tcpdump :
> 
> IP 10.0.0.241.59560 > 10.0.0.36.5201: Flags [.], seq 84863089:84872006, ack 1, win 491, options [exp-edo hlen 64 seglen 8981 ,nop,nop,TS val 3197826953 ecr 2719828622,mptcp 22 dss ack 3678162956 seq 2861367221292019959 subseq 2596157316 len 8917,nop,nop], length 8917
> ---8<---
> 
> 
> Moreover, there is a debug sysctl knob to include NOPs after EDO.
> 
> ---8<---
> $ sudo sysctl net.ipv4.tcp_nops=128
> 
> tcpdump:
> IP 10.0.0.36.5201 > 10.0.0.241.44092: Flags [.], ack 47501509, win 41719, options [exp-edo hlen 180 seglen 180 ,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,nop,TS val 2719945746 ecr 3197944077,mptcp 12 dss ack 8813009640328282684], length 0
> ---8<---
> 
> 
> Please let me know if you find weird/non-compliant behaviour.

I’m not sure what you mean; is there some way you think this might be unexpected?

> I hope this help make things forward.
> 
> Best regards,
> Kuniyuki
> 
> 
> 
> 
>