Skip to content
This repository has been archived by the owner on Apr 18, 2024. It is now read-only.

Redundant scheduler works unexpectedly in real networks. #474

Closed
ytxing opened this issue Apr 6, 2022 · 7 comments
Closed

Redundant scheduler works unexpectedly in real networks. #474

ytxing opened this issue Apr 6, 2022 · 7 comments
Labels

Comments

@ytxing
Copy link
Contributor

ytxing commented Apr 6, 2022

Yes, redundant again.

I am trying to test MPTCP in real network environments. I deploy a server on the cloud, with MPTCP v0.95.2 (4.19.234+ by uname -r). It works well when using default and round-robin scheduler, but not when it switches to redundant.

To be specific, we download some files from the server and record the completion time. When using the two schedulers, it can achieve about 90mbps. But with redundant, it can barely reach 1mbps. We also check the byte count in /proc/net/dev before and after downloading then we can get how many bytes each nic receives. When downloading a 10M file, using redundant scheduler receives 239221246 (for LTE) plus 1103235282 (for WLAN) bytes. In a word, it is way more than 10M.

The server has only one nic and the client has two. Using fullmesh for both side. Using requests packet of python3 to download files and using python3 httpserver for the server.

Need help. Any idea why?

@ytxing
Copy link
Contributor Author

ytxing commented Apr 12, 2022

I am working on testing different schedulers of MPTCP v0.95.2. And as I mentioned above, the redundant scheduler works wired.

I get a 10M file from my MPTCP-enabled server with only one interface. And it is using fullmesh path-manager:

net.mptcp.mptcp_path_manager = fullmesh

Then when I use the default scheduler, it works well:
image

But when using the redundant scheduler, the overall throughput is very low:
image
image

It seems that the redundant scheduler not only sends repeated packets over the two subflows but also sends lots of redundant data on the same subflow -- the throughput of specific interfaces is high, but the overall throughput is very low.

I don't know why it works like this. Does anyone have any idea?

@ytxing
Copy link
Contributor Author

ytxing commented Apr 13, 2022

Hi,
I think there is something wrong with the following codes (around line 191, in function redsched_correct_skb_pointers of mptcp_redundant.c):

if (red_p->skb &&
    (!after(red_p->skb_start_seq, meta_tp->snd_una) ||
    after(red_p->skb_end_seq, meta_tp->snd_nxt)))
    red_p->skb = NULL;

This may cause redundant scheduler set red_p->skb = NULL and send repeated skbs on the same subflow for many time, especially when there is only one active subflow. That is because when sending a skb, red_p->skb_start_seq = TCP_SKB_CB(skb)->seq (line 323 and some other lines), which makes red_p->skb_start_seq == meta_tp->snd_una and the first checking holds.

I replace the !after with before, which seems to work now. Am I right about this change?

@matttbe
Copy link
Member

matttbe commented Apr 19, 2022

Hi,

Thank you for the bug report and having spot that!

Yes, this looks strange. It has been introduced in commit 7835e78 and when you look at it, it looks strange to keep !after.

@cpaasch I guess you wanted to use before(), no?

@ytxing : do you plan to send a patch / PR for that?

@cpaasch
Copy link
Member

cpaasch commented Apr 19, 2022

Yes, I think you are right. This should be before().

@ytxing
Copy link
Contributor Author

ytxing commented Apr 19, 2022 via email

@matttbe
Copy link
Member

matttbe commented Apr 20, 2022

@ytxing : thanks! If it is easier, you can also send a patch to the mailing-list: https://multipath-tcp.org/pmwiki.php/Developer/SubmitAPatch
(Or because it is only one line and we already discuss about it, you can also attach a patch here)

What is important is to have an explanation of the fix in the commit message, your "Signed-off-by" and ideally these two lines at the end, just above your Signed-off-by:

Closes: https://github.com/multipath-tcp/mptcp/issues/474
Fixes: 7835e7847026 ("mptcp: Fix use-after-free in the redundant scheduler")

ytxing added a commit to ytxing/mptcp that referenced this issue Apr 21, 2022
Previously redsched_correct_skb_pointers set red_p->skb = NULL when red_p->skb_start_seq == meta_tp->snd_una.
This issue causes the redundant scheduler keep sending reinjected packets.
Replace !after with before in line 192.

Closes: multipath-tcp#474
Fixes: 7835e78 ("mptcp: Fix use-after-free in the redundant scheduler")
Signed-off-by: ytxing <[email protected]>
matttbe pushed a commit that referenced this issue Apr 25, 2022
Previously redsched_correct_skb_pointers set red_p->skb = NULL when
red_p->skb_start_seq == meta_tp->snd_una.

This issue causes the redundant scheduler to keep sending reinjected
packets again and again.
Replace '!after' with 'before' fixes this issue.

Closes: #474
Fixes: 7835e78 ("mptcp: Fix use-after-free in the redundant scheduler")
Signed-off-by: ytxing <[email protected]>
Signed-off-by: Matthieu Baerts <[email protected]>
matttbe pushed a commit that referenced this issue Apr 25, 2022
Previously redsched_correct_skb_pointers set red_p->skb = NULL when
red_p->skb_start_seq == meta_tp->snd_una.

This issue causes the redundant scheduler to keep sending reinjected
packets again and again.
Replace '!after' with 'before' fixes this issue.

Closes: #474
Fixes: 7835e78 ("mptcp: Fix use-after-free in the redundant scheduler")
Signed-off-by: ytxing <[email protected]>
Signed-off-by: Matthieu Baerts <[email protected]>
(cherry picked from commit 5ffee1d)
Signed-off-by: Matthieu Baerts <[email protected]>
@ken32293355
Copy link

Hello, the goodput seems to be good now. However, the multiple retransmissions problem still exists when throuput setting is a little high (ex: 2Mbps). Although the goodput works as expected, many packets are dropped due to multiple retransmissions.

The multiple retransmissions problem disappear when the throughput setting is slow (0.2 Mpbs).

Thanks

psndna88 pushed a commit to psndna88/AGNi-xanmod_x86-64 that referenced this issue May 3, 2022
Previously redsched_correct_skb_pointers set red_p->skb = NULL when
red_p->skb_start_seq == meta_tp->snd_una.

This issue causes the redundant scheduler to keep sending reinjected
packets again and again.
Replace '!after' with 'before' fixes this issue.

Closes: multipath-tcp/mptcp#474
Fixes: 7835e78 ("mptcp: Fix use-after-free in the redundant scheduler")
Signed-off-by: ytxing <[email protected]>
Signed-off-by: Matthieu Baerts <[email protected]>
(cherry picked from commit 5ffee1dcb2617a8a2f496addca614cb56e2e123f)
Signed-off-by: Matthieu Baerts <[email protected]>
Diaz1401 pushed a commit to mengkernel/kernel_xiaomi_sm8250 that referenced this issue Aug 30, 2022
Previously redsched_correct_skb_pointers set red_p->skb = NULL when
red_p->skb_start_seq == meta_tp->snd_una.

This issue causes the redundant scheduler to keep sending reinjected
packets again and again.
Replace '!after' with 'before' fixes this issue.

Closes: multipath-tcp/mptcp#474
Fixes: 7835e7847026 ("mptcp: Fix use-after-free in the redundant scheduler")
Signed-off-by: ytxing <[email protected]>
Signed-off-by: Matthieu Baerts <[email protected]>
(cherry picked from commit 5ffee1dcb2617a8a2f496addca614cb56e2e123f)
Signed-off-by: Matthieu Baerts <[email protected]>
(cherry picked from commit 55abfd02c06ac3f7704efb8e94e0099d1965a4bc)
Signed-off-by: Matthieu Baerts <[email protected]>
Signed-off-by: Diaz1401 <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants