Hi Marcus,
I looked over the wip-addr branch a bit. I have two basic
questions/concerns:
1) In commit
https://github.com/ceph/ceph/commit/73b09090466a43d5aceb979a4802de3f3f5bf24a
we switch the type field from sa_family to transport_type. This seems
like the way to go, but we need to deal with the fact that lots of
clusters out there are IPv6 and have AF_INET6 filled in here. Probably
both types should be interpreted to mean "existing/legacy IP messenger" or
whatever we want to call SimpleMessenger/AsyncMessenger's protocol.
I think encode needs to make sure it fills in that value for the type when
encoding the legacy entity_addr_t encoding, but could use a/the single
valid value for the new encoding. And any get_transport_type() accessor
should also return the single valid value.
2) In the later commit
https://github.com/ceph/ceph/commit/9d203a2058f76414703b4fc212a1a0a960d0c672
you introduce a grammar for printing/parsing the addrs. This also makes
sense since e.g. xio uses an IP to identify an endpoint. I think we
should identify these based on the *protocol* and not the implementation,
though... whether we use SimpleMessenger or AsyncMessenger is not a
property of the address. Maybe "tcp://" makes more sense here? Or
perhaps no prefix at all (a bare IP address), so that this looks the same
as it did before in the case where the default protocol(s) are in use.
I assume the xio protocol (whether it is rdma or tcp) is closely tied to
libxio itself.. is that right? If so, using xio in that prefix makes
sense. I'd include xio somewhere in the rdma prefix though (xrdma:// and
xtcp://)?
What do you think?
Logistically, I think the steps for getting this ready for merge are:
1) Separate out the preliminary patches that pass a feature to the addr
encoding.. without any of the other cohortfs patches that are currently on
this branch. Once this builds we can merge it separately from the rest...
2) The entity_addrvec_t type.
3) The type -> transport_type switch.
4) We should make the new entity_addr_t encoding encode sockaddr piece
more compactly instead of eating up a full 80-byte sockaddr_storage even
for the ~8-byte IPv4 sockaddr_in. Maybe just need to encode an
explicit length for the sockaddr_* piece?
Something like that?
sage
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
