From cce797263d1d7b2c3368a5a4a2b405e7c4105685 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Tue, 17 May 2022 14:51:16 -0700 Subject: [PATCH] fix handling of pva_ctrl_msg::SetEndian --- documentation/releasenotes.rst | 1 + src/client.cpp | 2 +- src/clientconn.cpp | 10 +++++----- src/clientget.cpp | 8 +++++--- src/clientintrospect.cpp | 2 +- src/clientmon.cpp | 6 +++--- src/conn.cpp | 22 +++++++++++++++++++++- src/conn.h | 1 + src/serverchan.cpp | 8 ++++---- src/serverconn.cpp | 8 ++++---- src/serverget.cpp | 2 +- src/serverintrospect.cpp | 2 +- src/servermon.cpp | 2 +- 13 files changed, 49 insertions(+), 25 deletions(-) diff --git a/documentation/releasenotes.rst b/documentation/releasenotes.rst index 7fa771b..6732089 100644 --- a/documentation/releasenotes.rst +++ b/documentation/releasenotes.rst @@ -6,6 +6,7 @@ Release Notes 0.3.0 (UNRELEASED) ------------------ +* Fix protocol **incompatibility** with Big Endian servers. * Add support for IPv4 multicast and IPv6 uni/multicast for UDP. And IPv6 unicast for TCP. See :ref:`addrspec` for entries which may now appear in **EPICS_PVA*_ADDR_LIST**. * PVXS now attempts to fanout unicast searches through the loopback interface, and diff --git a/src/client.cpp b/src/client.cpp index a79f0e6..5d09952 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -140,7 +140,7 @@ void Channel::disconnect(const std::shared_ptr& self) { (void)evbuffer_drain(current->txBody.get(), evbuffer_get_length(current->txBody.get())); - EvOutBuf R(hostBE, current->txBody.get()); + EvOutBuf R(current->sendBE, current->txBody.get()); to_wire(R, sid); to_wire(R, cid); diff --git a/src/clientconn.cpp b/src/clientconn.cpp index 6024c07..2232166 100644 --- a/src/clientconn.cpp +++ b/src/clientconn.cpp @@ -67,7 +67,7 @@ void Connection::createChannels() { (void)evbuffer_drain(txBody.get(), evbuffer_get_length(txBody.get())); - EvOutBuf R(hostBE, txBody.get()); + EvOutBuf R(sendBE, txBody.get()); to_wire(R, uint16_t(1u)); to_wire(R, chan->cid); @@ -90,7 +90,7 @@ void Connection::sendDestroyRequest(uint32_t sid, uint32_t ioid) { (void)evbuffer_drain(txBody.get(), evbuffer_get_length(txBody.get())); - EvOutBuf R(hostBE, txBody.get()); + EvOutBuf R(sendBE, txBody.get()); to_wire(R, sid); to_wire(R, ioid); @@ -232,7 +232,7 @@ void Connection::handle_CONNECTION_VALIDATION() { (void)evbuffer_drain(txBody.get(), evbuffer_get_length(txBody.get())); - EvOutBuf R(hostBE, txBody.get()); + EvOutBuf R(sendBE, txBody.get()); // serverReceiveBufferSize, not used to_wire(R, uint32_t(0x10000)); @@ -316,7 +316,7 @@ void Connection::handle_CREATE_CHANNEL() { (void)evbuffer_drain(txBody.get(), evbuffer_get_length(txBody.get())); - EvOutBuf R(hostBE, txBody.get()); + EvOutBuf R(sendBE, txBody.get()); to_wire(R, sid); to_wire(R, cid); } @@ -400,7 +400,7 @@ void Connection::tickEcho() auto tx = bufferevent_get_output(bev.get()); - to_evbuf(tx, Header{CMD_ECHO, 0u, 0u}, hostBE); + to_evbuf(tx, Header{CMD_ECHO, 0u, 0u}, sendBE); // maybe help reduce latency bufferevent_flush(bev.get(), EV_WRITE, BEV_FLUSH); diff --git a/src/clientget.cpp b/src/clientget.cpp index 61537e2..b140873 100644 --- a/src/clientget.cpp +++ b/src/clientget.cpp @@ -286,9 +286,11 @@ struct GPROp : public OperationBase // act on new operation state { - (void)evbuffer_drain(chan->conn->txBody.get(), evbuffer_get_length(chan->conn->txBody.get())); + auto& conn = chan->conn; - EvOutBuf R(hostBE, chan->conn->txBody.get()); + (void)evbuffer_drain(conn->txBody.get(), evbuffer_get_length(conn->txBody.get())); + + EvOutBuf R(conn->sendBE, conn->txBody.get()); to_wire(R, chan->sid); to_wire(R, ioid); @@ -339,7 +341,7 @@ struct GPROp : public OperationBase { (void)evbuffer_drain(conn->txBody.get(), evbuffer_get_length(conn->txBody.get())); - EvOutBuf R(hostBE, conn->txBody.get()); + EvOutBuf R(conn->sendBE, conn->txBody.get()); to_wire(R, chan->sid); to_wire(R, ioid); diff --git a/src/clientintrospect.cpp b/src/clientintrospect.cpp index a7712d6..bab7e27 100644 --- a/src/clientintrospect.cpp +++ b/src/clientintrospect.cpp @@ -83,7 +83,7 @@ struct InfoOp : public OperationBase { (void)evbuffer_drain(conn->txBody.get(), evbuffer_get_length(conn->txBody.get())); - EvOutBuf R(hostBE, conn->txBody.get()); + EvOutBuf R(conn->sendBE, conn->txBody.get()); to_wire(R, chan->sid); to_wire(R, ioid); diff --git a/src/clientmon.cpp b/src/clientmon.cpp index ad53ab9..2535ae5 100644 --- a/src/clientmon.cpp +++ b/src/clientmon.cpp @@ -116,7 +116,7 @@ struct SubscriptionImpl : public OperationBase, public Subscription (void)evbuffer_drain(conn->txBody.get(), evbuffer_get_length(conn->txBody.get())); - EvOutBuf R(hostBE, conn->txBody.get()); + EvOutBuf R(conn->sendBE, conn->txBody.get()); to_wire(R, chan->sid); to_wire(R, ioid); @@ -272,7 +272,7 @@ struct SubscriptionImpl : public OperationBase, public Subscription (void)evbuffer_drain(conn->txBody.get(), evbuffer_get_length(conn->txBody.get())); - EvOutBuf R(hostBE, conn->txBody.get()); + EvOutBuf R(conn->sendBE, conn->txBody.get()); to_wire(R, chan->sid); to_wire(R, ioid); @@ -360,7 +360,7 @@ struct SubscriptionImpl : public OperationBase, public Subscription { (void)evbuffer_drain(conn->txBody.get(), evbuffer_get_length(conn->txBody.get())); - EvOutBuf R(hostBE, conn->txBody.get()); + EvOutBuf R(conn->sendBE, conn->txBody.get()); to_wire(R, chan->sid); to_wire(R, ioid); diff --git a/src/conn.cpp b/src/conn.cpp index 3fa6118..dfb28aa 100644 --- a/src/conn.cpp +++ b/src/conn.cpp @@ -22,6 +22,7 @@ ConnBase::ConnBase(bool isClient, bufferevent* bev, const SockAddr& peerAddr) ,peerName(peerAddr.tostring()) ,bev(bev) ,isClient(isClient) + ,sendBE(EPICS_BYTE_ORDER==EPICS_ENDIAN_BIG) ,peerBE(true) // arbitrary choice, default should be overwritten before use ,expectSeg(false) ,segCmd(0xff) @@ -46,7 +47,7 @@ size_t ConnBase::enqueueTxBody(pva_app_msg_t cmd) to_evbuf(tx, Header{cmd, uint8_t(isClient ? 0u : pva_flags::Server), uint32_t(blen)}, - hostBE); + sendBE); auto err = evbuffer_add_buffer(tx, txBody.get()); assert(!err); // could only fail if frozen/pinned, which is not the case statTx += 8u + blen; @@ -121,6 +122,25 @@ void ConnBase::bevRead() "%s %s Receive header\n", peerLabel(), peerName.c_str()); if(header[2]&pva_flags::Control) { + if(header[3]==pva_ctrl_msg::SetEndian) { + /* This should be the first message sent by a (supposedly) server. + * However, old pvAccess* accepts it from either peer at any time. + * + * The protocol spec. claims that we should inspect the size field + * (bytes 4-7) and act as follows. + * 0x00000000 - Send future messages using endianness on this (received) + * message. Peer will ignore MSB flag in our headers! + * 0xffffffff - Send future messages as we like. Peer will test the + * MSB flag. + * + * However, neither pvAccessCPP nor pvAccessJava actually test this. + * Instead the 0x00000000 behavior is assumed. + * + * So we latch the byte order here, as the peer should ignore the MSB + * flag subsequent messages... + */ + sendBE = header[2]&pva_flags::MSB; + } // Control messages are not actually useful evbuffer_drain(rx, 8); statRx += 8u; diff --git a/src/conn.h b/src/conn.h index ff638b6..5431d85 100644 --- a/src/conn.h +++ b/src/conn.h @@ -27,6 +27,7 @@ struct ConnBase TypeStore rxRegistry; const bool isClient; + bool sendBE; bool peerBE; bool expectSeg; diff --git a/src/serverchan.cpp b/src/serverchan.cpp index 058c2a9..46653bc 100644 --- a/src/serverchan.cpp +++ b/src/serverchan.cpp @@ -163,7 +163,7 @@ void ServerChannelControl::close() conn->peerName.c_str(), ch->name.c_str()); auto tx = bufferevent_get_output(conn->bev.get()); - EvOutBuf R(hostBE, tx); + EvOutBuf R(conn->sendBE, tx); to_wire(R, Header{CMD_DESTROY_CHANNEL, pva_flags::Server, 8}); to_wire(R, ch->sid); to_wire(R, ch->cid); @@ -251,7 +251,7 @@ void ServerConn::handle_SEARCH() { (void)evbuffer_drain(txBody.get(), evbuffer_get_length(txBody.get())); - EvOutBuf R(hostBE, txBody.get()); + EvOutBuf R(sendBE, txBody.get()); _to_wire<12>(R, iface->server->effective.guid.data(), false, __FILE__, __LINE__); to_wire(R, searchID); @@ -363,7 +363,7 @@ void ServerConn::handle_CREATE_CHANNEL() { (void)evbuffer_drain(txBody.get(), evbuffer_get_length(txBody.get())); - EvOutBuf R(hostBE, txBody.get()); + EvOutBuf R(sendBE, txBody.get()); to_wire(R, cid); to_wire(R, sid); to_wire(R, sts); @@ -417,7 +417,7 @@ void ServerConn::handle_DESTROY_CHANNEL() { auto tx = bufferevent_get_output(bev.get()); - EvOutBuf R(hostBE, tx); + EvOutBuf R(sendBE, tx); to_wire(R, Header{CMD_DESTROY_CHANNEL, pva_flags::Server, 8}); to_wire(R, sid); to_wire(R, cid); diff --git a/src/serverconn.cpp b/src/serverconn.cpp index ec5aa62..80d18a4 100644 --- a/src/serverconn.cpp +++ b/src/serverconn.cpp @@ -71,7 +71,7 @@ ServerConn::ServerConn(ServIface* iface, evutil_socket_t sock, struct sockaddr * // queue connection validation message { - VectorOutBuf M(hostBE, buf); + VectorOutBuf M(sendBE, buf); to_wire(M, Header{pva_ctrl_msg::SetEndian, pva_flags::Control|pva_flags::Server, 0}); auto save = M.save(); @@ -87,7 +87,7 @@ ServerConn::ServerConn(ServIface* iface, evutil_socket_t sock, struct sockaddr * to_wire(M, "ca"); auto bend = M.save(); - FixedBuf H(hostBE, save, 8); + FixedBuf H(sendBE, save, 8); to_wire(H, Header{CMD_CONNECTION_VALIDATION, pva_flags::Server, uint32_t(bend-bstart)}); assert(M.good() && H.good()); @@ -123,7 +123,7 @@ void ServerConn::handle_ECHO() auto tx = bufferevent_get_output(bev.get()); uint32_t len = evbuffer_get_length(segBuf.get()); - to_evbuf(tx, Header{CMD_ECHO, pva_flags::Server, len}, hostBE); + to_evbuf(tx, Header{CMD_ECHO, pva_flags::Server, len}, sendBE); auto err = evbuffer_add_buffer(tx, segBuf.get()); assert(!err); @@ -140,7 +140,7 @@ void auth_complete(ServerConn *self, const Status& sts) (void)evbuffer_drain(self->txBody.get(), evbuffer_get_length(self->txBody.get())); { - EvOutBuf M(hostBE, self->txBody.get()); + EvOutBuf M(self->sendBE, self->txBody.get()); to_wire(M, sts); } diff --git a/src/serverget.cpp b/src/serverget.cpp index 4bad1e8..1ea8b7f 100644 --- a/src/serverget.cpp +++ b/src/serverget.cpp @@ -69,7 +69,7 @@ struct ServerGPR : public ServerOp { (void)evbuffer_drain(conn->txBody.get(), evbuffer_get_length(conn->txBody.get())); - EvOutBuf R(hostBE, conn->txBody.get()); + EvOutBuf R(conn->sendBE, conn->txBody.get()); to_wire(R, uint32_t(ioid)); to_wire(R, subcmd); to_wire(R, sts); diff --git a/src/serverintrospect.cpp b/src/serverintrospect.cpp index 2e316e5..a5e528b 100644 --- a/src/serverintrospect.cpp +++ b/src/serverintrospect.cpp @@ -35,7 +35,7 @@ struct ServerIntrospect : public ServerOp { (void)evbuffer_drain(conn->txBody.get(), evbuffer_get_length(conn->txBody.get())); - EvOutBuf R(hostBE, conn->txBody.get()); + EvOutBuf R(conn->sendBE, conn->txBody.get()); to_wire(R, uint32_t(ioid)); to_wire(R, sts); if(type) diff --git a/src/servermon.cpp b/src/servermon.cpp index 2da6174..301d842 100644 --- a/src/servermon.cpp +++ b/src/servermon.cpp @@ -120,7 +120,7 @@ struct MonitorOp : public ServerOp, { (void)evbuffer_drain(conn->txBody.get(), evbuffer_get_length(conn->txBody.get())); - EvOutBuf R(hostBE, conn->txBody.get()); + EvOutBuf R(conn->sendBE, conn->txBody.get()); to_wire(R, uint32_t(ioid)); to_wire(R, subcmd); if(subcmd&0x08) {