wip-addr

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [CEPH Users]     [Ceph Large]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux