Closed gdetal closed 11 years ago
It is considered good to complement new decoders with a small .pcap file and relevant text output, that together constitute a basic regression test of the make check
command (see a few recent commits for an example).
I know that not all files are consistent, but tcpdump generally uses an 8 space initial indent and 4 or 8 space subsequent indent. 2 space indentation, therefore, ends up looking weird. Can you use something closer to what other files use?
Also, github says that your patch may need to be rebased first. Very happy to accept MPTCP... the next release will go out during the IETF86 meeting.
Thanks for the feedback, I have adapted my commit accordingly.
@infrastation I've added two regression test @fenner I was using tab and so I switched to 8 spaces indent @mcr I have rebased my branch
Hello,
Am I missing something ? I've updated the commit and have done a push -f on my branch. Yet the merge is not yet accepted. Is is the right way to do this ?
Don't worry, sometimes it takes a few days.
@infrastation Your comment disappeared but thanks for the feedback. I updated my commit to use the PRI output format.
Functions mp_fail_print()
and mp_fast_close_print()
should return a value (warning seen when compiling with make CCOPT=-Wall
).
There is some more space for improvement in this code, please see below.
tcp_option_values[]
should be lowercase.tcp_print()
seems to require LENCHECK(datalen)
in the newly added case block.mptcp_print()
should check that the option length is at least 3 before dispatching further.mp_capable_print()
should report a malformed option for length other than 12 or 20. The function does not print or check the value of the Version field, which is defined in RFC6824 to be 0 (if it is not 0, would the option format be different?). The function converts neither mpc->sender_key
nor mpc->receiver_key
from network byte order before printing (alternatively, should the keying material be printed as 01:23:45:67:89:ab:cd:ef
instead?).mp_join_print()
should report a malformed option for length other than 12, 16 or 24. The function does not convert the value of mpj->u.syn.token
from network byte order before printing and does not print the value of mpj->u.syn.nonce
at all. The function handles only the 12-octet (for Initial SYN) format of MP_JOIN option, there are also 16-octet (for Responding SYN/ACK) and 24-octet (for Third ACK) formats. The current code does not interpret whether the format (12/16/24) matches the TCP flags of the current packet (if anyone wants to implement this, the flags would have to be expedited from tcp_print()
).mp_dss_len()
has its second argument equal to 1, should it be omitted (the presence of Checksum field is already covered with the M-bit of the DSS option)?mp_dss_print()
should report a malformed option for length other than 4, 8, 12, 16, 20, 24 or 28. The function should check if the length matches the flags.add_addr_print()
should report a malformed option for length other than 8, 10, 20 or 22. The function should check that the length matches the value of IPVer field. The function should not report a malformed option when tcpdump is built without IPv6 support, but rather should print a placeholder string instead of the IPv6 address.mp_prio_print()
should report a malformed option for length other than 3 or 4.mp_fail_print()
should report a malformed option for length other than 12.mp_fast_close_print()
should report a malformed option for length other than 12. It also has the same issue printing the value of receiver's key as mp_capable_print()
above.Cherry-picked to the 4.4 branch.
Functions mp_fail_print() and mp_fast_close_print() should return a value (warning seen when compiling with make CCOPT=-Wall).
I've checked in a fix for that and cherry-picked it to the 4.4 branch.
Multipath TCP (MPTCP) is a new extension to TCP that is being standardised at the IETF. We are currently maintaining the reference Linux implementation @multipath-tcp.
The patch allows tcpdump to parse MPTCP options (that are in fact TCP options) in order to print useful information. A sample output can be found at http://pastebin.com/BsJP1w4a.
Signed-off-by: Gregory Detal gregory.detal@uclouvain.be