-
Notifications
You must be signed in to change notification settings - Fork 120
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
Doc add photon-architecture.md #190
Conversation
net/utils.cpp
Outdated
hints.ai_socktype = SOCK_STREAM; | ||
hints.ai_flags = AI_PASSIVE; | ||
hints.ai_family = AF_INET6; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gethostbyname
is deprecated, and does not support ipv6
getaddrinfo
is the new alternative
net/socket.h
Outdated
} | ||
bool is_ipv4() const { | ||
return addr.is_ipv4(); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sockaddr_storage
is defined in #include <netinet/in.h>
, and is wide enough to hold any type of socket address
net/kernel_socket.cpp
Outdated
return log.printf(addr.a, '.', addr.b, '.', addr.c, '.', addr.d); | ||
else { | ||
return log.printf(addr.to_string()); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ipv6 addr may has the ::
form. Even we could print every u32 number by order, but using the inet_ntop
is much simpler.
net/socket.h
Outdated
} | ||
}; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: need a better hash function
net/socket.h
Outdated
|
||
#include <photon/common/stream.h> | ||
#include <photon/common/callback.h> | ||
#include <photon/common/object.h> | ||
|
||
#ifdef __linux__ | ||
#define _in_addr_field s6_addr32 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using or typedef
net/socket.h
Outdated
@@ -21,80 +21,215 @@ limitations under the License. | |||
#include <netinet/ip.h> | |||
#include <arpa/inet.h> | |||
#include <cstring> | |||
#include <string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try to avoid including string in .h. It's slow.
Do not use exception. Most of the code is not exception-safe.
net/socket.h
Outdated
|
||
// Check if it's actually an IPv4 address mapped in IPV6 | ||
bool is_ipv4() const { | ||
if (ntohl(addr._in_addr_field[2]) != 0x0000ffff) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ntohl() is needed?
net/socket.h
Outdated
|
||
public: | ||
static const IPAddr V6None() { | ||
return IPAddr(htonl(0xffffffff), htonl(0xffffffff), htonl(0xffffffff), htonl(0xffffffff)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to htonl?
net/socket.h
Outdated
return !(*this == rhs); | ||
} | ||
|
||
const std::string to_string() const { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no std::string involved. Just to_string(char* buf, size_t size).
net/socket.h
Outdated
} | ||
|
||
// We regard the default IPv4 0.0.0.0 as empty | ||
bool empty() const { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are is_any() or undefined() better?
net/socket.h
Outdated
|
||
bool is_link_local() const { | ||
if (is_ipv4()) { | ||
return (to_nl() & htonl(0xffff0000)) == htonl(0xa9fe0000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we simplify these expression?
net/socket.h
Outdated
sockaddr* get_sockaddr(const sockaddr_storage& storage) const { | ||
if (addr.is_ipv4()) { | ||
auto* in4 = (sockaddr_in*) &storage; | ||
in4->sin_family = AF_INET; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is purely a conversion function?
net/socket.h
Outdated
return hasher((x.addr.to_nl() << 16) | x.port); | ||
} else { | ||
hash<std::string_view> hasher; | ||
return hasher(std::string_view((const char*) &x, sizeof(x))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can simply use hashstd::string_view for either type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to distinguish between v4 and v6?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to distinguish between v4 and v6?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to distinguish between v4 and v6?
Because string_view hash is not balanced enough for integers. For simplicity, we call use string_view hash.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will change both of them to string_view hash.
net/datagram_socket.cpp
Outdated
@@ -139,23 +139,23 @@ class UDP : public DatagramSocketBase { | |||
virtual int connect(const Addr* addr, size_t addr_len) override { | |||
auto ep = (EndPoint*)addr; | |||
assert(ep && addr_len == sizeof(*ep)); | |||
auto in = ep->to_sockaddr_in(); | |||
return do_connect((sockaddr*)&in, sizeof(in)); | |||
sockaddr_storage s{}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can define a new class with the same name sockaddr_storage in namespace photon, adding a new constructor that accept EndPoint, so that we can write sockaddr_storage s(ep);
here, and use s
directly in do_connect invocation.
串了..
等会重新提一个PR吧