Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mptcp: add regression test for disconnect() #33

Merged
merged 1 commit into from
Jan 22, 2021

Conversation

pabeni
Copy link

@pabeni pabeni commented Jan 14, 2021

This simply trigger the in-kernel mptcp disconnect
code path, so that we can have coverage for:

https://marc.info/?l=linux-netdev&m=161053335929207&w=2

Signed-off-by: Paolo Abeni pabeni@redhat.com

@pabeni pabeni requested review from dcaratti and matttbe January 14, 2021 12:13
Copy link
Member

@matttbe matttbe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding new tests!

Looks good! Maybe two small bits to change?

  • remove unrelated comment
  • rename the file

WDYT? :)

@@ -0,0 +1,13 @@
// This test case covers the regression fixed by ("mptcp: be careful on MPTCP-level ack.")
// bad read on on unconnected socket
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this comment should be updated :) (from unconnected_read.pkt)

+0.0 connect(3, ..., ...) = -1 EINPROGRESS (Operation now in progress)
+0.0 > S 0:0(0) <mss 1460,sackOK,TS val 100 ecr 0,nop,wscale 8,mpcapable v1 flags[flag_h] nokey>

+0.0 shutdown(3, SHUT_WR) = 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we maybe rename this test not to have it too generic? e.g. unconnected_shutdown.pkt (or connect_in_progress_shutdown.pkt or something else :) )

This simply trigger the in-kernel mptcp disconnect
code path, so that we can have coverage for:

https://marc.info/?l=linux-netdev&m=161053335929207&w=2

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
@pabeni pabeni force-pushed the mptcp_mptcp-net-next branch from 8987957 to 8653fdf Compare January 14, 2021 15:59
Copy link
Member

@matttbe matttbe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx for the update! LGTM!

@@ -0,0 +1,14 @@
// This test case covers the regression fixed by ("mptcp: fix locking in mptcp_disconnect()")
// shutdown a socket with connect pending, so that mptcp_disconnect()
// is inoked
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

invoked I guess, a detail :)

Copy link
Member

@matttbe matttbe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx for the patch!

Maybe we should not remove the "win 0" ones?


// more data, expect a reset and win0 (no socket anymore)
+1.0 < P. 2001:3001(1000) ack 1 win 450 <nop, nop, dss dack8=1 dsn8=2001 ssn=2001 dll=1000 nocs>
+0.01 > R 1:1(0) ack 0 win 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one should be fine, no?


// send rst fastclose with a key that should not match.
+0 < R. 1001:1001(0) win 450 <mp_fastclose ckey=42 >

// more data, but expect rst: the subflow is gone.
+1.0 < P. 1001:2001(1000) ack 1 win 450 <nop, nop, dss dack8=1 dsn8=1001 ssn=1001 dll=1000 nocs>
+0.02 > R 1:1(0) ack 0 win 0
+0.02 > R 1:1(0) ack 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also this one should be fine, no?


// send another rst fastclose, this time with matching key
+0.1 < R. 1001:1001(0) win 450 <mp_fastclose>

// more data, expect a reset and win0 (no socket anymore)
+1.0 < P. 2001:3001(1000) ack 1 win 450 <nop, nop, dss dack8=1 dsn8=2001 ssn=1001 dll=1000 nocs>
+0.01 > R 1:1(0) ack 0 win 0
+0.01 > R 1:1(0) ack 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and this one? :)

@pabeni pabeni force-pushed the mptcp_mptcp-net-next branch from 068e5f7 to 8653fdf Compare January 22, 2021 18:19
Copy link
Member

@matttbe matttbe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx for the update!

@matttbe matttbe merged commit 3b67ae3 into multipath-tcp:mptcp-net-next Jan 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants