From 9fc5168135a749dd2c12fc9bfb16056a5a271575 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 --- 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 +- 12 files changed, 48 insertions(+), 25 deletions(-) diff --git a/src/client.cpp b/src/client.cpp index a79f0e6c7..5d0995252 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 6024c0720..2232166e0 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 61537e298..b140873ba 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 a7712d626..bab7e2799 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 ad53ab929..2535ae533 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 3fa611898..303fdc9dc 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) a 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 will ignore the MSB + * flag in our 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 ff638b6bb..5431d85da 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 058c2a949..46653bc22 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 ec5aa6294..80d18a4e1 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 4bad1e8ab..1ea8b7f2f 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 2e316e510..a5e528b3f 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 2da6174fb..301d842e8 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) {