search

mtcp-stack / mtcp

mTCP: A Highly Scalable User-level TCP Stack for Multicore Systems
Other
1.98k stars 435 forks source link

Add support for SPDK #274

Closed qingmin-liu closed 4 years ago

qingmin-liu commented 4 years ago

SPDK poller will register its own handler per thread. RunMTCPLoop needs to split into pieces for spdk caller. and it also does not affect all other features with no enable-spdk.

Thanks a lot. Qingmin

ajamshed commented 4 years ago

Hi @qingmin-liu ,

Thanks for your contribution. Enabling SPDK support definitely is an exciting new feature. But I think we are a bit light on documentation and sample applications that can be used to compile and run sample programs. Can you please submit these? That will also help me in testing out the new code. :)

Thanks!

qingmin-liu commented 4 years ago

OK. For spdk support, in fact mtcp only needs to change a little and it will not affect all previous examples and test cases.

SPDK will use libmtcp.a and register recv poller.

We will describe this scenario and most of work are at spdk side. For those changes we need more time to upstream.

Thanks a lot.

Best wishes, Qingmin

From: Asim Jamshed notifications@github.com Sent: Thursday, November 28, 2019 10:41:08 AM To: mtcp-stack/mtcp mtcp@noreply.github.com Cc: Qingmin Liu calmarrow@gmail.com; Author author@noreply.github.com Subject: Re: [mtcp-stack/mtcp] Add support for SPDK (#274)

Hi @qingmin-liuhttps://github.com/qingmin-liu ,

Thanks for your contribution. Enabling SPDK support definitely is an exciting new feature. But I think we are a bit light on documentation and sample applications that can be used to compile and run sample programs. Can you please submit these? That will also help me in testing out the new code. :)

Thanks!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/mtcp-stack/mtcp/pull/274?email_source=notifications&email_token=AAT74C6KQIFZFT22JN5YH4LQV4VUJA5CNFSM4JRFUPC2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFLIJCA#issuecomment-559318152, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAT74C7VNI7O3GHWQN4GN6DQV4VUJANCNFSM4JRFUPCQ.

qingmin-liu commented 4 years ago

Hi Asim, Current now we have no test cases for mtcp + spdk because spdk changes are not merged now. For mtcp, may need to add spdk submodule and pull down from spdk git.

Now with this patch set, those changes are transparent for current mtcp app and test cases. So just run normal mtcp test cases to confirm it's no side effect with these changes.

We just plan to push all spdk related changes to upstream but that may need more time. As I explained before, now for mtcp, it only exports mtcp_run_instance and MTCPRunThread for spdk caller. and that's working.

At this stage, I'm not sure how to document and give you test cases with spdk although we have run nvmeotcp with this.

Thanks a lot. Qingmin

ajamshed commented 4 years ago

Thanks. I do plan to go over the code, and either:

1- Give review comments, and/or

2- merge the code with the devel branch.

Please allow me a few more days.

qingmin-liu commented 4 years ago

OK. Thanks a lot. Now we have already verified with nvmf-tgt on our card. However as talked with intel, they still are working on kernel stack to improve the nvmeotcp performance. and not sure whether if it could merge into spdk upstream.

Thanks a lot. Qingmin

qingmin-liu commented 4 years ago

Hi Asim, We just adjust the code and prefer to abandon this PR. Will write doc and add more things and then send u Patch Set for review. Also we found the devel branch + mtcp on broadcom card has some stable issues. Without CCP related changes, it's stable. So I will first use my local stable branch to develop all nvmeotcp target/initiator integration and then open a new PR for review.

Thanks a lot. Qingmin

ajamshed commented 4 years ago

@qingmin-liu ,

I suggest that you contact CCP authors if you have any related questions. It is possible that they will be able to help debug the issue in depth. I have only now found some time to work on mTCP. I look forward to your next PR.

qingmin-liu commented 4 years ago

Thanks Ajam. Is there possible to help create a branch "spdk_mtcp_dev" with head 75a9e93343cc ? I will open a new PR to first get a stable version with spdk 19.07. Now even with ccp disabled, latest master branch is still not stable and will lead nvmeotcp traffic crash. We did not dig into this but will delay to check this later.

In my github I have created a branch with spdk_mtcp_dev and now we have done with integration and verified with spdk 19.07.

Thanks a lot. Qingmin

ajamshed commented 4 years ago

@qingmin-liu,

Is mTCP stack crashing even when ccp is disabled? I realized that the commit id is the point when I merged the code from the CCP authors. Please do let me know the details of the gdb stacktrace. It is possible that the CCP authors introduced some piece of code that may have introduced some issues.

In the mean time, I will create a branch at 75a9e93. But I am hoping in the future the SPDK-related patches will go through the devel branch and we should not really need any tertiary branches.

qingmin-liu commented 4 years ago

Hi Ajam, Thanks a lot. Will open a new PR to spdk_mtcp_dev branch.

For the issue, we need some times to figure it out. No crash but mbuf leaks and traffic stops. When basic functionality stable and will switch to try to target and solve this issue. After that, will merge into devel branch. Thanks for your help.

Thanks a lot. Qingmin