| Message ID | 20201105170126.5627-1-fw@strlen.de |
|---|---|
| Headers | show |
| Series | mptcp: add reset and fastclose option support | expand |
On Thu, 5 Nov 2020, Florian Westphal wrote: > This series adds reset and fast close options. > Both need common infrastructure: > 1. need to parse mptcp option in tcp reset packets, and making > them available to the stack > 2. ability to add mptcp option to tcp reset packets > > First two patches move code around and add mptcp-callsite for > option deciding from rst packets. > > Patch 3 adds reset support (both decoding and adding them). > Patch 4 adds the fastclose receive processing. > Patch 5 sends a fastclose packet when userspace calls close() > while there is unread data in the mptcp rx queue. > > Patch 2) is not strictly need ATM, I could rework the series > to remove it and only handle fastclose for the time being. > Last patch could be removed as well so we only handle the rx case > (there is nothing in RFC that says we must send fastclose). > > Comments welcome. I had a couple of minor questions on patch 3, but nothing that would keep this from going in to the export branch for some more testing. Might be easier to coordinate with Paolo's refactor changes to the workqueue that way. Do the selftests get any code coverage on this, for example in the new-subflow-not-allowed case? And thanks for looking at packetdrill tests for this, saw the conversation on IRC. -- Mat Martineau Intel
Mat Martineau <mathew.j.martineau@linux.intel.com> wrote: > I had a couple of minor questions on patch 3, but nothing that would keep > this from going in to the export branch for some more testing. Might be > easier to coordinate with Paolo's refactor changes to the workqueue that > way. > > Do the selftests get any code coverage on this, for example in the > new-subflow-not-allowed case? How would you do that? I could add a mib counter and check that in the script. But other than that I do not see how to expose any of this to the script/mptcp_connect. > And thanks for looking at packetdrill tests for this, > saw the conversation on IRC. Yes, I plan to send a pull request for fastclose and reset. As for further direction, I think next step is to add genl notifications so mptcpd can learn about subflows that are added/removed, the reset code could then be exposed to userspace in the 'subflow was reset' case.
On Thu, 12 Nov 2020, Florian Westphal wrote: > Mat Martineau <mathew.j.martineau@linux.intel.com> wrote: >> I had a couple of minor questions on patch 3, but nothing that would keep >> this from going in to the export branch for some more testing. Might be >> easier to coordinate with Paolo's refactor changes to the workqueue that >> way. >> >> Do the selftests get any code coverage on this, for example in the >> new-subflow-not-allowed case? > > How would you do that? I could add a mib counter and check that in the > script. But other than that I do not see how to expose any of this to > the script/mptcp_connect. I meant it in a different sense: not that there's a specific self test result that looks at a count of MPTCP_RST, but that there is some self test (such as denying an additional subflow due to a configured limit) that would cause MPTCP_RST to be sent and processed. That way the new code is at least running during self test and, if memory leak checks/lockdep/etc. were enabled we would at least know some portion of the MPTCP_RST code was being exercised and checked during the tests even if they aren't explictly part of the results. I should just answer my own question by capturing the pcaps during the self tests, but I haven't had a chance to build & run the changes yet and was curious if you had already checked. If there are MIBs that make sense for real-world use, that's great and would make for better tests, but I'm not pushing for that now. > >> And thanks for looking at packetdrill tests for this, >> saw the conversation on IRC. > > Yes, I plan to send a pull request for fastclose and reset. > > As for further direction, I think next step is to add genl notifications > so mptcpd can learn about subflows that are added/removed, the reset > code could then be exposed to userspace in the 'subflow was reset' > case. > Good plan. Thanks. -- Mat Martineau Intel
This series adds reset and fast close options. Both need common infrastructure: 1. need to parse mptcp option in tcp reset packets, and making them available to the stack 2. ability to add mptcp option to tcp reset packets First two patches move code around and add mptcp-callsite for option deciding from rst packets. Patch 3 adds reset support (both decoding and adding them). Patch 4 adds the fastclose receive processing. Patch 5 sends a fastclose packet when userspace calls close() while there is unread data in the mptcp rx queue. Patch 2) is not strictly need ATM, I could rework the series to remove it and only handle fastclose for the time being. Last patch could be removed as well so we only handle the rx case (there is nothing in RFC that says we must send fastclose). Comments welcome.