mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Hindman <b...@eecs.berkeley.edu>
Subject Re: Creating IP address abstraction
Date Fri, 23 Jan 2015 15:52:23 GMT
I didn't want to use 'Endpoint' because we use it to refer to our HTTP API,
e.g., ip:port/path/to/endpoint?arg1=foo.

Chromium is such a fascinating example so I'm glad you brought it up. It's
actually one of the libraries I consulted when doing research for a name.
While Chromium does have IPEndPoint, it also has SockaddrStorage
<https://code.google.com/p/chromium/codesearch#chromium/src/net/base/net_util.h&cl=GROK&ct=xref_jump_to_def&l=118&gsn=SockaddrStorage>!
SockaddrStorage
represents an "address" and is what is used by some of their Socket API
calls like Bind
<https://code.google.com/p/chromium/codesearch#chromium/src/net/socket/socket_libevent.h&sq=package:chromium&type=cs&l=41>
as
well as for getting the local *address* (GetLocalAddress
<https://code.google.com/p/chromium/codesearch#chromium/src/net/socket/socket_libevent.h&sq=package:chromium&type=cs&l=72>)
or
peer *address* (GetPeerAddress
<https://code.google.com/p/chromium/codesearch#chromium/src/net/socket/socket_libevent.h&sq=package:chromium&type=cs&l=73>
).

What's even more interesting, however, is in many places where IPEndPoint
is used instead of SockaddrStorage, they still use the name *address* for
parameters or in the function itself! Consider StreamSocket
<https://code.google.com/p/chromium/codesearch#chromium/src/net/socket/stream_socket.h&sq=package:chromium&type=cs&rcl=1421932609&l=18>,
one of their subclasses of Socket that ends up being the subclass of the
TCP type sockets. StreamSocket has GetLocalAddress
<https://code.google.com/p/chromium/codesearch#chromium/src/net/socket/stream_socket.h&sq=package:chromium&type=cs&rcl=1421932609&l=62>
and GetPeerAddress
<https://code.google.com/p/chromium/codesearch#chromium/src/net/socket/stream_socket.h&sq=package:chromium&type=cs&rcl=1421932609&l=58>
as well which has an IPEndPoint now instead of a SockaddrStorage as an out
parameter but they call the parameter name 'address'! This is madness to
me, they're using the term "address" for everything with IPEndPoint.

Ironically, what finally sealed the deal for me was Chromium's AddressList
<https://code.google.com/p/chromium/codesearch#chromium/src/net/base/address_list.h&sq=package:chromium&type=cs&rcl=1421932609&l=22>
which
is actually a vector of IPEndPoints. Yes, you read that correctly. ;-)

I had also toyed with the idea of Socket::Address instead of Address but at
the end of the day I really wanted to capture the new 'struct addrinfo' and
'getaddrinfo' that's exposed because that's the POSIX interface and that's
what's going to have the most documentation out there across many
languages. Moreover, the goal is for Address to approach addrinfo in terms
of the information that it stores/provides.




On Fri, Jan 23, 2015 at 2:31 AM, Alex Rukletsov <alex@mesosphere.io> wrote:

> +1 for a single class;
> +1 for "Endpoint", "Address" may be too vague.
>
> By the way, I like the way it's implemented in Chromium sources: there is
> IPEndPoint class which represents a pair address + port, and
> IPAddressNumber type which wraps the address (link1
> <
> https://chromium.googlesource.com/chromium/src.git/+/master/net/base/ip_endpoint.h
> >,
> link2
> <
> https://chromium.googlesource.com/chromium/src.git/+/master/net/base/net_util.h
> >
> ).
>
> On Fri, Jan 23, 2015 at 2:30 AM, Benjamin Hindman <benh@eecs.berkeley.edu>
> wrote:
>
> > >
> > > 1. implementation of IPvXaddress abstraction
> > >   - my preference, and the implementation Evelina has under review, is
> a
> > > single class that abstracts away the family
> >
> >   - others have suggested an abstract 'Address' with inheritance to
> manage
> > > the different families
> > >   - in general we have used composition in the existing code-base
> > >   - given how non-extensible the family is, it makes sense to me to
> > > continue with composition
> > >
> >
> > I like the idea of a single 'IP' class that abstracts IP. What we saw in
> > the past from Evelina's reviews was both an IP and an IPAddress, at which
> > point we proposed she make IPAddress be IP::Address. But only having a
> > single IP class would be much much much better.
> >
> >
> > > 2. rename of Node
> > >   - this has come up before, and always been determined to be
> unnecessary
> > > churn
> > >   - renaming to Address adds confusion as it is actually an address
> and a
> > > port
> > >   - 'Endpoint' could be an alternative
> > >
> >
> > I had called this Node originally because I didn't do my homework. This
> was
> > my bad. C libraries always called this sockaddr. Now they've introduced a
> > new structure called addrinfo to better handle IPv6 addresses. This is
> not
> > meant to be churn, this is better capturing the naming from the the
> > underlying libraries. I didn't want to call this SocketAddress even
> though
> > a bunch of libraries that abstract sockaddr call it that because
> ultimately
> > we want to "wrap" addrinfo.  Moreover, I didn't want to call it
> AddressInfo
> > because the 'Info' seems unnecessary and conflicts with our protobuf
> naming
> > scheme (where things end with the suffix 'Info', like FrameworkInfo,
> > SlaveInfo, etc). Finally, man pages around these structures simply just
> > call the object "address", hence Address.
> >
> >
> > On Thu, Jan 22, 2015 at 1:05 PM, Evelina Dumitrescu <
> > > evelina_dumitrescu@yahoo.com.invalid> wrote:
> > >
> > > > Hey,
> > > >
> > > > A few weeks ago I proposed an abstraction for the IP address.
> > > > There has been a long discussion on how to implement this (eg: what
> > kind
> > > > of pattern to use, composition/inheritance).
> > > >
> > > > Ben Hindman proposed a replacement of the Node class with Address and
> > > > moving several network functions to the network namespace, which
> > affects
> > > my
> > > > implementation[1].
> > > >
> > > > Here are my code reviews[2] and the Jira discussion on this issue[3].
> > > > To speed up the review process, we decided to move this discussion of
> > the
> > > > dev mailing list so anyone can chime in.
> > > >
> > > > Thanks,
> > > > Evelina
> > > >
> > > >
> > > > [1]
> > > > https://reviews.apache.org/r/29538/
> > > > https://reviews.apache.org/r/29540/
> > > > https://reviews.apache.org/r/29541/
> > > >
> > > > [2]
> > > > https://reviews.apache.org/r/29288/
> > > > https://reviews.apache.org/r/29289/
> > > > https://reviews.apache.org/r/29290/
> > > >
> > > > [3]
> > > > https://issues.apache.org/jira/browse/MESOS-1919
> > > >
> > > >
> > >
> > >
> > > --
> > > Dominic Hamon | @mrdo | Twitter
> > > *There are no bad ideas; only good ideas that go horribly wrong.*
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message