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

msg: client bind #12901

Merged
merged 4 commits into from Jan 23, 2017
Merged

msg: client bind #12901

merged 4 commits into from Jan 23, 2017

Conversation

yuyuyu101
Copy link
Member

No description provided.

@yuyuyu101
Copy link
Member Author

refactor from #7256

@yuyuyu101
Copy link
Member Author

yuyuyu101 commented Jan 12, 2017

we also met this problem when public and cluster are different net but can be interconnect. when injecting public network fault, it will cause mon electing always and osd false down

Signed-off-by: Haomai Wang <haomai@xsky.com>
Signed-off-by: Haomai Wang <haomai@xsky.com>
Signed-off-by: Haomai Wang <haomai@xsky.com>
@yuyuyu101 yuyuyu101 force-pushed the wip-msgr-client-bind branch 2 times, most recently from 9a58f6b to dbb737d Compare January 12, 2017 08:20
@liewegas liewegas changed the title Wip msgr client bind msg: client bind Jan 12, 2017
@@ -859,6 +859,18 @@ ssize_t AsyncConnection::_process_connection()
cs.close();
}

{
entity_addr_t addr2bind = msgr->get_myaddr();
if (msgr->cct->ms_bind_before_connect && (!addr2bind.is_blank_ip())) {
Copy link
Member

Choose a reason for hiding this comment

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

get_myaddr() returns by reference, right? can use it directly instead of copying the value to addr2bind

Copy link
Member

Choose a reason for hiding this comment

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

eh, i guess if we have this option enabled by default it doesn't matter

Copy link
Member Author

Choose a reason for hiding this comment

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

we will modify addr2bind, so nonreference is suitable

@liewegas
Copy link
Member

/home/jenkins-build/build/workspace/ceph-pull-requests/src/msg/simple/Pipe.cc: In member function ‘int Pipe::accept()’:
/home/jenkins-build/build/workspace/ceph-pull-requests/src/msg/simple/Pipe.cc:361:20: error: ‘class CephContext’ has no member named ‘ms_bind_before_connect’
     if (msgr->cct->ms_bind_before_connect && (!addr2bind.is_blank_ip())) {
                    ^

@yuyuyu101
Copy link
Member Author

retest this please

@yuyuyu101 yuyuyu101 force-pushed the wip-msgr-client-bind branch 11 times, most recently from 952938b to 80c1b68 Compare January 16, 2017 07:11
@@ -563,6 +573,9 @@ int main(int argc, const char **argv)
r = ms_hb_front_server->bind(hb_front_addr);
if (r < 0)
exit(1);
r = ms_hb_front_client->client_bind(hb_back_addr);
Copy link
Member Author

Choose a reason for hiding this comment

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

@yuriw sorry, I have a mistake. you may need to update this pr to branch and schedule qa

Signed-off-by: Zengran Zhang <zhangzengran@h3c.com>
Signed-off-by: Haomai Wang <haomai@xsky.com>
@yuriw yuriw merged commit f9f9b63 into ceph:master Jan 23, 2017
@yuyuyu101 yuyuyu101 deleted the wip-msgr-client-bind branch January 24, 2017 03:57
{
lock.Lock();
if (did_bind) {
assert(my_inst.addr == bind_addr);
Copy link
Contributor

Choose a reason for hiding this comment

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

seems that miss lock.Unlock() ?

Copy link
Member Author

Choose a reason for hiding this comment

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

good catch

@dillaman
Copy link

It looks like this will break librados functionality when used behind a NAT since it learns the address of the gateway and then attempts to bind using it instead of its local IP address [1].

[1] http://tracker.ceph.com/issues/20049

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants